CLDC-2134 Bulk upload tabs (#1436)

* tweak bulk upload email copy formatting

* change bulk upload summary error report

- this is now tabbed with the full report
- also summary report has now changed from single table to multiple
  tables

* bulk upload cannot navigate to summary report

- as when sent to view full report the user does not have enough
  errors that need summarising

app/components/bulk_upload_error_summary_table_component.html.erb

@@ -1,24 +1,16 @@
-<%= govuk_table do |table| %>
-  <% table.caption(size: "m", text: bulk_upload.filename) %>
-
-  <% table.head do |head| %>
-    <% head.row do |row| %>
-      <% row.cell(text: "Column", header: true) %>
-      <% row.cell(text: "Number of rows", header: true) %>
-      <% row.cell(text: "Question", header: true) %>
-      <% row.cell(text: "Error", header: true) %>
-      <% row.cell(text: "Specification", header: true) %>
-    <% end %>
-  <% end %>
+<% sorted_errors.each do |error| %>
+  <%= govuk_table do |table| %>
+    <% table.head do |head| %>
+      <% head.row do |row| %>
+        <% row.cell(text: question_for_field(error[0][1].to_sym), header: true) %>
+        <% row.cell(text: "Column #{error[0][0]}", header: true, numeric: true) %>
+      <% end %>
 
-  <% table.body do |body| %>
-    <% sorted_errors.each do |error| %>
-      <% body.row do |row| %>
-        <% row.cell(text: error[0][0]) %>
-        <% row.cell(text: error[1]) %>
-        <% row.cell(text: question_for_field(error[0][1].to_sym)) %>
-        <% row.cell(text: error[0][2]) %>
-        <% row.cell(text: error[0][1]) %>
+      <% table.body do |body| %>
+        <% body.row do |row| %>
+          <% row.cell(text: error[0][2]) %>
+          <% row.cell(text: pluralize(error[1], "error"), numeric: true) %>
+        <% end %>
       <% end %>
     <% end %>
   <% end %>

app/mailers/bulk_upload_mailer.rb

@@ -83,7 +83,7 @@ class BulkUploadMailer < NotifyMailer
       .keys
       .sort_by { |_col, field| field }
       .map do |col, field|
-        "- Column #{col} (#{row_parser_class.question_for_field(field.to_sym)})"
+        "- #{row_parser_class.question_for_field(field.to_sym)} (Column #{col})"
       end
 
     send_email(

app/views/bulk_upload_lettings_results/show.html.erb

@@ -1,9 +1,3 @@
-<% if BulkUploadErrorSummaryTableComponent.new(bulk_upload: @bulk_upload).errors? %>
-  <% content_for :before_content do %>
-    <%= govuk_back_link(text: "Back", href: summary_bulk_upload_lettings_result_path(@bulk_upload)) %>
-  <% end %>
-<% end %>
-
 <div class="govuk-grid-row">
   <div class="govuk-grid-column-two-thirds">
     <span class="govuk-caption-l">Bulk upload for lettings (<%= @bulk_upload.year_combo %>)</span>

app/views/bulk_upload_lettings_results/summary.html.erb

@@ -1,22 +1,34 @@
 <div class="govuk-grid-row">
   <div class="govuk-grid-column-two-thirds">
     <span class="govuk-caption-l">Bulk upload for lettings (<%= @bulk_upload.year_combo %>)</span>
-    <h1 class="govuk-heading-l">Correct data export and reupload</h1>
+    <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 noticed that you have a lot of similar errors for some questions. You can download the specification which we reference below to understand how to correct the data. Once you have fixed these errors you can upload again.
+      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 specification to help you fix the cells in your CSV file.
+    </p>
+
+    <p class="govuk-body-l">
+      Filename: <%= @bulk_upload.filename %>
     </p>
   </div>
 </div>
 
-<%= render BulkUploadErrorSummaryTableComponent.new(bulk_upload: @bulk_upload) %>
-
 <div class="govuk-grid-row">
-  <div class="govuk-grid-column-two-thirds">
-    <p class="govuk-body">
-      You also have other errors in your file which you can either fix them in the CSV file or you can reupload and fix on CORE. <%= govuk_link_to "View the full report", bulk_upload_lettings_result_path(@bulk_upload) %>
-    </p>
-  </div>
+  <%= 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 %>
+
+    <% c.with_tab(label: "Full error report") do %>
+      <% @bulk_upload.bulk_upload_errors.group_by(&:row).each do |_row, errors_for_row| %>
+        <%= render BulkUploadErrorRowComponent.new(bulk_upload_errors: errors_for_row) %>
+      <% end %>
+    <% end %>
+  <% end %>
 </div>
 
 <%= govuk_button_link_to "Upload your file again", start_bulk_upload_lettings_logs_path %>

spec/components/bulk_upload_error_summary_table_component_spec.rb

@@ -10,9 +10,9 @@ RSpec.describe BulkUploadErrorSummaryTableComponent, type: :component do
   end
 
   context "when no errors" do
-    it "does not renders any rows" do
+    it "does not renders any tables" do
       result = render_inline(component)
-      expect(result).not_to have_selector("tbody tr")
+      expect(result).not_to have_selector("table")
     end
   end
 
@@ -23,9 +23,9 @@ RSpec.describe BulkUploadErrorSummaryTableComponent, type: :component do
       create(:bulk_upload_error, bulk_upload:, col: "A", row: 1)
     end
 
-    it "does not render rows" do
+    it "does not render tables" do
       result = render_inline(component)
-      expect(result).to have_selector("tbody tr", count: 0)
+      expect(result).to have_selector("table", count: 0)
     end
   end
 
@@ -33,38 +33,36 @@ RSpec.describe BulkUploadErrorSummaryTableComponent, type: :component 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 rows for each error" do
+    it "renders table for each error" do
       result = render_inline(component)
-      expect(result).to have_selector("tbody tr", count: 2)
+      expect(result).to have_selector("table", count: 2)
     end
 
-    it "renders rows by col order" do
+    it "renders by col order" do
       result = render_inline(component)
-      order = result.css("tbody tr td:nth-of-type(1)").map(&:content)
-      expect(order).to eql(%w[A B])
+      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)
 
-      row_1 = result.css("tbody tr:nth-of-type(1) td").map(&:content)
+      table_1 = result.css("table").first.css("th, td").map(&:content)
 
-      expect(row_1).to eql([
-        "A",
-        "1",
-        bulk_upload.prefix_namespace::RowParser.question_for_field(error_1.field.to_sym),
+      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,
-        error_1.field,
+        "1 error",
       ])
 
-      row_2 = result.css("tbody tr:nth-of-type(2) td").map(&:content)
+      table_2 = result.css("table")[1].css("th, td").map(&:content)
 
-      expect(row_2).to eql([
-        "B",
-        "1",
-        bulk_upload.prefix_namespace::RowParser.question_for_field(error_2.field.to_sym),
+      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,
-        error_2.field,
+        "1 error",
       ])
     end
   end
@@ -76,22 +74,21 @@ RSpec.describe BulkUploadErrorSummaryTableComponent, type: :component do
       create(:bulk_upload_error, bulk_upload:, col: "A", row: 2, field: "field_1")
     end
 
-    it "renders 1 row combining the errors" do
+    it "renders 1 table combining the errors" do
       result = render_inline(component)
-      expect(result).to have_selector("tbody tr", count: 1)
+      expect(result).to have_selector("table", count: 1)
     end
 
     it "render correct data" do
       result = render_inline(component)
 
-      row_1 = result.css("tbody tr:nth-of-type(1) td").map(&:content)
+      table_1 = result.css("table").css("th, td").map(&:content)
 
-      expect(row_1).to eql([
-        "A",
-        "2",
-        bulk_upload.prefix_namespace::RowParser.question_for_field(error_1.field.to_sym),
+      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,
-        error_1.field,
+        "2 errors",
       ])
     end
   end

spec/mailers/bulk_upload_mailer_spec.rb

@@ -21,8 +21,8 @@ RSpec.describe BulkUploadMailer do
 
     let(:expected_errors) do
       [
-        "- Column A (What is the letting type?)",
-        "- Column E (Management group code)",
+        "- What is the letting type? (Column A)",
+        "- Management group code (Column E)",
       ]
     end