Add Reports::Base

Serves as the basis of all reports that are actioned through the
report service.

Refactored existing reports to participate in this protocol.

app/controllers/system_admin/reports_controller.rb

@@ -3,10 +3,10 @@ class ReportsController < AdminController
     def index; end
 
     def show
-      service = Report.call(params[:id], **report_params)
-      create_audit(action: "Downloaded #{service.report_name} report")
+      report = Report.call(params[:id], **report_params)
+      create_audit(action: "Downloaded #{report.name} report")
 
-      send_data(service.data, filename: service.filename)
+      send_data(report.data, filename: report.filename)
     end
 
   private

app/models/reports/applications.rb

@@ -1,11 +1,5 @@
 module Reports
-  class Applications
-    def name
-      current_time = Time.zone.now.strftime("%Y%m%d-%H%M%S")
-
-      "Applications-Report-#{current_time}.csv"
-    end
-
+  class Applications < Base
     def csv
       CSV.generate do |csv|
         csv << header

app/models/reports/base.rb

@@ -0,0 +1,20 @@
+module Reports
+  class Base
+    def initialize(**kwargs)
+      @kwargs = kwargs
+      @name = self.class.name.titleize.tr(" /", "-").downcase
+    end
+
+    attr_reader :name, :kwargs
+
+    def filename
+      current_time = Time.zone.now.strftime("%Y%m%d-%H%M%S")
+
+      "#{name}-#{current_time}.csv"
+    end
+
+    def csv; end
+
+    def post_generation_hook; end
+  end
+end

app/models/reports/home_office.rb

@@ -1,20 +1,16 @@
 # frozen_string_literal: true
 
 module Reports
-  class HomeOffice
-    def name
-      current_time = Time.zone.now.strftime("%Y%m%d-%H%M%S")
-
-      "Home-Office-Report-#{current_time}.csv"
-    end
-
+  class HomeOffice < Base
     def csv
-      csv_file = CSV.generate do |csv|
+      CSV.generate do |csv|
         csv << header
         rows.each { |row| csv << row }
       end
+    end
+
+    def post_generation_hook
       applications.update_all(home_office_csv_downloaded_at: Time.zone.now) # rubocop:disable Rails/SkipsModelValidations
-      csv_file
     end
 
   private
@@ -40,7 +36,7 @@ def rows
     end
 
     def applications
-      Application
+      @applications ||= Application
         .joins(:application_progress)
         .includes(:applicant)
         .where.not(application_progresses: { initial_checks_completed_at: nil })

app/models/reports/payroll.rb

@@ -1,20 +1,16 @@
 # frozen_string_literal: true
 
 module Reports
-  class Payroll
-    def name
-      current_time = Time.zone.now.strftime("%Y%m%d-%H%M%S")
-
-      "Payroll-Report-#{current_time}.csv"
-    end
-
+  class Payroll < Base
     def csv
-      csv_file = CSV.generate do |csv|
+      CSV.generate do |csv|
         csv << header
         rows.each { |row| csv << row }
       end
+    end
+
+    def post_generation_hook
       applications.update_all(payroll_csv_downloaded_at: Time.zone.now) # rubocop:disable Rails/SkipsModelValidations
-      csv_file
     end
 
   private
@@ -56,7 +52,7 @@ def rows
     end
 
     def applications
-      Application
+      @applications ||= Application
         .joins(:application_progress)
         .joins(applicant: :address)
         .where.not(application_progresses: { banking_approval_completed_at: nil })

app/models/reports/qa_report.rb

@@ -1,15 +1,8 @@
 module Reports
-  class QaReport
-    attr_reader :applications, :status
-
-    def initialize(applications, status)
-      @applications = applications
-      @status = status
-    end
-
-    def name
-      current_time = Time.zone.now.strftime("%Y_%m_%d-%H_%M_%S")
-      "QA-Report-#{status}-#{current_time}.csv"
+  class QaReport < Base
+    def initialize(...)
+      super(...)
+      @name = [@name, status].join("-")
     end
 
     def csv
@@ -19,8 +12,27 @@ def csv
       end
     end
 
+    def post_generation_hook
+      applications.each(&:mark_as_qa!)
+    end
+
+    def status
+      kwargs.fetch(:status)
+    end
+
   private
 
