From c21febe7e725d45933fd2f1091111fb66aeba662 Mon Sep 17 00:00:00 2001
From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com>
Date: Fri, 2 Aug 2024 16:41:47 +0100
Subject: [PATCH] Separate soft and hard validation errors in bulk upload

---
 .../bulk_upload_error_row_component.html.erb  | 46 +++++++++++++++--
 .../bulk_upload_error_row_component.rb        |  6 +--
 app/frontend/styles/application.scss          | 13 +++++
 .../summary.html.erb                          |  2 +-
 .../summary.html.erb                          |  2 +-
 .../bulk_upload_error_row_component_spec.rb   | 49 +++++++++++++++++++
 6 files changed, 108 insertions(+), 10 deletions(-)

diff --git a/app/components/bulk_upload_error_row_component.html.erb b/app/components/bulk_upload_error_row_component.html.erb
index 907c73298..e4f77b975 100644
--- a/app/components/bulk_upload_error_row_component.html.erb
+++ b/app/components/bulk_upload_error_row_component.html.erb
@@ -8,6 +8,8 @@
   </div>
 
   <div class="govuk-summary-card__content">
+    <h2 class="govuk-heading-m">Critical errors</h2>
+    <p class="govuk-body">These errors must be fixed to complete your logs</p>
     <%= govuk_table do |table| %>
       <%= table.with_head do |head| %>
         <% head.with_row do |row| %>
@@ -18,16 +20,50 @@
         <% end %>
 
         <%= table.with_body do |body| %>
-          <% bulk_upload_errors.each do |error| %>
+          <% bulk_upload_errors.select { |error| error.category != "soft_validation" }.each do |error| %>
             <% body.with_row do |row| %>
-              <% row.with_cell(header: true, text: error.cell) %>
-              <% row.with_cell(text: question_for_field(error.field)) %>
-              <% row.with_cell(text: error.error) %>
-              <% row.with_cell(text: error.field.humanize) %>
+                <% row.with_cell(text: error.cell) %>
+                <% row.with_cell(text: question_for_field(error.field)) %>
+                <% row.with_cell(header: true, text: error.error) %>
+                <% row.with_cell(text: error.field.humanize) %>
             <% end %>
           <% end %>
         <% end %>
       <% end %>
     <% end %>
+
+    <% potential_errors = bulk_upload_errors.select { |error| error.category == "soft_validation" } %>
+    <% if potential_errors.any? %>
+      <h2 class="govuk-heading-m">Potential errors</h2>
+      <p class="govuk-body">The following groups of cells might have conflicting data. Check the answers and fix any incorrect data.<br><br>If the answers are correct, fix the critical errors and reupload the file. You'll need to confirm that the following data is correct when the file only contains potential errors.</p>
+        <%= govuk_table do |table| %>
+          <%= table.with_head do |head| %>
+            <% head.with_row do |row| %>
+              <% row.with_cell(header: true, text: "Cell") %>
+              <% row.with_cell(header: true, text: "Question") %>
+              <% row.with_cell(header: true, text: "Potential error") %>
+              <% row.with_cell(header: true, text: "Specification") %>
+            <% end %>
+          <% end %>
+
+          <%= table.with_body do |body| %>
+            <% potential_errors.group_by(&:error).each do |error_message, errors| %>
+              <% errors.each_with_index do |error, index| %>
+                <% row_class = "grouped-rows" %>
+                <% row_class += " first-row" if index == 0 %>
+                <% row_class += " last-row" if index == errors.size - 1 %>
+                <% body.with_row(html_attributes: { class: row_class }) do |row| %>
+                  <% row.with_cell(text: error.cell) %>
+                  <% row.with_cell(text: question_for_field(error.field)) %>
+                  <% if index == 0 %>
+                    <% row.with_cell(header: true, text: error_message, rowspan: errors.size) %>
+                  <% end %>
+                  <% row.with_cell(text: error.field.humanize) %>
+                <% end %>
+              <% end %>
+            <% end %>
+          <% end %>
+        <% end %>
+    <% end %>
   </div>
 </div>
diff --git a/app/components/bulk_upload_error_row_component.rb b/app/components/bulk_upload_error_row_component.rb
index 887eef10c..e5ae9087f 100644
--- a/app/components/bulk_upload_error_row_component.rb
+++ b/app/components/bulk_upload_error_row_component.rb
@@ -18,7 +18,7 @@ class BulkUploadErrorRowComponent < ViewComponent::Base
   def tenant_code_html
     return if tenant_code.blank?
 
-    content_tag :span, class: "govuk-!-margin-left-3" do
+    content_tag :span, class: "govuk-!-margin-left-3", style: "font-weight: 400;" do
       "Tenant code: #{tenant_code}"
     end
   end
@@ -30,7 +30,7 @@ class BulkUploadErrorRowComponent < ViewComponent::Base
   def purchaser_code_html
     return if purchaser_code.blank?
 
-    content_tag :span, class: "govuk-!-margin-left-3" do
+    content_tag :span, class: "govuk-!-margin-left-3", style: "font-weight: 400;" do
       "Purchaser code: #{purchaser_code}"
     end
   end
@@ -42,7 +42,7 @@ class BulkUploadErrorRowComponent < ViewComponent::Base
   def property_ref_html
     return if property_ref.blank?
 
