test: caching behavior

app/finders/conversation_finder.rb

@@ -39,7 +39,8 @@ class ConversationFinder
   def perform
     set_up
 
-    mine_count, unassigned_count, all_count, = set_count_for_all_conversations
+    mine_count = compute_mine_count
+    unassigned_count, all_count = compute_unassigned_and_all_count
     assigned_count = all_count - unassigned_count
 
     filter_by_assignee_type
@@ -153,12 +154,54 @@ class ConversationFinder
     @conversations = @conversations.where(contact_inboxes: { source_id: params[:source_id] })
   end
 
-  def set_count_for_all_conversations
-    [
-      @conversations.assigned_to(current_user).count,
-      @conversations.unassigned.count,
-      @conversations.count
-    ]
+  def compute_mine_count
+    # Mine count is always computed fresh, never cached
+    @conversations.assigned_to(current_user).count
+  end
+
+  def compute_unassigned_and_all_count
+    return compute_unassigned_and_all_count_without_cache if additional_filters? || !should_cache?
+
+    cached_counts = ::Redis::Alfred.get(cache_key)
+    if cached_counts.present?
+      begin
+        return JSON.parse(cached_counts)
+      rescue JSON::ParserError
+        compute_unassigned_and_all_count_without_cache
+      end
+    end
+
+    counts = compute_unassigned_and_all_count_without_cache
+    ::Redis::Alfred.setex(cache_key, counts, cache_expiry(counts[0]))
+
+    counts
+  end
+
+  def compute_unassigned_and_all_count_without_cache
+    unassigned_count = @conversations.unassigned.count
+    all_count = @conversations.count
+    [unassigned_count, all_count]
+  end
+
+  def should_cache?
+    current_account.feature_enabled?('cache_meta_counts')
+  end
+
+  def cache_key
+    @cache_key ||= format(::Redis::Alfred::CONVERSATION_COUNTS, inbox_ids: @inbox_ids.sort.join('-'), status: params[:status] || DEFAULT_STATUS)
+  end
+
+  def cache_expiry(unassigned_count)
+    # these numbers are intentionally chosen to balance between cache hit rate and freshness
+    return 5.seconds if unassigned_count < 100
+    return 10.seconds if unassigned_count < 200
+    return 15.seconds if unassigned_count < 500
+
+    20.seconds
+  end
+
+  def additional_filters?
+    [:q, :conversation_type, :team_id, :labels, :source_id, :inbox_id].any? { |key| params[key].present? }
   end
 
   def current_page

app/javascript/dashboard/components/ChatList.vue

@@ -61,7 +61,7 @@ import {
   getUserPermissions,
   filterItemsByPermission,
 } from 'dashboard/helper/permissionsHelper.js';
-// import { matchesFilters } from '../store/modules/conversations/helpers/filterHelpers';
+import { matchesFilters } from '../store/modules/conversations/helpers/filterHelpers';
 import { CONVERSATION_EVENTS } from '../helper/AnalyticsHelper/events';
 import { ASSIGNEE_TYPE_TAB_PERMISSIONS } from 'dashboard/constants/permissions.js';
 
@@ -106,7 +106,7 @@ const advancedFilterTypes = ref(
 );
 
 const currentUser = useMapGetter('getCurrentUser');
-const chatLists = useMapGetter('getAllConversations');
+const chatLists = useMapGetter('getFilteredConversations');
 const mineChatsList = useMapGetter('getMineChats');
 const allChatList = useMapGetter('getAllStatusChats');
 const unAssignedChatsList = useMapGetter('getUnAssignedChats');
@@ -326,12 +326,12 @@ const conversationList = computed(() => {
     localConversationList = [...chatLists.value];
   }
 
-  // if (activeFolder.value) {
-  //   const { payload } = activeFolder.value.query;
-  //   localConversationList = localConversationList.filter(conversation => {
-  //     return matchesFilters(conversation, payload);
-  //   });
-  // }
+  if (activeFolder.value) {
+    const { payload } = activeFolder.value.query;
+    localConversationList = localConversationList.filter(conversation => {
+      return matchesFilters(conversation, payload);
+    });
+  }
 
   return localConversationList;
 });

app/javascript/dashboard/helper/ConversationMetaThrottleManager.js

