From ea3b74b603754eb4222f7c34ed85e8c5b5914776 Mon Sep 17 00:00:00 2001 From: Tomas Apodaca Date: Mon, 6 Mar 2023 14:59:48 -0800 Subject: [PATCH 1/6] make analytics methods private --- app/jobs/get_usps_proofing_results_job.rb | 84 +++++++++++------------ 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/app/jobs/get_usps_proofing_results_job.rb b/app/jobs/get_usps_proofing_results_job.rb index 16dcb4d0aec..377f77223f8 100644 --- a/app/jobs/get_usps_proofing_results_job.rb +++ b/app/jobs/get_usps_proofing_results_job.rb @@ -13,48 +13,6 @@ class GetUspsProofingResultsJob < ApplicationJob queue_as :long_running - def email_analytics_attributes(enrollment) - { - timestamp: Time.zone.now, - user_id: enrollment.user_id, - service_provider: enrollment.issuer, - wait_until: mail_delivery_params(enrollment.proofed_at)[:wait_until], - } - end - - def enrollment_analytics_attributes(enrollment, complete:) - { - enrollment_code: enrollment.enrollment_code, - enrollment_id: enrollment.id, - minutes_since_last_status_check: enrollment.minutes_since_last_status_check, - minutes_since_last_status_update: enrollment.minutes_since_last_status_update, - minutes_since_established: enrollment.minutes_since_established, - minutes_to_completion: complete ? enrollment.minutes_since_established : nil, - issuer: enrollment.issuer, - } - end - - def response_analytics_attributes(response) - return { response_present: false } unless response.present? - - { - fraud_suspected: response['fraudSuspected'], - primary_id_type: response['primaryIdType'], - secondary_id_type: response['secondaryIdType'], - failure_reason: response['failureReason'], - transaction_end_date_time: parse_usps_timestamp(response['transactionEndDateTime']), - transaction_start_date_time: parse_usps_timestamp(response['transactionStartDateTime']), - status: response['status'], - assurance_level: response['assuranceLevel'], - proofing_post_office: response['proofingPostOffice'], - proofing_city: response['proofingCity'], - proofing_state: response['proofingState'], - scan_count: response['scanCount'], - response_message: response['responseMessage'], - response_present: true, - } - end - def perform(_now) return true unless IdentityConfig.store.in_person_proofing_enabled @@ -140,6 +98,48 @@ def analytics(user: AnonymousUser.new) Analytics.new(user: user, request: nil, session: {}, sp: nil) end + def email_analytics_attributes(enrollment) + { + timestamp: Time.zone.now, + user_id: enrollment.user_id, + service_provider: enrollment.issuer, + wait_until: mail_delivery_params(enrollment.proofed_at)[:wait_until], + } + end + + def enrollment_analytics_attributes(enrollment, complete:) + { + enrollment_code: enrollment.enrollment_code, + enrollment_id: enrollment.id, + minutes_since_last_status_check: enrollment.minutes_since_last_status_check, + minutes_since_last_status_update: enrollment.minutes_since_last_status_update, + minutes_since_established: enrollment.minutes_since_established, + minutes_to_completion: complete ? enrollment.minutes_since_established : nil, + issuer: enrollment.issuer, + } + end + + def response_analytics_attributes(response) + return { response_present: false } unless response.present? + + { + fraud_suspected: response['fraudSuspected'], + primary_id_type: response['primaryIdType'], + secondary_id_type: response['secondaryIdType'], + failure_reason: response['failureReason'], + transaction_end_date_time: parse_usps_timestamp(response['transactionEndDateTime']), + transaction_start_date_time: parse_usps_timestamp(response['transactionStartDateTime']), + status: response['status'], + assurance_level: response['assuranceLevel'], + proofing_post_office: response['proofingPostOffice'], + proofing_city: response['proofingCity'], + proofing_state: response['proofingState'], + scan_count: response['scanCount'], + response_message: response['responseMessage'], + response_present: true, + } + end + def handle_bad_request_error(err, enrollment) response_body = err.response_body response_message = response_body&.[]('responseMessage') From e86a0c167eff5a2d14d2d38257cf7b490474f44b Mon Sep 17 00:00:00 2001 From: Tomas Apodaca Date: Mon, 6 Mar 2023 15:04:48 -0800 Subject: [PATCH 2/6] make proofer & request delay instance variables --- app/jobs/get_usps_proofing_results_job.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/app/jobs/get_usps_proofing_results_job.rb b/app/jobs/get_usps_proofing_results_job.rb index 377f77223f8..6b973f5611e 100644 --- a/app/jobs/get_usps_proofing_results_job.rb +++ b/app/jobs/get_usps_proofing_results_job.rb @@ -24,6 +24,9 @@ def perform(_now) enrollments_in_progress: 0, enrollments_passed: 0, } + @request_delay_in_seconds = IdentityConfig.store. + get_usps_proofing_results_job_request_delay_milliseconds / MILLISECONDS_PER_SECOND + reprocess_delay_minutes = IdentityConfig.store. get_usps_proofing_results_job_reprocess_delay_minutes enrollments = InPersonEnrollment.needs_usps_status_check( @@ -49,14 +52,15 @@ def perform(_now) private attr_accessor :enrollment_outcomes + attr_accessor :request_delay_in_seconds DEFAULT_EMAIL_DELAY_IN_HOURS = 1 - def check_enrollments(enrollments) - request_delay_in_seconds = IdentityConfig.store. - get_usps_proofing_results_job_request_delay_milliseconds / MILLISECONDS_PER_SECOND - proofer = UspsInPersonProofing::Proofer.new + def proofer + @proofer ||= UspsInPersonProofing::Proofer.new + end + def check_enrollments(enrollments) enrollments.each_with_index do |enrollment, idx| # Add a unique ID for enrollments that don't have one enrollment.update(unique_id: enrollment.usps_unique_id) if enrollment.unique_id.blank? From 809b3523fe7d2df92e2c51e666e2ade651b9f94a Mon Sep 17 00:00:00 2001 From: Tomas Apodaca Date: Mon, 6 Mar 2023 15:11:17 -0800 Subject: [PATCH 3/6] move individual enrollment check into a new method --- app/jobs/get_usps_proofing_results_job.rb | 65 ++++++++++++----------- 1 file changed, 35 insertions(+), 30 deletions(-) diff --git a/app/jobs/get_usps_proofing_results_job.rb b/app/jobs/get_usps_proofing_results_job.rb index 6b973f5611e..036ba562e94 100644 --- a/app/jobs/get_usps_proofing_results_job.rb +++ b/app/jobs/get_usps_proofing_results_job.rb @@ -61,41 +61,46 @@ def proofer end def check_enrollments(enrollments) + last_enrollment_index = enrollments.length - 1 enrollments.each_with_index do |enrollment, idx| - # Add a unique ID for enrollments that don't have one - enrollment.update(unique_id: enrollment.usps_unique_id) if enrollment.unique_id.blank? + check_enrollment(enrollment, (idx < last_enrollment_index)) + end + end - status_check_attempted_at = Time.zone.now - enrollment_outcomes[:enrollments_checked] += 1 - response = nil + def check_enrollment(enrollment, is_not_last_enrollment) + # Add a unique ID for enrollments that don't have one + enrollment.update(unique_id: enrollment.usps_unique_id) if enrollment.unique_id.blank? - begin - response = proofer.request_proofing_results( - enrollment.unique_id, enrollment.enrollment_code - ) - rescue Faraday::BadRequestError => err - # 400 status code. This is used for some status updates and some common client errors - handle_bad_request_error(err, enrollment) - rescue Faraday::ClientError, Faraday::ServerError => err - # 4xx or 5xx status code. These are unexpected but will have some sort of - # response body that we can try to log data from - handle_client_or_server_error(err, enrollment) - rescue Faraday::Error => err - # Timeouts, failed connections, parsing errors, and other HTTP errors. These - # generally won't have a response body - handle_faraday_error(err, enrollment) - rescue StandardError => err - handle_standard_error(err, enrollment) - else - process_enrollment_response(enrollment, response) - ensure - # Record the attempt to update the enrollment - enrollment.update(status_check_attempted_at: status_check_attempted_at) - end + status_check_attempted_at = Time.zone.now + enrollment_outcomes[:enrollments_checked] += 1 + response = nil - # Sleep for a while before we attempt to make another call to USPS - sleep request_delay_in_seconds if idx < enrollments.length - 1 + begin + response = proofer.request_proofing_results( + enrollment.unique_id, enrollment.enrollment_code + ) + rescue Faraday::BadRequestError => err + # 400 status code. This is used for some status updates and some common client errors + handle_bad_request_error(err, enrollment) + rescue Faraday::ClientError, Faraday::ServerError => err + # 4xx or 5xx status code. These are unexpected but will have some sort of + # response body that we can try to log data from + handle_client_or_server_error(err, enrollment) + rescue Faraday::Error => err + # Timeouts, failed connections, parsing errors, and other HTTP errors. These + # generally won't have a response body + handle_faraday_error(err, enrollment) + rescue StandardError => err + handle_standard_error(err, enrollment) + else + process_enrollment_response(enrollment, response) + ensure + # Record the attempt to update the enrollment + enrollment.update(status_check_attempted_at: status_check_attempted_at) end + + # Sleep for a while before we attempt to make another call to USPS + sleep request_delay_in_seconds if is_not_last_enrollment end def analytics(user: AnonymousUser.new) From 40d25a361e5a56ac64a50cad8ba0a9e256d269f5 Mon Sep 17 00:00:00 2001 From: Tomas Apodaca Date: Mon, 6 Mar 2023 15:34:49 -0800 Subject: [PATCH 4/6] add changelog changelog: Internal, refactor, GetUspsProofingResultsJob refactor From 10bc47facfa90eac897992213d9d0e02a1f95666 Mon Sep 17 00:00:00 2001 From: Tomas Apodaca Date: Tue, 7 Mar 2023 09:06:17 -0800 Subject: [PATCH 5/6] reformat check_enrollment & move sleep up --- app/jobs/get_usps_proofing_results_job.rb | 53 +++++++++++------------ 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/app/jobs/get_usps_proofing_results_job.rb b/app/jobs/get_usps_proofing_results_job.rb index 036ba562e94..c9df99d2d38 100644 --- a/app/jobs/get_usps_proofing_results_job.rb +++ b/app/jobs/get_usps_proofing_results_job.rb @@ -63,11 +63,13 @@ def proofer def check_enrollments(enrollments) last_enrollment_index = enrollments.length - 1 enrollments.each_with_index do |enrollment, idx| - check_enrollment(enrollment, (idx < last_enrollment_index)) + check_enrollment(enrollment) + # Sleep for a while before we attempt to make another call to USPS + sleep request_delay_in_seconds if idx < last_enrollment_index end end - def check_enrollment(enrollment, is_not_last_enrollment) + def check_enrollment(enrollment) # Add a unique ID for enrollments that don't have one enrollment.update(unique_id: enrollment.usps_unique_id) if enrollment.unique_id.blank? @@ -75,32 +77,27 @@ def check_enrollment(enrollment, is_not_last_enrollment) enrollment_outcomes[:enrollments_checked] += 1 response = nil - begin - response = proofer.request_proofing_results( - enrollment.unique_id, enrollment.enrollment_code - ) - rescue Faraday::BadRequestError => err - # 400 status code. This is used for some status updates and some common client errors - handle_bad_request_error(err, enrollment) - rescue Faraday::ClientError, Faraday::ServerError => err - # 4xx or 5xx status code. These are unexpected but will have some sort of - # response body that we can try to log data from - handle_client_or_server_error(err, enrollment) - rescue Faraday::Error => err - # Timeouts, failed connections, parsing errors, and other HTTP errors. These - # generally won't have a response body - handle_faraday_error(err, enrollment) - rescue StandardError => err - handle_standard_error(err, enrollment) - else - process_enrollment_response(enrollment, response) - ensure - # Record the attempt to update the enrollment - enrollment.update(status_check_attempted_at: status_check_attempted_at) - end - - # Sleep for a while before we attempt to make another call to USPS - sleep request_delay_in_seconds if is_not_last_enrollment + response = proofer.request_proofing_results( + enrollment.unique_id, enrollment.enrollment_code + ) + rescue Faraday::BadRequestError => err + # 400 status code. This is used for some status updates and some common client errors + handle_bad_request_error(err, enrollment) + rescue Faraday::ClientError, Faraday::ServerError => err + # 4xx or 5xx status code. These are unexpected but will have some sort of + # response body that we can try to log data from + handle_client_or_server_error(err, enrollment) + rescue Faraday::Error => err + # Timeouts, failed connections, parsing errors, and other HTTP errors. These + # generally won't have a response body + handle_faraday_error(err, enrollment) + rescue StandardError => err + handle_standard_error(err, enrollment) + else + process_enrollment_response(enrollment, response) + ensure + # Record the attempt to update the enrollment + enrollment.update(status_check_attempted_at: status_check_attempted_at) end def analytics(user: AnonymousUser.new) From 04256af7ce2b5671fec3ce83831bb42fe3f39636 Mon Sep 17 00:00:00 2001 From: Tomas Apodaca Date: Tue, 7 Mar 2023 09:26:25 -0800 Subject: [PATCH 6/6] move request delay into constant --- app/jobs/get_usps_proofing_results_job.rb | 9 ++++----- spec/jobs/get_usps_proofing_results_job_spec.rb | 7 ++++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/jobs/get_usps_proofing_results_job.rb b/app/jobs/get_usps_proofing_results_job.rb index c9df99d2d38..7327917e345 100644 --- a/app/jobs/get_usps_proofing_results_job.rb +++ b/app/jobs/get_usps_proofing_results_job.rb @@ -24,8 +24,6 @@ def perform(_now) enrollments_in_progress: 0, enrollments_passed: 0, } - @request_delay_in_seconds = IdentityConfig.store. - get_usps_proofing_results_job_request_delay_milliseconds / MILLISECONDS_PER_SECOND reprocess_delay_minutes = IdentityConfig.store. get_usps_proofing_results_job_reprocess_delay_minutes @@ -52,9 +50,10 @@ def perform(_now) private attr_accessor :enrollment_outcomes - attr_accessor :request_delay_in_seconds DEFAULT_EMAIL_DELAY_IN_HOURS = 1 + REQUEST_DELAY_IN_SECONDS = IdentityConfig.store. + get_usps_proofing_results_job_request_delay_milliseconds / MILLISECONDS_PER_SECOND def proofer @proofer ||= UspsInPersonProofing::Proofer.new @@ -64,8 +63,8 @@ def check_enrollments(enrollments) last_enrollment_index = enrollments.length - 1 enrollments.each_with_index do |enrollment, idx| check_enrollment(enrollment) - # Sleep for a while before we attempt to make another call to USPS - sleep request_delay_in_seconds if idx < last_enrollment_index + # Sleep briefly after each call to USPS + sleep REQUEST_DELAY_IN_SECONDS if idx < last_enrollment_index end end diff --git a/spec/jobs/get_usps_proofing_results_job_spec.rb b/spec/jobs/get_usps_proofing_results_job_spec.rb index a39addc7f90..bd17b08c838 100644 --- a/spec/jobs/get_usps_proofing_results_job_spec.rb +++ b/spec/jobs/get_usps_proofing_results_job_spec.rb @@ -166,9 +166,10 @@ allow(job).to receive(:analytics).and_return(job_analytics) allow(IdentityConfig.store).to receive(:get_usps_proofing_results_job_reprocess_delay_minutes). and_return(reprocess_delay_minutes) - allow(IdentityConfig.store). - to receive(:get_usps_proofing_results_job_request_delay_milliseconds). - and_return(request_delay_ms) + stub_const( + 'GetUspsProofingResultsJob::REQUEST_DELAY_IN_SECONDS', + request_delay_ms / GetUspsProofingResultsJob::MILLISECONDS_PER_SECOND, + ) stub_request_token if respond_to?(:pending_enrollment) pending_enrollment.update(enrollment_established_at: 3.days.ago)