DEV: Remove full group refreshes from tests (#25414)

We have all these calls to Group.refresh_automatic_groups! littered throughout the tests. Including tests that are seemingly unrelated to groups. This is because automatic group memberships aren't fabricated when making a vanilla user. There are two places where you'd want to use this: You have fabricated a user that needs a certain trust level (which is now based on group membership.) You need the system user to have a certain trust level. In the first case, we can pass refresh_auto_groups: true to the fabricator instead. This is a more lightweight operation that only considers a single user, instead of all users in all groups. The second case is no longer a thing after #25400.
2025-06-06 13:06:56 +08:00 · 2024-01-25 14:28:26 +08:00
parent 74fd883a89
commit 57ea56ee05
64 changed files with 131 additions and 269 deletions
--- a/plugins/chat/spec/components/chat/mailer_spec.rb
+++ b/plugins/chat/spec/components/chat/mailer_spec.rb
@ -4,8 +4,15 @@ require "rails_helper"

 describe Chat::Mailer do
  fab!(:chatters_group) { Fabricate(:group) }
-  fab!(:sender) { Fabricate(:user, group_ids: [chatters_group.id]) }
-  fab!(:user_1) { Fabricate(:user, group_ids: [chatters_group.id], last_seen_at: 15.minutes.ago) }
+  fab!(:sender) { Fabricate(:user, group_ids: [chatters_group.id], refresh_auto_groups: true) }
+  fab!(:user_1) do
+    Fabricate(
+      :user,
+      group_ids: [chatters_group.id],
+      last_seen_at: 15.minutes.ago,
+      refresh_auto_groups: true,
+    )
+  end
  fab!(:chat_channel) { Fabricate(:category_channel) }
  fab!(:chat_message) { Fabricate(:chat_message, user: sender, chat_channel: chat_channel) }
  fab!(:user_1_chat_channel_membership) do
@ -17,7 +24,6 @@ describe Chat::Mailer do
    )
  end
  fab!(:private_chat_channel) do
-    Group.refresh_automatic_groups!
    result =
      Chat::CreateDirectMessageChannel.call(
        guardian: sender.guardian,
--- a/plugins/chat/spec/integration/auto_channel_user_removal_spec.rb
+++ b/plugins/chat/spec/integration/auto_channel_user_removal_spec.rb
@ -1,9 +1,9 @@
 # frozen_string_literal: true

 describe "Automatic user removal from channels" do
-  fab!(:user_1) { Fabricate(:user, trust_level: TrustLevel[1]) }
+  fab!(:user_1) { Fabricate(:user, trust_level: TrustLevel[1], refresh_auto_groups: true) }
  let(:user_1_guardian) { Guardian.new(user_1) }
-  fab!(:user_2) { Fabricate(:user, trust_level: TrustLevel[1]) }
+  fab!(:user_2) { Fabricate(:user, trust_level: TrustLevel[1], refresh_auto_groups: true) }

  fab!(:secret_group) { Fabricate(:group) }
  fab!(:private_category) { Fabricate(:private_category, group: secret_group) }
@ -15,7 +15,6 @@ describe "Automatic user removal from channels" do
  before do
    SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:trust_level_1]
    SiteSetting.chat_enabled = true
-    Group.refresh_automatic_groups!
    Jobs.run_immediately!

    secret_group.add(user_1)
--- a/plugins/chat/spec/integration/thread_replies_count_cache_accuracy_spec.rb
+++ b/plugins/chat/spec/integration/thread_replies_count_cache_accuracy_spec.rb
@ -3,7 +3,7 @@
 RSpec.describe "Chat::Thread replies_count cache accuracy" do
  include ActiveSupport::Testing::TimeHelpers

-  fab!(:user)
+  fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
  fab!(:thread) { Fabricate(:chat_thread) }

  let(:guardian) { user.guardian }
@ -12,7 +12,6 @@ RSpec.describe "Chat::Thread replies_count cache accuracy" do
    SiteSetting.chat_enabled = true
    thread.add(user)
    thread.channel.add(user)
-    Group.refresh_automatic_groups!
  end

  it "keeps an accurate replies_count cache" do
