From 45b098ad4b55f5e5520a8330aa7613a0a2267a70 Mon Sep 17 00:00:00 2001 From: Jack <113976590+bibblobcode@users.noreply.github.com> Date: Tue, 24 Jan 2023 13:27:44 +0000 Subject: [PATCH 1/8] CLDC-1457 Update checkbox validation message (#1207) --- app/models/form/sales/questions/buyers_organisations.rb | 4 ++++ spec/models/form/sales/questions/buyers_organisations_spec.rb | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/app/models/form/sales/questions/buyers_organisations.rb b/app/models/form/sales/questions/buyers_organisations.rb index 6457b6866..735a1563d 100644 --- a/app/models/form/sales/questions/buyers_organisations.rb +++ b/app/models/form/sales/questions/buyers_organisations.rb @@ -15,4 +15,8 @@ class Form::Sales::Questions::BuyersOrganisations < ::Form::Question "pregla" => { "value" => "Local Authority" }, "pregghb" => { "value" => "Help to Buy Agent" }, }.freeze + + def unanswered_error_message + "At least one option must be selected of these four" + end end diff --git a/spec/models/form/sales/questions/buyers_organisations_spec.rb b/spec/models/form/sales/questions/buyers_organisations_spec.rb index 14bb944f4..d9f61df5f 100644 --- a/spec/models/form/sales/questions/buyers_organisations_spec.rb +++ b/spec/models/form/sales/questions/buyers_organisations_spec.rb @@ -35,6 +35,10 @@ RSpec.describe Form::Sales::Questions::BuyersOrganisations, type: :model do expect(question.hint_text).to eq("Select all that apply") end + it "has the correct unanswered_error_message" do + expect(question.unanswered_error_message).to eq("At least one option must be selected of these four") + end + it "has the correct answer_options" do expect(question.answer_options).to eq( { From cf92c9559c6d1c4c474b349f06ffe9a8c8533631 Mon Sep 17 00:00:00 2001 From: Jack <113976590+bibblobcode@users.noreply.github.com> Date: Tue, 24 Jan 2023 13:28:50 +0000 Subject: [PATCH 2/8] Add disability and wheelchair questions validation (#1215) * Add Page#interruption_screen? method * Use form helper method * Validate household disability questions * Update CYA --- app/controllers/form_controller.rb | 30 ++++++++----- app/models/form/page.rb | 4 ++ .../questions/household_wheelchair_check.rb | 1 + .../form/sales/subsections/household_needs.rb | 1 + .../validations/sales/soft_validations.rb | 4 +- spec/fixtures/forms/2021_2022.json | 2 +- spec/models/form/page_spec.rb | 16 +++++++ .../sales/subsections/household_needs_spec.rb | 1 + spec/models/form_handler_spec.rb | 4 +- .../sales/soft_validations_spec.rb | 45 +++++++++++++++++-- 10 files changed, 87 insertions(+), 21 deletions(-) diff --git a/app/controllers/form_controller.rb b/app/controllers/form_controller.rb index b0468c875..24e3d1b32 100644 --- a/app/controllers/form_controller.rb +++ b/app/controllers/form_controller.rb @@ -6,7 +6,7 @@ class FormController < ApplicationController def submit_form if @log - @page = @log.form.get_page(params[@log.model_name.param_key][:page]) + @page = form.get_page(params[@log.model_name.param_key][:page]) responses_for_page = responses_for_page(@page) mandatory_questions_with_no_response = mandatory_questions_with_no_response(responses_for_page) @@ -30,7 +30,7 @@ class FormController < ApplicationController def check_answers if @log current_url = request.env["PATH_INFO"] - subsection = @log.form.get_subsection(current_url.split("/")[-2]) + subsection = form.get_subsection(current_url.split("/")[-2]) render "form/check_answers", locals: { subsection:, current_user: } else render_not_found @@ -49,8 +49,8 @@ class FormController < ApplicationController if @log restore_error_field_values page_id = request.path.split("/")[-1].underscore - @page = @log.form.get_page(page_id) - @subsection = @log.form.subsection_for_page(@page) + @page = form.get_page(page_id) + @subsection = form.subsection_for_page(@page) if @page.routed_to?(@log, current_user) render "form/page" else @@ -71,7 +71,7 @@ private end if session["fields"] session["fields"].each do |field, value| - if @log.form.get_question(field, @log)&.type != "date" && @log.respond_to?(field) + if form.get_question(field, @log)&.type != "date" && @log.respond_to?(field) @log[field] = value end end @@ -129,20 +129,26 @@ private def successful_redirect_path if is_referrer_check_answers? - page_ids = @log.form.subsection_for_page(@page).pages.map(&:id) + page_ids = form.subsection_for_page(@page).pages.map(&:id) page_index = page_ids.index(@page.id) - next_page = @log.form.next_page(@page, @log, current_user) - previous_page = @log.form.previous_page(page_ids, page_index, @log, current_user) - if next_page.to_s.include?("value_check") || next_page == previous_page - return send("#{@log.class.name.underscore}_#{next_page}_path", @log, { referrer: "check_answers" }) + next_page_id = form.next_page(@page, @log, current_user) + next_page = form.get_page(next_page_id) + previous_page = form.previous_page(page_ids, page_index, @log, current_user) + + if next_page&.interruption_screen? || next_page_id == previous_page + return send("#{@log.class.name.underscore}_#{next_page_id}_path", @log, { referrer: "check_answers" }) else - return send("#{@log.model_name.param_key}_#{@log.form.subsection_for_page(@page).id}_check_answers_path", @log) + return send("#{@log.model_name.param_key}_#{form.subsection_for_page(@page).id}_check_answers_path", @log) end end - redirect_path = @log.form.next_page_redirect_path(@page, @log, current_user) + redirect_path = form.next_page_redirect_path(@page, @log, current_user) send(redirect_path, @log) end + def form + @log&.form + end + def mandatory_questions_with_no_response(responses_for_page) session["fields"] = {} calc_questions = @page.questions.map(&:result_field) diff --git a/app/models/form/page.rb b/app/models/form/page.rb index 33e944eed..ea57bac66 100644 --- a/app/models/form/page.rb +++ b/app/models/form/page.rb @@ -32,6 +32,10 @@ class Form::Page end end + def interruption_screen? + questions.all? { |question| question.type == "interruption_screen" } + end + private def conditional_question_ids diff --git a/app/models/form/sales/questions/household_wheelchair_check.rb b/app/models/form/sales/questions/household_wheelchair_check.rb index 1a9db3f3a..f2c15b9ac 100644 --- a/app/models/form/sales/questions/household_wheelchair_check.rb +++ b/app/models/form/sales/questions/household_wheelchair_check.rb @@ -19,5 +19,6 @@ class Form::Sales::Questions::HouseholdWheelchairCheck < ::Form::Question }, ], } + @page = page end end diff --git a/app/models/form/sales/subsections/household_needs.rb b/app/models/form/sales/subsections/household_needs.rb index d411d5101..b5d4d528e 100644 --- a/app/models/form/sales/subsections/household_needs.rb +++ b/app/models/form/sales/subsections/household_needs.rb @@ -12,6 +12,7 @@ class Form::Sales::Subsections::HouseholdNeeds < ::Form::Subsection Form::Sales::Pages::BuyerStillServing.new(nil, nil, self), Form::Sales::Pages::ArmedForcesSpouse.new(nil, nil, self), Form::Sales::Pages::HouseholdDisability.new(nil, nil, self), + Form::Sales::Pages::HouseholdWheelchairCheck.new("disability_wheelchair_check", nil, self), Form::Sales::Pages::HouseholdWheelchair.new(nil, nil, self), Form::Sales::Pages::HouseholdWheelchairCheck.new("wheelchair_check", nil, self), ] diff --git a/app/models/validations/sales/soft_validations.rb b/app/models/validations/sales/soft_validations.rb index 3deb92aaa..19c2a13ec 100644 --- a/app/models/validations/sales/soft_validations.rb +++ b/app/models/validations/sales/soft_validations.rb @@ -22,9 +22,9 @@ module Validations::Sales::SoftValidations end def wheelchair_when_not_disabled? - return false unless disabled == 2 + return unless disabled && wheel - wheel == 1 + wheel == 1 && disabled == 2 end def savings_over_soft_max? diff --git a/spec/fixtures/forms/2021_2022.json b/spec/fixtures/forms/2021_2022.json index 486bc126c..ec13bdabb 100644 --- a/spec/fixtures/forms/2021_2022.json +++ b/spec/fixtures/forms/2021_2022.json @@ -208,7 +208,7 @@ "check_answer_label": "Retirement age soft validation", "hidden_in_check_answers": true, "header": "Are you sure this person is retired?", - "type": "radio", + "type": "interruption_screen", "answer_options": { "0": { "value": "Yes" diff --git a/spec/models/form/page_spec.rb b/spec/models/form/page_spec.rb index bb448aca6..4a870fe46 100644 --- a/spec/models/form/page_spec.rb +++ b/spec/models/form/page_spec.rb @@ -47,6 +47,22 @@ RSpec.describe Form::Page, type: :model do end end + describe "#interruption_screen?" do + context "when it has regular questions" do + it "returns false" do + expect(page.interruption_screen?).to be false + end + end + + context "when it has interruption_screen question" do + let(:page) { form.get_page("retirement_value_check") } + + it "returns true" do + expect(page.interruption_screen?).to be true + end + end + end + context "with a lettings log" do let(:lettings_log) { FactoryBot.build(:lettings_log, :in_progress) } diff --git a/spec/models/form/sales/subsections/household_needs_spec.rb b/spec/models/form/sales/subsections/household_needs_spec.rb index 5220f2d05..5395da095 100644 --- a/spec/models/form/sales/subsections/household_needs_spec.rb +++ b/spec/models/form/sales/subsections/household_needs_spec.rb @@ -18,6 +18,7 @@ RSpec.describe Form::Sales::Subsections::HouseholdNeeds, type: :model do buyer_still_serving armed_forces_spouse household_disability + disability_wheelchair_check household_wheelchair wheelchair_check ], diff --git a/spec/models/form_handler_spec.rb b/spec/models/form_handler_spec.rb index c272ac4a9..965e79690 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(182) + expect(form.pages.count).to eq(183) 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(182) + expect(form.pages.count).to eq(183) expect(form.name).to eq("2021_2022_sales") end end diff --git a/spec/models/validations/sales/soft_validations_spec.rb b/spec/models/validations/sales/soft_validations_spec.rb index 902790009..34f49d847 100644 --- a/spec/models/validations/sales/soft_validations_spec.rb +++ b/spec/models/validations/sales/soft_validations_spec.rb @@ -7,14 +7,14 @@ RSpec.describe Validations::Sales::SoftValidations do context "when validating soft min" do it "returns false if no income1 is given" do record.income1 = nil - expect(record) - .not_to be_income1_under_soft_min + + expect(record).not_to be_income1_under_soft_min end it "returns false if no ecstat1 is given" do record.ecstat1 = nil - expect(record) - .not_to be_income1_under_soft_min + + expect(record).not_to be_income1_under_soft_min end [ @@ -292,4 +292,41 @@ RSpec.describe Validations::Sales::SoftValidations do expect(record).not_to be_hodate_3_years_or_more_saledate end end + + describe "wheelchair_when_not_disabled" do + it "when hodate not set" do + record.disabled = 2 + record.wheel = nil + + expect(record).not_to be_wheelchair_when_not_disabled + end + + it "when disabled not set" do + record.disabled = nil + record.wheel = 1 + + expect(record).not_to be_wheelchair_when_not_disabled + end + + it "when disabled and wheel not set" do + record.disabled = nil + record.wheel = nil + + expect(record).not_to be_wheelchair_when_not_disabled + end + + it "when disabled == 2 && wheel == 1" do + record.disabled = 2 + record.wheel = 1 + + expect(record).to be_wheelchair_when_not_disabled + end + + it "when disabled == 2 && wheel != 1" do + record.disabled = 2 + record.wheel = 2 + + expect(record).not_to be_wheelchair_when_not_disabled + end + end end From 3308d86a106b2bde9fe1dae41af687b98c46526d Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Tue, 24 Jan 2023 15:21:09 +0000 Subject: [PATCH 3/8] Last settled accommodation and discounted ownership property postcodes must match (#1213) --- .../sales/household_validations.rb | 9 +++ config/locales/en.yml | 4 +- .../sales/household_validations_spec.rb | 58 +++++++++++++++++++ 3 files changed, 70 insertions(+), 1 deletion(-) diff --git a/app/models/validations/sales/household_validations.rb b/app/models/validations/sales/household_validations.rb index 8ee00fdfd..42846273c 100644 --- a/app/models/validations/sales/household_validations.rb +++ b/app/models/validations/sales/household_validations.rb @@ -32,6 +32,15 @@ module Validations::Sales::HouseholdValidations end end + def validate_previous_postcode(record) + return unless record.postcode_full && record.ppostcode_full && record.discounted_ownership_sale? + + unless record.postcode_full == record.ppostcode_full + record.errors.add :postcode_full, I18n.t("validations.household.postcode.discounted_ownership") + record.errors.add :ppostcode_full, I18n.t("validations.household.postcode.discounted_ownership") + end + end + private def validate_person_age_matches_relationship(record, person_num) diff --git a/config/locales/en.yml b/config/locales/en.yml index d84429637..68c7199de 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -369,7 +369,9 @@ en: condition_effects: no_choices: "You cannot answer this question as you told us nobody in the household has a physical or mental health condition (or other illness) expected to last 12 months or more" old_persons_shared_ownership: "Are you sure? At least one buyer should be aged over 64 for Older persons‘ shared ownership scheme" - + postcode: + discounted_ownership: "Last settled accommodation and discounted ownership property postcodes must match" + tenancy: length: fixed_term_not_required: "You must only answer the length of the tenancy if it's fixed-term" diff --git a/spec/models/validations/sales/household_validations_spec.rb b/spec/models/validations/sales/household_validations_spec.rb index 61a204878..5b227c36d 100644 --- a/spec/models/validations/sales/household_validations_spec.rb +++ b/spec/models/validations/sales/household_validations_spec.rb @@ -280,4 +280,62 @@ RSpec.describe Validations::Sales::HouseholdValidations do end end end + + describe "previous postcode validations" do + let(:record) { build(:sales_log) } + + context "with a discounted sale" do + before do + record.ownershipsch = 2 + end + + it "adds an error when previous and current postcodes are not the same" do + record.postcode_full = "SO32 3PT" + record.ppostcode_full = "DN6 7FB" + household_validator.validate_previous_postcode(record) + expect(record.errors["postcode_full"]) + .to include(match I18n.t("validations.household.postcode.discounted_ownership")) + expect(record.errors["ppostcode_full"]) + .to include(match I18n.t("validations.household.postcode.discounted_ownership")) + end + + it "allows same postcodes" do + record.postcode_full = "SO32 3PT" + record.ppostcode_full = "SO32 3PT" + household_validator.validate_previous_postcode(record) + expect(record.errors["postcode_full"]).to be_empty + expect(record.errors["ppostcode_full"]).to be_empty + end + + it "does not add an error when postcode is missing" do + record.postcode_full = nil + record.ppostcode_full = "SO32 3PT" + household_validator.validate_previous_postcode(record) + expect(record.errors["postcode_full"]).to be_empty + expect(record.errors["ppostcode_full"]).to be_empty + end + + it "does not add an error when previous postcode is missing" do + record.postcode_full = "SO32 3PT" + record.ppostcode_full = nil + household_validator.validate_previous_postcode(record) + expect(record.errors["postcode_full"]).to be_empty + expect(record.errors["ppostcode_full"]).to be_empty + end + end + + context "without a discounted sale" do + before do + record.ownershipsch = 1 + end + + it "allows different postcodes" do + record.postcode_full = "SO32 3PT" + record.ppostcode_full = "DN6 7FB" + household_validator.validate_previous_postcode(record) + expect(record.errors["postcode_full"]).to be_empty + expect(record.errors["ppostcode_full"]).to be_empty + end + end + end end From 10536ae13fb7536d491d261a0ba4281762e131de Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Tue, 24 Jan 2023 16:45:33 +0000 Subject: [PATCH 4/8] Update reson answer options (#1209) --- config/forms/2021_2022.json | 6 +++--- config/forms/2022_2023.json | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/config/forms/2021_2022.json b/config/forms/2021_2022.json index f586b3069..e47191f6e 100644 --- a/config/forms/2021_2022.json +++ b/config/forms/2021_2022.json @@ -6166,10 +6166,10 @@ "1": { "value": "Permanently decanted from another property owned by this landlord" }, - "45": { + "46": { "value": "Discharged from long-stay hospital or similar institution" }, - "44": { + "45": { "value": "Discharged from prison" }, "2": { @@ -6181,7 +6181,7 @@ "9": { "value": "Asked to leave by family or friends" }, - "46": { + "44": { "value": "Death of household member in last settled accommodation" }, "8": { diff --git a/config/forms/2022_2023.json b/config/forms/2022_2023.json index a5048b958..7c54b67d4 100644 --- a/config/forms/2022_2023.json +++ b/config/forms/2022_2023.json @@ -6165,10 +6165,10 @@ "1": { "value": "Permanently decanted from another property owned by this landlord" }, - "45": { + "46": { "value": "Discharged from long-stay hospital or similar institution" }, - "44": { + "45": { "value": "Discharged from prison" }, "2": { @@ -6180,7 +6180,7 @@ "9": { "value": "Asked to leave by family or friends" }, - "46": { + "44": { "value": "Death of household member in last settled accommodation" }, "8": { From 8ca9cbfb499ce455752587019794f3a9ccdc3b5b Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Wed, 25 Jan 2023 16:34:16 +0000 Subject: [PATCH 5/8] Change numeric validation to display prefixes in error message (#1227) * change numeric validation * Fix test, add delimiter and suffix to the numeric error mesage --- .../form/sales/questions/deposit_amount.rb | 2 +- .../sales/sale_information_validations.rb | 8 ---- app/models/validations/shared_validations.rb | 8 +++- .../sales/questions/deposit_amount_spec.rb | 2 +- .../sale_information_validations_spec.rb | 42 ------------------- .../validations/shared_validations_spec.rb | 19 +++++++++ 6 files changed, 27 insertions(+), 54 deletions(-) diff --git a/app/models/form/sales/questions/deposit_amount.rb b/app/models/form/sales/questions/deposit_amount.rb index 05403dbab..e4fd4ceb2 100644 --- a/app/models/form/sales/questions/deposit_amount.rb +++ b/app/models/form/sales/questions/deposit_amount.rb @@ -7,7 +7,7 @@ class Form::Sales::Questions::DepositAmount < ::Form::Question @type = "numeric" @min = 0 @width = 5 - @max = 9_999_999 + @max = 999_999 @prefix = "£" @hint_text = "Enter the total cash sum paid by the buyer towards the property that was not funded by the mortgage" @derived = true diff --git a/app/models/validations/sales/sale_information_validations.rb b/app/models/validations/sales/sale_information_validations.rb index 49e39d052..7b0b32206 100644 --- a/app/models/validations/sales/sale_information_validations.rb +++ b/app/models/validations/sales/sale_information_validations.rb @@ -1,12 +1,4 @@ module Validations::Sales::SaleInformationValidations - def validate_deposit_range(record) - return if record.deposit.blank? - - unless record.deposit >= 0 && record.deposit <= 999_999 - record.errors.add :deposit, "Cash deposit must be £0 - £999,999" - end - end - def validate_pratical_completion_date_before_saledate(record) return if record.saledate.blank? || record.hodate.blank? diff --git a/app/models/validations/shared_validations.rb b/app/models/validations/shared_validations.rb index 93a81e938..2d7ac0c9e 100644 --- a/app/models/validations/shared_validations.rb +++ b/app/models/validations/shared_validations.rb @@ -1,4 +1,6 @@ module Validations::SharedValidations + include ActionView::Helpers::NumberHelper + def validate_other_field(record, value_other = nil, main_field = nil, other_field = nil, main_label = nil, other_label = nil) return unless main_field || other_field @@ -19,17 +21,19 @@ module Validations::SharedValidations next unless record[question.id] field = question.check_answer_label || question.id + min = [question.prefix, number_with_delimiter(question.min, delimiter: ","), question.suffix].join("") + max = [question.prefix, number_with_delimiter(question.max, delimiter: ","), question.suffix].join("") begin answer = Float(record.public_send("#{question.id}_before_type_cast")) rescue ArgumentError - record.errors.add question.id.to_sym, I18n.t("validations.numeric.valid", field:, min: question.min, max: question.max) + record.errors.add question.id.to_sym, I18n.t("validations.numeric.valid", field:, min:, max:) end next unless answer if (question.min && question.min > answer) || (question.max && question.max < answer) - record.errors.add question.id.to_sym, I18n.t("validations.numeric.valid", field:, min: question.min, max: question.max) + record.errors.add question.id.to_sym, I18n.t("validations.numeric.valid", field:, min:, max:) end end end diff --git a/spec/models/form/sales/questions/deposit_amount_spec.rb b/spec/models/form/sales/questions/deposit_amount_spec.rb index 961576d62..33a5dcf1d 100644 --- a/spec/models/form/sales/questions/deposit_amount_spec.rb +++ b/spec/models/form/sales/questions/deposit_amount_spec.rb @@ -48,6 +48,6 @@ RSpec.describe Form::Sales::Questions::DepositAmount, type: :model do end it "has correct max" do - expect(question.max).to eq(9_999_999) + expect(question.max).to eq(999_999) end end diff --git a/spec/models/validations/sales/sale_information_validations_spec.rb b/spec/models/validations/sales/sale_information_validations_spec.rb index fafe2786a..052e171be 100644 --- a/spec/models/validations/sales/sale_information_validations_spec.rb +++ b/spec/models/validations/sales/sale_information_validations_spec.rb @@ -5,48 +5,6 @@ RSpec.describe Validations::Sales::SaleInformationValidations do let(:validator_class) { Class.new { include Validations::Sales::SaleInformationValidations } } - describe "#validate_deposit_range" do - context "when within permitted bounds" do - let(:record) { build(:sales_log, deposit: 0) } - - it "does not add an error" do - sale_information_validator.validate_deposit_range(record) - - expect(record.errors[:deposit]).not_to be_present - end - end - - context "when blank" do - let(:record) { build(:sales_log, deposit: nil) } - - it "does not add an error" do - sale_information_validator.validate_deposit_range(record) - - expect(record.errors[:deposit]).not_to be_present - end - end - - context "when below lower bound" do - let(:record) { build(:sales_log, deposit: -1) } - - it "adds an error" do - sale_information_validator.validate_deposit_range(record) - - expect(record.errors[:deposit]).to be_present - end - end - - context "when higher than upper bound" do - let(:record) { build(:sales_log, deposit: 1_000_000) } - - it "adds an error" do - sale_information_validator.validate_deposit_range(record) - - expect(record.errors[:deposit]).to be_present - end - end - end - describe "#validate_pratical_completion_date_before_saledate" do context "when hodate blank" do let(:record) { build(:sales_log, hodate: nil) } diff --git a/spec/models/validations/shared_validations_spec.rb b/spec/models/validations/shared_validations_spec.rb index a88b5dda2..abdde5f45 100644 --- a/spec/models/validations/shared_validations_spec.rb +++ b/spec/models/validations/shared_validations_spec.rb @@ -5,6 +5,7 @@ RSpec.describe Validations::SharedValidations do let(:validator_class) { Class.new { include Validations::SharedValidations } } let(:record) { FactoryBot.create(:lettings_log) } + let(:sales_record) { FactoryBot.create(:sales_log) } let(:fake_2021_2022_form) { Form.new("spec/fixtures/forms/2021_2022.json") } describe "numeric min max validations" do @@ -67,6 +68,24 @@ RSpec.describe Validations::SharedValidations do expect(record.errors["age6"]).to be_empty end end + + context "when validating percent" do + it "validates that % suffix is added in the error message" do + sales_record.stairbought = "random" + 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 percent", max: "100 percent")) + end + end + + context "when validating price" do + it "validates that £ prefix and , is added in the error message" do + sales_record.income1 = "random" + 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")) + end + end end describe "radio options validations" do From 1a5a64e18ede90000ab296123429bf6e0771b644 Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Wed, 25 Jan 2023 16:44:26 +0000 Subject: [PATCH 6/8] handle bulk upload files with boms (#1233) --- app/services/bulk_upload/lettings/csv_parser.rb | 2 +- .../bulk_upload/lettings/csv_parser_spec.rb | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/app/services/bulk_upload/lettings/csv_parser.rb b/app/services/bulk_upload/lettings/csv_parser.rb index b8e66678b..ae29f4a6f 100644 --- a/app/services/bulk_upload/lettings/csv_parser.rb +++ b/app/services/bulk_upload/lettings/csv_parser.rb @@ -50,7 +50,7 @@ private def normalised_string return @normalised_string if @normalised_string - @normalised_string = File.read(path) + @normalised_string = File.read(path, encoding: "bom|utf-8") @normalised_string.gsub!("\r\n", "\n") @normalised_string diff --git a/spec/services/bulk_upload/lettings/csv_parser_spec.rb b/spec/services/bulk_upload/lettings/csv_parser_spec.rb index 6711d82cd..9d381a5a0 100644 --- a/spec/services/bulk_upload/lettings/csv_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/csv_parser_spec.rb @@ -35,4 +35,21 @@ RSpec.describe BulkUpload::Lettings::CsvParser do expect(service.row_parsers[0].field_12).to eql(log.age1) end end + + context "when parsing with BOM aka byte order mark" do + let(:file) { Tempfile.new } + let(:path) { file.path } + let(:log) { build(:lettings_log, :completed) } + let(:bom) { "\uFEFF" } + + before do + file.write(bom) + file.write(BulkUpload::LogToCsv.new(log:, col_offset: 0).to_csv_row) + file.close + end + + it "parses csv correctly" do + expect(service.row_parsers[0].field_12).to eql(log.age1) + end + end end From 56a9cb4bdf9ed5f520d14ed676084399991c398c Mon Sep 17 00:00:00 2001 From: Jack <113976590+bibblobcode@users.noreply.github.com> Date: Thu, 26 Jan 2023 09:09:17 +0000 Subject: [PATCH 7/8] CLDC-864 Add contract exchange date validations (#1231) * CLDC-864 Add exdate validations * Raise missing translation errors * Fix missing translation * Remove unused method * Remove redundant check --- app/helpers/tab_nav_helper.rb | 7 -- .../sales/sale_information_validations.rb | 10 +++ config/environments/development.rb | 2 +- config/environments/test.rb | 2 +- config/locales/en.yml | 19 +++-- spec/helpers/tab_nav_helper_spec.rb | 22 ------ .../sale_information_validations_spec.rb | 76 +++++++++++++++++++ 7 files changed, 100 insertions(+), 38 deletions(-) diff --git a/app/helpers/tab_nav_helper.rb b/app/helpers/tab_nav_helper.rb index 801560b91..c29ae5d86 100644 --- a/app/helpers/tab_nav_helper.rb +++ b/app/helpers/tab_nav_helper.rb @@ -21,11 +21,4 @@ module TabNavHelper role = "#{user.role.to_s.humanize}" [user.organisation.name, role].join("\n") end - - def tab_items(user) - [ - { name: t("Details"), url: details_organisation_path(user.organisation) }, - { name: t("Users"), url: users_organisation_path(user.organisation) }, - ] - end end diff --git a/app/models/validations/sales/sale_information_validations.rb b/app/models/validations/sales/sale_information_validations.rb index 7b0b32206..c70a8d57d 100644 --- a/app/models/validations/sales/sale_information_validations.rb +++ b/app/models/validations/sales/sale_information_validations.rb @@ -7,6 +7,16 @@ module Validations::Sales::SaleInformationValidations end end + def validate_exchange_date(record) + return unless record.exdate && record.saledate + + record.errors.add(:exdate, I18n.t("validations.sale_information.exdate.must_be_before_saledate")) if record.exdate > record.saledate + + return if (record.saledate.to_date - record.exdate.to_date).to_i / 365 < 1 + + record.errors.add(:exdate, I18n.t("validations.sale_information.exdate.must_be_less_than_1_year_from_saledate")) + end + def validate_previous_property_unit_type(record) return unless record.fromprop && record.frombeds diff --git a/config/environments/development.rb b/config/environments/development.rb index 24155edfa..6261c29c8 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -73,7 +73,7 @@ Rails.application.configure do config.active_record.verbose_query_logs = true # Raises error for missing translations. - # config.i18n.raise_on_missing_translations = true + config.i18n.raise_on_missing_translations = true # Annotate rendered view with file names. # config.action_view.annotate_rendered_view_with_filenames = true diff --git a/config/environments/test.rb b/config/environments/test.rb index dca19f3d1..1c399b19d 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -57,7 +57,7 @@ Rails.application.configure do config.active_support.disallowed_deprecation_warnings = [] # Raises error for missing translations. - # config.i18n.raise_on_missing_translations = true + config.i18n.raise_on_missing_translations = true # Annotate rendered view with file names. # config.action_view.annotate_rendered_view_with_filenames = true diff --git a/config/locales/en.yml b/config/locales/en.yml index 68c7199de..cdad12a52 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -301,7 +301,7 @@ en: child_over_20: "Answer cannot be 20 or over as the relationship is ‘child’" not_student_16_19: "Answer cannot be between 16 and 19 as person %{person_num} is a child of the lead tenant but is not a full-time student" student_16_19: - cannot_be_16_19: + cannot_be_16_19: student_not_child: "Person cannot be aged 16-19 if they are a student but don't have relationship ‘child’" child_not_student: "Person cannot be aged 16-19 if they have relationship ‘child’ but are not a student" must_be_16_19: "Person must be aged 16-19 if they are a student and have relationship ‘child’" @@ -313,8 +313,8 @@ en: child_under_16: "Person’s %{person_num} working situation must be ’child under 16‘ as you told us they’re under 16" child_over_16: "Answer cannot be ‘child under 16’ as you told us the person %{person_num} is older than 16" not_student_16_19: "Person’s %{person_num} working situation must be full-time student or prefers not to say as you told us they’re between 16 and 19." - student_16_19: - cannot_be_student: + student_16_19: + cannot_be_student: child_not_16_19: "Person cannot be a student if they are not aged 16-19 but have relationship ‘child’" 16_19_not_child: "Person cannot be a student if they are aged 16-19 but don‘t have relationship ‘child’" must_be_student: "Person must be a student if they are aged 16-19 and have relationship ‘child’" @@ -326,8 +326,8 @@ en: child_over_20: "Answer cannot be ‘child’ if the person's age is 20 or over" one_partner: "Number of partners cannot be greater than 1" not_student_16_19: "Answer cannot be ‘child’ as you told us the person %{person_num} is between 16 and 19 and is not a full-time student" - student_16_19: - cannot_be_child: + student_16_19: + cannot_be_child: student_not_16_19: "Answer cannot be ‘child’ if the person is a student but not aged 16-19" 16_19_not_student: "Answer cannot be ‘child’ if the person is aged 16-19 but not a student" must_be_child: "Answer must be ‘child’ if the person is aged 16-19 and a student" @@ -413,9 +413,14 @@ en: deactivation: during_deactivated_period: "The location is already deactivated during this date, please enter a different date" sale_information: - previous_property_beds: + exdate: + must_be_before_saledate: + Contract exchange date must be less than 1 year before completion date + must_be_less_than_1_year_from_saledate: + Contract exchange date must be less than 1 year before completion date + previous_property_beds: property_type_bedsit: "Bedsit bedroom maximum 1" - previous_property_type: + previous_property_type: property_type_bedsit: "A bedsit can not have more than 1 bedroom" soft_validations: diff --git a/spec/helpers/tab_nav_helper_spec.rb b/spec/helpers/tab_nav_helper_spec.rb index b2273aa11..7b4efc5eb 100644 --- a/spec/helpers/tab_nav_helper_spec.rb +++ b/spec/helpers/tab_nav_helper_spec.rb @@ -34,26 +34,4 @@ RSpec.describe TabNavHelper do expect(scheme_cell(scheme)).to match(expected_html) end end - - describe "#tab_items" do - context "when user is a data_coordinator" do - let(:user) { FactoryBot.build(:user, :data_coordinator, organisation:) } - - it "returns details and user tabs" do - result = tab_items(user).map { |i| i[:name] } - expect(result.count).to eq(2) - expect(result.first).to match("Details") - expect(result.second).to match("Users") - end - end - - context "when user is a data_provider" do - it "returns details and user tabs" do - result = tab_items(user).map { |i| i[:name] } - expect(result.count).to eq(2) - expect(result.first).to match("Details") - expect(result.second).to match("Users") - end - end - end end diff --git a/spec/models/validations/sales/sale_information_validations_spec.rb b/spec/models/validations/sales/sale_information_validations_spec.rb index 052e171be..409972040 100644 --- a/spec/models/validations/sales/sale_information_validations_spec.rb +++ b/spec/models/validations/sales/sale_information_validations_spec.rb @@ -67,6 +67,82 @@ RSpec.describe Validations::Sales::SaleInformationValidations do end end + describe "#validate_exchange_date" do + context "when exdate blank" do + let(:record) { build(:sales_log, exdate: nil) } + + it "does not add an error" do + sale_information_validator.validate_exchange_date(record) + + expect(record.errors[:exdate]).not_to be_present + end + end + + context "when saledate blank" do + let(:record) { build(:sales_log, saledate: nil) } + + it "does not add an error" do + sale_information_validator.validate_exchange_date(record) + + expect(record.errors[:exdate]).not_to be_present + end + end + + context "when saledate and exdate blank" do + let(:record) { build(:sales_log, exdate: nil, saledate: nil) } + + it "does not add an error" do + sale_information_validator.validate_exchange_date(record) + + expect(record.errors[:exdate]).not_to be_present + end + end + + context "when exdate before saledate" do + let(:record) { build(:sales_log, exdate: 2.months.ago, saledate: 1.month.ago) } + + it "does not add the error" do + sale_information_validator.validate_exchange_date(record) + + expect(record.errors[:exdate]).not_to be_present + end + end + + context "when exdate more than 1 year before saledate" do + let(:record) { build(:sales_log, exdate: 2.years.ago, saledate: 1.month.ago) } + + it "does not add the error" 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"], + ) + end + end + + context "when exdate after saledate" do + let(:record) { build(:sales_log, exdate: 1.month.ago, saledate: 2.months.ago) } + + it "adds error" 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"], + ) + end + end + + context "when exdate == saledate" do + let(:record) { build(:sales_log, exdate: Time.zone.parse("2023-07-01"), saledate: Time.zone.parse("2023-07-01")) } + + it "does not add an error" do + sale_information_validator.validate_exchange_date(record) + + expect(record.errors[:exdate]).not_to be_present + end + end + end + describe "#validate_previous_property_unit_type" do context "when number of bedrooms is <= 1" do let(:record) { FactoryBot.build(:sales_log, frombeds: 1, fromprop: 2) } From c2c1b4dbb0bd84189c712679ec6c74b160f039c8 Mon Sep 17 00:00:00 2001 From: Jack <113976590+bibblobcode@users.noreply.github.com> Date: Thu, 26 Jan 2023 09:48:30 +0000 Subject: [PATCH 8/8] Add mortgage length hint (#1237) --- app/models/form/sales/questions/mortgage_length.rb | 1 + spec/models/form/sales/questions/mortgage_length_spec.rb | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app/models/form/sales/questions/mortgage_length.rb b/app/models/form/sales/questions/mortgage_length.rb index 317489763..56a358f01 100644 --- a/app/models/form/sales/questions/mortgage_length.rb +++ b/app/models/form/sales/questions/mortgage_length.rb @@ -8,5 +8,6 @@ class Form::Sales::Questions::MortgageLength < ::Form::Question @min = 0 @width = 5 @suffix = " years" + @hint_text = "You should round up to the nearest year. Value should not exceed 60 years." end end diff --git a/spec/models/form/sales/questions/mortgage_length_spec.rb b/spec/models/form/sales/questions/mortgage_length_spec.rb index 519289607..e96a05174 100644 --- a/spec/models/form/sales/questions/mortgage_length_spec.rb +++ b/spec/models/form/sales/questions/mortgage_length_spec.rb @@ -32,7 +32,9 @@ RSpec.describe Form::Sales::Questions::MortgageLength, type: :model do end it "has the correct hint" do - expect(question.hint_text).to be_nil + expect(question.hint_text).to eq( + "You should round up to the nearest year. Value should not exceed 60 years.", + ) end it "has correct width" do