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

Fixes #8002: Add ability to change username #10497

Merged
merged 35 commits into from Sep 9, 2019

Conversation

@maxme
Copy link
Contributor

commented Sep 9, 2019

Fixes #8002: Add ability to change username

To test:

  • Open Account Settings.
  • Tap "username".
  • Enter base name and wait for suggestions.
  • Pick a suggestion.
  • Enter confirmation.
  • Check the username was updated.
  • Hit back, check the username is updated on the "Me" screen.

Note: this is the merge of the feature branch. Code and feature in this branch was already reviewed and tested in:

There was one remaining issue and it was fixed in FluxC via wordpress-mobile/WordPress-FluxC-Android#1378 - This PR targets a recent FluxC beta (1.4.0-beta-1) including that fix.

cc @jd-alexander @malinajirka

Update release notes:

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary. Updated in 8827832
jd-alexander and others added 30 commits Aug 20, 2019
This flag will determine if dialog will be dismissed after confirm is clicked. It's default behavior is to dismiss after confirm since the existing implementations utilize this.
…_fix

Added dismissAfterConfirm flag to FullScreenDialogFragment
…ality can be shared.

The class now has the common functionality that will be needed in the sign-up and the account settings flow. The behaviour that differs with the header & analytics will be implemented in the respective classes that extend it.
…d Injector method since it makes it easier to test.

The FragmentInjector can be replaced during tests. The older method requires modification to the TestComponents/TestModules etc. Also corrected how injections were taking place by moving injection to the subtype instead of the base type since the best practice is to inject subtypes.
Verifies that the suggestions functionality is still intact even though the implementation has been modified. Created an environment where the fragment could be tested in isolation utilizing Mockito,Dagger's Android Injection and a fragment container.
…gment_abstract

UsernameChangerFullScreenDialogFragment made abstract for extensibility
…een.

- Create the new fragment with the functionality specific to the Settings behaviour.
- The Account Settings username component now ties into the feature and gets updated once the username has been changed.
- Updated FluxC hash to utilize the changes related to this PR from the username changer feature branch.
- Added the fragment to the ApplicationModule and made a minor typo fix.
…t each time username changer fragment

is being opened.
…cessary getters for the fields that needed to be exposed.
…asses extending need to have different implementations of it.
This reverts commit 8817437.
…anger_functionality

Implemented change username functionality in the Account Settings
malinajirka and others added 5 commits Aug 30, 2019
@maxme maxme added the /Me label Sep 9, 2019
@maxme maxme added this to the 13.3 milestone Sep 9, 2019
@maxme maxme self-assigned this Sep 9, 2019
@maxme maxme changed the title Issue/8002 change username Fixes #8002: Add ability to change username Sep 9, 2019
@peril-wordpress-mobile

This comment has been minimized.

Copy link

commented Sep 9, 2019

You can test the changes on this Pull Request by downloading the APK here.

@maxme

This comment has been minimized.

Copy link
Contributor Author

commented Sep 9, 2019

Feature works as expected.
Errors (suggestions and changes) are correctly handled.
Espresso tests were removed - a3f5c51
In memory AccountModel is correctly updated after username change.
a12s can't update their username, other users can.

:shipit:

@maxme maxme requested review from shiki and malinajirka Sep 9, 2019
@shiki
shiki approved these changes Sep 9, 2019
Copy link
Member

left a comment

🚢

@shiki shiki merged commit ade5efa into develop Sep 9, 2019
6 checks passed
6 checks passed
Peril All green. Well done.
Details
ci/circleci: Installable Build Your tests passed on CircleCI!
Details
ci/circleci: connected-tests Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: strings-check Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
@shiki shiki deleted the issue/8002-change-username branch Sep 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.