Skip to content

Commit

Permalink
PM-13886 show dialog when no logins were imported (#4139)
Browse files Browse the repository at this point in the history
  • Loading branch information
dseverns-livefront authored Oct 24, 2024
1 parent a55fbca commit b0885ff
Show file tree
Hide file tree
Showing 8 changed files with 210 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1336,7 +1336,12 @@ class VaultRepositoryImpl(
userId = userId,
lastSyncTime = clock.instant(),
)
return SyncVaultDataResult.Success
val itemsAvailable = vaultDiskSource
.getCiphers(userId)
.firstOrNull()
?.isNotEmpty()
?: false
return SyncVaultDataResult.Success(itemsAvailable = itemsAvailable)
}
},
onFailure = {
Expand Down Expand Up @@ -1381,7 +1386,8 @@ class VaultRepositoryImpl(
lastSyncTime = clock.instant(),
)
vaultDiskSource.replaceVaultData(userId = userId, vault = syncResponse)
return SyncVaultDataResult.Success
val itemsAvailable = syncResponse.ciphers?.isNotEmpty() ?: false
return SyncVaultDataResult.Success(itemsAvailable = itemsAvailable)
},
onFailure = { throwable ->
updateVaultStateFlowsToError(throwable)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ package com.x8bit.bitwarden.data.vault.repository.model
sealed class SyncVaultDataResult {
/**
* Indicates a successful sync operation.
*
* @property itemsAvailable indicated whether the sync returned any vault items or not.
*/
data object Success : SyncVaultDataResult()
data class Success(val itemsAvailable: Boolean) : SyncVaultDataResult()

/**
* Indicates a failed sync operation.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ private fun ImportLoginsDialogContent(
)
}

ImportLoginsState.DialogState.Error -> {
is ImportLoginsState.DialogState.Error -> {
BitwardenTwoButtonDialog(
title = dialogState.title?.invoke(),
message = dialogState.message(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,22 +82,33 @@ class ImportLoginsViewModel @Inject constructor(
private fun handleVaultSyncResultReceived(
action: ImportLoginsAction.Internal.VaultSyncResultReceived,
) {
when (action.result) {
when (val result = action.result) {
is SyncVaultDataResult.Error -> {
mutableStateFlow.update {
it.copy(
isVaultSyncing = false,
dialogState = ImportLoginsState.DialogState.Error,
dialogState = ImportLoginsState.DialogState.Error(),
)
}
}

SyncVaultDataResult.Success -> {
mutableStateFlow.update {
it.copy(
showBottomSheet = true,
isVaultSyncing = false,
)
is SyncVaultDataResult.Success -> {
if (result.itemsAvailable) {
mutableStateFlow.update {
it.copy(
showBottomSheet = true,
isVaultSyncing = false,
)
}
} else {
mutableStateFlow.update {
it.copy(
isVaultSyncing = false,
dialogState = ImportLoginsState.DialogState.Error(
R.string.no_logins_were_imported.asText(),
),
)
}
}
}
}
Expand Down Expand Up @@ -215,9 +226,10 @@ data class ImportLoginsState(
/**
* Show a dialog with an error message.
*/
data object Error : DialogState() {
data class Error(
override val message: Text = R.string.generic_error_message.asText(),
) : DialogState() {
override val title: Text? = null
override val message: Text = R.string.generic_error_message.asText()
}
}

Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1072,4 +1072,5 @@ Do you want to switch to this account?</string>
<string name="manage_your_logins_from_anywhere_with_bitwarden_tools">Manage your logins from anywhere with Bitwarden tools for web and desktop.</string>
<string name="bitwarden_tools">Bitwarden Tools</string>
<string name="got_it">Got it</string>
<string name="no_logins_were_imported">No logins were imported</string>
</resources>
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,11 @@ class VaultRepositoryTest {
private val sendsService: SendsService = mockk()
private val ciphersService: CiphersService = mockk()
private val folderService: FolderService = mockk()
private val mutableGetCiphersFlow: MutableStateFlow<List<SyncResponseJson.Cipher>> =
MutableStateFlow(listOf(createMockCipher(1)))
private val vaultDiskSource: VaultDiskSource = mockk {
coEvery { resyncVaultData(any()) } just runs
every { getCiphers(any()) } returns mutableGetCiphersFlow
}
private val totpCodeManager: TotpCodeManager = mockk()
private val vaultSdkSource: VaultSdkSource = mockk {
Expand Down Expand Up @@ -4393,39 +4396,77 @@ class VaultRepositoryTest {
}
}

@Suppress("MaxLineLength")
@Test
fun `syncForResult should return result`() = runTest {
fakeAuthDiskSource.userState = MOCK_USER_STATE
val userId = "mockId-1"
val mockSyncResponse = createMockSyncResponse(number = 1)
coEvery {
syncService.sync()
} returns mockSyncResponse.asSuccess()
coEvery {
vaultSdkSource.initializeOrganizationCrypto(
userId = userId,
request = InitOrgCryptoRequest(
organizationKeys = createMockOrganizationKeys(1),
),
)
} returns InitializeCryptoResult.Success.asSuccess()
coEvery {
vaultDiskSource.replaceVaultData(
userId = MOCK_USER_STATE.activeUserId,
vault = mockSyncResponse,
)
} just runs
fun `syncForResult should return success result with itemsAvailable = true when sync succeeds and ciphers list is not empty`() =
runTest {
fakeAuthDiskSource.userState = MOCK_USER_STATE
val userId = "mockId-1"
val mockSyncResponse = createMockSyncResponse(number = 1)
coEvery {
syncService.sync()
} returns mockSyncResponse.asSuccess()
coEvery {
vaultSdkSource.initializeOrganizationCrypto(
userId = userId,
request = InitOrgCryptoRequest(
organizationKeys = createMockOrganizationKeys(1),
),
)
} returns InitializeCryptoResult.Success.asSuccess()
coEvery {
vaultDiskSource.replaceVaultData(
userId = MOCK_USER_STATE.activeUserId,
vault = mockSyncResponse,
)
} just runs

every {
settingsDiskSource.storeLastSyncTime(
MOCK_USER_STATE.activeUserId,
clock.instant(),
)
} just runs
every {
settingsDiskSource.storeLastSyncTime(
MOCK_USER_STATE.activeUserId,
clock.instant(),
)
} just runs

val syncResult = vaultRepository.syncForResult()
assertEquals(SyncVaultDataResult.Success, syncResult)
}
val syncResult = vaultRepository.syncForResult()
assertEquals(SyncVaultDataResult.Success(itemsAvailable = true), syncResult)
}

@Suppress("MaxLineLength")
@Test
fun `syncForResult should return success result with itemsAvailable = false when sync succeeds and ciphers list is empty`() =
runTest {
fakeAuthDiskSource.userState = MOCK_USER_STATE
val userId = "mockId-1"
val mockSyncResponse = createMockSyncResponse(number = 1).copy(ciphers = emptyList())
coEvery {
syncService.sync()
} returns mockSyncResponse.asSuccess()
coEvery {
vaultSdkSource.initializeOrganizationCrypto(
userId = userId,
request = InitOrgCryptoRequest(
organizationKeys = createMockOrganizationKeys(1),
),
)
} returns InitializeCryptoResult.Success.asSuccess()
coEvery {
vaultDiskSource.replaceVaultData(
userId = MOCK_USER_STATE.activeUserId,
vault = mockSyncResponse,
)
} just runs

every {
settingsDiskSource.storeLastSyncTime(
MOCK_USER_STATE.activeUserId,
clock.instant(),
)
} just runs

val syncResult = vaultRepository.syncForResult()
assertEquals(SyncVaultDataResult.Success(itemsAvailable = false), syncResult)
}

@Test
fun `syncForResult should return error when getAccountRevisionDateMillis fails`() =
Expand Down Expand Up @@ -4456,6 +4497,30 @@ class VaultRepositoryTest {
)
}

@Suppress("MaxLineLength")
@Test
fun `syncForResult when the last sync time is more recent than the revision date should return result from disk source data`() =
runTest {
val userId = "mockId-1"
fakeAuthDiskSource.userState = MOCK_USER_STATE
every {
settingsDiskSource.getLastSyncTime(userId = userId)
} returns clock.instant().plus(2, ChronoUnit.MINUTES)
mutableGetCiphersFlow.update { emptyList() }
val result = vaultRepository.syncForResult()
assertEquals(
SyncVaultDataResult.Success(itemsAvailable = false),
result,
)
verify(exactly = 1) {
settingsDiskSource.storeLastSyncTime(
userId = userId,
lastSyncTime = clock.instant(),
)
}
coVerify(exactly = 0) { syncService.sync() }
}

//region Helper functions

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ import androidx.compose.ui.test.performScrollTo
import androidx.compose.ui.test.performSemanticsAction
import androidx.compose.ui.test.printToLog
import androidx.core.net.toUri
import com.x8bit.bitwarden.R
import com.x8bit.bitwarden.data.platform.repository.util.bufferedMutableSharedFlow
import com.x8bit.bitwarden.data.util.advanceTimeByAndRunCurrent
import com.x8bit.bitwarden.ui.platform.base.BaseComposeTest
import com.x8bit.bitwarden.ui.platform.base.util.asText
import com.x8bit.bitwarden.ui.platform.manager.intent.IntentManager
import com.x8bit.bitwarden.ui.util.assertNoDialogExists
import io.mockk.every
Expand Down Expand Up @@ -342,7 +344,7 @@ class ImportLoginsScreenTest : BaseComposeTest() {
fun `Error dialog is displayed when dialog state is Error`() {
mutableImportLoginsStateFlow.update {
it.copy(
dialogState = ImportLoginsState.DialogState.Error,
dialogState = ImportLoginsState.DialogState.Error(),
)
}
composeTestRule
Expand Down Expand Up @@ -371,6 +373,41 @@ class ImportLoginsScreenTest : BaseComposeTest() {
verifyActionSent(ImportLoginsAction.FailedSyncAcknowledged)
}

@Test
fun `Error dialog is displayed when dialog state is Error for no logins`() {
mutableImportLoginsStateFlow.tryEmit(
DEFAULT_STATE.copy(
dialogState = ImportLoginsState.DialogState.Error(
message = R.string.no_logins_were_imported.asText(),
),
),
)
composeTestRule
.onNode(isDialog())
.assertIsDisplayed()
composeTestRule
.onAllNodesWithText(
text = "No logins were imported",
useUnmergedTree = true,
)
.filterToOne(hasAnyAncestor(isDialog()))
.assertIsDisplayed()

composeTestRule
.onAllNodesWithText("Try again")
.filterToOne(hasAnyAncestor(isDialog()))
.assertIsDisplayed()
.performClick()
verifyActionSent(ImportLoginsAction.RetryVaultSync)

composeTestRule
.onAllNodesWithText("Ok")
.filterToOne(hasAnyAncestor(isDialog()))
.assertIsDisplayed()
.performClick()
verifyActionSent(ImportLoginsAction.FailedSyncAcknowledged)
}

@Test
fun `Success bottom sheet is shown when state is updated`() {
mutableImportLoginsStateFlow.update {
Expand Down
Loading

0 comments on commit b0885ff

Please sign in to comment.