From 0121aa5b1dc7a52d451e08989fff6cf27619211c Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Wed, 14 Oct 2026 17:49:53 +0200 Subject: [PATCH] Keep the edit form's results from landing on the next one (#178) (#397) Follow-up to #394, from two review passes on the tap-opens-editor change. - The edit form's view model is reused between opens, so work still running for a closed form could land on the next one. Backing out of a slow load and tapping another event showed the old event, and Save wrote to it. A delete finishing after back closed the next editor. Results from a closed form are now dropped. - Back is ignored while a save or delete runs. The write can't be called back, so the form waits for the result and then closes or reports the error. - Unsaved changes are now measured against the form as it opened, defaults included, instead of a flag every setter had to set. - Taps read the setting and whether the calendar takes writes when they happen, so a tap right after a cold start or a return to the app opens the right screen. - Without write access, the first tap asks once. A refusal opens the details from then on, until the permission is granted. - Pinning an event to the tapped occurrence lives in one helper (`EventDetail.atOccurrence`), used by the detail, edit and selection-bar paths. Closes #178 Co-authored-by: Jean-Luc Makiola Reviewed-on: https://codeberg.org/jlmakiola/calendula/pulls/397 --- .../jeanlucmakiola/calendula/domain/Models.kt | 11 +++ .../calendula/ui/CalendarHost.kt | 26 +++-- .../calendula/ui/CalendarHostViewModel.kt | 10 +- .../calendula/ui/common/EventActions.kt | 11 ++- .../ui/common/EventActionsViewModel.kt | 18 +++- .../ui/detail/EventDetailViewModel.kt | 19 ++-- .../calendula/ui/edit/EventEditScreen.kt | 29 +++--- .../calendula/ui/edit/EventEditViewModel.kt | 99 +++++++++++-------- .../ui/edit/EventEditViewModelTest.kt | 78 ++++++++++++++- 9 files changed, 211 insertions(+), 90 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/domain/Models.kt b/app/src/main/java/de/jeanlucmakiola/calendula/domain/Models.kt index 89f086a..0dad2ed 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/domain/Models.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/domain/Models.kt @@ -165,6 +165,17 @@ data class EventDetail( val isException: Boolean = false, ) +/** + * This event on one occurrence's times. The Events row holds the series start, + * so a recurring event needs the tapped occurrence pinned to render and share right. + */ +fun EventDetail.atOccurrence(beginMillis: Long, endMillis: Long): EventDetail = copy( + instance = instance.copy( + start = Instant.fromEpochMilliseconds(beginMillis), + end = Instant.fromEpochMilliseconds(endMillis), + ), +) + /** * One selectable event colour published by a calendar's account * (`CalendarContract.Colors`, `TYPE_EVENT`): [key] is the account-scoped diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/CalendarHost.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/CalendarHost.kt index cf06ac1..b16611f 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/CalendarHost.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/CalendarHost.kt @@ -30,6 +30,7 @@ import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.saveable.listSaver import androidx.compose.runtime.saveable.rememberSaveable +import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.Modifier import de.jeanlucmakiola.calendula.ui.common.screen import de.jeanlucmakiola.calendula.ui.common.ViewScreen @@ -61,6 +62,7 @@ import de.jeanlucmakiola.calendula.ui.common.EventMoveHost import de.jeanlucmakiola.calendula.ui.common.EventSelectionHost import de.jeanlucmakiola.calendula.ui.common.eventActionFailureMessage import de.jeanlucmakiola.calendula.ui.common.rememberFabSelection +import de.jeanlucmakiola.calendula.ui.common.canWriteCalendar import de.jeanlucmakiola.calendula.ui.common.rememberWriteGateOr import de.jeanlucmakiola.calendula.ui.common.EventMoveScope import de.jeanlucmakiola.calendula.ui.common.LocalEventMove @@ -456,15 +458,25 @@ fun CalendarHost( editKey = key } // A tap opens the edit form instead of the detail screen when the user chose - // so (#178) — only for events whose calendar takes writes. - val tapOpensEditor = viewModel.tapOpensEditor.collectAsStateWithLifecycle().value - val actionCalendars by eventActions.calendars.collectAsStateWithLifecycle() + // so (#178) — only for events whose calendar takes writes. Both are read at + // tap time, so a tap before they've loaded still routes right. + val tapScope = rememberCoroutineScope() + val tapContext = LocalContext.current val requireWriteOr = rememberWriteGateOr() + // A refused write upgrade is asked once; later taps just open the details. + var tapWriteRefused by rememberSaveable { mutableStateOf(false) } val onEventClick: (EventInstance) -> Unit = { event -> - if (tapOpensEditor && actionCalendars[event.calendarId]?.canModifyContents == true) { - requireWriteOr({ onEditEvent(event) }, { onOpenDetail(event) }) - } else { - onOpenDetail(event) + tapScope.launch { + val toEditor = viewModel.tapOpensEditor() && eventActions.canModify(event.calendarId) + when { + !toEditor -> onOpenDetail(event) + tapContext.canWriteCalendar() -> onEditEvent(event) + tapWriteRefused -> onOpenDetail(event) + else -> requireWriteOr({ onEditEvent(event) }) { + tapWriteRefused = true + onOpenDetail(event) + } + } } } // Off by preference (#173) switches off the gesture, not the scope: blocks diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/CalendarHostViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/CalendarHostViewModel.kt index d3f6477..ba9d915 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/CalendarHostViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/CalendarHostViewModel.kt @@ -11,6 +11,7 @@ import de.jeanlucmakiola.calendula.ui.common.IMPLEMENTED_VIEWS import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.stateIn import kotlinx.datetime.LocalDate import javax.inject.Inject @@ -79,11 +80,8 @@ class CalendarHostViewModel @Inject constructor( initialValue = false, ) + private val tapOpensEditorPref = prefs.tapOpensEditor + /** Whether tapping an event opens the edit form rather than the detail screen (#178). */ - val tapOpensEditor: StateFlow = prefs.tapOpensEditor - .stateIn( - scope = viewModelScope, - started = SharingStarted.WhileSubscribed(5_000L), - initialValue = false, - ) + suspend fun tapOpensEditor(): Boolean = tapOpensEditorPref.first() } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventActions.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventActions.kt index 319c65b..4527911 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventActions.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/EventActions.kt @@ -73,6 +73,11 @@ suspend fun CalendarRepository.deleteEvent( /** How a [tryDeleteEvent] ended. */ enum class DeleteOutcome { Deleted, Gone, NeedsPermission, Failed } +/** Whether this install may write to the calendar; v1.0 installs only hold read access. */ +fun Context.canWriteCalendar(): Boolean = + ContextCompat.checkSelfPermission(this, Manifest.permission.WRITE_CALENDAR) == + PackageManager.PERMISSION_GRANTED + /** [deleteEvent], with the provider's failures mapped to a [DeleteOutcome]. */ suspend fun CalendarRepository.tryDeleteEvent( eventId: Long, @@ -115,11 +120,7 @@ fun rememberWriteGateOr(): (action: () -> Unit, fallback: () -> Unit) -> Unit { pending = null } return { action, fallback -> - val granted = ContextCompat.checkSelfPermission( - context, - Manifest.permission.WRITE_CALENDAR, - ) == PackageManager.PERMISSION_GRANTED - if (granted) { + if (context.canWriteCalendar()) { action() } else { pending = action to fallback 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 8837a3d..5a5d2df 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 @@ -13,6 +13,7 @@ import de.jeanlucmakiola.calendula.domain.EventDetail import de.jeanlucmakiola.calendula.domain.EventForm import de.jeanlucmakiola.calendula.domain.EventInstance import de.jeanlucmakiola.calendula.domain.RecurringWriteScope +import de.jeanlucmakiola.calendula.domain.atOccurrence import de.jeanlucmakiola.calendula.domain.toDuplicateForm import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.flow.MutableStateFlow @@ -20,6 +21,7 @@ import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.catch +import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn @@ -109,10 +111,22 @@ class EventActionsViewModel @Inject constructor( _failure.value = null } + /** Whether [calendarId] takes writes, asking the provider while [calendars] hasn't loaded yet. */ + suspend fun canModify(calendarId: Long): Boolean { + val calendar = calendars.value[calendarId] ?: try { + repository.calendars().first().firstOrNull { it.id == calendarId } + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + null + } + return calendar?.canModifyContents == true + } + private suspend fun load(event: EventInstance): EventDetail? = try { - val detail = repository.eventDetail(event.eventId) // The row holds the series start; the actions work on this occurrence. - detail.copy(instance = detail.instance.copy(start = event.start, end = event.end)) + repository.eventDetail(event.eventId) + .atOccurrence(event.start.toEpochMilliseconds(), event.end.toEpochMilliseconds()) } catch (e: CancellationException) { throw e } catch (e: NoSuchEventException) { 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 72ad8b4..95369b5 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 @@ -11,6 +11,7 @@ import de.jeanlucmakiola.calendula.data.ics.IcsExporter import de.jeanlucmakiola.calendula.domain.EventForm 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.toDuplicateForm import de.jeanlucmakiola.calendula.ui.common.DeleteOutcome @@ -33,7 +34,6 @@ import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch import kotlinx.datetime.TimeZone import kotlin.coroutines.cancellation.CancellationException -import kotlin.time.Instant import javax.inject.Inject /** @@ -113,8 +113,8 @@ class EventDetailViewModel @Inject constructor( if (_deleteState.value == DeleteUiState.Deleting) return viewModelScope.launch { _deleteState.value = DeleteUiState.Deleting - // Already gone is what a delete wants anyway. _deleteState.value = when (repository.tryDeleteEvent(target.eventId, target.beginMillis, scope)) { + // Already gone is what a delete wants anyway. DeleteOutcome.Deleted, DeleteOutcome.Gone -> DeleteUiState.Deleted DeleteOutcome.NeedsPermission -> DeleteUiState.NeedsPermission DeleteOutcome.Failed -> DeleteUiState.Failed @@ -148,20 +148,13 @@ class EventDetailViewModel @Inject constructor( private suspend fun loadDetail(target: Target): EventDetailUiState = try { val detail = repository.eventDetail(target.eventId) - // The Events row holds the series start; replace it with this - // occurrence's time so recurring events render correctly. An external - // "open event" that names no occurrence ([NO_OCCURRENCE_TIME] — e.g. a - // bare content://.../events/ VIEW intent, issue #48) keeps the row's - // own DTSTART/DTEND instead of overriding it to the epoch. + // An external "open event" that names no occurrence ([NO_OCCURRENCE_TIME] — + // e.g. a bare content://.../events/ VIEW intent, issue #48) keeps the + // row's own DTSTART/DTEND instead of overriding it to the epoch. val corrected = if (target.beginMillis == NO_OCCURRENCE_TIME) { detail } else { - detail.copy( - instance = detail.instance.copy( - start = Instant.fromEpochMilliseconds(target.beginMillis), - end = Instant.fromEpochMilliseconds(target.endMillis), - ), - ) + detail.atOccurrence(target.beginMillis, target.endMillis) } val calendar = repository.calendars().first() .firstOrNull { it.id == corrected.instance.calendarId } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditScreen.kt index ea31c6f..f72acb4 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/edit/EventEditScreen.kt @@ -4,7 +4,6 @@ import android.Manifest import android.app.Activity import android.content.Context import android.content.Intent -import android.content.pm.PackageManager import android.net.Uri import android.provider.ContactsContract import android.util.Patterns @@ -38,6 +37,7 @@ import androidx.compose.material.icons.filled.Add import de.jeanlucmakiola.floret.identity.LocalFloretDarkTheme import kotlinx.coroutines.launch import de.jeanlucmakiola.calendula.ui.common.sendEventShare +import de.jeanlucmakiola.calendula.ui.common.canWriteCalendar import de.jeanlucmakiola.calendula.ui.common.rememberWriteGate import de.jeanlucmakiola.calendula.ui.common.DeleteEventDialog import androidx.compose.runtime.rememberCoroutineScope @@ -115,7 +115,6 @@ import androidx.compose.ui.text.input.KeyboardCapitalization import androidx.compose.ui.text.input.KeyboardType import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp -import androidx.core.content.ContextCompat import androidx.hilt.navigation.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import de.jeanlucmakiola.calendula.R @@ -268,8 +267,14 @@ fun EventEditScreen( val hasUnsavedChanges by viewModel.hasUnsavedChanges.collectAsStateWithLifecycle() var showLeavePrompt by rememberSaveable { mutableStateOf(false) } var showDeleteDialog by rememberSaveable { mutableStateOf(false) } + // A running save or delete can't be called back, so leaving waits for it; + // its result then closes the form or reports why it didn't. val requestClose = { - if (hasUnsavedChanges) showLeavePrompt = true else close() + when { + state?.saveState == SaveUiState.Saving -> Unit + hasUnsavedChanges -> showLeavePrompt = true + else -> close() + } } // Something else wants this form's place, e.g. a second share (#306). LaunchedEffect(leaveRequested) { @@ -287,26 +292,22 @@ fun EventEditScreen( val context = LocalContext.current val snackbarHostState = remember { SnackbarHostState() } - // Contextual WRITE_CALENDAR upgrade for v1.0 installs, like delete. - val writePermissionLauncher = rememberLauncherForActivityResult( + val scope = rememberCoroutineScope() + val requireWrite = rememberWriteGate() + // Save keeps its own launcher: its result calls straight into the ViewModel, + // so a save waiting on the permission dialog survives a rotation. + val savePermissionLauncher = rememberLauncherForActivityResult( contract = ActivityResultContracts.RequestPermission(), ) { granted -> if (granted) viewModel.save() } val onSaveClick = { - val granted = ContextCompat.checkSelfPermission( - context, - Manifest.permission.WRITE_CALENDAR, - ) == PackageManager.PERMISSION_GRANTED - if (granted) { + if (context.canWriteCalendar()) { viewModel.save() } else { - writePermissionLauncher.launch(Manifest.permission.WRITE_CALENDAR) + savePermissionLauncher.launch(Manifest.permission.WRITE_CALENDAR) } } - - val scope = rememberCoroutineScope() - val requireWrite = rememberWriteGate() val shareFailedMessage = stringResource(R.string.event_share_failed) val shareChooserTitle = stringResource(R.string.event_share_chooser_title) val onShareClick: () -> Unit = { 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 c8bfd8e..9c19c09 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 @@ -34,6 +34,7 @@ import de.jeanlucmakiola.calendula.domain.withTimes import de.jeanlucmakiola.calendula.domain.withTimezone import de.jeanlucmakiola.calendula.domain.EventFormField import de.jeanlucmakiola.calendula.domain.RecurringWriteScope +import de.jeanlucmakiola.calendula.domain.atOccurrence import de.jeanlucmakiola.calendula.domain.isEventTarget import de.jeanlucmakiola.calendula.domain.populatedFields import de.jeanlucmakiola.calendula.domain.problems @@ -178,9 +179,11 @@ class EventEditViewModel @Inject constructor( // A one-time offer, raised when a .ics import opens, to replace the file's // reminders with the settings default (#49). Null while there's nothing to ask. private val _importReminderPrompt = MutableStateFlow(null) - // Set by the first change the user makes, so a form only holding its own - // defaults or an import's prefill can be left without asking. - private val _edited = MutableStateFlow(false) + // The form as the user received it: the loaded event, or a new form with its + // prefill and auto-applied defaults. Anything else is an unsaved change. + private val _baseline = MutableStateFlow(null) + // Bumped by reset(), so work started for a closed form never lands on the next. + private var session = 0 /** True when the event to edit couldn't be loaded; the screen closes itself. */ val loadFailed: StateFlow = _loadFailed.asStateFlow() @@ -343,15 +346,11 @@ class EventEditViewModel @Inject constructor( ) /** - * Whether leaving now would lose something the user did (#178): an edit that - * differs from the loaded event, or any change to a new or imported form. + * Whether leaving now would lose something the user did (#178): the form + * differs from how it opened, counting the defaults it applied itself. */ - val hasUnsavedChanges: StateFlow = combine(_form, _editTarget, _edited) { form, target, edited -> - when { - form == null -> false - target != null -> form != target.original - else -> edited - } + val hasUnsavedChanges: StateFlow = combine(_form, _baseline) { form, baseline -> + form != null && form != baseline }.stateIn(viewModelScope, SharingStarted.Eagerly, false) /** @@ -381,7 +380,7 @@ class EventEditViewModel @Inject constructor( else -> LocalDateTime(date, LocalTime(9, 0)).toInstant(zone) } val endAt = startAt + SettingsPrefs.DEFAULT_EVENT_DURATION.minutes - _form.value = EventForm(calendarId = null, start = startAt.toFormTime(zone), end = endAt.toFormTime(zone)) + open(EventForm(calendarId = null, start = startAt.toFormTime(zone), end = endAt.toFormTime(zone))) applyDefaultReminder() applyDefaultDuration() } @@ -396,7 +395,7 @@ class EventEditViewModel @Inject constructor( fun openDraft(start: Instant, end: Instant) { if (_form.value != null) return val zone = zoneSource.current() - _form.value = EventForm(calendarId = null, start = start.toFormTime(zone), end = end.toFormTime(zone)) + open(EventForm(calendarId = null, start = start.toFormTime(zone), end = end.toFormTime(zone))) _durationTouched.value = true applyDefaultReminder() applyDefaultDuration() @@ -430,7 +429,7 @@ class EventEditViewModel @Inject constructor( fun openImported(form: EventForm, source: ImportSource) { if (_form.value != null || _editTarget.value != null) return _revealed.value = form.populatedFields() - _form.value = form + open(form) if (source.ownsDuration) _durationTouched.value = true else applyDefaultDuration() when (source) { ImportSource.Insert, ImportSource.InsertOpenEnded -> @@ -455,16 +454,18 @@ class EventEditViewModel @Inject constructor( */ private fun applyDefaultReminder(calendarId: Long? = null) { if (_editTarget.value != null || _remindersTouched.value) return + val opened = session viewModelScope.launch { val defaults = reminderDefaults() val targetId = calendarId ?: resolvedCalendarId.first() // Re-check after suspending: bail if the form closed or the user edited. + if (opened != session) return@launch val form = _form.value ?: return@launch if (_editTarget.value != null || _remindersTouched.value) return@launch val reminders = defaults.resolveFor(targetId, form.isAllDay) - // Write through update(): the duration default resolves in parallel, - // so both must compose onto the current form, not a pre-suspend copy. - update { it.copy(reminders = reminders) } + // The duration default resolves in parallel, so both must compose + // onto the current form, not a pre-suspend copy. + applyDefault { it.copy(reminders = reminders) } // Surface the section so an auto-applied default is visible and // removable, even when Reminders isn't a default-shown field. if (reminders.isNotEmpty()) { @@ -493,6 +494,7 @@ class EventEditViewModel @Inject constructor( */ private fun applyDefaultDuration(calendarId: Long? = null) { if (_editTarget.value != null || _durationTouched.value) return + val opened = session viewModelScope.launch { val global = settingsPrefs.defaultEventDurationMinutes.first() val overrides = settingsPrefs.perCalendarEventDuration.first() @@ -504,10 +506,11 @@ class EventEditViewModel @Inject constructor( else -> calendarId ?: resolvedCalendarId.first() } // Re-check after suspending: bail if the form closed or the user edited. + if (opened != session) return@launch if (_editTarget.value != null || _durationTouched.value) return@launch if (_form.value?.isAllDay != false) return@launch val duration = resolveDefaultEventDuration(global, overrides, targetId) - update { form -> + applyDefault { form -> val zone = form.resolvedZone(zoneSource.current()) val start = form.startInstant(zone) form.withTimes(start, start + duration.minutes, zone) @@ -523,10 +526,12 @@ class EventEditViewModel @Inject constructor( * configured and it isn't already exactly what the file carried. */ private fun maybePromptImportedReminderDefault(form: EventForm) { + val opened = session viewModelScope.launch { val targetId = resolvedCalendarId.first() val default = reminderDefaults().resolveFor(targetId, form.isAllDay) // Bail if the form closed or became an edit while we resolved. + if (opened != session) return@launch val current = _form.value ?: return@launch if (_editTarget.value != null) return@launch if (default.isEmpty() || default == current.reminders) return@launch @@ -562,37 +567,35 @@ class EventEditViewModel @Inject constructor( */ fun openForEdit(eventId: Long, beginMillis: Long, endMillis: Long) { if (_form.value != null || _editTarget.value != null) return + val opened = session viewModelScope.launch { val detail = try { repository.eventDetail(eventId) } catch (e: CancellationException) { throw e } catch (e: Exception) { - _loadFailed.value = true + if (opened == session) _loadFailed.value = true return@launch } + if (opened != session) return@launch val zone = zoneSource.current() val begin = beginMillis.takeUnless { it == NO_OCCURRENCE_TIME } ?: detail.instance.start.toEpochMilliseconds() val end = endMillis.takeUnless { it == NO_OCCURRENCE_TIME } ?: detail.instance.end.toEpochMilliseconds() val snapshot = detail.toEditSnapshot(begin, end, zone) - val occurrence = detail.copy( - instance = detail.instance.copy( - start = Instant.fromEpochMilliseconds(begin), - end = Instant.fromEpochMilliseconds(end), - ), - ) - _editTarget.value = EditTarget(eventId, snapshot, begin, end, zone, occurrence) + _editTarget.value = EditTarget(eventId, snapshot, begin, end, zone, detail.atOccurrence(begin, end)) // Sections holding data must show even when not in the defaults. _revealed.value = snapshot.form.populatedFields() - _form.value = snapshot.form + open(snapshot.form) } } /** Forget the open form; the next [openNew]/[openForEdit] starts clean. */ fun reset() { + session++ _form.value = null + _baseline.value = null _saveState.value = SaveUiState.Idle _showProblems.value = false _revealed.value = emptySet() @@ -601,7 +604,6 @@ class EventEditViewModel @Inject constructor( _remindersTouched.value = false _durationTouched.value = false _importReminderPrompt.value = null - _edited.value = false } /** Unfold one optional field, picked in the "more fields" dialog. */ @@ -789,8 +791,13 @@ class EventEditViewModel @Inject constructor( ignoreConflict: Boolean = false, ) { val target = _editTarget.value + val opened = session + // A form closed mid-save drops the result rather than handing it to the next one. + fun settle(result: SaveUiState) { + if (opened == session) _saveState.value = result + } viewModelScope.launch { - _saveState.value = SaveUiState.Saving + settle(SaveUiState.Saving) // No locking (plan 03, decision 5): right before writing, re-read // the event and compare against what the form loaded. An external // change parks the save in a conflict dialog instead of silently @@ -802,18 +809,18 @@ class EventEditViewModel @Inject constructor( } catch (e: CancellationException) { throw e } catch (e: NoSuchEventException) { - _saveState.value = SaveUiState.Gone + settle(SaveUiState.Gone) return@launch } catch (e: Exception) { // Can't verify — proceed; a real problem fails the write itself. null } if (fresh != null && fresh != target.snapshot) { - _saveState.value = SaveUiState.AwaitingConflict(scope) + settle(SaveUiState.AwaitingConflict(scope)) return@launch } } - _saveState.value = try { + val result = try { if (target == null) { repository.createEvent(form) prefs.setLastUsedCalendarId(requireNotNull(form.calendarId)) @@ -864,6 +871,7 @@ class EventEditViewModel @Inject constructor( Log.w(TAG, "Save failed (scope=$scope, eventId=${target?.eventId})", e) SaveUiState.Failed } + settle(result) } } @@ -883,10 +891,13 @@ class EventEditViewModel @Inject constructor( fun delete(scope: RecurringWriteScope) { val target = _editTarget.value ?: return if (_saveState.value == SaveUiState.Saving) return + val opened = session + _saveState.value = SaveUiState.Saving viewModelScope.launch { - _saveState.value = SaveUiState.Saving - // Already gone is what a delete wants anyway. - _saveState.value = when (repository.tryDeleteEvent(target.eventId, target.beginMillis, scope)) { + val outcome = repository.tryDeleteEvent(target.eventId, target.beginMillis, scope) + if (opened != session) return@launch + _saveState.value = when (outcome) { + // Already gone is what a delete wants anyway. DeleteOutcome.Deleted, DeleteOutcome.Gone -> SaveUiState.Deleted DeleteOutcome.NeedsPermission -> SaveUiState.DeleteDenied DeleteOutcome.Failed -> SaveUiState.DeleteFailed @@ -914,15 +925,19 @@ class EventEditViewModel @Inject constructor( private fun opened(form: EventForm): EventForm? = _editTarget.value?.original?.takeIf { it.timezone == form.timezone && !it.isAllDay } - private inline fun update(block: (EventForm) -> EventForm) { - _form.value = _form.value?.let(block) + private fun open(form: EventForm) { + _form.value = form + _baseline.value = form } - /** A change the user made, as opposed to a default the form applied itself. */ + /** A default the form applied itself; it moves the baseline too, so it's no unsaved change. */ + private inline fun applyDefault(block: (EventForm) -> EventForm) { + _form.value = _form.value?.let(block) + _baseline.value = _baseline.value?.let(block) + } + + /** A change the user made. */ private inline fun edit(block: (EventForm) -> EventForm) { - val current = _form.value ?: return - val next = block(current) - if (next != current) _edited.value = true - _form.value = next + _form.value = _form.value?.let(block) } } 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 cedd0ed..92e869a 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 @@ -25,6 +25,7 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.Job import kotlinx.coroutines.launch +import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.resetMain @@ -108,9 +109,12 @@ class EventEditViewModelTest { // DataStore on the same file would clash, so it is built only once. s: SettingsPrefs = settings(tempDir), zones: FakeZoneSource = FakeZoneSource(), + // A queued dispatcher here holds provider reads and writes until the test + // advances, so it can close the form while one is in flight. + io: CoroutineDispatcher = dispatcher, ): EventEditViewModel { val p = prefs(tempDir) - val repo = CalendarRepositoryImpl(fake, p, s, dispatcher as CoroutineDispatcher) + val repo = CalendarRepositoryImpl(fake, p, s, io) // Only `shareUri()` touches the exporter, and nothing here shares. return EventEditViewModel(repo, p, s, IcsExporter(ContextWrapper(null)), dispatcher, zones) } @@ -465,6 +469,10 @@ class EventEditViewModelTest { vm.setTitle("Lunch") advanceUntilIdle() assertThat(vm.hasUnsavedChanges.value).isTrue() + + vm.setTitle("") + advanceUntilIdle() + assertThat(vm.hasUnsavedChanges.value).isFalse() job.cancel() } @@ -658,4 +666,72 @@ class EventEditViewModelTest { assertThat(vm.state.value!!.form).isEqualTo(opened) job.cancel() } + + @Test + fun `a load abandoned by closing the form never lands on the next one`( + @TempDir tempDir: Path, + ) = runTest(dispatcher) { + val fake = FakeCalendarDataSource().apply { + calendarsResult = listOf(cal(1L)) + eventDetailResult = { id -> + val base = detail(calendarId = 1L) + base.copy(instance = base.instance.copy(eventId = id, title = "Event $id")) + } + } + val vm = viewModel(tempDir, fake, io = StandardTestDispatcher(testScheduler)) + val job = activate(vm) + + vm.openForEdit(eventId = 1L, beginMillis = beginMillis, endMillis = endMillis) + vm.reset() + advanceUntilIdle() + vm.openForEdit(eventId = 2L, beginMillis = beginMillis, endMillis = endMillis) + advanceUntilIdle() + + assertThat(vm.state.value?.form?.title).isEqualTo("Event 2") + job.cancel() + } + + @Test + fun `a failed load abandoned by closing the form doesn't close the next one`( + @TempDir tempDir: Path, + ) = runTest(dispatcher) { + val fake = FakeCalendarDataSource().apply { + calendarsResult = listOf(cal(1L)) + eventDetailResult = { null } + } + val vm = viewModel(tempDir, fake, io = StandardTestDispatcher(testScheduler)) + val job = activate(vm) + + vm.openForEdit(eventId = 1L, beginMillis = beginMillis, endMillis = endMillis) + vm.reset() + advanceUntilIdle() + vm.openNew(LocalDate(2026, 10, 12), startMinutes = 9 * 60) + advanceUntilIdle() + + assertThat(vm.loadFailed.value).isFalse() + job.cancel() + } + + @Test + fun `a delete still running when the form closes doesn't report to the next one`( + @TempDir tempDir: Path, + ) = runTest(dispatcher) { + val fake = FakeCalendarDataSource().apply { + calendarsResult = listOf(cal(1L)) + eventDetailResult = { detail(calendarId = 1L) } + } + val vm = viewModel(tempDir, fake, io = StandardTestDispatcher(testScheduler)) + val job = activate(vm) + + vm.openForEdit(eventId = 42L, beginMillis = beginMillis, endMillis = endMillis) + advanceUntilIdle() + vm.delete(RecurringWriteScope.AllEvents) + vm.reset() + vm.openNew(LocalDate(2026, 10, 12), startMinutes = 9 * 60) + advanceUntilIdle() + + assertThat(fake.deletedEventIds).containsExactly(42L) + assertThat(vm.state.value?.saveState).isEqualTo(SaveUiState.Idle) + job.cancel() + } }