From 13b2fc61c1b047d7e5494b07f543a86826de1556 Mon Sep 17 00:00:00 2001 From: Samuel Young Date: Thu, 30 Apr 2026 15:16:27 +0100 Subject: [PATCH] CLDC-3280: Remove error report upload again when viewing before decision (#3303) * CLDC-3280: Add back link to error summary page * CLDC-3280: Hide upload button if navigating from a view url * CLDC-3280: Update tests * CLDC-3280: Make read only error report opt-in this means we can be happy only the touched pages will have a change in behaviour * CLDC-3280: Rename fix errors in tests to reupload file Co-authored-by: Oscar Richardson <116292912+oscar-richardson-softwire@users.noreply.github.com> --------- Co-authored-by: Oscar Richardson <116292912+oscar-richardson-softwire@users.noreply.github.com> --- .../forms/bulk_upload_resume/confirm.rb | 6 ++-- .../forms/bulk_upload_resume/fix_choice.rb | 6 ++-- .../show.html.erb | 4 ++- .../summary.html.erb | 8 ++++- .../confirm.html.erb | 2 +- .../fix_choice.html.erb | 2 +- .../bulk_upload_sales_results/show.html.erb | 4 ++- .../summary.html.erb | 8 ++++- .../bulk_upload_sales_resume/confirm.html.erb | 2 +- .../fix_choice.html.erb | 2 +- ...upload_lettings_results_controller_spec.rb | 36 +++++++++++++++++++ ...lk_upload_sales_results_controller_spec.rb | 36 +++++++++++++++++++ 12 files changed, 102 insertions(+), 14 deletions(-) diff --git a/app/models/forms/bulk_upload_resume/confirm.rb b/app/models/forms/bulk_upload_resume/confirm.rb index 96a1fe1bc..596018d50 100644 --- a/app/models/forms/bulk_upload_resume/confirm.rb +++ b/app/models/forms/bulk_upload_resume/confirm.rb @@ -20,11 +20,11 @@ module Forms send("resume_bulk_upload_#{log_type}_result_path", bulk_upload) end - def error_report_path + def error_report_path(read_only: false) if BulkUploadErrorSummaryTableComponent.new(bulk_upload:).errors? - send("summary_bulk_upload_#{log_type}_result_path", bulk_upload) + send("summary_bulk_upload_#{log_type}_result_path", bulk_upload, hide_upload_button: read_only ? "true" : nil) else - send("bulk_upload_#{log_type}_result_path", bulk_upload) + send("bulk_upload_#{log_type}_result_path", bulk_upload, hide_upload_button: read_only ? "true" : nil) end end diff --git a/app/models/forms/bulk_upload_resume/fix_choice.rb b/app/models/forms/bulk_upload_resume/fix_choice.rb index c731dbf93..b14970643 100644 --- a/app/models/forms/bulk_upload_resume/fix_choice.rb +++ b/app/models/forms/bulk_upload_resume/fix_choice.rb @@ -34,11 +34,11 @@ module Forms end end - def error_report_path + def error_report_path(read_only: false) if BulkUploadErrorSummaryTableComponent.new(bulk_upload:).errors? - send("summary_bulk_upload_#{log_type}_result_path", bulk_upload) + send("summary_bulk_upload_#{log_type}_result_path", bulk_upload, hide_upload_button: read_only ? "true" : nil) else - send("bulk_upload_#{log_type}_result_path", bulk_upload) + send("bulk_upload_#{log_type}_result_path", bulk_upload, hide_upload_button: read_only ? "true" : nil) end end diff --git a/app/views/bulk_upload_lettings_results/show.html.erb b/app/views/bulk_upload_lettings_results/show.html.erb index 5eedefe49..cf8c62eeb 100644 --- a/app/views/bulk_upload_lettings_results/show.html.erb +++ b/app/views/bulk_upload_lettings_results/show.html.erb @@ -33,4 +33,6 @@ -<%= govuk_button_link_to "Upload your file again", start_bulk_upload_lettings_logs_path(organisation_id: @bulk_upload.organisation_id) %> +<% if params[:hide_upload_button] != "true" %> + <%= govuk_button_link_to "Upload your file again", start_bulk_upload_lettings_logs_path(organisation_id: @bulk_upload.organisation_id) %> +<% end %> diff --git a/app/views/bulk_upload_lettings_results/summary.html.erb b/app/views/bulk_upload_lettings_results/summary.html.erb index ab60212e9..1a31eb0a9 100644 --- a/app/views/bulk_upload_lettings_results/summary.html.erb +++ b/app/views/bulk_upload_lettings_results/summary.html.erb @@ -1,3 +1,7 @@ +<% content_for :before_content do %> + <%= govuk_back_link(href: :back) %> +<% end %> + <%= render partial: "bulk_upload_shared/moved_user_banner" %>
@@ -34,4 +38,6 @@ <% end %>
-<%= govuk_button_link_to "Upload your file again", start_bulk_upload_lettings_logs_path(organisation_id: @bulk_upload.organisation_id) %> +<% if params[:hide_upload_button] != "true" %> + <%= govuk_button_link_to "Upload your file again", start_bulk_upload_lettings_logs_path(organisation_id: @bulk_upload.organisation_id) %> +<% end %> diff --git a/app/views/bulk_upload_lettings_resume/confirm.html.erb b/app/views/bulk_upload_lettings_resume/confirm.html.erb index bb15c7101..c72cab57b 100644 --- a/app/views/bulk_upload_lettings_resume/confirm.html.erb +++ b/app/views/bulk_upload_lettings_resume/confirm.html.erb @@ -9,7 +9,7 @@

<%= logs_and_errors_warning(@bulk_upload) %> - <%= govuk_link_to "View the error report", @form.error_report_path %> + <%= govuk_link_to "View the error report", @form.error_report_path(read_only: true) %>

<% if unique_answers_to_be_cleared(@bulk_upload).present? %> diff --git a/app/views/bulk_upload_lettings_resume/fix_choice.html.erb b/app/views/bulk_upload_lettings_resume/fix_choice.html.erb index 225fb07bf..3bb326d02 100644 --- a/app/views/bulk_upload_lettings_resume/fix_choice.html.erb +++ b/app/views/bulk_upload_lettings_resume/fix_choice.html.erb @@ -19,7 +19,7 @@
- <%= govuk_link_to "View the error report", @form.error_report_path %> + <%= govuk_link_to "View the error report", @form.error_report_path(read_only: true) %>
<%= govuk_details(summary_text: "How to choose between fixing errors on the CORE site or in the CSV") do %> diff --git a/app/views/bulk_upload_sales_results/show.html.erb b/app/views/bulk_upload_sales_results/show.html.erb index 2276285fe..68aaf7311 100644 --- a/app/views/bulk_upload_sales_results/show.html.erb +++ b/app/views/bulk_upload_sales_results/show.html.erb @@ -33,4 +33,6 @@ -<%= govuk_button_link_to "Upload your file again", start_bulk_upload_sales_logs_path(organisation_id: @bulk_upload.organisation_id) %> +<% if params[:hide_upload_button] != "true" %> + <%= govuk_button_link_to "Upload your file again", start_bulk_upload_sales_logs_path(organisation_id: @bulk_upload.organisation_id) %> +<% end %> diff --git a/app/views/bulk_upload_sales_results/summary.html.erb b/app/views/bulk_upload_sales_results/summary.html.erb index ed2ec14b3..fc9f39d82 100644 --- a/app/views/bulk_upload_sales_results/summary.html.erb +++ b/app/views/bulk_upload_sales_results/summary.html.erb @@ -1,3 +1,7 @@ +<% content_for :before_content do %> + <%= govuk_back_link(href: :back) %> +<% end %> + <%= render partial: "bulk_upload_shared/moved_user_banner" %>
@@ -34,4 +38,6 @@ <% end %>
-<%= govuk_button_link_to "Upload your file again", start_bulk_upload_sales_logs_path(organisation_id: @bulk_upload.organisation_id) %> +<% if params[:hide_upload_button] != "true" %> + <%= govuk_button_link_to "Upload your file again", start_bulk_upload_sales_logs_path(organisation_id: @bulk_upload.organisation_id) %> +<% end %> diff --git a/app/views/bulk_upload_sales_resume/confirm.html.erb b/app/views/bulk_upload_sales_resume/confirm.html.erb index b47619053..1259d8bf9 100644 --- a/app/views/bulk_upload_sales_resume/confirm.html.erb +++ b/app/views/bulk_upload_sales_resume/confirm.html.erb @@ -9,7 +9,7 @@

<%= logs_and_errors_warning(@bulk_upload) %> - <%= govuk_link_to "View the error report", @form.error_report_path %> + <%= govuk_link_to "View the error report", @form.error_report_path(read_only: true) %>

<% if unique_answers_to_be_cleared(@bulk_upload).present? %> diff --git a/app/views/bulk_upload_sales_resume/fix_choice.html.erb b/app/views/bulk_upload_sales_resume/fix_choice.html.erb index b376ee62d..8a2887678 100644 --- a/app/views/bulk_upload_sales_resume/fix_choice.html.erb +++ b/app/views/bulk_upload_sales_resume/fix_choice.html.erb @@ -19,7 +19,7 @@
- <%= govuk_link_to "View the error report", @form.error_report_path %> + <%= govuk_link_to "View the error report", @form.error_report_path(read_only: true) %>
<%= govuk_details(summary_text: "How to choose between fixing errors on the CORE site or in the CSV") do %> diff --git a/spec/requests/bulk_upload_lettings_results_controller_spec.rb b/spec/requests/bulk_upload_lettings_results_controller_spec.rb index 3416b6da9..e3ec4b4c1 100644 --- a/spec/requests/bulk_upload_lettings_results_controller_spec.rb +++ b/spec/requests/bulk_upload_lettings_results_controller_spec.rb @@ -82,6 +82,24 @@ RSpec.describe BulkUploadLettingsResultsController, type: :request do expect(response.body).to include("You moved to a different organisation since this file was uploaded. Upload the file again to get an accurate error report.") end end + + context "and user has upload button shown" do + it "displays a link to reupload file" do + get "/lettings-logs/bulk-upload-results/#{bulk_upload.id}/summary" + + expect(response.body).to include("Upload your file again") + expect(response.body).to include("/lettings-logs/bulk-upload-logs/start") + end + end + + context "and user has upload button hidden" do + it "does not display a link to reupload file" do + get "/lettings-logs/bulk-upload-results/#{bulk_upload.id}/summary?hide_upload_button=true" + + expect(response.body).not_to include("Upload your file again") + expect(response.body).not_to include("/lettings-logs/bulk-upload-logs/start") + end + end end end @@ -152,5 +170,23 @@ RSpec.describe BulkUploadLettingsResultsController, type: :request do expect(response.body).to include("You moved to a different organisation since this file was uploaded. Upload the file again to get an accurate error report.") end end + + context "and user has upload button shown" do + it "displays a link to reupload file" do + get "/lettings-logs/bulk-upload-results/#{bulk_upload.id}" + + expect(response.body).to include("Upload your file again") + expect(response.body).to include("/lettings-logs/bulk-upload-logs/start") + end + end + + context "and user has upload button hidden" do + it "does not display a link to reupload file" do + get "/lettings-logs/bulk-upload-results/#{bulk_upload.id}?hide_upload_button=true" + + expect(response.body).not_to include("Upload your file again") + expect(response.body).not_to include("/lettings-logs/bulk-upload-logs/start") + end + end end end diff --git a/spec/requests/bulk_upload_sales_results_controller_spec.rb b/spec/requests/bulk_upload_sales_results_controller_spec.rb index 2236475fa..c6d31b0f4 100644 --- a/spec/requests/bulk_upload_sales_results_controller_spec.rb +++ b/spec/requests/bulk_upload_sales_results_controller_spec.rb @@ -44,6 +44,24 @@ RSpec.describe BulkUploadSalesResultsController, type: :request do expect(response.body).to include("You moved to a different organisation since this file was uploaded. Upload the file again to get an accurate error report.") end end + + context "and user has upload button shown" do + it "displays a link to reupload file" do + get "/sales-logs/bulk-upload-results/#{bulk_upload.id}/summary" + + expect(response.body).to include("Upload your file again") + expect(response.body).to include("/sales-logs/bulk-upload-logs/start") + end + end + + context "and user has upload button hidden" do + it "does not display a link to reupload file" do + get "/sales-logs/bulk-upload-results/#{bulk_upload.id}/summary?hide_upload_button=true" + + expect(response.body).not_to include("Upload your file again") + expect(response.body).not_to include("/sales-logs/bulk-upload-logs/start") + end + end end end @@ -127,5 +145,23 @@ RSpec.describe BulkUploadSalesResultsController, type: :request do expect(response.body).to include("You moved to a different organisation since this file was uploaded. Upload the file again to get an accurate error report.") end end + + context "and user has upload button shown" do + it "displays a link to reupload file" do + get "/sales-logs/bulk-upload-results/#{bulk_upload.id}" + + expect(response.body).to include("Upload your file again") + expect(response.body).to include("/sales-logs/bulk-upload-logs/start") + end + end + + context "and user has upload button hidden" do + it "does not display a link to reupload file" do + get "/sales-logs/bulk-upload-results/#{bulk_upload.id}?hide_upload_button=true" + + expect(response.body).not_to include("Upload your file again") + expect(response.body).not_to include("/sales-logs/bulk-upload-logs/start") + end + end end end