+    def applications
+      @applications ||= Application
+                          .includes(
+                            :applicant,
+                            :application_progress,
+                            applicant: :address,
+                          )
+                          .filter_by_status(status)
+                          .reject(&:qa?)
+    end
+
     def rows
       applications.map do |application|
         [

app/models/reports/standing_data.rb

@@ -1,20 +1,16 @@
 # frozen_string_literal: true
 
 module Reports
-  class StandingData
-    def name
-      current_time = Time.zone.now.strftime("%Y%m%d-%H%M%S")
-
-      "Standing-Data-Report-#{current_time}.csv"
-    end
-
+  class StandingData < Base
     def csv
-      csv_file = CSV.generate do |csv|
+      CSV.generate do |csv|
         csv << header
         rows.each { |row| csv << row }
       end
+    end
+
+    def post_generation_hook
       applications.update_all(standing_data_csv_downloaded_at: Time.zone.now) # rubocop:disable Rails/SkipsModelValidations
-      csv_file
     end
 
   private
@@ -34,7 +30,7 @@ def rows
     end
 
     def applications
-      Application
+      @applications ||= Application
         .joins(:application_progress)
         .joins(applicant: :address)
         .where.not(application_progresses: { school_checks_completed_at: nil })

app/services/report.rb

@@ -8,9 +8,10 @@ class Report
   }.freeze
 
   def self.call(...)
-    service = new(...)
-    service.data
-    service
+    report = new(...)
+    report.data
+    report.post_generation_hook
+    report
   end
 
   def initialize(report_id, **kwargs)
@@ -20,13 +21,9 @@ def initialize(report_id, **kwargs)
     raise(ArgumentError, "Invalid report id #{report_id}")
   end
 
-  def report_name
-    report_class.to_s.capitalize
-  end
-
-  def filename
-    report.name
-  end
+  delegate :name, to: :report
+  delegate :filename, to: :report
+  delegate :post_generation_hook, to: :report
 
   def data
     @data ||= report.csv
@@ -37,25 +34,6 @@ def data
   attr_reader :report_class, :kwargs
 
   def report
-    return @report if @report
-    return @report = report_class.new(*report_args) if report_args
-
-    @report = report_class.new
-  end
-
-  def report_args
-    return qa_report_args if report_class == Reports::QaReport
-
-    nil
-  end
-
-  def qa_report_args
-    return @qa_report_args if @qa_report_args
-
-    status = kwargs.fetch(:status)
-    applications = Application.filter_by_status(status).reject(&:qa?)
-    applications.each(&:mark_as_qa!)
-
-    @qa_report_args = [applications, status]
+    @report ||= report_class.new(**kwargs)
   end
 end

spec/features/admin_console/reports_spec.rb

@@ -50,31 +50,31 @@
   def then_the_standing_data_csv_report_is_downloaded
     expect(page.response_headers["Content-Type"]).to match(/text\/csv/)
     expect(page.response_headers["Content-Disposition"]).to include "attachment"
