diff --git a/app/helpers/filters_helper.rb b/app/helpers/filters_helper.rb index 80f3199d5..f3d03cc01 100644 --- a/app/helpers/filters_helper.rb +++ b/app/helpers/filters_helper.rb @@ -91,8 +91,8 @@ private filters.each.sum do |category, category_filters| if %w[status years bulk_upload_id].include?(category) category_filters.count(&:present?) - elsif %w[assigned_to organisation].include?(category) - category_filters != "all" ? 1 : 0 + elsif %w[user owning_organisation managing_organisation].include?(category) + 1 else 0 end diff --git a/app/helpers/logs_helper.rb b/app/helpers/logs_helper.rb index becc2d213..23a854cde 100644 --- a/app/helpers/logs_helper.rb +++ b/app/helpers/logs_helper.rb @@ -41,9 +41,19 @@ module LogsHelper end end - def pluralize_logs_and_errors_warning(log_count, error_count) - is_or_are = log_count == 1 ? "is" : "are" - need_or_needs = error_count == 1 ? "needs" : "need" - "There #{is_or_are} #{pluralize(log_count, 'log')} in this bulk upload with #{pluralize(error_count, 'error')} that still #{need_or_needs} to be fixed after upload." + def logs_and_errors_warning(bulk_upload) + this_or_these_errors = bulk_upload.bulk_upload_errors.count == 1 ? "This error" : "These errors" + + "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." + end + + def logs_and_soft_validations_warning(bulk_upload) + this_or_these_unexpected_answers = bulk_upload.bulk_upload_errors.count == 1 ? "This unexpected answer" : "These unexpected answers" + + "You will upload #{pluralize(bulk_upload.total_logs_count, 'log')}. There are unexpected answers in #{pluralize(bulk_upload.logs_with_errors_count, 'log')}, and #{pluralize(bulk_upload.bulk_upload_errors.count, 'unexpected answer')} in total. #{this_or_these_unexpected_answers} will be marked as correct." + end + + 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 end diff --git a/app/models/bulk_upload.rb b/app/models/bulk_upload.rb index 65a25507b..e98103b6d 100644 --- a/app/models/bulk_upload.rb +++ b/app/models/bulk_upload.rb @@ -107,6 +107,10 @@ class BulkUpload < ApplicationRecord end end + def logs_with_errors_count + bulk_upload_errors.distinct.count("row") + end + private def generate_identifier diff --git a/app/models/forms/bulk_upload_lettings_resume/fix_choice.rb b/app/models/forms/bulk_upload_lettings_resume/fix_choice.rb index 76ee10d17..cf86f2e16 100644 --- a/app/models/forms/bulk_upload_lettings_resume/fix_choice.rb +++ b/app/models/forms/bulk_upload_lettings_resume/fix_choice.rb @@ -39,9 +39,9 @@ module Forms def recommendation if BulkUploadErrorSummaryTableComponent.new(bulk_upload:).errors? - "For this many errors we recommend to fix errors in the CSV and re-upload as you may be able to edit many fields at once in a CSV." + "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." else - "For this many errors we recommend to upload logs and fix errors on site as you can easily see the questions and select the appropriate answer." + "We recommend uploading logs and fixing errors on site as you can easily see the questions and select the appropriate answer. However, you can also fix these errors in the CSV." end end diff --git a/app/models/forms/bulk_upload_sales_resume/fix_choice.rb b/app/models/forms/bulk_upload_sales_resume/fix_choice.rb index fc565e2f6..d26cd073d 100644 --- a/app/models/forms/bulk_upload_sales_resume/fix_choice.rb +++ b/app/models/forms/bulk_upload_sales_resume/fix_choice.rb @@ -39,9 +39,9 @@ module Forms def recommendation if BulkUploadErrorSummaryTableComponent.new(bulk_upload:).errors? - "For this many errors we recommend to fix errors in the CSV and re-upload as you may be able to edit many fields at once in a CSV." + "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." else - "For this many errors we recommend to upload logs and fix errors on site as you can easily see the questions and select the appropriate answer." + "We recommend uploading logs and fixing errors on site as you can easily see the questions and select the appropriate answer. However, you can also fix these errors in the CSV." end end diff --git a/app/models/validations/household_validations.rb b/app/models/validations/household_validations.rb index 5609f1e1c..07a3fa467 100644 --- a/app/models/validations/household_validations.rb +++ b/app/models/validations/household_validations.rb @@ -60,9 +60,9 @@ module Validations::HouseholdValidations record.errors.add :prevten, :non_temp_accommodation, message: I18n.t("validations.household.prevten.non_temp_accommodation") end - if record.age1.present? && record.age1 > 19 && record.previous_tenancy_was_foster_care? - record.errors.add :prevten, :over_20_foster_care, message: I18n.t("validations.household.prevten.over_20_foster_care") - record.errors.add :age1, I18n.t("validations.household.age.lead.over_20") + if record.age1.present? && record.age1 > 25 && record.previous_tenancy_was_foster_care? + record.errors.add :prevten, :over_25_foster_care, message: I18n.t("validations.household.prevten.over_25_foster_care") + record.errors.add :age1, I18n.t("validations.household.age.lead.over_25") end if record.sex1 == "M" && record.previous_tenancy_was_refuge? diff --git a/app/services/bulk_upload/lettings/validator.rb b/app/services/bulk_upload/lettings/validator.rb index 14c31b6d7..0759a5cd2 100644 --- a/app/services/bulk_upload/lettings/validator.rb +++ b/app/services/bulk_upload/lettings/validator.rb @@ -73,6 +73,10 @@ class BulkUpload::Lettings::Validator row_parsers.any?(&:log_already_exists?) end + def total_logs_count + csv_parser.body_rows.count + end + private # n^2 algo diff --git a/app/services/bulk_upload/processor.rb b/app/services/bulk_upload/processor.rb index 711996fcf..3fbb1e2d4 100644 --- a/app/services/bulk_upload/processor.rb +++ b/app/services/bulk_upload/processor.rb @@ -10,6 +10,7 @@ class BulkUpload::Processor download + @bulk_upload.update!(total_logs_count: validator.total_logs_count) return send_failure_mail(errors: validator.errors.full_messages) if validator.invalid? validator.call diff --git a/app/services/bulk_upload/sales/validator.rb b/app/services/bulk_upload/sales/validator.rb index 9552bbc69..5e5cf3a89 100644 --- a/app/services/bulk_upload/sales/validator.rb +++ b/app/services/bulk_upload/sales/validator.rb @@ -67,6 +67,10 @@ class BulkUpload::Sales::Validator errors.count == errors.where(category: "soft_validation").count && errors.count.positive? end + def total_logs_count + csv_parser.body_rows.count + end + private # n^2 algo diff --git a/app/services/imports/lettings_logs_import_service.rb b/app/services/imports/lettings_logs_import_service.rb index b25318e43..e798665a9 100644 --- a/app/services/imports/lettings_logs_import_service.rb +++ b/app/services/imports/lettings_logs_import_service.rb @@ -308,7 +308,7 @@ module Imports %i[referral internal_transfer_non_social_housing] => %w[referral], %i[referral internal_transfer_fixed_or_lifetime] => %w[referral], %i[tenancylength tenancylength_invalid] => %w[tenancylength tenancy], - %i[prevten over_20_foster_care] => %w[prevten age1], + %i[prevten over_25_foster_care] => %w[prevten age1], %i[prevten non_temp_accommodation] => %w[prevten rsnvac], %i[joint not_joint_tenancy] => %w[joint], %i[offered outside_the_range] => %w[offered], diff --git a/app/views/bulk_upload_lettings_resume/confirm.html.erb b/app/views/bulk_upload_lettings_resume/confirm.html.erb index a2e3cef8e..841fd9e5e 100644 --- a/app/views/bulk_upload_lettings_resume/confirm.html.erb +++ b/app/views/bulk_upload_lettings_resume/confirm.html.erb @@ -5,12 +5,12 @@
Bulk upload for lettings (<%= @bulk_upload.year_combo %>) -

