[AC-2787] Remove the unassigned items dialog (#1466)

This commit is contained in:
David Perez 2024-06-19 10:01:33 -05:00 committed by Álison Fernandes
parent f03f9e63d4
commit 0faa1be4e4
12 changed files with 1 additions and 364 deletions

View file

@ -73,17 +73,6 @@ interface SettingsDiskSource {
*/
fun clearData(userId: String)
/**
* Retrieves the preference indicating whether we should check for unassigned organization
* ciphers.
*/
fun getShouldCheckOrgUnassignedItems(userId: String): Boolean?
/**
* Stores the given [shouldCheckOrgUnassignedItems] for the given [userId].
*/
fun storeShouldCheckOrgUnassignedItems(userId: String, shouldCheckOrgUnassignedItems: Boolean?)
/**
* Retrieves the biometric integrity validity for the given [userId] and
* [systemBioIntegrityState].

View file

@ -33,7 +33,6 @@ private const val CRASH_LOGGING_ENABLED_KEY = "crashLoggingEnabled"
private const val CLEAR_CLIPBOARD_INTERVAL_KEY = "clearClipboard"
private const val INITIAL_AUTOFILL_DIALOG_SHOWN = "addSitePromptShown"
private const val HAS_USER_LOGGED_IN_OR_CREATED_AN_ACCOUNT_KEY = "hasUserLoggedInOrCreatedAccount"
private const val SHOULD_CHECK_ORG_UNASSIGNED_ITEMS = "shouldCheckOrganizationUnassignedItems"
/**
* Primary implementation of [SettingsDiskSource].
@ -155,7 +154,6 @@ class SettingsDiskSourceImpl(
storeBlockedAutofillUris(userId = userId, blockedAutofillUris = null)
storeLastSyncTime(userId = userId, lastSyncTime = null)
storeClearClipboardFrequencySeconds(userId = userId, frequency = null)
storeShouldCheckOrgUnassignedItems(userId = userId, shouldCheckOrgUnassignedItems = null)
removeWithPrefix(prefix = ACCOUNT_BIOMETRIC_INTEGRITY_VALID_KEY.appendIdentifier(userId))
// The following are intentionally not cleared so they can be
@ -186,19 +184,6 @@ class SettingsDiskSourceImpl(
)
}
override fun getShouldCheckOrgUnassignedItems(userId: String): Boolean? =
getBoolean(key = SHOULD_CHECK_ORG_UNASSIGNED_ITEMS.appendIdentifier(userId))
override fun storeShouldCheckOrgUnassignedItems(
userId: String,
shouldCheckOrgUnassignedItems: Boolean?,
) {
putBoolean(
key = SHOULD_CHECK_ORG_UNASSIGNED_ITEMS.appendIdentifier(userId),
value = shouldCheckOrgUnassignedItems,
)
}
override fun getAutoCopyTotpDisabled(userId: String): Boolean? =
getBoolean(key = DISABLE_AUTO_TOTP_COPY_KEY.appendIdentifier(userId))

View file

@ -234,15 +234,4 @@ interface VaultRepository : CipherManager, VaultLockManager {
* Attempt to get the user's vault data for export.
*/
suspend fun exportVaultDataToString(format: ExportFormat): ExportVaultDataResult
/**
* Checks if the user should see the unassigned items message.
*/
suspend fun shouldShowUnassignedItemsInfo(): Boolean
/**
* Sets the value indicating that the user has or has not acknowledged that their organization
* has unassigned items.
*/
fun acknowledgeUnassignedItemsInfo(hasAcknowledged: Boolean)
}

View file

@ -863,26 +863,6 @@ class VaultRepositoryImpl(
)
}
@Suppress("ReturnCount")
override suspend fun shouldShowUnassignedItemsInfo(): Boolean {
val userId = activeUserId ?: return false
if (settingsDiskSource.getShouldCheckOrgUnassignedItems(userId = userId) == false) {
return false
}
return ciphersService.hasUnassignedCiphers().fold(
onFailure = { false },
onSuccess = { it },
)
}
override fun acknowledgeUnassignedItemsInfo(hasAcknowledged: Boolean) {
val userId = activeUserId ?: return
settingsDiskSource.storeShouldCheckOrgUnassignedItems(
userId = userId,
shouldCheckOrgUnassignedItems = !hasAcknowledged,
)
}
/**
* Checks if the given [userId] has an associated encrypted PIN key but not a pin-protected user
* key. This indicates a scenario in which a user has requested PIN unlocking but requires

View file

@ -355,18 +355,6 @@ private fun VaultDialogs(
onDismissRequest = vaultHandlers.dialogDismiss,
)
VaultState.DialogState.UnassignedItems -> BitwardenTwoButtonDialog(
title = stringResource(id = R.string.notice),
message = stringResource(
id = R.string.organization_unassigned_items_message_useu_description_long,
),
confirmButtonText = stringResource(id = R.string.remind_me_later),
dismissButtonText = stringResource(id = R.string.ok),
onConfirmClick = vaultHandlers.dialogDismiss,
onDismissClick = vaultHandlers.unassignedItemsAcknowledgeClick,
onDismissRequest = vaultHandlers.dialogDismiss,
)
null -> Unit
}
}

View file

@ -122,11 +122,6 @@ class VaultViewModel @Inject constructor(
sendAction(VaultAction.Internal.UserStateUpdateReceive(userState = it))
}
.launchIn(viewModelScope)
viewModelScope.launch {
val result = vaultRepository.shouldShowUnassignedItemsInfo()
sendAction(VaultAction.Internal.ReceiveUnassignedItemsResult(result))
}
}
override fun handleAction(action: VaultAction) {
@ -159,7 +154,6 @@ class VaultViewModel @Inject constructor(
handleMasterPasswordRepromptSubmit(action)
}
VaultAction.UnassignedItemsAcknowledgeClick -> handleUnassignedItemsAcknowledgeClick()
is VaultAction.Internal -> handleInternalAction(action)
}
}
@ -356,11 +350,6 @@ class VaultViewModel @Inject constructor(
}
}
private fun handleUnassignedItemsAcknowledgeClick() {
vaultRepository.acknowledgeUnassignedItemsInfo(hasAcknowledged = true)
mutableStateFlow.update { it.copy(dialog = null) }
}
private fun handleCopyNoteClick(action: ListingItemOverflowAction.VaultAction.CopyNoteClick) {
clipboardManager.setText(action.notes)
}
@ -420,10 +409,6 @@ class VaultViewModel @Inject constructor(
handlePullToRefreshEnableReceive(action)
}
is VaultAction.Internal.ReceiveUnassignedItemsResult -> {
handleReceiveUnassignedItemsResult(action)
}
is VaultAction.Internal.UserStateUpdateReceive -> handleUserStateUpdateReceive(action)
is VaultAction.Internal.VaultDataReceive -> handleVaultDataReceive(action)
is VaultAction.Internal.IconLoadingSettingReceive -> handleIconLoadingSettingReceive(
@ -455,14 +440,6 @@ class VaultViewModel @Inject constructor(
}
}
private fun handleReceiveUnassignedItemsResult(
action: VaultAction.Internal.ReceiveUnassignedItemsResult,
) {
if (action.result) {
mutableStateFlow.update { it.copy(dialog = VaultState.DialogState.UnassignedItems) }
}
}
private fun handleUserStateUpdateReceive(action: VaultAction.Internal.UserStateUpdateReceive) {
// Leave the current data alone if there is no UserState; we are in the process of logging
// out.
@ -541,13 +518,7 @@ class VaultViewModel @Inject constructor(
hasMasterPassword = state.hasMasterPassword,
vaultFilterType = vaultFilterTypeOrDefault,
),
dialog = when (it.dialog) {
VaultState.DialogState.UnassignedItems -> VaultState.DialogState.UnassignedItems
is VaultState.DialogState.Error,
VaultState.DialogState.Syncing,
null,
-> null
},
dialog = null,
)
}
sendEvent(VaultEvent.DismissPullToRefresh)
@ -929,12 +900,6 @@ data class VaultState(
val title: Text,
val message: Text,
) : DialogState()
/**
* Represents a dialog indicating that the user has unassigned items.
*/
@Parcelize
data object UnassignedItems : DialogState()
}
}
@ -1149,11 +1114,6 @@ sealed class VaultAction {
val password: String,
) : VaultAction()
/**
* The user has acknowledged the unassigned items and we do not want to show the message again.
*/
data object UnassignedItemsAcknowledgeClick : VaultAction()
/**
* Models actions that the [VaultViewModel] itself might send.
*/
@ -1178,14 +1138,6 @@ sealed class VaultAction {
*/
data class PullToRefreshEnableReceive(val isPullToRefreshEnabled: Boolean) : Internal()
/**
* Indicates that we have received the data concerning whether we should display the
* unassigned items dialog.
*/
data class ReceiveUnassignedItemsResult(
val result: Boolean,
) : Internal()
/**
* Indicates a change in user state has been received.
*/

View file

@ -34,7 +34,6 @@ data class VaultHandlers(
val dialogDismiss: () -> Unit,
val overflowOptionClick: (ListingItemOverflowAction.VaultAction) -> Unit,
val masterPasswordRepromptSubmit: (ListingItemOverflowAction.VaultAction, String) -> Unit,
val unassignedItemsAcknowledgeClick: () -> Unit,
) {
companion object {
/**
@ -89,9 +88,6 @@ data class VaultHandlers(
),
)
},
unassignedItemsAcknowledgeClick = {
viewModel.trySendAction(VaultAction.UnassignedItemsAcknowledgeClick)
},
)
}
}

View file

@ -142,10 +142,6 @@ class SettingsDiskSourceTest {
systemBioIntegrityState = systemBioIntegrityState,
value = true,
)
settingsDiskSource.storeShouldCheckOrgUnassignedItems(
userId = userId,
shouldCheckOrgUnassignedItems = true,
)
settingsDiskSource.clearData(userId = userId)
@ -169,7 +165,6 @@ class SettingsDiskSourceTest {
systemBioIntegrityState = systemBioIntegrityState,
),
)
assertNull(settingsDiskSource.getShouldCheckOrgUnassignedItems(userId = userId))
}
@Suppress("MaxLineLength")
@ -957,40 +952,6 @@ class SettingsDiskSourceTest {
)
}
@Test
fun `storeShouldCheckOrgUnassignedItems should update shared preferences`() {
val baseKey = "bwPreferencesStorage:shouldCheckOrganizationUnassignedItems"
val mockUserId = "mockUserId"
val key = "${baseKey}_$mockUserId"
assertNull(settingsDiskSource.getShouldCheckOrgUnassignedItems(userId = mockUserId))
// Updating the disk source updates shared preferences
settingsDiskSource.storeShouldCheckOrgUnassignedItems(
userId = mockUserId,
shouldCheckOrgUnassignedItems = true,
)
assertTrue(fakeSharedPreferences.contains(key))
}
@Test
fun `getShouldCheckOrgUnassignedItems should pull from SharedPreferences`() {
val baseKey = "bwPreferencesStorage:shouldCheckOrganizationUnassignedItems"
val mockUserId = "mockUserId"
val expectedValue = true
val key = "${baseKey}_$mockUserId"
assertNull(settingsDiskSource.getShouldCheckOrgUnassignedItems(userId = mockUserId))
// Update SharedPreferences updates the disk source
fakeSharedPreferences.edit { putBoolean(key, expectedValue) }
assertEquals(
expectedValue,
settingsDiskSource.getShouldCheckOrgUnassignedItems(userId = mockUserId),
)
}
@Test
fun `initialAutofillDialogShown should pull from and update SharedPreferences`() {
val initialAutofillDialogShownKey = "bwPreferencesStorage:addSitePromptShown"

View file

@ -60,7 +60,6 @@ class FakeSettingsDiskSource : SettingsDiskSource {
private val storedScreenCaptureAllowed = mutableMapOf<String, Boolean?>()
private var storedSystemBiometricIntegritySource: String? = null
private val storedAccountBiometricIntegrityValidity = mutableMapOf<String, Boolean?>()
private val storedShouldCheckOrgUnassignedItems = mutableMapOf<String, Boolean?>()
override var appLanguage: AppLanguage? = null
@ -147,23 +146,12 @@ class FakeSettingsDiskSource : SettingsDiskSource {
storedInlineAutofillEnabled.remove(userId)
storedBlockedAutofillUris.remove(userId)
storedClearClipboardFrequency.remove(userId)
storedShouldCheckOrgUnassignedItems.remove(userId)
mutableVaultTimeoutActionsFlowMap.remove(userId)
mutableVaultTimeoutInMinutesFlowMap.remove(userId)
mutableLastSyncCallFlowMap.remove(userId)
}
override fun getShouldCheckOrgUnassignedItems(userId: String): Boolean? =
storedShouldCheckOrgUnassignedItems[userId]
override fun storeShouldCheckOrgUnassignedItems(
userId: String,
shouldCheckOrgUnassignedItems: Boolean?,
) {
storedShouldCheckOrgUnassignedItems[userId] = shouldCheckOrgUnassignedItems
}
override fun getLastSyncTime(userId: String): Instant? = storedLastSyncTime[userId]
override fun getLastSyncTimeFlow(userId: String): Flow<Instant?> =

View file

@ -116,7 +116,6 @@ import kotlinx.coroutines.flow.update
import kotlinx.coroutines.test.runTest
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertFalse
import org.junit.jupiter.api.Assertions.assertTrue
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
@ -4161,99 +4160,6 @@ class VaultRepositoryTest {
)
}
@Test
fun `shouldShowUnassignedItemsInfo should return false if there is no active user`() = runTest {
val result = vaultRepository.shouldShowUnassignedItemsInfo()
assertFalse(result)
}
@Suppress("MaxLineLength")
@Test
fun `shouldShowUnassignedItemsInfo should return false if getShouldCheckOrgUnassignedItems is false`() =
runTest {
val userId = MOCK_USER_STATE.activeUserId
fakeAuthDiskSource.userState = MOCK_USER_STATE
every {
settingsDiskSource.getShouldCheckOrgUnassignedItems(userId = userId)
} returns false
val result = vaultRepository.shouldShowUnassignedItemsInfo()
assertFalse(result)
verify(exactly = 1) {
settingsDiskSource.getShouldCheckOrgUnassignedItems(userId = userId)
}
}
@Suppress("MaxLineLength")
@Test
fun `shouldShowUnassignedItemsInfo should return true if getShouldCheckOrgUnassignedItems is true and hasUnassignedCiphers is true`() =
runTest {
val userId = MOCK_USER_STATE.activeUserId
fakeAuthDiskSource.userState = MOCK_USER_STATE
every {
settingsDiskSource.getShouldCheckOrgUnassignedItems(userId = userId)
} returns true
coEvery { ciphersService.hasUnassignedCiphers() } returns true.asSuccess()
val result = vaultRepository.shouldShowUnassignedItemsInfo()
assertTrue(result)
verify(exactly = 1) {
settingsDiskSource.getShouldCheckOrgUnassignedItems(userId = userId)
}
coVerify(exactly = 1) {
ciphersService.hasUnassignedCiphers()
}
}
@Suppress("MaxLineLength")
@Test
fun `shouldShowUnassignedItemsInfo should return false if getShouldCheckOrgUnassignedItems is true and hasUnassignedCiphers is an error`() =
runTest {
val userId = MOCK_USER_STATE.activeUserId
fakeAuthDiskSource.userState = MOCK_USER_STATE
every {
settingsDiskSource.getShouldCheckOrgUnassignedItems(userId = userId)
} returns true
coEvery { ciphersService.hasUnassignedCiphers() } returns Throwable().asFailure()
val result = vaultRepository.shouldShowUnassignedItemsInfo()
assertFalse(result)
verify(exactly = 1) {
settingsDiskSource.getShouldCheckOrgUnassignedItems(userId = userId)
}
coVerify(exactly = 1) {
ciphersService.hasUnassignedCiphers()
}
}
@Test
fun `acknowledgeUnassignedItemsInfo should do nothing if there is no active user`() {
val hasAcknowledged = true
vaultRepository.acknowledgeUnassignedItemsInfo(hasAcknowledged = hasAcknowledged)
verify(exactly = 0) {
settingsDiskSource.storeShouldCheckOrgUnassignedItems(
userId = any(),
shouldCheckOrgUnassignedItems = !hasAcknowledged,
)
}
}
@Test
fun `acknowledgeUnassignedItemsInfo should do store the appropriate value`() {
val hasAcknowledged = true
every {
settingsDiskSource.storeShouldCheckOrgUnassignedItems(
userId = MOCK_USER_STATE.activeUserId,
shouldCheckOrgUnassignedItems = !hasAcknowledged,
)
} just runs
fakeAuthDiskSource.userState = MOCK_USER_STATE
vaultRepository.acknowledgeUnassignedItemsInfo(hasAcknowledged = hasAcknowledged)
verify(exactly = 1) {
settingsDiskSource.storeShouldCheckOrgUnassignedItems(
userId = MOCK_USER_STATE.activeUserId,
shouldCheckOrgUnassignedItems = !hasAcknowledged,
)
}
}
//region Helper functions
/**

View file

@ -497,60 +497,6 @@ class VaultScreenTest : BaseComposeTest() {
composeTestRule.onNodeWithContentDescription(fabDescription).assertIsDisplayed()
}
@Suppress("MaxLineLength")
@Test
fun `unassigned items dialog should be shown or hidden according to the state`() {
val title = "Notice"
val message =
"Unassigned organization items are no longer visible in the All Vaults view and only accessible via the Admin Console. Assign these items to a collection from the Admin Console to make them visible."
composeTestRule.assertNoDialogExists()
composeTestRule.onNodeWithText(title).assertDoesNotExist()
composeTestRule.onNodeWithText(message).assertDoesNotExist()
mutableStateFlow.update { it.copy(dialog = VaultState.DialogState.UnassignedItems) }
composeTestRule
.onAllNodesWithText(title)
.filterToOne(hasAnyAncestor(isDialog()))
.assertIsDisplayed()
composeTestRule
.onAllNodesWithText(message)
.filterToOne(hasAnyAncestor(isDialog()))
.assertIsDisplayed()
composeTestRule
.onAllNodesWithText("Ok")
.filterToOne(hasAnyAncestor(isDialog()))
.assertIsDisplayed()
composeTestRule
.onAllNodesWithText("Remind me later")
.filterToOne(hasAnyAncestor(isDialog()))
.assertIsDisplayed()
}
@Test
fun `OK button click in unassigned items dialog should send UnassignedItemsAcknowledgeClick`() {
mutableStateFlow.update { it.copy(dialog = VaultState.DialogState.UnassignedItems) }
composeTestRule
.onAllNodesWithText("Ok")
.filterToOne(hasAnyAncestor(isDialog()))
.performClick()
verify { viewModel.trySendAction(VaultAction.UnassignedItemsAcknowledgeClick) }
}
@Test
fun `Remind me later button click in unassigned items dialog should send DialogDismiss`() {
mutableStateFlow.update { it.copy(dialog = VaultState.DialogState.UnassignedItems) }
composeTestRule
.onAllNodesWithText("Remind me later")
.filterToOne(hasAnyAncestor(isDialog()))
.performClick()
verify { viewModel.trySendAction(VaultAction.DialogDismiss) }
}
@Test
fun `error dialog should be shown or hidden according to the state`() {
val errorTitle = "Error title"

View file

@ -96,7 +96,6 @@ class VaultViewModelTest : BaseViewModelTest() {
every { syncIfNecessary() } just runs
every { lockVaultForCurrentUser() } just runs
every { lockVault(any()) } just runs
coEvery { shouldShowUnassignedItemsInfo() } returns false
}
@Test
@ -1100,48 +1099,6 @@ class VaultViewModelTest : BaseViewModelTest() {
verify { vaultRepository.sync() }
}
@Suppress("MaxLineLength")
@Test
fun `UnassignedItemsAcknowledgeClick should acknowledge the message and clear the dialog state`() {
coEvery { vaultRepository.shouldShowUnassignedItemsInfo() } returns true
every { vaultRepository.acknowledgeUnassignedItemsInfo(hasAcknowledged = true) } just runs
val viewModel = createViewModel()
viewModel.trySendAction(VaultAction.UnassignedItemsAcknowledgeClick)
verify(exactly = 1) {
vaultRepository.acknowledgeUnassignedItemsInfo(hasAcknowledged = true)
}
}
@Suppress("MaxLineLength")
@Test
fun `ReceiveUnassignedItemsResult should display the unassigned items dialog when result is true`() {
val initialState = DEFAULT_STATE
val viewModel = createViewModel()
viewModel.trySendAction(VaultAction.Internal.ReceiveUnassignedItemsResult(result = true))
assertEquals(
initialState.copy(dialog = VaultState.DialogState.UnassignedItems),
viewModel.stateFlow.value,
)
}
@Suppress("MaxLineLength")
@Test
fun `ReceiveUnassignedItemsResult should not display the unassigned items dialog when result is false`() {
val initialState = DEFAULT_STATE
val viewModel = createViewModel()
viewModel.trySendAction(VaultAction.Internal.ReceiveUnassignedItemsResult(result = false))
assertEquals(
initialState.copy(dialog = null),
viewModel.stateFlow.value,
)
}
@Test
fun `DialogDismiss should clear the active dialog`() {
// Show the No Network error dialog