FIX: do not use return in block (#26260)

We were incorrectly using `return` in a block which was causing exceptions at runtime. These exceptions were not causing much issues as they are in defer block.

While working on writing a test for this specific case, I noticed that our `upsert_custom_fields` function was using rails `update_all` which is not updating the `updated_at` timestamp. This commit also fixes it and adds a test for it.

app/models/concerns/has_custom_fields.rb

@@ -270,7 +270,7 @@ module HasCustomFields
   # set of fields.
   def upsert_custom_fields(fields)
     fields.each do |k, v|
-      row_count = _custom_fields.where(name: k).update_all(value: v)
+      row_count = _custom_fields.where(name: k).update_all(value: v, updated_at: Time.now)
       _custom_fields.create!(name: k, value: v) if row_count == 0
 
       custom_fields[k.to_s] = v # We normalize custom_fields as strings

plugins/chat/app/services/chat/list_channel_messages.rb

@@ -174,7 +174,7 @@ module Chat
 
     def update_user_last_channel(guardian:, channel:)
       Scheduler::Defer.later "Chat::ListChannelMessages - defer update_user_last_channel" do
-        return if guardian.user.custom_fields[::Chat::LAST_CHAT_CHANNEL_ID] == channel.id
+        next if guardian.user.custom_fields[::Chat::LAST_CHAT_CHANNEL_ID] == channel.id
         guardian.user.upsert_custom_fields(::Chat::LAST_CHAT_CHANNEL_ID => channel.id)
       end
     end

plugins/chat/spec/services/chat/list_channel_messages_spec.rb

@@ -218,5 +218,12 @@ RSpec.describe Chat::ListChannelMessages do
         channel.id,
       )
     end
+
+    it "doesn’t update the custom field when it was already set to this value" do
+      user.upsert_custom_fields(::Chat::LAST_CHAT_CHANNEL_ID => channel.id)
+      field = UserCustomField.find_by(name: Chat::LAST_CHAT_CHANNEL_ID, user_id: user.id)
+
+      expect { result }.to_not change { field.reload.updated_at }
+    end
   end
 end

spec/lib/concern/has_custom_fields_spec.rb

@@ -459,6 +459,17 @@ RSpec.describe HasCustomFields do
         expect(test_item.custom_fields["abc"]).to eq("ghi")
       end
 
+      it "updates the updated_at timestamp" do
+        test_item = CustomFieldsTestItem.create
+        test_item.upsert_custom_fields(foo: "bar")
+        field = CustomFieldsTestItemCustomField.find_by(name: "foo")
+        prev_updated_at = field.updated_at
+
+        test_item.upsert_custom_fields(foo: "baz")
+
+        expect(prev_updated_at).not_to eq(field.reload.updated_at)
+      end
+
       it "allows upsert to use keywords" do
         test_item = CustomFieldsTestItem.create
         test_item.upsert_custom_fields(hello: "world", abc: "def")