--- a/plugins/chat/spec/jobs/regular/chat/notify_mentioned_spec.rb
+++ b/plugins/chat/spec/jobs/regular/chat/notify_mentioned_spec.rb
@ -5,12 +5,11 @@ require "rails_helper"
 describe Jobs::Chat::NotifyMentioned do
  subject(:job) { described_class.new }

-  fab!(:user_1) { Fabricate(:user) }
-  fab!(:user_2) { Fabricate(:user) }
+  fab!(:user_1) { Fabricate(:user, refresh_auto_groups: true) }
+  fab!(:user_2) { Fabricate(:user, refresh_auto_groups: true) }
  fab!(:public_channel) { Fabricate(:category_channel) }

  before do
-    Group.refresh_automatic_groups!
    user_1.reload
    user_2.reload

--- a/plugins/chat/spec/lib/chat/channel_hashtag_data_source_spec.rb
+++ b/plugins/chat/spec/lib/chat/channel_hashtag_data_source_spec.rb
@ -1,7 +1,7 @@
 # frozen_string_literal: true

 RSpec.describe Chat::ChannelHashtagDataSource do
-  fab!(:user)
+  fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
  fab!(:category)
  fab!(:group)
  fab!(:private_category) { Fabricate(:private_category, group: group) }
@ -26,10 +26,7 @@ RSpec.describe Chat::ChannelHashtagDataSource do
  end
  let!(:guardian) { Guardian.new(user) }

-  before do
-    SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:trust_level_1]
-    Group.refresh_automatic_groups!
-  end
+  before { SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:trust_level_1] }

  describe "#enabled?" do
    it "returns false if public channels are disabled" do
@ -87,7 +84,6 @@ RSpec.describe Chat::ChannelHashtagDataSource do

    it "returns nothing if the user cannot chat" do
      SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:staff]
-      Group.refresh_automatic_groups!
      expect(described_class.lookup(Guardian.new(user), ["random"])).to be_empty
    end
  end
@ -152,7 +148,6 @@ RSpec.describe Chat::ChannelHashtagDataSource do

    it "returns nothing if the user cannot chat" do
      SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:staff]
-      Group.refresh_automatic_groups!
      expect(described_class.search(Guardian.new(user), "rand", 10)).to be_empty
    end
  end
@ -195,7 +190,6 @@ RSpec.describe Chat::ChannelHashtagDataSource do

    it "returns nothing if the user cannot chat" do
      SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:staff]
-      Group.refresh_automatic_groups!
      expect(described_class.search_without_term(Guardian.new(user), 10)).to be_empty
    end
  end
--- a/plugins/chat/spec/lib/chat/guardian_extensions_spec.rb
+++ b/plugins/chat/spec/lib/chat/guardian_extensions_spec.rb
@ -4,7 +4,7 @@ require "rails_helper"

 RSpec.describe Chat::GuardianExtensions do
  fab!(:chatters) { Fabricate(:group) }
-  fab!(:user) { Fabricate(:user, group_ids: [chatters.id]) }
+  fab!(:user) { Fabricate(:user, group_ids: [chatters.id], refresh_auto_groups: true) }
  fab!(:staff) { Fabricate(:user, admin: true) }
  fab!(:chat_group) { Fabricate(:group) }
  fab!(:channel) { Fabricate(:category_channel) }
@ -25,10 +25,8 @@ RSpec.describe Chat::GuardianExtensions do
  end

  it "allows TL1 to chat by default and by extension higher trust levels" do
-    Group.refresh_automatic_groups!
    expect(guardian.can_chat?).to eq(true)
-    user.update!(trust_level: TrustLevel[3])
-    Group.refresh_automatic_groups!
+    user.change_trust_level!(TrustLevel[3])
    expect(guardian.can_chat?).to eq(true)
  end

@ -588,8 +586,6 @@ RSpec.describe Chat::GuardianExtensions do
      end

      context "for direct message channels" do
