Auto switch account if user only has 1 device connected when login #5281
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This stack of pull requests is managed by Graphite. Learn more about stacking. |
This was referenced Nov 15, 2024
cmonfortep
force-pushed
the
feature/cristian/sync/offer_join_account_as_option
branch
from
95ba72c to
3941975
Compare
cmonfortep
force-pushed
the
feature/cristian/sync/auto_switch_single_device_account
branch
from
5e3ae88 to
b60c4a3
Compare
cmonfortep
force-pushed
the
feature/cristian/sync/offer_join_account_as_option
branch
from
5f14b03 to
7ab217c
Compare
cmonfortep
force-pushed
the
feature/cristian/sync/auto_switch_single_device_account
branch
2 times, most recently
from
2f94d43 to
0d49333
Compare
cmonfortep
force-pushed
the
feature/cristian/sync/offer_join_account_as_option
branch
from
7ab217c to
29721ff
Compare
cmonfortep
force-pushed
the
feature/cristian/sync/auto_switch_single_device_account
branch
from
0d49333 to
4965dc7
Compare
CDRussell
approved these changes
Nov 20, 2024
Comment on lines
+37
to
+47
| <issue | ||
| id="DenyListedApi" | ||
| message="If you find yourself using this API in production, you're doing something wrong!!" | ||
| errorLine1=" this.seamlessAccountSwitching().setRawStoredState(State(true))" | ||
| errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"> | ||
| <location | ||
| file="src/test/java/com/duckduckgo/sync/impl/AppSyncAccountRepositoryTest.kt" | ||
| line="100" | ||
| column="9"/> | ||
| </issue> | ||
|
|
There was a problem hiding this comment.
Nit: For tests, think we should just suppress the lint warning instead of using the baseline (to avoid lint errors resurfacing in the future when the file is changed).
| @@ -288,6 +300,7 @@ class AppSyncAccountRepository @Inject constructor( | |||
|
|
|||
| return when (val result = syncApi.getDevices(token)) { | |||
| is Error -> { | |||
| connectedDevicesCached.clear() | |||
There was a problem hiding this comment.
why is this added? if there is a blip in the network and getDevices call fails, do we want to clear the cached list?
There was a problem hiding this comment.
It's a convenient, quick solution, for us to have the latest response from get device in memory, so we can use it in the prompt logic. It's fine to clear on error, and only update/store on success.
cmonfortep
force-pushed
the
feature/cristian/sync/offer_join_account_as_option
branch
from
29721ff to
94f20f0
Compare
cmonfortep
force-pushed
the
feature/cristian/sync/auto_switch_single_device_account
branch
from
4965dc7 to
65c9705
Compare
cmonfortep
force-pushed
the
feature/cristian/sync/offer_join_account_as_option
branch
from
94f20f0 to
043a10a
Compare
cmonfortep
force-pushed
the
feature/cristian/sync/auto_switch_single_device_account
branch
from
65c9705 to
a88564f
Compare
Task/Issue URL: https://app.asana.com/0/72649045549333/1208755822305614/f ### Description Adds metrics around dialog engagement and errors. ### Steps to test this PR _single device flow metrics_ - [x] Fresh install - [x] Go to Feature flags and enable FF seamlessAccountSwitching - [x] Create a sync account - [x] Save or copy the recovery code - [x] logout - [x] Create a new sync account - [x] Go to "Sync With Another Device", and read or paste the recovery code - [x] Ensure user is automatically switched to the other account - [x] Ensure following pixels where triggered: sync_user_switched_account _multiple devices flow metrics_ - [x] Fresh install - [x] Go to Feature flags and enable FF seamlessAccountSwitching - [x] Create a sync account - [x] Save or copy the recovery code - [x] logout - [x] Join this account -> https://app.asana.com/0/1203822806345703/1208795798275452/f - [x] you are connected to an account with more devices - [x] Go to "Sync With Another Device", and read or paste the recovery code from the first account you created - [x] Ensure you are asked to confirm switching accounts - [x] Ensure pixel is triggered: sync_ask_user_to_switch_account - [ ] Click on Cancel and ensure pixel is triggered: sync_user_cancelled_switching_account - [x] Repeat the connection but now click "Switch" - [x] Ensure pixel is triggered: sync_user_accepted_switching_account - [x] If Switch is successful, ensure pixel is triggered: sync_user_switched_account _error metrics_ - [x] Fresh install - [x] Go to Feature flags and enable FF seamlessAccountSwitching - [x] Create a sync account - [x] Save or copy the recovery code - [x] INSTEAD OF LOGOUT, DELETE THE ACCOUNT (scroll to the bottom part of the screen) - [x] Create a new sync account - [x] Go to "Sync With Another Device", and read or paste the recovery code - [x] This will fail since the account doesn't exist (you will see another error that says invalid_login_credentials, it's expected) - [x] Ensure following pixels where triggered: sync_user_switched_logout_error - [x] Going back you will be in logout state ### UI changes | Before | After | | ------ | ----- | !(Upload before screenshot)|(Upload after screenshot)|
cmonfortep
merged commit 581a091
into
feature/cristian/sync/offer_join_account_as_option
4 of 5 checks passed
cmonfortep
deleted the
feature/cristian/sync/auto_switch_single_device_account
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Task/Issue URL: https://app.asana.com/0/1203822806345703/1208755822305613/f
Description
Introduces logic to switch the user account if sync has only 1 device connected.
When multiple devices connected, we still ask the user.
Steps to test this PR
Feature 1
Feature 1
UI changes