Skip to content

Commit

Permalink
Auto switch account if user only has 1 device connected when login (#…
Browse files Browse the repository at this point in the history
…5281)

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_
- [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

_Feature 1_
- [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



### UI changes
| Before  | After |
| ------ | ----- |
!(Upload before screenshot)|(Upload after screenshot)|
  • Loading branch information
cmonfortep authored Nov 22, 2024
1 parent 043a10a commit 581a091
Show file tree
Hide file tree
Showing 13 changed files with 265 additions and 17 deletions.
15 changes: 13 additions & 2 deletions sync/sync-impl/lint-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,17 @@
message="Defined here, included via layout/activity_sync.xml => layout/view_sync_disabled.xml defines @+id/syncSetupOtherPlatforms"/>
</issue>

<issue
id="DenyListedApi"
message="If you find yourself using this API in production, you&apos;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>

<issue
id="DenyListedApi"
message="No one remembers what these constants map to. Use the API level integer value directly since it&apos;s self-defining."
Expand Down Expand Up @@ -107,7 +118,7 @@
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
<location
file="src/test/java/com/duckduckgo/sync/impl/ui/EnterCodeViewModelTest.kt"
line="62"
line="63"
column="9"/>
</issue>

Expand All @@ -118,7 +129,7 @@
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
<location
file="src/test/java/com/duckduckgo/sync/impl/ui/EnterCodeViewModelTest.kt"
line="136"
line="137"
column="9"/>
</issue>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,11 @@ class AppSyncAccountRepository @Inject constructor(
private val syncPixels: SyncPixels,
@AppCoroutineScope private val appCoroutineScope: CoroutineScope,
private val dispatcherProvider: DispatcherProvider,
private val syncFeature: SyncFeature,
) : SyncAccountRepository {

private val connectedDevicesCached: MutableList<ConnectedDevice> = mutableListOf()

override fun isSyncSupported(): Boolean {
return syncStore.isEncryptionSupported()
}
Expand Down Expand Up @@ -108,9 +111,20 @@ class AppSyncAccountRepository @Inject constructor(
}

private fun login(recoveryCode: RecoveryCode): Result<Boolean> {
var wasUserLogout = false
if (isSignedIn()) {
return Error(code = ALREADY_SIGNED_IN.code, reason = "Already signed in")
.alsoFireAlreadySignedInErrorPixel()
val allowSwitchAccount = syncFeature.seamlessAccountSwitching().isEnabled()
val error = Error(code = ALREADY_SIGNED_IN.code, reason = "Already signed in").alsoFireAlreadySignedInErrorPixel()
if (allowSwitchAccount && connectedDevicesCached.size == 1) {
val thisDeviceId = syncStore.deviceId.orEmpty()
val result = logout(thisDeviceId)
if (result is Error) {
return result
}
wasUserLogout = true
} else {
return error
}
}

val primaryKey = recoveryCode.primaryKey
Expand All @@ -120,6 +134,9 @@ class AppSyncAccountRepository @Inject constructor(

return performLogin(userId, deviceId, deviceName, primaryKey).onFailure {
it.alsoFireLoginErrorPixel()
if (wasUserLogout) {
syncPixels.fireUserSwitchedLoginError()
}
return it.copy(code = LOGIN_FAILED.code)
}
}
Expand Down Expand Up @@ -288,6 +305,7 @@ class AppSyncAccountRepository @Inject constructor(

return when (val result = syncApi.getDevices(token)) {
is Error -> {
connectedDevicesCached.clear()
result.alsoFireAccountErrorPixel().copy(code = GENERIC_ERROR.code)
}

Expand Down Expand Up @@ -315,6 +333,11 @@ class AppSyncAccountRepository @Inject constructor(
}
}.sortedWith { a, b ->
if (a.thisDevice) -1 else 1
}.also {
connectedDevicesCached.apply {
clear()
addAll(it)
}
},
)
}
Expand All @@ -326,8 +349,17 @@ class AppSyncAccountRepository @Inject constructor(
override fun logoutAndJoinNewAccount(stringCode: String): Result<Boolean> {
val thisDeviceId = syncStore.deviceId.orEmpty()
return when (val result = logout(thisDeviceId)) {
is Error -> result
is Result.Success -> processCode(stringCode)
is Error -> {
syncPixels.fireUserSwitchedLogoutError()
result
}
is Result.Success -> {
val loginResult = processCode(stringCode)
if (loginResult is Error) {
syncPixels.fireUserSwitchedLoginError()
}
loginResult
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,6 @@ interface SyncFeature {
@Toggle.DefaultValue(true)
fun gzipPatchRequests(): Toggle

@Toggle.DefaultValue(false)
@Toggle.DefaultValue(true)
fun seamlessAccountSwitching(): Toggle
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,13 @@ class SyncPixelParamRemovalPlugin @Inject constructor() : PixelParamRemovalPlugi
SyncPixelName.SYNC_GET_OTHER_DEVICES_SCREEN_SHOWN.pixelName to PixelParameter.removeAtb(),
SyncPixelName.SYNC_GET_OTHER_DEVICES_LINK_COPIED.pixelName to PixelParameter.removeAtb(),
SyncPixelName.SYNC_GET_OTHER_DEVICES_LINK_SHARED.pixelName to PixelParameter.removeAtb(),

SyncPixelName.SYNC_ASK_USER_TO_SWITCH_ACCOUNT.pixelName to PixelParameter.removeAtb(),
SyncPixelName.SYNC_USER_ACCEPTED_SWITCHING_ACCOUNT.pixelName to PixelParameter.removeAtb(),
SyncPixelName.SYNC_USER_CANCELLED_SWITCHING_ACCOUNT.pixelName to PixelParameter.removeAtb(),
SyncPixelName.SYNC_USER_SWITCHED_ACCOUNT.pixelName to PixelParameter.removeAtb(),
SyncPixelName.SYNC_USER_SWITCHED_LOGOUT_ERROR.pixelName to PixelParameter.removeAtb(),
SyncPixelName.SYNC_USER_SWITCHED_LOGIN_ERROR.pixelName to PixelParameter.removeAtb(),
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,13 @@ interface SyncPixels {
feature: SyncableType,
apiError: Error,
)

fun fireAskUserToSwitchAccount()
fun fireUserAcceptedSwitchingAccount()
fun fireUserCancelledSwitchingAccount()
fun fireUserSwitchedAccount()
fun fireUserSwitchedLogoutError()
fun fireUserSwitchedLoginError()
}

@ContributesBinding(AppScope::class)
Expand Down Expand Up @@ -255,6 +262,30 @@ class RealSyncPixels @Inject constructor(
}
}

override fun fireUserSwitchedAccount() {
pixel.fire(SyncPixelName.SYNC_USER_SWITCHED_ACCOUNT)
}

override fun fireAskUserToSwitchAccount() {
pixel.fire(SyncPixelName.SYNC_ASK_USER_TO_SWITCH_ACCOUNT)
}

override fun fireUserAcceptedSwitchingAccount() {
pixel.fire(SyncPixelName.SYNC_USER_ACCEPTED_SWITCHING_ACCOUNT)
}

override fun fireUserCancelledSwitchingAccount() {
pixel.fire(SyncPixelName.SYNC_USER_CANCELLED_SWITCHING_ACCOUNT)
}

override fun fireUserSwitchedLoginError() {
pixel.fire(SyncPixelName.SYNC_USER_SWITCHED_LOGIN_ERROR)
}

override fun fireUserSwitchedLogoutError() {
pixel.fire(SyncPixelName.SYNC_USER_SWITCHED_LOGOUT_ERROR)
}

companion object {
private const val SYNC_PIXELS_PREF_FILE = "com.duckduckgo.sync.pixels.v1"
}
Expand Down Expand Up @@ -302,6 +333,12 @@ enum class SyncPixelName(override val pixelName: String) : Pixel.PixelName {
SYNC_GET_OTHER_DEVICES_SCREEN_SHOWN("sync_get_other_devices"),
SYNC_GET_OTHER_DEVICES_LINK_COPIED("sync_get_other_devices_copy"),
SYNC_GET_OTHER_DEVICES_LINK_SHARED("sync_get_other_devices_share"),
SYNC_ASK_USER_TO_SWITCH_ACCOUNT("sync_ask_user_to_switch_account"),
SYNC_USER_ACCEPTED_SWITCHING_ACCOUNT("sync_user_accepted_switching_account"),
SYNC_USER_CANCELLED_SWITCHING_ACCOUNT("sync_user_cancelled_switching_account"),
SYNC_USER_SWITCHED_ACCOUNT("sync_user_switched_account"),
SYNC_USER_SWITCHED_LOGOUT_ERROR("sync_user_switched_logout_error"),
SYNC_USER_SWITCHED_LOGIN_ERROR("sync_user_switched_login_error"),
}

object SyncPixelParameters {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ class EnterCodeActivity : DuckDuckGoActivity() {
}

private fun askUserToSwitchAccount(it: AskToSwitchAccount) {
viewModel.onUserAskedToSwitchAccount()
TextAlertDialogBuilder(this)
.setTitle(R.string.sync_dialog_switch_account_header)
.setMessage(R.string.sync_dialog_switch_account_description)
Expand All @@ -141,6 +142,10 @@ class EnterCodeActivity : DuckDuckGoActivity() {
override fun onPositiveButtonClicked() {
viewModel.onUserAcceptedJoiningNewAccount(it.encodedStringCode)
}

override fun onNegativeButtonClicked() {
viewModel.onUserCancelledJoiningNewAccount()
}
},
).show()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import com.duckduckgo.sync.impl.SyncAccountRepository
import com.duckduckgo.sync.impl.SyncFeature
import com.duckduckgo.sync.impl.pixels.SyncPixels
import com.duckduckgo.sync.impl.ui.EnterCodeViewModel.Command.AskToSwitchAccount
import com.duckduckgo.sync.impl.ui.EnterCodeViewModel.Command.LoginSuccess
import com.duckduckgo.sync.impl.ui.EnterCodeViewModel.Command.ShowError
import com.duckduckgo.sync.impl.ui.EnterCodeViewModel.Command.SwitchAccountSuccess
import javax.inject.*
Expand Down Expand Up @@ -90,9 +91,17 @@ class EnterCodeViewModel @Inject constructor(
private suspend fun authFlow(
pastedCode: String,
) {
val result = syncAccountRepository.processCode(pastedCode)
when (result) {
is Result.Success -> command.send(Command.LoginSuccess)
val userSignedIn = syncAccountRepository.isSignedIn()
when (val result = syncAccountRepository.processCode(pastedCode)) {
is Result.Success -> {
val commandSuccess = if (userSignedIn) {
syncPixels.fireUserSwitchedAccount()
SwitchAccountSuccess
} else {
LoginSuccess
}
command.send(commandSuccess)
}
is Result.Error -> {
processError(result, pastedCode)
}
Expand Down Expand Up @@ -128,6 +137,7 @@ class EnterCodeViewModel @Inject constructor(

fun onUserAcceptedJoiningNewAccount(encodedStringCode: String) {
viewModelScope.launch(dispatchers.io()) {
syncPixels.fireUserAcceptedSwitchingAccount()
val result = syncAccountRepository.logoutAndJoinNewAccount(encodedStringCode)
if (result is Error) {
when (result.code) {
Expand All @@ -143,9 +153,17 @@ class EnterCodeViewModel @Inject constructor(
)
}
} else {
syncPixels.fireLoginPixel()
syncPixels.fireUserSwitchedAccount()
command.send(SwitchAccountSuccess)
}
}
}

fun onUserCancelledJoiningNewAccount() {
syncPixels.fireUserCancelledSwitchingAccount()
}

fun onUserAskedToSwitchAccount() {
syncPixels.fireAskUserToSwitchAccount()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -136,14 +136,21 @@ class SyncWithAnotherActivityViewModel @Inject constructor(

fun onQRCodeScanned(qrCode: String) {
viewModelScope.launch(dispatchers.io()) {
val userSignedIn = syncAccountRepository.isSignedIn()
when (val result = syncAccountRepository.processCode(qrCode)) {
is Error -> {
emitError(result, qrCode)
}

is Success -> {
syncPixels.fireLoginPixel()
command.send(LoginSuccess)
val commandSuccess = if (userSignedIn) {
syncPixels.fireUserSwitchedAccount()
SwitchAccountSuccess
} else {
LoginSuccess
}
command.send(commandSuccess)
}
}
}
Expand Down Expand Up @@ -175,6 +182,7 @@ class SyncWithAnotherActivityViewModel @Inject constructor(

fun onUserAcceptedJoiningNewAccount(encodedStringCode: String) {
viewModelScope.launch(dispatchers.io()) {
syncPixels.fireUserAcceptedSwitchingAccount()
val result = syncAccountRepository.logoutAndJoinNewAccount(encodedStringCode)
if (result is Error) {
when (result.code) {
Expand All @@ -191,6 +199,7 @@ class SyncWithAnotherActivityViewModel @Inject constructor(
}
} else {
syncPixels.fireLoginPixel()
syncPixels.fireUserSwitchedAccount()
command.send(SwitchAccountSuccess)
}
}
Expand All @@ -211,4 +220,12 @@ class SyncWithAnotherActivityViewModel @Inject constructor(
}
}
}

fun onUserCancelledJoiningNewAccount() {
syncPixels.fireUserCancelledSwitchingAccount()
}

fun onUserAskedToSwitchAccount() {
syncPixels.fireAskUserToSwitchAccount()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ class SyncWithAnotherDeviceActivity : DuckDuckGoActivity() {
}

private fun askUserToSwitchAccount(it: AskToSwitchAccount) {
viewModel.onUserAskedToSwitchAccount()
TextAlertDialogBuilder(this)
.setTitle(R.string.sync_dialog_switch_account_header)
.setMessage(R.string.sync_dialog_switch_account_description)
Expand All @@ -143,6 +144,10 @@ class SyncWithAnotherDeviceActivity : DuckDuckGoActivity() {
override fun onPositiveButtonClicked() {
viewModel.onUserAcceptedJoiningNewAccount(it.encodedStringCode)
}

override fun onNegativeButtonClicked() {
viewModel.onUserCancelledJoiningNewAccount()
}
},
).show()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,8 @@ object TestSyncFixtures {
)
val loginSuccessResponse: Response<LoginResponse> = Response.success(loginResponseBody)

val listOfDevices = listOf(Device(deviceId = deviceId, deviceName = deviceName, jwIat = "", deviceType = deviceFactor))
val aDevice = Device(deviceId = deviceId, deviceName = deviceName, jwIat = "", deviceType = deviceFactor)
val listOfDevices = listOf(aDevice)
val deviceResponse = DeviceResponse(DeviceEntries(listOfDevices))
val getDevicesBodySuccessResponse: Response<DeviceResponse> = Response.success(deviceResponse)
val getDevicesBodyErrorResponse: Response<DeviceResponse> = Response.error(
Expand Down
Loading

0 comments on commit 581a091

Please sign in to comment.