Merge pull request #3420 from DFE-Digital/CAPT-1991-tps-background-job

CAPT-1991 - Move processing TPS CSV to a background job

app/controllers/admin/tps_data_uploads_controller.rb

@@ -6,17 +6,16 @@ def new
     end
 
     def create
-      @tps_data_importer = TeachersPensionsServiceImporter.new(params[:file])
+      file = params[:file]
+      @tps_data_importer = TeachersPensionsServiceImporter.new(file)
 
       if @tps_data_importer.errors.any?
         render :new
       else
-        @tps_data_importer.run
-        if @tps_data_importer.errors.any?
-          render :new and return
-        end
-        EmploymentCheckJob.perform_later
-        redirect_to admin_claims_path, notice: "Teachers Pensions Service data uploaded successfully"
+        file_upload = FileUpload.create(uploaded_by: admin_user, body: File.read(file))
+        ImportTeachersPensionServiceDataJob.perform_later(file_upload.id)
+
+        redirect_to admin_claims_path, notice: "Teachers Pensions Service data file uploaded and queued to be imported"
       end
     rescue ActiveRecord::RecordInvalid => e
       Rollbar.error(e)

app/jobs/import_teachers_pension_service_data_job.rb

@@ -0,0 +1,8 @@
+class ImportTeachersPensionServiceDataJob < FileImporterJob
+  import_with TeachersPensionsServiceImporter do
+    Rails.logger.info "TPS data imported; queue employment check job"
+
+    EmploymentCheckJob.perform_later
+  end
+  notify_with AdminMailer, success: :tps_csv_processing_success, failure: :tps_csv_processing_error
+end

app/mailers/admin_mailer.rb

@@ -14,4 +14,20 @@ def census_csv_processing_error(email_address)
       reply_to_id: GENERIC_NOTIFY_REPLY_TO_ID
     )
   end
+
+  def tps_csv_processing_success(email_address)
+    template_mail(
+      TPS_CSV_PROCESSING_SUCCESS_ID,
+      to: email_address,
+      reply_to_id: GENERIC_NOTIFY_REPLY_TO_ID
+    )
+  end
+
+  def tps_csv_processing_error(email_address)
+    template_mail(
+      TPS_CSV_PROCESSING_ERROR_ID,
+      to: email_address,
+      reply_to_id: GENERIC_NOTIFY_REPLY_TO_ID
+    )
+  end
 end

app/mailers/application_mailer.rb

@@ -6,6 +6,8 @@ class ApplicationMailer < Mail::Notify::Mailer
   REMINDER_SET_NOTIFY_TEMPLATE_ID = "0dc80ba9-adae-43cd-98bf-58882ee401c3".freeze
   CENSUS_CSV_PROCESSING_ERROR_ID = "873170c9-4535-441f-b929-4670f022ecc9".freeze
   CENSUS_CSV_PROCESSING_SUCCESS_ID = "81862fc7-f842-4f85-a6e7-63dffb931999".freeze
+  TPS_CSV_PROCESSING_ERROR_ID = "f29753a1-5b9c-4e37-8b5a-43150b3bca64".freeze
+  TPS_CSV_PROCESSING_SUCCESS_ID = "5d817617-06c3-4df0-b228-f3d25510701e".freeze
 
   STUDENT_LOANS = {
     CLAIM_RECEIVED_NOTIFY_TEMPLATE_ID: "f9e39fcd-301a-4427-9159-6831fd484e39".freeze,

config/brakeman.ignore

@@ -23,37 +23,14 @@
       ],
       "note": ""
     },
-    {
-      "warning_type": "SQL Injection",
-      "warning_code": 0,
-      "fingerprint": "0ac253105d2af7d0dd324537b4be4a392ded73c0f3df8ca4d32db09cac7b586d",
-      "check_name": "SQL",
-      "message": "Possible SQL injection",
-      "file": "app/models/claim/search.rb",
-      "line": 35,
-      "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
-      "code": "Claim.submitted.where(\"LOWER(#{attribute}) = LOWER(?)\", search_term)",
-      "render_path": null,
-      "location": {
-        "type": "method",
-        "class": "Search",
-        "method": "search_by"
-      },
-      "user_input": "attribute",
-      "confidence": "Medium",
-      "cwe_id": [
-        89
-      ],
-      "note": ""
-    },
     {
       "warning_type": "Dynamic Render Path",
       "warning_code": 15,
       "fingerprint": "23e1b8f42ab87b9f17f883de7ac8b7b11261ec3dd8f8ad11f06aaae2a3b6fc51",
       "check_name": "Render",
       "message": "Render path contains parameter value",
       "file": "app/controllers/admin/tasks_controller.rb",
-      "line": 47,
+      "line": 48,
       "link": "https://brakemanscanner.org/docs/warning_types/dynamic_render_path/",
       "code": "render(action => Claim.includes(:tasks).find(params[:claim_id]).tasks.where(:name => params[:name]).first.name, {})",
       "render_path": null,
@@ -115,6 +92,29 @@
       ],
       "note": ""
     },
