Browse Source

CLDC-2972 Warn users about answers that will be cleared in bulk upload (#2142)

* Empty commit

* feat: base lettings behaviour

* feat: formatting improvements

* Empty commit

* Empty commit

* Empty commit

* refactor: lint

* feat: more page styling

* feat: update tests

* refactor: lint

* feat: add requests spec

* refactor: simplify

* feat: add tests

* feat: copy change to reflect that the count in the header is not necessarily as large as the errors shown (it excludes duplicate errors on fields, and not_answered errors)

* feat: copy changes to sales as well

* feat: remove LegacyBulkUpload MVC

* feat: fix typo

* feat: update tests

* feat: mark additional not_answered sales q

* feat: update routing and only display warnign text when errors will be cleared

* feat: update copy and error rows

* refactor: lint

* feat: don't show soft validations in deletion report

* feat: update routing so deletion report is not shown once logs uploaded

* feat: update tests

* feat: make unique count track repeated errors across rows

* refactor: lint
pull/2163/head
natdeanlewissoftwire 11 months ago committed by GitHub
parent
commit
b4cd777897
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 1
      Gemfile.lock
  2. 26
      app/controllers/bulk_upload_controller.rb
  3. 8
      app/controllers/bulk_upload_lettings_resume_controller.rb
  4. 8
      app/controllers/bulk_upload_sales_resume_controller.rb
  5. 2
      app/controllers/form_controller.rb
  6. 28
      app/helpers/logs_helper.rb
  7. 8
      app/models/forms/bulk_upload_lettings_resume/confirm.rb
  8. 28
      app/models/forms/bulk_upload_lettings_resume/deletion_report.rb
  9. 14
      app/models/forms/bulk_upload_lettings_resume/fix_choice.rb
  10. 8
      app/models/forms/bulk_upload_sales_resume/confirm.rb
  11. 14
      app/models/forms/bulk_upload_sales_resume/fix_choice.rb
  12. 197
      app/models/legacy_bulk_upload.rb
  13. 4
      app/policies/bulk_upload_policy.rb
  14. 20
      app/services/bulk_upload/lettings/year2023/row_parser.rb
  15. 10
      app/services/bulk_upload/sales/year2023/row_parser.rb
  16. 4
      app/views/bulk_upload_lettings_results/show.html.erb
  17. 23
      app/views/bulk_upload_lettings_resume/confirm.html.erb
  18. 31
      app/views/bulk_upload_lettings_resume/deletion_report.html.erb
  19. 4
      app/views/bulk_upload_lettings_resume/fix_choice.html.erb
  20. 4
      app/views/bulk_upload_sales_results/show.html.erb
  21. 23
      app/views/bulk_upload_sales_resume/confirm.html.erb
  22. 31
      app/views/bulk_upload_sales_resume/deletion_report.html.erb
  23. 6
      app/views/bulk_upload_sales_resume/fix_choice.html.erb
  24. 11
      app/views/logs/bulk_upload.html.erb
  25. 5
      config/routes.rb
  26. 139
      spec/helpers/logs_helper_spec.rb
  27. 126
      spec/requests/bulk_upload_controller_spec.rb
  28. 46
      spec/requests/bulk_upload_lettings_resume_controller_spec.rb
  29. 4
      spec/requests/bulk_upload_sales_results_controller_spec.rb
  30. 46
      spec/requests/bulk_upload_sales_resume_controller_spec.rb

1
Gemfile.lock

@ -436,6 +436,7 @@ GEM
PLATFORMS
arm64-darwin-21
arm64-darwin-22
arm64-darwin-23
x86_64-darwin-19
x86_64-darwin-20
x86_64-darwin-21

26
app/controllers/bulk_upload_controller.rb

@ -1,26 +0,0 @@
class BulkUploadController < ApplicationController
before_action :authenticate_user!
def show
@bulk_upload = LegacyBulkUpload.new(nil, nil)
render "logs/bulk_upload"
end
def bulk_upload
file = upload_params.tempfile
content_type = upload_params.content_type
@bulk_upload = LegacyBulkUpload.new(file, content_type)
@bulk_upload.process(current_user)
if @bulk_upload.errors.present?
render "logs/bulk_upload", status: :unprocessable_entity
else
redirect_to(lettings_logs_path)
end
end
private
def upload_params
params.require("bulk_upload")["lettings_log_bulk_upload"]
end
end

8
app/controllers/bulk_upload_lettings_resume_controller.rb

@ -31,6 +31,12 @@ class BulkUploadLettingsResumeController < ApplicationController
end
end
def deletion_report
@bulk_upload = BulkUpload.lettings.find(params[:id])
authorize @bulk_upload
end
private
def form
@ -41,6 +47,8 @@ private
Forms::BulkUploadLettingsResume::Chosen.new(form_params.merge(bulk_upload: @bulk_upload))
when "confirm"
Forms::BulkUploadLettingsResume::Confirm.new(form_params.merge(bulk_upload: @bulk_upload))
when "deletion-report"
Forms::BulkUploadLettingsResume::DeletionReport.new(form_params.merge(bulk_upload: @bulk_upload))
else
raise "invalid form"
end

8
app/controllers/bulk_upload_sales_resume_controller.rb

@ -31,6 +31,12 @@ class BulkUploadSalesResumeController < ApplicationController
end
end
def deletion_report
@bulk_upload = BulkUpload.sales.find(params[:id])
authorize @bulk_upload
end
private
def form
@ -41,6 +47,8 @@ private
Forms::BulkUploadSalesResume::Chosen.new(form_params.merge(bulk_upload: @bulk_upload))
when "confirm"
Forms::BulkUploadSalesResume::Confirm.new(form_params.merge(bulk_upload: @bulk_upload))
when "deletion-report"
Forms::BulkUploadSalesResume::DeletionReport.new(form_params.merge(bulk_upload: @bulk_upload))
else
raise "invalid form"
end

2
app/controllers/form_controller.rb

@ -15,7 +15,7 @@ class FormController < ApplicationController
redirect_to(successful_redirect_path)
else
mandatory_questions_with_no_response.map do |question|
@log.errors.add question.id.to_sym, question.unanswered_error_message
@log.errors.add question.id.to_sym, question.unanswered_error_message, category: :not_answered
end
Rails.logger.info "User triggered validation(s) on: #{@log.errors.map(&:attribute).join(', ')}"
@subsection = form.subsection_for_page(@page)

28
app/helpers/logs_helper.rb

@ -49,9 +49,10 @@ module LogsHelper
end
def logs_and_errors_warning(bulk_upload)
this_or_these_errors = bulk_upload.bulk_upload_errors.count == 1 ? "This error" : "These errors"
is_or_are = bulk_upload.total_logs_count == 1 ? "is" : "are"
needs_or_need = bulk_upload.bulk_upload_errors.count == 1 ? "needs" : "need"
"You will upload #{pluralize(bulk_upload.total_logs_count, 'log')}. There are errors in #{pluralize(bulk_upload.logs_with_errors_count, 'log')}, and #{pluralize(bulk_upload.bulk_upload_errors.count, 'error')} in total. #{this_or_these_errors} will need to be fixed on the CORE site."
"There #{is_or_are} #{pluralize(bulk_upload.total_logs_count, 'log')} in this bulk upload with #{pluralize(bulk_upload.bulk_upload_errors.count, 'error')} that still #{needs_or_need} to be fixed after upload."
end
def logs_and_soft_validations_warning(bulk_upload)
@ -63,4 +64,27 @@ module LogsHelper
def bulk_upload_error_summary(bulk_upload)
"You have tried to upload #{bulk_upload.total_logs_count ? pluralize(bulk_upload.total_logs_count, 'log') : 'logs'}. There are errors in #{pluralize(bulk_upload.logs_with_errors_count, 'log')}, and #{pluralize(bulk_upload.bulk_upload_errors.count, 'error')} in total."
end
def deleted_errors_warning_text(bulk_upload)
unique_answers_to_be_cleared_count = unique_answers_to_be_cleared(bulk_upload).count
this_or_these = unique_answers_to_be_cleared_count == 1 ? "this" : "these"
it_is_or_they_are = unique_answers_to_be_cleared_count == 1 ? "it is" : "they are"
"#{pluralize(unique_answers_to_be_cleared_count, 'answer')} will be deleted because #{it_is_or_they_are} invalid. You will have to answer #{this_or_these} #{'question'.pluralize(unique_answers_to_be_cleared_count)} again on the site."
end
def all_answers_to_be_cleared(bulk_upload_errors)
bulk_upload_errors.reject { |e| %w[not_answered soft_validation].include?(e.category) }
end
def unique_answers_to_be_cleared(bulk_upload)
all_answers_to_be_cleared(bulk_upload.bulk_upload_errors).uniq { |error| [error.field, error.row] }
end
def answers_to_be_deleted_title_text(bulk_upload)
unique_answers_to_be_cleared_count = unique_answers_to_be_cleared(bulk_upload).count
this_or_these = unique_answers_to_be_cleared_count == 1 ? "This" : "These"
"#{this_or_these} #{pluralize(unique_answers_to_be_cleared(bulk_upload).count, 'answer')} will be deleted if you upload the #{'log'.pluralize(bulk_upload.logs.count)}"
end
end

8
app/models/forms/bulk_upload_lettings_resume/confirm.rb

@ -19,6 +19,14 @@ module Forms
resume_bulk_upload_lettings_result_path(bulk_upload)
end
def error_report_path
if BulkUploadErrorSummaryTableComponent.new(bulk_upload:).errors?
summary_bulk_upload_lettings_result_path(bulk_upload)
else
bulk_upload_lettings_result_path(bulk_upload)
end
end
def save!
ApplicationRecord.transaction do
processor = BulkUpload::Processor.new(bulk_upload:)

28
app/models/forms/bulk_upload_lettings_resume/deletion_report.rb

@ -0,0 +1,28 @@
module Forms
module BulkUploadLettingsResume
class DeletionReport
include ActiveModel::Model
include ActiveModel::Attributes
include Rails.application.routes.url_helpers
attribute :bulk_upload
def view_path
"bulk_upload_lettings_resume/deletion_report"
end
def preflight_valid?
bulk_upload.choice != "create-fix-inline" && bulk_upload.choice != "bulk-confirm-soft-validations"
end
def preflight_redirect
case bulk_upload.choice
when "create-fix-inline"
page_bulk_upload_lettings_resume_path(bulk_upload, :chosen)
when "bulk-confirm-soft-validations"
page_bulk_upload_lettings_soft_validations_check_path(bulk_upload, :chosen)
end
end
end
end
end

14
app/models/forms/bulk_upload_lettings_resume/fix_choice.rb

@ -27,16 +27,20 @@ module Forms
when "create-fix-inline"
page_bulk_upload_lettings_resume_path(bulk_upload, page: "confirm")
when "upload-again"
if BulkUploadErrorSummaryTableComponent.new(bulk_upload:).errors?
summary_bulk_upload_lettings_result_path(bulk_upload)
else
bulk_upload_lettings_result_path(bulk_upload)
end
error_report_path
else
raise "invalid choice"
end
end
def error_report_path
if BulkUploadErrorSummaryTableComponent.new(bulk_upload:).errors?
summary_bulk_upload_lettings_result_path(bulk_upload)
else
bulk_upload_lettings_result_path(bulk_upload)
end
end
def recommendation
if BulkUploadErrorSummaryTableComponent.new(bulk_upload:).errors?
"We recommend fixing these errors in the CSV, as you may be able to edit multiple fields at once. However, you can also upload these logs and fix the errors on the CORE site."

8
app/models/forms/bulk_upload_sales_resume/confirm.rb

@ -19,6 +19,14 @@ module Forms
resume_bulk_upload_sales_result_path(bulk_upload)
end
def error_report_path
if BulkUploadErrorSummaryTableComponent.new(bulk_upload:).errors?
summary_bulk_upload_sales_result_path(bulk_upload)
else
bulk_upload_sales_result_path(bulk_upload)
end
end
def save!
ApplicationRecord.transaction do
processor = BulkUpload::Processor.new(bulk_upload:)

14
app/models/forms/bulk_upload_sales_resume/fix_choice.rb

@ -27,16 +27,20 @@ module Forms
when "create-fix-inline"
page_bulk_upload_sales_resume_path(bulk_upload, page: "confirm")
when "upload-again"
if BulkUploadErrorSummaryTableComponent.new(bulk_upload:).errors?
summary_bulk_upload_sales_result_path(bulk_upload)
else
bulk_upload_sales_result_path(bulk_upload)
end
error_report_path
else
raise "invalid choice"
end
end
def error_report_path
if BulkUploadErrorSummaryTableComponent.new(bulk_upload:).errors?
summary_bulk_upload_sales_result_path(bulk_upload)
else
bulk_upload_sales_result_path(bulk_upload)
end
end
def recommendation
if BulkUploadErrorSummaryTableComponent.new(bulk_upload:).errors?
"We recommend fixing these errors in the CSV, as you may be able to edit multiple fields at once. However, you can also upload these logs and fix the errors on the CORE site."

197
app/models/legacy_bulk_upload.rb

@ -1,197 +0,0 @@
class LegacyBulkUpload
include ActiveModel::Model
include ActiveModel::Validations
include ActiveModel::Conversion
SPREADSHEET_CONTENT_TYPES = %w[
application/vnd.ms-excel
application/vnd.openxmlformats-officedocument.spreadsheetml.sheet
].freeze
FIRST_DATA_ROW = 7
def initialize(file, content_type)
@file = file
@content_type = content_type
end
def process(current_user)
return unless valid_content_type?
xlsx = Roo::Spreadsheet.open(@file, extension: :xlsx)
sheet = xlsx.sheet(0)
last_row = sheet.last_row
if last_row < FIRST_DATA_ROW
errors.add(:lettings_log_bulk_upload, "No data found")
else
data_range = FIRST_DATA_ROW..last_row
data_range.map do |row_num|
row = sheet.row(row_num)
# owning_organisation = Organisation.find(row[111])
# managing_organisation = Organisation.find(row[113])
lettings_log = LettingsLog.create!(
owning_organisation: current_user.organisation,
managing_organisation: current_user.organisation,
created_by: current_user,
)
map_row(row).each do |attr_key, attr_val|
_update = lettings_log.update(attr_key => attr_val)
rescue ArgumentError
# TODO: determine what we want to do when bulk upload contains totally invalid data for a field.
end
end
end
end
def valid_content_type?
if SPREADSHEET_CONTENT_TYPES.include?(@content_type)
true
else
errors.add(:lettings_log_bulk_upload, "Invalid file type")
false
end
end
def map_row(row)
{
lettype: row[1],
# reg_num_la_core_code: row[3],
# managementgroup: row[4],
# schemecode: row[5],
# firstletting: row[6],
tenancycode: row[7],
startertenancy: row[8],
tenancy: row[9],
tenancyother: row[10],
# tenancyduration: row[11],
hhmemb: hhmemb(row),
age1: row[12],
age2: row[13],
age3: row[14],
age4: row[15],
age5: row[16],
age6: row[17],
age7: row[18],
age8: row[19],
sex1: row[20],
sex2: row[21],
sex3: row[22],
sex4: row[23],
sex5: row[24],
sex6: row[25],
sex7: row[26],
sex8: row[27],
relat2: row[28],
relat3: row[29],
relat4: row[30],
relat5: row[31],
relat6: row[32],
relat7: row[33],
relat8: row[34],
ecstat1: row[35],
ecstat2: row[36],
ecstat3: row[37],
ecstat4: row[38],
ecstat5: row[39],
ecstat6: row[40],
ecstat7: row[41],
ecstat8: row[42],
ethnic: row[43],
national: row[44],
armedforces: row[45],
reservist: row[46],
preg_occ: row[47],
hb: row[48],
benefits: row[49],
net_income_known: row[50].present? ? 1 : nil,
earnings: row[50],
# increfused: row[51],
reason: row[52],
reasonother: row[53],
underoccupation_benefitcap: row[54],
housingneeds_a: row[55],
housingneeds_b: row[56],
housingneeds_c: row[57],
housingneeds_f: row[58],
housingneeds_g: row[59],
housingneeds_h: row[60],
prevten: row[61],
prevloc: row[62],
# prevpco_unknown: row[65],
layear: row[66],
waityear: row[67],
homeless: row[68],
reasonpref: row[69],
rp_homeless: row[70],
rp_insan_unsat: row[71],
rp_medwel: row[72],
rp_hardship: row[73],
rp_dontknow: row[74],
cbl: row[75],
chr: row[76],
cap: row[77],
# referral_source: row[78],
period: row[79],
brent: row[80],
scharge: row[81],
pscharge: row[82],
supcharg: row[83],
tcharge: row[84],
# tcharge_care_homes: row[85],
# no_rent_or_charge: row[86],
hbrentshortfall: row[87],
tshortfall: row[88],
voiddate: row[89].to_s + row[90].to_s + row[91].to_s,
majorrepairs: row[92].present? ? "1" : nil,
mrcdate: row[92].to_s + row[93].to_s + row[94].to_s,
# supported_scheme: row[95],
startdate: date_time(row[98], row[97], row[96]),
# startdate_day: row[96],
# startdate_month: row[97],
# startdate_year: row[98],
offered: row[99],
# property_reference: row[100],
beds: row[101],
unittype_gn: row[102],
builtype: row[103],
wchair: row[104],
property_relet: row[105],
rsnvac: row[106],
la: row[107],
# row[110] removed
# row[111] is owning organisation used above
# username: row[112],
# row[113] is managing organisation used above
leftreg: row[114],
# uprn: row[115],
incfreq: row[116],
# sheltered_accom: row[117],
illness: row[118],
illness_type_1: row[119],
illness_type_2: row[120],
illness_type_3: row[121],
illness_type_4: row[122],
illness_type_8: row[123],
illness_type_5: row[124],
illness_type_6: row[125],
illness_type_7: row[126],
illness_type_9: row[127],
illness_type_10: row[128],
# london_affordable: row[129],
rent_type: row[130],
irproduct_other: row[131],
# data_protection: row[132],
declaration: 1,
}
end
def date_time(year, month, day)
return unless year && month && day
Time.zone.local("20#{year}", month.to_s, day.to_s)
end
def hhmemb(row)
[14, 15, 16, 17, 18, 19, 20].count { |idx| row[idx].present? }
end
end

4
app/policies/bulk_upload_policy.rb

@ -18,6 +18,10 @@ class BulkUploadPolicy
owner? || same_org? || user.support?
end
def deletion_report?
owner? || same_org? || user.support?
end
private
def owner?

20
app/services/bulk_upload/lettings/year2023/row_parser.rb

@ -539,7 +539,7 @@ private
def validate_uprn_exists_if_any_key_address_fields_are_blank
if field_18.blank? && (field_19.blank? || field_21.blank?)
errors.add(:field_18, I18n.t("validations.not_answered", question: "UPRN"))
errors.add(:field_18, I18n.t("validations.not_answered", question: "UPRN"), category: :not_answered)
end
end
@ -623,10 +623,10 @@ private
def validate_no_housing_needs_questions_answered
if [field_83, field_84, field_85, field_86, field_87, field_88].all?(&:blank?)
errors.add(:field_87, I18n.t("validations.not_answered", question: "anybody with disabled access needs"))
errors.add(:field_86, I18n.t("validations.not_answered", question: "other access needs"))
errors.add(:field_87, I18n.t("validations.not_answered", question: "anybody with disabled access needs"), category: :not_answered)
errors.add(:field_86, I18n.t("validations.not_answered", question: "other access needs"), category: :not_answered)
%i[field_83 field_84 field_85].each do |field|
errors.add(field, I18n.t("validations.not_answered", question: "disabled access needs type"))
errors.add(field, I18n.t("validations.not_answered", question: "disabled access needs type"), category: :not_answered)
end
end
end
@ -635,7 +635,7 @@ private
reason_fields = %i[field_111 field_112 field_113 field_114 field_115]
if field_110 == 1 && reason_fields.all? { |field| attributes[field.to_s].blank? }
reason_fields.each do |field|
errors.add(field, I18n.t("validations.not_answered", question: "reason for reasonable preference"))
errors.add(field, I18n.t("validations.not_answered", question: "reason for reasonable preference"), category: :not_answered)
end
end
end
@ -650,16 +650,16 @@ private
end
elsif illness_option_fields.all? { |field| attributes[field.to_s].blank? }
illness_option_fields.each do |field|
errors.add(field, I18n.t("validations.not_answered", question: "how is person affected by condition or illness"))
errors.add(field, I18n.t("validations.not_answered", question: "how is person affected by condition or illness"), category: :not_answered)
end
end
end
def validate_lettings_allocation
if cbl.blank? && cap.blank? && chr.blank?
errors.add(:field_116, I18n.t("validations.not_answered", question: "was the letting made under the Choice-Based Lettings (CBL)?"))
errors.add(:field_117, I18n.t("validations.not_answered", question: "was the letting made under the Common Allocation Policy (CAP)?"))
errors.add(:field_118, I18n.t("validations.not_answered", question: "was the letting made under the Common Housing Register (CHR)?"))
errors.add(:field_116, I18n.t("validations.not_answered", question: "was the letting made under the Choice-Based Lettings (CBL)?"), category: :not_answered)
errors.add(:field_117, I18n.t("validations.not_answered", question: "was the letting made under the Common Allocation Policy (CAP)?"), category: :not_answered)
errors.add(:field_118, I18n.t("validations.not_answered", question: "was the letting made under the Common Housing Register (CHR)?"), category: :not_answered)
end
end
@ -743,7 +743,7 @@ private
fields.each do |field|
unless errors.any? { |e| fields.include?(e.attribute) }
question_text = question.error_display_label.presence || "this question"
errors.add(field, I18n.t("validations.not_answered", question: question_text.downcase))
errors.add(field, I18n.t("validations.not_answered", question: question_text.downcase), category: :not_answered)
end
end
end

10
app/services/bulk_upload/sales/year2023/row_parser.rb

@ -575,7 +575,7 @@ private
organisations_fields = %i[field_67 field_68 field_69 field_70]
if organisations_fields.all? { |field| attributes[field.to_s].blank? }
organisations_fields.each do |field|
errors.add(field, "At least one option must be selected of these four")
errors.add(field, "At least one option must be selected of these four", category: :not_answered)
end
end
end
@ -608,18 +608,18 @@ private
def validate_uprn_exists_if_any_key_address_fields_are_blank
if field_19.blank? && (field_20.blank? || field_22.blank?)
errors.add(:field_19, I18n.t("validations.not_answered", question: "UPRN"))
errors.add(:field_19, I18n.t("validations.not_answered", question: "UPRN"), category: :not_answered)
end
end
def validate_address_fields
if field_19.blank? || log.errors.attribute_names.include?(:uprn)
if field_20.blank?
errors.add(:field_20, I18n.t("validations.not_answered", question: "address line 1"))
errors.add(:field_20, I18n.t("validations.not_answered", question: "address line 1"), category: :not_answered)
end
if field_22.blank?
errors.add(:field_22, I18n.t("validations.not_answered", question: "town or city"))
errors.add(:field_22, I18n.t("validations.not_answered", question: "town or city"), category: :not_answered)
end
end
end
@ -1245,7 +1245,7 @@ private
else
fields.each do |field|
unless errors.any? { |e| fields.include?(e.attribute) }
errors.add(field, I18n.t("validations.not_answered", question: question.error_display_label&.downcase))
errors.add(field, I18n.t("validations.not_answered", question: question.error_display_label&.downcase), category: :not_answered)
end
end
end

4
app/views/bulk_upload_lettings_results/show.html.erb

@ -1,3 +1,7 @@
<% content_for :before_content do %>
<%= govuk_back_link(href: :back) %>
<% 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>

23
app/views/bulk_upload_lettings_resume/confirm.html.erb

@ -5,14 +5,25 @@
<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">You have chosen to upload all logs from this bulk upload.</h1>
<h1 class="govuk-heading-l">Are you sure you want to upload all logs from this bulk upload?</h1>
<p class="govuk-body"><%= logs_and_errors_warning(@bulk_upload) %></p>
<p class="govuk-body">
<%= logs_and_errors_warning(@bulk_upload) %>
<%= govuk_link_to "View the error report", @form.error_report_path %>
</p>
<%= form_with model: @form, scope: :form, url: page_bulk_upload_lettings_resume_path(@bulk_upload, page: "confirm"), method: :patch do |f| %>
<%= f.govuk_submit "Confirm" %>
<%= govuk_button_link_to "Cancel", @form.back_path, secondary: true %>
<% if unique_answers_to_be_cleared(@bulk_upload).present? %>
<%= govuk_warning_text do %>
<%= deleted_errors_warning_text(@bulk_upload) %>
<%= govuk_link_to "See which answers will be deleted", deletion_report_bulk_upload_lettings_resume_path %>
<% end %>
<% end %>
<div class="govuk-button-group">
<%= form_with model: @form, scope: :form, url: page_bulk_upload_lettings_resume_path(@bulk_upload, page: "confirm"), method: :patch do |f| %>
<%= f.govuk_submit "Confirm" %>
<%= govuk_button_link_to "Cancel", @form.back_path, secondary: true %>
<% end %>
</div>
</div>
</div>

31
app/views/bulk_upload_lettings_resume/deletion_report.html.erb

@ -0,0 +1,31 @@
<% content_for :before_content do %>
<%= govuk_back_link(href: :back) %>
<% 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>
<h1 class="govuk-heading-l"><%= answers_to_be_deleted_title_text(@bulk_upload) %></h1>
<p>The following cells contain data this is incorrect.</p>
<p>If you upload the logs, these answers will be deleted. You will have to re-enter the data on the site and resolve these errors.</p>
<p>If you do not want these answers to be deleted, correct the data in the CSV and upload the file again.</p>
<h2 class="govuk-heading-m">File: <%= @bulk_upload.filename %></h2>
</div>
</div>
<div class="govuk-grid-row">
<div class="govuk-grid-column-full">
<% @bulk_upload.bulk_upload_errors.order_by_row.order_by_cell.group_by(&:row).each do |_row, errors_for_row| %>
<%= render BulkUploadErrorRowComponent.new(bulk_upload_errors: all_answers_to_be_cleared(errors_for_row)) %>
<% end %>
</div>
</div>
<div class="govuk-button-group">
<%= form_with model: @form, scope: :form, url: page_bulk_upload_lettings_resume_path(@bulk_upload, "confirm"), method: :patch do |f| %>
<%= f.govuk_submit "Clear this data and upload the logs" %>
<%= govuk_button_link_to "I have fixed these errors and I want to reupload the file", start_bulk_upload_lettings_logs_path, secondary: true %>
<% end %>
</div>

4
app/views/bulk_upload_lettings_resume/fix_choice.html.erb

@ -18,6 +18,10 @@
<%= @form.recommendation %>
</div>
<div class="govuk-body">
<%= govuk_link_to "View the error report", @form.error_report_path %>
</div>
<%= govuk_details(summary_text: "How to choose between fixing errors on the CORE site or in the CSV") do %>
<p class="govuk-body govuk-!-margin-bottom-2">You may find it easier to fix the errors in the CSV file if:</p>
<ul class="govuk-list govuk-list--bullet">

4
app/views/bulk_upload_sales_results/show.html.erb

@ -1,3 +1,7 @@
<% content_for :before_content do %>
<%= govuk_back_link(href: :back) %>
<% end %>
<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds">
<span class="govuk-caption-l">Bulk Upload for sales (<%= @bulk_upload.year_combo %>)</span>

23
app/views/bulk_upload_sales_resume/confirm.html.erb

@ -5,14 +5,25 @@
<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds">
<span class="govuk-caption-l">Bulk upload for sales (<%= @bulk_upload.year_combo %>)</span>
<h1 class="govuk-heading-l">You have chosen to upload all logs from this bulk upload.</h1>
<h1 class="govuk-heading-l">Are you sure you want to upload all logs from this bulk upload?</h1>
<p class="govuk-body"><%= logs_and_errors_warning(@bulk_upload) %></p>
<p class="govuk-body">
<%= logs_and_errors_warning(@bulk_upload) %>
<%= govuk_link_to "View the error report", @form.error_report_path %>
</p>
<%= form_with model: @form, scope: :form, url: page_bulk_upload_sales_resume_path(@bulk_upload, page: "confirm"), method: :patch do |f| %>
<%= f.govuk_submit %>
<%= govuk_button_link_to "Cancel", @form.back_path, secondary: true %>
<% if unique_answers_to_be_cleared(@bulk_upload).present? %>
<%= govuk_warning_text do %>
<%= deleted_errors_warning_text(@bulk_upload) %>
<%= govuk_link_to "See which answers will be deleted", deletion_report_bulk_upload_sales_resume_path %>
<% end %>
<% end %>
<div class="govuk-button-group">
<%= form_with model: @form, scope: :form, url: page_bulk_upload_sales_resume_path(@bulk_upload, page: "confirm"), method: :patch do |f| %>
<%= f.govuk_submit "Confirm" %>
<%= govuk_button_link_to "Cancel", @form.back_path, secondary: true %>
<% end %>
</div>
</div>
</div>

31
app/views/bulk_upload_sales_resume/deletion_report.html.erb

@ -0,0 +1,31 @@
<% content_for :before_content do %>
<%= govuk_back_link(href: :back) %>
<% end %>
<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds">
<span class="govuk-caption-l">Bulk upload for sales (<%= @bulk_upload.year_combo %>)</span>
<h1 class="govuk-heading-l"><%= answers_to_be_deleted_title_text(@bulk_upload) %></h1>
<p>The following cells contain data this is incorrect.</p>
<p>If you upload the logs, these answers will be deleted. You will have to re-enter the data on the site and resolve these errors.</p>
<p>If you do not want these answers to be deleted, correct the data in the CSV and upload the file again.</p>
<h2 class="govuk-heading-m">File: <%= @bulk_upload.filename %></h2>
</div>
</div>
<div class="govuk-grid-row">
<div class="govuk-grid-column-full">
<% @bulk_upload.bulk_upload_errors.order_by_row.order_by_cell.group_by(&:row).each do |_row, errors_for_row| %>
<%= render BulkUploadErrorRowComponent.new(bulk_upload_errors: all_answers_to_be_cleared(errors_for_row)) %>
<% end %>
</div>
</div>
<div class="govuk-button-group">
<%= form_with model: @form, scope: :form, url: page_bulk_upload_sales_resume_path(@bulk_upload, "confirm"), method: :patch do |f| %>
<%= f.govuk_submit "Clear this data and upload the logs" %>
<%= govuk_button_link_to "I have fixed these errors and I want to reupload the file", start_bulk_upload_sales_logs_path, secondary: true %>
<% end %>
</div>

6
app/views/bulk_upload_sales_resume/fix_choice.html.erb

@ -18,6 +18,10 @@
<%= @form.recommendation %>
</div>
<div class="govuk-body">
<%= govuk_link_to "View the error report", @form.error_report_path %>
</div>
<%= govuk_details(summary_text: "How to choose between fixing errors on the CORE site or in the CSV") do %>
<p class="govuk-body govuk-!-margin-bottom-2">You may find it easier to fix the errors in the CSV file if:</p>
<ul class="govuk-list govuk-list--bullet">
@ -39,7 +43,7 @@
:name,
legend: { hidden: true } %>
<div class="govuk-button-group">
<div class="govuk-button-group">
<%= f.govuk_submit %>
<% if @soft_errors_only %>
<%= govuk_button_link_to "Cancel", bulk_upload_sales_soft_validations_check_url(@bulk_upload, page: "confirm-soft-errors"), secondary: true %>

11
app/views/logs/bulk_upload.html.erb

@ -1,11 +0,0 @@
<% content_for :title, "Bulk upload" %>
<%= form_for @bulk_upload, scope: :bulk_upload, url: bulk_upload_lettings_logs_path, method: "post" do |f| %>
<%= f.govuk_error_summary %>
<%= f.govuk_file_field :lettings_log_bulk_upload,
label: { text: content_for(:title), size: "l" },
hint: { text: "Upload a spreadsheet using the template" } %>
<%= f.govuk_submit "Upload" %>
<% end %>

5
config/routes.rb

@ -205,9 +205,6 @@ Rails.application.routes.draw do
get "delete-duplicates", to: "duplicate_logs#delete_duplicates"
collection do
post "bulk-upload", to: "bulk_upload#bulk_upload"
get "bulk-upload", to: "bulk_upload#show"
get "csv-download", to: "lettings_logs#download_csv"
post "email-csv", to: "lettings_logs#email_csv"
get "csv-confirmation", to: "lettings_logs#csv_confirmation"
@ -236,6 +233,7 @@ Rails.application.routes.draw do
get "*page", to: "bulk_upload_lettings_resume#show", as: "page"
patch "*page", to: "bulk_upload_lettings_resume#update"
get "deletion-report"
end
end
@ -297,6 +295,7 @@ Rails.application.routes.draw do
resources :bulk_upload_sales_resume, path: "bulk-upload-resume", only: %i[show update] do
member do
get :start
get "deletion-report"
get "*page", to: "bulk_upload_sales_resume#show", as: "page"
patch "*page", to: "bulk_upload_sales_resume#update"

139
spec/helpers/logs_helper_spec.rb

@ -0,0 +1,139 @@
require "rails_helper"
RSpec.describe LogsHelper, type: :helper do
describe "#unique_answers_to_be_cleared" do
let(:result) { unique_answers_to_be_cleared(bulk_upload) }
context "with a lettings bulk upload with various errors" do
let(:bulk_upload) { create(:bulk_upload, :lettings) }
context "with one row" do
before do
errors = [OpenStruct.new(attribute: "field_50", message: "you must answer field 50", category: :not_answered),
OpenStruct.new(attribute: "field_60", message: "some compound error", category: :other_category),
OpenStruct.new(attribute: "field_61", message: "some compound error", category: :other_category),
OpenStruct.new(attribute: "field_61", message: "some other compound error that overlaps with a previous error field", category: :another_category),
OpenStruct.new(attribute: "field_62", message: "some other compound error that overlaps with a previous error field", category: :another_category),
OpenStruct.new(attribute: "field_63", message: "some soft validation error", category: :soft_validation)]
errors.each do |error|
bulk_upload.bulk_upload_errors.create!(
field: error.attribute,
error: error.message,
tenant_code: "test",
property_ref: "test",
row: "test",
cell: "test",
col: "test",
category: error.category,
)
end
end
it "returns the correct unique answers to be cleared" do
expect(result.count).to eq(3)
expect(result.map(&:field)).to match_array(%w[field_60 field_61 field_62])
end
end
context "with multiple rows" do
before do
errors = [OpenStruct.new(attribute: "field_50", message: "you must answer field 50", category: :not_answered, row: 1),
OpenStruct.new(attribute: "field_60", message: "some compound error", category: :other_category, row: 1),
OpenStruct.new(attribute: "field_61", message: "some compound error", category: :other_category, row: 1),
OpenStruct.new(attribute: "field_61", message: "some other compound error that overlaps with a previous error field", category: :another_category, row: 1),
OpenStruct.new(attribute: "field_62", message: "some other compound error that overlaps with a previous error field", category: :another_category, row: 1),
OpenStruct.new(attribute: "field_63", message: "some soft validation error", category: :soft_validation, row: 1),
OpenStruct.new(attribute: "field_50", message: "you must answer field 50", category: :not_answered, row: 2),
OpenStruct.new(attribute: "field_60", message: "some compound error", category: :other_category, row: 2),
OpenStruct.new(attribute: "field_61", message: "some compound error", category: :other_category, row: 2),
OpenStruct.new(attribute: "field_61", message: "some other compound error that overlaps with a previous error field", category: :another_category, row: 2),
OpenStruct.new(attribute: "field_62", message: "some other compound error that overlaps with a previous error field", category: :another_category, row: 2),
OpenStruct.new(attribute: "field_63", message: "some soft validation error", category: :soft_validation, row: 2)]
errors.each do |error|
bulk_upload.bulk_upload_errors.create!(
field: error.attribute,
error: error.message,
tenant_code: "test",
property_ref: "test",
row: error.row,
cell: "test",
col: "test",
category: error.category,
)
end
end
it "returns the correct unique answers to be cleared" do
expect(result.count).to eq(6)
expect(result.map(&:field)).to match_array(%w[field_60 field_61 field_62 field_60 field_61 field_62])
end
end
end
context "with a sales bulk upload with various errors" do
let(:bulk_upload) { create(:bulk_upload, :sales) }
context "with one row" do
before do
errors = [OpenStruct.new(attribute: "field_50", message: "you must answer field 50", category: :not_answered),
OpenStruct.new(attribute: "field_60", message: "some compound error", category: :other_category),
OpenStruct.new(attribute: "field_61", message: "some compound error", category: :other_category),
OpenStruct.new(attribute: "field_61", message: "some other compound error that overlaps with a previous error field", category: :another_category),
OpenStruct.new(attribute: "field_62", message: "some other compound error that overlaps with a previous error field", category: :another_category),
OpenStruct.new(attribute: "field_63", message: "some soft validation error", category: :soft_validation)]
errors.each do |error|
bulk_upload.bulk_upload_errors.create!(
field: error.attribute,
error: error.message,
tenant_code: "test",
property_ref: "test",
row: "test",
cell: "test",
col: "test",
category: error.category,
)
end
end
it "returns the correct unique answers to be cleared" do
expect(result.count).to eq(3)
expect(result.map(&:field)).to match_array(%w[field_60 field_61 field_62])
end
end
context "with multiple rows" do
before do
errors = [OpenStruct.new(attribute: "field_50", message: "you must answer field 50", category: :not_answered, row: 1),
OpenStruct.new(attribute: "field_60", message: "some compound error", category: :other_category, row: 1),
OpenStruct.new(attribute: "field_61", message: "some compound error", category: :other_category, row: 1),
OpenStruct.new(attribute: "field_61", message: "some other compound error that overlaps with a previous error field", category: :another_category, row: 1),
OpenStruct.new(attribute: "field_62", message: "some other compound error that overlaps with a previous error field", category: :another_category, row: 1),
OpenStruct.new(attribute: "field_63", message: "some soft validation error", category: :soft_validation, row: 1),
OpenStruct.new(attribute: "field_50", message: "you must answer field 50", category: :not_answered, row: 2),
OpenStruct.new(attribute: "field_60", message: "some compound error", category: :other_category, row: 2),
OpenStruct.new(attribute: "field_61", message: "some compound error", category: :other_category, row: 2),
OpenStruct.new(attribute: "field_61", message: "some other compound error that overlaps with a previous error field", category: :another_category, row: 2),
OpenStruct.new(attribute: "field_62", message: "some other compound error that overlaps with a previous error field", category: :another_category, row: 2),
OpenStruct.new(attribute: "field_63", message: "some soft validation error", category: :soft_validation, row: 2)]
errors.each do |error|
bulk_upload.bulk_upload_errors.create!(
field: error.attribute,
error: error.message,
tenant_code: "test",
property_ref: "test",
row: error.row,
cell: "test",
col: "test",
category: error.category,
)
end
end
it "returns the correct unique answers to be cleared" do
expect(result.count).to eq(6)
expect(result.map(&:field)).to match_array(%w[field_60 field_61 field_62 field_60 field_61 field_62])
end
end
end
end
end

126
spec/requests/bulk_upload_controller_spec.rb

@ -1,126 +0,0 @@
require "rails_helper"
RSpec.describe BulkUploadController, type: :request do
let(:url) { "/lettings-logs/bulk-upload" }
let(:user) { FactoryBot.create(:user) }
let(:organisation) { user.organisation }
let(:valid_file) { fixture_file_upload("2021_22_lettings_bulk_upload.xlsx", "application/vnd.ms-excel") }
let(:invalid_file) { fixture_file_upload("random.txt", "text/plain") }
let(:empty_file) { fixture_file_upload("2021_22_lettings_bulk_upload_empty.xlsx", "application/vnd.ms-excel") }
before do
allow(Organisation).to receive(:find).with(107_242).and_return(organisation)
end
context "when a user is not signed in" do
describe "GET #start" do
before { get start_bulk_upload_lettings_logs_path, headers:, params: {} }
it "does not let you see the bulk upload page" do
expect(response).to redirect_to("/account/sign-in")
end
end
describe "GET #show" do
before { get url, headers:, params: {} }
it "does not let you see the bulk upload page" do
expect(response).to redirect_to("/account/sign-in")
end
end
describe "POST #bulk upload" do
before { post url, params: { bulk_upload: { lettings_log_bulk_upload: valid_file } } }
it "does not let you submit bulk uploads" do
expect(response).to redirect_to("/account/sign-in")
end
end
end
context "when a user is signed in" do
before do
sign_in user
end
describe "GET #show" do
before do
get url, params: {}
end
it "returns a success response" do
expect(response).to be_successful
end
it "returns a page with a file upload form" do
expect(response.body).to match(/<input id="legacy-bulk-upload-lettings-log-bulk-upload-field" class="govuk-file-upload"/)
expect(response.body).to match(/<button type="submit" formnovalidate="formnovalidate" class="govuk-button"/)
end
end
describe "GET #start" do
before do
Timecop.freeze(time)
get start_bulk_upload_lettings_logs_path
end
after do
Timecop.unfreeze
end
context "when not crossover period" do
let(:time) { Time.utc(2023, 2, 8) }
it "redirects to bulk upload path" do
expect(request).to redirect_to(
bulk_upload_lettings_log_path(
id: "prepare-your-file",
form: { year: 2022 },
),
)
end
end
context "when crossover period" do
let(:time) { Time.utc(2022, 6, 8) }
it "redirects to bulk upload path" do
expect(request).to redirect_to(
bulk_upload_lettings_log_path(id: "year"),
)
end
end
end
describe "POST #bulk upload" do
context "with a valid file based on the upload template" do
let(:request) { post url, params: { bulk_upload: { lettings_log_bulk_upload: valid_file } } }
it "creates lettings logs for each row in the template" do
expect { request }.to change(LettingsLog, :count).by(9)
end
it "redirects to the lettings log index page" do
expect(request).to redirect_to(lettings_logs_path)
end
end
context "with an invalid file type" do
before { post url, params: { bulk_upload: { lettings_log_bulk_upload: invalid_file } } }
it "displays an error message" do
expect(response.body).to match(/Invalid file type/)
end
end
context "with an empty file" do
let(:request) { post url, params: { bulk_upload: { lettings_log_bulk_upload: empty_file } } }
it "displays an error message" do
request
expect(response.body).to match(/No data found/)
end
end
end
end
end

46
spec/requests/bulk_upload_lettings_resume_controller_spec.rb

@ -42,6 +42,7 @@ RSpec.describe BulkUploadLettingsResumeController, type: :request do
expect(response.body).to include("Bulk upload for lettings")
expect(response.body).to include("2022/23")
expect(response.body).to include("View the error report")
expect(response.body).to include("How would you like to fix the errors?")
expect(response.body).to include(bulk_upload.filename)
expect(response.body).not_to include("Cancel")
@ -123,7 +124,10 @@ RSpec.describe BulkUploadLettingsResumeController, type: :request do
expect(response).to be_successful
expect(response.body).to include("You have chosen to upload all logs from this bulk upload.")
expect(response.body).to include("Are you sure you want to upload all logs from this bulk upload?")
expect(response.body).to include("View the error report")
expect(response.body).to include("2 answers will be deleted because they are invalid.")
expect(response.body).to include("See which answers will be deleted")
end
it "sets no cache headers" do
@ -168,4 +172,44 @@ RSpec.describe BulkUploadLettingsResumeController, type: :request do
expect(response).to redirect_to("/lettings-logs/bulk-upload-results/#{bulk_upload.id}/resume")
end
end
describe "GET /lettings-logs/bulk-upload-resume/:ID/deletion-report" do
it "renders the page correctly" do
get "/lettings-logs/bulk-upload-resume/#{bulk_upload.id}/deletion-report"
expect(response).to be_successful
expect(response.body).to include("Bulk upload for lettings")
expect(response.body).to include("2022/23")
expect(response.body).to include("These 2 answers will be deleted if you upload the log")
expect(response.body).to include(bulk_upload.filename)
expect(response.body).to include("Clear this data and upload the logs")
end
it "sets no cache headers" do
get "/lettings-logs/bulk-upload-resume/#{bulk_upload.id}/deletion-report"
expect(response.headers["Cache-Control"]).to eql("no-store")
end
context "and previously told us to fix inline" do
let(:bulk_upload) { create(:bulk_upload, :lettings, user:, bulk_upload_errors:, choice: "create-fix-inline") }
it "redirects to chosen" do
get "/lettings-logs/bulk-upload-resume/#{bulk_upload.id}/deletion-report"
expect(response).to redirect_to("/lettings-logs/bulk-upload-resume/#{bulk_upload.id}/chosen")
end
end
context "and previously told us to bulk confirm soft validations" do
let(:bulk_upload) { create(:bulk_upload, :lettings, user:, bulk_upload_errors:, choice: "bulk-confirm-soft-validations") }
it "redirects to soft validations check chosen" do
get "/lettings-logs/bulk-upload-resume/#{bulk_upload.id}/deletion-report"
expect(response).to redirect_to("/lettings-logs/bulk-upload-soft-validations-check/#{bulk_upload.id}/chosen")
end
end
end
end

4
spec/requests/bulk_upload_sales_results_controller_spec.rb

@ -2,11 +2,13 @@ require "rails_helper"
RSpec.describe BulkUploadSalesResultsController, type: :request do
let(:user) { create(:user) }
let(:support_user) { create(:user, :support) }
let(:bulk_upload) { create(:bulk_upload, :sales, user:, bulk_upload_errors:) }
let(:bulk_upload_errors) { create_list(:bulk_upload_error, 2) }
let(:viewing_user) { user }
before do
sign_in user
sign_in viewing_user
end
describe "GET /sales-logs/bulk-upload-results/:ID" do

46
spec/requests/bulk_upload_sales_resume_controller_spec.rb

@ -17,6 +17,7 @@ RSpec.describe BulkUploadSalesResumeController, type: :request do
expect(response.body).to include("Bulk upload for sales")
expect(response.body).to include("2022/23")
expect(response.body).to include("View the error report")
expect(response.body).to include("How would you like to fix the errors?")
expect(response.body).to include(bulk_upload.filename)
expect(response.body).not_to include("Cancel")
@ -98,7 +99,10 @@ RSpec.describe BulkUploadSalesResumeController, type: :request do
expect(response).to be_successful
expect(response.body).to include("You have chosen to upload all logs from this bulk upload.")
expect(response.body).to include("Are you sure you want to upload all logs from this bulk upload?")
expect(response.body).to include("View the error report")
expect(response.body).to include("2 answers will be deleted because they are invalid.")
expect(response.body).to include("See which answers will be deleted")
end
it "sets no cache headers" do
@ -158,4 +162,44 @@ RSpec.describe BulkUploadSalesResumeController, type: :request do
end
end
end
describe "GET /sales-logs/bulk-upload-resume/:ID/deletion-report" do
it "renders the page correctly" do
get "/sales-logs/bulk-upload-resume/#{bulk_upload.id}/deletion-report"
expect(response).to be_successful
expect(response.body).to include("Bulk upload for sales")
expect(response.body).to include("2022/23")
expect(response.body).to include("These 2 answers will be deleted if you upload the log")
expect(response.body).to include(bulk_upload.filename)
expect(response.body).to include("Clear this data and upload the logs")
end
it "sets no cache headers" do
get "/sales-logs/bulk-upload-resume/#{bulk_upload.id}/deletion-report"
expect(response.headers["Cache-Control"]).to eql("no-store")
end
context "and previously told us to fix inline" do
let(:bulk_upload) { create(:bulk_upload, :sales, user:, bulk_upload_errors:, choice: "create-fix-inline") }
it "redirects to chosen" do
get "/sales-logs/bulk-upload-resume/#{bulk_upload.id}/fix-choice"
expect(response).to redirect_to("/sales-logs/bulk-upload-resume/#{bulk_upload.id}/chosen")
end
end
context "and previously told us to bulk confirm soft validations" do
let(:bulk_upload) { create(:bulk_upload, :sales, user:, bulk_upload_errors:, choice: "bulk-confirm-soft-validations") }
it "redirects to soft validations check chosen" do
get "/sales-logs/bulk-upload-resume/#{bulk_upload.id}/fix-choice"
expect(response).to redirect_to("/sales-logs/bulk-upload-soft-validations-check/#{bulk_upload.id}/chosen")
end
end
end
end

Loading…
Cancel
Save