-        before { Group.refresh_automatic_groups! }
-
        it "it still allows the user to message even if they are not in direct_message_enabled_groups because they are not creating the channel" do
          SiteSetting.direct_message_enabled_groups = Group::AUTO_GROUPS[:trust_level_4]
          dm_channel.update!(status: :open)
--- a/plugins/chat/spec/lib/chat/notifier_spec.rb
+++ b/plugins/chat/spec/lib/chat/notifier_spec.rb
@ -5,7 +5,7 @@ require "rails_helper"
 describe Chat::Notifier do
  describe "#notify_new" do
    fab!(:channel) { Fabricate(:category_channel) }
-    fab!(:user_1) { Fabricate(:user) }
+    fab!(:user_1) { Fabricate(:user, refresh_auto_groups: true) }
    fab!(:user_2) { Fabricate(:user) }

    before do
@ -424,7 +424,6 @@ describe Chat::Notifier do

      context "when in a personal message" do
        let(:personal_chat_channel) do
-          Group.refresh_automatic_groups!
          result =
            Chat::CreateDirectMessageChannel.call(
              guardian: user_1.guardian,
--- a/plugins/chat/spec/lib/chat/review_queue_spec.rb
+++ b/plugins/chat/spec/lib/chat/review_queue_spec.rb
@ -6,7 +6,7 @@ describe Chat::ReviewQueue do
  subject(:queue) { described_class.new }

  fab!(:message_poster) { Fabricate(:user) }
-  fab!(:flagger) { Fabricate(:user) }
+  fab!(:flagger) { Fabricate(:user, refresh_auto_groups: true) }
  fab!(:chat_channel) { Fabricate(:category_channel) }
  fab!(:message) { Fabricate(:chat_message, user: message_poster, chat_channel: chat_channel) }
  fab!(:admin)
@ -17,7 +17,6 @@ describe Chat::ReviewQueue do
  before do
    chat_channel.add(message_poster)
    chat_channel.add(flagger)
-    Group.refresh_automatic_groups!
  end

  describe "#flag_message" do
--- a/plugins/chat/spec/mailers/user_notifications_spec.rb
+++ b/plugins/chat/spec/mailers/user_notifications_spec.rb
@ -4,8 +4,8 @@ require "rails_helper"

 describe UserNotifications do
  fab!(:chatters_group) { Fabricate(:group) }
-  fab!(:sender) { Fabricate(:user, group_ids: [chatters_group.id]) }
-  fab!(:user) { Fabricate(:user, group_ids: [chatters_group.id]) }
+  fab!(:sender) { Fabricate(:user, group_ids: [chatters_group.id], refresh_auto_groups: true) }
+  fab!(:user) { Fabricate(:user, group_ids: [chatters_group.id], refresh_auto_groups: true) }

  before do
    SiteSetting.chat_enabled = true
@ -13,7 +13,6 @@ describe UserNotifications do
  end

  def refresh_auto_groups
-    Group.refresh_automatic_groups!
    user.reload
    sender.reload
  end
@ -111,11 +110,11 @@ describe UserNotifications do
        end

        it "displays a count when there are more than two DMs with unread messages" do
-          user = Fabricate(:user, group_ids: [chatters_group.id])
+          user = Fabricate(:user, group_ids: [chatters_group.id], refresh_auto_groups: true)
          senders = []

          3.times do
-            sender = Fabricate(:user, group_ids: [chatters_group.id])
+            sender = Fabricate(:user, group_ids: [chatters_group.id], refresh_auto_groups: true)
            refresh_auto_groups
            sender.reload
            senders << sender
--- a/plugins/chat/spec/requests/chat/api/channel_threads_controller_spec.rb
+++ b/plugins/chat/spec/requests/chat/api/channel_threads_controller_spec.rb
@ -9,7 +9,7 @@ RSpec.describe Chat::Api::ChannelThreadsController do
  before do
    SiteSetting.chat_enabled = true
    SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone]
-    Group.refresh_automatic_groups!
+
    sign_in(current_user)
  end

--- a/plugins/chat/spec/requests/chat/api/direct_messages_controller_spec.rb
+++ b/plugins/chat/spec/requests/chat/api/direct_messages_controller_spec.rb
@ -1,7 +1,7 @@
 # frozen_string_literal: true

 RSpec.describe Chat::Api::DirectMessagesController do
