Fix the two remaining known limitations: recurring item date editing, field-by-field conflict merge
- CalendarItem gains recurrenceMasterStart/End/Due, the true unresolved master date, tracked separately from start/end/due (which hold the resolved next occurrence for a recurring item). The edit screen now shows and edits the real master date for recurring items instead of disabling those fields outright, with a note that the change applies to the whole series. Verified against a real recurring event on-device. - New util/ConflictDiff.kt computes exactly which user-editable fields differ between a local edit and the server's version after a 412. The conflict dialog is now a per-field mine/theirs picker (with "All mine"/"All theirs" shortcuts) instead of an all-or-nothing choice. - Extracted statusLabel() to util/StatusLabels.kt so both the edit screen and the new conflict diff can share it. - Two more test files (ConflictDiffTest, extended IcsMapperTest).
This commit is contained in:
parent
14ee70d71d
commit
79cc6925e8
GPG Key ID:
B94562C9B7535805
38
README.md
38
README.md
|
|
@ -57,8 +57,9 @@ is the fourth option: one app, one account, one sync engine.
|
|||
condition of never/after N times/on a date) for both events and tasks, backed by a
|
||||
real iCal `RRULE`.
|
||||
- **Conflict handling** — a stale save (someone else changed or deleted the item since
|
||||
you opened it) surfaces both versions in a dialog and lets you keep your edit or
|
||||
take theirs, instead of just failing with an error.
|
||||
you opened it) surfaces every field that actually differs and lets you pick, per
|
||||
field, whether your edit or the server's current value wins ("All mine"/"All
|
||||
theirs" shortcuts too), instead of just failing with an error.
|
||||
- **Reminders** — add one or more reminders to any event or task (at the scheduled
|
||||
time, or a preset offset before it: 5 min up to 1 week). Reminders fire as real
|
||||
system notifications via `AlarmManager` exact alarms — not a best-effort background
|
||||
|
|
@ -75,7 +76,7 @@ is the fourth option: one app, one account, one sync engine.
|
|||
Nextcloud's own sidebar.
|
||||
- **Recurring events and tasks** — `RRULE` (`FREQ=DAILY/WEEKLY/MONTHLY/YEARLY` with
|
||||
`INTERVAL`/`COUNT`/`UNTIL`) is evaluated client-side to re-date each item to its
|
||||
next upcoming occurrence. See "Known limitations" below for what this doesn't cover.
|
||||
next upcoming occurrence. See "Editing recurring items" below for what this doesn't cover.
|
||||
|
||||
## Home screen widgets
|
||||
|
||||
|
|
@ -138,23 +139,21 @@ nothing.
|
|||
Run the (small, growing) JVM unit test suite with `./gradlew test` — no emulator
|
||||
needed.
|
||||
|
||||
## Known limitations
|
||||
## Editing recurring items
|
||||
|
||||
- **No recurrence editing for a recurring item's start/end time.** You can set/change
|
||||
*whether* something repeats and how (Daily/Weekly/Monthly/Yearly, interval, end
|
||||
condition) via the Repeats picker, and edit all of a recurring item's other fields
|
||||
freely - but the Starts/Ends/Due date fields become read-only the moment an item is
|
||||
recurring, because [CalendarItem.start]/`end`/`due` hold the *resolved next
|
||||
occurrence* for a recurring item, not the master date, and writing that back would
|
||||
silently shift the whole series forward on every edit. Change the actual start date
|
||||
of a recurring series in Nextcloud's web UI. A rule the picker can't represent
|
||||
(`BYDAY`/`BYMONTH`/etc, or anything outside Daily/Weekly/Monthly/Yearly) is left
|
||||
completely untouched rather than flattened into something simpler. Both guarantees
|
||||
are regression-tested in `IcsMapperTest`.
|
||||
- **Conflict resolution is a two-choice dialog, not a field-by-field merge.** A stale
|
||||
write (HTTP 412 - the item changed or was deleted elsewhere since you opened it)
|
||||
shows both versions and lets you keep your edit or take theirs; there's no way to
|
||||
merge individual fields from each.
|
||||
Recurring items get two guarantees, both regression-tested in `IcsMapperTest`:
|
||||
|
||||
- **Editing the series' actual start/end/due time is safe.** [CalendarItem.start]/`end`/
|
||||
`due` hold the *resolved next occurrence* for a recurring item (what every list/grid
|
||||
displays), not the master date - so the edit screen shows/edits a separate
|
||||
`recurrenceMasterStart`/`End`/`Due` for a recurring item instead, with a note that
|
||||
the change applies to the whole series. Editing them writes the true master
|
||||
`DTSTART`/`DTEND`/`DUE`; leaving them untouched round-trips the original value
|
||||
exactly, so an unrelated edit (e.g. just the title) can never silently shift a
|
||||
series forward the way it used to.
|
||||
- **A rule the Repeats picker can't represent is left completely alone.** `BYDAY`/
|
||||
`BYMONTH`/etc-based rules (or anything outside Daily/Weekly/Monthly/Yearly) show as
|
||||
"Does not repeat" in the picker but are never flattened or overwritten by it.
|
||||
|
||||
## Where to look first if you want to extend it
|
||||
|
||||
|
|
@ -170,6 +169,7 @@ needed.
|
|||
- `util/TaskTree.kt` — the parent/child ordering shared by the Tasks screen and widget.
|
||||
- `util/RecurrenceUtils.kt` — re-dates a recurring item to its next occurrence for display.
|
||||
- `data/repository/SaveConflictException.kt` — the 412-conflict data carried to the edit screen's dialog.
|
||||
- `util/ConflictDiff.kt` — computes which fields actually differ for the merge dialog.
|
||||
- `widget/` — the two Glance home screen widgets.
|
||||
|
||||
## Building a signed release
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@ import net.sqlcipher.database.SupportFactory
|
|||
|
||||
@Database(
|
||||
entities = [CollectionEntity::class, ItemEntity::class],
|
||||
version = 5,
|
||||
version = 6,
|
||||
exportSchema = false
|
||||
)
|
||||
abstract class AppDatabase : RoomDatabase() {
|
||||
|
|
|
|||
|
|
@ -33,6 +33,9 @@ data class ItemEntity(
|
|||
val recurrenceCount: Int?,
|
||||
val recurrenceUntil: Long?,
|
||||
val isRecurring: Boolean,
|
||||
val recurrenceMasterStart: Long?,
|
||||
val recurrenceMasterEnd: Long?,
|
||||
val recurrenceMasterDue: Long?,
|
||||
val rawIcs: String?
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -45,7 +45,15 @@ data class CalendarItem(
|
|||
val recurrenceUntil: Long? = null, // mutually exclusive with recurrenceCount
|
||||
val isRecurring: Boolean = false, // true for ANY RRULE, even ones recurrenceFrequency can't represent -
|
||||
// [start]/[end]/[due] are the resolved *next occurrence* when true, not
|
||||
// the master date, so the edit screen can't let those be edited here
|
||||
// the master date - see recurrenceMaster* below for that
|
||||
|
||||
// The *true*, unresolved master DTSTART/DTEND/DUE for a recurring item - only meaningful
|
||||
// when isRecurring. The edit screen shows/edits these instead of start/end/due for a
|
||||
// recurring item, so editing the series' actual start time is possible without the bug
|
||||
// where saving would've overwritten the master date with the resolved next occurrence.
|
||||
val recurrenceMasterStart: Long? = null,
|
||||
val recurrenceMasterEnd: Long? = null, // events only
|
||||
val recurrenceMasterDue: Long? = null, // tasks only
|
||||
|
||||
val rawIcs: String? = null
|
||||
) {
|
||||
|
|
|
|||
|
|
@ -221,7 +221,9 @@ class NextcloudRepository(context: Context) {
|
|||
parentUid = parentUid, reminderMinutes = reminderMinutes.toMinutesList(),
|
||||
recurrenceFrequency = recurrenceFrequency?.let { runCatching { RecurrenceFrequency.valueOf(it) }.getOrNull() },
|
||||
recurrenceInterval = recurrenceInterval, recurrenceCount = recurrenceCount, recurrenceUntil = recurrenceUntil,
|
||||
isRecurring = isRecurring, rawIcs = rawIcs
|
||||
isRecurring = isRecurring,
|
||||
recurrenceMasterStart = recurrenceMasterStart, recurrenceMasterEnd = recurrenceMasterEnd, recurrenceMasterDue = recurrenceMasterDue,
|
||||
rawIcs = rawIcs
|
||||
)
|
||||
|
||||
private fun CalendarItem.toEntity() = ItemEntity(
|
||||
|
|
@ -231,7 +233,9 @@ class NextcloudRepository(context: Context) {
|
|||
status = status.name, percentComplete = percentComplete,
|
||||
priority = priority, parentUid = parentUid, reminderMinutes = reminderMinutes.toCsv(),
|
||||
recurrenceFrequency = recurrenceFrequency?.name, recurrenceInterval = recurrenceInterval,
|
||||
recurrenceCount = recurrenceCount, recurrenceUntil = recurrenceUntil, isRecurring = isRecurring, rawIcs = rawIcs
|
||||
recurrenceCount = recurrenceCount, recurrenceUntil = recurrenceUntil, isRecurring = isRecurring,
|
||||
recurrenceMasterStart = recurrenceMasterStart, recurrenceMasterEnd = recurrenceMasterEnd, recurrenceMasterDue = recurrenceMasterDue,
|
||||
rawIcs = rawIcs
|
||||
)
|
||||
|
||||
private fun String.toMinutesList(): List<Int> =
|
||||
|
|
|
|||
|
|
@ -32,6 +32,7 @@ import androidx.compose.material3.IconButton
|
|||
import androidx.compose.material3.MaterialTheme
|
||||
import androidx.compose.material3.MenuAnchorType
|
||||
import androidx.compose.material3.OutlinedTextField
|
||||
import androidx.compose.material3.RadioButton
|
||||
import androidx.compose.material3.Scaffold
|
||||
import androidx.compose.material3.Slider
|
||||
import androidx.compose.material3.Text
|
||||
|
|
@ -55,6 +56,7 @@ import com.homelab.ncal.data.model.RecurrenceFrequency
|
|||
import com.homelab.ncal.data.model.TaskStatus
|
||||
import com.homelab.ncal.util.priorityColor
|
||||
import com.homelab.ncal.util.priorityLabel
|
||||
import com.homelab.ncal.util.statusLabel
|
||||
import java.text.SimpleDateFormat
|
||||
import java.util.Calendar
|
||||
import java.util.Date
|
||||
|
|
@ -147,7 +149,10 @@ fun ItemEditScreen(
|
|||
if (item.type == ItemType.EVENT) {
|
||||
Text("Starts", style = MaterialTheme.typography.labelLarge, modifier = Modifier.padding(top = 16.dp))
|
||||
if (item.isRecurring) {
|
||||
RecurringDateNotice(item.start)
|
||||
RecurringDateTimePicker(
|
||||
millis = item.recurrenceMasterStart,
|
||||
onChange = { v -> viewModel.update { it.copy(recurrenceMasterStart = v) } }
|
||||
)
|
||||
} else {
|
||||
DateTimePicker(
|
||||
millis = item.start,
|
||||
|
|
@ -156,7 +161,10 @@ fun ItemEditScreen(
|
|||
}
|
||||
Text("Ends", style = MaterialTheme.typography.labelLarge, modifier = Modifier.padding(top = 8.dp))
|
||||
if (item.isRecurring) {
|
||||
RecurringDateNotice(item.end)
|
||||
RecurringDateTimePicker(
|
||||
millis = item.recurrenceMasterEnd,
|
||||
onChange = { v -> viewModel.update { it.copy(recurrenceMasterEnd = v) } }
|
||||
)
|
||||
} else {
|
||||
DateTimePicker(
|
||||
millis = item.end,
|
||||
|
|
@ -177,7 +185,10 @@ fun ItemEditScreen(
|
|||
} else {
|
||||
Text("Starts", style = MaterialTheme.typography.labelLarge, modifier = Modifier.padding(top = 16.dp))
|
||||
if (item.isRecurring) {
|
||||
RecurringDateNotice(item.start)
|
||||
RecurringDateTimePicker(
|
||||
millis = item.recurrenceMasterStart,
|
||||
onChange = { v -> viewModel.update { it.copy(recurrenceMasterStart = v) } }
|
||||
)
|
||||
} else {
|
||||
DateTimePicker(
|
||||
millis = item.start,
|
||||
|
|
@ -186,7 +197,10 @@ fun ItemEditScreen(
|
|||
}
|
||||
Text("Due", style = MaterialTheme.typography.labelLarge, modifier = Modifier.padding(top = 8.dp))
|
||||
if (item.isRecurring) {
|
||||
RecurringDateNotice(item.due)
|
||||
RecurringDateTimePicker(
|
||||
millis = item.recurrenceMasterDue,
|
||||
onChange = { v -> viewModel.update { it.copy(recurrenceMasterDue = v) } }
|
||||
)
|
||||
} else {
|
||||
DateTimePicker(
|
||||
millis = item.due,
|
||||
|
|
@ -321,6 +335,16 @@ fun ItemEditScreen(
|
|||
}
|
||||
|
||||
state.conflict?.let { conflict ->
|
||||
if (conflict.serverVersion != null && state.conflictFields.isNotEmpty()) {
|
||||
ConflictMergeDialog(
|
||||
itemTypeLabel = if (item.type == ItemType.TASK) "task" else "event",
|
||||
fields = state.conflictFields,
|
||||
choices = state.conflictChoices,
|
||||
onChoiceChange = viewModel::setConflictFieldChoice,
|
||||
onSetAll = viewModel::setAllConflictFields,
|
||||
onConfirm = viewModel::resolveConflictMerge
|
||||
)
|
||||
} else {
|
||||
AlertDialog(
|
||||
onDismissRequest = {}, // force an explicit choice - this isn't safely dismissible
|
||||
title = { Text("This ${if (item.type == ItemType.TASK) "task" else "event"} changed elsewhere") },
|
||||
|
|
@ -328,7 +352,7 @@ fun ItemEditScreen(
|
|||
Column {
|
||||
Text(
|
||||
if (conflict.serverVersion != null) {
|
||||
"It was edited somewhere else (another device, or Nextcloud's own web UI) since you opened it here."
|
||||
"It was edited somewhere else (another device, or Nextcloud's own web UI) since you opened it here, though nothing you can edit here actually differs."
|
||||
} else {
|
||||
"It was deleted somewhere else since you opened it here."
|
||||
}
|
||||
|
|
@ -354,6 +378,58 @@ fun ItemEditScreen(
|
|||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Composable
|
||||
private fun ConflictMergeDialog(
|
||||
itemTypeLabel: String,
|
||||
fields: List<com.homelab.ncal.util.ConflictField>,
|
||||
choices: Map<String, Boolean>,
|
||||
onChoiceChange: (key: String, takeTheirs: Boolean) -> Unit,
|
||||
onSetAll: (takeTheirs: Boolean) -> Unit,
|
||||
onConfirm: () -> Unit
|
||||
) {
|
||||
AlertDialog(
|
||||
onDismissRequest = {}, // force an explicit choice - this isn't safely dismissible
|
||||
title = { Text("This $itemTypeLabel changed elsewhere") },
|
||||
text = {
|
||||
Column(Modifier.verticalScroll(rememberScrollState())) {
|
||||
Text("It was edited somewhere else since you opened it here. Pick which version wins for each field that differs:")
|
||||
Row(modifier = Modifier.padding(top = 8.dp)) {
|
||||
TextButton(onClick = { onSetAll(false) }) { Text("All mine") }
|
||||
TextButton(onClick = { onSetAll(true) }) { Text("All theirs") }
|
||||
}
|
||||
fields.forEach { field ->
|
||||
val takeTheirs = choices[field.key] == true
|
||||
Column(modifier = Modifier.padding(top = 12.dp)) {
|
||||
Text(field.label, style = MaterialTheme.typography.labelLarge)
|
||||
Row(
|
||||
verticalAlignment = Alignment.CenterVertically,
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.clickable { onChoiceChange(field.key, false) }
|
||||
) {
|
||||
RadioButton(selected = !takeTheirs, onClick = { onChoiceChange(field.key, false) })
|
||||
Text("Mine: ${field.mineDisplay}")
|
||||
}
|
||||
Row(
|
||||
verticalAlignment = Alignment.CenterVertically,
|
||||
modifier = Modifier
|
||||
.fillMaxWidth()
|
||||
.clickable { onChoiceChange(field.key, true) }
|
||||
) {
|
||||
RadioButton(selected = takeTheirs, onClick = { onChoiceChange(field.key, true) })
|
||||
Text("Theirs: ${field.theirsDisplay}")
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
confirmButton = {
|
||||
TextButton(onClick = onConfirm) { Text("Save merged version") }
|
||||
}
|
||||
)
|
||||
}
|
||||
|
||||
@OptIn(ExperimentalMaterial3Api::class)
|
||||
@Composable
|
||||
|
|
@ -444,15 +520,18 @@ private fun PriorityPicker(selected: Int, onSelect: (Int) -> Unit) {
|
|||
}
|
||||
}
|
||||
|
||||
/** Edits a recurring item's true master date ([CalendarItem.recurrenceMasterStart] etc.) - NOT
|
||||
* the same field the non-recurring [DateTimePicker] call sites bind to, since that one holds
|
||||
* the resolved next occurrence rather than the series' actual start. */
|
||||
@Composable
|
||||
private fun RecurringDateNotice(millis: Long?) {
|
||||
val display = millis?.let { SimpleDateFormat("EEE, MMM d yyyy h:mm a", Locale.getDefault()).format(Date(it)) } ?: "Not set"
|
||||
Column(modifier = Modifier.padding(vertical = 8.dp)) {
|
||||
Text(display)
|
||||
private fun RecurringDateTimePicker(millis: Long?, onChange: (Long?) -> Unit) {
|
||||
Column {
|
||||
DateTimePicker(millis = millis, onChange = onChange)
|
||||
Text(
|
||||
"This is a recurring item - changing when the series starts isn't supported here yet. Edit it in Nextcloud's web UI if you need to.",
|
||||
"This changes the whole series, not just one occurrence.",
|
||||
style = MaterialTheme.typography.bodySmall,
|
||||
color = MaterialTheme.colorScheme.onSurfaceVariant
|
||||
color = MaterialTheme.colorScheme.onSurfaceVariant,
|
||||
modifier = Modifier.padding(bottom = 4.dp)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
|
@ -613,12 +692,6 @@ private fun RecurrenceEndPicker(
|
|||
}
|
||||
}
|
||||
|
||||
private fun statusLabel(status: TaskStatus): String = when (status) {
|
||||
TaskStatus.NEEDS_ACTION -> "Not started"
|
||||
TaskStatus.IN_PROCESS -> "In progress"
|
||||
TaskStatus.COMPLETED -> "Completed"
|
||||
TaskStatus.CANCELLED -> "Cancelled"
|
||||
}
|
||||
|
||||
@OptIn(ExperimentalMaterial3Api::class)
|
||||
@Composable
|
||||
|
|
|
|||
|
|
@ -8,6 +8,8 @@ import com.homelab.ncal.data.model.CalendarItem
|
|||
import com.homelab.ncal.data.model.ItemType
|
||||
import com.homelab.ncal.data.repository.NextcloudRepository
|
||||
import com.homelab.ncal.data.repository.SaveConflictException
|
||||
import com.homelab.ncal.util.ConflictField
|
||||
import com.homelab.ncal.util.diffConflictFields
|
||||
import com.homelab.ncal.util.toUserMessage
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.StateFlow
|
||||
|
|
@ -25,7 +27,9 @@ data class ItemEditUiState(
|
|||
val error: String? = null,
|
||||
val done: Boolean = false,
|
||||
val deleted: Boolean = false,
|
||||
val conflict: SaveConflictException? = null
|
||||
val conflict: SaveConflictException? = null,
|
||||
val conflictFields: List<ConflictField> = emptyList(),
|
||||
val conflictChoices: Map<String, Boolean> = emptyMap() // field key -> true means "take theirs"
|
||||
) {
|
||||
/** Tasks eligible to be set as this task's parent: same calendar, not itself, not one of its own descendants. */
|
||||
val availableParentTasks: List<CalendarItem> get() {
|
||||
|
|
@ -122,8 +126,12 @@ class ItemEditViewModel(
|
|||
repo.save(item)
|
||||
_state.value = _state.value.copy(saving = false, done = true)
|
||||
} catch (e: SaveConflictException) {
|
||||
android.util.Log.d("NCalConflict", "save() 412 conflict for href=${item.href}, server version present=${e.serverVersion != null}")
|
||||
_state.value = _state.value.copy(saving = false, conflict = e)
|
||||
val fields = e.serverVersion?.let { diffConflictFields(item, it) } ?: emptyList()
|
||||
android.util.Log.d(
|
||||
"NCalConflict",
|
||||
"save() 412 conflict for href=${item.href}, server version present=${e.serverVersion != null}, differing fields=${fields.map { it.key }}"
|
||||
)
|
||||
_state.value = _state.value.copy(saving = false, conflict = e, conflictFields = fields, conflictChoices = emptyMap())
|
||||
} catch (e: Exception) {
|
||||
android.util.Log.e("NCalError", "save() failed for href=${item.href} calendar=${item.calendarUrl}", e)
|
||||
_state.value = _state.value.copy(saving = false, error = e.toUserMessage())
|
||||
|
|
@ -131,28 +139,56 @@ class ItemEditViewModel(
|
|||
}
|
||||
}
|
||||
|
||||
/** Conflict resolution: retry the write with the server's current etag, overwriting whatever
|
||||
* changed there with this edit. */
|
||||
/** Conflict resolution (simple path - used when there's nothing to diff, e.g. the item was
|
||||
* deleted server-side): retry the write with the server's current etag, overwriting
|
||||
* whatever changed there with this edit. */
|
||||
fun keepMyChanges() {
|
||||
val conflict = _state.value.conflict ?: return
|
||||
_state.value = _state.value.copy(
|
||||
conflict = null,
|
||||
conflict = null, conflictFields = emptyList(), conflictChoices = emptyMap(),
|
||||
item = conflict.localEdit.copy(etag = conflict.serverVersion?.etag)
|
||||
)
|
||||
save()
|
||||
}
|
||||
|
||||
/** Conflict resolution: discard this edit and load whatever's on the server now (or leave
|
||||
* the screen entirely if it was deleted there) instead. */
|
||||
/** Conflict resolution (simple path): discard this edit and load whatever's on the server
|
||||
* now (or leave the screen entirely if it was deleted there) instead. */
|
||||
fun discardMyChanges() {
|
||||
val conflict = _state.value.conflict ?: return
|
||||
_state.value = if (conflict.serverVersion != null) {
|
||||
_state.value.copy(conflict = null, item = conflict.serverVersion)
|
||||
_state.value.copy(conflict = null, conflictFields = emptyList(), conflictChoices = emptyMap(), item = conflict.serverVersion)
|
||||
} else {
|
||||
_state.value.copy(conflict = null, deleted = true)
|
||||
_state.value.copy(conflict = null, conflictFields = emptyList(), conflictChoices = emptyMap(), deleted = true)
|
||||
}
|
||||
}
|
||||
|
||||
/** Per-field conflict resolution: toggle whether [key] should take the server's value
|
||||
* ([takeTheirs] = true) or keep the local edit's value (false, the default). */
|
||||
fun setConflictFieldChoice(key: String, takeTheirs: Boolean) {
|
||||
_state.value = _state.value.copy(conflictChoices = _state.value.conflictChoices + (key to takeTheirs))
|
||||
}
|
||||
|
||||
fun setAllConflictFields(takeTheirs: Boolean) {
|
||||
_state.value = _state.value.copy(
|
||||
conflictChoices = _state.value.conflictFields.associate { it.key to takeTheirs }
|
||||
)
|
||||
}
|
||||
|
||||
/** Applies the chosen per-field picks onto the local edit and retries the save with the
|
||||
* server's current etag. */
|
||||
fun resolveConflictMerge() {
|
||||
val conflict = _state.value.conflict ?: return
|
||||
val theirs = conflict.serverVersion ?: return
|
||||
var merged = conflict.localEdit.copy(etag = theirs.etag)
|
||||
_state.value.conflictFields.forEach { field ->
|
||||
if (_state.value.conflictChoices[field.key] == true) {
|
||||
merged = field.takeTheirs(merged, theirs)
|
||||
}
|
||||
}
|
||||
_state.value = _state.value.copy(conflict = null, conflictFields = emptyList(), conflictChoices = emptyMap(), item = merged)
|
||||
save()
|
||||
}
|
||||
|
||||
fun delete() {
|
||||
val item = _state.value.item ?: return
|
||||
if (item.isNew) return
|
||||
|
|
|
|||
|
|
@ -0,0 +1,89 @@
|
|||
package com.homelab.ncal.util
|
||||
|
||||
import com.homelab.ncal.data.model.CalendarItem
|
||||
import com.homelab.ncal.data.model.ItemType
|
||||
import java.text.SimpleDateFormat
|
||||
import java.util.Date
|
||||
import java.util.Locale
|
||||
|
||||
/**
|
||||
* One user-editable field that differs between a local edit and the server's current version
|
||||
* after a save conflict (HTTP 412). [takeTheirs] applies just this field from `theirs` onto
|
||||
* `target`, leaving everything else in `target` untouched - so a set of chosen fields can be
|
||||
* folded onto the local edit one at a time to build a real per-field merge.
|
||||
*/
|
||||
data class ConflictField(
|
||||
val key: String,
|
||||
val label: String,
|
||||
val mineDisplay: String,
|
||||
val theirsDisplay: String,
|
||||
val takeTheirs: (target: CalendarItem, theirs: CalendarItem) -> CalendarItem
|
||||
)
|
||||
|
||||
private fun formatConflictDate(millis: Long?): String =
|
||||
millis?.let { SimpleDateFormat("EEE, MMM d yyyy h:mm a", Locale.getDefault()).format(Date(it)) } ?: "Not set"
|
||||
|
||||
private fun displayOrEmpty(value: String): String = value.ifBlank { "(empty)" }
|
||||
|
||||
/**
|
||||
* Every user-editable field that differs between [mine] (the edit that just failed to save) and
|
||||
* [theirs] (the server's current version), in the same order they appear on the edit form. Only
|
||||
* differing fields are included, so the merge UI stays focused on what actually needs a decision.
|
||||
*/
|
||||
fun diffConflictFields(mine: CalendarItem, theirs: CalendarItem): List<ConflictField> {
|
||||
val fields = mutableListOf<ConflictField>()
|
||||
|
||||
fun add(key: String, label: String, mineDisplay: String, theirsDisplay: String, takeTheirs: (CalendarItem, CalendarItem) -> CalendarItem) {
|
||||
fields += ConflictField(key, label, mineDisplay, theirsDisplay, takeTheirs)
|
||||
}
|
||||
|
||||
if (mine.summary != theirs.summary) {
|
||||
add("summary", "Title", mine.summary, theirs.summary) { t, th -> t.copy(summary = th.summary) }
|
||||
}
|
||||
if (mine.description != theirs.description) {
|
||||
add("description", "Description", displayOrEmpty(mine.description), displayOrEmpty(theirs.description)) { t, th -> t.copy(description = th.description) }
|
||||
}
|
||||
if (mine.location != theirs.location) {
|
||||
add("location", "Location", displayOrEmpty(mine.location), displayOrEmpty(theirs.location)) { t, th -> t.copy(location = th.location) }
|
||||
}
|
||||
if (mine.url != theirs.url) {
|
||||
add("url", "URL", displayOrEmpty(mine.url), displayOrEmpty(theirs.url)) { t, th -> t.copy(url = th.url) }
|
||||
}
|
||||
if (mine.priority != theirs.priority) {
|
||||
add("priority", "Priority", priorityLabel(mine.priority), priorityLabel(theirs.priority)) { t, th -> t.copy(priority = th.priority) }
|
||||
}
|
||||
|
||||
if (mine.type == ItemType.EVENT) {
|
||||
if (mine.start != theirs.start) {
|
||||
add("start", "Starts", formatConflictDate(mine.start), formatConflictDate(theirs.start)) { t, th -> t.copy(start = th.start) }
|
||||
}
|
||||
if (mine.end != theirs.end) {
|
||||
add("end", "Ends", formatConflictDate(mine.end), formatConflictDate(theirs.end)) { t, th -> t.copy(end = th.end) }
|
||||
}
|
||||
if (mine.allDay != theirs.allDay) {
|
||||
add("allDay", "All day", if (mine.allDay) "Yes" else "No", if (theirs.allDay) "Yes" else "No") { t, th -> t.copy(allDay = th.allDay) }
|
||||
}
|
||||
} else {
|
||||
if (mine.start != theirs.start) {
|
||||
add("start", "Starts", formatConflictDate(mine.start), formatConflictDate(theirs.start)) { t, th -> t.copy(start = th.start) }
|
||||
}
|
||||
if (mine.due != theirs.due) {
|
||||
add("due", "Due", formatConflictDate(mine.due), formatConflictDate(theirs.due)) { t, th -> t.copy(due = th.due) }
|
||||
}
|
||||
if (mine.status != theirs.status) {
|
||||
add("status", "Status", statusLabel(mine.status), statusLabel(theirs.status)) { t, th -> t.withStatus(th.status) }
|
||||
}
|
||||
if (mine.percentComplete != theirs.percentComplete) {
|
||||
add("percentComplete", "% complete", "${mine.percentComplete}%", "${theirs.percentComplete}%") { t, th -> t.copy(percentComplete = th.percentComplete) }
|
||||
}
|
||||
if (mine.parentUid != theirs.parentUid) {
|
||||
add("parentUid", "Parent task", mine.parentUid ?: "None", theirs.parentUid ?: "None") { t, th -> t.copy(parentUid = th.parentUid) }
|
||||
}
|
||||
}
|
||||
|
||||
if (mine.reminderMinutes.toSet() != theirs.reminderMinutes.toSet()) {
|
||||
add("reminders", "Reminders", "${mine.reminderMinutes.size} set", "${theirs.reminderMinutes.size} set") { t, th -> t.copy(reminderMinutes = th.reminderMinutes) }
|
||||
}
|
||||
|
||||
return fields
|
||||
}
|
||||
|
|
@ -128,6 +128,8 @@ object IcsMapper {
|
|||
recurrenceCount = recurrence?.count,
|
||||
recurrenceUntil = recurrence?.until?.time,
|
||||
isRecurring = recurrence != null,
|
||||
recurrenceMasterStart = originalStart,
|
||||
recurrenceMasterEnd = originalEnd,
|
||||
rawIcs = ics
|
||||
)
|
||||
}
|
||||
|
|
@ -182,6 +184,8 @@ object IcsMapper {
|
|||
recurrenceCount = recurrence?.count,
|
||||
recurrenceUntil = recurrence?.until?.time,
|
||||
isRecurring = recurrence != null,
|
||||
recurrenceMasterStart = originalStart,
|
||||
recurrenceMasterDue = originalDue,
|
||||
rawIcs = ics
|
||||
)
|
||||
}
|
||||
|
|
@ -203,11 +207,12 @@ object IcsMapper {
|
|||
*
|
||||
* Recurring items get two extra guards, both load-bearing:
|
||||
* - [CalendarItem.start]/[due]/[end] hold the *resolved next occurrence* for a recurring
|
||||
* item (see [parse]), not the master DTSTART/DTEND/DUE. Writing them back verbatim would
|
||||
* shift the whole series forward on every single edit. So for an item that was *already*
|
||||
* recurring going in, the master date(s) are left exactly as parsed - only a freshly
|
||||
* recurring-for-the-first-time item (or a plain non-recurring one) takes its date from
|
||||
* the model.
|
||||
* item (see [parse]), not the master DTSTART/DTEND/DUE - writing them back verbatim would
|
||||
* shift the whole series forward on every single edit. So an item that was *already*
|
||||
* recurring going in takes its master date from [CalendarItem.recurrenceMasterStart]/
|
||||
* `End`/`Due` instead (what the edit screen actually shows/edits for a recurring item);
|
||||
* a freshly recurring-for-the-first-time item (or a plain non-recurring one) takes it
|
||||
* from the model's start/end/due as usual, since there's no prior master date to preserve.
|
||||
* - A rule the picker can't fully represent (BYDAY/BYMONTH/etc, or a frequency other than
|
||||
* the 4 simple ones - see [Recurrence.toRecurrenceFrequency]) is left completely alone
|
||||
* whenever the model's `recurrenceFrequency` is null, rather than being replaced by
|
||||
|
|
@ -226,7 +231,10 @@ object IcsMapper {
|
|||
ev.setDescription(item.description.takeIf { it.isNotBlank() })
|
||||
ev.setLocation(item.location.takeIf { it.isNotBlank() })
|
||||
ev.setUrl(item.url.takeIf { it.isNotBlank() })
|
||||
if (!wasRecurring) {
|
||||
if (wasRecurring) {
|
||||
ev.setDateStart(item.recurrenceMasterStart?.let { Date(it) }, !item.allDay)
|
||||
ev.setDateEnd(item.recurrenceMasterEnd?.let { Date(it) }, !item.allDay)
|
||||
} else {
|
||||
ev.setDateStart(item.start?.let { Date(it) }, !item.allDay)
|
||||
ev.setDateEnd(item.end?.let { Date(it) }, !item.allDay)
|
||||
}
|
||||
|
|
@ -250,7 +258,10 @@ object IcsMapper {
|
|||
td.setDescription(item.description.takeIf { it.isNotBlank() })
|
||||
td.setLocation(item.location.takeIf { it.isNotBlank() })
|
||||
td.setUrl(item.url.takeIf { it.isNotBlank() })
|
||||
if (!wasRecurring) {
|
||||
if (wasRecurring) {
|
||||
td.setDateStart(item.recurrenceMasterStart?.let { Date(it) })
|
||||
td.setDateDue(item.recurrenceMasterDue?.let { Date(it) })
|
||||
} else {
|
||||
td.setDateStart(item.start?.let { Date(it) })
|
||||
td.setDateDue(item.due?.let { Date(it) })
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,10 @@
|
|||
package com.homelab.ncal.util
|
||||
|
||||
import com.homelab.ncal.data.model.TaskStatus
|
||||
|
||||
fun statusLabel(status: TaskStatus): String = when (status) {
|
||||
TaskStatus.NEEDS_ACTION -> "Not started"
|
||||
TaskStatus.IN_PROCESS -> "In progress"
|
||||
TaskStatus.COMPLETED -> "Completed"
|
||||
TaskStatus.CANCELLED -> "Cancelled"
|
||||
}
|
||||
|
|
@ -0,0 +1,81 @@
|
|||
package com.homelab.ncal.util
|
||||
|
||||
import com.homelab.ncal.data.model.CalendarItem
|
||||
import com.homelab.ncal.data.model.ItemType
|
||||
import com.homelab.ncal.data.model.TaskStatus
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
|
||||
class ConflictDiffTest {
|
||||
|
||||
private fun event(summary: String = "Standup", location: String = "", start: Long? = 1000L, end: Long? = 2000L) = CalendarItem(
|
||||
uid = "u1", href = "https://example.com/e1.ics", etag = "\"1\"", calendarUrl = "https://example.com/cal/",
|
||||
type = ItemType.EVENT, summary = summary, location = location, start = start, end = end
|
||||
)
|
||||
|
||||
@Test
|
||||
fun `identical items produce no diff fields`() {
|
||||
val mine = event()
|
||||
val theirs = event()
|
||||
assertTrue(diffConflictFields(mine, theirs).isEmpty())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `only the fields that actually differ are reported`() {
|
||||
val mine = event(summary = "Standup", location = "Room A")
|
||||
val theirs = event(summary = "Standup", location = "Room B")
|
||||
|
||||
val fields = diffConflictFields(mine, theirs)
|
||||
|
||||
assertEquals(listOf("location"), fields.map { it.key })
|
||||
assertEquals("Room A", fields.single().mineDisplay)
|
||||
assertEquals("Room B", fields.single().theirsDisplay)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `takeTheirs applies only that one field, not the whole item`() {
|
||||
val mine = event(summary = "Mine title", location = "Mine location")
|
||||
val theirs = event(summary = "Theirs title", location = "Theirs location")
|
||||
|
||||
val fields = diffConflictFields(mine, theirs)
|
||||
val locationField = fields.first { it.key == "location" }
|
||||
|
||||
val merged = locationField.takeTheirs(mine, theirs)
|
||||
|
||||
assertEquals("Theirs location", merged.location)
|
||||
assertEquals("Mine title", merged.summary) // untouched - only "location" was applied
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `task-only fields are compared for tasks and not events`() {
|
||||
val mine = CalendarItem(
|
||||
uid = "t1", href = "https://example.com/t1.ics", etag = "\"1\"", calendarUrl = "https://example.com/cal/",
|
||||
type = ItemType.TASK, summary = "Task", status = TaskStatus.NEEDS_ACTION, percentComplete = 0
|
||||
)
|
||||
val theirs = mine.withStatus(TaskStatus.COMPLETED)
|
||||
|
||||
val fields = diffConflictFields(mine, theirs)
|
||||
|
||||
assertTrue(fields.any { it.key == "status" })
|
||||
assertTrue(fields.any { it.key == "percentComplete" })
|
||||
// event-only fields must never appear for a task, even if irrelevant values differ
|
||||
assertTrue(fields.none { it.key == "allDay" })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `applying a status takeTheirs keeps completed and percentComplete in sync`() {
|
||||
val mine = CalendarItem(
|
||||
uid = "t1", href = "https://example.com/t1.ics", etag = "\"1\"", calendarUrl = "https://example.com/cal/",
|
||||
type = ItemType.TASK, summary = "Task", status = TaskStatus.NEEDS_ACTION, percentComplete = 0
|
||||
)
|
||||
val theirs = mine.withStatus(TaskStatus.COMPLETED)
|
||||
|
||||
val statusField = diffConflictFields(mine, theirs).first { it.key == "status" }
|
||||
val merged = statusField.takeTheirs(mine, theirs)
|
||||
|
||||
assertEquals(TaskStatus.COMPLETED, merged.status)
|
||||
assertTrue(merged.completed)
|
||||
assertEquals(100, merged.percentComplete)
|
||||
}
|
||||
}
|
||||
|
|
@ -68,6 +68,21 @@ class IcsMapperTest {
|
|||
assertTrue("master DTEND must stay exactly as originally authored", rebuiltIcs.contains("DTEND:20200106T100000Z"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `deliberately changing a recurring event's master start via recurrenceMasterStart is applied`() {
|
||||
val item = IcsMapper.parse(recurringEventIcs, href = "https://example.com/e1.ics", etag = null, calendarUrl = "https://example.com/cal/")!!
|
||||
assertEquals(1578301200000L, item.recurrenceMasterStart) // 2020-01-06T09:00:00Z, parsed from the fixture
|
||||
|
||||
// Simulate the user actually moving the Repeats picker's "Starts" field, not just editing
|
||||
// an unrelated field - this is the one case where the master date SHOULD change.
|
||||
val newMasterStart = 1578304800000L // 2020-01-06T10:00:00Z, one hour later
|
||||
val edited = item.copy(recurrenceMasterStart = newMasterStart)
|
||||
val rebuiltIcs = IcsMapper.toIcs(edited)
|
||||
|
||||
assertTrue("master DTSTART must reflect the deliberate change", rebuiltIcs.contains("DTSTART:20200106T100000Z"))
|
||||
assertTrue("RRULE must still survive", rebuiltIcs.contains("RRULE") && rebuiltIcs.contains("FREQ=WEEKLY"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `unsupported BYDAY-based rule is left completely untouched when the picker fields are null`() {
|
||||
val item = IcsMapper.parse(recurringEventIcs, href = "https://example.com/e1.ics", etag = null, calendarUrl = "https://example.com/cal/")!!
|
||||
|
|
|
|||
Loading…
Reference in New Issue