diff --git a/app/controllers/system_admin/applicants_controller.rb b/app/controllers/system_admin/applicants_controller.rb index 206bc98e..06bbca36 100644 --- a/app/controllers/system_admin/applicants_controller.rb +++ b/app/controllers/system_admin/applicants_controller.rb @@ -21,19 +21,6 @@ def index session[:application_ids] = results.map(&:id) end - def download_qa_csv - status = session[:filter_status] - application_ids = session[:application_ids] - - applications = Application.where(id: application_ids).reject(&:qa?) - - applications.each(&:mark_as_qa!) - - report = Reports::QaReport.new(applications, status) - create_audit(action: "Downloaded QA CSV report (#{status.humanize})") - send_data(report.csv, filename: report.name) - end - def duplicates @pagy, @duplicates = pagy(DuplicateApplication.search(params[:search]).select("DISTINCT ON (urn) *")) end diff --git a/app/controllers/system_admin/reports_controller.rb b/app/controllers/system_admin/reports_controller.rb index 2415c20e..b09876d9 100644 --- a/app/controllers/system_admin/reports_controller.rb +++ b/app/controllers/system_admin/reports_controller.rb @@ -3,22 +3,16 @@ class ReportsController < AdminController def index; end def show - report = find_report(params[:id]) - create_audit(action: "Downloaded #{report.class.to_s.capitalize} report") + service = Report.call(params[:id], **report_params) + create_audit(action: "Downloaded #{service.report_name} report") - # TODO: Execute report is background job - send_data(report.csv, filename: report.name) + send_data(service.data, filename: service.filename) end private - def find_report(report_id) - { - home_office: Reports::HomeOffice.new, - standing_data: Reports::StandingData.new, - payroll: Reports::Payroll.new, - applications: Reports::Applications.new, - }.with_indifferent_access.fetch(report_id) + def report_params + params.permit(:id, :status) end end end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index dd7e9147..e5e2f9cd 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -20,4 +20,18 @@ def mailto_irp_express_interest def banner_feedback_form govuk_link_to("feedback", "https://forms.office.com/e/p45Wm1Vmxg", target: "_blank") end + + def application_statuses + ApplicationProgress + .statuses + .keys + .map { |status| [status.humanize, status] } + end + + def application_statuses_options(selected: nil, all_statuses: false) + statuses = application_statuses + statuses = application_statuses.unshift(["All statuses", ""]) if all_statuses + + options_for_select(statuses, selected:) + end end diff --git a/app/services/report.rb b/app/services/report.rb new file mode 100644 index 00000000..dbfb50b4 --- /dev/null +++ b/app/services/report.rb @@ -0,0 +1,61 @@ +class Report + REGISTERED_REPORTS = { + home_office: Reports::HomeOffice, + standing_data: Reports::StandingData, + payroll: Reports::Payroll, + applications: Reports::Applications, + qa: Reports::QaReport, + }.freeze + + def self.call(...) + service = new(...) + service.data + service + end + + def initialize(report_id, **kwargs) + @kwargs = kwargs&.symbolize_keys || {} + @report_class = REGISTERED_REPORTS.with_indifferent_access.fetch(report_id) + rescue KeyError + raise(ArgumentError, "Invalid report id #{report_id}") + end + + def report_name + report_class.to_s.capitalize + end + + def filename + report.name + end + + def data + report.csv + end + +private + + 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] + end +end diff --git a/app/views/system_admin/applicants/index.html.erb b/app/views/system_admin/applicants/index.html.erb index dc6637ab..5aba9eb6 100644 --- a/app/views/system_admin/applicants/index.html.erb +++ b/app/views/system_admin/applicants/index.html.erb @@ -1,8 +1,3 @@ -<% statuses = options_for_select( - ApplicationProgress.statuses.keys.map { |status| [status.humanize, status] }.unshift(['All statuses', '']), - selected: params[:status] - ) -%> <%= form_with(url: applicants_path, method: :get, id: :search) do |f| %>
<%= f.govuk_text_field :search, value: params[:search], label: { text: 'Search' }, hint: { text: 'Search by name, email, passport number or unique reference number (URN)' } %> @@ -10,7 +5,7 @@
- <%= f.govuk_select :status, statuses, label: { text: "Filter by application status" } %> + <%= f.govuk_select :status, application_statuses_options(selected: params[:status], all_statuses: true), label: { text: "Filter by application status" } %>
@@ -25,9 +20,6 @@
<%= f.govuk_submit 'Search', secondary: true %>
- <% if session[:filter_status].present? %> - <%= link_to "Download QA CSV", download_qa_csv_applicants_path, class: "govuk-button" %> - <% end %> <% end %> <%= govuk_table(classes: "applicants-table") do |table| diff --git a/app/views/system_admin/reports/index.html.erb b/app/views/system_admin/reports/index.html.erb index 57c2bb57..5e78d0f7 100644 --- a/app/views/system_admin/reports/index.html.erb +++ b/app/views/system_admin/reports/index.html.erb @@ -38,3 +38,23 @@ <%= link_to "Download", report_path(:applications), class: "govuk-button" %>

+ +
+

QA reports

+

+ Download a QA CSV file +

+

+ + <%= form_with(url: report_path(:qa), method: :get) do |f| %> +

+
+ <%= f.govuk_select :status, application_statuses_options, label: { text: "Filter by application status" } %> +
+
+ <%= f.govuk_submit 'Download', class: "govuk-button"%> +
+
+ <% end %> +

+
diff --git a/spec/features/admin_console/download_qa_csv_file_spec.rb b/spec/features/admin_console/download_qa_csv_file_spec.rb deleted file mode 100644 index 35fc74a8..00000000 --- a/spec/features/admin_console/download_qa_csv_file_spec.rb +++ /dev/null @@ -1,76 +0,0 @@ -require "rails_helper" - -describe "Download QA CSV functionality" do - include AdminHelpers - - it "shows the 'Download QA CSV' button only if a status filter is applied" do - given_there_are_few_applications - given_i_am_signed_as_an_admin - when_i_am_in_the_applications_list_page - then_i_should_not_see_the_download_button - when_i_filter_by_a_status - then_i_should_see_the_download_button - end - - it "marks applications as qa before generating the csv" do - given_there_are_few_applications - given_i_am_signed_as_an_admin - when_i_am_in_the_applications_list_page - when_i_filter_by_a_status - when_i_download_the_csv - - then_all_applications_should_be_marked_as_qa - end - - it "generates an empty csv when downloaded twice consecutively" do - given_there_are_few_applications - given_i_am_signed_as_an_admin - when_i_am_in_the_applications_list_page - when_i_filter_by_a_status - when_i_download_the_csv - when_i_am_in_the_applications_list_page - when_i_filter_by_a_status - when_i_download_the_csv_again - - then_csv_should_be_empty - end - - def given_there_are_few_applications - create_list(:application, 5) - end - - def when_i_am_in_the_applications_list_page - visit(applicants_path) - end - - def when_i_filter_by_a_status - select "Initial checks", from: "status" - click_button "Search" - end - - def then_i_should_not_see_the_download_button - expect(page).not_to have_link("Download QA CSV") - end - - def then_i_should_see_the_download_button - expect(page).to have_link("Download QA CSV") - end - - def when_i_download_the_csv - click_link "Download QA CSV" - end - - def when_i_download_the_csv_again - when_i_download_the_csv - end - - def then_all_applications_should_be_marked_as_qa - Application.all.each do |app| - expect(app.reload.qa?).to be(true) - end - end - - def then_csv_should_be_empty - expect(page.body.lines.count).to eq(1) - end -end diff --git a/spec/features/admin_console/reports_spec.rb b/spec/features/admin_console/reports_spec.rb index 41e5aec7..f01b734e 100644 --- a/spec/features/admin_console/reports_spec.rb +++ b/spec/features/admin_console/reports_spec.rb @@ -29,6 +29,22 @@ then_the_payroll_data_csv_report_is_downloaded end + it "exports Application CSV" do + given_i_am_signed_as_an_admin + when_i_am_in_the_reports_page + and_i_click_on_the_applications_csv_link + + then_the_applications_csv_report_is_downloaded + end + + it "exports Qa report CSV" do + given_i_am_signed_as_an_admin + when_i_am_in_the_reports_page + and_i_click_on_the_qa_report_csv_button + + then_the_qa_report_csv_report_is_downloaded + end + private def then_the_standing_data_csv_report_is_downloaded @@ -49,6 +65,18 @@ def then_the_payroll_data_csv_report_is_downloaded expect(page.response_headers["Content-Disposition"]).to match(/filename="Payroll-Report.*/) 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.*/) + 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*/) + end + def and_i_click_on_the_home_office_csv_link within ".home-office" do click_on "Download" @@ -67,6 +95,18 @@ def and_i_click_on_the_payroll_data_csv_link end end + def and_i_click_on_the_applications_csv_link + within ".applications" do + click_on "Download" + end + end + + def and_i_click_on_the_qa_report_csv_button + within ".applications-qa" do + click_on "Download" + end + end + def when_i_am_in_the_reports_page visit reports_path end diff --git a/spec/services/report_spec.rb b/spec/services/report_spec.rb new file mode 100644 index 00000000..046ad024 --- /dev/null +++ b/spec/services/report_spec.rb @@ -0,0 +1,76 @@ +require "rails_helper" + +RSpec.describe Report do + subject(:report) { described_class } + + describe "registered reports" do + subject(:registered_reports) { described_class::REGISTERED_REPORTS } + + let(:expected_ids) do + %i[home_office standing_data payroll applications qa] + end + + let(:expected_classes) do + [ + Reports::HomeOffice, + Reports::StandingData, + Reports::Payroll, + Reports::Applications, + Reports::QaReport, + ] + end + + it { expect(registered_reports.keys).to match_array(expected_ids) } + it { expect(registered_reports.values).to match_array(expected_classes) } + end + + describe ".call" do + subject(:service) { described_class.new(report_id, status:) } + + let(:report_id) { "qa" } + let(:status) { "initial_checks" } + + context "report_name" do + it { expect(service.report_name).to eq("Reports::qareport") } + end + + context "filename" do + include ActiveSupport::Testing::TimeHelpers + 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" + + expect(service.filename).to eq(expected_name) + end + end + end + + # rubocop:disable RSpec/VerifiedDoubles + context "qa report data" do + let(:report) { spy(Reports::QaReport) } + + before do + allow(Reports::QaReport).to receive(:new).and_return(report) + service.data + end + + it { expect(Reports::QaReport).to have_received(:new).with(kind_of(Array), status) } + it { expect(report).to have_received(:csv) } + end + + context "other report data" do + let(:report_id) { "home_office" } + let(:report) { spy(Reports::HomeOffice) } + + before do + allow(Reports::HomeOffice).to receive(:new).and_return(report) + service.data + end + + it { expect(Reports::HomeOffice).to have_received(:new) } + it { expect(report).to have_received(:csv) } + end + # rubocop:enable RSpec/VerifiedDoubles + end +end