CLDC-2170 Bulk upload summary tweaks (#1522)

* show summary when on error threshold

* remove bulk upload mailer template

- this template is no longer used

* setup errors do not consider threshold

- return approprate intro test depending if there are setup errors or
  not

app/components/bulk_upload_error_summary_table_component.html.erb

@@ -1,3 +1,7 @@
+<p class="govuk-body">
+  <%= intro %>
+</p>
+
 <% sorted_errors.each do |error| %>
   <%= govuk_table do |table| %>
     <% table.head do |head| %>

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

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,23 +72,10 @@ class BulkUploadMailer < NotifyMailer
   end
 
   def send_bulk_upload_failed_file_setup_error_mail(bulk_upload:)
-    bulk_upload_link = if bulk_upload.lettings?
+    bulk_upload_link = if BulkUploadErrorSummaryTableComponent.new(bulk_upload:).errors?
-                         start_bulk_upload_lettings_logs_url
+                         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(
@@ -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

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:)

app/views/bulk_upload_lettings_results/summary.html.erb

@@ -16,10 +16,6 @@
 <div class="govuk-grid-row">
   <%= govuk_tabs(title: "Error reports") do |c| %>
     <% c.with_tab(label: "Summary") do %>
-      <p class="govuk-body">
-        This summary shows questions that have at least <%= BulkUploadErrorSummaryTableComponent::DISPLAY_THRESHOLD %> errors or more. See full error report for more details.
-      </p>
-
       <%= render BulkUploadErrorSummaryTableComponent.new(bulk_upload: @bulk_upload) %>
     <% end %>
 

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
 

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: summary_bulk_upload_lettings_result_url(bulk_upload),
-          bulk_upload_link: start_bulk_upload_lettings_logs_url,
         },
       )
 
@@ -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