@@ -1,29 +0,0 @@
-class ConversationMetaThrottleManager {
-  constructor() {
-    this.lastUpdatedTime = null;
-  }
-
-  shouldThrottle(threshold = 10000) {
-    if (!this.lastUpdatedTime) {
-      return false;
-    }
-
-    const currentTime = new Date().getTime();
-    const lastUpdatedTime = new Date(this.lastUpdatedTime).getTime();
-
-    if (currentTime - lastUpdatedTime < threshold) {
-      return true;
-    }
-    return false;
-  }
-
-  markUpdate() {
-    this.lastUpdatedTime = new Date();
-  }
-
-  reset() {
-    this.lastUpdatedTime = null;
-  }
-}
-
-export default new ConversationMetaThrottleManager();

app/javascript/dashboard/helper/specs/ConversationMetaThrottleManager.spec.js

@@ -1,34 +0,0 @@
-import ConversationMetaThrottleManager from '../ConversationMetaThrottleManager';
-
-describe('ConversationMetaThrottleManager', () => {
-  beforeEach(() => {
-    // Reset the lastUpdatedTime before each test
-    ConversationMetaThrottleManager.lastUpdatedTime = null;
-  });
-
-  describe('shouldThrottle', () => {
-    it('returns false when lastUpdatedTime is not set', () => {
-      expect(ConversationMetaThrottleManager.shouldThrottle()).toBe(false);
-    });
-
-    it('returns true when time difference is less than threshold', () => {
-      ConversationMetaThrottleManager.markUpdate();
-      expect(ConversationMetaThrottleManager.shouldThrottle()).toBe(true);
-    });
-
-    it('returns false when time difference is more than threshold', () => {
-      ConversationMetaThrottleManager.lastUpdatedTime = new Date(
-        Date.now() - 11000
-      );
-      expect(ConversationMetaThrottleManager.shouldThrottle()).toBe(false);
-    });
-
-    it('respects custom threshold value', () => {
-      ConversationMetaThrottleManager.lastUpdatedTime = new Date(
-        Date.now() - 5000
-      );
-      expect(ConversationMetaThrottleManager.shouldThrottle(3000)).toBe(false);
-      expect(ConversationMetaThrottleManager.shouldThrottle(6000)).toBe(true);
-    });
-  });
-});

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

@@ -1,40 +1,28 @@
 import types from '../mutation-types';
 import ConversationApi from '../../api/inbox/conversation';
 
-import ConversationMetaThrottleManager from 'dashboard/helper/ConversationMetaThrottleManager';
-
 const state = {
   mineCount: 0,
   unAssignedCount: 0,
   allCount: 0,
+  updatedOn: null,
 };
 
 export const getters = {
   getStats: $state => $state,
 };
 
