diff --git a/app/controllers/system_admin/reports_controller.rb b/app/controllers/system_admin/reports_controller.rb index b09876d9..c7b51a29 100644 --- a/app/controllers/system_admin/reports_controller.rb +++ b/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 diff --git a/app/models/reports/applications.rb b/app/models/reports/applications.rb index 870ce2eb..1b8d1927 100644 --- a/app/models/reports/applications.rb +++ b/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 diff --git a/app/models/reports/base.rb b/app/models/reports/base.rb new file mode 100644 index 00000000..70fa021b --- /dev/null +++ b/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 diff --git a/app/models/reports/home_office.rb b/app/models/reports/home_office.rb index b8aad595..291eb5ea 100644 --- a/app/models/reports/home_office.rb +++ b/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 }) diff --git a/app/models/reports/payroll.rb b/app/models/reports/payroll.rb index e6d583a9..630be075 100644 --- a/app/models/reports/payroll.rb +++ b/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 }) diff --git a/app/models/reports/qa_report.rb b/app/models/reports/qa_report.rb index b4913e1c..9f6871cb 100644 --- a/app/models/reports/qa_report.rb +++ b/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| [ diff --git a/app/models/reports/standing_data.rb b/app/models/reports/standing_data.rb index 046e01e9..0cf68e6e 100644 --- a/app/models/reports/standing_data.rb +++ b/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 }) diff --git a/app/services/report.rb b/app/services/report.rb index 5a6de19b..ff1ae156 100644 --- a/app/services/report.rb +++ b/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 diff --git a/spec/features/admin_console/reports_spec.rb b/spec/features/admin_console/reports_spec.rb index f01b734e..2db39a78 100644 --- a/spec/features/admin_console/reports_spec.rb +++ b/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 diff --git a/spec/models/reports/applications_spec.rb b/spec/models/reports/applications_spec.rb index 1b1928a4..a327be5a 100644 --- a/spec/models/reports/applications_spec.rb +++ b/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 diff --git a/spec/models/reports/home_office_spec.rb b/spec/models/reports/home_office_spec.rb index 2d413db2..673b275b 100644 --- a/spec/models/reports/home_office_spec.rb +++ b/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 diff --git a/spec/models/reports/payroll_spec.rb b/spec/models/reports/payroll_spec.rb index 3d0a7240..59d1946b 100644 --- a/spec/models/reports/payroll_spec.rb +++ b/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 diff --git a/spec/models/reports/qa_report_spec.rb b/spec/models/reports/qa_report_spec.rb index d260fa6d..1fd0a83f 100644 --- a/spec/models/reports/qa_report_spec.rb +++ b/spec/models/reports/qa_report_spec.rb @@ -7,17 +7,16 @@ module Reports describe QaReport do include ActiveSupport::Testing::TimeHelpers - subject(:report) { described_class.new(applications, status) } + subject(:report) { described_class.new(status:) } - let(:applications) { [] } - let(:status) { "submitted" } + let(:status) { "initial_checks" } - 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 = "QA-Report-submitted-2023_07_17-12_30_45.csv" + expected_name = "reports-qa-report-initial_checks-20230717-123045.csv" - expect(report.name).to eq(expected_name) + expect(report.filename).to eq(expected_name) end end @@ -25,7 +24,6 @@ module Reports context "when the status is not 'rejected'" do it "returns the data in CSV format" do application = create(:application) - applications << application expect(report.csv).to include([ application.urn, @@ -51,7 +49,6 @@ module Reports it "returns the data including rejection reasons in CSV format" do application = create(:application, application_progress: build(:application_progress, rejection_completed_at: Time.zone.now, status: :rejected, rejection_reason: :request_to_re_submit, rejection_details: "Some details")) - applications << application expect(report.csv).to include([ application.urn, diff --git a/spec/models/reports/standing_data_spec.rb b/spec/models/reports/standing_data_spec.rb index 263e8916..c4cd9f6b 100644 --- a/spec/models/reports/standing_data_spec.rb +++ b/spec/models/reports/standing_data_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 = "Standing-Data-Report-20230717-123045.csv" + expected_name = "reports-standing-data-20230717-123045.csv" report = described_class.new - actual_name = report.name + actual_name = report.filename expect(actual_name).to eq(expected_name) end @@ -83,16 +83,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.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 # rubocop:enable RSpec/ExampleLength diff --git a/spec/services/report_spec.rb b/spec/services/report_spec.rb index 046ad024..8f61a0b6 100644 --- a/spec/services/report_spec.rb +++ b/spec/services/report_spec.rb @@ -31,7 +31,7 @@ let(:status) { "initial_checks" } context "report_name" do - it { expect(service.report_name).to eq("Reports::qareport") } + it { expect(service.name).to eq("reports-qa-report-initial_checks") } end context "filename" do @@ -39,7 +39,7 @@ it "returns the name of the Report" do frozen_time = Time.zone.local(2023, 7, 17, 12, 30, 45) travel_to frozen_time do - expected_name = "QA-Report-initial_checks-2023_07_17-12_30_45.csv" + expected_name = "reports-qa-report-initial_checks-20230717-123045.csv" expect(service.filename).to eq(expected_name) end @@ -55,7 +55,7 @@ service.data end - it { expect(Reports::QaReport).to have_received(:new).with(kind_of(Array), status) } + it { expect(Reports::QaReport).to have_received(:new).with(status:) } it { expect(report).to have_received(:csv) } end