Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure the in-person verifcation profile has an enrollment record #11315

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

h-m-m
Copy link
Contributor

@h-m-m h-m-m commented Oct 4, 2024

🛠 Summary of changes

In theory, it should never happen that the profile is pending verification and the enrollment record does not exist. A previous discussion in a meeting that I missed brought up the point that the factory does not address this properly.

This is essentially pulling @aduth's work out of #11129 , one of the smallest pieces of that PR that is still useful to the development team on its own. In fact, in pairing with @eileen-nava on this, I basically retraced much of what @aduth had done before @eileen-nava pointed out the repetition.

We looked at pulling in more from #11129 , and doing so looked like it might double the level of effort

📜 Testing Plan

At @eileen-nava's suggestion when we were working together, we walked through inserting a binding.pry in a test that uses this factory and made sure that we got appropriate results back. I'm greatful to Eileen for walking me through a complicated process that I didn't understand and pointing out which details should be checked. For example:

From: /Users/hmiller/workspace/identity-idp/spec/models/profile_spec.rb:1053 :

    1048:   describe '#deactivate_due_to_in_person_verification_cancelled' do
    1049:     let(:profile) { create(:profile, :in_person_verification_pending) }
    1050:     context 'when the profile does not have a deactivation reason' do
    1051:       it 'updates the profile and sets the deactivation reason to "verification_cancelled"' do
    1052:         binding.pry
 => 1053:         expect(profile.deactivation_reason).to be_nil

[1] pry(#<RSpec::ExampleGroups::Profile::DeactivateDueToInPersonVerificationCancelled::WhenTheProfileDoesNotHaveADeactivationReason>)> profile.user.has_establishing_in_person_enrollment?
=> false
[2] pry(#<RSpec::ExampleGroups::Profile::DeactivateDueToInPersonVerificationCancelled::WhenTheProfileDoesNotHaveADeactivationReason>)> profile.in_person_verification_pending?
=> true
[3] pry(#<RSpec::ExampleGroups::Profile::DeactivateDueToInPersonVerificationCancelled::WhenTheProfileDoesNotHaveADeactivationReason>)> profile.user.has_in_person_enrollment?
=> true

as well as checking other relational data.

In addition, our test suite is thorough enough that I have high confidence if the full test suite passes with this change. While making only some of these changes, many tests failed loudly.

In theory, it should never happen that the profile is pending verification and the enrollment record does not exist.

changelog: Internal, Automated Testing, Improve test setup for enrolling profiles
@h-m-m h-m-m force-pushed the rspec-league/improve-ipp-pending-profile-factory branch from 9b8cd7e to 570b052 Compare October 8, 2024 13:51
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.

2 participants