-    content_tag :span, class: "govuk-!-margin-left-3" do
+    content_tag :span, class: "govuk-!-margin-left-3", style: "font-weight: 400;" do
       "Property reference: #{property_ref}"
     end
   end
diff --git a/app/frontend/styles/application.scss b/app/frontend/styles/application.scss
index a81214894..ee4366aa3 100644
--- a/app/frontend/styles/application.scss
+++ b/app/frontend/styles/application.scss
@@ -81,3 +81,16 @@ $govuk-breakpoints: (
 .govuk-footer {
   border-top: govuk-spacing(2) solid $govuk-brand-colour;
 }
+
+.grouped-rows td {
+  border-top: none;
+  border-bottom: none;
+}
+
+.grouped-rows.first-row td {
+  border-top: 1px solid #b1b4b6;
+}
+
+.grouped-rows.last-row td {
+  border-bottom: 1px solid #b1b4b6;
+}
diff --git a/app/views/bulk_upload_lettings_results/summary.html.erb b/app/views/bulk_upload_lettings_results/summary.html.erb
index d6a1127af..8c36af632 100644
--- a/app/views/bulk_upload_lettings_results/summary.html.erb
+++ b/app/views/bulk_upload_lettings_results/summary.html.erb
@@ -4,7 +4,7 @@
     <h1 class="govuk-heading-l">Fix <%= pluralize(@bulk_upload.bulk_upload_errors.count, "error") %> and upload file again</h1>
 
     <p class="govuk-body-l">
-      We could not create logs from your bulk upload. Below is a list of everything that you need to fix your spreadsheet. You can download the <%= govuk_link_to "specification", Forms::BulkUploadLettings::PrepareYourFile.new(year: @bulk_upload.year).specification_path, target: "_blank" %> to help you fix the cells in your CSV file.
+      We could not create logs from your bulk upload because of the following errors. Download the <%= govuk_link_to "specification", Forms::BulkUploadLettings::PrepareYourFile.new(year: @bulk_upload.year).specification_path, target: "_blank" %> to help you fix the cells in your CSV file.
     </p>
 
     <p class="govuk-body-l">
diff --git a/app/views/bulk_upload_sales_results/summary.html.erb b/app/views/bulk_upload_sales_results/summary.html.erb
index 8fd6c22ed..af504acbd 100644
--- a/app/views/bulk_upload_sales_results/summary.html.erb
+++ b/app/views/bulk_upload_sales_results/summary.html.erb
@@ -4,7 +4,7 @@
     <h1 class="govuk-heading-l">Fix <%= pluralize(@bulk_upload.bulk_upload_errors.count, "error") %> and upload file again</h1>
 
     <p class="govuk-body-l">
-      We could not create logs from your bulk upload. Below is a list of everything that you need to fix your spreadsheet. You can download the <%= govuk_link_to "specification", Forms::BulkUploadSales::PrepareYourFile.new(year: @bulk_upload.year).specification_path, target: "_blank" %> to help you fix the cells in your CSV file.
+      We could not create logs from your bulk upload because of the following errors. Download the <%= govuk_link_to "specification", Forms::BulkUploadSales::PrepareYourFile.new(year: @bulk_upload.year).specification_path, target: "_blank" %> to help you fix the cells in your CSV file.
     </p>
 
     <p class="govuk-body-l">
diff --git a/spec/components/bulk_upload_error_row_component_spec.rb b/spec/components/bulk_upload_error_row_component_spec.rb
index 01af041cb..b593a0048 100644
--- a/spec/components/bulk_upload_error_row_component_spec.rb
+++ b/spec/components/bulk_upload_error_row_component_spec.rb
@@ -108,4 +108,53 @@ RSpec.describe BulkUploadErrorRowComponent, type: :component do
       expect(result).to have_content(expected)
     end
   end
+
+  context "when there are potential errors" do
+    let(:row) { rand(9_999) }
+    let(:tenant_code) { SecureRandom.hex(4) }
+    let(:property_ref) { SecureRandom.hex(4) }
+    let(:purchaser_code) { nil }
+    let(:category) { "soft_validation" }
+    let(:field_46) { 40 }
+    let(:field_50) { 5 }
+    let(:error) { "You told us this person is aged 40 years and retired." }
+    let(:bulk_upload) { create(:bulk_upload, :lettings) }
+    let(:bulk_upload_errors) do
+      [
+        FactoryBot.build(
+          :bulk_upload_error,
+          bulk_upload:,
+          row:,
+          tenant_code:,
+          property_ref:,
+          purchaser_code:,
+          field: :field_46,
+          error:,
+          category:,
+        ),
+        FactoryBot.build(
+          :bulk_upload_error,
+          bulk_upload:,
+          row:,
+          tenant_code:,
+          property_ref:,
+          purchaser_code:,
+          field: :field_50,
+          error:,
+          category:,
+        ),
+      ]
+    end
+
+    it "renders the potential errors section" do
+      result = render_inline(described_class.new(bulk_upload_errors:))
+      expect(result).to have_content("Potential errors")
+    end
+
+    it "renders the potential error message" do
+      expected = error
+      result = render_inline(described_class.new(bulk_upload_errors:))
+      expect(result).to have_content(expected, count: 1)
+    end
+  end
 end