fix(pickers): keep non-syncing calendars out of the event targets (#78)
The new row states were documented as the reasons a calendar is missing from the event and import pickers, but only three of the four were: a calendar whose account keeps its events off the device is dimmed, switchless and marked "Not synced" in the manager, and was still offered as a place to save a new or imported event. Nothing there ever reaches the account, and CalendarProvider2 wipes the calendar's rows outright when the subscription is switched back on — so the event is a dead end, saved into a calendar that also can't show it. Both pickers now filter on one predicate, isEventTarget, which is the label set restated: what the manager marks is what the picker leaves out, which is what its "Missing a calendar?" footer promises. The manager's restore-from-backup row uses it too, so the entry point and the picker behind it agree on whether there is anywhere to import to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
- 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
|
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 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
|
— the calendar simply looked broken. They are no longer offered when you pick
|
||||||
that account's own app's decision ([#78]).
|
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
|
- 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
|
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.
|
new event: anything you put there would be removed again on the next sync.
|
||||||
|
|||||||
@@ -41,6 +41,25 @@ val CalendarSource.isNotSynced: Boolean
|
|||||||
val CalendarSource.hasVisibilitySwitch: Boolean
|
val CalendarSource.hasVisibilitySwitch: Boolean
|
||||||
get() = !isNotSynced
|
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. */
|
/** Every state worth naming on this calendar's row, in reading order. */
|
||||||
fun CalendarSource.stateLabels(): List<CalendarStateLabel> = buildList {
|
fun CalendarSource.stateLabels(): List<CalendarStateLabel> = buildList {
|
||||||
if (isManaged) add(CalendarStateLabel.MANAGED)
|
if (isManaged) add(CalendarStateLabel.MANAGED)
|
||||||
|
|||||||
@@ -94,6 +94,7 @@ import de.jeanlucmakiola.calendula.data.prefs.SettingsPrefs
|
|||||||
import de.jeanlucmakiola.calendula.domain.CalendarSource
|
import de.jeanlucmakiola.calendula.domain.CalendarSource
|
||||||
import de.jeanlucmakiola.calendula.domain.CalendarStateLabel
|
import de.jeanlucmakiola.calendula.domain.CalendarStateLabel
|
||||||
import de.jeanlucmakiola.calendula.domain.hasVisibilitySwitch
|
import de.jeanlucmakiola.calendula.domain.hasVisibilitySwitch
|
||||||
|
import de.jeanlucmakiola.calendula.domain.isEventTarget
|
||||||
import de.jeanlucmakiola.calendula.domain.isNotSynced
|
import de.jeanlucmakiola.calendula.domain.isNotSynced
|
||||||
import de.jeanlucmakiola.calendula.domain.orderedForManager
|
import de.jeanlucmakiola.calendula.domain.orderedForManager
|
||||||
import de.jeanlucmakiola.calendula.domain.stateLabels
|
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
|
// safety net. Offered only when there is something exportable: the user's
|
||||||
// own local calendars (managed special-dates mirrors don't count).
|
// own local calendars (managed special-dates mirrors don't count).
|
||||||
val exportable = local.filter { it.canModifyContents && !it.isManaged }
|
val exportable = local.filter { it.canModifyContents && !it.isManaged }
|
||||||
// Restore/import can target any writable, non-managed calendar (local or
|
// Restore/import can target any calendar the import picker would offer
|
||||||
// synced), so its availability is broader than export's.
|
// (local or synced), so its availability is broader than export's.
|
||||||
val canImport = (local + synced).any { it.canModifyContents && !it.isManaged }
|
val canImport = (local + synced).any { it.isEventTarget }
|
||||||
if (exportable.isNotEmpty()) {
|
if (exportable.isNotEmpty()) {
|
||||||
Spacer(Modifier.height(16.dp))
|
Spacer(Modifier.height(16.dp))
|
||||||
SectionHeader(stringResource(R.string.calendars_backup_header))
|
SectionHeader(stringResource(R.string.calendars_backup_header))
|
||||||
|
|||||||
@@ -19,6 +19,7 @@ import de.jeanlucmakiola.calendula.domain.EventColorOption
|
|||||||
import de.jeanlucmakiola.calendula.domain.EventForm
|
import de.jeanlucmakiola.calendula.domain.EventForm
|
||||||
import de.jeanlucmakiola.calendula.domain.EventFormField
|
import de.jeanlucmakiola.calendula.domain.EventFormField
|
||||||
import de.jeanlucmakiola.calendula.domain.RecurringWriteScope
|
import de.jeanlucmakiola.calendula.domain.RecurringWriteScope
|
||||||
|
import de.jeanlucmakiola.calendula.domain.isEventTarget
|
||||||
import de.jeanlucmakiola.calendula.domain.populatedFields
|
import de.jeanlucmakiola.calendula.domain.populatedFields
|
||||||
import de.jeanlucmakiola.calendula.domain.problems
|
import de.jeanlucmakiola.calendula.domain.problems
|
||||||
import de.jeanlucmakiola.calendula.domain.toEditSnapshot
|
import de.jeanlucmakiola.calendula.domain.toEditSnapshot
|
||||||
@@ -177,19 +178,16 @@ class EventEditViewModel @Inject constructor(
|
|||||||
repository.calendars().catch { emit(emptyList()) }
|
repository.calendars().catch { emit(emptyList()) }
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Writable calendars — the only valid event targets. Calendars switched off
|
* The calendars a new event can be saved to ([isEventTarget]): writable,
|
||||||
* in Settings → Calendars are excluded, so you can't create into one you've
|
* switched on, not a contact-filled mirror, not a non-syncing subscription.
|
||||||
* turned off; a last-used preselect landing on a now-off calendar falls back
|
* A last-used preselect landing on an excluded calendar falls back to the
|
||||||
* to the first remaining writable one (handled by [resolvedCalendarId] and
|
* first remaining one (handled by [resolvedCalendarId] and [state]).
|
||||||
* [state]). Managed special-dates calendars are excluded too: their events
|
|
||||||
* are owned by the contact sync, which would delete any user event created
|
|
||||||
* there.
|
|
||||||
*
|
*
|
||||||
* This is the list of *targets*. An event already living in an excluded
|
* This is the list of *targets*. An event already living in an excluded
|
||||||
* calendar keeps it — [state] adds it back to the picker.
|
* calendar keeps it — [state] adds it back to the picker.
|
||||||
*/
|
*/
|
||||||
private val writableCalendars: Flow<List<CalendarSource>> = allCalendars.map { calendars ->
|
private val writableCalendars: Flow<List<CalendarSource>> = 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. */
|
/** The target calendar id, resolved exactly as the form shows it. */
|
||||||
|
|||||||
@@ -14,6 +14,7 @@ import de.jeanlucmakiola.calendula.domain.ics.IcsParseWarning
|
|||||||
import de.jeanlucmakiola.calendula.domain.ics.IcsParser
|
import de.jeanlucmakiola.calendula.domain.ics.IcsParser
|
||||||
import de.jeanlucmakiola.calendula.domain.ics.ParsedIcsEvent
|
import de.jeanlucmakiola.calendula.domain.ics.ParsedIcsEvent
|
||||||
import de.jeanlucmakiola.calendula.domain.ics.toEventForm
|
import de.jeanlucmakiola.calendula.domain.ics.toEventForm
|
||||||
|
import de.jeanlucmakiola.calendula.domain.isEventTarget
|
||||||
import kotlinx.coroutines.CoroutineDispatcher
|
import kotlinx.coroutines.CoroutineDispatcher
|
||||||
import kotlinx.coroutines.flow.MutableStateFlow
|
import kotlinx.coroutines.flow.MutableStateFlow
|
||||||
import kotlinx.coroutines.flow.StateFlow
|
import kotlinx.coroutines.flow.StateFlow
|
||||||
@@ -85,18 +86,14 @@ class ImportViewModel @Inject constructor(
|
|||||||
warnings = parsed.warnings,
|
warnings = parsed.warnings,
|
||||||
)
|
)
|
||||||
else -> {
|
else -> {
|
||||||
// A calendar switched off in Settings → Calendars is off
|
// The same targets the event form offers ([isEventTarget]):
|
||||||
// everywhere, so it can't be an import target — exclude it
|
// an import is a bulk create, so a calendar that can't hold
|
||||||
// alongside the read-only ones. Managed special-dates
|
// one event can't hold thirty.
|
||||||
// calendars are contact-derived and editor-locked, so
|
|
||||||
// they're not a valid destination either.
|
|
||||||
ImportUiState.Many(
|
ImportUiState.Many(
|
||||||
events = parsed.events,
|
events = parsed.events,
|
||||||
warnings = parsed.warnings,
|
warnings = parsed.warnings,
|
||||||
calendars = repository.calendars().first()
|
calendars = repository.calendars().first()
|
||||||
.filter {
|
.filter { it.isEventTarget },
|
||||||
it.canModifyContents && !it.isManaged && it.isVisibleInSystem
|
|
||||||
},
|
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -70,6 +70,30 @@ class CalendarRowStateTest {
|
|||||||
assertThat(calendar.hasVisibilitySwitch).isTrue()
|
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
|
@Test
|
||||||
fun `manager order puts non-syncing calendars last and is otherwise stable`() {
|
fun `manager order puts non-syncing calendars last and is otherwise stable`() {
|
||||||
val ordered = listOf(
|
val ordered = listOf(
|
||||||
|
|||||||
@@ -45,9 +45,14 @@ class EventEditViewModelTest {
|
|||||||
private val beginMillis = 1_781_164_800_000L
|
private val beginMillis = 1_781_164_800_000L
|
||||||
private val endMillis = beginMillis + 3_600_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",
|
id = id, displayName = "Cal $id", accountName = "acc@local", accountType = "LOCAL",
|
||||||
color = 0xFF112233.toInt(), isVisibleInSystem = visible, canModifyContents = true,
|
color = 0xFF112233.toInt(), isVisibleInSystem = visible, canModifyContents = true,
|
||||||
|
syncsEvents = syncsEvents,
|
||||||
)
|
)
|
||||||
|
|
||||||
private fun detail(calendarId: Long, rrule: String? = null): EventDetail = EventDetail(
|
private fun detail(calendarId: Long, rrule: String? = null): EventDetail = EventDetail(
|
||||||
@@ -108,6 +113,27 @@ class EventEditViewModelTest {
|
|||||||
job.cancel()
|
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
|
@Test
|
||||||
fun `editing an event in a switched-off calendar keeps it in the picker`(
|
fun `editing an event in a switched-off calendar keeps it in the picker`(
|
||||||
@TempDir tempDir: Path,
|
@TempDir tempDir: Path,
|
||||||
|
|||||||
Reference in New Issue
Block a user