mirror of
https://github.com/chatwoot/chatwoot.git
synced 2026-06-04 21:02:35 +08:00
## 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>
65 lines
1.5 KiB
Ruby
65 lines
1.5 KiB
Ruby
class Reports::DataSource
|
|
include TimezoneHelper
|
|
|
|
attr_reader :account, :metric, :dimension_type, :dimension_id,
|
|
:scope, :range, :group_by, :timezone_offset,
|
|
:business_hours
|
|
|
|
class << self
|
|
def for(**context)
|
|
# TODO: Route to Reports::RollupDataSource when rollup reads are implemented
|
|
Reports::RawDataSource.new(**context)
|
|
end
|
|
end
|
|
|
|
def initialize(**context)
|
|
@account = context[:account]
|
|
@metric = context[:metric]
|
|
@dimension_type = (context[:dimension_type].presence || 'account').to_s
|
|
@dimension_id = context[:dimension_id]
|
|
@scope = context[:scope]
|
|
@range = context[:range]
|
|
@group_by = context[:group_by].to_s.presence || 'day'
|
|
@timezone_offset = context[:timezone_offset]
|
|
@business_hours = context[:business_hours]
|
|
end
|
|
|
|
private
|
|
|
|
def report_metric
|
|
@report_metric ||= Reports::ReportMetricRegistry.fetch(metric)
|
|
end
|
|
|
|
def average_metric?
|
|
report_metric&.average?
|
|
end
|
|
|
|
def count_metric?
|
|
!average_metric?
|
|
end
|
|
|
|
def rollup_metric
|
|
report_metric&.rollup_metric
|
|
end
|
|
|
|
def raw_event_name
|
|
report_metric&.raw_event_name
|
|
end
|
|
|
|
def raw_count_strategy
|
|
report_metric&.raw_count_strategy
|
|
end
|
|
|
|
def summary_metrics
|
|
@summary_metrics ||= Reports::ReportMetricRegistry.summary_metrics
|
|
end
|
|
|
|
def timezone
|
|
@timezone ||= timezone_name_from_offset(timezone_offset)
|
|
end
|
|
|
|
def use_business_hours?
|
|
ActiveModel::Type::Boolean.new.cast(business_hours)
|
|
end
|
|
end
|