Api 12293 remove extra validations (#9588)

* Removes extra validations from models.

* Adds tests to ensure that schema validations are covering the removal of the model validations.

* Removes comments, removes unnessecary test.

* Removes model level tests that check model validation of dates.

* Adds work on failing tests.

* Fixes tests.

* Adds tests for HLR v1 and NOD v1.

* Fixes test.

* Adds back line in test suite that was accidentally removed.

* Reverts change on test.

modules/appeals_api/app/models/appeals_api/higher_level_review.rb

@@ -37,9 +37,7 @@ def self.date_from_string(string)
     # the controller applies the JSON Schemas in modules/appeals_api/config/schemas/
     # further validations:
     validate(
-      :birth_date_is_a_date,
       :birth_date_is_in_the_past,
-      :contestable_issue_dates_are_valid_dates,
       :claimant_birth_date_is_in_the_past,
       :required_claimant_data_is_present,
       if: proc { |a| a.form_data.present? }
@@ -330,11 +328,6 @@ def veterans_timezone
       veteran_data&.dig('timezone').presence&.strip
     end
 
-    # validation (header)
-    def birth_date_is_a_date
-      add_error("Veteran birth date isn't a date: #{birth_date_string.inspect}") unless birth_date
-    end
-
     # validation (header)
     def birth_date_is_in_the_past
       return unless birth_date

modules/appeals_api/app/models/appeals_api/supplemental_claim.rb

@@ -35,9 +35,7 @@ def self.date_from_string(string)
     # the controller applies the JSON Schemas in modules/appeals_api/config/schemas/
     # further validations:
     validate(
-      :birth_date_is_a_date,
       :birth_date_is_in_the_past,
-      :contestable_issue_dates_are_valid_dates,
       if: proc { |a| a.form_data.present? }
     )
 
@@ -286,11 +284,6 @@ def veterans_timezone
       veteran&.dig('timezone').presence&.strip
     end
 
-    # validation (header)
-    def birth_date_is_a_date
-      add_error("Veteran birth date isn't a date: #{birth_date_string.inspect}") unless birth_date
-    end
-
     # validation (header)
     def birth_date_is_in_the_past
       return unless birth_date

modules/appeals_api/spec/models/higher_level_review_spec.rb

@@ -187,16 +187,6 @@
     end
     let(:api_version) { 'V1' }
 
-    context 'birth date isn\'t a date' do
-      let(:auth_headers) { default_auth_headers.merge 'X-VA-Birth-Date' => 'apricot' }
-
-      it 'using a birth date that isn\'t a date creates an invalid record' do
-        expect(higher_level_review.valid?).to be false
-        expect(higher_level_review.errors.to_a.length).to eq 1
-        expect(higher_level_review.errors.to_a.first.downcase).to include 'isn\'t a date'
-      end
-    end
-
     context 'birth date isn\'t in the past' do
       let(:auth_headers) { default_auth_headers.merge 'X-VA-Birth-Date' => (Time.zone.today + 2).to_s }
 
@@ -237,13 +227,6 @@
           ]
         }
       end
-
-      it 'bad decision dates will create an invalid record' do
-        expect(higher_level_review.valid?).to be false
-        expect(higher_level_review.errors.to_a.length).to eq 2
-        expect(higher_level_review.errors.to_a.first).to include 'decisionDate'
-        expect(higher_level_review.errors.to_a.second).to include 'decisionDate'
-      end
     end
 
     describe 'V2' do
@@ -474,8 +457,8 @@
     describe '#pdf_output_prep' do
       it 'clears memoized values' do
         expected = 'Smartquotes: “”‘’'
-        higher_level_review.form_data['included'][0]['attributes']['issue'] = expected
         expect(higher_level_review.contestable_issues[0].text).to eq 'tinnitus'
+        higher_level_review.form_data['included'][0]['attributes']['issue'] = expected
         higher_level_review.pdf_output_prep
         expect(higher_level_review.contestable_issues[0].text).to eq expected
       end

modules/appeals_api/spec/requests/v1/higher_level_reviews_controller_spec.rb

