CLDC-4297: refactor and simplify

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

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["has_mscharge"]).to eq(1)
-            expect(log["mscharge"]).to be_nil
+            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["has_mscharge"]).to eq(1)
-            expect(log["mscharge"]).to be_nil
+            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["has_mscharge"]).to eq(1)
-            expect(log["mscharge"]).to be_nil
+            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["has_mscharge"]).to eq(1)
-            expect(log["mscharge"]).to be_nil
+            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["has_mscharge"]).to eq(1)
-            expect(log["mscharge"]).to be_nil
+            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["has_mscharge"]).to eq(1)
-            expect(log["mscharge"]).to be_nil
+            expect(log["mscharge"]).to eq(0)
           end
         end