-export const shouldThrottle = conversationCount => {
-  // The threshold for throttling is different for normal users and large accounts
-  // Normal users: 2 seconds
-  // Large accounts: 10 seconds
-  // We would only update the conversation stats based on the threshold above.
-  // This is done to reduce the number of /meta request made to the server.
-  const NORMAL_USER_THRESHOLD = 500;
-  const LARGE_ACCOUNT_THRESHOLD = 10000;
-
-  const threshold =
-    conversationCount > 100 ? LARGE_ACCOUNT_THRESHOLD : NORMAL_USER_THRESHOLD;
-  return ConversationMetaThrottleManager.shouldThrottle(threshold);
-};
-
 export const actions = {
   get: async ({ commit, state: $state }, params) => {
-    if (shouldThrottle($state.allCount)) {
+    const currentTime = new Date();
+    const lastUpdatedTime = new Date($state.updatedOn);
+
+    // Skip large accounts from making too many requests
+    if (currentTime - lastUpdatedTime < 10000 && $state.allCount > 100) {
       // eslint-disable-next-line no-console
-      console.warn('Throttle /meta fetch, will resume after threshold');
+      console.warn('Skipping conversation meta fetch');
       return;
     }
-    ConversationMetaThrottleManager.markUpdate();
 
     try {
       const response = await ConversationApi.meta(params);

app/javascript/dashboard/store/modules/specs/conversationStats/helper.spec.js

@@ -1,37 +0,0 @@
-import { describe, it, expect, vi, beforeEach } from 'vitest';
-import ConversationMetaThrottleManager from 'dashboard/helper/ConversationMetaThrottleManager';
-import { shouldThrottle } from '../../conversationStats';
-
-vi.mock('dashboard/helper/ConversationMetaThrottleManager', () => ({
-  default: {
-    shouldThrottle: vi.fn(),
-  },
-}));
-
-describe('shouldThrottle', () => {
-  beforeEach(() => {
-    vi.clearAllMocks();
-  });
-
-  it('uses normal threshold for accounts with 100 or fewer conversations', () => {
-    shouldThrottle(100);
-    expect(ConversationMetaThrottleManager.shouldThrottle).toHaveBeenCalledWith(
-      2000
-    );
-  });
-
-  it('uses large account threshold for accounts with more than 100 conversations', () => {
-    shouldThrottle(101);
-    expect(ConversationMetaThrottleManager.shouldThrottle).toHaveBeenCalledWith(
-      10000
-    );
-  });
-
-  it('returns the throttle value from ConversationMetaThrottleManager', () => {
-    ConversationMetaThrottleManager.shouldThrottle.mockReturnValue(true);
-    expect(shouldThrottle(50)).toBe(true);
-
-    ConversationMetaThrottleManager.shouldThrottle.mockReturnValue(false);
-    expect(shouldThrottle(150)).toBe(false);
-  });
-});

app/javascript/widget/assets/scss/woot.scss

@@ -10,7 +10,7 @@
 
 html,
 body {
-  @apply antialiased h-full;
+  @apply antialiased h-full bg-n-background;
 }
 
 .is-mobile {

app/models/account.rb

@@ -56,7 +56,6 @@ class Account < ApplicationRecord
   has_many :data_imports, dependent: :destroy_async
   has_many :email_channels, dependent: :destroy_async, class_name: '::Channel::Email'
   has_many :facebook_pages, dependent: :destroy_async, class_name: '::Channel::FacebookPage'
-  has_many :instagram_channels, dependent: :destroy_async, class_name: '::Channel::Instagram'
   has_many :hooks, dependent: :destroy_async, class_name: 'Integrations::Hook'
   has_many :inboxes, dependent: :destroy_async
   has_many :labels, dependent: :destroy_async

app/models/channel/instagram.rb

@@ -1,28 +0,0 @@
-# == Schema Information
-#
-# Table name: channel_instagram
-#
-#  id           :bigint           not null, primary key
-#  access_token :string           not null
-#  expires_at   :datetime         not null
-#  created_at   :datetime         not null
-#  updated_at   :datetime         not null
-#  account_id   :integer          not null
-#  instagram_id :string           not null
-#
-# Indexes
-#
-#  index_channel_instagram_on_instagram_id  (instagram_id) UNIQUE
-#
-class Channel::Instagram < ApplicationRecord
-  include Channelable
-
-  self.table_name = 'channel_instagram'
-
-  validates :access_token, presence: true
-  validates :instagram_id, uniqueness: true, presence: true
-
-  def name
-    'Instagram'
-  end
-end

config/features.yml

@@ -161,3 +161,6 @@
 - name: search_with_gin
   display_name: Search messages with GIN
   enabled: false
+- name: cache_meta_counts
+  display_name: Cache Meta Counts
+  enabled: false

db/migrate/20250326034635_add_instagram_channel.rb

@@ -1,13 +0,0 @@
-class AddInstagramChannel < ActiveRecord::Migration[7.0]
-  def change
-    create_table :channel_instagram do |t|
-      t.string :access_token, null: false
-      t.datetime :expires_at, null: false
-      t.integer :account_id, null: false
-      t.string :instagram_id, null: false
-      t.timestamps
-    end
-
-    add_index :channel_instagram, :instagram_id, unique: true
-  end
-end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2025_03_26_034635) do
+ActiveRecord::Schema[7.0].define(version: 2025_03_15_202035) do
   # These extensions should be enabled to support this database
   enable_extension "pg_stat_statements"
   enable_extension "pg_trgm"
@@ -377,16 +377,6 @@ ActiveRecord::Schema[7.0].define(version: 2025_03_26_034635) do
     t.index ["page_id"], name: "index_channel_facebook_pages_on_page_id"
   end
 
-  create_table "channel_instagram", force: :cascade do |t|
-    t.string "access_token", null: false
-    t.datetime "expires_at", null: false
-    t.integer "account_id", null: false
-    t.string "instagram_id", null: false
-    t.datetime "created_at", null: false
-    t.datetime "updated_at", null: false
-    t.index ["instagram_id"], name: "index_channel_instagram_on_instagram_id", unique: true
-  end
-
   create_table "channel_line", force: :cascade do |t|
     t.integer "account_id", null: false
     t.string "line_channel_id", null: false

lib/redis/redis_keys.rb

@@ -41,4 +41,7 @@ module Redis::RedisKeys
   IG_MESSAGE_MUTEX = 'IG_MESSAGE_CREATE_LOCK::%<sender_id>s::%<ig_account_id>s'.freeze
   SLACK_MESSAGE_MUTEX = 'SLACK_MESSAGE_LOCK::%<conversation_id>s::%<reference_id>s'.freeze
   EMAIL_MESSAGE_MUTEX = 'EMAIL_CHANNEL_LOCK::%<inbox_id>s'.freeze
+
+  # Cache for storing conversation meta counts
+  CONVERSATION_COUNTS = 'CONVERSATION::COUNTS::%<inbox_ids>s::%<status>s'.freeze
 end

spec/factories/channel/channel_instagram.rb

@@ -1,13 +0,0 @@
-FactoryBot.define do
-  factory :channel_instagram, class: 'Channel::Instagram' do
-    account
-    access_token { SecureRandom.hex(32) }
-    instagram_id { SecureRandom.hex(16) }
-    expires_at { 60.days.from_now }
-    updated_at { 25.hours.ago }
-
-    after(:create) do |channel|
-      create(:inbox, channel: channel, account: channel.account)
-    end
-  end
-end

spec/finders/conversation_finder_spec.rb

@@ -20,6 +20,7 @@ describe ConversationFinder do
     create(:conversation, account: account, inbox: inbox, assignee: user_2, contact_inbox: contact_inbox)
     # unassigned conversation
     create(:conversation, account: account, inbox: inbox)
+    account.disable_features!('cache_meta_counts')
     Current.account = account
   end
 
@@ -193,5 +194,81 @@ describe ConversationFinder do
         expect(result[:conversations].length).to be 2
       end
     end
+
+    describe 'caching behavior' do
+      let(:params) { { status: 'open' } }
+      let(:cache_key) { format(Redis::Alfred::CONVERSATION_COUNTS, inbox_ids: inbox.id, status: 'open') }
+
+      before do
+        account.enable_features!('cache_meta_counts')
+      end
+
+      context 'when should_cache? returns true' do
+        before do
+          Redis::Alfred.delete(cache_key) if cache_key
+        end
+
+        it 'caches the unassigned and all counts' do
+          expect(Redis::Alfred).to receive(:setex).with(
+            cache_key,
+            [1, 4], # unassigned_count and all_count
+            instance_of(Integer)
+          )
+
+          conversation_finder.perform
+        end
+
+        it 'uses cached counts on subsequent calls' do
+          # First call should cache the counts
+          first_result = conversation_finder.perform
+
+          # Create a new conversation that shouldn't be counted due to caching
+          create(:conversation, account: account, inbox: inbox)
+
+          # Second call should use cached counts
+          second_result = conversation_finder.perform
+
+          expect(second_result[:count][:unassigned_count]).to eq(first_result[:count][:unassigned_count])
+          expect(second_result[:count][:all_count]).to eq(first_result[:count][:all_count])
+        end
+
+        it 'always computes mine_count fresh' do
+          # First call
+          first_result = conversation_finder.perform
+
+          # Create a new conversation assigned to user_1
+          create(:conversation, account: account, inbox: inbox, assignee: user_1)
+
+          # Second call should have updated mine_count
+          second_result = conversation_finder.perform
+
+          expect(second_result[:count][:mine_count]).to eq(first_result[:count][:mine_count] + 1)
+        end
+      end
+
+      context 'when has_additional_filters? returns true' do
+        let(:params) { { status: 'open', labels: ['test'] } }
+
+        it 'does not use cache' do
+          expect(Redis::Alfred).not_to receive(:get)
+          expect(Redis::Alfred).not_to receive(:setex)
+
+          conversation_finder.perform
+        end
+      end
+
+      context 'when should_cache? returns false' do
+        before do
+          account.disable_features!('cache_meta_counts')
+        end
+
+        it 'does not use cache' do
+          expect(Redis::Alfred).not_to receive(:get)
+          expect(Redis::Alfred).not_to receive(:setex)
+
+          conversation_finder.perform
+        end
+      end
+    end
   end
 end

spec/models/channel/instagram_spec.rb

@@ -1,17 +0,0 @@
-# frozen_string_literal: true
-
-require 'rails_helper'
-
-RSpec.describe Channel::Instagram do
-  let(:channel) { create(:channel_instagram) }
-
-  it { is_expected.to validate_presence_of(:account_id) }
-  it { is_expected.to validate_presence_of(:access_token) }
-  it { is_expected.to validate_presence_of(:instagram_id) }
-  it { is_expected.to belong_to(:account) }
-  it { is_expected.to have_one(:inbox).dependent(:destroy_async) }
-
-  it 'has a valid name' do
-    expect(channel.name).to eq('Instagram')
-  end
-end