Code structure for new reports

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:

  1. Dedicate one controller for each report, with the index action handling this.
  2. 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:

  1. 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.
  2. EnterpriseFeeSummary::ReportData - Presenter which will handle all the report data logic. This would expose very specific methods such as order_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.
  3. 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?

LOVE IT! looks really good, seriously. That’s what reports needs. I would simply be cautious with trying to design all upfront. I like when getting to a first implementation, and following TDD, the best design reveals itself. So, let’s get your plan and then see what new abstractions we come up with.

Just as a super minor detail, I prefer to pass the particular format class as a strategy to the report_data. Something like:

format.xls { ReportData.new(XLSReport).render, ...}
format.pdf { ReportData.new(PDFReport).render, ...}

You know, then implementing new formats becomes just implementing the particular WhateverFormatReport which doesn’t deal with the data retrieval, just formatting.

Great idea to implement the report format generation classes as strategies, @sauloperez! :grinning: