+ <%= intro %> +
+ <% sorted_errors.each do |error| %> <%= govuk_table do |table| %> <% table.head do |head| %> diff --git a/app/components/bulk_upload_error_summary_table_component.rb b/app/components/bulk_upload_error_summary_table_component.rb index 8dddb61d2..7b483077f 100644 --- a/app/components/bulk_upload_error_summary_table_component.rb +++ b/app/components/bulk_upload_error_summary_table_component.rb @@ -15,7 +15,7 @@ class BulkUploadErrorSummaryTableComponent < ViewComponent::Base @sorted_errors ||= setup_errors.presence || bulk_upload .bulk_upload_errors .group(:col, :field, :error) - .having("count(*) > ?", display_threshold) + .having("count(*) >= ?", display_threshold) .count .sort_by { |el| el[0][0].rjust(3, "0") } end @@ -24,6 +24,14 @@ class BulkUploadErrorSummaryTableComponent < ViewComponent::Base sorted_errors.present? end + def intro + if setup_errors.present? + "This summary shows important questions that have errors. See full error report for more details." + else + "This summary shows questions that have more than #{BulkUploadErrorSummaryTableComponent::DISPLAY_THRESHOLD - 1} errors. See full error report for more details." + end + end + private def setup_errors @@ -31,7 +39,6 @@ private .bulk_upload_errors .where(category: "setup") .group(:col, :field, :error) - .having("count(*) > ?", display_threshold) .count .sort_by { |el| el[0][0].rjust(3, "0") } end diff --git a/app/mailers/bulk_upload_mailer.rb b/app/mailers/bulk_upload_mailer.rb index 2d0b20a92..b6388213a 100644 --- a/app/mailers/bulk_upload_mailer.rb +++ b/app/mailers/bulk_upload_mailer.rb @@ -5,7 +5,6 @@ class BulkUploadMailer < NotifyMailer FAILED_CSV_ERRORS_TEMPLATE_ID = "e27abcd4-5295-48c2-b127-e9ee4b781b75".freeze FAILED_FILE_SETUP_ERROR_TEMPLATE_ID = "24c9f4c7-96ad-470a-ba31-eb51b7cbafd9".freeze FAILED_SERVICE_ERROR_TEMPLATE_ID = "c3f6288c-7a74-4e77-99ee-6c4a0f6e125a".freeze - WITH_ERRORS_TEMPLATE_ID = "eb539005-6234-404e-812d-167728cf4274".freeze HOW_FIX_UPLOAD_TEMPLATE_ID = "21a07b26-f625-4846-9f4d-39e30937aa24".freeze def send_how_fix_upload_mail(bulk_upload:) @@ -73,25 +72,12 @@ class BulkUploadMailer < NotifyMailer end def send_bulk_upload_failed_file_setup_error_mail(bulk_upload:) - bulk_upload_link = if bulk_upload.lettings? - start_bulk_upload_lettings_logs_url + bulk_upload_link = if BulkUploadErrorSummaryTableComponent.new(bulk_upload:).errors? + summary_bulk_upload_lettings_result_url(bulk_upload) else - start_bulk_upload_sales_logs_url + bulk_upload_lettings_result_url(bulk_upload) end - row_parser_class = bulk_upload.prefix_namespace::RowParser - - errors = bulk_upload - .bulk_upload_errors - .where(category: "setup") - .group(:col, :field) - .count - .keys - .sort_by { |_col, field| field } - .map do |col, field| - "- #{row_parser_class.question_for_field(field.to_sym)} (Column #{col})" - end - send_email( bulk_upload.user.email, FAILED_FILE_SETUP_ERROR_TEMPLATE_ID, @@ -100,7 +86,6 @@ class BulkUploadMailer < NotifyMailer upload_timestamp: bulk_upload.created_at.to_fs(:govuk_date_and_time), lettings_or_sales: bulk_upload.log_type, year_combo: bulk_upload.year_combo, - errors_list: errors.join("\n"), bulk_upload_link:, }, ) @@ -126,26 +111,4 @@ class BulkUploadMailer < NotifyMailer }, ) end - - def send_bulk_upload_with_errors_mail(bulk_upload:) - count = bulk_upload.logs.where.not(status: %w[completed]).count - - n_logs = pluralize(count, "log") - - title = "We found #{n_logs} with errors" - - error_description = "We created logs from your #{bulk_upload.year_combo} #{bulk_upload.log_type} data. There was a problem with #{count} of the logs. Click the below link to fix these logs." - - send_email( - bulk_upload.user.email, - WITH_ERRORS_TEMPLATE_ID, - { - title:, - filename: bulk_upload.filename, - upload_timestamp: bulk_upload.created_at.to_fs(:govuk_date_and_time), - error_description:, - summary_report_link: resume_bulk_upload_lettings_result_url(bulk_upload), - }, - ) - end end diff --git a/app/services/bulk_upload/processor.rb b/app/services/bulk_upload/processor.rb index 4fe449348..49d44261f 100644 --- a/app/services/bulk_upload/processor.rb +++ b/app/services/bulk_upload/processor.rb @@ -59,12 +59,6 @@ private .deliver_later end - def send_fix_errors_mail - BulkUploadMailer - .send_bulk_upload_with_errors_mail(bulk_upload:) - .deliver_later - end - def send_success_mail BulkUploadMailer .send_bulk_upload_complete_mail(user:, bulk_upload:) diff --git a/app/views/bulk_upload_lettings_results/summary.html.erb b/app/views/bulk_upload_lettings_results/summary.html.erb index a6072fe89..533e2af05 100644 --- a/app/views/bulk_upload_lettings_results/summary.html.erb +++ b/app/views/bulk_upload_lettings_results/summary.html.erb @@ -16,10 +16,6 @@- This summary shows questions that have at least <%= BulkUploadErrorSummaryTableComponent::DISPLAY_THRESHOLD %> errors or more. See full error report for more details. -
- <%= render BulkUploadErrorSummaryTableComponent.new(bulk_upload: @bulk_upload) %> <% end %> diff --git a/spec/components/bulk_upload_error_summary_table_component_spec.rb b/spec/components/bulk_upload_error_summary_table_component_spec.rb index 32da38119..79be32901 100644 --- a/spec/components/bulk_upload_error_summary_table_component_spec.rb +++ b/spec/components/bulk_upload_error_summary_table_component_spec.rb @@ -30,6 +30,25 @@ RSpec.describe BulkUploadErrorSummaryTableComponent, type: :component do end end + context "when on threshold" do + before do + stub_const("BulkUploadErrorSummaryTableComponent::DISPLAY_THRESHOLD", 1) + + create(:bulk_upload_error, bulk_upload:, col: "A", row: 1) + end + + it "renders tables" do + result = render_inline(component) + expect(result).to have_selector("table", count: 1) + end + + it "renders intro with threshold" do + result = render_inline(component) + + expect(result).to have_content("This summary shows questions that have more than 0 errors. See full error report for more details.") + end + end + context "when there are 2 independent errors" do let!(:error_2) { create(:bulk_upload_error, bulk_upload:, col: "B", row: 2) } let!(:error_1) { create(:bulk_upload_error, bulk_upload:, col: "A", row: 1) } @@ -99,6 +118,8 @@ RSpec.describe BulkUploadErrorSummaryTableComponent, type: :component do before do create(:bulk_upload_error, bulk_upload:, col: "B", row: 2, category: nil) + + stub_const("BulkUploadErrorSummaryTableComponent::DISPLAY_THRESHOLD", 16) end it "only returns the setup errors" do @@ -117,6 +138,12 @@ RSpec.describe BulkUploadErrorSummaryTableComponent, type: :component do "1 error", ]) end + + it "renders intro with setup errors" do + result = render_inline(component) + + expect(result).to have_content("This summary shows important questions that have errors. See full error report for more details.") + end end end diff --git a/spec/mailers/bulk_upload_mailer_spec.rb b/spec/mailers/bulk_upload_mailer_spec.rb index cd2c4767d..20f78de4d 100644 --- a/spec/mailers/bulk_upload_mailer_spec.rb +++ b/spec/mailers/bulk_upload_mailer_spec.rb @@ -19,13 +19,6 @@ RSpec.describe BulkUploadMailer do create(:bulk_upload_error, bulk_upload:, col: "F", field: "field_5") end - let(:expected_errors) do - [ - "- What is the letting type? (Column A)", - "- Management group code (Column E)", - ] - end - it "sends correctly formed email" do expect(notify_client).to receive(:send_email).with( email_address: bulk_upload.user.email, @@ -35,8 +28,7 @@ RSpec.describe BulkUploadMailer do upload_timestamp: bulk_upload.created_at.to_fs(:govuk_date_and_time), lettings_or_sales: bulk_upload.log_type, year_combo: bulk_upload.year_combo, - errors_list: expected_errors.join("\n"), - bulk_upload_link: start_bulk_upload_lettings_logs_url, + bulk_upload_link: summary_bulk_upload_lettings_result_url(bulk_upload), }, ) @@ -81,34 +73,6 @@ RSpec.describe BulkUploadMailer do end end - context "when bulk upload has log which is not completed" do - before do - create(:lettings_log, :in_progress, bulk_upload:) - end - - describe "#send_bulk_upload_with_errors_mail" do - let(:error_description) do - "We created logs from your 2022/23 lettings data. There was a problem with 1 of the logs. Click the below link to fix these logs." - end - - it "sends correctly formed email" do - expect(notify_client).to receive(:send_email).with( - email_address: bulk_upload.user.email, - template_id: described_class::WITH_ERRORS_TEMPLATE_ID, - personalisation: { - title: "We found 1 log with errors", - filename: bulk_upload.filename, - upload_timestamp: bulk_upload.created_at.to_fs(:govuk_date_and_time), - error_description:, - summary_report_link: "http://localhost:3000/lettings-logs/bulk-upload-results/#{bulk_upload.id}/resume", - }, - ) - - mailer.send_bulk_upload_with_errors_mail(bulk_upload:) - end - end - end - describe "#send_correct_and_upload_again_mail" do context "when 2 columns with errors" do before do