Skip to content

LG-6872: Creation USPS enrollment when IPP user re-enters their password#6615

Merged
sheldon-b merged 23 commits intomainfrom
sbachstein/lg-6872-create-usps-enrollment-in-ipp-flow
Jul 21, 2022
Merged

LG-6872: Creation USPS enrollment when IPP user re-enters their password#6615
sheldon-b merged 23 commits intomainfrom
sbachstein/lg-6872-create-usps-enrollment-in-ipp-flow

Conversation

@sheldon-b
Copy link
Contributor

@sheldon-b sheldon-b commented Jul 21, 2022

Begins creating USPS enrollments for user who complete the in-person proofing flow

  • When a user re-enters their password after completing the in-person proofing flow we create an InPersonProofingEnrollment record and make an API request to enroll the user in USPS IPPaaS
  • Adds a mock_usps_fallback feature flag which, when active (which is the default), causes the application not to make real requests to the USPS API
  • Adds a new UspsInPersonProofing module with a mock proofer. Moves the existing proofer into this module

📋 Future work

There are several chunks of work that I didn't have time to complete in this first PR. I'm hoping that we can merge this PR the morning of 7/21/2022 so that we can use it to bug bash and then continue to flesh this work out in a future PR.

  • Error handling for when requests to the USPS IPPaaS return errors or unexpected data
  • Showing error indicators to the user when USPS IPPaaS requests fail
  • Sending the user's address2 information to USPS

first_name: pii.first_name,
last_name: pii.last_name,
address: pii.address1,
# do we need address2?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do, I just didn't have time to add it in this PR. Will add in a follow-up

Comment on lines +89 to +90
response = proofer.request_enroll(applicant)
response['enrollmentCode']
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'm planning on adding error handling in a follow-up PR

Copy link
Contributor

Choose a reason for hiding this comment

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

WDYT of creating a Struct-like object to repesent the response here so we could do a direct property access response.enrollment_code rather than digging into a hash? Makes mocking responses easier as well

# Returns the value to use for the USPS enrollment ID
def usps_unique_id
user_id.to_s
user.uuid.delete('-').slice(0, 18)
Copy link
Contributor Author

@sheldon-b sheldon-b Jul 21, 2022

Choose a reason for hiding this comment

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

This is just a temporary solution. There's a separate ticket to think through the best way to generate unique IDs

@@ -0,0 +1,182 @@
module UspsInPersonProofing
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 just moved this file and moved the class into a module. The namespace and class name have changed but otherwise this is unchanged

Copy link
Contributor

Choose a reason for hiding this comment

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

Should move & rename the test file as well, from

spec/services/usps_in_person_proofer_spec.rb

to

spec/services/usps_in_person_proofing/proofer_spec.rb

@sheldon-b sheldon-b changed the title (WIP) LG-6872: Creation USPS enrollment when IPP user re-enters their password LG-6872: Creation USPS enrollment when IPP user re-enters their password Jul 21, 2022
@sheldon-b sheldon-b marked this pull request as ready for review July 21, 2022 05:58
@sheldon-b sheldon-b requested a review from a team July 21, 2022 15:00
Comment on lines +89 to +90
response = proofer.request_enroll(applicant)
response['enrollmentCode']
Copy link
Contributor

Choose a reason for hiding this comment

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

WDYT of creating a Struct-like object to repesent the response here so we could do a direct property access response.enrollment_code rather than digging into a hash? Makes mocking responses easier as well

Comment on lines +96 to +99
enrollment = InPersonEnrollment.create!(
profile: profile,
user: user,
)
Copy link
Contributor

Choose a reason for hiding this comment

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

so I had a thought I dropped in slack this morning about maybe we flip the order of these.. could be a future PR if needed!

Copy link
Contributor

@tomas-nava tomas-nava left a comment

Choose a reason for hiding this comment

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

approve! with some non-blocking comments & suggestions

I do wonder if there's a better way to name & organize the proofing modules & structs, but this works for now.

@@ -59,6 +60,52 @@ def in_person_enrollment?(user)
# WILLFIX: After LG-6872 and we have enrollment saved, reference enrollment instead.
Copy link
Contributor

Choose a reason for hiding this comment

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

Anything we need to think about here? I think the assumption at the time this was written was that we might create an enrollment earlier in the process.

Copy link
Contributor Author

@sheldon-b sheldon-b Jul 22, 2022

Choose a reason for hiding this comment

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

I think it's fine as-is. This method gets used twice: once before the enrollment is created and once after. The enrollment is only a useful indicator of this being an in-person flow after it is created

I'm going to leave it as-is in this file and will begin using the enrollment on the next page, in the personal key controller

# update the enrollment to status pending
enrollment.enrollment_code = enrollment_code
enrollment.status = :pending
enrollment.save!
Copy link
Contributor

Choose a reason for hiding this comment

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

Will need error handling here too, in case there's an existing pending enrollment.

@@ -0,0 +1,182 @@
module UspsInPersonProofing
Copy link
Contributor

Choose a reason for hiding this comment

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

Should move & rename the test file as well, from

spec/services/usps_in_person_proofer_spec.rb

to

spec/services/usps_in_person_proofing/proofer_spec.rb

@sheldon-b
Copy link
Contributor Author

Thanks for the reviews! I'm going to merge this so we can use it in bug bash and I'll address all of the comments in a separate PR

@sheldon-b sheldon-b merged commit e3a24d9 into main Jul 21, 2022
@sheldon-b sheldon-b deleted the sbachstein/lg-6872-create-usps-enrollment-in-ipp-flow branch July 21, 2022 16:19
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.

3 participants