-    expect(page.response_headers["Content-Disposition"]).to match(/filename="Standing-Data-Report.*/)
+    expect(page.response_headers["Content-Disposition"]).to match(/filename="reports-standing-data.*/)
   end
 
   def then_the_home_office_csv_report_is_downloaded
     expect(page.response_headers["Content-Type"]).to match(/text\/csv/)
     expect(page.response_headers["Content-Disposition"]).to include "attachment"
-    expect(page.response_headers["Content-Disposition"]).to match(/filename="Home-Office-Report.*/)
+    expect(page.response_headers["Content-Disposition"]).to match(/filename="reports-home-office.*/)
   end
 
   def then_the_payroll_data_csv_report_is_downloaded
     expect(page.response_headers["Content-Type"]).to match(/text\/csv/)
     expect(page.response_headers["Content-Disposition"]).to include "attachment"
-    expect(page.response_headers["Content-Disposition"]).to match(/filename="Payroll-Report.*/)
+    expect(page.response_headers["Content-Disposition"]).to match(/filename="reports-payroll.*/)
   end
 
   def then_the_applications_csv_report_is_downloaded
     expect(page.response_headers["Content-Type"]).to match(/text\/csv/)
     expect(page.response_headers["Content-Disposition"]).to include "attachment"
-    expect(page.response_headers["Content-Disposition"]).to match(/filename="Applications-Report.*/)
+    expect(page.response_headers["Content-Disposition"]).to match(/filename="reports-applications.*/)
   end
 
   def then_the_qa_report_csv_report_is_downloaded
     expect(page.response_headers["Content-Type"]).to match(/text\/csv/)
     expect(page.response_headers["Content-Disposition"]).to include "attachment"
-    expect(page.response_headers["Content-Disposition"]).to match(/filename="QA-Report-initial_checks*/)
+    expect(page.response_headers["Content-Disposition"]).to match(/filename="reports-qa-report-initial_checks*/)
   end
 
   def and_i_click_on_the_home_office_csv_link

spec/models/reports/applications_spec.rb

@@ -6,13 +6,13 @@ module Reports
 
     subject(:report) { described_class.new }
 
-    it "returns the name of the Report" do
+    it "returns the filename of the Report" do
       frozen_time = Time.zone.local(2023, 7, 17, 12, 30, 45)
       travel_to frozen_time do
-        expected_name = "Applications-Report-20230717-123045.csv"
+        expected_name = "reports-applications-20230717-123045.csv"
 
         report = described_class.new
-        actual_name = report.name
+        actual_name = report.filename
 
         expect(actual_name).to eq(expected_name)
       end

spec/models/reports/home_office_spec.rb

@@ -8,13 +8,13 @@ module Reports
 
     subject(:report) { described_class.new }
 
-    it "returns the name of the Report" do
+    it "returns the filename of the Report" do
       frozen_time = Time.zone.local(2023, 7, 17, 12, 30, 45)
       travel_to frozen_time do
-        expected_name = "Home-Office-Report-20230717-123045.csv"
+        expected_name = "reports-home-office-20230717-123045.csv"
 
         report = described_class.new
-        actual_name = report.name
+        actual_name = report.filename
 
         expect(actual_name).to eq(expected_name)
       end
@@ -88,16 +88,19 @@ module Reports
         expect(report.csv).to include(expected_header)
       end
 
-      it "excludes applications from the csv after they've been downloaded once" do
-        app = create(:application, application_progress: build(:application_progress, :home_office_pending))
+      context "includes applications from the csv before invoking `post_generation_hook`" do
+        let(:app) { create(:application, application_progress: build(:application_progress, :home_office_pending)) }
+        let(:csv) { report.csv }
 
-        first_csv = report.csv
+        before { app }
 
-        expect(first_csv).to include(app.urn)
+        it { expect(csv).to include(app.urn) }
 
-        second_csv = report.csv
+        context "excludes applications from the csv after invoking `post_generation_hook`" do
+          before { report.post_generation_hook }
 
-        expect(second_csv).not_to include(app.urn)
+          it { expect(csv).not_to include(app.urn) }
+        end
       end
     end
   end

spec/models/reports/payroll_spec.rb

@@ -9,13 +9,13 @@ module Reports
 
     subject(:report) { described_class.new }
 
-    it "returns the name of the Report" do
+    it "returns the filename of the Report" do
       frozen_time = Time.zone.local(2023, 7, 17, 12, 30, 45)
       travel_to frozen_time do
-        expected_name = "Payroll-Report-20230717-123045.csv"
+        expected_name = "reports-payroll-20230717-123045.csv"
 
         report = described_class.new
-        actual_name = report.name
+        actual_name = report.filename
 
         expect(actual_name).to eq(expected_name)
       end
@@ -119,16 +119,19 @@ module Reports
         expect(report.csv).to include(expected_header)
       end
 
-      it "excludes applications from the csv after they've been downloaded once" do
-        app = create(:application, application_progress: progress)
+      context "includes applications from the csv before invoking `post_generation_hook`" do
+        let(:app) { create(:application, application_progress: progress) }
+        let(:csv) { report.csv }
 
-        first_csv = report.csv
+        before { app }
 
-        expect(first_csv).to include(app.applicant.email_address)
+        it { expect(csv).to include(app.applicant.email_address) }
 
-        second_csv = report.csv
+        context "excludes applications from the csv after invoking `post_generation_hook`" do
+          before { report.post_generation_hook }
 
-        expect(second_csv).not_to include(app.applicant.email_address)
+          it { expect(csv).not_to include(app.urn) }
+        end
       end
     end
   end