From c7bf7a037d451bda8f8ec8e10f313c3f5b19b30d Mon Sep 17 00:00:00 2001 From: Manny Dinssa <44172848+Dinssa@users.noreply.github.com> Date: Thu, 8 Aug 2024 14:54:39 +0100 Subject: [PATCH] CLDC-2588-Separate soft and hard validation errors in bulk upload (#2545) * Separate soft and hard validation errors in bulk upload --- .../bulk_upload_error_row_component.html.erb | 50 ++++++++++++++++--- app/frontend/styles/application.scss | 14 ++++++ .../summary.html.erb | 2 +- .../summary.html.erb | 2 +- .../bulk_upload_error_row_component_spec.rb | 49 ++++++++++++++++++ .../show.html.erb_spec.rb | 2 +- .../summary.html.erb_spec.rb | 2 +- .../show.html.erb_spec.rb | 3 +- .../summary.html.erb_spec.rb | 2 +- 9 files changed, 112 insertions(+), 14 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..4e9ff1390 100644 --- a/app/components/bulk_upload_error_row_component.html.erb +++ b/app/components/bulk_upload_error_row_component.html.erb @@ -1,13 +1,16 @@
<% if lettings? %> -

Row <%= row %> <%= tenant_code_html %> <%= property_ref_html %>

+

Row <%= row %> <%= tenant_code_html %> <%= property_ref_html %>

<% else %> -

Row <%= row %> <%= purchaser_code_html %>

+

Row <%= row %> <%= purchaser_code_html %>

<% end %>
+ <% potential_errors, critical_errors = bulk_upload_errors.partition { |error| error.category == "soft_validation" } %> +

Critical errors

+

These errors must be fixed to complete your logs.

<%= govuk_table do |table| %> <%= table.with_head do |head| %> <% head.with_row do |row| %> @@ -18,16 +21,49 @@ <% end %> <%= table.with_body do |body| %> - <% bulk_upload_errors.each do |error| %> + <% critical_errors.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(text: error.error, html_attributes: { class: "govuk-!-font-weight-bold" }) %> + <% row.with_cell(text: error.field.humanize) %> <% end %> <% end %> <% end %> <% end %> <% end %> + + <% if potential_errors.any? %> +

Potential errors

+

The following groups of cells might have conflicting data. Check the answers and fix any incorrect data.

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.

+ <%= 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.zero? %> + <% 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(text: error_message, rowspan: errors.size, html_attributes: { class: "govuk-!-font-weight-bold grouped-multirow-cell" }) %> + <% end %> + <% row.with_cell(text: error.field.humanize) %> + <% end %> + <% end %> + <% end %> + <% end %> + <% end %> + <% end %>
diff --git a/app/frontend/styles/application.scss b/app/frontend/styles/application.scss index a81214894..62d4e932e 100644 --- a/app/frontend/styles/application.scss +++ b/app/frontend/styles/application.scss @@ -81,3 +81,17 @@ $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, +.grouped-rows .grouped-multirow-cell { + 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 @@

Fix <%= pluralize(@bulk_upload.bulk_upload_errors.count, "error") %> and upload file 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 <%= 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.

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 @@

Fix <%= pluralize(@bulk_upload.bulk_upload_errors.count, "error") %> and upload file 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 <%= 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.

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 diff --git a/spec/views/bulk_upload_lettings_results/show.html.erb_spec.rb b/spec/views/bulk_upload_lettings_results/show.html.erb_spec.rb index 3dea3ce92..637dfa3a9 100644 --- a/spec/views/bulk_upload_lettings_results/show.html.erb_spec.rb +++ b/spec/views/bulk_upload_lettings_results/show.html.erb_spec.rb @@ -33,7 +33,7 @@ RSpec.describe "bulk_upload_lettings_results/show.html.erb" do fragment = Capybara::Node::Simple.new(rendered) - expect(fragment.find_css("table tbody th").map(&:inner_text)).to eql(%w[Z100 AA100]) + expect(fragment.find_css("table tbody td").map(&:inner_text).values_at(0, 4)).to eql(%w[Z100 AA100]) end end end diff --git a/spec/views/bulk_upload_lettings_results/summary.html.erb_spec.rb b/spec/views/bulk_upload_lettings_results/summary.html.erb_spec.rb index 05e2dce85..ac0c1f82d 100644 --- a/spec/views/bulk_upload_lettings_results/summary.html.erb_spec.rb +++ b/spec/views/bulk_upload_lettings_results/summary.html.erb_spec.rb @@ -33,7 +33,7 @@ RSpec.describe "bulk_upload_lettings_results/summary.html.erb" do fragment = Capybara::Node::Simple.new(rendered) - expect(fragment.find_css("table tbody th").map(&:inner_text)).to eql(%w[Z100 AA100]) + expect(fragment.find_css("table tbody td").map(&:inner_text).values_at(0, 4)).to eql(%w[Z100 AA100]) end end end diff --git a/spec/views/bulk_upload_sales_results/show.html.erb_spec.rb b/spec/views/bulk_upload_sales_results/show.html.erb_spec.rb index 785d830b3..dc6751dc8 100644 --- a/spec/views/bulk_upload_sales_results/show.html.erb_spec.rb +++ b/spec/views/bulk_upload_sales_results/show.html.erb_spec.rb @@ -32,8 +32,7 @@ RSpec.describe "bulk_upload_sales_results/show.html.erb" do render fragment = Capybara::Node::Simple.new(rendered) - - expect(fragment.find_css("table tbody th").map(&:inner_text)).to eql(%w[Z100 AA100]) + expect(fragment.find_css("table tbody td").map(&:inner_text).values_at(0, 4)).to eql(%w[Z100 AA100]) end end end diff --git a/spec/views/bulk_upload_sales_results/summary.html.erb_spec.rb b/spec/views/bulk_upload_sales_results/summary.html.erb_spec.rb index 6c00e5adf..b3d9aa006 100644 --- a/spec/views/bulk_upload_sales_results/summary.html.erb_spec.rb +++ b/spec/views/bulk_upload_sales_results/summary.html.erb_spec.rb @@ -33,7 +33,7 @@ RSpec.describe "bulk_upload_sales_results/summary.html.erb" do fragment = Capybara::Node::Simple.new(rendered) - expect(fragment.find_css("table tbody th").map(&:inner_text)).to eql(%w[Z100 AA100]) + expect(fragment.find_css("table tbody td").map(&:inner_text).values_at(0, 4)).to eql(%w[Z100 AA100]) end end end