From 6e1bbf8ec47739faec5d59e42e7201512a197ecc Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Fri, 13 Nov 2026 19:47:29 +0100 Subject: [PATCH] fix(#429): drop a requested calendar's colour in one place, pin the shown calendar on colour picks --- .../calendula/domain/EventForm.kt | 23 ++++------- .../ui/common/EventActionsViewModel.kt | 4 +- .../calendula/ui/detail/EventDetailUiState.kt | 2 - .../ui/detail/EventDetailViewModel.kt | 4 +- .../calendula/ui/edit/EventEditViewModel.kt | 41 +++++++++++++++---- .../ui/settings/SettingsViewModel.kt | 17 ++++---- .../calendula/domain/EventFormTest.kt | 17 +++----- .../ui/edit/EventEditViewModelTest.kt | 26 ++++++++++++ 8 files changed, 86 insertions(+), 48 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/domain/EventForm.kt b/app/src/main/java/de/jeanlucmakiola/calendula/domain/EventForm.kt index 1f940253..0ba1fd22 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/domain/EventForm.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/domain/EventForm.kt @@ -320,22 +320,17 @@ fun EventForm.problems(deviceZone: TimeZone): Set = buildSet { /** * A create form seeded from this occurrence, to save a copy as a new, * independent event (#52). The recurrence is dropped — a duplicate is a single - * event. The source calendar is kept only when [keepCalendar] (it can take new - * events), otherwise the id is cleared so the create resolves like a new event, - * which lets a read-only event be copied into an editable one; a palette colour - * is account-scoped, so it goes with the calendar. [instance]'s own times carry over. + * event. The source calendar is only requested: the editor keeps it while it can + * take new events and otherwise resolves the calendar like a new event, which + * lets a read-only event be copied into an editable one. [instance]'s own times + * carry over. */ -fun EventDetail.toDuplicateForm(keepCalendar: Boolean, zone: TimeZone): EventForm = +fun EventDetail.toDuplicateForm(zone: TimeZone): EventForm = toEditForm( beginMillis = instance.start.toEpochMilliseconds(), endMillis = instance.end.toEpochMilliseconds(), zone = zone, - ).let { form -> - form.copy( - rrule = null, - calendarId = instance.calendarId.takeIf { keepCalendar }, - colorKey = form.colorKey.takeIf { keepCalendar }, - color = form.color.takeIf { keepCalendar || form.colorKey == null }, - status = status.forNewEvent(), - ) - } + ).copy( + rrule = null, + status = status.forNewEvent(), + ) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventActionsViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventActionsViewModel.kt index 5c0930ec..c61b8225 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventActionsViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventActionsViewModel.kt @@ -12,7 +12,6 @@ import de.jeanlucmakiola.calendula.domain.CalendarSource import de.jeanlucmakiola.calendula.domain.EventDetail import de.jeanlucmakiola.calendula.domain.EventForm import de.jeanlucmakiola.calendula.domain.EventInstance -import de.jeanlucmakiola.calendula.domain.isEventTarget import de.jeanlucmakiola.calendula.domain.RecurringWriteScope import de.jeanlucmakiola.calendula.domain.atOccurrence import de.jeanlucmakiola.calendula.domain.toDuplicateForm @@ -65,8 +64,7 @@ class EventActionsViewModel @Inject constructor( /** A create form copying [event], or null when it can't be read. */ suspend fun duplicateForm(event: EventInstance): EventForm? { val detail = load(event) ?: return null - val isTarget = calendars.value[event.calendarId]?.isEventTarget == true - return detail.toDuplicateForm(isTarget, zoneSource.current()) + return detail.toDuplicateForm(zoneSource.current()) } /** A staged `.ics` for [event], or null — reported — when it can't be built. */ diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/detail/EventDetailUiState.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/detail/EventDetailUiState.kt index f08bfe19..18c278cf 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/detail/EventDetailUiState.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/detail/EventDetailUiState.kt @@ -16,8 +16,6 @@ sealed interface EventDetailUiState { val calendarName: String?, /** Whether the owning calendar allows modifying events (shows edit/delete). */ val canModify: Boolean = false, - /** Whether the owning calendar takes new events, so a duplicate can stay in it. */ - val isEventTarget: Boolean = false, /** * The event belongs to a managed special-dates calendar: its editable * fields (reminders, notes) can still be edited, but delete is hidden — diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/detail/EventDetailViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/detail/EventDetailViewModel.kt index 6b571828..f8baba32 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/detail/EventDetailViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/detail/EventDetailViewModel.kt @@ -13,7 +13,6 @@ import de.jeanlucmakiola.calendula.domain.FailureReason import de.jeanlucmakiola.calendula.domain.RecurringWriteScope import de.jeanlucmakiola.calendula.domain.atOccurrence import de.jeanlucmakiola.calendula.domain.descriptionRichText -import de.jeanlucmakiola.calendula.domain.isEventTarget import de.jeanlucmakiola.calendula.domain.toDuplicateForm import de.jeanlucmakiola.calendula.ui.common.DeleteOutcome import de.jeanlucmakiola.calendula.ui.common.tryDeleteEvent @@ -143,7 +142,7 @@ class EventDetailViewModel @Inject constructor( */ fun duplicateForm(): EventForm? { val loaded = state.value as? EventDetailUiState.Success ?: return null - return loaded.detail.toDuplicateForm(loaded.isEventTarget, zoneSource.current()) + return loaded.detail.toDuplicateForm(zoneSource.current()) } private suspend fun loadDetail(target: Target): EventDetailUiState = try { @@ -165,7 +164,6 @@ class EventDetailViewModel @Inject constructor( detail = corrected, calendarName = calendar?.displayName, canModify = calendar?.canModifyContents == true, - isEventTarget = calendar?.isEventTarget == true, isManaged = calendar?.isManaged == true, // Found here, off the main thread and once per load: the platform // matcher loads its metadata on first use. 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 57ee7de3..5f6e6ae5 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 @@ -306,7 +306,13 @@ class EventEditViewModel @Inject constructor( ) { local, external, palette, allCalendars, recentTimeZones -> val form = local.form ?: return@combine null val resolvedId = resolveEventCalendar(form.calendarId, external.hints, allCalendars) - val resolved = form.copy(calendarId = resolvedId) + // A colour is scoped to the calendar it came from, so one that came with + // a requested calendar the form couldn't take goes, as on [setCalendar]. + val requestRefused = form.calendarId == null && + external.hints.requested.let { it != null && it != resolvedId } + val resolved = form.copy(calendarId = resolvedId).let { + if (requestRefused) it.copy(colorKey = null, color = null) else it + } val resolvedCalendar = allCalendars.firstOrNull { it.id == resolvedId } // Editing an event in a managed calendar locks its synced fields. Keyed // off the calendar's durable marker, not a stored id, so it holds after a @@ -677,9 +683,11 @@ class EventEditViewModel @Inject constructor( * The event falls back to the new calendar's colour until re-picked. */ fun setCalendar(id: Long) { - // Re-picking the calendar the form already shows changes nothing, so it - // keeps a duplicate's colour and raises no unsaved change. - if (id == state.value?.form?.calendarId) return + // Confirming the calendar the form shows pins it and changes nothing else. + if (id == state.value?.form?.calendarId) { + pinShownCalendar() + return + } edit { it.copy(calendarId = id, colorKey = null, color = null) } // A fresh event re-inherits the new calendar's default reminder and // length unless the user has already hand-edited them (guarded inside). @@ -691,13 +699,32 @@ class EventEditViewModel @Inject constructor( fun setAccessLevel(value: AccessLevel) = edit { it.copy(accessLevel = value) } /** Pick a palette colour (round-trips via its key); [argb] is its swatch. */ - fun setColorKey(key: String, argb: Int) = edit { it.copy(colorKey = key, color = argb) } + fun setColorKey(key: String, argb: Int) { + pinShownCalendar() + edit { it.copy(colorKey = key, color = argb) } + } /** Pick a raw colour (local / opted-in calendars), clearing any palette key. */ - fun setColorRaw(argb: Int) = edit { it.copy(colorKey = null, color = argb) } + fun setColorRaw(argb: Int) { + pinShownCalendar() + edit { it.copy(colorKey = null, color = argb) } + } /** Clear the colour so the event inherits its calendar's. */ - fun clearColor() = edit { it.copy(colorKey = null, color = null) } + fun clearColor() { + pinShownCalendar() + edit { it.copy(colorKey = null, color = null) } + } + + /** + * Fix a still-resolving form to the calendar and colour it shows, so a colour + * picked for that calendar stays with it. Not a change of its own. + */ + private fun pinShownCalendar() { + val shown = state.value?.form ?: return + if (_form.value?.calendarId != null) return + applyDefault { it.copy(calendarId = shown.calendarId, colorKey = shown.colorKey, color = shown.color) } + } /** Bare RRULE value from the recurrence picker; null = does not repeat. */ fun setRecurrence(rrule: String?) = edit { it.copy(rrule = rrule) } 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 08021713..0e58158f 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 @@ -36,9 +36,8 @@ import de.jeanlucmakiola.calendula.data.prefs.WeekStartPref import de.jeanlucmakiola.calendula.data.reminders.ReminderScanner 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.FontRole import de.jeanlucmakiola.calendula.domain.isEventTarget import de.jeanlucmakiola.calendula.domain.contacts.SpecialDateType import de.jeanlucmakiola.calendula.ui.agenda.AgendaRange @@ -91,10 +90,12 @@ class SettingsViewModel @Inject constructor( initialValue = SettingsUiState(dynamicColorAvailable = dynamicColorAvailable), ) - private fun Preferences.toUiState( - calendars: List, - storedDefault: DefaultCalendarRef? = this[SettingsKeys.defaultNewEventCalendar], - ) = SettingsUiState( + private fun Preferences.toUiState(calendars: List): SettingsUiState { + val defaultCalendar = this[SettingsKeys.defaultNewEventCalendar]?.find(calendars) + return toUiState(calendars, defaultCalendar) + } + + private fun Preferences.toUiState(calendars: List, defaultCalendar: CalendarSource?) = SettingsUiState( themeMode = this[SettingsKeys.themeMode], dynamicColor = this[SettingsKeys.dynamicColor] && dynamicColorAvailable, dynamicColorAvailable = dynamicColorAvailable, @@ -132,8 +133,8 @@ class SettingsViewModel @Inject constructor( perCalendarAllDayReminderOverride = this[SettingsKeys.perCalendarAllDayReminderOverride], writableCalendars = calendars.filter { it.canModifyContents && it.isVisibleInSystem }, eventTargetCalendars = calendars.filter { it.isEventTarget }, - defaultNewEventCalendar = effectiveDefaultCalendar(storedDefault, calendars), - unavailableDefaultCalendar = storedDefault?.find(calendars)?.takeUnless { it.isEventTarget }, + defaultNewEventCalendar = defaultCalendar?.takeIf { it.isEventTarget }, + unavailableDefaultCalendar = defaultCalendar?.takeUnless { it.isEventTarget }, allowColorOnUnsupportedCalendars = this[SettingsKeys.allowColorOnUnsupportedCalendars], managedCalendarIds = specialDatesCalendars.values.toSet(), ) diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/domain/EventFormTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/domain/EventFormTest.kt index 484ae16c..6ee95aa2 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/domain/EventFormTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/domain/EventFormTest.kt @@ -583,21 +583,16 @@ class EventFormTest { @Test fun `a copy of a cancelled event starts confirmed, a tentative one stays tentative`() { val zone = TimeZone.of("Europe/Berlin") - assertThat(testDetail(status = EventStatus.Cancelled).toDuplicateForm(keepCalendar = true, zone).status) + assertThat(testDetail(status = EventStatus.Cancelled).toDuplicateForm(zone).status) .isEqualTo(EventStatus.Confirmed) - assertThat(testDetail(status = EventStatus.Tentative).toDuplicateForm(keepCalendar = true, zone).status) + assertThat(testDetail(status = EventStatus.Tentative).toDuplicateForm(zone).status) .isEqualTo(EventStatus.Tentative) } @Test - fun `a copy leaving its calendar drops a palette colour but keeps a raw one`() { - val zone = TimeZone.of("Europe/Berlin") - val keyed = testDetail(eventColor = 7, eventColorKey = "3") - assertThat(keyed.toDuplicateForm(keepCalendar = true, zone).colorKey).isEqualTo("3") - val moved = keyed.toDuplicateForm(keepCalendar = false, zone) - assertThat(moved.calendarId).isNull() - assertThat(moved.colorKey).isNull() - assertThat(moved.color).isNull() - assertThat(testDetail(eventColor = 7).toDuplicateForm(keepCalendar = false, zone).color).isEqualTo(7) + fun `a copy requests its source calendar and keeps its colour`() { + val form = testDetail(eventColor = 7, eventColorKey = "3").toDuplicateForm(TimeZone.of("Europe/Berlin")) + assertThat(form.calendarId).isEqualTo(testEvent().calendarId) + assertThat(form.colorKey).isEqualTo("3") } } 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 b9bd71ce..342b6816 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 @@ -841,4 +841,30 @@ class EventEditViewModelTest { assertThat(vm.hasUnsavedChanges.value).isFalse() job.cancel() } + + @Test + fun `a duplicate that can't stay in its 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() + + vm.setColorRaw(9) + advanceUntilIdle() + + assertThat(vm.state.value?.form?.calendarId).isEqualTo(1L) + assertThat(vm.state.value?.form?.color).isEqualTo(9) + job.cancel() + } }