From d4dd5c64a73963ce0970b0bcab4666490f092686 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Wed, 10 Jun 2026 13:16:04 +0530 Subject: [PATCH] refactor: drive cache freshness from pushed key maps --- app/controllers/api/v1/accounts_controller.rb | 7 +- .../dashboard/api/CacheEnabledApiClient.js | 63 ++++++++------ app/javascript/dashboard/api/account.js | 7 -- app/javascript/dashboard/api/inboxes.js | 11 ++- app/javascript/dashboard/api/labels.js | 11 ++- app/javascript/dashboard/api/teams.js | 18 +--- .../CacheHelper/dispatchCacheRevalidations.js | 36 ++++++++ .../dashboard/helper/ReconnectService.js | 15 +--- .../dashboard/helper/actionCable.js | 6 +- .../dispatchCacheRevalidations.spec.js | 86 +++++++++++++++++++ .../helper/specs/ReconnectService.spec.js | 25 +----- .../dashboard/store/modules/accounts.js | 4 - .../dashboard/store/modules/inboxes.js | 11 --- .../dashboard/store/modules/labels.js | 12 --- .../modules/specs/inboxes/actions.spec.js | 8 ++ .../modules/specs/labels/actions.spec.js | 8 ++ .../store/modules/specs/teams/actions.spec.js | 10 ++- .../modules/specs/teams/mutations.spec.js | 18 ++-- .../dashboard/store/modules/teams/actions.js | 13 --- .../store/modules/teams/mutations.js | 16 ++-- .../dashboard/store/modules/teams/types.js | 1 - .../api/v1/accounts_controller_spec.rb | 7 +- 22 files changed, 216 insertions(+), 177 deletions(-) create mode 100644 app/javascript/dashboard/helper/CacheHelper/dispatchCacheRevalidations.js create mode 100644 app/javascript/dashboard/helper/specs/CacheHelper/dispatchCacheRevalidations.spec.js diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index fb991949a..46087ec36 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -50,7 +50,6 @@ class Api::V1::AccountsController < Api::BaseController end def cache_keys - expires_in 10.seconds, public: false, stale_while_revalidate: 5.minutes render json: { cache_keys: cache_keys_for_account }, status: :ok end @@ -93,11 +92,7 @@ class Api::V1::AccountsController < Api::BaseController end def cache_keys_for_account - { - label: fetch_value_for_key(params[:id], Label.name.underscore), - inbox: fetch_value_for_key(params[:id], Inbox.name.underscore), - team: fetch_value_for_key(params[:id], Team.name.underscore) - } + @account.cache_keys end def fetch_account diff --git a/app/javascript/dashboard/api/CacheEnabledApiClient.js b/app/javascript/dashboard/api/CacheEnabledApiClient.js index 9af939c00..e00af7f82 100644 --- a/app/javascript/dashboard/api/CacheEnabledApiClient.js +++ b/app/javascript/dashboard/api/CacheEnabledApiClient.js @@ -5,14 +5,15 @@ import ApiClient from './ApiClient'; class CacheEnabledApiClient extends ApiClient { constructor(resource, options = {}) { super(resource, options); + // `cacheModel` is the Rails Model.name.underscore value — simultaneously + // the server cache-key name and the IDB object-store name. + this.cacheModelName = options.cacheModel; + // inbox/label endpoints wrap collections in { payload }; the rest return + // the bare array. + this.payloadEnvelope = options.payloadEnvelope || false; this.dataManager = new DataManager(this.accountIdFromRoute); } - // eslint-disable-next-line class-methods-use-this - get cacheModelName() { - throw new Error('cacheModelName is not defined'); - } - get(cache = false) { if (cache) { return this.getFromCache(); @@ -25,14 +26,14 @@ class CacheEnabledApiClient extends ApiClient { return axios.get(this.url); } - // eslint-disable-next-line class-methods-use-this extractDataFromResponse(response) { - return response.data.payload; + return this.payloadEnvelope ? response.data.payload : response.data; } - // eslint-disable-next-line class-methods-use-this marshallData(dataToParse) { - return { data: { payload: dataToParse } }; + return this.payloadEnvelope + ? { data: { payload: dataToParse } } + : { data: dataToParse }; } async getFromCache() { @@ -43,24 +44,23 @@ class CacheEnabledApiClient extends ApiClient { return this.getFromNetwork(); } - const { data } = await axios.get( - `/api/v1/accounts/${this.accountIdFromRoute}/cache_keys` - ); - const cacheKeyFromApi = data.cache_keys[this.cacheModelName]; - const isCacheValid = await this.validateCacheKey(cacheKeyFromApi); + // Trust the IDB cache. Freshness is maintained by the + // account.cache_invalidated event alone: RoomChannel pushes the cache-key + // map on every (re)subscribe — boot and reconnect included — and the + // server broadcasts it on every change. Skipping a per-call /cache_keys + // preflight eliminates N GET requests per cold settings-page load. + const localData = await this.dataManager.get({ + modelName: this.cacheModelName, + }); - let localData = []; - if (isCacheValid) { - localData = await this.dataManager.get({ - modelName: this.cacheModelName, - }); + if (localData.length > 0) { + return this.marshallData(localData); } - if (localData.length === 0) { - return this.refetchAndCommit(cacheKeyFromApi); - } - - return this.marshallData(localData); + // Empty IDB (first load or wiped): fetch data without a cache key. The + // next pushed key map won't match the missing key and will refetch once, + // stamping the authoritative key — the client never pulls keys itself. + return this.refetchAndCommit(null); } async refetchAndCommit(newKey = null) { @@ -69,7 +69,9 @@ class CacheEnabledApiClient extends ApiClient { try { await this.dataManager.initDb(); - this.dataManager.replace({ + // Await replace so data is persisted before the cache key is — otherwise + // a concurrent reader could see a fresh key paired with stale data. + await this.dataManager.replace({ modelName: this.cacheModelName, data: this.extractDataFromResponse(response), }); @@ -89,8 +91,15 @@ class CacheEnabledApiClient extends ApiClient { await this.dataManager.initDb(); } - const cachekey = await this.dataManager.getCacheKey(this.cacheModelName); - return cacheKeyFromApi === cachekey; + const cacheKey = await this.dataManager.getCacheKey(this.cacheModelName); + if (cacheKey === undefined) { + const localData = await this.dataManager.get({ + modelName: this.cacheModelName, + }); + return localData.length === 0; + } + + return cacheKeyFromApi === cacheKey; } } diff --git a/app/javascript/dashboard/api/account.js b/app/javascript/dashboard/api/account.js index c0dcf05f3..82b0c434c 100644 --- a/app/javascript/dashboard/api/account.js +++ b/app/javascript/dashboard/api/account.js @@ -9,13 +9,6 @@ class AccountAPI extends ApiClient { createAccount(data) { return axios.post(`${this.apiVersion}/accounts`, data); } - - async getCacheKeys() { - const response = await axios.get( - `/api/v1/accounts/${this.accountIdFromRoute}/cache_keys` - ); - return response.data.cache_keys; - } } export default new AccountAPI(); diff --git a/app/javascript/dashboard/api/inboxes.js b/app/javascript/dashboard/api/inboxes.js index 114dbb6f4..9af8fc765 100644 --- a/app/javascript/dashboard/api/inboxes.js +++ b/app/javascript/dashboard/api/inboxes.js @@ -3,12 +3,11 @@ import CacheEnabledApiClient from './CacheEnabledApiClient'; class Inboxes extends CacheEnabledApiClient { constructor() { - super('inboxes', { accountScoped: true }); - } - - // eslint-disable-next-line class-methods-use-this - get cacheModelName() { - return 'inbox'; + super('inboxes', { + accountScoped: true, + cacheModel: 'inbox', + payloadEnvelope: true, + }); } getCampaigns(inboxId) { diff --git a/app/javascript/dashboard/api/labels.js b/app/javascript/dashboard/api/labels.js index 2b521b058..a3278342a 100644 --- a/app/javascript/dashboard/api/labels.js +++ b/app/javascript/dashboard/api/labels.js @@ -2,12 +2,11 @@ import CacheEnabledApiClient from './CacheEnabledApiClient'; class LabelsAPI extends CacheEnabledApiClient { constructor() { - super('labels', { accountScoped: true }); - } - - // eslint-disable-next-line class-methods-use-this - get cacheModelName() { - return 'label'; + super('labels', { + accountScoped: true, + cacheModel: 'label', + payloadEnvelope: true, + }); } } diff --git a/app/javascript/dashboard/api/teams.js b/app/javascript/dashboard/api/teams.js index 5413af96b..c795355d3 100644 --- a/app/javascript/dashboard/api/teams.js +++ b/app/javascript/dashboard/api/teams.js @@ -1,25 +1,9 @@ /* global axios */ -// import ApiClient from './ApiClient'; import CacheEnabledApiClient from './CacheEnabledApiClient'; export class TeamsAPI extends CacheEnabledApiClient { constructor() { - super('teams', { accountScoped: true }); - } - - // eslint-disable-next-line class-methods-use-this - get cacheModelName() { - return 'team'; - } - - // eslint-disable-next-line class-methods-use-this - extractDataFromResponse(response) { - return response.data; - } - - // eslint-disable-next-line class-methods-use-this - marshallData(dataToParse) { - return { data: dataToParse }; + super('teams', { accountScoped: true, cacheModel: 'team' }); } getAgents({ teamId }) { diff --git a/app/javascript/dashboard/helper/CacheHelper/dispatchCacheRevalidations.js b/app/javascript/dashboard/helper/CacheHelper/dispatchCacheRevalidations.js new file mode 100644 index 000000000..f1339f668 --- /dev/null +++ b/app/javascript/dashboard/helper/CacheHelper/dispatchCacheRevalidations.js @@ -0,0 +1,36 @@ +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, +}; + +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 => revalidateModel(store, model, keys[model.name])) + ); diff --git a/app/javascript/dashboard/helper/ReconnectService.js b/app/javascript/dashboard/helper/ReconnectService.js index 12ada24bd..f2149a380 100644 --- a/app/javascript/dashboard/helper/ReconnectService.js +++ b/app/javascript/dashboard/helper/ReconnectService.js @@ -98,17 +98,6 @@ class ReconnectService { await this.store.dispatch('notifications/index', { ...filter, page: 1 }); }; - revalidateCaches = async () => { - const { label, inbox, team } = await this.store.dispatch( - 'accounts/getCacheKeys' - ); - await Promise.all([ - this.store.dispatch('labels/revalidate', { newKey: label }), - this.store.dispatch('inboxes/revalidate', { newKey: inbox }), - this.store.dispatch('teams/revalidate', { newKey: team }), - ]); - }; - handleRouteSpecificFetch = async () => { const currentRoute = this.router.currentRoute.value.name; if (isAConversationRoute(currentRoute, true)) { @@ -138,9 +127,11 @@ class ReconnectService { this.setConversationLastMessageId(); }; + // Cached workspace config needs no explicit revalidation here: ActionCable + // auto-resubscribes after a drop, and RoomChannel pushes the cache-key map + // on every subscribe via the account.cache_invalidated event. onReconnect = async () => { await this.handleRouteSpecificFetch(); - await this.revalidateCaches(); emitter.emit(BUS_EVENTS.WEBSOCKET_RECONNECT_COMPLETED); }; } diff --git a/app/javascript/dashboard/helper/actionCable.js b/app/javascript/dashboard/helper/actionCable.js index e6f2993e4..c7958b6f5 100644 --- a/app/javascript/dashboard/helper/actionCable.js +++ b/app/javascript/dashboard/helper/actionCable.js @@ -14,6 +14,7 @@ import { import { VOICE_CALL_PROVIDERS } from 'dashboard/helper/inbox'; import { VOICE_CALL_DIRECTION } from 'dashboard/components-next/message/constants'; import { FEATURE_FLAGS } from 'dashboard/featureFlags'; +import { dispatchCacheRevalidations } from './CacheHelper/dispatchCacheRevalidations'; const { isImpersonating } = useImpersonation(); const UNREAD_COUNTS_REFETCH_THROTTLE_MS = 5000; @@ -269,10 +270,7 @@ class ActionCableConnector extends BaseActionCableConnector { }; onCacheInvalidate = data => { - const keys = data.cache_keys; - this.app.$store.dispatch('labels/revalidate', { newKey: keys.label }); - this.app.$store.dispatch('inboxes/revalidate', { newKey: keys.inbox }); - this.app.$store.dispatch('teams/revalidate', { newKey: keys.team }); + dispatchCacheRevalidations(this.app.$store, data.cache_keys); }; onVoiceCallIncoming = data => { diff --git a/app/javascript/dashboard/helper/specs/CacheHelper/dispatchCacheRevalidations.spec.js b/app/javascript/dashboard/helper/specs/CacheHelper/dispatchCacheRevalidations.spec.js new file mode 100644 index 000000000..4439e5c7e --- /dev/null +++ b/app/javascript/dashboard/helper/specs/CacheHelper/dispatchCacheRevalidations.spec.js @@ -0,0 +1,86 @@ +import { dispatchCacheRevalidations } from '../../CacheHelper/dispatchCacheRevalidations'; +import InboxesAPI from 'dashboard/api/inboxes'; +import LabelsAPI from 'dashboard/api/labels'; +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(), + }, +})); + +describe('dispatchCacheRevalidations', () => { + 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', + }); + + 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 () => { + await dispatchCacheRevalidations(store); + + 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')); + TeamsAPI.validateCacheKey.mockResolvedValue(false); + TeamsAPI.refetchAndCommit.mockResolvedValue({ data: [] }); + TeamsAPI.extractDataFromResponse.mockReturnValue([{ id: 7 }]); + + await dispatchCacheRevalidations(store, { + inbox: 'inbox-key', + team: 'team-key', + }); + + expect(store.commit).toHaveBeenCalledWith('teams/SET_TEAMS', [{ id: 7 }]); + expect(store.commit).toHaveBeenCalledTimes(1); + }); +}); diff --git a/app/javascript/dashboard/helper/specs/ReconnectService.spec.js b/app/javascript/dashboard/helper/specs/ReconnectService.spec.js index 60bd825ee..608451d82 100644 --- a/app/javascript/dashboard/helper/specs/ReconnectService.spec.js +++ b/app/javascript/dashboard/helper/specs/ReconnectService.spec.js @@ -253,27 +253,6 @@ describe('ReconnectService', () => { }); }); - describe('revalidateCaches', () => { - it('should dispatch revalidate actions for labels, inboxes, and teams', async () => { - storeMock.dispatch.mockResolvedValueOnce({ - label: 'labelKey', - inbox: 'inboxKey', - team: 'teamKey', - }); - await reconnectService.revalidateCaches(); - expect(storeMock.dispatch).toHaveBeenCalledWith('accounts/getCacheKeys'); - expect(storeMock.dispatch).toHaveBeenCalledWith('labels/revalidate', { - newKey: 'labelKey', - }); - expect(storeMock.dispatch).toHaveBeenCalledWith('inboxes/revalidate', { - newKey: 'inboxKey', - }); - expect(storeMock.dispatch).toHaveBeenCalledWith('teams/revalidate', { - newKey: 'teamKey', - }); - }); - }); - describe('handleRouteSpecificFetch', () => { it('should fetch conversations and messages if current route is a conversation route', async () => { isAConversationRoute.mockReturnValue(true); @@ -335,12 +314,10 @@ describe('ReconnectService', () => { }); describe('onReconnect', () => { - it('should handle route-specific fetch, revalidate caches, and emit WEBSOCKET_RECONNECT_COMPLETED event', async () => { + it('should handle route-specific fetch and emit WEBSOCKET_RECONNECT_COMPLETED event', async () => { reconnectService.handleRouteSpecificFetch = vi.fn(); - reconnectService.revalidateCaches = vi.fn(); await reconnectService.onReconnect(); expect(reconnectService.handleRouteSpecificFetch).toHaveBeenCalled(); - expect(reconnectService.revalidateCaches).toHaveBeenCalled(); expect(emitter.emit).toHaveBeenCalledWith( BUS_EVENTS.WEBSOCKET_RECONNECT_COMPLETED ); diff --git a/app/javascript/dashboard/store/modules/accounts.js b/app/javascript/dashboard/store/modules/accounts.js index 68fd37010..a7521fd6f 100644 --- a/app/javascript/dashboard/store/modules/accounts.js +++ b/app/javascript/dashboard/store/modules/accounts.js @@ -163,10 +163,6 @@ export const actions = { commit(types.default.SET_ACCOUNT_UI_FLAG, { isFetchingLimits: false }); } }, - - getCacheKeys: async () => { - return AccountAPI.getCacheKeys(); - }, }; export const mutations = { diff --git a/app/javascript/dashboard/store/modules/inboxes.js b/app/javascript/dashboard/store/modules/inboxes.js index ce24ef653..1d4572ad5 100644 --- a/app/javascript/dashboard/store/modules/inboxes.js +++ b/app/javascript/dashboard/store/modules/inboxes.js @@ -189,17 +189,6 @@ const sendAnalyticsEvent = channelType => { }; export const actions = { - revalidate: async ({ commit }, { newKey }) => { - try { - const isExistingKeyValid = await InboxesAPI.validateCacheKey(newKey); - if (!isExistingKeyValid) { - const response = await InboxesAPI.refetchAndCommit(newKey); - commit(types.default.SET_INBOXES, response.data.payload); - } - } catch (error) { - // Ignore error - } - }, 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 658f212fd..36205f3c9 100644 --- a/app/javascript/dashboard/store/modules/labels.js +++ b/app/javascript/dashboard/store/modules/labels.js @@ -32,18 +32,6 @@ export const getters = { }; export const actions = { - revalidate: async function revalidate({ commit }, { newKey }) { - try { - const isExistingKeyValid = await LabelsAPI.validateCacheKey(newKey); - if (!isExistingKeyValid) { - const response = await LabelsAPI.refetchAndCommit(newKey); - commit(types.SET_LABELS, response.data.payload); - } - } catch (error) { - // Ignore error - } - }, - get: async function getLabels({ commit }) { commit(types.SET_LABEL_UI_FLAG, { isFetching: true }); try { diff --git a/app/javascript/dashboard/store/modules/specs/inboxes/actions.spec.js b/app/javascript/dashboard/store/modules/specs/inboxes/actions.spec.js index edbc3f764..6a2c557fa 100644 --- a/app/javascript/dashboard/store/modules/specs/inboxes/actions.spec.js +++ b/app/javascript/dashboard/store/modules/specs/inboxes/actions.spec.js @@ -1,12 +1,20 @@ import axios from 'axios'; import { actions } from '../../inboxes'; import * as types from '../../../mutation-types'; +import InboxesAPI from '../../../../api/inboxes'; import inboxList 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 InboxesAPI.dataManager.initDb(); + await InboxesAPI.dataManager.db.clear(InboxesAPI.cacheModelName); +}); + describe('#actions', () => { describe('#get', () => { it('sends correct actions if API is success', async () => { diff --git a/app/javascript/dashboard/store/modules/specs/labels/actions.spec.js b/app/javascript/dashboard/store/modules/specs/labels/actions.spec.js index 9ffefc276..141ede430 100644 --- a/app/javascript/dashboard/store/modules/specs/labels/actions.spec.js +++ b/app/javascript/dashboard/store/modules/specs/labels/actions.spec.js @@ -1,12 +1,20 @@ import axios from 'axios'; import { actions } from '../../labels'; import * as types from '../../../mutation-types'; +import LabelsAPI from '../../../../api/labels'; import labelsList 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 LabelsAPI.dataManager.initDb(); + await LabelsAPI.dataManager.db.clear(LabelsAPI.cacheModelName); +}); + describe('#actions', () => { describe('#get', () => { it('sends correct actions if API is success', async () => { diff --git a/app/javascript/dashboard/store/modules/specs/teams/actions.spec.js b/app/javascript/dashboard/store/modules/specs/teams/actions.spec.js index 7359f82b6..07e8ac6ab 100644 --- a/app/javascript/dashboard/store/modules/specs/teams/actions.spec.js +++ b/app/javascript/dashboard/store/modules/specs/teams/actions.spec.js @@ -2,18 +2,25 @@ import axios from 'axios'; import { actions } from '../../teams/actions'; import { SET_TEAM_UI_FLAG, - CLEAR_TEAMS, SET_TEAMS, SET_TEAM_ITEM, EDIT_TEAM, DELETE_TEAM, } from '../../teams/types'; +import TeamsAPI from '../../../../api/teams'; import teamsList 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 TeamsAPI.dataManager.initDb(); + await TeamsAPI.dataManager.db.clear(TeamsAPI.cacheModelName); +}); + describe('#actions', () => { describe('#get', () => { it('sends correct actions if API is success', async () => { @@ -33,7 +40,6 @@ describe('#actions', () => { await actions.get({ commit }); expect(commit.mock.calls).toEqual([ [SET_TEAM_UI_FLAG, { isFetching: true }], - [CLEAR_TEAMS], [SET_TEAMS, teamsList[1]], [SET_TEAM_UI_FLAG, { isFetching: false }], ]); diff --git a/app/javascript/dashboard/store/modules/specs/teams/mutations.spec.js b/app/javascript/dashboard/store/modules/specs/teams/mutations.spec.js index f20ea1df9..3a0cc2f7d 100644 --- a/app/javascript/dashboard/store/modules/specs/teams/mutations.spec.js +++ b/app/javascript/dashboard/store/modules/specs/teams/mutations.spec.js @@ -1,5 +1,4 @@ import { - CLEAR_TEAMS, SET_TEAMS, SET_TEAM_ITEM, EDIT_TEAM, @@ -11,10 +10,15 @@ describe('#mutations', () => { describe('#SET_teams', () => { it('set teams records', () => { const state = { records: {} }; - mutations[SET_TEAMS](state, [teams[1]]); - mutations[SET_TEAMS](state, [teams[2]]); + mutations[SET_TEAMS](state, [teams[1], teams[2]]); expect(state.records).toEqual(teams); }); + + it('drops records absent from the new list', () => { + const state = { records: { ...teams } }; + mutations[SET_TEAMS](state, [teams[1]]); + expect(state.records).toEqual({ 1: teams[1] }); + }); }); describe('#ADD_TEAM', () => { @@ -43,12 +47,4 @@ describe('#mutations', () => { expect(state.records).toEqual({}); }); }); - - describe('#CLEAR_TEAMS', () => { - it('delete teams record', () => { - const state = { records: { 1: teams[1] } }; - mutations[CLEAR_TEAMS](state); - expect(state.records).toEqual({}); - }); - }); }); diff --git a/app/javascript/dashboard/store/modules/teams/actions.js b/app/javascript/dashboard/store/modules/teams/actions.js index a8eab88b5..5509b56e9 100644 --- a/app/javascript/dashboard/store/modules/teams/actions.js +++ b/app/javascript/dashboard/store/modules/teams/actions.js @@ -1,6 +1,5 @@ import { SET_TEAM_UI_FLAG, - CLEAR_TEAMS, SET_TEAMS, SET_TEAM_ITEM, EDIT_TEAM, @@ -22,22 +21,10 @@ export const actions = { commit(SET_TEAM_UI_FLAG, { isCreating: false }); } }, - revalidate: async ({ commit }, { newKey }) => { - try { - const isExistingKeyValid = await TeamsAPI.validateCacheKey(newKey); - if (!isExistingKeyValid) { - const response = await TeamsAPI.refetchAndCommit(newKey); - commit(SET_TEAMS, response.data); - } - } catch (error) { - // Ignore error - } - }, get: async ({ commit }) => { commit(SET_TEAM_UI_FLAG, { isFetching: true }); try { const { data } = await TeamsAPI.get(true); - commit(CLEAR_TEAMS); commit(SET_TEAMS, data); } catch (error) { throw new Error(error); diff --git a/app/javascript/dashboard/store/modules/teams/mutations.js b/app/javascript/dashboard/store/modules/teams/mutations.js index 1c1c3fed2..fd27b252e 100644 --- a/app/javascript/dashboard/store/modules/teams/mutations.js +++ b/app/javascript/dashboard/store/modules/teams/mutations.js @@ -1,6 +1,5 @@ import { SET_TEAM_UI_FLAG, - CLEAR_TEAMS, SET_TEAMS, SET_TEAM_ITEM, EDIT_TEAM, @@ -15,19 +14,14 @@ export const mutations = { }; }, - [CLEAR_TEAMS]: $state => { - $state.records = {}; - }, - + // Replaces (not merges) so rows deleted server-side never survive as + // phantoms — SET_TEAMS only ever receives the full list. [SET_TEAMS]: ($state, data) => { - const updatedRecords = { ...$state.records }; + const records = {}; data.forEach(team => { - updatedRecords[team.id] = { - ...(updatedRecords[team.id] || {}), - ...team, - }; + records[team.id] = team; }); - $state.records = updatedRecords; + $state.records = records; }, [SET_TEAM_ITEM]: ($state, data) => { diff --git a/app/javascript/dashboard/store/modules/teams/types.js b/app/javascript/dashboard/store/modules/teams/types.js index 7c0a3e8f4..256031ebe 100644 --- a/app/javascript/dashboard/store/modules/teams/types.js +++ b/app/javascript/dashboard/store/modules/teams/types.js @@ -1,5 +1,4 @@ export const SET_TEAM_UI_FLAG = 'SET_TEAM_UI_FLAG'; -export const CLEAR_TEAMS = 'CLEAR_TEAMS'; export const SET_TEAMS = 'SET_TEAMS'; export const SET_TEAM_ITEM = 'SET_TEAM_ITEM'; export const EDIT_TEAM = 'EDIT_TEAM'; diff --git a/spec/controllers/api/v1/accounts_controller_spec.rb b/spec/controllers/api/v1/accounts_controller_spec.rb index d93503418..579114abf 100644 --- a/spec/controllers/api/v1/accounts_controller_spec.rb +++ b/spec/controllers/api/v1/accounts_controller_spec.rb @@ -216,14 +216,15 @@ RSpec.describe 'Accounts API', type: :request do expect(response.parsed_body['cache_keys'].keys).to match_array(%w[label inbox team]) end - it 'sets the appropriate cache headers' do + it 'does not allow cached cache key responses' do get "/api/v1/accounts/#{account.id}/cache_keys", headers: admin.create_new_auth_token, as: :json - expect(response.headers['Cache-Control']).to include('max-age=10') + expect(response.headers['Cache-Control']).to include('max-age=0') expect(response.headers['Cache-Control']).to include('private') - expect(response.headers['Cache-Control']).to include('stale-while-revalidate=300') + expect(response.headers['Cache-Control']).to include('must-revalidate') + expect(response.headers['Cache-Control']).not_to include('stale-while-revalidate') end end