From c2b93c43e0ba66604961d09546e44171cf6fcf21 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Fri, 16 Oct 2026 13:26:29 +0200 Subject: [PATCH] Filter uses the shared account groups --- .../calendula/ui/filter/CalendarFilterList.kt | 23 ++++----- .../calendula/ui/filter/FilterUiState.kt | 28 +++-------- .../calendula/ui/filter/FilterViewModel.kt | 28 +---------- .../AccountGroupsTest.kt} | 47 +++++-------------- 4 files changed, 28 insertions(+), 98 deletions(-) rename app/src/test/java/de/jeanlucmakiola/calendula/ui/{filter/FilterGroupingTest.kt => common/AccountGroupsTest.kt} (59%) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/filter/CalendarFilterList.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/filter/CalendarFilterList.kt index 7b292b9..68191a8 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/filter/CalendarFilterList.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/filter/CalendarFilterList.kt @@ -20,9 +20,10 @@ import androidx.hilt.navigation.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import de.jeanlucmakiola.calendula.R import de.jeanlucmakiola.calendula.domain.FailureReason +import de.jeanlucmakiola.calendula.ui.common.CalendarAccountGroup import de.jeanlucmakiola.calendula.ui.common.CalendarColorChip +import de.jeanlucmakiola.calendula.ui.common.accountGroupTitle import de.jeanlucmakiola.calendula.ui.common.orUnnamed -import de.jeanlucmakiola.calendula.ui.common.sourceAppName import de.jeanlucmakiola.floret.components.GroupedListInset import de.jeanlucmakiola.floret.components.GroupedRow import de.jeanlucmakiola.floret.components.positionOf @@ -44,6 +45,7 @@ fun CalendarFilterList( is FilterUiState.Failure -> FilterMessage(s.reason, modifier) is FilterUiState.Success -> FilterList( groups = s.groups, + hidden = s.hidden, onSetVisible = viewModel::setVisible, modifier = modifier, ) @@ -57,23 +59,15 @@ fun CalendarFilterList( */ @Composable private fun FilterList( - groups: List, + groups: List, + hidden: Set, onSetVisible: (Long, Boolean) -> Unit, modifier: Modifier = Modifier, ) { Column(modifier = modifier.fillMaxWidth()) { groups.forEach { group -> - val account = group.account.orUnnamed() Text( - text = if (group.ambiguous) { - stringResource( - R.string.calendars_account_from_source, - account, - sourceAppName(group.accountType), - ) - } else { - account - }, + text = accountGroupTitle(group), style = MaterialTheme.typography.labelMedium, color = MaterialTheme.colorScheme.onSurfaceVariant, modifier = Modifier.padding( @@ -84,6 +78,7 @@ private fun FilterList( ), ) group.calendars.forEachIndexed { index, cal -> + val visible = cal.id !in hidden GroupedRow( title = cal.displayName.orUnnamed(), position = positionOf(index, group.calendars.size), @@ -91,11 +86,11 @@ private fun FilterList( leading = { CalendarColorChip(cal.color) }, trailing = { Checkbox( - checked = cal.visible, + checked = visible, onCheckedChange = { onSetVisible(cal.id, it) }, ) }, - onClick = { onSetVisible(cal.id, !cal.visible) }, + onClick = { onSetVisible(cal.id, !visible) }, ) } } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/filter/FilterUiState.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/filter/FilterUiState.kt index ac693f5..765d9ed 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/filter/FilterUiState.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/filter/FilterUiState.kt @@ -1,6 +1,7 @@ package de.jeanlucmakiola.calendula.ui.filter import de.jeanlucmakiola.calendula.domain.FailureReason +import de.jeanlucmakiola.calendula.ui.common.CalendarAccountGroup /** * State for the calendar-filter sheet (M3). The user toggles per-calendar @@ -10,26 +11,9 @@ import de.jeanlucmakiola.calendula.domain.FailureReason sealed interface FilterUiState { data object Loading : FilterUiState data class Failure(val reason: FailureReason) : FilterUiState - data class Success(val groups: List) : FilterUiState + /** A calendar shows when its id is *not* in [hidden]. */ + data class Success( + val groups: List, + val hidden: Set, + ) : FilterUiState } - -/** - * Calendars grouped under the account that owns them (Nextcloud / Local / …). - * - * [accountType] and [ambiguous] carry what the header needs to tell two - * same-named accounts from different apps apart (#77); the label itself is - * built in the UI layer, which is where the source app's name can be looked up. - */ -data class AccountGroup( - val account: String, - val accountType: String, - val ambiguous: Boolean, - val calendars: List, -) - -data class CalendarRow( - val id: Long, - val displayName: String, - val color: Int, - val visible: Boolean, -) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/filter/FilterViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/filter/FilterViewModel.kt index 5d615c4..fda4c1f 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/filter/FilterViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/filter/FilterViewModel.kt @@ -5,7 +5,6 @@ import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel import de.jeanlucmakiola.calendula.data.calendar.CalendarRepository import de.jeanlucmakiola.calendula.data.prefs.CalendarPrefs -import de.jeanlucmakiola.calendula.domain.CalendarSource import de.jeanlucmakiola.calendula.domain.FailureReason import de.jeanlucmakiola.calendula.domain.calendarListFailure import de.jeanlucmakiola.calendula.ui.common.groupByAccount @@ -39,7 +38,7 @@ class FilterViewModel @Inject constructor( if (failure != null) { FilterUiState.Failure(failure) } else { - FilterUiState.Success(groupCalendarsForFilter(enabled, hidden)) + FilterUiState.Success(enabled.groupByAccount(), hidden) } } .catch { emit(FilterUiState.Failure(FailureReason.ProviderUnavailable)) } @@ -59,28 +58,3 @@ class FilterViewModel @Inject constructor( } } -/** - * Group calendars under their owning account — by name *and* type, so two - * accounts that merely share a name stay apart (#77) — preserving the - * provider's order within each group and ordering groups by first appearance. - * A calendar is "visible" when its id is *not* in [hidden]. - */ -internal fun groupCalendarsForFilter( - calendars: List, - hidden: Set, -): List = - calendars.groupByAccount().map { group -> - AccountGroup( - account = group.label, - accountType = group.accountType, - ambiguous = group.ambiguous, - calendars = group.calendars.map { c -> - CalendarRow( - id = c.id, - displayName = c.displayName, - color = c.color, - visible = c.id !in hidden, - ) - }, - ) - } diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/ui/filter/FilterGroupingTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/AccountGroupsTest.kt similarity index 59% rename from app/src/test/java/de/jeanlucmakiola/calendula/ui/filter/FilterGroupingTest.kt rename to app/src/test/java/de/jeanlucmakiola/calendula/ui/common/AccountGroupsTest.kt index 7900f72..60a4500 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/ui/filter/FilterGroupingTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/AccountGroupsTest.kt @@ -1,10 +1,10 @@ -package de.jeanlucmakiola.calendula.ui.filter +package de.jeanlucmakiola.calendula.ui.common import com.google.common.truth.Truth.assertThat import de.jeanlucmakiola.calendula.domain.CalendarSource import org.junit.jupiter.api.Test -class FilterGroupingTest { +class AccountGroupsTest { private fun cal( id: Long, @@ -28,35 +28,18 @@ class FilterGroupingTest { cal(3, "Shared", "team@dav"), ) - val groups = groupCalendarsForFilter(calendars, hidden = emptySet()) + val groups = calendars.groupByAccount() - assertThat(groups.map { it.account }).containsExactly("alice@dav", "team@dav").inOrder() + assertThat(groups.map { it.label }).containsExactly("alice@dav", "team@dav").inOrder() assertThat(groups[0].calendars.map { it.displayName }) .containsExactly("Personal", "Work").inOrder() assertThat(groups[1].calendars.map { it.id }).containsExactly(3L) } - @Test - fun `hidden ids mark calendars not visible`() { - val calendars = listOf( - cal(1, "Personal", "alice@dav"), - cal(2, "Work", "alice@dav"), - ) - - val groups = groupCalendarsForFilter(calendars, hidden = setOf(2L)) - val rows = groups.single().calendars.associateBy { it.id } - - assertThat(rows.getValue(1L).visible).isTrue() - assertThat(rows.getValue(2L).visible).isFalse() - } - @Test fun `blank account name falls back to type`() { - val groups = groupCalendarsForFilter( - listOf(cal(1, "Birthdays", account = "", type = "LOCAL")), - hidden = emptySet(), - ) - assertThat(groups.single().account).isEqualTo("LOCAL") + val groups = listOf(cal(1, "Birthdays", account = "", type = "LOCAL")).groupByAccount() + assertThat(groups.single().label).isEqualTo("LOCAL") } /** #77: same address, two apps — two accounts, and they must read as two. */ @@ -67,7 +50,7 @@ class FilterGroupingTest { cal(2, "Shared", "me@example.com", type = "bitfire.at.davdroid"), ) - val groups = groupCalendarsForFilter(calendars, hidden = emptySet()) + val groups = calendars.groupByAccount() assertThat(groups).hasSize(2) assertThat(groups.map { it.accountType }) @@ -78,10 +61,7 @@ class FilterGroupingTest { @Test fun `one name under one type is not ambiguous`() { - val groups = groupCalendarsForFilter( - listOf(cal(1, "Personal", "me@example.com")), - hidden = emptySet(), - ) + val groups = listOf(cal(1, "Personal", "me@example.com")).groupByAccount() assertThat(groups.single().ambiguous).isFalse() } @@ -89,13 +69,10 @@ class FilterGroupingTest { /** Two different names from the same app are still two plain groups. */ @Test fun `different names under one type are not ambiguous`() { - val groups = groupCalendarsForFilter( - listOf( - cal(1, "Personal", "alice@example.com"), - cal(2, "Shared", "bob@example.com"), - ), - hidden = emptySet(), - ) + val groups = listOf( + cal(1, "Personal", "alice@example.com"), + cal(2, "Shared", "bob@example.com"), + ).groupByAccount() assertThat(groups).hasSize(2) assertThat(groups.none { it.ambiguous }).isTrue()