From c161a1c03d512b5311b7b8ff68810c22f7044bb7 Mon Sep 17 00:00:00 2001 From: SiteRelEnby <125829806+SiteRelEnby@users.noreply.github.com> Date: Fri, 30 Oct 2026 02:34:16 -0400 Subject: [PATCH] fix(history): credentials, queued deletions, status clearing, and time rewrites Four from the docs pass, all reproduced against a live instance first. Deleting an entry sent a bare DELETE with no body. System Safety's auth tier guards every deletion whether or not Fronts is a ticked category, so the server answered 400 "Password required" and the screen showed "Delete failed" with nowhere to type. Confirmed with Fronts unticked. The delete now carries credentials and asks for them first, through the same sheet the sharing screens use. That sheet's copy was hardcoded to publishing, so it now takes a title and rationale, defaulting to what it already said; the other callers are all privacy raises and are unchanged. A deletion the server only queued was treated as done. With a grace period the server answers 202 and keeps the entry, but the call returned nothing, so the row was removed and came back on the next load with nothing marking it. The row already knows how to render a pending delete, dimmed with a badge, so it just had to stop being removed. The confirmation no longer promises "cannot be undone" when the entry will be held for a grace period instead. Emptying the custom status did nothing. The sheet trims a blank status to null and the adapter omits null, which the server reads as "leave as-is", so the old status survived. Adds clearCustomStatus alongside the existing clearEndedAt, which is the same problem already solved one field up. Every edit rewrote both timestamps. The pickers drop seconds and the sheet resent both times on every save, so a status-only edit rounded them to the minute. That shows up as edits you did not make in the web history, and it breaks contiguous-front coalescing, which the server walks by strict equality between one entry's end and the next one's start. Reproduced: a status-only edit moved a member's fronting-since from 07:00 to 09:15. Edits now send only the fields that changed, which also preserves the seconds, because an absent field leaves the stored value alone. --- CHANGELOG.md | 27 +++ .../sheaf/data/api/FrontUpdateJsonAdapter.kt | 23 +- .../lupine/sheaf/data/api/SheafApiService.kt | 32 ++- .../systems/lupine/sheaf/data/model/Models.kt | 20 +- .../lupine/sheaf/ui/history/HistoryScreen.kt | 51 ++++- .../sheaf/ui/history/HistoryViewModel.kt | 204 ++++++++++++++++-- .../lupine/sheaf/ui/sharing/StepUpSheet.kt | 15 +- .../sheaf/ui/history/FrontDeleteSafetyTest.kt | 51 +++++ .../sheaf/ui/history/FrontEditPatchTest.kt | 147 +++++++++++++ 9 files changed, 539 insertions(+), 31 deletions(-) create mode 100644 sheaf/app/src/test/java/systems/lupine/sheaf/ui/history/FrontDeleteSafetyTest.kt create mode 100644 sheaf/app/src/test/java/systems/lupine/sheaf/ui/history/FrontEditPatchTest.kt 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) + } +}