From 3e65534e68bc3dc903291141ffee43134e560b28 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Wed, 10 Jun 2026 12:59:04 +0530 Subject: [PATCH] refactor: fold cache revalidation into the event dispatcher --- .../helper/CacheHelper/cacheableModels.js | 39 ++---- .../CacheHelper/dispatchCacheRevalidations.js | 38 +++++- .../dispatchCacheRevalidations.spec.js | 113 ++++++++++++++---- .../dashboard/store/modules/agents.js | 5 - .../dashboard/store/modules/attributes.js | 5 - .../dashboard/store/modules/cannedResponse.js | 6 - .../dashboard/store/modules/inboxes.js | 6 - .../dashboard/store/modules/labels.js | 7 -- .../dashboard/store/modules/teams/actions.js | 5 - .../dashboard/store/utils/cacheRevalidate.js | 14 --- .../store/utils/specs/cacheRevalidate.spec.js | 82 ------------- 11 files changed, 136 insertions(+), 184 deletions(-) delete mode 100644 app/javascript/dashboard/store/utils/cacheRevalidate.js delete mode 100644 app/javascript/dashboard/store/utils/specs/cacheRevalidate.spec.js diff --git a/app/javascript/dashboard/helper/CacheHelper/cacheableModels.js b/app/javascript/dashboard/helper/CacheHelper/cacheableModels.js index 5ff42bbb5..dcef9941a 100644 --- a/app/javascript/dashboard/helper/CacheHelper/cacheableModels.js +++ b/app/javascript/dashboard/helper/CacheHelper/cacheableModels.js @@ -4,39 +4,18 @@ // so the server's `cache_keys` payload (and the IDB object store name) lines up // with what the client looks up. // -// `dispatchPath` is the full Vuex dispatch path for the revalidate action. -// `setMutation` is the full commit path used by paintStoresFromCache to seed -// Vuex from IDB. Every SET_* mutation must REPLACE its records (not merge) so -// rows deleted server-side never survive as phantoms. +// `setMutation` is the full commit path used to seed Vuex from IDB (boot +// paint) and to swap in refetched rows (event-driven revalidation). Every +// SET_* mutation must REPLACE its records (not merge) so rows deleted +// server-side never survive as phantoms. export const cacheableModels = [ - { - name: 'inbox', - dispatchPath: 'inboxes/revalidate', - setMutation: 'inboxes/SET_INBOXES', - }, - { - name: 'label', - dispatchPath: 'labels/revalidate', - setMutation: 'labels/SET_LABELS', - }, - { - name: 'team', - dispatchPath: 'teams/revalidate', - setMutation: 'teams/SET_TEAMS', - }, - { - name: 'canned_response', - dispatchPath: 'revalidateCannedResponses', - setMutation: 'SET_CANNED', - }, - { - name: 'account_user', - dispatchPath: 'agents/revalidate', - setMutation: 'agents/SET_AGENTS', - }, + { 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', - dispatchPath: 'attributes/revalidate', setMutation: 'attributes/SET_CUSTOM_ATTRIBUTE', }, ]; diff --git a/app/javascript/dashboard/helper/CacheHelper/dispatchCacheRevalidations.js b/app/javascript/dashboard/helper/CacheHelper/dispatchCacheRevalidations.js index 72c5797a4..49a215d80 100644 --- a/app/javascript/dashboard/helper/CacheHelper/dispatchCacheRevalidations.js +++ b/app/javascript/dashboard/helper/CacheHelper/dispatchCacheRevalidations.js @@ -1,10 +1,42 @@ +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'; import { cacheableModels } from './cacheableModels'; +// model name → cache-enabled API client. Lives here rather than in +// cacheableModels to keep that module import-cycle-free: the API clients +// import DataManager, which imports cacheableModels. +const apiByModel = { + inbox: InboxesAPI, + label: LabelsAPI, + team: TeamsAPI, + canned_response: CannedResponseAPI, + account_user: AgentAPI, + custom_attribute_definition: AttributeAPI, +}; + +const revalidateModel = async (store, model, newKey) => { + try { + const api = apiByModel[model.name]; + if (await api.validateCacheKey(newKey)) return; + + const response = await api.refetchAndCommit(newKey); + store.commit(model.setMutation, api.extractDataFromResponse(response)); + } catch { + // Ignore error — a failed refetch leaves the painted data in place; the + // next pushed key map retries. + } +}; + +// The single freshness engine: given a pushed { model_name => key } map +// (RoomChannel transmits one on every (re)subscribe, the server broadcasts +// one on every change), diff each key against IDB and refetch mismatches. export const dispatchCacheRevalidations = (store, keys = {}) => Promise.all( cacheableModels .filter(model => keys[model.name] !== undefined) - .map(model => - store.dispatch(model.dispatchPath, { newKey: keys[model.name] }) - ) + .map(model => revalidateModel(store, model, keys[model.name])) ); diff --git a/app/javascript/dashboard/helper/specs/CacheHelper/dispatchCacheRevalidations.spec.js b/app/javascript/dashboard/helper/specs/CacheHelper/dispatchCacheRevalidations.spec.js index e31559f0f..82c3f427d 100644 --- a/app/javascript/dashboard/helper/specs/CacheHelper/dispatchCacheRevalidations.spec.js +++ b/app/javascript/dashboard/helper/specs/CacheHelper/dispatchCacheRevalidations.spec.js @@ -1,37 +1,108 @@ 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', () => ({ + default: { + validateCacheKey: vi.fn(), + refetchAndCommit: vi.fn(), + extractDataFromResponse: vi.fn(), + }, +})); +vi.mock('dashboard/api/labels', () => ({ + default: { + validateCacheKey: vi.fn(), + refetchAndCommit: vi.fn(), + extractDataFromResponse: vi.fn(), + }, +})); +vi.mock('dashboard/api/teams', () => ({ + default: { + validateCacheKey: vi.fn(), + refetchAndCommit: vi.fn(), + 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', () => { - it('dispatches revalidate actions for cacheable models present in the key payload', async () => { - const store = { - dispatch: vi.fn().mockResolvedValue(), - }; + let store; + + beforeEach(() => { + vi.clearAllMocks(); + store = { commit: vi.fn() }; + }); + + it('refetches stale models and commits via their setMutation', async () => { + InboxesAPI.validateCacheKey.mockResolvedValue(false); + InboxesAPI.refetchAndCommit.mockResolvedValue({ data: { payload: [] } }); + InboxesAPI.extractDataFromResponse.mockReturnValue([{ id: 1 }]); + LabelsAPI.validateCacheKey.mockResolvedValue(true); await dispatchCacheRevalidations(store, { inbox: 'inbox-key', label: 'label-key', - canned_response: 'canned-key', - unknown_model: 'ignored-key', }); - expect(store.dispatch).toHaveBeenCalledWith('inboxes/revalidate', { - newKey: 'inbox-key', - }); - expect(store.dispatch).toHaveBeenCalledWith('labels/revalidate', { - newKey: 'label-key', - }); - expect(store.dispatch).toHaveBeenCalledWith('revalidateCannedResponses', { - newKey: 'canned-key', - }); - expect(store.dispatch).toHaveBeenCalledTimes(3); + expect(InboxesAPI.refetchAndCommit).toHaveBeenCalledWith('inbox-key'); + expect(store.commit).toHaveBeenCalledWith('inboxes/SET_INBOXES', [ + { id: 1 }, + ]); + expect(LabelsAPI.refetchAndCommit).not.toHaveBeenCalled(); + expect(store.commit).toHaveBeenCalledTimes(1); + }); + + it('skips models absent from the key payload', async () => { + InboxesAPI.validateCacheKey.mockResolvedValue(true); + + await dispatchCacheRevalidations(store, { inbox: 'inbox-key' }); + + expect(TeamsAPI.validateCacheKey).not.toHaveBeenCalled(); + expect(store.commit).not.toHaveBeenCalled(); }); it('treats missing keys as an empty payload', async () => { - const store = { - dispatch: vi.fn(), - }; - await dispatchCacheRevalidations(store); - expect(store.dispatch).not.toHaveBeenCalled(); + expect(InboxesAPI.validateCacheKey).not.toHaveBeenCalled(); + expect(store.commit).not.toHaveBeenCalled(); + }); + + 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')); + CannedResponseAPI.validateCacheKey.mockResolvedValue(false); + CannedResponseAPI.refetchAndCommit.mockResolvedValue({ data: [] }); + CannedResponseAPI.extractDataFromResponse.mockReturnValue([{ id: 7 }]); + + await dispatchCacheRevalidations(store, { + inbox: 'inbox-key', + canned_response: 'canned-key', + }); + + 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 5dd567b5d..bd2778670 100644 --- a/app/javascript/dashboard/store/modules/agents.js +++ b/app/javascript/dashboard/store/modules/agents.js @@ -1,7 +1,6 @@ import * as MutationHelpers from 'shared/helpers/vuex/mutationHelpers'; import * as types from '../mutation-types'; import AgentAPI from '../../api/agents'; -import { createCacheRevalidateAction } from '../utils/cacheRevalidate'; export const state = { records: [], @@ -52,10 +51,6 @@ export const actions = { commit(types.default.SET_AGENT_FETCHING_STATUS, false); } }, - revalidate: createCacheRevalidateAction({ - api: AgentAPI, - mutation: types.default.SET_AGENTS, - }), create: async ({ commit }, agentInfo) => { commit(types.default.SET_AGENT_CREATING_STATUS, true); try { diff --git a/app/javascript/dashboard/store/modules/attributes.js b/app/javascript/dashboard/store/modules/attributes.js index d90966d4d..1c7826cea 100644 --- a/app/javascript/dashboard/store/modules/attributes.js +++ b/app/javascript/dashboard/store/modules/attributes.js @@ -2,7 +2,6 @@ import * as MutationHelpers from 'shared/helpers/vuex/mutationHelpers'; import types from '../mutation-types'; import AttributeAPI from '../../api/attributes'; import camelcaseKeys from 'camelcase-keys'; -import { createCacheRevalidateAction } from '../utils/cacheRevalidate'; export const state = { records: [], @@ -55,10 +54,6 @@ export const actions = { commit(types.SET_CUSTOM_ATTRIBUTE_UI_FLAG, { isFetching: false }); } }, - revalidate: createCacheRevalidateAction({ - api: AttributeAPI, - mutation: types.SET_CUSTOM_ATTRIBUTE, - }), create: async function createAttribute({ commit }, attributeObj) { commit(types.SET_CUSTOM_ATTRIBUTE_UI_FLAG, { isCreating: true }); try { diff --git a/app/javascript/dashboard/store/modules/cannedResponse.js b/app/javascript/dashboard/store/modules/cannedResponse.js index c06c50b80..568150392 100644 --- a/app/javascript/dashboard/store/modules/cannedResponse.js +++ b/app/javascript/dashboard/store/modules/cannedResponse.js @@ -1,5 +1,4 @@ import { throwErrorMessage } from 'dashboard/store/utils/api'; -import { createCacheRevalidateAction } from 'dashboard/store/utils/cacheRevalidate'; import * as MutationHelpers from 'shared/helpers/vuex/mutationHelpers'; import * as types from '../mutation-types'; import CannedResponseAPI from '../../api/cannedResponse'; @@ -48,11 +47,6 @@ const actions = { } }, - revalidateCannedResponses: createCacheRevalidateAction({ - api: CannedResponseAPI, - mutation: types.default.SET_CANNED, - }), - createCannedResponse: async function createCannedResponse( { commit }, cannedObj diff --git a/app/javascript/dashboard/store/modules/inboxes.js b/app/javascript/dashboard/store/modules/inboxes.js index aa2f7d69a..1d4572ad5 100644 --- a/app/javascript/dashboard/store/modules/inboxes.js +++ b/app/javascript/dashboard/store/modules/inboxes.js @@ -7,7 +7,6 @@ import FBChannel from '../../api/channel/fbChannel'; import TwilioChannel from '../../api/channel/twilioChannel'; import WhatsappChannel from '../../api/channel/whatsappChannel'; import { throwErrorMessage } from '../utils/api'; -import { createCacheRevalidateAction } from '../utils/cacheRevalidate'; import AnalyticsHelper from '../../helper/AnalyticsHelper'; import camelcaseKeys from 'camelcase-keys'; import { ACCOUNT_EVENTS } from '../../helper/AnalyticsHelper/events'; @@ -190,11 +189,6 @@ const sendAnalyticsEvent = channelType => { }; export const actions = { - revalidate: createCacheRevalidateAction({ - api: InboxesAPI, - mutation: types.default.SET_INBOXES, - getData: response => response.data.payload, - }), get: async ({ commit }) => { commit(types.default.SET_INBOXES_UI_FLAG, { isFetching: true }); try { diff --git a/app/javascript/dashboard/store/modules/labels.js b/app/javascript/dashboard/store/modules/labels.js index b06bce998..36205f3c9 100644 --- a/app/javascript/dashboard/store/modules/labels.js +++ b/app/javascript/dashboard/store/modules/labels.js @@ -3,7 +3,6 @@ import types from '../mutation-types'; import LabelsAPI from '../../api/labels'; import AnalyticsHelper from '../../helper/AnalyticsHelper'; import { LABEL_EVENTS } from '../../helper/AnalyticsHelper/events'; -import { createCacheRevalidateAction } from '../utils/cacheRevalidate'; export const state = { records: [], @@ -33,12 +32,6 @@ export const getters = { }; export const actions = { - revalidate: createCacheRevalidateAction({ - api: LabelsAPI, - mutation: types.SET_LABELS, - getData: response => response.data.payload, - }), - get: async function getLabels({ commit }) { commit(types.SET_LABEL_UI_FLAG, { isFetching: true }); try { diff --git a/app/javascript/dashboard/store/modules/teams/actions.js b/app/javascript/dashboard/store/modules/teams/actions.js index adfd57f3a..5509b56e9 100644 --- a/app/javascript/dashboard/store/modules/teams/actions.js +++ b/app/javascript/dashboard/store/modules/teams/actions.js @@ -6,7 +6,6 @@ import { DELETE_TEAM, } from './types'; import TeamsAPI from '../../../api/teams'; -import { createCacheRevalidateAction } from '../../utils/cacheRevalidate'; export const actions = { create: async ({ commit }, teamInfo) => { @@ -22,10 +21,6 @@ export const actions = { commit(SET_TEAM_UI_FLAG, { isCreating: false }); } }, - revalidate: createCacheRevalidateAction({ - api: TeamsAPI, - mutation: SET_TEAMS, - }), get: async ({ commit }) => { commit(SET_TEAM_UI_FLAG, { isFetching: true }); try { diff --git a/app/javascript/dashboard/store/utils/cacheRevalidate.js b/app/javascript/dashboard/store/utils/cacheRevalidate.js deleted file mode 100644 index 68847696e..000000000 --- a/app/javascript/dashboard/store/utils/cacheRevalidate.js +++ /dev/null @@ -1,14 +0,0 @@ -export const createCacheRevalidateAction = - ({ api, mutation, clearMutation, getData = response => response.data }) => - async ({ commit }, { newKey }) => { - try { - const isExistingKeyValid = await api.validateCacheKey(newKey); - if (isExistingKeyValid) return; - - const response = await api.refetchAndCommit(newKey); - if (clearMutation) commit(clearMutation); - commit(mutation, getData(response)); - } catch (error) { - // Ignore error - } - }; diff --git a/app/javascript/dashboard/store/utils/specs/cacheRevalidate.spec.js b/app/javascript/dashboard/store/utils/specs/cacheRevalidate.spec.js deleted file mode 100644 index b9b9624c8..000000000 --- a/app/javascript/dashboard/store/utils/specs/cacheRevalidate.spec.js +++ /dev/null @@ -1,82 +0,0 @@ -import { createCacheRevalidateAction } from '../cacheRevalidate'; - -describe('#createCacheRevalidateAction', () => { - const commit = vi.fn(); - - beforeEach(() => { - commit.mockReset(); - }); - - it('refetches and commits data when the cache key is stale', async () => { - const api = { - validateCacheKey: vi.fn().mockResolvedValue(false), - refetchAndCommit: vi.fn().mockResolvedValue({ data: [{ id: 1 }] }), - }; - - const action = createCacheRevalidateAction({ - api, - mutation: 'SET_RECORDS', - }); - - await action({ commit }, { newKey: 'new-key' }); - - expect(api.validateCacheKey).toHaveBeenCalledWith('new-key'); - expect(api.refetchAndCommit).toHaveBeenCalledWith('new-key'); - expect(commit).toHaveBeenCalledWith('SET_RECORDS', [{ id: 1 }]); - }); - - it('skips refetch when the cache key is current', async () => { - const api = { - validateCacheKey: vi.fn().mockResolvedValue(true), - refetchAndCommit: vi.fn(), - }; - - const action = createCacheRevalidateAction({ - api, - mutation: 'SET_RECORDS', - }); - - await action({ commit }, { newKey: 'new-key' }); - - expect(api.refetchAndCommit).not.toHaveBeenCalled(); - expect(commit).not.toHaveBeenCalled(); - }); - - it('supports clear-before-set and custom response data extraction', async () => { - const api = { - validateCacheKey: vi.fn().mockResolvedValue(false), - refetchAndCommit: vi - .fn() - .mockResolvedValue({ data: { payload: [{ id: 1 }] } }), - }; - - const action = createCacheRevalidateAction({ - api, - mutation: 'SET_RECORDS', - clearMutation: 'CLEAR_RECORDS', - getData: response => response.data.payload, - }); - - await action({ commit }, { newKey: 'new-key' }); - - expect(commit.mock.calls).toEqual([ - ['CLEAR_RECORDS'], - ['SET_RECORDS', [{ id: 1 }]], - ]); - }); - - it('ignores revalidation errors', async () => { - const api = { - validateCacheKey: vi.fn().mockRejectedValue(new Error('boom')), - refetchAndCommit: vi.fn(), - }; - - const action = createCacheRevalidateAction({ - api, - mutation: 'SET_RECORDS', - }); - - await expect(action({ commit }, { newKey: 'new-key' })).resolves.toBe(); - expect(commit).not.toHaveBeenCalled(); - }); -});