diff --git a/app/components/bulk_upload_error_summary_table_component.html.erb b/app/components/bulk_upload_error_summary_table_component.html.erb index 9cf146dc0..2f9643271 100644 --- a/app/components/bulk_upload_error_summary_table_component.html.erb +++ b/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 %> diff --git a/app/mailers/bulk_upload_mailer.rb b/app/mailers/bulk_upload_mailer.rb index cb1fac03c..144acb3e1 100644 --- a/app/mailers/bulk_upload_mailer.rb +++ b/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( diff --git a/app/views/bulk_upload_lettings_results/show.html.erb b/app/views/bulk_upload_lettings_results/show.html.erb index 9a8ccddec..30e0f6890 100644 --- a/app/views/bulk_upload_lettings_results/show.html.erb +++ b/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 %> -
Bulk upload for lettings (<%= @bulk_upload.year_combo %>) diff --git a/app/views/bulk_upload_lettings_results/summary.html.erb b/app/views/bulk_upload_lettings_results/summary.html.erb index 04d8bdb92..2ea2214b3 100644 --- a/app/views/bulk_upload_lettings_results/summary.html.erb +++ b/app/views/bulk_upload_lettings_results/summary.html.erb @@ -1,22 +1,34 @@
Bulk upload for lettings (<%= @bulk_upload.year_combo %>) -

Correct data export and reupload

+

Fix <%= pluralize(@bulk_upload.bulk_upload_errors.count, "error") %> and upload file again

- 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. +

+ +

+ Filename: <%= @bulk_upload.filename %>

-<%= render BulkUploadErrorSummaryTableComponent.new(bulk_upload: @bulk_upload) %> -
-
-

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

-
+ <%= govuk_tabs(title: "Error reports") do |c| %> + <% c.with_tab(label: "Summary") do %> +

+ 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 %> + + <% 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 %>
<%= govuk_button_link_to "Upload your file again", start_bulk_upload_lettings_logs_path %> 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 7fb7b65ed..4c307515c 100644 --- a/spec/components/bulk_upload_error_summary_table_component_spec.rb +++ b/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 diff --git a/spec/mailers/bulk_upload_mailer_spec.rb b/spec/mailers/bulk_upload_mailer_spec.rb index a0d9a3cd1..9f59de365 100644 --- a/spec/mailers/bulk_upload_mailer_spec.rb +++ b/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