diff --git a/app/jobs/get_usps_proofing_results_job.rb b/app/jobs/get_usps_proofing_results_job.rb index 521a6e7ba4f..c0da8c79f14 100644 --- a/app/jobs/get_usps_proofing_results_job.rb +++ b/app/jobs/get_usps_proofing_results_job.rb @@ -28,18 +28,19 @@ def perform(_now) reprocess_delay_minutes = IdentityConfig.store. get_usps_proofing_results_job_reprocess_delay_minutes - enrollments = InPersonEnrollment.needs_usps_status_check( + pending_enrollments = InPersonEnrollment.needs_usps_status_check( ...reprocess_delay_minutes.minutes.ago, ) started_at = Time.zone.now + pending_enrollments.update(last_batch_claimed_at: started_at) + enrollments_to_check = InPersonEnrollment.needs_usps_status_check_batch(started_at) analytics.idv_in_person_usps_proofing_results_job_started( - enrollments_count: enrollments.count, + enrollments_count: enrollments_to_check.count, reprocess_delay_minutes: reprocess_delay_minutes, job_name: self.class.name, ) - - check_enrollments(enrollments) + check_enrollments(enrollments_to_check) analytics.idv_in_person_usps_proofing_results_job_completed( **enrollment_outcomes, diff --git a/app/models/in_person_enrollment.rb b/app/models/in_person_enrollment.rb index 93e5430ad25..dc9c2400dea 100644 --- a/app/models/in_person_enrollment.rb +++ b/app/models/in_person_enrollment.rb @@ -60,8 +60,15 @@ def self.needs_late_email_reminder(early_benchmark, late_benchmark) def self.needs_usps_status_check(check_interval) where(status: :pending). and( - where(status_check_attempted_at: check_interval). - or(where(status_check_attempted_at: nil)), + where(last_batch_claimed_at: check_interval). + or(where(last_batch_claimed_at: nil)), + ) + end + + def self.needs_usps_status_check_batch(batch_at) + where(status: :pending). + and( + where(last_batch_claimed_at: batch_at), ). order(status_check_attempted_at: :asc) end @@ -69,8 +76,8 @@ def self.needs_usps_status_check(check_interval) # Does this enrollment need a status check via the USPS API? def needs_usps_status_check?(check_interval) pending? && ( - status_check_attempted_at.nil? || - check_interval.cover?(status_check_attempted_at) + last_batch_claimed_at.nil? || + check_interval.cover?(last_batch_claimed_at) ) end diff --git a/db/primary_migrate/20230814130423_add_last_batch_claimed_at_to_in_person_enrollment.rb b/db/primary_migrate/20230814130423_add_last_batch_claimed_at_to_in_person_enrollment.rb new file mode 100644 index 00000000000..0a18b9e298e --- /dev/null +++ b/db/primary_migrate/20230814130423_add_last_batch_claimed_at_to_in_person_enrollment.rb @@ -0,0 +1,6 @@ +class AddLastBatchClaimedAtToInPersonEnrollment < ActiveRecord::Migration[7.0] + def change + add_column :in_person_enrollments, :last_batch_claimed_at, :datetime + InPersonEnrollment.update_all("last_batch_claimed_at = status_check_attempted_at") + end +end diff --git a/db/schema.rb b/db/schema.rb index 904574f2213..7dd15040377 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.0].define(version: 2023_08_03_143215) do +ActiveRecord::Schema[7.0].define(version: 2023_08_14_130423) do # These are extensions that must be enabled in order to support this database enable_extension "pg_stat_statements" enable_extension "pgcrypto" @@ -311,6 +311,7 @@ t.datetime "status_check_completed_at", comment: "The last time a status check was successfully completed" t.boolean "ready_for_status_check", default: false t.datetime "notification_sent_at", comment: "The time a notification was sent" + t.datetime "last_batch_claimed_at" t.index ["profile_id"], name: "index_in_person_enrollments_on_profile_id" t.index ["ready_for_status_check"], name: "index_in_person_enrollments_on_ready_for_status_check", where: "(ready_for_status_check = true)" t.index ["status_check_attempted_at"], name: "index_in_person_enrollments_on_status_check_attempted_at", where: "(status = 1)" diff --git a/spec/jobs/get_usps_proofing_results_job_spec.rb b/spec/jobs/get_usps_proofing_results_job_spec.rb index 628f3d740af..0a8df4ac944 100644 --- a/spec/jobs/get_usps_proofing_results_job_spec.rb +++ b/spec/jobs/get_usps_proofing_results_job_spec.rb @@ -149,7 +149,8 @@ RSpec.shared_examples 'enrollment_encountering_an_error_that_has_a_nil_response' do |error_type:| it 'logs that response is not present' do - expect(NewRelic::Agent).to receive(:notice_error).with(instance_of(error_type)) + expect(NewRelic::Agent).to receive(:notice_error). + with(instance_of(error_type)).at_least(1).times job.perform(Time.zone.now) expect(job_analytics).to have_logged_event( @@ -217,35 +218,22 @@ describe 'IPP enabled' do describe 'DAV not enabled' do let!(:pending_enrollments) do - [ + ['BALTIMORE', 'FRIENDSHIP', 'WASHINGTON', 'ARLINGTON', 'DEANWOOD'].map do |name| create( - :in_person_enrollment, :pending, - selected_location_details: { name: 'BALTIMORE' }, - issuer: 'http://localhost:3000' - ), - create( - :in_person_enrollment, :pending, - selected_location_details: { name: 'FRIENDSHIP' } - ), - create( - :in_person_enrollment, :pending, - selected_location_details: { name: 'WASHINGTON' } - ), - create( - :in_person_enrollment, :pending, - selected_location_details: { name: 'ARLINGTON' } - ), - create( - :in_person_enrollment, :pending, - selected_location_details: { name: 'DEANWOOD' } - ), - ] + :in_person_enrollment, + :pending, + :with_notification_phone_configuration, + issuer: 'http://localhost:3000', + selected_location_details: { name: name }, + ) + end end - let(:pending_enrollment) { pending_enrollments[0] } + let(:pending_enrollment) { pending_enrollments.first } before do + enrollment_records = InPersonEnrollment.where(id: pending_enrollments.map(&:id)) allow(InPersonEnrollment).to receive(:needs_usps_status_check). - and_return([pending_enrollment]) + and_return(enrollment_records) allow(IdentityConfig.store).to receive(:in_person_proofing_enabled).and_return(true) end @@ -263,8 +251,9 @@ end it 'records the last attempted status check regardless of response code and contents' do + enrollment_records = InPersonEnrollment.where(id: pending_enrollments.map(&:id)) allow(InPersonEnrollment).to receive(:needs_usps_status_check). - and_return(pending_enrollments) + and_return(enrollment_records) stub_request_proofing_results_with_responses( request_failed_proofing_results_args, request_in_progress_proofing_results_args, @@ -308,8 +297,9 @@ end it 'logs a message when the job starts' do + enrollment_records = InPersonEnrollment.where(id: pending_enrollments.map(&:id)) allow(InPersonEnrollment).to receive(:needs_usps_status_check). - and_return(pending_enrollments) + and_return(enrollment_records) stub_request_proofing_results_with_responses( request_failed_proofing_results_args, request_in_progress_proofing_results_args, @@ -328,8 +318,9 @@ end it 'logs a message with counts of various outcomes when the job completes (errored > 0)' do + enrollment_records = InPersonEnrollment.where(id: pending_enrollments.map(&:id)) allow(InPersonEnrollment).to receive(:needs_usps_status_check). - and_return(pending_enrollments) + and_return(enrollment_records) stub_request_proofing_results_with_responses( request_passed_proofing_results_args, request_in_progress_proofing_results_args, @@ -337,7 +328,6 @@ request_failed_proofing_results_args, request_expired_proofing_results_args, ) - job.perform(Time.zone.now) expect(job_analytics).to have_logged_event( @@ -360,8 +350,9 @@ end it 'logs a message with counts of various outcomes when the job completes (errored = 0)' do + enrollment_records = InPersonEnrollment.where(id: pending_enrollments.map(&:id)) allow(InPersonEnrollment).to receive(:needs_usps_status_check). - and_return(pending_enrollments) + and_return(enrollment_records) stub_request_proofing_results_with_responses( request_passed_proofing_results_args, ) @@ -390,7 +381,7 @@ it 'logs a message with counts of various outcomes when the job completes (no enrollments)' do allow(InPersonEnrollment).to receive(:needs_usps_status_check). - and_return([]) + and_return(InPersonEnrollment.none) stub_request_proofing_results_with_responses( request_passed_proofing_results_args, ) @@ -424,7 +415,7 @@ it 'logs failure details' do allow(UspsInPersonProofing::Proofer).to receive(:new).and_return(proofer) allow(proofer).to receive(:request_proofing_results).and_raise(error) - expect(NewRelic::Agent).to receive(:notice_error).with(error) + expect(NewRelic::Agent).to receive(:notice_error).with(error).at_least(1).times job.perform(Time.zone.now) @@ -444,8 +435,9 @@ let(:request_delay_ms) { 750 } it 'adds a delay between requests to USPS' do + enrollment_records = InPersonEnrollment.where(id: pending_enrollments.map(&:id)) allow(InPersonEnrollment).to receive(:needs_usps_status_check). - and_return(pending_enrollments) + and_return(enrollment_records) stub_request_passed_proofing_results expect(job).to receive(:sleep).exactly(pending_enrollments.length - 1).times. with(0.75) @@ -458,10 +450,10 @@ it 'generates a backwards-compatible unique ID' do pending_enrollment.update(unique_id: nil) stub_request_passed_proofing_results - expect(pending_enrollment).to receive(:usps_unique_id).and_call_original + expect_any_instance_of(InPersonEnrollment).to receive(:usps_unique_id).and_call_original job.perform(Time.zone.now) - + pending_enrollment.reload expect(pending_enrollment.unique_id).not_to be_nil end end @@ -642,9 +634,11 @@ expected_wait_until = 1.hour.from_now expect do job.perform(Time.zone.now) + pending_enrollment.reload end.to have_enqueued_job(InPerson::SendProofingNotificationJob). with(pending_enrollment.id).at(expected_wait_until).on_queue(:intentionally_delayed) end + expect(pending_enrollment.proofed_at).to eq(transaction_end_date_time) expect(job_analytics).to have_logged_event( 'GetUspsProofingResultsJob: Enrollment status updated', @@ -687,6 +681,7 @@ job.perform(Time.zone.now) end + pending_enrollment.reload expect(pending_enrollment.proofed_at).to eq(transaction_end_date_time) expect(job_analytics).to have_logged_event( 'GetUspsProofingResultsJob: Enrollment status updated', @@ -729,6 +724,7 @@ job.perform(Time.zone.now) end + pending_enrollment.reload expect(pending_enrollment.proofed_at).to eq(transaction_end_date_time) expect(job_analytics).to have_logged_event( 'GetUspsProofingResultsJob: Enrollment status updated', @@ -772,6 +768,7 @@ job.perform Time.zone.now end + pending_enrollment.reload expect(pending_enrollment.proofed_at).to eq(transaction_end_date_time) expect(job_analytics).to have_logged_event( 'GetUspsProofingResultsJob: Enrollment status updated', @@ -889,8 +886,10 @@ context 'when an enrollment code is invalid' do # this enrollment code is hardcoded into the fixture # request_unexpected_invalid_enrollment_code_response.json - let(:pending_enrollment) do - create(:in_person_enrollment, :pending, enrollment_code: '1234567890123456') + let(:pending_enrollments) do + [ + create(:in_person_enrollment, :pending, enrollment_code: '1234567890123456'), + ] end before(:each) do stub_request_unexpected_invalid_enrollment_code @@ -914,8 +913,10 @@ context 'when a unique id is invalid' do # this unique id is hardcoded into the fixture # request_unexpected_invalid_applicant_response.json - let(:pending_enrollment) do - create(:in_person_enrollment, :pending, unique_id: '123456789abcdefghi') + let(:pending_enrollments) do + [ + create(:in_person_enrollment, :pending, unique_id: '123456789abcdefghi'), + ] end before(:each) do stub_request_unexpected_invalid_applicant @@ -1005,7 +1006,7 @@ it 'logs the error to NewRelic' do expect(NewRelic::Agent).to receive(:notice_error). - with(instance_of(Faraday::BadRequestError)) + with(instance_of(Faraday::BadRequestError)).at_least(1).times job.perform(Time.zone.now) end end @@ -1031,7 +1032,7 @@ it 'logs the error to NewRelic' do expect(NewRelic::Agent).to receive(:notice_error). - with(instance_of(Faraday::ClientError)) + with(instance_of(Faraday::ClientError)).at_least(1).times job.perform(Time.zone.now) end end @@ -1051,7 +1052,7 @@ it 'logs the error to NewRelic' do expect(NewRelic::Agent).to receive(:notice_error). - with(instance_of(Faraday::ServerError)) + with(instance_of(Faraday::ServerError)).at_least(1).times job.perform(Time.zone.now) end end @@ -1127,8 +1128,6 @@ ) end before do - allow(InPersonEnrollment).to receive(:needs_usps_status_check). - and_return([pending_enrollment]) allow(IdentityConfig.store).to receive(:in_person_proofing_enabled).and_return(true) end @@ -1152,6 +1151,7 @@ freeze_time do expect do job.perform Time.zone.now + pending_enrollment.reload end.to have_enqueued_job(InPerson::SendProofingNotificationJob). with(pending_enrollment.id).at(1.hour.from_now).on_queue(:intentionally_delayed) end diff --git a/spec/models/in_person_enrollment_spec.rb b/spec/models/in_person_enrollment_spec.rb index 99758257f47..372604202c0 100644 --- a/spec/models/in_person_enrollment_spec.rb +++ b/spec/models/in_person_enrollment_spec.rb @@ -148,7 +148,7 @@ let!(:failing_enrollment) { create(:in_person_enrollment, :failed) } let!(:expired_enrollment) { create(:in_person_enrollment, :expired) } let!(:checked_pending_enrollment) do - create(:in_person_enrollment, :pending, status_check_attempted_at: Time.zone.now) + create(:in_person_enrollment, :pending, last_batch_claimed_at: Time.zone.now) end let!(:needy_enrollments) do [ @@ -193,7 +193,7 @@ end let!(:checked_pending_enrollment) do create( - :in_person_enrollment, :pending, status_check_attempted_at: Time.zone.now, + :in_person_enrollment, :pending, last_batch_claimed_at: Time.zone.now, ready_for_status_check: true ) end