feat: invalidate cache after inbox members or team members update (#10869)

At the moment, when updating the inbox members, or team members the
account cache used for IndexedDB is not invalidated. This can cause
inconsistencies in the UI. This PR fixes this by adding explicit
invalidation after performing the member changes

### Summary of changes

1. Added a new method `add_members` and `remove_members` to both `team`
and `inbox` models. The change was necessary for two reasons
- Since the individual `add_member` and `remove_member` is called in a
loop, it's wasteful to run the cache invalidation in the method.
- Moving the account cache invalidation call in the controller pollutes
the controller business logic
2. Updated tests across the board

### More improvements

We can make a concern called `Memberable` with usage like
`memberable_with :inbox_members`, that can encapsulate the functionality

---

Related: https://github.com/chatwoot/chatwoot/issues/10578

app/controllers/api/v1/accounts/inbox_members_controller.rb

@@ -10,7 +10,7 @@ class Api::V1::Accounts::InboxMembersController < Api::V1::Accounts::BaseControl
   def create
     authorize @inbox, :create?
     ActiveRecord::Base.transaction do
-      agents_to_be_added_ids.map { |user_id| @inbox.add_member(user_id) }
+      @inbox.add_members(agents_to_be_added_ids)
     end
     fetch_updated_agents
   end
@@ -24,7 +24,7 @@ class Api::V1::Accounts::InboxMembersController < Api::V1::Accounts::BaseControl
   def destroy
     authorize @inbox, :destroy?
     ActiveRecord::Base.transaction do
-      params[:user_ids].map { |user_id| @inbox.remove_member(user_id) }
+      @inbox.remove_members(params[:user_ids])
     end
     head :ok
   end
@@ -41,8 +41,8 @@ class Api::V1::Accounts::InboxMembersController < Api::V1::Accounts::BaseControl
     # the missing ones are the agents which are to be deleted from the inbox
     # the new ones are the agents which are to be added to the inbox
     ActiveRecord::Base.transaction do
-      agents_to_be_added_ids.each { |user_id| @inbox.add_member(user_id) }
+      @inbox.add_members(agents_to_be_added_ids)
-      agents_to_be_removed_ids.each { |user_id| @inbox.remove_member(user_id) }
+      @inbox.remove_members(agents_to_be_removed_ids)
     end
   end
 

app/controllers/api/v1/accounts/team_members_controller.rb

@@ -9,14 +9,14 @@ class Api::V1::Accounts::TeamMembersController < Api::V1::Accounts::BaseControll
 
   def create
     ActiveRecord::Base.transaction do
-      @team_members = members_to_be_added_ids.map { |user_id| @team.add_member(user_id) }
+      @team_members = @team.add_members(members_to_be_added_ids)
     end
   end
 
   def update
     ActiveRecord::Base.transaction do
-      members_to_be_added_ids.each { |user_id| @team.add_member(user_id) }
+      @team.add_members(members_to_be_added_ids)
-      members_to_be_removed_ids.each { |user_id| @team.remove_member(user_id) }
+      @team.remove_members(members_to_be_removed_ids)
     end
     @team_members = @team.members
     render action: 'create'
@@ -24,7 +24,7 @@ class Api::V1::Accounts::TeamMembersController < Api::V1::Accounts::BaseControll
 
   def destroy
     ActiveRecord::Base.transaction do
-      params[:user_ids].map { |user_id| @team.remove_member(user_id) }
+      @team.remove_members(params[:user_ids])
     end
     head :ok
   end

app/models/inbox.rb

@@ -82,14 +82,20 @@ class Inbox < ApplicationRecord
 
   scope :order_by_name, -> { order('lower(name) ASC') }
 
-  def add_member(user_id)
+  # Adds multiple members to the inbox
-    member = inbox_members.new(user_id: user_id)
+  # @param user_ids [Array<Integer>] Array of user IDs to add as members
-    member.save!
+  # @return [void]
+  def add_members(user_ids)
+    inbox_members.create!(user_ids.map { |user_id| { user_id: user_id } })
+    update_account_cache
   end
 
-  def remove_member(user_id)
+  # Removes multiple members from the inbox
-    member = inbox_members.find_by!(user_id: user_id)
+  # @param user_ids [Array<Integer>] Array of user IDs to remove
-    member.try(:destroy)
+  # @return [void]
+  def remove_members(user_ids)
+    inbox_members.where(user_id: user_ids).destroy_all
+    update_account_cache
   end
 
   def facebook?

app/models/team.rb

@@ -31,12 +31,24 @@ class Team < ApplicationRecord
     self.name = name.downcase if attribute_present?('name')
   end
 
-  def add_member(user_id)
+  # Adds multiple members to the team
-    team_members.find_or_create_by(user_id: user_id)&.user
+  # @param user_ids [Array<Integer>] Array of user IDs to add as members
+  # @return [Array<User>] Array of newly added members
+  def add_members(user_ids)
+    team_members_to_create = user_ids.map { |user_id| { user_id: user_id } }
+    created_members = team_members.create(team_members_to_create)
+    added_users = created_members.filter_map(&:user)
+
+    update_account_cache
+    added_users
   end
 
-  def remove_member(user_id)
+  # Removes multiple members from the team
-    team_members.find_by(user_id: user_id)&.destroy!
+  # @param user_ids [Array<Integer>] Array of user IDs to remove
+  # @return [void]
+  def remove_members(user_ids)
+    team_members.where(user_id: user_ids).destroy_all
+    update_account_cache
   end
 
   def messages

spec/controllers/api/v1/accounts/inbox_members_controller_spec.rb

@@ -255,28 +255,30 @@ RSpec.describe 'Inbox Member API', type: :request do
         expect(response).to have_http_status(:not_found)
       end
 
-      it 'renders error on invalid params' do
+      it 'ignores invalid params' do
         params = { inbox_id: inbox.id, user_ids: ['invalid'] }
+        original_count = inbox.inbox_members&.count
 
         delete "/api/v1/accounts/#{account.id}/inbox_members",
                headers: administrator.create_new_auth_token,
                params: params,
                as: :json
 
-        expect(response).to have_http_status(:not_found)
+        expect(response).to have_http_status(:success)
-        expect(response.body).to include('Resource could not be found')
+        expect(inbox.inbox_members&.count).to eq(original_count)
       end
 
-      it 'renders error on non member params' do
+      it 'ignores non member params' do
         params = { inbox_id: inbox.id, user_ids: [non_member_agent.id] }
+        original_count = inbox.inbox_members&.count
 
         delete "/api/v1/accounts/#{account.id}/inbox_members",
                headers: administrator.create_new_auth_token,
                params: params,
                as: :json
 
-        expect(response).to have_http_status(:not_found)
+        expect(response).to have_http_status(:success)
-        expect(response.body).to include('Resource could not be found')
+        expect(inbox.inbox_members&.count).to eq(original_count)
       end
     end
   end

spec/models/inbox_spec.rb

@@ -41,29 +41,50 @@ RSpec.describe Inbox do
     it_behaves_like 'avatarable'
   end
 
-  describe '#add_member' do
+  describe '#add_members' do
     let(:inbox) { FactoryBot.create(:inbox) }
-    let(:user) { FactoryBot.create(:user) }
 
-    it do
+    before do
-      expect(inbox.inbox_members.size).to eq(0)
+      allow(Rails.configuration.dispatcher).to receive(:dispatch)
+    end
 
-      inbox.add_member(user.id)
+    it 'handles adds all members and resets cache keys' do
-      expect(inbox.reload.inbox_members.size).to eq(1)
+      users = FactoryBot.create_list(:user, 3)
+      inbox.add_members(users.map(&:id))
+      expect(inbox.reload.inbox_members.size).to eq(3)
+
+      expect(Rails.configuration.dispatcher).to have_received(:dispatch).at_least(:once)
+                                                                        .with(
+                                                                          'account.cache_invalidated',
+                                                                          kind_of(Time),
+                                                                          account: inbox.account,
+                                                                          cache_keys: inbox.account.cache_keys
+                                                                        )
     end
   end
 
-  describe '#remove_member' do
+  describe '#remove_members' do
     let(:inbox) { FactoryBot.create(:inbox) }
-    let(:user) { FactoryBot.create(:user) }
+    let(:users) { FactoryBot.create_list(:user, 3) }
 
-    before { inbox.add_member(user.id) }
+    before do
+      inbox.add_members(users.map(&:id))
+      allow(Rails.configuration.dispatcher).to receive(:dispatch)
+    end
 
-    it do
+    it 'removes the members and resets cache keys' do
-      expect(inbox.inbox_members.size).to eq(1)
+      expect(inbox.reload.inbox_members.size).to eq(3)
 
-      inbox.remove_member(user.id)
+      inbox.remove_members(users.map(&:id))
       expect(inbox.reload.inbox_members.size).to eq(0)
+
+      expect(Rails.configuration.dispatcher).to have_received(:dispatch).at_least(:once)
+                                                                        .with(
+                                                                          'account.cache_invalidated',
+                                                                          kind_of(Time),
+                                                                          account: inbox.account,
+                                                                          cache_keys: inbox.account.cache_keys
+                                                                        )
     end
   end
 

spec/models/team_spec.rb

@@ -6,4 +6,51 @@ RSpec.describe Team do
     it { is_expected.to have_many(:conversations) }
     it { is_expected.to have_many(:team_members) }
   end
+
+  describe '#add_members' do
+    let(:team) { FactoryBot.create(:team) }
+
+    before do
+      allow(Rails.configuration.dispatcher).to receive(:dispatch)
+    end
+
+    it 'handles adds all members and resets cache keys' do
+      users = FactoryBot.create_list(:user, 3)
+      team.add_members(users.map(&:id))
+      expect(team.reload.team_members.size).to eq(3)
+
+      expect(Rails.configuration.dispatcher).to have_received(:dispatch).at_least(:once)
+                                                                        .with(
+                                                                          'account.cache_invalidated',
+                                                                          kind_of(Time),
+                                                                          account: team.account,
+                                                                          cache_keys: team.account.cache_keys
+                                                                        )
+    end
+  end
+
+  describe '#remove_members' do
+    let(:team) { FactoryBot.create(:team) }
+    let(:users) { FactoryBot.create_list(:user, 3) }
+
+    before do
+      team.add_members(users.map(&:id))
+      allow(Rails.configuration.dispatcher).to receive(:dispatch)
+    end
+
+    it 'removes the members and resets cache keys' do
+      expect(team.reload.team_members.size).to eq(3)
+
+      team.remove_members(users.map(&:id))
+      expect(team.reload.team_members.size).to eq(0)
+
+      expect(Rails.configuration.dispatcher).to have_received(:dispatch).at_least(:once)
+                                                                        .with(
+                                                                          'account.cache_invalidated',
+                                                                          kind_of(Time),
+                                                                          account: team.account,
+                                                                          cache_keys: team.account.cache_keys
+                                                                        )
+    end
+  end
 end