Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,13 @@ uses semantic versioning (`MAJOR.MINOR.PATCH`).

### Fixed

- **Adding an ongoing front entry explains itself instead of failing.** An
entry left open starts a front, which ends the ones already running. If you
back-dated it to before someone currently fronting began, that was impossible
and the server answered "Internal server error". The sheet now says who it
clashes with and when they started, and suggests a later start or giving the
entry an end time to record it as history.

- **Adding a past front entry no longer disturbs who is fronting now.** The
app created the entry as if it were a switch and then closed it, so between
those two steps it counted as the current front: whoever was really fronting
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,18 @@ fun HistoryScreen(
}

if (showAddSheet) {
// Fresh every time the sheet opens: who is fronting can change between
// openings, and a stale answer either blocks a valid entry or waves
// through the one that 500s.
LaunchedEffect(Unit) { viewModel.loadOpenFrontContext() }
FrontEntrySheet(
allMembers = state.allMembers,
initialFront = null,
groups = state.groups,
memberGroups = state.memberGroups,
openFronts = state.openFronts,
replaceFrontsDefault = state.replaceFrontsDefault,
members = state.members,
onDismiss = { showAddSheet = false },
onConfirm = { memberIds, startedAt, endedAt, customStatus ->
viewModel.addFrontEntry(memberIds, startedAt, endedAt, customStatus)
Expand All @@ -181,6 +188,10 @@ fun HistoryScreen(
initialFront = front,
groups = state.groups,
memberGroups = state.memberGroups,
// Editing never replaces anything, so the check does not apply.
openFronts = emptyList(),
replaceFrontsDefault = state.replaceFrontsDefault,
members = state.members,
onDismiss = { frontToEdit = null },
onConfirm = { memberIds, startedAt, endedAt, customStatus ->
viewModel.updateFrontEntry(front.id, memberIds, startedAt, endedAt, customStatus)
Expand Down Expand Up @@ -704,6 +715,10 @@ private fun FrontEntrySheet(
initialFront: FrontRead?,
groups: List<systems.lupine.sheaf.data.model.GroupRead>,
memberGroups: Map<String, Set<String>>,
// Only consulted for a new entry left open; see the check below.
openFronts: List<FrontRead>,
replaceFrontsDefault: Boolean,
members: Map<String, MemberRead>,
onDismiss: () -> Unit,
onConfirm: (memberIds: List<String>, startedAt: String, endedAt: String?, customStatus: String?) -> Unit,
) {
Expand Down Expand Up @@ -907,6 +922,44 @@ private fun FrontEntrySheet(
)
}

// A new entry left open is a switch, not history: the server ends
// every open front at this start. Back-date it past when one of
// them began and that end would land before its own beginning,
// which the database refuses and the whole request 500s on. Only
// for a new open entry: editing never replaces anything, a closed
// entry is recorded without touching the live roster, and with
// replace off nothing gets ended in the first place.
val clashes = remember(isEditing, stillOngoing, replaceFrontsDefault, openFronts, startInstant) {
if (isEditing || !stillOngoing || !replaceFrontsDefault) emptyList()
else openFrontsStartingAfter(startInstant, openFronts)
}
if (clashes.isNotEmpty()) {
val who = clashes
.flatMap { it.memberIds }
.distinct()
.mapNotNull { members[it]?.displayNameOrName }
.ifEmpty { listOf("whoever is fronting") }
val earliest = clashes
.mapNotNull { runCatching { Instant.parse(it.startedAt) }.getOrNull() }
.minOrNull()
Text(
buildString {
append("An entry left open starts a front, which ends the ones already ")
append("running. ")
append(who.joinToString(", "))
append(if (who.size == 1) " has been fronting" else " have been fronting")
earliest?.let {
append(" since ${formatTimestamp(it.toString(), LocalDisplayTimeZone.current)}")
}
append(", which is after this start, so it can't be ended there. ")
append("Pick a later start, or give this entry an end time to record it ")
append("as history instead.")
},
style = MaterialTheme.typography.bodySmall,
color = MaterialTheme.colorScheme.error,
)
}

Button(
onClick = {
val statusOut = customStatusDraft.trim().ifEmpty { null }
Expand All @@ -917,7 +970,7 @@ private fun FrontEntrySheet(
statusOut,
)
},
enabled = selectedIds.isNotEmpty() && !endBeforeStart,
enabled = selectedIds.isNotEmpty() && !endBeforeStart && clashes.isEmpty(),
modifier = Modifier.fillMaxWidth(),
) { Text(if (isEditing) "Save Changes" else "Add Entry") }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ data class HistoryUiState(
val deleteError: String? = null,
val groups: List<GroupRead> = emptyList(),
val memberGroups: Map<String, Set<String>> = emptyMap(),
// Who is fronting right now, and whether starting a front would end them.
// Both only for validating a new open entry; see loadOpenFrontContext.
val openFronts: List<FrontRead> = emptyList(),
val replaceFrontsDefault: Boolean = true,
// Pagination config.
val view: HistoryView = HistoryView.Infinite,
val pageSize: Int = DEFAULT_PAGE_SIZE,
Expand All @@ -53,6 +57,31 @@ data class HistoryUiState(

val PAGE_SIZE_OPTIONS = listOf(25, 50, 100, 200)

/**
* The open fronts that a new front starting at [startedAt] could not legally
* end, because they had not begun yet.
*
* Starting a front ends every open one *at the new start*. An open front that
* began after that moment would get an end before its own beginning, which
* `ck_fronts_ended_after_started` refuses, taking the whole request down with
* it. Empty means the start is safe.
*
* Equal timestamps are fine: a front that began at exactly this instant ends
* at exactly this instant, which is a zero-length entry rather than a
* backwards one, and the constraint allows it.
*
* A front with an unparseable `started_at` is left out rather than treated as
* a clash: the server is the authority on its own rows, and guessing here
* would block a legitimate entry over a timestamp this client could not read.
*/
fun openFrontsStartingAfter(
startedAt: java.time.Instant,
openFronts: List<FrontRead>,
): List<FrontRead> = openFronts.filter { front ->
val began = runCatching { java.time.Instant.parse(front.startedAt) }.getOrNull()
began != null && began.isAfter(startedAt)
}

@HiltViewModel
class HistoryViewModel @Inject constructor(
private val api: SheafApiService,
Expand Down Expand Up @@ -412,6 +441,33 @@ class HistoryViewModel @Inject constructor(

fun clearError() { _state.update { it.copy(error = null) } }

/**
* What a new *open* entry would do to the fronts already running.
*
* Only the add sheet needs this, and only for the "still ongoing" case:
* an open entry is a switch, so the server ends every open front at the
* new start. Back-date that start past when one of them began and its end
* lands before its own beginning, which the database refuses and the
* request 500s on. Checking it here turns that into a sentence.
*
* Asked for rather than read out of the loaded page: the history list is
* paginated, so an open front is only *probably* on it, and a validation
* that is only probably right is worse than none.
*/
fun loadOpenFrontContext() {
viewModelScope.launch {
val replaceDefault = runCatching { cache.getSystem()?.replaceFrontsDefault }
.getOrNull() ?: true
val open = runCatching { api.getCurrentFronts() }.getOrNull()
_state.update {
it.copy(
openFronts = open ?: it.openFronts,
replaceFrontsDefault = replaceDefault,
)
}
}
}

fun loadGroupsForFilter() {
if (_state.value.groups.isNotEmpty()) return
viewModelScope.launch {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package systems.lupine.sheaf.ui.history

import systems.lupine.sheaf.data.model.FrontRead
import java.time.Instant
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertTrue

/**
* Which open fronts a back-dated start would break.
*
* Starting a front ends every open one at the new start. An open front that
* began *after* that moment would be given an end before its own beginning,
* which `ck_fronts_ended_after_started` refuses, and the server answers the
* whole request with a 500 the user cannot act on. This is the check that
* turns that into a sentence before the request goes out.
*/
class OpenFrontClashTest {

private fun front(id: String, startedAt: String) = FrontRead(
id = id,
systemId = "s1",
memberIds = listOf("m1"),
startedAt = startedAt,
endedAt = null,
customStatus = null,
)

private val noon = Instant.parse("2026-10-12T12:00:00Z")

@Test fun `a start before an open front is the case that breaks`() {
val live = front("f1", "2026-10-12T13:00:00Z")
assertEquals(listOf("f1"), openFrontsStartingAfter(noon, listOf(live)).map { it.id })
}

@Test fun `a start after every open front is fine`() {
val live = front("f1", "2026-10-12T09:00:00Z")
assertTrue(openFrontsStartingAfter(noon, listOf(live)).isEmpty())
}

@Test fun `an identical start is allowed`() {
// The front would end at the instant it began: a zero-length entry,
// not a backwards one, and the constraint permits it.
val live = front("f1", "2026-10-12T12:00:00Z")
assertTrue(openFrontsStartingAfter(noon, listOf(live)).isEmpty())
}

@Test fun `every clashing front is reported, not just the first`() {
// All of them get ended, so all of them have to be named: fixing one
// and resubmitting into the next is a bad way to find out.
val fronts = listOf(
front("early", "2026-10-12T09:00:00Z"),
front("late", "2026-10-12T13:00:00Z"),
front("later", "2026-10-12T14:00:00Z"),
)
assertEquals(
listOf("late", "later"),
openFrontsStartingAfter(noon, fronts).map { it.id },
)
}

@Test fun `nothing open means nothing to clash with`() {
assertTrue(openFrontsStartingAfter(noon, emptyList()).isEmpty())
}

@Test fun `an unreadable timestamp does not block the entry`() {
// The server is the authority on its own rows. Treating a timestamp
// this client cannot parse as a clash would refuse a legitimate entry
// over a formatting disagreement.
assertTrue(openFrontsStartingAfter(noon, listOf(front("f1", "whenever"))).isEmpty())
}
}
Loading