## Description Reverts [#14726](https://github.com/chatwoot/chatwoot/pull/14726) (\"feat: Add sidebar unread counts for filters (CW-7262)\"), which shipped in 4.15.0. After 4.15.0 rolled out to prod the unread-counts-for-filters code path caused a cascading incident: - `Counter#ensure_filters_cache!` fires on every `/unread_counts/index` and `update_last_seen` request. - On cache miss it calls `Builder#build_filters_for!`, which: - invokes `store.clear_user_filters!` -> `delete_matching` -> a Redis `SCAN_each` over a per-user pattern keyspace, and - runs 4 fresh SQL passes per user (mentions, participating, unattended, and per-folder `Conversations::FilterService` queries). - Threads blocked in the SCAN held their DB connections, the connection pool exhausted, Sidekiq jobs were discarded with `ActiveJob::DeserializationError: could not obtain a connection from the pool`, and the enqueued queue blew past 200K. Related: [CW-7262](https://linear.app/chatwoot/issue/CW-7262/unread-counts-for-filters-folders) ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] 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 ## How Has This Been Tested? ## Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [ ] I have commented on my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published in downstream modules
99 lines
4.0 KiB
Ruby
99 lines
4.0 KiB
Ruby
require 'rails_helper'
|
|
|
|
RSpec.describe Conversations::UnreadCounts::Counter do
|
|
let(:account) { create(:account) }
|
|
let(:agent) { create(:user, account: account, role: :agent) }
|
|
let(:admin) { create(:user, account: account, role: :administrator) }
|
|
let(:visible_inbox) { create(:inbox, account: account) }
|
|
let(:hidden_inbox) { create(:inbox, account: account) }
|
|
let(:label) { create(:label, account: account, title: 'billing', show_on_sidebar: true) }
|
|
let(:hidden_label) { create(:label, account: account, title: 'internal', show_on_sidebar: false) }
|
|
let(:visible_team) { create(:team, account: account, allow_auto_assign: false) }
|
|
let(:store) { Conversations::UnreadCounts::Store }
|
|
|
|
before do
|
|
create(:inbox_member, user: agent, inbox: visible_inbox)
|
|
create(:team_member, user: agent, team: visible_team)
|
|
end
|
|
|
|
after do
|
|
store.clear_account!(account.id)
|
|
end
|
|
|
|
it 'builds the base cache on demand' do
|
|
create_unread_conversation(account: account, inbox: visible_inbox, labels: [label.title], team: visible_team)
|
|
|
|
described_class.new(account: account, user: agent).perform
|
|
|
|
expect(store.base_ready?(account.id)).to be(true)
|
|
end
|
|
|
|
it 'uses a Redis lock while building the base cache on demand' do
|
|
lock_key = "UNREAD_CONVERSATIONS::V1::ACCOUNT::#{account.id}::BUILD_LOCK::BASE"
|
|
lock_manager = instance_double(Redis::LockManager)
|
|
allow(Redis::LockManager).to receive(:new).and_return(lock_manager)
|
|
allow(lock_manager).to receive(:with_lock).with(lock_key, described_class::BUILD_LOCK_TTL).and_yield.and_return(true)
|
|
|
|
create_unread_conversation(account: account, inbox: visible_inbox, labels: [label.title], team: visible_team)
|
|
|
|
described_class.new(account: account, user: agent).perform
|
|
|
|
expect(lock_manager).to have_received(:with_lock).with(lock_key, described_class::BUILD_LOCK_TTL)
|
|
end
|
|
|
|
it 'waits instead of rebuilding when another process owns the base build lock' do
|
|
lock_manager = instance_double(Redis::LockManager, with_lock: false)
|
|
counter = described_class.new(account: account, user: agent)
|
|
|
|
allow(Redis::LockManager).to receive(:new).and_return(lock_manager)
|
|
allow(counter).to receive(:wait_for_cache_ready) { store.mark_base_ready!(account.id) }
|
|
expect(Conversations::UnreadCounts::Builder).not_to receive(:new)
|
|
|
|
counter.perform
|
|
|
|
expect(counter).to have_received(:wait_for_cache_ready)
|
|
expect(store.base_ready?(account.id)).to be(true)
|
|
end
|
|
|
|
it 'counts unread conversations only across inboxes visible to a normal agent' do
|
|
create_unread_conversation(account: account, inbox: visible_inbox, labels: [label.title], team: visible_team)
|
|
create_unread_conversation(account: account, inbox: hidden_inbox, labels: [label.title], team: visible_team)
|
|
|
|
result = described_class.new(account: account, user: agent).perform
|
|
|
|
expect(result).to eq(
|
|
all_count: 1,
|
|
inboxes: { visible_inbox.id.to_s => 1 },
|
|
labels: { label.id.to_s => 1 },
|
|
teams: { visible_team.id.to_s => 1 }
|
|
)
|
|
end
|
|
|
|
it 'counts unread conversations across all account inboxes for admins' do
|
|
create_unread_conversation(account: account, inbox: visible_inbox, labels: [label.title], team: visible_team)
|
|
create_unread_conversation(account: account, inbox: hidden_inbox, labels: [label.title], team: visible_team)
|
|
|
|
result = described_class.new(account: account, user: admin).perform
|
|
|
|
expect(result).to eq(
|
|
all_count: 2,
|
|
inboxes: { visible_inbox.id.to_s => 1, hidden_inbox.id.to_s => 1 },
|
|
labels: { label.id.to_s => 2 },
|
|
teams: { visible_team.id.to_s => 2 }
|
|
)
|
|
end
|
|
|
|
it 'does not return zero counts or labels hidden from the sidebar' do
|
|
create_unread_conversation(account: account, inbox: visible_inbox, labels: [hidden_label.title], team: visible_team)
|
|
|
|
result = described_class.new(account: account, user: agent).perform
|
|
|
|
expect(result).to eq(
|
|
all_count: 1,
|
|
inboxes: { visible_inbox.id.to_s => 1 },
|
|
labels: {},
|
|
teams: { visible_team.id.to_s => 1 }
|
|
)
|
|
end
|
|
end
|