Skip to content

LG-0385 Write to in_person_verification_pending_at timestamp.#8960

Merged
theabrad merged 14 commits intomainfrom
abrad-lg-10385-ipp-write-columns
Aug 9, 2023
Merged

LG-0385 Write to in_person_verification_pending_at timestamp.#8960
theabrad merged 14 commits intomainfrom
abrad-lg-10385-ipp-write-columns

Conversation

@theabrad
Copy link
Contributor

@theabrad theabrad commented Aug 8, 2023

🎫 Ticket

LG-10383

🛠 Summary of changes

We are deprecating the profile#deactivation_reason(:in_person_verification_pending) enum for a in_person_verification_pending_at timestamp. This PR will write to both for now until we complete a backfill to add timestamps to all users who are undergoing in person verification.

theabrad and others added 7 commits August 7, 2023 17:06
we no longer pass in_person_verification_pending to the initialize
method for profile maker

Co-authored-by: Sonia Connolly <sonia.connolly@gsa.gov>
added this method and split from
profile#deactivate_for_in_person_verification_and_schedule_enrollment
profile#deactivate_for_in_person_verification_and_schedule_enrollment

removed this method as coupling them didn't work when creating a profile
for phone finder.
@theabrad theabrad requested a review from a team August 8, 2023 19:28
Copy link
Contributor

@kbighorse kbighorse left a comment

Choose a reason for hiding this comment

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

question:

We now have the following:

  1. User#pending_in_person_enrollment
  2. Idv::GpoVerifyController#pending_in_person_enrollment? (calls 1.)
  3. Profile#pending_in_person_enrollment? (calls Profile#proofing_components)
  4. Idv::Session#pending_in_person_enrollment? (calls User#proofing_component)

Is there a way to consolidate/differentiate some of this? Might be worth some future consideration.

Otherwise 👍🏾 🚀

if in_person_verification_pending
profile.deactivation_reason = :in_person_verification_pending
end
profile.deactivate_for_in_person_verification if in_person_verification_needed
Copy link
Contributor

Choose a reason for hiding this comment

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

# note: deactivation reason will be replaced by timestamp column
deactivation_reason == 'in_person_verification_pending'
deactivation_reason == 'in_person_verification_pending' ||
in_person_verification_pending_at?
Copy link
Contributor

Choose a reason for hiding this comment

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

This is reading rather than writing. Are we ready to start reading the new value?


expect(profile.activated_at).to be_nil
expect(profile.active).to eq(false)
expect(profile.deactivation_reason).to eq('in_person_verification_pending')
Copy link
Contributor

Choose a reason for hiding this comment

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

Should there be an expectation for profile.in_person_verification_pending_at on this spec and the others in this file?

Copy link
Contributor

Choose a reason for hiding this comment

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

also ProfileSpec

expect(profile.deactivation_reason).to eq 'in_person_verification_pending' # to change
expect(profile.fraud_review_pending?).to eq(true) # to change
expect(profile.gpo_verification_pending_at).to be_nil
expect(profile.in_person_verification_pending_at).to_not be_nil
Copy link
Contributor

Choose a reason for hiding this comment

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

add this before and after test throughout the other tests

@soniaconnolly soniaconnolly self-requested a review August 9, 2023 15:13
Copy link
Contributor

@soniaconnolly soniaconnolly left a comment

Choose a reason for hiding this comment

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

LGTM

@theabrad theabrad merged commit 9a1d144 into main Aug 9, 2023
@theabrad theabrad deleted the abrad-lg-10385-ipp-write-columns branch August 9, 2023 18:46
@jmhooper jmhooper mentioned this pull request Aug 10, 2023
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