Remove feature flipper for JSON schemer and always use it (#20271)

* Remove feature flipper for JSON schemer and always use it

* Rubocop:

* Fix spec

* Rubocop

* Fixes spec

* rubocop

app/models/saved_claim.rb

@@ -150,66 +150,20 @@ def va_notification?(email_template_id)
 
   private
 
-  # Depending on the feature flipper, validate the *entire schema*
-  # via either the json_schema or json_schemer gem.
-  # This is tied to vets-api #19684
   def validate_schema(schema)
-    if Flipper.enabled?(:validate_saved_claims_with_json_schemer)
-      validate_schema_with_json_schemer(schema)
-    else
-      validate_schema_with_json_schema(schema)
-    end
-  end
-
-  # Depending on the feature flipper, validate the *parsed form*
-  # via either the json_schema or the json_schemer gem.
-  # This is tied to vets-api #19684
-  def validate_form(schema, clear_cache)
-    if Flipper.enabled?(:validate_saved_claims_with_json_schemer)
-      validate_form_with_json_schemer(schema)
-    else
-      validate_form_with_json_schema(schema, clear_cache)
-    end
-  end
-
-  # For json_schemer, the default behavior is not to raise an exception
-  # on validation, so we return an array of errors if they exist.
-  # This method validates the *entire schema*.
-  def validate_schema_with_json_schemer(schema)
     errors = JSONSchemer.validate_schema(schema).to_a
     return [] if errors.empty?
 
     reformatted_schemer_errors(errors)
   end
 
-  # For json_schema, validation errors raise an exception.
-  # This method validates the *entire schema*.
-  def validate_schema_with_json_schema(schema)
-    JSON::Validator.fully_validate_schema(schema, { errors_as_objects: true })
-  rescue => e
-    Rails.logger.error('Error during schema validation!', { error: e.message, backtrace: e.backtrace, schema: })
-    raise
-  end
-
-  # This method validates the *parsed form* with json_schemer.
-  def validate_form_with_json_schemer(schema)
+  def validate_form(schema, _clear_cache)
     errors = JSONSchemer.schema(schema).validate(parsed_form).to_a
     return [] if errors.empty?
 
     reformatted_schemer_errors(errors)
   end
 
-  # This method validates the *parsed form* with json_schema.
-  def validate_form_with_json_schema(schema, clear_cache)
-    JSON::Validator.fully_validate(schema, parsed_form, { errors_as_objects: true, clear_cache: })
-  rescue => e
-    PersonalInformationLog.create(data: { schema:, parsed_form:, params: { errors_as_objects: true, clear_cache: } },
-                                  error_class: 'SavedClaim FormValidationError')
-    Rails.logger.error('Error during form validation!',
-                       { error: e.message, backtrace: e.backtrace, schema:, clear_cache: })
-    raise
-  end
-
   # This method exists to change the json_schemer errors
   # to be formatted like json_schema errors, which keeps
   # the error logging smooth and identical for both options.

config/features.yml

@@ -1545,10 +1545,6 @@ features:
     actor_type: user
     description: When enabled, the VAProfile::V3::ContactInformation will be enabled
     enable_in_development: true
-  validate_saved_claims_with_json_schemer:
-    actor_type: user
-    description: When enabled, Saved Claims will be validated using the JSON Schemer gem rather than JSON Schema
-    enable_in_development: false
   veteran_onboarding_beta_flow:
     actor_type: user
     description: Conditionally display the new veteran onboarding flow to user

modules/pensions/spec/models/pensions/saved_claim_spec.rb

@@ -49,32 +49,7 @@
       )
     end
 
-    context 'using JSON Schema' do
-      before do
-        allow(Flipper).to receive(:enabled?).with(:validate_saved_claims_with_json_schemer).and_return(false)
-      end
-
-      describe '#process_attachments!' do
-        it 'sets the attachments saved_claim_id' do
-          expect(Lighthouse::SubmitBenefitsIntakeClaim).not_to receive(:perform_async).with(claim.id)
-          claim.process_attachments!
-          expect(claim.persistent_attachments.size).to eq(2)
-        end
-      end
-
-      describe '#destroy' do
-        it 'also destroys the persistent_attachments' do
-          claim.process_attachments!
-          expect { claim.destroy }.to change(PersistentAttachment, :count).by(-2)
-        end
-      end
-    end
-
     context 'using JSON Schemer' do
-      before do
-        allow(Flipper).to receive(:enabled?).with(:validate_saved_claims_with_json_schemer).and_return(true)
-      end
-
       describe '#process_attachments!' do
         it 'sets the attachments saved_claim_id' do
           expect(Lighthouse::SubmitBenefitsIntakeClaim).not_to receive(:perform_async).with(claim.id)

