Vishnu NarayananandGitHub
f66b551c7d
## 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
183 lines
9.1 KiB
Ruby
183 lines
9.1 KiB
Ruby
require 'rails_helper'
|
|
|
|
RSpec.describe Conversations::UnreadCounts::Refresher do
|
|
let(:account) { create(:account) }
|
|
let(:inbox) { create(:inbox, account: account) }
|
|
let(:label) { create(:label, account: account, title: 'urgent', show_on_sidebar: true) }
|
|
let(:new_label) { create(:label, account: account, title: 'billing', show_on_sidebar: true) }
|
|
let(:team) { create(:team, account: account, allow_auto_assign: false) }
|
|
let(:new_team) { create(:team, account: account, allow_auto_assign: false) }
|
|
let(:assignee) { create(:user, account: account, role: :agent) }
|
|
let(:other_assignee) { create(:user, account: account, role: :agent) }
|
|
let(:store) { Conversations::UnreadCounts::Store }
|
|
|
|
after do
|
|
store.clear_account!(account.id)
|
|
end
|
|
|
|
it 'does not update redis when unread caches are not ready' do
|
|
conversation = create_unread_conversation(account: account, inbox: inbox, labels: [label.title])
|
|
|
|
expect(described_class.new(conversation).perform).to be(false)
|
|
expect(store.counts_for_keys([store.inbox_key(account.id, inbox.id)])).to eq(store.inbox_key(account.id, inbox.id) => 0)
|
|
end
|
|
|
|
it 'adds an unread conversation to base cache' do
|
|
conversation = create(:conversation, account: account, inbox: inbox, agent_last_seen_at: 1.hour.ago)
|
|
conversation.update_labels([label.title])
|
|
store.mark_base_ready!(account.id)
|
|
|
|
create(:message, account: account, inbox: inbox, conversation: conversation, message_type: :incoming, created_at: 5.minutes.ago)
|
|
|
|
expect(described_class.new(conversation.reload).perform).to be(true)
|
|
|
|
expect(store.counts_for_keys(base_keys)).to eq(
|
|
store.inbox_key(account.id, inbox.id) => 1,
|
|
store.label_inbox_key(account.id, label.id, inbox.id) => 1
|
|
)
|
|
end
|
|
|
|
it 'returns false when refresh does not change unread counts' do
|
|
conversation = create_unread_conversation(account: account, inbox: inbox, labels: [label.title])
|
|
Conversations::UnreadCounts::Builder.new(account).build_base!
|
|
|
|
create(:message, account: account, inbox: inbox, conversation: conversation, message_type: :incoming)
|
|
|
|
expect(described_class.new(conversation.reload).perform).to be(false)
|
|
expect(store.counts_for_keys(base_keys)).to eq(
|
|
store.inbox_key(account.id, inbox.id) => 1,
|
|
store.label_inbox_key(account.id, label.id, inbox.id) => 1
|
|
)
|
|
end
|
|
|
|
it 'removes a conversation from base cache when it becomes read' do
|
|
conversation = create_unread_conversation(account: account, inbox: inbox, labels: [label.title])
|
|
Conversations::UnreadCounts::Builder.new(account).build_base!
|
|
|
|
conversation.update!(agent_last_seen_at: 1.minute.from_now)
|
|
expect(described_class.new(conversation.reload).perform).to be(true)
|
|
|
|
expect(store.counts_for_keys(base_keys).values).to all(eq(0))
|
|
end
|
|
|
|
it 'moves base label membership when labels change' do
|
|
conversation = create_unread_conversation(account: account, inbox: inbox, labels: [label.title])
|
|
Conversations::UnreadCounts::Builder.new(account).build_base!
|
|
|
|
conversation.update_labels([new_label.title])
|
|
expect(described_class.new(conversation.reload, changed_attributes: { label_list: [[label.title], [new_label.title]] }).perform).to be(true)
|
|
|
|
expect(store.counts_for_keys([
|
|
store.label_inbox_key(account.id, label.id, inbox.id),
|
|
store.label_inbox_key(account.id, new_label.id, inbox.id)
|
|
])).to eq(
|
|
store.label_inbox_key(account.id, label.id, inbox.id) => 0,
|
|
store.label_inbox_key(account.id, new_label.id, inbox.id) => 1
|
|
)
|
|
end
|
|
|
|
it 'moves base team membership when team changes' do
|
|
conversation = create_unread_conversation(account: account, inbox: inbox, team: team)
|
|
Conversations::UnreadCounts::Builder.new(account).build_base!
|
|
|
|
conversation.update!(team: new_team)
|
|
expect(described_class.new(conversation.reload, changed_attributes: { team_id: [team.id, new_team.id] }).perform).to be(true)
|
|
|
|
expect(store.counts_for_keys([
|
|
store.team_inbox_key(account.id, team.id, inbox.id),
|
|
store.team_inbox_key(account.id, new_team.id, inbox.id)
|
|
])).to eq(
|
|
store.team_inbox_key(account.id, team.id, inbox.id) => 0,
|
|
store.team_inbox_key(account.id, new_team.id, inbox.id) => 1
|
|
)
|
|
end
|
|
|
|
it 'moves assignment-aware membership when assignee changes' do
|
|
conversation = create_unread_conversation(account: account, inbox: inbox, labels: [label.title], assignee: assignee)
|
|
Conversations::UnreadCounts::Builder.new(account).build_assignment!
|
|
|
|
conversation.update!(assignee: other_assignee)
|
|
described_class.new(conversation.reload, changed_attributes: { assignee_id: [assignee.id, other_assignee.id] }).perform
|
|
|
|
expect(store.counts_for_keys([
|
|
store.inbox_assignee_key(account.id, inbox.id, assignee.id),
|
|
store.inbox_assignee_key(account.id, inbox.id, other_assignee.id)
|
|
])).to eq(
|
|
store.inbox_assignee_key(account.id, inbox.id, assignee.id) => 0,
|
|
store.inbox_assignee_key(account.id, inbox.id, other_assignee.id) => 1
|
|
)
|
|
end
|
|
|
|
it 'does not remove unassigned membership when assignee did not change' do
|
|
conversation = create_unread_conversation(account: account, inbox: inbox, labels: [label.title], assignee: assignee)
|
|
Conversations::UnreadCounts::Builder.new(account).build_assignment!
|
|
allow(store).to receive(:remove_assignment_membership).and_call_original
|
|
|
|
conversation.update_labels([new_label.title])
|
|
described_class.new(conversation.reload, changed_attributes: { label_list: [[label.title], [new_label.title]] }).perform
|
|
|
|
expect(store).to have_received(:remove_assignment_membership).with(hash_including(assignee_ids: [assignee.id]))
|
|
end
|
|
|
|
it 'moves assignment-aware unassigned label membership when labels change' do
|
|
conversation = create_unread_conversation(account: account, inbox: inbox, labels: [label.title])
|
|
Conversations::UnreadCounts::Builder.new(account).build_assignment!
|
|
|
|
conversation.update_labels([new_label.title])
|
|
result = described_class.new(
|
|
conversation.reload,
|
|
changed_attributes: { label_list: [[label.title], [new_label.title]] }
|
|
).perform
|
|
|
|
expect(result).to be(true)
|
|
expect(store.counts_for_keys([
|
|
store.label_inbox_unassigned_key(account.id, label.id, inbox.id),
|
|
store.label_inbox_unassigned_key(account.id, new_label.id, inbox.id)
|
|
])).to eq(
|
|
store.label_inbox_unassigned_key(account.id, label.id, inbox.id) => 0,
|
|
store.label_inbox_unassigned_key(account.id, new_label.id, inbox.id) => 1
|
|
)
|
|
end
|
|
|
|
it 'removes assignment-aware unassigned membership when conversation is resolved' do
|
|
conversation = create_unread_conversation(account: account, inbox: inbox, labels: [label.title])
|
|
Conversations::UnreadCounts::Builder.new(account).build_assignment!
|
|
|
|
conversation.update!(status: :resolved)
|
|
result = described_class.new(conversation.reload, changed_attributes: { status: %w[open resolved] }).perform
|
|
|
|
expect(result).to be(true)
|
|
expect(store.counts_for_keys([
|
|
store.inbox_unassigned_key(account.id, inbox.id),
|
|
store.label_inbox_unassigned_key(account.id, label.id, inbox.id)
|
|
])).to eq(
|
|
store.inbox_unassigned_key(account.id, inbox.id) => 0,
|
|
store.label_inbox_unassigned_key(account.id, label.id, inbox.id) => 0
|
|
)
|
|
end
|
|
|
|
it 'moves assignment-aware team membership when team changes' do
|
|
create(:team_member, user: assignee, team: new_team)
|
|
conversation = create_unread_conversation(account: account, inbox: inbox, assignee: assignee, team: team)
|
|
Conversations::UnreadCounts::Builder.new(account).build_assignment!
|
|
|
|
conversation.update!(team: new_team)
|
|
described_class.new(conversation.reload, changed_attributes: { team_id: [team.id, new_team.id] }).perform
|
|
|
|
expect(store.counts_for_keys([
|
|
store.team_inbox_assignee_key(account.id, team.id, inbox.id, assignee.id),
|
|
store.team_inbox_assignee_key(account.id, new_team.id, inbox.id, assignee.id)
|
|
])).to eq(
|
|
store.team_inbox_assignee_key(account.id, team.id, inbox.id, assignee.id) => 0,
|
|
store.team_inbox_assignee_key(account.id, new_team.id, inbox.id, assignee.id) => 1
|
|
)
|
|
end
|
|
|
|
def base_keys
|
|
[
|
|
store.inbox_key(account.id, inbox.id),
|
|
store.label_inbox_key(account.id, label.id, inbox.id)
|
|
]
|
|
end
|
|
end
|