diff --git a/CHANGELOG.md b/CHANGELOG.md index f54a13e..17978a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,8 +14,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Calendars your device isn't syncing are marked **Not synced**, moved to the bottom of their account and left without a switch. None of their events are on the device, so the switch they used to have could not have shown you anything - — the calendar simply looked broken. Whether an account syncs a calendar stays - that account's own app's decision ([#78]). + — the calendar simply looked broken. They are no longer offered when you pick + a calendar for a new or an imported event either: an event saved there would + never reach the account. Whether an account syncs a calendar stays that + account's own app's decision ([#78]). - The birthday and anniversary calendars Calendula fills from your contacts are marked **Filled from your contacts**, which is why they can't be picked for a new event: anything you put there would be removed again on the next sync. diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/domain/CalendarRowState.kt b/app/src/main/java/de/jeanlucmakiola/calendula/domain/CalendarRowState.kt index a90912b..0272370 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/domain/CalendarRowState.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/domain/CalendarRowState.kt @@ -41,6 +41,25 @@ val CalendarSource.isNotSynced: Boolean val CalendarSource.hasVisibilitySwitch: Boolean get() = !isNotSynced +/** + * Whether this calendar can be offered as a target for a new or imported event. + * The one predicate behind both pickers, so the states [CalendarStateLabel] + * names on a manager row are exactly the states that keep a calendar out of + * them (#76): + * + * - read-only has nowhere to write; + * - switched off would hide the event the moment it was saved; + * - a managed mirror has the next contact sync delete it; + * - a non-syncing one never carries the event up to the account, and + * `CalendarProvider2` wipes the calendar's rows outright when the + * subscription is switched back on — a saved event is a dead end either way. + * + * This is the test for *targets*. An event already living in an excluded + * calendar keeps it; the editor adds that calendar back to its picker. + */ +val CalendarSource.isEventTarget: Boolean + get() = canModifyContents && isVisibleInSystem && !isManaged && !isNotSynced + /** Every state worth naming on this calendar's row, in reading order. */ fun CalendarSource.stateLabels(): List = buildList { if (isManaged) add(CalendarStateLabel.MANAGED) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsScreen.kt index b755318..b940c03 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsScreen.kt @@ -94,6 +94,7 @@ import de.jeanlucmakiola.calendula.data.prefs.SettingsPrefs import de.jeanlucmakiola.calendula.domain.CalendarSource import de.jeanlucmakiola.calendula.domain.CalendarStateLabel import de.jeanlucmakiola.calendula.domain.hasVisibilitySwitch +import de.jeanlucmakiola.calendula.domain.isEventTarget import de.jeanlucmakiola.calendula.domain.isNotSynced import de.jeanlucmakiola.calendula.domain.orderedForManager import de.jeanlucmakiola.calendula.domain.stateLabels @@ -335,9 +336,9 @@ private fun CalendarsList( // safety net. Offered only when there is something exportable: the user's // own local calendars (managed special-dates mirrors don't count). val exportable = local.filter { it.canModifyContents && !it.isManaged } - // Restore/import can target any writable, non-managed calendar (local or - // synced), so its availability is broader than export's. - val canImport = (local + synced).any { it.canModifyContents && !it.isManaged } + // Restore/import can target any calendar the import picker would offer + // (local or synced), so its availability is broader than export's. + val canImport = (local + synced).any { it.isEventTarget } if (exportable.isNotEmpty()) { Spacer(Modifier.height(16.dp)) SectionHeader(stringResource(R.string.calendars_backup_header)) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModel.kt index 42cee09..d5284ec 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModel.kt @@ -19,6 +19,7 @@ import de.jeanlucmakiola.calendula.domain.EventColorOption import de.jeanlucmakiola.calendula.domain.EventForm import de.jeanlucmakiola.calendula.domain.EventFormField import de.jeanlucmakiola.calendula.domain.RecurringWriteScope +import de.jeanlucmakiola.calendula.domain.isEventTarget import de.jeanlucmakiola.calendula.domain.populatedFields import de.jeanlucmakiola.calendula.domain.problems import de.jeanlucmakiola.calendula.domain.toEditSnapshot @@ -177,19 +178,16 @@ class EventEditViewModel @Inject constructor( repository.calendars().catch { emit(emptyList()) } /** - * Writable calendars — the only valid event targets. Calendars switched off - * in Settings → Calendars are excluded, so you can't create into one you've - * turned off; a last-used preselect landing on a now-off calendar falls back - * to the first remaining writable one (handled by [resolvedCalendarId] and - * [state]). Managed special-dates calendars are excluded too: their events - * are owned by the contact sync, which would delete any user event created - * there. + * The calendars a new event can be saved to ([isEventTarget]): writable, + * switched on, not a contact-filled mirror, not a non-syncing subscription. + * A last-used preselect landing on an excluded calendar falls back to the + * first remaining one (handled by [resolvedCalendarId] and [state]). * * This is the list of *targets*. An event already living in an excluded * calendar keeps it — [state] adds it back to the picker. */ private val writableCalendars: Flow> = allCalendars.map { calendars -> - calendars.filter { it.canModifyContents && it.isVisibleInSystem && !it.isManaged } + calendars.filter { it.isEventTarget } } /** The target calendar id, resolved exactly as the form shows it. */ diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/imports/ImportViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/imports/ImportViewModel.kt index d7c536a..2e8e066 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/imports/ImportViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/imports/ImportViewModel.kt @@ -14,6 +14,7 @@ import de.jeanlucmakiola.calendula.domain.ics.IcsParseWarning import de.jeanlucmakiola.calendula.domain.ics.IcsParser import de.jeanlucmakiola.calendula.domain.ics.ParsedIcsEvent import de.jeanlucmakiola.calendula.domain.ics.toEventForm +import de.jeanlucmakiola.calendula.domain.isEventTarget import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow @@ -85,18 +86,14 @@ class ImportViewModel @Inject constructor( warnings = parsed.warnings, ) else -> { - // A calendar switched off in Settings → Calendars is off - // everywhere, so it can't be an import target — exclude it - // alongside the read-only ones. Managed special-dates - // calendars are contact-derived and editor-locked, so - // they're not a valid destination either. + // The same targets the event form offers ([isEventTarget]): + // an import is a bulk create, so a calendar that can't hold + // one event can't hold thirty. ImportUiState.Many( events = parsed.events, warnings = parsed.warnings, calendars = repository.calendars().first() - .filter { - it.canModifyContents && !it.isManaged && it.isVisibleInSystem - }, + .filter { it.isEventTarget }, ) } } diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/domain/CalendarRowStateTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/domain/CalendarRowStateTest.kt index 92ed263..376bcd0 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/domain/CalendarRowStateTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/domain/CalendarRowStateTest.kt @@ -70,6 +70,30 @@ class CalendarRowStateTest { assertThat(calendar.hasVisibilitySwitch).isTrue() } + @Test + fun `every named state keeps a calendar out of the pickers`() { + // The labels and the picker exclusion are the same set, stated twice — + // a labelled row the pickers still offered would make the footer's + // "manage your calendars to see why" a lie (#76). + assertThat(cal().isEventTarget).isTrue() + listOf( + cal(writable = false), + cal(syncsEvents = false), + cal(local = true, managed = true), + ).forEach { calendar -> + assertThat(calendar.stateLabels()).isNotEmpty() + assertThat(calendar.isEventTarget).isFalse() + } + } + + @Test + fun `a switched-off calendar is no target although it carries no label`() { + // The switch is right there on the row, so the state speaks for itself. + val calendar = cal().copy(isVisibleInSystem = false) + assertThat(calendar.stateLabels()).isEmpty() + assertThat(calendar.isEventTarget).isFalse() + } + @Test fun `manager order puts non-syncing calendars last and is otherwise stable`() { val ordered = listOf( diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModelTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModelTest.kt index 184be1b..e087fbe 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModelTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/ui/edit/EventEditViewModelTest.kt @@ -45,9 +45,14 @@ class EventEditViewModelTest { private val beginMillis = 1_781_164_800_000L private val endMillis = beginMillis + 3_600_000L - private fun cal(id: Long, visible: Boolean = true): CalendarSource = CalendarSource( + private fun cal( + id: Long, + visible: Boolean = true, + syncsEvents: Boolean = true, + ): CalendarSource = CalendarSource( id = id, displayName = "Cal $id", accountName = "acc@local", accountType = "LOCAL", color = 0xFF112233.toInt(), isVisibleInSystem = visible, canModifyContents = true, + syncsEvents = syncsEvents, ) private fun detail(calendarId: Long, rrule: String? = null): EventDetail = EventDetail( @@ -108,6 +113,27 @@ class EventEditViewModelTest { job.cancel() } + @Test + fun `a calendar whose account is not synced to this device is not a target`( + @TempDir tempDir: Path, + ) = runTest(dispatcher) { + // Writable and switched on, but the account keeps its events off the + // device: nothing saved here ever reaches it, and the provider drops the + // rows when the subscription comes back (#76). + val fake = FakeCalendarDataSource().apply { + calendarsResult = listOf(cal(1L), cal(2L, syncsEvents = false)) + eventDetailResult = { detail(calendarId = 1L) } + } + val vm = viewModel(tempDir, fake) + val job = activate(vm) + + vm.openForEdit(eventId = 42L, beginMillis = beginMillis, endMillis = endMillis) + advanceUntilIdle() + + assertThat(vm.state.value?.calendars?.map { it.id }).containsExactly(1L) + job.cancel() + } + @Test fun `editing an event in a switched-off calendar keeps it in the picker`( @TempDir tempDir: Path,