spec/lib/sidekiq/form526_backup_submission_process/submit_spec.rb

@@ -12,7 +12,6 @@
   before do
     Sidekiq::Job.clear_all
     Flipper.disable(ApiProviderFactory::FEATURE_TOGGLE_GENERATE_PDF)
-    allow(Flipper).to receive(:enabled?).with(:validate_saved_claims_with_json_schemer).and_return(false)
     allow(Flipper).to receive(:enabled?).with(:form526_send_backup_submission_exhaustion_email_notice).and_return(false)
   end
 
@@ -106,119 +105,110 @@
   end
 
   %w[single multi].each do |payload_method|
-    [true, false].each do |flipper|
-      describe ".perform_async, enabled, #{payload_method} payload", skip: 'Flakey test' do
-        before do
-          allow(Settings.form526_backup).to receive_messages(submission_method: payload_method, enabled: true)
-        end
+    describe ".perform_async, enabled, #{payload_method} payload" do
+      before do
+        allow(Settings.form526_backup).to receive_messages(submission_method: payload_method, enabled: true)
+      end
 
-        let!(:submission) { create(:form526_submission, :with_everything) }
-        let!(:upload_data) { submission.form[Form526Submission::FORM_526_UPLOADS] }
+      let!(:submission) { create(:form526_submission, :with_everything) }
+      let!(:upload_data) { submission.form[Form526Submission::FORM_526_UPLOADS] }
 
-        context "when json_schemer flipper is #{flipper}" do
-          before do
-            allow(Flipper).to receive(:enabled?).with(:validate_saved_claims_with_json_schemer).and_return(flipper)
+      context 'successfully' do
+        before do
+          upload_data.each do |ud|
+            file = Rack::Test::UploadedFile.new('spec/fixtures/files/doctors-note.pdf', 'application/pdf')
+            sea = SupportingEvidenceAttachment.find_or_create_by(guid: ud['confirmationCode'])
+            sea.set_file_data!(file)
+            sea.save!
           end
+        end
 
-          context 'successfully' do
-            before do
-              upload_data.each do |ud|
-                file = Rack::Test::UploadedFile.new('spec/fixtures/files/doctors-note.pdf', 'application/pdf')
-                sea = SupportingEvidenceAttachment.find_or_create_by(guid: ud['confirmationCode'])
-                sea.set_file_data!(file)
-                sea.save!
-              end
-            end
-
-            it 'creates a job for submission' do
-              expect { subject.perform_async(submission.id) }.to change(subject.jobs, :size).by(1)
-            end
+        it 'creates a job for submission' do
+          expect { subject.perform_async(submission.id) }.to change(subject.jobs, :size).by(1)
+        end
 