Are you sure you want to upload all logs from this bulk upload?

+

You have chosen to upload all logs from this bulk upload.

-

<%= pluralize_logs_and_errors_warning(@bulk_upload.logs.count, @bulk_upload.bulk_upload_errors.count) %>

+

<%= logs_and_errors_warning(@bulk_upload) %>

<%= form_with model: @form, scope: :form, url: page_bulk_upload_lettings_resume_path(@bulk_upload, page: "confirm"), method: :patch do |f| %> - <%= f.govuk_submit %> + <%= f.govuk_submit "Confirm" %> <%= govuk_button_link_to "Cancel", @form.back_path, secondary: true %> <% end %> 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 05af0fd35..0a46c8c35 100644 --- a/app/views/bulk_upload_lettings_resume/fix_choice.html.erb +++ b/app/views/bulk_upload_lettings_resume/fix_choice.html.erb @@ -4,24 +4,33 @@ <%= f.govuk_error_summary %> Bulk upload for lettings (<%= @bulk_upload.year_combo %>) -

How would you like to fix <%= pluralize(@bulk_upload.bulk_upload_errors.count, "error") %>?

+

How would you like to fix the errors?

File: <%= @bulk_upload.filename %>
+
+ <%= bulk_upload_error_summary(@bulk_upload) %> +
+
<%= @form.recommendation %>
<%= govuk_details(summary_text: "How to choose between fixing errors on the CORE site or in the CSV") do %> -

