diff --git a/CHANGELOG.md b/CHANGELOG.md index b6bbf78..e8eff7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,33 @@ All notable changes to the Sheaf Android client are recorded here. Format loosel follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); the project uses semantic versioning (`MAJOR.MINOR.PATCH`). +## [Unreleased] + +### Fixed + +- **Deleting a history entry works when System Safety asks for a password or + code.** The delete had no way to carry either, and the auth tier guards every + deletion whether or not Fronts is one of the ticked categories, so the server + refused and you got "Delete failed" with nowhere to type. It now asks first. + +- **A deletion held for a grace period stays on the list, marked pending.** It + used to disappear on the spot and come back on the next refresh, because the + app could not tell "deleted" from "queued for deletion in N days". The entry + now stays put, dimmed with its pending badge, and the app says when it will + go and where to cancel it. + +- **Emptying the custom status on an entry now clears it.** Saving an empty + status left the old one in place: the app read a blank field as "leave this + alone" rather than "remove it". + +- **Editing an entry no longer rewrites its start and end times.** Both were + resent on every save, rounded down to the minute, so changing just a status + rewrote both timestamps. That added edits you did not make to the entry's + history on the web, and it broke "fronting since" for coalesced fronts: a + chain of back-to-back entries is joined by the times matching exactly, so + rounding one side split the chain and restarted that member's timer. Only the + fields you actually changed are sent now. + ## [1.6.1] - 2026-10-24 ### Fixed diff --git a/sheaf/app/src/main/java/systems/lupine/sheaf/data/api/FrontUpdateJsonAdapter.kt b/sheaf/app/src/main/java/systems/lupine/sheaf/data/api/FrontUpdateJsonAdapter.kt index 4b5b5a8..3ce53ab 100644 --- a/sheaf/app/src/main/java/systems/lupine/sheaf/data/api/FrontUpdateJsonAdapter.kt +++ b/sheaf/app/src/main/java/systems/lupine/sheaf/data/api/FrontUpdateJsonAdapter.kt @@ -48,7 +48,18 @@ class FrontUpdateJsonAdapter : JsonAdapter() { writer.endArray() } value.startedAt?.let { writer.name("started_at").value(it) } - value.customStatus?.let { writer.name("custom_status").value(it) } + when { + value.clearCustomStatus -> { + // Same forced-null dance as ended_at: the server keys on the + // field being present, so the null has to survive the writer's + // global serializeNulls setting. + val previous = writer.serializeNulls + writer.serializeNulls = true + writer.name("custom_status").nullValue() + writer.serializeNulls = previous + } + value.customStatus != null -> writer.name("custom_status").value(value.customStatus) + } writer.endObject() } @@ -58,6 +69,7 @@ class FrontUpdateJsonAdapter : JsonAdapter() { var memberIds: List? = null var startedAt: String? = null var customStatus: String? = null + var clearCustomStatus = false reader.beginObject() while (reader.hasNext()) { when (reader.nextName()) { @@ -82,8 +94,12 @@ class FrontUpdateJsonAdapter : JsonAdapter() { if (reader.peek() == JsonReader.Token.NULL) reader.nextNull() else startedAt = reader.nextString() "custom_status" -> - if (reader.peek() == JsonReader.Token.NULL) reader.nextNull() - else customStatus = reader.nextString() + if (reader.peek() == JsonReader.Token.NULL) { + reader.nextNull() + clearCustomStatus = true + } else { + customStatus = reader.nextString() + } else -> reader.skipValue() } } @@ -94,6 +110,7 @@ class FrontUpdateJsonAdapter : JsonAdapter() { memberIds = memberIds, startedAt = startedAt, customStatus = customStatus, + clearCustomStatus = clearCustomStatus, ) } } diff --git a/sheaf/app/src/main/java/systems/lupine/sheaf/data/api/SheafApiService.kt b/sheaf/app/src/main/java/systems/lupine/sheaf/data/api/SheafApiService.kt index 4523d38..e4b2ace 100644 --- a/sheaf/app/src/main/java/systems/lupine/sheaf/data/api/SheafApiService.kt +++ b/sheaf/app/src/main/java/systems/lupine/sheaf/data/api/SheafApiService.kt @@ -350,8 +350,22 @@ interface SheafApiService { @PATCH("/v1/fronts/{id}") suspend fun updateFront(@Path("id") id: String, @Body body: FrontUpdate): FrontRead - @DELETE("/v1/fronts/{id}") - suspend fun deleteFront(@Path("id") id: String) + /** + * Delete a history entry. + * + * Carries a body, because System Safety's auth tier guards every deletion + * whether or not Fronts is a ticked category, and a bare DELETE can answer + * nothing: it came back 400 "Password required" and the screen showed + * "Delete failed" with nowhere to type. + * + * Returns the raw response so the caller can tell 204 (gone) from 202 + * (queued for a grace period, still listed). See [deleteFrontOrQueue]. + */ + @HTTP(method = "DELETE", path = "/v1/fronts/{id}", hasBody = true) + suspend fun deleteFront( + @Path("id") id: String, + @Body body: MemberDeleteConfirm = MemberDeleteConfirm(), + ): Response /** * End one open front and start its replacement in a single transaction, @@ -1193,6 +1207,20 @@ suspend fun SheafApiService.deleteTagOrQueue( return if (resp.code() == 202) resp.body() else null } +/** Returns null when deletion was immediate (204) or queued payload when safeguarded (202). */ +suspend fun SheafApiService.deleteFrontOrQueue( + id: String, + password: String? = null, + totpCode: String? = null, +): FrontDeletePending? { + val resp = deleteFront( + id, + MemberDeleteConfirm(password?.ifBlank { null }, totpCode?.ifBlank { null }), + ) + if (!resp.isSuccessful) throw retrofit2.HttpException(resp) + return if (resp.code() == 202) resp.body() else null +} + /** Returns null when deletion was immediate (200) or queued payload when image-safeguarded (202). */ suspend fun SheafApiService.deleteFileOrQueue( id: String, diff --git a/sheaf/app/src/main/java/systems/lupine/sheaf/data/model/Models.kt b/sheaf/app/src/main/java/systems/lupine/sheaf/data/model/Models.kt index fd8b4cb..580d45a 100644 --- a/sheaf/app/src/main/java/systems/lupine/sheaf/data/model/Models.kt +++ b/sheaf/app/src/main/java/systems/lupine/sheaf/data/model/Models.kt @@ -761,9 +761,25 @@ data class FrontUpdate( val clearEndedAt: Boolean = false, val memberIds: List? = null, val startedAt: String? = null, - // null still omits the field (leave as-is). Clearing a custom status via - // edit remains a follow-up to be wired up alongside audit edit. + // null omits the field, which the server reads as "leave as-is". val customStatus: String? = null, + // When true, emit an explicit `custom_status: null` to CLEAR the status. + // Same shape as clearEndedAt above and for the same reason: null already + // means "untouched", so emptying the field needs a way to say so. Without + // it the edit sheet trimmed an empty status to null, the adapter dropped + // it, and the old status survived a save that looked like it removed one. + val clearCustomStatus: Boolean = false, +) + +/** + * The 202 answer to deleting a front, when System Safety is holding it for a + * grace period. The entry is still there and still listed, now carrying + * `pending_delete_at`; this says when it will actually go. + */ +@JsonClass(generateAdapter = true) +data class FrontDeletePending( + @Json(name = "pending_action_id") val pendingActionId: String? = null, + @Json(name = "finalize_after") val finalizeAfter: String? = null, ) // ── Groups ──────────────────────────────────────────────────────────────────── diff --git a/sheaf/app/src/main/java/systems/lupine/sheaf/ui/history/HistoryScreen.kt b/sheaf/app/src/main/java/systems/lupine/sheaf/ui/history/HistoryScreen.kt index 975befb..144be6e 100644 --- a/sheaf/app/src/main/java/systems/lupine/sheaf/ui/history/HistoryScreen.kt +++ b/sheaf/app/src/main/java/systems/lupine/sheaf/ui/history/HistoryScreen.kt @@ -127,7 +127,19 @@ fun HistoryScreen( AlertDialog( onDismissRequest = { frontToDelete = null }, title = { Text("Delete front entry?") }, - text = { Text("This will permanently delete the front entry for $names. This cannot be undone.") }, + text = { + val days = state.deleteSafety.gracePeriodDays + Text( + if (state.deleteSafety.appliesToFronts && days > 0) { + "The front entry for $names will be queued for deletion in " + + "$days day${if (days == 1) "" else "s"}. You can cancel it from " + + "System Safety before then." + } else { + "This will permanently delete the front entry for $names. " + + "This cannot be undone." + }, + ) + }, confirmButton = { TextButton( onClick = { viewModel.deleteFront(front.id); frontToDelete = null }, @@ -140,6 +152,36 @@ fun HistoryScreen( ) } + // System Safety wants a password or a code before a deletion. Same sheet + // the sharing and custom-field screens use, so re-auth looks the same + // wherever it is asked for. + if (state.stepUpDeleteId != null) { + systems.lupine.sheaf.ui.sharing.StepUpSheet( + authTier = state.deleteSafety.authTier, + totpEnabled = state.deleteSafety.totpEnabled, + isBusy = false, + errorMessage = state.stepUpError, + onConfirm = { password, totp -> viewModel.confirmDeleteStepUp(password, totp) }, + onDismiss = { viewModel.dismissDeleteStepUp() }, + title = "Confirm before deleting", + rationale = "System Safety is set to ask for your credentials before " + + "anything is deleted.", + ) + } + + // The server held the deletion rather than carrying it out. The entry is + // still listed, dimmed with a pending badge, so this only has to say why. + state.deleteQueuedMessage?.let { message -> + AlertDialog( + onDismissRequest = { viewModel.clearDeleteQueuedMessage() }, + title = { Text("Queued for deletion") }, + text = { Text(message) }, + confirmButton = { + TextButton(onClick = { viewModel.clearDeleteQueuedMessage() }) { Text("OK") } + }, + ) + } + if (state.deleteError != null) { // Nothing cleared deleteError, so the dialog used to re-render forever: // OK reloaded the list behind it, and back / scrim taps hit a no-op @@ -739,10 +781,13 @@ private fun FrontEntrySheet( mutableStateOf(initialFront?.customStatus ?: "") } var startDate by remember { mutableStateOf(initialStart?.toLocalDate() ?: LocalDate.now()) } - var startTime by remember { mutableStateOf(initialStart?.toLocalTime()?.withSecond(0)?.withNano(0) ?: LocalTime.now().withSecond(0).withNano(0)) } + // An existing entry keeps its seconds. The pickers only show hours and + // minutes, so rounding here made every save rewrite the timestamp, which + // breaks contiguous-front coalescing (chains match on exact equality). + var startTime by remember { mutableStateOf(initialStart?.toLocalTime() ?: LocalTime.now().withSecond(0).withNano(0)) } var stillOngoing by remember { mutableStateOf(if (isEditing) initialFront?.endedAt == null else false) } var endDate by remember { mutableStateOf(initialEnd?.toLocalDate() ?: LocalDate.now()) } - var endTime by remember { mutableStateOf(initialEnd?.toLocalTime()?.withSecond(0)?.withNano(0) ?: LocalTime.now().withSecond(0).withNano(0)) } + var endTime by remember { mutableStateOf(initialEnd?.toLocalTime() ?: LocalTime.now().withSecond(0).withNano(0)) } var showStartDatePicker by remember { mutableStateOf(false) } var showEndDatePicker by remember { mutableStateOf(false) } diff --git a/sheaf/app/src/main/java/systems/lupine/sheaf/ui/history/HistoryViewModel.kt b/sheaf/app/src/main/java/systems/lupine/sheaf/ui/history/HistoryViewModel.kt index 9ebadca..841ac1a 100644 --- a/sheaf/app/src/main/java/systems/lupine/sheaf/ui/history/HistoryViewModel.kt +++ b/sheaf/app/src/main/java/systems/lupine/sheaf/ui/history/HistoryViewModel.kt @@ -3,6 +3,7 @@ package systems.lupine.sheaf.ui.history import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import systems.lupine.sheaf.data.api.SheafApiService +import systems.lupine.sheaf.data.api.deleteFrontOrQueue import systems.lupine.sheaf.data.db.LocalCache import systems.lupine.sheaf.data.model.FrontCreate import systems.lupine.sheaf.data.model.FrontRead @@ -11,6 +12,9 @@ import systems.lupine.sheaf.data.model.GroupRead import systems.lupine.sheaf.data.model.MemberRead import systems.lupine.sheaf.data.network.NetworkMonitor import systems.lupine.sheaf.data.repository.PreferencesRepository +import systems.lupine.sheaf.ui.sharing.ShareError +import systems.lupine.sheaf.ui.sharing.message +import systems.lupine.sheaf.ui.sharing.toShareError import systems.lupine.sheaf.util.toUserMessage import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.async @@ -22,6 +26,23 @@ import javax.inject.Inject enum class HistoryView { Infinite, Paged } +/** + * The System Safety settings a deletion has to satisfy. + * + * The auth tier guards EVERY deletion, whether or not Fronts is a ticked + * category, so it is read on load and the credentials are asked for before the + * request rather than after a refusal. [appliesToFronts] only decides whether + * the deletion is then held for the grace period. + */ +data class FrontDeleteSafety( + val authTier: String = "none", + val totpEnabled: Boolean = false, + val appliesToFronts: Boolean = false, + val gracePeriodDays: Int = 0, +) { + val needsCredentials: Boolean get() = authTier != "none" +} + data class HistoryUiState( val fronts: List = emptyList(), val members: Map = emptyMap(), @@ -32,6 +53,13 @@ data class HistoryUiState( val hasMore: Boolean = true, val error: String? = null, val deleteError: String? = null, + /** What System Safety will ask for, and how long it holds a deletion. */ + val deleteSafety: FrontDeleteSafety = FrontDeleteSafety(), + /** The entry waiting on credentials, replayed verbatim once they arrive. */ + val stepUpDeleteId: String? = null, + val stepUpError: String? = null, + /** Said after a deletion the server queued rather than carried out. */ + val deleteQueuedMessage: String? = null, val groups: List = emptyList(), val memberGroups: Map> = emptyMap(), // Who is fronting right now, and whether starting a front would end them. @@ -82,6 +110,62 @@ fun openFrontsStartingAfter( began != null && began.isAfter(startedAt) } +/** + * Compare two timestamps at the precision the edit sheet can actually show. + * + * The time pickers offer hours and minutes, so the seconds on a stored + * timestamp are not something the user can see or change. Treating a + * same-minute value as unchanged is what keeps them: the field is left out of + * the patch entirely and the server keeps what it already had. + */ +private fun sameMinute(a: String?, b: String?): Boolean { + if (a == null || b == null) return a == b + val left = runCatching { java.time.Instant.parse(a) }.getOrNull() ?: return false + val right = runCatching { java.time.Instant.parse(b) }.getOrNull() ?: return false + return left.truncatedTo(java.time.temporal.ChronoUnit.MINUTES) == + right.truncatedTo(java.time.temporal.ChronoUnit.MINUTES) +} + +/** + * What an edit actually changed, as a patch carrying only those fields. + * + * The sheet used to submit every field on every save, with both timestamps + * rebuilt from pickers that had already dropped their seconds. So changing + * only a status rewrote both times to the minute, which shows up as a spurious + * edit in the web history and, worse, breaks contiguous-front coalescing: + * chains are walked by strict equality between one entry's end and the next + * one's start, so rounding one side detaches the chain and resets that + * member's "fronting since". + * + * Omitting a field is also how seconds survive. The server reads an absent + * field as "leave it alone", so an untouched time keeps its original value + * exactly, sub-second part and all. + */ +internal fun frontEditPatch( + original: FrontRead, + memberIds: List, + startedAt: String, + endedAt: String?, + customStatus: String?, +): FrontUpdate { + val membersChanged = memberIds.toSet() != original.memberIds.toSet() + val endStateChanged = when { + endedAt == null -> original.endedAt != null + else -> original.endedAt == null || !sameMinute(endedAt, original.endedAt) + } + val statusChanged = customStatus != original.customStatus + return FrontUpdate( + memberIds = memberIds.takeIf { membersChanged }, + startedAt = startedAt.takeIf { !sameMinute(it, original.startedAt) }, + endedAt = endedAt.takeIf { endStateChanged }, + // Only when the entry really had an end and now has none, so an edit + // that never touched the end cannot reopen a closed entry. + clearEndedAt = endStateChanged && endedAt == null, + customStatus = customStatus.takeIf { statusChanged }, + clearCustomStatus = statusChanged && customStatus == null, + ) +} + @HiltViewModel class HistoryViewModel @Inject constructor( private val api: SheafApiService, @@ -113,6 +197,29 @@ class HistoryViewModel @Inject constructor( } loadInitial() } + loadDeleteSafety() + } + + /** + * What a deletion will be asked for, fetched up front. + * + * Best-effort: not knowing means the delete goes out bare, and the server's + * own refusal still routes to the credentials sheet. Knowing just means the + * sheet opens before the refusal rather than after it. + */ + private fun loadDeleteSafety() { + viewModelScope.launch { + runCatching { + val safety = api.getSystemSafety() + val totp = runCatching { api.getMe().totpEnabled }.getOrDefault(false) + FrontDeleteSafety( + authTier = safety.settings.authTier, + totpEnabled = totp, + appliesToFronts = safety.settings.appliesToFronts, + gracePeriodDays = safety.settings.gracePeriodDays, + ) + }.onSuccess { safety -> _state.update { it.copy(deleteSafety = safety) } } + } } fun loadInitial() { @@ -346,23 +453,87 @@ class HistoryViewModel @Inject constructor( loadInitial() } - fun deleteFront(id: String) { + /** + * Delete a history entry, or ask for credentials first if System Safety + * wants them. + * + * Two things this used to get wrong. It sent a bare DELETE, which the + * server refuses with 400 "Password required" under any auth tier, ticked + * category or not, leaving a "Delete failed" dialog with nowhere to type. + * And it removed the row on any 2xx, including the 202 that means the + * server has only QUEUED the deletion for a grace period, so the entry + * reappeared on the next load with nothing marking it as pending. + */ + fun deleteFront(id: String, password: String? = null, totpCode: String? = null) { + val safety = _state.value.deleteSafety + if (safety.needsCredentials && password == null && totpCode == null) { + _state.update { it.copy(stepUpDeleteId = id, stepUpError = null) } + return + } viewModelScope.launch { - runCatching { api.deleteFront(id) } - .onSuccess { - _state.update { - it.copy( - fronts = it.fronts.filterNot { f -> f.id == id }, - deleteError = null, - // Total drops by one when we successfully delete. - totalCount = it.totalCount?.let { t -> (t - 1).coerceAtLeast(0) }, - ) + runCatching { api.deleteFrontOrQueue(id, password, totpCode) } + .onSuccess { pending -> + if (pending?.pendingActionId != null) { + // Still there, now carrying pending_delete_at. The row + // already knows how to render that (dimmed, with a + // badge), so reload rather than guessing at the date. + val days = safety.gracePeriodDays + _state.update { + it.copy( + deleteError = null, + stepUpDeleteId = null, + stepUpError = null, + deleteQueuedMessage = "This entry is queued for deletion in " + + "$days day${if (days == 1) "" else "s"}. Cancel it from " + + "System Safety before then.", + ) + } + loadInitial() + } else { + _state.update { + it.copy( + fronts = it.fronts.filterNot { f -> f.id == id }, + deleteError = null, + stepUpDeleteId = null, + stepUpError = null, + // Total drops by one when we really delete. + totalCount = it.totalCount?.let { t -> (t - 1).coerceAtLeast(0) }, + ) + } + } + } + .onFailure { e -> + // The tier can change between load and now, and a wrong + // credential comes back the same way, so both land on the + // sheet rather than on a dead-end error dialog. + when (val err = e.toShareError("Couldn't delete this entry")) { + is ShareError.StepUpRequired -> _state.update { + it.copy(stepUpDeleteId = id, stepUpError = null) + } + is ShareError.BadCredentials -> _state.update { + it.copy(stepUpDeleteId = id, stepUpError = err.message) + } + else -> _state.update { + it.copy( + stepUpDeleteId = null, + deleteError = err.message("Couldn't delete this entry"), + ) + } } } - .onFailure { e -> _state.update { it.copy(deleteError = e.toUserMessage()) } } } } + fun confirmDeleteStepUp(password: String?, totpCode: String?) { + val id = _state.value.stepUpDeleteId ?: return + deleteFront(id, password?.ifBlank { null }, totpCode?.ifBlank { null }) + } + + fun dismissDeleteStepUp() = + _state.update { it.copy(stepUpDeleteId = null, stepUpError = null) } + + fun clearDeleteQueuedMessage() = _state.update { it.copy(deleteQueuedMessage = null) } + fun clearDeleteError() = _state.update { it.copy(deleteError = null) } fun addFrontEntry( @@ -416,19 +587,22 @@ class HistoryViewModel @Inject constructor( endedAt: String?, customStatus: String?, ) { + val original = _state.value.fronts.firstOrNull { it.id == id } viewModelScope.launch { runCatching { api.updateFront( id, - FrontUpdate( + // Only what changed. Sending everything rewrote both + // timestamps on every save, including status-only edits. + original?.let { + frontEditPatch(it, memberIds, startedAt, endedAt, customStatus) + } ?: FrontUpdate( memberIds = memberIds, startedAt = startedAt, endedAt = endedAt, - // The edit dialog always submits a definite end state, - // so a null endedAt here means "still ongoing" — clear - // it on the server rather than leaving it untouched. clearEndedAt = endedAt == null, customStatus = customStatus, + clearCustomStatus = customStatus == null, ), ) }.onSuccess { updated -> diff --git a/sheaf/app/src/main/java/systems/lupine/sheaf/ui/sharing/StepUpSheet.kt b/sheaf/app/src/main/java/systems/lupine/sheaf/ui/sharing/StepUpSheet.kt index 639530e..1ce989a 100644 --- a/sheaf/app/src/main/java/systems/lupine/sheaf/ui/sharing/StepUpSheet.kt +++ b/sheaf/app/src/main/java/systems/lupine/sheaf/ui/sharing/StepUpSheet.kt @@ -36,6 +36,13 @@ fun StepUpSheet( errorMessage: String?, onConfirm: (password: String?, totpCode: String?) -> Unit, onDismiss: () -> Unit, + // Defaults to the publishing wording this sheet was written for. System + // Safety also gates destructive actions, which are not publishing + // anything, so those pass their own: "Confirm before publishing" over a + // delete reads as the wrong dialog rather than a strict one. + title: String = "Confirm before publishing", + rationale: String = "This makes something visible to people outside your system, " + + "so it needs your credentials first.", ) { val needsPassword = authTier == "password" || authTier == "both" val needsTotp = (authTier == "totp" || authTier == "both") && totpEnabled @@ -44,14 +51,10 @@ fun StepUpSheet( AlertDialog( onDismissRequest = { if (!isBusy) onDismiss() }, - title = { Text("Confirm before publishing") }, + title = { Text(title) }, text = { Column(verticalArrangement = Arrangement.spacedBy(12.dp)) { - Text( - "This makes something visible to people outside your system, " + - "so it needs your credentials first.", - style = MaterialTheme.typography.bodyMedium, - ) + Text(rationale, style = MaterialTheme.typography.bodyMedium) if (needsPassword) { OutlinedTextField( value = password, diff --git a/sheaf/app/src/test/java/systems/lupine/sheaf/ui/history/FrontDeleteSafetyTest.kt b/sheaf/app/src/test/java/systems/lupine/sheaf/ui/history/FrontDeleteSafetyTest.kt new file mode 100644 index 0000000..bfa9df9 --- /dev/null +++ b/sheaf/app/src/test/java/systems/lupine/sheaf/ui/history/FrontDeleteSafetyTest.kt @@ -0,0 +1,51 @@ +package systems.lupine.sheaf.ui.history + +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +/** + * When deleting a history entry has to ask for credentials first. + * + * The delete used to be a bare DELETE with no body, which the server refuses + * with 400 "Password required" under any auth tier. Confirmed against a live + * instance with Fronts NOT ticked: the tier guards every deletion on its own, + * and the ticked category only decides whether the deletion is then held for + * the grace period. So the prompt keys on the tier, never on the category. + */ +class FrontDeleteSafetyTest { + + @Test fun `no tier means no prompt`() { + assertFalse(FrontDeleteSafety(authTier = "none").needsCredentials) + } + + @Test fun `every tier other than none prompts`() { + for (tier in listOf("password", "totp", "both")) { + assertTrue( + FrontDeleteSafety(authTier = tier).needsCredentials, + "$tier should ask for credentials", + ) + } + } + + @Test fun `the ticked category does not decide whether to prompt`() { + // The trap: Fronts unticked reads as "fronts are not safeguarded", but + // the server still demands the credential. Keying on the category + // would send another bare delete straight into a 400. + assertTrue( + FrontDeleteSafety(authTier = "password", appliesToFronts = false).needsCredentials, + ) + } + + @Test fun `a grace period alone does not prompt`() { + // Grace with tier none queues the deletion (202) without asking for + // anything, which is a different path entirely. + assertFalse( + FrontDeleteSafety( + authTier = "none", + appliesToFronts = true, + gracePeriodDays = 7, + ).needsCredentials, + ) + } +} diff --git a/sheaf/app/src/test/java/systems/lupine/sheaf/ui/history/FrontEditPatchTest.kt b/sheaf/app/src/test/java/systems/lupine/sheaf/ui/history/FrontEditPatchTest.kt new file mode 100644 index 0000000..3e38840 --- /dev/null +++ b/sheaf/app/src/test/java/systems/lupine/sheaf/ui/history/FrontEditPatchTest.kt @@ -0,0 +1,147 @@ +package systems.lupine.sheaf.ui.history + +import systems.lupine.sheaf.data.model.FrontRead +import systems.lupine.sheaf.di.NetworkModule +import systems.lupine.sheaf.data.model.FrontUpdate +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNull +import kotlin.test.assertTrue + +/** + * What an edit sends. + * + * Every save used to submit both timestamps, rebuilt from pickers that had + * already dropped their seconds, so changing only a status rewrote both times + * to the minute. That showed up as a spurious edit in the web history and, + * worse, broke contiguous-front coalescing: the server walks a chain by strict + * equality between one entry's end and the next one's start, so rounding one + * side detaches it and resets that member's "fronting since". Reproduced on a + * live instance, where a status-only edit moved a member's fronting-since from + * 07:00 to 09:15. + */ +class FrontEditPatchTest { + + private val original = FrontRead( + id = "f1", + systemId = "s1", + startedAt = "2026-10-06T07:00:00.123456Z", + endedAt = "2026-10-06T09:15:30.250000Z", + memberIds = listOf("a"), + customStatus = "tired", + ) + + private fun patch( + memberIds: List = original.memberIds, + startedAt: String = original.startedAt, + endedAt: String? = original.endedAt, + customStatus: String? = original.customStatus, + ) = frontEditPatch(original, memberIds, startedAt, endedAt, customStatus) + + @Test fun `a status-only edit leaves both timestamps alone`() { + // The case that broke coalescing. The sheet hands back times rounded + // to the minute, which are the same minute as the originals, so + // neither is sent and the server keeps them exactly. + val update = patch( + startedAt = "2026-10-06T07:00:00Z", + endedAt = "2026-10-06T09:15:30Z", + customStatus = "wired", + ) + assertNull(update.startedAt) + assertNull(update.endedAt) + assertFalse(update.clearEndedAt) + assertEquals("wired", update.customStatus) + } + + @Test fun `an edit that changes nothing sends nothing`() { + val update = patch() + assertEquals(FrontUpdate(), update) + } + + @Test fun `a real time change is sent`() { + val update = patch(startedAt = "2026-10-06T08:30:00Z") + assertEquals("2026-10-06T08:30:00Z", update.startedAt) + assertNull(update.endedAt) + } + + @Test fun `emptying the status clears it`() { + // The screen trims an empty status to null. Null alone means "leave + // as-is", so the clear needs saying explicitly or the old status + // survives a save that looked like it removed one. + val update = patch(customStatus = null) + assertTrue(update.clearCustomStatus) + assertNull(update.customStatus) + } + + @Test fun `an entry with no status does not ask to clear one`() { + val fresh = original.copy(customStatus = null) + val update = frontEditPatch(fresh, fresh.memberIds, fresh.startedAt, fresh.endedAt, null) + assertFalse(update.clearCustomStatus) + } + + @Test fun `reopening an entry clears its end`() { + val update = patch(endedAt = null) + assertTrue(update.clearEndedAt) + assertNull(update.endedAt) + } + + @Test fun `an already-open entry is not reopened by an untouched edit`() { + // clearEndedAt used to be set from `endedAt == null` alone, so every + // save on an open entry restated the clear. + val open = original.copy(endedAt = null) + val update = frontEditPatch(open, open.memberIds, open.startedAt, null, open.customStatus) + assertFalse(update.clearEndedAt) + } + + @Test fun `closing an open entry sends the end`() { + val open = original.copy(endedAt = null) + val update = frontEditPatch( + open, open.memberIds, open.startedAt, "2026-10-06T10:00:00Z", open.customStatus, + ) + assertEquals("2026-10-06T10:00:00Z", update.endedAt) + assertFalse(update.clearEndedAt) + } + + @Test fun `a membership change is sent, and reordering alone is not`() { + assertEquals(listOf("a", "b"), patch(memberIds = listOf("a", "b")).memberIds) + val reordered = original.copy(memberIds = listOf("a", "b")) + val update = frontEditPatch( + reordered, listOf("b", "a"), reordered.startedAt, reordered.endedAt, + reordered.customStatus, + ) + assertNull(update.memberIds) + } + + @Test fun `an unparseable timestamp is sent rather than assumed unchanged`() { + val odd = original.copy(startedAt = "not a timestamp") + val update = frontEditPatch( + odd, odd.memberIds, "2026-10-06T07:00:00Z", odd.endedAt, odd.customStatus, + ) + assertEquals("2026-10-06T07:00:00Z", update.startedAt) + } + + // ── What reaches the wire ──────────────────────────────────────────────── + + private val adapter = NetworkModule.provideMoshi().adapter(FrontUpdate::class.java) + + @Test fun `clearing the status puts an explicit null on the wire`() { + // Moshi drops nulls, so without the flag the field never appeared and + // the server read the save as "status untouched". + val json = adapter.toJson(patch(customStatus = null)) + assertTrue(""""custom_status":null""" in json, json) + } + + @Test fun `a status-only edit mentions no timestamps at all`() { + val json = adapter.toJson( + patch( + startedAt = "2026-10-06T07:00:00Z", + endedAt = "2026-10-06T09:15:30Z", + customStatus = "wired", + ), + ) + assertFalse("started_at" in json, json) + assertFalse("ended_at" in json, json) + assertTrue("custom_status" in json, json) + } +}