Skip to content

LG-8834 add phone to ready to verify email#7767

Merged
svalexander merged 3 commits intomainfrom
shannon/lg-8834-add-phone-to-ready-to-verify-email
Feb 3, 2023
Merged

LG-8834 add phone to ready to verify email#7767
svalexander merged 3 commits intomainfrom
shannon/lg-8834-add-phone-to-ready-to-verify-email

Conversation

@svalexander
Copy link
Contributor

🎫 Ticket

Lg-8834

🛠 Summary of changes

Get phone from selected_location_details so that it is included in ready to verify email

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

👀 Screenshots

If relevant, include a screenshot or screen capture of the changes.

Before: Screen Shot 2023-02-03 at 11 12 31 AM
After: Screen Shot 2023-02-03 at 11 12 14 AM

@eileen-nava eileen-nava self-requested a review February 3, 2023 16:31
@eileen-nava
Copy link
Contributor

@svalexander This looks good, could you please add a test? Thanks!

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.

Once the build passes, I think this is good to merge.

@svalexander svalexander changed the title Shannon/lg 8834 add phone to ready to verify email LG-8834 add phone to ready to verify email Feb 3, 2023
@svalexander svalexander merged commit c7230cb into main Feb 3, 2023
@svalexander svalexander deleted the shannon/lg-8834-add-phone-to-ready-to-verify-email branch February 3, 2023 17:40
</div>
<div>
<%= @presenter.selected_location_details[:phone] %>
<%= @presenter.selected_location_details['phone'] %>
Copy link
Contributor

Choose a reason for hiding this comment

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

I've seen we've had a few similar instances of this sort of problem since it was introduced. I wonder if we could make this hash a HashWithIndifferentAccess so that it would support keys referenced either as strings or as symbols. It's complicated by the fact that this is a database field, but maybe it's possible to override the method on the InPersonEnrollment class?

@aduth aduth mentioned this pull request Feb 9, 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