When it comes to fixing errors, there are pros and cons to doing it on a CSV versus doing it on a website.

- -

Fixing errors on a CSV file can be beneficial because it allows you to easily make changes to multiple records at once, and you can use tools like Excel to quickly identify and correct errors. However, if the CSV file is not properly formatted, it can be difficult to identify which records contain errors.

- -

Fixing errors on a website can be convenient because you can see the data in context and make changes in real-time. However, this approach can be time-consuming if you need to make changes to multiple records, and it may be more difficult to identify errors in a large dataset.

- -

Ultimately, the best approach will depend on the specific situation and the nature of the errors that need to be fixed.

+

You may find it easier to fix the errors in the CSV file if:

+ +

You may find it easier to fix the errors on the CORE site if:

+ <% end %> <%= f.govuk_collection_radio_buttons :choice, diff --git a/app/views/bulk_upload_lettings_soft_validations_check/confirm.html.erb b/app/views/bulk_upload_lettings_soft_validations_check/confirm.html.erb index 095bf77b4..fcb039eac 100644 --- a/app/views/bulk_upload_lettings_soft_validations_check/confirm.html.erb +++ b/app/views/bulk_upload_lettings_soft_validations_check/confirm.html.erb @@ -5,9 +5,9 @@
Bulk upload for lettings (<%= @bulk_upload.year_combo %>) -

Are you sure you want to upload all logs from this bulk upload?

+

You have chosen to upload all logs from this bulk upload.

-

There are <%= pluralize(@bulk_upload.logs.count, "log") %> in this bulk upload, and <%= pluralize(@bulk_upload.bulk_upload_errors.count, "unexpected answer") %> will be marked as correct.

+

<%= logs_and_soft_validations_warning(@bulk_upload) %>

<%= form_with model: @form, scope: :form, url: page_bulk_upload_lettings_soft_validations_check_path(@bulk_upload, page: "confirm"), method: :patch do |f| %> <%= f.govuk_submit %> diff --git a/app/views/bulk_upload_sales_resume/confirm.html.erb b/app/views/bulk_upload_sales_resume/confirm.html.erb index e0081f46b..05a8cfd41 100644 --- a/app/views/bulk_upload_sales_resume/confirm.html.erb +++ b/app/views/bulk_upload_sales_resume/confirm.html.erb @@ -5,9 +5,9 @@
Bulk upload for sales (<%= @bulk_upload.year_combo %>) -

Are you sure you want to upload all logs from this bulk upload?

+

You have chosen to upload all logs from this bulk upload.

-

There are <%= pluralize(@bulk_upload.logs.count, "log") %> in this bulk upload with <%= pluralize(@bulk_upload.bulk_upload_errors.count, "error") %> that still need to be fixed after upload.

+

<%= logs_and_errors_warning(@bulk_upload) %>

<%= form_with model: @form, scope: :form, url: page_bulk_upload_sales_resume_path(@bulk_upload, page: "confirm"), method: :patch do |f| %> <%= f.govuk_submit %> 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 050eb4da2..dcdd2b081 100644 --- a/app/views/bulk_upload_sales_resume/fix_choice.html.erb +++ b/app/views/bulk_upload_sales_resume/fix_choice.html.erb @@ -4,24 +4,33 @@ <%= f.govuk_error_summary %> Bulk upload for sales (<%= @bulk_upload.year_combo %>) -

How would you like to fix <%= pluralize(@bulk_upload.bulk_upload_errors.count, "error") %>?

+

How would you like to fix the errors?

