diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/domain/NewEventCalendar.kt b/app/src/main/java/de/jeanlucmakiola/calendula/domain/NewEventCalendar.kt index ef1514d7..5e35ece8 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/domain/NewEventCalendar.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/domain/NewEventCalendar.kt @@ -2,9 +2,8 @@ package de.jeanlucmakiola.calendula.domain /** * The calendar chosen as default for new events (#429). Provider ids are - * device-local and can be handed out again, so the account and name ride along - * and the choice only holds while the calendar under [id] still matches them, - * e.g. not after a backup restore onto another device. + * device-local and get reassigned when an account is re-added or a backup is + * restored onto another device, so the account and name ride along: see [find]. */ data class DefaultCalendarRef( val id: Long, @@ -12,11 +11,16 @@ data class DefaultCalendarRef( val accountName: String, val displayName: String, ) { - fun matches(calendar: CalendarSource): Boolean = - calendar.id == id && - calendar.accountType == accountType && - calendar.accountName == accountName && - calendar.displayName == displayName + /** + * The calendar this refers to: the exact one, else the account's calendar of + * that name (the id changed), else the id within the account (it was renamed). + */ + fun find(calendars: List): CalendarSource? { + val inAccount = calendars.filter { it.accountType == accountType && it.accountName == accountName } + return inAccount.firstOrNull { it.id == id && it.displayName == displayName } + ?: inAccount.singleOrNull { it.displayName == displayName } + ?: inAccount.firstOrNull { it.id == id } + } fun encode(): String = listOf(id.toString(), accountType, accountName, displayName).joinToString(SEPARATOR) @@ -55,23 +59,18 @@ fun resolveEventCalendar( picked: Long?, hints: NewEventCalendarHints, calendars: List, -): Long? = picked - ?: hints.requested?.takeIf { id -> calendars.any { it.id == id && it.isEventTarget } } - ?: effectiveDefaultCalendar(hints.default, calendars)?.id - ?: hints.lastUsed?.takeIf { id -> calendars.any { it.id == id && it.isEventTarget } } - ?: calendars.firstOrNull { it.isEventTarget }?.id +): Long? { + fun Long.ifTarget() = takeIf { id -> calendars.any { it.id == id && it.isEventTarget } } + return picked + ?: hints.requested?.ifTarget() + ?: effectiveDefaultCalendar(hints.default, calendars)?.id + ?: hints.lastUsed?.ifTarget() + ?: calendars.firstOrNull { it.isEventTarget }?.id +} /** * The default calendar setting as it applies now: null ("Automatic") when unset, * or when the stored calendar is gone, read-only or switched off. */ fun effectiveDefaultCalendar(default: DefaultCalendarRef?, calendars: List): CalendarSource? = - default?.let { ref -> calendars.firstOrNull { ref.matches(it) && it.isEventTarget } } - -/** - * The stored default calendar while it still exists but can't take new events - * (switched off, read-only, managed), so the setting can say why it reads - * Automatic. Null when it applies, is unset or is gone. - */ -fun unavailableDefaultCalendar(default: DefaultCalendarRef?, calendars: List): CalendarSource? = - default?.let { ref -> calendars.firstOrNull { ref.matches(it) && !it.isEventTarget } } + default?.find(calendars)?.takeIf { it.isEventTarget } 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 6f1eb67a..b2161156 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 @@ -441,6 +441,7 @@ class EventEditViewModel @Inject constructor( _revealed.value = form.populatedFields() _requestedCalendarId.value = form.calendarId open(form.copy(calendarId = null)) + dropColorOffRequestedCalendar(form) if (source.ownsDuration) _durationTouched.value = true else applyDefaultDuration() when (source) { ImportSource.Insert, ImportSource.InsertOpenEnded -> @@ -455,6 +456,20 @@ class EventEditViewModel @Inject constructor( } } + /** + * A duplicate carries its source's colour, which is scoped to that calendar's + * account. When the form lands elsewhere, the colour goes as on [setCalendar]. + */ + private fun dropColorOffRequestedCalendar(form: EventForm) { + if (form.colorKey == null && form.color == null) return + val opened = session + viewModelScope.launch { + val resolved = resolvedCalendarId.first() + if (opened != session || _form.value?.calendarId != null) return@launch + if (resolved != form.calendarId) applyDefault { it.copy(colorKey = null, color = null) } + } + } + /** * Prefill a new event's reminders from the settings default — the all-day * default for all-day events, otherwise the resolved calendar's per-calendar diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsViewModel.kt index f19d4498..8c4fb122 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsViewModel.kt @@ -38,8 +38,6 @@ import de.jeanlucmakiola.calendula.domain.CalendarSource import de.jeanlucmakiola.calendula.domain.EventFormField import de.jeanlucmakiola.calendula.domain.FontRole import de.jeanlucmakiola.calendula.domain.DefaultCalendarRef -import de.jeanlucmakiola.calendula.domain.effectiveDefaultCalendar -import de.jeanlucmakiola.calendula.domain.unavailableDefaultCalendar import de.jeanlucmakiola.calendula.domain.isEventTarget import de.jeanlucmakiola.calendula.domain.contacts.SpecialDateType import de.jeanlucmakiola.calendula.ui.agenda.AgendaRange @@ -92,7 +90,10 @@ class SettingsViewModel @Inject constructor( initialValue = SettingsUiState(dynamicColorAvailable = dynamicColorAvailable), ) - private fun Preferences.toUiState(calendars: List) = SettingsUiState( + private fun Preferences.toUiState( + calendars: List, + defaultCalendar: CalendarSource? = this[SettingsKeys.defaultNewEventCalendar]?.find(calendars), + ) = SettingsUiState( themeMode = this[SettingsKeys.themeMode], dynamicColor = this[SettingsKeys.dynamicColor] && dynamicColorAvailable, dynamicColorAvailable = dynamicColorAvailable, @@ -130,8 +131,8 @@ class SettingsViewModel @Inject constructor( perCalendarAllDayReminderOverride = this[SettingsKeys.perCalendarAllDayReminderOverride], writableCalendars = calendars.filter { it.canModifyContents && it.isVisibleInSystem }, eventTargetCalendars = calendars.filter { it.isEventTarget }, - defaultNewEventCalendar = effectiveDefaultCalendar(this[SettingsKeys.defaultNewEventCalendar], calendars), - unavailableDefaultCalendar = unavailableDefaultCalendar(this[SettingsKeys.defaultNewEventCalendar], calendars), + defaultNewEventCalendar = defaultCalendar?.takeIf { it.isEventTarget }, + unavailableDefaultCalendar = defaultCalendar?.takeUnless { it.isEventTarget }, allowColorOnUnsupportedCalendars = this[SettingsKeys.allowColorOnUnsupportedCalendars], managedCalendarIds = specialDatesCalendars.values.toSet(), ) diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 6b652455..a27a457c 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -707,8 +707,8 @@ Fields shown by default — everything else sits behind \"More fields\" Default calendar Automatic - Automatic while %1$s is switched off or read-only - The calendar you last added an event to + Automatic while %1$s can’t take new events + The calendar you last saved an event to Focus title on new event When you start a new event, place the cursor in the title field and open the keyboard right away. Default duration diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/domain/NewEventCalendarTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/domain/NewEventCalendarTest.kt index 57a58c45..c2881ab0 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/domain/NewEventCalendarTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/domain/NewEventCalendarTest.kt @@ -65,10 +65,8 @@ class NewEventCalendarTest { val hints = NewEventCalendarHints(default = default, lastUsed = 2L) assertThat(resolveEventCalendar(null, hints, calendars)).isEqualTo(2L) assertThat(effectiveDefaultCalendar(default, calendars)).isNull() - assertThat(unavailableDefaultCalendar(default, calendars)?.id).isEqualTo(id) } assertThat(effectiveDefaultCalendar(ref(99L), calendars)).isNull() - assertThat(unavailableDefaultCalendar(ref(99L), calendars)).isNull() } @Test @@ -81,17 +79,16 @@ class NewEventCalendarTest { @Test fun `the effective default is the stored calendar while it is a target`() { assertThat(effectiveDefaultCalendar(ref(3L), calendars)?.id).isEqualTo(3L) - assertThat(unavailableDefaultCalendar(ref(3L), calendars)).isNull() assertThat(effectiveDefaultCalendar(null, calendars)).isNull() } @Test - fun `a reused id under another calendar is not the default`() { - val reused = ref(3L).copy(displayName = "Deleted calendar") - assertThat(effectiveDefaultCalendar(reused, calendars)).isNull() + fun `the default survives a rename and a new id, but not another account`() { + assertThat(effectiveDefaultCalendar(ref(3L).copy(displayName = "Old name"), calendars)?.id).isEqualTo(3L) + assertThat(effectiveDefaultCalendar(ref(3L).copy(id = 99L), calendars)?.id).isEqualTo(3L) + // The id was handed to another calendar of the account; the name decides. + assertThat(effectiveDefaultCalendar(ref(3L).copy(id = 2L), calendars)?.id).isEqualTo(3L) assertThat(effectiveDefaultCalendar(ref(3L).copy(accountName = "other"), calendars)).isNull() - assertThat(resolveEventCalendar(null, NewEventCalendarHints(default = reused, lastUsed = 2L), calendars)) - .isEqualTo(2L) } @Test 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 ba4268e9..7034b248 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 @@ -801,4 +801,44 @@ class EventEditViewModelTest { assertThat(vm.state.value?.form?.calendarId).isEqualTo(2L) job.cancel() } + + @Test + fun `a duplicate that falls back to another calendar drops its colour`( + @TempDir tempDir: Path, + ) = runTest(dispatcher) { + val fake = FakeCalendarDataSource().apply { calendarsResult = listOf(cal(1L), cal(2L, visible = false)) } + val vm = viewModel(tempDir, fake) + val job = activate(vm) + + vm.openImported( + importedForm(end = LocalDateTime(2030, 1, 15, 10, 0)).copy(calendarId = 2L, colorKey = "3", color = 7), + ImportSource.Insert, + ) + advanceUntilIdle() + + assertThat(vm.state.value?.form?.calendarId).isEqualTo(1L) + assertThat(vm.state.value?.form?.colorKey).isNull() + assertThat(vm.state.value?.form?.color).isNull() + assertThat(vm.hasUnsavedChanges.value).isFalse() + job.cancel() + } + + @Test + fun `a duplicate in its own calendar keeps its colour`( + @TempDir tempDir: Path, + ) = runTest(dispatcher) { + val fake = FakeCalendarDataSource().apply { calendarsResult = listOf(cal(1L), cal(2L)) } + val vm = viewModel(tempDir, fake) + val job = activate(vm) + + vm.openImported( + importedForm(end = LocalDateTime(2030, 1, 15, 10, 0)).copy(calendarId = 2L, colorKey = "3", color = 7), + ImportSource.Insert, + ) + advanceUntilIdle() + + assertThat(vm.state.value?.form?.calendarId).isEqualTo(2L) + assertThat(vm.state.value?.form?.colorKey).isEqualTo("3") + job.cancel() + } }