From 737702c1af4f0c909c88dfe757acc85ab2d2e19f Mon Sep 17 00:00:00 2001
From: Phil Lee <asmega@users.noreply.github.com>
Date: Thu, 5 Jan 2023 09:29:58 +0000
Subject: [PATCH] move validation from parser to model

---
 .../bulk_upload/lettings/row_parser.rb        | 36 ++++------
 .../bulk_upload/lettings/row_parser_spec.rb   | 72 ++++---------------
 2 files changed, 26 insertions(+), 82 deletions(-)

diff --git a/app/services/bulk_upload/lettings/row_parser.rb b/app/services/bulk_upload/lettings/row_parser.rb
index 23d037416..e2831b799 100644
--- a/app/services/bulk_upload/lettings/row_parser.rb
+++ b/app/services/bulk_upload/lettings/row_parser.rb
@@ -137,19 +137,25 @@ class BulkUpload::Lettings::RowParser
   attribute :field_133, :integer
   attribute :field_134, :integer
 
-  validates :field_1, presence: true, numericality: { in: (1..12) }
-  validates :field_4, numericality: { in: (1..999), allow_blank: true }
-  validates :field_4, presence: true, if: :field_4_presence_check
+  def valid?
+    log.valid?
 
-  validate :validate_possible_answers
+    errors.clear
 
-# delegate :valid?, to: :native_object
-# delegate :errors, to: :native_object
+    log.errors.each do |error|
+      field = field_for_attribute(error.attribute)
+      errors.add(field, error.type)
+    end
+  end
 
 private
 
-  def native_object
-    @native_object ||= LettingsLog.new(attributes_for_log)
+  def log
+    @log ||= LettingsLog.new(attributes_for_log)
+  end
+
+  def field_for_attribute(attribute)
+    field_mapping.invert[attribute]
   end
 
   def field_mapping
@@ -158,16 +164,6 @@ private
     }
   end
 
-  def validate_possible_answers
-    field_mapping.each do |field, attribute|
-      possible_answers = FormHandler.instance.current_lettings_form.questions.find { |q| q.id == attribute.to_s }.answer_options.keys
-
-      unless possible_answers.include?(public_send(field))
-        errors.add(field, "Value supplied is not one of the permitted values")
-      end
-    end
-  end
-
   def attributes_for_log
     hash = field_mapping.invert
     attributes = {}
@@ -178,8 +174,4 @@ private
 
     attributes
   end
-
-  def field_4_presence_check
-    [1, 3, 5, 7, 9, 11].include?(field_1)
-  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 16b65fab7..50a9e8772 100644
--- a/spec/services/bulk_upload/lettings/row_parser_spec.rb
+++ b/spec/services/bulk_upload/lettings/row_parser_spec.rb
@@ -3,77 +3,29 @@ require "rails_helper"
 RSpec.describe BulkUpload::Lettings::RowParser do
   subject(:parser) { described_class.new(attributes) }
 
+  let(:attributes) { {} }
+
   describe "validations" do
     before do
       parser.valid?
     end
 
-    describe "field_1" do
-      context "when null" do
-        let(:attributes) { { field_1: nil } }
-
-        it "returns an error" do
-          expect(parser.errors).to include(:field_1)
-        end
-      end
-
-      context "when outside permited range" do
-        let(:attributes) { { field_1: "13" } }
-
-        it "returns an error" do
-          expect(parser.errors).to include(:field_1)
-        end
-      end
-
-      context "when valid" do
-        let(:attributes) { { field_1: 1 } }
-
-        it "is valid" do
-          expect(parser.errors).not_to include(:field_1)
-        end
-      end
-    end
-
-    describe "field_4" do
-      context "when text" do
-        let(:attributes) { { field_4: "R" } }
-
-        it "is not valid" do
-          expect(parser.errors).to include(:field_4)
-        end
-      end
-
-      context "when valid" do
-        let(:attributes) { { field_4: "3" } }
-
-        it "is valid" do
-          expect(parser.errors).not_to include(:field_4)
-        end
-      end
-
-      context "when allowed to be null" do
-        let(:attributes) { { field_1: "2", field_4: "" } }
-
-        it "is valid" do
-          expect(parser.errors).not_to include(:field_4)
-        end
-      end
-
-      context "when not allowed to be null" do
-        let(:attributes) { { field_1: "3", field_4: "" } }
+    describe "#valid?" do
+      let(:attributes) { { field_134: 3 } }
 
-        it "is not valid" do
-          expect(parser.errors).to include(:field_4)
+      context "when calling the method multiple times" do
+        it "does not add keep adding errors to the pile" do
+          expect { parser.valid? }.not_to change(parser.errors, :count)
         end
       end
     end
 
-    describe "#field_134" do
-      context "when not a possible value" do
-        let(:attributes) { { field_134: "3" } }
+    describe "field_134" do
+      context "when an unpermitted value" do
+        let(:attributes) { { field_134: 3 } }
 
-        it "is not valid" do
-          expect(parser.errors).to include(:field_134)
+        it "has errors on the field" do
+          expect(parser.errors[:field_134]).to be_present
         end
       end
     end