diff --git a/app/controllers/api/v1/accounts/conversations/participants_controller.rb b/app/controllers/api/v1/accounts/conversations/participants_controller.rb index ebd02380f..7569142b2 100644 --- a/app/controllers/api/v1/accounts/conversations/participants_controller.rb +++ b/app/controllers/api/v1/accounts/conversations/participants_controller.rb @@ -1,27 +1,40 @@ class Api::V1::Accounts::Conversations::ParticipantsController < Api::V1::Accounts::Conversations::BaseController + include Events::Types + def show @participants = @conversation.conversation_participants end def create + participant_ids_to_add = participants_to_be_added_ids + ActiveRecord::Base.transaction do - @participants = participants_to_be_added_ids.map { |user_id| @conversation.conversation_participants.find_or_create_by(user_id: user_id) } + @participants = participant_ids_to_add.map { |user_id| @conversation.conversation_participants.find_or_create_by(user_id: user_id) } end + notify_unread_count_change if participant_ids_to_add.any? end def update + participant_ids_to_add = participants_to_be_added_ids + participant_ids_to_remove = participants_to_be_removed_ids + changed_participant_ids = participant_ids_to_add + participant_ids_to_remove + ActiveRecord::Base.transaction do - participants_to_be_added_ids.each { |user_id| @conversation.conversation_participants.find_or_create_by(user_id: user_id) } - participants_to_be_removed_ids.each { |user_id| @conversation.conversation_participants.find_by(user_id: user_id)&.destroy } + participant_ids_to_add.each { |user_id| @conversation.conversation_participants.find_or_create_by(user_id: user_id) } + participant_ids_to_remove.each { |user_id| @conversation.conversation_participants.find_by(user_id: user_id)&.destroy } end + notify_unread_count_change if changed_participant_ids.any? @participants = @conversation.conversation_participants render action: 'show' end def destroy + participant_ids_to_remove = current_participant_ids & params[:user_ids] + ActiveRecord::Base.transaction do params[:user_ids].map { |user_id| @conversation.conversation_participants.find_by(user_id: user_id)&.destroy } end + notify_unread_count_change if participant_ids_to_remove.any? head :ok end @@ -38,4 +51,11 @@ class Api::V1::Accounts::Conversations::ParticipantsController < Api::V1::Accoun def current_participant_ids @current_participant_ids ||= @conversation.conversation_participants.pluck(:user_id) end + + def notify_unread_count_change + return unless Current.account.feature_enabled?('conversation_unread_counts') + return unless Current.account.feature_enabled?('unread_count_for_filters') + + Rails.configuration.dispatcher.dispatch(CONVERSATION_UNREAD_COUNT_CHANGED, Time.zone.now, conversation: @conversation) + end end diff --git a/app/controllers/api/v1/accounts/conversations/unread_counts_controller.rb b/app/controllers/api/v1/accounts/conversations/unread_counts_controller.rb index d9f15613b..0b2335475 100644 --- a/app/controllers/api/v1/accounts/conversations/unread_counts_controller.rb +++ b/app/controllers/api/v1/accounts/conversations/unread_counts_controller.rb @@ -2,12 +2,28 @@ class Api::V1::Accounts::Conversations::UnreadCountsController < Api::V1::Accoun before_action :ensure_unread_counts_enabled def index - counts = ::Conversations::UnreadCounts::Counter.new(account: Current.account, user: Current.user).perform + counts = if filtered_unread_counts_enabled? + instrumentation.summarize_request(account_id: Current.account.id) { unread_counts } + else + unread_counts + end render json: { payload: counts } end private + def unread_counts + ::Conversations::UnreadCounts::Counter.new(account: Current.account, user: Current.user).perform + end + + def filtered_unread_counts_enabled? + Current.account.feature_enabled?(::Conversations::UnreadCounts::FilteredCounter::FEATURE_FLAG) + end + + def instrumentation + ::Conversations::UnreadCounts::FilteredCountInstrumentation + end + def ensure_unread_counts_enabled return if Current.account.feature_enabled?('conversation_unread_counts') diff --git a/app/controllers/api/v1/accounts/conversations_controller.rb b/app/controllers/api/v1/accounts/conversations_controller.rb index 2e53fa7c9..38e8d94aa 100644 --- a/app/controllers/api/v1/accounts/conversations_controller.rb +++ b/app/controllers/api/v1/accounts/conversations_controller.rb @@ -164,6 +164,7 @@ class Api::V1::Accounts::ConversationsController < Api::V1::Accounts::BaseContro # rubocop:enable Rails/SkipsModelValidations ::Conversations::UnreadCounts::Notifier.new(@conversation).perform + ::Conversations::UnreadCounts::FilteredCountInvalidator.new(Current.account).conversation_changed! end def should_update_last_seen? diff --git a/app/helpers/filters/filter_helper.rb b/app/helpers/filters/filter_helper.rb index 4f345676e..d32c9468a 100644 --- a/app/helpers/filters/filter_helper.rb +++ b/app/helpers/filters/filter_helper.rb @@ -68,6 +68,8 @@ module Filters::FilterHelper when 'text_case_insensitive' text_case_insensitive_filter(query_hash, filter_operator_value) else + return text_cast_filter(query_hash, filter_operator_value) if text_search_on_display_id?(query_hash) + default_filter(query_hash, filter_operator_value) end end @@ -82,10 +84,18 @@ module Filters::FilterHelper "#{filter_operator_value} #{query_hash[:query_operator]}" end + def text_cast_filter(query_hash, filter_operator_value) + "(#{filter_config[:table_name]}.#{query_hash[:attribute_key]})::text #{filter_operator_value} #{query_hash[:query_operator]}" + end + def default_filter(query_hash, filter_operator_value) "#{filter_config[:table_name]}.#{query_hash[:attribute_key]} #{filter_operator_value} #{query_hash[:query_operator]}" end + def text_search_on_display_id?(query_hash) + query_hash[:attribute_key] == 'display_id' && %w[contains does_not_contain].include?(query_hash[:filter_operator]) + end + def validate_single_condition(condition) return if condition['query_operator'].nil? return if condition['query_operator'].empty? diff --git a/app/javascript/dashboard/components-next/sidebar/Sidebar.vue b/app/javascript/dashboard/components-next/sidebar/Sidebar.vue index 0e6b3468b..a78460ca2 100644 --- a/app/javascript/dashboard/components-next/sidebar/Sidebar.vue +++ b/app/javascript/dashboard/components-next/sidebar/Sidebar.vue @@ -75,6 +75,16 @@ const hasConversationUnreadCounts = computed(() => { ); }); +const hasFilteredUnreadCounts = computed(() => { + return ( + hasConversationUnreadCounts.value && + isFeatureEnabledonAccount.value( + accountId.value, + FEATURE_FLAGS.UNREAD_COUNT_FOR_FILTERS + ) + ); +}); + const fetchConversationUnreadCounts = ([currentAccountId, isEnabled]) => { if (!currentAccountId) return; @@ -199,6 +209,18 @@ const getLabelUnreadCount = useMapGetter( const getTeamUnreadCount = useMapGetter( 'conversationUnreadCounts/getTeamUnreadCount' ); +const mentionsUnreadCount = useMapGetter( + 'conversationUnreadCounts/getMentionsUnreadCount' +); +const participatingUnreadCount = useMapGetter( + 'conversationUnreadCounts/getParticipatingUnreadCount' +); +const unattendedUnreadCount = useMapGetter( + 'conversationUnreadCounts/getUnattendedUnreadCount' +); +const getFolderUnreadCount = useMapGetter( + 'conversationUnreadCounts/getFolderUnreadCount' +); const teams = useMapGetter('teams/getMyTeams'); const contactCustomViews = useMapGetter('customViews/getContactCustomViews'); const conversationCustomViews = useMapGetter( @@ -226,14 +248,22 @@ watch([accountId, currentUserId], fetchSidebarSortPreferences, { immediate: true, }); +const hasUnreadCountsForSection = section => { + if (section === SIDEBAR_SORT_SECTIONS.FOLDERS) { + return hasFilteredUnreadCounts.value; + } + + return hasConversationUnreadCounts.value; +}; + const getSortOptionsForSection = section => getSidebarSortOptions(section, { - hasUnreadCounts: hasConversationUnreadCounts.value, + hasUnreadCounts: hasUnreadCountsForSection(section), }); const getSortForSection = section => resolveSidebarSort(section, getSidebarSectionSort.value(section), { - hasUnreadCounts: hasConversationUnreadCounts.value, + hasUnreadCounts: hasUnreadCountsForSection(section), }); const updateSortPreference = (section, sortBy) => { @@ -253,6 +283,7 @@ const sortedFolders = computed(() => sortSidebarItems(conversationCustomViews.value, { sortBy: getSortForSection(SIDEBAR_SORT_SECTIONS.FOLDERS), labelKey: view => view.name, + unreadCountKey: view => getFolderUnreadCount.value(view.id), }) ); @@ -342,6 +373,9 @@ const menuItems = computed(() => { name: 'Mentions', label: t('SIDEBAR.MENTIONED_CONVERSATIONS'), icon: 'i-lucide-at-sign', + badgeCount: hasFilteredUnreadCounts.value + ? mentionsUnreadCount.value + : 0, activeOn: ['conversation_through_mentions'], to: accountScopedRoute('conversation_mentions'), }, @@ -349,6 +383,9 @@ const menuItems = computed(() => { name: 'Participating', label: t('SIDEBAR.PARTICIPATING_CONVERSATIONS'), icon: 'i-lucide-user-round-check', + badgeCount: hasFilteredUnreadCounts.value + ? participatingUnreadCount.value + : 0, activeOn: ['conversation_through_participating'], to: accountScopedRoute('conversation_participating'), }, @@ -357,6 +394,9 @@ const menuItems = computed(() => { activeOn: ['conversation_through_unattended'], label: t('SIDEBAR.UNATTENDED_CONVERSATIONS'), icon: 'i-lucide-clock-alert', + badgeCount: hasFilteredUnreadCounts.value + ? unattendedUnreadCount.value + : 0, to: accountScopedRoute('conversation_unattended'), }, { @@ -370,6 +410,9 @@ const menuItems = computed(() => { children: sortedFolders.value.map(view => ({ name: `${view.name}-${view.id}`, label: view.name, + badgeCount: hasFilteredUnreadCounts.value + ? getFolderUnreadCount.value(view.id) + : 0, to: accountScopedRoute('folder_conversations', { id: view.id }), })), }, diff --git a/app/javascript/dashboard/featureFlags.js b/app/javascript/dashboard/featureFlags.js index 00a79763b..b9f59eba0 100644 --- a/app/javascript/dashboard/featureFlags.js +++ b/app/javascript/dashboard/featureFlags.js @@ -47,6 +47,7 @@ export const FEATURE_FLAGS = { ADVANCED_SEARCH: 'advanced_search', CONVERSATION_REQUIRED_ATTRIBUTES: 'conversation_required_attributes', CONVERSATION_UNREAD_COUNTS: 'conversation_unread_counts', + UNREAD_COUNT_FOR_FILTERS: 'unread_count_for_filters', }; export const PREMIUM_FEATURES = [ diff --git a/app/javascript/dashboard/helper/actionCable.js b/app/javascript/dashboard/helper/actionCable.js index e6f2993e4..ef03b1c01 100644 --- a/app/javascript/dashboard/helper/actionCable.js +++ b/app/javascript/dashboard/helper/actionCable.js @@ -17,6 +17,13 @@ import { FEATURE_FLAGS } from 'dashboard/featureFlags'; const { isImpersonating } = useImpersonation(); const UNREAD_COUNTS_REFETCH_THROTTLE_MS = 5000; +const FILTERED_UNREAD_COUNTS_REFRESH_RETRY_MS = 30000; +const FILTERED_UNREAD_COUNTS_REFRESH_RETRY_JITTER_MS = 15000; +const MENTION_UNREAD_COUNTS_REFETCH_DELAY_MS = + UNREAD_COUNTS_REFETCH_THROTTLE_MS; +const getFilteredUnreadCountsRefreshRetryDelay = () => + FILTERED_UNREAD_COUNTS_REFRESH_RETRY_MS + + Math.random() * FILTERED_UNREAD_COUNTS_REFRESH_RETRY_JITTER_MS; class ActionCableConnector extends BaseActionCableConnector { constructor(app, pubsubToken) { @@ -25,6 +32,9 @@ class ActionCableConnector extends BaseActionCableConnector { this.CancelTyping = []; this.lastUnreadCountsFetchAt = null; this.unreadCountsFetchTimer = null; + this.mentionUnreadCountsFetchTimer = null; + this.mentionUnreadCountsRetryTimer = null; + this.filteredUnreadCountsRetryTimer = null; this.events = { 'message.created': this.onMessageCreated, 'message.updated': this.onMessageUpdated, @@ -140,7 +150,12 @@ class ActionCableConnector extends BaseActionCableConnector { }; onConversationUnreadCountChanged = () => { + this.refreshConversationUnreadCountsWithFilteredRetry(); + }; + + refreshConversationUnreadCountsWithFilteredRetry = () => { this.throttledFetchConversationUnreadCounts(); + this.scheduleFilteredUnreadCountsRetry(); }; throttledFetchConversationUnreadCounts = () => { @@ -171,6 +186,51 @@ class ActionCableConnector extends BaseActionCableConnector { this.unreadCountsFetchTimer = null; }; + scheduleMentionUnreadCountsFetch = () => { + if (!this.isFilteredUnreadCountsEnabled()) return; + + // Mention invalidation runs through the async dispatcher, and stale snapshots + // can be served until the filtered-count backend refresh window opens. + this.scheduleUnreadCountsFetchAfter( + 'mentionUnreadCountsFetchTimer', + MENTION_UNREAD_COUNTS_REFETCH_DELAY_MS + ); + this.scheduleUnreadCountsFetchAfter( + 'mentionUnreadCountsRetryTimer', + getFilteredUnreadCountsRefreshRetryDelay(), + { reset: true } + ); + }; + + scheduleFilteredUnreadCountsRetry = () => { + if (!this.isFilteredUnreadCountsEnabled()) return; + + // Filtered snapshots can intentionally stay stale until the backend + // refresh window opens. + this.scheduleUnreadCountsFetchAfter( + 'filteredUnreadCountsRetryTimer', + getFilteredUnreadCountsRefreshRetryDelay(), + { reset: true } + ); + }; + + scheduleUnreadCountsFetchAfter = ( + timerName, + delay, + { reset = false } = {} + ) => { + if (this[timerName]) { + if (!reset) return; + + clearTimeout(this[timerName]); + } + + this[timerName] = setTimeout(() => { + this[timerName] = null; + this.throttledFetchConversationUnreadCounts(); + }, delay); + }; + fetchConversationUnreadCounts = () => { if (!this.isConversationUnreadCountsEnabled()) return; @@ -189,6 +249,17 @@ class ActionCableConnector extends BaseActionCableConnector { ); }; + isFilteredUnreadCountsEnabled = () => { + const accountId = this.app.$store.getters.getCurrentAccountId; + const isFeatureEnabled = + this.app.$store.getters['accounts/isFeatureEnabledonAccount']; + + return ( + isFeatureEnabled?.(accountId, FEATURE_FLAGS.CONVERSATION_UNREAD_COUNTS) && + isFeatureEnabled?.(accountId, FEATURE_FLAGS.UNREAD_COUNT_FOR_FILTERS) + ); + }; + onTypingOn = ({ conversation, user }) => { const conversationId = conversation.id; @@ -212,6 +283,7 @@ class ActionCableConnector extends BaseActionCableConnector { onConversationMentioned = data => { this.app.$store.dispatch('addMentions', data); + this.scheduleMentionUnreadCountsFetch(); }; clearTimer = conversationId => { @@ -273,6 +345,12 @@ 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 }); + + if (this.isFilteredUnreadCountsEnabled()) { + // Inbox/team/label visibility changes can change the accessible set used + // by filtered unread counts even when no conversation row changes. + this.refreshConversationUnreadCountsWithFilteredRetry(); + } }; onVoiceCallIncoming = data => { diff --git a/app/javascript/dashboard/helper/sidebarSort.js b/app/javascript/dashboard/helper/sidebarSort.js index 801dd94e2..f1167f70c 100644 --- a/app/javascript/dashboard/helper/sidebarSort.js +++ b/app/javascript/dashboard/helper/sidebarSort.js @@ -20,6 +20,8 @@ export const SIDEBAR_SORT_OPTIONS_BY_SECTION = Object.freeze({ SIDEBAR_SORT_KEYS.CREATED_ASC, SIDEBAR_SORT_KEYS.ALPHABETICAL_ASC, SIDEBAR_SORT_KEYS.ALPHABETICAL_DESC, + SIDEBAR_SORT_KEYS.UNREAD_COUNT_DESC, + SIDEBAR_SORT_KEYS.UNREAD_COUNT_ASC, ], [SIDEBAR_SORT_SECTIONS.TEAMS]: [ SIDEBAR_SORT_KEYS.CREATED_DESC, diff --git a/app/javascript/dashboard/helper/specs/actionCable.spec.js b/app/javascript/dashboard/helper/specs/actionCable.spec.js index 8ba411a5f..b288aee83 100644 --- a/app/javascript/dashboard/helper/specs/actionCable.spec.js +++ b/app/javascript/dashboard/helper/specs/actionCable.spec.js @@ -1,5 +1,6 @@ import { describe, it, beforeEach, afterEach, expect, vi } from 'vitest'; import ActionCableConnector from '../actionCable'; +import { FEATURE_FLAGS } from 'dashboard/featureFlags'; vi.mock('shared/helpers/mitt', () => ({ emitter: { @@ -17,6 +18,9 @@ global.chatwootConfig = { websocketURL: 'wss://test.chatwoot.com', }; +const mockRetryJitter = value => + vi.spyOn(Math, 'random').mockReturnValue(value); + describe('ActionCableConnector - Copilot Tests', () => { let store; let actionCable; @@ -39,6 +43,8 @@ describe('ActionCableConnector - Copilot Tests', () => { }); afterEach(() => { + vi.restoreAllMocks(); + vi.clearAllTimers(); vi.useRealTimers(); }); describe('copilot event handlers', () => { @@ -81,12 +87,223 @@ describe('ActionCableConnector - Copilot Tests', () => { }); it('should refetch unread counts when unread count changes', () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date('2026-01-01T00:00:00Z')); + mockRetryJitter(0.5); + actionCable.onReceived({ event: 'conversation.unread_count_changed', data: { account_id: 1 }, }); expect(mockDispatch).toHaveBeenCalledWith('conversationUnreadCounts/get'); + + vi.advanceTimersByTime(37499); + expect(mockDispatch).toHaveBeenCalledTimes(1); + + vi.advanceTimersByTime(1); + expect(mockDispatch).toHaveBeenCalledTimes(2); + expect(mockDispatch).toHaveBeenLastCalledWith( + 'conversationUnreadCounts/get' + ); + }); + + it('does not retry unread count changes when filtered counts are disabled', () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date('2026-01-01T00:00:00Z')); + store.$store.getters[ + 'accounts/isFeatureEnabledonAccount' + ].mockImplementation( + (_, featureFlag) => + featureFlag === FEATURE_FLAGS.CONVERSATION_UNREAD_COUNTS + ); + + actionCable.onReceived({ + event: 'conversation.unread_count_changed', + data: { account_id: 1 }, + }); + + expect(mockDispatch).toHaveBeenCalledTimes(1); + + vi.advanceTimersByTime(45000); + expect(mockDispatch).toHaveBeenCalledTimes(1); + }); + + it('delays unread count refetch when a conversation is mentioned', () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date('2026-01-01T00:00:00Z')); + + const conversation = { id: 1, account_id: 1 }; + + actionCable.onReceived({ + event: 'conversation.mentioned', + data: conversation, + }); + + expect(mockDispatch).toHaveBeenCalledWith('addMentions', conversation); + expect(mockDispatch).not.toHaveBeenCalledWith( + 'conversationUnreadCounts/get' + ); + + vi.advanceTimersByTime(4999); + expect(mockDispatch).not.toHaveBeenCalledWith( + 'conversationUnreadCounts/get' + ); + + vi.advanceTimersByTime(1); + expect(mockDispatch).toHaveBeenCalledWith('conversationUnreadCounts/get'); + }); + + it('does not schedule mention unread count fetches when filtered counts are disabled', () => { + vi.useFakeTimers(); + store.$store.getters[ + 'accounts/isFeatureEnabledonAccount' + ].mockImplementation( + (_, featureFlag) => + featureFlag === FEATURE_FLAGS.CONVERSATION_UNREAD_COUNTS + ); + + const conversation = { id: 1, account_id: 1 }; + + actionCable.onReceived({ + event: 'conversation.mentioned', + data: conversation, + }); + + expect(mockDispatch).toHaveBeenCalledWith('addMentions', conversation); + + vi.advanceTimersByTime(45000); + expect(mockDispatch).not.toHaveBeenCalledWith( + 'conversationUnreadCounts/get' + ); + }); + + it('retries mentioned unread counts after the backend refresh window', () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date('2026-01-01T00:00:00Z')); + mockRetryJitter(0.5); + + actionCable.onReceived({ + event: 'conversation.mentioned', + data: { id: 1, account_id: 1 }, + }); + + const unreadCountFetches = () => + mockDispatch.mock.calls.filter( + ([action]) => action === 'conversationUnreadCounts/get' + ); + + vi.advanceTimersByTime(5000); + expect(unreadCountFetches()).toHaveLength(1); + + vi.advanceTimersByTime(32499); + expect(unreadCountFetches()).toHaveLength(1); + + vi.advanceTimersByTime(1); + expect(unreadCountFetches()).toHaveLength(2); + }); + + it('reschedules mentioned unread count retries for later invalidations', () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date('2026-01-01T00:00:00Z')); + mockRetryJitter(0); + + const unreadCountFetches = () => + mockDispatch.mock.calls.filter( + ([action]) => action === 'conversationUnreadCounts/get' + ); + + actionCable.onReceived({ + event: 'conversation.mentioned', + data: { id: 1, account_id: 1 }, + }); + + vi.advanceTimersByTime(5000); + expect(unreadCountFetches()).toHaveLength(1); + + vi.advanceTimersByTime(10000); + actionCable.onReceived({ + event: 'conversation.mentioned', + data: { id: 1, account_id: 1 }, + }); + + vi.advanceTimersByTime(5000); + expect(unreadCountFetches()).toHaveLength(2); + + vi.advanceTimersByTime(10000); + expect(unreadCountFetches()).toHaveLength(2); + + vi.advanceTimersByTime(15000); + expect(unreadCountFetches()).toHaveLength(3); + }); + + it('refetches filtered unread counts after account cache invalidation', () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date('2026-01-01T00:00:00Z')); + mockRetryJitter(0.5); + + const cacheKeys = { + label: 'label-key', + inbox: 'inbox-key', + team: 'team-key', + }; + const unreadCountFetches = () => + mockDispatch.mock.calls.filter( + ([action]) => action === 'conversationUnreadCounts/get' + ); + + actionCable.onReceived({ + event: 'account.cache_invalidated', + data: { account_id: 1, cache_keys: cacheKeys }, + }); + + expect(mockDispatch).toHaveBeenCalledWith('labels/revalidate', { + newKey: cacheKeys.label, + }); + expect(mockDispatch).toHaveBeenCalledWith('inboxes/revalidate', { + newKey: cacheKeys.inbox, + }); + expect(mockDispatch).toHaveBeenCalledWith('teams/revalidate', { + newKey: cacheKeys.team, + }); + expect(unreadCountFetches()).toHaveLength(1); + + vi.advanceTimersByTime(37499); + expect(unreadCountFetches()).toHaveLength(1); + + vi.advanceTimersByTime(1); + expect(unreadCountFetches()).toHaveLength(2); + }); + + it('does not refetch unread counts after cache invalidation when filtered counts are disabled', () => { + vi.useFakeTimers(); + store.$store.getters[ + 'accounts/isFeatureEnabledonAccount' + ].mockImplementation( + (_, featureFlag) => + featureFlag === FEATURE_FLAGS.CONVERSATION_UNREAD_COUNTS + ); + + actionCable.onReceived({ + event: 'account.cache_invalidated', + data: { + account_id: 1, + cache_keys: { + label: 'label-key', + inbox: 'inbox-key', + team: 'team-key', + }, + }, + }); + + expect(mockDispatch).not.toHaveBeenCalledWith( + 'conversationUnreadCounts/get' + ); + + vi.advanceTimersByTime(45000); + expect(mockDispatch).not.toHaveBeenCalledWith( + 'conversationUnreadCounts/get' + ); }); it('does not refetch unread counts when unread count feature is disabled', () => { diff --git a/app/javascript/dashboard/helper/specs/sidebarSort.spec.js b/app/javascript/dashboard/helper/specs/sidebarSort.spec.js index 3919252c2..7b36e0910 100644 --- a/app/javascript/dashboard/helper/specs/sidebarSort.spec.js +++ b/app/javascript/dashboard/helper/specs/sidebarSort.spec.js @@ -148,7 +148,7 @@ describe('#normalizeSidebarSortPreferences', () => { it('falls back to defaults for unsupported preferences', () => { const preferences = normalizeSidebarSortPreferences({ - [SIDEBAR_SORT_SECTIONS.FOLDERS]: SIDEBAR_SORT_KEYS.UNREAD_COUNT_DESC, + [SIDEBAR_SORT_SECTIONS.FOLDERS]: 'unsupported_sort', }); expect(preferences).toEqual(DEFAULT_SIDEBAR_SORT_PREFERENCES); @@ -171,6 +171,15 @@ describe('#getSidebarSortOptions', () => { expect(options).toContain(SIDEBAR_SORT_KEYS.UNREAD_COUNT_ASC); }); + it('keeps folder unread count options when filtered unread counts are enabled', () => { + const options = getSidebarSortOptions(SIDEBAR_SORT_SECTIONS.FOLDERS, { + hasUnreadCounts: true, + }); + + expect(options).toContain(SIDEBAR_SORT_KEYS.UNREAD_COUNT_DESC); + expect(options).toContain(SIDEBAR_SORT_KEYS.UNREAD_COUNT_ASC); + }); + it('removes unread count options when unread counts are disabled', () => { const options = getSidebarSortOptions(SIDEBAR_SORT_SECTIONS.TEAMS, { hasUnreadCounts: false, @@ -180,6 +189,16 @@ describe('#getSidebarSortOptions', () => { expect(options).not.toContain(SIDEBAR_SORT_KEYS.UNREAD_COUNT_ASC); expect(options).toContain(SIDEBAR_SORT_KEYS.ALPHABETICAL_ASC); }); + + it('removes folder unread count options when filtered unread counts are disabled', () => { + const options = getSidebarSortOptions(SIDEBAR_SORT_SECTIONS.FOLDERS, { + hasUnreadCounts: false, + }); + + expect(options).not.toContain(SIDEBAR_SORT_KEYS.UNREAD_COUNT_DESC); + expect(options).not.toContain(SIDEBAR_SORT_KEYS.UNREAD_COUNT_ASC); + expect(options).toContain(SIDEBAR_SORT_KEYS.ALPHABETICAL_ASC); + }); }); describe('#resolveSidebarSort', () => { @@ -202,4 +221,14 @@ describe('#resolveSidebarSort', () => { expect(sortBy).toBe(SIDEBAR_SORT_KEYS.ALPHABETICAL_ASC); }); + + it('falls back to alphabetical sort for folders when filtered unread counts are disabled', () => { + const sortBy = resolveSidebarSort( + SIDEBAR_SORT_SECTIONS.FOLDERS, + SIDEBAR_SORT_KEYS.UNREAD_COUNT_DESC, + { hasUnreadCounts: false } + ); + + expect(sortBy).toBe(SIDEBAR_SORT_KEYS.ALPHABETICAL_ASC); + }); }); diff --git a/app/javascript/dashboard/store/modules/conversationUnreadCounts.js b/app/javascript/dashboard/store/modules/conversationUnreadCounts.js index c03249311..03bbb31eb 100644 --- a/app/javascript/dashboard/store/modules/conversationUnreadCounts.js +++ b/app/javascript/dashboard/store/modules/conversationUnreadCounts.js @@ -6,6 +6,10 @@ export const state = { inboxes: {}, labels: {}, teams: {}, + mentionsCount: 0, + participatingCount: 0, + unattendedCount: 0, + folders: {}, }; const normalizeCount = count => { @@ -37,6 +41,18 @@ export const getters = { getTeamUnreadCount: $state => teamId => { return $state.teams[String(teamId)] || 0; }, + getMentionsUnreadCount($state) { + return $state.mentionsCount; + }, + getParticipatingUnreadCount($state) { + return $state.participatingCount; + }, + getUnattendedUnreadCount($state) { + return $state.unattendedCount; + }, + getFolderUnreadCount: $state => folderId => { + return $state.folders[String(folderId)] || 0; + }, getInboxUnreadCounts($state) { return $state.inboxes; }, @@ -46,6 +62,9 @@ export const getters = { getTeamUnreadCounts($state) { return $state.teams; }, + getFolderUnreadCounts($state) { + return $state.folders; + }, }; export const actions = { @@ -68,6 +87,10 @@ export const mutations = { $state.inboxes = normalizeCounts(payload.inboxes); $state.labels = normalizeCounts(payload.labels); $state.teams = normalizeCounts(payload.teams); + $state.mentionsCount = normalizeCount(payload.mentions_count); + $state.participatingCount = normalizeCount(payload.participating_count); + $state.unattendedCount = normalizeCount(payload.unattended_count); + $state.folders = normalizeCounts(payload.folders); }, }; diff --git a/app/javascript/dashboard/store/modules/conversationWatchers.js b/app/javascript/dashboard/store/modules/conversationWatchers.js index c690da21f..da29acaf6 100644 --- a/app/javascript/dashboard/store/modules/conversationWatchers.js +++ b/app/javascript/dashboard/store/modules/conversationWatchers.js @@ -1,8 +1,15 @@ import types from '../mutation-types'; import { throwErrorMessage } from 'dashboard/store/utils/api'; +import { FEATURE_FLAGS } from 'dashboard/featureFlags'; import ConversationInboxApi from '../../api/inbox/conversation'; +const FILTERED_UNREAD_COUNTS_REFRESH_RETRY_MS = 30000; +const FILTERED_UNREAD_COUNTS_REFRESH_RETRY_JITTER_MS = 15000; +const getFilteredUnreadCountsRefreshRetryDelay = () => + FILTERED_UNREAD_COUNTS_REFRESH_RETRY_MS + + Math.random() * FILTERED_UNREAD_COUNTS_REFRESH_RETRY_JITTER_MS; + const state = { records: {}, uiFlags: { @@ -20,6 +27,43 @@ export const getters = { }, }; +const hasFeatureEnabled = (rootGetters, featureFlag) => { + const accountId = rootGetters?.getCurrentAccountId; + const isFeatureEnabled = rootGetters?.['accounts/isFeatureEnabledonAccount']; + + return Boolean(accountId && isFeatureEnabled?.(accountId, featureFlag)); +}; + +const hasCurrentUser = (participants, currentUserId) => + (Array.isArray(participants) ? participants : []).some( + participant => participant.id === currentUserId + ); + +const refreshConversationUnreadCounts = dispatch => { + dispatch('conversationUnreadCounts/get', {}, { root: true }); + setTimeout( + () => dispatch('conversationUnreadCounts/get', {}, { root: true }), + getFilteredUnreadCountsRefreshRetryDelay() + ); +}; + +const shouldRefreshConversationUnreadCounts = ( + { rootGetters, state: moduleState }, + conversationId, + participants +) => { + const currentUserId = + rootGetters?.getCurrentUserID || rootGetters?.getCurrentUser?.id; + + return ( + currentUserId && + hasFeatureEnabled(rootGetters, FEATURE_FLAGS.CONVERSATION_UNREAD_COUNTS) && + hasFeatureEnabled(rootGetters, FEATURE_FLAGS.UNREAD_COUNT_FOR_FILTERS) && + hasCurrentUser(moduleState.records[conversationId], currentUserId) !== + hasCurrentUser(participants, currentUserId) + ); +}; + export const actions = { show: async ({ commit }, { conversationId }) => { commit(types.SET_CONVERSATION_PARTICIPANTS_UI_FLAG, { @@ -42,7 +86,10 @@ export const actions = { } }, - update: async ({ commit }, { conversationId, userIds }) => { + update: async ( + { commit, dispatch, rootGetters, state: moduleState }, + { conversationId, userIds } + ) => { commit(types.SET_CONVERSATION_PARTICIPANTS_UI_FLAG, { isUpdating: true, }); @@ -52,10 +99,18 @@ export const actions = { conversationId, userIds, }); + const shouldRefreshUnreadCounts = shouldRefreshConversationUnreadCounts( + { rootGetters, state: moduleState }, + conversationId, + response.data + ); commit(types.SET_CONVERSATION_PARTICIPANTS, { conversationId, data: response.data, }); + if (shouldRefreshUnreadCounts) { + refreshConversationUnreadCounts(dispatch); + } } catch (error) { throwErrorMessage(error); } finally { diff --git a/app/javascript/dashboard/store/modules/customViews.js b/app/javascript/dashboard/store/modules/customViews.js index 388388e0f..b4dff3610 100644 --- a/app/javascript/dashboard/store/modules/customViews.js +++ b/app/javascript/dashboard/store/modules/customViews.js @@ -1,11 +1,17 @@ import * as MutationHelpers from 'shared/helpers/vuex/mutationHelpers'; import types from '../mutation-types'; import CustomViewsAPI from '../../api/customViews'; +import { FEATURE_FLAGS } from 'dashboard/featureFlags'; const VIEW_TYPES = { CONVERSATION: 'conversation', CONTACT: 'contact', }; +const FILTERED_UNREAD_COUNTS_REFRESH_RETRY_MS = 30000; +const FILTERED_UNREAD_COUNTS_REFRESH_RETRY_JITTER_MS = 15000; +const getFilteredUnreadCountsRefreshRetryDelay = () => + FILTERED_UNREAD_COUNTS_REFRESH_RETRY_MS + + Math.random() * FILTERED_UNREAD_COUNTS_REFRESH_RETRY_JITTER_MS; // use to normalize the filter type const FILTER_KEYS = { @@ -21,6 +27,38 @@ const getFolderContactId = folder => folder?.query?.payload?.find(filter => filter.attribute_key === 'contact_id') ?.values?.[0]; +const hasFeatureEnabled = (rootGetters, featureFlag) => { + const accountId = rootGetters?.getCurrentAccountId; + const isFeatureEnabled = rootGetters?.['accounts/isFeatureEnabledonAccount']; + + return Boolean(accountId && isFeatureEnabled?.(accountId, featureFlag)); +}; + +const shouldRefreshConversationUnreadCounts = (filterType, rootGetters) => { + return ( + FILTER_KEYS[filterType] === VIEW_TYPES.CONVERSATION && + hasFeatureEnabled(rootGetters, FEATURE_FLAGS.CONVERSATION_UNREAD_COUNTS) && + hasFeatureEnabled(rootGetters, FEATURE_FLAGS.UNREAD_COUNT_FOR_FILTERS) + ); +}; + +const dispatchConversationUnreadCounts = dispatch => { + dispatch('conversationUnreadCounts/get', {}, { root: true }); +}; + +const refreshConversationUnreadCounts = ( + { dispatch, rootGetters }, + filterType +) => { + if (!shouldRefreshConversationUnreadCounts(filterType, rootGetters)) return; + + dispatchConversationUnreadCounts(dispatch); + setTimeout( + () => dispatchConversationUnreadCounts(dispatch), + getFilteredUnreadCountsRefreshRetryDelay() + ); +}; + export const state = { [VIEW_TYPES.CONVERSATION]: { records: [], @@ -71,14 +109,19 @@ export const actions = { commit(types.SET_CUSTOM_VIEW_UI_FLAG, { isFetching: false }); } }, - create: async function createCustomViews({ commit }, obj) { + create: async function createCustomViews( + { commit, dispatch, rootGetters }, + obj + ) { commit(types.SET_CUSTOM_VIEW_UI_FLAG, { isCreating: true }); try { const response = await CustomViewsAPI.create(obj); + const filterType = FILTER_KEYS[obj.filter_type]; commit(types.ADD_CUSTOM_VIEW, { data: response.data, - filterType: FILTER_KEYS[obj.filter_type], + filterType, }); + refreshConversationUnreadCounts({ dispatch, rootGetters }, filterType); return response; } catch (error) { const errorMessage = error?.response?.data?.message; @@ -87,14 +130,19 @@ export const actions = { commit(types.SET_CUSTOM_VIEW_UI_FLAG, { isCreating: false }); } }, - update: async function updateCustomViews({ commit }, obj) { + update: async function updateCustomViews( + { commit, dispatch, rootGetters }, + obj + ) { commit(types.SET_CUSTOM_VIEW_UI_FLAG, { isCreating: true }); try { const response = await CustomViewsAPI.update(obj.id, obj); + const filterType = FILTER_KEYS[obj.filter_type]; commit(types.UPDATE_CUSTOM_VIEW, { data: response.data, - filterType: FILTER_KEYS[obj.filter_type], + filterType, }); + refreshConversationUnreadCounts({ dispatch, rootGetters }, filterType); } catch (error) { const errorMessage = error?.response?.data?.message; throw new Error(errorMessage); @@ -102,11 +150,12 @@ export const actions = { commit(types.SET_CUSTOM_VIEW_UI_FLAG, { isCreating: false }); } }, - delete: async ({ commit }, { id, filterType }) => { + delete: async ({ commit, dispatch, rootGetters }, { id, filterType }) => { commit(types.SET_CUSTOM_VIEW_UI_FLAG, { isDeleting: true }); try { await CustomViewsAPI.deleteCustomViews(id, filterType); commit(types.DELETE_CUSTOM_VIEW, { data: id, filterType }); + refreshConversationUnreadCounts({ dispatch, rootGetters }, filterType); } catch (error) { throw new Error(error); } finally { diff --git a/app/javascript/dashboard/store/modules/specs/conversationUnreadCounts/actions.spec.js b/app/javascript/dashboard/store/modules/specs/conversationUnreadCounts/actions.spec.js index 29fadc897..c3d040d84 100644 --- a/app/javascript/dashboard/store/modules/specs/conversationUnreadCounts/actions.spec.js +++ b/app/javascript/dashboard/store/modules/specs/conversationUnreadCounts/actions.spec.js @@ -19,6 +19,10 @@ describe('#actions', () => { inboxes: { 1: '2' }, labels: { 3: 4 }, teams: { 5: 6 }, + mentions_count: 7, + participating_count: 8, + unattended_count: 9, + folders: { 10: 11 }, }; axios.get.mockResolvedValue({ data: { payload } }); diff --git a/app/javascript/dashboard/store/modules/specs/conversationUnreadCounts/getters.spec.js b/app/javascript/dashboard/store/modules/specs/conversationUnreadCounts/getters.spec.js index 9fe19e22d..9b7467ae1 100644 --- a/app/javascript/dashboard/store/modules/specs/conversationUnreadCounts/getters.spec.js +++ b/app/javascript/dashboard/store/modules/specs/conversationUnreadCounts/getters.spec.js @@ -7,6 +7,7 @@ describe('#getters', () => { inboxes: { 1: 2 }, labels: {}, teams: {}, + folders: {}, }; expect(getters.getInboxUnreadCount(state)(1)).toBe(2); @@ -20,6 +21,7 @@ describe('#getters', () => { inboxes: {}, labels: { 3: 4 }, teams: {}, + folders: {}, }; expect(getters.getLabelUnreadCount(state)(3)).toBe(4); @@ -33,6 +35,7 @@ describe('#getters', () => { inboxes: {}, labels: {}, teams: { 5: 6 }, + folders: {}, }; expect(getters.getTeamUnreadCount(state)(5)).toBe(6); @@ -46,21 +49,44 @@ describe('#getters', () => { inboxes: {}, labels: {}, teams: {}, + folders: {}, }; expect(getters.getAllUnreadCount(state)).toBe(7); }); + it('returns filtered unread counts', () => { + const state = { + allCount: 0, + inboxes: {}, + labels: {}, + teams: {}, + mentionsCount: 1, + participatingCount: 2, + unattendedCount: 3, + folders: { 8: 4 }, + }; + + expect(getters.getMentionsUnreadCount(state)).toBe(1); + expect(getters.getParticipatingUnreadCount(state)).toBe(2); + expect(getters.getUnattendedUnreadCount(state)).toBe(3); + expect(getters.getFolderUnreadCount(state)(8)).toBe(4); + expect(getters.getFolderUnreadCount(state)('8')).toBe(4); + expect(getters.getFolderUnreadCount(state)(9)).toBe(0); + }); + it('returns unread count maps', () => { const state = { allCount: 0, inboxes: { 1: 2 }, labels: { 3: 4 }, teams: { 5: 6 }, + folders: { 7: 8 }, }; expect(getters.getInboxUnreadCounts(state)).toEqual({ 1: 2 }); expect(getters.getLabelUnreadCounts(state)).toEqual({ 3: 4 }); expect(getters.getTeamUnreadCounts(state)).toEqual({ 5: 6 }); + expect(getters.getFolderUnreadCounts(state)).toEqual({ 7: 8 }); }); }); diff --git a/app/javascript/dashboard/store/modules/specs/conversationUnreadCounts/mutations.spec.js b/app/javascript/dashboard/store/modules/specs/conversationUnreadCounts/mutations.spec.js index 8941d7430..60785593c 100644 --- a/app/javascript/dashboard/store/modules/specs/conversationUnreadCounts/mutations.spec.js +++ b/app/javascript/dashboard/store/modules/specs/conversationUnreadCounts/mutations.spec.js @@ -4,7 +4,16 @@ import { mutations } from '../../conversationUnreadCounts'; describe('#mutations', () => { describe('#SET_CONVERSATION_UNREAD_COUNTS', () => { it('normalizes unread count payload', () => { - const state = { allCount: 0, inboxes: {}, labels: {}, teams: {} }; + const state = { + allCount: 0, + inboxes: {}, + labels: {}, + teams: {}, + mentionsCount: 0, + participatingCount: 0, + unattendedCount: 0, + folders: {}, + }; mutations[types.SET_CONVERSATION_UNREAD_COUNTS](state, { all_count: '3', @@ -21,6 +30,13 @@ describe('#mutations', () => { 6: '7', 7: 0, }, + mentions_count: '8', + participating_count: 9, + unattended_count: 0, + folders: { + 10: '11', + 12: -1, + }, }); expect(state).toEqual({ @@ -28,6 +44,10 @@ describe('#mutations', () => { inboxes: { 1: 2 }, labels: { 4: 5 }, teams: { 6: 7 }, + mentionsCount: 8, + participatingCount: 9, + unattendedCount: 0, + folders: { 10: 11 }, }); }); @@ -37,6 +57,10 @@ describe('#mutations', () => { inboxes: { 1: 2 }, labels: { 4: 5 }, teams: { 6: 7 }, + mentionsCount: 8, + participatingCount: 9, + unattendedCount: 10, + folders: { 11: 12 }, }; mutations[types.SET_CONVERSATION_UNREAD_COUNTS](state, {}); @@ -46,17 +70,36 @@ describe('#mutations', () => { inboxes: {}, labels: {}, teams: {}, + mentionsCount: 0, + participatingCount: 0, + unattendedCount: 0, + folders: {}, }); }); it('normalizes invalid aggregate counts to zero', () => { - const state = { allCount: 2, inboxes: {}, labels: {}, teams: {} }; + const state = { + allCount: 2, + inboxes: {}, + labels: {}, + teams: {}, + mentionsCount: 2, + participatingCount: 3, + unattendedCount: 4, + folders: {}, + }; mutations[types.SET_CONVERSATION_UNREAD_COUNTS](state, { all_count: 'invalid', + mentions_count: 'invalid', + participating_count: -1, + unattended_count: 0, }); expect(state.allCount).toBe(0); + expect(state.mentionsCount).toBe(0); + expect(state.participatingCount).toBe(0); + expect(state.unattendedCount).toBe(0); }); }); }); diff --git a/app/javascript/dashboard/store/modules/specs/conversationWatchers/actions.spec.js b/app/javascript/dashboard/store/modules/specs/conversationWatchers/actions.spec.js index b3cfabeba..defa9a3db 100644 --- a/app/javascript/dashboard/store/modules/specs/conversationWatchers/actions.spec.js +++ b/app/javascript/dashboard/store/modules/specs/conversationWatchers/actions.spec.js @@ -1,11 +1,32 @@ import axios from 'axios'; import { actions } from '../../conversationWatchers'; import types from '../../../mutation-types'; +import { FEATURE_FLAGS } from '../../../../featureFlags'; const commit = vi.fn(); global.axios = axios; vi.mock('axios'); +const mockRetryJitter = value => + vi.spyOn(Math, 'random').mockReturnValue(value); + +afterEach(() => { + vi.restoreAllMocks(); + vi.clearAllTimers(); + vi.useRealTimers(); +}); + +const conversationUnreadCountsEnabledRootGetters = { + getCurrentAccountId: 1, + getCurrentUserID: 1, + 'accounts/isFeatureEnabledonAccount': vi.fn((_, featureFlag) => + [ + FEATURE_FLAGS.CONVERSATION_UNREAD_COUNTS, + FEATURE_FLAGS.UNREAD_COUNT_FOR_FILTERS, + ].includes(featureFlag) + ), +}; + describe('#actions', () => { describe('#get', () => { it('sends correct actions if API is success', async () => { @@ -48,6 +69,56 @@ describe('#actions', () => { [types.SET_CONVERSATION_PARTICIPANTS_UI_FLAG, { isUpdating: false }], ]); }); + it('refetches unread counts when the current user starts watching', async () => { + vi.useFakeTimers(); + mockRetryJitter(0.5); + const dispatch = vi.fn(); + const moduleState = { records: { 2: [] } }; + const mutatingCommit = vi.fn((mutation, payload) => { + if (mutation === types.SET_CONVERSATION_PARTICIPANTS) { + moduleState.records[payload.conversationId] = payload.data; + } + }); + axios.patch.mockResolvedValue({ data: [{ id: 1 }] }); + + await actions.update( + { + commit: mutatingCommit, + dispatch, + rootGetters: conversationUnreadCountsEnabledRootGetters, + state: moduleState, + }, + { conversationId: 2, userIds: [1] } + ); + + expect(dispatch).toHaveBeenCalledWith( + 'conversationUnreadCounts/get', + {}, + { root: true } + ); + + vi.advanceTimersByTime(37499); + expect(dispatch).toHaveBeenCalledTimes(1); + + vi.advanceTimersByTime(1); + expect(dispatch).toHaveBeenCalledTimes(2); + }); + it('does not refetch unread counts when another watcher changes', async () => { + const dispatch = vi.fn(); + axios.patch.mockResolvedValue({ data: [{ id: 1 }, { id: 2 }] }); + + await actions.update( + { + commit, + dispatch, + rootGetters: conversationUnreadCountsEnabledRootGetters, + state: { records: { 2: [{ id: 1 }] } }, + }, + { conversationId: 2, userIds: [1, 2] } + ); + + expect(dispatch).not.toHaveBeenCalled(); + }); it('sends correct actions if API is error', async () => { axios.patch.mockRejectedValue({ message: 'Incorrect header' }); await expect( diff --git a/app/javascript/dashboard/store/modules/specs/customViews/actions.spec.js b/app/javascript/dashboard/store/modules/specs/customViews/actions.spec.js index a09dda17f..a1ea0638d 100644 --- a/app/javascript/dashboard/store/modules/specs/customViews/actions.spec.js +++ b/app/javascript/dashboard/store/modules/specs/customViews/actions.spec.js @@ -1,6 +1,7 @@ import axios from 'axios'; import * as types from '../../../mutation-types'; import { actions } from '../../customViews'; +import { FEATURE_FLAGS } from '../../../../featureFlags'; import { contactFilterView, customViewList, @@ -11,6 +12,25 @@ const commit = vi.fn(); global.axios = axios; vi.mock('axios'); +const mockRetryJitter = value => + vi.spyOn(Math, 'random').mockReturnValue(value); + +const conversationUnreadCountsEnabledRootGetters = { + getCurrentAccountId: 1, + 'accounts/isFeatureEnabledonAccount': vi.fn((_, featureFlag) => + [ + FEATURE_FLAGS.CONVERSATION_UNREAD_COUNTS, + FEATURE_FLAGS.UNREAD_COUNT_FOR_FILTERS, + ].includes(featureFlag) + ), +}; + +afterEach(() => { + vi.restoreAllMocks(); + vi.clearAllTimers(); + vi.useRealTimers(); +}); + describe('#actions', () => { describe('#get', () => { it('sends correct actions if API is success', async () => { @@ -49,6 +69,36 @@ describe('#actions', () => { [types.default.SET_CUSTOM_VIEW_UI_FLAG, { isCreating: false }], ]); }); + + it('refetches unread counts after creating a conversation folder', async () => { + vi.useFakeTimers(); + mockRetryJitter(0.5); + const dispatch = vi.fn(); + const firstItem = customViewList[0]; + axios.post.mockResolvedValue({ data: firstItem }); + + await actions.create( + { + commit, + dispatch, + rootGetters: conversationUnreadCountsEnabledRootGetters, + }, + firstItem + ); + + expect(dispatch).toHaveBeenCalledWith( + 'conversationUnreadCounts/get', + {}, + { root: true } + ); + + vi.advanceTimersByTime(37499); + expect(dispatch).toHaveBeenCalledTimes(1); + + vi.advanceTimersByTime(1); + expect(dispatch).toHaveBeenCalledTimes(2); + }); + it('sends correct actions if API is error', async () => { axios.post.mockRejectedValue({ message: 'Incorrect header' }); await expect(actions.create({ commit })).rejects.toThrow(Error); @@ -69,6 +119,44 @@ describe('#actions', () => { [types.default.SET_CUSTOM_VIEW_UI_FLAG, { isDeleting: false }], ]); }); + + it('refetches unread counts after deleting a conversation folder', async () => { + vi.useFakeTimers(); + const dispatch = vi.fn(); + axios.delete.mockResolvedValue({ data: customViewList[0] }); + + await actions.delete( + { + commit, + dispatch, + rootGetters: conversationUnreadCountsEnabledRootGetters, + }, + { id: 1, filterType: 'conversation' } + ); + + expect(dispatch).toHaveBeenCalledWith( + 'conversationUnreadCounts/get', + {}, + { root: true } + ); + }); + + it('does not refetch unread counts after deleting a contact segment', async () => { + const dispatch = vi.fn(); + axios.delete.mockResolvedValue({ data: contactFilterView }); + + await actions.delete( + { + commit, + dispatch, + rootGetters: conversationUnreadCountsEnabledRootGetters, + }, + { id: 1, filterType: 'contact' } + ); + + expect(dispatch).not.toHaveBeenCalled(); + }); + it('sends correct actions if API is error', async () => { axios.delete.mockRejectedValue({ message: 'Incorrect header' }); await expect(actions.delete({ commit }, 1)).rejects.toThrow(Error); @@ -93,6 +181,29 @@ describe('#actions', () => { [types.default.SET_CUSTOM_VIEW_UI_FLAG, { isCreating: false }], ]); }); + + it('refetches unread counts after updating a conversation folder', async () => { + vi.useFakeTimers(); + const dispatch = vi.fn(); + const item = updateCustomViewList[0]; + axios.patch.mockResolvedValue({ data: item }); + + await actions.update( + { + commit, + dispatch, + rootGetters: conversationUnreadCountsEnabledRootGetters, + }, + item + ); + + expect(dispatch).toHaveBeenCalledWith( + 'conversationUnreadCounts/get', + {}, + { root: true } + ); + }); + it('sends correct actions if API is error', async () => { axios.patch.mockRejectedValue({ message: 'Incorrect header' }); await expect(actions.update({ commit }, 1)).rejects.toThrow(Error); diff --git a/app/javascript/dashboard/store/modules/specs/sidebarSortPreferences/actions.spec.js b/app/javascript/dashboard/store/modules/specs/sidebarSortPreferences/actions.spec.js index 7fee1d018..5242f2125 100644 --- a/app/javascript/dashboard/store/modules/specs/sidebarSortPreferences/actions.spec.js +++ b/app/javascript/dashboard/store/modules/specs/sidebarSortPreferences/actions.spec.js @@ -83,7 +83,7 @@ describe('#actions', () => { ); }); - it('ignores invalid preferences', () => { + it('ignores invalid sort values', () => { actions.setSectionSort( { commit, @@ -95,7 +95,7 @@ describe('#actions', () => { }, { section: SIDEBAR_SORT_SECTIONS.FOLDERS, - sortBy: SIDEBAR_SORT_KEYS.UNREAD_COUNT_DESC, + sortBy: 'invalid_sort', } ); diff --git a/app/jobs/agents/destroy_job.rb b/app/jobs/agents/destroy_job.rb index 8596ca1cc..fe0276bc6 100644 --- a/app/jobs/agents/destroy_job.rb +++ b/app/jobs/agents/destroy_job.rb @@ -31,7 +31,11 @@ class Agents::DestroyJob < ApplicationJob def unassign_conversations(account, user) # rubocop:disable Rails/SkipsModelValidations - user.assigned_conversations.where(account: account).in_batches.update_all(assignee_id: nil) + unassigned_count = user.assigned_conversations.where(account: account).in_batches.update_all(assignee_id: nil) # rubocop:enable Rails/SkipsModelValidations + + return unless unassigned_count.positive? + + ::Conversations::UnreadCounts::FilteredCountInvalidator.new(account).conversation_changed! end end diff --git a/app/models/account_user.rb b/app/models/account_user.rb index bbcb0e010..cdacb9b0e 100644 --- a/app/models/account_user.rb +++ b/app/models/account_user.rb @@ -39,6 +39,8 @@ class AccountUser < ApplicationRecord 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 :invalidate_filtered_unread_count_visibility, on: [:create, :destroy] + after_update_commit :invalidate_filtered_unread_count_visibility_update, if: :filtered_unread_count_visibility_changed? validates :user_id, uniqueness: { scope: :account_id } @@ -79,6 +81,22 @@ class AccountUser < ApplicationRecord def update_presence_in_redis OnlineStatusTracker.set_status(account.id, user.id, availability) end + + def filtered_unread_count_visibility_changed? + previous_changes.key?('role') || previous_changes.key?('custom_role_id') + end + + def invalidate_filtered_unread_count_visibility + ::Conversations::UnreadCounts::FilteredCountInvalidator.new(account).user_visibility_changed!(user_id: user_id) + end + + def invalidate_filtered_unread_count_visibility_update + dispatch_account_cache_invalidated if invalidate_filtered_unread_count_visibility + end + + def dispatch_account_cache_invalidated + Rails.configuration.dispatcher.dispatch(ACCOUNT_CACHE_INVALIDATED, Time.zone.now, account: account, cache_keys: account.cache_keys) + end end AccountUser.prepend_mod_with('AccountUser') diff --git a/app/models/campaign.rb b/app/models/campaign.rb index 1d4da7712..479b5bdd0 100644 --- a/app/models/campaign.rb +++ b/app/models/campaign.rb @@ -53,6 +53,7 @@ class Campaign < ApplicationRecord before_validation :ensure_correct_campaign_attributes after_commit :set_display_id, unless: :display_id? + after_destroy_commit :invalidate_filtered_unread_count_filters def trigger! return unless one_off? @@ -88,6 +89,15 @@ class Campaign < ApplicationRecord end end + def invalidate_filtered_unread_count_filters + filters_changed = ::Conversations::UnreadCounts::FilteredCountInvalidator.new(account).conversation_changed! + dispatch_account_cache_invalidated if filters_changed + end + + def dispatch_account_cache_invalidated + Rails.configuration.dispatcher.dispatch(ACCOUNT_CACHE_INVALIDATED, Time.zone.now, account: account, cache_keys: account.cache_keys) + end + def set_display_id reload end diff --git a/app/models/concerns/account_cache_revalidator.rb b/app/models/concerns/account_cache_revalidator.rb index b5ff5a473..d2bfaf294 100644 --- a/app/models/concerns/account_cache_revalidator.rb +++ b/app/models/concerns/account_cache_revalidator.rb @@ -6,6 +6,8 @@ module AccountCacheRevalidator end def update_account_cache + return if account.blank? + account.update_cache_key(self.class.name.underscore) end end diff --git a/app/models/conversation.rb b/app/models/conversation.rb index d9c06c4d8..1e9c38c41 100644 --- a/app/models/conversation.rb +++ b/app/models/conversation.rb @@ -62,6 +62,14 @@ class Conversation < ApplicationRecord include PushDataHelper include ConversationMuteHelpers + CONVERSATION_UPDATED_ADDITIONAL_ATTRIBUTE_KEYS = %w[conversation_language].freeze + FILTERED_UNREAD_COUNT_ADDITIONAL_ATTRIBUTE_KEYS = %w[browser_language conversation_language mail_subject referer].freeze + FILTERED_UNREAD_COUNT_UPDATE_KEYS = %w[ + cached_label_list campaign_id custom_attributes first_reply_created_at label_list last_activity_at priority snoozed_until waiting_since + ].freeze + private_constant :CONVERSATION_UPDATED_ADDITIONAL_ATTRIBUTE_KEYS, :FILTERED_UNREAD_COUNT_ADDITIONAL_ATTRIBUTE_KEYS, + :FILTERED_UNREAD_COUNT_UPDATE_KEYS + validates :account_id, presence: true validates :inbox_id, presence: true validates :contact_id, presence: true @@ -247,6 +255,7 @@ class Conversation < ApplicationRecord handle_resolved_status_change notify_status_change create_activity + invalidate_filtered_unread_count_conversation notify_conversation_updation end @@ -313,10 +322,23 @@ class Conversation < ApplicationRecord end def allowed_keys? - ( - previous_changes.keys.intersect?(list_of_keys) || - (previous_changes['additional_attributes'].present? && previous_changes['additional_attributes'][1].keys.intersect?(%w[conversation_language])) - ) + previous_changes.keys.intersect?(list_of_keys) || + additional_attributes_changed?(CONVERSATION_UPDATED_ADDITIONAL_ATTRIBUTE_KEYS) + end + + def invalidate_filtered_unread_count_conversation + return unless filtered_unread_count_update? + + ::Conversations::UnreadCounts::FilteredCountInvalidator.new(account).conversation_changed! + end + + def filtered_unread_count_update? + previous_changes.keys.intersect?(FILTERED_UNREAD_COUNT_UPDATE_KEYS) || + additional_attributes_changed?(FILTERED_UNREAD_COUNT_ADDITIONAL_ATTRIBUTE_KEYS) + end + + def additional_attributes_changed?(keys) + Array(previous_changes['additional_attributes']).compact.any? { |attributes| attributes.keys.intersect?(keys) } end def load_attributes_created_by_db_triggers diff --git a/app/models/conversation_participant.rb b/app/models/conversation_participant.rb index 830eb7baa..4103deda4 100644 --- a/app/models/conversation_participant.rb +++ b/app/models/conversation_participant.rb @@ -28,6 +28,7 @@ class ConversationParticipant < ApplicationRecord belongs_to :user before_validation :ensure_account_id + after_commit :invalidate_filtered_unread_count_visibility, on: [:create, :destroy] private @@ -38,4 +39,8 @@ class ConversationParticipant < ApplicationRecord def ensure_inbox_access errors.add(:user, 'must have inbox access') if conversation && conversation.inbox.assignable_agents.exclude?(user) end + + def invalidate_filtered_unread_count_visibility + ::Conversations::UnreadCounts::FilteredCountInvalidator.new(account).user_visibility_changed!(user_id: user_id) + end end diff --git a/app/models/custom_attribute_definition.rb b/app/models/custom_attribute_definition.rb index 35f822335..8270cc70e 100644 --- a/app/models/custom_attribute_definition.rb +++ b/app/models/custom_attribute_definition.rb @@ -48,6 +48,8 @@ class CustomAttributeDefinition < ApplicationRecord belongs_to :account after_update :update_widget_pre_chat_custom_fields, unless: :company_attribute? after_destroy :sync_widget_pre_chat_custom_fields, unless: :company_attribute? + after_update_commit :invalidate_filtered_unread_count_filters_update, if: :conversation_attribute_before_or_after? + after_destroy_commit :invalidate_filtered_unread_count_filters_destroy, if: :conversation_attribute? private @@ -64,6 +66,27 @@ class CustomAttributeDefinition < ApplicationRecord ::Inboxes::UpdateWidgetPreChatCustomFieldsJob.perform_later(account, self) end + def invalidate_filtered_unread_count_filters_update + invalidate_filtered_unread_count_filters + end + + def invalidate_filtered_unread_count_filters_destroy + invalidate_filtered_unread_count_filters + end + + def invalidate_filtered_unread_count_filters + filters_changed = ::Conversations::UnreadCounts::FilteredCountInvalidator.new(account).custom_attribute_definition_changed!(self) + dispatch_account_cache_invalidated if filters_changed + end + + def dispatch_account_cache_invalidated + Rails.configuration.dispatcher.dispatch(ACCOUNT_CACHE_INVALIDATED, Time.zone.now, account: account, cache_keys: account.cache_keys) + end + + def conversation_attribute_before_or_after? + conversation_attribute? || attribute_model_previously_was == 'conversation_attribute' + end + def attribute_must_not_conflict model_keys = attribute_model.to_s.delete_suffix('_attribute').to_sym standard_attributes = STANDARD_ATTRIBUTES[model_keys] diff --git a/app/models/custom_filter.rb b/app/models/custom_filter.rb index 6d64c0447..6e2461a74 100644 --- a/app/models/custom_filter.rb +++ b/app/models/custom_filter.rb @@ -22,10 +22,31 @@ class CustomFilter < ApplicationRecord enum filter_type: { conversation: 0, contact: 1, report: 2 } validate :validate_number_of_filters + after_create_commit :invalidate_filtered_unread_count_create + after_update_commit :invalidate_filtered_unread_count_update + after_destroy_commit :invalidate_filtered_unread_count_destroy def validate_number_of_filters return true if account.custom_filters.where(user_id: user_id).size < Limits::MAX_CUSTOM_FILTERS_PER_USER errors.add :account_id, I18n.t('errors.custom_filters.number_of_records') end + + private + + def invalidate_filtered_unread_count_create + filtered_count_invalidator.custom_filter_created!(self) + end + + def invalidate_filtered_unread_count_update + filtered_count_invalidator.custom_filter_updated!(self) + end + + def invalidate_filtered_unread_count_destroy + filtered_count_invalidator.custom_filter_destroyed!(self) + end + + def filtered_count_invalidator + ::Conversations::UnreadCounts::FilteredCountInvalidator.new(account) + end end diff --git a/app/models/inbox.rb b/app/models/inbox.rb index 15bfe77dd..1b0cfc587 100644 --- a/app/models/inbox.rb +++ b/app/models/inbox.rb @@ -77,10 +77,12 @@ class Inbox < ApplicationRecord enum sender_name_type: { friendly: 0, professional: 1 } + before_destroy :capture_filtered_unread_count_user_ids, prepend: true after_destroy :delete_round_robin_agents after_create_commit :dispatch_create_event after_update_commit :dispatch_update_event + after_destroy_commit :invalidate_filtered_unread_counts_after_destroy scope :order_by_name, -> { order('lower(name) ASC') } @@ -261,6 +263,18 @@ class Inbox < ApplicationRecord ::AutoAssignment::InboxRoundRobinService.new(inbox: self).clear_queue end + def capture_filtered_unread_count_user_ids + return if account.blank? + + @filtered_unread_count_user_ids = (inbox_members.pluck(:user_id) + account.account_users.administrator.pluck(:user_id)).uniq + end + + def invalidate_filtered_unread_counts_after_destroy + invalidator = ::Conversations::UnreadCounts::FilteredCountInvalidator.new(account) + invalidator.conversation_changed! + invalidator.users_visibility_changed!(user_ids: @filtered_unread_count_user_ids) + end + def check_channel_type? ['Channel::Email', 'Channel::Api', 'Channel::WebWidget'].include?(channel_type) end diff --git a/app/models/inbox_member.rb b/app/models/inbox_member.rb index d4a36ccc8..bc4014da9 100644 --- a/app/models/inbox_member.rb +++ b/app/models/inbox_member.rb @@ -24,6 +24,7 @@ class InboxMember < ApplicationRecord after_create :add_agent_to_round_robin after_destroy :remove_agent_from_round_robin + after_commit :invalidate_filtered_unread_count_visibility, on: [:create, :destroy] private @@ -34,6 +35,10 @@ class InboxMember < ApplicationRecord def remove_agent_from_round_robin ::AutoAssignment::InboxRoundRobinService.new(inbox: inbox).remove_agent_from_queue(user_id) if inbox.present? end + + def invalidate_filtered_unread_count_visibility + ::Conversations::UnreadCounts::FilteredCountInvalidator.new(inbox&.account).user_visibility_changed!(user_id: user_id) + end end InboxMember.include_mod_with('Audit::InboxMember') diff --git a/app/models/team.rb b/app/models/team.rb index 15de8ab56..b19f215ca 100644 --- a/app/models/team.rb +++ b/app/models/team.rb @@ -25,6 +25,9 @@ class Team < ApplicationRecord has_many :members, through: :team_members, source: :user has_many :conversations, dependent: :nullify + before_destroy :capture_filtered_unread_count_member_ids, prepend: true + after_destroy_commit :invalidate_filtered_unread_counts_after_destroy + validates :name, presence: { message: I18n.t('errors.validations.presence') }, uniqueness: { scope: :account_id } @@ -69,6 +72,18 @@ class Team < ApplicationRecord icon_color: icon_color } end + + private + + def capture_filtered_unread_count_member_ids + @filtered_unread_count_member_ids = team_members.pluck(:user_id) + end + + def invalidate_filtered_unread_counts_after_destroy + invalidator = ::Conversations::UnreadCounts::FilteredCountInvalidator.new(account) + invalidator.conversation_changed! + invalidator.users_visibility_changed!(user_ids: @filtered_unread_count_member_ids) + end end Team.include_mod_with('Audit::Team') diff --git a/app/models/team_member.rb b/app/models/team_member.rb index f99af264b..0d0e0aa22 100644 --- a/app/models/team_member.rb +++ b/app/models/team_member.rb @@ -18,6 +18,14 @@ class TeamMember < ApplicationRecord belongs_to :user belongs_to :team validates :user_id, uniqueness: { scope: :team_id } + + after_commit :invalidate_filtered_unread_count_visibility, on: [:create, :destroy] + + private + + def invalidate_filtered_unread_count_visibility + ::Conversations::UnreadCounts::FilteredCountInvalidator.new(team&.account).user_visibility_changed!(user_id: user_id) + end end TeamMember.include_mod_with('Audit::TeamMember') diff --git a/app/services/conversations/unread_counts.rb b/app/services/conversations/unread_counts.rb index 668395063..1b3ee3fb2 100644 --- a/app/services/conversations/unread_counts.rb +++ b/app/services/conversations/unread_counts.rb @@ -1,4 +1,10 @@ module Conversations::UnreadCounts READY_TTL = 24.hours.to_i SET_TTL = 25.hours.to_i + FILTERED_COUNT_FRESH_TTL = 5.minutes.to_i + FILTERED_COUNT_STALE_WINDOW = 30.minutes.to_i + FILTERED_COUNT_REDIS_TTL = FILTERED_COUNT_FRESH_TTL + FILTERED_COUNT_STALE_WINDOW + FILTERED_COUNT_VERSION_TTL = SET_TTL + FILTERED_COUNT_MIN_REFRESH_INTERVAL = 30.seconds.to_i + MAX_INLINE_FILTER_BUILDS = 10 end diff --git a/app/services/conversations/unread_counts/counter.rb b/app/services/conversations/unread_counts/counter.rb index b1ba5ddb0..2a015f3a6 100644 --- a/app/services/conversations/unread_counts/counter.rb +++ b/app/services/conversations/unread_counts/counter.rb @@ -14,7 +14,7 @@ class Conversations::UnreadCounts::Counter end def perform - return empty_counts if permission_mode == :none + return with_filtered_counts(empty_counts, build: false) if permission_mode == :none ensure_base_cache! ensure_assignment_cache! if assignment_mode? @@ -26,11 +26,17 @@ class Conversations::UnreadCounts::Counter inboxes: inbox_counts, labels: unread_label_counts, teams: unread_team_counts - } + }.then { |counts| with_filtered_counts(counts) } end private + def with_filtered_counts(counts, build: true) + return counts unless account.feature_enabled?(::Conversations::UnreadCounts::FilteredCounter::FEATURE_FLAG) + + counts.merge(build ? filtered_counter.perform : ::Conversations::UnreadCounts::FilteredCounter.empty_counts) + end + def ensure_base_cache! ensure_cache_ready!( ready: -> { store.base_ready?(account.id) }, @@ -200,4 +206,8 @@ class Conversations::UnreadCounts::Counter def store ::Conversations::UnreadCounts::Store end + + def filtered_counter + @filtered_counter ||= ::Conversations::UnreadCounts::FilteredCounter.new(account: account, user: user) + end end diff --git a/app/services/conversations/unread_counts/filter_query_counter.rb b/app/services/conversations/unread_counts/filter_query_counter.rb new file mode 100644 index 000000000..238bed2a1 --- /dev/null +++ b/app/services/conversations/unread_counts/filter_query_counter.rb @@ -0,0 +1,151 @@ +class Conversations::UnreadCounts::FilterQueryCounter < Conversations::FilterService + BOOLEAN_VALUES = %w[0 1 false f n no off on t true y yes].freeze + DATABASE_CAST_ERROR_CLASS_NAMES = %w[ + PG::DatetimeFieldOverflow + PG::InvalidDatetimeFormat + PG::InvalidTextRepresentation + PG::NumericValueOutOfRange + ].freeze + DAYS_BEFORE_FILTER_OPERATOR = 'days_before'.freeze + MALFORMED_QUERY_ERRORS = [NoMethodError, TypeError].freeze + NUMERIC_ATTRIBUTE_KEYS = %w[assignee_id inbox_id].freeze + TEXT_DATA_TYPES = %w[labels link text text_case_insensitive].freeze + TEXT_FILTER_OPERATORS = %w[contains does_not_contain].freeze + TYPED_DATA_TYPES = %w[boolean date number numeric].freeze + VALID_QUERY_OPERATORS = %w[AND OR].freeze + VALIDATION_DATA_TYPES = (TEXT_DATA_TYPES + TYPED_DATA_TYPES).freeze + VALUELESS_FILTER_OPERATORS = %w[is_present is_not_present].freeze + + def initialize(account:, user:, query:) + super(query.with_indifferent_access, user, account) + end + + def perform + return unless valid_query? + return unless valid_typed_values? + + validate_query_operator + query_builder(@filters['conversations']).count + rescue *MALFORMED_QUERY_ERRORS + nil + rescue ActiveRecord::StatementInvalid => e + raise unless database_cast_error?(e) + + nil + end + + def base_relation + Conversations::PermissionFilterService.new(unread_conversations, @user, @account).perform + end + + private + + def valid_query? + @params[:payload].is_a?(Array) && valid_query_operator_positions? + end + + def database_cast_error?(error) + DATABASE_CAST_ERROR_CLASS_NAMES.include?(error.cause&.class&.name) + end + + def valid_query_operator_positions? + @params[:payload].each_with_index.all? do |query_hash, index| + query_operator_position_valid?(query_hash[:query_operator], last_query?(index)) + end + end + + def query_operator_position_valid?(query_operator, last_query) + return query_operator.blank? if last_query + + VALID_QUERY_OPERATORS.include?(query_operator.to_s.upcase) + end + + def last_query?(index) + index == @params[:payload].length - 1 + end + + def valid_typed_values? + @params[:payload].all? do |query_hash| + next true if VALUELESS_FILTER_OPERATORS.include?(query_hash[:filter_operator]) + + data_type = validation_data_type(query_hash) + next true if data_type.blank? + next false if text_filter_operator?(query_hash) && TYPED_DATA_TYPES.include?(data_type) + + valid_typed_values_for?(query_hash[:values], data_type, query_hash[:filter_operator]) + end + end + + def validation_data_type(query_hash) + attribute_key = query_hash[:attribute_key] + data_type = filter_data_type(query_hash) + + return nil if text_search_on_display_id?(query_hash) + return 'number' if NUMERIC_ATTRIBUTE_KEYS.include?(attribute_key) + return data_type if VALIDATION_DATA_TYPES.include?(data_type) + + nil + end + + def filter_data_type(query_hash) + attribute_key = query_hash[:attribute_key] + data_type = @filters.dig('conversations', attribute_key, 'data_type') + return data_type.to_s.downcase if data_type.present? + + custom_attribute_data_type(query_hash) + end + + def custom_attribute_data_type(query_hash) + custom_attribute_type = query_hash[:custom_attribute_type].presence || self.class::ATTRIBUTE_MODEL + custom_attribute = @account.custom_attribute_definitions.where( + attribute_model: custom_attribute_type + ).find_by(attribute_key: query_hash[:attribute_key]) + + self.class::ATTRIBUTE_TYPES[custom_attribute&.attribute_display_type].to_s + end + + def valid_typed_values_for?(values, data_type, filter_operator) + Array.wrap(values).all? do |value| + valid_typed_value?(value, data_type, filter_operator) + end + end + + def text_filter_operator?(query_hash) + TEXT_FILTER_OPERATORS.include?(query_hash[:filter_operator]) + end + + def valid_typed_value?(value, data_type, filter_operator) + case data_type + when 'boolean' + BOOLEAN_VALUES.include?(value.to_s.downcase) + when 'date' + return Integer(value.to_s, exception: false).present? if filter_operator == DAYS_BEFORE_FILTER_OPERATOR + + Date.iso8601(value.to_s).present? + when 'numeric' + BigDecimal(value.to_s, exception: false).present? + when *TEXT_DATA_TYPES + value.is_a?(String) + else + Integer(value.to_s, exception: false).present? + end + rescue ArgumentError + false + end + + def unread_conversations + @account.conversations + .joins(:messages) + .merge(Message.incoming.reorder(nil)) + .where(messages: { account_id: @account.id }) + .where(unread_since_last_seen_condition) + .distinct + end + + def unread_since_last_seen_condition + conversations = Conversation.arel_table + messages = Message.arel_table + + conversations[:agent_last_seen_at].eq(nil).or(messages[:created_at].gt(conversations[:agent_last_seen_at])) + end +end diff --git a/app/services/conversations/unread_counts/filtered_count_instrumentation.rb b/app/services/conversations/unread_counts/filtered_count_instrumentation.rb new file mode 100644 index 000000000..707c63f33 --- /dev/null +++ b/app/services/conversations/unread_counts/filtered_count_instrumentation.rb @@ -0,0 +1,188 @@ +class Conversations::UnreadCounts::FilteredCountInstrumentation + # Centralizes the rollout-critical filtered unread count signals: + # API response duration, counter duration, snapshot build duration, snapshot state distribution, + # refresh claim rate, build lock acquisition rate, and invalidation/version bump rate. + EVENT_NAME = 'FilteredUnreadCounts'.freeze + METRIC_PREFIX = 'Custom/Conversations/UnreadCounts/Filtered'.freeze + SUMMARY_KEY = :filtered_unread_counts_request_summary + AGGREGATED_INCREMENT_OPERATIONS = %i[snapshot_state refresh_claim build_lock].freeze + SNAPSHOT_STATUSES = %i[fresh stale missing expired].freeze + SNAPSHOT_SCOPES = %i[built_in_filter folder_index filter].freeze + SUMMARY_DEFAULTS = begin + defaults = { + snapshot_total_count: 0, + refresh_claimed_count: 0, + refresh_skipped_count: 0, + build_lock_acquired_count: 0, + build_lock_missed_count: 0, + snapshot_build_success_count: 0, + snapshot_build_error_count: 0 + } + + SNAPSHOT_STATUSES.each { |status| defaults[:"snapshot_#{status}_count"] = 0 } + SNAPSHOT_SCOPES.each do |scope| + defaults[:"#{scope}_snapshot_count"] = 0 + defaults[:"#{scope}_refresh_claimed_count"] = 0 + defaults[:"#{scope}_refresh_skipped_count"] = 0 + defaults[:"#{scope}_build_lock_acquired_count"] = 0 + defaults[:"#{scope}_build_lock_missed_count"] = 0 + defaults[:"#{scope}_snapshot_build_success_count"] = 0 + defaults[:"#{scope}_snapshot_build_error_count"] = 0 + end + + defaults.freeze + end + private_constant :SUMMARY_KEY, :AGGREGATED_INCREMENT_OPERATIONS, :SNAPSHOT_STATUSES, :SNAPSHOT_SCOPES, :SUMMARY_DEFAULTS + + class << self + def summarize_request(account_id:) + previous_summary = current_summary + summary = request_summary(account_id) + Thread.current[SUMMARY_KEY] = summary + started_at = monotonic_time + status = :success + + yield + rescue StandardError => e + status = :error + summary[:error_class] = e.class.name + raise + ensure + record_request_summary(summary, status, started_at) + Thread.current[SUMMARY_KEY] = previous_summary + end + + def observe(operation, attributes = {}) + started_at = monotonic_time + + yield.tap do + record_observation(operation, attributes, started_at, status: :success) + end + rescue StandardError => e + record_observation(operation, attributes.merge(error_class: e.class.name), started_at, status: :error) + raise + end + + def increment(operation, attributes = {}) + record_increment_summary(operation, attributes) if aggregated_increment?(operation) + record_event(operation, attributes) unless aggregated_increment?(operation) + record_metric("#{metric_name(operation)}/count", 1) + end + + def record_event(operation, attributes = {}) + agent = new_relic_agent + return unless agent.respond_to?(:record_custom_event) + + agent.record_custom_event(EVENT_NAME, sanitized_attributes(attributes.merge(operation: operation))) + rescue StandardError + nil + end + + private + + def record_observation(operation, attributes, started_at, status:) + duration_ms = elapsed_ms_since(started_at) + record_observation_summary(operation, attributes, status: status) + record_metric("#{metric_name(operation)}/duration_ms", duration_ms) + end + + def record_request_summary(summary, status, started_at) + duration_ms = elapsed_ms_since(started_at) + summary[:status] = status + summary[:duration_ms] = duration_ms + record_metric("#{metric_name(:api_response)}/duration_ms", duration_ms) + record_event(:request_summary, summary) + end + + def record_metric(name, value) + agent = new_relic_agent + return unless agent.respond_to?(:record_metric) + + agent.record_metric(name, value) + rescue StandardError + nil + end + + def metric_name(operation) + "#{METRIC_PREFIX}/#{operation}" + end + + def request_summary(account_id) + SUMMARY_DEFAULTS.dup.merge(account_id: account_id) + end + + def current_summary + Thread.current[SUMMARY_KEY] + end + + def aggregated_increment?(operation) + operation.in?(AGGREGATED_INCREMENT_OPERATIONS) + end + + def record_increment_summary(operation, attributes) + case operation + when :snapshot_state + record_snapshot_state_summary(attributes) + when :refresh_claim + result = attributes[:claimed] ? :claimed : :skipped + increment_summary_count(:"refresh_#{result}_count") + increment_scoped_summary_count(attributes[:snapshot_scope], :"refresh_#{result}_count") + when :build_lock + result = attributes[:acquired] ? :acquired : :missed + increment_summary_count(:"build_lock_#{result}_count") + increment_scoped_summary_count(attributes[:snapshot_scope], :"build_lock_#{result}_count") + end + end + + def record_snapshot_state_summary(attributes) + increment_summary_count(:snapshot_total_count) + increment_summary_count(:"snapshot_#{attributes[:snapshot_status]}_count") + increment_scoped_summary_count(attributes[:snapshot_scope], :snapshot_count) + end + + def record_observation_summary(operation, attributes, status:) + return unless operation == :snapshot_build + + result = status == :success ? :success : :error + increment_summary_count(:"snapshot_build_#{result}_count") + increment_scoped_summary_count(attributes[:snapshot_scope], :"snapshot_build_#{result}_count") + end + + def increment_scoped_summary_count(scope, suffix) + return if scope.blank? + + increment_summary_count(:"#{scope}_#{suffix}") + end + + def increment_summary_count(key) + return if current_summary.blank? + + current_summary[key] = current_summary.fetch(key, 0) + 1 + end + + def sanitized_attributes(attributes) + attributes.compact.transform_values do |value| + case value + when String, Integer, Float, TrueClass, FalseClass + value + else + value.to_s + end + end + end + + def elapsed_ms_since(started_at) + ((monotonic_time - started_at) * 1000).round(2) + end + + def monotonic_time + Process.clock_gettime(Process::CLOCK_MONOTONIC) + end + + def new_relic_agent + return unless defined?(::NewRelic::Agent) + + ::NewRelic::Agent + end + end +end diff --git a/app/services/conversations/unread_counts/filtered_count_invalidator.rb b/app/services/conversations/unread_counts/filtered_count_invalidator.rb new file mode 100644 index 000000000..6546bb58f --- /dev/null +++ b/app/services/conversations/unread_counts/filtered_count_invalidator.rb @@ -0,0 +1,184 @@ +class Conversations::UnreadCounts::FilteredCountInvalidator + FEATURE_FLAG = 'unread_count_for_filters'.freeze + + attr_reader :account + + def initialize(account) + @account = account + end + + def conversation_changed! + return false unless enabled? + + version = store.bump_conversation_version!(account.id) + record_invalidation(:conversation, reason: :conversation_changed, version: version) + true + end + + def user_visibility_changed!(user_id:) + return false unless enabled? && user_id.present? + + version = store.bump_built_in_filter_version!(account_id: account.id, user_id: user_id) + record_invalidation(:built_in_filter, reason: :user_visibility_changed, version: version) + true + end + + def users_visibility_changed!(user_ids:) + return false unless enabled? + + user_ids = Array(user_ids).compact_blank.uniq + return false if user_ids.blank? + + bump_built_in_filter_versions(user_ids).each_value do |version| + record_invalidation(:built_in_filter, reason: :user_visibility_changed, version: version) + end + true + end + + def custom_filter_created!(custom_filter) + return false unless conversation_filter?(custom_filter) + + bump_folder_index_version!(custom_filter, reason: :custom_filter_created) + bump_filter_version!(custom_filter, reason: :custom_filter_created) + true + end + + def custom_filter_updated!(custom_filter) + return false unless enabled? && conversation_filter_before_or_after?(custom_filter) + + filter_type_changed = filter_type_changed?(custom_filter) + query_changed = query_changed?(custom_filter) + return false unless filter_type_changed || query_changed + + bump_folder_index_version!(custom_filter, reason: :custom_filter_updated) if filter_type_changed + bump_filter_version!(custom_filter, reason: :custom_filter_updated) + store.delete_filter_count!(account_id: account.id, filter_id: custom_filter.id) if moved_out_of_conversation_filters?(custom_filter) + true + end + + def custom_filter_destroyed!(custom_filter) + return false unless conversation_filter?(custom_filter) + + bump_folder_index_version!(custom_filter, reason: :custom_filter_destroyed) + store.delete_filter_count!(account_id: account.id, filter_id: custom_filter.id) + true + end + + def custom_attribute_definition_changed!(custom_attribute_definition) + return false unless enabled? && conversation_attribute_before_or_after?(custom_attribute_definition) + + affected_filters = affected_custom_attribute_filters(custom_attribute_definition) + return false if affected_filters.blank? + + affected_filters.each { |custom_filter| bump_filter_version!(custom_filter, reason: :custom_attribute_definition_changed) } + true + end + + private + + def bump_built_in_filter_versions(user_ids) + results = Redis::Alfred.pipelined do |pipeline| + user_ids.each do |user_id| + key = store.built_in_filter_version_key(account.id, user_id) + pipeline.incr(key) + pipeline.expire(key, Conversations::UnreadCounts::FILTERED_COUNT_VERSION_TTL) + end + end + + user_ids.zip(results.each_slice(2).map(&:first)).to_h + end + + def enabled? + account&.feature_enabled?(FEATURE_FLAG) + end + + def conversation_filter?(custom_filter) + enabled? && custom_filter.conversation? + end + + def conversation_filter_before_or_after?(custom_filter) + custom_filter.conversation? || previous_filter_type(custom_filter) == 'conversation' + end + + def moved_out_of_conversation_filters?(custom_filter) + filter_type_changed?(custom_filter) && previous_filter_type(custom_filter) == 'conversation' && !custom_filter.conversation? + end + + def filter_type_changed?(custom_filter) + custom_filter.previous_changes.key?('filter_type') + end + + def query_changed?(custom_filter) + custom_filter.previous_changes.key?('query') + end + + def previous_filter_type(custom_filter) + raw_filter_type = custom_filter.previous_changes.dig('filter_type', 0) + return if raw_filter_type.blank? + return raw_filter_type if CustomFilter.filter_types.key?(raw_filter_type) + return CustomFilter.filter_types.key(raw_filter_type) if raw_filter_type.is_a?(Integer) + + CustomFilter.filter_types.key(raw_filter_type.to_i) || raw_filter_type.to_s + end + + def conversation_attribute_before_or_after?(custom_attribute_definition) + custom_attribute_definition.conversation_attribute? || previous_attribute_model(custom_attribute_definition) == 'conversation_attribute' + end + + def previous_attribute_model(custom_attribute_definition) + raw_attribute_model = custom_attribute_definition.previous_changes.dig('attribute_model', 0) + return if raw_attribute_model.blank? + return raw_attribute_model if CustomAttributeDefinition.attribute_models.key?(raw_attribute_model) + return CustomAttributeDefinition.attribute_models.key(raw_attribute_model) if raw_attribute_model.is_a?(Integer) + + CustomAttributeDefinition.attribute_models.key(raw_attribute_model.to_i) || raw_attribute_model.to_s + end + + def affected_custom_attribute_filters(custom_attribute_definition) + attribute_keys = custom_attribute_keys(custom_attribute_definition) + account.custom_filters.conversation.select do |custom_filter| + custom_filter_references_conversation_attribute?(custom_filter, attribute_keys) + end + end + + def custom_attribute_keys(custom_attribute_definition) + [custom_attribute_definition.attribute_key, custom_attribute_definition.previous_changes.dig('attribute_key', 0)].compact_blank.map(&:to_s).uniq + end + + def custom_filter_references_conversation_attribute?(custom_filter, attribute_keys) + payload = custom_filter.query.with_indifferent_access[:payload] + Array(payload).any? do |condition| + condition = condition.with_indifferent_access + condition[:attribute_key].to_s.in?(attribute_keys) && + (condition[:custom_attribute_type].presence || 'conversation_attribute') == 'conversation_attribute' + end + end + + def bump_folder_index_version!(custom_filter, reason:) + version = store.bump_folder_index_version!(account_id: account.id, user_id: custom_filter.user_id) + record_invalidation(:folder_index, reason: reason, version: version) + end + + def bump_filter_version!(custom_filter, reason:) + version = store.bump_filter_version!(account_id: account.id, filter_id: custom_filter.id) + record_invalidation(:filter, reason: reason, version: version) + end + + def record_invalidation(scope, reason:, version:) + instrumentation.increment( + :invalidation, + account_id: account.id, + invalidation_scope: scope, + reason: reason, + version: version + ) + end + + def store + ::Conversations::UnreadCounts::FilteredCountStore + end + + def instrumentation + ::Conversations::UnreadCounts::FilteredCountInstrumentation + end +end diff --git a/app/services/conversations/unread_counts/filtered_count_snapshot_resolver.rb b/app/services/conversations/unread_counts/filtered_count_snapshot_resolver.rb new file mode 100644 index 000000000..8685aa891 --- /dev/null +++ b/app/services/conversations/unread_counts/filtered_count_snapshot_resolver.rb @@ -0,0 +1,67 @@ +class Conversations::UnreadCounts::FilteredCountSnapshotResolver + BUILD_LOCK_TTL = 15.minutes.to_i + + attr_reader :account, :now, :store, :lock_manager + + def initialize(account:, now:, store:, lock_manager:) + @account = account + @now = now + @store = store + @lock_manager = lock_manager + end + + # Version mismatches make a snapshot stale immediately, but refresh_after keeps DB rebuilds throttled. + def resolve(scope:, state:, lock_key:, claim_refresh:, &) + record_snapshot_state(scope, state) + return state.payload if state.fresh? + + stale_payload = state.payload if state.stale? + return stale_payload if refresh_not_due?(stale_payload) + return stale_payload unless refresh_claimed?(scope, claim_refresh) + + build_with_lock(scope, lock_key, stale_payload, &) + end + + private + + def record_snapshot_state(scope, state) + instrumentation.increment( + :snapshot_state, + account_id: account.id, + snapshot_scope: scope, + snapshot_status: state.status + ) + end + + def refresh_not_due?(stale_payload) + stale_payload.present? && !store.refresh_due?(stale_payload, now: now) + end + + def refresh_claimed?(scope, claim_refresh) + claimed = claim_refresh.call + instrumentation.increment(:refresh_claim, account_id: account.id, snapshot_scope: scope, claimed: claimed) + claimed + end + + def build_with_lock(scope, lock_key, stale_payload, &) + built_payload = nil + lock_acquired = false + + begin + lock_manager.with_lock(lock_key, BUILD_LOCK_TTL) do + lock_acquired = true + built_payload = instrumentation.observe(:snapshot_build, account_id: account.id, snapshot_scope: scope, &) + rescue ActiveRecord::StatementInvalid + built_payload = stale_payload + end + ensure + instrumentation.increment(:build_lock, account_id: account.id, snapshot_scope: scope, acquired: lock_acquired) + end + + lock_acquired ? built_payload : stale_payload + end + + def instrumentation + ::Conversations::UnreadCounts::FilteredCountInstrumentation + end +end diff --git a/app/services/conversations/unread_counts/filtered_count_store.rb b/app/services/conversations/unread_counts/filtered_count_store.rb new file mode 100644 index 000000000..b88e01627 --- /dev/null +++ b/app/services/conversations/unread_counts/filtered_count_store.rb @@ -0,0 +1,214 @@ +class Conversations::UnreadCounts::FilteredCountStore + extend Conversations::UnreadCounts::FilteredCountStoreKeys + + SnapshotResult = Struct.new(:status, :payload, keyword_init: true) do + def fresh? = status == :fresh + def stale? = status == :stale + def expired? = status == :expired + def missing? = status == :missing + end + + VERSION_KEY_METHODS = { + conversation: :conversation_version_key, + built_in_filter: :built_in_filter_version_key, + folder_index: :folder_index_version_key, + filter: :filter_version_key + }.freeze + REFRESH_THROTTLE_KEY_METHODS = { + built_in_filter: :built_in_filter_refresh_throttle_key, + folder_index: :folder_index_refresh_throttle_key, + filter: :filter_refresh_throttle_key + }.freeze + private_constant :VERSION_KEY_METHODS, :REFRESH_THROTTLE_KEY_METHODS + + class << self + def conversation_version(account_id) = current_version_for(:conversation, account_id) + def built_in_filter_version(account_id:, user_id:) = current_version_for(:built_in_filter, account_id, user_id) + def folder_index_version(account_id:, user_id:) = current_version_for(:folder_index, account_id, user_id) + def filter_version(account_id:, filter_id:) = current_version_for(:filter, account_id, filter_id) + + def bump_conversation_version!(account_id) = bump_version_for!(:conversation, account_id) + def bump_built_in_filter_version!(account_id:, user_id:) = bump_version_for!(:built_in_filter, account_id, user_id) + def bump_folder_index_version!(account_id:, user_id:) = bump_version_for!(:folder_index, account_id, user_id) + def bump_filter_version!(account_id:, filter_id:) = bump_version_for!(:filter, account_id, filter_id) + + # Keep version dimensions explicit so callers cannot write a snapshot without the freshness contract it depends on. + def write_built_in_filter_counts!(account_id:, user_id:, counts:, account_version:, built_in_filter_version:, built_at: Time.current, meta: {}) # rubocop:disable Metrics/ParameterLists + payload = snapshot_payload(built_at).merge( + account_version: account_version, + built_in_filter_version: built_in_filter_version, + user_id: user_id, + counts: counts, + meta: meta + ) + write_snapshot(built_in_filter_counts_key(account_id, user_id), payload) + end + + def built_in_filter_counts(account_id:, user_id:) + read_snapshot(built_in_filter_counts_key(account_id, user_id)) + end + + def built_in_filter_counts_state(account_id:, user_id:, versions: nil, now: Time.current) + snapshot_state( + built_in_filter_counts(account_id: account_id, user_id: user_id), + versions: versions || { + account_version: conversation_version(account_id), + built_in_filter_version: built_in_filter_version(account_id: account_id, user_id: user_id) + }, + now: now + ) + end + + def write_folder_index!(account_id:, user_id:, filter_ids:, folder_index_version:, built_at: Time.current) + payload = snapshot_payload(built_at).merge( + folder_index_version: folder_index_version, + user_id: user_id, + filter_ids: Array(filter_ids).map(&:to_i) + ) + write_snapshot(folder_index_key(account_id, user_id), payload) + end + + def folder_index(account_id:, user_id:) + read_snapshot(folder_index_key(account_id, user_id)) + end + + def folder_index_state(account_id:, user_id:, versions: nil, now: Time.current) + snapshot_state( + folder_index(account_id: account_id, user_id: user_id), + versions: versions || { folder_index_version: folder_index_version(account_id: account_id, user_id: user_id) }, + now: now + ) + end + + # Saved folder snapshots depend on account, filter, and owner visibility versions; keep all three visible at the callsite. + def write_filter_count!(account_id:, filter_id:, user_id:, count:, account_version:, filter_version:, owner_built_in_filter_version:, # rubocop:disable Metrics/ParameterLists + built_at: Time.current, meta: {}) + payload = snapshot_payload(built_at).merge( + account_version: account_version, + filter_version: filter_version, + owner_built_in_filter_version: owner_built_in_filter_version, + filter_id: filter_id, + user_id: user_id, + count: count, + meta: meta + ) + write_snapshot(filter_count_key(account_id, filter_id), payload) + end + + def filter_count(account_id:, filter_id:) + read_snapshot(filter_count_key(account_id, filter_id)) + end + + def filter_count_state(account_id:, filter_id:, owner_user_id: nil, versions: nil, now: Time.current) + snapshot = filter_count(account_id: account_id, filter_id: filter_id) + return SnapshotResult.new(status: :missing, payload: nil) if snapshot.blank? + + owner_user_id ||= snapshot[:user_id] + snapshot_state( + snapshot, + versions: versions || { + account_version: conversation_version(account_id), + filter_version: filter_version(account_id: account_id, filter_id: filter_id), + owner_built_in_filter_version: built_in_filter_version(account_id: account_id, user_id: owner_user_id) + }, + now: now + ) + end + + def refresh_due?(snapshot, now: Time.current) + return true if snapshot.blank? + + refresh_after = parse_time(snapshot[:refresh_after]) + refresh_after.blank? || now >= refresh_after + end + + def claim_built_in_filter_refresh!(account_id:, user_id:) = claim_refresh_for!(:built_in_filter, account_id, user_id) + def claim_folder_index_refresh!(account_id:, user_id:) = claim_refresh_for!(:folder_index, account_id, user_id) + def claim_filter_refresh!(account_id:, filter_id:) = claim_refresh_for!(:filter, account_id, filter_id) + + def delete_filter_count!(account_id:, filter_id:) + Redis::Alfred.delete(filter_count_key(account_id, filter_id)) + end + + private + + # Keep the public API domain-specific while centralizing direct Redis version/throttle operations. + def current_version_for(scope, *key_args) + current_version(public_send(VERSION_KEY_METHODS.fetch(scope), *key_args)) + end + + def bump_version_for!(scope, *key_args) + key = public_send(VERSION_KEY_METHODS.fetch(scope), *key_args) + + Redis::Alfred.with do |conn| + conn.multi do |transaction| + transaction.incr(key) + transaction.expire(key, Conversations::UnreadCounts::FILTERED_COUNT_VERSION_TTL) + end.first + end + end + + def claim_refresh_for!(scope, *key_args) + claim_refresh_throttle(public_send(REFRESH_THROTTLE_KEY_METHODS.fetch(scope), *key_args)) + end + + def current_version(key) + Redis::Alfred.get(key).to_i + end + + def snapshot_payload(built_at) + # expires_at marks the end of the fresh window. Redis keeps the snapshot for the additional stale window. + { + built_at: built_at.iso8601, + refresh_after: (built_at + Conversations::UnreadCounts::FILTERED_COUNT_MIN_REFRESH_INTERVAL).iso8601, + expires_at: (built_at + Conversations::UnreadCounts::FILTERED_COUNT_FRESH_TTL).iso8601 + } + end + + def write_snapshot(key, payload) + Redis::Alfred.setex(key, JSON.generate(payload), Conversations::UnreadCounts::FILTERED_COUNT_REDIS_TTL) + end + + def read_snapshot(key) + value = Redis::Alfred.get(key) + return if value.blank? + + JSON.parse(value, symbolize_names: true) + end + + def snapshot_state(snapshot, versions:, now:) + return SnapshotResult.new(status: :missing, payload: nil) if snapshot.blank? + return SnapshotResult.new(status: :expired, payload: snapshot) unless inside_stale_window?(snapshot, now) + return SnapshotResult.new(status: :fresh, payload: snapshot) if versions_match?(snapshot, versions) && inside_fresh_window?(snapshot, now) + + SnapshotResult.new(status: :stale, payload: snapshot) + end + + def versions_match?(snapshot, versions) + versions.all? { |key, value| snapshot[key].to_i == value.to_i } + end + + def inside_fresh_window?(snapshot, now) + expires_at = parse_time(snapshot[:expires_at]) + expires_at.present? && now <= expires_at + end + + def inside_stale_window?(snapshot, now) + expires_at = parse_time(snapshot[:expires_at]) + expires_at.present? && now <= expires_at + Conversations::UnreadCounts::FILTERED_COUNT_STALE_WINDOW + end + + def parse_time(value) + return value if value.is_a?(Time) || value.is_a?(ActiveSupport::TimeWithZone) + return value.to_time if value.respond_to?(:to_time) && !value.is_a?(String) + + Time.zone.parse(value.to_s) + rescue ArgumentError, TypeError + nil + end + + def claim_refresh_throttle(key) + Redis::Alfred.set(key, Time.current.to_i, nx: true, ex: Conversations::UnreadCounts::FILTERED_COUNT_MIN_REFRESH_INTERVAL) ? true : false + end + end +end diff --git a/app/services/conversations/unread_counts/filtered_count_store_keys.rb b/app/services/conversations/unread_counts/filtered_count_store_keys.rb new file mode 100644 index 000000000..5072503eb --- /dev/null +++ b/app/services/conversations/unread_counts/filtered_count_store_keys.rb @@ -0,0 +1,67 @@ +module Conversations::UnreadCounts::FilteredCountStoreKeys + def conversation_version_key(account_id) + account_key(Redis::Alfred::UNREAD_CONVERSATIONS_V2_CONVERSATION_VERSION, account_id) + end + + def built_in_filter_version_key(account_id, user_id) + user_key(Redis::Alfred::UNREAD_CONVERSATIONS_V2_BUILT_IN_FILTER_VERSION, account_id, user_id) + end + + def built_in_filter_counts_key(account_id, user_id) + user_key(Redis::Alfred::UNREAD_CONVERSATIONS_V2_BUILT_IN_FILTER_COUNTS, account_id, user_id) + end + + def built_in_filter_build_lock_key(account_id, user_id) + user_key(Redis::Alfred::UNREAD_CONVERSATIONS_V2_BUILT_IN_FILTER_BUILD_LOCK, account_id, user_id) + end + + def built_in_filter_refresh_throttle_key(account_id, user_id) + user_key(Redis::Alfred::UNREAD_CONVERSATIONS_V2_BUILT_IN_FILTER_REFRESH_THROTTLE, account_id, user_id) + end + + def folder_index_version_key(account_id, user_id) + user_key(Redis::Alfred::UNREAD_CONVERSATIONS_V2_FOLDER_INDEX_VERSION, account_id, user_id) + end + + def folder_index_key(account_id, user_id) + user_key(Redis::Alfred::UNREAD_CONVERSATIONS_V2_FOLDER_INDEX, account_id, user_id) + end + + def folder_index_build_lock_key(account_id, user_id) + user_key(Redis::Alfred::UNREAD_CONVERSATIONS_V2_FOLDER_INDEX_BUILD_LOCK, account_id, user_id) + end + + def folder_index_refresh_throttle_key(account_id, user_id) + user_key(Redis::Alfred::UNREAD_CONVERSATIONS_V2_FOLDER_INDEX_REFRESH_THROTTLE, account_id, user_id) + end + + def filter_version_key(account_id, filter_id) + filter_key(Redis::Alfred::UNREAD_CONVERSATIONS_V2_FILTER_VERSION, account_id, filter_id) + end + + def filter_count_key(account_id, filter_id) + filter_key(Redis::Alfred::UNREAD_CONVERSATIONS_V2_FILTER_COUNT, account_id, filter_id) + end + + def filter_build_lock_key(account_id, filter_id) + filter_key(Redis::Alfred::UNREAD_CONVERSATIONS_V2_FILTER_BUILD_LOCK, account_id, filter_id) + end + + def filter_refresh_throttle_key(account_id, filter_id) + filter_key(Redis::Alfred::UNREAD_CONVERSATIONS_V2_FILTER_REFRESH_THROTTLE, account_id, filter_id) + end + + private + + def account_key(format_string, account_id) + format(format_string, account_id: account_id) + end + + def user_key(format_string, account_id, user_id) + format(format_string, account_id: account_id, user_id: user_id) + end + + def filter_key(format_string, account_id, filter_id) + format(format_string, account_id: account_id, filter_id: filter_id) + end +end diff --git a/app/services/conversations/unread_counts/filtered_count_version_cache.rb b/app/services/conversations/unread_counts/filtered_count_version_cache.rb new file mode 100644 index 000000000..5aa73269a --- /dev/null +++ b/app/services/conversations/unread_counts/filtered_count_version_cache.rb @@ -0,0 +1,29 @@ +class Conversations::UnreadCounts::FilteredCountVersionCache + attr_reader :account, :user, :store + + def initialize(account:, user:, store:) + @account = account + @user = user + @store = store + end + + def built_in_filter = { account_version: account_version, built_in_filter_version: built_in_filter_version } + + def folder_index = { folder_index_version: store.folder_index_version(account_id: account.id, user_id: user.id) } + + def filter(filter_id) + { + account_version: account_version, + filter_version: filter_version(filter_id), + owner_built_in_filter_version: built_in_filter_version + } + end + + private + + def account_version = @account_version ||= store.conversation_version(account.id) + + def built_in_filter_version = @built_in_filter_version ||= store.built_in_filter_version(account_id: account.id, user_id: user.id) + + def filter_version(filter_id) = (@filter_versions ||= {})[filter_id] ||= store.filter_version(account_id: account.id, filter_id: filter_id) +end diff --git a/app/services/conversations/unread_counts/filtered_counter.rb b/app/services/conversations/unread_counts/filtered_counter.rb new file mode 100644 index 000000000..1194951b6 --- /dev/null +++ b/app/services/conversations/unread_counts/filtered_counter.rb @@ -0,0 +1,216 @@ +class Conversations::UnreadCounts::FilteredCounter + FEATURE_FLAG = 'unread_count_for_filters'.freeze + EMPTY_COUNTS = { + mentions_count: 0, + participating_count: 0, + unattended_count: 0, + folders: {} + }.freeze + + attr_reader :account, :user, :now + + def self.empty_counts = EMPTY_COUNTS.deep_dup + + def initialize(account:, user:, now: Time.current) + @account = account + @user = user + @now = now + end + + def perform = instrumentation.observe(:counter_perform, account_id: account.id) { built_in_counts.merge(folders: folder_counts) } + + private + + def built_in_counts = counts_from_built_in_snapshot(built_in_counts_snapshot) || self.class.empty_counts.except(:folders) + + def built_in_counts_snapshot + versions = version_cache.built_in_filter + snapshot_or_build( + scope: :built_in_filter, + state: store.built_in_filter_counts_state(account_id: account.id, user_id: user.id, versions: versions, now: now), + lock_key: store.built_in_filter_build_lock_key(account.id, user.id), + claim_refresh: -> { store.claim_built_in_filter_refresh!(account_id: account.id, user_id: user.id) } + ) { build_built_in_counts!(versions) } + end + + def counts_from_built_in_snapshot(snapshot) = snapshot&.fetch(:counts, nil)&.slice(:mentions_count, :participating_count, :unattended_count) + + def folder_counts + folder_index = folder_index_snapshot + return {} if folder_index.blank? + + @inline_filter_builds = 0 + folder_index[:filter_ids].each_with_object({}) do |filter_id, counts| + count = filter_count(filter_id) + counts[filter_id.to_s] = count if count.to_i.positive? + end + end + + def folder_index_snapshot + versions = version_cache.folder_index + snapshot_or_build( + scope: :folder_index, + state: store.folder_index_state(account_id: account.id, user_id: user.id, versions: versions, now: now), + lock_key: store.folder_index_build_lock_key(account.id, user.id), + claim_refresh: -> { store.claim_folder_index_refresh!(account_id: account.id, user_id: user.id) } + ) { build_folder_index!(versions) } + end + + def filter_count(filter_id) + versions = version_cache.filter(filter_id) + snapshot = snapshot_or_build( + scope: :filter, + state: store.filter_count_state(account_id: account.id, filter_id: filter_id, owner_user_id: user.id, versions: versions, now: now), + lock_key: store.filter_build_lock_key(account.id, filter_id), + claim_refresh: -> { filter_build_available? && store.claim_filter_refresh!(account_id: account.id, filter_id: filter_id) } + ) do + track_filter_build! + build_filter_count!(filter_id, versions) + end + + snapshot&.fetch(:count, nil) + end + + def snapshot_or_build(scope:, state:, lock_key:, claim_refresh:, &) + snapshot_resolver.resolve(scope: scope, state: state, lock_key: lock_key, claim_refresh: claim_refresh, &) + end + + def filter_build_available? = @inline_filter_builds.to_i < Conversations::UnreadCounts::MAX_INLINE_FILTER_BUILDS + + def track_filter_build! = @inline_filter_builds = @inline_filter_builds.to_i + 1 + + def build_built_in_counts!(versions) + store.write_built_in_filter_counts!(**built_in_count_snapshot_payload(versions)) + store.built_in_filter_counts(account_id: account.id, user_id: user.id) + end + + def built_in_count_snapshot_payload(versions) + { + account_id: account.id, + user_id: user.id, + counts: built_in_counts_from_database, + account_version: versions.fetch(:account_version), + built_in_filter_version: versions.fetch(:built_in_filter_version), + built_at: now + } + end + + def built_in_counts_from_database + { + mentions_count: count_relation(mentioned_unread_conversations), + participating_count: count_relation(participating_unread_conversations), + unattended_count: count_relation(unread_open_accessible_conversations.unattended) + } + end + + def mentioned_unread_conversations + unread_open_accessible_conversations + .joins(:mentions) + .where(mentions: { account_id: account.id, user_id: user.id }) + end + + def participating_unread_conversations + unread_open_accessible_conversations + .joins(:conversation_participants) + .where(conversation_participants: { user_id: user.id }) + end + + def build_folder_index!(versions) + store.write_folder_index!( + account_id: account.id, + user_id: user.id, + filter_ids: folder_filter_ids_from_database, + folder_index_version: versions.fetch(:folder_index_version), + built_at: now + ) + store.folder_index(account_id: account.id, user_id: user.id) + end + + def folder_filter_ids_from_database = account.custom_filters.where(user_id: user.id, filter_type: :conversation).pluck(:id) + + def build_filter_count!(filter_id, versions) + custom_filter = account.custom_filters.find_by(id: filter_id, user_id: user.id, filter_type: :conversation) + return delete_filter_count!(filter_id) if custom_filter.blank? + + count = filter_query_count(custom_filter) + return delete_filter_count!(filter_id) if count.nil? + + write_filter_count!(filter_id, count, versions) + store.filter_count(account_id: account.id, filter_id: filter_id) + rescue CustomExceptions::CustomFilter::InvalidAttribute, + CustomExceptions::CustomFilter::InvalidOperator, + CustomExceptions::CustomFilter::InvalidQueryOperator, + CustomExceptions::CustomFilter::InvalidValue + delete_filter_count!(filter_id) + end + + def filter_query_count(custom_filter) + ::Conversations::UnreadCounts::FilterQueryCounter.new( + account: account, + user: user, + query: custom_filter.query + ).perform + end + + def write_filter_count!(filter_id, count, versions) + store.write_filter_count!( + account_id: account.id, + filter_id: filter_id, + user_id: user.id, + count: count, + account_version: versions.fetch(:account_version), + filter_version: versions.fetch(:filter_version), + owner_built_in_filter_version: versions.fetch(:owner_built_in_filter_version), + built_at: now + ) + end + + def version_cache = @version_cache ||= ::Conversations::UnreadCounts::FilteredCountVersionCache.new(account: account, user: user, store: store) + + def delete_filter_count!(filter_id) = store.delete_filter_count!(account_id: account.id, filter_id: filter_id).then { nil } + + def unread_open_accessible_conversations + @unread_open_accessible_conversations ||= Conversations::PermissionFilterService.new( + unread_conversations.open, + user, + account + ).perform + end + + def unread_conversations + account.conversations + .joins(:messages) + .merge(Message.incoming.reorder(nil)) + .where(messages: { account_id: account.id }) + .where(unread_since_last_seen_condition) + .distinct + end + + def unread_since_last_seen_condition + conversations = Conversation.arel_table + messages = Message.arel_table + + conversations[:agent_last_seen_at].eq(nil).or(messages[:created_at].gt(conversations[:agent_last_seen_at])) + end + + def count_relation(relation) = relation.unscope(:order).count + + def lock_manager = @lock_manager ||= Redis::LockManager.new + + def snapshot_resolver + @snapshot_resolver ||= ::Conversations::UnreadCounts::FilteredCountSnapshotResolver.new( + account: account, + now: now, + store: store, + lock_manager: lock_manager + ) + end + + def store + ::Conversations::UnreadCounts::FilteredCountStore + end + + def instrumentation + ::Conversations::UnreadCounts::FilteredCountInstrumentation + end +end diff --git a/app/services/conversations/unread_counts/listener.rb b/app/services/conversations/unread_counts/listener.rb index 28792d884..59f15f5bc 100644 --- a/app/services/conversations/unread_counts/listener.rb +++ b/app/services/conversations/unread_counts/listener.rb @@ -1,34 +1,59 @@ class Conversations::UnreadCounts::Listener < BaseListener include Events::Types + FILTERED_CONVERSATION_UPDATE_KEYS = %w[ + additional_attributes cached_label_list campaign_id custom_attributes first_reply_created_at label_list last_activity_at priority snoozed_until + waiting_since + ].freeze + private_constant :FILTERED_CONVERSATION_UPDATE_KEYS + def message_created(event) message, = extract_message_and_account(event) - return unless message.incoming? - return unless message.account.feature_enabled?('conversation_unread_counts') + account = message.account + return unless account.feature_enabled?('conversation_unread_counts') || account.feature_enabled?(filtered_count_feature_flag) - refresh(message.conversation) + conversation = message.conversation + refreshed = refresh(conversation) if message.incoming? && account.feature_enabled?('conversation_unread_counts') + + invalidate_filtered_conversation(conversation) + + notify_filtered_count_change(conversation) unless message.incoming? && refreshed end def conversation_status_changed(event) conversation, = extract_conversation_and_account(event) - refresh(conversation, event.data[:changed_attributes]) + refresh_then_invalidate(conversation, event.data[:changed_attributes]) end def conversation_updated(event) - return unless label_changed?(event.data[:changed_attributes]) - conversation, = extract_conversation_and_account(event) - refresh(conversation, event.data[:changed_attributes]) + changed_attributes = event.data[:changed_attributes] + notify_filtered_count_change(conversation) if filtered_conversation_update_changed?(changed_attributes) && !label_changed?(changed_attributes) + return unless label_changed?(changed_attributes) + + refresh(conversation, changed_attributes) + end + + def conversation_contact_changed(event) + conversation, = extract_conversation_and_account(event) + invalidate_filtered_conversation(conversation) + notify_filtered_count_change(conversation) end def assignee_changed(event) conversation, = extract_conversation_and_account(event) - refresh(conversation, event.data[:changed_attributes]) + refresh_then_invalidate(conversation, event.data[:changed_attributes]) end def team_changed(event) conversation, = extract_conversation_and_account(event) - refresh(conversation, event.data[:changed_attributes]) + refresh_then_invalidate(conversation, event.data[:changed_attributes]) + end + + def conversation_mentioned(event) + conversation, = extract_conversation_and_account(event) + user = event.data[:user] + filtered_count_invalidator(conversation.account).user_visibility_changed!(user_id: user&.id) end def conversation_deleted(event) @@ -36,14 +61,23 @@ class Conversations::UnreadCounts::Listener < BaseListener return if conversation_data.blank? account = Account.find_by(id: conversation_data[:account_id]) - return unless account&.feature_enabled?('conversation_unread_counts') - return unless remove_deleted_conversation(account, conversation_data) + return if account.blank? + + removed = account.feature_enabled?('conversation_unread_counts') && remove_deleted_conversation(account, conversation_data) + filtered_count_invalidator(account).conversation_changed! + return notify_deleted_filtered_count_change(account, conversation_data) unless removed Rails.configuration.dispatcher.dispatch(CONVERSATION_UNREAD_COUNT_CHANGED, Time.zone.now, conversation_data: conversation_data.to_h) end private + def refresh_then_invalidate(conversation, changed_attributes = nil) + refreshed = refresh(conversation, changed_attributes) + invalidate_filtered_conversation(conversation) + notify_filtered_count_change(conversation) unless refreshed + end + def refresh(conversation, changed_attributes = nil) ::Conversations::UnreadCounts::Notifier.new(conversation, changed_attributes: changed_attributes).perform end @@ -90,6 +124,37 @@ class Conversations::UnreadCounts::Listener < BaseListener changed_attributes.key?('cached_label_list') || changed_attributes.key?(:cached_label_list) end + def filtered_conversation_update_changed?(changed_attributes) + return false if changed_attributes.blank? + + changed_attributes.keys.map(&:to_s).intersect?(FILTERED_CONVERSATION_UPDATE_KEYS) + end + + def invalidate_filtered_conversation(conversation) + filtered_count_invalidator(conversation.account).conversation_changed! + end + + def notify_filtered_count_change(conversation) + return unless conversation.account.feature_enabled?('conversation_unread_counts') + return unless conversation.account.feature_enabled?(filtered_count_feature_flag) + + Rails.configuration.dispatcher.dispatch(CONVERSATION_UNREAD_COUNT_CHANGED, Time.zone.now, conversation: conversation) + end + + def notify_deleted_filtered_count_change(account, conversation_data) + return unless account.feature_enabled?(filtered_count_feature_flag) + + Rails.configuration.dispatcher.dispatch(CONVERSATION_UNREAD_COUNT_CHANGED, Time.zone.now, conversation_data: conversation_data.to_h) + end + + def filtered_count_invalidator(account) + ::Conversations::UnreadCounts::FilteredCountInvalidator.new(account) + end + + def filtered_count_feature_flag + ::Conversations::UnreadCounts::FilteredCountInvalidator::FEATURE_FLAG + end + def store ::Conversations::UnreadCounts::Store end diff --git a/app/services/conversations/unread_counts/notifier.rb b/app/services/conversations/unread_counts/notifier.rb index 652fbde3a..6075b4abc 100644 --- a/app/services/conversations/unread_counts/notifier.rb +++ b/app/services/conversations/unread_counts/notifier.rb @@ -10,10 +10,20 @@ class Conversations::UnreadCounts::Notifier def perform return false unless conversation.account.feature_enabled?('conversation_unread_counts') + return dispatch_unread_count_changed if ::Conversations::UnreadCounts::Refresher.new(conversation, changed_attributes: changed_attributes).perform + return false unless conversation.account.feature_enabled?(filtered_count_feature_flag) - return false unless ::Conversations::UnreadCounts::Refresher.new(conversation, changed_attributes: changed_attributes).perform + dispatch_unread_count_changed + end + private + + def dispatch_unread_count_changed Rails.configuration.dispatcher.dispatch(CONVERSATION_UNREAD_COUNT_CHANGED, Time.zone.now, conversation: conversation) true end + + def filtered_count_feature_flag + ::Conversations::UnreadCounts::FilteredCountInvalidator::FEATURE_FLAG + end end diff --git a/app/services/labels/destroy_service.rb b/app/services/labels/destroy_service.rb index 080e708e7..b5d7add33 100644 --- a/app/services/labels/destroy_service.rb +++ b/app/services/labels/destroy_service.rb @@ -2,19 +2,25 @@ class Labels::DestroyService pattr_initialize [:label_title!, :account_id!, :label_deleted_at!] def perform - remove_conversation_labels + conversation_labels_removed = remove_conversation_labels remove_contact_labels + invalidate_filtered_unread_count_conversations if conversation_labels_removed end private def remove_conversation_labels + conversation_labels_removed = false + tagged_conversations.find_in_batches do |conversation_batch| conversation_batch.each do |conversation| update_conversation_cached_labels(conversation) end delete_label_taggings('Conversation', conversation_batch.map(&:id)) + conversation_labels_removed = true end + + conversation_labels_removed end def remove_contact_labels @@ -57,4 +63,8 @@ class Labels::DestroyService def account @account ||= Account.find(account_id) end + + def invalidate_filtered_unread_count_conversations + ::Conversations::UnreadCounts::FilteredCountInvalidator.new(account).conversation_changed! + end end diff --git a/config/features.yml b/config/features.yml index 55a1e815d..fe2ef2122 100644 --- a/config/features.yml +++ b/config/features.yml @@ -223,10 +223,10 @@ display_name: Reply Mailer Migration enabled: false chatwoot_internal: true -- name: quoted_email_reply - display_name: Quoted Email Reply +- name: unread_count_for_filters + display_name: Unread Count For Filters enabled: false - deprecated: true + chatwoot_internal: true - name: companies display_name: Companies enabled: false diff --git a/db/migrate/20260629000000_repurpose_quoted_email_reply_flag_for_unread_count_for_filters.rb b/db/migrate/20260629000000_repurpose_quoted_email_reply_flag_for_unread_count_for_filters.rb new file mode 100644 index 000000000..82f15a964 --- /dev/null +++ b/db/migrate/20260629000000_repurpose_quoted_email_reply_flag_for_unread_count_for_filters.rb @@ -0,0 +1,22 @@ +class RepurposeQuotedEmailReplyFlagForUnreadCountForFilters < ActiveRecord::Migration[7.1] + def up + # The quoted_email_reply flag (deprecated) has been renamed to unread_count_for_filters. + # Disable it on any accounts that had quoted_email_reply enabled so the repurposed + # flag starts in its intended default-off state. + Account.feature_unread_count_for_filters.find_each(batch_size: 100) do |account| + account.disable_features(:unread_count_for_filters) + account.save!(validate: false) + end + + # Remove the stale quoted_email_reply entry from ACCOUNT_LEVEL_FEATURE_DEFAULTS. + # ConfigLoader only adds new flags; it never removes renamed ones. + # Leaving it would cause NoMethodError in enable_default_features when + # creating new accounts (feature_quoted_email_reply= no longer exists). + config = InstallationConfig.find_by(name: 'ACCOUNT_LEVEL_FEATURE_DEFAULTS') + return if config&.value.blank? + + config.value = config.value.reject { |feature| feature['name'] == 'quoted_email_reply' } + config.save! + GlobalConfig.clear_cache + end +end diff --git a/enterprise/app/models/custom_role.rb b/enterprise/app/models/custom_role.rb index 666f91378..1eb39d1f7 100644 --- a/enterprise/app/models/custom_role.rb +++ b/enterprise/app/models/custom_role.rb @@ -28,6 +28,10 @@ class CustomRole < ApplicationRecord belongs_to :account has_many :account_users, dependent: :nullify + before_destroy :capture_filtered_unread_count_user_ids, prepend: true + after_update_commit :invalidate_filtered_unread_count_visibility_update, if: :filtered_unread_count_permissions_changed? + after_destroy_commit :invalidate_filtered_unread_count_visibility_destroy + PERMISSIONS = %w[ conversation_manage conversation_unassigned_manage @@ -39,4 +43,33 @@ class CustomRole < ApplicationRecord validates :name, presence: true validates :permissions, inclusion: { in: PERMISSIONS } + + private + + def filtered_unread_count_permissions_changed? + previous_changes.key?('permissions') + end + + def capture_filtered_unread_count_user_ids + @filtered_unread_count_user_ids = account_users.pluck(:user_id) + end + + def invalidate_filtered_unread_count_visibility_update + invalidate_filtered_unread_count_visibility(account_users.pluck(:user_id)) + end + + def invalidate_filtered_unread_count_visibility_destroy + invalidate_filtered_unread_count_visibility(@filtered_unread_count_user_ids) + end + + def invalidate_filtered_unread_count_visibility(user_ids) + invalidator = ::Conversations::UnreadCounts::FilteredCountInvalidator.new(account) + visibility_changed = invalidator.users_visibility_changed!(user_ids: user_ids) + + dispatch_account_cache_invalidated if visibility_changed + end + + def dispatch_account_cache_invalidated + Rails.configuration.dispatcher.dispatch(ACCOUNT_CACHE_INVALIDATED, Time.zone.now, account: account, cache_keys: account.cache_keys) + end end diff --git a/enterprise/app/services/enterprise/conversations/permission_filter_service.rb b/enterprise/app/services/enterprise/conversations/permission_filter_service.rb index f55265a90..118ae3d14 100644 --- a/enterprise/app/services/enterprise/conversations/permission_filter_service.rb +++ b/enterprise/app/services/enterprise/conversations/permission_filter_service.rb @@ -23,17 +23,22 @@ module Enterprise::Conversations::PermissionFilterService elsif permissions.include?('conversation_unassigned_manage') filter_unassigned_and_mine elsif permissions.include?('conversation_participating_manage') - accessible_conversations.assigned_to(user) + filter_participating_and_mine else Conversation.none end end - def filter_unassigned_and_mine - mine = accessible_conversations.assigned_to(user) - unassigned = accessible_conversations.unassigned + def filter_participating_and_mine + conversations = accessible_conversations + participant_conversation_ids = ConversationParticipant.where(account_id: account.id, user_id: user.id).select(:conversation_id) - Conversation.from("(#{mine.to_sql} UNION #{unassigned.to_sql}) as conversations") - .where(account_id: account.id) + conversations + .where(assignee_id: user.id) + .or(conversations.where(id: participant_conversation_ids)) + end + + def filter_unassigned_and_mine + accessible_conversations.where(assignee_id: [nil, user.id]) end end diff --git a/lib/redis/redis_keys.rb b/lib/redis/redis_keys.rb index 6d503df22..b782270ef 100644 --- a/lib/redis/redis_keys.rb +++ b/lib/redis/redis_keys.rb @@ -33,6 +33,23 @@ module Redis::RedisKeys 'UNREAD_CONVERSATIONS::V1::ACCOUNT::%d::TEAM::%d::INBOX::%d::UNASSIGNED'.freeze UNREAD_CONVERSATIONS_TEAM_INBOX_ASSIGNEE = 'UNREAD_CONVERSATIONS::V1::ACCOUNT::%d::TEAM::%d::INBOX::%d::ASSIGNEE::%d'.freeze + UNREAD_CONVERSATIONS_V2_ACCOUNT_PREFIX = 'UNREAD_CONVERSATIONS::V2::ACCOUNT::%d'.freeze + UNREAD_CONVERSATIONS_V2_USER_PREFIX = "#{UNREAD_CONVERSATIONS_V2_ACCOUNT_PREFIX}::USER::%d".freeze + UNREAD_CONVERSATIONS_V2_FILTER_PREFIX = "#{UNREAD_CONVERSATIONS_V2_ACCOUNT_PREFIX}::FILTER::%d".freeze + UNREAD_CONVERSATIONS_V2_CONVERSATION_VERSION = "#{UNREAD_CONVERSATIONS_V2_ACCOUNT_PREFIX}::CONVERSATION_VERSION".freeze + UNREAD_CONVERSATIONS_V2_BUILT_IN_FILTER_VERSION = "#{UNREAD_CONVERSATIONS_V2_USER_PREFIX}::BUILT_IN_FILTER_VERSION".freeze + UNREAD_CONVERSATIONS_V2_BUILT_IN_FILTER_COUNTS = "#{UNREAD_CONVERSATIONS_V2_USER_PREFIX}::BUILT_IN_FILTER_COUNTS".freeze + UNREAD_CONVERSATIONS_V2_BUILT_IN_FILTER_BUILD_LOCK = "#{UNREAD_CONVERSATIONS_V2_USER_PREFIX}::BUILT_IN_FILTER_BUILD_LOCK".freeze + UNREAD_CONVERSATIONS_V2_BUILT_IN_FILTER_REFRESH_THROTTLE = + "#{UNREAD_CONVERSATIONS_V2_USER_PREFIX}::BUILT_IN_FILTER_REFRESH_THROTTLE".freeze + UNREAD_CONVERSATIONS_V2_FOLDER_INDEX_VERSION = "#{UNREAD_CONVERSATIONS_V2_USER_PREFIX}::FOLDER_INDEX_VERSION".freeze + UNREAD_CONVERSATIONS_V2_FOLDER_INDEX = "#{UNREAD_CONVERSATIONS_V2_USER_PREFIX}::FOLDER_INDEX".freeze + UNREAD_CONVERSATIONS_V2_FOLDER_INDEX_BUILD_LOCK = "#{UNREAD_CONVERSATIONS_V2_USER_PREFIX}::FOLDER_INDEX_BUILD_LOCK".freeze + UNREAD_CONVERSATIONS_V2_FOLDER_INDEX_REFRESH_THROTTLE = "#{UNREAD_CONVERSATIONS_V2_USER_PREFIX}::FOLDER_INDEX_REFRESH_THROTTLE".freeze + UNREAD_CONVERSATIONS_V2_FILTER_VERSION = "#{UNREAD_CONVERSATIONS_V2_FILTER_PREFIX}::VERSION".freeze + UNREAD_CONVERSATIONS_V2_FILTER_COUNT = "#{UNREAD_CONVERSATIONS_V2_FILTER_PREFIX}::COUNT".freeze + UNREAD_CONVERSATIONS_V2_FILTER_BUILD_LOCK = "#{UNREAD_CONVERSATIONS_V2_FILTER_PREFIX}::BUILD_LOCK".freeze + UNREAD_CONVERSATIONS_V2_FILTER_REFRESH_THROTTLE = "#{UNREAD_CONVERSATIONS_V2_FILTER_PREFIX}::REFRESH_THROTTLE".freeze ## User Keys # SSO Auth Tokens diff --git a/spec/controllers/api/v1/accounts/conversations/participants_controller_spec.rb b/spec/controllers/api/v1/accounts/conversations/participants_controller_spec.rb index 6238314ab..33d3f64be 100644 --- a/spec/controllers/api/v1/accounts/conversations/participants_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/conversations/participants_controller_spec.rb @@ -68,6 +68,23 @@ RSpec.describe 'Conversation Participants API', type: :request do expect(response.body).to include(participant.email) expect(conversation.conversation_participants.count).to eq(1) end + + it 'notifies unread counts when a participant is added' do + account.enable_features!(:conversation_unread_counts, :unread_count_for_filters) + allow(Rails.configuration.dispatcher).to receive(:dispatch) + params = { user_ids: [participant.id] } + + post api_v1_account_conversation_participants_url(account_id: account.id, conversation_id: conversation.display_id), + params: params, + headers: agent.create_new_auth_token, + as: :json + + expect(Rails.configuration.dispatcher).to have_received(:dispatch).with( + 'conversation.unread_count_changed', + kind_of(ActiveSupport::TimeWithZone), + conversation: conversation + ) + end end end @@ -106,6 +123,25 @@ RSpec.describe 'Conversation Participants API', type: :request do expect(response.body).to include(participant_to_be_added.email) expect(conversation.conversation_participants.count).to eq(2) end + + it 'notifies unread counts when participant membership changes' do + account.enable_features!(:conversation_unread_counts, :unread_count_for_filters) + allow(Rails.configuration.dispatcher).to receive(:dispatch) + params = { user_ids: [participant.id, participant_to_be_added.id] } + create(:conversation_participant, conversation: conversation, user: participant) + create(:conversation_participant, conversation: conversation, user: participant_to_be_removed) + + put api_v1_account_conversation_participants_url(account_id: account.id, conversation_id: conversation.display_id), + params: params, + headers: agent.create_new_auth_token, + as: :json + + expect(Rails.configuration.dispatcher).to have_received(:dispatch).with( + 'conversation.unread_count_changed', + kind_of(ActiveSupport::TimeWithZone), + conversation: conversation + ) + end end end @@ -137,6 +173,24 @@ RSpec.describe 'Conversation Participants API', type: :request do expect(response).to have_http_status(:success) expect(conversation.conversation_participants.count).to eq(0) end + + it 'notifies unread counts when a participant is removed' do + account.enable_features!(:conversation_unread_counts, :unread_count_for_filters) + allow(Rails.configuration.dispatcher).to receive(:dispatch) + params = { user_ids: [participant.id] } + create(:conversation_participant, conversation: conversation, user: participant) + + delete api_v1_account_conversation_participants_url(account_id: account.id, conversation_id: conversation.display_id), + params: params, + headers: agent.create_new_auth_token, + as: :json + + expect(Rails.configuration.dispatcher).to have_received(:dispatch).with( + 'conversation.unread_count_changed', + kind_of(ActiveSupport::TimeWithZone), + conversation: conversation + ) + end end end end diff --git a/spec/controllers/api/v1/accounts/conversations_controller_spec.rb b/spec/controllers/api/v1/accounts/conversations_controller_spec.rb index bdb8117ac..b0eddd639 100644 --- a/spec/controllers/api/v1/accounts/conversations_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/conversations_controller_spec.rb @@ -159,6 +159,28 @@ RSpec.describe 'Conversations API', type: :request do expect(response).to have_http_status(:success) expect(response.parsed_body['payload']['teams']).to eq(team.id.to_s => 1) end + + it 'returns filtered unread counts when the filtered count feature is enabled' do + account.enable_features!(:unread_count_for_filters) + allow(Conversations::UnreadCounts::FilteredCountInstrumentation).to receive(:summarize_request) do |**_attributes, &block| + block.call + end + mentioned = create_unread_conversation(account: account, inbox: visible_inbox) + create(:mention, account: account, conversation: mentioned, user: agent) + + get "/api/v1/accounts/#{account.id}/conversations/unread_counts", + headers: agent.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:success) + expect(response.parsed_body['payload']).to include( + 'mentions_count' => 1, + 'participating_count' => 0, + 'unattended_count' => 1, + 'folders' => {} + ) + expect(Conversations::UnreadCounts::FilteredCountInstrumentation).to have_received(:summarize_request).with(account_id: account.id) + end end it 'returns forbidden when conversation unread counts feature is disabled' do @@ -865,6 +887,59 @@ RSpec.describe 'Conversations API', type: :request do Conversations::UnreadCounts::Store.clear_account!(account.id) end + it 'refreshes unread count cache before invalidating filtered counts when conversation is marked read' do + account.enable_features!(:conversation_unread_counts, :unread_count_for_filters) + conversation.update!(agent_last_seen_at: 1.hour.ago) + create(:message, account: account, inbox: conversation.inbox, conversation: conversation, message_type: :incoming, created_at: 5.minutes.ago) + notifier = instance_double(Conversations::UnreadCounts::Notifier) + invalidator = instance_double(Conversations::UnreadCounts::FilteredCountInvalidator) + + allow(Conversations::UnreadCounts::Notifier).to receive(:new).with(conversation).and_return(notifier) + allow(Conversations::UnreadCounts::FilteredCountInvalidator).to receive(:new).with(account).and_return(invalidator) + expect(notifier).to receive(:perform).ordered.and_return(true) + expect(invalidator).to receive(:conversation_changed!).ordered.and_return(true) + + post "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}/update_last_seen", + headers: agent.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:success) + end + + it 'invalidates filtered unread counts when conversation is marked read' do + conversation.update!(agent_last_seen_at: 1.hour.ago) + create(:message, account: account, inbox: conversation.inbox, conversation: conversation, message_type: :incoming, created_at: 5.minutes.ago) + account.enable_features!(:unread_count_for_filters) + + expect do + post "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}/update_last_seen", + headers: agent.create_new_auth_token, + as: :json + end.to change { Conversations::UnreadCounts::FilteredCountStore.conversation_version(account.id) }.by(1) + expect(response).to have_http_status(:success) + end + + it 'notifies clients when marking read only affects filtered counts' do + account.enable_features!(:conversation_unread_counts, :unread_count_for_filters) + conversation.update!(agent_last_seen_at: 1.hour.ago) + create(:message, account: account, inbox: conversation.inbox, conversation: conversation, message_type: :incoming, created_at: 5.minutes.ago) + allow(Conversations::UnreadCounts::Refresher).to receive(:new).and_return( + instance_double(Conversations::UnreadCounts::Refresher, perform: false) + ) + allow(Rails.configuration.dispatcher).to receive(:dispatch) + + post "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}/update_last_seen", + headers: agent.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:success) + expect(Rails.configuration.dispatcher).to have_received(:dispatch).with( + 'conversation.unread_count_changed', + kind_of(Time), + conversation: conversation + ) + end + it 'updates both if one timestamp is old even when the other is recent' do conversation.update!(assignee_id: agent.id, agent_last_seen_at: 2.hours.ago, assignee_last_seen_at: 30.minutes.ago) # Ensure all messages are older than assignee_last_seen_at (no unread messages) @@ -951,6 +1026,56 @@ RSpec.describe 'Conversations API', type: :request do ensure Conversations::UnreadCounts::Store.clear_account!(account.id) end + + it 'refreshes unread count cache before invalidating filtered counts when conversation is marked unread' do + account.enable_features!(:conversation_unread_counts, :unread_count_for_filters) + conversation.update!(agent_last_seen_at: 1.minute.from_now, assignee_last_seen_at: 1.minute.from_now) + notifier = instance_double(Conversations::UnreadCounts::Notifier) + invalidator = instance_double(Conversations::UnreadCounts::FilteredCountInvalidator) + + allow(Conversations::UnreadCounts::Notifier).to receive(:new).with(conversation).and_return(notifier) + allow(Conversations::UnreadCounts::FilteredCountInvalidator).to receive(:new).with(account).and_return(invalidator) + expect(notifier).to receive(:perform).ordered.and_return(true) + expect(invalidator).to receive(:conversation_changed!).ordered.and_return(true) + + post "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}/unread", + headers: agent.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:success) + end + + it 'invalidates filtered unread counts when conversation is marked unread' do + conversation.update!(agent_last_seen_at: 1.minute.from_now, assignee_last_seen_at: 1.minute.from_now) + account.enable_features!(:unread_count_for_filters) + + expect do + post "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}/unread", + headers: agent.create_new_auth_token, + as: :json + end.to change { Conversations::UnreadCounts::FilteredCountStore.conversation_version(account.id) }.by(1) + expect(response).to have_http_status(:success) + end + + it 'notifies clients when marking unread only affects filtered counts' do + account.enable_features!(:conversation_unread_counts, :unread_count_for_filters) + conversation.update!(agent_last_seen_at: 1.minute.from_now, assignee_last_seen_at: 1.minute.from_now) + allow(Conversations::UnreadCounts::Refresher).to receive(:new).and_return( + instance_double(Conversations::UnreadCounts::Refresher, perform: false) + ) + allow(Rails.configuration.dispatcher).to receive(:dispatch) + + post "/api/v1/accounts/#{account.id}/conversations/#{conversation.display_id}/unread", + headers: agent.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:success) + expect(Rails.configuration.dispatcher).to have_received(:dispatch).with( + 'conversation.unread_count_changed', + kind_of(Time), + conversation: conversation + ) + end end end diff --git a/spec/enterprise/finders/conversation_finder_spec.rb b/spec/enterprise/finders/conversation_finder_spec.rb new file mode 100644 index 000000000..f415ac473 --- /dev/null +++ b/spec/enterprise/finders/conversation_finder_spec.rb @@ -0,0 +1,41 @@ +require 'rails_helper' + +RSpec.describe ConversationFinder do + describe '#perform_meta_only' do + let(:account) { create(:account) } + let(:agent) { create(:user, account: account, role: :agent) } + let(:other_agent) { create(:user, account: account, role: :agent) } + let(:inbox) { create(:inbox, account: account) } + + before do + Current.account = account + create(:inbox_member, user: agent, inbox: inbox) + account.account_users.find_by(user: agent).update!( + role: :agent, + custom_role: create(:custom_role, account: account, permissions: %w[conversation_participating_manage]) + ) + end + + it 'counts participant-filtered conversations once when assigned conversations have multiple participants' do + assigned_conversation = create(:conversation, account: account, inbox: inbox, assignee: agent) + participating_conversation = create(:conversation, account: account, inbox: inbox, assignee: other_agent) + create(:conversation, account: account, inbox: inbox, assignee: other_agent) + + 2.times do + participant = create(:user, account: account, role: :agent) + create(:inbox_member, user: participant, inbox: inbox) + create(:conversation_participant, account: account, conversation: assigned_conversation, user: participant) + end + create(:conversation_participant, account: account, conversation: participating_conversation, user: agent) + + result = described_class.new(agent, { status: 'open' }).perform_meta_only + + expect(result[:count]).to eq({ + mine_count: 1, + assigned_count: 2, + unassigned_count: 0, + all_count: 2 + }) + end + end +end diff --git a/spec/enterprise/models/account_user_spec.rb b/spec/enterprise/models/account_user_spec.rb index fb572e86a..c79cfb4b6 100644 --- a/spec/enterprise/models/account_user_spec.rb +++ b/spec/enterprise/models/account_user_spec.rb @@ -29,6 +29,29 @@ RSpec.describe AccountUser, type: :model do end end + describe 'filtered unread count invalidation' do + it 'invalidates filtered counts when the custom role assignment changes' do + account = create(:account) + user = create(:user) + account_user = create(:account_user, account: account, user: user) + custom_role = create(:custom_role, account: account) + invalidator = instance_double(Conversations::UnreadCounts::FilteredCountInvalidator, user_visibility_changed!: true) + + allow(Conversations::UnreadCounts::FilteredCountInvalidator).to receive(:new).and_return(invalidator) + allow(Rails.configuration.dispatcher).to receive(:dispatch) + + account_user.update!(custom_role_id: custom_role.id) + + expect(invalidator).to have_received(:user_visibility_changed!).with(user_id: user.id) + expect(Rails.configuration.dispatcher).to have_received(:dispatch).with( + 'account.cache_invalidated', + kind_of(Time), + account: account, + cache_keys: account.cache_keys + ) + end + end + describe 'audit log' do context 'when account user is created' do it 'has associated audit log created' do diff --git a/spec/enterprise/models/custom_role_spec.rb b/spec/enterprise/models/custom_role_spec.rb index f63f3c2dd..5ee3353b3 100644 --- a/spec/enterprise/models/custom_role_spec.rb +++ b/spec/enterprise/models/custom_role_spec.rb @@ -9,4 +9,49 @@ RSpec.describe CustomRole, type: :model do describe 'validations' do it { is_expected.to validate_presence_of(:name) } end + + describe 'filtered unread count invalidation' do + let(:account) { create(:account) } + let(:custom_role) { create(:custom_role, account: account, permissions: ['conversation_manage']) } + let(:user) { create(:user) } + let(:other_user) { create(:user) } + let(:invalidator) { instance_double(Conversations::UnreadCounts::FilteredCountInvalidator, users_visibility_changed!: true) } + + before do + create(:account_user, account: account, user: user, custom_role: custom_role) + create(:account_user, account: account, user: other_user, custom_role: custom_role) + allow(Conversations::UnreadCounts::FilteredCountInvalidator).to receive(:new).with(account).and_return(invalidator) + allow(Rails.configuration.dispatcher).to receive(:dispatch) + end + + it 'invalidates filtered counts for assigned users when permissions change' do + custom_role.update!(permissions: ['conversation_participating_manage']) + + expect(invalidator).to have_received(:users_visibility_changed!).with(user_ids: contain_exactly(user.id, other_user.id)) + expect(Rails.configuration.dispatcher).to have_received(:dispatch).with( + 'account.cache_invalidated', + kind_of(Time), + account: account, + cache_keys: account.cache_keys + ) + end + + it 'does not invalidate filtered counts when permissions are unchanged' do + custom_role.update!(name: 'Support manager') + + expect(invalidator).not_to have_received(:users_visibility_changed!) + end + + it 'invalidates filtered counts for assigned users when the role is deleted' do + custom_role.destroy! + + expect(invalidator).to have_received(:users_visibility_changed!).with(user_ids: contain_exactly(user.id, other_user.id)) + expect(Rails.configuration.dispatcher).to have_received(:dispatch).with( + 'account.cache_invalidated', + kind_of(Time), + account: account, + cache_keys: account.cache_keys + ) + end + end end diff --git a/spec/enterprise/services/conversations/unread_counts/filtered_counter_spec.rb b/spec/enterprise/services/conversations/unread_counts/filtered_counter_spec.rb new file mode 100644 index 000000000..81c9ae4d0 --- /dev/null +++ b/spec/enterprise/services/conversations/unread_counts/filtered_counter_spec.rb @@ -0,0 +1,58 @@ +require 'rails_helper' + +RSpec.describe Conversations::UnreadCounts::FilteredCounter do + let(:account) { create(:account) } + let(:agent) { create(:user, account: account, role: :agent) } + let(:other_agent) { create(:user, account: account, role: :agent) } + let(:inbox) { create(:inbox, account: account) } + let(:account_user) { account.account_users.find_by(user: agent) } + let(:store) { Conversations::UnreadCounts::FilteredCountStore } + + before do + create(:inbox_member, user: agent, inbox: inbox) + account_user.update!(custom_role: create(:custom_role, account: account, permissions: ['conversation_participating_manage'])) + end + + after do + redis_keys.each { |key| Redis::Alfred.delete(key) } + end + + it 'counts participating conversations inside the permission-filtered accessible set' do + assigned_participating = create_unread_conversation(account: account, inbox: inbox, assignee: agent) + unassigned_participating = create_unread_conversation(account: account, inbox: inbox) + assigned_to_other_participating = create_unread_conversation(account: account, inbox: inbox, assignee: other_agent) + assigned_not_participating = create_unread_conversation(account: account, inbox: inbox, assignee: agent) + create(:conversation_participant, account: account, conversation: assigned_participating, user: agent) + create(:conversation_participant, account: account, conversation: unassigned_participating, user: agent) + create(:conversation_participant, account: account, conversation: assigned_to_other_participating, user: agent) + + result = described_class.new(account: account, user: agent).perform + + expect(result[:participating_count]).to eq(3) + expect(result[:mentions_count]).to eq(0) + expect(result[:folders]).to eq({}) + expect(assigned_not_participating.assignee).to eq(agent) + end + + def redis_keys + [store.conversation_version_key(account.id)] + built_in_filter_keys + folder_index_keys + end + + def built_in_filter_keys + [ + store.built_in_filter_version_key(account.id, agent.id), + store.built_in_filter_counts_key(account.id, agent.id), + store.built_in_filter_build_lock_key(account.id, agent.id), + store.built_in_filter_refresh_throttle_key(account.id, agent.id) + ] + end + + def folder_index_keys + [ + store.folder_index_version_key(account.id, agent.id), + store.folder_index_key(account.id, agent.id), + store.folder_index_build_lock_key(account.id, agent.id), + store.folder_index_refresh_throttle_key(account.id, agent.id) + ] + end +end diff --git a/spec/enterprise/services/enterprise/conversations/permission_filter_service_spec.rb b/spec/enterprise/services/enterprise/conversations/permission_filter_service_spec.rb index 0cfec97eb..08d900b6e 100644 --- a/spec/enterprise/services/enterprise/conversations/permission_filter_service_spec.rb +++ b/spec/enterprise/services/enterprise/conversations/permission_filter_service_spec.rb @@ -86,7 +86,7 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do end context 'when user has conversation_participating_manage permission' do - it 'returns only conversations assigned to the agent' do + it 'returns conversations assigned to the agent or where the agent is a participant' do # Create a new isolated test environment test_account = create(:account) test_inbox = create(:inbox, account: test_account) @@ -105,7 +105,9 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do # Create some conversations other_conversation = create(:conversation, account: test_account, inbox: test_inbox) assigned_conversation = create(:conversation, account: test_account, inbox: test_inbox, assignee: test_agent) + participating_conversation = create(:conversation, account: test_account, inbox: test_inbox, assignee: create(:user, account: test_account)) other_inbox_conversation = create(:conversation, account: test_account, inbox: test_inbox2, assignee: nil) + create(:conversation_participant, account: test_account, conversation: participating_conversation, user: test_agent) # Run the test result = Conversations::PermissionFilterService.new( @@ -114,10 +116,10 @@ RSpec.describe Enterprise::Conversations::PermissionFilterService do test_account ).perform - # Should only see conversations assigned to this agent - expect(result.count).to eq(1) - expect(result.first.assignee).to eq(test_agent) + # Should only see conversations assigned to this agent or where the agent participates + expect(result.count).to eq(2) expect(result).to include(assigned_conversation) + expect(result).to include(participating_conversation) expect(result).not_to include(other_conversation) expect(result).not_to include(other_inbox_conversation) end diff --git a/spec/jobs/agents/destroy_job_spec.rb b/spec/jobs/agents/destroy_job_spec.rb index cf6bb2ffa..7c1c16e0c 100644 --- a/spec/jobs/agents/destroy_job_spec.rb +++ b/spec/jobs/agents/destroy_job_spec.rb @@ -7,6 +7,7 @@ RSpec.describe Agents::DestroyJob do let(:user) { create(:user, account: account) } let(:team1) { create(:team, account: account) } let!(:inbox) { create(:inbox, account: account) } + let(:store) { Conversations::UnreadCounts::FilteredCountStore } before do create(:team_member, team: team1, user: user) @@ -30,5 +31,13 @@ RSpec.describe Agents::DestroyJob do expect(user.notification_settings.length).to eq 0 expect(user.assigned_conversations.where(account: account).length).to eq 0 end + + it 'invalidates saved filter snapshots when assigned conversations are unassigned' do + account.enable_features!(:unread_count_for_filters) + + expect do + described_class.perform_now(account, user) + end.to change { store.conversation_version(account.id) }.by(1) + end end end diff --git a/spec/lib/captain/reply_suggestion_service_spec.rb b/spec/lib/captain/reply_suggestion_service_spec.rb index 608db43a2..9b5cbfa9e 100644 --- a/spec/lib/captain/reply_suggestion_service_spec.rb +++ b/spec/lib/captain/reply_suggestion_service_spec.rb @@ -12,6 +12,7 @@ RSpec.describe Captain::ReplySuggestionService do before do create(:installation_config, name: 'CAPTAIN_OPEN_AI_API_KEY', value: 'test-key') create(:message, conversation: conversation, message_type: :incoming, content: 'I need help') + allow(account).to receive(:feature_enabled?).and_call_original allow(account).to receive(:feature_enabled?).with('captain_tasks').and_return(true) mock_response = instance_double(RubyLLM::Message, content: 'Sure, I can help!', input_tokens: 50, output_tokens: 20) diff --git a/spec/listeners/action_cable_listener_spec.rb b/spec/listeners/action_cable_listener_spec.rb index cdb9a93cb..db387c5dc 100644 --- a/spec/listeners/action_cable_listener_spec.rb +++ b/spec/listeners/action_cable_listener_spec.rb @@ -13,6 +13,30 @@ describe ActionCableListener do Current.account = nil end + describe '#account_cache_invalidated' do + let!(:event) do + Events::Base.new( + :'account.cache_invalidated', + Time.zone.now, + account: account, + cache_keys: account.cache_keys + ) + end + + it 'sends cache invalidation to account agents and admins' do + expect(ActionCableBroadcastJob).to receive(:perform_later).with( + a_collection_containing_exactly(agent.pubsub_token, admin.pubsub_token), + 'account.cache_invalidated', + { + cache_keys: account.cache_keys, + account_id: account.id + } + ) + + listener.account_cache_invalidated(event) + end + end + describe '#message_created' do let(:event_name) { :'message.created' } let!(:message) do diff --git a/spec/models/account_user_spec.rb b/spec/models/account_user_spec.rb index e5a560fe9..394654db4 100644 --- a/spec/models/account_user_spec.rb +++ b/spec/models/account_user_spec.rb @@ -42,4 +42,43 @@ RSpec.describe AccountUser do expect(user.assigned_conversations.count).to eq(0) end end + + describe 'filtered unread count invalidation' do + let(:account) { create(:account) } + let(:user) { create(:user) } + let(:invalidator) { instance_double(Conversations::UnreadCounts::FilteredCountInvalidator, user_visibility_changed!: true) } + + before do + allow(Conversations::UnreadCounts::FilteredCountInvalidator).to receive(:new).and_return(invalidator) + allow(Rails.configuration.dispatcher).to receive(:dispatch) + end + + it 'invalidates filtered counts when the user is added to an account' do + create(:account_user, account: account, user: user) + + expect(invalidator).to have_received(:user_visibility_changed!).with(user_id: user.id) + end + + it 'invalidates filtered counts when the user role changes' do + account_user = create(:account_user, account: account, user: user) + + account_user.update!(role: :administrator) + + expect(invalidator).to have_received(:user_visibility_changed!).with(user_id: user.id).twice + expect(Rails.configuration.dispatcher).to have_received(:dispatch).with( + 'account.cache_invalidated', + kind_of(Time), + account: account, + cache_keys: account.cache_keys + ) + end + + it 'invalidates filtered counts when the user is removed from an account' do + account_user = create(:account_user, account: account, user: user) + + account_user.destroy! + + expect(invalidator).to have_received(:user_visibility_changed!).with(user_id: user.id).twice + end + end end diff --git a/spec/models/campaign_spec.rb b/spec/models/campaign_spec.rb index 2be6bd588..e4bdd05e4 100644 --- a/spec/models/campaign_spec.rb +++ b/spec/models/campaign_spec.rb @@ -3,11 +3,48 @@ require 'rails_helper' RSpec.describe Campaign do + let(:store) { Conversations::UnreadCounts::FilteredCountStore } + describe 'associations' do it { is_expected.to belong_to(:account) } it { is_expected.to belong_to(:inbox) } end + describe '#destroy' do + let(:account) { create(:account) } + let(:campaign) { create(:campaign, account: account) } + + before do + campaign + allow(Rails.configuration.dispatcher).to receive(:dispatch) + end + + after do + Redis::Alfred.delete(store.conversation_version_key(account.id)) + end + + it 'invalidates and refreshes filtered counts when conversations are detached from a deleted campaign' do + account.enable_features!(:unread_count_for_filters) + + expect do + campaign.destroy! + end.to change { store.conversation_version(account.id) }.by(1) + + expect(Rails.configuration.dispatcher).to have_received(:dispatch).with( + 'account.cache_invalidated', + kind_of(Time), + account: account, + cache_keys: account.cache_keys + ) + end + + it 'does not notify filtered count refreshes when the feature is disabled' do + campaign.destroy! + + expect(Rails.configuration.dispatcher).not_to have_received(:dispatch) + end + end + describe '.before_create' do let(:account) { create(:account) } let(:website_channel) { create(:channel_widget, account: account) } diff --git a/spec/models/conversation_participants_spec.rb b/spec/models/conversation_participants_spec.rb index c078f69f6..f8801d461 100644 --- a/spec/models/conversation_participants_spec.rb +++ b/spec/models/conversation_participants_spec.rb @@ -29,4 +29,30 @@ RSpec.describe ConversationParticipant do expect(participant.errors.messages[:user]).to eq(['must have inbox access']) end end + + describe 'filtered unread count invalidation' do + let(:account) { create(:account) } + let(:conversation) { create(:conversation, account: account) } + let(:user) { create(:user, account: account) } + let(:store) { Conversations::UnreadCounts::FilteredCountStore } + + before do + account.enable_features!(:unread_count_for_filters) + create(:inbox_member, inbox: conversation.inbox, user: user) + end + + it 'invalidates the participant built-in filter version when a participant is added' do + expect do + create(:conversation_participant, account: account, conversation: conversation, user: user) + end.to change { store.built_in_filter_version(account_id: account.id, user_id: user.id) }.by(1) + end + + it 'invalidates the participant built-in filter version when a participant is removed' do + participant = create(:conversation_participant, account: account, conversation: conversation, user: user) + + expect do + participant.destroy! + end.to change { store.built_in_filter_version(account_id: account.id, user_id: user.id) }.by(1) + end + end end diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb index 58d64ea94..43bbab56f 100644 --- a/spec/models/conversation_spec.rb +++ b/spec/models/conversation_spec.rb @@ -117,6 +117,7 @@ RSpec.describe Conversation do end let(:assignment_mailer) { instance_double(AssignmentMailer, deliver: true) } let(:label) { create(:label, account: account) } + let(:filtered_store) { Conversations::UnreadCounts::FilteredCountStore } before do create(:inbox_member, user: old_assignee, inbox: conversation.inbox) @@ -125,6 +126,10 @@ RSpec.describe Conversation do Current.user = old_assignee end + after do + Redis::Alfred.delete(filtered_store.conversation_version_key(account.id)) + end + it 'sends conversation updated event if labels are updated' do conversation.update(label_list: [label.title]) changed_attributes = conversation.previous_changes @@ -139,6 +144,33 @@ RSpec.describe Conversation do ) end + it 'invalidates filtered counts without sending conversation updated event if last activity time is updated' do + account.enable_features!(:unread_count_for_filters) + + expect do + conversation.update!(last_activity_at: 1.hour.from_now) + end.to change { filtered_store.conversation_version(account.id) }.by(1) + expect(Rails.configuration.dispatcher).not_to have_received(:dispatch).with( + described_class::CONVERSATION_UPDATED, + kind_of(Time), + anything + ) + end + + it 'invalidates filtered counts without sending conversation updated event if campaign assignment is updated' do + account.enable_features!(:unread_count_for_filters) + campaign = create(:campaign, account: account, inbox: conversation.inbox) + + expect do + conversation.update!(campaign: campaign) + end.to change { filtered_store.conversation_version(account.id) }.by(1) + expect(Rails.configuration.dispatcher).not_to have_received(:dispatch).with( + described_class::CONVERSATION_UPDATED, + kind_of(Time), + anything + ) + end + it 'runs after_update callbacks' do conversation.update( status: :resolved, @@ -174,20 +206,47 @@ RSpec.describe Conversation do .with(described_class::CONVERSATION_UPDATED, kind_of(Time), conversation: conversation, notifiable_assignee_change: true) end - it 'will run conversation_updated event for conversation_language in additional_attributes' do - conversation.additional_attributes[:conversation_language] = 'es' - conversation.save! + it 'will run conversation_updated event for conversation language changes' do + conversation.update!(additional_attributes: { 'conversation_language' => 'es' }) changed_attributes = conversation.previous_changes + expect(Rails.configuration.dispatcher).to have_received(:dispatch) .with(described_class::CONVERSATION_UPDATED, kind_of(Time), conversation: conversation, notifiable_assignee_change: false, changed_attributes: changed_attributes, performed_by: nil) end - it 'will not run conversation_updated event for bowser_language in additional_attributes' do - conversation.additional_attributes[:browser_language] = 'es' + it 'invalidates filtered counts without sending conversation_updated for filtered-only additional_attributes' do + account.enable_features!(:unread_count_for_filters) + + expect do + conversation.update!(additional_attributes: { 'browser_language' => 'es' }) + end.to change { filtered_store.conversation_version(account.id) }.by(1) + expect(Rails.configuration.dispatcher).not_to have_received(:dispatch).with( + described_class::CONVERSATION_UPDATED, + kind_of(Time), + anything + ) + end + + it 'invalidates filtered counts when filterable additional_attributes are removed' do + account.enable_features!(:unread_count_for_filters) + conversation.update!(additional_attributes: { 'referer' => 'https://www.chatwoot.com/' }) + + expect do + conversation.update!(additional_attributes: {}) + end.to change { filtered_store.conversation_version(account.id) }.by(1) + expect(Rails.configuration.dispatcher).not_to have_received(:dispatch).with( + described_class::CONVERSATION_UPDATED, + kind_of(Time), + anything + ) + end + + it 'will not run conversation_updated event for non-filterable additional_attributes' do + conversation.additional_attributes[:source_id] = 'es' conversation.save! expect(Rails.configuration.dispatcher).not_to have_received(:dispatch) - .with(described_class::CONVERSATION_UPDATED, kind_of(Time), conversation: conversation, notifiable_assignee_change: true) + .with(described_class::CONVERSATION_UPDATED, kind_of(Time), anything) end it 'creates conversation activities' do diff --git a/spec/models/custom_attribute_definition_spec.rb b/spec/models/custom_attribute_definition_spec.rb index c529fc609..8cf9065e0 100644 --- a/spec/models/custom_attribute_definition_spec.rb +++ b/spec/models/custom_attribute_definition_spec.rb @@ -68,5 +68,51 @@ RSpec.describe CustomAttributeDefinition do expect(cad.attribute_display_name).to eq('Order Date') end end + + describe 'filtered unread count invalidation' do + let(:invalidator) { instance_double(Conversations::UnreadCounts::FilteredCountInvalidator, custom_attribute_definition_changed!: true) } + + before do + allow(Conversations::UnreadCounts::FilteredCountInvalidator).to receive(:new).with(account).and_return(invalidator) + allow(Rails.configuration.dispatcher).to receive(:dispatch) + end + + it 'invalidates conversation filters when a conversation custom attribute definition changes' do + cad = create(:custom_attribute_definition, account: account, attribute_model: 'conversation_attribute') + + cad.update!(attribute_display_name: 'Updated Order Date') + + expect(invalidator).to have_received(:custom_attribute_definition_changed!).with(cad) + expect(Rails.configuration.dispatcher).to have_received(:dispatch).with( + 'account.cache_invalidated', + kind_of(Time), + account: account, + cache_keys: account.cache_keys + ) + end + + it 'invalidates conversation filters when a conversation custom attribute definition is deleted' do + cad = create(:custom_attribute_definition, account: account, attribute_model: 'conversation_attribute') + + cad.destroy! + + expect(invalidator).to have_received(:custom_attribute_definition_changed!).with(cad) + expect(Rails.configuration.dispatcher).to have_received(:dispatch).with( + 'account.cache_invalidated', + kind_of(Time), + account: account, + cache_keys: account.cache_keys + ) + end + + it 'ignores contact custom attribute definition changes' do + cad = create(:custom_attribute_definition, account: account, attribute_model: 'contact_attribute') + + cad.update!(attribute_display_name: 'Updated Contact Field') + + expect(invalidator).not_to have_received(:custom_attribute_definition_changed!) + expect(Rails.configuration.dispatcher).not_to have_received(:dispatch) + end + end end end diff --git a/spec/models/custom_filter_spec.rb b/spec/models/custom_filter_spec.rb new file mode 100644 index 000000000..ecb3f2baa --- /dev/null +++ b/spec/models/custom_filter_spec.rb @@ -0,0 +1,58 @@ +require 'rails_helper' + +RSpec.describe CustomFilter do + let(:account) { create(:account) } + let(:user) { create(:user, account: account) } + let(:store) { Conversations::UnreadCounts::FilteredCountStore } + + before do + account.enable_features!(:unread_count_for_filters) + end + + describe 'filtered unread count invalidation' do + it 'invalidates the folder index and filter version when a conversation filter is created' do + custom_filter = nil + + expect do + custom_filter = create(:custom_filter, account: account, user: user, filter_type: :conversation) + end.to change { store.folder_index_version(account_id: account.id, user_id: user.id) }.by(1) + expect(store.filter_version(account_id: account.id, filter_id: custom_filter.id)).to eq(1) + end + + it 'invalidates only the filter version when the query changes' do + custom_filter = create(:custom_filter, account: account, user: user, filter_type: :conversation) + folder_index_version = store.folder_index_version(account_id: account.id, user_id: user.id) + + expect do + custom_filter.update!(query: { payload: [{ attribute_key: 'status', values: ['resolved'] }] }) + end.to(change { store.filter_version(account_id: account.id, filter_id: custom_filter.id) }.by(1)) + expect(store.folder_index_version(account_id: account.id, user_id: user.id)).to eq(folder_index_version) + end + + it 'does not invalidate counts when only the name changes' do + custom_filter = create(:custom_filter, account: account, user: user, filter_type: :conversation) + + expect do + custom_filter.update!(name: 'Renamed filter') + end.not_to(change { store.filter_version(account_id: account.id, filter_id: custom_filter.id) }) + end + + it 'invalidates the folder index and deletes the count when a conversation filter is destroyed' do + custom_filter = create(:custom_filter, account: account, user: user, filter_type: :conversation) + store.write_filter_count!( + account_id: account.id, + filter_id: custom_filter.id, + user_id: user.id, + count: 3, + account_version: 0, + filter_version: 0, + owner_built_in_filter_version: 0 + ) + + expect do + custom_filter.destroy! + end.to change { store.folder_index_version(account_id: account.id, user_id: user.id) }.by(1) + expect(store.filter_count(account_id: account.id, filter_id: custom_filter.id)).to be_nil + end + end +end diff --git a/spec/models/inbox_member_spec.rb b/spec/models/inbox_member_spec.rb index b081c546c..67f662cac 100644 --- a/spec/models/inbox_member_spec.rb +++ b/spec/models/inbox_member_spec.rb @@ -18,4 +18,46 @@ RSpec.describe InboxMember do end end end + + describe 'filtered unread count invalidation' do + let(:account) { create(:account) } + let(:inbox) { create(:inbox, account: account) } + let(:user) { create(:user) } + let(:store) { Conversations::UnreadCounts::FilteredCountStore } + + before do + account.enable_features!(:unread_count_for_filters) + end + + it 'invalidates the user built-in filter version when inbox access is added' do + expect do + create(:inbox_member, inbox: inbox, user: user) + end.to change { store.built_in_filter_version(account_id: account.id, user_id: user.id) }.by(1) + end + + it 'invalidates the user built-in filter version when inbox access is removed' do + inbox_member = create(:inbox_member, inbox: inbox, user: user) + + expect do + inbox_member.destroy! + end.to change { store.built_in_filter_version(account_id: account.id, user_id: user.id) }.by(1) + end + + it 'invalidates the user built-in filter version when the parent inbox is removed' do + create(:inbox_member, inbox: inbox, user: user) + + expect do + perform_enqueued_jobs { inbox.destroy! } + end.to change { store.built_in_filter_version(account_id: account.id, user_id: user.id) }.by(1) + end + + it 'invalidates administrator built-in filter versions when the parent inbox is removed' do + admin = create(:user) + create(:account_user, account: account, user: admin, role: :administrator) + + expect do + perform_enqueued_jobs { inbox.destroy! } + end.to change { store.built_in_filter_version(account_id: account.id, user_id: admin.id) }.by(1) + end + end end diff --git a/spec/models/inbox_spec.rb b/spec/models/inbox_spec.rb index 92fddcd78..617a74388 100644 --- a/spec/models/inbox_spec.rb +++ b/spec/models/inbox_spec.rb @@ -41,6 +41,34 @@ RSpec.describe Inbox do it_behaves_like 'avatarable' end + describe 'account teardown' do + it 'destroys an orphaned inbox after its account has been deleted' do + account = create(:account) + inbox = create(:inbox, account: account) + account.delete + + orphaned_inbox = described_class.find(inbox.id) + + expect { orphaned_inbox.destroy! }.not_to raise_error + end + end + + describe 'filtered unread count invalidation' do + let(:account) { create(:account) } + let(:inbox) { create(:inbox, account: account) } + let(:store) { Conversations::UnreadCounts::FilteredCountStore } + + before do + account.enable_features!(:unread_count_for_filters) + end + + it 'invalidates saved folder snapshots when destroyed' do + expect do + inbox.destroy! + end.to change { store.conversation_version(account.id) }.by(1) + end + end + describe '#add_members' do let(:inbox) { FactoryBot.create(:inbox) } diff --git a/spec/models/team_member_spec.rb b/spec/models/team_member_spec.rb index f37f70425..d57c59996 100644 --- a/spec/models/team_member_spec.rb +++ b/spec/models/team_member_spec.rb @@ -1,8 +1,51 @@ require 'rails_helper' RSpec.describe TeamMember do + include ActiveJob::TestHelper + describe 'associations' do it { is_expected.to belong_to(:team) } it { is_expected.to belong_to(:user) } end + + describe 'filtered unread count invalidation' do + let(:account) { create(:account) } + let(:team) { create(:team, account: account) } + let(:user) { create(:user) } + let(:store) { Conversations::UnreadCounts::FilteredCountStore } + + before do + account.enable_features!(:unread_count_for_filters) + end + + it 'invalidates the user built-in filter version when team access is added' do + expect do + create(:team_member, team: team, user: user) + end.to change { store.built_in_filter_version(account_id: account.id, user_id: user.id) }.by(1) + end + + it 'invalidates the user built-in filter version when team access is removed' do + team_member = create(:team_member, team: team, user: user) + + expect do + team_member.destroy! + end.to change { store.built_in_filter_version(account_id: account.id, user_id: user.id) }.by(1) + end + + it 'invalidates the user built-in filter version when the parent team is removed' do + create(:team_member, team: team, user: user) + + expect do + perform_enqueued_jobs { team.destroy! } + end.to change { store.built_in_filter_version(account_id: account.id, user_id: user.id) }.by(1) + end + + it 'invalidates saved filter snapshots when the parent team is removed' do + create(:conversation, account: account, team: team) + + expect do + perform_enqueued_jobs { team.destroy! } + end.to change { store.conversation_version(account.id) }.by(1) + end + end end diff --git a/spec/services/conversations/filter_service_spec.rb b/spec/services/conversations/filter_service_spec.rb index fa054b330..eecbaec2e 100644 --- a/spec/services/conversations/filter_service_spec.rb +++ b/spec/services/conversations/filter_service_spec.rb @@ -231,6 +231,24 @@ describe Conversations::FilterService do expect(result[:count][:all_count]).to be 2 end + it 'filters conversations by display_id substring' do + conversation = create(:conversation, account: account, inbox: inbox, assignee: user_1) + create(:conversation, account: account, inbox: inbox, assignee: user_1) + + params[:payload] = [{ + attribute_key: 'display_id', + filter_operator: 'contains', + values: [conversation.display_id.to_s], + query_operator: nil, + custom_attribute_type: '' + }.with_indifferent_access] + + result = filter_service.new(params, user_1, account).perform + + expect(result[:count][:all_count]).to eq(1) + expect(result[:conversations].pluck(:id)).to contain_exactly(conversation.id) + end + it 'filters items with does not contain filter operator with values being an array' do params[:payload] = [{ attribute_key: 'browser_language', diff --git a/spec/services/conversations/unread_counts/counter_spec.rb b/spec/services/conversations/unread_counts/counter_spec.rb index 723f2d437..50efd7d43 100644 --- a/spec/services/conversations/unread_counts/counter_spec.rb +++ b/spec/services/conversations/unread_counts/counter_spec.rb @@ -95,4 +95,22 @@ RSpec.describe Conversations::UnreadCounts::Counter do teams: { visible_team.id.to_s => 1 } ) end + + it 'merges filtered counts when the filtered count feature is enabled' do + account.enable_features!(:unread_count_for_filters) + filtered_counter = instance_double( + Conversations::UnreadCounts::FilteredCounter, + perform: { mentions_count: 1, participating_count: 2, unattended_count: 3, folders: { '4' => 5 } } + ) + allow(Conversations::UnreadCounts::FilteredCounter).to receive(:new).and_return(filtered_counter) + + result = described_class.new(account: account, user: agent).perform + + expect(result).to include( + mentions_count: 1, + participating_count: 2, + unattended_count: 3, + folders: { '4' => 5 } + ) + end end diff --git a/spec/services/conversations/unread_counts/filtered_count_instrumentation_spec.rb b/spec/services/conversations/unread_counts/filtered_count_instrumentation_spec.rb new file mode 100644 index 000000000..4fb6c6d36 --- /dev/null +++ b/spec/services/conversations/unread_counts/filtered_count_instrumentation_spec.rb @@ -0,0 +1,137 @@ +require 'rails_helper' + +RSpec.describe Conversations::UnreadCounts::FilteredCountInstrumentation do + let(:new_relic_agent) do + Class.new do + def self.record_custom_event(*) end + def self.record_metric(*) end + end + end + + before do + stub_const('NewRelic::Agent', new_relic_agent) + allow(new_relic_agent).to receive(:record_custom_event) + allow(new_relic_agent).to receive(:record_metric) + end + + describe '.observe' do + it 'records duration metrics without custom events around successful operations' do + result = described_class.observe(:counter_perform, account_id: 1, snapshot_scope: :built_in_filter) { 'ok' } + + expect(result).to eq('ok') + expect(new_relic_agent).not_to have_received(:record_custom_event) + expect(new_relic_agent).to have_received(:record_metric).with( + 'Custom/Conversations/UnreadCounts/Filtered/counter_perform/duration_ms', + kind_of(Float) + ) + end + + it 'records failed operations and re-raises the original error' do + error = StandardError.new('boom') + + expect do + described_class.observe(:snapshot_build, account_id: 1) { raise error } + end.to raise_error(error) + expect(new_relic_agent).not_to have_received(:record_custom_event) + expect(new_relic_agent).to have_received(:record_metric).with( + 'Custom/Conversations/UnreadCounts/Filtered/snapshot_build/duration_ms', + kind_of(Float) + ) + end + end + + describe '.increment' do + it 'records count metrics without custom events for aggregated read-path operations' do + described_class.increment(:snapshot_state, account_id: 1, snapshot_status: :fresh) + + expect(new_relic_agent).not_to have_received(:record_custom_event) + expect(new_relic_agent).to have_received(:record_metric).with( + 'Custom/Conversations/UnreadCounts/Filtered/snapshot_state/count', + 1 + ) + end + + it 'keeps custom events for invalidation signals' do + described_class.increment(:invalidation, account_id: 1, invalidation_scope: :conversation) + + expect(new_relic_agent).to have_received(:record_custom_event).with( + 'FilteredUnreadCounts', + hash_including( + account_id: 1, + invalidation_scope: 'conversation', + operation: 'invalidation' + ) + ) + expect(new_relic_agent).to have_received(:record_metric).with( + 'Custom/Conversations/UnreadCounts/Filtered/invalidation/count', + 1 + ) + end + + it 'does not raise when New Relic is unavailable' do + allow(described_class).to receive(:new_relic_agent).and_return(nil) + + expect { described_class.increment(:snapshot_state, account_id: 1) }.not_to raise_error + end + end + + describe '.summarize_request' do + it 'records one custom event with aggregated request counters' do + result = described_class.summarize_request(account_id: 1) do + described_class.increment(:snapshot_state, account_id: 1, snapshot_scope: :built_in_filter, snapshot_status: :fresh) + described_class.increment(:snapshot_state, account_id: 1, snapshot_scope: :filter, snapshot_status: :missing) + described_class.increment(:refresh_claim, account_id: 1, snapshot_scope: :filter, claimed: true) + described_class.increment(:refresh_claim, account_id: 1, snapshot_scope: :filter, claimed: false) + described_class.increment(:build_lock, account_id: 1, snapshot_scope: :filter, acquired: true) + described_class.observe(:snapshot_build, account_id: 1, snapshot_scope: :filter) { 'built' } + + 'ok' + end + + expect(result).to eq('ok') + expect(new_relic_agent).to have_received(:record_custom_event).once.with( + 'FilteredUnreadCounts', + hash_including( + account_id: 1, + build_lock_acquired_count: 1, + duration_ms: kind_of(Float), + filter_build_lock_acquired_count: 1, + filter_refresh_claimed_count: 1, + filter_refresh_skipped_count: 1, + filter_snapshot_build_success_count: 1, + filter_snapshot_count: 1, + operation: 'request_summary', + refresh_claimed_count: 1, + refresh_skipped_count: 1, + snapshot_build_success_count: 1, + snapshot_fresh_count: 1, + snapshot_missing_count: 1, + snapshot_total_count: 2, + status: 'success' + ) + ) + expect(new_relic_agent).to have_received(:record_metric).with( + 'Custom/Conversations/UnreadCounts/Filtered/api_response/duration_ms', + kind_of(Float) + ) + end + + it 'records summary errors and re-raises the original error' do + error = StandardError.new('boom') + + expect do + described_class.summarize_request(account_id: 1) { raise error } + end.to raise_error(error) + + expect(new_relic_agent).to have_received(:record_custom_event).with( + 'FilteredUnreadCounts', + hash_including( + account_id: 1, + error_class: 'StandardError', + operation: 'request_summary', + status: 'error' + ) + ) + end + end +end diff --git a/spec/services/conversations/unread_counts/filtered_count_invalidator_spec.rb b/spec/services/conversations/unread_counts/filtered_count_invalidator_spec.rb new file mode 100644 index 000000000..e0324c4b5 --- /dev/null +++ b/spec/services/conversations/unread_counts/filtered_count_invalidator_spec.rb @@ -0,0 +1,244 @@ +require 'rails_helper' + +RSpec.describe Conversations::UnreadCounts::FilteredCountInvalidator do + subject(:invalidator) { described_class.new(account) } + + let(:account) { create(:account) } + let(:user) { create(:user, account: account) } + let(:other_user) { create(:user, account: account) } + let(:filter_id) { 123 } + let(:store) { Conversations::UnreadCounts::FilteredCountStore } + + after do + redis_keys.each { |key| Redis::Alfred.delete(key) } + end + + describe '#conversation_changed!' do + it 'bumps the account conversation version when the feature is enabled' do + account.enable_features!(:unread_count_for_filters) + + expect { invalidator.conversation_changed! }.to change { store.conversation_version(account.id) }.by(1) + end + + it 'records invalidation instrumentation when the feature is enabled' do + account.enable_features!(:unread_count_for_filters) + allow(Conversations::UnreadCounts::FilteredCountInstrumentation).to receive(:increment) + + invalidator.conversation_changed! + + expect(Conversations::UnreadCounts::FilteredCountInstrumentation).to have_received(:increment).with( + :invalidation, + account_id: account.id, + invalidation_scope: :conversation, + reason: :conversation_changed, + version: 1 + ) + end + + it 'does not write Redis keys when the feature is disabled' do + expect { invalidator.conversation_changed! }.not_to(change { store.conversation_version(account.id) }) + end + end + + describe '#user_visibility_changed!' do + it 'bumps the user built-in filter version' do + account.enable_features!(:unread_count_for_filters) + + expect do + invalidator.user_visibility_changed!(user_id: user.id) + end.to change { store.built_in_filter_version(account_id: account.id, user_id: user.id) }.by(1) + end + end + + describe '#users_visibility_changed!' do + it 'pipelines built-in filter version bumps for multiple users' do + account.enable_features!(:unread_count_for_filters) + user_ids = [user.id, other_user.id] + allow(Redis::Alfred).to receive(:pipelined).and_call_original + + expect do + invalidator.users_visibility_changed!(user_ids: user_ids + [user.id, nil]) + end.to change { built_in_filter_version_for(user) }.by(1) + .and change { built_in_filter_version_for(other_user) }.by(1) + + expect(Redis::Alfred).to have_received(:pipelined).once + end + + it 'does not write Redis keys when no user ids are present' do + account.enable_features!(:unread_count_for_filters) + + expect(invalidator.users_visibility_changed!(user_ids: [nil, ''])).to be(false) + end + end + + describe '#custom_filter_created!' do + it 'bumps the folder index and saved filter versions for conversation filters' do + account.enable_features!(:unread_count_for_filters) + filter_version = store.filter_version(account_id: account.id, filter_id: filter_id) + + expect do + invalidator.custom_filter_created!(conversation_filter) + end.to change { store.folder_index_version(account_id: account.id, user_id: user.id) }.by(1) + expect(store.filter_version(account_id: account.id, filter_id: filter_id)).to eq(filter_version + 1) + end + + it 'ignores non-conversation filters' do + account.enable_features!(:unread_count_for_filters) + + expect do + invalidator.custom_filter_created!(conversation_filter(is_conversation: false)) + end.not_to(change { store.folder_index_version(account_id: account.id, user_id: user.id) }) + end + end + + describe '#custom_filter_updated!' do + it 'bumps only the filter version when the query changes' do + account.enable_features!(:unread_count_for_filters) + filter = conversation_filter(previous_changes: { 'query' => [{ status: 'open' }, { status: 'resolved' }] }) + folder_index_version = store.folder_index_version(account_id: account.id, user_id: user.id) + + expect do + invalidator.custom_filter_updated!(filter) + end.to change { store.filter_version(account_id: account.id, filter_id: filter_id) }.by(1) + expect(store.folder_index_version(account_id: account.id, user_id: user.id)).to eq(folder_index_version) + end + + it 'ignores name-only updates' do + account.enable_features!(:unread_count_for_filters) + filter = conversation_filter(previous_changes: { 'name' => %w[Open Resolved] }) + + expect do + invalidator.custom_filter_updated!(filter) + end.not_to(change { store.filter_version(account_id: account.id, filter_id: filter_id) }) + end + + it 'bumps versions and deletes the saved count when the filter moves away from conversations' do + account.enable_features!(:unread_count_for_filters) + filter = conversation_filter( + is_conversation: false, + previous_changes: { 'filter_type' => %w[conversation contact] } + ) + store.write_filter_count!( + account_id: account.id, + filter_id: filter_id, + user_id: user.id, + count: 4, + account_version: 0, + filter_version: 0, + owner_built_in_filter_version: 0 + ) + filter_version = store.filter_version(account_id: account.id, filter_id: filter_id) + + expect do + invalidator.custom_filter_updated!(filter) + end.to change { store.folder_index_version(account_id: account.id, user_id: user.id) }.by(1) + expect(store.filter_version(account_id: account.id, filter_id: filter_id)).to eq(filter_version + 1) + expect(store.filter_count(account_id: account.id, filter_id: filter_id)).to be_nil + end + end + + describe '#custom_filter_destroyed!' do + it 'bumps the folder index version and deletes the saved count' do + account.enable_features!(:unread_count_for_filters) + store.write_filter_count!( + account_id: account.id, + filter_id: filter_id, + user_id: user.id, + count: 2, + account_version: 0, + filter_version: 0, + owner_built_in_filter_version: 0 + ) + + expect do + invalidator.custom_filter_destroyed!(conversation_filter) + end.to change { store.folder_index_version(account_id: account.id, user_id: user.id) }.by(1) + expect(store.filter_count(account_id: account.id, filter_id: filter_id)).to be_nil + end + end + + describe '#custom_attribute_definition_changed!' do + it 'bumps affected conversation saved filter versions' do + account.enable_features!(:unread_count_for_filters) + definition = create(:custom_attribute_definition, account: account, attribute_key: 'plan', attribute_model: 'conversation_attribute') + matching_filter = create(:custom_filter, account: account, user: user, query: custom_attribute_query('plan')) + blank_type_filter = create(:custom_filter, account: account, user: user, query: custom_attribute_query('plan', '')) + contact_filter = create(:custom_filter, account: account, user: user, query: custom_attribute_query('plan', 'contact_attribute')) + other_filter = create(:custom_filter, account: account, user: user, query: custom_attribute_query('tier')) + versions = filter_versions(matching_filter, blank_type_filter, contact_filter, other_filter) + + invalidator.custom_attribute_definition_changed!(definition) + + expect(store.filter_version(account_id: account.id, filter_id: matching_filter.id)).to eq(versions[matching_filter.id] + 1) + expect(store.filter_version(account_id: account.id, filter_id: blank_type_filter.id)).to eq(versions[blank_type_filter.id] + 1) + expect(store.filter_version(account_id: account.id, filter_id: contact_filter.id)).to eq(versions[contact_filter.id]) + expect(store.filter_version(account_id: account.id, filter_id: other_filter.id)).to eq(versions[other_filter.id]) + end + + it 'bumps filters referencing the previous attribute key when the key changes' do + definition = create(:custom_attribute_definition, account: account, attribute_key: 'plan', attribute_model: 'conversation_attribute') + matching_filter = create(:custom_filter, account: account, user: user, query: custom_attribute_query('plan')) + version = store.filter_version(account_id: account.id, filter_id: matching_filter.id) + + definition.update!(attribute_key: 'new_plan') + account.enable_features!(:unread_count_for_filters) + + expect do + invalidator.custom_attribute_definition_changed!(definition) + end.to change { store.filter_version(account_id: account.id, filter_id: matching_filter.id) }.from(version).to(version + 1) + end + end + + def conversation_filter(is_conversation: true, previous_changes: {}) + instance_double( + CustomFilter, + id: filter_id, + user_id: user.id, + conversation?: is_conversation, + previous_changes: previous_changes + ) + end + + def custom_attribute_query(attribute_key, custom_attribute_type = 'conversation_attribute') + { + payload: [ + { + attribute_key: attribute_key, + filter_operator: 'equal_to', + values: ['gold'], + custom_attribute_type: custom_attribute_type + } + ] + } + end + + def filter_versions(*custom_filters) + custom_filters.to_h { |custom_filter| [custom_filter.id, store.filter_version(account_id: account.id, filter_id: custom_filter.id)] } + end + + def built_in_filter_version_for(user) + store.built_in_filter_version(account_id: account.id, user_id: user.id) + end + + def redis_keys + base_redis_keys + custom_filter_version_keys + end + + def base_redis_keys + [ + store.conversation_version_key(account.id), + *built_in_filter_version_keys, + store.folder_index_version_key(account.id, user.id), + store.filter_version_key(account.id, filter_id), + store.filter_count_key(account.id, filter_id) + ] + end + + def built_in_filter_version_keys + [user.id, other_user.id].map { |user_id| store.built_in_filter_version_key(account.id, user_id) } + end + + def custom_filter_version_keys + CustomFilter.where(account_id: account.id).pluck(:id).map { |id| store.filter_version_key(account.id, id) } + end +end diff --git a/spec/services/conversations/unread_counts/filtered_count_store_spec.rb b/spec/services/conversations/unread_counts/filtered_count_store_spec.rb new file mode 100644 index 000000000..7732eb2dd --- /dev/null +++ b/spec/services/conversations/unread_counts/filtered_count_store_spec.rb @@ -0,0 +1,262 @@ +require 'rails_helper' + +RSpec.describe Conversations::UnreadCounts::FilteredCountStore do + let(:account_id) { 1 } + let(:user_id) { 2 } + let(:filter_id) { 3 } + let(:built_at) { Time.zone.parse('2026-06-29 10:00:00 UTC') } + + after do + redis_keys.each { |key| Redis::Alfred.delete(key) } + end + + describe 'key builders' do + it 'builds V2 keys for built-in filters, folder indexes, and saved filters' do + expect(described_class.conversation_version_key(account_id)).to eq( + 'UNREAD_CONVERSATIONS::V2::ACCOUNT::1::CONVERSATION_VERSION' + ) + expect(described_class.built_in_filter_version_key(account_id, user_id)).to eq( + 'UNREAD_CONVERSATIONS::V2::ACCOUNT::1::USER::2::BUILT_IN_FILTER_VERSION' + ) + expect(described_class.built_in_filter_counts_key(account_id, user_id)).to eq( + 'UNREAD_CONVERSATIONS::V2::ACCOUNT::1::USER::2::BUILT_IN_FILTER_COUNTS' + ) + expect(described_class.folder_index_key(account_id, user_id)).to eq( + 'UNREAD_CONVERSATIONS::V2::ACCOUNT::1::USER::2::FOLDER_INDEX' + ) + expect(described_class.filter_count_key(account_id, filter_id)).to eq( + 'UNREAD_CONVERSATIONS::V2::ACCOUNT::1::FILTER::3::COUNT' + ) + end + end + + describe 'version metadata' do + it 'defaults missing version keys to zero' do + expect(described_class.conversation_version(account_id)).to eq(0) + expect(described_class.built_in_filter_version(account_id: account_id, user_id: user_id)).to eq(0) + expect(described_class.folder_index_version(account_id: account_id, user_id: user_id)).to eq(0) + expect(described_class.filter_version(account_id: account_id, filter_id: filter_id)).to eq(0) + end + + it 'increments independent version keys' do + expect(described_class.bump_conversation_version!(account_id)).to eq(1) + expect(described_class.bump_built_in_filter_version!(account_id: account_id, user_id: user_id)).to eq(1) + expect(described_class.bump_folder_index_version!(account_id: account_id, user_id: user_id)).to eq(1) + expect(described_class.bump_filter_version!(account_id: account_id, filter_id: filter_id)).to eq(1) + end + + it 'increments and expires version keys in one Redis transaction' do + key = described_class.conversation_version_key(account_id) + connection = instance_double(Redis) + transaction = instance_double(Redis::MultiConnection) + + allow(Redis::Alfred).to receive(:with).and_yield(connection) + expect(connection).to receive(:multi).and_yield(transaction).and_return([1, true]) + expect(transaction).to receive(:incr).with(key) + expect(transaction).to receive(:expire).with(key, Conversations::UnreadCounts::FILTERED_COUNT_VERSION_TTL) + + expect(described_class.bump_conversation_version!(account_id)).to eq(1) + end + + it 'expires version keys after bumping them' do + described_class.bump_conversation_version!(account_id) + described_class.bump_built_in_filter_version!(account_id: account_id, user_id: user_id) + described_class.bump_folder_index_version!(account_id: account_id, user_id: user_id) + described_class.bump_filter_version!(account_id: account_id, filter_id: filter_id) + + version_keys.each do |key| + expect(ttl_for(key)).to be_within(5).of(Conversations::UnreadCounts::FILTERED_COUNT_VERSION_TTL) + end + end + end + + describe 'built-in filter count snapshots' do + it 'round-trips counts and classifies fresh, stale, expired, and missing snapshots' do + account_version = described_class.bump_conversation_version!(account_id) + built_in_filter_version = described_class.bump_built_in_filter_version!(account_id: account_id, user_id: user_id) + + described_class.write_built_in_filter_counts!( + account_id: account_id, + user_id: user_id, + account_version: account_version, + built_in_filter_version: built_in_filter_version, + built_at: built_at, + counts: { mentions_count: 3, participating_count: 4, unattended_count: 5 }, + meta: { permission_mode: 'base' } + ) + + snapshot = described_class.built_in_filter_counts(account_id: account_id, user_id: user_id) + expect(snapshot[:counts]).to eq(mentions_count: 3, participating_count: 4, unattended_count: 5) + expect(snapshot[:meta]).to eq(permission_mode: 'base') + expect(ttl_for(described_class.built_in_filter_counts_key(account_id, user_id))).to be_within(5).of( + Conversations::UnreadCounts::FILTERED_COUNT_REDIS_TTL + ) + + expect(described_class.built_in_filter_counts_state(account_id: account_id, user_id: user_id, now: built_at + 1.minute)).to be_fresh + + described_class.bump_built_in_filter_version!(account_id: account_id, user_id: user_id) + expect(described_class.built_in_filter_counts_state(account_id: account_id, user_id: user_id, now: built_at + 2.minutes)).to be_stale + expect(described_class.built_in_filter_counts_state(account_id: account_id, user_id: user_id, now: built_at + 36.minutes)).to be_expired + + Redis::Alfred.delete(described_class.built_in_filter_counts_key(account_id, user_id)) + expect(described_class.built_in_filter_counts_state(account_id: account_id, user_id: user_id)).to be_missing + end + end + + describe 'folder index snapshots' do + it 'round-trips folder ids and classifies freshness against the folder index version' do + folder_index_version = described_class.bump_folder_index_version!(account_id: account_id, user_id: user_id) + + described_class.write_folder_index!( + account_id: account_id, + user_id: user_id, + folder_index_version: folder_index_version, + built_at: built_at, + filter_ids: %w[10 11] + ) + + expect(described_class.folder_index(account_id: account_id, user_id: user_id)[:filter_ids]).to eq([10, 11]) + expect(described_class.folder_index_state(account_id: account_id, user_id: user_id, now: built_at + 1.minute)).to be_fresh + + described_class.bump_folder_index_version!(account_id: account_id, user_id: user_id) + expect(described_class.folder_index_state(account_id: account_id, user_id: user_id, now: built_at + 2.minutes)).to be_stale + end + end + + describe 'saved filter count snapshots' do + it 'round-trips counts and uses account, filter, and owner built-in filter versions for freshness' do + account_version = described_class.bump_conversation_version!(account_id) + filter_version = described_class.bump_filter_version!(account_id: account_id, filter_id: filter_id) + owner_built_in_filter_version = described_class.bump_built_in_filter_version!(account_id: account_id, user_id: user_id) + + described_class.write_filter_count!( + account_id: account_id, + filter_id: filter_id, + user_id: user_id, + count: 7, + account_version: account_version, + filter_version: filter_version, + owner_built_in_filter_version: owner_built_in_filter_version, + built_at: built_at, + meta: { status: 'ok', timed_out: false, invalid_filter: false } + ) + + snapshot = described_class.filter_count(account_id: account_id, filter_id: filter_id) + expect(snapshot[:count]).to eq(7) + expect(snapshot[:meta]).to eq(status: 'ok', timed_out: false, invalid_filter: false) + expect(described_class.filter_count_state(account_id: account_id, filter_id: filter_id, now: built_at + 1.minute)).to be_fresh + + described_class.bump_filter_version!(account_id: account_id, filter_id: filter_id) + expect(described_class.filter_count_state(account_id: account_id, filter_id: filter_id, now: built_at + 2.minutes)).to be_stale + + described_class.delete_filter_count!(account_id: account_id, filter_id: filter_id) + expect(described_class.filter_count(account_id: account_id, filter_id: filter_id)).to be_nil + end + + it 'uses caller-provided versions when classifying snapshots' do + account_version = described_class.bump_conversation_version!(account_id) + filter_version = described_class.bump_filter_version!(account_id: account_id, filter_id: filter_id) + owner_built_in_filter_version = described_class.bump_built_in_filter_version!(account_id: account_id, user_id: user_id) + + described_class.write_filter_count!( + account_id: account_id, + filter_id: filter_id, + user_id: user_id, + count: 7, + account_version: account_version, + filter_version: filter_version, + owner_built_in_filter_version: owner_built_in_filter_version, + built_at: built_at + ) + + versions = { + account_version: account_version, + filter_version: filter_version, + owner_built_in_filter_version: owner_built_in_filter_version + } + + expect(described_class).not_to receive(:conversation_version) + expect(described_class).not_to receive(:filter_version) + expect(described_class).not_to receive(:built_in_filter_version) + + expect( + described_class.filter_count_state( + account_id: account_id, + filter_id: filter_id, + versions: versions, + now: built_at + 1.minute + ) + ).to be_fresh + end + end + + describe 'refresh throttles' do + it 'uses refresh_after and independent throttle keys to suppress duplicate rebuilds' do + described_class.write_built_in_filter_counts!( + account_id: account_id, + user_id: user_id, + account_version: 0, + built_in_filter_version: 0, + built_at: built_at, + counts: {} + ) + + snapshot = described_class.built_in_filter_counts(account_id: account_id, user_id: user_id) + expect(described_class.refresh_due?(snapshot, now: built_at + 10.seconds)).to be(false) + expect(described_class.refresh_due?(snapshot, now: built_at + 31.seconds)).to be(true) + + expect(described_class.claim_built_in_filter_refresh!(account_id: account_id, user_id: user_id)).to be(true) + expect(described_class.claim_built_in_filter_refresh!(account_id: account_id, user_id: user_id)).to be(false) + expect(described_class.claim_folder_index_refresh!(account_id: account_id, user_id: user_id)).to be(true) + expect(described_class.claim_filter_refresh!(account_id: account_id, filter_id: filter_id)).to be(true) + end + end + + describe 'Redis access pattern' do + it 'does not scan Redis keys' do + expect(Redis::Alfred).not_to receive(:scan_each) + + described_class.bump_conversation_version!(account_id) + described_class.write_folder_index!(account_id: account_id, user_id: user_id, folder_index_version: 0, filter_ids: [filter_id]) + described_class.folder_index_state(account_id: account_id, user_id: user_id) + described_class.claim_filter_refresh!(account_id: account_id, filter_id: filter_id) + described_class.delete_filter_count!(account_id: account_id, filter_id: filter_id) + end + end + + def ttl_for(key) + Redis::Alfred.ttl(key) + end + + def redis_keys + version_keys + snapshot_keys + lock_and_throttle_keys + end + + def version_keys + [ + described_class.conversation_version_key(account_id), + described_class.built_in_filter_version_key(account_id, user_id), + described_class.folder_index_version_key(account_id, user_id), + described_class.filter_version_key(account_id, filter_id) + ] + end + + def snapshot_keys + [ + described_class.built_in_filter_counts_key(account_id, user_id), + described_class.folder_index_key(account_id, user_id), + described_class.filter_count_key(account_id, filter_id) + ] + end + + def lock_and_throttle_keys + [ + described_class.built_in_filter_build_lock_key(account_id, user_id), + described_class.built_in_filter_refresh_throttle_key(account_id, user_id), + described_class.folder_index_build_lock_key(account_id, user_id), + described_class.folder_index_refresh_throttle_key(account_id, user_id), + described_class.filter_build_lock_key(account_id, filter_id), + described_class.filter_refresh_throttle_key(account_id, filter_id) + ] + end +end diff --git a/spec/services/conversations/unread_counts/filtered_counter_spec.rb b/spec/services/conversations/unread_counts/filtered_counter_spec.rb new file mode 100644 index 000000000..bb5d419a6 --- /dev/null +++ b/spec/services/conversations/unread_counts/filtered_counter_spec.rb @@ -0,0 +1,549 @@ +require 'rails_helper' + +RSpec.describe Conversations::UnreadCounts::FilteredCounter do + subject(:counter) { described_class.new(account: account, user: agent, now: now) } + + let(:account) { create(:account) } + let(:agent) { create(:user, account: account, role: :agent) } + let(:visible_inbox) { create(:inbox, account: account) } + let(:hidden_inbox) { create(:inbox, account: account) } + let(:now) { Time.zone.parse('2026-06-29 10:00:00 UTC') } + let(:store) { Conversations::UnreadCounts::FilteredCountStore } + + before do + create(:inbox_member, user: agent, inbox: visible_inbox) + end + + after do + redis_keys.each { |key| Redis::Alfred.delete(key) } + end + + it 'builds built-in filter counts from unread open conversations visible to the user' do + mentioned = create_visible_unread_conversation + participating = create_visible_unread_conversation + create_visible_unread_conversation(unattended: true) + hidden_mention = create_unread_conversation(account: account, inbox: hidden_inbox) + resolved_mention = create_visible_unread_conversation(status: :resolved) + read_mention = create_visible_unread_conversation(agent_last_seen_at: 1.minute.from_now) + + [mentioned, hidden_mention, resolved_mention, read_mention].each do |conversation| + create(:mention, account: account, conversation: conversation, user: agent) + end + create(:conversation_participant, account: account, conversation: participating, user: agent) + + expect(counter.perform).to include( + mentions_count: 1, + participating_count: 1, + unattended_count: 1 + ) + end + + it 'returns stale built-in counts until the refresh interval elapses' do + mentioned = create_visible_unread_conversation + create(:mention, account: account, conversation: mentioned, user: agent) + + expect(counter.perform[:mentions_count]).to eq(1) + + second_mention = create_visible_unread_conversation + create(:mention, account: account, conversation: second_mention, user: agent) + store.bump_conversation_version!(account.id) + + expect(described_class.new(account: account, user: agent, now: now + 10.seconds).perform[:mentions_count]).to eq(1) + + Redis::Alfred.delete(store.built_in_filter_refresh_throttle_key(account.id, agent.id)) + expect(described_class.new(account: account, user: agent, now: now + 31.seconds).perform[:mentions_count]).to eq(2) + end + + it 'returns stale built-in counts when a refresh build hits a database error' do + mentioned = create_visible_unread_conversation + create(:mention, account: account, conversation: mentioned, user: agent) + + expect(counter.perform[:mentions_count]).to eq(1) + + store.bump_conversation_version!(account.id) + Redis::Alfred.delete(store.built_in_filter_refresh_throttle_key(account.id, agent.id)) + failing_counter = described_class.new(account: account, user: agent, now: now + 31.seconds) + allow(failing_counter).to receive(:built_in_counts_from_database).and_raise(ActiveRecord::StatementInvalid.new('statement timeout')) + + expect(failing_counter.perform[:mentions_count]).to eq(1) + end + + it 'tags built-in snapshots with versions captured before the DB read' do + race_counter = described_class.new(account: account, user: agent, now: now) + allow(race_counter).to receive(:built_in_counts_from_database) do + store.bump_conversation_version!(account.id) + { mentions_count: 1, participating_count: 0, unattended_count: 0 } + end + + race_counter.perform + + snapshot = store.built_in_filter_counts(account_id: account.id, user_id: agent.id) + expect(snapshot[:account_version]).to eq(0) + expect(store.built_in_filter_counts_state(account_id: account.id, user_id: agent.id, now: now)).to be_stale + end + + it 'tags folder indexes with versions captured before the DB read' do + race_counter = described_class.new(account: account, user: agent, now: now) + allow(race_counter).to receive(:folder_filter_ids_from_database) do + store.bump_folder_index_version!(account_id: account.id, user_id: agent.id) + [] + end + + race_counter.send(:build_folder_index!, race_counter.send(:version_cache).folder_index) + + snapshot = store.folder_index(account_id: account.id, user_id: agent.id) + expect(snapshot[:folder_index_version]).to eq(0) + expect(store.folder_index_state(account_id: account.id, user_id: agent.id, now: now)).to be_stale + end + + it 'builds saved folder counts from unread conversations matching the saved filter query' do + resolved = create_visible_unread_conversation(status: :resolved) + create_visible_unread_conversation(status: :open) + hidden_resolved = create_unread_conversation(account: account, inbox: hidden_inbox) + hidden_resolved.update!(status: :resolved) + custom_filter = create( + :custom_filter, + account: account, + user: agent, + filter_type: :conversation, + query: filter_query(attribute_key: 'status', values: ['resolved']) + ) + + expect(counter.perform[:folders]).to eq(custom_filter.id.to_s => 1) + expect(store.filter_count(account_id: account.id, filter_id: custom_filter.id)[:count]).to eq(1) + expect(resolved.reload.status).to eq('resolved') + end + + it 'caps inline saved filter builds per request' do + create_visible_unread_conversation(status: :open) + max_inline_filter_builds = Conversations::UnreadCounts::MAX_INLINE_FILTER_BUILDS + custom_filters = Array.new(max_inline_filter_builds + 1) do + create( + :custom_filter, + account: account, + user: agent, + filter_type: :conversation, + query: filter_query(attribute_key: 'status', values: ['open']) + ) + end + query_counter = instance_double(Conversations::UnreadCounts::FilterQueryCounter, perform: 1) + allow(Conversations::UnreadCounts::FilterQueryCounter).to receive(:new).and_return(query_counter) + + result = counter.perform + + expect(result[:folders].size).to eq(max_inline_filter_builds) + expect(Conversations::UnreadCounts::FilterQueryCounter).to have_received(:new).exactly(max_inline_filter_builds).times + expect(custom_filters.count { |custom_filter| store.filter_count(account_id: account.id, filter_id: custom_filter.id).present? }).to eq( + max_inline_filter_builds + ) + end + + it 'reuses shared versions while resolving multiple saved filters' do + create_visible_unread_conversation(status: :open) + 2.times do + create( + :custom_filter, + account: account, + user: agent, + filter_type: :conversation, + query: filter_query(attribute_key: 'status', values: ['open']) + ) + end + query_counter = instance_double(Conversations::UnreadCounts::FilterQueryCounter, perform: 1) + allow(Conversations::UnreadCounts::FilterQueryCounter).to receive(:new).and_return(query_counter) + expect(store).to receive(:conversation_version).with(account.id).once.and_call_original + expect(store).to receive(:built_in_filter_version).with(account_id: account.id, user_id: agent.id).once.and_call_original + + expect(counter.perform[:folders].size).to eq(2) + end + + it 'tags saved filter counts with versions captured before the DB read' do + custom_filter = create( + :custom_filter, + account: account, + user: agent, + filter_type: :conversation, + query: filter_query(attribute_key: 'status', values: ['open']) + ) + race_counter = described_class.new(account: account, user: agent, now: now) + allow(race_counter).to receive(:filter_query_count) do + store.bump_filter_version!(account_id: account.id, filter_id: custom_filter.id) + 1 + end + + race_counter.send(:build_filter_count!, custom_filter.id, race_counter.send(:version_cache).filter(custom_filter.id)) + + snapshot = store.filter_count(account_id: account.id, filter_id: custom_filter.id) + expect(snapshot[:filter_version]).to eq(0) + expect(store.filter_count_state(account_id: account.id, filter_id: custom_filter.id, owner_user_id: agent.id, now: now)).to be_stale + end + + it 'tags saved filter counts with versions captured before loading the filter row' do + custom_filter = create( + :custom_filter, + account: account, + user: agent, + filter_type: :conversation, + query: filter_query(attribute_key: 'status', values: ['open']) + ) + filters = account.custom_filters + allow(account).to receive(:custom_filters).and_return(filters) + allow(filters).to receive(:find_by) do + store.bump_filter_version!(account_id: account.id, filter_id: custom_filter.id) + custom_filter + end + + counter.send(:build_filter_count!, custom_filter.id, counter.send(:version_cache).filter(custom_filter.id)) + + snapshot = store.filter_count(account_id: account.id, filter_id: custom_filter.id) + expect(snapshot[:filter_version]).to eq(0) + expect(store.filter_count_state(account_id: account.id, filter_id: custom_filter.id, owner_user_id: agent.id, now: now)).to be_stale + end + + it 'omits invalid saved folders without writing a badge count' do + custom_filter = create( + :custom_filter, + account: account, + user: agent, + filter_type: :conversation, + query: filter_query(attribute_key: 'unknown_attribute', values: ['value']) + ) + + expect(counter.perform[:folders]).to eq({}) + expect(store.filter_count(account_id: account.id, filter_id: custom_filter.id)).to be_nil + end + + it 'records snapshot lifecycle instrumentation while calculating counts' do + allow(Conversations::UnreadCounts::FilteredCountInstrumentation).to receive(:observe) do |_operation, _attributes, &block| + block.call + end + allow(Conversations::UnreadCounts::FilteredCountInstrumentation).to receive(:increment) + + counter.perform + + expect(Conversations::UnreadCounts::FilteredCountInstrumentation).to have_received(:observe).with(:counter_perform, account_id: account.id) + expect(Conversations::UnreadCounts::FilteredCountInstrumentation).to have_received(:observe).with( + :snapshot_build, + account_id: account.id, + snapshot_scope: :built_in_filter + ) + expect(Conversations::UnreadCounts::FilteredCountInstrumentation).to have_received(:increment).with( + :snapshot_state, + account_id: account.id, + snapshot_scope: :built_in_filter, + snapshot_status: :missing + ) + expect(Conversations::UnreadCounts::FilteredCountInstrumentation).to have_received(:increment).with( + :refresh_claim, + account_id: account.id, + snapshot_scope: :built_in_filter, + claimed: true + ) + end + + it 'records acquired build locks when snapshot builds fail' do + error = StandardError.new('snapshot failed') + lock_manager = instance_double(Redis::LockManager) + resolver = Conversations::UnreadCounts::FilteredCountSnapshotResolver.new( + account: account, + now: now, + store: store, + lock_manager: lock_manager + ) + state = Conversations::UnreadCounts::FilteredCountStore::SnapshotResult.new(status: :missing, payload: nil) + + allow(lock_manager).to receive(:with_lock) + .with('lock-key', Conversations::UnreadCounts::FilteredCountSnapshotResolver::BUILD_LOCK_TTL) + .and_yield + .and_return(true) + allow(Conversations::UnreadCounts::FilteredCountInstrumentation).to receive(:observe) do |_operation, _attributes, &block| + block.call + end + allow(Conversations::UnreadCounts::FilteredCountInstrumentation).to receive(:increment) + + expect do + resolver.resolve(scope: :built_in_filter, state: state, lock_key: 'lock-key', claim_refresh: -> { true }) { raise error } + end.to raise_error(error) + expect(Conversations::UnreadCounts::FilteredCountInstrumentation).to have_received(:increment).with( + :build_lock, + account_id: account.id, + snapshot_scope: :built_in_filter, + acquired: true + ) + end + + it 'omits saved folders with malformed query payloads' do + custom_filter = create( + :custom_filter, + account: account, + user: agent, + filter_type: :conversation, + query: filter_query(attribute_key: 'status', values: 'open') + ) + + expect(counter.perform[:folders]).to eq({}) + expect(store.filter_count(account_id: account.id, filter_id: custom_filter.id)).to be_nil + end + + it 'omits saved folders with trailing query operators' do + query = filter_query(attribute_key: 'status', values: ['open']) + query[:payload].first[:query_operator] = 'AND' + custom_filter = create( + :custom_filter, + account: account, + user: agent, + filter_type: :conversation, + query: query + ) + + expect(counter.perform[:folders]).to eq({}) + expect(store.filter_count(account_id: account.id, filter_id: custom_filter.id)).to be_nil + end + + it 'omits saved folders with invalid typed values' do + custom_filter = create( + :custom_filter, + account: account, + user: agent, + filter_type: :conversation, + query: filter_query(attribute_key: 'team_id', values: ['abc']) + ) + + expect(counter.perform[:folders]).to eq({}) + expect(store.filter_count(account_id: account.id, filter_id: custom_filter.id)).to be_nil + end + + it 'omits saved folders with invalid ID values' do + custom_filter = create( + :custom_filter, + account: account, + user: agent, + filter_type: :conversation, + query: filter_query(attribute_key: 'assignee_id', values: ['abc']) + ) + + expect(counter.perform[:folders]).to eq({}) + expect(store.filter_count(account_id: account.id, filter_id: custom_filter.id)).to be_nil + end + + it 'counts saved folders with display_id substring filters' do + conversation = create_visible_unread_conversation + create_visible_unread_conversation + custom_filter = create( + :custom_filter, + account: account, + user: agent, + filter_type: :conversation, + query: filter_query(attribute_key: 'display_id', filter_operator: 'contains', values: [conversation.display_id.to_s]) + ) + + expect(counter.perform[:folders]).to eq(custom_filter.id.to_s => 1) + expect(store.filter_count(account_id: account.id, filter_id: custom_filter.id)[:count]).to eq(1) + end + + it 'counts saved folders with display_id text fragment filters' do + create_visible_unread_conversation + create_visible_unread_conversation + custom_filter = create( + :custom_filter, + account: account, + user: agent, + filter_type: :conversation, + query: filter_query(attribute_key: 'display_id', filter_operator: 'does_not_contain', values: ['abc']) + ) + + expect(counter.perform[:folders]).to eq(custom_filter.id.to_s => 2) + expect(store.filter_count(account_id: account.id, filter_id: custom_filter.id)[:count]).to eq(2) + end + + it 'omits saved folders with invalid typed custom attribute values' do + create( + :custom_attribute_definition, + account: account, + attribute_model: :conversation_attribute, + attribute_key: 'budget', + attribute_display_type: :number + ) + custom_filter = create( + :custom_filter, + account: account, + user: agent, + filter_type: :conversation, + query: filter_query(attribute_key: 'budget', values: ['abc']) + ) + + expect(counter.perform[:folders]).to eq({}) + expect(store.filter_count(account_id: account.id, filter_id: custom_filter.id)).to be_nil + end + + it 'omits saved folders with text operators on typed custom attributes' do + create( + :custom_attribute_definition, + account: account, + attribute_model: :conversation_attribute, + attribute_key: 'budget', + attribute_display_type: :number + ) + custom_filter = create( + :custom_filter, + account: account, + user: agent, + filter_type: :conversation, + query: filter_query(attribute_key: 'budget', filter_operator: 'contains', values: ['123']) + ) + + expect(counter.perform[:folders]).to eq({}) + expect(store.filter_count(account_id: account.id, filter_id: custom_filter.id)).to be_nil + end + + it 'omits saved folders with invalid label values' do + custom_filter = create( + :custom_filter, + account: account, + user: agent, + filter_type: :conversation, + query: filter_query(attribute_key: 'labels', values: [1]) + ) + + expect(counter.perform[:folders]).to eq({}) + expect(store.filter_count(account_id: account.id, filter_id: custom_filter.id)).to be_nil + end + + it 'omits saved folders with invalid text values' do + custom_filter = create( + :custom_filter, + account: account, + user: agent, + filter_type: :conversation, + query: filter_query(attribute_key: 'mail_subject', values: [1]) + ) + + expect(counter.perform[:folders]).to eq({}) + expect(store.filter_count(account_id: account.id, filter_id: custom_filter.id)).to be_nil + end + + it 'omits saved folders with invalid date custom attribute values' do + create( + :custom_attribute_definition, + account: account, + attribute_model: :conversation_attribute, + attribute_key: 'renewal_on', + attribute_display_type: :date + ) + custom_filter = create( + :custom_filter, + account: account, + user: agent, + filter_type: :conversation, + query: filter_query(attribute_key: 'renewal_on', values: ['not-a-date']) + ) + + expect(counter.perform[:folders]).to eq({}) + expect(store.filter_count(account_id: account.id, filter_id: custom_filter.id)).to be_nil + end + + it 'omits saved folders when stored custom attribute values cannot be cast' do + create( + :custom_attribute_definition, + account: account, + attribute_model: :conversation_attribute, + attribute_key: 'budget', + attribute_display_type: :number + ) + query_counter = Conversations::UnreadCounts::FilterQueryCounter.new( + account: account, + user: agent, + query: filter_query(attribute_key: 'budget', filter_operator: 'is_present', values: []) + ) + relation = instance_double(ActiveRecord::Relation) + cast_error = ActiveRecord::StatementInvalid.new('PG::InvalidTextRepresentation: invalid input syntax for type numeric') + allow(cast_error).to receive(:cause).and_return(PG::InvalidTextRepresentation.new('invalid input syntax for type numeric')) + allow(query_counter).to receive(:query_builder).and_return(relation) + allow(relation).to receive(:count).and_raise(cast_error) + + expect(query_counter.perform).to be_nil + end + + it 'counts saved folders with days_before date filters' do + old_conversation = create_visible_unread_conversation + old_conversation.update!(created_at: 8.days.ago) + create_visible_unread_conversation + custom_filter = create( + :custom_filter, + account: account, + user: agent, + filter_type: :conversation, + query: filter_query(attribute_key: 'created_at', filter_operator: 'days_before', values: [7]) + ) + + expect(counter.perform[:folders]).to eq(custom_filter.id.to_s => 1) + expect(store.filter_count(account_id: account.id, filter_id: custom_filter.id)[:count]).to eq(1) + end + + def create_visible_unread_conversation(status: :open, agent_last_seen_at: 1.hour.ago, unattended: false) + conversation = create_unread_conversation(account: account, inbox: visible_inbox) + conversation.update!( + status: status, + agent_last_seen_at: agent_last_seen_at, + first_reply_created_at: unattended ? nil : Time.current, + waiting_since: unattended ? 5.minutes.ago : nil + ) + conversation + end + + def filter_query(attribute_key:, values:, filter_operator: 'equal_to') + { + payload: [{ + attribute_key: attribute_key, + attribute_model: 'standard', + filter_operator: filter_operator, + values: values + }] + } + end + + def redis_keys + version_keys + snapshot_keys + lock_and_throttle_keys + end + + def filter_ids + CustomFilter.where(account_id: account.id).pluck(:id) + end + + def version_keys + [ + store.conversation_version_key(account.id), + store.built_in_filter_version_key(account.id, agent.id), + store.folder_index_version_key(account.id, agent.id) + ] + filter_ids.map { |filter_id| store.filter_version_key(account.id, filter_id) } + end + + def snapshot_keys + [ + store.built_in_filter_counts_key(account.id, agent.id), + store.folder_index_key(account.id, agent.id) + ] + filter_ids.map { |filter_id| store.filter_count_key(account.id, filter_id) } + end + + def lock_and_throttle_keys + user_lock_and_throttle_keys + filter_lock_and_throttle_keys + end + + def user_lock_and_throttle_keys + [ + store.built_in_filter_build_lock_key(account.id, agent.id), + store.built_in_filter_refresh_throttle_key(account.id, agent.id), + store.folder_index_build_lock_key(account.id, agent.id), + store.folder_index_refresh_throttle_key(account.id, agent.id) + ] + end + + def filter_lock_and_throttle_keys + filter_ids.flat_map do |filter_id| + [ + store.filter_build_lock_key(account.id, filter_id), + store.filter_refresh_throttle_key(account.id, filter_id) + ] + end + end +end diff --git a/spec/services/conversations/unread_counts/listener_spec.rb b/spec/services/conversations/unread_counts/listener_spec.rb index fbb0a0835..f13f70ce0 100644 --- a/spec/services/conversations/unread_counts/listener_spec.rb +++ b/spec/services/conversations/unread_counts/listener_spec.rb @@ -5,6 +5,7 @@ RSpec.describe Conversations::UnreadCounts::Listener do let(:account) { create(:account) } let(:conversation) { create(:conversation, account: account) } let(:notifier) { instance_double(Conversations::UnreadCounts::Notifier, perform: true) } + let(:filtered_store) { Conversations::UnreadCounts::FilteredCountStore } before do allow(Conversations::UnreadCounts::Notifier).to receive(:new).and_return(notifier) @@ -21,6 +22,19 @@ RSpec.describe Conversations::UnreadCounts::Listener do expect(notifier).to have_received(:perform) end + it 'refreshes unread count memberships before invalidating filtered counts when an incoming message is created' do + account.enable_features!(:conversation_unread_counts, :unread_count_for_filters) + message = create(:message, account: account, inbox: conversation.inbox, conversation: conversation, message_type: :incoming) + event = Events::Base.new('message.created', Time.zone.now, message: message) + invalidator = instance_double(Conversations::UnreadCounts::FilteredCountInvalidator) + + allow(Conversations::UnreadCounts::FilteredCountInvalidator).to receive(:new).with(account).and_return(invalidator) + expect(notifier).to receive(:perform).ordered.and_return(true) + expect(invalidator).to receive(:conversation_changed!).ordered.and_return(true) + + listener.message_created(event) + end + it 'ignores outgoing message creation' do message = create(:message, account: account, inbox: conversation.inbox, conversation: conversation, message_type: :outgoing) event = Events::Base.new('message.created', Time.zone.now, message: message) @@ -41,6 +55,32 @@ RSpec.describe Conversations::UnreadCounts::Listener do expect(Conversations::UnreadCounts::Notifier).not_to have_received(:new) end + it 'invalidates filtered counts when any message is created' do + account.enable_features!(:unread_count_for_filters) + message = create(:message, account: account, inbox: conversation.inbox, conversation: conversation, message_type: :outgoing) + event = Events::Base.new('message.created', Time.zone.now, message: message) + + expect do + listener.message_created(event) + end.to change { filtered_store.conversation_version(account.id) }.by(1) + expect(Conversations::UnreadCounts::Notifier).not_to have_received(:new) + end + + it 'notifies clients when outgoing message activity changes filtered counts' do + account.enable_features!(:conversation_unread_counts, :unread_count_for_filters) + allow(Rails.configuration.dispatcher).to receive(:dispatch) + message = create(:message, account: account, inbox: conversation.inbox, conversation: conversation, message_type: :outgoing) + event = Events::Base.new('message.created', Time.zone.now, message: message) + + listener.message_created(event) + + expect(Rails.configuration.dispatcher).to have_received(:dispatch).with( + 'conversation.unread_count_changed', + kind_of(Time), + conversation: conversation + ) + end + it 'refreshes unread counts when conversation status changes' do changed_attributes = { 'status' => %w[open resolved] } event = Events::Base.new('conversation.status_changed', Time.zone.now, conversation: conversation, changed_attributes: changed_attributes) @@ -51,6 +91,45 @@ RSpec.describe Conversations::UnreadCounts::Listener do expect(notifier).to have_received(:perform) end + it 'refreshes unread count memberships before invalidating filtered counts when conversation status changes' do + account.enable_features!(:conversation_unread_counts, :unread_count_for_filters) + changed_attributes = { 'status' => %w[open resolved] } + event = Events::Base.new('conversation.status_changed', Time.zone.now, conversation: conversation, changed_attributes: changed_attributes) + invalidator = instance_double(Conversations::UnreadCounts::FilteredCountInvalidator) + + allow(Conversations::UnreadCounts::FilteredCountInvalidator).to receive(:new).with(account).and_return(invalidator) + expect(notifier).to receive(:perform).ordered.and_return(true) + expect(invalidator).to receive(:conversation_changed!).ordered.and_return(true) + + listener.conversation_status_changed(event) + end + + it 'invalidates filtered counts when conversation status changes' do + account.enable_features!(:unread_count_for_filters) + changed_attributes = { 'status' => %w[open resolved] } + event = Events::Base.new('conversation.status_changed', Time.zone.now, conversation: conversation, changed_attributes: changed_attributes) + + expect do + listener.conversation_status_changed(event) + end.to change { filtered_store.conversation_version(account.id) }.by(1) + end + + it 'notifies clients when a status change only affects filtered counts' do + account.enable_features!(:conversation_unread_counts, :unread_count_for_filters) + allow(notifier).to receive(:perform).and_return(false) + allow(Rails.configuration.dispatcher).to receive(:dispatch) + changed_attributes = { 'status' => %w[pending resolved] } + event = Events::Base.new('conversation.status_changed', Time.zone.now, conversation: conversation, changed_attributes: changed_attributes) + + listener.conversation_status_changed(event) + + expect(Rails.configuration.dispatcher).to have_received(:dispatch).with( + 'conversation.unread_count_changed', + kind_of(Time), + conversation: conversation + ) + end + it 'refreshes unread counts when labels change' do changed_attributes = { label_list: [%w[old], %w[new]] } event = Events::Base.new('conversation.updated', Time.zone.now, conversation: conversation, changed_attributes: changed_attributes) @@ -61,14 +140,61 @@ RSpec.describe Conversations::UnreadCounts::Listener do expect(notifier).to have_received(:perform) end - it 'ignores conversation updates unrelated to unread count dimensions' do + it 'does not invalidate filtered counts from conversation updated events' do + account.enable_features!(:unread_count_for_filters) + event = Events::Base.new('conversation.updated', Time.zone.now, conversation: conversation, changed_attributes: { priority: [nil, 'high'] }) + + expect do + listener.conversation_updated(event) + end.not_to(change { filtered_store.conversation_version(account.id) }) + expect(Conversations::UnreadCounts::Notifier).not_to have_received(:new) + end + + it 'notifies clients when filtered conversation fields change' do + account.enable_features!(:conversation_unread_counts, :unread_count_for_filters) + allow(Rails.configuration.dispatcher).to receive(:dispatch) event = Events::Base.new('conversation.updated', Time.zone.now, conversation: conversation, changed_attributes: { priority: [nil, 'high'] }) listener.conversation_updated(event) + expect(Rails.configuration.dispatcher).to have_received(:dispatch).with( + 'conversation.unread_count_changed', + kind_of(Time), + conversation: conversation + ) + end + + it 'ignores conversation updates unrelated to unread count dimensions' do + event = Events::Base.new('conversation.updated', Time.zone.now, conversation: conversation, changed_attributes: { identifier: %w[old new] }) + + listener.conversation_updated(event) + expect(Conversations::UnreadCounts::Notifier).not_to have_received(:new) end + it 'invalidates filtered counts when the conversation contact changes' do + account.enable_features!(:unread_count_for_filters) + event = Events::Base.new('conversation.contact_changed', Time.zone.now, conversation: conversation) + + expect do + listener.conversation_contact_changed(event) + end.to change { filtered_store.conversation_version(account.id) }.by(1) + end + + it 'notifies clients when the conversation contact changes' do + account.enable_features!(:conversation_unread_counts, :unread_count_for_filters) + allow(Rails.configuration.dispatcher).to receive(:dispatch) + event = Events::Base.new('conversation.contact_changed', Time.zone.now, conversation: conversation) + + listener.conversation_contact_changed(event) + + expect(Rails.configuration.dispatcher).to have_received(:dispatch).with( + 'conversation.unread_count_changed', + kind_of(Time), + conversation: conversation + ) + end + it 'refreshes unread counts when assignee changes' do changed_attributes = { assignee_id: [nil, 1] } event = Events::Base.new('assignee.changed', Time.zone.now, conversation: conversation, changed_attributes: changed_attributes) @@ -79,6 +205,45 @@ RSpec.describe Conversations::UnreadCounts::Listener do expect(notifier).to have_received(:perform) end + it 'notifies clients when an assignee change only affects filtered counts' do + account.enable_features!(:conversation_unread_counts, :unread_count_for_filters) + allow(notifier).to receive(:perform).and_return(false) + allow(Rails.configuration.dispatcher).to receive(:dispatch) + changed_attributes = { assignee_id: [nil, 1] } + event = Events::Base.new('assignee.changed', Time.zone.now, conversation: conversation, changed_attributes: changed_attributes) + + listener.assignee_changed(event) + + expect(Rails.configuration.dispatcher).to have_received(:dispatch).with( + 'conversation.unread_count_changed', + kind_of(Time), + conversation: conversation + ) + end + + it 'refreshes unread count memberships before invalidating filtered counts when assignee changes' do + account.enable_features!(:conversation_unread_counts, :unread_count_for_filters) + changed_attributes = { assignee_id: [nil, 1] } + event = Events::Base.new('assignee.changed', Time.zone.now, conversation: conversation, changed_attributes: changed_attributes) + invalidator = instance_double(Conversations::UnreadCounts::FilteredCountInvalidator) + + allow(Conversations::UnreadCounts::FilteredCountInvalidator).to receive(:new).with(account).and_return(invalidator) + expect(notifier).to receive(:perform).ordered.and_return(true) + expect(invalidator).to receive(:conversation_changed!).ordered.and_return(true) + + listener.assignee_changed(event) + end + + it 'invalidates filtered counts when a user is mentioned' do + account.enable_features!(:unread_count_for_filters) + user = create(:user, account: account) + event = Events::Base.new('conversation.mentioned', Time.zone.now, conversation: conversation, user: user) + + expect do + listener.conversation_mentioned(event) + end.to change { filtered_store.built_in_filter_version(account_id: account.id, user_id: user.id) }.by(1) + end + it 'refreshes unread counts when team changes' do changed_attributes = { team_id: [nil, 1] } event = Events::Base.new('team.changed', Time.zone.now, conversation: conversation, changed_attributes: changed_attributes) @@ -89,6 +254,47 @@ RSpec.describe Conversations::UnreadCounts::Listener do expect(notifier).to have_received(:perform) end + it 'notifies clients when a team change only affects filtered counts' do + account.enable_features!(:conversation_unread_counts, :unread_count_for_filters) + allow(notifier).to receive(:perform).and_return(false) + allow(Rails.configuration.dispatcher).to receive(:dispatch) + changed_attributes = { team_id: [nil, 1] } + event = Events::Base.new('team.changed', Time.zone.now, conversation: conversation, changed_attributes: changed_attributes) + + listener.team_changed(event) + + expect(Rails.configuration.dispatcher).to have_received(:dispatch).with( + 'conversation.unread_count_changed', + kind_of(Time), + conversation: conversation + ) + end + + it 'invalidates filtered counts when a conversation is deleted' do + account.enable_features!(:unread_count_for_filters) + conversation_data = deleted_conversation_data(conversation) + + expect do + listener.conversation_deleted(Events::Base.new('conversation.deleted', Time.zone.now, conversation_data: conversation_data)) + end.to change { filtered_store.conversation_version(account.id) }.by(1) + end + + it 'notifies clients when a deleted conversation only affects filtered counts' do + account.enable_features!(:conversation_unread_counts, :unread_count_for_filters) + conversation_data = deleted_conversation_data(conversation) + allow(Rails.configuration.dispatcher).to receive(:dispatch) + + listener.conversation_deleted(Events::Base.new('conversation.deleted', Time.zone.now, conversation_data: conversation_data)) + + expect(Rails.configuration.dispatcher).to have_received(:dispatch).with( + 'conversation.unread_count_changed', + kind_of(Time), + conversation_data: conversation_data.stringify_keys + ) + ensure + store.clear_account!(account.id) + end + it 'removes unread count memberships when a conversation is deleted' do account.enable_features!(:conversation_unread_counts) label = create(:label, account: account) @@ -131,6 +337,29 @@ RSpec.describe Conversations::UnreadCounts::Listener do store.clear_account!(account.id) end + it 'removes unread count memberships before invalidating filtered counts when a conversation is deleted' do + account.enable_features!(:conversation_unread_counts, :unread_count_for_filters) + conversation_data = deleted_conversation_data(conversation) + invalidator = instance_double(Conversations::UnreadCounts::FilteredCountInvalidator) + + store.mark_base_ready!(account.id) + store.add_base_membership( + account_id: account.id, + inbox_id: conversation.inbox_id, + label_ids: [], + conversation_id: conversation.id + ) + + allow(Conversations::UnreadCounts::FilteredCountInvalidator).to receive(:new).with(account).and_return(invalidator) + allow(Rails.configuration.dispatcher).to receive(:dispatch) + expect(store).to receive(:remove_base_membership).ordered.and_call_original + expect(invalidator).to receive(:conversation_changed!).ordered.and_return(true) + + listener.conversation_deleted(Events::Base.new('conversation.deleted', Time.zone.now, conversation_data: conversation_data)) + ensure + store.clear_account!(account.id) + end + def deleted_conversation_data(conversation) { id: conversation.id, diff --git a/spec/services/conversations/unread_counts/notifier_spec.rb b/spec/services/conversations/unread_counts/notifier_spec.rb index 1b35d37f6..a8c46be0c 100644 --- a/spec/services/conversations/unread_counts/notifier_spec.rb +++ b/spec/services/conversations/unread_counts/notifier_spec.rb @@ -29,6 +29,18 @@ RSpec.describe Conversations::UnreadCounts::Notifier do expect(Rails.configuration.dispatcher).not_to have_received(:dispatch) end + + it 'dispatches unread count changed event when filtered counts are enabled' do + conversation.account.enable_features!(:unread_count_for_filters) + + described_class.new(conversation).perform + + expect(Rails.configuration.dispatcher).to have_received(:dispatch).with( + 'conversation.unread_count_changed', + kind_of(Time), + conversation: conversation + ) + end end context 'when conversation unread counts feature is disabled' do diff --git a/spec/services/labels/destroy_service_spec.rb b/spec/services/labels/destroy_service_spec.rb index 7d06b72d0..7f14273b3 100644 --- a/spec/services/labels/destroy_service_spec.rb +++ b/spec/services/labels/destroy_service_spec.rb @@ -6,6 +6,7 @@ describe Labels::DestroyService do let(:label) { create(:label, account: account) } let(:contact) { conversation.contact } let(:label_deleted_at) { Time.zone.parse('2026-05-07 10:00:00 UTC') } + let(:store) { Conversations::UnreadCounts::FilteredCountStore } before do conversation.label_list.add(label.title) @@ -74,6 +75,18 @@ describe Labels::DestroyService do ).perform end + it 'invalidates filtered counts when conversation label associations are removed' do + account.enable_features!(:unread_count_for_filters) + + expect do + described_class.new( + label_title: label.title, + account_id: account.id, + label_deleted_at: label_deleted_at + ).perform + end.to change { store.conversation_version(account.id) }.by(1) + end + it 'does not remove label associations created after the label was deleted' do other_conversation = create(:conversation, account: account) other_conversation.label_list.add(label.title)