From bd05c2f0ddd663714c25077cc2d7dc71704f1ef4 Mon Sep 17 00:00:00 2001 From: Peri McLaren <141954992+pmclaren19@users.noreply.github.com> Date: Wed, 18 Dec 2024 14:13:34 -0700 Subject: [PATCH 01/13] added back changes from previous pr and started writing tests in document upload files --- app/models/evidence_submission.rb | 6 + app/sidekiq/evss/document_upload.rb | 58 ++++--- app/sidekiq/evss/failure_notification.rb | 38 ---- .../document_upload.rb | 47 ++--- .../failure_notification_email_job.rb | 91 ++++++++++ .../lighthouse/failure_notification.rb | 38 ---- ...1217210622_create_evidence_submissions.rb} | 3 +- db/schema.rb | 7 +- lib/lighthouse/benefits_claims/service.rb | 4 +- lib/periodic_jobs.rb | 3 + spec/sidekiq/evss/document_upload_spec.rb | 91 +++++----- .../sidekiq/evss/failure_notification_spec.rb | 55 ------ .../document_upload_spec.rb | 140 +++++++++++++++ .../failure_notification_email_job_spec.rb} | 6 +- .../lighthouse/document_upload_spec.rb | 162 ------------------ 15 files changed, 346 insertions(+), 403 deletions(-) delete mode 100644 app/sidekiq/evss/failure_notification.rb rename app/sidekiq/lighthouse/{ => benefits_documents}/document_upload.rb (79%) create mode 100644 app/sidekiq/lighthouse/benefits_documents/failure_notification_email_job.rb delete mode 100644 app/sidekiq/lighthouse/failure_notification.rb rename db/migrate/{20240925160219_create_evidence_submissions.rb => 20241217210622_create_evidence_submissions.rb} (83%) delete mode 100644 spec/sidekiq/evss/failure_notification_spec.rb create mode 100644 spec/sidekiq/lighthouse/benefits_documents/document_upload_spec.rb rename spec/sidekiq/lighthouse/{failure_notification_spec.rb => benefits_documents/failure_notification_email_job_spec.rb} (91%) delete mode 100644 spec/sidekiq/lighthouse/document_upload_spec.rb diff --git a/app/models/evidence_submission.rb b/app/models/evidence_submission.rb index 3bc3bb19a7d..754c15133f0 100644 --- a/app/models/evidence_submission.rb +++ b/app/models/evidence_submission.rb @@ -1,7 +1,13 @@ # frozen_string_literal: true +require 'lighthouse/benefits_documents/constants' + class EvidenceSubmission < ApplicationRecord belongs_to :user_account has_kms_key has_encrypted :template_metadata, key: :kms_key, **lockbox_options + + scope :va_notify_email_not_sent, lambda { + where(upload_status: BenefitsDocuments::Constants::UPLOAD_STATUS[:FAILED], va_notify_date: nil) + } end diff --git a/app/sidekiq/evss/document_upload.rb b/app/sidekiq/evss/document_upload.rb index c8486287b51..8da03f1854f 100644 --- a/app/sidekiq/evss/document_upload.rb +++ b/app/sidekiq/evss/document_upload.rb @@ -3,7 +3,7 @@ require 'ddtrace' require 'timeout' require 'logging/third_party_transaction' -require 'evss/failure_notification' +require 'lighthouse/benefits_documents/constants' class EVSS::DocumentUpload include Sidekiq::Job @@ -13,11 +13,6 @@ class EVSS::DocumentUpload FILENAME_EXTENSION_MATCHER = /\.\w*$/ OBFUSCATED_CHARACTER_MATCHER = /[a-zA-Z\d]/ - DD_ZSF_TAGS = ['service:claim-status', 'function: evidence upload to EVSS'].freeze - - NOTIFY_SETTINGS = Settings.vanotify.services.benefits_management_tools - MAILER_TEMPLATE_ID = NOTIFY_SETTINGS.template_id.evidence_submission_failure_email - attr_accessor :auth_headers, :user_uuid, :document_hash wrap_with_logging( @@ -33,35 +28,42 @@ class EVSS::DocumentUpload ) # retry for one day - sidekiq_options retry: 16, queue: 'low' + sidekiq_options retry: 0, queue: 'low' # Set minimum retry time to ~1 hour - sidekiq_retry_in do |count, _exception| - rand(3600..3660) if count < 9 - end + # sidekiq_retry_in do |count, _exception| + # rand(3600..3660) if count < 9 + # end sidekiq_retries_exhausted do |msg, _ex| - # There should be 3 values in msg['args']: - # 1) Auth headers needed to authenticate with EVSS - # 2) The uuid of the record in the UserAccount table - # 3) Document metadata - - next unless Flipper.enabled?('cst_send_evidence_failure_emails') - - icn = UserAccount.find(msg['args'][1]).icn - first_name = msg['args'].first['va_eauth_firstName'].titleize + byebug + puts 'hello test' + + job_id = msg['jid'] + job_class = 'EVSS::DocumentUpload' + first_name = msg['args'][0]['va_eauth_firstName'].titleize + claim_id = msg['args'][2]['evss_claim_id'] + tracked_item_id = msg['args'][2]['tracked_item_id'] filename = obscured_filename(msg['args'][2]['file_name']) date_submitted = format_issue_instant_for_mailers(msg['created_at']) date_failed = format_issue_instant_for_mailers(msg['failed_at']) - - EVSS::FailureNotification.perform_async(icn, first_name, filename, date_submitted, date_failed) - - ::Rails.logger.info('EVSS::DocumentUpload exhaustion handler email queued') - StatsD.increment('silent_failure_avoided_no_confirmation', tags: DD_ZSF_TAGS) + upload_status = BenefitsDocuments::Constants::UPLOAD_STATUS[:FAILED] + uuid = msg['args'][2]['uuid'] + user_account = UserAccount.find_or_create_by(id: uuid) + personalisation = { first_name:, filename:, date_submitted:, date_failed: } + puts 'peri test' + EvidenceSubmission.create( + job_id:, + job_class:, + claim_id:, + tracked_item_id:, + upload_status:, + user_account:, + template_metadata_ciphertext: { personalisation: }.to_json + ) + puts 'peri test2' rescue => e - ::Rails.logger.error('EVSS::DocumentUpload exhaustion handler email error', - { message: e.message }) - StatsD.increment('silent_failure', tags: DD_ZSF_TAGS) - log_exception_to_sentry(e) + puts 'there was an error' + puts e end def perform(auth_headers, user_uuid, document_hash) diff --git a/app/sidekiq/evss/failure_notification.rb b/app/sidekiq/evss/failure_notification.rb deleted file mode 100644 index 0fb573acbed..00000000000 --- a/app/sidekiq/evss/failure_notification.rb +++ /dev/null @@ -1,38 +0,0 @@ -# frozen_string_literal: true - -class EVSS::FailureNotification - include Sidekiq::Job - include SentryLogging - - NOTIFY_SETTINGS = Settings.vanotify.services.benefits_management_tools - MAILER_TEMPLATE_ID = NOTIFY_SETTINGS.template_id.evidence_submission_failure_email - - # retry for one day - sidekiq_options retry: 14, queue: 'low' - # Set minimum retry time to ~1 hour - sidekiq_retry_in do |count, _exception| - rand(3600..3660) if count < 9 - end - - sidekiq_retries_exhausted do - ::Rails.logger.info('EVSS::FailureNotification email could not be sent') - end - - def notify_client - VaNotify::Service.new(NOTIFY_SETTINGS.api_key) - end - - def perform(icn, first_name, filename, date_submitted, date_failed) - notify_client.send_email( - recipient_identifier: { id_value: icn, id_type: 'ICN' }, - template_id: MAILER_TEMPLATE_ID, - personalisation: { first_name:, filename:, date_submitted:, date_failed: } - ) - - ::Rails.logger.info('EVSS::FailureNotification email sent') - rescue => e - ::Rails.logger.error('EVSS::FailureNotification email error', - { message: e.message }) - log_exception_to_sentry(e) - end -end diff --git a/app/sidekiq/lighthouse/document_upload.rb b/app/sidekiq/lighthouse/benefits_documents/document_upload.rb similarity index 79% rename from app/sidekiq/lighthouse/document_upload.rb rename to app/sidekiq/lighthouse/benefits_documents/document_upload.rb index 55c0fadc876..89f3ee923bd 100644 --- a/app/sidekiq/lighthouse/document_upload.rb +++ b/app/sidekiq/lighthouse/benefits_documents/document_upload.rb @@ -4,9 +4,8 @@ require 'timeout' require 'lighthouse/benefits_documents/worker_service' require 'lighthouse/benefits_documents/constants' -require 'lighthouse/failure_notification' -class Lighthouse::DocumentUpload +class Lighthouse::BenefitsDocuments::DocumentUpload include Sidekiq::Job extend SentryLogging @@ -15,11 +14,6 @@ class Lighthouse::DocumentUpload FILENAME_EXTENSION_MATCHER = /\.\w*$/ OBFUSCATED_CHARACTER_MATCHER = /[a-zA-Z\d]/ - DD_ZSF_TAGS = ['service:claim-status', 'function: evidence upload to Lighthouse'].freeze - - NOTIFY_SETTINGS = Settings.vanotify.services.benefits_management_tools - MAILER_TEMPLATE_ID = NOTIFY_SETTINGS.template_id.evidence_submission_failure_email - # retry for 2d 1h 47m 12s # https://github.com/sidekiq/sidekiq/wiki/Error-Handling sidekiq_options retry: 16, queue: 'low' @@ -29,27 +23,28 @@ class Lighthouse::DocumentUpload end sidekiq_retries_exhausted do |msg, _ex| - # There should be 2 values in msg['args']: - # 1) The ICN of the user - # 2) Document metadata - - next unless Flipper.enabled?('cst_send_evidence_failure_emails') - - icn = msg['args'].first + job_id = msg['jid'] + job_class = 'Lighthouse::BenefitsDocuments::DocumentUpload' first_name = msg['args'][1]['first_name'].titleize + claim_id = msg['args'][1]['claim_id'] + tracked_item_id = msg['args'][1]['tracked_item_id'] filename = obscured_filename(msg['args'][1]['file_name']) date_submitted = format_issue_instant_for_mailers(msg['created_at']) date_failed = format_issue_instant_for_mailers(msg['failed_at']) - - Lighthouse::FailureNotification.perform_async(icn, first_name, filename, date_submitted, date_failed) - - ::Rails.logger.info('Lighthouse::DocumentUpload exhaustion handler email queued') - StatsD.increment('silent_failure_avoided_no_confirmation', tags: DD_ZSF_TAGS) - rescue => e - ::Rails.logger.error('Lighthouse::DocumentUpload exhaustion handler email error', - { message: e.message }) - StatsD.increment('silent_failure', tags: DD_ZSF_TAGS) - log_exception_to_sentry(e) + upload_status = BenefitsDocuments::Constants::UPLOAD_STATUS[:FAILED] + uuid = msg['args'][1]['uuid'] + user_account = UserAccount.find_or_create_by(id: uuid) + personalisation = { first_name:, filename:, date_submitted:, date_failed: } + + EvidenceSubmission.create( + job_id:, + job_class:, + claim_id:, + tracked_item_id:, + upload_status:, + user_account:, + template_metadata_ciphertext: { personalisation: }.to_json + ) end def self.obscured_filename(original_filename) @@ -75,10 +70,6 @@ def self.format_issue_instant_for_mailers(issue_instant) timestamp.strftime('%B %-d, %Y %-l:%M %P %Z').sub(/([ap])m/, '\1.m.') end - def self.notify_client - VaNotify::Service.new(NOTIFY_SETTINGS.api_key) - end - def perform(user_icn, document_hash, user_account_uuid, claim_id, tracked_item_id) @user_icn = user_icn @document_hash = document_hash diff --git a/app/sidekiq/lighthouse/benefits_documents/failure_notification_email_job.rb b/app/sidekiq/lighthouse/benefits_documents/failure_notification_email_job.rb new file mode 100644 index 00000000000..707956628df --- /dev/null +++ b/app/sidekiq/lighthouse/benefits_documents/failure_notification_email_job.rb @@ -0,0 +1,91 @@ +require 'sidekiq' +require 'lighthouse/benefits_documents/constants' + +module BenefitsDocuments + class FailureNotificationEmailJob + include Sidekiq::Job + + sidekiq_options retry: false, unique_for: 30.minutes + + FAILED_STATUS = BenefitsDocuments::Constants::UPLOAD_STATUS[:FAILED] + MAILER_TEMPLATE_ID = NOTIFY_SETTINGS.template_id.evidence_submission_failure_email + DD_ZSF_TAGS = ['service:claim-status', 'function: evidence upload to Lighthouse'].freeze + + # TODO: need to add statsd logic + # STATSD_KEY_PREFIX = '' + # + + def perform + return unless should_perform? + + send_failed_evidence_submissions + + nil + end + + def should_perform? + failed_uploads.present? + end + + # Fetches FAILED evidence submission records for BenefitsDocuments that dont have a va_notify_date + def failed_uploads + @failed_uploads ||= EvidenceSubmission.va_notify_email_not_sent + end + + def notify_client + VaNotify::Service.new(NOTIFY_SETTINGS.api_key) + end + + def send_failed_evidence_submissions + failed_uploads.each do |upload| + response = notify_client.send_email( + recipient_identifier: { id_value: upload.user_account.icn, id_type: 'ICN' }, + template_id: MAILER_TEMPLATE_ID, + personalisation: upload.template_metadata_ciphertext.personalisation + ) + + record_email_send_success(upload, response) + rescue => e + record_email_send_failure(upload, e) + end + + nil + end + + def record_email_send_success(upload, response) + EvidenceSubmission.update(id: upload.id, va_notify_id: response.id, va_notify_date: DateTime.now) + ::Rails.logger.info("#{upload.job_class} va notify failure email sent") + StatsD.increment('silent_failure_avoided_no_confirmation', tags: DD_ZSF_TAGS) + end + + def record_email_send_failure(upload, error) + ::Rails.logger.error("#{upload.job_class} va notify failure email failed to send", + { message: error.message }) + StatsD.increment('silent_failure', tags: DD_ZSF_TAGS) + log_exception_to_sentry(e) + end + + # add va_notify_date to the evidence_submissions table - done + # grab failed records that dont have a va_notify_date - done + # reach out to va notify with an id of the template - go off of what was in the + # call for this job app/sidekiq/lighthouse/failure_notification.rb, no retrys + # for each file - done + # va notify should return an id when a record is created (take a look at record_evidence_email_send_successful() for an example) + # update evidence submissions with a va notify id, and va notify date - done + # + # use job_class from the es table to determine if we + # should send evss or lighthouse log - done + # Update app/sidekiq/lighthouse/document_upload.rb method sidekiq_retries_exhausted() + # and create a new record and set the upload status to FAILED - done + # Update app/sidekiq/evss/document_upload.rb method sidekiq_retries_exhausted() + # and set the upload status to FAILED - done + # + # Remove app/sidekiq/evss/failure_notification.rb and tests - done + # Remove app/sidekiq/lighthouse/failure_notification.rb and tests - done + # Update tests for doc upload lighthouse and evss + # Add tests for failure notification email job + # + # update periodic_job.rb with your job + # modules/decision_reviews/app/sidekiq/decision_reviews/failure_notification_email_job.rb + end +end diff --git a/app/sidekiq/lighthouse/failure_notification.rb b/app/sidekiq/lighthouse/failure_notification.rb deleted file mode 100644 index 77d6345500e..00000000000 --- a/app/sidekiq/lighthouse/failure_notification.rb +++ /dev/null @@ -1,38 +0,0 @@ -# frozen_string_literal: true - -class Lighthouse::FailureNotification - include Sidekiq::Job - include SentryLogging - - NOTIFY_SETTINGS = Settings.vanotify.services.benefits_management_tools - MAILER_TEMPLATE_ID = NOTIFY_SETTINGS.template_id.evidence_submission_failure_email - - # retry for one day - sidekiq_options retry: 14, queue: 'low' - # Set minimum retry time to ~1 hour - sidekiq_retry_in do |count, _exception| - rand(3600..3660) if count < 9 - end - - sidekiq_retries_exhausted do - ::Rails.logger.info('Lighthouse::FailureNotification email could not be sent') - end - - def notify_client - VaNotify::Service.new(NOTIFY_SETTINGS.api_key) - end - - def perform(icn, first_name, filename, date_submitted, date_failed) - notify_client.send_email( - recipient_identifier: { id_value: icn, id_type: 'ICN' }, - template_id: MAILER_TEMPLATE_ID, - personalisation: { first_name:, filename:, date_submitted:, date_failed: } - ) - - ::Rails.logger.info('Lighthouse::FailureNotification email sent') - rescue => e - ::Rails.logger.error('Lighthouse::FailureNotification email error', - { message: e.message }) - log_exception_to_sentry(e) - end -end diff --git a/db/migrate/20240925160219_create_evidence_submissions.rb b/db/migrate/20241217210622_create_evidence_submissions.rb similarity index 83% rename from db/migrate/20240925160219_create_evidence_submissions.rb rename to db/migrate/20241217210622_create_evidence_submissions.rb index be0dc24dfea..3af733e27fb 100644 --- a/db/migrate/20240925160219_create_evidence_submissions.rb +++ b/db/migrate/20241217210622_create_evidence_submissions.rb @@ -1,4 +1,4 @@ -class CreateEvidenceSubmissions < ActiveRecord::Migration[7.1] +class CreateEvidenceSubmissions < ActiveRecord::Migration[7.2] def change create_table :evidence_submissions do |t| t.string :job_id @@ -12,6 +12,7 @@ def change t.string :va_notify_id t.string :va_notify_status t.date :delete_date + t.datetime :va_notify_date t.string :tracked_item_id t.timestamps diff --git a/db/schema.rb b/db/schema.rb index 494ca59c768..2726a285e41 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2024_12_13_173113) do +ActiveRecord::Schema[7.2].define(version: 2024_12_17_210622) do # These are extensions that must be enabled in order to support this database enable_extension "btree_gin" enable_extension "fuzzystrmatch" @@ -645,16 +645,18 @@ t.string "job_class" t.string "request_id" t.string "claim_id" - t.string "user_account_id" + t.uuid "user_account_id", null: false t.json "template_metadata_ciphertext" t.text "encrypted_kms_key" t.string "upload_status" t.string "va_notify_id" t.string "va_notify_status" t.date "delete_date" + t.datetime "va_notify_date" t.string "tracked_item_id" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.index ["user_account_id"], name: "index_evidence_submissions_on_user_account_id" end create_table "evss_claims", id: :serial, force: :cascade do |t| @@ -1740,6 +1742,7 @@ add_foreign_key "deprecated_user_accounts", "user_accounts" add_foreign_key "deprecated_user_accounts", "user_verifications" add_foreign_key "education_stem_automated_decisions", "user_accounts" + add_foreign_key "evidence_submissions", "user_accounts" add_foreign_key "evss_claims", "user_accounts" add_foreign_key "form526_submission_remediations", "form526_submissions" add_foreign_key "form526_submissions", "user_accounts" diff --git a/lib/lighthouse/benefits_claims/service.rb b/lib/lighthouse/benefits_claims/service.rb index 594627bb54c..625c7c84ace 100644 --- a/lib/lighthouse/benefits_claims/service.rb +++ b/lib/lighthouse/benefits_claims/service.rb @@ -13,7 +13,9 @@ class Service < Common::Client::Base FILTERED_STATUSES = %w[CANCELED ERRORED PENDING].freeze def initialize(icn) - @icn = icn + # @icn = icn + # @icn = '1012830774V793840' # icn for user 23 + @icn = '1012830712V627751' # icn for user 19 if icn.blank? raise ArgumentError, 'no ICN passed in for LH API request.' else diff --git a/lib/periodic_jobs.rb b/lib/periodic_jobs.rb index e1e8273f2c0..8a5d239d2ac 100644 --- a/lib/periodic_jobs.rb +++ b/lib/periodic_jobs.rb @@ -255,4 +255,7 @@ mgr.register('45 05 * * 1-5', 'Vye::DawnDash') # Daily 1900 job for Vye: clears deactivated BDNs every evening. mgr.register('00 19 * * 1-5', 'Vye::SundownSweep') + + # Send Benefits Documents failure notification emails to Veteran for failed evidence submissions + mgr.register('5 0 * * *', 'Lighthouse::BenefitsDocuments::FailureNotificationEmailJob') } diff --git a/spec/sidekiq/evss/document_upload_spec.rb b/spec/sidekiq/evss/document_upload_spec.rb index 6c348c8113c..a722536d803 100644 --- a/spec/sidekiq/evss/document_upload_spec.rb +++ b/spec/sidekiq/evss/document_upload_spec.rb @@ -1,29 +1,54 @@ # frozen_string_literal: true require 'rails_helper' +require 'sidekiq/testing' +Sidekiq::Testing.fake! require 'evss/document_upload' require 'va_notify/service' RSpec.describe EVSS::DocumentUpload, type: :job do - subject { described_class } + subject(:job) do + described_class.perform_async(auth_headers, user.uuid, document_data.to_serializable_hash) + end + # subject { described_class } let(:client_stub) { instance_double('EVSS::DocumentsService') } + let(:job_id) { job } + let(:job_class) { 'EVSS::DocumentUpload' } + let(:claim_id) { '4567' } + let(:tracked_item_id) { '1234' } + let(:document_type) { 'L023' } + + let(:formatted_submit_date) do + # We want to return all times in EDT + timestamp = Time.at(issue_instant).in_time_zone('America/New_York') + + # We display dates in mailers in the format "May 1, 2024 3:01 p.m. EDT" + timestamp.strftime('%B %-d, %Y %-l:%M %P %Z').sub(/([ap])m/, '\1.m.') + end + let(:created_at) { DateTime.new(2023, 4, 2) } + let(:notify_client_stub) { instance_double(VaNotify::Service) } let(:uploader_stub) { instance_double('EVSSClaimDocumentUploader') } - let(:user_account) { create(:user_account) } let(:user_account_uuid) { user_account.id } let(:user) { FactoryBot.create(:user, :loa3) } let(:filename) { 'doctors-note.pdf' } let(:document_data) do EVSSClaimDocument.new( - evss_claim_id: 189_625, + evss_claim_id: claim_id, file_name: filename, - tracked_item_id: 33, - document_type: 'L023' + tracked_item_id:, + document_type: ) end + let(:personalisation) do + { first_name: user.first_name, + filename:, + date_submitted: created_at.strftime('%B %d, %Y'), + date_failed: created_at.strftime('%B %d, %Y') } + end let(:auth_headers) { EVSS::AuthHeaders.new(user).to_h } let(:issue_instant) { Time.now.to_i } @@ -34,11 +59,9 @@ 'failed_at' => issue_instant } end - let(:tags) { subject::DD_ZSF_TAGS } before do allow(Rails.logger).to receive(:info) - allow(StatsD).to receive(:increment) end it 'retrieves the file and uploads to EVSS' do @@ -52,46 +75,20 @@ described_class.new.perform(auth_headers, user.uuid, document_data.to_serializable_hash) end - context 'when cst_send_evidence_failure_emails is enabled' do - before do - Flipper.enable(:cst_send_evidence_failure_emails) - end - - let(:formatted_submit_date) do - # We want to return all times in EDT - timestamp = Time.at(issue_instant).in_time_zone('America/New_York') - - # We display dates in mailers in the format "May 1, 2024 3:01 p.m. EDT" - timestamp.strftime('%B %-d, %Y %-l:%M %P %Z').sub(/([ap])m/, '\1.m.') - end - - it 'calls EVSS::FailureNotification' do - subject.within_sidekiq_retries_exhausted_block(args) do - expect(EVSS::FailureNotification).to receive(:perform_async).with( - user_account.icn, - 'Bob', # first_name - 'docXXXX-XXte.pdf', # filename - formatted_submit_date, # date_submitted - formatted_submit_date # date_failed - ) - - expect(Rails.logger) - .to receive(:info) - .with('EVSS::DocumentUpload exhaustion handler email queued') - expect(StatsD).to receive(:increment).with('silent_failure_avoided_no_confirmation', tags:) - end - end - end - - context 'when cst_send_evidence_failure_emails is disabled' do - before do - Flipper.disable(:cst_send_evidence_failure_emails) - end - - it 'does not call Lighthouse::Failure Notification' do - subject.within_sidekiq_retries_exhausted_block(args) do - expect(EVSS::FailureNotification).not_to receive(:perform_async) - end + it 'creates a failed evidence submission record' do + subject.within_sidekiq_retries_exhausted_block(args) do + byebug + expect(EvidenceSubmission.va_notify_email_not_sent.length).to equal(1) + # expect do + # post :create, + # params: { job_id:, + # job_class:, + # claim_id:, + # tracked_item_id:, + # upload_status: BenefitsDocuments::Constants::UPLOAD_STATUS[:FAILED], + # user_account:, + # template_metadata_ciphertext: { personalisation: }.to_json } + # end.to change(EvidenceSubmission.va_notify_email_not_sent, :count).by(1) end end end diff --git a/spec/sidekiq/evss/failure_notification_spec.rb b/spec/sidekiq/evss/failure_notification_spec.rb deleted file mode 100644 index e64e65aa28b..00000000000 --- a/spec/sidekiq/evss/failure_notification_spec.rb +++ /dev/null @@ -1,55 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -require 'evss/failure_notification' -require 'va_notify/service' - -RSpec.describe EVSS::FailureNotification, type: :job do - subject { described_class } - - let(:notify_client_stub) { instance_double(VaNotify::Service) } - let(:user_account) { create(:user_account) } - let(:filename) { 'docXXXX-XXte.pdf' } - let(:icn) { user_account.icn } - let(:first_name) { 'Bob' } - let(:issue_instant) { Time.now.to_i } - let(:formatted_submit_date) do - # We want to return all times in EDT - timestamp = Time.at(issue_instant).in_time_zone('America/New_York') - - # We display dates in mailers in the format "May 1, 2024 3:01 p.m. EDT" - timestamp.strftime('%B %-d, %Y %-l:%M %P %Z').sub(/([ap])m/, '\1.m.') - end - let(:date_submitted) { formatted_submit_date } - let(:date_failed) { formatted_submit_date } - - before do - allow(Rails.logger).to receive(:info) - end - - context 'when EVSS::FailureNotification is called' do - it 'enqueues a failure notification mailer to send to the veteran' do - allow(VaNotify::Service).to receive(:new) { notify_client_stub } - - subject.perform_async(icn, first_name, filename, date_submitted, date_failed) do - expect(notify_client_stub).to receive(:send_email).with( - { - recipient_identifier: { id_value: user_account.icn, id_type: 'ICN' }, - template_id: 'fake_template_id', - personalisation: { - first_name: 'Bob', - filename: 'docXXXX-XXte.pdf', - date_submitted: formatted_submit_date, - date_failed: formatted_submit_date - } - } - ) - - expect(Rails.logger) - .to receive(:info) - .with('EVSS::FailureNotification email sent') - end - end - end -end diff --git a/spec/sidekiq/lighthouse/benefits_documents/document_upload_spec.rb b/spec/sidekiq/lighthouse/benefits_documents/document_upload_spec.rb new file mode 100644 index 00000000000..c0a0f048d28 --- /dev/null +++ b/spec/sidekiq/lighthouse/benefits_documents/document_upload_spec.rb @@ -0,0 +1,140 @@ +# frozen_string_literal: true + +require 'rails_helper' +require 'sidekiq/testing' +Sidekiq::Testing.fake! + +require 'lighthouse/benefits_documents/document_upload' +require 'va_notify/service' +require 'lighthouse/benefits_documents/constants' + +RSpec.describe Lighthouse::BenefitsDocuments::DocumentUpload, type: :job do + subject(:job) do + described_class.perform_async(user_icn, + document_data.to_serializable_hash, + user_account_uuid, claim_id, + tracked_item_ids) + end + + let(:client_stub) { instance_double(BenefitsDocuments::WorkerService) } + let(:job_id) { job } + let(:evidence_submission_stub) do + evidence_submission = EvidenceSubmission.new(claim_id: '4567', + tracked_item_id: tracked_item_ids, + job_id: job_id, + job_class: described_class, + upload_status: 'pending') + evidence_submission.user_account = user_account + evidence_submission.save! + evidence_submission + end + let(:formatted_submit_date) do + # We want to return all times in EDT + timestamp = Time.at(issue_instant).in_time_zone('America/New_York') + + # We display dates in mailers in the format "May 1, 2024 3:01 p.m. EDT" + timestamp.strftime('%B %-d, %Y %-l:%M %P %Z').sub(/([ap])m/, '\1.m.') + end + let(:notify_client_stub) { instance_double(VaNotify::Service) } + let(:uploader_stub) { instance_double(LighthouseDocumentUploader) } + let(:user_account) { create(:user_account) } + let(:user_account_uuid) { user_account.id } + let(:filename) { 'doctors-note.pdf' } + let(:file) { Rails.root.join('spec', 'fixtures', 'files', filename).read } + let(:user_icn) { user_account.icn } + let(:tracked_item_ids) { '1234' } + let(:document_type) { 'L029' } + let(:password) { 'Password_123' } + let(:claim_id) { '4567' } + let(:job_class) { 'Lighthouse::DocumentUpload' } + let(:document_data) do + LighthouseDocument.new( + first_name: 'First Name', + participant_id: '1111', + claim_id: claim_id, + # file_obj: file, + uuid: SecureRandom.uuid, + file_extension: 'pdf', + file_name: filename, + tracked_item_id: tracked_item_ids, + document_type: + ) + end + let(:response) do + { + data: { + success: true, + requestId: '12345678' + } + } + end + let(:failure_response) do + { + data: { + success: false + } + } + end + + let(:issue_instant) { Time.now.to_i } + let(:args) do + { + 'args' => [user_account.icn, { 'file_name' => filename, 'first_name' => 'Bob' }], + 'created_at' => issue_instant, + 'failed_at' => issue_instant + } + end + let(:tags) { described_class::DD_ZSF_TAGS } + + before do + allow(Rails.logger).to receive(:info) + allow(StatsD).to receive(:increment) + end + + + it 'calls Lighthouse::FailureNotification' do + subject.within_sidekiq_retries_exhausted_block(args) do + expect(Lighthouse::FailureNotification).to receive(:perform_async).with( + user_account.icn, + 'Bob', # first_name + 'docXXXX-XXte.pdf', # filename + formatted_submit_date, # date_submitted + formatted_submit_date # date_failed + ) + + expect(Rails.logger) + .to receive(:info) + .with('Lighthouse::DocumentUpload exhaustion handler email queued') + expect(StatsD).to receive(:increment).with('silent_failure_avoided_no_confirmation', tags:) + end + end + + it 'retrieves the file and uploads to Lighthouse' do + allow(LighthouseDocumentUploader).to receive(:new) { uploader_stub } + allow(BenefitsDocuments::WorkerService).to receive(:new) { client_stub } + allow(uploader_stub).to receive(:retrieve_from_store!).with(filename) { file } + allow(uploader_stub).to receive(:read_for_upload) { file } + allow(client_stub).to receive(:upload_document).with(file, document_data) + expect(uploader_stub).to receive(:remove!).once + expect(client_stub).to receive(:upload_document).with(file, document_data).and_return(response) + allow(EvidenceSubmission).to receive(:find_or_create_by) + .with({ claim_id:, + tracked_item_id: tracked_item_ids, + job_id:, + job_class:, + upload_status: BenefitsDocuments::Constants::UPLOAD_STATUS[:PENDING] }) + .and_return(evidence_submission_stub) + described_class.drain # runs all queued jobs of this class + # After running DocumentUpload job, there should be a new EvidenceSubmission record + # with the response request_id + expect(EvidenceSubmission.find_by(job_id: job_id).request_id).to eql(response.dig(:data, :requestId)) + end + + it 'raises an error when Lighthouse returns a failure response' do + allow(client_stub).to receive(:upload_document).with(file, document_data).and_return(failure_response) + expect do + job + described_class.drain + end.to raise_error(StandardError) + end +end diff --git a/spec/sidekiq/lighthouse/failure_notification_spec.rb b/spec/sidekiq/lighthouse/benefits_documents/failure_notification_email_job_spec.rb similarity index 91% rename from spec/sidekiq/lighthouse/failure_notification_spec.rb rename to spec/sidekiq/lighthouse/benefits_documents/failure_notification_email_job_spec.rb index 222681ee9bf..ebf5639dc8f 100644 --- a/spec/sidekiq/lighthouse/failure_notification_spec.rb +++ b/spec/sidekiq/lighthouse/benefits_documents/failure_notification_email_job_spec.rb @@ -2,10 +2,10 @@ require 'rails_helper' -require 'lighthouse/failure_notification' +require 'lighthouse/benefits_documents/failure_notification' require 'va_notify/service' -RSpec.describe Lighthouse::FailureNotification, type: :job do +RSpec.describe Lighthouse::BenefitsDocuments::FailureNotificationEmailJob, type: :job do subject { described_class } let(:notify_client_stub) { instance_double(VaNotify::Service) } @@ -52,4 +52,4 @@ end end end -end +end \ No newline at end of file diff --git a/spec/sidekiq/lighthouse/document_upload_spec.rb b/spec/sidekiq/lighthouse/document_upload_spec.rb deleted file mode 100644 index b0ee8bf0b51..00000000000 --- a/spec/sidekiq/lighthouse/document_upload_spec.rb +++ /dev/null @@ -1,162 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' -require 'sidekiq/testing' -Sidekiq::Testing.fake! - -require 'lighthouse/document_upload' -require 'va_notify/service' -require 'lighthouse/benefits_documents/constants' - -RSpec.describe Lighthouse::DocumentUpload, type: :job do - subject(:job) do - described_class.perform_async(user_icn, - document_data.to_serializable_hash, - user_account_uuid, claim_id, - tracked_item_ids) - end - - let(:client_stub) { instance_double(BenefitsDocuments::WorkerService) } - let(:notify_client_stub) { instance_double(VaNotify::Service) } - let(:uploader_stub) { instance_double(LighthouseDocumentUploader) } - let(:user_account) { create(:user_account) } - let(:user_account_uuid) { user_account.id } - let(:filename) { 'doctors-note.pdf' } - let(:file) { Rails.root.join('spec', 'fixtures', 'files', filename).read } - let(:user_icn) { user_account.icn } - let(:tracked_item_ids) { '1234' } - let(:document_type) { 'L029' } - let(:password) { 'Password_123' } - let(:claim_id) { '4567' } - let(:job_class) { 'Lighthouse::DocumentUpload' } - let(:document_data) do - LighthouseDocument.new( - first_name: 'First Name', - participant_id: '1111', - claim_id: claim_id, - # file_obj: file, - uuid: SecureRandom.uuid, - file_extension: 'pdf', - file_name: filename, - tracked_item_id: tracked_item_ids, - document_type: - ) - end - let(:response) do - { - data: { - success: true, - requestId: '12345678' - } - } - end - let(:failure_response) do - { - data: { - success: false - } - } - end - - let(:issue_instant) { Time.now.to_i } - let(:args) do - { - 'args' => [user_account.icn, { 'file_name' => filename, 'first_name' => 'Bob' }], - 'created_at' => issue_instant, - 'failed_at' => issue_instant - } - end - let(:tags) { described_class::DD_ZSF_TAGS } - - before do - allow(Rails.logger).to receive(:info) - allow(StatsD).to receive(:increment) - end - - context 'when cst_send_evidence_failure_emails is enabled' do - before do - Flipper.enable(:cst_send_evidence_failure_emails) - allow(Lighthouse::FailureNotification).to receive(:perform_async) - end - - let(:job_id) { job } - let(:evidence_submission_stub) do - evidence_submission = EvidenceSubmission.new(claim_id: '4567', - tracked_item_id: tracked_item_ids, - job_id: job_id, - job_class: described_class, - upload_status: 'pending') - evidence_submission.user_account = user_account - evidence_submission.save! - evidence_submission - end - - let(:formatted_submit_date) do - # We want to return all times in EDT - timestamp = Time.at(issue_instant).in_time_zone('America/New_York') - - # We display dates in mailers in the format "May 1, 2024 3:01 p.m. EDT" - timestamp.strftime('%B %-d, %Y %-l:%M %P %Z').sub(/([ap])m/, '\1.m.') - end - - it 'calls Lighthouse::FailureNotification' do - subject.within_sidekiq_retries_exhausted_block(args) do - expect(Lighthouse::FailureNotification).to receive(:perform_async).with( - user_account.icn, - 'Bob', # first_name - 'docXXXX-XXte.pdf', # filename - formatted_submit_date, # date_submitted - formatted_submit_date # date_failed - ) - - expect(Rails.logger) - .to receive(:info) - .with('Lighthouse::DocumentUpload exhaustion handler email queued') - expect(StatsD).to receive(:increment).with('silent_failure_avoided_no_confirmation', tags:) - end - end - - it 'retrieves the file and uploads to Lighthouse' do - allow(LighthouseDocumentUploader).to receive(:new) { uploader_stub } - allow(BenefitsDocuments::WorkerService).to receive(:new) { client_stub } - allow(uploader_stub).to receive(:retrieve_from_store!).with(filename) { file } - allow(uploader_stub).to receive(:read_for_upload) { file } - allow(client_stub).to receive(:upload_document).with(file, document_data) - expect(uploader_stub).to receive(:remove!).once - expect(client_stub).to receive(:upload_document).with(file, document_data).and_return(response) - allow(EvidenceSubmission).to receive(:find_or_create_by) - .with({ claim_id:, - tracked_item_id: tracked_item_ids, - job_id:, - job_class:, - upload_status: BenefitsDocuments::Constants::UPLOAD_STATUS[:PENDING] }) - .and_return(evidence_submission_stub) - described_class.drain # runs all queued jobs of this class - # After running DocumentUpload job, there should be a new EvidenceSubmission record - # with the response request_id - expect(EvidenceSubmission.find_by(job_id: job_id).request_id).to eql(response.dig(:data, :requestId)) - end - - it 'raises an error when Lighthouse returns a failure response' do - allow(client_stub).to receive(:upload_document).with(file, document_data).and_return(failure_response) - expect do - job - described_class.drain - end.to raise_error(StandardError) - end - end - - context 'when cst_send_evidence_failure_emails is disabled' do - before do - Flipper.disable(:cst_send_evidence_failure_emails) - end - - let(:issue_instant) { Time.now.to_i } - - it 'does not call Lighthouse::Failure Notification' do - subject.within_sidekiq_retries_exhausted_block(args) do - expect(Lighthouse::FailureNotification).not_to receive(:perform_async) - end - end - end -end From 34d292ba781106f23da9c5bb6b5ca018cbf8fd65 Mon Sep 17 00:00:00 2001 From: Peri McLaren <141954992+pmclaren19@users.noreply.github.com> Date: Thu, 19 Dec 2024 09:52:14 -0700 Subject: [PATCH 02/13] add tests --- app/sidekiq/evss/document_upload.rb | 20 ++- .../benefits_documents/document_upload.rb | 4 + .../failure_notification_email_job.rb | 15 ++- .../pensions/pension_benefit_intake_job.rb | 1 + .../benefits_documents/evidence_submission.rb | 21 ++++ spec/sidekiq/evss/document_upload_spec.rb | 116 +++++++++--------- .../document_upload_spec.rb | 2 - 7 files changed, 100 insertions(+), 79 deletions(-) create mode 100644 spec/factories/lighthouse/benefits_documents/evidence_submission.rb diff --git a/app/sidekiq/evss/document_upload.rb b/app/sidekiq/evss/document_upload.rb index 8da03f1854f..b709b6fcf8b 100644 --- a/app/sidekiq/evss/document_upload.rb +++ b/app/sidekiq/evss/document_upload.rb @@ -12,6 +12,7 @@ class EVSS::DocumentUpload FILENAME_EXTENSION_MATCHER = /\.\w*$/ OBFUSCATED_CHARACTER_MATCHER = /[a-zA-Z\d]/ + DD_ZSF_TAGS = ['service:claim-status', 'function: evidence upload to EVSS'].freeze attr_accessor :auth_headers, :user_uuid, :document_hash @@ -28,16 +29,13 @@ class EVSS::DocumentUpload ) # retry for one day - sidekiq_options retry: 0, queue: 'low' + sidekiq_options retry: 16, queue: 'low' # Set minimum retry time to ~1 hour - # sidekiq_retry_in do |count, _exception| - # rand(3600..3660) if count < 9 - # end + sidekiq_retry_in do |count, _exception| + rand(3600..3660) if count < 9 + end sidekiq_retries_exhausted do |msg, _ex| - byebug - puts 'hello test' - job_id = msg['jid'] job_class = 'EVSS::DocumentUpload' first_name = msg['args'][0]['va_eauth_firstName'].titleize @@ -50,7 +48,7 @@ class EVSS::DocumentUpload uuid = msg['args'][2]['uuid'] user_account = UserAccount.find_or_create_by(id: uuid) personalisation = { first_name:, filename:, date_submitted:, date_failed: } - puts 'peri test' + EvidenceSubmission.create( job_id:, job_class:, @@ -60,10 +58,10 @@ class EVSS::DocumentUpload user_account:, template_metadata_ciphertext: { personalisation: }.to_json ) - puts 'peri test2' rescue => e - puts 'there was an error' - puts e + error_message = "#{job_class} failed to create EvidenceSubmission" + ::Rails.logger.info(error_message, { messsage: e.message }) + StatsD.increment('silent_failure', tags: ['service:claim-status', "function: #{error_message}"]) end def perform(auth_headers, user_uuid, document_hash) diff --git a/app/sidekiq/lighthouse/benefits_documents/document_upload.rb b/app/sidekiq/lighthouse/benefits_documents/document_upload.rb index 89f3ee923bd..e700e490236 100644 --- a/app/sidekiq/lighthouse/benefits_documents/document_upload.rb +++ b/app/sidekiq/lighthouse/benefits_documents/document_upload.rb @@ -45,6 +45,10 @@ class Lighthouse::BenefitsDocuments::DocumentUpload user_account:, template_metadata_ciphertext: { personalisation: }.to_json ) + rescue => e + error_message = "#{job_class} failed to create EvidenceSubmission" + ::Rails.logger.info(error_message, { messsage: e.message }) + StatsD.increment('silent_failure', tags: ['service:claim-status', "function: #{error_message}"]) end def self.obscured_filename(original_filename) diff --git a/app/sidekiq/lighthouse/benefits_documents/failure_notification_email_job.rb b/app/sidekiq/lighthouse/benefits_documents/failure_notification_email_job.rb index 707956628df..e0794b92fb3 100644 --- a/app/sidekiq/lighthouse/benefits_documents/failure_notification_email_job.rb +++ b/app/sidekiq/lighthouse/benefits_documents/failure_notification_email_job.rb @@ -9,11 +9,8 @@ class FailureNotificationEmailJob FAILED_STATUS = BenefitsDocuments::Constants::UPLOAD_STATUS[:FAILED] MAILER_TEMPLATE_ID = NOTIFY_SETTINGS.template_id.evidence_submission_failure_email - DD_ZSF_TAGS = ['service:claim-status', 'function: evidence upload to Lighthouse'].freeze - # TODO: need to add statsd logic # STATSD_KEY_PREFIX = '' - # def perform return unless should_perform? @@ -54,14 +51,16 @@ def send_failed_evidence_submissions def record_email_send_success(upload, response) EvidenceSubmission.update(id: upload.id, va_notify_id: response.id, va_notify_date: DateTime.now) - ::Rails.logger.info("#{upload.job_class} va notify failure email sent") - StatsD.increment('silent_failure_avoided_no_confirmation', tags: DD_ZSF_TAGS) + error_message = "#{upload.job_class} va notify failure email queued" + ::Rails.logger.info(error_message) + StatsD.increment('silent_failure_avoided_no_confirmation', + tags: ['service:claim-status', "function: #{error_message}"]) end def record_email_send_failure(upload, error) - ::Rails.logger.error("#{upload.job_class} va notify failure email failed to send", - { message: error.message }) - StatsD.increment('silent_failure', tags: DD_ZSF_TAGS) + error_message = "#{upload.job_class} va notify failure email errored" + ::Rails.logger.error(error_message, { message: error.message }) + StatsD.increment('silent_failure', tags: ['service:claim-status', "function: #{error_message}"]) log_exception_to_sentry(e) end diff --git a/modules/pensions/app/sidekiq/pensions/pension_benefit_intake_job.rb b/modules/pensions/app/sidekiq/pensions/pension_benefit_intake_job.rb index 6c4901a3dfd..224f220bed2 100644 --- a/modules/pensions/app/sidekiq/pensions/pension_benefit_intake_job.rb +++ b/modules/pensions/app/sidekiq/pensions/pension_benefit_intake_job.rb @@ -33,6 +33,7 @@ class PensionBenefitIntakeError < StandardError; end # retry exhaustion sidekiq_retries_exhausted do |msg| + byebug begin claim = Pensions::SavedClaim.find(msg['args'].first) rescue diff --git a/spec/factories/lighthouse/benefits_documents/evidence_submission.rb b/spec/factories/lighthouse/benefits_documents/evidence_submission.rb new file mode 100644 index 00000000000..0e57a52b977 --- /dev/null +++ b/spec/factories/lighthouse/benefits_documents/evidence_submission.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'lighthouse/benefits_documents/constants' + +FactoryBot.define do + factory :bd_evidence_submission, class: 'EvidenceSubmission' do + association :user_account, factory: :user_account + created_at { DateTime.now.utc } + end + + factory :bd_evidence_submission_timeout, class: 'EvidenceSubmission' do + association :user_account, factory: :user_account + created_at { DateTime.new(1985, 10, 26).utc } + end + + factory :bd_evidence_submission_failed, class: 'EvidenceSubmission' do + association :user_account, factory: :user_account + created_at { DateTime.now.utc } + upload_status { BenefitsDocuments::Constants::UPLOAD_STATUS[:FAILED] } + end +end diff --git a/spec/sidekiq/evss/document_upload_spec.rb b/spec/sidekiq/evss/document_upload_spec.rb index a722536d803..cfc5311a12b 100644 --- a/spec/sidekiq/evss/document_upload_spec.rb +++ b/spec/sidekiq/evss/document_upload_spec.rb @@ -11,30 +11,15 @@ subject(:job) do described_class.perform_async(auth_headers, user.uuid, document_data.to_serializable_hash) end - # subject { described_class } - let(:client_stub) { instance_double('EVSS::DocumentsService') } - let(:job_id) { job } - let(:job_class) { 'EVSS::DocumentUpload' } - let(:claim_id) { '4567' } - let(:tracked_item_id) { '1234' } - let(:document_type) { 'L023' } - - let(:formatted_submit_date) do - # We want to return all times in EDT - timestamp = Time.at(issue_instant).in_time_zone('America/New_York') - - # We display dates in mailers in the format "May 1, 2024 3:01 p.m. EDT" - timestamp.strftime('%B %-d, %Y %-l:%M %P %Z').sub(/([ap])m/, '\1.m.') - end - let(:created_at) { DateTime.new(2023, 4, 2) } - - let(:notify_client_stub) { instance_double(VaNotify::Service) } - let(:uploader_stub) { instance_double('EVSSClaimDocumentUploader') } + let(:auth_headers) { EVSS::AuthHeaders.new(user).to_h } + let(:user) { FactoryBot.create(:user, :loa3) } let(:user_account) { create(:user_account) } let(:user_account_uuid) { user_account.id } - let(:user) { FactoryBot.create(:user, :loa3) } + let(:claim_id) { '4567' } let(:filename) { 'doctors-note.pdf' } + let(:tracked_item_id) { '1234' } + let(:document_type) { 'L023' } let(:document_data) do EVSSClaimDocument.new( evss_claim_id: claim_id, @@ -43,52 +28,67 @@ document_type: ) end - let(:personalisation) do - { first_name: user.first_name, - filename:, - date_submitted: created_at.strftime('%B %d, %Y'), - date_failed: created_at.strftime('%B %d, %Y') } - end - let(:auth_headers) { EVSS::AuthHeaders.new(user).to_h } + let(:job_id) { job } - let(:issue_instant) { Time.now.to_i } - let(:args) do - { - 'args' => [{ 'va_eauth_firstName' => 'Bob' }, user_account_uuid, { 'file_name' => filename }], - 'created_at' => issue_instant, - 'failed_at' => issue_instant - } - end + let(:client_stub) { instance_double('EVSS::DocumentsService') } + let(:notify_client_stub) { instance_double(VaNotify::Service) } before do - allow(Rails.logger).to receive(:info) + allow(StatsD).to receive(:increment) end - it 'retrieves the file and uploads to EVSS' do - allow(EVSSClaimDocumentUploader).to receive(:new) { uploader_stub } - allow(EVSS::DocumentsService).to receive(:new) { client_stub } - file = File.read("#{::Rails.root}/spec/fixtures/files/#{filename}") - allow(uploader_stub).to receive(:retrieve_from_store!).with(filename) { file } - allow(uploader_stub).to receive(:read_for_upload) { file } - expect(uploader_stub).to receive(:remove!).once - expect(client_stub).to receive(:upload).with(file, document_data) - described_class.new.perform(auth_headers, user.uuid, document_data.to_serializable_hash) + context 'when upload succeeds' do + let(:uploader_stub) { instance_double('EVSSClaimDocumentUploader') } + let(:file) { Rails.root.join('spec', 'fixtures', 'files', filename).read } + + it 'retrieves the file and uploads to EVSS' do + allow(EVSSClaimDocumentUploader).to receive(:new) { uploader_stub } + allow(EVSS::DocumentsService).to receive(:new) { client_stub } + allow(uploader_stub).to receive(:retrieve_from_store!).with(filename) { file } + allow(uploader_stub).to receive(:read_for_upload) { file } + expect(uploader_stub).to receive(:remove!).once + expect(client_stub).to receive(:upload).with(file, document_data) + described_class.new.perform(auth_headers, user.uuid, document_data.to_serializable_hash) + end end - it 'creates a failed evidence submission record' do - subject.within_sidekiq_retries_exhausted_block(args) do - byebug + context 'when upload fails' do + let(:issue_instant) { Time.now.to_i } + let(:msg) do + { + 'args' => [{ 'va_eauth_firstName' => 'Bob' }, user_account_uuid, { 'file_name' => filename }], + 'created_at' => issue_instant, + 'failed_at' => issue_instant + } + end + let(:msg_with_errors) do + { + 'args' => [{ 'va_eauth_firstName' => 'Bob' }, 'test', user_account_uuid, { 'file_name' => filename }], + 'created_at' => issue_instant, + 'failed_at' => issue_instant + } + end + let(:evidence_submission_failed) { create(:bd_evidence_submission_failed) } + let(:job_class) { 'EVSS::DocumentUpload' } + let(:error_message) { "#{job_class} failed to create EvidenceSubmission" } + let(:tags) { ['service:claim-status', "function: #{error_message}"] } + + it 'creates a failed evidence submission record' do + EVSS::DocumentUpload.within_sidekiq_retries_exhausted_block(msg) do + expect(EvidenceSubmission).to receive(:create).and_return(evidence_submission_failed) + end expect(EvidenceSubmission.va_notify_email_not_sent.length).to equal(1) - # expect do - # post :create, - # params: { job_id:, - # job_class:, - # claim_id:, - # tracked_item_id:, - # upload_status: BenefitsDocuments::Constants::UPLOAD_STATUS[:FAILED], - # user_account:, - # template_metadata_ciphertext: { personalisation: }.to_json } - # end.to change(EvidenceSubmission.va_notify_email_not_sent, :count).by(1) + end + + it 'fails to create a failed evidence submission record when args malformed' do + EVSS::DocumentUpload.within_sidekiq_retries_exhausted_block(msg_with_errors) do + expect(EvidenceSubmission).not_to receive(:create) + expect(Rails.logger) + .to receive(:info) + .with('EVSS::DocumentUpload failed to create EvidenceSubmission', + { messsage: "undefined method `[]' for nil" }) + expect(StatsD).to receive(:increment).with('silent_failure', tags: tags) + end end end end diff --git a/spec/sidekiq/lighthouse/benefits_documents/document_upload_spec.rb b/spec/sidekiq/lighthouse/benefits_documents/document_upload_spec.rb index c0a0f048d28..0ef4fe5bae5 100644 --- a/spec/sidekiq/lighthouse/benefits_documents/document_upload_spec.rb +++ b/spec/sidekiq/lighthouse/benefits_documents/document_upload_spec.rb @@ -87,11 +87,9 @@ let(:tags) { described_class::DD_ZSF_TAGS } before do - allow(Rails.logger).to receive(:info) allow(StatsD).to receive(:increment) end - it 'calls Lighthouse::FailureNotification' do subject.within_sidekiq_retries_exhausted_block(args) do expect(Lighthouse::FailureNotification).to receive(:perform_async).with( From 6e9882df9674681c1271697abb9160f7283b00aa Mon Sep 17 00:00:00 2001 From: Peri McLaren <141954992+pmclaren19@users.noreply.github.com> Date: Thu, 19 Dec 2024 11:47:10 -0700 Subject: [PATCH 03/13] update tests --- app/sidekiq/evss/document_upload.rb | 1 + app/sidekiq/lighthouse/benefits_documents/document_upload.rb | 1 + lib/lighthouse/benefits_claims/service.rb | 4 +--- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/sidekiq/evss/document_upload.rb b/app/sidekiq/evss/document_upload.rb index b709b6fcf8b..3112816106b 100644 --- a/app/sidekiq/evss/document_upload.rb +++ b/app/sidekiq/evss/document_upload.rb @@ -62,6 +62,7 @@ class EVSS::DocumentUpload error_message = "#{job_class} failed to create EvidenceSubmission" ::Rails.logger.info(error_message, { messsage: e.message }) StatsD.increment('silent_failure', tags: ['service:claim-status', "function: #{error_message}"]) + log_exception_to_sentry(e) end def perform(auth_headers, user_uuid, document_hash) diff --git a/app/sidekiq/lighthouse/benefits_documents/document_upload.rb b/app/sidekiq/lighthouse/benefits_documents/document_upload.rb index e700e490236..c8075c2086a 100644 --- a/app/sidekiq/lighthouse/benefits_documents/document_upload.rb +++ b/app/sidekiq/lighthouse/benefits_documents/document_upload.rb @@ -49,6 +49,7 @@ class Lighthouse::BenefitsDocuments::DocumentUpload error_message = "#{job_class} failed to create EvidenceSubmission" ::Rails.logger.info(error_message, { messsage: e.message }) StatsD.increment('silent_failure', tags: ['service:claim-status', "function: #{error_message}"]) + log_exception_to_sentry(e) end def self.obscured_filename(original_filename) diff --git a/lib/lighthouse/benefits_claims/service.rb b/lib/lighthouse/benefits_claims/service.rb index 625c7c84ace..594627bb54c 100644 --- a/lib/lighthouse/benefits_claims/service.rb +++ b/lib/lighthouse/benefits_claims/service.rb @@ -13,9 +13,7 @@ class Service < Common::Client::Base FILTERED_STATUSES = %w[CANCELED ERRORED PENDING].freeze def initialize(icn) - # @icn = icn - # @icn = '1012830774V793840' # icn for user 23 - @icn = '1012830712V627751' # icn for user 19 + @icn = icn if icn.blank? raise ArgumentError, 'no ICN passed in for LH API request.' else From 8f7005613f5ee793ce2706d8df973eb5a8c8b42a Mon Sep 17 00:00:00 2001 From: Peri McLaren <141954992+pmclaren19@users.noreply.github.com> Date: Thu, 19 Dec 2024 11:54:20 -0700 Subject: [PATCH 04/13] remove byebug --- .../pensions/app/sidekiq/pensions/pension_benefit_intake_job.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/pensions/app/sidekiq/pensions/pension_benefit_intake_job.rb b/modules/pensions/app/sidekiq/pensions/pension_benefit_intake_job.rb index 224f220bed2..6c4901a3dfd 100644 --- a/modules/pensions/app/sidekiq/pensions/pension_benefit_intake_job.rb +++ b/modules/pensions/app/sidekiq/pensions/pension_benefit_intake_job.rb @@ -33,7 +33,6 @@ class PensionBenefitIntakeError < StandardError; end # retry exhaustion sidekiq_retries_exhausted do |msg| - byebug begin claim = Pensions::SavedClaim.find(msg['args'].first) rescue From 9e84aa94986d3cfadeb16b11f83bdf24d47afb5d Mon Sep 17 00:00:00 2001 From: Peri McLaren <141954992+pmclaren19@users.noreply.github.com> Date: Thu, 19 Dec 2024 12:26:45 -0700 Subject: [PATCH 05/13] added codeowners and removed files that were deleted --- .github/CODEOWNERS | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index c2f51d6c084..f60c3d4abc0 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -631,7 +631,6 @@ app/sidekiq/evss/delete_old_claims.rb @department-of-veterans-affairs/mbs-core-t app/sidekiq/evss/dependents_application_job.rb @department-of-veterans-affairs/benefits-dependents-management @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group app/sidekiq/evss/disability_compensation_form @department-of-veterans-affairs/Disability-Experience @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group app/sidekiq/evss/document_upload.rb @department-of-veterans-affairs/benefits-management-tools-be @department-of-veterans-affairs/backend-review-group -app/sidekiq/evss/failure_notification.rb @department-of-veterans-affairs/benefits-management-tools-be @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group @department-of-veterans-affairs/benefits-management-tools-be app/sidekiq/evss/disability_compensation_form/form526_document_upload_failure_email.rb @department-of-veterans-affairs/Disability-Experience @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group app/sidekiq/evss/retrieve_claims_from_remote_job.rb @department-of-veterans-affairs/benefits-management-tools-be @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group app/sidekiq/evss/request_decision.rb @department-of-veterans-affairs/benefits-management-tools-be @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group @@ -655,10 +654,10 @@ app/sidekiq/income_limits @department-of-veterans-affairs/vfs-public-websites-fr app/sidekiq/in_progress_form_cleaner.rb @department-of-veterans-affairs/vfs-authenticated-experience-backend @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group app/sidekiq/kms_key_rotation @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group app/sidekiq/lighthouse @department-of-veterans-affairs/backend-review-group +app/sidekiq/lighthouse/benefits_documents/failure_notification_email_job.rb @department-of-veterans-affairs/benefits-management-tools-be, @department-of-veterans-affairs/backend-review-group and @department-of-veterans-affairs/va-api-engineers app/sidekiq/lighthouse/submit_career_counseling_job.rb @department-of-veterans-affairs/my-education-benefits @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group app/sidekiq/lighthouse/create_intent_to_file_job.rb @department-of-veterans-affairs/pension-and-burials @department-of-veterans-affairs/backend-review-group app/sidekiq/lighthouse/income_and_assets_intake_job.rb @department-of-veterans-affairs/pension-and-burials @department-of-veterans-affairs/backend-review-group -app/sidekiq/lighthouse/failure_notification.rb @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group @department-of-veterans-affairs/benefits-management-tools-be app/sidekiq/load_average_days_for_claim_completion_job.rb @department-of-veterans-affairs/benefits-microservices @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group app/sidekiq/mhv @department-of-veterans-affairs/vfs-mhv-medical-records @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group app/sidekiq/mhv/account_creator_job.rb @department-of-veterans-affairs/octo-identity @@ -1219,6 +1218,7 @@ spec/factories/gi_bill_status_response.rb @department-of-veterans-affairs/govcio spec/factories/gids_response.rb @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group spec/factories/hca_attachment.rb @department-of-veterans-affairs/vfs-authenticated-experience-backend @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group spec/factories/health_care_application.rb @department-of-veterans-affairs/vfs-authenticated-experience-backend @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group +spec/factories/lighthouse/benefits_documents/evidence_submission.rb @department-of-veterans-affairs/benefits-management-tools-be, @department-of-veterans-affairs/backend-review-group and @department-of-veterans-affairs/va-api-engineers spec/factories/iam_introspection_responses.rb @department-of-veterans-affairs/octo-identity spec/factories/iam_user_identities.rb @department-of-veterans-affairs/octo-identity spec/factories/iam_users.rb @department-of-veterans-affairs/octo-identity @@ -1386,6 +1386,7 @@ spec/sidekiq/income_limits @department-of-veterans-affairs/vfs-public-websites-f spec/sidekiq/in_progress_form_cleaner_spec.rb @department-of-veterans-affairs/vfs-authenticated-experience-backend @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group spec/sidekiq/kms_key_rotation @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group spec/sidekiq/lighthouse @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group +spec/sidekiq/lighthouse/benefits_documents/document_upload_spec.rb @department-of-veterans-affairs/benefits-management-tools-be, @department-of-veterans-affairs/backend-review-group and @department-of-veterans-affairs/va-api-engineers spec/sidekiq/lighthouse/failure_notification_spec.rb @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group @department-of-veterans-affairs/benefits-management-tools-be spec/sidekiq/load_average_days_for_claim_completion_job_spec.rb @department-of-veterans-affairs/benefits-microservices @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group spec/sidekiq/mhv @department-of-veterans-affairs/vfs-mhv-medical-records @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group From 8d1d2b5db88a1891ac68db4fab787c824c675a07 Mon Sep 17 00:00:00 2001 From: Peri McLaren <141954992+pmclaren19@users.noreply.github.com> Date: Thu, 19 Dec 2024 13:44:25 -0700 Subject: [PATCH 06/13] add tests --- app/sidekiq/evss/document_upload.rb | 2 +- .../benefits_documents/document_upload.rb | 2 +- spec/sidekiq/evss/document_upload_spec.rb | 7 +- .../document_upload_spec.rb | 178 +++++++++--------- 4 files changed, 93 insertions(+), 96 deletions(-) diff --git a/app/sidekiq/evss/document_upload.rb b/app/sidekiq/evss/document_upload.rb index 3112816106b..cd6eaff95a8 100644 --- a/app/sidekiq/evss/document_upload.rb +++ b/app/sidekiq/evss/document_upload.rb @@ -38,7 +38,7 @@ class EVSS::DocumentUpload sidekiq_retries_exhausted do |msg, _ex| job_id = msg['jid'] job_class = 'EVSS::DocumentUpload' - first_name = msg['args'][0]['va_eauth_firstName'].titleize + first_name = msg['args'][0]['va_eauth_firstName'].titleize unless msg['args'][0]['va_eauth_firstName'].nil? claim_id = msg['args'][2]['evss_claim_id'] tracked_item_id = msg['args'][2]['tracked_item_id'] filename = obscured_filename(msg['args'][2]['file_name']) diff --git a/app/sidekiq/lighthouse/benefits_documents/document_upload.rb b/app/sidekiq/lighthouse/benefits_documents/document_upload.rb index c8075c2086a..97146be6adf 100644 --- a/app/sidekiq/lighthouse/benefits_documents/document_upload.rb +++ b/app/sidekiq/lighthouse/benefits_documents/document_upload.rb @@ -25,7 +25,7 @@ class Lighthouse::BenefitsDocuments::DocumentUpload sidekiq_retries_exhausted do |msg, _ex| job_id = msg['jid'] job_class = 'Lighthouse::BenefitsDocuments::DocumentUpload' - first_name = msg['args'][1]['first_name'].titleize + first_name = msg['args'][1]['first_name'].titleize unless msg['args'][1]['first_name'].nil? claim_id = msg['args'][1]['claim_id'] tracked_item_id = msg['args'][1]['tracked_item_id'] filename = obscured_filename(msg['args'][1]['file_name']) diff --git a/spec/sidekiq/evss/document_upload_spec.rb b/spec/sidekiq/evss/document_upload_spec.rb index cfc5311a12b..18b20d32ca2 100644 --- a/spec/sidekiq/evss/document_upload_spec.rb +++ b/spec/sidekiq/evss/document_upload_spec.rb @@ -33,10 +33,6 @@ let(:client_stub) { instance_double('EVSS::DocumentsService') } let(:notify_client_stub) { instance_double(VaNotify::Service) } - before do - allow(StatsD).to receive(:increment) - end - context 'when upload succeeds' do let(:uploader_stub) { instance_double('EVSSClaimDocumentUploader') } let(:file) { Rails.root.join('spec', 'fixtures', 'files', filename).read } @@ -85,8 +81,7 @@ expect(EvidenceSubmission).not_to receive(:create) expect(Rails.logger) .to receive(:info) - .with('EVSS::DocumentUpload failed to create EvidenceSubmission', - { messsage: "undefined method `[]' for nil" }) + .with(error_message, { messsage: "undefined method `[]' for nil" }) expect(StatsD).to receive(:increment).with('silent_failure', tags: tags) end end diff --git a/spec/sidekiq/lighthouse/benefits_documents/document_upload_spec.rb b/spec/sidekiq/lighthouse/benefits_documents/document_upload_spec.rb index 0ef4fe5bae5..a6b029bdd54 100644 --- a/spec/sidekiq/lighthouse/benefits_documents/document_upload_spec.rb +++ b/spec/sidekiq/lighthouse/benefits_documents/document_upload_spec.rb @@ -16,37 +16,11 @@ tracked_item_ids) end - let(:client_stub) { instance_double(BenefitsDocuments::WorkerService) } - let(:job_id) { job } - let(:evidence_submission_stub) do - evidence_submission = EvidenceSubmission.new(claim_id: '4567', - tracked_item_id: tracked_item_ids, - job_id: job_id, - job_class: described_class, - upload_status: 'pending') - evidence_submission.user_account = user_account - evidence_submission.save! - evidence_submission - end - let(:formatted_submit_date) do - # We want to return all times in EDT - timestamp = Time.at(issue_instant).in_time_zone('America/New_York') - - # We display dates in mailers in the format "May 1, 2024 3:01 p.m. EDT" - timestamp.strftime('%B %-d, %Y %-l:%M %P %Z').sub(/([ap])m/, '\1.m.') - end - let(:notify_client_stub) { instance_double(VaNotify::Service) } - let(:uploader_stub) { instance_double(LighthouseDocumentUploader) } - let(:user_account) { create(:user_account) } - let(:user_account_uuid) { user_account.id } - let(:filename) { 'doctors-note.pdf' } - let(:file) { Rails.root.join('spec', 'fixtures', 'files', filename).read } let(:user_icn) { user_account.icn } + let(:claim_id) { '4567' } + let(:filename) { 'doctors-note.pdf' } let(:tracked_item_ids) { '1234' } let(:document_type) { 'L029' } - let(:password) { 'Password_123' } - let(:claim_id) { '4567' } - let(:job_class) { 'Lighthouse::DocumentUpload' } let(:document_data) do LighthouseDocument.new( first_name: 'First Name', @@ -60,79 +34,107 @@ document_type: ) end - let(:response) do - { - data: { - success: true, - requestId: '12345678' - } - } - end - let(:failure_response) do - { - data: { - success: false - } - } - end + let(:user_account) { create(:user_account) } + let(:user_account_uuid) { user_account.id } + let(:job_id) { job } - let(:issue_instant) { Time.now.to_i } - let(:args) do + let(:client_stub) { instance_double(BenefitsDocuments::WorkerService) } + let(:job_class) { 'Lighthouse::BenefitsDocuments::DocumentUpload' } + let(:msg) do { 'args' => [user_account.icn, { 'file_name' => filename, 'first_name' => 'Bob' }], 'created_at' => issue_instant, 'failed_at' => issue_instant } end - let(:tags) { described_class::DD_ZSF_TAGS } + let(:file) { Rails.root.join('spec', 'fixtures', 'files', filename).read } + + context 'when upload succeeds' do + let(:uploader_stub) { instance_double(LighthouseDocumentUploader) } + let(:evidence_submission_stub) do + evidence_submission = EvidenceSubmission.new(claim_id: '4567', + tracked_item_id: tracked_item_ids, + job_id: job_id, + job_class: described_class, + upload_status: 'pending') + evidence_submission.user_account = user_account + evidence_submission.save! + evidence_submission + end + let(:response) do + { + data: { + success: true, + requestId: '12345678' + } + } + end - before do - allow(StatsD).to receive(:increment) + it 'retrieves the file and uploads to Lighthouse' do + allow(LighthouseDocumentUploader).to receive(:new) { uploader_stub } + allow(BenefitsDocuments::WorkerService).to receive(:new) { client_stub } + allow(uploader_stub).to receive(:retrieve_from_store!).with(filename) { file } + allow(uploader_stub).to receive(:read_for_upload) { file } + allow(client_stub).to receive(:upload_document).with(file, document_data) + expect(uploader_stub).to receive(:remove!).once + expect(client_stub).to receive(:upload_document).with(file, document_data).and_return(response) + allow(EvidenceSubmission).to receive(:find_or_create_by) + .with({ claim_id:, + tracked_item_id: tracked_item_ids, + job_id:, + job_class:, + upload_status: BenefitsDocuments::Constants::UPLOAD_STATUS[:PENDING] }) + .and_return(evidence_submission_stub) + described_class.drain # runs all queued jobs of this class + # After running DocumentUpload job, there should be a new EvidenceSubmission record + # with the response request_id + expect(EvidenceSubmission.find_by(job_id: job_id).request_id).to eql(response.dig(:data, :requestId)) + end end - it 'calls Lighthouse::FailureNotification' do - subject.within_sidekiq_retries_exhausted_block(args) do - expect(Lighthouse::FailureNotification).to receive(:perform_async).with( - user_account.icn, - 'Bob', # first_name - 'docXXXX-XXte.pdf', # filename - formatted_submit_date, # date_submitted - formatted_submit_date # date_failed - ) + context 'when upload fails' do + let(:issue_instant) { Time.now.to_i } + let(:msg_with_errors) do + { + 'args' => ['test', user_account.icn, { 'file_name' => filename, 'first_name' => 'Bob' }], + 'created_at' => issue_instant, + 'failed_at' => issue_instant + } + end + let(:failure_response) do + { + data: { + success: false + } + } + end + let(:evidence_submission_failed) { create(:bd_evidence_submission_failed) } + let(:error_message) { "#{job_class} failed to create EvidenceSubmission" } + let(:tags) { ['service:claim-status', "function: #{error_message}"] } - expect(Rails.logger) - .to receive(:info) - .with('Lighthouse::DocumentUpload exhaustion handler email queued') - expect(StatsD).to receive(:increment).with('silent_failure_avoided_no_confirmation', tags:) + it 'creates a failed evidence submission record' do + Lighthouse::BenefitsDocuments::DocumentUpload.within_sidekiq_retries_exhausted_block(msg) do + expect(EvidenceSubmission).to receive(:create).and_return(evidence_submission_failed) + end + expect(EvidenceSubmission.va_notify_email_not_sent.length).to equal(1) end - end - it 'retrieves the file and uploads to Lighthouse' do - allow(LighthouseDocumentUploader).to receive(:new) { uploader_stub } - allow(BenefitsDocuments::WorkerService).to receive(:new) { client_stub } - allow(uploader_stub).to receive(:retrieve_from_store!).with(filename) { file } - allow(uploader_stub).to receive(:read_for_upload) { file } - allow(client_stub).to receive(:upload_document).with(file, document_data) - expect(uploader_stub).to receive(:remove!).once - expect(client_stub).to receive(:upload_document).with(file, document_data).and_return(response) - allow(EvidenceSubmission).to receive(:find_or_create_by) - .with({ claim_id:, - tracked_item_id: tracked_item_ids, - job_id:, - job_class:, - upload_status: BenefitsDocuments::Constants::UPLOAD_STATUS[:PENDING] }) - .and_return(evidence_submission_stub) - described_class.drain # runs all queued jobs of this class - # After running DocumentUpload job, there should be a new EvidenceSubmission record - # with the response request_id - expect(EvidenceSubmission.find_by(job_id: job_id).request_id).to eql(response.dig(:data, :requestId)) - end + it 'fails to create a failed evidence submission record when args malformed' do + Lighthouse::BenefitsDocuments::DocumentUpload.within_sidekiq_retries_exhausted_block(msg_with_errors) do + expect(EvidenceSubmission).not_to receive(:create) + expect(Rails.logger) + .to receive(:info) + .with(error_message, { messsage: "undefined method `[]' for nil" }) + expect(StatsD).to receive(:increment).with('silent_failure', tags: tags) + end + end - it 'raises an error when Lighthouse returns a failure response' do - allow(client_stub).to receive(:upload_document).with(file, document_data).and_return(failure_response) - expect do - job - described_class.drain - end.to raise_error(StandardError) + it 'raises an error when Lighthouse returns a failure response' do + allow(client_stub).to receive(:upload_document).with(file, document_data).and_return(failure_response) + expect do + job + described_class.drain + end.to raise_error(StandardError) + end end end From 0c3279c4233c197dd5f2a25488bfabaaa0714a28 Mon Sep 17 00:00:00 2001 From: Peri McLaren <141954992+pmclaren19@users.noreply.github.com> Date: Thu, 19 Dec 2024 14:10:52 -0700 Subject: [PATCH 07/13] updates --- app/sidekiq/evss/document_upload.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/sidekiq/evss/document_upload.rb b/app/sidekiq/evss/document_upload.rb index cd6eaff95a8..c4e998cb20a 100644 --- a/app/sidekiq/evss/document_upload.rb +++ b/app/sidekiq/evss/document_upload.rb @@ -12,7 +12,6 @@ class EVSS::DocumentUpload FILENAME_EXTENSION_MATCHER = /\.\w*$/ OBFUSCATED_CHARACTER_MATCHER = /[a-zA-Z\d]/ - DD_ZSF_TAGS = ['service:claim-status', 'function: evidence upload to EVSS'].freeze attr_accessor :auth_headers, :user_uuid, :document_hash From b63cc686ada302874e2d06dc99420020e3b2abb7 Mon Sep 17 00:00:00 2001 From: Peri McLaren <141954992+pmclaren19@users.noreply.github.com> Date: Thu, 19 Dec 2024 16:18:10 -0700 Subject: [PATCH 08/13] added tests and factories --- app/models/evidence_submission.rb | 3 + app/sidekiq/evss/document_upload.rb | 2 - .../benefits_documents/document_upload.rb | 2 - .../failure_notification_email_job.rb | 40 +++------- lib/periodic_jobs.rb | 2 +- .../benefits_documents/evidence_submission.rb | 23 ++++++ .../failure_notification_email_job_spec.rb | 2 +- .../document_upload_spec.rb | 17 ++-- .../failure_notification_email_job_spec.rb | 80 +++++++++++++------ 9 files changed, 99 insertions(+), 72 deletions(-) diff --git a/app/models/evidence_submission.rb b/app/models/evidence_submission.rb index 754c15133f0..948a8cab1ca 100644 --- a/app/models/evidence_submission.rb +++ b/app/models/evidence_submission.rb @@ -7,6 +7,9 @@ class EvidenceSubmission < ApplicationRecord has_kms_key has_encrypted :template_metadata, key: :kms_key, **lockbox_options + scope :va_notify_email_sent, lambda { + where('upload_status = ? AND va_notify_date IS NOT NULL', BenefitsDocuments::Constants::UPLOAD_STATUS[:FAILED]) + } scope :va_notify_email_not_sent, lambda { where(upload_status: BenefitsDocuments::Constants::UPLOAD_STATUS[:FAILED], va_notify_date: nil) } diff --git a/app/sidekiq/evss/document_upload.rb b/app/sidekiq/evss/document_upload.rb index c4e998cb20a..2eb29607c84 100644 --- a/app/sidekiq/evss/document_upload.rb +++ b/app/sidekiq/evss/document_upload.rb @@ -7,7 +7,6 @@ class EVSS::DocumentUpload include Sidekiq::Job - extend SentryLogging extend Logging::ThirdPartyTransaction::MethodWrapper FILENAME_EXTENSION_MATCHER = /\.\w*$/ @@ -61,7 +60,6 @@ class EVSS::DocumentUpload error_message = "#{job_class} failed to create EvidenceSubmission" ::Rails.logger.info(error_message, { messsage: e.message }) StatsD.increment('silent_failure', tags: ['service:claim-status', "function: #{error_message}"]) - log_exception_to_sentry(e) end def perform(auth_headers, user_uuid, document_hash) diff --git a/app/sidekiq/lighthouse/benefits_documents/document_upload.rb b/app/sidekiq/lighthouse/benefits_documents/document_upload.rb index 97146be6adf..27f01092407 100644 --- a/app/sidekiq/lighthouse/benefits_documents/document_upload.rb +++ b/app/sidekiq/lighthouse/benefits_documents/document_upload.rb @@ -7,7 +7,6 @@ class Lighthouse::BenefitsDocuments::DocumentUpload include Sidekiq::Job - extend SentryLogging attr_accessor :user_icn, :document_hash @@ -49,7 +48,6 @@ class Lighthouse::BenefitsDocuments::DocumentUpload error_message = "#{job_class} failed to create EvidenceSubmission" ::Rails.logger.info(error_message, { messsage: e.message }) StatsD.increment('silent_failure', tags: ['service:claim-status', "function: #{error_message}"]) - log_exception_to_sentry(e) end def self.obscured_filename(original_filename) diff --git a/app/sidekiq/lighthouse/benefits_documents/failure_notification_email_job.rb b/app/sidekiq/lighthouse/benefits_documents/failure_notification_email_job.rb index e0794b92fb3..7210710750c 100644 --- a/app/sidekiq/lighthouse/benefits_documents/failure_notification_email_job.rb +++ b/app/sidekiq/lighthouse/benefits_documents/failure_notification_email_job.rb @@ -4,10 +4,10 @@ module BenefitsDocuments class FailureNotificationEmailJob include Sidekiq::Job + include SentryLogging sidekiq_options retry: false, unique_for: 30.minutes - - FAILED_STATUS = BenefitsDocuments::Constants::UPLOAD_STATUS[:FAILED] + NOTIFY_SETTINGS = Settings.vanotify.services.benefits_management_tools MAILER_TEMPLATE_ID = NOTIFY_SETTINGS.template_id.evidence_submission_failure_email # TODO: need to add statsd logic # STATSD_KEY_PREFIX = '' @@ -35,14 +35,16 @@ def notify_client def send_failed_evidence_submissions failed_uploads.each do |upload| + byebug response = notify_client.send_email( recipient_identifier: { id_value: upload.user_account.icn, id_type: 'ICN' }, template_id: MAILER_TEMPLATE_ID, personalisation: upload.template_metadata_ciphertext.personalisation ) - + byebug record_email_send_success(upload, response) rescue => e + byebug record_email_send_failure(upload, e) end @@ -50,41 +52,19 @@ def send_failed_evidence_submissions end def record_email_send_success(upload, response) + byebug EvidenceSubmission.update(id: upload.id, va_notify_id: response.id, va_notify_date: DateTime.now) - error_message = "#{upload.job_class} va notify failure email queued" - ::Rails.logger.info(error_message) + message = "#{upload.job_class} va notify failure email queued" + ::Rails.logger.info(message) StatsD.increment('silent_failure_avoided_no_confirmation', - tags: ['service:claim-status', "function: #{error_message}"]) + tags: ['service:claim-status', "function: #{message}"]) end def record_email_send_failure(upload, error) error_message = "#{upload.job_class} va notify failure email errored" ::Rails.logger.error(error_message, { message: error.message }) StatsD.increment('silent_failure', tags: ['service:claim-status', "function: #{error_message}"]) - log_exception_to_sentry(e) + log_exception_to_sentry(error) end - - # add va_notify_date to the evidence_submissions table - done - # grab failed records that dont have a va_notify_date - done - # reach out to va notify with an id of the template - go off of what was in the - # call for this job app/sidekiq/lighthouse/failure_notification.rb, no retrys - # for each file - done - # va notify should return an id when a record is created (take a look at record_evidence_email_send_successful() for an example) - # update evidence submissions with a va notify id, and va notify date - done - # - # use job_class from the es table to determine if we - # should send evss or lighthouse log - done - # Update app/sidekiq/lighthouse/document_upload.rb method sidekiq_retries_exhausted() - # and create a new record and set the upload status to FAILED - done - # Update app/sidekiq/evss/document_upload.rb method sidekiq_retries_exhausted() - # and set the upload status to FAILED - done - # - # Remove app/sidekiq/evss/failure_notification.rb and tests - done - # Remove app/sidekiq/lighthouse/failure_notification.rb and tests - done - # Update tests for doc upload lighthouse and evss - # Add tests for failure notification email job - # - # update periodic_job.rb with your job - # modules/decision_reviews/app/sidekiq/decision_reviews/failure_notification_email_job.rb end end diff --git a/lib/periodic_jobs.rb b/lib/periodic_jobs.rb index 8a5d239d2ac..fcb5b0d1058 100644 --- a/lib/periodic_jobs.rb +++ b/lib/periodic_jobs.rb @@ -257,5 +257,5 @@ mgr.register('00 19 * * 1-5', 'Vye::SundownSweep') # Send Benefits Documents failure notification emails to Veteran for failed evidence submissions - mgr.register('5 0 * * *', 'Lighthouse::BenefitsDocuments::FailureNotificationEmailJob') + mgr.register('5 0 * * *', 'BenefitsDocuments::FailureNotificationEmailJob') } diff --git a/spec/factories/lighthouse/benefits_documents/evidence_submission.rb b/spec/factories/lighthouse/benefits_documents/evidence_submission.rb index 0e57a52b977..92e90d0617b 100644 --- a/spec/factories/lighthouse/benefits_documents/evidence_submission.rb +++ b/spec/factories/lighthouse/benefits_documents/evidence_submission.rb @@ -13,9 +13,32 @@ created_at { DateTime.new(1985, 10, 26).utc } end + factory :bd_evidence_submission_pending, class: 'EvidenceSubmission' do + association :user_account, factory: :user_account + created_at { DateTime.now.utc } + upload_status { BenefitsDocuments::Constants::UPLOAD_STATUS[:PENDING] } + end + factory :bd_evidence_submission_failed, class: 'EvidenceSubmission' do association :user_account, factory: :user_account created_at { DateTime.now.utc } upload_status { BenefitsDocuments::Constants::UPLOAD_STATUS[:FAILED] } + template_metadata_ciphertext do + 'personalisation' => { + 'first_name' => 'test', + 'filename' => 'test.txt', + 'date_submitted' => "#{DateTime.now.utc}", + 'date_failed' => "#{DateTime.now.utc}" + } + end + end + + factory :bd_evidence_submission_failed_va_notify_email_sent, class: 'EvidenceSubmission' do + association :user_account, factory: :user_account + created_at { DateTime.now.utc - 2 } + upload_status { BenefitsDocuments::Constants::UPLOAD_STATUS[:FAILED] } + va_notify_id { 123 } + va_notify_date { DateTime.now.utc } + va_notify_status { 'success' } end end diff --git a/spec/sidekiq/decision_review/failure_notification_email_job_spec.rb b/spec/sidekiq/decision_review/failure_notification_email_job_spec.rb index 340e61846f9..5c548538a88 100644 --- a/spec/sidekiq/decision_review/failure_notification_email_job_spec.rb +++ b/spec/sidekiq/decision_review/failure_notification_email_job_spec.rb @@ -119,7 +119,7 @@ submission2 = AppealSubmission.find_by(submitted_appeal_uuid: guid2) expect(submission2.failure_notification_sent_at).to eq DateTime.new(2023, 1, 2) - submission3 = AppealSubmission.find_by(submitted_appeal_uuid: guid3) + submission3 = AppealSubmission.find_by (submitted_appeal_uuid: guid3) expect(submission3.failure_notification_sent_at).to be_nil expect(mpi_service).not_to have_received(:find_profile_by_identifier) diff --git a/spec/sidekiq/lighthouse/benefits_documents/document_upload_spec.rb b/spec/sidekiq/lighthouse/benefits_documents/document_upload_spec.rb index a6b029bdd54..362b08a684f 100644 --- a/spec/sidekiq/lighthouse/benefits_documents/document_upload_spec.rb +++ b/spec/sidekiq/lighthouse/benefits_documents/document_upload_spec.rb @@ -51,15 +51,12 @@ context 'when upload succeeds' do let(:uploader_stub) { instance_double(LighthouseDocumentUploader) } - let(:evidence_submission_stub) do - evidence_submission = EvidenceSubmission.new(claim_id: '4567', - tracked_item_id: tracked_item_ids, - job_id: job_id, - job_class: described_class, - upload_status: 'pending') - evidence_submission.user_account = user_account - evidence_submission.save! - evidence_submission + let(:evidence_submission_pending) do + create(:bd_evidence_submission_pending, + tracked_item_id: tracked_item_ids, + claim_id:, + job_id:, + job_class: described_class) end let(:response) do { @@ -84,7 +81,7 @@ job_id:, job_class:, upload_status: BenefitsDocuments::Constants::UPLOAD_STATUS[:PENDING] }) - .and_return(evidence_submission_stub) + .and_return(evidence_submission_pending) described_class.drain # runs all queued jobs of this class # After running DocumentUpload job, there should be a new EvidenceSubmission record # with the response request_id diff --git a/spec/sidekiq/lighthouse/benefits_documents/failure_notification_email_job_spec.rb b/spec/sidekiq/lighthouse/benefits_documents/failure_notification_email_job_spec.rb index ebf5639dc8f..6fdeef1a376 100644 --- a/spec/sidekiq/lighthouse/benefits_documents/failure_notification_email_job_spec.rb +++ b/spec/sidekiq/lighthouse/benefits_documents/failure_notification_email_job_spec.rb @@ -2,10 +2,10 @@ require 'rails_helper' -require 'lighthouse/benefits_documents/failure_notification' +require 'lighthouse/benefits_documents/failure_notification_email_job' require 'va_notify/service' -RSpec.describe Lighthouse::BenefitsDocuments::FailureNotificationEmailJob, type: :job do +RSpec.describe BenefitsDocuments::FailureNotificationEmailJob, type: :job do subject { described_class } let(:notify_client_stub) { instance_double(VaNotify::Service) } @@ -24,32 +24,60 @@ let(:date_submitted) { formatted_submit_date } let(:date_failed) { formatted_submit_date } + let(:notification_id) { SecureRandom.uuid } + + let(:vanotify_service) do + service = instance_double(VaNotify::Service) + + response = instance_double(Notifications::Client::ResponseNotification, id: notification_id) + allow(service).to receive(:send_email).and_return(response) + + service + end + before do - allow(Rails.logger).to receive(:info) + allow(VaNotify::Service).to receive(:new).and_return(vanotify_service) end - context 'when Lighthouse::FailureNotification is called' do - it 'enqueues a failure notification mailer to send to the veteran' do - allow(VaNotify::Service).to receive(:new) { notify_client_stub } - - subject.perform_async(icn, first_name, filename, date_submitted, date_failed) do - expect(notify_client_stub).to receive(:send_email).with( - { - recipient_identifier: { id_value: user_account.icn, id_type: 'ICN' }, - template_id: 'fake_template_id', - personalisation: { - first_name: 'Bob', - filename: 'docXXXX-XXte.pdf', - date_submitted: formatted_submit_date, - date_failed: formatted_submit_date - } - } - ) - - expect(Rails.logger) - .to receive(:info) - .with('Lighthouse::FailureNotification email sent') - end + # context 'when there are no FAILED records' do + # it 'doesnt send anything' do + # expect(EvidenceSubmission).not_to receive(:update) + # expect(EvidenceSubmission.va_notify_email_sent.length).to equal(0) + # end + # end + + context 'when there is a FAILED record with a va_notify_date' do + before do + allow(EvidenceSubmission).to receive(:va_notify_email_not_sent) + create(:bd_evidence_submission_failed_va_notify_email_sent) + end + + it 'doesnt update an evidence submission record' do + expect(EvidenceSubmission.va_notify_email_sent.length).to equal(1) + + subject.new.perform + expect(EvidenceSubmission).not_to receive(:update) + + # This is 1 since is has already been sent + expect(EvidenceSubmission.va_notify_email_sent.length).to equal(1) + end + end + + context 'when there is 1 FAILED record without a va_notify_date' do + before do + allow(EvidenceSubmission).to receive(:va_notify_email_not_sent).and_return([evidence_submission_failed]) + allow(EvidenceSubmission).to receive(:update) + end + + let!(:evidence_submission_failed) { create(:bd_evidence_submission_failed) } + + it 'successfully enqueues a failure notification mailer to send to the veteran' do + expect(EvidenceSubmission.va_notify_email_sent.length).to equal(0) + + subject.new.perform + # expect(EvidenceSubmission).to receive(:va_notify_email_not_sent).and_return(evidence_submission_failed) + expect(EvidenceSubmission).to receive(:update) + expect(EvidenceSubmission.va_notify_email_sent.length).to equal(1) end end -end \ No newline at end of file +end From 65875935aed7714efa9f04b78bd22f51930c9730 Mon Sep 17 00:00:00 2001 From: Peri McLaren <141954992+pmclaren19@users.noreply.github.com> Date: Fri, 20 Dec 2024 13:04:17 -0700 Subject: [PATCH 09/13] add test changes --- app/models/evidence_submission.rb | 10 ++- .../failure_notification_email_job.rb | 9 +- .../benefits_documents/evidence_submission.rb | 25 ++++-- spec/sidekiq/evss/document_upload_spec.rb | 2 +- .../document_upload_spec.rb | 2 +- .../failure_notification_email_job_spec.rb | 82 ++++++++++++++----- 6 files changed, 89 insertions(+), 41 deletions(-) diff --git a/app/models/evidence_submission.rb b/app/models/evidence_submission.rb index 948a8cab1ca..01d142236c1 100644 --- a/app/models/evidence_submission.rb +++ b/app/models/evidence_submission.rb @@ -7,10 +7,12 @@ class EvidenceSubmission < ApplicationRecord has_kms_key has_encrypted :template_metadata, key: :kms_key, **lockbox_options - scope :va_notify_email_sent, lambda { - where('upload_status = ? AND va_notify_date IS NOT NULL', BenefitsDocuments::Constants::UPLOAD_STATUS[:FAILED]) + scope :va_notify_email_queued, lambda { + where(upload_status: BenefitsDocuments::Constants::UPLOAD_STATUS[:FAILED]) + .where.not(va_notify_date: nil) + .where.not(va_notify_id: nil) } - scope :va_notify_email_not_sent, lambda { - where(upload_status: BenefitsDocuments::Constants::UPLOAD_STATUS[:FAILED], va_notify_date: nil) + scope :va_notify_email_not_queued, lambda { + where(upload_status: BenefitsDocuments::Constants::UPLOAD_STATUS[:FAILED], va_notify_id: nil, va_notify_date: nil) } end diff --git a/app/sidekiq/lighthouse/benefits_documents/failure_notification_email_job.rb b/app/sidekiq/lighthouse/benefits_documents/failure_notification_email_job.rb index 7210710750c..dec0b2cf036 100644 --- a/app/sidekiq/lighthouse/benefits_documents/failure_notification_email_job.rb +++ b/app/sidekiq/lighthouse/benefits_documents/failure_notification_email_job.rb @@ -26,7 +26,7 @@ def should_perform? # Fetches FAILED evidence submission records for BenefitsDocuments that dont have a va_notify_date def failed_uploads - @failed_uploads ||= EvidenceSubmission.va_notify_email_not_sent + @failed_uploads ||= EvidenceSubmission.va_notify_email_not_queued end def notify_client @@ -35,16 +35,13 @@ def notify_client def send_failed_evidence_submissions failed_uploads.each do |upload| - byebug response = notify_client.send_email( recipient_identifier: { id_value: upload.user_account.icn, id_type: 'ICN' }, template_id: MAILER_TEMPLATE_ID, - personalisation: upload.template_metadata_ciphertext.personalisation + personalisation: upload.template_metadata_ciphertext['personalisation'] ) - byebug record_email_send_success(upload, response) rescue => e - byebug record_email_send_failure(upload, e) end @@ -52,8 +49,8 @@ def send_failed_evidence_submissions end def record_email_send_success(upload, response) + upload.update(va_notify_id: response.id, va_notify_date: DateTime.now) byebug - EvidenceSubmission.update(id: upload.id, va_notify_id: response.id, va_notify_date: DateTime.now) message = "#{upload.job_class} va notify failure email queued" ::Rails.logger.info(message) StatsD.increment('silent_failure_avoided_no_confirmation', diff --git a/spec/factories/lighthouse/benefits_documents/evidence_submission.rb b/spec/factories/lighthouse/benefits_documents/evidence_submission.rb index 92e90d0617b..e6ba4bc8c44 100644 --- a/spec/factories/lighthouse/benefits_documents/evidence_submission.rb +++ b/spec/factories/lighthouse/benefits_documents/evidence_submission.rb @@ -23,20 +23,31 @@ association :user_account, factory: :user_account created_at { DateTime.now.utc } upload_status { BenefitsDocuments::Constants::UPLOAD_STATUS[:FAILED] } + job_class { 'Lighthouse::BenefitsDocuments::DocumentUpload' } template_metadata_ciphertext do - 'personalisation' => { + { 'personalisation' => { 'first_name' => 'test', - 'filename' => 'test.txt', - 'date_submitted' => "#{DateTime.now.utc}", - 'date_failed' => "#{DateTime.now.utc}" - } + 'filename' => 'test.txt', + 'date_submitted' => DateTime.now.utc.to_s, + 'date_failed' => DateTime.now.utc.to_s + } } end end - factory :bd_evidence_submission_failed_va_notify_email_sent, class: 'EvidenceSubmission' do + factory :bd_evidence_submission_failed_va_notify_email_enqueued, class: 'EvidenceSubmission' do association :user_account, factory: :user_account - created_at { DateTime.now.utc - 2 } + created_at { 5.days.ago } upload_status { BenefitsDocuments::Constants::UPLOAD_STATUS[:FAILED] } + job_class { 'Lighthouse::BenefitsDocuments::DocumentUpload' } + va_notify_id { 123 } + va_notify_date { DateTime.now.utc } + end + + factory :bd_evidence_submission_failed_va_notify_email_queued, class: 'EvidenceSubmission' do + association :user_account, factory: :user_account + created_at { 5.days.ago } + upload_status { BenefitsDocuments::Constants::UPLOAD_STATUS[:FAILED] } + job_class { 'Lighthouse::BenefitsDocuments::DocumentUpload' } va_notify_id { 123 } va_notify_date { DateTime.now.utc } va_notify_status { 'success' } diff --git a/spec/sidekiq/evss/document_upload_spec.rb b/spec/sidekiq/evss/document_upload_spec.rb index 18b20d32ca2..88256a90d7f 100644 --- a/spec/sidekiq/evss/document_upload_spec.rb +++ b/spec/sidekiq/evss/document_upload_spec.rb @@ -73,7 +73,7 @@ EVSS::DocumentUpload.within_sidekiq_retries_exhausted_block(msg) do expect(EvidenceSubmission).to receive(:create).and_return(evidence_submission_failed) end - expect(EvidenceSubmission.va_notify_email_not_sent.length).to equal(1) + expect(EvidenceSubmission.va_notify_email_not_queued.length).to equal(1) end it 'fails to create a failed evidence submission record when args malformed' do diff --git a/spec/sidekiq/lighthouse/benefits_documents/document_upload_spec.rb b/spec/sidekiq/lighthouse/benefits_documents/document_upload_spec.rb index 362b08a684f..d0ac6c0c1f1 100644 --- a/spec/sidekiq/lighthouse/benefits_documents/document_upload_spec.rb +++ b/spec/sidekiq/lighthouse/benefits_documents/document_upload_spec.rb @@ -113,7 +113,7 @@ Lighthouse::BenefitsDocuments::DocumentUpload.within_sidekiq_retries_exhausted_block(msg) do expect(EvidenceSubmission).to receive(:create).and_return(evidence_submission_failed) end - expect(EvidenceSubmission.va_notify_email_not_sent.length).to equal(1) + expect(EvidenceSubmission.va_notify_email_not_queued.length).to equal(1) end it 'fails to create a failed evidence submission record when args malformed' do diff --git a/spec/sidekiq/lighthouse/benefits_documents/failure_notification_email_job_spec.rb b/spec/sidekiq/lighthouse/benefits_documents/failure_notification_email_job_spec.rb index 6fdeef1a376..d343a0f4113 100644 --- a/spec/sidekiq/lighthouse/benefits_documents/failure_notification_email_job_spec.rb +++ b/spec/sidekiq/lighthouse/benefits_documents/failure_notification_email_job_spec.rb @@ -37,47 +37,85 @@ before do allow(VaNotify::Service).to receive(:new).and_return(vanotify_service) + allow(Rails.logger).to receive(:info) end - # context 'when there are no FAILED records' do - # it 'doesnt send anything' do - # expect(EvidenceSubmission).not_to receive(:update) - # expect(EvidenceSubmission.va_notify_email_sent.length).to equal(0) - # end - # end + context 'when there are no FAILED records' do + it 'doesnt send anything' do + expect(EvidenceSubmission).not_to receive(:update) + expect(vanotify_service).not_to receive(:send_email) + expect(EvidenceSubmission.va_notify_email_queued.length).to equal(0) + subject.new.perform + end + end context 'when there is a FAILED record with a va_notify_date' do before do - allow(EvidenceSubmission).to receive(:va_notify_email_not_sent) - create(:bd_evidence_submission_failed_va_notify_email_sent) + allow(VaNotify::Service).to receive(:new).and_return(vanotify_service) + create(:bd_evidence_submission_failed_va_notify_email_enqueued) end - it 'doesnt update an evidence submission record' do - expect(EvidenceSubmission.va_notify_email_sent.length).to equal(1) - + it 'doesnt update an evidence submission record or queue failure email' do + expect(EvidenceSubmission.va_notify_email_queued.length).to eq(1) subject.new.perform - expect(EvidenceSubmission).not_to receive(:update) - - # This is 1 since is has already been sent - expect(EvidenceSubmission.va_notify_email_sent.length).to equal(1) + expect(vanotify_service).not_to receive(:send_email) + # This is 1 since is has already been queued + expect(EvidenceSubmission.va_notify_email_queued.length).to eq(1) end end - context 'when there is 1 FAILED record without a va_notify_date' do + context 'when there is a FAILED record without a va_notify_date and an error occurs' do before do - allow(EvidenceSubmission).to receive(:va_notify_email_not_sent).and_return([evidence_submission_failed]) - allow(EvidenceSubmission).to receive(:update) + allow(VaNotify::Service).to receive(:new).and_raise(StandardError) + allow(EvidenceSubmission).to receive(:va_notify_email_not_queued).and_return([evidence_submission_failed]) + allow(Rails.logger).to receive(:error) + allow(StatsD).to receive(:increment) end let!(:evidence_submission_failed) { create(:bd_evidence_submission_failed) } + let(:error_message) { "#{evidence_submission_failed.job_class} va notify failure email errored" } + let(:tags) { ['service:claim-status', "function: #{error_message}"] } + + it 'handles the error and increments the statsd metric' do + expect(EvidenceSubmission.count).to eq(1) + expect(EvidenceSubmission.va_notify_email_queued.length).to eq(0) + expect(vanotify_service).not_to receive(:send_email) + expect(Rails.logger) + .to receive(:error) + .with(error_message, { message: 'StandardError' }) + expect(StatsD).to receive(:increment).with('silent_failure', tags: tags) + subject.new.perform + end + end + + context 'when there is 1 FAILED record without a va_notify_date' do + let(:evidence_submission_stub) { instance_double(EvidenceSubmission) } + let(:tags) { ['service:claim-status', "function: #{message}"] } + let!(:evidence_submission_failed) { create(:bd_evidence_submission_failed) } + let(:message) { "#{evidence_submission_failed.job_class} va notify failure email queued" } + + before do + # allow(VaNotify::Service).to receive(:new) + # allow(EvidenceSubmission).to receive(:va_notify_email_not_queued) + allow(EvidenceSubmission).to receive(:where) + # allow(EvidenceSubmission).to receive(:update) + allow(EvidenceSubmission).to receive(:new).and_return(evidence_submission_stub) + + allow(Rails.logger).to receive(:info) + allow(StatsD).to receive(:increment) + end it 'successfully enqueues a failure notification mailer to send to the veteran' do - expect(EvidenceSubmission.va_notify_email_sent.length).to equal(0) + expect(EvidenceSubmission.count).to eq(1) + expect(EvidenceSubmission.va_notify_email_queued.length).to eq(0) + expect(vanotify_service).to receive(:send_email) + expect(evidence_submission_stub).to receive(:update) + expect(Rails.logger).to receive(:info).with(message) + expect(StatsD).to receive(:increment).with('silent_failure_avoided_no_confirmation', tags: tags) + expect(EvidenceSubmission.va_notify_email_not_queued.length).to eq(1) subject.new.perform - # expect(EvidenceSubmission).to receive(:va_notify_email_not_sent).and_return(evidence_submission_failed) - expect(EvidenceSubmission).to receive(:update) - expect(EvidenceSubmission.va_notify_email_sent.length).to equal(1) + expect(EvidenceSubmission.va_notify_email_queued.length).to eq(1) end end end From 4f63bb724b6d0ce89454ef00e9434c3a976c0188 Mon Sep 17 00:00:00 2001 From: Peri McLaren <141954992+pmclaren19@users.noreply.github.com> Date: Fri, 20 Dec 2024 14:10:07 -0700 Subject: [PATCH 10/13] updated tests --- .../failure_notification_email_job.rb | 1 - .../failure_notification_email_job_spec.rb | 14 +++----------- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/app/sidekiq/lighthouse/benefits_documents/failure_notification_email_job.rb b/app/sidekiq/lighthouse/benefits_documents/failure_notification_email_job.rb index dec0b2cf036..3ccc25a87fa 100644 --- a/app/sidekiq/lighthouse/benefits_documents/failure_notification_email_job.rb +++ b/app/sidekiq/lighthouse/benefits_documents/failure_notification_email_job.rb @@ -50,7 +50,6 @@ def send_failed_evidence_submissions def record_email_send_success(upload, response) upload.update(va_notify_id: response.id, va_notify_date: DateTime.now) - byebug message = "#{upload.job_class} va notify failure email queued" ::Rails.logger.info(message) StatsD.increment('silent_failure_avoided_no_confirmation', diff --git a/spec/sidekiq/lighthouse/benefits_documents/failure_notification_email_job_spec.rb b/spec/sidekiq/lighthouse/benefits_documents/failure_notification_email_job_spec.rb index d343a0f4113..129a293eba3 100644 --- a/spec/sidekiq/lighthouse/benefits_documents/failure_notification_email_job_spec.rb +++ b/spec/sidekiq/lighthouse/benefits_documents/failure_notification_email_job_spec.rb @@ -89,31 +89,23 @@ end context 'when there is 1 FAILED record without a va_notify_date' do - let(:evidence_submission_stub) { instance_double(EvidenceSubmission) } let(:tags) { ['service:claim-status', "function: #{message}"] } let!(:evidence_submission_failed) { create(:bd_evidence_submission_failed) } let(:message) { "#{evidence_submission_failed.job_class} va notify failure email queued" } before do - # allow(VaNotify::Service).to receive(:new) - # allow(EvidenceSubmission).to receive(:va_notify_email_not_queued) - allow(EvidenceSubmission).to receive(:where) - # allow(EvidenceSubmission).to receive(:update) - allow(EvidenceSubmission).to receive(:new).and_return(evidence_submission_stub) - + allow(EvidenceSubmission).to receive(:va_notify_email_not_queued).and_return([evidence_submission_failed]) allow(Rails.logger).to receive(:info) allow(StatsD).to receive(:increment) end it 'successfully enqueues a failure notification mailer to send to the veteran' do expect(EvidenceSubmission.count).to eq(1) - expect(EvidenceSubmission.va_notify_email_queued.length).to eq(0) + expect(EvidenceSubmission.va_notify_email_not_queued.length).to eq(1) expect(vanotify_service).to receive(:send_email) - expect(evidence_submission_stub).to receive(:update) + expect(evidence_submission_failed).to receive(:update).and_call_original expect(Rails.logger).to receive(:info).with(message) expect(StatsD).to receive(:increment).with('silent_failure_avoided_no_confirmation', tags: tags) - expect(EvidenceSubmission.va_notify_email_not_queued.length).to eq(1) - subject.new.perform expect(EvidenceSubmission.va_notify_email_queued.length).to eq(1) end From 1bc590f9e44d86ba090b024029ef0e228c3e2b55 Mon Sep 17 00:00:00 2001 From: Peri McLaren <141954992+pmclaren19@users.noreply.github.com> Date: Fri, 20 Dec 2024 14:25:29 -0700 Subject: [PATCH 11/13] remove change --- .../decision_review/failure_notification_email_job_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/sidekiq/decision_review/failure_notification_email_job_spec.rb b/spec/sidekiq/decision_review/failure_notification_email_job_spec.rb index 5c548538a88..340e61846f9 100644 --- a/spec/sidekiq/decision_review/failure_notification_email_job_spec.rb +++ b/spec/sidekiq/decision_review/failure_notification_email_job_spec.rb @@ -119,7 +119,7 @@ submission2 = AppealSubmission.find_by(submitted_appeal_uuid: guid2) expect(submission2.failure_notification_sent_at).to eq DateTime.new(2023, 1, 2) - submission3 = AppealSubmission.find_by (submitted_appeal_uuid: guid3) + submission3 = AppealSubmission.find_by(submitted_appeal_uuid: guid3) expect(submission3.failure_notification_sent_at).to be_nil expect(mpi_service).not_to have_received(:find_profile_by_identifier) From b38319288a6a7ac1afb9a84a6cf774db696c2705 Mon Sep 17 00:00:00 2001 From: Peri McLaren <141954992+pmclaren19@users.noreply.github.com> Date: Fri, 20 Dec 2024 15:19:15 -0700 Subject: [PATCH 12/13] updates --- lib/lighthouse/benefits_documents/service.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/lighthouse/benefits_documents/service.rb b/lib/lighthouse/benefits_documents/service.rb index 1af64871b48..2c9b6c6abcf 100644 --- a/lib/lighthouse/benefits_documents/service.rb +++ b/lib/lighthouse/benefits_documents/service.rb @@ -76,8 +76,8 @@ def document_upload(user_icn, document_hash, user_account_uuid, claim_id, tracke if Flipper.enabled?(:cst_synchronous_evidence_uploads, @user) Lighthouse::DocumentUploadSynchronous.upload(user_icn, document_hash) else - Lighthouse::DocumentUpload.perform_async(user_icn, document_hash, user_account_uuid, - claim_id, tracked_item_id) + Lighthouse::BenefitsDocuments::DocumentUpload.perform_async(user_icn, document_hash, user_account_uuid, + claim_id, tracked_item_id) end end From 53306f95b9b1028a8e16434a21565303ff5295a1 Mon Sep 17 00:00:00 2001 From: Peri McLaren <141954992+pmclaren19@users.noreply.github.com> Date: Fri, 20 Dec 2024 18:51:39 -0700 Subject: [PATCH 13/13] update failure notification email job --- .../benefits_documents/failure_notification_email_job.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/sidekiq/lighthouse/benefits_documents/failure_notification_email_job.rb b/app/sidekiq/lighthouse/benefits_documents/failure_notification_email_job.rb index 3ccc25a87fa..fa6d685cc57 100644 --- a/app/sidekiq/lighthouse/benefits_documents/failure_notification_email_job.rb +++ b/app/sidekiq/lighthouse/benefits_documents/failure_notification_email_job.rb @@ -35,10 +35,11 @@ def notify_client def send_failed_evidence_submissions failed_uploads.each do |upload| + personalisation = JSON.parse(upload.template_metadata_ciphertext)['personalisation'] response = notify_client.send_email( recipient_identifier: { id_value: upload.user_account.icn, id_type: 'ICN' }, template_id: MAILER_TEMPLATE_ID, - personalisation: upload.template_metadata_ciphertext['personalisation'] + personalisation: ) record_email_send_success(upload, response) rescue => e