File: <%= @bulk_upload.filename %>
+
+ <%= bulk_upload_error_summary(@bulk_upload) %> +
+
<%= @form.recommendation %>
<%= govuk_details(summary_text: "How to choose between fixing errors on the CORE site or in the CSV") do %> -

When it comes to fixing errors, there are pros and cons to doing it on a CSV versus doing it on a website.

- -

Fixing errors on a CSV file can be beneficial because it allows you to easily make changes to multiple records at once, and you can use tools like Excel to quickly identify and correct errors. However, if the CSV file is not properly formatted, it can be difficult to identify which records contain errors.

- -

Fixing errors on a website can be convenient because you can see the data in context and make changes in real-time. However, this approach can be time-consuming if you need to make changes to multiple records, and it may be more difficult to identify errors in a large dataset.

- -

Ultimately, the best approach will depend on the specific situation and the nature of the errors that need to be fixed.

+

You may find it easier to fix the errors in the CSV file if:

+
    +
  • you have a lot of errors
  • +
  • the CSV file is formatted incorrectly and you can see where the errors are
  • +
  • you need to fix multiple errors at once
  • +
+

You may find it easier to fix the errors on the CORE site if:

+
    +
  • you need to see the data in context
  • +
  • you have a smaller file, with a few errors
  • +
  • you are not sure where the errors are
  • +
<% end %> <%= f.govuk_collection_radio_buttons :choice, diff --git a/app/views/bulk_upload_sales_soft_validations_check/confirm.html.erb b/app/views/bulk_upload_sales_soft_validations_check/confirm.html.erb index 11c69c1eb..ca9f7fd4f 100644 --- a/app/views/bulk_upload_sales_soft_validations_check/confirm.html.erb +++ b/app/views/bulk_upload_sales_soft_validations_check/confirm.html.erb @@ -5,9 +5,9 @@
Bulk upload for sales (<%= @bulk_upload.year_combo %>) -

Are you sure you want to upload all logs from this bulk upload?

+

You have chosen to upload all logs from this bulk upload.

-

There are <%= pluralize(@bulk_upload.logs.count, "log") %> in this bulk upload, and <%= pluralize(@bulk_upload.bulk_upload_errors.count, "unexpected answer") %> will be marked as correct.

+

<%= logs_and_soft_validations_warning(@bulk_upload) %>

<%= form_with model: @form, scope: :form, url: page_bulk_upload_sales_soft_validations_check_path(@bulk_upload, page: "confirm"), method: :patch do |f| %> <%= f.govuk_submit %> diff --git a/app/views/logs/index.html.erb b/app/views/logs/index.html.erb index eac53ed7c..12bddff79 100644 --- a/app/views/logs/index.html.erb +++ b/app/views/logs/index.html.erb @@ -28,7 +28,7 @@ <% else %> <%= render partial: "organisations/headings", locals: { - main: "You need to fix #{pluralize(@pagy.count, 'log')} from your bulk upload", + main: "Fix the errors from this bulk upload", sub: "#{log_type_for_controller(controller).capitalize} logs (#{@bulk_upload.year_combo})", } %> @@ -36,7 +36,7 @@

