feat(settings): Enable changing recovery phone#18913
Conversation
vpomerleau
changed the title
nshirley
left a comment
nshirley
left a comment
There was a problem hiding this comment.
Nicely done! Glad to see the endpoint appears to be working too! Just one question I had but nothing blocking 😃
| const el = await this.page.waitForSelector( | ||
| `[data-testid=${this.id}-unit-row]` | ||
| ); | ||
| const el = this.page.getByTestId(`${this.id}-unit-row`); |
There was a problem hiding this comment.
Appreciate the clean up to use better functions for selecting elements! 🎉
| return result; | ||
| } | ||
|
|
||
| async changeRecoveryPhone(code: string) { |
There was a problem hiding this comment.
question (non-blocking) Is there a reason to change the language from replace to change here? I honestly like the verbiage of saying change more than replace, but I'm wondering if it would make sense to go in and update the backend to match this in a followup ticket (i.e., recoveryPhoneChange() and update all the way down)
There was a problem hiding this comment.
That's a good call out - it should be consistent. I'll clean this up to match the wording chosen for the button text ("Change").
vpomerleau
force-pushed
the
FXA-10372
branch
2 times, most recently
from
7bf9105 to
7f6e98f
Compare
Because: * We want to allow users to change their recovery phone This commit: * Adds an alternate CTA in the backup phone row to change the recovery phone if a phone number is already registered * Re-use the existing phone setup page with minimal conditional changes for the phone replacement flow * Add some stories and unit tests * Add reason code for glean events * Add functional test Closes #FXA-10372
3 participants
Because
This pull request
Issue that this pull request solves
Closes :FXA-10372
Checklist
Put an
xin the boxes that applyScreenshots (Optional)
Other information (Optional)
Draft until - all glean events verified/updated, additional unit and functional tests added