From 81bdd1475c21918c67d068e357a25149b6849fd3 Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Mon, 30 Jan 2023 16:35:23 +0000 Subject: [PATCH] CLDC-1780 Bulk upload summary (#1242) * add placeholder for bulk upload error summary * persist column for bulk upload errors * replace bulk upload summary table placeholder --- ...oad_error_summary_table_component.html.erb | 25 ++++++ ...lk_upload_error_summary_table_component.rb | 17 ++++ ...bulk_upload_lettings_results_controller.rb | 4 + app/models/bulk_upload.rb | 3 +- .../show.html.erb | 6 +- .../summary.html.erb | 22 +++++ config/routes.rb | 1 + ...load_error_summary_table_component_spec.rb | 81 +++++++++++++++++++ spec/factories/bulk_upload_error.rb | 3 +- ...upload_lettings_results_controller_spec.rb | 17 +++- 10 files changed, 175 insertions(+), 4 deletions(-) create mode 100644 app/components/bulk_upload_error_summary_table_component.html.erb create mode 100644 app/components/bulk_upload_error_summary_table_component.rb create mode 100644 app/views/bulk_upload_lettings_results/summary.html.erb create mode 100644 spec/components/bulk_upload_error_summary_table_component_spec.rb diff --git a/app/components/bulk_upload_error_summary_table_component.html.erb b/app/components/bulk_upload_error_summary_table_component.html.erb new file mode 100644 index 000000000..f1749ee92 --- /dev/null +++ b/app/components/bulk_upload_error_summary_table_component.html.erb @@ -0,0 +1,25 @@ +<%= 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 %> + + <% 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: BulkUpload::Lettings::Validator.question_for_field(error[0][1].to_sym)) %> + <% row.cell(text: error[0][2]) %> + <% row.cell(text: error[0][1]) %> + <% end %> + <% end %> + <% end %> +<% end %> diff --git a/app/components/bulk_upload_error_summary_table_component.rb b/app/components/bulk_upload_error_summary_table_component.rb new file mode 100644 index 000000000..ac236af4d --- /dev/null +++ b/app/components/bulk_upload_error_summary_table_component.rb @@ -0,0 +1,17 @@ +class BulkUploadErrorSummaryTableComponent < ViewComponent::Base + attr_reader :bulk_upload + + def initialize(bulk_upload:) + @bulk_upload = bulk_upload + + super + end + + def sorted_errors + @sorted_errors ||= bulk_upload + .bulk_upload_errors + .group(:col, :field, :error) + .count + .sort_by { |el| el[0][0].rjust(3, "0") } + end +end diff --git a/app/controllers/bulk_upload_lettings_results_controller.rb b/app/controllers/bulk_upload_lettings_results_controller.rb index 1c5342cd8..6f92c0375 100644 --- a/app/controllers/bulk_upload_lettings_results_controller.rb +++ b/app/controllers/bulk_upload_lettings_results_controller.rb @@ -19,6 +19,10 @@ class BulkUploadLettingsResultsController < ApplicationController end end + def summary + @bulk_upload = current_user.bulk_uploads.lettings.find(params[:id]) + end + private def reset_logs_filters diff --git a/app/models/bulk_upload.rb b/app/models/bulk_upload.rb index 7933ac204..3d0ef93f0 100644 --- a/app/models/bulk_upload.rb +++ b/app/models/bulk_upload.rb @@ -3,7 +3,8 @@ class BulkUpload < ApplicationRecord belongs_to :user - has_many :bulk_upload_errors + has_many :bulk_upload_errors, dependent: :destroy + has_many :lettings_logs has_many :sales_logs diff --git a/app/views/bulk_upload_lettings_results/show.html.erb b/app/views/bulk_upload_lettings_results/show.html.erb index 452926787..84374e26a 100644 --- a/app/views/bulk_upload_lettings_results/show.html.erb +++ b/app/views/bulk_upload_lettings_results/show.html.erb @@ -1,6 +1,10 @@ +<% content_for :before_content do %> + <%= govuk_back_link(text: "Back", href: summary_bulk_upload_lettings_result_path(@bulk_upload)) %> +<% end %> +
- Bulk Upload for lettings (<%= @bulk_upload.year_combo %>) + Bulk upload for lettings (<%= @bulk_upload.year_combo %>)

We found <%= pluralize(@bulk_upload.bulk_upload_errors.count, "error") %> in your file

diff --git a/app/views/bulk_upload_lettings_results/summary.html.erb b/app/views/bulk_upload_lettings_results/summary.html.erb new file mode 100644 index 000000000..04d8bdb92 --- /dev/null +++ b/app/views/bulk_upload_lettings_results/summary.html.erb @@ -0,0 +1,22 @@ +
+
+ Bulk upload for lettings (<%= @bulk_upload.year_combo %>) +

Correct data export and reupload

+ +

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

+
+
+ +<%= 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_button_link_to "Upload your file again", start_bulk_upload_lettings_logs_path %> diff --git a/config/routes.rb b/config/routes.rb index 913212dd2..be765c8c0 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -137,6 +137,7 @@ Rails.application.routes.draw do resources :bulk_upload_lettings_results, path: "bulk-upload-results", only: [:show] do member do get :resume + get :summary end end diff --git a/spec/components/bulk_upload_error_summary_table_component_spec.rb b/spec/components/bulk_upload_error_summary_table_component_spec.rb new file mode 100644 index 000000000..cdb0b58bf --- /dev/null +++ b/spec/components/bulk_upload_error_summary_table_component_spec.rb @@ -0,0 +1,81 @@ +require "rails_helper" + +RSpec.describe BulkUploadErrorSummaryTableComponent, type: :component do + subject(:component) { described_class.new(bulk_upload:) } + + let(:bulk_upload) { create(:bulk_upload) } + + context "when no errors" do + it "does not renders any rows" do + result = render_inline(component) + expect(result).not_to have_selector("tbody tr") + 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) } + + it "renders rows for each error" do + result = render_inline(component) + expect(result).to have_selector("tbody tr", count: 2) + end + + it "renders rows 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]) + end + + it "render correct data" do + result = render_inline(component) + + row_1 = result.css("tbody tr:nth-of-type(1) td").map(&:content) + + expect(row_1).to eql([ + "A", + "1", + BulkUpload::Lettings::Validator.question_for_field(error_1.field.to_sym), + error_1.error, + error_1.field, + ]) + + row_2 = result.css("tbody tr:nth-of-type(2) td").map(&:content) + + expect(row_2).to eql([ + "B", + "1", + BulkUpload::Lettings::Validator.question_for_field(error_2.field.to_sym), + error_2.error, + error_2.field, + ]) + 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") } + + before do + create(:bulk_upload_error, bulk_upload:, col: "A", row: 2, field: "field_1") + end + + it "renders 1 row combining the errors" do + result = render_inline(component) + expect(result).to have_selector("tbody tr", 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) + + expect(row_1).to eql([ + "A", + "2", + BulkUpload::Lettings::Validator.question_for_field(error_1.field.to_sym), + error_1.error, + error_1.field, + ]) + end + end +end diff --git a/spec/factories/bulk_upload_error.rb b/spec/factories/bulk_upload_error.rb index 1f42f763d..885f27d9d 100644 --- a/spec/factories/bulk_upload_error.rb +++ b/spec/factories/bulk_upload_error.rb @@ -4,7 +4,8 @@ FactoryBot.define do factory :bulk_upload_error do bulk_upload row { rand(9_999) } - cell { "#{('A'..'Z').to_a.sample}#{row}" } + col { ("A".."Z").to_a.sample } + cell { "#{col}#{row}" } tenant_code { SecureRandom.hex(4) } property_ref { SecureRandom.hex(4) } purchaser_code { SecureRandom.hex(4) } diff --git a/spec/requests/bulk_upload_lettings_results_controller_spec.rb b/spec/requests/bulk_upload_lettings_results_controller_spec.rb index 15ba0b7bb..c15fef5b9 100644 --- a/spec/requests/bulk_upload_lettings_results_controller_spec.rb +++ b/spec/requests/bulk_upload_lettings_results_controller_spec.rb @@ -9,12 +9,27 @@ RSpec.describe BulkUploadLettingsResultsController, type: :request do sign_in user end + describe "GET /lettings-logs/bulk-upload-results/:ID/summary" do + it "renders year combo" do + get "/lettings-logs/bulk-upload-results/#{bulk_upload.id}/summary" + + expect(response).to be_successful + expect(response.body).to include("Bulk upload for lettings (2022/23)") + end + + it "renders the bulk upload filename" do + get "/lettings-logs/bulk-upload-results/#{bulk_upload.id}/summary" + + expect(response.body).to include(bulk_upload.filename) + end + end + describe "GET /lettings-logs/bulk-upload-results/:ID" do it "renders correct year" do get "/lettings-logs/bulk-upload-results/#{bulk_upload.id}" expect(response).to be_successful - expect(response.body).to include("Bulk Upload for lettings (2022/23)") + expect(response.body).to include("Bulk upload for lettings (2022/23)") end it "renders correct number of errors" do