Skip to content

LG-9821: Consolidate Phone setup Controller PT 1#8966

Merged
mdiarra3 merged 17 commits intomainfrom
LG-9821-consolidate-phone-controllers
Aug 23, 2023
Merged

LG-9821: Consolidate Phone setup Controller PT 1#8966
mdiarra3 merged 17 commits intomainfrom
LG-9821-consolidate-phone-controllers

Conversation

@mdiarra3
Copy link
Contributor

@mdiarra3 mdiarra3 commented Aug 9, 2023

🎫 Ticket

LG-9821

🛠 Summary of changes

Adds a check and redirect to standard phone setup page.

@mdiarra3 mdiarra3 requested a review from a team August 9, 2023 15:09
@mdiarra3 mdiarra3 requested a review from aduth August 17, 2023 13:34
Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

This is looking good! Left a couple comments

Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

LGTM 👍

phone: @new_phone_form.phone,
selected_delivery_method: @new_phone_form.otp_delivery_preference,
phone_type: @new_phone_form.phone_info&.type,
selected_default_number: @new_phone_form.otp_make_default_number,
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed a slight difference from main, the selected_default_number parameter is now always present in the URL when confirming a number, and it's false when confirming the first phone number. I don't think this makes a meaningful impact, since I checked that in main, the absence of the parameter is effectively the same as it being false.

@mdiarra3 mdiarra3 merged commit 862d2ab into main Aug 23, 2023
@mdiarra3 mdiarra3 deleted the LG-9821-consolidate-phone-controllers branch August 23, 2023 13:48
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