Skip to content

LG-4258: Changing Default phone number should require multiple numbers associated with account#7095

Merged
jmdembe merged 12 commits intomainfrom
LG-4258-default-phone-number
Oct 12, 2022
Merged

LG-4258: Changing Default phone number should require multiple numbers associated with account#7095
jmdembe merged 12 commits intomainfrom
LG-4258-default-phone-number

Conversation

@jmdembe
Copy link
Contributor

@jmdembe jmdembe commented Oct 5, 2022

🎫 Ticket

LG-4258

🛠 Summary of changes

If a user has only one phone number set up on their account, they should not be able to select or deselect the "Make default number" checkbox. With this change, the checkbox will be disabled when there is one number associated with an account as well as a messaging that lays out why they cannot change their default number.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

  • Unit tests pass

👀 Screenshots

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

Before After
Image of manage phone number before checkbox change is made image of change with checkbox disabled

🚀 Notes for Deployment

N/A

@jmdembe jmdembe force-pushed the LG-4258-default-phone-number branch from 9033bfe to 525a317 Compare October 5, 2022 19:56
@jmdembe jmdembe changed the title Lg 4258 default phone number LG-4258: Changing Default phone number should require multiple numbers associated with account Oct 5, 2022
@jmdembe jmdembe force-pushed the LG-4258-default-phone-number branch 2 times, most recently from e2ed402 to ca0ea52 Compare October 5, 2022 21:19
@jmdembe jmdembe marked this pull request as ready for review October 6, 2022 14:52
@jmdembe jmdembe requested a review from a team October 6, 2022 14:53
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have an issue with phone showing default even if its just one? or do we think its not needed information.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on the design, it was deemed as not needing that information

Copy link
Contributor

Choose a reason for hiding this comment

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

The EditPhoneForm class doesn't currently have great test coverage for its methods, but could we try adding some for this new method?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree 🙂

@jmdembe jmdembe force-pushed the LG-4258-default-phone-number branch 2 times, most recently from c5a8707 to fcb2522 Compare October 6, 2022 18:32
@jmdembe jmdembe force-pushed the LG-4258-default-phone-number branch from fcb2522 to 32efa09 Compare October 6, 2022 19:24
Copy link
Contributor

@mitchellhenke mitchellhenke left a comment

Choose a reason for hiding this comment

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

Looks great!

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree 🙂

@jmdembe jmdembe force-pushed the LG-4258-default-phone-number branch 3 times, most recently from d18c478 to cccdfef Compare October 7, 2022 19:36
@jmdembe jmdembe force-pushed the LG-4258-default-phone-number branch from cccdfef to 72bccd2 Compare October 7, 2022 19:50
@jmdembe jmdembe merged commit 2e862d6 into main Oct 12, 2022
@jmdembe jmdembe deleted the LG-4258-default-phone-number branch October 12, 2022 17:24
jskinne3 pushed a commit that referenced this pull request Oct 12, 2022
…s associated with account (#7095)

* do not display "manage phone number" if a user only has one phone method

* add method that returns boolean for 1 phone method configuered

* add test to reflect change in default text on account screen

* add switch to display text when user only has 1 phone method set up

* add logic and text for default number instructions

* add and edit supporting tests

changelog: Improvements, UX, disable checkbox when only one phone number is set

* remove unused code

* add missing french keys

* address lint errors

* Address PR comments-edit text and re-work test

* write a test to test one_phone_configuration method

* fix test for edit form spec
@jmdembe jmdembe mentioned this pull request Oct 13, 2022
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.

4 participants