From: Tom Hughes <tom@compton.nu>
Date: Wed, 21 Aug 2024 18:04:06 +0000 (+0100)
Subject: Merge remote-tracking branch 'upstream/pull/5069'
X-Git-Tag: live~675
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/754a0a9cb5c49114181fd1d1181eea0d757dbac4?hp=-c

Merge remote-tracking branch 'upstream/pull/5069'
---

754a0a9cb5c49114181fd1d1181eea0d757dbac4
diff --combined app/controllers/user_blocks_controller.rb
index ae6e3c1cd,73c6ccd69..71a0b4c31
--- a/app/controllers/user_blocks_controller.rb
+++ b/app/controllers/user_blocks_controller.rb
@@@ -26,6 -26,7 +26,7 @@@ class UserBlocksController < Applicatio
    def show
      if current_user && current_user == @user_block.user
        @user_block.needs_view = false
+       @user_block.deactivates_at = [@user_block.ends_at, Time.now.utc].max
        @user_block.save!
      end
    end
@@@ -49,6 -50,7 +50,7 @@@
          :ends_at => now + @block_period.hours,
          :needs_view => params[:user_block][:needs_view]
        )
+       @user_block.deactivates_at = @user_block.ends_at unless @user_block.needs_view
  
        if @user_block.save
          flash[:notice] = t(".flash", :name => @user.display_name)
@@@ -72,12 -74,16 +74,17 @@@
          @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
+         @user_block.deactivates_at = (@user_block.ends_at unless @user_block.needs_view)
 +        @user_block.revoker = current_user if user_block_was_active && !@user_block.active?
          if !user_block_was_active && @user_block.active?
            flash.now[:error] = t(".inactive_block_cannot_be_reactivated")
            render :action => "edit"
          else
-           @user_block.ends_at = @user_block.ends_at_was unless user_block_was_active
+           unless user_block_was_active
+             @user_block.ends_at = @user_block.ends_at_was
+             @user_block.deactivates_at = @user_block.deactivates_at_was
+             @user_block.deactivates_at = [@user_block.ends_at, @user_block.updated_at].max unless @user_block.deactivates_at # take updated_at into account before deactivates_at is backfilled
+           end
            if @user_block.save
              flash[:notice] = t(".success")
              redirect_to @user_block
diff --combined test/controllers/user_blocks_controller_test.rb
index 696d3c8d0,3d96199f3..a3926cff0
--- a/test/controllers/user_blocks_controller_test.rb
+++ b/test/controllers/user_blocks_controller_test.rb
@@@ -493,28 -493,6 +493,28 @@@ class UserBlocksControllerTest < Action
      check_inactive_block_updates(block)
    end
  
 +  ##
 +  # test the update action revoking the block
 +  def test_revoke_using_update
 +    moderator_user = create(:moderator_user)
 +    block = create(:user_block, :creator => moderator_user)
 +
 +    session_for(moderator_user)
 +    put user_block_path(block,
 +                        :user_block_period => "24",
 +                        :user_block => { :needs_view => false, :reason => "Updated Reason" })
 +    block.reload
 +    assert_predicate block, :active?
 +    assert_nil block.revoker
 +
 +    put user_block_path(block,
 +                        :user_block_period => "0",
 +                        :user_block => { :needs_view => false, :reason => "Updated Reason" })
 +    block.reload
 +    assert_not_predicate block, :active?
 +    assert_equal moderator_user, block.revoker
 +  end
 +
    ##
    # test the revoke action
    def test_revoke
@@@ -641,6 -619,134 +641,134 @@@
      assert_not_equal moderator_user, expired_block1.revoker
    end
  
