Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(phone): Add removePhoneNumber to RecoveryPhoneService #18035

Merged
merged 1 commit into from
Nov 25, 2024
Merged

Conversation

vbudhram
Copy link
Contributor

Because

  • We need to be able to remove a recovery phone number

This pull request

  • Adds removePhoneNumber, which throws error if nothing was removed

Issue that this pull request solves

Closes: https://mozilla-hub.atlassian.net/browse/FXA-10349

Checklist

  • My commit is GPG signed.
  • If applicable, I have modified or added tests which pass locally.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).

@vbudhram vbudhram requested a review from a team as a code owner November 18, 2024 16:27
@vbudhram vbudhram self-assigned this Nov 18, 2024
@vbudhram vbudhram requested a review from dschom November 18, 2024 16:27
mockRecoveryPhoneManager.removePhoneNumber.mockRejectedValueOnce(
new RecoveryNumberNotExistsError(uid)
);
expect(service.removePhoneNumber(uid)).rejects.toThrow();
Copy link
Contributor

Choose a reason for hiding this comment

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

tiny nit, check that it throws a RecoveryNumberNotExistsError and not a different error.

* Remove phone number from an account. Each user can only have one associated
* phone number.
*
* @param uid
Copy link
Contributor

Choose a reason for hiding this comment

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

Document what uid is. (This is just for consistency... we all know what it is ;p)

Copy link
Contributor

@dschom dschom left a comment

Choose a reason for hiding this comment

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

LGTM

@vbudhram vbudhram merged commit 4abcedf into main Nov 25, 2024
25 checks passed
@vbudhram vbudhram deleted the fxa-10349 branch November 25, 2024 21:53
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