@@ -71,6 +71,29 @@ def base_path(path)
         expect(parsed['errors'][0]['detail']).to include('Informal conference rep will not fit on form')
       end
 
+      context 'returns 422 when birth date is not a date' do
+        it 'when given a string for the birth date ' do
+          headers = @minimum_required_headers
+          headers['X-VA-Birth-Date'] = 'apricot'
+
+          post(path, params: @data.to_json, headers: headers)
+          expect(response.status).to eq(422)
+          expect(parsed['errors']).to be_an Array
+        end
+      end
+
+      context 'returns 422 when decison date is not a date' do
+        it 'when given a string for the contestable issues decision date ' do
+          data = JSON.parse(@data)
+          data['included'][0]['attributes'].merge!('decisionDate' => 'banana')
+
+          post(path, params: data.to_json, headers: @minimum_required_headers)
+          expect(response.status).to eq(422)
+          expect(parsed['errors']).to be_an Array
+          expect(parsed['errors'][0]['detail']).to include(' did not match')
+        end
+      end
+
       it 'does not sunset in the next 30 days' do
         # Safety test. Will fail if the sunset_date is within 30 days. We got burned by this before,
         # so being heavy handed with it here.

modules/appeals_api/spec/requests/v1/notice_of_disagreements_controller_spec.rb

@@ -10,7 +10,7 @@ def base_path(path)
     "/services/appeals/v1/decision_reviews/#{path}"
   end
 
-  before(:all) do
+  before do
     @data = fixture_to_s 'valid_10182.json', version: 'v1'
     @minimum_valid_data = fixture_to_s 'valid_10182_minimum.json', version: 'v1'
     @invalid_data = fixture_to_s 'invalid_10182.json', version: 'v1'
@@ -92,6 +92,35 @@ def base_path(path)
       end
     end
 
+    context 'returns 422 when birth date is not a date' do
+      let(:error_content) do
+        { 'status' => 422, 'detail' => "'apricot' did not match the defined pattern" }
+      end
+
+      it 'when given a string for the birth date ' do
+        @headers.merge!('X-VA-Birth-Date' => 'apricot')
+        post(path, params: @data.to_json, headers: @headers)
+        expect(response.status).to eq(422)
+        expect(parsed['errors']).to be_an Array
+      end
+    end
+
+    context 'returns 422 when decison date is not a date' do
+      let(:error_content) do
+        { 'status' => 422, 'detail' => "'banana' did not fit within the defined length limits" }
+      end
+
+      it 'when given a string for the contestable issues decision date ' do
+        data = JSON.parse(@data)
+        data['included'][0]['attributes'].merge!('decisionDate' => 'banana')
+        post(path, params: data.to_json, headers: @headers)
+        expect(response.status).to eq(422)
+        expect(parsed['errors']).to be_an Array
+        expect(parsed['errors'][0]['title']).to include('Invalid')
+        expect(parsed['errors'][0]['detail']).to include("'banana' did not fit")
+      end
+    end
+
     context 'keeps track of which version of the api it is serving' do
       it 'V1' do
         path = base_path('notice_of_disagreements')

modules/appeals_api/spec/requests/v2/higher_level_reviews_controller_spec.rb

@@ -10,7 +10,7 @@ def base_path(path)
     "/services/appeals/v2/decision_reviews/#{path}"
   end
 
-  before(:all) do
+  before do
     @data = fixture_to_s 'valid_200996_minimum.json', version: 'v2'
     @data_extra = fixture_to_s 'valid_200996_extra.json', version: 'v2'
     @invalid_data = fixture_to_s 'invalid_200996.json', version: 'v2'
@@ -40,6 +40,7 @@ def base_path(path)
       it 'creates an HLR and persists the data' do
         post(path, params: @data, headers: @minimum_required_headers)
         expect(parsed['data']['type']).to eq('higherLevelReview')
+        expect(parsed['data']['attributes']['formData']['data']['attributes']['benefitType']).to eq('lifeInsurance')
         expect(parsed['data']['attributes']['status']).to eq('pending')
       end
     end
@@ -91,6 +92,30 @@ def base_path(path)
       end
     end
 
