diff --git a/app/controllers/api/v1/profiles_controller.rb b/app/controllers/api/v1/profiles_controller.rb index 141253d0d..fdae6a7fa 100644 --- a/app/controllers/api/v1/profiles_controller.rb +++ b/app/controllers/api/v1/profiles_controller.rb @@ -13,11 +13,17 @@ class Api::V1::ProfilesController < Api::BaseController @user.assign_attributes(profile_params) @user.custom_attributes.merge!(custom_attributes_params) @user.save! + + # Profile updates can change cached agent fields, including avatar-backed thumbnails. + @user.invalidate_avatar_cache end def avatar @user.avatar.attachment.destroy! if @user.avatar.attached? @user.reload + + # Agent thumbnails are cached separately, and avatar attachment deletes do not dirty user columns. + @user.invalidate_avatar_cache end def auto_offline diff --git a/app/javascript/dashboard/api/agents.js b/app/javascript/dashboard/api/agents.js index cfc6b36ff..2f3b98903 100644 --- a/app/javascript/dashboard/api/agents.js +++ b/app/javascript/dashboard/api/agents.js @@ -1,10 +1,10 @@ /* global axios */ -import ApiClient from './ApiClient'; +import CacheEnabledApiClient from './CacheEnabledApiClient'; -class Agents extends ApiClient { +class Agents extends CacheEnabledApiClient { constructor() { - super('agents', { accountScoped: true }); + super('agents', { accountScoped: true, cacheModel: 'account_user' }); } bulkInvite({ emails }) { diff --git a/app/javascript/dashboard/api/attributes.js b/app/javascript/dashboard/api/attributes.js index 3552bb909..2abf144d5 100644 --- a/app/javascript/dashboard/api/attributes.js +++ b/app/javascript/dashboard/api/attributes.js @@ -1,13 +1,15 @@ -/* global axios */ -import ApiClient from './ApiClient'; +import CacheEnabledApiClient from './CacheEnabledApiClient'; -class AttributeAPI extends ApiClient { +class AttributeAPI extends CacheEnabledApiClient { constructor() { - super('custom_attribute_definitions', { accountScoped: true }); + super('custom_attribute_definitions', { + accountScoped: true, + cacheModel: 'custom_attribute_definition', + }); } getAttributesByModel() { - return axios.get(this.url); + return super.get(true); } } diff --git a/app/javascript/dashboard/api/cannedResponse.js b/app/javascript/dashboard/api/cannedResponse.js index f558dcaca..3a946dc9f 100644 --- a/app/javascript/dashboard/api/cannedResponse.js +++ b/app/javascript/dashboard/api/cannedResponse.js @@ -1,15 +1,20 @@ /* global axios */ -import ApiClient from './ApiClient'; +import CacheEnabledApiClient from './CacheEnabledApiClient'; -class CannedResponse extends ApiClient { +class CannedResponse extends CacheEnabledApiClient { constructor() { - super('canned_responses', { accountScoped: true }); + super('canned_responses', { + accountScoped: true, + cacheModel: 'canned_response', + }); } - get({ searchKey }) { - const url = searchKey ? `${this.url}?search=${searchKey}` : this.url; - return axios.get(url); + get({ searchKey } = {}) { + if (searchKey) { + return axios.get(`${this.url}?search=${searchKey}`); + } + return super.get(true); } } diff --git a/app/javascript/dashboard/helper/CacheHelper/cacheableModels.js b/app/javascript/dashboard/helper/CacheHelper/cacheableModels.js index b782f999f..dcef9941a 100644 --- a/app/javascript/dashboard/helper/CacheHelper/cacheableModels.js +++ b/app/javascript/dashboard/helper/CacheHelper/cacheableModels.js @@ -12,6 +12,12 @@ export const cacheableModels = [ { name: 'inbox', setMutation: 'inboxes/SET_INBOXES' }, { name: 'label', setMutation: 'labels/SET_LABELS' }, { name: 'team', setMutation: 'teams/SET_TEAMS' }, + { name: 'canned_response', setMutation: 'SET_CANNED' }, + { name: 'account_user', setMutation: 'agents/SET_AGENTS' }, + { + name: 'custom_attribute_definition', + setMutation: 'attributes/SET_CUSTOM_ATTRIBUTE', + }, ]; export const cacheableModelNames = cacheableModels.map(model => model.name); diff --git a/app/javascript/dashboard/helper/CacheHelper/dispatchCacheRevalidations.js b/app/javascript/dashboard/helper/CacheHelper/dispatchCacheRevalidations.js index f1339f668..49a215d80 100644 --- a/app/javascript/dashboard/helper/CacheHelper/dispatchCacheRevalidations.js +++ b/app/javascript/dashboard/helper/CacheHelper/dispatchCacheRevalidations.js @@ -1,3 +1,6 @@ +import AgentAPI from 'dashboard/api/agents'; +import AttributeAPI from 'dashboard/api/attributes'; +import CannedResponseAPI from 'dashboard/api/cannedResponse'; import InboxesAPI from 'dashboard/api/inboxes'; import LabelsAPI from 'dashboard/api/labels'; import TeamsAPI from 'dashboard/api/teams'; @@ -10,6 +13,9 @@ const apiByModel = { inbox: InboxesAPI, label: LabelsAPI, team: TeamsAPI, + canned_response: CannedResponseAPI, + account_user: AgentAPI, + custom_attribute_definition: AttributeAPI, }; const revalidateModel = async (store, model, newKey) => { diff --git a/app/javascript/dashboard/helper/CacheHelper/version.js b/app/javascript/dashboard/helper/CacheHelper/version.js index 07bd897f5..3d5893b56 100644 --- a/app/javascript/dashboard/helper/CacheHelper/version.js +++ b/app/javascript/dashboard/helper/CacheHelper/version.js @@ -1,3 +1,9 @@ -// Monday, 13 March 2023 -// Change this version if you want to invalidate old data -export const DATA_VERSION = '1678706392'; +// Bump DATA_VERSION to (a) add new object stores to the IDB schema or (b) +// flush bad/stale cache globally. The `upgrade()` callback in DataManager runs +// only when the stored DB version is less than the requested version; on any +// such bump it clears every existing store (a full cache reset) and then +// idempotently creates any missing stores. So bump this whenever a cached +// model's serializer shape changes, or to force all clients to refetch. +// +// Thursday, 28 May 2026 — bumped to add canned_response + account_user stores + custom_attribute_definition store +export const DATA_VERSION = '1748390400'; diff --git a/app/javascript/dashboard/helper/specs/CacheHelper/dispatchCacheRevalidations.spec.js b/app/javascript/dashboard/helper/specs/CacheHelper/dispatchCacheRevalidations.spec.js index 4439e5c7e..82c3f427d 100644 --- a/app/javascript/dashboard/helper/specs/CacheHelper/dispatchCacheRevalidations.spec.js +++ b/app/javascript/dashboard/helper/specs/CacheHelper/dispatchCacheRevalidations.spec.js @@ -1,6 +1,7 @@ import { dispatchCacheRevalidations } from '../../CacheHelper/dispatchCacheRevalidations'; import InboxesAPI from 'dashboard/api/inboxes'; import LabelsAPI from 'dashboard/api/labels'; +import CannedResponseAPI from 'dashboard/api/cannedResponse'; import TeamsAPI from 'dashboard/api/teams'; vi.mock('dashboard/api/inboxes', () => ({ @@ -24,6 +25,27 @@ vi.mock('dashboard/api/teams', () => ({ extractDataFromResponse: vi.fn(), }, })); +vi.mock('dashboard/api/cannedResponse', () => ({ + default: { + validateCacheKey: vi.fn(), + refetchAndCommit: vi.fn(), + extractDataFromResponse: vi.fn(), + }, +})); +vi.mock('dashboard/api/agents', () => ({ + default: { + validateCacheKey: vi.fn(), + refetchAndCommit: vi.fn(), + extractDataFromResponse: vi.fn(), + }, +})); +vi.mock('dashboard/api/attributes', () => ({ + default: { + validateCacheKey: vi.fn(), + refetchAndCommit: vi.fn(), + extractDataFromResponse: vi.fn(), + }, +})); describe('dispatchCacheRevalidations', () => { let store; @@ -71,16 +93,16 @@ describe('dispatchCacheRevalidations', () => { it('swallows per-model errors so one failure does not block the rest', async () => { InboxesAPI.validateCacheKey.mockResolvedValue(false); InboxesAPI.refetchAndCommit.mockRejectedValue(new Error('network down')); - TeamsAPI.validateCacheKey.mockResolvedValue(false); - TeamsAPI.refetchAndCommit.mockResolvedValue({ data: [] }); - TeamsAPI.extractDataFromResponse.mockReturnValue([{ id: 7 }]); + CannedResponseAPI.validateCacheKey.mockResolvedValue(false); + CannedResponseAPI.refetchAndCommit.mockResolvedValue({ data: [] }); + CannedResponseAPI.extractDataFromResponse.mockReturnValue([{ id: 7 }]); await dispatchCacheRevalidations(store, { inbox: 'inbox-key', - team: 'team-key', + canned_response: 'canned-key', }); - expect(store.commit).toHaveBeenCalledWith('teams/SET_TEAMS', [{ id: 7 }]); + expect(store.commit).toHaveBeenCalledWith('SET_CANNED', [{ id: 7 }]); expect(store.commit).toHaveBeenCalledTimes(1); }); }); diff --git a/app/javascript/dashboard/store/modules/agents.js b/app/javascript/dashboard/store/modules/agents.js index 26ad05e65..bd2778670 100644 --- a/app/javascript/dashboard/store/modules/agents.js +++ b/app/javascript/dashboard/store/modules/agents.js @@ -44,7 +44,7 @@ export const actions = { get: async ({ commit }) => { commit(types.default.SET_AGENT_FETCHING_STATUS, true); try { - const response = await AgentAPI.get(); + const response = await AgentAPI.get(true); commit(types.default.SET_AGENT_FETCHING_STATUS, false); commit(types.default.SET_AGENTS, response.data); } catch (error) { diff --git a/app/javascript/dashboard/store/modules/specs/agents/actions.spec.js b/app/javascript/dashboard/store/modules/specs/agents/actions.spec.js index 1a43e9200..c65917909 100644 --- a/app/javascript/dashboard/store/modules/specs/agents/actions.spec.js +++ b/app/javascript/dashboard/store/modules/specs/agents/actions.spec.js @@ -1,6 +1,7 @@ import axios from 'axios'; import { actions } from '../../agents'; import * as types from '../../../mutation-types'; +import AgentAPI from '../../../../api/agents'; import agentList from './fixtures'; const commit = vi.fn(); @@ -8,6 +9,13 @@ const dispatch = vi.fn(); global.axios = axios; vi.mock('axios'); +// Clear the IDB-backed cache between tests so each case starts from a known +// empty state and isn't affected by data persisted by a previous test. +beforeEach(async () => { + await AgentAPI.dataManager.initDb(); + await AgentAPI.dataManager.db.clear(AgentAPI.cacheModelName); +}); + describe('#actions', () => { describe('#get', () => { it('sends correct actions if API is success', async () => { diff --git a/app/javascript/dashboard/store/modules/specs/attributes/actions.spec.js b/app/javascript/dashboard/store/modules/specs/attributes/actions.spec.js index d3df969ac..6fe9c0d53 100644 --- a/app/javascript/dashboard/store/modules/specs/attributes/actions.spec.js +++ b/app/javascript/dashboard/store/modules/specs/attributes/actions.spec.js @@ -1,12 +1,20 @@ import axios from 'axios'; import { actions } from '../../attributes'; import * as types from '../../../mutation-types'; +import AttributeAPI from '../../../../api/attributes'; import attributesList from './fixtures'; const commit = vi.fn(); global.axios = axios; vi.mock('axios'); +// Clear the IDB-backed cache between tests so each case starts from a known +// empty state and isn't affected by data persisted by a previous test. +beforeEach(async () => { + await AttributeAPI.dataManager.initDb(); + await AttributeAPI.dataManager.db.clear(AttributeAPI.cacheModelName); +}); + describe('#actions', () => { describe('#get', () => { it('sends correct actions if API is success', async () => { diff --git a/app/jobs/avatar/avatar_from_url_job.rb b/app/jobs/avatar/avatar_from_url_job.rb index 49bf25803..c079ee79c 100644 --- a/app/jobs/avatar/avatar_from_url_job.rb +++ b/app/jobs/avatar/avatar_from_url_job.rb @@ -52,6 +52,9 @@ class Avatar::AvatarFromUrlJob < ApplicationJob filename: avatar_file.original_filename, content_type: avatar_file.content_type ) + + # Agent thumbnails are cached separately, and avatar attachments do not dirty user columns. + avatarable.invalidate_avatar_cache if avatarable.respond_to?(:invalidate_avatar_cache) end def log_http_error(avatar_url, error) diff --git a/app/models/account_user.rb b/app/models/account_user.rb index bbcb0e010..0f4071536 100644 --- a/app/models/account_user.rb +++ b/app/models/account_user.rb @@ -36,10 +36,16 @@ class AccountUser < ApplicationRecord accepts_nested_attributes_for :account + AGENT_CACHE_RELEVANT_COLUMNS = %w[role availability auto_offline custom_role_id].freeze + after_create_commit :notify_creation, :create_notification_setting after_destroy :notify_deletion, :remove_user_from_account after_save :update_presence_in_redis, if: :saved_change_to_availability? + after_commit -> { account.update_cache_key('account_user') }, on: [:create, :destroy] + after_update_commit -> { account.update_cache_key('account_user') }, + if: -> { saved_changes.keys.intersect?(AGENT_CACHE_RELEVANT_COLUMNS) } + validates :user_id, uniqueness: { scope: :account_id } def create_notification_setting diff --git a/app/models/canned_response.rb b/app/models/canned_response.rb index b70f1c32d..b641f3faf 100644 --- a/app/models/canned_response.rb +++ b/app/models/canned_response.rb @@ -11,6 +11,8 @@ # class CannedResponse < ApplicationRecord + include AccountCacheRevalidator + validates :content, presence: true validates :short_code, presence: true validates :account, presence: true diff --git a/app/models/concerns/cache_keys.rb b/app/models/concerns/cache_keys.rb index f37c126ad..ff02071df 100644 --- a/app/models/concerns/cache_keys.rb +++ b/app/models/concerns/cache_keys.rb @@ -12,7 +12,7 @@ module CacheKeys included do class_attribute :cacheable_models - self.cacheable_models = [Label, Inbox, Team] + self.cacheable_models = [Label, Inbox, Team, CannedResponse, AccountUser, CustomAttributeDefinition] end def cache_keys diff --git a/app/models/custom_attribute_definition.rb b/app/models/custom_attribute_definition.rb index 35f822335..a37feea8e 100644 --- a/app/models/custom_attribute_definition.rb +++ b/app/models/custom_attribute_definition.rb @@ -22,6 +22,8 @@ # index_custom_attribute_definitions_on_account_id (account_id) # class CustomAttributeDefinition < ApplicationRecord + include AccountCacheRevalidator + STANDARD_ATTRIBUTES = { :conversation => %w[status priority assignee_id inbox_id team_id display_id campaign_id labels browser_language country_code referer created_at last_activity_at], diff --git a/app/models/user.rb b/app/models/user.rb index 4aa38bbcd..30a6b3b98 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -116,8 +116,12 @@ class User < ApplicationRecord has_many :macros, foreign_key: 'created_by_id', inverse_of: :created_by # rubocop:enable Rails/HasManyOrHasOneDependent + AGENT_CACHE_RELEVANT_COLUMNS = %w[name email display_name confirmed_at custom_attributes].freeze + before_validation :set_password_and_uid, on: :create after_destroy :remove_macros + after_update_commit :bump_account_user_cache_keys, + if: -> { saved_changes.keys.intersect?(AGENT_CACHE_RELEVANT_COLUMNS) } scope :order_by_full_name, -> { order('lower(name) ASC') } @@ -212,11 +216,19 @@ class User < ApplicationRecord super end + def invalidate_avatar_cache + bump_account_user_cache_keys + end + private def remove_macros macros.personal.destroy_all end + + def bump_account_user_cache_keys + accounts.each { |account| account.update_cache_key('account_user') } + end end User.include_mod_with('Audit::User') diff --git a/enterprise/app/models/custom_role.rb b/enterprise/app/models/custom_role.rb index 666f91378..5b3ec8f85 100644 --- a/enterprise/app/models/custom_role.rb +++ b/enterprise/app/models/custom_role.rb @@ -39,4 +39,11 @@ class CustomRole < ApplicationRecord validates :name, presence: true validates :permissions, inclusion: { in: PERMISSIONS } + + # CustomRole details are embedded into the cached account_user payload via + # api/v1/models/_account_user.json.jbuilder, so bump that cache key on any + # change. `dependent: :nullify` updates account_users via update_all (which + # skips their callbacks), so the deletion is bumped here directly. + after_update_commit -> { account.update_cache_key('account_user') } + after_destroy_commit -> { account.update_cache_key('account_user') } end diff --git a/spec/channels/room_channel_spec.rb b/spec/channels/room_channel_spec.rb index 22b1a54a6..4d142f666 100644 --- a/spec/channels/room_channel_spec.rb +++ b/spec/channels/room_channel_spec.rb @@ -27,7 +27,7 @@ RSpec.describe RoomChannel do cache_event = transmissions.find { |message| message['event'] == 'account.cache_invalidated' } expect(cache_event['data']['account_id']).to eq(account.id) - expect(cache_event['data']['cache_keys'].keys).to match_array(%w[label inbox team]) + expect(cache_event['data']['cache_keys'].keys).to match_array(%w[label inbox team canned_response account_user custom_attribute_definition]) end it 'does not transmit cache keys to contact subscribers' do diff --git a/spec/controllers/api/v1/accounts_controller_spec.rb b/spec/controllers/api/v1/accounts_controller_spec.rb index 579114abf..1a67e441c 100644 --- a/spec/controllers/api/v1/accounts_controller_spec.rb +++ b/spec/controllers/api/v1/accounts_controller_spec.rb @@ -213,7 +213,7 @@ RSpec.describe 'Accounts API', type: :request do as: :json expect(response).to have_http_status(:success) - expect(response.parsed_body['cache_keys'].keys).to match_array(%w[label inbox team]) + expect(response.parsed_body['cache_keys'].keys).to match_array(%w[account_user canned_response custom_attribute_definition inbox label team]) end it 'does not allow cached cache key responses' do diff --git a/spec/controllers/super_admin/accounts_controller_spec.rb b/spec/controllers/super_admin/accounts_controller_spec.rb index e4ff81a08..6944667b0 100644 --- a/spec/controllers/super_admin/accounts_controller_spec.rb +++ b/spec/controllers/super_admin/accounts_controller_spec.rb @@ -45,7 +45,7 @@ RSpec.describe 'Super Admin accounts API', type: :request do context 'when it is an authenticated user' do it 'shows the list of accounts' do - expect(account.cache_keys.keys).to contain_exactly(:inbox, :label, :team) + expect(account.cache_keys.keys).to contain_exactly(:inbox, :label, :team, :canned_response, :account_user, :custom_attribute_definition) sign_in(super_admin, scope: :super_admin) now_timestamp = Time.now.utc.to_i diff --git a/spec/enterprise/models/custom_role_spec.rb b/spec/enterprise/models/custom_role_spec.rb index f63f3c2dd..e6222d6ad 100644 --- a/spec/enterprise/models/custom_role_spec.rb +++ b/spec/enterprise/models/custom_role_spec.rb @@ -9,4 +9,19 @@ RSpec.describe CustomRole, type: :model do describe 'validations' do it { is_expected.to validate_presence_of(:name) } end + + describe 'account_user cache invalidation' do + let(:custom_role) { create(:custom_role) } + + it 'bumps the account_user cache key after update' do + expect(custom_role.account).to receive(:update_cache_key).with('account_user') + custom_role.update(name: 'New Name') + end + + it 'bumps the account_user cache key after destroy' do + custom_role + expect(custom_role.account).to receive(:update_cache_key).with('account_user') + custom_role.destroy + end + end end