feat: cache canned responses in the browser (#15401)

app/controllers/api/v1/accounts_controller.rb

@@ -1,6 +1,5 @@
 class Api::V1::AccountsController < Api::BaseController
   include AuthHelper
-  include CacheKeysHelper
 
   skip_before_action :authenticate_user!, :set_current_user, :handle_with_exception,
                      only: [:create], raise: false
@@ -52,7 +51,7 @@ class Api::V1::AccountsController < Api::BaseController
 
   def cache_keys
     expires_in 10.seconds, public: false, stale_while_revalidate: 5.minutes
-    render json: { cache_keys: cache_keys_for_account }, status: :ok
+    render json: { cache_keys: @account.cache_keys }, status: :ok
   end
 
   def update
@@ -97,14 +96,6 @@ class Api::V1::AccountsController < Api::BaseController
     raise CustomExceptions::Account::InvalidParams.new({})
   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)
-    }
-  end
-
   def fetch_account
     @account = current_user.accounts.find(params[:id])
     @current_account_user = @account.account_users.find_by(user_id: current_user.id)

app/javascript/dashboard/api/CacheEnabledApiClient.js

@@ -5,7 +5,22 @@ import ApiClient from './ApiClient';
 class CacheEnabledApiClient extends ApiClient {
   constructor(resource, options = {}) {
     super(resource, options);
-    this.dataManager = new DataManager(this.accountIdFromRoute);
+    this.accountDataManager = null;
+  }
+
+  // These clients are module level singletons, so they are constructed before the router
+  // has settled on an account: a boot at /app redirects to /app/accounts/:id without
+  // re-evaluating the bundle. Resolving the store once in the constructor therefore pointed
+  // every account at a single `cw-store-` database, where a shared default cache key could
+  // hand one account another's rows. The account has to come from the route at read time.
+  get dataManager() {
+    const accountId = this.accountIdFromRoute;
+
+    if (this.accountDataManager?.accountId !== accountId) {
+      this.accountDataManager = new DataManager(accountId);
+    }
+
+    return this.accountDataManager;
   }
 
   // eslint-disable-next-line class-methods-use-this
@@ -69,7 +84,7 @@ class CacheEnabledApiClient extends ApiClient {
     try {
       await this.dataManager.initDb();
 
-      this.dataManager.replace({
+      await this.dataManager.replace({
         modelName: this.cacheModelName,
         data: this.extractDataFromResponse(response),
       });

app/javascript/dashboard/api/cannedResponse.js

@@ -1,17 +1,24 @@
-/* global axios */
+import CacheEnabledApiClient from './CacheEnabledApiClient';
 
-import ApiClient from './ApiClient';
-
-class CannedResponse extends ApiClient {
+class CannedResponse extends CacheEnabledApiClient {
   constructor() {
     super('canned_responses', { accountScoped: true });
   }
 
-  get({ searchKey, signal } = {}) {
-    return axios.get(this.url, {
-      params: searchKey ? { search: searchKey } : undefined,
-      signal,
-    });
+  // eslint-disable-next-line class-methods-use-this
+  get cacheModelName() {
+    return 'canned_response';
+  }
+
+  // The index endpoint returns a bare array instead of a payload wrapper
+  // 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 };
   }
 }
 

app/javascript/dashboard/api/specs/CacheEnabledApiClient.spec.js

@@ -0,0 +1,223 @@
+// The subclasses below are test doubles standing in for the two response shapes the
+// cached clients have to support
+/* eslint-disable max-classes-per-file */
+import axios from 'axios';
+import { deleteDB } from 'idb';
+import CacheEnabledApiClient from '../CacheEnabledApiClient';
+
+global.axios = axios;
+vi.mock('axios');
+
+const ACCOUNT_ID = '7';
+
+// Most cached resources answer with a `payload` wrapper
+class WrappedClient extends CacheEnabledApiClient {
+  constructor() {
+    super('inboxes', { accountScoped: true });
+  }
+
+  // eslint-disable-next-line class-methods-use-this
+  get cacheModelName() {
+    return 'inbox';
+  }
+}
+
+// Canned responses answer with a bare array, so they override both hooks. The cached read
+// has to hand back the same shape as the network read or callers break on a cache hit.
+class BareArrayClient extends CacheEnabledApiClient {
+  constructor() {
+    super('canned_responses', { accountScoped: true });
+  }
+
+  // eslint-disable-next-line class-methods-use-this
+  get cacheModelName() {
+    return 'canned_response';
+  }
+
+  // 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 };
+  }
+}
+
+describe('CacheEnabledApiClient', () => {
+  const inboxes = [
+    { id: 1, name: 'inbox-1' },
+    { id: 2, name: 'inbox-2' },
+  ];
+  const cannedResponses = [{ id: 1, short_code: 'hello', content: 'Hi there' }];
+
+  let openClients = [];
+
+  const stubEndpoints = ({ cacheKeys, payload }) => {
+    axios.get.mockImplementation(url =>
+      url.includes('cache_keys')
+        ? Promise.resolve({ data: { cache_keys: cacheKeys } })
+        : Promise.resolve({ data: payload })
+    );
+  };
+
+  const listRequests = () =>
+    axios.get.mock.calls.filter(([url]) => !url.includes('cache_keys'));
+
+  const buildClient = Klass => {
+    const client = new Klass();
+    openClients.push(client);
+    return client;
+  };
+
+  beforeEach(async () => {
+    window.history.pushState({}, '', `/app/accounts/${ACCOUNT_ID}/dashboard`);
+    await deleteDB(`cw-store-${ACCOUNT_ID}`);
+    openClients = [];
+  });
+
+  afterEach(() => {
+    openClients.forEach(client => client.dataManager.db?.close());
+  });
+
+  it('throws when a subclass does not declare a cache model', () => {
+    class Incomplete extends CacheEnabledApiClient {}
+
+    expect(() => buildClient(Incomplete).cacheModelName).toThrow();
+  });
+
+  it('does not serve one account its data to another account', async () => {
+    // The bundle boots at /app and the router redirects into an account without
+    // re-evaluating it, so the singleton client outlives the account in the route
+    window.history.pushState({}, '', '/app');
+    const client = buildClient(BareArrayClient);
+    await deleteDB('cw-store-');
+    await deleteDB('cw-store-9');
+
+    // A never-written cache key is the same '0000000000' for every account
+    const sharedKey = '0000000000';
+    stubEndpoints({
+      cacheKeys: { canned_response: sharedKey },
+      payload: cannedResponses,
+    });
+    window.history.pushState({}, '', `/app/accounts/${ACCOUNT_ID}/dashboard`);
+    await client.get(true);
+    client.dataManager.db?.close();
+
+    const otherAccountResponses = [
+      { id: 99, short_code: 'other-account', content: 'Should stay private' },
+    ];
+    window.history.pushState({}, '', '/app/accounts/9/dashboard');
+    axios.get.mockClear();
+    stubEndpoints({
+      cacheKeys: { canned_response: sharedKey },
+      payload: otherAccountResponses,
+    });
+    const response = await client.get(true);
+
+    expect(response.data).toEqual(otherAccountResponses);
+    expect(listRequests()).toHaveLength(1);
+  });
+
+  it('skips the cache entirely when asked for a network read', async () => {
+    stubEndpoints({
+      cacheKeys: { inbox: 'key-1' },
+      payload: { payload: inboxes },
+    });
+    const client = buildClient(WrappedClient);
+
+    const response = await client.get(false);
+
+    expect(response.data.payload).toEqual(inboxes);
+    // No cache key lookup either, so the request count is exactly one
+    expect(axios.get).toHaveBeenCalledTimes(1);
+  });
+
+  it('fetches from the network and stores locally when the cache is cold', async () => {
+    stubEndpoints({
+      cacheKeys: { inbox: 'key-1' },
+      payload: { payload: inboxes },
+    });
+    const client = buildClient(WrappedClient);
+
+    const response = await client.get(true);
+
+    expect(response.data.payload).toEqual(inboxes);
+    expect(listRequests()).toHaveLength(1);
+    expect(await client.dataManager.get({ modelName: 'inbox' })).toEqual(
+      inboxes
+    );
+    expect(await client.dataManager.getCacheKey('inbox')).toBe('key-1');
+  });
+
+  it('serves local data without a list request while the cache key is unchanged', async () => {
+    stubEndpoints({
+      cacheKeys: { inbox: 'key-1' },
+      payload: { payload: inboxes },
+    });
+    const client = buildClient(WrappedClient);
+    await client.get(true);
+
+    axios.get.mockClear();
+    const response = await client.get(true);
+
+    expect(response.data.payload).toEqual(inboxes);
+    expect(listRequests()).toHaveLength(0);
+  });
+
+  it('refetches and overwrites local data once the cache key moves', async () => {
+    stubEndpoints({
+      cacheKeys: { inbox: 'key-1' },
+      payload: { payload: inboxes },
+    });
+    const client = buildClient(WrappedClient);
+    await client.get(true);
+
+    const renamed = [{ id: 1, name: 'inbox-1-renamed' }];
+    axios.get.mockClear();
+    stubEndpoints({
+      cacheKeys: { inbox: 'key-2' },
+      payload: { payload: renamed },
+    });
+    const response = await client.get(true);
+
+    expect(response.data.payload).toEqual(renamed);
+    expect(listRequests()).toHaveLength(1);
+    expect(await client.dataManager.get({ modelName: 'inbox' })).toEqual(
+      renamed
+    );
+    expect(await client.dataManager.getCacheKey('inbox')).toBe('key-2');
+  });
+
+  it('returns the same shape from the cache as from the network for a bare array resource', async () => {
+    stubEndpoints({
+      cacheKeys: { canned_response: 'key-1' },
+      payload: cannedResponses,
+    });
+    const client = buildClient(BareArrayClient);
+
+    const fromNetwork = await client.get(true);
+    axios.get.mockClear();
+    const fromCache = await client.get(true);
+
+    expect(fromNetwork.data).toEqual(cannedResponses);
+    expect(fromCache.data).toEqual(cannedResponses);
+    expect(listRequests()).toHaveLength(0);
+  });
+
+  it('falls back to the network when IndexedDB is unavailable', async () => {
+    stubEndpoints({
+      cacheKeys: { inbox: 'key-1' },
+      payload: { payload: inboxes },
+    });
+    const client = buildClient(WrappedClient);
+    vi.spyOn(client.dataManager, 'initDb').mockRejectedValue(
+      new Error('IndexedDB is disabled in private mode')
+    );
+
+    const response = await client.get(true);
+
+    expect(response.data.payload).toEqual(inboxes);
+  });
+});

app/javascript/dashboard/components/widgets/conversation/CannedResponse.vue

@@ -1,9 +1,8 @@
 <script setup>
-import { computed, ref, watch, onMounted } from 'vue';
+import { computed, ref, onMounted } from 'vue';
 import { useI18n } from 'vue-i18n';
-import { useTimeoutFn } from '@vueuse/core';
+import { picoSearch } from '@scmmishra/pico-search';
 import { useStore, useMapGetter } from 'dashboard/composables/store';
-import { useAbortableRequest } from 'dashboard/composables/useAbortableRequest';
 import {
   resolveVariablesInMessage,
   stripUnsupportedFormatting,
@@ -34,7 +33,6 @@ const emit = defineEmits(['replace', 'close', 'removeTrigger']);
 
 // Characters kept before the match when a snippet has to skip ahead
 const SNIPPET_LEAD = 24;
-const SEARCH_DEBOUNCE = 200;
 const HIGHLIGHT_CLASS = 'text-n-blue-text';
 
 const store = useStore();
@@ -42,6 +40,7 @@ const { t } = useI18n();
 const { getPlainText, formatMessage, highlightContent } = useMessageFormatter();
 
 const cannedResponses = useMapGetter('getCannedResponses');
+const uiFlags = useMapGetter('getUIFlags');
 // The trigger can already be followed by text, from a draft or a caret moved back onto it
 const searchQuery = ref(props.searchKey);
 
@@ -86,8 +85,17 @@ const records = computed(() =>
   })
 );
 
+const filteredRecords = computed(() => {
+  if (!searchTerm.value) return records.value;
+
+  return picoSearch(records.value, searchTerm.value, [
+    { name: 'shortCode', weight: 1 },
+    'plainText',
+  ]);
+});
+
 const items = computed(() =>
-  records.value.map(record => ({
+  filteredRecords.value.map(record => ({
     id: record.id,
     content: record.content,
     resolved: record.resolved,
@@ -99,28 +107,7 @@ const items = computed(() =>
 
 const onSelect = item => emit('replace', item.content);
 
-const { run: runFetch, isPending: isFetching } = useAbortableRequest();
-
-const fetchCannedResponses = () => {
-  runFetch(signal =>
-    store.dispatch('getCannedResponse', {
-      searchKey: searchTerm.value,
-      signal,
-    })
-  );
-};
-
-const { start: scheduleFetch, isPending: isDebouncing } = useTimeoutFn(
-  fetchCannedResponses,
-  SEARCH_DEBOUNCE,
-  { immediate: false }
-);
-
-const isLoading = computed(() => isFetching.value || isDebouncing.value);
-
-watch(searchTerm, scheduleFetch);
-
-onMounted(fetchCannedResponses);
+onMounted(() => store.dispatch('getCannedResponse'));
 </script>
 
 <template>
@@ -129,7 +116,7 @@ onMounted(fetchCannedResponses);
     :caret-position="caretPosition"
     :items="items"
     :search-placeholder="t('COMBOBOX.SEARCH_PLACEHOLDER')"
-    :is-loading="isLoading"
+    :is-loading="uiFlags.fetchingList"
     :empty-label="
       searchTerm
         ? t('COMBOBOX.EMPTY_SEARCH_RESULTS', { searchTerm })

app/javascript/dashboard/helper/CacheHelper/DataManager.js

@@ -3,7 +3,7 @@ import { DATA_VERSION } from './version';
 
 export class DataManager {
   constructor(accountId) {
-    this.modelsToSync = ['inbox', 'label', 'team'];
+    this.modelsToSync = ['inbox', 'label', 'team', 'canned_response'];
     this.accountId = accountId;
     this.db = null;
   }
@@ -13,10 +13,18 @@ export class DataManager {
     const dbName = `cw-store-${this.accountId}`;
     this.db = await openDB(`cw-store-${this.accountId}`, DATA_VERSION, {
       upgrade(db) {
-        db.createObjectStore('cache-keys');
-        db.createObjectStore('inbox', { keyPath: 'id' });
-        db.createObjectStore('label', { keyPath: 'id' });
-        db.createObjectStore('team', { keyPath: 'id' });
+        // Existing databases already carry the stores added in earlier versions,
+        // and createObjectStore throws on a name that is already taken.
+        const createStore = (name, options) => {
+          if (db.objectStoreNames.contains(name)) return;
+          db.createObjectStore(name, options);
+        };
+
+        createStore('cache-keys');
+        createStore('inbox', { keyPath: 'id' });
+        createStore('label', { keyPath: 'id' });
+        createStore('team', { keyPath: 'id' });
+        createStore('canned_response', { keyPath: 'id' });
       },
     });
 
@@ -41,7 +49,7 @@ export class DataManager {
   async replace({ modelName, data }) {
     this.validateModel(modelName);
 
-    this.db.clear(modelName);
+    await this.db.clear(modelName);
     return this.push({ modelName, data });
   }
 

app/javascript/dashboard/helper/CacheHelper/version.js

@@ -1,3 +1,3 @@
-// Monday, 13 March 2023
+// Monday, 10 August 2026
 // Change this version if you want to invalidate old data
-export const DATA_VERSION = '1678706392';
+export const DATA_VERSION = '1786233600';

app/javascript/dashboard/helper/actionCable.js

@@ -347,6 +347,9 @@ class ActionCableConnector extends BaseActionCableConnector {
     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 });
+    this.app.$store.dispatch('revalidateCannedResponses', {
+      newKey: keys.canned_response,
+    });
 
     if (this.isFilteredUnreadCountsEnabled()) {
       // Inbox/team/label visibility changes can change the accessible set used

app/javascript/dashboard/helper/specs/CacheHelper/DataManger.spec.js

@@ -1,3 +1,4 @@
+import { openDB, deleteDB } from 'idb';
 import { DataManager } from '../../CacheHelper/DataManager';
 
 describe('DataManager', () => {
@@ -30,6 +31,33 @@ describe('DataManager', () => {
       const db2 = await dataManager.initDb();
       expect(db1).toBe(db2);
     });
+
+    it('should add new stores to a database left behind by an earlier version', async () => {
+      const legacyAccountId = 'legacy-account';
+      const dbName = `cw-store-${legacyAccountId}`;
+      await deleteDB(dbName);
+
+      // The schema as it shipped before canned responses joined the cached models
+      const legacyDb = await openDB(dbName, 1, {
+        upgrade(db) {
+          db.createObjectStore('cache-keys');
+          db.createObjectStore('inbox', { keyPath: 'id' });
+          db.createObjectStore('label', { keyPath: 'id' });
+          db.createObjectStore('team', { keyPath: 'id' });
+        },
+      });
+      await legacyDb.put('cache-keys', 'existing-key', 'inbox');
+      legacyDb.close();
+
+      const legacyManager = new DataManager(legacyAccountId);
+      await legacyManager.initDb();
+
+      expect([...legacyManager.db.objectStoreNames]).toContain(
+        'canned_response'
+      );
+      expect(await legacyManager.getCacheKey('inbox')).toBe('existing-key');
+      legacyManager.db.close();
+    });
   });
 
   describe('validateModel', () => {
@@ -66,6 +94,22 @@ describe('DataManager', () => {
       const result = await dataManager.get({ modelName: 'inbox' });
       expect(result).toEqual(newData);
     });
+
+    it('should replace data whose keys overlap the existing rows', async () => {
+      const inboxData = [
+        { id: 1, name: 'inbox-1' },
+        { id: 2, name: 'inbox-2' },
+      ];
+      const newData = [
+        { id: 1, name: 'inbox-1-renamed' },
+        { id: 2, name: 'inbox-2-renamed' },
+      ];
+
+      await dataManager.push({ modelName: 'inbox', data: inboxData });
+      await dataManager.replace({ modelName: 'inbox', data: newData });
+      const result = await dataManager.get({ modelName: 'inbox' });
+      expect(result).toEqual(newData);
+    });
   });
 
   describe('push', () => {

app/javascript/dashboard/store/modules/cannedResponse.js

@@ -33,13 +33,26 @@ const getters = {
 };
 
 const actions = {
-  getCannedResponse: async function getCannedResponse(
+  revalidateCannedResponses: async function revalidateCannedResponses(
     { commit },
-    { searchKey, signal } = {}
+    { newKey }
   ) {
+    try {
+      const isExistingKeyValid =
+        await CannedResponseAPI.validateCacheKey(newKey);
+      if (!isExistingKeyValid) {
+        const response = await CannedResponseAPI.refetchAndCommit(newKey);
+        commit(types.default.SET_CANNED, response.data);
+      }
+    } catch (error) {
+      // Ignore error
+    }
+  },
+
+  getCannedResponse: async function getCannedResponse({ commit }) {
     commit(types.default.SET_CANNED_UI_FLAG, { fetchingList: true });
     try {
-      const response = await CannedResponseAPI.get({ searchKey, signal });
+      const response = await CannedResponseAPI.get(true);
       commit(types.default.SET_CANNED, response.data);
       commit(types.default.SET_CANNED_UI_FLAG, { fetchingList: false });
     } catch (error) {

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

app/models/concerns/cache_keys.rb

@@ -8,7 +8,7 @@ module CacheKeys
 
   included do
     class_attribute :cacheable_models
-    self.cacheable_models = [Label, Inbox, Team]
+    self.cacheable_models = [Label, Inbox, Team, CannedResponse]
   end
 
   def cache_keys