+    context 'returns 422 when birth date is not a date' do
+      it 'when given a string for the birth date ' do
+        headers = @minimum_required_headers
+        headers['X-VA-Birth-Date'] = 'apricot'
+
+        post(path, params: @data.to_json, headers: headers)
+        expect(response.status).to eq(422)
+        expect(parsed['errors']).to be_an Array
+      end
+    end
+
+    context 'returns 422 when decison date is not a date' do
+      it 'when given a string for the contestable issues decision date ' do
+        data = JSON.parse(@data)
+        data['included'][0]['attributes'].merge!('decisionDate' => 'banana')
+
+        post(path, params: data.to_json, headers: @minimum_required_headers)
+        expect(response.status).to eq(422)
+        expect(parsed['errors']).to be_an Array
+        expect(parsed['errors'][0]['title']).to include('Invalid pattern')
+        expect(parsed['errors'][0]['detail']).to include(' did not match the defined pattern')
+      end
+    end
+
     it 'create the job to build the PDF' do
       client_stub = instance_double('CentralMail::Service')
       faraday_response = instance_double('Faraday::Response')

modules/appeals_api/spec/requests/v2/notice_of_disagreements_controller_spec.rb

@@ -49,6 +49,36 @@ def base_path(path)
       end
     end
 
+    context 'returns 422 when birth date is not a date' do
+      let(:error_content) do
+        { 'status' => 422, 'detail' => "'apricot' did not match the defined pattern" }
+      end
+
+      it 'when given a string for the birth date ' do
+        @headers.merge!('X-VA-Birth-Date' => 'apricot')
+        post(path, params: @data.to_json, headers: @headers)
+        expect(response.status).to eq(422)
+        expect(parsed['errors']).to be_an Array
+      end
+    end
+
+    context 'returns 422 when decison date is not a date' do
+      let(:error_content) do
+        { 'status' => 422, 'detail' => "'banana' did not fit within the defined length limits" }
+      end
+
+      it 'when given a string for the contestable issues decision date ' do
+        data = JSON.parse(@max_data)
+        data['included'][0]['attributes'].merge!('decisionDate' => 'banana')
+
+        post(path, params: data.to_json, headers: @headers)
+        expect(response.status).to eq(422)
+        expect(parsed['errors']).to be_an Array
+        expect(parsed['errors'][0]['title']).to include('Invalid')
+        expect(parsed['errors'][0]['detail']).to include("'banana' did not fit")
+      end
+    end
+
     it 'errors when included issue text is too long' do
       mod_data = fixture_as_json 'valid_10182.json', version: 'v2'
       mod_data['included'][0]['attributes']['issue'] = Faker::Lorem.characters(number: 500)

modules/appeals_api/spec/requests/v2/supplemental_claims_controller_spec.rb

@@ -85,6 +85,37 @@ def base_path(path)
       end
     end
 
+    context 'returns 422 when birth date is not a date' do
+      let(:error_content) do
+        { 'status' => 422, 'detail' => "'apricot' did not match the defined pattern" }
+      end
+
+      it 'when given a string for the birth date ' do
+        headers.merge!({ 'X-VA-Birth-Date' => 'apricot' })
+
+        post(path, params: data.to_json, headers: headers)
+        expect(response.status).to eq(422)
+        expect(parsed['errors']).to be_an Array
+      end
+    end
+
+    context 'returns 422 when decison date is not a date' do
+      let(:error_content) do
+        { 'status' => 422, 'detail' => "'banana' did not match the defined pattern" }
+      end
+
+      it 'when given a string for the contestable issues decision date ' do
+        sc_data = JSON.parse(data)
+        sc_data['included'][0]['attributes'].merge!('decisionDate' => 'banana')
+
+        post(path, params: sc_data.to_json, headers: headers)
+        expect(response.status).to eq(422)
+        expect(parsed['errors']).to be_an Array
+        expect(parsed['errors'][0]['title']).to include('Invalid')
+        expect(parsed['errors'][0]['detail']).to include("'banana' did not fit within the defined length limits")
+      end
+    end
+
     context 'form5103Acknowledged' do
       context 'when benefitType = compensation' do
         it 'fails if form5103Acknowledged = false' do