Skip to content

Update account page#11143

Closed
eileen-nava wants to merge 2 commits intomainfrom
em/update_account_page
Closed

Update account page#11143
eileen-nava wants to merge 2 commits intomainfrom
em/update_account_page

Conversation

@eileen-nava
Copy link
Copy Markdown
Contributor

No description provided.

…lowing up for expired, failed, and cancelled in_person_enrollments
@aduth
Copy link
Copy Markdown
Contributor

aduth commented Aug 26, 2024

Why not implement this in in_person_verification_pending? ?

Comment on lines +66 to +75
in_person_enrollment = user&.in_person_enrollments&.first
if in_person_enrollment&.expired?
user.pending_in_person_enrollment.present?
elsif in_person_enrollment&.cancelled?
user.pending_in_person_enrollment.present?
elsif in_person_enrollment&.failed?
user.pending_in_person_enrollment.present?
else
!!user.pending_profile&.in_person_verification_pending?
end
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe simpler option to check that the enrollment associated with the pending profile is explicitly pending?

Suggested change
in_person_enrollment = user&.in_person_enrollments&.first
if in_person_enrollment&.expired?
user.pending_in_person_enrollment.present?
elsif in_person_enrollment&.cancelled?
user.pending_in_person_enrollment.present?
elsif in_person_enrollment&.failed?
user.pending_in_person_enrollment.present?
else
!!user.pending_profile&.in_person_verification_pending?
end
!!user.pending_profile&.in_person_enrollment&.pending?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback. 🙏🏻 I am going to close this PR because, as Gina mentioned here, Joy decided to backfill the data. This PR is no longer relevant.

(I am going to merge the regression specs in a separate PR.)

@eileen-nava
Copy link
Copy Markdown
Contributor Author

I am closing this PR because we decided to backfill the data. The change in AccountShowPresenter is no longer relevant.

I will merge the regression specs in PR #11141.

@eileen-nava eileen-nava deleted the em/update_account_page branch December 12, 2024 15:03
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