From: Anton Khorev <tony29@yandex.ru>
Date: Thu, 15 Aug 2024 09:59:49 +0000 (+0300)
Subject: Prevent reactivating inactive blocks
X-Git-Tag: live~230^2
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/323b7032baf2cb664dbdf80b22e2edce5632fb09

Prevent reactivating inactive blocks
---

diff --git a/app/controllers/user_blocks_controller.rb b/app/controllers/user_blocks_controller.rb
index 664ac5681..3b7ef30f9 100644
--- a/app/controllers/user_blocks_controller.rb
+++ b/app/controllers/user_blocks_controller.rb
@@ -67,15 +67,20 @@ class UserBlocksController < ApplicationController
          current_user != @user_block.revoker
         flash[:error] = t(@user_block.revoker ? ".only_creator_or_revoker_can_edit" : ".only_creator_can_edit")
         redirect_to :action => "edit"
-      elsif @user_block.update(
-        :ends_at => Time.now.utc + @block_period.hours,
-        :reason => params[:user_block][:reason],
-        :needs_view => params[:user_block][:needs_view]
-      )
-        flash[:notice] = t(".success")
-        redirect_to(@user_block)
       else
-        render :action => "edit"
+        user_block_was_active = @user_block.active?
+        @user_block.reason = params[:user_block][:reason]
+        @user_block.needs_view = params[:user_block][:needs_view]
+        @user_block.ends_at = Time.now.utc + @block_period.hours
+        if !user_block_was_active && @user_block.active?
+          flash.now[:error] = t(".inactive_block_cannot_be_reactivated")
+          render :action => "edit"
+        elsif @user_block.save
+          flash[:notice] = t(".success")
+          redirect_to @user_block
+        else
+          render :action => "edit"
+        end
       end
     else
       redirect_to edit_user_block_path(:id => params[:id])
diff --git a/config/locales/en.yml b/config/locales/en.yml
index ac41952c7..c4cae4b22 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -2950,6 +2950,7 @@ en:
     update:
       only_creator_can_edit: "Only the moderator who created this block can edit it."
       only_creator_or_revoker_can_edit: "Only the moderators who created or revoked this block can edit it."
+      inactive_block_cannot_be_reactivated: "This block is inactive and cannot be reactivated."
       success: "Block updated."
     index:
       title: "User blocks"
diff --git a/test/controllers/user_blocks_controller_test.rb b/test/controllers/user_blocks_controller_test.rb
index b172945ec..d28194fdc 100644
--- a/test/controllers/user_blocks_controller_test.rb
+++ b/test/controllers/user_blocks_controller_test.rb
@@ -469,7 +469,7 @@ class UserBlocksControllerTest < ActionDispatch::IntegrationTest
     assert_equal "Original Reason", block.reason
 
     session_for(creator_user)
-    check_block_updates(block)
+    check_inactive_block_updates(block)
   end
 
   ##
@@ -491,10 +491,10 @@ class UserBlocksControllerTest < ActionDispatch::IntegrationTest
     assert_equal "Original Reason", block.reason
 
     session_for(creator_user)
-    check_block_updates(block)
+    check_inactive_block_updates(block)
 
     session_for(revoker_user)
-    check_block_updates(block)
+    check_inactive_block_updates(block)
   end
 
   ##
@@ -798,7 +798,7 @@ class UserBlocksControllerTest < ActionDispatch::IntegrationTest
     end
   end
 
-  def check_block_updates(block)
+  def check_inactive_block_updates(block)
     put user_block_path(block,
                         :user_block_period => "0",
                         :user_block => { :needs_view => false, :reason => "Updated Reason" })
@@ -810,12 +810,30 @@ class UserBlocksControllerTest < ActionDispatch::IntegrationTest
 
     put user_block_path(block,
                         :user_block_period => "0",
-                        :user_block => { :needs_view => true, :reason => "Updated Reason 2" })
+                        :user_block => { :needs_view => true, :reason => "Updated Reason Needs View" })
+    assert_response :success
+    assert_equal "This block is inactive and cannot be reactivated.", flash[:error]
+    block.reload
+    assert_not_predicate block, :active?
+    assert_equal "Updated Reason", block.reason
+
+    put user_block_path(block,
+                        :user_block_period => "1",
+                        :user_block => { :needs_view => false, :reason => "Updated Reason Duration Extended" })
+    assert_response :success
+    assert_equal "This block is inactive and cannot be reactivated.", flash[:error]
+    block.reload
+    assert_not_predicate block, :active?
+    assert_equal "Updated Reason", block.reason
+
+    put user_block_path(block,
+                        :user_block_period => "0",
+                        :user_block => { :needs_view => false, :reason => "Updated Reason Again" })
     assert_redirected_to user_block_path(block)
     assert_equal "Block updated.", flash[:notice]
     block.reload
-    assert_predicate block, :active?
-    assert_equal "Updated Reason 2", block.reason
+    assert_not_predicate block, :active?
+    assert_equal "Updated Reason Again", block.reason
   end
 
   def check_user_blocks_table(user_blocks)