+   ##
+   # test changes to end/deactivation dates
+   def test_dates_when_viewed_before_end
+     blocked_user = create(:user)
+     moderator_user = create(:moderator_user)
+ 
+     freeze_time do
+       session_for(moderator_user)
+       assert_difference "UserBlock.count", 1 do
+         post user_blocks_path(:display_name => blocked_user.display_name,
+                               :user_block_period => "48",
+                               :user_block => { :needs_view => true, :reason => "Testing deactivates_at" })
+       end
+       block = UserBlock.last
+       assert_equal Time.now.utc + 2.days, block.ends_at
+       assert_nil block.deactivates_at
+ 
+       travel 1.day
+       session_for(blocked_user)
+       get user_block_path(block)
+       block.reload
+       assert_equal Time.now.utc + 1.day, block.ends_at
+       assert_equal Time.now.utc + 1.day, block.deactivates_at
+     end
+   end
+ 
+   def test_dates_when_viewed_after_end
+     blocked_user = create(:user)
+     moderator_user = create(:moderator_user)
+ 
+     freeze_time do
+       session_for(moderator_user)
+       assert_difference "UserBlock.count", 1 do
+         post user_blocks_path(:display_name => blocked_user.display_name,
+                               :user_block_period => "24",
+                               :user_block => { :needs_view => true, :reason => "Testing deactivates_at" })
+       end
+       block = UserBlock.last
+       assert_equal Time.now.utc + 1.day, block.ends_at
+       assert_nil block.deactivates_at
+ 
+       travel 2.days
+       session_for(blocked_user)
+       get user_block_path(block)
+       block.reload
+       assert_equal Time.now.utc - 1.day, block.ends_at
+       assert_equal Time.now.utc, block.deactivates_at
+     end
+   end
+ 
+   def test_dates_when_edited_before_end
+     blocked_user = create(:user)
+     moderator_user = create(:moderator_user)
+ 
+     freeze_time do
+       session_for(moderator_user)
+       assert_difference "UserBlock.count", 1 do
+         post user_blocks_path(:display_name => blocked_user.display_name,
+                               :user_block_period => "48",
+                               :user_block => { :needs_view => false, :reason => "Testing deactivates_at" })
+       end
+       block = UserBlock.last
+       assert_equal Time.now.utc + 2.days, block.ends_at
+       assert_equal Time.now.utc + 2.days, block.deactivates_at
+ 
+       travel 1.day
+       put user_block_path(block,
+                           :user_block_period => "48",
+                           :user_block => { :needs_view => false, :reason => "Testing deactivates_at updated" })
+       block.reload
+       assert_equal Time.now.utc + 2.days, block.ends_at
+       assert_equal Time.now.utc + 2.days, block.deactivates_at
+     end
+   end
+ 
+   def test_dates_when_edited_after_end
+     blocked_user = create(:user)
+     moderator_user = create(:moderator_user)
+ 
+     freeze_time do
+       session_for(moderator_user)
+       assert_difference "UserBlock.count", 1 do
+         post user_blocks_path(:display_name => blocked_user.display_name,
+                               :user_block_period => "24",
+                               :user_block => { :needs_view => false, :reason => "Testing deactivates_at" })
+       end
+       block = UserBlock.last
+       assert_equal Time.now.utc + 1.day, block.ends_at
+       assert_equal Time.now.utc + 1.day, block.deactivates_at
+ 
+       travel 2.days
+       put user_block_path(block,
+                           :user_block_period => "0",
+                           :user_block => { :needs_view => false, :reason => "Testing deactivates_at updated" })
+       block.reload
+       assert_equal Time.now.utc - 1.day, block.ends_at
+       assert_equal Time.now.utc - 1.day, block.deactivates_at
+     end
+   end
+ 
+   ##
+   # test updates on legacy records without correctly initialized deactivates_at
+   def test_update_legacy_deactivates_at
+     blocked_user = create(:user)
+     moderator_user = create(:moderator_user)
+ 
+     freeze_time do
+       block = UserBlock.new :user => blocked_user,
+                             :creator => moderator_user,
+                             :reason => "because",
+                             :ends_at => Time.now.utc + 1.day,
+                             :needs_view => false
+ 
+       assert_difference "UserBlock.count", 1 do
+         block.save :validate => false
+       end
+ 
+       travel 2.days
+       session_for(moderator_user)
+       put user_block_path(block,
+                           :user_block_period => "0",
+                           :user_block => { :needs_view => false, :reason => "Testing legacy block update" })
+       block.reload
+       assert_equal Time.now.utc - 1.day, block.ends_at
+       assert_equal Time.now.utc - 1.day, block.deactivates_at
+     end
+   end
+ 
    ##
    # test the blocks_on action
    def test_blocks_on