Merge pull request #3427 from DFE-Digital/CAPT-2020/dont-pass-task-if…

…-setting-details-errors

Move error handling

app/jobs/student_loan_amount_check_job.rb

@@ -6,6 +6,9 @@ def perform
     claims.each do |claim|
       ClaimStudentLoanDetailsUpdater.call(claim)
       AutomatedChecks::ClaimVerifiers::StudentLoanAmount.new(claim:).perform
+    rescue => e
+      # If something goes wrong, log the error and continue
+      Rollbar.error(e)
     end
   end
 

app/jobs/student_loan_plan_check_job.rb

@@ -12,6 +12,9 @@ def perform
     claims.each do |claim|
       ClaimStudentLoanDetailsUpdater.call(claim)
       AutomatedChecks::ClaimVerifiers::StudentLoanPlan.new(claim:).perform
+    rescue => e
+      # If something goes wrong, log the error and continue
+      Rollbar.error(e)
     end
   end
 

app/models/claim_student_loan_details_updater.rb

@@ -15,9 +15,6 @@ def update_claim_with_latest_data
 
       claim.save!(context: :"student-loan")
     end
-  rescue => e
-    # If something goes wrong, log the error and continue
-    Rollbar.error(e)
   end
 
   private

spec/jobs/student_loan_amount_check_job_spec.rb

@@ -10,100 +10,134 @@
   let!(:journey_configuration) { create(:journey_configuration, :student_loans) }
 
   describe "#perform" do
-    before do
-      allow(ClaimStudentLoanDetailsUpdater).to receive(:call)
-    end
-
-    shared_examples :skip_check do
+    context "without error" do
       before do
-        allow(AutomatedChecks::ClaimVerifiers::StudentLoanAmount).to receive(:new)
+        allow(ClaimStudentLoanDetailsUpdater).to receive(:call)
       end
 
-      it "excludes the claim from the check", :aggregate_failures do
-        expect(ClaimStudentLoanDetailsUpdater).not_to receive(:call).with(claim)
-        expect(AutomatedChecks::ClaimVerifiers::StudentLoanAmount).not_to receive(:new).with(claim: claim)
-        perform_job
-      end
-    end
+      shared_examples :skip_check do
+        before do
+          allow(AutomatedChecks::ClaimVerifiers::StudentLoanAmount).to receive(:new)
+        end
 
-    context "when the previous student loan amount check was run manually" do
-      let!(:previous_task) { create(:task, claim: claim, name: "student_loan_amount", claim_verifier_match: nil, manual: true) }
+        it "excludes the claim from the check", :aggregate_failures do
+          expect(ClaimStudentLoanDetailsUpdater).not_to receive(:call).with(claim)
+          expect(AutomatedChecks::ClaimVerifiers::StudentLoanAmount).not_to receive(:new).with(claim: claim)
+          perform_job
+        end
+      end
 
-      include_examples :skip_check
-    end
+      context "when the previous student loan amount check was run manually" do
+        let!(:previous_task) { create(:task, claim: claim, name: "student_loan_amount", claim_verifier_match: nil, manual: true) }
 
-    context "when a claim is not awaiting decision" do
-      let(:claim_status) { :approved }
+        include_examples :skip_check
+      end
 
-      include_examples :skip_check
-    end
+      context "when a claim is not awaiting decision" do
+        let(:claim_status) { :approved }
 
-    context "when a claim was submitted using the student loan questions" do
-      before do
-        claim.update!(submitted_using_slc_data: nil)
+        include_examples :skip_check
       end
 
-      include_examples :skip_check
-    end
+      context "when a claim was submitted using the student loan questions" do
+        before do
+          claim.update!(submitted_using_slc_data: nil)
+        end
 
-    context "when the student loan amount check did not run before" do
-      it "updates the student loan details" do
-        expect(ClaimStudentLoanDetailsUpdater).to receive(:call).with(claim)
-        perform_job
+        include_examples :skip_check
       end
 
-      it "runs the task" do
-        expect { perform_job }
-          .to change { claim.reload.notes.count }
-          .and change { claim.tasks.count }
+      context "when the student loan amount check did not run before" do
+        it "updates the student loan details" do
+          expect(ClaimStudentLoanDetailsUpdater).to receive(:call).with(claim)
+          perform_job
+        end
+
+        it "runs the task" do
+          expect { perform_job }
+            .to change { claim.reload.notes.count }
+            .and change { claim.tasks.count }
+        end
       end
-    end
 
-    context "when the previous student loan amount check outcome was NO DATA" do
-      let!(:previous_task) { create(:task, claim: claim, name: "student_loan_amount", claim_verifier_match: nil, manual: false) }
+      context "when the previous student loan amount check outcome was NO DATA" do
+        let!(:previous_task) { create(:task, claim: claim, name: "student_loan_amount", claim_verifier_match: nil, manual: false) }
 
-      it "updates the student loan details" do
-        expect(ClaimStudentLoanDetailsUpdater).to receive(:call).with(claim)
-        perform_job
-      end
+        it "updates the student loan details" do
+          expect(ClaimStudentLoanDetailsUpdater).to receive(:call).with(claim)
+          perform_job
+        end
 
-      it "re-runs the task" do
-        expect { perform_job }
-          .to change { claim.reload.notes.count }
-          .and change { claim.tasks.last.updated_at }
-          .and not_change { claim.reload.tasks.count }
+        it "re-runs the task" do
+          expect { perform_job }
+            .to change { claim.reload.notes.count }
+            .and change { claim.tasks.last.updated_at }
+            .and not_change { claim.reload.tasks.count }
+        end
       end