-  fab!(:current_user) { Fabricate(:user) }
+  fab!(:current_user) { Fabricate(:user, refresh_auto_groups: true) }
  fab!(:user1) { Fabricate(:user) }
  fab!(:user2) { Fabricate(:user) }
  fab!(:user3) { Fabricate(:user) }
@ -21,8 +21,6 @@ RSpec.describe Chat::Api::DirectMessagesController do
  end

  describe "#create" do
-    before { Group.refresh_automatic_groups! }
-
    describe "dm with one other user" do
      let(:usernames) { user1.username }
      let(:direct_message_user_ids) { [current_user.id, user1.id] }
--- a/plugins/chat/spec/requests/core_ext/latest_performance_spec.rb
+++ b/plugins/chat/spec/requests/core_ext/latest_performance_spec.rb
@ -5,7 +5,6 @@ describe ListController do

  before do
    SiteSetting.chat_enabled = true
-    Group.refresh_automatic_groups!
    sign_in(current_user)
  end

--- a/plugins/chat/spec/serializer/chat/chat_message_serializer_spec.rb
+++ b/plugins/chat/spec/serializer/chat/chat_message_serializer_spec.rb
@ -8,7 +8,7 @@ describe Chat::MessageSerializer do
  fab!(:chat_channel) { Fabricate(:category_channel) }
  fab!(:message_poster) { Fabricate(:user) }
  fab!(:message_1) { Fabricate(:chat_message, user: message_poster, chat_channel: chat_channel) }
-  fab!(:guardian_user) { Fabricate(:user) }
+  fab!(:guardian_user) { Fabricate(:user, refresh_auto_groups: true) }

  let(:guardian) { Guardian.new(guardian_user) }

@ -83,8 +83,6 @@ describe Chat::MessageSerializer do
  end

  describe "#available_flags" do
-    before { Group.refresh_automatic_groups! }
-
    context "when flagging on a regular channel" do
      let(:options) { { scope: guardian, root: nil, chat_channel: message_1.chat_channel } }

@ -167,7 +165,6 @@ describe Chat::MessageSerializer do

      it "doesn't include notify_user if they are not in a PM allowed group" do
        SiteSetting.personal_message_enabled_groups = Group::AUTO_GROUPS[:trust_level_4]
-        Group.refresh_automatic_groups!

        serialized = described_class.new(message_1, options).as_json

@ -175,9 +172,8 @@ describe Chat::MessageSerializer do
      end

      it "returns an empty list if the user needs a higher TL to flag" do
-        guardian.user.update!(trust_level: TrustLevel[2])
+        guardian.user.change_trust_level!(TrustLevel[2])
        SiteSetting.chat_message_flag_allowed_groups = Group::AUTO_GROUPS[:trust_level_3]
-        Group.refresh_automatic_groups!

        serialized = described_class.new(message_1, options).as_json

--- a/plugins/chat/spec/services/auto_remove/handle_chat_allowed_groups_change_spec.rb
+++ b/plugins/chat/spec/services/auto_remove/handle_chat_allowed_groups_change_spec.rb
@ -5,8 +5,8 @@ RSpec.describe Chat::AutoRemove::HandleChatAllowedGroupsChange do
    subject(:result) { described_class.call(params) }

    let(:params) { { new_allowed_groups: new_allowed_groups } }
-    fab!(:user_1) { Fabricate(:user) }
-    fab!(:user_2) { Fabricate(:user) }
+    fab!(:user_1) { Fabricate(:user, refresh_auto_groups: true) }
+    fab!(:user_2) { Fabricate(:user, refresh_auto_groups: true) }
    fab!(:admin_1) { Fabricate(:admin) }
    fab!(:admin_2) { Fabricate(:admin) }

@ -124,7 +124,6 @@ RSpec.describe Chat::AutoRemove::HandleChatAllowedGroupsChange do
        before do
          public_channel_1.add(user_1)
          public_channel_2.add(user_1)
-          Group.refresh_automatic_groups!
        end

        it "does nothing" do