-            it 'submits' do
-              new_form_data = submission.saved_claim.parsed_form
-              new_form_data['startedFormVersion'] = nil
-              submission.saved_claim.form = new_form_data.to_json
-              submission.saved_claim.save
-              VCR.use_cassette('lighthouse/benefits_intake/200_lighthouse_intake_upload_location') do
-                VCR.use_cassette('form526_backup/200_evss_get_pdf') do
-                  VCR.use_cassette('lighthouse/benefits_intake/200_lighthouse_intake_upload') do
-                    jid = subject.perform_async(submission.id)
-                    last = subject.jobs.last
-                    jid_from_jobs = last['jid']
-                    expect(jid).to eq(jid_from_jobs)
-                    described_class.drain
-                    expect(jid).not_to be_empty
-
-                    # The Backup Submission process gathers form 526 and any ancillary forms
-                    # to send to Central Mail at the same time
-
-                    # Form 4142 Backup Submission Process
-                    expect(submission.form['form4142']).not_to be_nil
-                    form4142_processor = DecisionReviewV1::Processor::Form4142Processor.new(
-                      form_data: submission.form['form4142'], submission_id: submission.id
-                    )
-                    request_body = form4142_processor.request_body
-                    metadata_hash = JSON.parse(request_body['metadata'])
-                    form4142_received_date = metadata_hash['receiveDt'].in_time_zone('Central Time (US & Canada)')
-                    expect(
-                      submission.created_at.in_time_zone('Central Time (US & Canada)')
-                    ).to be_within(1.second).of(form4142_received_date)
-
-                    # Form 0781 Backup Submission Process
-                    expect(submission.form['form0781']).not_to be_nil
-                    # not really a way to test the dates here
-
-                    job_status = Form526JobStatus.last
-                    expect(job_status.form526_submission_id).to eq(submission.id)
-                    expect(job_status.job_class).to eq('BackupSubmission')
-                    expect(job_status.job_id).to eq(jid)
-                    expect(job_status.status).to eq('success')
-                    submission = Form526Submission.last
-                    expect(submission.backup_submitted_claim_id).not_to be_nil
-                    expect(submission.submit_endpoint).to eq('benefits_intake_api')
-                  end
-                end
+        it 'submits' do
+          new_form_data = submission.saved_claim.parsed_form
+          new_form_data['startedFormVersion'] = nil
+          submission.saved_claim.form = new_form_data.to_json
+          submission.saved_claim.save
+          VCR.use_cassette('lighthouse/benefits_intake/200_lighthouse_intake_upload_location') do
+            VCR.use_cassette('form526_backup/200_evss_get_pdf') do
+              VCR.use_cassette('lighthouse/benefits_intake/200_lighthouse_intake_upload') do
+                jid = subject.perform_async(submission.id)
+                last = subject.jobs.last
+                jid_from_jobs = last['jid']
+                expect(jid).to eq(jid_from_jobs)
+                described_class.drain
+                expect(jid).not_to be_empty
+                # The Backup Submission process gathers form 526 and any ancillary forms
+                # to send to Central Mail at the same time
+
+                # Form 4142 Backup Submission Process
+                expect(submission.form['form4142']).not_to be_nil
+                form4142_processor = DecisionReviewV1::Processor::Form4142Processor.new(
+                  form_data: submission.form['form4142'], submission_id: submission.id
+                )
+                request_body = form4142_processor.request_body
+                metadata_hash = JSON.parse(request_body['metadata'])
+                form4142_received_date = metadata_hash['receiveDt'].in_time_zone('Central Time (US & Canada)')
+                expect(
+                  submission.created_at.in_time_zone('Central Time (US & Canada)')
+                ).to be_within(1.second).of(form4142_received_date)
+
+                # Form 0781 Backup Submission Process
+                expect(submission.form['form0781']).not_to be_nil
+                # not really a way to test the dates here
+
+                job_status = Form526JobStatus.last
+                expect(job_status.form526_submission_id).to eq(submission.id)
+                expect(job_status.job_class).to eq('BackupSubmission')
+                expect(job_status.job_id).to eq(jid)
+                expect(job_status.status).to eq('success')
+                submission = Form526Submission.last
+                expect(submission.backup_submitted_claim_id).not_to be_nil
+                expect(submission.submit_endpoint).to eq('benefits_intake_api')
               end
             end
           end
+        end
+      end
 
-          context 'with a submission timeout' do
-            before do
-              allow_any_instance_of(Faraday::Connection).to receive(:post).and_raise(Faraday::TimeoutError)
-            end
+      context 'with a submission timeout' do
+        before do
+          allow_any_instance_of(Faraday::Connection).to receive(:post).and_raise(Faraday::TimeoutError)
+        end
 
-            it 'raises a gateway timeout error' do
-              jid = subject.perform_async(submission.id)
-              expect { described_class.drain }.to raise_error(Common::Exceptions::GatewayTimeout)
-              job_status = Form526JobStatus.find_by(job_id: jid)
-              expect(job_status.form526_submission_id).to eq(submission.id)
-              expect(job_status.job_class).to eq('BackupSubmission')
-              expect(job_status.job_id).to eq(jid)
-              expect(job_status.status).to eq('retryable_error')
-              error = job_status.bgjob_errors
-              expect(error.first.last['error_class']).to eq('Common::Exceptions::GatewayTimeout')
-              expect(error.first.last['error_message']).to eq('Gateway timeout')
-            end
-          end
+        it 'raises a gateway timeout error' do
+          jid = subject.perform_async(submission.id)
+          expect { described_class.drain }.to raise_error(Common::Exceptions::GatewayTimeout)
+          job_status = Form526JobStatus.find_by(job_id: jid)
+          expect(job_status.form526_submission_id).to eq(submission.id)
+          expect(job_status.job_class).to eq('BackupSubmission')
+          expect(job_status.job_id).to eq(jid)
+          expect(job_status.status).to eq('retryable_error')
+          error = job_status.bgjob_errors
+          expect(error.first.last['error_class']).to eq('Common::Exceptions::GatewayTimeout')
+          expect(error.first.last['error_message']).to eq('Gateway timeout')
+        end
+      end
 