+    {
+      "warning_type": "File Access",
+      "warning_code": 16,
+      "fingerprint": "6a4206d355fcfb4a8c32cd37bd3511a5d31136565add04cf060e63a8a7aff810",
+      "check_name": "FileAccess",
+      "message": "Parameter value used in file name",
+      "file": "app/controllers/admin/tps_data_uploads_controller.rb",
+      "line": 15,
+      "link": "https://brakemanscanner.org/docs/warning_types/file_access/",
+      "code": "File.read(params[:file])",
+      "render_path": null,
+      "location": {
+        "type": "method",
+        "class": "Admin::TpsDataUploadsController",
+        "method": "create"
+      },
+      "user_input": "params[:file]",
+      "confidence": "High",
+      "cwe_id": [
+        22
+      ],
+      "note": ""
+    },
     {
       "warning_type": "Dynamic Render Path",
       "warning_code": 15,
@@ -145,7 +145,7 @@
       "check_name": "SQL",
       "message": "Possible SQL injection",
       "file": "app/models/claim.rb",
-      "line": 222,
+      "line": 229,
       "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
       "code": "joins(\"JOIN (\\n  #{Policies::POLICIES.map do\n \"\\n                SELECT\\n                id,\\n                #{policy.award_amount_column} AS award_amount,\\n                '#{policy::Eligibility}' AS eligibility_type\\n                FROM #{policy::Eligibility.table_name}\\n              \"\n end.join(\" UNION ALL \")}\\n) AS eligibilities\\nON claims.eligibility_id = eligibilities.id\\nAND claims.eligibility_type = eligibilities.eligibility_type\\n\")",
       "render_path": null,
@@ -185,6 +185,6 @@
       "note": ""
     }
   ],
-  "updated": "2024-10-30 16:55:54 +0000",
+  "updated": "2024-11-20 17:38:36 +0000",
   "brakeman_version": "6.2.1"
 }

spec/jobs/import_teachers_pension_service_data_job_spec.rb

@@ -0,0 +1,62 @@
+require "rails_helper"
+
+RSpec.describe ImportTeachersPensionServiceDataJob do
+  describe "#perform" do
+    context "csv data processes successfully" do
+      let(:mail) { AdminMailer.tps_csv_processing_success(file_upload.uploaded_by.email) }
+      let(:file_upload) { create(:file_upload, body: csv) }
+      let(:csv) do
+        <<~CSV
+          Teacher reference number,NINO,Start Date,End Date,Annual salary,Monthly pay, ,LA URN,School URN,Employer ID
+          1234567,ZX043155C,01/09/2019,30/09/2019,24373,2031.08,5016,383,4026,1122
+        CSV
+      end
+
+      it "imports tps data, sends success email and deletes the file upload" do
+        expect(EmploymentCheckJob).to receive(:perform_later)
+
+        subject.perform(file_upload.id)
+
+        # imports the tps data
+        expect(TeachersPensionsService.count).to eq(1)
+
+        # success email
+        expect(ActionMailer::Base.deliveries.count).to eq(1)
+        expect(mail.template_id).to eq "5d817617-06c3-4df0-b228-f3d25510701e"
+
+        # deletes the file upload
+        expect(FileUpload.find_by_id(file_upload.id)).to be_nil
+      end
+    end
+
+    context "csv data encounters an error" do
+      let(:mail) { AdminMailer.tps_csv_processing_error(file_upload.uploaded_by.email) }
+      let(:file_upload) { create(:file_upload, body: csv) }
+      let(:csv) do
+        <<~CSV
+          Teacher reference number,NINO,Start Date,End Date,Annual salary,Monthly pay, ,LA URN,School URN,Employer ID
+          1234567,ZX043155C,01/09/2019,30/09/2019,24373,2031.08,5016,383,4026,1122
+        CSV
+      end
+
+      before do
+        allow(TeachersPensionsService).to receive(:insert_all).and_raise(ActiveRecord::RecordInvalid)
+        allow(Rollbar).to receive(:error)
+      end
+
+      it "sends error email and keeps the file upload" do
+        subject.perform(file_upload.id)
+
+        # error email
+        expect(ActionMailer::Base.deliveries.count).to eq(1)
+        expect(mail.template_id).to eq "f29753a1-5b9c-4e37-8b5a-43150b3bca64"
+
+        # Rollbar error report
+        expect(Rollbar).to have_received(:error)
+
+        # keeps the file upload for debugging
+        expect(FileUpload.find_by_id(file_upload.id)).to be_present
+      end
+    end
+  end
+end