From 74dea157d655a9fbc95c146f879cadbd3032c22b Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Fri, 1 Mar 2024 10:02:45 +0000 Subject: [PATCH 1/5] CLDC-3253 Validate nationality in bulk upload (#2257) * Validate nationality in bulk upload * Update valid_nationality_options --- .../lettings/year2024/row_parser.rb | 14 +++++++++- .../bulk_upload/sales/year2024/row_parser.rb | 22 ++++++++++++++-- config/locales/en.yml | 1 + .../lettings/year2024/row_parser_spec.rb | 13 +++++++++- .../sales/year2024/row_parser_spec.rb | 26 +++++++++++++++++-- 5 files changed, 70 insertions(+), 6 deletions(-) diff --git a/app/services/bulk_upload/lettings/year2024/row_parser.rb b/app/services/bulk_upload/lettings/year2024/row_parser.rb index 08fbec85a..6ba9ab69a 100644 --- a/app/services/bulk_upload/lettings/year2024/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2024/row_parser.rb @@ -383,6 +383,7 @@ class BulkUpload::Lettings::Year2024::RowParser validate :validate_incomplete_soft_validations, on: :after_log validate :validate_all_charges_given, on: :after_log, if: proc { is_carehome.zero? } + validate :validate_nationality, on: :after_log def self.question_for_field(field) QUESTIONS[field] @@ -557,6 +558,12 @@ private end end + def validate_nationality + if field_45.present? && !valid_nationality_options.include?(field_45.to_s) + errors.add(:field_45, I18n.t("validations.household.nationality")) + end + end + def duplicate_check_fields [ "startdate", @@ -914,6 +921,7 @@ private ethnic_group: %i[field_44], ethnic: %i[field_44], nationality_all: %i[field_45], + nationality_all_group: %i[field_45], relat2: %i[field_47], relat3: %i[field_51], @@ -1087,7 +1095,7 @@ private attributes["ethnic_group"] = ethnic_group_from_ethnic attributes["ethnic"] = field_44 - attributes["nationality_all"] = field_45 + attributes["nationality_all"] = field_45 if field_45.present? && valid_nationality_options.include?(field_45.to_s) attributes["nationality_all_group"] = nationality_group(attributes["nationality_all"]) attributes["relat2"] = field_47 @@ -1486,6 +1494,10 @@ private end end + def valid_nationality_options + %w[0] + GlobalConstants::COUNTRIES_ANSWER_OPTIONS.keys # 0 is "Prefers not to say" + end + def nationality_group(nationality_value) return unless nationality_value return 0 if nationality_value.zero? diff --git a/app/services/bulk_upload/sales/year2024/row_parser.rb b/app/services/bulk_upload/sales/year2024/row_parser.rb index 2c1b6089a..70aaee49a 100644 --- a/app/services/bulk_upload/sales/year2024/row_parser.rb +++ b/app/services/bulk_upload/sales/year2024/row_parser.rb @@ -469,6 +469,8 @@ class BulkUpload::Sales::Year2024::RowParser validate :validate_if_log_already_exists, on: :after_log, if: -> { FeatureToggle.bulk_upload_duplicate_log_check_enabled? } validate :validate_buyers_organisations, on: :after_log + validate :validate_nationality, on: :after_log + validate :validate_buyer_2_nationality, on: :after_log def self.question_for_field(field) QUESTIONS[field] @@ -834,7 +836,7 @@ private attributes["ethnic_group"] = ethnic_group_from_ethnic attributes["ethnic"] = field_33 - attributes["nationality_all"] = field_34 + attributes["nationality_all"] = field_34 if field_34.present? && valid_nationality_options.include?(field_34.to_s) attributes["nationality_all_group"] = nationality_group(attributes["nationality_all"]) attributes["income1nk"] = field_77 == "R" ? 1 : 0 @@ -942,7 +944,7 @@ private attributes["ethnic_group2"] = infer_buyer2_ethnic_group_from_ethnic attributes["ethnicbuy2"] = field_40 - attributes["nationality_all_buyer2"] = field_41 + attributes["nationality_all_buyer2"] = field_41 if field_41.present? && valid_nationality_options.include?(field_41.to_s) attributes["nationality_all_buyer2_group"] = nationality_group(attributes["nationality_all_buyer2"]) attributes["buy2living"] = field_70 @@ -1341,4 +1343,20 @@ private errors.add(:field_35, "Buyer 1 cannot be a child under 16") end end + + def validate_nationality + if field_34.present? && !valid_nationality_options.include?(field_34.to_s) + errors.add(:field_34, I18n.t("validations.household.nationality")) + end + end + + def validate_buyer_2_nationality + if field_41.present? && !valid_nationality_options.include?(field_41.to_s) + errors.add(:field_41, I18n.t("validations.household.nationality")) + end + end + + def valid_nationality_options + %w[0] + GlobalConstants::COUNTRIES_ANSWER_OPTIONS.keys # 0 is "Prefers not to say" + end end diff --git a/config/locales/en.yml b/config/locales/en.yml index 9746ba13f..0ed12c806 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -552,6 +552,7 @@ en: buylivein: buyers_will_live_in_property_values_inconsistent_setup: "You have already told us that both buyer 1 and buyer 2 will not live in the property" buyers_will_live_in_property_values_inconsistent: "You have already told us that the buyers will live in the property. Either buyer 1 or buyer 2 must live in the property" + nationality: "Select a valid nationality" tenancy: length: diff --git a/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb b/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb index dddf8a744..73e639449 100644 --- a/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb @@ -149,7 +149,7 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do field_73: "M", field_44: "17", - field_45: "18", + field_45: "826", field_47: "P", field_51: "C", @@ -1594,6 +1594,17 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do expect(parser.log.nationality_all_group).to be(826) end end + + context "when field_45 is not a valid option" do + let(:attributes) { setup_section_params.merge({ field_45: "123123" }) } + + it "is correctly set" do + parser.log.save! + expect(parser.log.nationality_all).to be(nil) + expect(parser.log.nationality_all_group).to be(nil) + expect(parser.errors["field_45"]).to include("Select a valid nationality") + end + end end describe "soft validations" do diff --git a/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb b/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb index 5a1a9e4df..6a357f46a 100644 --- a/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb +++ b/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb @@ -57,14 +57,14 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do field_31: "32", field_32: "M", field_33: "12", - field_34: "18", + field_34: "28", field_35: "1", field_36: "1", field_37: "R", field_38: "32", field_39: "F", field_40: "17", - field_41: "13", + field_41: "28", field_42: "2", field_43: "1", field_44: "0", @@ -1208,6 +1208,17 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do expect(parser.log.nationality_all_group).to be(826) end end + + context "when field_34 is not a valid option" do + let(:attributes) { setup_section_params.merge({ field_34: "123123" }) } + + it "is correctly set" do + parser.valid? + expect(parser.log.nationality_all).to be(nil) + expect(parser.log.nationality_all_group).to be(nil) + expect(parser.errors["field_34"]).to include("Select a valid nationality") + end + end end describe "#nationality_all_buyer2" do @@ -1282,6 +1293,17 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do expect(parser.log.nationality_all_buyer2_group).to be(826) end end + + context "when field_41 is not a valid option" do + let(:attributes) { setup_section_params.merge({ field_41: "123123" }) } + + it "is correctly set" do + parser.valid? + expect(parser.log.nationality_all_buyer2).to be(nil) + expect(parser.log.nationality_all_buyer2_group).to be(nil) + expect(parser.errors["field_41"]).to include("Select a valid nationality") + end + end end describe "#buy2living" do From 6d2fc666700e65846885bd6ee7f8b4b0c5915a4d Mon Sep 17 00:00:00 2001 From: Robert Sullivan Date: Fri, 1 Mar 2024 11:28:25 +0000 Subject: [PATCH 2/5] Fix test failures due to BST changeover (#2283) --- .../merge/merge_organisations_service_spec.rb | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/spec/services/merge/merge_organisations_service_spec.rb b/spec/services/merge/merge_organisations_service_spec.rb index 8d9b9ff46..44fce4a1c 100644 --- a/spec/services/merge/merge_organisations_service_spec.rb +++ b/spec/services/merge/merge_organisations_service_spec.rb @@ -221,19 +221,21 @@ RSpec.describe Merge::MergeOrganisationsService do expect(absorbed_scheme.locations.count).to eq(1) absorbed_location = absorbed_scheme.locations.first - expect(absorbed_scheme.startdate).to eq(Time.zone.today + 1.month) - expect(absorbed_location.startdate).to eq(Time.zone.today + 1.month) + expected_date = Time.zone.today + 1.month + expect(absorbed_scheme.startdate).to eq(expected_date.in_time_zone) + expect(absorbed_location.startdate).to eq(expected_date.in_time_zone) end it "deactivates schemes and locations on the merged organisation on the startdate" do merge_organisations_service.call + expected_date = Time.zone.today + 1.month expect(scheme.owning_organisation).to eq(merging_organisation) expect(location.scheme).to eq(scheme) expect(scheme.scheme_deactivation_periods.count).to eq(1) - expect(scheme.scheme_deactivation_periods.first.deactivation_date).to eq(Time.zone.today + 1.month) + expect(scheme.scheme_deactivation_periods.first.deactivation_date).to eq(expected_date.in_time_zone) expect(location.location_deactivation_periods.count).to eq(1) - expect(location.location_deactivation_periods.first.deactivation_date).to eq(Time.zone.today + 1.month) + expect(location.location_deactivation_periods.first.deactivation_date).to eq(expected_date.in_time_zone) end end end @@ -376,15 +378,16 @@ RSpec.describe Merge::MergeOrganisationsService do expect(absorbed_scheme.locations.count).to eq(1) absorbed_location = absorbed_scheme.locations.first + expected_reactivation_date = Time.zone.today + 1.month expect(absorbed_scheme.startdate).to eq(Time.zone.today) expect(absorbed_scheme.scheme_deactivation_periods.count).to eq(1) expect(absorbed_scheme.scheme_deactivation_periods.first.deactivation_date).to eq(Time.zone.today) - expect(absorbed_scheme.scheme_deactivation_periods.first.reactivation_date).to eq(Time.zone.today + 1.month) + expect(absorbed_scheme.scheme_deactivation_periods.first.reactivation_date).to eq(expected_reactivation_date.in_time_zone) expect(absorbed_location.startdate).to eq(Time.zone.today) expect(absorbed_location.location_deactivation_periods.count).to eq(1) expect(absorbed_location.location_deactivation_periods.first.deactivation_date).to eq(Time.zone.today) - expect(absorbed_location.location_deactivation_periods.first.reactivation_date).to eq(Time.zone.today + 1.month) + expect(absorbed_location.location_deactivation_periods.first.reactivation_date).to eq(expected_reactivation_date.in_time_zone) end it "deactivates schemes and locations on the merged organisation" do From 66bde20de263b16fe4e57f018b308f9edc274dde Mon Sep 17 00:00:00 2001 From: Rachael Booth Date: Fri, 1 Mar 2024 11:47:57 +0000 Subject: [PATCH 3/5] Remove remaining cloud foundry config and manifests (#2281) --- .cfignore | 2 -- config/cloud_foundry/review_manifest.yml | 21 ----------- lib/tasks/cf.rake | 8 ----- manifest.yml | 46 ------------------------ 4 files changed, 77 deletions(-) delete mode 100644 .cfignore delete mode 100644 config/cloud_foundry/review_manifest.yml delete mode 100644 lib/tasks/cf.rake delete mode 100644 manifest.yml diff --git a/.cfignore b/.cfignore deleted file mode 100644 index 9aae254ab..000000000 --- a/.cfignore +++ /dev/null @@ -1,2 +0,0 @@ -docs/* -spec/* diff --git a/config/cloud_foundry/review_manifest.yml b/config/cloud_foundry/review_manifest.yml deleted file mode 100644 index a0f1afb4e..000000000 --- a/config/cloud_foundry/review_manifest.yml +++ /dev/null @@ -1,21 +0,0 @@ ---- -defaults: &defaults - buildpacks: - - https://github.com/cloudfoundry/ruby-buildpack.git - processes: - - type: web - command: bundle exec rake cf:on_first_instance db:migrate db:seed && bin/rails server - instances: 1 - memory: 1G - - type: worker - command: bundle exec sidekiq -t 3 - health-check-type: process - instances: 1 - health-check-type: http - health-check-http-endpoint: /health - -applications: - - name: dluhc-core-review - <<: *defaults - env: - RAILS_ENV: review diff --git a/lib/tasks/cf.rake b/lib/tasks/cf.rake deleted file mode 100644 index 111ea7395..000000000 --- a/lib/tasks/cf.rake +++ /dev/null @@ -1,8 +0,0 @@ -namespace :cf do - desc "Only run on the first application instance" - task on_first_instance: :environment do - # We expect this information to be always available or break otherwise - instance_index = JSON.parse(ENV["VCAP_APPLICATION"])["instance_index"] - exit(0) unless instance_index.zero? - end -end diff --git a/manifest.yml b/manifest.yml deleted file mode 100644 index 86793319f..000000000 --- a/manifest.yml +++ /dev/null @@ -1,46 +0,0 @@ ---- -defaults: &defaults - buildpacks: - - https://github.com/cloudfoundry/ruby-buildpack.git - processes: - - type: web - command: bundle exec rake cf:on_first_instance db:migrate && bin/rails server - instances: 2 - memory: 1G - - type: worker - command: bundle exec sidekiq -t 3 - disk_quota: 4G - health-check-type: process - instances: 2 - memory: 8G - health-check-type: http - health-check-http-endpoint: /health - -applications: - - name: dluhc-core-staging - <<: *defaults - env: - RAILS_ENV: staging - services: - - dluhc-core-staging-postgres - - dluhc-core-staging-redis - - - name: dluhc-core-production - <<: *defaults - processes: - - type: web - command: bundle exec rake cf:on_first_instance db:migrate && bin/rails server - instances: 4 - memory: 1G - - type: worker - command: bundle exec sidekiq -t 3 - disk_quota: 4G - health-check-type: process - instances: 2 - memory: 8G - env: - RAILS_ENV: production - host: submit-social-housing-lettings-sales-data - services: - - dluhc-core-production-postgres - - dluhc-core-production-redis From 1dcdc0806a47c440d7101caafca82ab851687c05 Mon Sep 17 00:00:00 2001 From: Rachael Booth Date: Mon, 4 Mar 2024 09:42:43 +0000 Subject: [PATCH 4/5] CLDC-3187: Display org id on stock owner / managing agent pages (#2274) * CLDC-3187: Display org id on stock owner / managing agent pages * Fix indentation * Refactor stock owner / managing agent list partials into one shared partial * Fix linting * Use managing agents instead of agents in search component display * Update tests to reflect agents -> managing agents change --- .../_managing_agent_list.erb | 22 ------------ .../_related_org_list.erb | 36 +++++++++++++++++++ .../_stock_owner_list.erb | 22 ------------ .../managing_agents.html.erb | 11 +++++- .../stock_owners.html.erb | 11 +++++- ...anisation_relationships_controller_spec.rb | 6 ++-- 6 files changed, 59 insertions(+), 49 deletions(-) delete mode 100644 app/views/organisation_relationships/_managing_agent_list.erb create mode 100644 app/views/organisation_relationships/_related_org_list.erb delete mode 100644 app/views/organisation_relationships/_stock_owner_list.erb diff --git a/app/views/organisation_relationships/_managing_agent_list.erb b/app/views/organisation_relationships/_managing_agent_list.erb deleted file mode 100644 index 81a1f6070..000000000 --- a/app/views/organisation_relationships/_managing_agent_list.erb +++ /dev/null @@ -1,22 +0,0 @@ -
- <%= govuk_table do |table| %> - <%= table.with_caption(classes: %w[govuk-!-font-size-19 govuk-!-font-weight-regular]) do |caption| %> - <%= render(SearchResultCaptionComponent.new(searched:, count: pagy.count, item_label:, total_count:, item: "agents", filters_count: 0)) %> - <% end %> - <% @managing_agents.each do |managing_agent| %> - <%= table.with_body do |body| %> - <%= body.with_row do |row| %> - <% row.with_cell(text: managing_agent.name) %> - <% if current_user.data_coordinator? || current_user.support? %> - <% row.with_cell(html_attributes: { - scope: "row", - class: "govuk-!-text-align-right", - }) do %> - <%= govuk_link_to("Remove", managing_agents_remove_organisation_path(target_organisation_id: managing_agent.id)) %> - <% end %> - <% end %> - <% end %> - <% end %> - <% end %> - <% end %> -
diff --git a/app/views/organisation_relationships/_related_org_list.erb b/app/views/organisation_relationships/_related_org_list.erb new file mode 100644 index 000000000..e4c32f23f --- /dev/null +++ b/app/views/organisation_relationships/_related_org_list.erb @@ -0,0 +1,36 @@ +
+ <%= govuk_table do |table| %> + <%= table.with_caption(classes: %w[govuk-!-font-size-19 govuk-!-font-weight-regular]) do |caption| %> + <%= render(SearchResultCaptionComponent.new(searched:, count: pagy.count, item_label:, total_count:, item: search_item, filters_count: 0)) %> + <% end %> + <%= table.with_head do |head| %> + <%= head.with_row do |row| %> + <% row.with_cell(header: true, text: "Organisation name", html_attributes: { scope: "col", class: "govuk-!-width-one-half" }) %> + <% row.with_cell(header: true, text: "Organisation ID", html_attributes: { scope: "col", class: "govuk-!-width-one-half" }) %> + <% if current_user.data_coordinator? || current_user.support? %> + <% row.with_cell %> + <% end %> + <% end %> + <% end %> + <%= table.with_body do |body| %> + <% related_orgs.each do |org| %> + <%= body.with_row do |row| %> + <% if current_user.support? %> + <% row.with_cell(text: simple_format(govuk_link_to(org.name, organisation_path(org)), { class: "govuk-!-font-weight-bold" }, wrapper_tag: "div")) %> + <% else %> + <% row.with_cell(text: org.name) %> + <% end %> + <% row.with_cell(text: "ORG#{org.id}") %> + <% if current_user.data_coordinator? || current_user.support? %> + <% row.with_cell(html_attributes: { + scope: "row", + class: "govuk-!-text-align-right", + }) do %> + <%= govuk_link_to("Remove", remove_path.call(org.id)) %> + <% end %> + <% end %> + <% end %> + <% end %> + <% end %> + <% end %> +
diff --git a/app/views/organisation_relationships/_stock_owner_list.erb b/app/views/organisation_relationships/_stock_owner_list.erb deleted file mode 100644 index 2fb0de80b..000000000 --- a/app/views/organisation_relationships/_stock_owner_list.erb +++ /dev/null @@ -1,22 +0,0 @@ -
- <%= govuk_table do |table| %> - <%= table.with_caption(classes: %w[govuk-!-font-size-19 govuk-!-font-weight-regular]) do |caption| %> - <%= render(SearchResultCaptionComponent.new(searched:, count: pagy.count, item_label:, total_count:, item: "stock owners", filters_count: 0)) %> - <% end %> - <% @stock_owners.each do |stock_owner| %> - <%= table.with_body do |body| %> - <%= body.with_row do |row| %> - <% row.with_cell(text: stock_owner.name) %> - <% if current_user.data_coordinator? || current_user.support? %> - <% row.with_cell(html_attributes: { - scope: "row", - class: "govuk-!-text-align-right", - }) do %> - <%= govuk_link_to("Remove", stock_owners_remove_organisation_path(target_organisation_id: stock_owner.id)) %> - <% end %> - <% end %> - <% end %> - <% end %> - <% end %> - <% end %> -
diff --git a/app/views/organisation_relationships/managing_agents.html.erb b/app/views/organisation_relationships/managing_agents.html.erb index 09d326b8b..a01a354d2 100644 --- a/app/views/organisation_relationships/managing_agents.html.erb +++ b/app/views/organisation_relationships/managing_agents.html.erb @@ -22,6 +22,15 @@ <% end %> <% if @total_count != 0 %> <%= render SearchComponent.new(current_user:, search_label: "Search for a managing agent", value: @searched) %> - <%= render partial: "organisation_relationships/managing_agent_list", locals: { index: @managing_agents, title: "Managing agents", pagy: @pagy, searched: @searched, item_label:, total_count: @total_count } %> + <%= render partial: "organisation_relationships/related_org_list", locals: { + related_orgs: @managing_agents, + title: "Managing agents", + pagy: @pagy, + searched: @searched, + item_label:, + search_item: "managing agents", + total_count: @total_count, + remove_path: ->(org_id) { managing_agents_remove_organisation_path(target_organisation_id: org_id) }, + } %> <%= render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "managing agents" } %> <% end %> diff --git a/app/views/organisation_relationships/stock_owners.html.erb b/app/views/organisation_relationships/stock_owners.html.erb index 8149a261c..4be22f7b9 100644 --- a/app/views/organisation_relationships/stock_owners.html.erb +++ b/app/views/organisation_relationships/stock_owners.html.erb @@ -19,6 +19,15 @@ <% end %> <% if @total_count != 0 %> <%= render SearchComponent.new(current_user:, search_label: "Search for a stock owner", value: @searched) %> - <%= render partial: "organisation_relationships/stock_owner_list", locals: { index: @stock_owners, title: "Stock owners", pagy: @pagy, searched: @searched, item_label:, total_count: @total_count } %> + <%= render partial: "organisation_relationships/related_org_list", locals: { + related_orgs: @stock_owners, + title: "Stock owners", + pagy: @pagy, + searched: @searched, + item_label:, + search_item: "stock owners", + total_count: @total_count, + remove_path: ->(org_id) { stock_owners_remove_organisation_path(target_organisation_id: org_id) }, + } %> <%= render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "stock owners" } %> <% end %> diff --git a/spec/requests/organisation_relationships_controller_spec.rb b/spec/requests/organisation_relationships_controller_spec.rb index a93e77d76..4c22a2aa2 100644 --- a/spec/requests/organisation_relationships_controller_spec.rb +++ b/spec/requests/organisation_relationships_controller_spec.rb @@ -113,7 +113,7 @@ RSpec.describe OrganisationRelationshipsController, type: :request do end it "shows the pagination count" do - expect(page).to have_content("1 total agents") + expect(page).to have_content("1 total managing agents") end end @@ -421,7 +421,7 @@ RSpec.describe OrganisationRelationshipsController, type: :request do end it "shows the pagination count" do - expect(page).to have_content("1 total agents") + expect(page).to have_content("1 total managing agents") end end @@ -637,7 +637,7 @@ RSpec.describe OrganisationRelationshipsController, type: :request do end it "shows the pagination count" do - expect(page).to have_content("1 total agents") + expect(page).to have_content("1 total managing agents") end it "shows remove link(s)" do From 36553f47779b809f85724669f29532e32b65dfd3 Mon Sep 17 00:00:00 2001 From: Rachael Booth Date: Mon, 4 Mar 2024 09:43:29 +0000 Subject: [PATCH 5/5] CLDC-1970: Use more relevant error message for shared ownership income below 0 (#2270) --- .../sales/financial_validations.rb | 16 ++-- config/locales/en.yml | 6 +- .../sales/financial_validations_spec.rb | 76 ++++++++++++++----- 3 files changed, 67 insertions(+), 31 deletions(-) diff --git a/app/models/validations/sales/financial_validations.rb b/app/models/validations/sales/financial_validations.rb index 9e30d49f3..77bd60102 100644 --- a/app/models/validations/sales/financial_validations.rb +++ b/app/models/validations/sales/financial_validations.rb @@ -6,10 +6,10 @@ module Validations::Sales::FinancialValidations return unless record.income1 && record.la && record.shared_ownership_scheme? relevant_fields = %i[income1 ownershipsch uprn la postcode_full] - if record.london_property? && record.income1 > 90_000 - relevant_fields.each { |field| record.errors.add field, :over_hard_max_for_london, message: I18n.t("validations.financial.income.over_hard_max_for_london") } - elsif record.property_not_in_london? && record.income1 > 80_000 - relevant_fields.each { |field| record.errors.add field, :over_hard_max_for_outside_london, message: I18n.t("validations.financial.income.over_hard_max_for_outside_london") } + if record.london_property? && !record.income1.between?(0, 90_000) + relevant_fields.each { |field| record.errors.add field, :outside_london_income_range, message: I18n.t("validations.financial.income.outside_london_income_range") } + elsif record.property_not_in_london? && !record.income1.between?(0, 80_000) + relevant_fields.each { |field| record.errors.add field, :outside_non_london_income_range, message: I18n.t("validations.financial.income.outside_non_london_income_range") } end end @@ -17,10 +17,10 @@ module Validations::Sales::FinancialValidations return unless record.income2 && record.la && record.shared_ownership_scheme? relevant_fields = %i[income2 ownershipsch uprn la postcode_full] - if record.london_property? && record.income2 > 90_000 - relevant_fields.each { |field| record.errors.add field, :over_hard_max_for_london, message: I18n.t("validations.financial.income.over_hard_max_for_london") } - elsif record.property_not_in_london? && record.income2 > 80_000 - relevant_fields.each { |field| record.errors.add field, :over_hard_max_for_outside_london, message: I18n.t("validations.financial.income.over_hard_max_for_outside_london") } + if record.london_property? && !record.income2.between?(0, 90_000) + relevant_fields.each { |field| record.errors.add field, :outside_london_income_range, message: I18n.t("validations.financial.income.outside_london_income_range") } + elsif record.property_not_in_london? && !record.income2.between?(0, 80_000) + relevant_fields.each { |field| record.errors.add field, :outside_non_london_income_range, message: I18n.t("validations.financial.income.outside_non_london_income_range") } end end diff --git a/config/locales/en.yml b/config/locales/en.yml index 0ed12c806..70c0ac398 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -368,8 +368,8 @@ en: freq_missing: "Select how often the household receives income" earnings_missing: "Enter how much income the household has in total" income: - over_hard_max_for_london: "Income must be £90,000 or lower for properties within a London local authority" - over_hard_max_for_outside_london: "Income must be £80,000 or lower for properties outside London local authority" + outside_london_income_range: "Income must be between £0 and £90,000 for properties within a London local authority" + outside_non_london_income_range: "Income must be between £0 and £80,000 for properties in a non-London local authority" combined_over_hard_max_for_london: "Combined income must be £90,000 or lower for properties within a London local authority" combined_over_hard_max_for_outside_london: "Combined income must be £80,000 or lower for properties outside London local authorities" child_has_income: "Child's income must be £0" @@ -627,7 +627,7 @@ en: value: over_discounted_london_max: "The percentage discount multiplied by the purchase price is %{discount_value}. This figure should not be more than £136,400 for properties in London." over_discounted_max: "The percentage discount multiplied by the purchase price is %{discount_value}. This figure should not be more than £102,400 for properties outside of London." - non_staircasing_mortgage: + non_staircasing_mortgage: mortgage_used: "The mortgage and deposit added together is %{mortgage_and_deposit_total} and the purchase price times by the equity is %{expected_shared_ownership_deposit_value}. These figures should be the same." mortgage_not_used: "The deposit is %{deposit} and the purchase price times by the equity is %{expected_shared_ownership_deposit_value}. As no mortgage was used, these figures should be the same." staircasing_mortgage: diff --git a/spec/models/validations/sales/financial_validations_spec.rb b/spec/models/validations/sales/financial_validations_spec.rb index 8a57df6b6..470148478 100644 --- a/spec/models/validations/sales/financial_validations_spec.rb +++ b/spec/models/validations/sales/financial_validations_spec.rb @@ -17,33 +17,51 @@ RSpec.describe Validations::Sales::FinancialValidations do it "adds errors if buyer 1 has income over 80,000" do record.income1 = 85_000 financial_validator.validate_income1(record) - expect(record.errors["income1"]).to include(match I18n.t("validations.financial.income.over_hard_max_for_outside_london")) - expect(record.errors["ownershipsch"]).to include(match I18n.t("validations.financial.income.over_hard_max_for_outside_london")) - expect(record.errors["la"]).to include(match I18n.t("validations.financial.income.over_hard_max_for_outside_london")) - expect(record.errors["postcode_full"]).to include(match I18n.t("validations.financial.income.over_hard_max_for_outside_london")) + expect(record.errors["income1"]).to include(match I18n.t("validations.financial.income.outside_non_london_income_range")) + expect(record.errors["ownershipsch"]).to include(match I18n.t("validations.financial.income.outside_non_london_income_range")) + expect(record.errors["la"]).to include(match I18n.t("validations.financial.income.outside_non_london_income_range")) + expect(record.errors["postcode_full"]).to include(match I18n.t("validations.financial.income.outside_non_london_income_range")) end it "adds errors if buyer 2 has income over 80,000" do record.income2 = 85_000 financial_validator.validate_income2(record) - expect(record.errors["income2"]).to include(match I18n.t("validations.financial.income.over_hard_max_for_outside_london")) - expect(record.errors["ownershipsch"]).to include(match I18n.t("validations.financial.income.over_hard_max_for_outside_london")) - expect(record.errors["la"]).to include(match I18n.t("validations.financial.income.over_hard_max_for_outside_london")) - expect(record.errors["postcode_full"]).to include(match I18n.t("validations.financial.income.over_hard_max_for_outside_london")) + expect(record.errors["income2"]).to include(match I18n.t("validations.financial.income.outside_non_london_income_range")) + expect(record.errors["ownershipsch"]).to include(match I18n.t("validations.financial.income.outside_non_london_income_range")) + expect(record.errors["la"]).to include(match I18n.t("validations.financial.income.outside_non_london_income_range")) + expect(record.errors["postcode_full"]).to include(match I18n.t("validations.financial.income.outside_non_london_income_range")) end - it "does not add errors if buyer 1 has income below 80_000" do + it "does not add errors if buyer 1 has income above 0 and below 80_000" do record.income1 = 75_000 financial_validator.validate_income1(record) expect(record.errors).to be_empty end - it "does not add errors if buyer 2 has income below 80_000" do + it "does not add errors if buyer 2 has income above 0 and below 80_000" do record.income2 = 75_000 financial_validator.validate_income2(record) expect(record.errors).to be_empty end + it "adds errors if buyer 1 has income below 0" do + record.income1 = -500 + financial_validator.validate_income1(record) + expect(record.errors["income1"]).to include(match I18n.t("validations.financial.income.outside_non_london_income_range")) + expect(record.errors["ownershipsch"]).to include(match I18n.t("validations.financial.income.outside_non_london_income_range")) + expect(record.errors["la"]).to include(match I18n.t("validations.financial.income.outside_non_london_income_range")) + expect(record.errors["postcode_full"]).to include(match I18n.t("validations.financial.income.outside_non_london_income_range")) + end + + it "adds errors if buyer 2 has income below 0" do + record.income2 = -5 + financial_validator.validate_income2(record) + expect(record.errors["income2"]).to include(match I18n.t("validations.financial.income.outside_non_london_income_range")) + expect(record.errors["ownershipsch"]).to include(match I18n.t("validations.financial.income.outside_non_london_income_range")) + expect(record.errors["la"]).to include(match I18n.t("validations.financial.income.outside_non_london_income_range")) + expect(record.errors["postcode_full"]).to include(match I18n.t("validations.financial.income.outside_non_london_income_range")) + end + it "adds errors when combined income is over 80_000" do record.income1 = 45_000 record.income2 = 40_000 @@ -69,33 +87,51 @@ RSpec.describe Validations::Sales::FinancialValidations do it "adds errors if buyer 1 has income over 90,000" do record.income1 = 95_000 financial_validator.validate_income1(record) - expect(record.errors["income1"]).to include(match I18n.t("validations.financial.income.over_hard_max_for_london")) - expect(record.errors["ownershipsch"]).to include(match I18n.t("validations.financial.income.over_hard_max_for_london")) - expect(record.errors["la"]).to include(match I18n.t("validations.financial.income.over_hard_max_for_london")) - expect(record.errors["postcode_full"]).to include(match I18n.t("validations.financial.income.over_hard_max_for_london")) + expect(record.errors["income1"]).to include(match I18n.t("validations.financial.income.outside_london_income_range")) + expect(record.errors["ownershipsch"]).to include(match I18n.t("validations.financial.income.outside_london_income_range")) + expect(record.errors["la"]).to include(match I18n.t("validations.financial.income.outside_london_income_range")) + expect(record.errors["postcode_full"]).to include(match I18n.t("validations.financial.income.outside_london_income_range")) end it "adds errors if buyer 2 has income over 90,000" do record.income2 = 95_000 financial_validator.validate_income2(record) - expect(record.errors["income2"]).to include(match I18n.t("validations.financial.income.over_hard_max_for_london")) - expect(record.errors["ownershipsch"]).to include(match I18n.t("validations.financial.income.over_hard_max_for_london")) - expect(record.errors["la"]).to include(match I18n.t("validations.financial.income.over_hard_max_for_london")) - expect(record.errors["postcode_full"]).to include(match I18n.t("validations.financial.income.over_hard_max_for_london")) + expect(record.errors["income2"]).to include(match I18n.t("validations.financial.income.outside_london_income_range")) + expect(record.errors["ownershipsch"]).to include(match I18n.t("validations.financial.income.outside_london_income_range")) + expect(record.errors["la"]).to include(match I18n.t("validations.financial.income.outside_london_income_range")) + expect(record.errors["postcode_full"]).to include(match I18n.t("validations.financial.income.outside_london_income_range")) end - it "does not add errors if buyer 1 has income below 90_000" do + it "does not add errors if buyer 1 has income above 0 and below 90_000" do record.income1 = 75_000 financial_validator.validate_income1(record) expect(record.errors).to be_empty end - it "does not add errors if buyer 2 has income below 90_000" do + it "does not add errors if buyer 2 has income above 0 and below 90_000" do record.income2 = 75_000 financial_validator.validate_income2(record) expect(record.errors).to be_empty end + it "adds errors if buyer 1 has income below 0" do + record.income1 = -500 + financial_validator.validate_income1(record) + expect(record.errors["income1"]).to include(match I18n.t("validations.financial.income.outside_london_income_range")) + expect(record.errors["ownershipsch"]).to include(match I18n.t("validations.financial.income.outside_london_income_range")) + expect(record.errors["la"]).to include(match I18n.t("validations.financial.income.outside_london_income_range")) + expect(record.errors["postcode_full"]).to include(match I18n.t("validations.financial.income.outside_london_income_range")) + end + + it "adds errors if buyer 2 has income below 0" do + record.income2 = -2 + financial_validator.validate_income2(record) + expect(record.errors["income2"]).to include(match I18n.t("validations.financial.income.outside_london_income_range")) + expect(record.errors["ownershipsch"]).to include(match I18n.t("validations.financial.income.outside_london_income_range")) + expect(record.errors["la"]).to include(match I18n.t("validations.financial.income.outside_london_income_range")) + expect(record.errors["postcode_full"]).to include(match I18n.t("validations.financial.income.outside_london_income_range")) + end + it "adds errors when combined income is over 90_000" do record.income1 = 55_000 record.income2 = 40_000