CLDC-2134 Rework summary and email (#1451)

* remove column errors from bulk upload email

* summarise setup errors only if there are any

app/components/bulk_upload_error_summary_table_component.rb

@@ -12,7 +12,7 @@ class BulkUploadErrorSummaryTableComponent < ViewComponent::Base
   end
 
   def sorted_errors
-    @sorted_errors ||= bulk_upload
+    @sorted_errors ||= setup_errors.presence || bulk_upload
       .bulk_upload_errors
       .group(:col, :field, :error)
       .having("count(*) > ?", display_threshold)
@@ -26,6 +26,16 @@ class BulkUploadErrorSummaryTableComponent < ViewComponent::Base
 
 private
 
+  def setup_errors
+    @setup_errors ||= bulk_upload
+      .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
+
   def display_threshold
     DISPLAY_THRESHOLD
   end

app/mailers/bulk_upload_mailer.rb

@@ -33,19 +33,7 @@ class BulkUploadMailer < NotifyMailer
     )
   end
 
-  def columns_with_errors(bulk_upload:)
-    array = bulk_upload.columns_with_errors
-
-    if array.size > 3
-      "#{array.take(3).join(', ')} and more"
-    else
-      array.join(", ")
-    end
-  end
-
   def send_correct_and_upload_again_mail(bulk_upload:)
-    error_description = "We noticed that you have a lot of similar errors in column #{columns_with_errors(bulk_upload:)}. Please correct your data export and upload again."
-
     summary_report_link = if BulkUploadErrorSummaryTableComponent.new(bulk_upload:).errors?
                             summary_bulk_upload_lettings_result_url(bulk_upload)
                           else
@@ -60,7 +48,6 @@ class BulkUploadMailer < NotifyMailer
         upload_timestamp: bulk_upload.created_at.to_fs(:govuk_date_and_time),
         year_combo: bulk_upload.year_combo,
         lettings_or_sales: bulk_upload.log_type,
-        error_description:,
         summary_report_link:,
       },
     )

app/models/bulk_upload.rb

@@ -30,14 +30,6 @@ class BulkUpload < ApplicationRecord
               end
   end
 
-  def columns_with_errors
-    bulk_upload_errors
-      .select(:col)
-      .distinct(:col)
-      .pluck(:col)
-      .sort_by { |col| col.rjust(2, "0") }
-  end
-
   def general_needs?
     needstype == 1
   end

spec/components/bulk_upload_error_summary_table_component_spec.rb

@@ -9,87 +9,114 @@ RSpec.describe BulkUploadErrorSummaryTableComponent, type: :component do
     stub_const("BulkUploadErrorSummaryTableComponent::DISPLAY_THRESHOLD", 0)
   end
 
-  context "when no errors" do
-    it "does not renders any tables" do
-      result = render_inline(component)
-      expect(result).not_to have_selector("table")
+  describe "#sorted_errors" do
+    context "when no errors" do
+      it "does not renders any tables" do
+        result = render_inline(component)
+        expect(result).not_to have_selector("table")
+      end
     end
-  end
 
-  context "when below threshold" do
-    before do
-      stub_const("BulkUploadErrorSummaryTableComponent::DISPLAY_THRESHOLD", 16)
+    context "when below threshold" do
+      before do
+        stub_const("BulkUploadErrorSummaryTableComponent::DISPLAY_THRESHOLD", 16)
 
-      create(:bulk_upload_error, bulk_upload:, col: "A", row: 1)
-    end
+        create(:bulk_upload_error, bulk_upload:, col: "A", row: 1)
+      end
 
-    it "does not render tables" do
-      result = render_inline(component)
-      expect(result).to have_selector("table", count: 0)
+      it "does not render tables" do
+        result = render_inline(component)
+        expect(result).to have_selector("table", count: 0)
+      end
     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) }
+    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) }
 
-    it "renders table for each error" do
-      result = render_inline(component)
-      expect(result).to have_selector("table", count: 2)
-    end
+      it "renders table for each error" do
+        result = render_inline(component)
+        expect(result).to have_selector("table", count: 2)
+      end
 
-    it "renders by col order" do
-      result = render_inline(component)
-      order = result.css("table thead th:nth-of-type(2)").map(&:content)
-      expect(order).to eql(["Column A", "Column B"])
-    end
+      it "renders by col order" do
+        result = render_inline(component)
+        order = result.css("table thead th:nth-of-type(2)").map(&:content)
+        expect(order).to eql(["Column A", "Column B"])
+      end
 
-    it "render correct data" do
-      result = render_inline(component)
+      it "render correct data" do
+        result = render_inline(component)
 
-      table_1 = result.css("table").first.css("th, td").map(&:content)
+        table_1 = result.css("table").first.css("th, td").map(&:content)
 
-      expect(table_1).to eql([
-        bulk_upload.prefix_namespace::RowParser.question_for_field(error_1.field.to_sym).to_s,
-        "Column A",
-        error_1.error,
-        "1 error",
-      ])
+        expect(table_1).to eql([
+          bulk_upload.prefix_namespace::RowParser.question_for_field(error_1.field.to_sym).to_s,
+          "Column A",
+          error_1.error,
+          "1 error",
+        ])
 
-      table_2 = result.css("table")[1].css("th, td").map(&:content)
+        table_2 = result.css("table")[1].css("th, td").map(&:content)
 
