Skip to content

LG-13738 For Enhanced In-Person Enrollments Only, By-pass Primary ID Type Check in USPS Proofing Results Job#10917

Merged
gina-yamada merged 22 commits intomainfrom
yamada/LG-13738-By-pass-id-check-in-job-if-EIPP
Jul 12, 2024
Merged

LG-13738 For Enhanced In-Person Enrollments Only, By-pass Primary ID Type Check in USPS Proofing Results Job#10917
gina-yamada merged 22 commits intomainfrom
yamada/LG-13738-By-pass-id-check-in-job-if-EIPP

Conversation

@gina-yamada
Copy link
Contributor

@gina-yamada gina-yamada commented Jul 5, 2024

🎫 Ticket

LG-13738 By-pass Primary ID Type Check in USPS Proofing Results Job If EIPP

🛠 Summary of changes

  • For Enhanced In-Person Enrollments only, by pass the Primary ID Check in the USPS Proofing Job so an enrollment that passed proofing with unsupported id by USPS, that would otherwise fail, will pass and successfully create an enrollment

📜 Testing Plan

There are 4 tests that you should perform. All 4 must pass. For each test below, there is a description, instructions on how to simulate the scenario, and expected results. The bold shows the differences and should align with that specific test.

Before you start:

  1. Comment out: enrollment_outcomes[:enrollments_passed] += 1 in handle_successful_status_update inside app/jobs/get_usps_proofing_results_job.rb is commented out
  2. Comment out:enrollment_outcomes[:enrollments_failed] += 1 in handle_unsupported_id_type inside app/jobs/get_usps_proofing_results_job.rb

Test 1: In-Person Proofing Enrollment || Pass Proofing with Supported ID || Confirm existing behavior is working as expected (ie: Enrollment passes)

  • Come in from Sinatra. Pick Biometrics.
  • Create an IPP enrollment
  • Once on the Verify view (/verify/in_person/ready_to_verify), open a new terminal and run: rails c
  • Grab the enrollment: e = InPersonEnrollment.last
  • Mock a passed response: json = UspsInPersonProofing::Mock::Fixtures.request_passed_proofing_results_response
  • Parse it: res = JSON.parse(json)
  • Ensure that enrollment_outcomes[:enrollments_passed] += 1 in handle_successful_status_update inside app/jobs/get_usps_proofing_results_job.rb is commented out
  • Create a new job instance: job = GetUspsProofingResultsJob.new
  • Send it: job.send(:process_enrollment_response, e, res)
  • Update your cache: e.reload
  • Inspect enrollment to confirm status = passed

Test 2: In-Person Proofing Enrollment || Pass Proofing with Unsupported ID || Confirm existing behavior is working as expected (ie: Enrollment fails)

  • Come in from Sinatra. Pick Biometrics.
  • Create an IPP enrollment
  • Once on the Verify view (/verify/in_person/ready_to_verify), open a new terminal and run: rails c
  • Grab the enrollment: e = InPersonEnrollment.last
  • Mock a passed response with unsupported id: json = UspsInPersonProofing::Mock::Fixtures.request_passed_proofing_unsupported_id_results_response
  • Parse it: res = JSON.parse(json)
  • Ensure that enrollment_outcomes[:enrollments_failed] += 1 in handle_unsupported_id_type inside app/jobs/get_usps_proofing_results_job.rb is commented out
  • Create a new job instance: job = GetUspsProofingResultsJob.new
  • Send it: job.send(:process_enrollment_response, e, res)
  • Update your cache: e.reload
  • Inspect enrollment to confirm status = failed

Test 3: Enhanced In-Person Proofing Enrollment || Pass Proofing with Supported ID || Confirm existing behavior is working as expected (ie: Enrollment passes)

  • Come in from Sinatra. Pick Enhanced In-Person Proofing.
  • Create an Enhanced IPP enrollment
  • Once on the Verify view (/verify/in_person/ready_to_verify), open a new terminal and run: rails c
  • Grab the enrollment: e = InPersonEnrollment.last
  • Mock a passed response: json = UspsInPersonProofing::Mock::Fixtures.request_passed_proofing_results_response
  • Parse it: res = JSON.parse(json)
  • Ensure that enrollment_outcomes[:enrollments_passed] += 1 in handle_successful_status_update inside app/jobs/get_usps_proofing_results_job.rb is commented out
  • Create a new job instance: job = GetUspsProofingResultsJob.new
  • Send it: job.send(:process_enrollment_response, e, res)
  • Update your cache: e.reload
  • Inspect enrollment to confirm status = passed

Test 4: Enhanced In-Person Proofing Enrollment || Pass Proofing with Unsupported ID || Confirm new behavior is working as expected (ie: Enrollment passes)

  • Come in from Sinatra. Pick Enhanced In-Person Proofing.
  • Create an Enhanced IPP enrollment
  • Once on the Verify view (/verify/in_person/ready_to_verify), open a new terminal and run: rails c
  • Grab the enrollment: e = InPersonEnrollment.last
  • Mock a passed response with unsupported id: json = UspsInPersonProofing::Mock::Fixtures.request_passed_proofing_unsupported_id_results_response
  • Parse it: res = JSON.parse(json)
  • Ensure that enrollment_outcomes[:enrollments_passed] += 1 in handle_successful_status_update inside app/jobs/get_usps_proofing_results_job.rb is commented out
  • Create a new job instance: job = GetUspsProofingResultsJob.new
  • Send it: job.send(:process_enrollment_response, e, res)
  • Update your cache: e.reload
  • Inspect enrollment to confirm status = passed

