From f582e99931d9c7bb12769a749eaa4cf0602bb0bb Mon Sep 17 00:00:00 2001 From: Nat Dean-Lewis Date: Thu, 19 Mar 2026 11:24:53 +0000 Subject: [PATCH] CLDC-4297: don't allow 0 for mscharge and update tests --- .../bulk_upload/sales/year2026/row_parser.rb | 6 +- .../validations/sales/2026/bulk_upload.en.yml | 2 +- .../sales/year2026/row_parser_spec.rb | 80 +++++++++++++++++++ 3 files changed, 84 insertions(+), 4 deletions(-) diff --git a/app/services/bulk_upload/sales/year2026/row_parser.rb b/app/services/bulk_upload/sales/year2026/row_parser.rb index 149774dbd..b69313814 100644 --- a/app/services/bulk_upload/sales/year2026/row_parser.rb +++ b/app/services/bulk_upload/sales/year2026/row_parser.rb @@ -1400,12 +1400,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) + if shared_ownership_initial_purchase? && field_107.present? && (!field_107.match?(SERVICE_CHARGE_FORMAT) || field_107.to_d.zero?) errors.add(:field_107, message) end if staircasing? - if field_125.present? && !field_125.match?(SERVICE_CHARGE_FORMAT) + if field_125.present? && (!field_125.match?(SERVICE_CHARGE_FORMAT) || field_125.to_d.zero?) errors.add(:field_125, message) end @@ -1414,7 +1414,7 @@ private end end - if discounted_ownership? && field_136.present? && !field_136.match?(SERVICE_CHARGE_FORMAT) + if discounted_ownership? && field_136.present? && (!field_136.match?(SERVICE_CHARGE_FORMAT) || field_136.to_d.zero?) errors.add(:field_136, message) end end diff --git a/config/locales/validations/sales/2026/bulk_upload.en.yml b/config/locales/validations/sales/2026/bulk_upload.en.yml index 7ba1ad646..6d659889a 100644 --- a/config/locales/validations/sales/2026/bulk_upload.en.yml +++ b/config/locales/validations/sales/2026/bulk_upload.en.yml @@ -45,7 +45,7 @@ en: nationality: invalid: "Select a valid nationality." mscharge: - invalid: "Service charge must be a number or the letter R." + invalid: "Service charge must be a positive number or the letter R." newservicecharges: invalid: "New service charge must be a number or the letter R." mortlen: 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 0861c42c5..479091d21 100644 --- a/spec/services/bulk_upload/sales/year2026/row_parser_spec.rb +++ b/spec/services/bulk_upload/sales/year2026/row_parser_spec.rb @@ -1949,6 +1949,11 @@ RSpec.describe BulkUpload::Sales::Year2026::RowParser do context "when positive" do let(:attributes) { valid_attributes.merge(field_10: "2", field_107: "100") } + it "does not add a validation error" do + parser.valid? + expect(parser.errors[:field_107]).to be_blank + end + it "sets has_mscharge to yes and mscharge to the value" do log = parser.log expect(log["has_mscharge"]).to eq(1) @@ -1956,9 +1961,29 @@ RSpec.describe BulkUpload::Sales::Year2026::RowParser do end end + context "when set to 1" do + let(:attributes) { valid_attributes.merge(field_10: "2", field_107: "1") } + + it "does not add a validation error" do + parser.valid? + expect(parser.errors[:field_107]).to be_blank + end + + it "sets has_mscharge to yes and mscharge to the value" do + log = parser.log + expect(log["has_mscharge"]).to eq(1) + expect(log["mscharge"]).to eq(1) + end + end + context "when set to 0" do let(:attributes) { valid_attributes.merge(field_10: "2", field_107: "0") } + it "adds a validation error" do + parser.valid? + expect(parser.errors[:field_107]).to include(I18n.t("validations.sales.2026.bulk_upload.mscharge.invalid")) + end + it "sets has_mscharge to no and does not set mscharge" do log = parser.log expect(log["has_mscharge"]).to eq(0) @@ -2016,6 +2041,11 @@ RSpec.describe BulkUpload::Sales::Year2026::RowParser do context "when positive" do let(:attributes) { valid_attributes.merge(field_125: "100") } + it "does not add a validation error" do + parser.valid? + expect(parser.errors[:field_125]).to be_blank + end + it "sets has_mscharge to yes and mscharge to the value" do log = parser.log expect(log["has_mscharge"]).to eq(1) @@ -2023,9 +2053,29 @@ RSpec.describe BulkUpload::Sales::Year2026::RowParser do end end + context "when set to 1" do + let(:attributes) { valid_attributes.merge(field_125: "1") } + + it "does not add a validation error" do + parser.valid? + expect(parser.errors[:field_125]).to be_blank + end + + it "sets has_mscharge to yes and mscharge to the value" do + log = parser.log + expect(log["has_mscharge"]).to eq(1) + expect(log["mscharge"]).to eq(1) + end + end + context "when set to 0" do let(:attributes) { valid_attributes.merge(field_125: "0") } + it "adds a validation error" do + parser.valid? + expect(parser.errors[:field_125]).to include(I18n.t("validations.sales.2026.bulk_upload.mscharge.invalid")) + end + it "sets has_mscharge to no and does not set mscharge" do log = parser.log expect(log["has_mscharge"]).to eq(0) @@ -2083,6 +2133,11 @@ RSpec.describe BulkUpload::Sales::Year2026::RowParser do context "when positive" do let(:attributes) { valid_attributes.merge(field_8: "2", field_136: "100") } + it "does not add a validation error" do + parser.valid? + expect(parser.errors[:field_136]).to be_blank + end + it "sets has_mscharge to yes and mscharge to the value" do log = parser.log expect(log["has_mscharge"]).to eq(1) @@ -2090,9 +2145,29 @@ RSpec.describe BulkUpload::Sales::Year2026::RowParser do end end + context "when set to 1" do + let(:attributes) { valid_attributes.merge(field_8: "2", field_136: "1") } + + it "does not add a validation error" do + parser.valid? + expect(parser.errors[:field_136]).to be_blank + end + + it "sets has_mscharge to yes and mscharge to the value" do + log = parser.log + expect(log["has_mscharge"]).to eq(1) + expect(log["mscharge"]).to eq(1) + end + end + context "when set to 0" do let(:attributes) { valid_attributes.merge(field_8: "2", field_136: "0") } + it "adds a validation error" do + parser.valid? + expect(parser.errors[:field_136]).to include(I18n.t("validations.sales.2026.bulk_upload.mscharge.invalid")) + end + it "sets has_mscharge to no and does not set mscharge" do log = parser.log expect(log["has_mscharge"]).to eq(0) @@ -2150,6 +2225,11 @@ RSpec.describe BulkUpload::Sales::Year2026::RowParser do context "when positive" do let(:attributes) { valid_attributes.merge(field_126: "150") } + it "does not add a validation error" do + parser.valid? + expect(parser.errors[:field_126]).to be_blank + end + it "sets hasservicechargeschanged to yes and newservicecharges to the value" do log = parser.log expect(log["hasservicechargeschanged"]).to eq(1)