In this issue, @sauloperez suggested a new way to structure code for new reports based on how they are done in Katuma Reports.
From what I understand, his suggestions are:
- Dedicate one controller for each report, with the
index
action handling this. - Scope the report URL under their domain and probably put the report generation classes under the namespace too (for example,
Reports::OrderManagement
). This is actually in line with the concept of organizing code by domain discussed here, but only requires these two steps at this point.
I would also want to separate the following responsibilities into their own classes, to reduce complexity and for easier unit testing:
-
EnterpriseFeeSummary::Parameters
- Receive the form parameters, fetch the parameter objects, and also validate the form parameters. Pagination state could be passed as a form parameter. -
EnterpriseFeeSummary::ReportData
- Presenter which will handle all the report data logic. This would expose very specific methods such asorder_cycle_name
, so that the templates and classes in charge of generating different formats of the report would be forced to be very dumb and consistent with each other.
This class could have presenter subclasses also of this type, assigned to an attribute or added to an array attribute. For example, a multi-section report can have::SummaryReport
in#summary_report
and::BreakdownReport
in#breakdown_report
. The latter can have an array of::FeeDetails
in#fee_details
. - Report generation class for each supported non-template report format. For example a separate class for PDF and another one for XLS.
The report controller would then work like this (can be refactored):
def index
report_parameters = report_parameters_klass.new(params[:report_filters])
render_errors unless report_parameters.valid?
authorize!(:enterprise_fee_summary_report, report_parameters.enterprise)
@report_data = report_data_klass.new(report_parameters)
respond_to do |format|
format.html
format.xls { send_data report_xls_generation_klass.new(@report_data).render, ... }
format.pdf { send_data report_pdf_generation_klass.new(@report_data).render, ... }
end
end
What do you think? Do you have other suggestions?