@gina-yamada gina-yamada changed the title Yamada/lg 13738 by pass id check in job if eipp LG-13738 For Enhanced In-Person Enrollments Only, By-pass Primary ID Type Check in USPS Proofing Results Job Jul 5, 2024
Copy link
Contributor

@KeithNava KeithNava left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥 Amazing testing steps!! 💯

@gina-yamada gina-yamada requested a review from KeithNava July 10, 2024 14:22
Copy link
Contributor

@eileen-nava eileen-nava left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left some initial comments. This is looking good so far. I'll follow up after I run the testing plan.

end

context 'when the enrollment sponsor ID does not equal the EIPP sponsor ID' do
it 'returns false' do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above - I think it makes sense to move this setup out of the it block and into the context block.

You might be able to reduce duplication if you have the user/profile/enrollment setup in the describe block and then define the sponsor_id in the two context blocks.

Copy link
Contributor Author

@gina-yamada gina-yamada Jul 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • create is out of the it block - resolved with commit 01eff13
  • I passed a trait (:with_sponsor_id and :enhanced_ipp) into :in_person_enrolllment to add the sponsor id to the enrollment so I did not need to define it - resolved with commit b5eb47d
  • Shane helped me out with a much cleaner solution

@eileen-nava eileen-nava self-requested a review July 10, 2024 19:33
Copy link
Contributor

@eileen-nava eileen-nava left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran the testing plan and everything passed. Approved.

Copy link
Contributor

@jennyverdeyen jennyverdeyen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested all the scenarios locally and observed the expected behavior - looks great! 🎉

trait :with_sponsor_id do
sponsor_id { '123458' }
sponsor_id { IdentityConfig.store.usps_ipp_sponsor_id }
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eileen-nava Is there a reason I should not use config here and keep a random string?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will investigate and get back to you on that question. 👍🏻

Copy link
Contributor Author

@gina-yamada gina-yamada Jul 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, so here is what I have uncovered by testing:

  1. If we hard code the number here- enrollments using this bot will always have a sponsor_id
  2. If we don't hard code the number and use the config store, it will use the value passed into the spec when we mock it like so...
    allow(IdentityConfig.store).to receive(:usps_ipp_sponsor_id).and_return(usps_ipp_sponsor_id)
    allow(IdentityConfig.store).to receive(:usps_eipp_sponsor_id).and_return(usps_ipp_sponsor_id)
  3. If we don't hard code the number and use the config store, if there is no value passed into the spec it will be nil so sponsor id on enrollment will not be set. It looks like I can set usps_ipp_sponsor_id in application.yml.default in the test section and then for all of the specs without the mock setup- it will return that value.

I feel like using the config store above and adding a default config in application.yml.default is the best path forward.

UPDATE: I am going to delete your trait because it is no longer being used (and would then be in the in_person_enrollment factory if I add it)

Furthermore I think using the config rather than hard-coding is better. Inside spec/factories/profiles.rb- trait ::letter_sends_rate_limited is using the config store for max_mail_events. I am seeing that config set in config/application.yml.default in the test section.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like I can set usps_ipp_sponsor_id in application.yml.default in the test section and then for all of the specs without the mock setup- it will return that value.

I agree with this path forward. In response to the UPDATE, I also agree that using the config is better than hardcoding.

association :notification_phone_configuration
end

trait :with_sponsor_id do
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I deleted this trait now that it comes on :in_person_enrollment - see line 8 above

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me. 👍🏻

verify_personal_key_max_attempts: 2
usps_ipp_root_url: 'http://localhost:1000'
usps_ipp_sponsor_id: '111111111111111'
usps_eipp_sponsor_id: '222222222222222'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added these configs in the test section so that spec/factories/in_person_enrollments.rb will always have a value when they are not set in individual specs

Copy link
Contributor

@jennyverdeyen jennyverdeyen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-tested all the scenarios and they are all behaving as expected! LGTM

@gina-yamada
Copy link
Contributor Author

gina-yamada commented Jul 12, 2024

I had Eileen and Shane jump on and re-test. I was seeing behavior where I had to sign out and back in to see the Verified tag. (I first noticed on this branch but also observed the exact behavior on main. Updates to config/application.yml seemed to contributed to this (although I was in incognito.)) I was able to see the Verified tag successfully after I updated my config/application.yml to align with what Eileen had.

Eileen and Shane did not see this locally. Jenny also commented and had success.

What Eileen and Shane observed for test 1 after running the job
13738IdentityVerifiedAccountPage

What Gina observed for test 1 after running the job
Screenshot 2024-07-12 at 8 18 15 AM

@gina-yamada gina-yamada merged commit 5d4aed8 into main Jul 12, 2024
@gina-yamada gina-yamada deleted the yamada/LG-13738-By-pass-id-check-in-job-if-EIPP branch July 12, 2024 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants