## 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>
49 lines
1.5 KiB
Ruby
49 lines
1.5 KiB
Ruby
require 'rails_helper'
|
|
|
|
RSpec.describe V2::Reports::Conversations::MetricBuilder, type: :model do
|
|
subject { described_class.new(account, params) }
|
|
|
|
let(:account) { create(:account) }
|
|
let(:params) { { since: '2023-01-01', until: '2024-01-01' } }
|
|
let(:builder_instance) { instance_double(V2::Reports::Timeseries::ReportBuilder, aggregate_value: 42) }
|
|
|
|
before do
|
|
allow(V2::Reports::Timeseries::ReportBuilder).to receive(:new).and_return(builder_instance)
|
|
end
|
|
|
|
describe '#summary' do
|
|
it 'returns the correct summary values' do
|
|
summary = subject.summary
|
|
expect(summary).to eq(
|
|
{
|
|
conversations_count: 42,
|
|
incoming_messages_count: 42,
|
|
outgoing_messages_count: 42,
|
|
avg_first_response_time: 42,
|
|
avg_resolution_time: 42,
|
|
resolutions_count: 42,
|
|
reply_time: 42
|
|
}
|
|
)
|
|
end
|
|
|
|
it 'creates builders with proper params' do
|
|
subject.summary
|
|
expect(V2::Reports::Timeseries::ReportBuilder).to have_received(:new).with(account, params.merge(metric: 'conversations_count'))
|
|
expect(V2::Reports::Timeseries::ReportBuilder).to have_received(:new).with(account, params.merge(metric: 'avg_first_response_time'))
|
|
end
|
|
end
|
|
|
|
describe '#bot_summary' do
|
|
it 'returns a detailed summary of bot-specific conversation metrics' do
|
|
bot_summary = subject.bot_summary
|
|
expect(bot_summary).to eq(
|
|
{
|
|
bot_resolutions_count: 42,
|
|
bot_handoffs_count: 42
|
|
}
|
|
)
|
|
end
|
|
end
|
|
end
|