fix(#429): keep the default across renames and restores, drop a duplicate's colour off its calendar

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>): 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<CalendarSource>,
-): 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>): 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>): CalendarSource? =
-    default?.let { ref -> calendars.firstOrNull { ref.matches(it) && !it.isEventTarget } }
+    default?.find(calendars)?.takeIf { it.isEventTarget }

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

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<CalendarSource>) = SettingsUiState(
+    private fun Preferences.toUiState(
+        calendars: List<CalendarSource>,
+        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(),
     )

app/src/main/res/values/strings.xml

@@ -707,8 +707,8 @@
     <string name="settings_form_fields_hint">Fields shown by default — everything else sits behind \"More fields\"</string>
     <string name="settings_default_calendar">Default calendar</string>
     <string name="settings_default_calendar_automatic">Automatic</string>
-    <string name="settings_default_calendar_unavailable">Automatic while %1$s is switched off or read-only</string>
-    <string name="settings_default_calendar_automatic_hint">The calendar you last added an event to</string>
+    <string name="settings_default_calendar_unavailable">Automatic while %1$s can’t take new events</string>
+    <string name="settings_default_calendar_automatic_hint">The calendar you last saved an event to</string>
     <string name="settings_autofocus_title">Focus title on new event</string>
     <string name="settings_autofocus_title_hint">When you start a new event, place the cursor in the title field and open the keyboard right away.</string>
     <string name="settings_event_duration">Default duration</string>

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

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()
+    }
 }