From 49e72da0d99958146bf75f6d8d3249652f0fdedb Mon Sep 17 00:00:00 2001 From: Robert Sullivan Date: Tue, 5 Mar 2024 17:04:46 +0000 Subject: [PATCH] CLDC-3245: Make Q59 optional (#2271) --- .../derived_variables/sales_log_variables.rb | 1 + app/models/form/question.rb | 6 +++-- .../sales/questions/buyers_organisations.rb | 6 +---- app/models/sales_log.rb | 9 ++++++- .../bulk_upload/sales/year2023/row_parser.rb | 15 ++++------- .../bulk_upload/sales/year2024/row_parser.rb | 15 ++++------- .../questions/buyers_organisations_spec.rb | 8 ++---- spec/models/sales_log_spec.rb | 19 ++++++++++++++ .../sales/year2023/row_parser_spec.rb | 26 +++++++++++++------ .../sales/year2024/row_parser_spec.rb | 26 +++++++++++++------ 10 files changed, 81 insertions(+), 50 deletions(-) diff --git a/app/models/derived_variables/sales_log_variables.rb b/app/models/derived_variables/sales_log_variables.rb index ec8d21b30..a4817a230 100644 --- a/app/models/derived_variables/sales_log_variables.rb +++ b/app/models/derived_variables/sales_log_variables.rb @@ -4,6 +4,7 @@ module DerivedVariables::SalesLogVariables def set_derived_fields! reset_invalidated_derived_values!(DEPENDENCIES) + self.pregblank = 1 if no_buyer_organisation? self.ethnic = 17 if ethnic_refused? self.mscharge = nil if no_monthly_leasehold_charges? if exdate.present? diff --git a/app/models/form/question.rb b/app/models/form/question.rb index eac75dc5c..391771ad9 100644 --- a/app/models/form/question.rb +++ b/app/models/form/question.rb @@ -3,8 +3,10 @@ class Form::Question :type, :min, :max, :step, :width, :fields_to_add, :result_field, :conditional_for, :readonly, :answer_options, :page, :check_answer_label, :inferred_answers, :hidden_in_check_answers, :inferred_check_answers_value, - :top_guidance_partial, :bottom_guidance_partial, :prefix, :suffix, :requires_js, :fields_added, :derived, - :check_answers_card_number, :unresolved_hint_text, :question_number, :hide_question_number_on_page, :plain_label, :error_label + :top_guidance_partial, :bottom_guidance_partial, :prefix, :suffix, + :requires_js, :fields_added, :derived, :check_answers_card_number, + :unresolved_hint_text, :question_number, :hide_question_number_on_page, + :plain_label, :error_label def initialize(id, hsh, page) @id = id diff --git a/app/models/form/sales/questions/buyers_organisations.rb b/app/models/form/sales/questions/buyers_organisations.rb index 1ab8a81e8..9c1cdf0e1 100644 --- a/app/models/form/sales/questions/buyers_organisations.rb +++ b/app/models/form/sales/questions/buyers_organisations.rb @@ -5,7 +5,7 @@ class Form::Sales::Questions::BuyersOrganisations < ::Form::Question @check_answer_label = "Organisations buyers were registered with" @header = "What organisations were the buyers registered with?" @type = "checkbox" - @hint_text = "Select all that apply" + @hint_text = "Select all that apply. This question is optional. If no options are applicable, leave the options blank, and select save and continue." @answer_options = ANSWER_OPTIONS @question_number = 59 end @@ -26,8 +26,4 @@ class Form::Sales::Questions::BuyersOrganisations < ::Form::Question "pregghb" => { "value" => "Help to Buy Agent" }, } end - - def unanswered_error_message - "At least one option must be selected of these four" - end end diff --git a/app/models/sales_log.rb b/app/models/sales_log.rb index b4eab2093..b17956b3a 100644 --- a/app/models/sales_log.rb +++ b/app/models/sales_log.rb @@ -78,7 +78,7 @@ class SalesLog < Log scope.pluck("ARRAY_AGG(id)") } - OPTIONAL_FIELDS = %w[purchid othtype].freeze + OPTIONAL_FIELDS = %w[purchid othtype buyers_organisations].freeze RETIREMENT_AGES = { "M" => 65, "F" => 60, "X" => 65 }.freeze DUPLICATE_LOG_ATTRIBUTES = %w[owning_organisation_id purchid saledate age1_known age1 sex1 ecstat1 postcode_full].freeze @@ -394,6 +394,13 @@ class SalesLog < Log has_mscharge&.zero? end + def no_buyer_organisation? + pregyrha&.zero? && + pregla&.zero? && + pregghb&.zero? && + pregother&.zero? + end + def buyers_age_for_old_persons_shared_ownership_invalid? return unless old_persons_shared_ownership? diff --git a/app/services/bulk_upload/sales/year2023/row_parser.rb b/app/services/bulk_upload/sales/year2023/row_parser.rb index a8f322828..506bc7456 100644 --- a/app/services/bulk_upload/sales/year2023/row_parser.rb +++ b/app/services/bulk_upload/sales/year2023/row_parser.rb @@ -469,7 +469,6 @@ class BulkUpload::Sales::Year2023::RowParser validate :validate_if_log_already_exists, on: :after_log, if: -> { FeatureToggle.bulk_upload_duplicate_log_check_enabled? } validate :validate_data_protection_answered, on: :after_log - validate :validate_buyers_organisations, on: :after_log def self.question_for_field(field) QUESTIONS[field] @@ -571,15 +570,6 @@ private end end - def validate_buyers_organisations - organisations_fields = %i[field_67 field_68 field_69 field_70] - if organisations_fields.all? { |field| attributes[field.to_s].blank? } - organisations_fields.each do |field| - errors.add(field, "At least one option must be selected of these four", category: :not_answered) - end - end - end - def prevtenbuy2 case field_72 when "R" @@ -864,6 +854,7 @@ private attributes["pregla"] = field_69 attributes["pregghb"] = field_70 attributes["pregother"] = field_68 + attributes["pregblank"] = no_buyer_organisation attributes["disabled"] = field_76 attributes["wheel"] = field_77 @@ -1130,6 +1121,10 @@ private 0 if field_63 == 1 end + def no_buyer_organisation + [field_67, field_68, field_69, field_70].all?(&:blank?) ? 1 : nil? + end + def block_log_creation! self.block_log_creation = true end diff --git a/app/services/bulk_upload/sales/year2024/row_parser.rb b/app/services/bulk_upload/sales/year2024/row_parser.rb index 70aaee49a..f5697135b 100644 --- a/app/services/bulk_upload/sales/year2024/row_parser.rb +++ b/app/services/bulk_upload/sales/year2024/row_parser.rb @@ -468,7 +468,6 @@ class BulkUpload::Sales::Year2024::RowParser validate :validate_address_fields, on: :after_log validate :validate_if_log_already_exists, on: :after_log, if: -> { FeatureToggle.bulk_upload_duplicate_log_check_enabled? } - validate :validate_buyers_organisations, on: :after_log validate :validate_nationality, on: :after_log validate :validate_buyer_2_nationality, on: :after_log @@ -566,15 +565,6 @@ class BulkUpload::Sales::Year2024::RowParser private - def validate_buyers_organisations - organisations_fields = %i[field_66 field_67 field_68 field_69] - if organisations_fields.all? { |field| attributes[field.to_s].blank? } - organisations_fields.each do |field| - errors.add(field, "At least one option must be selected of these four") - end - end - end - def prevtenbuy2 case field_71 when "R" @@ -862,6 +852,7 @@ private attributes["pregla"] = field_68 attributes["pregghb"] = field_69 attributes["pregother"] = field_67 + attributes["pregblank"] = no_buyer_organisation attributes["disabled"] = field_75 attributes["wheel"] = field_76 @@ -1136,6 +1127,10 @@ private end end + def no_buyer_organisation + [field_66, field_67, field_68, field_69].all?(&:blank?) ? 1 : nil? + end + def block_log_creation! self.block_log_creation = true end diff --git a/spec/models/form/sales/questions/buyers_organisations_spec.rb b/spec/models/form/sales/questions/buyers_organisations_spec.rb index 88af9917d..b0c932acc 100644 --- a/spec/models/form/sales/questions/buyers_organisations_spec.rb +++ b/spec/models/form/sales/questions/buyers_organisations_spec.rb @@ -32,11 +32,7 @@ RSpec.describe Form::Sales::Questions::BuyersOrganisations, type: :model do end it "has the correct hint" do - expect(question.hint_text).to eq("Select all that apply") - end - - it "has the correct unanswered_error_message" do - expect(question.unanswered_error_message).to eq("At least one option must be selected of these four") + expect(question.hint_text).to eq("Select all that apply. This question is optional. If no options are applicable, leave the options blank, and select save and continue.") end it "has the correct answer_options" do @@ -51,7 +47,7 @@ RSpec.describe Form::Sales::Questions::BuyersOrganisations, type: :model do ) end - it "has the correct displayed_answer_options" do + it "has the correct displayed answer_options" do expect(question.displayed_answer_options(FactoryBot.create(:sales_log))).to eq( { "pregyrha" => { "value" => "Their private registered provider (PRP) - housing association" }, diff --git a/spec/models/sales_log_spec.rb b/spec/models/sales_log_spec.rb index 00792b0bd..1ad889e84 100644 --- a/spec/models/sales_log_spec.rb +++ b/spec/models/sales_log_spec.rb @@ -69,6 +69,7 @@ RSpec.describe SalesLog, type: :model do expect(sales_log.optional_fields).to eq(%w[ purchid othtype + buyers_organisations proplen mortlen frombeds @@ -83,6 +84,7 @@ RSpec.describe SalesLog, type: :model do expect(sales_log.optional_fields).to eq(%w[ purchid othtype + buyers_organisations address_line2 county postcode_full @@ -596,6 +598,23 @@ RSpec.describe SalesLog, type: :model do expect { sales_log.update!(nationality_all_buyer2_group: nil) }.not_to change(sales_log, :nationality_all_buyer2) end end + + it "sets pregblank field when no buyer organisation is selected" do + expect { + sales_log.update!(pregyrha: 0, + pregla: 0, + pregghb: 0, + pregother: 0) + }.to change(sales_log, :pregblank).to 1 + end + + %i[pregyrha pregla pregghb pregother].each do |field| + it "does not set pregblank field when #{field} is selected" do + expect { + sales_log.update!({ field => 1 }) + }.not_to change(sales_log, :pregblank) + end + end end context "when saving addresses" do diff --git a/spec/services/bulk_upload/sales/year2023/row_parser_spec.rb b/spec/services/bulk_upload/sales/year2023/row_parser_spec.rb index c4f4a26a6..7fde3c99e 100644 --- a/spec/services/bulk_upload/sales/year2023/row_parser_spec.rb +++ b/spec/services/bulk_upload/sales/year2023/row_parser_spec.rb @@ -959,14 +959,24 @@ RSpec.describe BulkUpload::Sales::Year2023::RowParser do end describe "#field_67 - 70" do # buyers organisations - context "when all nil" do - let(:attributes) { setup_section_params.merge(field_67: nil, field_68: nil, field_69: nil, field_70: nil) } - - it "returns correct errors" do - expect(parser.errors[:field_67]).to be_present - expect(parser.errors[:field_68]).to be_present - expect(parser.errors[:field_69]).to be_present - expect(parser.errors[:field_70]).to be_present + let(:empty_organisation_params) { setup_section_params.merge(field_67: nil, field_68: nil, field_69: nil, field_70: nil) } + + context "when all empty" do + let(:attributes) { empty_organisation_params } + + it "sets pregblank field" do + expect(parser.log.pregblank).to be(1) + end + end + + %i[field_67 field_68 field_69 field_70].each do |field_number| + context "when #{field_number} present" do + let(:attributes) { empty_organisation_params.merge({ field_number => 1 }) } + + it "does not set pregblank field" do + attributes[:field_number] = 1 + expect(parser.log.pregblank).to be(0) + end end end end diff --git a/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb b/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb index 6a357f46a..b45dfe10d 100644 --- a/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb +++ b/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb @@ -965,15 +965,25 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do end end - describe "#field_66 - 70" do # buyers organisations - context "when all nil" do - let(:attributes) { setup_section_params.merge(field_66: nil, field_67: nil, field_68: nil, field_69: nil) } + describe "#field_66 - 69" do # buyers organisations + let(:empty_organisation_params) { setup_section_params.merge(field_66: nil, field_67: nil, field_68: nil, field_69: nil) } - it "returns correct errors" do - expect(parser.errors[:field_66]).to be_present - expect(parser.errors[:field_67]).to be_present - expect(parser.errors[:field_68]).to be_present - expect(parser.errors[:field_69]).to be_present + context "when all empty" do + let(:attributes) { empty_organisation_params } + + it "sets pregblank field" do + expect(parser.log.pregblank).to be(1) + end + end + + %i[field_66 field_67 field_68 field_69].each do |field_number| + context "when #{field_number} present" do + let(:attributes) { empty_organisation_params.merge({ field_number => 1 }) } + + it "does not set pregblank field" do + attributes[:field_number] = 1 + expect(parser.log.pregblank).to be(0) + end end end end