-          context 'with an unexpected error' do
-            before do
-              allow_any_instance_of(Faraday::Connection).to receive(:post).and_raise(StandardError.new('foo'))
-            end
+      context 'with an unexpected error' do
+        before do
+          allow_any_instance_of(Faraday::Connection).to receive(:post).and_raise(StandardError.new('foo'))
+        end
 
-            it 'raises a standard error' do
-              jid = subject.perform_async(submission.id)
-              expect { described_class.drain }.to raise_error(StandardError)
-              job_status = Form526JobStatus.find_by(job_id: jid)
-              expect(job_status.form526_submission_id).to eq(submission.id)
-              expect(job_status.job_class).to eq('BackupSubmission')
-              expect(job_status.job_id).to eq(jid)
-              expect(job_status.status).to eq('retryable_error')
-              error = job_status.bgjob_errors
-              expect(error.first.last['error_class']).to eq('StandardError')
-              expect(error.first.last['error_message']).to eq('foo')
-            end
-          end
+        it 'raises a standard error' do
+          jid = subject.perform_async(submission.id)
+          expect { described_class.drain }.to raise_error(StandardError)
+          job_status = Form526JobStatus.find_by(job_id: jid)
+          expect(job_status.form526_submission_id).to eq(submission.id)
+          expect(job_status.job_class).to eq('BackupSubmission')
+          expect(job_status.job_id).to eq(jid)
+          expect(job_status.status).to eq('retryable_error')
+          error = job_status.bgjob_errors
+          expect(error.first.last['error_class']).to eq('StandardError')
+          expect(error.first.last['error_message']).to eq('foo')
         end
       end
     end

spec/lib/vre/monitor_spec.rb

@@ -28,42 +28,34 @@
   let(:encrypted_user) { KmsEncrypted::Box.new.encrypt(user_struct.to_h.to_json) }
 
   describe '#track_submission_exhaustion' do
-    [true, false].each do |flipper_value|
-      context "when json_schemer flipper is #{flipper_value}" do
-        before do
-          allow(Flipper).to receive(:enabled?).with(:validate_saved_claims_with_json_schemer).and_return(flipper_value)
-        end
+    it 'logs sidekiq job exhaustion failure avoided' do
+      msg = { 'args' => [claim.id, encrypted_user], error_message: 'Error!' }
 
-        it 'logs sidekiq job exhaustion failure avoided' do
-          msg = { 'args' => [claim.id, encrypted_user], error_message: 'Error!' }
+      log = "Failed all retries on VRE::Submit1900Job, last error: #{msg['error_message']}"
+      payload = {
+        message: msg
+      }
 
-          log = "Failed all retries on VRE::Submit1900Job, last error: #{msg['error_message']}"
-          payload = {
-            message: msg
-          }
+      expect(monitor).to receive(:log_silent_failure_avoided).with(payload, nil, anything)
+      expect(StatsD).to receive(:increment).with("#{submission_stats_key}.exhausted")
+      expect(Rails.logger).to receive(:error).with(log)
 
-          expect(monitor).to receive(:log_silent_failure_avoided).with(payload, nil, anything)
-          expect(StatsD).to receive(:increment).with("#{submission_stats_key}.exhausted")
-          expect(Rails.logger).to receive(:error).with(log)
-
-          monitor.track_submission_exhaustion(msg, user_struct.va_profile_email)
-        end
+      monitor.track_submission_exhaustion(msg, user_struct.va_profile_email)
+    end
 
-        it 'logs sidekiq job exhaustion failure' do
-          msg = { 'args' => [claim.id, encrypted_user], error_message: 'Error!' }
+    it 'logs sidekiq job exhaustion failure' do
+      msg = { 'args' => [claim.id, encrypted_user], error_message: 'Error!' }
 
-          log = "Failed all retries on VRE::Submit1900Job, last error: #{msg['error_message']}"
-          payload = {
-            message: msg
-          }
+      log = "Failed all retries on VRE::Submit1900Job, last error: #{msg['error_message']}"
+      payload = {
+        message: msg
+      }
 
-          expect(monitor).to receive(:log_silent_failure).with(payload, nil, anything)
-          expect(StatsD).to receive(:increment).with("#{submission_stats_key}.exhausted")
-          expect(Rails.logger).to receive(:error).with(log)
+      expect(monitor).to receive(:log_silent_failure).with(payload, nil, anything)
+      expect(StatsD).to receive(:increment).with("#{submission_stats_key}.exhausted")
+      expect(Rails.logger).to receive(:error).with(log)
 
-          monitor.track_submission_exhaustion(msg, nil)
-        end
-      end
+      monitor.track_submission_exhaustion(msg, nil)
     end
   end
 end