From 5490e2ecb037cb75ec5eac323631c6b2d1eeef27 Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Fri, 17 Feb 2023 10:20:39 +0000 Subject: [PATCH 01/13] CLDC-1892 Bulk upload validation for managing organisation (#1315) * bulk upload validation for owning org * block log creation at row parser level * bubble up block_log_creation so it blocks logs * owning org id looks up serveral fields * validate bulk upload managing org --- app/models/organisation.rb | 27 ++++++ .../bulk_upload/lettings/row_parser.rb | 67 ++++++++++++-- .../bulk_upload/lettings/validator.rb | 1 + spec/factories/organisation.rb | 4 + .../bulk_upload/lettings/row_parser_spec.rb | 88 +++++++++++++++++++ .../bulk_upload/lettings/validator_spec.rb | 20 ++++- 6 files changed, 199 insertions(+), 8 deletions(-) diff --git a/app/models/organisation.rb b/app/models/organisation.rb index 89d7d1f34..32d3540d1 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -17,8 +17,21 @@ class Organisation < ApplicationRecord has_many :managing_agent_relationships, foreign_key: :parent_organisation_id, class_name: "OrganisationRelationship" has_many :managing_agents, through: :managing_agent_relationships, source: :child_organisation + def affiliated_stock_owners + ids = [] + + if holds_own_stock? && persisted? + ids << id + end + + ids.concat(stock_owners.pluck(:id)) + + Organisation.where(id: ids) + end + scope :search_by_name, ->(name) { where("name ILIKE ?", "%#{name}%") } scope :search_by, ->(param) { search_by_name(param) } + has_paper_trail auto_strip_attributes :name @@ -35,6 +48,20 @@ class Organisation < ApplicationRecord validates :name, presence: { message: I18n.t("validations.organisation.name_missing") } validates :provider_type, presence: { message: I18n.t("validations.organisation.provider_type_missing") } + def self.find_by_id_on_mulitple_fields(id) + return if id.nil? + + if id.start_with?("ORG") + where(id: id[3..]).first + else + where(old_visible_id: id).first + end + end + + def can_be_managed_by?(organisation:) + organisation == self || managing_agents.include?(organisation) + end + def lettings_logs LettingsLog.filter_by_organisation(self) end diff --git a/app/services/bulk_upload/lettings/row_parser.rb b/app/services/bulk_upload/lettings/row_parser.rb index 005d5caf9..bdb35dbc6 100644 --- a/app/services/bulk_upload/lettings/row_parser.rb +++ b/app/services/bulk_upload/lettings/row_parser.rb @@ -3,6 +3,7 @@ class BulkUpload::Lettings::RowParser include ActiveModel::Attributes attribute :bulk_upload + attribute :block_log_creation, :boolean, default: -> { false } attribute :field_1, :integer attribute :field_2 @@ -114,9 +115,9 @@ class BulkUpload::Lettings::RowParser attribute :field_108, :string attribute :field_109, :string attribute :field_110 - attribute :field_111, :integer + attribute :field_111, :string attribute :field_112, :string - attribute :field_113, :integer + attribute :field_113, :string attribute :field_114, :integer attribute :field_115 attribute :field_116, :integer @@ -155,6 +156,13 @@ class BulkUpload::Lettings::RowParser validate :validate_dont_know_disabled_needs_conjunction validate :validate_no_and_dont_know_disabled_needs_conjunction + validate :validate_owning_org_permitted + validate :validate_owning_org_owns_stock + validate :validate_owning_org_exists + + validate :validate_managing_org_related + validate :validate_managing_org_exists + def valid? errors.clear @@ -173,15 +181,60 @@ class BulkUpload::Lettings::RowParser end def blank_row? - attribute_set.to_hash.reject { |k, _| %w[bulk_upload].include?(k) }.values.compact.empty? + attribute_set.to_hash.reject { |k, _| %w[bulk_upload block_log_creation].include?(k) }.values.compact.empty? end def log @log ||= LettingsLog.new(attributes_for_log) end + def block_log_creation! + self.block_log_creation = true + end + + def block_log_creation? + block_log_creation + end + private + def validate_managing_org_related + if owning_organisation && managing_organisation && !owning_organisation.can_be_managed_by?(organisation: managing_organisation) + block_log_creation! + errors.add(:field_113, "This managing organisation does not have a relationship with the owning organisation") + end + end + + def validate_managing_org_exists + if managing_organisation.nil? + errors.delete(:field_113) + errors.add(:field_113, "The managing organisation code is incorrect") + end + end + + def validate_owning_org_owns_stock + if owning_organisation && !owning_organisation.holds_own_stock? + block_log_creation! + errors.delete(:field_111) + errors.add(:field_111, "The owning organisation code provided is for an organisation that does not own stock") + end + end + + def validate_owning_org_exists + if owning_organisation.nil? + errors.delete(:field_111) + errors.add(:field_111, "The owning organisation code is incorrect") + end + end + + def validate_owning_org_permitted + if owning_organisation && !bulk_upload.user.organisation.affiliated_stock_owners.include?(owning_organisation) + block_log_creation! + errors.delete(:field_111) + errors.add(:field_111, "You do not have permission to add logs for this owning organisation") + end + end + def validate_no_and_dont_know_disabled_needs_conjunction if field_59 == 1 && field_60 == 1 errors.add(:field_59, I18n.t("validations.household.housingneeds.no_and_dont_know_disabled_needs_conjunction")) @@ -483,15 +536,19 @@ private end def owning_organisation - Organisation.find_by(old_visible_id: field_111) + Organisation.find_by_id_on_mulitple_fields(field_111) end def owning_organisation_id owning_organisation&.id end + def managing_organisation + Organisation.find_by_id_on_mulitple_fields(field_113) + end + def managing_organisation_id - Organisation.find_by(old_visible_id: field_113)&.id + managing_organisation&.id end def attributes_for_log diff --git a/app/services/bulk_upload/lettings/validator.rb b/app/services/bulk_upload/lettings/validator.rb index 6f37c0f3a..992f06196 100644 --- a/app/services/bulk_upload/lettings/validator.rb +++ b/app/services/bulk_upload/lettings/validator.rb @@ -176,6 +176,7 @@ class BulkUpload::Lettings::Validator def create_logs? return false if any_setup_sections_incomplete? return false if over_column_error_threshold? + return false if row_parsers.any?(&:block_log_creation?) row_parsers.all? { |row_parser| row_parser.log.valid? } end diff --git a/spec/factories/organisation.rb b/spec/factories/organisation.rb index 7aee3ad66..fa2650eb5 100644 --- a/spec/factories/organisation.rb +++ b/spec/factories/organisation.rb @@ -17,6 +17,10 @@ FactoryBot.define do trait :prp do provider_type { "PRP" } end + + trait :does_not_own_stock do + holds_own_stock { false } + end end factory :organisation_rent_period do diff --git a/spec/services/bulk_upload/lettings/row_parser_spec.rb b/spec/services/bulk_upload/lettings/row_parser_spec.rb index 4c0788033..36f145a5e 100644 --- a/spec/services/bulk_upload/lettings/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/row_parser_spec.rb @@ -8,8 +8,10 @@ RSpec.describe BulkUpload::Lettings::RowParser do let(:attributes) { { bulk_upload: } } let(:bulk_upload) { create(:bulk_upload, :lettings, user:) } let(:user) { create(:user, organisation: owning_org) } + let(:owning_org) { create(:organisation, :with_old_visible_id) } let(:managing_org) { create(:organisation, :with_old_visible_id) } + let(:setup_section_params) do { bulk_upload:, @@ -23,6 +25,10 @@ RSpec.describe BulkUpload::Lettings::RowParser do } end + before do + create(:organisation_relationship, parent_organisation: owning_org, child_organisation: managing_org) + end + around do |example| FormHandler.instance.use_real_forms! @@ -472,6 +478,68 @@ RSpec.describe BulkUpload::Lettings::RowParser do end end + describe "#field_111" do # owning org + context "when cannot find owning org" do + let(:attributes) { { bulk_upload:, field_111: "donotexist" } } + + it "is not permitted" do + expect(parser.errors[:field_111]).to eql(["The owning organisation code is incorrect"]) + end + end + + context "when org is not stock owning" do + let(:owning_org) { create(:organisation, :with_old_visible_id, :does_not_own_stock) } + + let(:attributes) { { bulk_upload:, field_111: owning_org.old_visible_id } } + + it "is not permitted" do + expect(parser.errors[:field_111]).to eql(["The owning organisation code provided is for an organisation that does not own stock"]) + end + + it "blocks log creation" do + expect(parser).to be_block_log_creation + end + end + + context "when not affiliated with owning org" do + let(:unaffiliated_org) { create(:organisation, :with_old_visible_id) } + + let(:attributes) { { bulk_upload:, field_111: unaffiliated_org.old_visible_id } } + + it "is not permitted" do + expect(parser.errors[:field_111]).to eql(["You do not have permission to add logs for this owning organisation"]) + end + + it "blocks log creation" do + expect(parser).to be_block_log_creation + end + end + end + + describe "#field_113" do # managing org + context "when cannot find managing org" do + let(:attributes) { { bulk_upload:, field_113: "donotexist" } } + + it "is not permitted" do + expect(parser.errors[:field_113]).to eql(["The managing organisation code is incorrect"]) + end + end + + context "when not affiliated with managing org" do + let(:unaffiliated_org) { create(:organisation, :with_old_visible_id) } + + let(:attributes) { { bulk_upload:, field_111: owning_org.old_visible_id, field_113: unaffiliated_org.old_visible_id } } + + it "is not permitted" do + expect(parser.errors[:field_113]).to eql(["This managing organisation does not have a relationship with the owning organisation"]) + end + + it "blocks log creation" do + expect(parser).to be_block_log_creation + end + end + end + describe "#field_134" do context "when an unpermitted value" do let(:attributes) { { bulk_upload:, field_134: 3 } } @@ -506,6 +574,26 @@ RSpec.describe BulkUpload::Lettings::RowParser do end describe "#log" do + describe "#owning_organisation" do + context "when lookup is via id prefixed with ORG" do + let(:attributes) { { bulk_upload:, field_111: "ORG#{owning_org.id}" } } + + it "assigns the correct org" do + expect(parser.log.owning_organisation).to eql(owning_org) + end + end + end + + describe "#managing_organisation" do + context "when lookup is via id prefixed with ORG" do + let(:attributes) { { bulk_upload:, field_113: "ORG#{managing_org.id}" } } + + it "assigns the correct org" do + expect(parser.log.managing_organisation).to eql(managing_org) + end + end + end + describe "#cbl" do context "when field_75 is yes ie 1" do let(:attributes) { { bulk_upload:, field_75: 1 } } diff --git a/spec/services/bulk_upload/lettings/validator_spec.rb b/spec/services/bulk_upload/lettings/validator_spec.rb index 263c83163..0aaeaac78 100644 --- a/spec/services/bulk_upload/lettings/validator_spec.rb +++ b/spec/services/bulk_upload/lettings/validator_spec.rb @@ -87,7 +87,7 @@ RSpec.describe BulkUpload::Lettings::Validator do end end - describe "#should_create_logs?" do + describe "#create_logs?" do context "when all logs are valid" do let(:target_path) { file_fixture("2022_23_lettings_bulk_upload.csv") } @@ -111,9 +111,7 @@ RSpec.describe BulkUpload::Lettings::Validator do expect(validator).not_to be_create_logs end end - end - describe "#create_logs?" do context "when a log is not valid?" do let(:log_1) { build(:lettings_log, :completed, created_by: user) } let(:log_2) { build(:lettings_log, :completed, created_by: user) } @@ -146,6 +144,22 @@ RSpec.describe BulkUpload::Lettings::Validator do end end + context "when a single log wants to block log creation" do + let(:unaffiliated_org) { create(:organisation) } + + let(:log_1) { build(:lettings_log, :completed, renttype: 1, created_by: user, owning_organisation: unaffiliated_org) } + + before do + file.write(BulkUpload::LogToCsv.new(log: log_1, line_ending: "\r\n", col_offset: 0).to_csv_row) + file.close + end + + it "will not create logs" do + validator.call + expect(validator).not_to be_create_logs + end + end + context "when a log has incomplete setup secion" do let(:log) { build(:lettings_log, :in_progress, created_by: user, startdate: Time.zone.local(2022, 5, 1)) } From b24ab538ce4ec8888fd7376e0ff52d6600b171d2 Mon Sep 17 00:00:00 2001 From: James Rose Date: Fri, 17 Feb 2023 12:07:14 +0000 Subject: [PATCH 02/13] Update collection year end dates for 2022 onwards (#1312) The source of truth for this update is: https://digital.dclg.gov.uk/confluence/pages/viewpage.action?spaceKey=MC&title=CORE+And+Collection+Years --- app/models/form.rb | 6 +++++- config/forms/2022_2023.json | 2 +- spec/helpers/tasklist_helper_spec.rb | 4 ++-- spec/models/form_spec.rb | 2 +- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/app/models/form.rb b/app/models/form.rb index 3da85d52e..f9959959e 100644 --- a/app/models/form.rb +++ b/app/models/form.rb @@ -6,7 +6,11 @@ class Form def initialize(form_path, start_year = "", sections_in_form = [], type = "lettings") if sales_or_start_year_after_2022?(type, start_year) @start_date = Time.zone.local(start_year, 4, 1) - @end_date = Time.zone.local(start_year + 1, 7, 1) + @end_date = if start_year && start_year.to_i > 2022 + Time.zone.local(start_year + 1, 7, 9) + else + Time.zone.local(start_year + 1, 7, 7) + end @setup_sections = type == "sales" ? [Form::Sales::Sections::Setup.new(nil, nil, self)] : [Form::Lettings::Sections::Setup.new(nil, nil, self)] @form_sections = sections_in_form.map { |sec| sec.new(nil, nil, self) } @type = type diff --git a/config/forms/2022_2023.json b/config/forms/2022_2023.json index ebd1bd594..e0ef19198 100644 --- a/config/forms/2022_2023.json +++ b/config/forms/2022_2023.json @@ -1,7 +1,7 @@ { "form_type": "lettings", "start_date": "2022-04-01T00:00:00.000+01:00", - "end_date": "2023-07-01T00:00:00.000+01:00", + "end_date": "2023-07-09T00:00:00.000+01:00", "unresolved_log_redirect_page_id": "tenancy_start_date", "sections": { "tenancy_and_property": { diff --git a/spec/helpers/tasklist_helper_spec.rb b/spec/helpers/tasklist_helper_spec.rb index e14e06ea4..1ac6cf738 100644 --- a/spec/helpers/tasklist_helper_spec.rb +++ b/spec/helpers/tasklist_helper_spec.rb @@ -113,7 +113,7 @@ RSpec.describe TasklistHelper do it "returns relevant text" do expect(review_log_text(lettings_log)).to eq( - "You can #{govuk_link_to 'review and make changes to this log', review_lettings_log_path(lettings_log)} until 1 July 2024.".html_safe, + "You can #{govuk_link_to 'review and make changes to this log', review_lettings_log_path(lettings_log)} until 9 July 2024.".html_safe, ) end end @@ -136,7 +136,7 @@ RSpec.describe TasklistHelper do it "returns relevant text" do expect(review_log_text(sales_log)).to eq( - "You can #{govuk_link_to 'review and make changes to this log', review_sales_log_path(id: sales_log, sales_log: true)} until 1 July 2023.".html_safe, + "You can #{govuk_link_to 'review and make changes to this log', review_sales_log_path(id: sales_log, sales_log: true)} until 7 July 2023.".html_safe, ) end end diff --git a/spec/models/form_spec.rb b/spec/models/form_spec.rb index 72223c1b5..cd58587dc 100644 --- a/spec/models/form_spec.rb +++ b/spec/models/form_spec.rb @@ -223,7 +223,7 @@ RSpec.describe Form, type: :model do 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")) + expect(form.end_date).to eq(Time.zone.parse("2023-07-07")) expect(form.unresolved_log_redirect_page_id).to eq(nil) end From 01b957cc1093c6e9578062dc4fe57f1c165d4e65 Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Fri, 17 Feb 2023 12:17:33 +0000 Subject: [PATCH 03/13] better validation on bulk upload start year (#1319) --- .../bulk_upload/lettings/row_parser.rb | 3 +++ config/locales/en.yml | 1 + .../bulk_upload/lettings/row_parser_spec.rb | 18 ++++++++++++++++++ 3 files changed, 22 insertions(+) diff --git a/app/services/bulk_upload/lettings/row_parser.rb b/app/services/bulk_upload/lettings/row_parser.rb index bdb35dbc6..9cb3edb78 100644 --- a/app/services/bulk_upload/lettings/row_parser.rb +++ b/app/services/bulk_upload/lettings/row_parser.rb @@ -143,6 +143,7 @@ class BulkUpload::Lettings::RowParser validates :field_1, presence: { message: I18n.t("validations.not_answered", question: "letting type") }, inclusion: { in: (1..12).to_a, message: I18n.t("validations.invalid_option", question: "letting type") } validates :field_4, presence: { if: proc { [2, 4, 6, 8, 10, 12].include?(field_1) } } + validates :field_98, format: { with: /\A\d{2}\z/, message: I18n.t("validations.setup.startdate.year_not_two_digits") } validate :validate_data_types validate :validate_nulls @@ -500,6 +501,8 @@ private def startdate Date.new(field_98 + 2000, field_97, field_96) if field_98.present? && field_97.present? && field_96.present? + rescue Date::Error + Date.new end def renttype diff --git a/config/locales/en.yml b/config/locales/en.yml index 23d39bbe8..747ed4414 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -156,6 +156,7 @@ en: before_scheme_end_date: "The tenancy start date must be before the end date for this supported housing scheme" after_void_date: "Enter a tenancy start date that is after the void date" after_major_repair_date: "Enter a tenancy start date that is after the major repair date" + year_not_two_digits: Tenancy start year must be 2 digits location: deactivated: "The location %{postcode} was deactivated on %{date} and was not available on the day you entered." reactivating_soon: "The location %{postcode} is not available until %{date}. Select another location or edit the tenancy start date" diff --git a/spec/services/bulk_upload/lettings/row_parser_spec.rb b/spec/services/bulk_upload/lettings/row_parser_spec.rb index 36f145a5e..d1792e9df 100644 --- a/spec/services/bulk_upload/lettings/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/row_parser_spec.rb @@ -443,6 +443,24 @@ RSpec.describe BulkUpload::Lettings::RowParser do end end + context "when field 98 is 4 digits instead of 2" do + let(:attributes) { { bulk_upload:, field_98: "2022" } } + + it "returns an error" do + parser.valid? + + expect(parser.errors[:field_98]).to include("Tenancy start year must be 2 digits") + end + end + + context "when invalid date given" do + let(:attributes) { { bulk_upload:, field_1: "1", field_96: "a", field_97: "12", field_98: "2022" } } + + it "does not raise an error" do + expect { parser.valid? }.not_to raise_error + end + end + context "when inside of collection year" do let(:attributes) { { bulk_upload:, field_96: "1", field_97: "10", field_98: "22" } } From e7e67c8b58c4014f17235cf2dbb1b48a1e98a744 Mon Sep 17 00:00:00 2001 From: Jack <113976590+bibblobcode@users.noreply.github.com> Date: Tue, 21 Feb 2023 10:36:31 +0000 Subject: [PATCH 04/13] Bump depts to fix security issues (#1328) * Bump depts * Fix deprecation --- Gemfile | 2 +- Gemfile.lock | 270 ++++++++++++++++++++++--------------------- spec/rails_helper.rb | 2 +- 3 files changed, 138 insertions(+), 136 deletions(-) diff --git a/Gemfile b/Gemfile index f77a70899..17e10865c 100644 --- a/Gemfile +++ b/Gemfile @@ -20,7 +20,7 @@ gem "bootsnap", ">= 1.4.4", require: false # GOV UK frontend components gem "govuk-components" # GOV UK component form builder DSL -gem "govuk_design_system_formbuilder" +gem "govuk_design_system_formbuilder", "3.1.2" # Convert Markdown into GOV.UK frontend-styled HTML gem "govuk_markdown" # GOV UK Notify diff --git a/Gemfile.lock b/Gemfile.lock index 76ab1021c..fa61ec504 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -13,67 +13,67 @@ GIT GEM remote: https://rubygems.org/ specs: - actioncable (7.0.4.1) - actionpack (= 7.0.4.1) - activesupport (= 7.0.4.1) + actioncable (7.0.4.2) + actionpack (= 7.0.4.2) + activesupport (= 7.0.4.2) nio4r (~> 2.0) websocket-driver (>= 0.6.1) - actionmailbox (7.0.4.1) - actionpack (= 7.0.4.1) - activejob (= 7.0.4.1) - activerecord (= 7.0.4.1) - activestorage (= 7.0.4.1) - activesupport (= 7.0.4.1) + actionmailbox (7.0.4.2) + actionpack (= 7.0.4.2) + activejob (= 7.0.4.2) + activerecord (= 7.0.4.2) + activestorage (= 7.0.4.2) + activesupport (= 7.0.4.2) mail (>= 2.7.1) net-imap net-pop net-smtp - actionmailer (7.0.4.1) - actionpack (= 7.0.4.1) - actionview (= 7.0.4.1) - activejob (= 7.0.4.1) - activesupport (= 7.0.4.1) + actionmailer (7.0.4.2) + actionpack (= 7.0.4.2) + actionview (= 7.0.4.2) + activejob (= 7.0.4.2) + activesupport (= 7.0.4.2) mail (~> 2.5, >= 2.5.4) net-imap net-pop net-smtp rails-dom-testing (~> 2.0) - actionpack (7.0.4.1) - actionview (= 7.0.4.1) - activesupport (= 7.0.4.1) + actionpack (7.0.4.2) + actionview (= 7.0.4.2) + activesupport (= 7.0.4.2) rack (~> 2.0, >= 2.2.0) rack-test (>= 0.6.3) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.2.0) - actiontext (7.0.4.1) - actionpack (= 7.0.4.1) - activerecord (= 7.0.4.1) - activestorage (= 7.0.4.1) - activesupport (= 7.0.4.1) + actiontext (7.0.4.2) + actionpack (= 7.0.4.2) + activerecord (= 7.0.4.2) + activestorage (= 7.0.4.2) + activesupport (= 7.0.4.2) globalid (>= 0.6.0) nokogiri (>= 1.8.5) - actionview (7.0.4.1) - activesupport (= 7.0.4.1) + actionview (7.0.4.2) + activesupport (= 7.0.4.2) builder (~> 3.1) erubi (~> 1.4) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.1, >= 1.2.0) - activejob (7.0.4.1) - activesupport (= 7.0.4.1) + activejob (7.0.4.2) + activesupport (= 7.0.4.2) globalid (>= 0.3.6) - activemodel (7.0.4.1) - activesupport (= 7.0.4.1) - activerecord (7.0.4.1) - activemodel (= 7.0.4.1) - activesupport (= 7.0.4.1) - activestorage (7.0.4.1) - actionpack (= 7.0.4.1) - activejob (= 7.0.4.1) - activerecord (= 7.0.4.1) - activesupport (= 7.0.4.1) + activemodel (7.0.4.2) + activesupport (= 7.0.4.2) + activerecord (7.0.4.2) + activemodel (= 7.0.4.2) + activesupport (= 7.0.4.2) + activestorage (7.0.4.2) + actionpack (= 7.0.4.2) + activejob (= 7.0.4.2) + activerecord (= 7.0.4.2) + activesupport (= 7.0.4.2) marcel (~> 1.0) mini_mime (>= 1.1.0) - activesupport (7.0.4.1) + activesupport (7.0.4.2) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (>= 1.6, < 2) minitest (>= 5.1) @@ -84,20 +84,20 @@ GEM auto_strip_attributes (2.6.0) activerecord (>= 4.0) aws-eventstream (1.2.0) - aws-partitions (1.635.0) - aws-sdk-core (3.153.0) + aws-partitions (1.714.0) + aws-sdk-core (3.170.0) aws-eventstream (~> 1, >= 1.0.2) - aws-partitions (~> 1, >= 1.525.0) - aws-sigv4 (~> 1.1) + aws-partitions (~> 1, >= 1.651.0) + aws-sigv4 (~> 1.5) jmespath (~> 1, >= 1.6.1) - aws-sdk-kms (1.58.0) - aws-sdk-core (~> 3, >= 3.127.0) + aws-sdk-kms (1.62.0) + aws-sdk-core (~> 3, >= 3.165.0) aws-sigv4 (~> 1.1) - aws-sdk-s3 (1.114.0) - aws-sdk-core (~> 3, >= 3.127.0) + aws-sdk-s3 (1.119.1) + aws-sdk-core (~> 3, >= 3.165.0) aws-sdk-kms (~> 1) aws-sigv4 (~> 1.4) - aws-sigv4 (1.5.1) + aws-sigv4 (1.5.2) aws-eventstream (~> 1, >= 1.0.2) bcrypt (3.1.18) better_html (2.0.1) @@ -108,14 +108,14 @@ GEM parser (>= 2.4) smart_properties bindex (0.8.1) - bootsnap (1.13.0) + bootsnap (1.16.0) msgpack (~> 1.2) builder (3.2.4) bundler-audit (0.9.1) bundler (>= 1.2.0, < 3) thor (~> 1.0) byebug (11.1.3) - capybara (3.37.1) + capybara (3.38.0) addressable matrix mini_mime (>= 0.1.3) @@ -124,14 +124,14 @@ GEM rack-test (>= 0.6.3) regexp_parser (>= 1.5, < 3.0) xpath (~> 3.2) - capybara-lockstep (1.2.1) + capybara-lockstep (1.3.0) activesupport (>= 3.2) capybara (>= 2.0) ruby2_keywords selenium-webdriver (>= 3) childprocess (4.1.0) coderay (1.1.3) - concurrent-ruby (1.1.10) + concurrent-ruby (1.2.0) connection_pool (2.3.0) crack (0.4.5) rexml @@ -150,7 +150,7 @@ GEM dotenv (= 2.8.1) railties (>= 3.2) encryptor (3.0.0) - erb_lint (0.2.0) + erb_lint (0.3.1) activesupport better_html (>= 2.0.1) parser (>= 2.7.1.4) @@ -160,19 +160,19 @@ GEM erubi (1.12.0) et-orbi (1.2.7) tzinfo - excon (0.92.5) + excon (0.99.0) factory_bot (6.2.1) activesupport (>= 5.0.0) factory_bot_rails (6.2.0) factory_bot (~> 6.2.0) railties (>= 5.0.0) - faker (2.23.0) + faker (3.1.1) i18n (>= 1.8.11, < 2) ffi (1.15.5) fugit (1.8.1) et-orbi (~> 1, >= 1.2.7) raabro (~> 1.4) - globalid (1.0.1) + globalid (1.1.0) activesupport (>= 5.0) govuk-components (3.2.1) actionpack (>= 6.1) @@ -186,7 +186,7 @@ GEM activemodel (>= 6.1) activesupport (>= 6.1) html-attributes-utils (~> 0.9, >= 0.9.2) - govuk_markdown (1.0.0) + govuk_markdown (2.0.0) activesupport redcarpet hashdiff (1.0.1) @@ -195,19 +195,19 @@ GEM i18n (1.12.0) concurrent-ruby (~> 1.0) iniparse (1.5.0) - jmespath (1.6.1) - jsbundling-rails (1.0.3) + jmespath (1.6.2) + jsbundling-rails (1.1.1) railties (>= 6.0.0) json-schema (3.0.0) addressable (>= 2.8) - jwt (2.5.0) - listen (3.7.1) + jwt (2.7.0) + listen (3.8.0) rb-fsevent (~> 0.10, >= 0.10.3) rb-inotify (~> 0.9, >= 0.9.10) loofah (2.19.1) crass (~> 1.0.2) nokogiri (>= 1.5.9) - mail (2.8.0.1) + mail (2.8.1) mini_mime (>= 0.1.1) net-imap net-pop @@ -217,7 +217,7 @@ GEM method_source (1.0.0) mini_mime (1.1.2) minitest (5.17.0) - msgpack (1.5.6) + msgpack (1.6.0) net-imap (0.3.4) date net-protocol @@ -228,33 +228,33 @@ GEM net-smtp (0.3.3) net-protocol nio4r (2.5.8) - nokogiri (1.14.0-arm64-darwin) + nokogiri (1.14.2-arm64-darwin) racc (~> 1.4) - nokogiri (1.14.0-x86_64-darwin) + nokogiri (1.14.2-x86_64-darwin) racc (~> 1.4) - nokogiri (1.14.0-x86_64-linux) + nokogiri (1.14.2-x86_64-linux) racc (~> 1.4) - notifications-ruby-client (5.3.0) + notifications-ruby-client (5.4.0) jwt (>= 1.5, < 3) orm_adapter (0.5.0) - overcommit (0.59.1) + overcommit (0.60.0) childprocess (>= 0.6.3, < 5) iniparse (~> 1.4) rexml (~> 3.2) pagy (5.10.1) activesupport - paper_trail (13.0.0) - activerecord (>= 5.2) - request_store (~> 1.1) + paper_trail (14.0.0) + activerecord (>= 6.0) + request_store (~> 1.4) paper_trail-globalid (0.2.0) globalid paper_trail (>= 3.0.0) parallel (1.22.1) - parallel_tests (4.0.0) + parallel_tests (4.2.0) parallel - parser (3.1.2.1) + parser (3.2.1.0) ast (~> 2.4.1) - pg (1.4.3) + pg (1.4.5) possessive (1.0.1) postcodes_io (0.4.0) excon (~> 0.39) @@ -263,13 +263,13 @@ GEM activesupport (>= 7.0.0) rack railties (>= 7.0.0) - pry (0.14.1) + pry (0.14.2) coderay (~> 1.1) method_source (~> 1.0) pry-byebug (3.10.1) byebug (~> 11.0) pry (>= 0.13, < 0.15) - public_suffix (5.0.0) + public_suffix (5.0.1) puma (5.6.5) nio4r (~> 2.0) raabro (1.4.0) @@ -281,28 +281,28 @@ GEM rack (>= 1.2.0) rack-test (2.0.2) rack (>= 1.3) - rails (7.0.4.1) - actioncable (= 7.0.4.1) - actionmailbox (= 7.0.4.1) - actionmailer (= 7.0.4.1) - actionpack (= 7.0.4.1) - actiontext (= 7.0.4.1) - actionview (= 7.0.4.1) - activejob (= 7.0.4.1) - activemodel (= 7.0.4.1) - activerecord (= 7.0.4.1) - activestorage (= 7.0.4.1) - activesupport (= 7.0.4.1) + rails (7.0.4.2) + actioncable (= 7.0.4.2) + actionmailbox (= 7.0.4.2) + actionmailer (= 7.0.4.2) + actionpack (= 7.0.4.2) + actiontext (= 7.0.4.2) + actionview (= 7.0.4.2) + activejob (= 7.0.4.2) + activemodel (= 7.0.4.2) + activerecord (= 7.0.4.2) + activestorage (= 7.0.4.2) + activesupport (= 7.0.4.2) bundler (>= 1.15.0) - railties (= 7.0.4.1) + railties (= 7.0.4.2) rails-dom-testing (2.0.3) activesupport (>= 4.2.0) nokogiri (>= 1.6) - rails-html-sanitizer (1.4.4) + rails-html-sanitizer (1.5.0) loofah (~> 2.19, >= 2.19.1) - railties (7.0.4.1) - actionpack (= 7.0.4.1) - activesupport (= 7.0.4.1) + railties (7.0.4.2) + actionpack (= 7.0.4.2) + activesupport (= 7.0.4.2) method_source rake (>= 12.2) thor (~> 1.0) @@ -313,36 +313,38 @@ GEM rb-fsevent (0.11.2) rb-inotify (0.10.1) ffi (~> 1.0) - redcarpet (3.5.1) - redis (4.8.0) - regexp_parser (2.5.0) + redcarpet (3.6.0) + redis (4.8.1) + redis-client (0.12.2) + connection_pool + regexp_parser (2.7.0) request_store (1.5.1) rack (>= 1.4) - responders (3.0.1) - actionpack (>= 5.0) - railties (>= 5.0) + responders (3.1.0) + actionpack (>= 5.2) + railties (>= 5.2) rexml (3.2.5) - roo (2.9.0) + roo (2.10.0) nokogiri (~> 1) rubyzip (>= 1.3.0, < 3.0.0) - rotp (6.2.0) - rspec-core (3.11.0) - rspec-support (~> 3.11.0) - rspec-expectations (3.11.1) + rotp (6.2.2) + rspec-core (3.12.1) + rspec-support (~> 3.12.0) + rspec-expectations (3.12.2) diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.11.0) - rspec-mocks (3.11.1) + rspec-support (~> 3.12.0) + rspec-mocks (3.12.3) diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.11.0) - rspec-rails (5.1.2) - actionpack (>= 5.2) - activesupport (>= 5.2) - railties (>= 5.2) - rspec-core (~> 3.10) - rspec-expectations (~> 3.10) - rspec-mocks (~> 3.10) - rspec-support (~> 3.10) - rspec-support (3.11.1) + rspec-support (~> 3.12.0) + rspec-rails (6.0.1) + actionpack (>= 6.1) + activesupport (>= 6.1) + railties (>= 6.1) + rspec-core (~> 3.11) + rspec-expectations (~> 3.11) + rspec-mocks (~> 3.11) + rspec-support (~> 3.11) + rspec-support (3.12.0) rubocop (1.25.0) parallel (~> 1.10) parser (>= 3.1.0.0) @@ -360,7 +362,7 @@ GEM rubocop-rails (= 2.13.2) rubocop-rake (= 0.6.0) rubocop-rspec (= 2.7.0) - rubocop-performance (1.15.0) + rubocop-performance (1.16.0) rubocop (>= 1.7.0, < 2.0) rubocop-ast (>= 0.4.0) rubocop-rails (2.13.2) @@ -374,39 +376,39 @@ GEM ruby-progressbar (1.11.0) ruby2_keywords (0.0.5) rubyzip (2.3.2) - selenium-webdriver (4.4.0) - childprocess (>= 0.5, < 5.0) + selenium-webdriver (4.8.1) rexml (~> 3.2, >= 3.2.5) rubyzip (>= 1.2.2, < 3.0) websocket (~> 1.0) - sentry-rails (5.4.2) + sentry-rails (5.8.0) railties (>= 5.0) - sentry-ruby (~> 5.4.2) - sentry-ruby (5.4.2) + sentry-ruby (~> 5.8.0) + sentry-ruby (5.8.0) concurrent-ruby (~> 1.0, >= 1.0.2) - sidekiq (6.5.5) - connection_pool (>= 2.2.2) - rack (~> 2.0) - redis (>= 4.5.0) - sidekiq-cron (1.8.0) - fugit (~> 1) + sidekiq (7.0.5) + concurrent-ruby (< 2) + connection_pool (>= 2.3.0) + rack (>= 2.2.4) + redis-client (>= 0.11.0) + sidekiq-cron (1.9.1) + fugit (~> 1.8) sidekiq (>= 4.2.1) - simplecov (0.21.2) + simplecov (0.22.0) docile (~> 1.1) simplecov-html (~> 0.11) simplecov_json_formatter (~> 0.1) simplecov-html (0.12.3) simplecov_json_formatter (0.1.4) smart_properties (1.17.0) - stimulus-rails (1.1.0) + stimulus-rails (1.2.1) railties (>= 6.0.0) thor (1.2.1) - timecop (0.9.5) - timeout (0.3.1) - tzinfo (2.0.5) + timecop (0.9.6) + timeout (0.3.2) + tzinfo (2.0.6) concurrent-ruby (~> 1.0) uk_postcode (2.1.8) - unicode-display_width (2.3.0) + unicode-display_width (2.4.2) view_component (2.69.0) activesupport (>= 5.0.0, < 8.0) concurrent-ruby (~> 1.0) @@ -428,7 +430,7 @@ GEM websocket-extensions (0.1.5) xpath (3.2.0) nokogiri (~> 1.8) - zeitwerk (2.6.6) + zeitwerk (2.6.7) PLATFORMS arm64-darwin-21 @@ -454,7 +456,7 @@ DEPENDENCIES factory_bot_rails faker govuk-components - govuk_design_system_formbuilder + govuk_design_system_formbuilder (= 3.1.2) govuk_markdown jsbundling-rails json-schema diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 7ae06f7e4..81d9246ec 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -13,7 +13,7 @@ Capybara.register_driver :headless do |app| options = Selenium::WebDriver::Firefox::Options.new options.add_argument("--headless") - Capybara::Selenium::Driver.new(app, browser: :firefox, capabilities: options) + Capybara::Selenium::Driver.new(app, browser: :firefox, options:) end Capybara.javascript_driver = :headless From 4a31906d45dac7875b8113a924dd4897d37f004b Mon Sep 17 00:00:00 2001 From: David May-Miller Date: Tue, 21 Feb 2023 16:53:30 +0000 Subject: [PATCH 05/13] CLDC-853 Added validations for sales income2 (#1101) * CLDC-853 Added hard validations for sales income2 * CLDC-853 Added soft validation for sales income2 * CLDC-853 Fix tests broken by new code * CLDC-853 Add new tests for new page and refactor slightly * CLDC-853 Fix linting errors * CLDC-853 Rename migration and update schema version * CLDC-853 Fix broken sales income2 test * CLDC-853 Rename migration * CLDC-853 Move income 2 to cya card 2 and commonise combined income validation * CLDC-853 Actually use the validate_combined_income method * combine duplicate methods after rebase, ensure hard validations are triggered on all relevant fields * move validation on child income to financial validations to stop it being triggered on lettings logs, minor amendments to tests broken by changes * revamp financial validations tests against income to reflect updates * amend child income validation to reflect specifications and write tests to cover this validation * correct linting errors and play a little code golf * change copy for some validations, add sales log method and amend interruption screen helper to support this * extract duplicate code to private method * update buyer 1 and 2 income value check to be consistent * remove ecstat from income checks, the only ecstat we care about is child which is dealt with elsewhere * rename constant struct with same anme as existing variable * amend tests to reflect the chagnes in validations and copy * enable currency formatting of numbers for inserting into informative_text or title_text * update evil test in form handler spec * rebase and fix conflicts and tests * change a variable name and correct minor rebase errors * update interruption screen helper tests * correct linting errors, minor test failure and typo * add tests for new sales log method for formatting currency * fix merge conflicts --------- Co-authored-by: Arthur Campbell --- app/helpers/interruption_screen_helper.rb | 33 ++- ...bout_price_shared_ownership_value_check.rb | 6 +- .../sales/pages/buyer1_income_value_check.rb | 15 ++ .../sales/pages/buyer2_income_value_check.rb | 35 +++ .../questions/buyer1_income_value_check.rb | 2 +- .../form/sales/questions/buyer2_income.rb | 1 + .../questions/buyer2_income_value_check.rb | 25 ++ .../subsections/household_characteristics.rb | 3 +- .../income_benefits_and_savings.rb | 1 + app/models/log.rb | 4 + app/models/sales_log.rb | 18 ++ .../sales/financial_validations.rb | 63 +++-- .../validations/sales/soft_validations.rb | 12 +- config/locales/en.yml | 12 +- .../20230105103134_add_income2_value_check.rb | 5 + db/schema.rb | 1 + .../interruption_screen_helper_spec.rb | 53 ++++- .../buyer1_income_value_check_spec.rb | 2 +- .../household_characteristics_spec.rb | 6 +- .../income_benefits_and_savings_spec.rb | 2 + spec/models/form_handler_spec.rb | 4 +- spec/models/sales_log_spec.rb | 23 ++ .../sales/financial_validations_spec.rb | 222 ++++++++++++------ 23 files changed, 420 insertions(+), 128 deletions(-) create mode 100644 app/models/form/sales/pages/buyer2_income_value_check.rb create mode 100644 app/models/form/sales/questions/buyer2_income_value_check.rb create mode 100644 db/migrate/20230105103134_add_income2_value_check.rb diff --git a/app/helpers/interruption_screen_helper.rb b/app/helpers/interruption_screen_helper.rb index c30ba7bfa..939f70bb2 100644 --- a/app/helpers/interruption_screen_helper.rb +++ b/app/helpers/interruption_screen_helper.rb @@ -1,17 +1,10 @@ module InterruptionScreenHelper - def display_informative_text(informative_text, lettings_log) + def display_informative_text(informative_text, log) return "" unless informative_text["arguments"] translation_params = {} informative_text["arguments"].each do |argument| - value = if argument["label"] - pre_casing_value = lettings_log.form.get_question(argument["key"], lettings_log).answer_label(lettings_log) - pre_casing_value.downcase - elsif argument["currency"] - number_to_currency(lettings_log.public_send(argument["key"]), delimiter: ",", format: "%n", unit: "£") - else - lettings_log.public_send(argument["key"]) - end + value = get_value_from_argument(log, argument) translation_params[argument["i18n_template"].to_sym] = value end @@ -24,21 +17,27 @@ module InterruptionScreenHelper end end - def display_title_text(title_text, lettings_log) + def display_title_text(title_text, log) return "" if title_text.nil? translation_params = {} arguments = title_text["arguments"] || {} arguments.each do |argument| - value = if argument["label"] - lettings_log.form.get_question(argument["key"], lettings_log).answer_label(lettings_log).downcase - elsif argument["currency"] - number_to_currency(lettings_log.public_send(argument["key"]), delimiter: ",", format: "%n", unit: "£") - else - lettings_log.public_send(argument["key"]) - end + value = get_value_from_argument(log, argument) translation_params[argument["i18n_template"].to_sym] = value end I18n.t(title_text["translation"], **translation_params).to_s end + +private + + def get_value_from_argument(log, argument) + if argument["label"] + log.form.get_question(argument["key"], log).answer_label(log).downcase + elsif argument["arguments_for_key"] + log.public_send(argument["key"], argument["arguments_for_key"]) + else + log.public_send(argument["key"]) + end + end end diff --git a/app/models/form/sales/pages/about_price_shared_ownership_value_check.rb b/app/models/form/sales/pages/about_price_shared_ownership_value_check.rb index f4f0955cd..5b668006a 100644 --- a/app/models/form/sales/pages/about_price_shared_ownership_value_check.rb +++ b/app/models/form/sales/pages/about_price_shared_ownership_value_check.rb @@ -20,14 +20,12 @@ class Form::Sales::Pages::AboutPriceSharedOwnershipValueCheck < ::Form::Page "translation" => "soft_validations.purchase_price.hint_text", "arguments" => [ { - "key" => "purchase_price_soft_min_or_soft_max", - "label" => false, + "key" => "field_formatted_as_currency", + "arguments_for_key" => "purchase_price_soft_min_or_soft_max", "i18n_template" => "soft_min_or_soft_max", - "currency" => true, }, { "key" => "purchase_price_min_or_max_text", - "label" => false, "i18n_template" => "min_or_max", }, ], diff --git a/app/models/form/sales/pages/buyer1_income_value_check.rb b/app/models/form/sales/pages/buyer1_income_value_check.rb index 04540c47f..48d8f5fff 100644 --- a/app/models/form/sales/pages/buyer1_income_value_check.rb +++ b/app/models/form/sales/pages/buyer1_income_value_check.rb @@ -6,6 +6,21 @@ class Form::Sales::Pages::Buyer1IncomeValueCheck < ::Form::Page "income1_under_soft_min?" => true, }, ] + @title_text = { + "translation" => "soft_validations.income.under_soft_min_for_economic_status", + "arguments" => [ + { + "key" => "field_formatted_as_currency", + "arguments_for_key" => "income1", + "i18n_template" => "income", + }, + { + "key" => "income_soft_min_for_ecstat", + "arguments_for_key" => "ecstat1", + "i18n_template" => "minimum", + }, + ], + } @informative_text = {} end diff --git a/app/models/form/sales/pages/buyer2_income_value_check.rb b/app/models/form/sales/pages/buyer2_income_value_check.rb new file mode 100644 index 000000000..598c4c7a6 --- /dev/null +++ b/app/models/form/sales/pages/buyer2_income_value_check.rb @@ -0,0 +1,35 @@ +class Form::Sales::Pages::Buyer2IncomeValueCheck < ::Form::Page + def initialize(id, hsh, subsection) + super + @header = "" + @description = "" + @subsection = subsection + @depends_on = [ + { + "income2_under_soft_min?" => true, + }, + ] + @title_text = { + "translation" => "soft_validations.income.under_soft_min_for_economic_status", + "arguments" => [ + { + "key" => "field_formatted_as_currency", + "arguments_for_key" => "income2", + "i18n_template" => "income", + }, + { + "key" => "income_soft_min_for_ecstat", + "arguments_for_key" => "ecstat2", + "i18n_template" => "minimum", + }, + ], + } + @informative_text = {} + end + + def questions + @questions ||= [ + Form::Sales::Questions::Buyer2IncomeValueCheck.new(nil, nil, self), + ] + end +end diff --git a/app/models/form/sales/questions/buyer1_income_value_check.rb b/app/models/form/sales/questions/buyer1_income_value_check.rb index 0913dd788..8843d6736 100644 --- a/app/models/form/sales/questions/buyer1_income_value_check.rb +++ b/app/models/form/sales/questions/buyer1_income_value_check.rb @@ -3,7 +3,7 @@ class Form::Sales::Questions::Buyer1IncomeValueCheck < ::Form::Question super @id = "income1_value_check" @check_answer_label = "Income confirmation" - @header = "Are you sure this income is correct?" + @header = "Are you sure this is correct?" @type = "interruption_screen" @answer_options = { "0" => { "value" => "Yes" }, diff --git a/app/models/form/sales/questions/buyer2_income.rb b/app/models/form/sales/questions/buyer2_income.rb index 680bf8ae7..f7d96720b 100644 --- a/app/models/form/sales/questions/buyer2_income.rb +++ b/app/models/form/sales/questions/buyer2_income.rb @@ -7,6 +7,7 @@ class Form::Sales::Questions::Buyer2Income < ::Form::Question @type = "numeric" @hint_text = "Provide the gross annual income (i.e. salary before tax) plus the annual amount of benefits, Universal Credit or pensions, and income from investments." @min = 0 + @max = 999_999 @step = 1 @width = 5 @prefix = "£" diff --git a/app/models/form/sales/questions/buyer2_income_value_check.rb b/app/models/form/sales/questions/buyer2_income_value_check.rb new file mode 100644 index 000000000..9508cc59a --- /dev/null +++ b/app/models/form/sales/questions/buyer2_income_value_check.rb @@ -0,0 +1,25 @@ +class Form::Sales::Questions::Buyer2IncomeValueCheck < ::Form::Question + def initialize(id, hsh, page) + super + @id = "income2_value_check" + @check_answer_label = "Income confirmation" + @header = "Are you sure this is correct?" + @type = "interruption_screen" + @answer_options = { + "0" => { "value" => "Yes" }, + "1" => { "value" => "No" }, + } + @hidden_in_check_answers = { + "depends_on" => [ + { + "income2_value_check" => 0, + }, + { + "income2_value_check" => 1, + }, + ], + } + @check_answers_card_number = 2 + @page = page + end +end diff --git a/app/models/form/sales/subsections/household_characteristics.rb b/app/models/form/sales/subsections/household_characteristics.rb index 55ec0b137..181460ae1 100644 --- a/app/models/form/sales/subsections/household_characteristics.rb +++ b/app/models/form/sales/subsections/household_characteristics.rb @@ -34,7 +34,8 @@ class Form::Sales::Subsections::HouseholdCharacteristics < ::Form::Subsection Form::Sales::Pages::RetirementValueCheck.new("gender_2_buyer_retirement_value_check", nil, self, person_index: 2), ethnic_pages_for_buyer_2, Form::Sales::Pages::Buyer2WorkingSituation.new(nil, nil, self), - Form::Sales::Pages::RetirementValueCheck.new("working_situation_2_buyer_retirement_value_check", nil, self, person_index: 2), + Form::Sales::Pages::RetirementValueCheck.new("working_situation_2_retirement_value_check_joint_purchase", nil, self, person_index: 2), + Form::Sales::Pages::Buyer2IncomeValueCheck.new("working_situation_buyer_2_income_value_check", nil, self), Form::Sales::Pages::Buyer2LiveInProperty.new(nil, nil, self), Form::Sales::Pages::NumberOfOthersInProperty.new(nil, nil, self), Form::Sales::Pages::PersonKnown.new("person_2_known", nil, self, person_index: 2), diff --git a/app/models/form/sales/subsections/income_benefits_and_savings.rb b/app/models/form/sales/subsections/income_benefits_and_savings.rb index fe95bfd8c..7642e84bd 100644 --- a/app/models/form/sales/subsections/income_benefits_and_savings.rb +++ b/app/models/form/sales/subsections/income_benefits_and_savings.rb @@ -15,6 +15,7 @@ class Form::Sales::Subsections::IncomeBenefitsAndSavings < ::Form::Subsection Form::Sales::Pages::MortgageValueCheck.new("buyer_1_mortgage_value_check", nil, self, 1), Form::Sales::Pages::Buyer2Income.new(nil, nil, self), Form::Sales::Pages::MortgageValueCheck.new("buyer_2_income_mortgage_value_check", nil, self, 2), + Form::Sales::Pages::Buyer2IncomeValueCheck.new("buyer_2_income_value_check", nil, self), Form::Sales::Pages::Buyer2Mortgage.new(nil, nil, self), Form::Sales::Pages::MortgageValueCheck.new("buyer_2_mortgage_value_check", nil, self, 2), Form::Sales::Pages::HousingBenefits.new(nil, nil, self), diff --git a/app/models/log.rb b/app/models/log.rb index 0cd3add92..d8aa9e236 100644 --- a/app/models/log.rb +++ b/app/models/log.rb @@ -147,4 +147,8 @@ private self[is_inferred_key] = false self[postcode_key] = nil end + + def format_as_currency(num_string) + ActionController::Base.helpers.number_to_currency(num_string, unit: "£") + end end diff --git a/app/models/sales_log.rb b/app/models/sales_log.rb index 6ed653955..447f37597 100644 --- a/app/models/sales_log.rb +++ b/app/models/sales_log.rb @@ -124,6 +124,10 @@ class SalesLog < Log la && LONDON_BOROUGHS.include?(la) end + def property_not_in_london? + !london_property? + end + def income1_used_for_mortgage? inc1mort == 1 end @@ -256,4 +260,18 @@ class SalesLog < Log def purchase_price_soft_max LaSaleRange.find_by(start_year: collection_start_year, la:, bedrooms: beds).soft_max end + + def income_soft_min_for_ecstat(ecstat_field) + economic_status_code = public_send(ecstat_field) + + return unless ALLOWED_INCOME_RANGES_SALES + + soft_min = ALLOWED_INCOME_RANGES_SALES[economic_status_code]&.soft_min + format_as_currency(soft_min) + end + + def field_formatted_as_currency(field_name) + field_value = public_send(field_name) + format_as_currency(field_value) + end end diff --git a/app/models/validations/sales/financial_validations.rb b/app/models/validations/sales/financial_validations.rb index df7dc8df5..21f3743ca 100644 --- a/app/models/validations/sales/financial_validations.rb +++ b/app/models/validations/sales/financial_validations.rb @@ -3,20 +3,36 @@ module Validations::Sales::FinancialValidations # or 'validate_' to run on submit as well def validate_income1(record) - if record.ecstat1 && record.income1 && record.la && record.ownershipsch == 1 - if record.london_property? - record.errors.add :income1, I18n.t("validations.financial.income1.over_hard_max", hard_max: 90_000) if record.income1 > 90_000 - record.errors.add :ecstat1, I18n.t("validations.financial.income1.over_hard_max", hard_max: 90_000) if record.income1 > 90_000 - record.errors.add :ownershipsch, I18n.t("validations.financial.income1.over_hard_max", hard_max: 90_000) if record.income1 > 90_000 - record.errors.add :la, I18n.t("validations.financial.income1.over_hard_max", hard_max: 90_000) if record.income1 > 90_000 - record.errors.add :postcode_full, I18n.t("validations.financial.income1.over_hard_max", hard_max: 90_000) if record.income1 > 90_000 - elsif record.income1 > 80_000 - record.errors.add :income1, I18n.t("validations.financial.income1.over_hard_max", hard_max: 80_000) - record.errors.add :ecstat1, I18n.t("validations.financial.income1.over_hard_max", hard_max: 80_000) - record.errors.add :ownershipsch, I18n.t("validations.financial.income1.over_hard_max", hard_max: 80_000) - record.errors.add :la, I18n.t("validations.financial.income1.over_hard_max", hard_max: 80_000) if record.income1 > 80_000 - record.errors.add :postcode_full, I18n.t("validations.financial.income1.over_hard_max", hard_max: 80_000) if record.income1 > 80_000 - end + return unless record.income1 && record.la && record.shared_ownership_scheme? + + relevant_fields = %i[income1 ownershipsch la postcode_full] + if record.london_property? && record.income1 > 90_000 + relevant_fields.each { |field| record.errors.add field, 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, I18n.t("validations.financial.income.over_hard_max_for_outside_london") } + end + end + + def validate_income2(record) + return unless record.income2 && record.la && record.shared_ownership_scheme? + + relevant_fields = %i[income2 ownershipsch la postcode_full] + if record.london_property? && record.income2 > 90_000 + relevant_fields.each { |field| record.errors.add field, 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, I18n.t("validations.financial.income.over_hard_max_for_outside_london") } + end + end + + def validate_combined_income(record) + return unless record.income1 && record.income2 && record.la && record.shared_ownership_scheme? + + combined_income = record.income1 + record.income2 + relevant_fields = %i[income1 income2 ownershipsch la postcode_full] + if record.london_property? && combined_income > 90_000 + relevant_fields.each { |field| record.errors.add field, I18n.t("validations.financial.income.combined_over_hard_max_for_london") } + elsif record.property_not_in_london? && combined_income > 80_000 + relevant_fields.each { |field| record.errors.add field, I18n.t("validations.financial.income.combined_over_hard_max_for_outside_london") } end end @@ -36,6 +52,15 @@ module Validations::Sales::FinancialValidations end end + def validate_child_income(record) + return unless record.income2 && record.ecstat2 + + if record.income2.positive? && is_economic_status_child?(record.ecstat2) + record.errors.add :ecstat2, I18n.t("validations.financial.income.child_has_income") + record.errors.add :income2, I18n.t("validations.financial.income.child_has_income") + end + end + def validate_percentage_owned_not_too_much_if_older_person(record) return unless record.old_persons_shared_ownership? && record.stairowned @@ -44,4 +69,14 @@ module Validations::Sales::FinancialValidations record.errors.add :type, I18n.t("validations.financial.staircasing.older_person_percentage_owned_maximum_75") end end + +private + + def is_relationship_child?(relationship) + relationship == "C" + end + + def is_economic_status_child?(economic_status) + economic_status == 9 + end end diff --git a/app/models/validations/sales/soft_validations.rb b/app/models/validations/sales/soft_validations.rb index a7b9fd4c0..c1704d948 100644 --- a/app/models/validations/sales/soft_validations.rb +++ b/app/models/validations/sales/soft_validations.rb @@ -1,5 +1,5 @@ module Validations::Sales::SoftValidations - ALLOWED_INCOME_RANGES = { + ALLOWED_INCOME_RANGES_SALES = { 1 => OpenStruct.new(soft_min: 5000), 2 => OpenStruct.new(soft_min: 1500), 3 => OpenStruct.new(soft_min: 1000), @@ -8,9 +8,15 @@ module Validations::Sales::SoftValidations }.freeze def income1_under_soft_min? - return false unless ecstat1 && income1 && ALLOWED_INCOME_RANGES[ecstat1] + return false unless ecstat1 && income1 && ALLOWED_INCOME_RANGES_SALES[ecstat1] - income1 < ALLOWED_INCOME_RANGES[ecstat1][:soft_min] + income1 < ALLOWED_INCOME_RANGES_SALES[ecstat1][:soft_min] + end + + def income2_under_soft_min? + return false unless ecstat2 && income2 && ALLOWED_INCOME_RANGES_SALES[ecstat2] + + income2 < ALLOWED_INCOME_RANGES_SALES[ecstat2][:soft_min] end def staircase_bought_above_fifty? diff --git a/config/locales/en.yml b/config/locales/en.yml index 747ed4414..bc54b74ce 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -223,8 +223,12 @@ en: under_hard_min: "Net income cannot be less than £%{hard_min} per week given the tenant’s working situation" freq_missing: "Select how often the household receives income" earnings_missing: "Enter how much income the household has in total" - income1: - over_hard_max: "Income must be lower than £%{hard_max}" + income: + over_hard_max_for_london: "Income must not exceed £90,000 for properties within London local authorities" + over_hard_max_for_outside_london: "Income must not exceed £80,000 for properties outside London local authorities" + combined_over_hard_max_for_london: "Combined income must not exceed £90,000 for properties within London local authorities" + combined_over_hard_max_for_outside_london: "Combined income must not exceed £80,000 for properties outside London local authorities" + child_has_income: "Child's income must be £0" negative_currency: "Enter an amount above 0" rent: less_than_shortfall: "Enter an amount that is more than the shortfall in basic rent" @@ -467,6 +471,8 @@ en: message: "Net income is lower than expected based on the lead tenant’s working situation. Are you sure this is correct?" 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?" + income: + under_soft_min_for_economic_status: "You said income was %{income}, which is below this working situation's minimum (%{minimum})" rent: 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}." @@ -559,4 +565,4 @@ en: one_argument: "This is based on the tenant’s work situation: %{ecstat1}" title_text: no_argument: "Some test text" - one_argument: "You said this: %{ecstat1}" + one_argument: "You said this: %{argument}" diff --git a/db/migrate/20230105103134_add_income2_value_check.rb b/db/migrate/20230105103134_add_income2_value_check.rb new file mode 100644 index 000000000..38a3ebc65 --- /dev/null +++ b/db/migrate/20230105103134_add_income2_value_check.rb @@ -0,0 +1,5 @@ +class AddIncome2ValueCheck < ActiveRecord::Migration[7.0] + def change + add_column :sales_logs, :income2_value_check, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index 2c1ff7692..6cb5420d5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -520,6 +520,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_02_10_143120) do t.integer "value_value_check" t.integer "old_persons_shared_ownership_value_check" t.integer "staircase_bought_value_check" + t.integer "income2_value_check" t.integer "monthly_charges_value_check" t.integer "details_known_5" t.integer "details_known_6" diff --git a/spec/helpers/interruption_screen_helper_spec.rb b/spec/helpers/interruption_screen_helper_spec.rb index 8153b9d7b..f46aca9ae 100644 --- a/spec/helpers/interruption_screen_helper_spec.rb +++ b/spec/helpers/interruption_screen_helper_spec.rb @@ -13,6 +13,7 @@ RSpec.describe InterruptionScreenHelper do earnings: 750, incfreq: 1, created_by: user, + sex1: "F", ) end @@ -94,6 +95,54 @@ RSpec.describe InterruptionScreenHelper do .to eq("") end end + + context "when an argument is given not for a label" do + translation = "test.title_text.one_argument" + it "returns the correct text" do + informative_text_hash = { + "translation" => translation, + "arguments" => [ + { + "key" => "earnings", + "i18n_template" => "argument", + }, + ], + } + expect(display_informative_text(informative_text_hash, lettings_log)).to eq(I18n.t(translation, argument: lettings_log.earnings)) + end + end + + context "when and argument is given with a key and arguments for the key" do + it "makes the correct method call" do + informative_text_hash = { + "arguments" => [ + { + "key" => "retirement_age_for_person", + "arguments_for_key" => 1, + "i18n_template" => "argument", + }, + ], + } + allow(lettings_log).to receive(:retirement_age_for_person) + display_informative_text(informative_text_hash, lettings_log) + expect(lettings_log).to have_received(:retirement_age_for_person).with(1) + end + + it "returns the correct text" do + translation = "test.title_text.one_argument" + informative_text_hash = { + "translation" => translation, + "arguments" => [ + { + "key" => "retirement_age_for_person", + "arguments_for_key" => 1, + "i18n_template" => "argument", + }, + ], + } + expect(display_informative_text(informative_text_hash, lettings_log)).to eq(I18n.t(translation, argument: lettings_log.retirement_age_for_person(1))) + end + end end describe "display_title_text" do @@ -113,12 +162,12 @@ RSpec.describe InterruptionScreenHelper do { "key" => "ecstat1", "label" => true, - "i18n_template" => "ecstat1", + "i18n_template" => "argument", }, ], } expect(display_title_text(title_text, lettings_log)) - .to eq(I18n.t("test.title_text.one_argument", ecstat1: lettings_log.form.get_question("ecstat1", lettings_log).answer_label(lettings_log).downcase)) + .to eq(I18n.t("test.title_text.one_argument", argument: lettings_log.form.get_question("ecstat1", lettings_log).answer_label(lettings_log).downcase)) end end diff --git a/spec/models/form/sales/questions/buyer1_income_value_check_spec.rb b/spec/models/form/sales/questions/buyer1_income_value_check_spec.rb index 552f4f56f..89801a398 100644 --- a/spec/models/form/sales/questions/buyer1_income_value_check_spec.rb +++ b/spec/models/form/sales/questions/buyer1_income_value_check_spec.rb @@ -16,7 +16,7 @@ RSpec.describe Form::Sales::Questions::Buyer1IncomeValueCheck, type: :model do end it "has the correct header" do - expect(question.header).to eq("Are you sure this income is correct?") + expect(question.header).to eq("Are you sure this is correct?") end it "has the correct check_answer_label" do diff --git a/spec/models/form/sales/subsections/household_characteristics_spec.rb b/spec/models/form/sales/subsections/household_characteristics_spec.rb index 76109b7f4..33e9a1409 100644 --- a/spec/models/form/sales/subsections/household_characteristics_spec.rb +++ b/spec/models/form/sales/subsections/household_characteristics_spec.rb @@ -46,7 +46,8 @@ RSpec.describe Form::Sales::Subsections::HouseholdCharacteristics, type: :model buyer_2_gender_identity gender_2_buyer_retirement_value_check buyer_2_working_situation - working_situation_2_buyer_retirement_value_check + working_situation_2_retirement_value_check_joint_purchase + working_situation_buyer_2_income_value_check buyer_2_live_in_property number_of_others_in_property person_2_known @@ -126,7 +127,8 @@ RSpec.describe Form::Sales::Subsections::HouseholdCharacteristics, type: :model buyer_2_ethnic_background_mixed buyer_2_ethnic_background_white buyer_2_working_situation - working_situation_2_buyer_retirement_value_check + working_situation_2_retirement_value_check_joint_purchase + working_situation_buyer_2_income_value_check buyer_2_live_in_property number_of_others_in_property person_2_known diff --git a/spec/models/form/sales/subsections/income_benefits_and_savings_spec.rb b/spec/models/form/sales/subsections/income_benefits_and_savings_spec.rb index 74002d5e0..cfc677233 100644 --- a/spec/models/form/sales/subsections/income_benefits_and_savings_spec.rb +++ b/spec/models/form/sales/subsections/income_benefits_and_savings_spec.rb @@ -27,6 +27,7 @@ RSpec.describe Form::Sales::Subsections::IncomeBenefitsAndSavings, type: :model buyer_1_mortgage_value_check buyer_2_income buyer_2_income_mortgage_value_check + buyer_2_income_value_check buyer_2_mortgage buyer_2_mortgage_value_check housing_benefits @@ -52,6 +53,7 @@ RSpec.describe Form::Sales::Subsections::IncomeBenefitsAndSavings, type: :model buyer_1_mortgage_value_check buyer_2_income buyer_2_income_mortgage_value_check + buyer_2_income_value_check buyer_2_mortgage buyer_2_mortgage_value_check housing_benefits diff --git a/spec/models/form_handler_spec.rb b/spec/models/form_handler_spec.rb index 9b4920198..b04980ce4 100644 --- a/spec/models/form_handler_spec.rb +++ b/spec/models/form_handler_spec.rb @@ -54,14 +54,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(179) + expect(form.pages.count).to eq(181) 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(179) + expect(form.pages.count).to eq(181) expect(form.name).to eq("2021_2022_sales") end end diff --git a/spec/models/sales_log_spec.rb b/spec/models/sales_log_spec.rb index 523c2924b..7b9f7c7ef 100644 --- a/spec/models/sales_log_spec.rb +++ b/spec/models/sales_log_spec.rb @@ -266,6 +266,7 @@ RSpec.describe SalesLog, type: :model do relat4: "X", relat5: "X", relat6: "P", + income2: 0, ecstat2: 9, ecstat3: 7, age1: 47, @@ -370,4 +371,26 @@ RSpec.describe SalesLog, type: :model do expect(completed_sales_log.expected_shared_ownership_deposit_value).to eq(500) end end + + describe "#field_formatted_as_currency" do + let(:completed_sales_log) { FactoryBot.create(:sales_log, :completed) } + + it "returns small numbers correctly formatted as currency" do + completed_sales_log.update!(savings: 4) + + expect(completed_sales_log.field_formatted_as_currency("savings")).to eq("£4.00") + end + + it "returns quite large numbers correctly formatted as currency" do + completed_sales_log.update!(savings: 40_000) + + expect(completed_sales_log.field_formatted_as_currency("savings")).to eq("£40,000.00") + end + + it "returns very large numbers correctly formatted as currency" do + completed_sales_log.update!(savings: 400_000_000) + + expect(completed_sales_log.field_formatted_as_currency("savings")).to eq("£400,000,000.00") + end + end end diff --git a/spec/models/validations/sales/financial_validations_spec.rb b/spec/models/validations/sales/financial_validations_spec.rb index 88f36943f..fae0e4536 100644 --- a/spec/models/validations/sales/financial_validations_spec.rb +++ b/spec/models/validations/sales/financial_validations_spec.rb @@ -5,75 +5,110 @@ RSpec.describe Validations::Sales::FinancialValidations do let(:validator_class) { Class.new { include Validations::Sales::FinancialValidations } } - describe "income validations" do - let(:record) { FactoryBot.create(:sales_log, ownershipsch: 1, la: "E08000035") } - - context "with shared ownership" do - context "and non london borough" do - (0..8).each do |ecstat| - it "adds an error when buyer 1 income is over hard max for ecstat #{ecstat}" do - record.income1 = 85_000 - record.ecstat1 = ecstat - financial_validator.validate_income1(record) - expect(record.errors["income1"]) - .to include(match I18n.t("validations.financial.income1.over_hard_max", hard_max: 80_000)) - expect(record.errors["ecstat1"]) - .to include(match I18n.t("validations.financial.income1.over_hard_max", hard_max: 80_000)) - expect(record.errors["ownershipsch"]) - .to include(match I18n.t("validations.financial.income1.over_hard_max", hard_max: 80_000)) - expect(record.errors["la"]) - .to include(match I18n.t("validations.financial.income1.over_hard_max", hard_max: 80_000)) - expect(record.errors["postcode_full"]) - .to include(match I18n.t("validations.financial.income1.over_hard_max", hard_max: 80_000)) - end - end - - it "validates that the income is within the expected range for the tenant’s employment status" do - record.income1 = 75_000 - record.ecstat1 = 1 - financial_validator.validate_income1(record) - expect(record.errors["income1"]).to be_empty - expect(record.errors["ecstat1"]).to be_empty - expect(record.errors["ownershipsch"]).to be_empty - expect(record.errors["la"]).to be_empty - expect(record.errors["postcode_full"]).to be_empty - end - end - - context "and a london borough" do - before do - record.update!(la: "E09000030") - record.reload - end - - (0..8).each do |ecstat| - it "adds an error when buyer 1 income is over hard max for ecstat #{ecstat}" do - record.income1 = 95_000 - record.ecstat1 = ecstat - financial_validator.validate_income1(record) - expect(record.errors["income1"]) - .to include(match I18n.t("validations.financial.income1.over_hard_max", hard_max: 90_000)) - expect(record.errors["ecstat1"]) - .to include(match I18n.t("validations.financial.income1.over_hard_max", hard_max: 90_000)) - expect(record.errors["ownershipsch"]) - .to include(match I18n.t("validations.financial.income1.over_hard_max", hard_max: 90_000)) - expect(record.errors["la"]) - .to include(match I18n.t("validations.financial.income1.over_hard_max", hard_max: 90_000)) - expect(record.errors["postcode_full"]) - .to include(match I18n.t("validations.financial.income1.over_hard_max", hard_max: 90_000)) - end - end - - it "validates that the income is within the expected range for the tenant’s employment status" do - record.income1 = 85_000 - record.ecstat1 = 1 - financial_validator.validate_income1(record) - expect(record.errors["income1"]).to be_empty - expect(record.errors["ecstat1"]).to be_empty - expect(record.errors["ownershipsch"]).to be_empty - expect(record.errors["la"]).to be_empty - expect(record.errors["postcode_full"]).to be_empty - end + describe "income validations for shared ownership" do + let(:record) { FactoryBot.create(:sales_log, ownershipsch: 1) } + + context "when buying in a non london borough" do + before do + record.update!(la: "E08000035") + record.reload + end + + 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")) + 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")) + end + + it "does not add errors if buyer 1 has income 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 + record.income2 = 75_000 + financial_validator.validate_income2(record) + expect(record.errors).to be_empty + end + + it "adds errors when combined income is over 80_000" do + record.income1 = 45_000 + record.income2 = 40_000 + financial_validator.validate_combined_income(record) + expect(record.errors["income1"]).to include(match I18n.t("validations.financial.income.combined_over_hard_max_for_outside_london")) + expect(record.errors["income2"]).to include(match I18n.t("validations.financial.income.combined_over_hard_max_for_outside_london")) + end + + it "does not add errors when combined income is under 80_000" do + record.income1 = 35_000 + record.income2 = 40_000 + financial_validator.validate_combined_income(record) + expect(record.errors).to be_empty + end + end + + context "when buying in a london borough" do + before do + record.update!(la: "E09000030") + record.reload + end + + 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")) + 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")) + end + + it "does not add errors if buyer 1 has income 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 + record.income2 = 75_000 + financial_validator.validate_income2(record) + expect(record.errors).to be_empty + end + + it "adds errors when combined income is over 90_000" do + record.income1 = 55_000 + record.income2 = 40_000 + financial_validator.validate_combined_income(record) + expect(record.errors["income1"]).to include(match I18n.t("validations.financial.income.combined_over_hard_max_for_london")) + expect(record.errors["income2"]).to include(match I18n.t("validations.financial.income.combined_over_hard_max_for_london")) + end + + it "does not add errors when combined income is under 90_000" do + record.income1 = 35_000 + record.income2 = 40_000 + financial_validator.validate_combined_income(record) + expect(record.errors).to be_empty end end end @@ -96,7 +131,7 @@ RSpec.describe Validations::Sales::FinancialValidations do it "does not add an error if the cash discount is in the expected range" do record.cashdis = 10_000 financial_validator.validate_cash_discount(record) - expect(record.errors["cashdis"]).to be_empty + expect(record.errors).to be_empty end end @@ -107,16 +142,14 @@ RSpec.describe Validations::Sales::FinancialValidations do record.stairbought = 20 record.stairowned = 40 financial_validator.validate_percentage_bought_not_greater_than_percentage_owned(record) - expect(record.errors["stairbought"]).to be_empty - expect(record.errors["stairowned"]).to be_empty + expect(record.errors).to be_empty end it "does not add an error if the percentage bought is equal to the percentage owned" do record.stairbought = 30 record.stairowned = 30 financial_validator.validate_percentage_bought_not_greater_than_percentage_owned(record) - expect(record.errors["stairbought"]).to be_empty - expect(record.errors["stairowned"]).to be_empty + expect(record.errors).to be_empty end it "adds an error to stairowned and not stairbought if the percentage bought is more than the percentage owned" do @@ -135,8 +168,7 @@ RSpec.describe Validations::Sales::FinancialValidations do record.type = 2 record.stairowned = 80 financial_validator.validate_percentage_owned_not_too_much_if_older_person(record) - expect(record.errors["stairowned"]).to be_empty - expect(record.errors["type"]).to be_empty + expect(record.errors).to be_empty end end @@ -145,8 +177,7 @@ RSpec.describe Validations::Sales::FinancialValidations do record.type = 24 record.stairowned = 50 financial_validator.validate_percentage_owned_not_too_much_if_older_person(record) - expect(record.errors["stairowned"]).to be_empty - expect(record.errors["type"]).to be_empty + expect(record.errors).to be_empty end it "adds an error when percentage owned after staircasing transaction exceeds 75%" do @@ -158,4 +189,39 @@ RSpec.describe Validations::Sales::FinancialValidations do end end end + + describe "#validate_child_income" do + let(:record) { FactoryBot.create(:sales_log) } + + context "when buyer 2 is not a child" do + before do + record.update!(ecstat2: rand(0..8)) + record.reload + end + + it "does not add an error if buyer 2 has an income" do + record.ecstat2 = rand(0..8) + record.income2 = 40_000 + financial_validator.validate_child_income(record) + expect(record.errors).to be_empty + end + end + + context "when buyer 2 is a child" do + it "does not add an error if buyer 2 has no income" do + record.ecstat2 = 9 + record.income2 = 0 + financial_validator.validate_child_income(record) + expect(record.errors).to be_empty + end + + it "adds errors if buyer 2 has an income" do + record.ecstat2 = 9 + record.income2 = 40_000 + financial_validator.validate_child_income(record) + expect(record.errors["ecstat2"]).to include(match I18n.t("validations.financial.income.child_has_income")) + expect(record.errors["income2"]).to include(match I18n.t("validations.financial.income.child_has_income")) + end + end + end end From 5f9151158adde342d1b74873a2940eb120c1a3ec Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Wed, 22 Feb 2023 09:53:57 +0000 Subject: [PATCH 06/13] CLDC-1889 Bulk upload schemes + locations (#1329) * bulk upload validates scheme data * bulk upload schemes can use new core IDs * bulk upload handles locations --- app/models/location.rb | 6 + app/models/scheme.rb | 10 ++ .../bulk_upload/lettings/row_parser.rb | 50 ++++++++- spec/factories/location.rb | 5 + spec/factories/scheme.rb | 4 + .../bulk_upload/lettings/row_parser_spec.rb | 106 +++++++++++++++++- 6 files changed, 177 insertions(+), 4 deletions(-) diff --git a/app/models/location.rb b/app/models/location.rb index 36a134f81..3798c57e8 100644 --- a/app/models/location.rb +++ b/app/models/location.rb @@ -366,6 +366,12 @@ class Location < ApplicationRecord enum type_of_unit: TYPE_OF_UNIT + def self.find_by_id_on_mulitple_fields(id) + return if id.nil? + + where(id:).or(where(old_visible_id: id)).first + end + def postcode=(postcode) if postcode super UKPostcode.parse(postcode).to_s diff --git a/app/models/scheme.rb b/app/models/scheme.rb index f8cf6bf63..ca3624e3a 100644 --- a/app/models/scheme.rb +++ b/app/models/scheme.rb @@ -110,6 +110,16 @@ class Scheme < ApplicationRecord enum arrangement_type: ARRANGEMENT_TYPE, _suffix: true + def self.find_by_id_on_mulitple_fields(id) + return if id.nil? + + if id.start_with?("S") + where(id: id[1..]).first + else + where(old_visible_id: id).first + end + end + def id_to_display "S#{id}" end diff --git a/app/services/bulk_upload/lettings/row_parser.rb b/app/services/bulk_upload/lettings/row_parser.rb index 9cb3edb78..dcba8be28 100644 --- a/app/services/bulk_upload/lettings/row_parser.rb +++ b/app/services/bulk_upload/lettings/row_parser.rb @@ -8,7 +8,7 @@ class BulkUpload::Lettings::RowParser attribute :field_1, :integer attribute :field_2 attribute :field_3 - attribute :field_4, :integer + attribute :field_4, :string attribute :field_5, :integer attribute :field_6 attribute :field_7, :string @@ -164,6 +164,12 @@ class BulkUpload::Lettings::RowParser validate :validate_managing_org_related validate :validate_managing_org_exists + validate :validate_scheme_related + validate :validate_scheme_exists + + validate :validate_location_related + validate :validate_location_exists + def valid? errors.clear @@ -199,6 +205,45 @@ class BulkUpload::Lettings::RowParser private + def validate_location_related + return if scheme.blank? || location.blank? + + unless location.scheme == scheme + block_log_creation! + errors.add(:field_5, "Scheme code must relate to a location that is owned by owning organisation or managing organisation") + end + end + + def location + return if scheme.nil? + + @location ||= scheme.locations.find_by_id_on_mulitple_fields(field_5) + end + + def validate_location_exists + if scheme && field_5.present? && location.nil? + errors.add(:field_5, "Location could be found with provided scheme code") + end + end + + def validate_scheme_related + return unless field_4.present? && scheme.present? + + owned_by_owning_org = owning_organisation && scheme.owning_organisation == owning_organisation + owned_by_managing_org = managing_organisation && scheme.owning_organisation == managing_organisation + + unless owned_by_owning_org || owned_by_managing_org + block_log_creation! + errors.add(:field_4, "This management group code does not belong to your organisation, or any of your stock owners / managing agents") + end + end + + def validate_scheme_exists + if field_4.present? && scheme.nil? + errors.add(:field_4, "The management group code is not correct") + end + end + def validate_managing_org_related if owning_organisation && managing_organisation && !owning_organisation.can_be_managed_by?(organisation: managing_organisation) block_log_creation! @@ -566,6 +611,7 @@ private attributes["managing_organisation_id"] = managing_organisation_id attributes["renewal"] = renewal attributes["scheme"] = scheme + attributes["location"] = location attributes["created_by"] = bulk_upload.user attributes["needstype"] = bulk_upload.needstype attributes["rent_type"] = rent_type @@ -943,6 +989,6 @@ private end def scheme - @scheme ||= Scheme.find_by(old_visible_id: field_4) + @scheme ||= Scheme.find_by_id_on_mulitple_fields(field_4) end end diff --git a/spec/factories/location.rb b/spec/factories/location.rb index 75b4380f5..f43da0ac8 100644 --- a/spec/factories/location.rb +++ b/spec/factories/location.rb @@ -10,6 +10,7 @@ FactoryBot.define do startdate { Time.zone.local(2022, 4, 1) } confirmed { true } scheme + trait :export do postcode { "SW1A 2AA" } name { "Downing Street" } @@ -19,5 +20,9 @@ FactoryBot.define do scheme { FactoryBot.create(:scheme, :export) } old_visible_id { "111" } end + + trait :with_old_visible_id do + old_visible_id { rand(9_999_999).to_s } + end end end diff --git a/spec/factories/scheme.rb b/spec/factories/scheme.rb index 6c08e269d..155dea11a 100644 --- a/spec/factories/scheme.rb +++ b/spec/factories/scheme.rb @@ -21,5 +21,9 @@ FactoryBot.define do primary_client_group { "G" } secondary_client_group { "M" } end + + trait :with_old_visible_id do + old_visible_id { rand(9_999_999) } + end end end diff --git a/spec/services/bulk_upload/lettings/row_parser_spec.rb b/spec/services/bulk_upload/lettings/row_parser_spec.rb index d1792e9df..7a09ba4b8 100644 --- a/spec/services/bulk_upload/lettings/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/row_parser_spec.rb @@ -11,6 +11,8 @@ RSpec.describe BulkUpload::Lettings::RowParser do let(:owning_org) { create(:organisation, :with_old_visible_id) } let(:managing_org) { create(:organisation, :with_old_visible_id) } + let(:scheme) { create(:scheme, :with_old_visible_id, owning_organisation: owning_org) } + let(:location) { create(:location, :with_old_visible_id, scheme:) } let(:setup_section_params) do { @@ -85,7 +87,7 @@ RSpec.describe BulkUpload::Lettings::RowParser do { bulk_upload:, field_1: "1", - field_4: "1", + field_4: scheme.old_visible_id, field_7: "123", field_96: now.day.to_s, field_97: now.month.to_s, @@ -296,10 +298,90 @@ RSpec.describe BulkUpload::Lettings::RowParser do context "when matching scheme cannot be found" do let(:attributes) { { bulk_upload:, field_1: "1", field_4: "123" } } - xit "returns an error" do + it "returns an error" do expect(parser.errors[:field_4]).to be_present end end + + context "when scheme belongs to someone else" do + let(:other_scheme) { create(:scheme, :with_old_visible_id) } + let(:attributes) { { bulk_upload:, field_1: "1", field_4: other_scheme.old_visible_id, field_111: owning_org.old_visible_id } } + + it "returns an error" do + expect(parser.errors[:field_4]).to include("This management group code does not belong to your organisation, or any of your stock owners / managing agents") + end + end + + context "when scheme belongs to owning org" do + let(:scheme) { create(:scheme, :with_old_visible_id, owning_organisation: owning_org) } + let(:attributes) { { bulk_upload:, field_1: "1", field_4: scheme.old_visible_id, field_111: owning_org.old_visible_id } } + + it "does not return an error" do + expect(parser.errors[:field_4]).to be_blank + end + end + + context "when scheme belongs to managing org" do + let(:scheme) { create(:scheme, :with_old_visible_id, owning_organisation: managing_org) } + let(:attributes) { { bulk_upload:, field_1: "1", field_4: scheme.old_visible_id, field_113: managing_org.old_visible_id } } + + it "does not return an error" do + expect(parser.errors[:field_4]).to be_blank + end + end + end + + describe "#field_5" do + context "when location does not exist" do + let(:scheme) { create(:scheme, :with_old_visible_id, owning_organisation: owning_org) } + let(:attributes) do + { + bulk_upload:, + field_1: "1", + field_4: scheme.old_visible_id, + field_5: "dontexist", + field_111: owning_org.old_visible_id, + } + end + + it "returns an error" do + expect(parser.errors[:field_5]).to be_present + end + end + + context "when location exists" do + let(:scheme) { create(:scheme, :with_old_visible_id, owning_organisation: owning_org) } + let(:attributes) do + { + bulk_upload:, + field_1: "1", + field_4: scheme.old_visible_id, + field_5: location.old_visible_id, + field_111: owning_org.old_visible_id, + } + end + + it "does not return an error" do + expect(parser.errors[:field_5]).to be_blank + end + end + + context "when location exists but not related" do + let(:location) { create(:scheme, :with_old_visible_id) } + let(:attributes) do + { + bulk_upload:, + field_1: "1", + field_4: scheme.old_visible_id, + field_5: location.old_visible_id, + field_111: owning_org.old_visible_id, + } + end + + it "returns an error" do + expect(parser.errors[:field_5]).to be_present + end + end end describe "#field_7" do @@ -592,6 +674,26 @@ RSpec.describe BulkUpload::Lettings::RowParser do end describe "#log" do + describe "#location" do + context "when lookup is via new core id" do + let(:attributes) { { bulk_upload:, field_4: scheme.old_visible_id, field_5: location.id, field_111: owning_org } } + + it "assigns the correct location" do + expect(parser.log.location).to eql(location) + end + end + end + + describe "#scheme" do + context "when lookup is via id prefixed with S" do + let(:attributes) { { bulk_upload:, field_4: "S#{scheme.id}", field_111: owning_org } } + + it "assigns the correct scheme" do + expect(parser.log.scheme).to eql(scheme) + end + end + end + describe "#owning_organisation" do context "when lookup is via id prefixed with ORG" do let(:attributes) { { bulk_upload:, field_111: "ORG#{owning_org.id}" } } From 68a1c17ae5d7d457bd8ceeb8a09eaaaca6ae72b7 Mon Sep 17 00:00:00 2001 From: Jack <113976590+bibblobcode@users.noreply.github.com> Date: Thu, 23 Feb 2023 09:08:28 +0000 Subject: [PATCH 07/13] CLDC-1916 Handle sales forms and validations during comparison period (#1317) * Add more time helpers * CLDC-1916 Validate this and next year start date in saleslogs * Add capybara-screenshot * Use sales_in_crossover_period? --- Gemfile | 1 + Gemfile.lock | 6 + app/helpers/collection_time_helper.rb | 16 ++ app/helpers/tasklist_helper.rb | 21 +-- app/models/form_handler.rb | 8 +- .../validations/sales/setup_validations.rb | 38 ++++- config/initializers/feature_toggle.rb | 7 +- config/locales/en.yml | 6 +- spec/helpers/tasklist_helper_spec.rb | 143 ++++++++---------- .../sales/setup_validations_spec.rb | 97 +++++++++--- spec/rails_helper.rb | 1 + 11 files changed, 222 insertions(+), 122 deletions(-) diff --git a/Gemfile b/Gemfile index 17e10865c..358e71070 100644 --- a/Gemfile +++ b/Gemfile @@ -91,6 +91,7 @@ end group :test do gem "capybara", require: false gem "capybara-lockstep" + gem "capybara-screenshot" gem "faker" gem "rspec-rails", require: false gem "selenium-webdriver", require: false diff --git a/Gemfile.lock b/Gemfile.lock index fa61ec504..7da2ef437 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -129,6 +129,9 @@ GEM capybara (>= 2.0) ruby2_keywords selenium-webdriver (>= 3) + capybara-screenshot (1.0.26) + capybara (>= 1.0, < 4) + launchy childprocess (4.1.0) coderay (1.1.3) concurrent-ruby (1.2.0) @@ -201,6 +204,8 @@ GEM json-schema (3.0.0) addressable (>= 2.8) jwt (2.7.0) + launchy (2.5.2) + addressable (~> 2.8) listen (3.8.0) rb-fsevent (~> 0.10, >= 0.10.3) rb-inotify (~> 0.9, >= 0.9.10) @@ -449,6 +454,7 @@ DEPENDENCIES byebug capybara capybara-lockstep + capybara-screenshot devise! devise_two_factor_authentication dotenv-rails diff --git a/app/helpers/collection_time_helper.rb b/app/helpers/collection_time_helper.rb index be0abae20..23014b8d1 100644 --- a/app/helpers/collection_time_helper.rb +++ b/app/helpers/collection_time_helper.rb @@ -23,4 +23,20 @@ module CollectionTimeHelper def current_collection_end_date Time.zone.local(current_collection_start_year + 1, 3, 31) end + + def previous_collection_end_date + current_collection_end_date - 1.year + end + + def next_collection_start_year + current_collection_start_year + 1 + end + + def previous_collection_start_year + current_collection_start_year - 1 + end + + def previous_collection_start_date + current_collection_start_date - 1.year + end end diff --git a/app/helpers/tasklist_helper.rb b/app/helpers/tasklist_helper.rb index f4f1d51dd..30112894f 100644 --- a/app/helpers/tasklist_helper.rb +++ b/app/helpers/tasklist_helper.rb @@ -11,15 +11,6 @@ module TasklistHelper log.form.subsections.count { |subsection| subsection.status(log) == status && subsection.applicable_questions(log).count.positive? } end - def next_page_or_check_answers(subsection, log, current_user) - path = if subsection.is_started?(log) - "#{log.class.name.underscore}_#{subsection.id}_check_answers_path" - else - "#{log.class.name.underscore}_#{next_question_page(subsection, log, current_user)}_path" - end - send(path, log) - end - def next_question_page(subsection, log, current_user) if subsection.pages.first.routed_to?(log, current_user) subsection.pages.first.id @@ -46,4 +37,16 @@ module TasklistHelper "This log is from the #{log.form.start_date.year}/#{log.form.start_date.year + 1} collection window, which is now closed." end end + +private + + def next_page_or_check_answers(subsection, log, current_user) + path = if subsection.is_started?(log) + "#{log.class.name.underscore}_#{subsection.id}_check_answers_path" + else + "#{log.class.name.underscore}_#{next_question_page(subsection, log, current_user)}_path" + end + + send(path, log) + end end diff --git a/app/models/form_handler.rb b/app/models/form_handler.rb index 8f34288b3..4ba300552 100644 --- a/app/models/form_handler.rb +++ b/app/models/form_handler.rb @@ -35,8 +35,8 @@ class FormHandler def sales_forms { "current_sales" => Form.new(nil, current_collection_start_year, SALES_SECTIONS, "sales"), - "previous_sales" => Form.new(nil, current_collection_start_year - 1, SALES_SECTIONS, "sales"), - "next_sales" => Form.new(nil, current_collection_start_year + 1, SALES_SECTIONS, "sales"), + "previous_sales" => Form.new(nil, previous_collection_start_year, SALES_SECTIONS, "sales"), + "next_sales" => Form.new(nil, next_collection_start_year, SALES_SECTIONS, "sales"), } end @@ -52,10 +52,10 @@ class FormHandler end if forms["previous_lettings"].blank? && current_collection_start_year >= 2022 - forms["previous_lettings"] = Form.new(nil, current_collection_start_year - 1, LETTINGS_SECTIONS, "lettings") + forms["previous_lettings"] = Form.new(nil, previous_collection_start_year, LETTINGS_SECTIONS, "lettings") end forms["current_lettings"] = Form.new(nil, current_collection_start_year, LETTINGS_SECTIONS, "lettings") if forms["current_lettings"].blank? - forms["next_lettings"] = Form.new(nil, current_collection_start_year + 1, LETTINGS_SECTIONS, "lettings") if forms["next_lettings"].blank? + forms["next_lettings"] = Form.new(nil, next_collection_start_year, LETTINGS_SECTIONS, "lettings") if forms["next_lettings"].blank? forms end diff --git a/app/models/validations/sales/setup_validations.rb b/app/models/validations/sales/setup_validations.rb index dadf85650..e7090aabf 100644 --- a/app/models/validations/sales/setup_validations.rb +++ b/app/models/validations/sales/setup_validations.rb @@ -1,11 +1,45 @@ module Validations::Sales::SetupValidations include Validations::SharedValidations + include CollectionTimeHelper def validate_saledate(record) return unless record.saledate && date_valid?("saledate", record) - unless record.saledate.between?(Time.zone.local(2022, 4, 1), Time.zone.local(2023, 3, 31)) || !FeatureToggle.saledate_collection_window_validation_enabled? - record.errors.add :saledate, I18n.t("validations.setup.saledate.financial_year") + unless record.saledate.between?(active_collection_start_date, current_collection_end_date) || !FeatureToggle.saledate_collection_window_validation_enabled? + record.errors.add :saledate, validation_error_message + end + end + +private + + def active_collection_start_date + if FormHandler.instance.sales_in_crossover_period? + previous_collection_start_date + else + current_collection_start_date + end + end + + def validation_error_message + current_end_year_long = current_collection_end_date.strftime("#{current_collection_end_date.day.ordinalize} %B %Y") + + if FormHandler.instance.sales_in_crossover_period? + I18n.t( + "validations.setup.saledate.previous_and_current_financial_year", + previous_start_year_short: previous_collection_start_date.strftime("%y"), + previous_end_year_short: previous_collection_end_date.strftime("%y"), + previous_start_year_long: previous_collection_start_date.strftime("#{previous_collection_start_date.day.ordinalize} %B %Y"), + current_end_year_short: current_collection_end_date.strftime("%y"), + current_end_year_long:, + ) + else + I18n.t( + "validations.setup.saledate.current_financial_year", + current_start_year_short: current_collection_start_date.strftime("%y"), + current_end_year_short: current_collection_end_date.strftime("%y"), + current_start_year_long: current_collection_start_date.strftime("#{current_collection_start_date.day.ordinalize} %B %Y"), + current_end_year_long:, + ) end end end diff --git a/config/initializers/feature_toggle.rb b/config/initializers/feature_toggle.rb index 60a97c95c..dda281e10 100644 --- a/config/initializers/feature_toggle.rb +++ b/config/initializers/feature_toggle.rb @@ -1,13 +1,14 @@ class FeatureToggle - def self.startdate_two_week_validation_enabled? + # Disable check on preview apps to allow for testing of future forms + def self.saledate_collection_window_validation_enabled? Rails.env.production? || Rails.env.test? || Rails.env.staging? end - def self.startdate_collection_window_validation_enabled? + def self.startdate_two_week_validation_enabled? Rails.env.production? || Rails.env.test? || Rails.env.staging? end - def self.saledate_collection_window_validation_enabled? + def self.startdate_collection_window_validation_enabled? Rails.env.production? || Rails.env.test? || Rails.env.staging? end diff --git a/config/locales/en.yml b/config/locales/en.yml index bc54b74ce..8665c022a 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -150,7 +150,11 @@ en: intermediate_rent_product_name: blank: "Enter name of other intermediate rent product" saledate: - financial_year: "Date must be from 22/23 financial year, which is between 1st April 2022 and 31st March 2023" + current_financial_year: + Enter a date within the %{current_start_year_short}/%{current_end_year_short} financial year, which is between %{current_start_year_long} and %{current_end_year_long} + previous_and_current_financial_year: + "Enter a date within the %{previous_start_year_short}/%{previous_end_year_short} or %{previous_end_year_short}/%{current_end_year_short} financial years, which is between %{previous_start_year_long} and %{current_end_year_long}" + 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" diff --git a/spec/helpers/tasklist_helper_spec.rb b/spec/helpers/tasklist_helper_spec.rb index 1ac6cf738..b5bfe89f5 100644 --- a/spec/helpers/tasklist_helper_spec.rb +++ b/spec/helpers/tasklist_helper_spec.rb @@ -1,100 +1,55 @@ require "rails_helper" RSpec.describe TasklistHelper do - describe "with lettings" do - let(:empty_lettings_log) { FactoryBot.create(:lettings_log) } - let(:lettings_log) { FactoryBot.create(:lettings_log, :in_progress, needstype: 1) } - let(:fake_2021_2022_form) { Form.new("spec/fixtures/forms/2021_2022.json") } + let(:now) { Time.utc(2022, 6, 1) } - context "with 2021 2022 form" do - before do - allow(FormHandler.instance).to receive(:current_lettings_form).and_return(fake_2021_2022_form) - end + around do |example| + Timecop.freeze(now) do + Singleton.__init__(FormHandler) + example.run + end + Timecop.return + Singleton.__init__(FormHandler) + end - describe "get next incomplete section" do - it "returns the first subsection name if it is not completed" do - expect(get_next_incomplete_section(lettings_log).id).to eq("household_characteristics") - end + describe "with lettings" do + let(:empty_lettings_log) { create(:lettings_log) } + let(:lettings_log) { build(:lettings_log, :in_progress, needstype: 1) } - it "returns the first subsection name if it is partially completed" do - lettings_log["tenancycode"] = 123 - expect(get_next_incomplete_section(lettings_log).id).to eq("household_characteristics") - end + describe "get next incomplete section" do + it "returns the first subsection name if it is not completed" do + expect(get_next_incomplete_section(lettings_log).id).to eq("household_characteristics") end - describe "get sections count" do - it "returns the total of sections if no status is given" do - expect(get_subsections_count(empty_lettings_log)).to eq(8) - end - - it "returns 0 sections for completed sections if no sections are completed" do - expect(get_subsections_count(empty_lettings_log, :completed)).to eq(0) - end - - it "returns the number of not started sections" do - expect(get_subsections_count(empty_lettings_log, :not_started)).to eq(8) - end - - it "returns the number of sections in progress" do - expect(get_subsections_count(lettings_log, :in_progress)).to eq(3) - end - - it "returns 0 for invalid state" do - expect(get_subsections_count(lettings_log, :fake)).to eq(0) - end + it "returns the first subsection name if it is partially completed" do + lettings_log["tenancycode"] = 123 + expect(get_next_incomplete_section(lettings_log).id).to eq("household_characteristics") end + end - describe "get_next_page_or_check_answers" do - let(:subsection) { lettings_log.form.get_subsection("household_characteristics") } - let(:user) { FactoryBot.build(:user) } - - it "returns the check answers page path if the section has been started already" do - expect(next_page_or_check_answers(subsection, lettings_log, user)).to match(/check-answers/) - end - - it "returns the first question page path for the section if it has not been started yet" do - expect(next_page_or_check_answers(subsection, empty_lettings_log, user)).to match(/tenant-code-test/) - end - - it "when first question being not routed to returns the next routed question link" do - empty_lettings_log.housingneeds_a = "No" - expect(next_page_or_check_answers(subsection, empty_lettings_log, user)).to match(/person-1-gender/) - end + describe "get sections count" do + it "returns the total of sections if no status is given" do + expect(get_subsections_count(empty_lettings_log)).to eq(1) end - describe "subsection link" do - let(:subsection) { lettings_log.form.get_subsection("household_characteristics") } - let(:user) { FactoryBot.build(:user) } - - context "with a subsection that's enabled" do - it "returns the subsection link url" do - expect(subsection_link(subsection, lettings_log, user)).to match(/household-characteristics/) - end - end + it "returns 0 sections for completed sections if no sections are completed" do + expect(get_subsections_count(empty_lettings_log, :completed)).to eq(0) + end - context "with a subsection that cannot be started yet" do - before do - allow(subsection).to receive(:status).with(lettings_log).and_return(:cannot_start_yet) - end + it "returns the number of not started sections" do + expect(get_subsections_count(empty_lettings_log, :not_started)).to eq(1) + end - it "returns the label instead of a link" do - expect(subsection_link(subsection, lettings_log, user)).to match(subsection.label) - end - end + it "returns the number of sections in progress" do + expect(get_subsections_count(lettings_log, :in_progress)).to eq(2) end - end - end - describe "#review_log_text" do - around do |example| - Timecop.freeze(now) do - Singleton.__init__(FormHandler) - example.run + it "returns 0 for invalid state" do + expect(get_subsections_count(lettings_log, :fake)).to eq(0) end - Singleton.__init__(FormHandler) end - context "with lettings log" do + describe "review_log_text" do context "when collection_period_open? == true" do context "with 2023 deadline" do let(:now) { Time.utc(2022, 6, 1) } @@ -129,6 +84,30 @@ RSpec.describe TasklistHelper do end end + describe "subsection link" do + let(:lettings_log) { create(:lettings_log, :completed) } + let(:subsection) { lettings_log.form.get_subsection("household_characteristics") } + let(:user) { build(:user) } + + context "with a subsection that's enabled" do + it "returns the subsection link url" do + expect(subsection_link(subsection, lettings_log, user)).to match(/household-characteristics/) + end + end + + context "with a subsection that cannot be started yet" do + before do + allow(subsection).to receive(:status).with(lettings_log).and_return(:cannot_start_yet) + end + + it "returns the label instead of a link" do + expect(subsection_link(subsection, lettings_log, user)).to match(subsection.label) + end + end + end + end + + describe "#review_log_text" do context "with sales log" do context "when collection_period_open? == true" do let(:now) { Time.utc(2022, 6, 1) } @@ -142,11 +121,13 @@ RSpec.describe TasklistHelper do end context "when collection_period_open? == false" do - let(:now) { Time.utc(2023, 7, 8) } - let(:sales_log) { create(:sales_log, :completed, saledate: Time.utc(2023, 2, 8)) } + let(:now) { Time.utc(2022, 6, 1) } + let!(:sales_log) { create(:sales_log, :completed) } it "returns relevant text" do - expect(review_log_text(sales_log)).to eq("This log is from the 2022/2023 collection window, which is now closed.") + Timecop.freeze(now + 1.year) do + expect(review_log_text(sales_log)).to eq("This log is from the 2021/2022 collection window, which is now closed.") + end end end end diff --git a/spec/models/validations/sales/setup_validations_spec.rb b/spec/models/validations/sales/setup_validations_spec.rb index 74b7834ab..4571ae5cd 100644 --- a/spec/models/validations/sales/setup_validations_spec.rb +++ b/spec/models/validations/sales/setup_validations_spec.rb @@ -6,43 +6,96 @@ RSpec.describe Validations::Sales::SetupValidations do let(:validator_class) { Class.new { include Validations::Sales::SetupValidations } } describe "#validate_saledate" do - context "when saledate is blank" do - let(:record) { FactoryBot.build(:sales_log, saledate: nil) } + context "with sales_in_crossover_period == false" do + context "when saledate is blank" do + let(:record) { build(:sales_log, saledate: nil) } - it "does not add an error" do - setup_validator.validate_saledate(record) + it "does not add an error" do + setup_validator.validate_saledate(record) - expect(record.errors).to be_empty + expect(record.errors).to be_empty + end end - end - context "when saledate is in the 22/23 financial year" do - let(:record) { FactoryBot.build(:sales_log, saledate: Time.zone.local(2023, 1, 1)) } + context "when saledate is in the 22/23 financial year" do + let(:record) { build(:sales_log, saledate: Time.zone.local(2023, 1, 1)) } - it "does not add an error" do - setup_validator.validate_saledate(record) + it "does not add an error" do + setup_validator.validate_saledate(record) - expect(record.errors).to be_empty + expect(record.errors).to be_empty + end + end + + context "when saledate is before the 22/23 financial year" do + let(:record) { build(:sales_log, saledate: Time.zone.local(2020, 1, 1)) } + + it "adds error" do + setup_validator.validate_saledate(record) + + expect(record.errors[:saledate]).to include("Enter a date within the 22/23 financial year, which is between 1st April 2022 and 31st March 2023") + end end - end - context "when saledate is before the 22/23 financial year" do - let(:record) { FactoryBot.build(:sales_log, saledate: Time.zone.local(2022, 1, 1)) } + context "when saledate is after the 22/23 financial year" do + let(:record) { build(:sales_log, saledate: Time.zone.local(2025, 4, 1)) } - it "adds error" do - setup_validator.validate_saledate(record) + it "adds error" do + setup_validator.validate_saledate(record) - expect(record.errors[:saledate]).to include(I18n.t("validations.setup.saledate.financial_year")) + expect(record.errors[:saledate]).to include("Enter a date within the 22/23 financial year, which is between 1st April 2022 and 31st March 2023") + end end end - context "when saledate is after the 22/23 financial year" do - let(:record) { FactoryBot.build(:sales_log, saledate: Time.zone.local(2023, 4, 1)) } + context "with sales_in_crossover_period == true" do + around do |example| + Timecop.freeze(Time.zone.local(2024, 5, 1)) do + Singleton.__init__(FormHandler) + example.run + end + Timecop.return + Singleton.__init__(FormHandler) + end + + context "when saledate is blank" do + let(:record) { build(:sales_log, saledate: nil) } + + it "does not add an error" do + setup_validator.validate_saledate(record) + + expect(record.errors).to be_empty + end + end + + context "when saledate is in the 22/23 financial year" do + let(:record) { build(:sales_log, saledate: Time.zone.local(2024, 1, 1)) } + + it "does not add an error" do + setup_validator.validate_saledate(record) + + expect(record.errors).to be_empty + end + end + + context "when saledate is before the 22/23 financial year" do + let(:record) { build(:sales_log, saledate: Time.zone.local(2020, 5, 1)) } + + it "adds error" do + setup_validator.validate_saledate(record) + + expect(record.errors[:saledate]).to include("Enter a date within the 23/24 or 24/25 financial years, which is between 1st April 2023 and 31st March 2025") + end + end + + context "when saledate is after the 22/23 financial year" do + let(:record) { build(:sales_log, saledate: Time.zone.local(2025, 4, 1)) } - it "adds error" do - setup_validator.validate_saledate(record) + it "adds error" do + setup_validator.validate_saledate(record) - expect(record.errors[:saledate]).to include(I18n.t("validations.setup.saledate.financial_year")) + expect(record.errors[:saledate]).to include("Enter a date within the 23/24 or 24/25 financial years, which is between 1st April 2023 and 31st March 2025") + end end end end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 81d9246ec..8e175ccf6 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -6,6 +6,7 @@ require File.expand_path("../config/environment", __dir__) abort("The Rails environment is running in production mode!") if Rails.env.production? require "rspec/rails" require "capybara/rspec" +require "capybara-screenshot/rspec" require "selenium-webdriver" require "view_component/test_helpers" From 66a528bd48412e7da8441985c8e5fbe506c62e49 Mon Sep 17 00:00:00 2001 From: kosiakkatrina <54268893+kosiakkatrina@users.noreply.github.com> Date: Thu, 23 Feb 2023 09:56:54 +0000 Subject: [PATCH 08/13] CLDC-1860 Change location question header for 23/24 (#1291) * Change location question header for 23/24 * Stup form in page tests --- .../form/lettings/questions/location_id.rb | 10 +++++++++- .../form/lettings/pages/location_spec.rb | 6 ++++++ .../lettings/questions/location_id_spec.rb | 18 ++++++++++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/app/models/form/lettings/questions/location_id.rb b/app/models/form/lettings/questions/location_id.rb index 5a99e7734..fc197cf82 100644 --- a/app/models/form/lettings/questions/location_id.rb +++ b/app/models/form/lettings/questions/location_id.rb @@ -2,7 +2,7 @@ class Form::Lettings::Questions::LocationId < ::Form::Question def initialize(_id, hsh, page) super("location_id", hsh, page) @check_answer_label = "Location" - @header = "Which location is this log for?" + @header = header_text @type = "radio" @answer_options = answer_options @inferred_answers = { @@ -47,4 +47,12 @@ private def selected_answer_option_is_derived?(_lettings_log) false end + + def header_text + if form.start_date && form.start_date.year >= 2023 + "Which location is this letting for?" + else + "Which location is this log for?" + end + end end diff --git a/spec/models/form/lettings/pages/location_spec.rb b/spec/models/form/lettings/pages/location_spec.rb index aefcd59a9..659a4dd26 100644 --- a/spec/models/form/lettings/pages/location_spec.rb +++ b/spec/models/form/lettings/pages/location_spec.rb @@ -6,6 +6,12 @@ RSpec.describe Form::Lettings::Pages::Location, type: :model do let(:page_id) { nil } let(:page_definition) { nil } let(:subsection) { instance_double(Form::Subsection) } + let(:form) { instance_double(Form) } + + before do + allow(form).to receive(:start_date).and_return(Time.zone.local(2022, 4, 1)) + allow(subsection).to receive(:form).and_return(form) + end it "has correct subsection" do expect(page.subsection).to eq(subsection) diff --git a/spec/models/form/lettings/questions/location_id_spec.rb b/spec/models/form/lettings/questions/location_id_spec.rb index ca3d402a9..afb39ce11 100644 --- a/spec/models/form/lettings/questions/location_id_spec.rb +++ b/spec/models/form/lettings/questions/location_id_spec.rb @@ -6,6 +6,14 @@ RSpec.describe Form::Lettings::Questions::LocationId, type: :model do let(:question_id) { nil } let(:question_definition) { nil } let(:page) { instance_double(Form::Page) } + let(:subsection) { instance_double(Form::Subsection) } + let(:form) { instance_double(Form) } + + before do + allow(form).to receive(:start_date).and_return(Time.zone.local(2022, 4, 1)) + allow(page).to receive(:subsection).and_return(subsection) + allow(subsection).to receive(:form).and_return(form) + end it "has correct page" do expect(question.page).to eq(page) @@ -103,4 +111,14 @@ RSpec.describe Form::Lettings::Questions::LocationId, type: :model do end end end + + context "with collection year on or after 2023" do + before do + allow(form).to receive(:start_date).and_return(Time.zone.local(2023, 4, 1)) + end + + it "has the correct header" do + expect(question.header).to eq("Which location is this letting for?") + end + end end From f1c7b8188f02d6e0ef71c776234fbb3968b06eec Mon Sep 17 00:00:00 2001 From: Jack <113976590+bibblobcode@users.noreply.github.com> Date: Thu, 23 Feb 2023 14:24:04 +0000 Subject: [PATCH 09/13] [CLDC-1915] Update privacy notice (#1332) * [CLDC-1915] Update privacy notice * Use generic notice --- app/views/content/privacy_notice.md | 45 +++++++++++++++-------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/app/views/content/privacy_notice.md b/app/views/content/privacy_notice.md index f80a5e4ca..b1188eb5a 100644 --- a/app/views/content/privacy_notice.md +++ b/app/views/content/privacy_notice.md @@ -1,46 +1,47 @@ -## How are we using your information? +## How do we use your information? -If your household has entered a new social housing tenancy, social housing providers will share your personal information with the Department for Levelling Up, Housing & Communities (DLUHC) for research and statistical purposes. +If your household enters a new social housing tenancy or purchases a social housing property, social housing providers will share your personal information with the Department for Levelling Up, Housing & Communities (DLUHC) for research and statistical purposes only. -## How is this information provided? +## How do we get this information? -The information is provided via ‘<%= t('service_name') %>’, a service funded and managed by DLUHC. It collects information on the tenants or buyers, tenancy or sale, and the dwelling itself. Some of this information is personal and sensitive, so DLUHC is responsible for ensuring that all data is processed in line with data protection legislation. +The information is provided via ‘<%= t('service_name') %>’, a service funded and managed by DLUHC. It collects information on the tenants or residents, tenancy or sale, and the dwelling itself. Some of this data is personal and sensitive, so DLUHC is responsible for ensuring it’s processed in line with data protection legislation. -## Why are we sharing this information? -Information collected using this service is shared with other government departments and agencies. Data is shared with the Greater London Authority and the Regulator of Social Housing. Data providers can also access data for their organisations via the online service. Data is only shared for research and statistical purposes. +## Why do we share this information? + +Information collected via CORE is shared with other government departments and agencies. It’s shared with the Greater London Authority and the Regulator of Social Housing. Data providers can also access data for their organisations via CORE. Data is only shared for research and statistical purposes. ## How does this affect you? -It will not affect your benefits, services or any treatments you receive. The information shared is anonymous and handled in accordance with the law. We are collecting and sharing your information to help us better understand the social housing market and inform social housing policy. +Information sharing will not affect your benefits, services or any treatments you receive. It’s anonymous and handled in accordance with the law. We collect and share your information to help us better understand the social housing market and inform social housing policy. -## If you want to know more +## To find out more… -Social housing lettings and sales data is collected on behalf of DLUHC for research and statistical purposes only. Data providers do not require the consent of tenants to provide the information, but tenants have the right to know how and for what purpose data is being collected, held and used. +Social housing lettings and sales data is collected on DLUHC’s behalf. Data providers do not require the tenant or buyer’s consent to provide this information, but tenants and buyers have the right to know how and for what purpose data is being collected, held and used. -The processing must have a lawful basis. In this case the processing is necessary for the performance of a task carried out in the public interest to meet a function of the Crown, a Minister of the Crown, or a government department. +Data processing must have a lawful basis. In this case it’s necessary for a task carried out in the public interest meeting a function of the Crown, a Minister of the Crown, or government department. -You have the right to object and you have the right to obtain confirmation that your data is being processed, and to access your personal data. You also have the right to have any incorrect personal data corrected. +You have the right to object, and obtain confirmation that your data is being processed, as well as access your personal data, and have any incorrect personal data corrected. -The information collected via this service relates to your tenancy, the dwelling you are living in or buying, and your household. Some of the information may have been provided by you as a tenant when signing the new tenancy or buying your property. Other information has been gathered from the housing management systems of social housing providers. +Information collected via CORE relates to your tenancy, the dwelling you are living in or buying, and your household. Some information may have been provided by you (as a tenant or buyer) when signing the new tenancy or buying your property. Other information has been gathered from the housing management systems of social housing providers. -Data collected will be held for as long as necessary for research and statistical purposes. When no longer needed, data will be deleted in a safe manner. We are aware that some of the data collected is particularly sensitive. For example: +Collected data will be held for as long as necessary for research and statistical purposes. When no longer needed, data will be deleted in a safe manner. We’re aware some collected data is particularly sensitive. For example: * ethnic group -* if previous tenure is a hospital or prison or approved probation hostel support +* if previous tenure is a hospital, prison or approved probation hostel support * if household left last settled home because discharged from prison, a long stay hospital or other institution -* if source of referral is probation or prison, youth offending team, community mental health team or health service +* if referral source is probation or prison, youth offending or community mental health team, or health service -All the information collected via this service is treated in accordance with data protection requirements and guidelines. +DLUHC publishes data annually, in aggregate form, as part of a report and complementary tables. -Data is published by DLUHC in aggregate form on an annual basis as part of a report and complementary tables. +* For annual lettings data, visit: [www.gov.uk/government/collections/rents-lettings-and-tenancies](www.gov.uk/government/collections/rents-lettings-and-tenancies) -You can visit to access the annual publications on lettings. Or visit to view the publications on sales. +* For annual sales data, visit: [www.gov.uk/government/collections/social-housing-sales-including-right-to-buy-and-transfers](www.gov.uk/government/collections/social-housing-sales-including-right-to-buy-and-transfers) -The detail level data is anonymised and protected to minimise the risk of identification and held with the UK Data Archive for research purposes. +Detail-level data is anonymised and protected, minimising identification risk. It's held with the UK Data Archive. -## Making a complaint +## Complaints -If you are unhappy with any aspect of this privacy notice, or how your personal information is being processed, contact the Department Data Protection Officer at: +If you’re unhappy with any privacy notice aspect, or how we process your information, contact the Department Data Protection Officer: -If you are still not happy, you have the right to lodge a complaint with the Information Commissioner’s Office (ICO) at [ico.org.uk/concern](https://ico.org.uk/concern). +You also have the right to complain to the Information Commissioner’s Office (ICO): [www.ico.org.uk/concern](www.ico.org.uk/concern) From 609c9ba91e7b779cd2912e2bfe9203be027fa607 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire <94526761+natdeanlewissoftwire@users.noreply.github.com> Date: Thu, 23 Feb 2023 14:43:05 +0000 Subject: [PATCH 10/13] feat: add hidden_in_check_answers (#1325) --- .../form/sales/questions/previous_postcode_known.rb | 10 ++++++++++ .../sales/questions/previous_postcode_known_spec.rb | 9 +++++++++ 2 files changed, 19 insertions(+) diff --git a/app/models/form/sales/questions/previous_postcode_known.rb b/app/models/form/sales/questions/previous_postcode_known.rb index 25ee906f8..9806d12e4 100644 --- a/app/models/form/sales/questions/previous_postcode_known.rb +++ b/app/models/form/sales/questions/previous_postcode_known.rb @@ -10,6 +10,16 @@ class Form::Sales::Questions::PreviousPostcodeKnown < ::Form::Question "ppostcode_full" => [0], } @hint_text = "This is also known as the household’s 'last settled home'" + @hidden_in_check_answers = { + "depends_on" => [ + { + "ppcodenk" => 0, + }, + { + "ppcodenk" => 1, + }, + ], + } end ANSWER_OPTIONS = { diff --git a/spec/models/form/sales/questions/previous_postcode_known_spec.rb b/spec/models/form/sales/questions/previous_postcode_known_spec.rb index 6febe027f..b1d244b1e 100644 --- a/spec/models/form/sales/questions/previous_postcode_known_spec.rb +++ b/spec/models/form/sales/questions/previous_postcode_known_spec.rb @@ -47,4 +47,13 @@ RSpec.describe Form::Sales::Questions::PreviousPostcodeKnown, type: :model do it "has the correct hint" do expect(question.hint_text).to eq("This is also known as the household’s 'last settled home'") end + + it "has the correct hidden_in_check_answers" do + expect(question.hidden_in_check_answers).to eq({ + "depends_on" => [ + { "ppcodenk" => 0 }, + { "ppcodenk" => 1 }, + ], + }) + end end From 06a48cecdaae808d885d09c364ec8ea18961b847 Mon Sep 17 00:00:00 2001 From: Phil Lee Date: Thu, 23 Feb 2023 14:57:56 +0000 Subject: [PATCH 11/13] bulk upload handles social housing first time let (#1337) - this question is present in the web form - however is not surfaced for bulk upload - therefore the answer must be inferred --- app/services/bulk_upload/lettings/row_parser.rb | 2 ++ .../bulk_upload/lettings/row_parser_spec.rb | 14 ++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/app/services/bulk_upload/lettings/row_parser.rb b/app/services/bulk_upload/lettings/row_parser.rb index dcba8be28..085a3662f 100644 --- a/app/services/bulk_upload/lettings/row_parser.rb +++ b/app/services/bulk_upload/lettings/row_parser.rb @@ -776,6 +776,8 @@ private case rsnvac when 15, 16, 17 1 + else + 0 end end diff --git a/spec/services/bulk_upload/lettings/row_parser_spec.rb b/spec/services/bulk_upload/lettings/row_parser_spec.rb index 7a09ba4b8..dabc8d1ab 100644 --- a/spec/services/bulk_upload/lettings/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/row_parser_spec.rb @@ -186,6 +186,12 @@ RSpec.describe BulkUpload::Lettings::RowParser do field_80: "1234.56", field_87: "1", field_88: "234.56", + + field_106: "15", + field_99: "0", + field_89: now.day.to_s, + field_90: now.month.to_s, + field_91: now.strftime("%g"), } end @@ -1154,6 +1160,14 @@ RSpec.describe BulkUpload::Lettings::RowParser do expect(parser.log.first_time_property_let_as_social_housing).to eq(1) end end + + context "when field_106 is not 15, 16, or 17" do + let(:attributes) { { bulk_upload:, field_106: "1" } } + + it "sets to 0" do + expect(parser.log.first_time_property_let_as_social_housing).to eq(0) + end + end end describe "#housingneeds" do From 223f1420601ff0babad49d2fd1aae2aafe47285e Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire <94526761+natdeanlewissoftwire@users.noreply.github.com> Date: Thu, 23 Feb 2023 15:05:15 +0000 Subject: [PATCH 12/13] CLDC-1812 Length of stay in property before purchase updates (#1296) * feat: conditional question * db:update * tests: add tests for year dependent behaviour * db: migrate --- .../sales/pages/living_before_purchase.rb | 11 +- .../sales/questions/living_before_purchase.rb | 31 +++-- .../questions/living_before_purchase_years.rb | 31 +++++ ...13140932_add_proplen_asked_to_sales_log.rb | 5 + db/schema.rb | 5 +- .../pages/living_before_purchase_spec.rb | 20 ++- .../questions/living_before_purchase_spec.rb | 31 ++--- .../living_before_purchase_years_spec.rb | 114 ++++++++++++++++++ 8 files changed, 209 insertions(+), 39 deletions(-) create mode 100644 app/models/form/sales/questions/living_before_purchase_years.rb create mode 100644 db/migrate/20230213140932_add_proplen_asked_to_sales_log.rb create mode 100644 spec/models/form/sales/questions/living_before_purchase_years_spec.rb diff --git a/app/models/form/sales/pages/living_before_purchase.rb b/app/models/form/sales/pages/living_before_purchase.rb index 5fa695bbc..ac61ab1de 100644 --- a/app/models/form/sales/pages/living_before_purchase.rb +++ b/app/models/form/sales/pages/living_before_purchase.rb @@ -1,7 +1,14 @@ class Form::Sales::Pages::LivingBeforePurchase < ::Form::Page def questions @questions ||= [ - Form::Sales::Questions::LivingBeforePurchase.new(nil, nil, self), - ] + living_before_purchase, + Form::Sales::Questions::LivingBeforePurchaseYears.new(nil, nil, self), + ].compact + end + + def living_before_purchase + if form.start_date.year >= 2023 + Form::Sales::Questions::LivingBeforePurchase.new(nil, nil, self) + end end end diff --git a/app/models/form/sales/questions/living_before_purchase.rb b/app/models/form/sales/questions/living_before_purchase.rb index feedaac63..f631220dc 100644 --- a/app/models/form/sales/questions/living_before_purchase.rb +++ b/app/models/form/sales/questions/living_before_purchase.rb @@ -1,15 +1,26 @@ class Form::Sales::Questions::LivingBeforePurchase < ::Form::Question def initialize(id, hsh, page) super - @id = "proplen" - @check_answer_label = "Number of years living in the property before purchase" - @header = "How long did the buyer(s) live in the property before purchase?" - @hint_text = "You should round this up to the nearest year. If the buyers haven't been living in the property, enter '0'" - @type = "numeric" - @min = 0 - @max = 80 - @step = 1 - @width = 5 - @suffix = " years" + @id = "proplen_asked" + @check_answer_label = "Buyer lived in the property before purchasing" + @header = "Did the buyer live in the property before purchasing it?" + @hint_text = nil + @type = "radio" + @answer_options = ANSWER_OPTIONS + @conditional_for = { + "proplen" => [0], + } + @hidden_in_check_answers = { + "depends_on" => [ + { + "proplen_asked" => 0, + }, + ], + } end + + ANSWER_OPTIONS = { + "0" => { "value" => "Yes" }, + "1" => { "value" => "No" }, + }.freeze end diff --git a/app/models/form/sales/questions/living_before_purchase_years.rb b/app/models/form/sales/questions/living_before_purchase_years.rb new file mode 100644 index 000000000..f3f1e58a7 --- /dev/null +++ b/app/models/form/sales/questions/living_before_purchase_years.rb @@ -0,0 +1,31 @@ +class Form::Sales::Questions::LivingBeforePurchaseYears < ::Form::Question + def initialize(id, hsh, page) + super + @id = "proplen" + @check_answer_label = "Number of years living in the property before purchase" + @header = header_text + @hint_text = hint_text + @type = "numeric" + @min = 0 + @max = 80 + @step = 1 + @width = 5 + @suffix = " years" + end + + def header_text + if form.start_date.year >= 2023 + "How long did they live there?" + else + "How long did the buyer(s) live in the property before purchase?" + end + end + + def hint_text + if form.start_date.year >= 2023 + "You should round up to the nearest year" + else + "You should round this up to the nearest year. If the buyers haven't been living in the property, enter '0'" + end + end +end diff --git a/db/migrate/20230213140932_add_proplen_asked_to_sales_log.rb b/db/migrate/20230213140932_add_proplen_asked_to_sales_log.rb new file mode 100644 index 000000000..e32a5836e --- /dev/null +++ b/db/migrate/20230213140932_add_proplen_asked_to_sales_log.rb @@ -0,0 +1,5 @@ +class AddProplenAskedToSalesLog < ActiveRecord::Migration[7.0] + def change + add_column :sales_logs, :proplen_asked, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index 6cb5420d5..1747fa6c5 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_02_10_143120) do +ActiveRecord::Schema[7.0].define(version: 2023_02_13_140932) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -490,6 +490,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_02_10_143120) do t.integer "prevten" t.integer "mortgageused" t.integer "wchair" + t.integer "income2_value_check" t.integer "armedforcesspouse" t.datetime "hodate", precision: nil t.integer "hoday" @@ -520,7 +521,6 @@ ActiveRecord::Schema[7.0].define(version: 2023_02_10_143120) do t.integer "value_value_check" t.integer "old_persons_shared_ownership_value_check" t.integer "staircase_bought_value_check" - t.integer "income2_value_check" t.integer "monthly_charges_value_check" t.integer "details_known_5" t.integer "details_known_6" @@ -529,6 +529,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_02_10_143120) do t.integer "staircasesale" t.integer "ethnic_group2" t.integer "ethnicbuy2" + t.integer "proplen_asked" 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/models/form/sales/pages/living_before_purchase_spec.rb b/spec/models/form/sales/pages/living_before_purchase_spec.rb index 435229831..76dbd7609 100644 --- a/spec/models/form/sales/pages/living_before_purchase_spec.rb +++ b/spec/models/form/sales/pages/living_before_purchase_spec.rb @@ -11,8 +11,24 @@ RSpec.describe Form::Sales::Pages::LivingBeforePurchase, type: :model do expect(page.subsection).to eq(subsection) end - it "has correct questions" do - expect(page.questions.map(&:id)).to eq(%w[proplen]) + describe "questions" do + let(:subsection) { instance_double(Form::Subsection, form: instance_double(Form, start_date:)) } + + context "when 2022" do + let(:start_date) { Time.utc(2022, 2, 8) } + + it "has correct questions" do + expect(page.questions.map(&:id)).to eq(%w[proplen]) + end + end + + context "when 2023" do + let(:start_date) { Time.utc(2023, 2, 8) } + + it "has correct questions" do + expect(page.questions.map(&:id)).to eq(%w[proplen_asked proplen]) + end + end end it "has the correct id" do diff --git a/spec/models/form/sales/questions/living_before_purchase_spec.rb b/spec/models/form/sales/questions/living_before_purchase_spec.rb index 29f785c06..530bb151a 100644 --- a/spec/models/form/sales/questions/living_before_purchase_spec.rb +++ b/spec/models/form/sales/questions/living_before_purchase_spec.rb @@ -12,19 +12,19 @@ RSpec.describe Form::Sales::Questions::LivingBeforePurchase, type: :model do end it "has the correct id" do - expect(question.id).to eq("proplen") + expect(question.id).to eq("proplen_asked") end it "has the correct header" do - expect(question.header).to eq("How long did the buyer(s) live in the property before purchase?") + expect(question.header).to eq("Did the buyer live in the property before purchasing it?") end it "has the correct check_answer_label" do - expect(question.check_answer_label).to eq("Number of years living in the property before purchase") + expect(question.check_answer_label).to eq("Buyer lived in the property before purchasing") end it "has the correct type" do - expect(question.type).to eq("numeric") + expect(question.type).to eq("radio") end it "is not marked as derived" do @@ -32,26 +32,11 @@ RSpec.describe Form::Sales::Questions::LivingBeforePurchase, type: :model do end it "has the correct hint" do - expect(question.hint_text).to eq("You should round this up to the nearest year. If the buyers haven't been living in the property, enter '0'") + expect(question.hint_text).to be_nil end - it "has correct width" do - expect(question.width).to eq(5) - end - - it "has correct step" do - expect(question.step).to eq(1) - end - - it "has correct suffix" do - expect(question.suffix).to eq(" years") - end - - it "has correct min" do - expect(question.min).to eq(0) - end - - it "has correct max" do - expect(question.max).to eq(80) + it "has the correct answer_options" do + expect(question.answer_options).to eq("0" => { "value" => "Yes" }, + "1" => { "value" => "No" }) end end diff --git a/spec/models/form/sales/questions/living_before_purchase_years_spec.rb b/spec/models/form/sales/questions/living_before_purchase_years_spec.rb new file mode 100644 index 000000000..5fdb5ccba --- /dev/null +++ b/spec/models/form/sales/questions/living_before_purchase_years_spec.rb @@ -0,0 +1,114 @@ +require "rails_helper" + +RSpec.describe Form::Sales::Questions::LivingBeforePurchaseYears, type: :model do + subject(:question) { described_class.new(question_id, question_definition, page) } + + let(:question_id) { nil } + let(:question_definition) { nil } + let(:subsection) { instance_double(Form::Subsection, form: instance_double(Form, start_date:)) } + let(:page) { instance_double(Form::Page, subsection:) } + + context "when 2022" do + let(:start_date) { Time.utc(2022, 2, 8) } + + it "has correct page" do + expect(question.page).to eq(page) + end + + it "has the correct id" do + expect(question.id).to eq("proplen") + end + + it "has the correct header" do + expect(question.header).to eq("How long did the buyer(s) live in the property before purchase?") + end + + it "has the correct check_answer_label" do + expect(question.check_answer_label).to eq("Number of years living in the property before purchase") + end + + it "has the correct type" do + expect(question.type).to eq("numeric") + 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 eq("You should round this up to the nearest year. If the buyers haven't been living in the property, enter '0'") + end + + it "has correct width" do + expect(question.width).to eq(5) + end + + it "has correct step" do + expect(question.step).to eq(1) + end + + it "has correct suffix" do + expect(question.suffix).to eq(" years") + end + + it "has correct min" do + expect(question.min).to eq(0) + end + + it "has correct max" do + expect(question.max).to eq(80) + end + end + + context "when 2023" do + let(:start_date) { Time.utc(2023, 2, 8) } + + it "has correct page" do + expect(question.page).to eq(page) + end + + it "has the correct id" do + expect(question.id).to eq("proplen") + end + + it "has the correct header" do + expect(question.header).to eq("How long did they live there?") + end + + it "has the correct check_answer_label" do + expect(question.check_answer_label).to eq("Number of years living in the property before purchase") + end + + it "has the correct type" do + expect(question.type).to eq("numeric") + 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 eq("You should round up to the nearest year") + end + + it "has correct width" do + expect(question.width).to eq(5) + end + + it "has correct step" do + expect(question.step).to eq(1) + end + + it "has correct suffix" do + expect(question.suffix).to eq(" years") + end + + it "has correct min" do + expect(question.min).to eq(0) + end + + it "has correct max" do + expect(question.max).to eq(80) + end + end +end From 2e279d251daa8506cd6802b62423ef44a4372dd3 Mon Sep 17 00:00:00 2001 From: natdeanlewissoftwire <94526761+natdeanlewissoftwire@users.noreply.github.com> Date: Thu, 23 Feb 2023 15:14:51 +0000 Subject: [PATCH 13/13] CLDC-1976 remove duplicate purchase price page (#1324) * feat: remove duplicated purchase price page and delete as it's not used elsewhere * test: update * feat: update * db: update --- app/models/form/sales/pages/purchase_price.rb | 14 -------- .../discounted_ownership_scheme.rb | 1 - .../form/sales/pages/purchase_price_spec.rb | 35 ------------------- .../discounted_ownership_scheme_spec.rb | 1 - spec/models/form_handler_spec.rb | 4 +-- 5 files changed, 2 insertions(+), 53 deletions(-) delete mode 100644 app/models/form/sales/pages/purchase_price.rb delete mode 100644 spec/models/form/sales/pages/purchase_price_spec.rb diff --git a/app/models/form/sales/pages/purchase_price.rb b/app/models/form/sales/pages/purchase_price.rb deleted file mode 100644 index af13fc682..000000000 --- a/app/models/form/sales/pages/purchase_price.rb +++ /dev/null @@ -1,14 +0,0 @@ -class Form::Sales::Pages::PurchasePrice < ::Form::Page - def initialize(id, hsh, subsection) - super - @depends_on = [ - { "ownershipsch" => 2, "rent_to_buy_full_ownership?" => false }, - ] - end - - def questions - @questions ||= [ - Form::Sales::Questions::PurchasePrice.new(nil, nil, self), - ] - end -end diff --git a/app/models/form/sales/subsections/discounted_ownership_scheme.rb b/app/models/form/sales/subsections/discounted_ownership_scheme.rb index 499411ffc..74e408cfd 100644 --- a/app/models/form/sales/subsections/discounted_ownership_scheme.rb +++ b/app/models/form/sales/subsections/discounted_ownership_scheme.rb @@ -13,7 +13,6 @@ class Form::Sales::Subsections::DiscountedOwnershipScheme < ::Form::Subsection Form::Sales::Pages::ExtraBorrowingValueCheck.new("extra_borrowing_price_value_check", nil, self), Form::Sales::Pages::AboutPriceNotRtb.new(nil, nil, self), Form::Sales::Pages::GrantValueCheck.new(nil, nil, self), - Form::Sales::Pages::PurchasePrice.new("purchase_price_discounted_ownership", nil, self), Form::Sales::Pages::PurchasePriceOutrightOwnership.new("purchase_price_outright_ownership", nil, self), Form::Sales::Pages::DepositAndMortgageValueCheck.new("discounted_ownership_deposit_and_mortgage_value_check_after_value_and_discount", nil, self), Form::Sales::Pages::Mortgageused.new("mortgage_used_discounted_ownership", nil, self), diff --git a/spec/models/form/sales/pages/purchase_price_spec.rb b/spec/models/form/sales/pages/purchase_price_spec.rb deleted file mode 100644 index 6ec2b7ac0..000000000 --- a/spec/models/form/sales/pages/purchase_price_spec.rb +++ /dev/null @@ -1,35 +0,0 @@ -require "rails_helper" - -RSpec.describe Form::Sales::Pages::PurchasePrice, 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([ - { "ownershipsch" => 2, "rent_to_buy_full_ownership?" => false }, - ]) - 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 5919abb24..12cf9b38a 100644 --- a/spec/models/form/sales/subsections/discounted_ownership_scheme_spec.rb +++ b/spec/models/form/sales/subsections/discounted_ownership_scheme_spec.rb @@ -19,7 +19,6 @@ RSpec.describe Form::Sales::Subsections::DiscountedOwnershipScheme, type: :model extra_borrowing_price_value_check 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 diff --git a/spec/models/form_handler_spec.rb b/spec/models/form_handler_spec.rb index b04980ce4..17ebd4db0 100644 --- a/spec/models/form_handler_spec.rb +++ b/spec/models/form_handler_spec.rb @@ -54,14 +54,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(181) + expect(form.pages.count).to eq(180) 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(181) + expect(form.pages.count).to eq(180) expect(form.name).to eq("2021_2022_sales") end end