PM-31656, PM-31658, PM-31659: Address Archive feature bugs (#6473)

This commit is contained in:
David Perez
2026-02-04 17:33:29 +00:00
committed by GitHub
parent 7ec4faf424
commit 15e217bc49
11 changed files with 258 additions and 69 deletions
@@ -90,8 +90,8 @@ class AuthenticatorBridgeRepositoryImpl(
// Vault is unlocked, query vault disk source for totp logins:
val totpUris = vaultDiskSource
.getTotpCiphers(userId = userId)
// Filter out any deleted ciphers.
.filter { it.deletedDate == null }
// Filter out any deleted and archived ciphers.
.filter { it.deletedDate == null && it.archivedDate == null }
.mapNotNull {
scopedVaultSdkSource
.decryptCipher(userId = userId, cipher = it.toEncryptedSdkCipher())
@@ -191,6 +191,10 @@ class CipherManagerImpl(
userId = userId,
cipher = it.toEncryptedNetworkCipherResponse(),
)
settingsDiskSource.storeIntroducingArchiveActionCardDismissed(
userId = userId,
isDismissed = true,
)
}
.fold(
onSuccess = { ArchiveCipherResult.Success },
@@ -501,7 +501,7 @@ private fun ActionCard(
tint = BitwardenTheme.colorScheme.icon.secondary,
)
},
onActionClick = vaultHandlers.archiveClick,
onActionClick = { vaultHandlers.actionCardClick(actionCardState) },
onDismissClick = { vaultHandlers.dismissActionCardClick(actionCardState) },
modifier = modifier,
)
@@ -320,6 +320,7 @@ class VaultViewModel @Inject constructor(
VaultAction.UpgradeToPremiumClick -> handleUpgradeToPremiumClick()
is VaultAction.DismissActionCardClick -> handleDismissActionCardClick(action)
is VaultAction.ActionCardClick -> handleActionCardClick(action)
}
}
@@ -377,6 +378,15 @@ class VaultViewModel @Inject constructor(
}
}
private fun handleActionCardClick(action: VaultAction.ActionCardClick) {
when (action.actionCard) {
VaultState.ActionCardState.IntroducingArchive -> {
settingsRepository.dismissIntroducingArchiveActionCard()
sendEvent(VaultEvent.NavigateToItemListing(VaultItemListingType.Archive))
}
}
}
private fun handleSelectAddItemType() {
// If policy is enable for any organization, exclude the card option
val excludedOptions = persistentListOfNotNull(
@@ -572,7 +582,18 @@ class VaultViewModel @Inject constructor(
}
private fun handleArchiveClick() {
sendEvent(VaultEvent.NavigateToItemListing(VaultItemListingType.Archive))
val archivedItemsCount = (state.viewState as? VaultState.ViewState.Content)
?.archivedItemsCount
?: 0
if (state.isPremium || archivedItemsCount > 0) {
// We still navigate even if the user does not have premium, since they have previously
// archived ciphers to view.
sendEvent(VaultEvent.NavigateToItemListing(VaultItemListingType.Archive))
} else {
mutableStateFlow.update {
it.copy(dialog = VaultState.DialogState.ArchiveRequiresPremium)
}
}
}
private fun handleTrashClick() {
@@ -2189,6 +2210,13 @@ sealed class VaultAction {
val actionCard: VaultState.ActionCardState,
) : VaultAction()
/**
* User clicked the primary button on an action card.
*/
data class ActionCardClick(
val actionCard: VaultState.ActionCardState,
) : VaultAction()
/**
* Models actions that the [VaultViewModel] itself might send.
*/
@@ -52,6 +52,7 @@ data class VaultHandlers(
val onDismissThirdPartyAutofillDialogClick: () -> Unit,
val upgradeToPremiumClick: () -> Unit,
val dismissActionCardClick: (VaultState.ActionCardState) -> Unit,
val actionCardClick: (VaultState.ActionCardState) -> Unit,
) {
@Suppress("UndocumentedPublicClass")
companion object {
@@ -151,6 +152,7 @@ data class VaultHandlers(
dismissActionCardClick = {
viewModel.trySendAction(VaultAction.DismissActionCardClick(it))
},
actionCardClick = { viewModel.trySendAction(VaultAction.ActionCardClick(it)) },
)
}
}
@@ -132,6 +132,9 @@ fun VaultData.toViewState(
noFolderItems.size < NO_FOLDER_ITEM_THRESHOLD
val totpItems = activeCipherViews.filter { it.login?.totp != null }
val cardCount = activeCipherViews.count { it.type is CipherListViewType.Card }
val archiveCount = allCipherViews.count {
it.archivedDate != null && it.deletedDate == null
}
VaultState.ViewState.Content(
itemTypesCount = itemTypesCount,
totpItemsCount = if (isPremium) {
@@ -215,15 +218,13 @@ fun VaultData.toViewState(
)
},
trashItemsCount = allCipherViews.count { it.deletedDate != null },
archivedItemsCount = allCipherViews
.count { it.archivedDate != null && it.deletedDate == null }
.takeIf { isPremium },
archivedItemsCount = archiveCount.takeIf { isPremium || archiveCount > 0 },
archiveEnabled = isArchiveEnabled,
archiveEndIcon = BitwardenDrawable.ic_locked.takeUnless { isPremium },
archiveEndIcon = BitwardenDrawable.ic_locked.takeIf { !isPremium && archiveCount == 0 },
archiveSubText = BitwardenString
.premium_subscription_required
.asText()
.takeUnless { isPremium },
.takeIf { !isPremium && archiveCount == 0 },
showCardGroup = cardCount != 0 || restrictItemTypesPolicyOrgIds.isEmpty(),
)
}
@@ -491,6 +491,13 @@ class FakeSettingsDiskSource(
return storedAppResumeScreenData[userId]?.let { Json.decodeFromStringOrNull(it) }
}
/**
* Asserts that the stored introducing archive action card dismissed matches the [expected] one.
*/
fun assertIntroducingArchiveActionCardDismissed(userId: String, expected: Boolean?) {
assertEquals(expected, storedIntroducingArchiveActionCardDismissed[userId])
}
/**
* Asserts that the stored last sync time matches the [expected] one.
*/
@@ -238,7 +238,7 @@ class AuthenticatorBridgeRepositoryTest {
@Test
@Suppress("MaxLineLength")
fun `getSharedAccounts should unlock and re-lock vault for both users and filter out deleted ciphers`() =
fun `getSharedAccounts should unlock and re-lock vault for both users and filter out archived and deleted ciphers`() =
runTest {
assertEquals(
BOTH_ACCOUNT_SUCCESS,
@@ -499,6 +499,7 @@ private val USER_STATE_JSON = UserStateJson(
private val USER_1_TOTP_CIPHER = mockk<SyncResponseJson.Cipher> {
every { login?.totp } returns "encryptedTotp1"
every { login?.username } returns "username"
every { archivedDate } returns null
every { deletedDate } returns null
every { name } returns "cipher1"
}
@@ -506,13 +507,23 @@ private val USER_1_TOTP_CIPHER = mockk<SyncResponseJson.Cipher> {
private val USER_1_DELETED_TOTP_CIPHER = mockk<SyncResponseJson.Cipher> {
every { login?.totp } returns "encryptedTotp1Deleted"
every { login?.username } returns "username"
every { archivedDate } returns null
every { deletedDate } returns ZonedDateTime.parse("2023-10-27T12:00:00Z")
every { name } returns "cipher1"
}
private val USER_1_ARCHIVED_TOTP_CIPHER = mockk<SyncResponseJson.Cipher> {
every { login?.totp } returns "encryptedTotp1Deleted"
every { login?.username } returns "username"
every { archivedDate } returns ZonedDateTime.parse("2023-10-27T12:00:00Z")
every { deletedDate } returns null
every { name } returns "cipher1"
}
private val USER_2_TOTP_CIPHER = mockk<SyncResponseJson.Cipher> {
every { login?.totp } returns "encryptedTotp2"
every { login?.username } returns "username"
every { archivedDate } returns null
every { deletedDate } returns null
every { name } returns "cipher2"
}
@@ -553,6 +564,7 @@ private val USER_2_SHARED_ACCOUNT = SharedAccountData.Account(
private val USER_1_CIPHERS = listOf(
USER_1_TOTP_CIPHER,
USER_1_DELETED_TOTP_CIPHER,
USER_1_ARCHIVED_TOTP_CIPHER,
)
private val USER_2_CIPHERS = listOf(
@@ -744,6 +744,10 @@ class CipherManagerTest {
cipher = createMockSdkCipher(number = 1, clock = clock),
)
val cipherView = createMockCipherView(number = 1)
fakeSettingsDiskSource.storeIntroducingArchiveActionCardDismissed(
userId = userId,
isDismissed = null,
)
coEvery {
vaultSdkSource.encryptCipher(userId = userId, cipherView = cipherView)
} returns encryptionContext.asSuccess()
@@ -770,6 +774,10 @@ class CipherManagerTest {
cipherView = cipherView,
)
fakeSettingsDiskSource.assertIntroducingArchiveActionCardDismissed(
userId = userId,
expected = true,
)
assertEquals(ArchiveCipherResult.Success, result)
}
@@ -1527,7 +1527,11 @@ class VaultScreenTest : BitwardenComposeTest() {
.performClick()
verify(exactly = 1) {
viewModel.trySendAction(VaultAction.ArchiveClick)
viewModel.trySendAction(
VaultAction.ActionCardClick(
actionCard = VaultState.ActionCardState.IntroducingArchive,
),
)
}
}
@@ -14,6 +14,7 @@ import com.bitwarden.ui.platform.base.BaseViewModelTest
import com.bitwarden.ui.platform.components.account.model.AccountSummary
import com.bitwarden.ui.platform.components.snackbar.model.BitwardenSnackbarData
import com.bitwarden.ui.platform.manager.snackbar.SnackbarRelayManager
import com.bitwarden.ui.platform.resource.BitwardenDrawable
import com.bitwarden.ui.platform.resource.BitwardenString
import com.bitwarden.ui.util.Text
import com.bitwarden.ui.util.asText
@@ -177,18 +178,17 @@ class VaultViewModelTest : BaseViewModelTest() {
} returns mutableIntroducingArchiveActionCardDismissedFlow
}
private val vaultRepository: VaultRepository =
mockk {
every { vaultFilterType = any() } just runs
every { vaultDataStateFlow } returns mutableVaultDataStateFlow
every { sync(forced = any()) } just runs
every { syncIfNecessary() } just runs
every { lockVaultForCurrentUser(any()) } just runs
every { lockVault(any(), any()) } just runs
coEvery {
getCipher(any())
} returns GetCipherResult.Success(createMockCipherView(number = 1))
}
private val vaultRepository: VaultRepository = mockk {
every { vaultFilterType = any() } just runs
every { vaultDataStateFlow } returns mutableVaultDataStateFlow
every { sync(forced = any()) } just runs
every { syncIfNecessary() } just runs
every { lockVaultForCurrentUser(any()) } just runs
every { lockVault(any(), any()) } just runs
coEvery {
getCipher(any())
} returns GetCipherResult.Success(createMockCipherView(number = 1))
}
private val organizationEventManager = mockk<OrganizationEventManager> {
every { trackEvent(event = any()) } just runs
@@ -255,6 +255,27 @@ class VaultViewModelTest : BaseViewModelTest() {
}
}
@Suppress("MaxLineLength")
@Test
fun `ActionCardClick with IntroducingArchive should call dismissIntroducingArchiveActionCard and emit NavigateTo`() =
runTest {
val viewModel = createViewModel()
viewModel.eventFlow.test {
viewModel.trySendAction(
VaultAction.ActionCardClick(VaultState.ActionCardState.IntroducingArchive),
)
assertEquals(
VaultEvent.NavigateToItemListing(VaultItemListingType.Archive),
awaitItem(),
)
}
verify(exactly = 1) {
settingsRepository.dismissIntroducingArchiveActionCard()
}
}
@Suppress("MaxLineLength")
@Test
fun `DismissActionCardClick with IntroducingArchive should call dismissIntroducingArchiveActionCard`() =
@@ -1760,16 +1781,114 @@ class VaultViewModelTest : BaseViewModelTest() {
}
@Test
fun `ArchiveClick should emit NavigateToItemListing event with Archive type`() = runTest {
val viewModel = createViewModel()
viewModel.eventFlow.test {
fun `ArchiveClick with premium should emit NavigateToItemListing event with Archive type`() =
runTest {
val viewModel = createViewModel()
viewModel.eventFlow.test {
viewModel.trySendAction(VaultAction.ArchiveClick)
assertEquals(
VaultEvent.NavigateToItemListing(VaultItemListingType.Archive),
awaitItem(),
)
}
}
@Suppress("MaxLineLength")
@Test
fun `ArchiveClick without premium and with archived ciphers should emit NavigateToItemListing event with Archive type`() =
runTest {
mutableUserStateFlow.update {
DEFAULT_USER_STATE.copy(
accounts = listOf(
DEFAULT_ACTIVE_ACCOUNT.copy(isPremium = false),
DEFAULT_INACTIVE_ACCOUNT,
),
)
}
mutableVaultDataStateFlow.update {
DataState.Loaded(
VaultData(
decryptCipherListResult = createMockDecryptCipherListResult(
number = 1,
successes = listOf(
createMockCipherListView(number = 1, isArchived = true),
),
failures = emptyList(),
),
collectionViewList = emptyList(),
folderViewList = emptyList(),
sendViewList = emptyList(),
),
)
}
val viewModel = createViewModel()
viewModel.eventFlow.test {
viewModel.trySendAction(VaultAction.ArchiveClick)
assertEquals(
VaultEvent.NavigateToItemListing(VaultItemListingType.Archive),
awaitItem(),
)
}
}
@Suppress("MaxLineLength")
@Test
fun `ArchiveClick without premium and archived ciphers should display the ArchiveRequiresPremium dialog`() =
runTest {
mutableUserStateFlow.update {
DEFAULT_USER_STATE.copy(
accounts = listOf(
DEFAULT_ACTIVE_ACCOUNT.copy(isPremium = false),
DEFAULT_INACTIVE_ACCOUNT,
),
)
}
mutableVaultDataStateFlow.update {
DataState.Loaded(
VaultData(
decryptCipherListResult = createMockDecryptCipherListResult(
number = 1,
successes = listOf(
createMockCipherListView(number = 1, isArchived = false),
),
failures = emptyList(),
),
collectionViewList = emptyList(),
folderViewList = emptyList(),
sendViewList = emptyList(),
),
)
}
val viewModel = createViewModel()
viewModel.trySendAction(VaultAction.ArchiveClick)
assertEquals(
VaultEvent.NavigateToItemListing(VaultItemListingType.Archive),
awaitItem(),
DEFAULT_STATE.copy(
isPremium = false,
dialog = VaultState.DialogState.ArchiveRequiresPremium,
viewState = VaultState.ViewState.Content(
loginItemsCount = 1,
cardItemsCount = 0,
identityItemsCount = 0,
secureNoteItemsCount = 0,
favoriteItems = listOf(),
folderItems = listOf(),
collectionItems = listOf(),
noFolderItems = listOf(),
trashItemsCount = 0,
totpItemsCount = 0,
itemTypesCount = 5,
sshKeyItemsCount = 0,
archivedItemsCount = null,
archiveEnabled = true,
archiveSubText = BitwardenString.premium_subscription_required.asText(),
archiveEndIcon = BitwardenDrawable.ic_locked,
showCardGroup = true,
),
),
viewModel.stateFlow.value,
)
}
}
@Test
fun `TrashClick should emit NavigateToItemListing event with Trash type`() = runTest {
@@ -3399,49 +3518,53 @@ private val DEFAULT_FIRST_TIME_STATE = FirstTimeState(
showImportLoginsCard = true,
)
private val DEFAULT_ACTIVE_ACCOUNT = UserState.Account(
userId = "activeUserId",
name = "Active User",
email = "active@bitwarden.com",
avatarColorHex = "#aa00aa",
environment = Environment.Us,
isPremium = true,
isLoggedIn = true,
isVaultUnlocked = true,
needsPasswordReset = false,
isBiometricsEnabled = false,
organizations = emptyList(),
needsMasterPassword = false,
trustedDevice = null,
hasMasterPassword = true,
isUsingKeyConnector = false,
onboardingStatus = OnboardingStatus.COMPLETE,
firstTimeState = DEFAULT_FIRST_TIME_STATE,
isExportable = true,
)
private val DEFAULT_INACTIVE_ACCOUNT = UserState.Account(
userId = "lockedUserId",
name = "Locked User",
email = "locked@bitwarden.com",
avatarColorHex = "#00aaaa",
environment = Environment.Us,
isPremium = false,
isLoggedIn = true,
isVaultUnlocked = false,
needsPasswordReset = false,
isBiometricsEnabled = false,
organizations = emptyList(),
needsMasterPassword = false,
trustedDevice = null,
hasMasterPassword = true,
isUsingKeyConnector = false,
onboardingStatus = OnboardingStatus.COMPLETE,
firstTimeState = DEFAULT_FIRST_TIME_STATE,
isExportable = true,
)
private val DEFAULT_USER_STATE = UserState(
activeUserId = "activeUserId",
accounts = listOf(
UserState.Account(
userId = "activeUserId",
name = "Active User",
email = "active@bitwarden.com",
avatarColorHex = "#aa00aa",
environment = Environment.Us,
isPremium = true,
isLoggedIn = true,
isVaultUnlocked = true,
needsPasswordReset = false,
isBiometricsEnabled = false,
organizations = emptyList(),
needsMasterPassword = false,
trustedDevice = null,
hasMasterPassword = true,
isUsingKeyConnector = false,
onboardingStatus = OnboardingStatus.COMPLETE,
firstTimeState = DEFAULT_FIRST_TIME_STATE,
isExportable = true,
),
UserState.Account(
userId = "lockedUserId",
name = "Locked User",
email = "locked@bitwarden.com",
avatarColorHex = "#00aaaa",
environment = Environment.Us,
isPremium = false,
isLoggedIn = true,
isVaultUnlocked = false,
needsPasswordReset = false,
isBiometricsEnabled = false,
organizations = emptyList(),
needsMasterPassword = false,
trustedDevice = null,
hasMasterPassword = true,
isUsingKeyConnector = false,
onboardingStatus = OnboardingStatus.COMPLETE,
firstTimeState = DEFAULT_FIRST_TIME_STATE,
isExportable = true,
),
DEFAULT_ACTIVE_ACCOUNT,
DEFAULT_INACTIVE_ACCOUNT,
),
)