## PR2: Report builder refactor — DataSource abstraction
The existing report builders (timeseries + summary) had their SQL
queries inlined — each builder constructed its own scopes, groupings,
and aggregations directly. This made it hard to swap the underlying data
source without duplicating builder logic.
This PR extracts all raw-event querying into a `Reports::RawDataSource`
behind a `Reports::DataSource` factory. Builders now call
`data_source.timeseries`, `.aggregate`, or `.summary` instead of
constructing queries themselves. Behavior is identical —
`DataSource.for(...)` returns `RawDataSource` in all cases today.
The timeseries path had two separate builders (`CountReportBuilder`,
`AverageReportBuilder`) that were selected via a metric-name case
statement in `Conversations::BaseReportBuilder`. These are replaced by a
single `ReportBuilder` that delegates to the data source. The metric
type (count vs average) is now decided inside the data source, not the
builder.
Summary builders similarly moved their inline SQL into
`RawDataSource#summary`, which returns a unified hash keyed by dimension
ID.
the rollup read path.
## Flow
### Before
```
ReportsController ──▶ case metric ──▶ AverageReportBuilder ──▶ inline SQL ──▶ DB
└──▶ CountReportBuilder ──▶ inline SQL ──▶ DB
SummaryController ──▶ AgentSummaryBuilder ──▶ inline SQL ──▶ DB
└──▶ InboxSummaryBuilder ──▶ inline SQL ──▶ DB
└──▶ TeamSummaryBuilder ──▶ inline SQL ──▶ DB
```
### After
```
ReportsController ──▶ ReportBuilder ──┐
├──▶ DataSource.for ──▶ RawDataSource ──▶ DB
SummaryController ──▶ SummaryBuilder ──┘
```
### Expected (after rollup read path)
```
ReportsController ──▶ ReportBuilder ──┐
├──▶ DataSource.for ──▶ RawDataSource ──▶ reporting_events
SummaryController ──▶ SummaryBuilder ──┘ └──▶ RollupDataSource ──▶ reporting_events_rollups
```
### What changed
- `Reports::DataSource` factory + `Reports::RawDataSource`
- `TimezoneHelper#timezone_name_from_params` — prefers IANA name, falls
back to offset
- Unified `Timeseries::ReportBuilder` replaces `CountReportBuilder` +
`AverageReportBuilder`
- Summary builders delegate to `DataSource` instead of querying directly
### How to test
This is a pure refactor — all existing report pages (Overview, Agent,
Inbox, Label, Team) should produce identical numbers. No feature flag or
new config needed.
---------
Co-authored-by: Muhsin Keloth <muhsinkeramam@gmail.com>
Co-authored-by: Tanmay Deep Sharma <tanmaydeepsharma21@gmail.com>
Co-authored-by: Tanmay Deep Sharma <32020192+tds-1@users.noreply.github.com>
144 lines
5.2 KiB
Ruby
144 lines
5.2 KiB
Ruby
require 'rails_helper'
|
|
|
|
describe EmailChannelFinder do
|
|
include ActionMailbox::TestHelper
|
|
|
|
let!(:channel_email) { create(:channel_email) }
|
|
|
|
describe '#perform' do
|
|
context 'with cc mail' do
|
|
let(:reply_cc_mail) { create_inbound_email_from_fixture('reply_cc.eml') }
|
|
|
|
it 'return channel with cc email' do
|
|
channel_email.update(email: 'test@example.com')
|
|
channel = described_class.new(reply_cc_mail.mail).perform
|
|
expect(channel).to eq(channel_email)
|
|
end
|
|
end
|
|
|
|
context 'with to mail' do
|
|
let(:reply_mail) { create_inbound_email_from_fixture('reply.eml') }
|
|
|
|
it 'return channel with to email' do
|
|
channel_email.update(email: 'test@example.com')
|
|
reply_mail.mail['to'] = 'test@example.com'
|
|
channel = described_class.new(reply_mail.mail).perform
|
|
expect(channel).to eq(channel_email)
|
|
end
|
|
|
|
it 'return channel with to+extension email' do
|
|
channel_email.update(email: 'test@example.com')
|
|
reply_mail.mail['to'] = 'test+123@example.com'
|
|
channel = described_class.new(reply_mail.mail).perform
|
|
expect(channel).to eq(channel_email)
|
|
end
|
|
|
|
it 'return channel with cc email' do
|
|
channel_email.update(email: 'test@example.com')
|
|
reply_mail.mail['to'] = nil
|
|
reply_mail.mail['cc'] = 'test@example.com'
|
|
channel = described_class.new(reply_mail.mail).perform
|
|
expect(channel).to eq(channel_email)
|
|
end
|
|
|
|
it 'return channel with bcc email' do
|
|
channel_email.update(email: 'test@example.com')
|
|
reply_mail.mail['to'] = nil
|
|
reply_mail.mail['bcc'] = 'test@example.com'
|
|
channel = described_class.new(reply_mail.mail).perform
|
|
expect(channel).to eq(channel_email)
|
|
end
|
|
|
|
it 'skip bcc email when account is configured to skip BCC processing' do
|
|
channel_email.update(email: 'test@example.com')
|
|
reply_mail.mail['to'] = nil
|
|
reply_mail.mail['bcc'] = 'test@example.com'
|
|
|
|
allow(GlobalConfigService).to receive(:load)
|
|
.with('SKIP_INCOMING_BCC_PROCESSING', '')
|
|
.and_return(channel_email.account_id.to_s)
|
|
|
|
channel = described_class.new(reply_mail.mail).perform
|
|
expect(channel).to be_nil
|
|
end
|
|
|
|
it 'skip bcc email when account is in multiple account ids config' do
|
|
channel_email.update(email: 'test@example.com')
|
|
reply_mail.mail['to'] = nil
|
|
reply_mail.mail['bcc'] = 'test@example.com'
|
|
|
|
# Include this account along with other account IDs
|
|
other_account_ids = [123, 456, channel_email.account_id, 789]
|
|
allow(GlobalConfigService).to receive(:load)
|
|
.with('SKIP_INCOMING_BCC_PROCESSING', '')
|
|
.and_return(other_account_ids.join(','))
|
|
|
|
channel = described_class.new(reply_mail.mail).perform
|
|
expect(channel).to be_nil
|
|
end
|
|
|
|
it 'process bcc email when account is not in skip config' do
|
|
channel_email.update(email: 'test@example.com')
|
|
reply_mail.mail['to'] = nil
|
|
reply_mail.mail['bcc'] = 'test@example.com'
|
|
|
|
# Configure other account IDs but not this one
|
|
other_account_ids = [channel_email.account_id + 1, channel_email.account_id + 2, channel_email.account_id + 3]
|
|
allow(GlobalConfigService).to receive(:load)
|
|
.with('SKIP_INCOMING_BCC_PROCESSING', '')
|
|
.and_return(other_account_ids.join(','))
|
|
|
|
channel = described_class.new(reply_mail.mail).perform
|
|
expect(channel).to eq(channel_email)
|
|
end
|
|
|
|
it 'process bcc email when skip config is empty' do
|
|
channel_email.update(email: 'test@example.com')
|
|
reply_mail.mail['to'] = nil
|
|
reply_mail.mail['bcc'] = 'test@example.com'
|
|
|
|
allow(GlobalConfigService).to receive(:load)
|
|
.with('SKIP_INCOMING_BCC_PROCESSING', '')
|
|
.and_return('')
|
|
|
|
channel = described_class.new(reply_mail.mail).perform
|
|
expect(channel).to eq(channel_email)
|
|
end
|
|
|
|
it 'process bcc email when skip config is nil' do
|
|
channel_email.update(email: 'test@example.com')
|
|
reply_mail.mail['to'] = nil
|
|
reply_mail.mail['bcc'] = 'test@example.com'
|
|
|
|
allow(GlobalConfigService).to receive(:load)
|
|
.with('SKIP_INCOMING_BCC_PROCESSING', '')
|
|
.and_return(nil)
|
|
|
|
channel = described_class.new(reply_mail.mail).perform
|
|
expect(channel).to eq(channel_email)
|
|
end
|
|
|
|
it 'return channel with X-Original-To email' do
|
|
channel_email.update(email: 'test@example.com')
|
|
reply_mail.mail['to'] = nil
|
|
reply_mail.mail['X-Original-To'] = 'test@example.com'
|
|
channel = described_class.new(reply_mail.mail).perform
|
|
expect(channel).to eq(channel_email)
|
|
end
|
|
|
|
it 'process X-Original-To email even when account is configured to skip BCC processing' do
|
|
channel_email.update(email: 'test@example.com')
|
|
reply_mail.mail['to'] = nil
|
|
reply_mail.mail['X-Original-To'] = 'test@example.com'
|
|
|
|
allow(GlobalConfigService).to receive(:load)
|
|
.with('SKIP_INCOMING_BCC_PROCESSING', '')
|
|
.and_return(channel_email.account_id.to_s)
|
|
|
|
channel = described_class.new(reply_mail.mail).perform
|
|
expect(channel).to eq(channel_email)
|
|
end
|
|
end
|
|
end
|
|
end
|