diff --git a/core/item/src/main/kotlin/de/davis/keygo/core/item/data/local/dao/ItemDao.kt b/core/item/src/main/kotlin/de/davis/keygo/core/item/data/local/dao/ItemDao.kt index 3514a0ce..c01cf11f 100644 --- a/core/item/src/main/kotlin/de/davis/keygo/core/item/data/local/dao/ItemDao.kt +++ b/core/item/src/main/kotlin/de/davis/keygo/core/item/data/local/dao/ItemDao.kt @@ -66,8 +66,8 @@ internal interface ItemDao { itemType: VaultItemType? = null, ): Flow> - @Query("UPDATE item SET pinned = :pinned WHERE id = :id") - suspend fun setPinned(id: ItemId, pinned: Boolean) + @Query("UPDATE item SET pinned = :pinned WHERE id IN (:ids)") + suspend fun setPinned(ids: Collection, pinned: Boolean) @Query("SELECT i.id, i.name, i.item_type as itemType, i.pinned FROM item i WHERE (:vaultId IS NULL OR vault_id = :vaultId)") fun observeLiteItems(vaultId: VaultId? = null): Flow> diff --git a/core/item/src/main/kotlin/de/davis/keygo/core/item/data/repository/ItemRepositoryImpl.kt b/core/item/src/main/kotlin/de/davis/keygo/core/item/data/repository/ItemRepositoryImpl.kt index d4fd0c11..8bf2cc6e 100644 --- a/core/item/src/main/kotlin/de/davis/keygo/core/item/data/repository/ItemRepositoryImpl.kt +++ b/core/item/src/main/kotlin/de/davis/keygo/core/item/data/repository/ItemRepositoryImpl.kt @@ -66,8 +66,8 @@ internal class ItemRepositoryImpl( itemDao.searchItem(query, Tag.normalize(query), itemType) .map { results -> results.map(LightweightItemSearchResult::toDomain) } - override suspend fun setPinned(itemId: ItemId, pinned: Boolean) = - itemDao.setPinned(itemId, pinned) + override suspend fun setPinned(itemIds: Set, pinned: Boolean) = + itemDao.setPinned(itemIds, pinned) override fun observeAllTags(): Flow> = tagDao.observeAllTags().map { it.map(TagEntity::toDomain) } diff --git a/core/item/src/main/kotlin/de/davis/keygo/core/item/domain/repository/ItemRepository.kt b/core/item/src/main/kotlin/de/davis/keygo/core/item/domain/repository/ItemRepository.kt index a6d4fc2c..a3e8fb0b 100644 --- a/core/item/src/main/kotlin/de/davis/keygo/core/item/domain/repository/ItemRepository.kt +++ b/core/item/src/main/kotlin/de/davis/keygo/core/item/domain/repository/ItemRepository.kt @@ -31,7 +31,7 @@ interface ItemRepository { itemType: VaultItemType? = null ): Flow> - suspend fun setPinned(itemId: ItemId, pinned: Boolean) + suspend fun setPinned(itemIds: Set, pinned: Boolean) fun observeAllTags(): Flow> diff --git a/core/item/src/testFixtures/kotlin/de/davis/keygo/core/item/FakeItemRepository.kt b/core/item/src/testFixtures/kotlin/de/davis/keygo/core/item/FakeItemRepository.kt index cb5f639e..6652bbcc 100644 --- a/core/item/src/testFixtures/kotlin/de/davis/keygo/core/item/FakeItemRepository.kt +++ b/core/item/src/testFixtures/kotlin/de/davis/keygo/core/item/FakeItemRepository.kt @@ -118,7 +118,13 @@ class FakeItemRepository( } } - override suspend fun setPinned(itemId: ItemId, pinned: Boolean) = Unit + override suspend fun setPinned(itemIds: Set, pinned: Boolean) { + allStores.update { store -> + store.mapValues { (id, item) -> + if (id in itemIds) item.copy(pinned = pinned) else item + } + } + } override fun observeLiteVaultItems(vaultId: VaultId?): Flow> = allStores.map { store -> diff --git a/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/creditcard/ViewCreditCardViewModel.kt b/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/creditcard/ViewCreditCardViewModel.kt index e16cdea1..4ac694c1 100644 --- a/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/creditcard/ViewCreditCardViewModel.kt +++ b/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/creditcard/ViewCreditCardViewModel.kt @@ -136,7 +136,7 @@ internal class ViewCreditCardViewModel( ViewCreditCardUiEvent.OnPinClick -> _itemId.value?.let { id -> viewModelScope.launch { - itemRepository.setPinned(id, !state.value.pinned) + itemRepository.setPinned(setOf(id), !state.value.pinned) } } diff --git a/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/ViewLoginViewModel.kt b/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/ViewLoginViewModel.kt index c338565d..cbb74dc0 100644 --- a/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/ViewLoginViewModel.kt +++ b/feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/ViewLoginViewModel.kt @@ -182,7 +182,7 @@ internal class ViewLoginViewModel( ViewLoginUiEvent.OnPinClick -> { _itemId.value?.let { id -> viewModelScope.launch { - itemRepository.setPinned(id, !state.value.pinned) + itemRepository.setPinned(setOf(id), !state.value.pinned) } } } diff --git a/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/ItemListScreen.kt b/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/ItemListScreen.kt index 5e5a77f4..a0127c79 100644 --- a/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/ItemListScreen.kt +++ b/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/ItemListScreen.kt @@ -120,6 +120,7 @@ fun ItemListScreen( onClearSelection = viewModel::onClearSelection, onSelectAll = viewModel::onSelectAll, onDeleteSelectedRequest = viewModel::onDeleteSelectedRequest, + onPinSelectedRequest = viewModel::onPinSelectedRequest, onDismissDeleteConfirmation = viewModel::onDismissDeleteConfirmation, onConfirmDeleteSelected = viewModel::onConfirmDeleteSelected, scrollBehavior = scrollBehavior, diff --git a/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/ItemListViewModel.kt b/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/ItemListViewModel.kt index 6e982ee2..6285e4dc 100644 --- a/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/ItemListViewModel.kt +++ b/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/ItemListViewModel.kt @@ -21,6 +21,7 @@ import de.davis.keygo.feature.list_screen.presentation.mapper.toBottomSheetState import de.davis.keygo.feature.list_screen.presentation.model.Event import de.davis.keygo.feature.list_screen.presentation.model.FilterAction import de.davis.keygo.feature.list_screen.presentation.model.FilterBottomSheetState +import de.davis.keygo.feature.list_screen.presentation.model.ItemSelection import de.davis.keygo.feature.list_screen.presentation.model.ListItemState import de.davis.keygo.feature.vault.domain.usecase.ObserveVaultsAndSelectionUseCase import kotlinx.coroutines.Dispatchers @@ -100,7 +101,7 @@ internal class ItemListViewModel( filterUseCase(filter, items, scores, tagIds) }.distinctUntilChanged() - private val selectedItemIds = MutableStateFlow(emptySet()) + private val selection = MutableStateFlow(ItemSelection()) private val highlightedId = MutableStateFlow(null) private val _isVaultFlowVisible = MutableStateFlow(false) private val _isDeleteConfirmationVisible = MutableStateFlow(false) @@ -122,17 +123,17 @@ internal class ItemListViewModel( vaultsAndSelection, filteredItems, searchResults, - selectedItemIds, + selection, submittedSearchQuery, highlightedId, _isVaultFlowVisible, _isDeleteConfirmationVisible, - ) { vaultsAndSel, items, searchResults, selectedIds, submittedSearchQuery, highlightedId, isVaultFlowVisible, isDeleteConfirmationVisible -> + ) { vaultsAndSel, items, searchResults, selection, submittedSearchQuery, highlightedId, isVaultFlowVisible, isDeleteConfirmationVisible -> ListItemState( items = items, searchResults = searchResults, hasSearchQuery = submittedSearchQuery.isNotBlank(), - selectedItemIds = selectedIds, + selection = selection, highlightedId = highlightedId, isVaultFlowVisible = isVaultFlowVisible, isDeleteConfirmationVisible = isDeleteConfirmationVisible, @@ -235,15 +236,25 @@ internal class ItemListViewModel( fun onSelectAll() { if (!enableSelection) return - selectedItemIds.update { listItemState.value.items.mapTo(mutableSetOf()) { it.id } } + selection.update { ItemSelection.of(listItemState.value.items) } } fun onClearSelection() { - selectedItemIds.update { emptySet() } + selection.update { ItemSelection() } } fun onDeleteSelectedRequest() { - if (selectedItemIds.value.isNotEmpty()) _isDeleteConfirmationVisible.update { true } + if (selection.value.isActive) _isDeleteConfirmationVisible.update { true } + } + + fun onPinSelectedRequest() { + val current = selection.value + if (!current.isActive) return + + val pinned = !current.allPinned + selection.update { it.withAllPinned(pinned) } + + viewModelScope.launch { itemRepository.setPinned(current.ids, pinned) } } fun onDismissDeleteConfirmation() { @@ -253,7 +264,7 @@ internal class ItemListViewModel( fun onConfirmDeleteSelected() { _isDeleteConfirmationVisible.update { false } - val deleted = selectedItemIds.getAndUpdate { emptySet() } + val deleted = selection.getAndUpdate { ItemSelection() }.ids if (deleted.isEmpty()) return // Read off the list still on screen: after the delete lands the flow has already dropped @@ -270,8 +281,8 @@ internal class ItemListViewModel( fun onItemClick(itemId: ItemId, forceSkipSelection: Boolean = false) { - if (enableSelection && !forceSkipSelection && selectedItemIds.value.isNotEmpty()) { - val isSelected = itemId in selectedItemIds.value + if (enableSelection && !forceSkipSelection && selection.value.isActive) { + val isSelected = itemId in selection.value.ids updateItemSelectionState(itemId, selected = !isSelected) } else { highlightedId.update { itemId } @@ -288,9 +299,12 @@ internal class ItemListViewModel( private fun updateItemSelectionState(id: ItemId, selected: Boolean) { - selectedItemIds.update { currentSelectedIds -> - if (selected) currentSelectedIds + id - else currentSelectedIds - id + // The pinned flag is read off the row being selected: the selection carries it from here + // on, so the top bar knows whether it can offer an unpin without asking the list again. + val pinned = listItemState.value.items.any { it.id == id && it.pinned } + selection.update { currentSelection -> + if (selected) currentSelection.select(id, pinned) + else currentSelection.deselect(id) } } diff --git a/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/components/ItemListContent.kt b/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/components/ItemListContent.kt index 3b5b41b2..c6d6f70a 100644 --- a/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/components/ItemListContent.kt +++ b/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/components/ItemListContent.kt @@ -78,6 +78,7 @@ internal fun ItemListContent( onClearSelection: () -> Unit, onSelectAll: () -> Unit, onDeleteSelectedRequest: () -> Unit, + onPinSelectedRequest: () -> Unit, onDismissDeleteConfirmation: () -> Unit, onConfirmDeleteSelected: () -> Unit, onVaultSelectorClick: () -> Unit, @@ -127,9 +128,11 @@ internal fun ItemListContent( SelectionTopBar( selectedCount = uiState.selectedItemIds.size, canDelete = enableDeletion, + allPinned = uiState.allSelectedPinned, onClearSelection = onClearSelection, onSelectAll = onSelectAll, onDeleteSelected = onDeleteSelectedRequest, + onPinSelected = onPinSelectedRequest, ) else AppBarWithSearch( @@ -275,7 +278,6 @@ private fun ItemListContentPreview() { searchResults = listOf(sampleItem), hasSearchQuery = false, highlightedId = null, - selectedItemIds = emptySet(), ) } val searchTextFieldState = rememberTextFieldState() @@ -314,6 +316,7 @@ private fun ItemListContentPreview() { onClearSelection = {}, onSelectAll = {}, onDeleteSelectedRequest = {}, + onPinSelectedRequest = {}, onDismissDeleteConfirmation = {}, onConfirmDeleteSelected = {}, onVaultSelectorClick = {}, diff --git a/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/components/SelectionTopBar.kt b/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/components/SelectionTopBar.kt index 9cd72576..d0233dba 100644 --- a/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/components/SelectionTopBar.kt +++ b/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/components/SelectionTopBar.kt @@ -3,7 +3,9 @@ package de.davis.keygo.feature.list_screen.presentation.components import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.Close import androidx.compose.material.icons.filled.Delete +import androidx.compose.material.icons.filled.PushPin import androidx.compose.material.icons.filled.SelectAll +import androidx.compose.material.icons.outlined.PushPin import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.IconButton @@ -21,9 +23,11 @@ import de.davis.keygo.feature.list_screen.R internal fun SelectionTopBar( selectedCount: Int, canDelete: Boolean, + allPinned: Boolean, onClearSelection: () -> Unit, onSelectAll: () -> Unit, onDeleteSelected: () -> Unit, + onPinSelected: () -> Unit, modifier: Modifier = Modifier, ) { TopAppBar( @@ -47,6 +51,15 @@ internal fun SelectionTopBar( ) } + IconButton(onClick = onPinSelected) { + Icon( + imageVector = if (allPinned) Icons.Default.PushPin else Icons.Outlined.PushPin, + contentDescription = stringResource( + if (allPinned) R.string.unpin_all else R.string.pin_all, + ), + ) + } + if (canDelete) IconButton(onClick = onDeleteSelected) { Icon( diff --git a/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/model/ItemSelection.kt b/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/model/ItemSelection.kt new file mode 100644 index 00000000..c10a254e --- /dev/null +++ b/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/model/ItemSelection.kt @@ -0,0 +1,28 @@ +package de.davis.keygo.feature.list_screen.presentation.model + +import androidx.compose.runtime.Immutable +import de.davis.keygo.core.item.domain.alias.ItemId +import de.davis.keygo.core.item.domain.model.lite.LiteItem + +@Immutable +internal data class ItemSelection(val pinnedById: Map = emptyMap()) { + + val ids: Set get() = pinnedById.keys + + val isActive: Boolean get() = pinnedById.isNotEmpty() + + val allPinned: Boolean get() = isActive && pinnedById.values.all { it } + + fun select(itemId: ItemId, pinned: Boolean): ItemSelection = + ItemSelection(pinnedById + (itemId to pinned)) + + fun deselect(itemId: ItemId): ItemSelection = ItemSelection(pinnedById - itemId) + + fun withAllPinned(pinned: Boolean): ItemSelection = + ItemSelection(pinnedById.mapValues { pinned }) + + companion object { + fun of(items: List): ItemSelection = + ItemSelection(items.associate { it.id to it.pinned }) + } +} diff --git a/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/model/ListItemState.kt b/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/model/ListItemState.kt index f33830b6..3d014ac3 100644 --- a/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/model/ListItemState.kt +++ b/feature/list_screen/src/main/kotlin/de/davis/keygo/feature/list_screen/presentation/model/ListItemState.kt @@ -11,12 +11,14 @@ internal data class ListItemState( val items: List = emptyList(), val searchResults: List = emptyList(), val hasSearchQuery: Boolean = false, - val selectedItemIds: Set = emptySet(), + val selection: ItemSelection = ItemSelection(), val highlightedId: ItemId? = null, val isVaultFlowVisible: Boolean = false, val isDeleteConfirmationVisible: Boolean = false, val vaults: List = emptyList(), val vaultContext: VaultContext = VaultContext.NoSpecific, ) { - val isSelectionActive: Boolean get() = selectedItemIds.isNotEmpty() + val selectedItemIds: Set get() = selection.ids + val isSelectionActive: Boolean get() = selection.isActive + val allSelectedPinned: Boolean get() = selection.allPinned } diff --git a/feature/list_screen/src/main/res/values/strings.xml b/feature/list_screen/src/main/res/values/strings.xml index f2fdd206..a9c18ef8 100644 --- a/feature/list_screen/src/main/res/values/strings.xml +++ b/feature/list_screen/src/main/res/values/strings.xml @@ -5,6 +5,8 @@ %d selected Clear selection Select all + Pin all + Unpin all Delete Cancel diff --git a/feature/list_screen/src/test/kotlin/de/davis/keygo/feature/list_screen/presentation/ItemListViewModelTest.kt b/feature/list_screen/src/test/kotlin/de/davis/keygo/feature/list_screen/presentation/ItemListViewModelTest.kt index d0d650f2..62537f01 100644 --- a/feature/list_screen/src/test/kotlin/de/davis/keygo/feature/list_screen/presentation/ItemListViewModelTest.kt +++ b/feature/list_screen/src/test/kotlin/de/davis/keygo/feature/list_screen/presentation/ItemListViewModelTest.kt @@ -69,7 +69,12 @@ class ItemListViewModelTest { loginRepository = loginRepository, ) - private fun login(name: String, id: ItemId = newItemId(), vault: VaultId = vaultId) = Login( + private fun login( + name: String, + id: ItemId = newItemId(), + vault: VaultId = vaultId, + pinned: Boolean = false, + ) = Login( id = id, name = name, username = null, @@ -78,7 +83,7 @@ class ItemListViewModelTest { totp = null, passkeys = emptySet(), note = null, - pinned = false, + pinned = pinned, vaultId = vault, keyInformation = KeyInformation(byteArrayOf(), byteArrayOf()), timestamp = Timestamp(), @@ -87,6 +92,11 @@ class ItemListViewModelTest { private suspend fun storedIds(): Set = itemRepository.observeLiteVaultItems().first().mapTo(mutableSetOf()) { it.id } + private suspend fun pinnedIds(): Set = + itemRepository.observeLiteVaultItems().first() + .filter { it.pinned } + .mapTo(mutableSetOf()) { it.id } + /** * The bug this flow replaced: each swipe hung its delete off a snackbar that the next swipe * replaced, so every item after the first was hidden but never actually deleted. @@ -263,6 +273,125 @@ class ItemListViewModelTest { assertEquals(setOf(item.id), storedIds()) assertEquals(emptySet(), vm.listItemState.value.selectedItemIds) } + + @Test + fun `pinning a selection pins every item in it`() = runTest(dispatcher) { + val first = login("First") + val second = login("Second") + loginRepository.seed(first, second) + + val vm = viewModel() + backgroundScope.launchCollect(vm) + advanceUntilIdle() + + vm.onItemLongClick(first.id) + vm.onItemClick(second.id) + vm.onPinSelectedRequest() + advanceUntilIdle() + + assertEquals(setOf(first.id, second.id), pinnedIds()) + assertTrue(vm.listItemState.value.allSelectedPinned) + } + + /** The mixed case: one unpinned item is enough to make the button pin rather than unpin. */ + @Test + fun `a selection holding one unpinned item pins the whole selection`() = runTest(dispatcher) { + val alreadyPinned = login("Pinned", pinned = true) + val plain = login("Plain") + loginRepository.seed(alreadyPinned, plain) + + val vm = viewModel() + backgroundScope.launchCollect(vm) + advanceUntilIdle() + + vm.onItemLongClick(alreadyPinned.id) + vm.onItemClick(plain.id) + advanceUntilIdle() + assertFalse(vm.listItemState.value.allSelectedPinned) + + vm.onPinSelectedRequest() + advanceUntilIdle() + + assertEquals(setOf(alreadyPinned.id, plain.id), pinnedIds()) + } + + @Test + fun `pinning a selection whose items are all pinned unpins them`() = runTest(dispatcher) { + val first = login("First", pinned = true) + val second = login("Second", pinned = true) + loginRepository.seed(first, second) + + val vm = viewModel() + backgroundScope.launchCollect(vm) + advanceUntilIdle() + + vm.onItemLongClick(first.id) + vm.onItemClick(second.id) + advanceUntilIdle() + assertTrue(vm.listItemState.value.allSelectedPinned) + + vm.onPinSelectedRequest() + advanceUntilIdle() + + assertEquals(emptySet(), pinnedIds()) + assertFalse(vm.listItemState.value.allSelectedPinned) + } + + /** + * The selection carries a pinned flag per item, so dropping the one unpinned item has to hand + * the action back to unpin. A single "are they all pinned" boolean could not recover this. + */ + @Test + fun `deselecting the only unpinned item turns the action back into an unpin`() = + runTest(dispatcher) { + val alreadyPinned = login("Pinned", pinned = true) + val plain = login("Plain") + loginRepository.seed(alreadyPinned, plain) + + val vm = viewModel() + backgroundScope.launchCollect(vm) + advanceUntilIdle() + + vm.onItemLongClick(alreadyPinned.id) + vm.onItemClick(plain.id) + advanceUntilIdle() + assertFalse(vm.listItemState.value.allSelectedPinned) + + vm.onItemClick(plain.id) + advanceUntilIdle() + + assertEquals(setOf(alreadyPinned.id), vm.listItemState.value.selectedItemIds) + assertTrue(vm.listItemState.value.allSelectedPinned) + } + + @Test + fun `select all carries the pinned state of every item it picks up`() = runTest(dispatcher) { + loginRepository.seed(login("A", pinned = true), login("B", pinned = true)) + + val vm = viewModel() + backgroundScope.launchCollect(vm) + advanceUntilIdle() + + vm.onSelectAll() + advanceUntilIdle() + + assertTrue(vm.listItemState.value.allSelectedPinned) + } + + @Test + fun `requesting a pin with nothing selected pins nothing`() = runTest(dispatcher) { + loginRepository.seed(login("Untouched")) + + val vm = viewModel() + backgroundScope.launchCollect(vm) + advanceUntilIdle() + + vm.onPinSelectedRequest() + advanceUntilIdle() + + assertEquals(emptySet(), pinnedIds()) + assertFalse(vm.listItemState.value.allSelectedPinned) + } } /**