- The following logs are from your recent bulk upload. They have some incorrect or incomplete data. You’ll need to answer a few more questions for each one to mark them as complete. + You have uploaded <%= pluralize(@bulk_upload.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. Select the log to fix the errors.

diff --git a/config/locales/en.yml b/config/locales/en.yml index 7d9f3d087..07142821d 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -442,7 +442,7 @@ en: must_be_16_19: "Person must be aged 16-19 if they are a student and have relationship ‘child’" partner_under_16: "Cannot be under 16 if the relationship is partner" lead: - over_20: "The lead tenant must be under 20 as you told us their housing situation immediately before this letting was a children’s home or foster care" + over_25: "The lead tenant must be under 26 as you told us their housing situation immediately before this letting was a children’s home or foster care" ecstat: retired_over_70: "Person %{person_num} must be retired if over 70" child_under_16: "Person %{person_num}’s working situation must be ‘child under 16’ as you told us they’re under 16" @@ -476,7 +476,7 @@ en: no_and_dont_know_disabled_needs_conjunction: "No disabled access needs and don’t know disabled access needs cannot be selected together" prevten: non_temp_accommodation: "Answer cannot be non-temporary accommodation as this is a re-let to a tenant who occupied the same property as temporary accommodation" - over_20_foster_care: "Answer cannot be a children’s home or foster care as the lead tenant is 20 or older" + over_25_foster_care: "Answer cannot be a children’s home or foster care as the lead tenant is 26 or older" male_refuge: "Answer cannot be a refuge as the lead tenant identifies as male" internal_transfer: "Answer cannot be %{prevten} as this tenancy is an internal transfer" la_general_needs: diff --git a/db/migrate/20230713140231_add_total_logs_count_to_bulk_upload.rb b/db/migrate/20230713140231_add_total_logs_count_to_bulk_upload.rb new file mode 100644 index 000000000..fbaeacb37 --- /dev/null +++ b/db/migrate/20230713140231_add_total_logs_count_to_bulk_upload.rb @@ -0,0 +1,5 @@ +class AddTotalLogsCountToBulkUpload < ActiveRecord::Migration[7.0] + def change + add_column :bulk_uploads, :total_logs_count, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index 177e171a9..81d233874 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_07_18_151955) do +ActiveRecord::Schema[7.0].define(version: 2023_07_13_140231) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -40,6 +40,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_07_18_151955) do t.text "filename" t.integer "needstype" t.text "choice" + t.integer "total_logs_count" t.index ["identifier"], name: "index_bulk_uploads_on_identifier", unique: true t.index ["user_id"], name: "index_bulk_uploads_on_user_id" end @@ -432,7 +433,6 @@ ActiveRecord::Schema[7.0].define(version: 2023_07_18_151955) do t.integer "unspecified_units" t.string "old_org_id" t.string "old_visible_id" - t.datetime "merge_date" t.index ["old_visible_id"], name: "index_organisations_on_old_visible_id", unique: true end diff --git a/spec/features/lettings_log_spec.rb b/spec/features/lettings_log_spec.rb index 8d245fffe..f849decc1 100644 --- a/spec/features/lettings_log_spec.rb +++ b/spec/features/lettings_log_spec.rb @@ -59,9 +59,17 @@ RSpec.describe "Lettings Log Features" do context "when filtering logs" do let(:user) { create(:user, last_sign_in_at: Time.zone.now) } + let(:stock_owner_1) { create(:organisation, name: "stock owner 1") } + let(:stock_owner_2) { create(:organisation, name: "stock owner 2") } + let(:managing_agent_1) { create(:organisation, name: "managing agent 1") } + let(:managing_agent_2) { create(:organisation, name: "managing agent 2") } context "when I am signed in" do before do + FactoryBot.create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: stock_owner_1) + FactoryBot.create(:organisation_relationship, child_organisation: user.organisation, parent_organisation: stock_owner_2) + FactoryBot.create(:organisation_relationship, child_organisation: managing_agent_1, parent_organisation: user.organisation) + FactoryBot.create(:organisation_relationship, child_organisation: managing_agent_2, parent_organisation: user.organisation) visit("/lettings-logs") fill_in("user[email]", with: user.email) fill_in("user[password]", with: user.password) @@ -80,11 +88,15 @@ RSpec.describe "Lettings Log Features" do check("Not started") check("In progress") choose("You") + choose("Specific owning organisation") + select(stock_owner_1.name, from: "owning_organisation") + choose("Specific managing organisation") + select(managing_agent_1.name, from: "managing_organisation") click_button("Apply filters") end it "displays the filters component with a correct count and clear button" do - expect(page).to have_content("3 filters applied") + expect(page).to have_content("5 filters applied") expect(page).to have_content("Clear") end diff --git a/spec/helpers/filters_helper_spec.rb b/spec/helpers/filters_helper_spec.rb index 58ecf738d..10b18ac1e 100644 --- a/spec/helpers/filters_helper_spec.rb +++ b/spec/helpers/filters_helper_spec.rb @@ -222,7 +222,7 @@ RSpec.describe FiltersHelper do "assigned_to" => "all", "status" => [""], "years" => [""], - "organisation" => "all", + "organisation_select" => "all", } end @@ -237,7 +237,8 @@ RSpec.describe FiltersHelper do "assigned_to" => "all", "status" => %w[in_progress completed], "years" => [""], - "organisation" => 2, + "organisation_select" => "specific_org", + "managing_organisation" => 2, } end diff --git a/spec/models/validations/household_validations_spec.rb b/spec/models/validations/household_validations_spec.rb index caf53695e..f44c5d255 100644 --- a/spec/models/validations/household_validations_spec.rb +++ b/spec/models/validations/household_validations_spec.rb @@ -624,15 +624,15 @@ RSpec.describe Validations::HouseholdValidations do end end - context "when the lead tenant is over 20" do + context "when the lead tenant is over 25" do it "cannot be children's home/foster care" do record.prevten = 13 - record.age1 = 21 + record.age1 = 26 household_validator.validate_previous_housing_situation(record) expect(record.errors["prevten"]) - .to include(match I18n.t("validations.household.prevten.over_20_foster_care")) + .to include(match I18n.t("validations.household.prevten.over_25_foster_care")) expect(record.errors["age1"]) - .to include(match I18n.t("validations.household.age.lead.over_20")) + .to include(match I18n.t("validations.household.age.lead.over_25")) end end diff --git a/spec/requests/bulk_upload_lettings_resume_controller_spec.rb b/spec/requests/bulk_upload_lettings_resume_controller_spec.rb index 7ba8bbc85..fddba7190 100644 --- a/spec/requests/bulk_upload_lettings_resume_controller_spec.rb +++ b/spec/requests/bulk_upload_lettings_resume_controller_spec.rb @@ -25,7 +25,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("How would you like to fix 2 errors?") + 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") end @@ -106,7 +106,7 @@ RSpec.describe BulkUploadLettingsResumeController, type: :request do expect(response).to be_successful - expect(response.body).to include("Are you sure") + expect(response.body).to include("You have chosen to upload all logs from this bulk upload.") end it "sets no cache headers" do diff --git a/spec/requests/bulk_upload_lettings_soft_validations_check_controller_spec.rb b/spec/requests/bulk_upload_lettings_soft_validations_check_controller_spec.rb index 0603e3d34..904c18573 100644 --- a/spec/requests/bulk_upload_lettings_soft_validations_check_controller_spec.rb +++ b/spec/requests/bulk_upload_lettings_soft_validations_check_controller_spec.rb @@ -2,7 +2,7 @@ require "rails_helper" RSpec.describe BulkUploadLettingsSoftValidationsCheckController, type: :request do let(:user) { create(:user) } - let(:bulk_upload) { create(:bulk_upload, :lettings, user:, bulk_upload_errors:) } + let(:bulk_upload) { create(:bulk_upload, :lettings, user:, bulk_upload_errors:, total_logs_count: 2) } let(:bulk_upload_errors) { create_list(:bulk_upload_error, 2) } before do @@ -98,8 +98,8 @@ RSpec.describe BulkUploadLettingsSoftValidationsCheckController, type: :request expect(response).to be_successful - expect(response.body).to include("Are you sure you want to upload all logs from this bulk upload?") - expect(response.body).to include("There are 2 logs in this bulk upload, and 2 unexpected answers will be marked as correct.") + expect(response.body).to include("You have chosen to upload all logs from this bulk upload.") + expect(response.body).to include("You will upload 2 logs. There are unexpected answers in 2 logs, and 2 unexpected answers in total. These unexpected answers will be marked as correct.") expect(response.body).not_to include("You’ve successfully uploaded") end end diff --git a/spec/requests/bulk_upload_sales_resume_controller_spec.rb b/spec/requests/bulk_upload_sales_resume_controller_spec.rb index 9c0a7112c..9e4ec4a2b 100644 --- a/spec/requests/bulk_upload_sales_resume_controller_spec.rb +++ b/spec/requests/bulk_upload_sales_resume_controller_spec.rb @@ -25,7 +25,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("How would you like to fix 2 errors?") + 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") end @@ -106,7 +106,7 @@ RSpec.describe BulkUploadSalesResumeController, type: :request do expect(response).to be_successful - expect(response.body).to include("Are you sure") + expect(response.body).to include("You have chosen to upload all logs from this bulk upload.") end it "sets no cache headers" do diff --git a/spec/requests/bulk_upload_sales_soft_validations_check_controller_spec.rb b/spec/requests/bulk_upload_sales_soft_validations_check_controller_spec.rb index 0b496aea4..3225f0f3f 100644 --- a/spec/requests/bulk_upload_sales_soft_validations_check_controller_spec.rb +++ b/spec/requests/bulk_upload_sales_soft_validations_check_controller_spec.rb @@ -2,7 +2,7 @@ require "rails_helper" RSpec.describe BulkUploadSalesSoftValidationsCheckController, type: :request do let(:user) { create(:user) } - let(:bulk_upload) { create(:bulk_upload, :sales, user:, bulk_upload_errors:) } + let(:bulk_upload) { create(:bulk_upload, :sales, user:, bulk_upload_errors:, total_logs_count: 2) } let(:bulk_upload_errors) { create_list(:bulk_upload_error, 2) } before do @@ -98,8 +98,8 @@ RSpec.describe BulkUploadSalesSoftValidationsCheckController, type: :request do expect(response).to be_successful - expect(response.body).to include("Are you sure you want to upload all logs from this bulk upload?") - expect(response.body).to include("There are 2 logs in this bulk upload, and 2 unexpected answers will be marked as correct.") + expect(response.body).to include("You have chosen to upload all logs from this bulk upload.") + expect(response.body).to include("You will upload 2 logs. There are unexpected answers in 2 logs, and 2 unexpected answers in total. These unexpected answers will be marked as correct.") expect(response.body).not_to include("You’ve successfully uploaded") end end diff --git a/spec/requests/lettings_logs_controller_spec.rb b/spec/requests/lettings_logs_controller_spec.rb index ae8770c7a..4e4cb96c2 100644 --- a/spec/requests/lettings_logs_controller_spec.rb +++ b/spec/requests/lettings_logs_controller_spec.rb @@ -477,6 +477,10 @@ RSpec.describe LettingsLogsController, type: :request do let!(:included_log) { create(:lettings_log, :in_progress, bulk_upload:, owning_organisation: organisation) } let!(:excluded_log) { create(:lettings_log, :in_progress, owning_organisation: organisation, tenancycode: "fake_code") } + before do + create(:bulk_upload_error, bulk_upload:, col: "A", row: 1) + end + it "returns logs only associated with the bulk upload" do get "/lettings-logs?bulk_upload_id[]=#{bulk_upload.id}" @@ -484,9 +488,9 @@ RSpec.describe LettingsLogsController, type: :request do expect(page).not_to have_content(excluded_log.tenancycode) end - it "displays how many logs remaining to fix" do + it "dislays bulk uplaoad header" do get "/lettings-logs?bulk_upload_id[]=#{bulk_upload.id}" - expect(page).to have_content("You need to fix 1 log") + expect(page).to have_content("Fix the errors from this bulk upload") end it "displays filter" do @@ -517,7 +521,7 @@ RSpec.describe LettingsLogsController, type: :request do it "displays card with help info" do get "/lettings-logs?bulk_upload_id[]=#{bulk_upload.id}" - expect(page).to have_content("The following logs are from your recent bulk upload") + expect(page).to have_content("You have uploaded 1 log. There are errors in 1 log, and 1 error in total. Select the log to fix the errors.") end it "displays meta info about the bulk upload" do diff --git a/spec/requests/sales_logs_controller_spec.rb b/spec/requests/sales_logs_controller_spec.rb index 0d5c0e029..e4e8322e7 100644 --- a/spec/requests/sales_logs_controller_spec.rb +++ b/spec/requests/sales_logs_controller_spec.rb @@ -355,6 +355,10 @@ RSpec.describe SalesLogsController, type: :request do let!(:included_log) { create(:sales_log, :in_progress, bulk_upload:, owning_organisation: organisation) } let!(:excluded_log) { create(:sales_log, :in_progress, owning_organisation: organisation) } + before do + create(:bulk_upload_error, bulk_upload:, col: "A", row: 1) + end + it "returns logs only associated with the bulk upload" do get "/sales-logs?bulk_upload_id[]=#{bulk_upload.id}" @@ -362,9 +366,9 @@ RSpec.describe SalesLogsController, type: :request do expect(page).not_to have_content(excluded_log.id) end - it "displays how many logs remaining to fix" do + it "dislays bulk upload banner" do get "/sales-logs?bulk_upload_id[]=#{bulk_upload.id}" - expect(page).to have_content("You need to fix 1 log") + expect(page).to have_content("Fix the errors from this bulk upload") end it "displays filter" do @@ -395,7 +399,7 @@ RSpec.describe SalesLogsController, type: :request do it "displays card with help info" do get "/sales-logs?bulk_upload_id[]=#{bulk_upload.id}" - expect(page).to have_content("The following logs are from your recent bulk upload") + expect(page).to have_content("You have uploaded 1 log. There are errors in 1 log, and 1 error in total. Select the log to fix the errors.") end it "displays meta info about the bulk upload" do diff --git a/spec/services/bulk_upload/lettings/validator_spec.rb b/spec/services/bulk_upload/lettings/validator_spec.rb index fa913e091..a9c181b00 100644 --- a/spec/services/bulk_upload/lettings/validator_spec.rb +++ b/spec/services/bulk_upload/lettings/validator_spec.rb @@ -65,6 +65,10 @@ RSpec.describe BulkUpload::Lettings::Validator do it "is valid" do expect(validator).to be_valid end + + it "returns correct total logs count" do + expect(validator.total_logs_count).to be(1) + end end context "and file has too few valid headers" do diff --git a/spec/services/bulk_upload/processor_spec.rb b/spec/services/bulk_upload/processor_spec.rb index 2314a325f..733f8ed5e 100644 --- a/spec/services/bulk_upload/processor_spec.rb +++ b/spec/services/bulk_upload/processor_spec.rb @@ -33,6 +33,7 @@ RSpec.describe BulkUpload::Processor do BulkUpload::Lettings::Validator, invalid?: true, call: nil, + total_logs_count: 1, errors: [], ) end @@ -58,6 +59,13 @@ RSpec.describe BulkUpload::Processor do expect(mock_validator).not_to have_received(:call) end + + it "sets total number of logs on bulk upload" do + processor.call + + bulk_upload.reload + expect(bulk_upload.total_logs_count).to eq(1) + end end context "when the bulk upload processing throws an error" do @@ -74,6 +82,7 @@ RSpec.describe BulkUpload::Processor do instance_double( BulkUpload::Lettings::Validator, invalid?: false, + total_logs_count: 1, ) end @@ -119,6 +128,7 @@ RSpec.describe BulkUpload::Processor do BulkUpload::Lettings::Validator, invalid?: false, call: nil, + total_logs_count: 1, any_setup_errors?: true, ) end diff --git a/spec/services/bulk_upload/sales/validator_spec.rb b/spec/services/bulk_upload/sales/validator_spec.rb index 851109772..8d543b4a6 100644 --- a/spec/services/bulk_upload/sales/validator_spec.rb +++ b/spec/services/bulk_upload/sales/validator_spec.rb @@ -246,4 +246,31 @@ RSpec.describe BulkUpload::Sales::Validator do end end end + + describe "#total_logs_count?" do + around do |example| + Timecop.freeze(Time.zone.local(2023, 2, 22)) do + Singleton.__init__(FormHandler) + example.run + end + Timecop.return + Singleton.__init__(FormHandler) + end + + context "when all logs are valid" do + let(:target_path) { file_fixture("completed_2022_23_sales_bulk_upload.csv") } + + before do + target_array = File.open(target_path).readlines + target_array[0..118].each do |line| + file.write line + end + file.rewind + end + + it "returns correct total logs count" do + expect(validator.total_logs_count).to be(1) + end + end + end end diff --git a/spec/services/imports/lettings_logs_import_service_spec.rb b/spec/services/imports/lettings_logs_import_service_spec.rb index bc4bc91b4..9a4ef025e 100644 --- a/spec/services/imports/lettings_logs_import_service_spec.rb +++ b/spec/services/imports/lettings_logs_import_service_spec.rb @@ -296,16 +296,16 @@ RSpec.describe Imports::LettingsLogsImportService do end end - context "and an lead tenant must be under 20 if childrens home or foster care" do + context "and an lead tenant must be under 26 if childrens home or foster care" do before do lettings_log_xml.at_xpath("//meta:status").content = "submitted" lettings_log_xml.at_xpath("//xmlns:Q11").content = "13" - lettings_log_xml.at_xpath("//xmlns:P1Age").content = "22" + lettings_log_xml.at_xpath("//xmlns:P1Age").content = "26" end it "intercepts the relevant validation error" do - expect(logger).to receive(:warn).with(/Removing prevten with error: Answer cannot be a children’s home or foster care as the lead tenant is 20 or older/) - expect(logger).to receive(:warn).with(/Removing age1 with error: Answer cannot be a children’s home or foster care as the lead tenant is 20 or older/) + expect(logger).to receive(:warn).with(/Removing prevten with error: Answer cannot be a children’s home or foster care as the lead tenant is 26 or older/) + expect(logger).to receive(:warn).with(/Removing age1 with error: Answer cannot be a children’s home or foster care as the lead tenant is 26 or older/) expect { lettings_log_service.send(:create_log, lettings_log_xml) } .not_to raise_error end