From 66cfb26c77a902937713033edca5794e6f62cc7e Mon Sep 17 00:00:00 2001 From: Sony Mathew Date: Wed, 8 Jul 2026 23:50:40 +0530 Subject: [PATCH] feat: Add unread count filters feature flag (1/6) (#14885) ## Description Adds the account-level `unread_count_for_filters` feature flag as the dark-launch gate for filtered sidebar unread counts. This reuses the deprecated `quoted_email_reply` flag slot, resets the reused bit for existing accounts, and removes stale defaults so new accounts do not reference the old flag. This also adds the feature where we are now calculating the unread counts for built in filters like mentions, participating and unattended along with unread count for saved filters/folders. Closes [CW-7262](https://linear.app/chatwoot/issue/CW-7262/unread-counts-for-filters-folders) ## Type of change - [ ] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality not to work as expected) - [ ] This change requires a documentation update --- .../conversations/participants_controller.rb | 26 +- .../conversations/unread_counts_controller.rb | 18 +- .../v1/accounts/conversations_controller.rb | 1 + app/helpers/filters/filter_helper.rb | 10 + .../components-next/sidebar/Sidebar.vue | 47 +- app/javascript/dashboard/featureFlags.js | 1 + .../dashboard/helper/actionCable.js | 78 +++ .../dashboard/helper/sidebarSort.js | 2 + .../helper/specs/actionCable.spec.js | 217 +++++++ .../helper/specs/sidebarSort.spec.js | 31 +- .../store/modules/conversationUnreadCounts.js | 23 + .../store/modules/conversationWatchers.js | 57 +- .../dashboard/store/modules/customViews.js | 59 +- .../conversationUnreadCounts/actions.spec.js | 4 + .../conversationUnreadCounts/getters.spec.js | 26 + .../mutations.spec.js | 47 +- .../conversationWatchers/actions.spec.js | 71 +++ .../modules/specs/customViews/actions.spec.js | 111 ++++ .../sidebarSortPreferences/actions.spec.js | 4 +- app/jobs/agents/destroy_job.rb | 6 +- app/models/account_user.rb | 18 + app/models/campaign.rb | 10 + .../concerns/account_cache_revalidator.rb | 2 + app/models/conversation.rb | 30 +- app/models/conversation_participant.rb | 5 + app/models/custom_attribute_definition.rb | 23 + app/models/custom_filter.rb | 21 + app/models/inbox.rb | 14 + app/models/inbox_member.rb | 5 + app/models/team.rb | 15 + app/models/team_member.rb | 8 + app/services/conversations/unread_counts.rb | 6 + .../conversations/unread_counts/counter.rb | 14 +- .../unread_counts/filter_query_counter.rb | 151 +++++ .../filtered_count_instrumentation.rb | 188 ++++++ .../filtered_count_invalidator.rb | 184 ++++++ .../filtered_count_snapshot_resolver.rb | 67 +++ .../unread_counts/filtered_count_store.rb | 214 +++++++ .../filtered_count_store_keys.rb | 67 +++ .../filtered_count_version_cache.rb | 29 + .../unread_counts/filtered_counter.rb | 216 +++++++ .../conversations/unread_counts/listener.rb | 87 ++- .../conversations/unread_counts/notifier.rb | 12 +- app/services/labels/destroy_service.rb | 12 +- config/features.yml | 6 +- ...reply_flag_for_unread_count_for_filters.rb | 22 + enterprise/app/models/custom_role.rb | 33 ++ .../permission_filter_service.rb | 17 +- lib/redis/redis_keys.rb | 17 + .../participants_controller_spec.rb | 54 ++ .../accounts/conversations_controller_spec.rb | 125 ++++ .../finders/conversation_finder_spec.rb | 41 ++ spec/enterprise/models/account_user_spec.rb | 23 + spec/enterprise/models/custom_role_spec.rb | 45 ++ .../unread_counts/filtered_counter_spec.rb | 58 ++ .../permission_filter_service_spec.rb | 10 +- spec/jobs/agents/destroy_job_spec.rb | 9 + .../captain/reply_suggestion_service_spec.rb | 1 + spec/listeners/action_cable_listener_spec.rb | 24 + spec/models/account_user_spec.rb | 39 ++ spec/models/campaign_spec.rb | 37 ++ spec/models/conversation_participants_spec.rb | 26 + spec/models/conversation_spec.rb | 71 ++- .../custom_attribute_definition_spec.rb | 46 ++ spec/models/custom_filter_spec.rb | 58 ++ spec/models/inbox_member_spec.rb | 42 ++ spec/models/inbox_spec.rb | 28 + spec/models/team_member_spec.rb | 43 ++ .../conversations/filter_service_spec.rb | 18 + .../unread_counts/counter_spec.rb | 18 + .../filtered_count_instrumentation_spec.rb | 137 +++++ .../filtered_count_invalidator_spec.rb | 244 ++++++++ .../filtered_count_store_spec.rb | 262 +++++++++ .../unread_counts/filtered_counter_spec.rb | 549 ++++++++++++++++++ .../unread_counts/listener_spec.rb | 231 +++++++- .../unread_counts/notifier_spec.rb | 12 + spec/services/labels/destroy_service_spec.rb | 13 + 77 files changed, 4539 insertions(+), 57 deletions(-) create mode 100644 app/services/conversations/unread_counts/filter_query_counter.rb create mode 100644 app/services/conversations/unread_counts/filtered_count_instrumentation.rb create mode 100644 app/services/conversations/unread_counts/filtered_count_invalidator.rb create mode 100644 app/services/conversations/unread_counts/filtered_count_snapshot_resolver.rb create mode 100644 app/services/conversations/unread_counts/filtered_count_store.rb create mode 100644 app/services/conversations/unread_counts/filtered_count_store_keys.rb create mode 100644 app/services/conversations/unread_counts/filtered_count_version_cache.rb create mode 100644 app/services/conversations/unread_counts/filtered_counter.rb create mode 100644 db/migrate/20260629000000_repurpose_quoted_email_reply_flag_for_unread_count_for_filters.rb create mode 100644 spec/enterprise/finders/conversation_finder_spec.rb create mode 100644 spec/enterprise/services/conversations/unread_counts/filtered_counter_spec.rb create mode 100644 spec/models/custom_filter_spec.rb create mode 100644 spec/services/conversations/unread_counts/filtered_count_instrumentation_spec.rb create mode 100644 spec/services/conversations/unread_counts/filtered_count_invalidator_spec.rb create mode 100644 spec/services/conversations/unread_counts/filtered_count_store_spec.rb create mode 100644 spec/services/conversations/unread_counts/filtered_counter_spec.rb 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)