From 1891e8bd642ac7d65ae71beac61654ed38d7f52a Mon Sep 17 00:00:00 2001 From: SiteRelEnby <125829806+SiteRelEnby@users.noreply.github.com> Date: Mon, 3 Aug 2026 00:30:38 -0400 Subject: [PATCH] fix(fronts): use the single-front replace endpoint for co-front changes Adds POST /v1/fronts/{id}/replace and routes every "change who is in this one front" path through it. Adding or removing a member from a co-front used to PATCH the front's member list in place. That has two problems the new endpoint exists to solve: editing the list rewrites one row, so each member's stint stops being its own history entry, and the change carries no notion of a boundary between the old composition and the new. Replacing ends the old front and opens its replacement in one transaction, keeping per-member history intact, and leaves every other open front alone - which matters when the member being removed is in one co-front and someone unrelated is fronting in another. Six call sites move over: add-to-front and remove-from-front on both the members list and the member profile, remove-from-front on Home, and the offline queue's replay of a removal (which passes the queued removal time as the boundary, so a switch made offline lands where the user actually made it). Removing the last member of a front still ends it rather than replacing it; replace requires a non-empty member list, and ending is what that action means. Separately, "set as sole fronter" ended every open front in a loop and then created the new one, which is a stop notification per front followed by a start. It is now a single create with replace_fronts, which the server does in one transaction and reports as one aggregated change. Not changed: the switch sheet and the quick-switch carousel already issue a single create with replace_fronts, and the server takes one before/after snapshot around it, so those were already emitting one notification. --- .../lupine/sheaf/data/api/SheafApiService.kt | 10 +++ .../systems/lupine/sheaf/data/model/Models.kt | 20 ++++++ .../lupine/sheaf/data/sync/SyncWorker.kt | 10 ++- .../lupine/sheaf/ui/home/HomeViewModel.kt | 6 +- .../sheaf/ui/members/MembersViewModel.kt | 66 +++++++++++++++---- 5 files changed, 98 insertions(+), 14 deletions(-) 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 c3d24e2..e3058c6 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 @@ -253,6 +253,16 @@ interface SheafApiService { @DELETE("/v1/fronts/{id}") suspend fun deleteFront(@Path("id") id: String) + /** + * End one open front and start its replacement in a single transaction, + * without touching any other open front. Use this whenever the change is + * "these people are fronting instead of those" within one front: it keeps + * per-member history entries intact and emits one aggregated notification + * rather than a stop followed by a start. + */ + @POST("/v1/fronts/{id}/replace") + suspend fun replaceFront(@Path("id") id: String, @Body body: FrontReplace): FrontRead + // ── Groups ──────────────────────────────────────────────────────────────── @GET("/v1/groups") 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 8bd8b4e..f3ba1e2 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 @@ -517,6 +517,26 @@ data class FrontCreate( @Json(name = "custom_status") val customStatus: String? = null, ) +/** + * Body for `POST /v1/fronts/{id}/replace`: end one specific open front and + * open a replacement in its place, atomically, leaving every other open front + * untouched. + * + * This is the correct way to change who is in a co-front. Editing the member + * list in place loses each member's stint as its own history entry, and + * ending-then-creating emits two notifications for one change; this does both + * halves in one transaction, so it reads as a single aggregated change. + * + * `memberIds` must be non-empty (to end a front entirely, end it instead). + * Omitting `customStatus` carries the replaced front's status over. + */ +@JsonClass(generateAdapter = true) +data class FrontReplace( + @Json(name = "member_ids") val memberIds: List, + @Json(name = "started_at") val startedAt: String? = null, + @Json(name = "custom_status") val customStatus: String? = null, +) + // Serialized by the hand-written FrontUpdateJsonAdapter rather than codegen, // because the wire contract is tristate-by-presence (omit = leave as-is, // JSON null = clear, value = set) and Moshi cannot emit an explicit null on diff --git a/sheaf/app/src/main/java/systems/lupine/sheaf/data/sync/SyncWorker.kt b/sheaf/app/src/main/java/systems/lupine/sheaf/data/sync/SyncWorker.kt index 47a8813..faa7caf 100644 --- a/sheaf/app/src/main/java/systems/lupine/sheaf/data/sync/SyncWorker.kt +++ b/sheaf/app/src/main/java/systems/lupine/sheaf/data/sync/SyncWorker.kt @@ -15,6 +15,7 @@ import systems.lupine.sheaf.data.db.SheafDatabase import systems.lupine.sheaf.data.model.FrontCreate import systems.lupine.sheaf.data.model.FrontUpdate import java.time.Instant +import systems.lupine.sheaf.data.model.FrontReplace @HiltWorker class SyncWorker @AssistedInject constructor( @@ -62,7 +63,14 @@ class SyncWorker @AssistedInject constructor( if (remaining.isEmpty()) { api.updateFront(front.id, FrontUpdate(endedAt = removedAtIso)) } else { - api.updateFront(front.id, FrontUpdate(memberIds = remaining)) + // Same replace-don't-edit rule as the online path, so a + // removal queued offline lands identically when it drains. + // startedAt carries the original removal time, keeping the + // history boundary where the user actually made the change. + api.replaceFront( + front.id, + FrontReplace(memberIds = remaining, startedAt = removedAtIso), + ) } } }.fold( diff --git a/sheaf/app/src/main/java/systems/lupine/sheaf/ui/home/HomeViewModel.kt b/sheaf/app/src/main/java/systems/lupine/sheaf/ui/home/HomeViewModel.kt index 6377f5c..c2615f1 100644 --- a/sheaf/app/src/main/java/systems/lupine/sheaf/ui/home/HomeViewModel.kt +++ b/sheaf/app/src/main/java/systems/lupine/sheaf/ui/home/HomeViewModel.kt @@ -34,6 +34,7 @@ import kotlinx.coroutines.flow.* import kotlinx.coroutines.launch import java.time.Instant import javax.inject.Inject +import systems.lupine.sheaf.data.model.FrontReplace data class HomeUiState( val user: UserRead? = null, @@ -529,7 +530,10 @@ class HomeViewModel @Inject constructor( if (remaining.isEmpty()) { api.updateFront(front.id, FrontUpdate(endedAt = Instant.now().toString())) } else { - api.updateFront(front.id, FrontUpdate(memberIds = remaining)) + // Replace, not an in-place member edit: keeps each + // remaining member's stint as its own history entry + // and emits one aggregated change. + api.replaceFront(front.id, FrontReplace(memberIds = remaining)) } } }.onFailure { e -> diff --git a/sheaf/app/src/main/java/systems/lupine/sheaf/ui/members/MembersViewModel.kt b/sheaf/app/src/main/java/systems/lupine/sheaf/ui/members/MembersViewModel.kt index fa910ba..5747808 100644 --- a/sheaf/app/src/main/java/systems/lupine/sheaf/ui/members/MembersViewModel.kt +++ b/sheaf/app/src/main/java/systems/lupine/sheaf/ui/members/MembersViewModel.kt @@ -88,7 +88,14 @@ class MembersViewModel @Inject constructor( runCatching { val activeFront = _state.value.currentFronts.firstOrNull() if (activeFront != null) { - api.updateFront(activeFront.id, FrontUpdate(memberIds = activeFront.memberIds + memberId)) + // Replace rather than edit the member list in place: this + // keeps each member's stint as its own history entry and + // lands as one aggregated notification instead of a stop + // and a start. Other open fronts are untouched. + api.replaceFront( + activeFront.id, + FrontReplace(memberIds = activeFront.memberIds + memberId), + ) } else { api.createFront(FrontCreate(memberIds = listOf(memberId), startedAt = Instant.now().toString())) } @@ -106,9 +113,17 @@ class MembersViewModel @Inject constructor( runCatching { _state.value.currentFronts.filter { memberId in it.memberIds }.forEach { front -> if (front.memberIds.size == 1) { + // Last one out: the front itself ends. Replace needs a + // non-empty member list, so this stays an end. api.updateFront(front.id, FrontUpdate(endedAt = Instant.now().toString())) } else { - api.updateFront(front.id, FrontUpdate(memberIds = front.memberIds - memberId)) + // Co-front shrinking. Replace keeps the remaining + // members' history entries intact and lands as one + // aggregated change; editing in place did neither. + api.replaceFront( + front.id, + FrontReplace(memberIds = front.memberIds - memberId), + ) } } }.onFailure { e -> @@ -123,10 +138,17 @@ class MembersViewModel @Inject constructor( viewModelScope.launch { _state.update { it.copy(error = null) } runCatching { - _state.value.currentFronts.forEach { front -> - api.updateFront(front.id, FrontUpdate(endedAt = Instant.now().toString())) - } - api.createFront(FrontCreate(memberIds = listOf(memberId), startedAt = Instant.now().toString())) + // One call, not an end-each-then-create: the server ends every + // open front and opens the new one in a single transaction, so + // this lands as one aggregated notification rather than a stop + // per front followed by a start. + api.createFront( + FrontCreate( + memberIds = listOf(memberId), + startedAt = Instant.now().toString(), + replaceFronts = true, + ) + ) }.onFailure { e -> _state.update { it.copy(error = e.toUserMessage()) } return@launch @@ -744,7 +766,12 @@ class MemberProfileViewModel @Inject constructor( runCatching { val active = _state.value.currentFronts.firstOrNull() if (active != null) { - api.updateFront(active.id, FrontUpdate(memberIds = active.memberIds + memberId)) + // See MembersViewModel.addToFront: replace rather than edit + // in place, so history and notifications both stay right. + api.replaceFront( + active.id, + FrontReplace(memberIds = active.memberIds + memberId), + ) } else { api.createFront(FrontCreate(memberIds = listOf(memberId), startedAt = Instant.now().toString())) } @@ -760,9 +787,17 @@ class MemberProfileViewModel @Inject constructor( runCatching { _state.value.currentFronts.filter { memberId in it.memberIds }.forEach { front -> if (front.memberIds.size == 1) { + // Last one out: the front itself ends. Replace needs a + // non-empty member list, so this stays an end. api.updateFront(front.id, FrontUpdate(endedAt = Instant.now().toString())) } else { - api.updateFront(front.id, FrontUpdate(memberIds = front.memberIds - memberId)) + // Co-front shrinking. Replace keeps the remaining + // members' history entries intact and lands as one + // aggregated change; editing in place did neither. + api.replaceFront( + front.id, + FrontReplace(memberIds = front.memberIds - memberId), + ) } } }.onFailure { e -> _state.update { it.copy(error = e.toUserMessage()) } @@ -775,10 +810,17 @@ class MemberProfileViewModel @Inject constructor( fun switchSoleFronter() { viewModelScope.launch { runCatching { - _state.value.currentFronts.forEach { front -> - api.updateFront(front.id, FrontUpdate(endedAt = Instant.now().toString())) - } - api.createFront(FrontCreate(memberIds = listOf(memberId), startedAt = Instant.now().toString())) + // One call, not an end-each-then-create: the server ends every + // open front and opens the new one in a single transaction, so + // this lands as one aggregated notification rather than a stop + // per front followed by a start. + api.createFront( + FrontCreate( + memberIds = listOf(memberId), + startedAt = Instant.now().toString(), + replaceFronts = true, + ) + ) }.onFailure { e -> _state.update { it.copy(error = e.toUserMessage()) } return@launch }