Skip to content

Commit

Permalink
[AC-2787] Remove the unassigned items dialog (#1466)
Browse files Browse the repository at this point in the history
  • Loading branch information
david-livefront authored Jun 19, 2024
1 parent ef03aa0 commit eb90069
Show file tree
Hide file tree
Showing 12 changed files with 1 addition and 364 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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].
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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].
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -159,7 +154,6 @@ class VaultViewModel @Inject constructor(
handleMasterPasswordRepromptSubmit(action)
}

VaultAction.UnassignedItemsAcknowledgeClick -> handleUnassignedItemsAcknowledgeClick()
is VaultAction.Internal -> handleInternalAction(action)
}
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
}
}

Expand Down Expand Up @@ -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.
*/
Expand All @@ -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.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
/**
Expand Down Expand Up @@ -89,9 +88,6 @@ data class VaultHandlers(
),
)
},
unassignedItemsAcknowledgeClick = {
viewModel.trySendAction(VaultAction.UnassignedItemsAcknowledgeClick)
},
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,6 @@ class SettingsDiskSourceTest {
systemBioIntegrityState = systemBioIntegrityState,
value = true,
)
settingsDiskSource.storeShouldCheckOrgUnassignedItems(
userId = userId,
shouldCheckOrgUnassignedItems = true,
)

settingsDiskSource.clearData(userId = userId)

Expand All @@ -169,7 +165,6 @@ class SettingsDiskSourceTest {
systemBioIntegrityState = systemBioIntegrityState,
),
)
assertNull(settingsDiskSource.getShouldCheckOrgUnassignedItems(userId = userId))
}

@Suppress("MaxLineLength")
Expand Down Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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?> =
Expand Down
Loading

0 comments on commit eb90069

Please sign in to comment.