--- a/plugins/chat/spec/services/auto_remove/handle_destroyed_group_spec.rb
+++ b/plugins/chat/spec/services/auto_remove/handle_destroyed_group_spec.rb
@ -5,8 +5,8 @@ RSpec.describe Chat::AutoRemove::HandleDestroyedGroup do
    subject(:result) { described_class.call(params) }

    let(:params) { { destroyed_group_user_ids: [admin_1.id, admin_2.id, user_1.id, user_2.id] } }
-    fab!(:user_1) { Fabricate(:user) }
-    fab!(:user_2) { Fabricate(:user) }
+    fab!(:user_1) { Fabricate(:user, refresh_auto_groups: true) }
+    fab!(:user_2) { Fabricate(:user, refresh_auto_groups: true) }
    fab!(:admin_1) { Fabricate(:admin) }
    fab!(:admin_2) { Fabricate(:admin) }

@ -130,7 +130,6 @@ RSpec.describe Chat::AutoRemove::HandleDestroyedGroup do
            channel_2.add(user_2)
            channel_1.add(admin_1)
            channel_1.add(admin_2)
-            Group.refresh_automatic_groups!
          end

          it "sets the service result as successful" do
@ -151,7 +150,6 @@ RSpec.describe Chat::AutoRemove::HandleDestroyedGroup do
            channel_2.add(user_2)
            channel_1.add(admin_1)
            channel_1.add(admin_2)
-            Group.refresh_automatic_groups!
          end

          it { is_expected.to fail_a_policy(:not_everyone_allowed) }
@ -170,7 +168,6 @@ RSpec.describe Chat::AutoRemove::HandleDestroyedGroup do
          channel_2.add(user_2)
          channel_1.add(admin_1)
          channel_1.add(admin_2)
-          Group.refresh_automatic_groups!
        end

        context "when channel category not read_restricted with no category_groups" do
--- a/plugins/chat/spec/services/chat/create_direct_message_channel_spec.rb
+++ b/plugins/chat/spec/services/chat/create_direct_message_channel_spec.rb
@ -30,7 +30,7 @@ RSpec.describe Chat::CreateDirectMessageChannel do
  describe ".call" do
    subject(:result) { described_class.call(params) }

-    fab!(:current_user) { Fabricate(:user, username: "guybrush") }
+    fab!(:current_user) { Fabricate(:user, username: "guybrush", refresh_auto_groups: true) }
    fab!(:user_1) { Fabricate(:user, username: "lechuck") }
    fab!(:user_2) { Fabricate(:user, username: "elaine") }
    fab!(:user_3) { Fabricate(:user) }
@ -41,8 +41,6 @@ RSpec.describe Chat::CreateDirectMessageChannel do
    let(:name) { "" }
    let(:params) { { guardian: guardian, target_usernames: target_usernames, name: name } }

-    before { Group.refresh_automatic_groups! }
-
    context "when all steps pass" do
      it "sets the service result as successful" do
        expect(result).to be_a_success
--- a/plugins/chat/spec/services/chat/update_message_spec.rb
+++ b/plugins/chat/spec/services/chat/update_message_spec.rb
@ -23,7 +23,7 @@ RSpec.describe Chat::UpdateMessage do
    let(:guardian) { Guardian.new(user1) }
    fab!(:admin1) { Fabricate(:admin) }
    fab!(:admin2) { Fabricate(:admin) }
-    fab!(:user1) { Fabricate(:user) }
+    fab!(:user1) { Fabricate(:user, refresh_auto_groups: true) }
    fab!(:user2) { Fabricate(:user) }
    fab!(:user3) { Fabricate(:user) }
    fab!(:user4) { Fabricate(:user) }
@ -46,7 +46,6 @@ RSpec.describe Chat::UpdateMessage do
      [admin1, admin2, user1, user2, user3, user4].each do |user|
        Fabricate(:user_chat_channel_membership, chat_channel: public_chat_channel, user: user)
      end
-      Group.refresh_automatic_groups!
    end

    def create_chat_message(user, message, channel, upload_ids: nil)