-    end
 
-    context "when the previous student loan amount check outcome was FAILED" do
-      let!(:previous_task) { create(:task, claim: claim, name: "student_loan_amount", claim_verifier_match: :none, passed: false, manual: false) }
+      context "when the previous student loan amount check outcome was FAILED" do
+        let!(:previous_task) { create(:task, claim: claim, name: "student_loan_amount", claim_verifier_match: :none, passed: false, manual: false) }
 
-      it "does not update the student loan details" do
-        expect(ClaimStudentLoanDetailsUpdater).not_to receive(:call)
-        perform_job
+        it "does not update the student loan details" do
+          expect(ClaimStudentLoanDetailsUpdater).not_to receive(:call)
+          perform_job
+        end
+
+        it "does not re-run the task" do
+          expect { perform_job }
+            .to not_change { claim.reload.notes.count }
+            .and not_change { claim.tasks.last.updated_at }
+            .and not_change { claim.reload.tasks.count }
+        end
       end
 
-      it "does not re-run the task" do
-        expect { perform_job }
-          .to not_change { claim.reload.notes.count }
-          .and not_change { claim.tasks.last.updated_at }
-          .and not_change { claim.reload.tasks.count }
+      context "when the previous student loan amount check outcome was PASSED" do
+        let!(:previous_task) { create(:task, claim: claim, name: "student_loan_amount", claim_verifier_match: :all, manual: false) }
+
+        it "does not update the student loan details" do
+          expect(ClaimStudentLoanDetailsUpdater).not_to receive(:call)
+          perform_job
+        end
+
+        it "does not re-run the task" do
+          expect { perform_job }
+            .to not_change { claim.reload.notes.count }
+            .and not_change { claim.tasks.last.updated_at }
+            .and not_change { claim.reload.tasks.count }
+        end
       end
     end
 
-    context "when the previous student loan amount check outcome was PASSED" do
-      let!(:previous_task) { create(:task, claim: claim, name: "student_loan_amount", claim_verifier_match: :all, manual: false) }
+    context "when there's an error" do
+      let(:exception) { ActiveRecord::RecordInvalid }
 
-      it "does not update the student loan details" do
-        expect(ClaimStudentLoanDetailsUpdater).not_to receive(:call)
+      before do
+        create(
+          :student_loans_data,
+          claim_reference: claim.reference,
+          nino: claim.national_insurance_number,
+          date_of_birth: claim.date_of_birth
+        )
+        allow_any_instance_of(Claim).to receive(:save!) { raise(exception) }
+        allow(Rollbar).to receive(:error)
+      end
+
+      it "suppresses the exception" do
+        expect { perform_job }.not_to raise_error
+      end
+
+      it "logs the exception" do
         perform_job
+
+        expect(Rollbar).to have_received(:error).with(exception)
       end
 
-      it "does not re-run the task" do
-        expect { perform_job }
-          .to not_change { claim.reload.notes.count }
-          .and not_change { claim.tasks.last.updated_at }
-          .and not_change { claim.reload.tasks.count }
+      it "does not update the student loan details or create a task or note" do
+        expect { perform_job }.to not_change { claim.student_loan_plan }
+          .and not_change { claim.eligibility.student_loan_repayment_amount }
+          .and not_change { claim.tasks.count }
+          .and not_change { claim.notes.count }
       end
     end
   end

spec/jobs/student_loan_plan_check_job_spec.rb

@@ -125,5 +125,36 @@
 
       include_examples :student_loan_plan_claim_verifier_not_called
     end
+
+    context "when an error occurs while updating" do
+      let(:exception) { ActiveRecord::RecordInvalid }
+
+      before do
+        create(
+          :student_loans_data,
+          claim_reference: claim.reference,
+          nino: claim.national_insurance_number,
+          date_of_birth: claim.date_of_birth
+        )
+        allow_any_instance_of(Claim).to receive(:save!) { raise(exception) }
+        allow(Rollbar).to receive(:error)
+      end
+
+      it "suppresses the exception" do
+        expect { perform_job }.not_to raise_error
+      end
+
+      it "logs the exception" do
+        perform_job
+
+        expect(Rollbar).to have_received(:error).with(exception)
+      end
+
+      it "does not update the student loan details or create a task or note" do
+        expect { perform_job }.to not_change { claim.student_loan_plan }
+          .and not_change { claim.tasks.count }
+          .and not_change { claim.notes.count }
+      end
+    end
   end
 end

spec/models/claim_student_loan_details_updater_spec.rb

@@ -108,31 +108,6 @@
       end
     end
 
-    context "when an error occurs while updating" do
-      let(:exception) { ActiveRecord::RecordInvalid }
-
-      before do
-        allow(claim).to receive(:save!) { raise(exception) }
-        allow(Rollbar).to receive(:error)
-      end
-
-      it "suppresses the exception" do
-        expect { call }.not_to raise_error
-      end
-
-      it "logs the exception" do
-        call
-
-        expect(Rollbar).to have_received(:error).with(exception)
-      end
-
-      it "does not update the student loan details" do
-        expect { call }.to not_change { claim.reload.has_student_loan }
-          .and not_change { claim.student_loan_plan }
-          .and not_change { claim.eligibility.student_loan_repayment_amount }
-      end
-    end
-
     context "when updating a claim after submission" do
       let(:claim) { create(:claim, :submitted, :with_no_student_loan, policy:) }