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
295 lines
13 KiB
Ruby
295 lines
13 KiB
Ruby
require 'rails_helper'
|
|
|
|
describe ConversationFinder do
|
|
subject(:conversation_finder) { described_class.new(user_1, params) }
|
|
|
|
let!(:account) { create(:account) }
|
|
let!(:user_1) { create(:user, account: account) }
|
|
let!(:user_2) { create(:user, account: account) }
|
|
let!(:admin) { create(:user, account: account, role: :administrator) }
|
|
let!(:inbox) { create(:inbox, account: account, enable_auto_assignment: false) }
|
|
let!(:contact_inbox) { create(:contact_inbox, inbox: inbox, source_id: 'testing_source_id') }
|
|
let!(:restricted_inbox) { create(:inbox, account: account) }
|
|
|
|
before do
|
|
create(:inbox_member, user: user_1, inbox: inbox)
|
|
create(:inbox_member, user: user_2, inbox: inbox)
|
|
create(:conversation, account: account, inbox: inbox, assignee: user_1)
|
|
create(:conversation, account: account, inbox: inbox, assignee: user_1)
|
|
create(:conversation, account: account, inbox: inbox, assignee: user_1, status: 'resolved')
|
|
create(:conversation, account: account, inbox: inbox, assignee: user_2, contact_inbox: contact_inbox)
|
|
# unassigned conversation
|
|
create(:conversation, account: account, inbox: inbox)
|
|
Current.account = account
|
|
end
|
|
|
|
describe '#perform' do
|
|
context 'with status' do
|
|
let(:params) { { status: 'open', assignee_type: 'me' } }
|
|
|
|
it 'filter conversations by status' do
|
|
result = conversation_finder.perform
|
|
expect(result[:conversations].length).to be 2
|
|
end
|
|
end
|
|
|
|
context 'with inbox' do
|
|
let!(:restricted_conversation) { create(:conversation, account: account, inbox_id: restricted_inbox.id) }
|
|
|
|
it 'returns conversation from any inbox if its admin' do
|
|
params = { inbox_id: restricted_inbox.id }
|
|
result = described_class.new(admin, params).perform
|
|
|
|
expect(result[:conversations].map(&:id)).to include(restricted_conversation.id)
|
|
end
|
|
|
|
it 'returns conversation from inbox if agent is its member' do
|
|
params = { inbox_id: restricted_inbox.id }
|
|
create(:inbox_member, user: user_1, inbox: restricted_inbox)
|
|
result = described_class.new(user_1, params).perform
|
|
|
|
expect(result[:conversations].map(&:id)).to include(restricted_conversation.id)
|
|
end
|
|
|
|
it 'does not return conversations from inboxes where agent is not a member' do
|
|
params = { inbox_id: restricted_inbox.id }
|
|
result = described_class.new(user_1, params).perform
|
|
|
|
expect(result[:conversations].map(&:id)).not_to include(restricted_conversation.id)
|
|
end
|
|
|
|
it 'returns only the conversations from the inbox if inbox_id filter is passed' do
|
|
conversation = create(:conversation, account: account, inbox_id: inbox.id)
|
|
params = { inbox_id: restricted_inbox.id }
|
|
result = described_class.new(admin, params).perform
|
|
|
|
conversation_ids = result[:conversations].map(&:id)
|
|
expect(conversation_ids).not_to include(conversation.id)
|
|
expect(conversation_ids).to include(restricted_conversation.id)
|
|
end
|
|
end
|
|
|
|
context 'with assignee_type all' do
|
|
let(:params) { { assignee_type: 'all' } }
|
|
|
|
it 'filter conversations by assignee type all' do
|
|
result = conversation_finder.perform
|
|
expect(result[:conversations].length).to be 4
|
|
end
|
|
end
|
|
|
|
context 'with assignee_type unassigned' do
|
|
let(:params) { { assignee_type: 'unassigned' } }
|
|
|
|
it 'filter conversations by assignee type unassigned' do
|
|
result = conversation_finder.perform
|
|
expect(result[:conversations].length).to be 1
|
|
end
|
|
end
|
|
|
|
context 'with status all' do
|
|
let(:params) { { status: 'all' } }
|
|
|
|
it 'returns all conversations' do
|
|
result = conversation_finder.perform
|
|
expect(result[:conversations].length).to be 5
|
|
end
|
|
end
|
|
|
|
context 'with unread sort' do
|
|
let(:params) { { status: 'open', sort_by: 'unread' } }
|
|
|
|
it 'returns all conversations matching the selected status with the highest unread count first' do
|
|
most_unread_conversation = create(:conversation, account: account, inbox: inbox,
|
|
agent_last_seen_at: 1.hour.ago)
|
|
unread_conversation = create(:conversation, account: account, inbox: inbox,
|
|
agent_last_seen_at: 1.hour.ago)
|
|
read_conversation = create(:conversation, account: account, inbox: inbox,
|
|
agent_last_seen_at: 1.minute.from_now)
|
|
resolved_unread_conversation = create(:conversation, account: account, inbox: inbox, status: 'resolved',
|
|
agent_last_seen_at: 1.hour.ago)
|
|
|
|
[most_unread_conversation, unread_conversation, read_conversation, resolved_unread_conversation].each do |conversation|
|
|
create(:message, account: account, inbox: inbox, conversation: conversation,
|
|
message_type: :incoming, created_at: 5.minutes.ago)
|
|
end
|
|
create(:message, account: account, inbox: inbox, conversation: most_unread_conversation,
|
|
message_type: :incoming, created_at: 4.minutes.ago)
|
|
resolved_unread_conversation.update!(status: 'resolved')
|
|
read_conversation.update!(last_activity_at: 1.minute.from_now)
|
|
unread_conversation.update!(last_activity_at: 2.minutes.from_now)
|
|
|
|
result = conversation_finder.perform
|
|
conversation_ids = result[:conversations].map(&:id)
|
|
|
|
expect(conversation_ids).to include(most_unread_conversation.id, unread_conversation.id, read_conversation.id)
|
|
expect(conversation_ids).not_to include(resolved_unread_conversation.id)
|
|
expect(conversation_ids.index(most_unread_conversation.id)).to be < conversation_ids.index(unread_conversation.id)
|
|
expect(conversation_ids.index(unread_conversation.id)).to be < conversation_ids.index(read_conversation.id)
|
|
end
|
|
|
|
it 'includes private incoming messages in unread counts used for ordering' do
|
|
private_unread_conversation = create(:conversation, account: account, inbox: inbox,
|
|
agent_last_seen_at: 1.hour.ago)
|
|
unread_conversation = create(:conversation, account: account, inbox: inbox,
|
|
agent_last_seen_at: 1.hour.ago)
|
|
read_conversation = create(:conversation, account: account, inbox: inbox,
|
|
agent_last_seen_at: 1.minute.from_now)
|
|
|
|
2.times do
|
|
create(:message, account: account, inbox: inbox, conversation: private_unread_conversation,
|
|
message_type: :incoming, private: true, created_at: 5.minutes.ago)
|
|
end
|
|
create(:message, account: account, inbox: inbox, conversation: unread_conversation,
|
|
message_type: :incoming, created_at: 5.minutes.ago)
|
|
create(:message, account: account, inbox: inbox, conversation: read_conversation,
|
|
message_type: :incoming, created_at: 5.minutes.ago)
|
|
private_unread_conversation.update!(last_activity_at: 10.minutes.ago)
|
|
unread_conversation.update!(last_activity_at: 2.minutes.from_now)
|
|
read_conversation.update!(last_activity_at: 1.minute.from_now)
|
|
|
|
result = conversation_finder.perform
|
|
conversation_ids = result[:conversations].map(&:id)
|
|
|
|
expect(private_unread_conversation.unread_incoming_messages.count).to eq 2
|
|
expect(conversation_ids.index(private_unread_conversation.id)).to be < conversation_ids.index(unread_conversation.id)
|
|
expect(conversation_ids.index(unread_conversation.id)).to be < conversation_ids.index(read_conversation.id)
|
|
end
|
|
end
|
|
|
|
context 'with assignee_type assigned' do
|
|
let(:params) { { assignee_type: 'assigned' } }
|
|
|
|
it 'filter conversations by assignee type assigned' do
|
|
result = conversation_finder.perform
|
|
expect(result[:conversations].length).to be 3
|
|
end
|
|
|
|
it 'returns the correct meta' do
|
|
result = conversation_finder.perform
|
|
expect(result[:count]).to eq({
|
|
mine_count: 2,
|
|
assigned_count: 3,
|
|
unassigned_count: 1,
|
|
all_count: 4
|
|
})
|
|
end
|
|
end
|
|
|
|
context 'with team' do
|
|
let(:team) { create(:team, account: account) }
|
|
let(:params) { { team_id: team.id } }
|
|
|
|
it 'filter conversations by team' do
|
|
create(:conversation, account: account, inbox: inbox, team: team)
|
|
result = conversation_finder.perform
|
|
expect(result[:conversations].length).to be 1
|
|
end
|
|
end
|
|
|
|
context 'with labels' do
|
|
let(:params) { { labels: ['resolved'] } }
|
|
|
|
it 'filter conversations by labels' do
|
|
conversation = inbox.conversations.first
|
|
conversation.update_labels('resolved')
|
|
|
|
result = conversation_finder.perform
|
|
expect(result[:conversations].length).to be 1
|
|
end
|
|
end
|
|
|
|
context 'with source_id' do
|
|
let(:params) { { source_id: 'testing_source_id' } }
|
|
|
|
it 'filter conversations by source id' do
|
|
result = conversation_finder.perform
|
|
expect(result[:conversations].length).to be 1
|
|
end
|
|
end
|
|
|
|
context 'without source' do
|
|
let(:params) { {} }
|
|
|
|
it 'returns conversations with any source' do
|
|
result = conversation_finder.perform
|
|
expect(result[:conversations].length).to be 4
|
|
end
|
|
end
|
|
|
|
context 'with updated_within' do
|
|
let(:params) { { updated_within: 20, assignee_type: 'unassigned', sort_by: 'created_at_asc' } }
|
|
|
|
it 'filters based on params, sort order but returns all conversations without pagination with in time range' do
|
|
# value of updated_within is in seconds
|
|
# write spec based on that
|
|
conversations = create_list(:conversation, 50, account: account,
|
|
inbox: inbox, assignee: nil,
|
|
updated_at: Time.now.utc - 30.seconds,
|
|
created_at: Time.now.utc - 30.seconds)
|
|
# update updated_at of 27 conversations to be with in 20 seconds
|
|
conversations[0..27].each do |conversation|
|
|
conversation.update(updated_at: Time.now.utc - 10.seconds)
|
|
end
|
|
result = conversation_finder.perform
|
|
# pagination is not applied
|
|
# filters are applied
|
|
# modified conversations + 1 conversation created during set up
|
|
expect(result[:conversations].length).to be 29
|
|
# ensure that the conversations are sorted by created_at
|
|
expect(result[:conversations].first.created_at).to be < result[:conversations].last.created_at
|
|
end
|
|
end
|
|
|
|
context 'with pagination' do
|
|
let(:params) { { status: 'open', assignee_type: 'me', page: 1 } }
|
|
|
|
it 'returns paginated conversations' do
|
|
create_list(:conversation, 50, account: account, inbox: inbox, assignee: user_1)
|
|
result = conversation_finder.perform
|
|
expect(result[:conversations].length).to be 25
|
|
end
|
|
end
|
|
|
|
context 'with perform_meta_only' do
|
|
let(:params) { { assignee_type: 'assigned' } }
|
|
|
|
it 'returns only count without conversations' do
|
|
result = conversation_finder.perform_meta_only
|
|
expect(result).to have_key(:count)
|
|
expect(result).not_to have_key(:conversations)
|
|
end
|
|
|
|
it 'returns the correct counts' do
|
|
result = conversation_finder.perform_meta_only
|
|
expect(result[:count]).to eq({
|
|
mine_count: 2,
|
|
assigned_count: 3,
|
|
unassigned_count: 1,
|
|
all_count: 4
|
|
})
|
|
end
|
|
|
|
it 'returns same counts as perform' do
|
|
meta_result = conversation_finder.perform_meta_only
|
|
full_result = conversation_finder.perform
|
|
expect(meta_result[:count]).to eq(full_result[:count])
|
|
end
|
|
end
|
|
|
|
context 'with unattended' do
|
|
let(:params) { { status: 'open', assignee_type: 'me', conversation_type: 'unattended' } }
|
|
|
|
it 'returns unattended conversations' do
|
|
create(:conversation, account: account, first_reply_created_at: Time.now.utc, assignee: user_1) # attended_conversation
|
|
create(:conversation, account: account, first_reply_created_at: nil, assignee: user_1) # unattended_conversation_no_first_reply
|
|
create(:conversation, account: account, first_reply_created_at: Time.now.utc,
|
|
assignee: user_1, waiting_since: Time.now.utc) # unattended_conversation_waiting_since
|
|
|
|
result = conversation_finder.perform
|
|
expect(result[:conversations].length).to be 2
|
|
end
|
|
end
|
|
end
|
|
end
|