Are you sure this is correct?
", "type": "interruption_screen", "answer_options": { "0": { @@ -8428,7 +8420,7 @@ } ], "title_text": { - "translation": "soft_validations.rent.max.title_text", + "translation": "soft_validations.rent.outside_range_title", "arguments": [ { "key": "brent", @@ -8437,16 +8429,7 @@ } ] }, - "informative_text": { - "translation": "soft_validations.rent.max.hint_text", - "arguments": [ - { - "key": "soft_max_for_period", - "label": false, - "i18n_template": "soft_max_for_period" - } - ] - }, + "informative_text": {}, "questions": { "rent_value_check": { "check_answer_label": "Total rent confirmation", @@ -8460,7 +8443,8 @@ } ] }, - "header": "Are you sure this is correct?", + "header": "This rent is higher than expected for this property type, in this area. Check:", + "hint_text": "Are you sure this is correct?
", "type": "interruption_screen", "answer_options": { "0": { diff --git a/config/locales/en.yml b/config/locales/en.yml index 7139223cf..ff38756cf 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -127,11 +127,13 @@ en: already_added: "You have already added this managing agent" not_answered: "You must answer %{question}" invalid_option: "Enter a valid value for %{question}" + invalid_number: "Enter a number for %{question}" other_field_missing: "If %{main_field_label} is other then %{other_field_label} must be provided" other_field_not_required: "%{other_field_label} must not be provided if %{main_field_label} was not other" numeric: - valid: "%{field} must be between %{min} and %{max}" + within_range: "%{field} must be between %{min} and %{max}" + above_min: "%{field} must be at least %{min}" date: invalid_date: "Enter a date in the correct format, for example 31 1 2022" outside_collection_window: "Enter a date within the current collection windows" @@ -148,7 +150,7 @@ en: intermediate_rent_product_name: blank: "Enter name of other intermediate rent product" saledate: - financial_year: "Date must be from 22/23 financial year" + financial_year: "Date must be from 22/23 financial year, which is between 1st April 2022 and 31st March 2023" startdate: later_than_14_days_after: "The tenancy start date must not be later than 14 days from today’s date" before_scheme_end_date: "The tenancy start date must be before the end date for this supported housing scheme" @@ -425,13 +427,18 @@ en: social_homebuy: "Social HomeBuy buyers should not have lived here before" rent_to_buy: "Rent to Buy buyers should not have lived here before" hodate: - must_be_before_exdate: "Practical completion or handover date must be before exchange date" + must_be_before_saledate: "Practical completion or handover date must be before exchange date" + must_be_less_than_3_years_from_saledate: "You told us practical completion or handover date is more than 3 years before completion date" exdate: - must_be_before_saledate: "Contract exchange date must be less than 1 year before completion date" + must_be_before_saledate: "Contract exchange date must be before completion date" must_be_less_than_1_year_from_saledate: "Contract exchange date must be less than 1 year before completion date" saledate: - must_be_after_exdate: "Completion date must be less than 1 year after contract exchange date" + must_be_after_exdate: "Completion date must be after contract exchange date" must_be_less_than_1_year_from_exdate: "Completion date must be less than 1 year after contract exchange date" + must_be_less_than_3_years_from_hodate: "You told us completion date is more than 3 years after practical completion or handover date" + must_be_after_hodate: "Completion date must be after practical completion or handover date" + previous_property_beds: + property_type_bedsit: "Bedsit bedroom maximum 1" previous_property_type: property_type_bedsit: "A bedsit can not have more than 1 bedroom" discounted_ownership_value: "Mortgage, deposit, and grant total must equal £%{value_with_discount}" @@ -447,12 +454,9 @@ en: in_soft_max_range: message: "Net income is higher than expected based on the lead tenant’s working situation. Are you sure this is correct?" rent: - min: - title_text: "You told us the rent is %{brent}" - hint_text: "The minimum rent expected for this type of property in this local authority is £%{soft_min_for_period}" - max: - title_text: "You told us the rent is %{brent}" - hint_text: "The maximum rent expected for this type of property in this local authority is £%{soft_max_for_period}" + outside_range_title: "You told us the rent is %{brent}" + min_hint_text: "The minimum rent expected for this type of property in this local authority is £%{soft_min_for_period}." + max_hint_text: "The maximum rent expected for this type of property in this local authority is £%{soft_max_for_period}." purchase_price: title_text: "You told us the purchase price is %{value}" hint_text: "The %{min_or_max} purchase price expected for this type of property in this local authority is %{soft_min_or_soft_max}" @@ -476,6 +480,7 @@ en: shared_ownership_deposit: title_text: "Mortgage, deposit and cash discount total should equal £%{expected_shared_ownership_deposit_value}" old_persons_shared_ownership: "At least one buyer should be aged over 64 for Older persons’ shared ownership scheme" + staircase_bought_seems_high: "You said %{percentage}% was bought in this staircasing transaction, which seems high. Are you sure?" monthly_charges_over_soft_max: title_text: "The amount of monthly charges is high for this type of property and sale type" diff --git a/db/migrate/20230203104238_add_saledate_check_to_sales_log.rb b/db/migrate/20230203104238_add_saledate_check_to_sales_log.rb new file mode 100644 index 000000000..8ee7fdc65 --- /dev/null +++ b/db/migrate/20230203104238_add_saledate_check_to_sales_log.rb @@ -0,0 +1,5 @@ +class AddSaledateCheckToSalesLog < ActiveRecord::Migration[7.0] + def change + add_column :sales_logs, :saledate_check, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index 1c19671a0..99aef7031 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_01_27_102334) do +ActiveRecord::Schema[7.0].define(version: 2023_02_03_104238) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -70,17 +70,6 @@ ActiveRecord::Schema[7.0].define(version: 2023_01_27_102334) do t.index ["start_year", "lettype", "beds", "la"], name: "index_la_rent_ranges_on_start_year_and_lettype_and_beds_and_la", unique: true end - create_table "la_sale_ranges", force: :cascade do |t| - t.string "la" - t.integer "bedrooms" - t.decimal "soft_min", precision: 10, scale: 2 - t.decimal "soft_max", precision: 10, scale: 2 - t.integer "start_year" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.index ["start_year", "bedrooms", "la"], name: "index_la_sale_ranges_on_start_year_bedrooms_la", unique: true - end - create_table "legacy_users", force: :cascade do |t| t.string "old_user_id" t.integer "user_id" @@ -518,10 +507,10 @@ ActiveRecord::Schema[7.0].define(version: 2023_01_27_102334) do t.integer "deposit_and_mortgage_value_check" t.integer "shared_ownership_deposit_value_check" t.integer "grant_value_check" - t.integer "value_value_check" t.integer "old_persons_shared_ownership_value_check" t.integer "staircase_bought_value_check" t.integer "monthly_charges_value_check" + t.integer "saledate_check" t.index ["bulk_upload_id"], name: "index_sales_logs_on_bulk_upload_id" t.index ["created_by_id"], name: "index_sales_logs_on_created_by_id" t.index ["owning_organisation_id"], name: "index_sales_logs_on_owning_organisation_id" diff --git a/spec/helpers/collection_time_helper_spec.rb b/spec/helpers/collection_time_helper_spec.rb new file mode 100644 index 000000000..3b02802f2 --- /dev/null +++ b/spec/helpers/collection_time_helper_spec.rb @@ -0,0 +1,34 @@ +require "rails_helper" + +RSpec.describe CollectionTimeHelper do + let(:current_user) { create(:user, :data_coordinator) } + let(:user) { create(:user, :data_coordinator) } + + around do |example| + Timecop.freeze(now) do + example.run + end + end + + describe "Current collection start year" do + context "when the date is after 1st of April" do + let(:now) { Time.utc(2022, 8, 3) } + + it "returns the same year as the current start year" do + expect(current_collection_start_year).to eq(2022) + end + + it "returns the correct current start date" do + expect(current_collection_start_date).to eq(Time.zone.local(2022, 4, 1)) + end + end + + context "with the date before 1st of April" do + let(:now) { Time.utc(2022, 2, 3) } + + it "returns the previous year as the current start year" do + expect(current_collection_start_year).to eq(2021) + end + end + end +end diff --git a/spec/mailers/bulk_upload_mailer_spec.rb b/spec/mailers/bulk_upload_mailer_spec.rb index 526f3584a..a3706beb3 100644 --- a/spec/mailers/bulk_upload_mailer_spec.rb +++ b/spec/mailers/bulk_upload_mailer_spec.rb @@ -5,7 +5,7 @@ RSpec.describe BulkUploadMailer do let(:notify_client) { instance_double(Notifications::Client) } let(:user) { create(:user, email: "user@example.com") } - let(:bulk_upload) { build(:bulk_upload, :lettings) } + let(:bulk_upload) { build(:bulk_upload, :lettings, user:) } before do allow(Notifications::Client).to receive(:new).and_return(notify_client) @@ -29,4 +29,50 @@ RSpec.describe BulkUploadMailer do mailer.send_bulk_upload_complete_mail(user:, bulk_upload:) end end + + describe "#send_bulk_upload_failed_service_error_mail" do + it "sends correctly formed email" do + expect(notify_client).to receive(:send_email).with( + email_address: user.email, + template_id: described_class::BULK_UPLOAD_FAILED_SERVICE_ERROR_TEMPLATE_ID, + personalisation: { + filename: bulk_upload.filename, + upload_timestamp: bulk_upload.created_at, + lettings_or_sales: bulk_upload.log_type, + year_combo: bulk_upload.year_combo, + bulk_upload_link: start_bulk_upload_lettings_logs_url, + }, + ) + + mailer.send_bulk_upload_failed_service_error_mail(bulk_upload:) + end + end + + context "when bulk upload has log which is not completed" do + before do + create(:lettings_log, :in_progress, bulk_upload:) + end + + describe "#send_bulk_upload_with_errors_mail" do + let(:error_description) do + "We created logs from your 2022/23 lettings data. There was a problem with 1 of the logs. Click the below link to fix these logs." + end + + it "sends correctly formed email" do + expect(notify_client).to receive(:send_email).with( + email_address: bulk_upload.user.email, + template_id: described_class::BULK_UPLOAD_WITH_ERRORS_TEMPLATE_ID, + personalisation: { + title: "We found 1 log with errors", + filename: bulk_upload.filename, + upload_timestamp: bulk_upload.created_at.to_fs(:govuk_date_and_time), + error_description:, + summary_report_link: "http://localhost:3000/lettings-logs/bulk-upload-results/#{bulk_upload.id}/resume", + }, + ) + + mailer.send_bulk_upload_with_errors_mail(bulk_upload:) + end + end + end end diff --git a/spec/models/form/lettings/questions/managing_organisation_spec.rb b/spec/models/form/lettings/questions/managing_organisation_spec.rb index 47cd30458..1a64ee973 100644 --- a/spec/models/form/lettings/questions/managing_organisation_spec.rb +++ b/spec/models/form/lettings/questions/managing_organisation_spec.rb @@ -199,4 +199,36 @@ RSpec.describe Form::Lettings::Questions::ManagingOrganisation, type: :model do end end end + + describe "#answer_label" do + context "when answered" do + let(:managing_organisation) { create(:organisation) } + let(:log) { create(:lettings_log, managing_organisation:) } + + it "returns org name" do + expect(question.answer_label(log)).to eq(managing_organisation.name) + end + end + + context "when unanswered" do + let(:log) { create(:lettings_log, managing_organisation: nil) } + + it "returns nil" do + expect(question.answer_label(log)).to be_nil + end + end + + context "when org does not exist" do + let(:managing_organisation) { create(:organisation) } + let(:log) { create(:lettings_log, managing_organisation:) } + + before do + allow(Organisation).to receive(:find_by).and_return(nil) + end + + it "returns nil" do + expect(question.answer_label(log)).to be_nil + end + end + end end diff --git a/spec/models/form/sales/pages/handover_date_check_spec.rb b/spec/models/form/sales/pages/handover_date_check_spec.rb index 86c74fda6..6bd3f3874 100644 --- a/spec/models/form/sales/pages/handover_date_check_spec.rb +++ b/spec/models/form/sales/pages/handover_date_check_spec.rb @@ -16,18 +16,28 @@ RSpec.describe Form::Sales::Pages::HandoverDateCheck, type: :model do end it "has the correct id" do - expect(page.id).to eq("") + expect(page.id).to eq("handover_date_check") end it "has the correct header" do expect(page.header).to be_nil end + it "has the correct title_text" do + expect(page.title_text).to eq({ + "translation" => "validations.sale_information.hodate.must_be_less_than_3_years_from_saledate", + "arguments" => [], + }) + end + + it "has the correct informative_text" do + expect(page.informative_text).to eq({}) + end + it "has correct depends_on" do expect(page.depends_on).to eq([ - { - "hodate_3_years_or_more_saledate?" => true, - }, + { "hodate_3_years_or_more_saledate?" => true, "saledate_check" => nil }, + { "hodate_3_years_or_more_saledate?" => true, "saledate_check" => 1 }, ]) end diff --git a/spec/models/form/sales/pages/purchase_price_outright_ownership_spec.rb b/spec/models/form/sales/pages/purchase_price_outright_ownership_spec.rb new file mode 100644 index 000000000..88c357034 --- /dev/null +++ b/spec/models/form/sales/pages/purchase_price_outright_ownership_spec.rb @@ -0,0 +1,35 @@ +require "rails_helper" + +RSpec.describe Form::Sales::Pages::PurchasePriceOutrightOwnership, type: :model do + subject(:page) { described_class.new(page_id, page_definition, subsection) } + + let(:page_id) { "purchase_price" } + let(:page_definition) { nil } + let(:subsection) { instance_double(Form::Subsection) } + + it "has correct subsection" do + expect(page.subsection).to eq(subsection) + end + + it "has correct questions" do + expect(page.questions.map(&:id)).to eq(%w[value]) + end + + it "has the correct id" do + expect(page.id).to eq("purchase_price") + end + + it "has the correct header" do + expect(page.header).to be_nil + end + + it "has the correct description" do + expect(page.description).to be_nil + end + + it "has correct depends_on" do + expect(page.depends_on).to eq([ + { "outright_sale_or_discounted_with_full_ownership?" => true }, + ]) + end +end diff --git a/spec/models/form/sales/pages/purchase_price_spec.rb b/spec/models/form/sales/pages/purchase_price_spec.rb index 4d0be701f..6ec2b7ac0 100644 --- a/spec/models/form/sales/pages/purchase_price_spec.rb +++ b/spec/models/form/sales/pages/purchase_price_spec.rb @@ -29,8 +29,7 @@ RSpec.describe Form::Sales::Pages::PurchasePrice, type: :model do it "has correct depends_on" do expect(page.depends_on).to eq([ - { "ownershipsch" => 3 }, - { "rent_to_buy_full_ownership?" => true }, + { "ownershipsch" => 2, "rent_to_buy_full_ownership?" => false }, ]) end end diff --git a/spec/models/form/sales/pages/sale_date_check_spec.rb b/spec/models/form/sales/pages/sale_date_check_spec.rb new file mode 100644 index 000000000..f1e0b3e2b --- /dev/null +++ b/spec/models/form/sales/pages/sale_date_check_spec.rb @@ -0,0 +1,47 @@ +require "rails_helper" + +RSpec.describe Form::Sales::Pages::SaleDateCheck, type: :model do + subject(:page) { described_class.new(page_id, page_definition, subsection) } + + let(:page_id) { "" } + let(:page_definition) { nil } + let(:subsection) { instance_double(Form::Subsection) } + + it "has correct subsection" do + expect(page.subsection).to eq(subsection) + end + + it "has correct questions" do + expect(page.questions.map(&:id)).to eq(%w[saledate_check]) + end + + it "has the correct id" do + expect(page.id).to eq("completion_date_check") + end + + it "has the correct header" do + expect(page.header).to be_nil + end + + it "has the correct title_text" do + expect(page.title_text).to eq({ + "translation" => "validations.sale_information.saledate.must_be_less_than_3_years_from_hodate", + "arguments" => [], + }) + end + + it "has the correct informative_text" do + expect(page.informative_text).to eq({}) + end + + it "has correct depends_on" do + expect(page.depends_on).to eq([ + { "hodate_3_years_or_more_saledate?" => true, "hodate_check" => nil }, + { "hodate_3_years_or_more_saledate?" => true, "hodate_check" => 1 }, + ]) + end + + it "is interruption screen page" do + expect(page.interruption_screen?).to eq(true) + end +end diff --git a/spec/models/form/sales/questions/handover_date_check_spec.rb b/spec/models/form/sales/questions/handover_date_check_spec.rb index 0e57abf35..86a46436b 100644 --- a/spec/models/form/sales/questions/handover_date_check_spec.rb +++ b/spec/models/form/sales/questions/handover_date_check_spec.rb @@ -16,7 +16,7 @@ RSpec.describe Form::Sales::Questions::HandoverDateCheck, type: :model do end it "has the correct header" do - expect(question.header).to eq("Are you sure practical completion or handover date is more than 3 years before exchange date?") + expect(question.header).to eq("Are you sure?") end it "has the correct check_answer_label" do @@ -43,6 +43,6 @@ RSpec.describe Form::Sales::Questions::HandoverDateCheck, type: :model do end it "has the correct hidden_in_check_answers" do - expect(question.hidden_in_check_answers).to eq({ "depends_on" => [{ "hodate_check" => 0 }, { "hodate_check" => 1 }] }) + expect(question.hidden_in_check_answers).to eq({ "depends_on" => [{ "hodate_check" => 0 }, { "hodate_check" => 1 }, { "saledate_check" => 0 }, { "saledate_check" => 1 }] }) end end diff --git a/spec/models/form/sales/questions/purchase_price_spec.rb b/spec/models/form/sales/questions/purchase_price_spec.rb index 8894a592f..ce0aaae9c 100644 --- a/spec/models/form/sales/questions/purchase_price_spec.rb +++ b/spec/models/form/sales/questions/purchase_price_spec.rb @@ -32,7 +32,9 @@ RSpec.describe Form::Sales::Questions::PurchasePrice, type: :model do end it "has the correct hint" do - expect(question.hint_text).to be_nil + expect(question.hint_text).to eq( + "For all schemes, including Right to Acquire (RTA), Right to Buy (RTB), Voluntary Right to Buy (VRTB) or Preserved Right to Buy (PRTB) sales, enter the full price of the property without any discount", + ) end it "has correct width" do diff --git a/spec/models/form/sales/questions/sale_date_check_spec.rb b/spec/models/form/sales/questions/sale_date_check_spec.rb new file mode 100644 index 000000000..c37b2e870 --- /dev/null +++ b/spec/models/form/sales/questions/sale_date_check_spec.rb @@ -0,0 +1,48 @@ +require "rails_helper" + +RSpec.describe Form::Sales::Questions::SaleDateCheck, type: :model do + subject(:question) { described_class.new(question_id, question_definition, page) } + + let(:question_id) { nil } + let(:question_definition) { nil } + let(:page) { instance_double(Form::Page) } + + it "has correct page" do + expect(question.page).to eq(page) + end + + it "has the correct id" do + expect(question.id).to eq("saledate_check") + end + + it "has the correct header" do + expect(question.header).to eq("Are you sure?") + end + + it "has the correct check_answer_label" do + expect(question.check_answer_label).to eq("Sale completion date check") + end + + it "has the correct type" do + expect(question.type).to eq("interruption_screen") + end + + it "is not marked as derived" do + expect(question.derived?).to be false + end + + it "has the correct hint" do + expect(question.hint_text).to be_nil + end + + it "has the correct answer_options" do + expect(question.answer_options).to eq({ + "0" => { "value" => "Yes" }, + "1" => { "value" => "No" }, + }) + end + + it "has the correct hidden_in_check_answers" do + expect(question.hidden_in_check_answers).to eq({ "depends_on" => [{ "hodate_check" => 0 }, { "hodate_check" => 1 }, { "saledate_check" => 0 }, { "saledate_check" => 1 }] }) + end +end diff --git a/spec/models/form/sales/subsections/discounted_ownership_scheme_spec.rb b/spec/models/form/sales/subsections/discounted_ownership_scheme_spec.rb index 5702f7783..525ad06b4 100644 --- a/spec/models/form/sales/subsections/discounted_ownership_scheme_spec.rb +++ b/spec/models/form/sales/subsections/discounted_ownership_scheme_spec.rb @@ -12,6 +12,7 @@ RSpec.describe Form::Sales::Subsections::DiscountedOwnershipScheme, type: :model end it "has correct pages" do + puts discounted_ownership_scheme.pages.map(&:id) expect(discounted_ownership_scheme.pages.map(&:id)).to eq( %w[ living_before_purchase_discounted_ownership @@ -20,6 +21,7 @@ RSpec.describe Form::Sales::Subsections::DiscountedOwnershipScheme, type: :model about_price_not_rtb grant_value_check purchase_price_discounted_ownership + purchase_price_outright_ownership discounted_ownership_deposit_and_mortgage_value_check_after_value_and_discount mortgage_used_discounted_ownership discounted_ownership_mortgage_used_mortgage_value_check diff --git a/spec/models/form/sales/subsections/setup_spec.rb b/spec/models/form/sales/subsections/setup_spec.rb index ac4f0246c..779b9537e 100644 --- a/spec/models/form/sales/subsections/setup_spec.rb +++ b/spec/models/form/sales/subsections/setup_spec.rb @@ -17,6 +17,7 @@ RSpec.describe Form::Sales::Subsections::Setup, type: :model do organisation created_by completion_date + completion_date_check purchaser_code ownership_scheme shared_ownership_type diff --git a/spec/models/form_handler_spec.rb b/spec/models/form_handler_spec.rb index ca42f55fc..fbc440d6f 100644 --- a/spec/models/form_handler_spec.rb +++ b/spec/models/form_handler_spec.rb @@ -52,14 +52,14 @@ RSpec.describe FormHandler do it "is able to load a current sales form" do form = form_handler.get_form("current_sales") expect(form).to be_a(Form) - expect(form.pages.count).to eq(212) + expect(form.pages.count).to eq(214) expect(form.name).to eq("2022_2023_sales") end it "is able to load a previous sales form" do form = form_handler.get_form("previous_sales") expect(form).to be_a(Form) - expect(form.pages.count).to eq(212) + expect(form.pages.count).to eq(214) expect(form.name).to eq("2021_2022_sales") end end diff --git a/spec/models/form_spec.rb b/spec/models/form_spec.rb index 71dfd4b60..72223c1b5 100644 --- a/spec/models/form_spec.rb +++ b/spec/models/form_spec.rb @@ -218,9 +218,9 @@ RSpec.describe Form, type: :model do expect(form.sections[0].class).to eq(Form::Sales::Sections::Setup) expect(form.subsections.count).to eq(1) expect(form.subsections.first.id).to eq("setup") - expect(form.pages.count).to eq(14) + expect(form.pages.count).to eq(15) expect(form.pages.first.id).to eq("organisation") - expect(form.questions.count).to eq(15) + expect(form.questions.count).to eq(16) expect(form.questions.first.id).to eq("owning_organisation_id") expect(form.start_date).to eq(Time.zone.parse("2022-04-01")) expect(form.end_date).to eq(Time.zone.parse("2023-07-01")) diff --git a/spec/models/sales_log_spec.rb b/spec/models/sales_log_spec.rb index 0b4fa4459..a5a57f36e 100644 --- a/spec/models/sales_log_spec.rb +++ b/spec/models/sales_log_spec.rb @@ -47,7 +47,7 @@ RSpec.describe SalesLog, type: :model do let(:sales_log) { build(:sales_log) } it "returns optional fields" do - expect(sales_log.optional_fields).to eq(%w[purchid monthly_charges_value_check old_persons_shared_ownership_value_check]) + expect(sales_log.optional_fields).to eq(%w[saledate_check purchid monthly_charges_value_check old_persons_shared_ownership_value_check]) end end diff --git a/spec/models/validations/household_validations_spec.rb b/spec/models/validations/household_validations_spec.rb index 0d1be05ae..daa3feef1 100644 --- a/spec/models/validations/household_validations_spec.rb +++ b/spec/models/validations/household_validations_spec.rb @@ -463,14 +463,14 @@ RSpec.describe Validations::HouseholdValidations do record.hhmemb = -1 household_validator.validate_numeric_min_max(record) expect(record.errors["hhmemb"]) - .to include(match I18n.t("validations.numeric.valid", field: "Number of Household Members", min: 0, max: 8)) + .to include(match I18n.t("validations.numeric.within_range", field: "Number of Household Members", min: 0, max: 8)) end it "validates that the number of household members cannot be more than 8" do record.hhmemb = 9 household_validator.validate_numeric_min_max(record) expect(record.errors["hhmemb"]) - .to include(match I18n.t("validations.numeric.valid", field: "Number of Household Members", min: 0, max: 8)) + .to include(match I18n.t("validations.numeric.within_range", field: "Number of Household Members", min: 0, max: 8)) end it "expects that the number of other household members is between the min and max" do diff --git a/spec/models/validations/sales/household_validations_spec.rb b/spec/models/validations/sales/household_validations_spec.rb index 26d369a35..10367390e 100644 --- a/spec/models/validations/sales/household_validations_spec.rb +++ b/spec/models/validations/sales/household_validations_spec.rb @@ -5,48 +5,6 @@ RSpec.describe Validations::Sales::HouseholdValidations do let(:validator_class) { Class.new { include Validations::Sales::HouseholdValidations } } - describe "#validate_number_of_other_people_living_in_the_property" do - context "when within permitted bounds" do - let(:record) { build(:sales_log, hholdcount: 2) } - - it "does not add an error" do - household_validator.validate_number_of_other_people_living_in_the_property(record) - - expect(record.errors[:hholdcount]).not_to be_present - end - end - - context "when blank" do - let(:record) { build(:sales_log, hholdcount: nil) } - - it "does not add an error" do - household_validator.validate_number_of_other_people_living_in_the_property(record) - - expect(record.errors[:hholdcount]).not_to be_present - end - end - - context "when below lower bound" do - let(:record) { build(:sales_log, hholdcount: -1) } - - it "adds an error" do - household_validator.validate_number_of_other_people_living_in_the_property(record) - - expect(record.errors[:hholdcount]).to be_present - end - end - - context "when higher than upper bound" do - let(:record) { build(:sales_log, hholdcount: 5) } - - it "adds an error" do - household_validator.validate_number_of_other_people_living_in_the_property(record) - - expect(record.errors[:hholdcount]).to be_present - end - end - end - describe "household member validations" do let(:record) { build(:sales_log) } diff --git a/spec/models/validations/sales/sale_information_validations_spec.rb b/spec/models/validations/sales/sale_information_validations_spec.rb index 170005572..179935da7 100644 --- a/spec/models/validations/sales/sale_information_validations_spec.rb +++ b/spec/models/validations/sales/sale_information_validations_spec.rb @@ -62,7 +62,7 @@ RSpec.describe Validations::Sales::SaleInformationValidations do it "does not add an error" do sale_information_validator.validate_practical_completion_date_before_saledate(record) - expect(record.errors[:hodate]).to be_present + expect(record.errors[:hodate]).not_to be_present end end end @@ -130,10 +130,10 @@ RSpec.describe Validations::Sales::SaleInformationValidations do sale_information_validator.validate_exchange_date(record) expect(record.errors[:exdate]).to eq( - ["Contract exchange date must be less than 1 year before completion date"], + ["Contract exchange date must be before completion date"], ) expect(record.errors[:saledate]).to eq( - ["Completion date must be less than 1 year after contract exchange date"], + ["Completion date must be after contract exchange date"], ) end end diff --git a/spec/models/validations/shared_validations_spec.rb b/spec/models/validations/shared_validations_spec.rb index c1d8cb3af..151050d8b 100644 --- a/spec/models/validations/shared_validations_spec.rb +++ b/spec/models/validations/shared_validations_spec.rb @@ -18,42 +18,42 @@ RSpec.describe Validations::SharedValidations do record.age1 = "random" shared_validator.validate_numeric_min_max(record) expect(record.errors["age1"]) - .to include(match I18n.t("validations.numeric.valid", field: "Lead tenant’s age", min: 16, max: 120)) + .to include(match I18n.t("validations.numeric.within_range", field: "Lead tenant’s age", min: 16, max: 120)) end it "validates that other household member ages are a number" do record.age2 = "random" shared_validator.validate_numeric_min_max(record) expect(record.errors["age2"]) - .to include(match I18n.t("validations.numeric.valid", field: "Person 2’s age", min: 1, max: 120)) + .to include(match I18n.t("validations.numeric.within_range", field: "Person 2’s age", min: 1, max: 120)) end it "validates that person 1's age is greater than 16" do record.age1 = 15 shared_validator.validate_numeric_min_max(record) expect(record.errors["age1"]) - .to include(match I18n.t("validations.numeric.valid", field: "Lead tenant’s age", min: 16, max: 120)) + .to include(match I18n.t("validations.numeric.within_range", field: "Lead tenant’s age", min: 16, max: 120)) end it "validates that other household member ages are greater than 1" do record.age2 = 0 shared_validator.validate_numeric_min_max(record) expect(record.errors["age2"]) - .to include(match I18n.t("validations.numeric.valid", field: "Person 2’s age", min: 1, max: 120)) + .to include(match I18n.t("validations.numeric.within_range", field: "Person 2’s age", min: 1, max: 120)) end it "validates that person 1's age is less than 121" do record.age1 = 121 shared_validator.validate_numeric_min_max(record) expect(record.errors["age1"]) - .to include(match I18n.t("validations.numeric.valid", field: "Lead tenant’s age", min: 16, max: 120)) + .to include(match I18n.t("validations.numeric.within_range", field: "Lead tenant’s age", min: 16, max: 120)) end it "validates that other household member ages are greater than 121" do record.age2 = 123 shared_validator.validate_numeric_min_max(record) expect(record.errors["age2"]) - .to include(match I18n.t("validations.numeric.valid", field: "Person 2’s age", min: 1, max: 120)) + .to include(match I18n.t("validations.numeric.within_range", field: "Person 2’s age", min: 1, max: 120)) end it "validates that person 1's age is between 16 and 120" do @@ -69,21 +69,27 @@ RSpec.describe Validations::SharedValidations do end end + it "adds the correct validation text when a question has a min but not a max" do + sales_record.savings = -10 + shared_validator.validate_numeric_min_max(sales_record) + expect(sales_record.errors["savings"]).to include(match I18n.t("validations.numeric.above_min", field: "Buyer’s total savings (to nearest £10) before any deposit paid", min: "£0")) + end + context "when validating percent" do it "validates that suffixes are added in the error message" do sales_record.stairbought = 150 shared_validator.validate_numeric_min_max(sales_record) expect(sales_record.errors["stairbought"]) - .to include(match I18n.t("validations.numeric.valid", field: "Percentage bought in this staircasing transaction", min: "0%", max: "100%")) + .to include(match I18n.t("validations.numeric.within_range", field: "Percentage bought in this staircasing transaction", min: "0%", max: "100%")) end end context "when validating price" do it "validates that £ prefix and , is added in the error message" do - sales_record.income1 = "random" + sales_record.income1 = -5 shared_validator.validate_numeric_min_max(sales_record) expect(sales_record.errors["income1"]) - .to include(match I18n.t("validations.numeric.valid", field: "Buyer 1’s gross annual income", min: "£0", max: "£999,999")) + .to include(match I18n.t("validations.numeric.within_range", field: "Buyer 1’s gross annual income", min: "£0", max: "£999,999")) end end end diff --git a/spec/requests/bulk_upload_controller_spec.rb b/spec/requests/bulk_upload_controller_spec.rb index fc3afd21c..7aa0d16e0 100644 --- a/spec/requests/bulk_upload_controller_spec.rb +++ b/spec/requests/bulk_upload_controller_spec.rb @@ -13,6 +13,14 @@ RSpec.describe BulkUploadController, type: :request do 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: {} } @@ -50,6 +58,40 @@ RSpec.describe BulkUploadController, type: :request do 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(2022, 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: 2021 }, + ), + ) + 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 } } } diff --git a/spec/requests/lettings_logs_controller_spec.rb b/spec/requests/lettings_logs_controller_spec.rb index 1bb0afbf9..57311b70d 100644 --- a/spec/requests/lettings_logs_controller_spec.rb +++ b/spec/requests/lettings_logs_controller_spec.rb @@ -82,7 +82,7 @@ RSpec.describe LettingsLogsController, type: :request do it "validates lettings log parameters" do json_response = JSON.parse(response.body) expect(response).to have_http_status(:unprocessable_entity) - expect(json_response["errors"]).to match_array([["offered", [I18n.t("validations.property.offered.relet_number")]], ["age1", [I18n.t("validations.numeric.valid", field: "Lead tenant’s age", min: 16, max: 120)]]]) + expect(json_response["errors"]).to match_array([["offered", [I18n.t("validations.property.offered.relet_number")]], ["age1", [I18n.t("validations.numeric.within_range", field: "Lead tenant’s age", min: 16, max: 120)]]]) end end diff --git a/spec/services/bulk_upload/lettings/validator_spec.rb b/spec/services/bulk_upload/lettings/validator_spec.rb index ec9b3b7f7..263c83163 100644 --- a/spec/services/bulk_upload/lettings/validator_spec.rb +++ b/spec/services/bulk_upload/lettings/validator_spec.rb @@ -42,15 +42,15 @@ RSpec.describe BulkUpload::Lettings::Validator do it "create validation error with correct values" do validator.call - error = BulkUploadError.first + error = BulkUploadError.order(:row, :field).first - expect(error.field).to eql("field_96") - expect(error.error).to eql("blank") + expect(error.field).to eql("field_11") + expect(error.error).to eql("You must only answer the length of the tenancy if it's fixed-term") expect(error.tenant_code).to eql("123") expect(error.property_ref).to be_nil expect(error.row).to eql("7") - expect(error.cell).to eql("CS7") - expect(error.col).to eql("CS") + expect(error.cell).to eql("L7") + expect(error.col).to eql("L") end end diff --git a/spec/services/bulk_upload/processor_spec.rb b/spec/services/bulk_upload/processor_spec.rb index 27fd9edd3..b751e7dd7 100644 --- a/spec/services/bulk_upload/processor_spec.rb +++ b/spec/services/bulk_upload/processor_spec.rb @@ -6,7 +6,7 @@ RSpec.describe BulkUpload::Processor do let(:bulk_upload) { create(:bulk_upload, :lettings) } describe "#call" do - context "when processing a bulk upload with errors" do + context "when the bulk upload itself is not considered valid" do let(:mock_downloader) do instance_double( BulkUpload::Downloader, @@ -16,12 +16,153 @@ RSpec.describe BulkUpload::Processor do ) end + let(:mock_validator) do + instance_double( + BulkUpload::Lettings::Validator, + invalid?: true, + call: nil, + ) + end + + before do + allow(BulkUpload::Downloader).to receive(:new).with(bulk_upload:).and_return(mock_downloader) + allow(BulkUpload::Lettings::Validator).to receive(:new).and_return(mock_validator) + end + + it "sends failure email" do + mail_double = instance_double("ActionMailer::MessageDelivery", deliver_later: nil) + + allow(BulkUploadMailer).to receive(:send_bulk_upload_failed_service_error_mail).and_return(mail_double) + + processor.call + + expect(BulkUploadMailer).to have_received(:send_bulk_upload_failed_service_error_mail) + expect(mail_double).to have_received(:deliver_later) + end + + it "does not attempt to validate the contents of the file" do + processor.call + + expect(mock_validator).not_to have_received(:call) + end + end + + context "when the bulk upload processing throws an error" do + let(:mock_downloader) do + instance_double( + BulkUpload::Downloader, + call: nil, + path: file_fixture("2022_23_lettings_bulk_upload.csv"), + delete_local_file!: nil, + ) + end + + let(:mock_validator) do + instance_double( + BulkUpload::Lettings::Validator, + invalid?: false, + ) + end + + before do + allow(BulkUpload::Downloader).to receive(:new).with(bulk_upload:).and_return(mock_downloader) + allow(BulkUpload::Lettings::Validator).to receive(:new).and_return(mock_validator) + + allow(mock_validator).to receive(:call).and_raise(StandardError) + end + + it "sends failure email" do + mail_double = instance_double("ActionMailer::MessageDelivery", deliver_later: nil) + + allow(BulkUploadMailer).to receive(:send_bulk_upload_failed_service_error_mail).and_return(mail_double) + + processor.call + + expect(BulkUploadMailer).to have_received(:send_bulk_upload_failed_service_error_mail) + expect(mail_double).to have_received(:deliver_later) + end + + it "we log the failure" do + allow(Sentry).to receive(:capture_exception) + + processor.call + + expect(Sentry).to have_received(:capture_exception) + end + end + + context "when processing a bulk upload with errors but below threshold (therefore creates logs)" do + let(:mock_downloader) do + instance_double( + BulkUpload::Downloader, + call: nil, + path: file_fixture("2022_23_lettings_bulk_upload.csv"), + delete_local_file!: nil, + ) + end + + let(:mock_validator) do + instance_double( + BulkUpload::Lettings::Validator, + invalid?: false, + call: nil, + create_logs?: true, + ) + end + before do allow(BulkUpload::Downloader).to receive(:new).with(bulk_upload:).and_return(mock_downloader) + allow(BulkUpload::Lettings::Validator).to receive(:new).and_return(mock_validator) end - it "persist the validation errors" do - expect { processor.call }.to change(BulkUploadError, :count) + it "deletes the local file afterwards" do + processor.call + + expect(mock_downloader).to have_received(:delete_local_file!) + end + + it "sends fix errors email" do + mail_double = instance_double("ActionMailer::MessageDelivery", deliver_later: nil) + + allow(BulkUploadMailer).to receive(:send_bulk_upload_with_errors_mail).and_return(mail_double) + + processor.call + + expect(BulkUploadMailer).to have_received(:send_bulk_upload_with_errors_mail) + expect(mail_double).to have_received(:deliver_later) + end + + it "does not send success email" do + allow(BulkUploadMailer).to receive(:send_bulk_upload_complete_mail).and_call_original + + processor.call + + expect(BulkUploadMailer).not_to have_received(:send_bulk_upload_complete_mail) + end + end + + context "when processing a bulk upload with errors but above threshold (therefore does not create logs)" do + let(:mock_downloader) do + instance_double( + BulkUpload::Downloader, + call: nil, + path: file_fixture("2022_23_lettings_bulk_upload.csv"), + delete_local_file!: nil, + ) + end + + let(:mock_validator) do + instance_double( + BulkUpload::Lettings::Validator, + invalid?: false, + call: nil, + create_logs?: false, + ) + end + + before do + allow(BulkUpload::Downloader).to receive(:new).with(bulk_upload:).and_return(mock_downloader) + allow(BulkUpload::Lettings::Validator).to receive(:new).and_return(mock_validator) end it "deletes the local file afterwards" do @@ -30,6 +171,14 @@ RSpec.describe BulkUpload::Processor do expect(mock_downloader).to have_received(:delete_local_file!) end + it "does not send fix errors email" do + allow(BulkUploadMailer).to receive(:send_bulk_upload_with_errors_mail).and_call_original + + processor.call + + expect(BulkUploadMailer).not_to have_received(:send_bulk_upload_with_errors_mail) + end + it "does not send success email" do allow(BulkUploadMailer).to receive(:send_bulk_upload_complete_mail).and_call_original @@ -56,6 +205,7 @@ RSpec.describe BulkUpload::Processor do BulkUpload::Lettings::Validator, call: nil, create_logs?: true, + invalid?: false, ) end @@ -79,6 +229,14 @@ RSpec.describe BulkUpload::Processor do expect(mock_creator).to have_received(:call) end + it "does not send fix errors email" do + allow(BulkUploadMailer).to receive(:send_bulk_upload_with_errors_mail).and_call_original + + processor.call + + expect(BulkUploadMailer).not_to have_received(:send_bulk_upload_with_errors_mail) + end + it "sends success email" do mail_double = instance_double("ActionMailer::MessageDelivery", deliver_later: nil)