From af29f0576ff57caed26887a80bde1c78407a4a23 Mon Sep 17 00:00:00 2001 From: Nat Dean-Lewis Date: Fri, 20 Mar 2026 11:46:31 +0000 Subject: [PATCH] CLDC-4297: refactor and simplify --- .../bulk_upload/sales/year2026/row_parser.rb | 40 +++++++++--- .../sales/year2026/row_parser_spec.rb | 64 ++++++++++--------- 2 files changed, 66 insertions(+), 38 deletions(-) diff --git a/app/services/bulk_upload/sales/year2026/row_parser.rb b/app/services/bulk_upload/sales/year2026/row_parser.rb index ec3959e53..c686fee44 100644 --- a/app/services/bulk_upload/sales/year2026/row_parser.rb +++ b/app/services/bulk_upload/sales/year2026/row_parser.rb @@ -956,9 +956,6 @@ private attributes["gender_same_as_sex6"] = field_68 attributes["gender_description6"] = field_69 - attributes["newservicecharges"] = field_126.to_d if field_126.present? && field_126 != "R" && field_126.match?(SERVICE_CHARGE_FORMAT) - attributes["hasservicechargeschanged"] = attributes["newservicecharges"].present? ? 1 : 2 if field_126.present? && field_126.match?(SERVICE_CHARGE_FORMAT) - attributes["relat2"] = relationship_from_is_partner(field_37) attributes["relat3"] = relationship_from_is_partner(field_47) attributes["relat4"] = relationship_from_is_partner(field_53) @@ -1032,8 +1029,6 @@ private attributes["cashdis"] = field_105 attributes["mrent"] = mrent - attributes["mscharge"] = mscharge.to_d if mscharge.present? && mscharge != "R" && mscharge.to_d.positive? - attributes["has_mscharge"] = attributes["mscharge"].present? ? 1 : 0 if mscharge.present? && mscharge.match?(SERVICE_CHARGE_FORMAT) attributes["grant"] = field_129 attributes["discount"] = field_130 @@ -1103,6 +1098,11 @@ private attributes["management_fee"] = field_108 attributes["has_management_fee"] = field_108.present? && field_108.positive? ? 1 : 0 + attributes["has_mscharge"] = has_mscharge_value + attributes["mscharge"] = mscharge_value + attributes["hasservicechargeschanged"] = hasservicechargeschanged_value + attributes["newservicecharges"] = newservicecharges_value + attributes end @@ -1268,6 +1268,30 @@ private field_136 if discounted_ownership? end + def has_mscharge_value + return unless mscharge.present? && mscharge.match?(SERVICE_CHARGE_FORMAT) + + mscharge.casecmp?("R") ? 0 : 1 + end + + def mscharge_value + return unless mscharge.present? && mscharge.match?(SERVICE_CHARGE_FORMAT) && !mscharge.casecmp?("R") + + mscharge.to_d + end + + def hasservicechargeschanged_value + return unless field_126.present? && field_126.match?(SERVICE_CHARGE_FORMAT) + + field_126.casecmp?("R") ? 2 : 1 + end + + def newservicecharges_value + return unless field_126.present? && field_126.match?(SERVICE_CHARGE_FORMAT) && !field_126.casecmp?("R") + + field_126.to_d + end + def mortlen return field_103 if shared_ownership? @@ -1400,12 +1424,12 @@ private def validate_service_charge_fields message = I18n.t("#{ERROR_BASE_KEY}.mscharge.invalid") - if shared_ownership_initial_purchase? && field_107.present? && (!field_107.match?(SERVICE_CHARGE_FORMAT) || (field_107 != "R" && field_107.to_d.zero?)) + if shared_ownership_initial_purchase? && field_107.present? && !field_107.match?(SERVICE_CHARGE_FORMAT) errors.add(:field_107, message) end if staircasing? - if field_125.present? && (!field_125.match?(SERVICE_CHARGE_FORMAT) || (field_125 != "R" && field_125.to_d.zero?)) + if field_125.present? && !field_125.match?(SERVICE_CHARGE_FORMAT) errors.add(:field_125, message) end @@ -1414,7 +1438,7 @@ private end end - if discounted_ownership? && field_136.present? && (!field_136.match?(SERVICE_CHARGE_FORMAT) || (field_136 != "R" && field_136.to_d.zero?)) + if discounted_ownership? && field_136.present? && !field_136.match?(SERVICE_CHARGE_FORMAT) errors.add(:field_136, message) end end diff --git a/spec/services/bulk_upload/sales/year2026/row_parser_spec.rb b/spec/services/bulk_upload/sales/year2026/row_parser_spec.rb index 988869798..4a81c12d8 100644 --- a/spec/services/bulk_upload/sales/year2026/row_parser_spec.rb +++ b/spec/services/bulk_upload/sales/year2026/row_parser_spec.rb @@ -1979,30 +1979,32 @@ RSpec.describe BulkUpload::Sales::Year2026::RowParser do context "when set to 0" do let(:attributes) { valid_attributes.merge(field_10: "2", field_107: "0") } - it "adds a validation error" do + it "does not add a bulk upload format validation error but adds a site validation error" do parser.valid? - expect(parser.errors[:field_107]).to include(I18n.t("validations.sales.2026.bulk_upload.mscharge.invalid")) + expect(parser.errors[:field_107]).not_to include(I18n.t("validations.sales.2026.bulk_upload.mscharge.invalid")) + expect(parser.errors[:field_107]).to include(I18n.t("validations.sales.financial.mscharge.monthly_leasehold_charges.not_zero")) end - it "sets has_mscharge to no and does not set mscharge" do + it "sets has_mscharge to yes and mscharge to the value" do log = parser.log - expect(log["has_mscharge"]).to eq(0) - expect(log["mscharge"]).to be_nil + expect(log["has_mscharge"]).to eq(1) + expect(log["mscharge"]).to eq(0) end end context "when set to 0.0" do let(:attributes) { valid_attributes.merge(field_10: "2", field_107: "0.0") } - it "adds a validation error" do + it "does not add a bulk upload format validation error but adds a site validation error" do parser.valid? - expect(parser.errors[:field_107]).to include(I18n.t("validations.sales.2026.bulk_upload.mscharge.invalid")) + expect(parser.errors[:field_107]).not_to include(I18n.t("validations.sales.2026.bulk_upload.mscharge.invalid")) + expect(parser.errors[:field_107]).to include(I18n.t("validations.sales.financial.mscharge.monthly_leasehold_charges.not_zero")) end - it "sets has_mscharge to no and does not set mscharge" do + it "sets has_mscharge to yes and mscharge to the value" do log = parser.log - expect(log["has_mscharge"]).to eq(0) - expect(log["mscharge"]).to be_nil + expect(log["has_mscharge"]).to eq(1) + expect(log["mscharge"]).to eq(0) end end @@ -2106,30 +2108,32 @@ RSpec.describe BulkUpload::Sales::Year2026::RowParser do context "when set to 0" do let(:attributes) { valid_attributes.merge(field_125: "0") } - it "adds a validation error" do + it "does not add a bulk upload format validation error but adds a site validation error" do parser.valid? - expect(parser.errors[:field_125]).to include(I18n.t("validations.sales.2026.bulk_upload.mscharge.invalid")) + expect(parser.errors[:field_125]).not_to include(I18n.t("validations.sales.2026.bulk_upload.mscharge.invalid")) + expect(parser.errors[:field_125]).to include(I18n.t("validations.sales.financial.mscharge.monthly_leasehold_charges.not_zero")) end - it "sets has_mscharge to no and does not set mscharge" do + it "sets has_mscharge to yes and mscharge to the value" do log = parser.log - expect(log["has_mscharge"]).to eq(0) - expect(log["mscharge"]).to be_nil + expect(log["has_mscharge"]).to eq(1) + expect(log["mscharge"]).to eq(0) end end context "when set to 0.0" do let(:attributes) { valid_attributes.merge(field_125: "0.0") } - it "adds a validation error" do + it "does not add a bulk upload format validation error but adds a site validation error" do parser.valid? - expect(parser.errors[:field_125]).to include(I18n.t("validations.sales.2026.bulk_upload.mscharge.invalid")) + expect(parser.errors[:field_125]).not_to include(I18n.t("validations.sales.2026.bulk_upload.mscharge.invalid")) + expect(parser.errors[:field_125]).to include(I18n.t("validations.sales.financial.mscharge.monthly_leasehold_charges.not_zero")) end - it "sets has_mscharge to no and does not set mscharge" do + it "sets has_mscharge to yes and mscharge to the value" do log = parser.log - expect(log["has_mscharge"]).to eq(0) - expect(log["mscharge"]).to be_nil + expect(log["has_mscharge"]).to eq(1) + expect(log["mscharge"]).to eq(0) end end @@ -2233,30 +2237,30 @@ RSpec.describe BulkUpload::Sales::Year2026::RowParser do context "when set to 0" do let(:attributes) { valid_attributes.merge(field_8: "2", field_136: "0") } - it "adds a validation error" do + it "does not add a validation error" do parser.valid? - expect(parser.errors[:field_136]).to include(I18n.t("validations.sales.2026.bulk_upload.mscharge.invalid")) + expect(parser.errors[:field_136]).to be_blank end - it "sets has_mscharge to no and does not set mscharge" do + it "sets has_mscharge to yes and mscharge to the value" do log = parser.log - expect(log["has_mscharge"]).to eq(0) - expect(log["mscharge"]).to be_nil + expect(log["has_mscharge"]).to eq(1) + expect(log["mscharge"]).to eq(0) end end context "when set to 0.0" do let(:attributes) { valid_attributes.merge(field_8: "2", field_136: "0.0") } - it "adds a validation error" do + it "does not add a validation error" do parser.valid? - expect(parser.errors[:field_136]).to include(I18n.t("validations.sales.2026.bulk_upload.mscharge.invalid")) + expect(parser.errors[:field_136]).to be_blank end - it "sets has_mscharge to no and does not set mscharge" do + it "sets has_mscharge to yes and mscharge to the value" do log = parser.log - expect(log["has_mscharge"]).to eq(0) - expect(log["mscharge"]).to be_nil + expect(log["has_mscharge"]).to eq(1) + expect(log["mscharge"]).to eq(0) end end