-      expect(table_2).to eql([
-        bulk_upload.prefix_namespace::RowParser.question_for_field(error_2.field.to_sym).to_s,
-        "Column B",
-        error_2.error,
-        "1 error",
-      ])
+        expect(table_2).to eql([
+          bulk_upload.prefix_namespace::RowParser.question_for_field(error_2.field.to_sym).to_s,
+          "Column B",
+          error_2.error,
+          "1 error",
+        ])
+      end
     end
-  end
 
-  context "when there are 2 grouped errors" do
-    let!(:error_1) { create(:bulk_upload_error, bulk_upload:, col: "A", row: 1, field: "field_1") }
+    context "when there are 2 grouped errors" do
+      let!(:error_1) { create(:bulk_upload_error, bulk_upload:, col: "A", row: 1, field: "field_1") }
 
-    before do
-      create(:bulk_upload_error, bulk_upload:, col: "A", row: 2, field: "field_1")
-    end
+      before do
+        create(:bulk_upload_error, bulk_upload:, col: "A", row: 2, field: "field_1")
+      end
+
+      it "renders 1 table combining the errors" do
+        result = render_inline(component)
+        expect(result).to have_selector("table", count: 1)
+      end
 
-    it "renders 1 table combining the errors" do
-      result = render_inline(component)
-      expect(result).to have_selector("table", count: 1)
+      it "render correct data" do
+        result = render_inline(component)
+
+        table_1 = result.css("table").css("th, td").map(&:content)
+
+        expect(table_1).to eql([
+          bulk_upload.prefix_namespace::RowParser.question_for_field(error_1.field.to_sym).to_s,
+          "Column A",
+          error_1.error,
+          "2 errors",
+        ])
+      end
     end
 
-    it "render correct data" do
-      result = render_inline(component)
+    context "when mix of setup and other errors" do
+      let!(:error_1) { create(:bulk_upload_error, bulk_upload:, col: "A", row: 1, category: "setup") }
 
-      table_1 = result.css("table").css("th, td").map(&:content)
+      before do
+        create(:bulk_upload_error, bulk_upload:, col: "B", row: 2, category: nil)
+      end
+
+      it "only returns the setup errors" do
+        result = render_inline(component)
+
+        tables = result.css("table")
 
-      expect(table_1).to eql([
-        bulk_upload.prefix_namespace::RowParser.question_for_field(error_1.field.to_sym).to_s,
-        "Column A",
-        error_1.error,
-        "2 errors",
-      ])
+        expect(tables.size).to be(1)
+
+        table = result.css("table").css("th, td").map(&:content)
+
+        expect(table).to eql([
+          bulk_upload.prefix_namespace::RowParser.question_for_field(error_1.field.to_sym).to_s,
+          "Column A",
+          error_1.error,
+          "1 error",
+        ])
+      end
     end
   end
 

spec/mailers/bulk_upload_mailer_spec.rb

@@ -116,7 +116,7 @@ RSpec.describe BulkUploadMailer do
         create(:bulk_upload_error, bulk_upload:, col: "B")
       end
 
-      it "sends correctly formed email with A, B" do
+      it "sends correctly formed email" do
         expect(notify_client).to receive(:send_email).with(
           email_address: user.email,
           template_id: described_class::BULK_UPLOAD_FAILED_CSV_ERRORS_TEMPLATE_ID,
@@ -125,7 +125,6 @@ RSpec.describe BulkUploadMailer do
             upload_timestamp: bulk_upload.created_at.to_fs(:govuk_date_and_time),
             year_combo: bulk_upload.year_combo,
             lettings_or_sales: bulk_upload.log_type,
-            error_description: "We noticed that you have a lot of similar errors in column A, B. Please correct your data export and upload again.",
             summary_report_link: "http://localhost:3000/lettings-logs/bulk-upload-results/#{bulk_upload.id}",
           },
         )
@@ -133,33 +132,5 @@ RSpec.describe BulkUploadMailer do
         mailer.send_correct_and_upload_again_mail(bulk_upload:)
       end
     end
-
-    context "when 4 columns with errors" do
-      before do
-        stub_const("BulkUploadErrorSummaryTableComponent::DISPLAY_THRESHOLD", 0)
-
-        create(:bulk_upload_error, bulk_upload:, col: "A")
-        create(:bulk_upload_error, bulk_upload:, col: "B")
-        create(:bulk_upload_error, bulk_upload:, col: "C")
-        create(:bulk_upload_error, bulk_upload:, col: "D")
-      end
-
-      it "sends correctly formed email with A, B, C and more" do
-        expect(notify_client).to receive(:send_email).with(
-          email_address: user.email,
-          template_id: described_class::BULK_UPLOAD_FAILED_CSV_ERRORS_TEMPLATE_ID,
-          personalisation: {
-            filename: bulk_upload.filename,
-            upload_timestamp: bulk_upload.created_at.to_fs(:govuk_date_and_time),
-            year_combo: bulk_upload.year_combo,
-            lettings_or_sales: bulk_upload.log_type,
-            error_description: "We noticed that you have a lot of similar errors in column A, B, C and more. Please correct your data export and upload again.",
-            summary_report_link: "http://localhost:3000/lettings-logs/bulk-upload-results/#{bulk_upload.id}/summary",
-          },
-        )
-
-        mailer.send_correct_and_upload_again_mail(bulk_upload:)
-      end
-    end
   end
 end