diff --git a/sheaf/app/src/main/java/systems/lupine/sheaf/data/api/FrontMutations.kt b/sheaf/app/src/main/java/systems/lupine/sheaf/data/api/FrontMutations.kt new file mode 100644 index 0000000..4a96998 --- /dev/null +++ b/sheaf/app/src/main/java/systems/lupine/sheaf/data/api/FrontMutations.kt @@ -0,0 +1,38 @@ +package systems.lupine.sheaf.data.api + +import retrofit2.HttpException +import systems.lupine.sheaf.data.model.FrontRead +import systems.lupine.sheaf.data.model.FrontReplace +import systems.lupine.sheaf.data.model.FrontUpdate + +/** + * Change which members are in one open front, leaving every other open front + * alone. + * + * Prefers `POST /v1/fronts/{id}/replace`, which ends the old front and opens + * its replacement in one transaction: each member's stint stays its own history + * entry, and the whole change reads as one notification. + * + * Falls back to the in-place `PATCH` on a server that predates that endpoint. + * Sheaf is multi-instance and self-hosters upgrade on their own schedule, so a + * client that hard-required a brand-new endpoint would break add-to-front and + * remove-from-front outright on any instance that hadn't caught up. The + * fallback is what those servers always did, history split and all. + * + * A 404 is ambiguous (missing endpoint, or a front that genuinely isn't there), + * and that is fine: if the front is really gone the PATCH fails the same way, + * costing one extra request on a path that was already failing. + */ +suspend fun SheafApiService.replaceFrontMembers( + frontId: String, + memberIds: List, + startedAt: String? = null, +): FrontRead = try { + replaceFront(frontId, FrontReplace(memberIds = memberIds, startedAt = startedAt)) +} catch (e: HttpException) { + if (e.code() == 404 || e.code() == 405) { + updateFront(frontId, FrontUpdate(memberIds = memberIds)) + } else { + throw e + } +} 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 faa7caf..8195ae3 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 @@ -16,6 +16,7 @@ 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 +import systems.lupine.sheaf.data.api.replaceFrontMembers @HiltWorker class SyncWorker @AssistedInject constructor( @@ -67,9 +68,10 @@ class SyncWorker @AssistedInject constructor( // 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( + api.replaceFrontMembers( front.id, - FrontReplace(memberIds = remaining, startedAt = removedAtIso), + memberIds = remaining, + startedAt = removedAtIso, ) } } 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 4731280..4a1b60c 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 @@ -35,6 +35,7 @@ import kotlinx.coroutines.launch import java.time.Instant import javax.inject.Inject import systems.lupine.sheaf.data.model.FrontReplace +import systems.lupine.sheaf.data.api.replaceFrontMembers data class HomeUiState( val user: UserRead? = null, @@ -551,7 +552,7 @@ class HomeViewModel @Inject constructor( // 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)) + api.replaceFrontMembers(front.id, 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 5747808..c6ee135 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 @@ -22,6 +22,7 @@ import okhttp3.MultipartBody import okhttp3.RequestBody.Companion.toRequestBody import java.time.Instant import javax.inject.Inject +import systems.lupine.sheaf.data.api.replaceFrontMembers // ── List ────────────────────────────────────────────────────────────────────── @@ -92,9 +93,9 @@ class MembersViewModel @Inject constructor( // 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( + api.replaceFrontMembers( activeFront.id, - FrontReplace(memberIds = activeFront.memberIds + memberId), + activeFront.memberIds + memberId, ) } else { api.createFront(FrontCreate(memberIds = listOf(memberId), startedAt = Instant.now().toString())) @@ -120,9 +121,9 @@ class MembersViewModel @Inject constructor( // Co-front shrinking. Replace keeps the remaining // members' history entries intact and lands as one // aggregated change; editing in place did neither. - api.replaceFront( + api.replaceFrontMembers( front.id, - FrontReplace(memberIds = front.memberIds - memberId), + front.memberIds - memberId, ) } } @@ -768,9 +769,9 @@ class MemberProfileViewModel @Inject constructor( if (active != null) { // See MembersViewModel.addToFront: replace rather than edit // in place, so history and notifications both stay right. - api.replaceFront( + api.replaceFrontMembers( active.id, - FrontReplace(memberIds = active.memberIds + memberId), + active.memberIds + memberId, ) } else { api.createFront(FrontCreate(memberIds = listOf(memberId), startedAt = Instant.now().toString())) @@ -794,9 +795,9 @@ class MemberProfileViewModel @Inject constructor( // Co-front shrinking. Replace keeps the remaining // members' history entries intact and lands as one // aggregated change; editing in place did neither. - api.replaceFront( + api.replaceFrontMembers( front.id, - FrontReplace(memberIds = front.memberIds - memberId), + front.memberIds - memberId, ) } } diff --git a/sheaf/app/src/test/java/systems/lupine/sheaf/data/api/ReplaceFrontMembersTest.kt b/sheaf/app/src/test/java/systems/lupine/sheaf/data/api/ReplaceFrontMembersTest.kt new file mode 100644 index 0000000..02a259d --- /dev/null +++ b/sheaf/app/src/test/java/systems/lupine/sheaf/data/api/ReplaceFrontMembersTest.kt @@ -0,0 +1,107 @@ +package systems.lupine.sheaf.data.api + +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.mockk +import kotlinx.coroutines.test.runTest +import okhttp3.MediaType.Companion.toMediaType +import okhttp3.ResponseBody.Companion.toResponseBody +import retrofit2.HttpException +import retrofit2.Response +import systems.lupine.sheaf.data.model.FrontRead +import systems.lupine.sheaf.data.model.FrontReplace +import systems.lupine.sheaf.data.model.FrontUpdate +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith + +/** + * Sheaf is multi-instance: a phone can be pointed at an instance running any + * older server. Add-to-front and remove-from-front are core actions, so they + * must keep working when the single-front replace endpoint isn't there yet. + */ +class ReplaceFrontMembersTest { + + private fun httpError(code: Int) = HttpException( + Response.error(code, "".toResponseBody("application/json".toMediaType())), + ) + + private val front = FrontRead( + id = "f1", + systemId = "s1", + memberIds = listOf("a"), + startedAt = "2026-08-01T00:00:00Z", + endedAt = null, + ) + + @Test fun `uses the replace endpoint when the server has it`() = runTest { + val api = mockk() + coEvery { api.replaceFront(any(), any()) } returns front + + api.replaceFrontMembers("f1", listOf("a", "b")) + + coVerify(exactly = 1) { + api.replaceFront("f1", FrontReplace(memberIds = listOf("a", "b"))) + } + coVerify(exactly = 0) { api.updateFront(any(), any()) } + } + + @Test fun `falls back to the in-place patch on a server without the endpoint`() = runTest { + // 404 is what a server predating the route returns for the subpath. + val api = mockk() + coEvery { api.replaceFront(any(), any()) } throws httpError(404) + coEvery { api.updateFront(any(), any()) } returns front + + val result = api.replaceFrontMembers("f1", listOf("a", "b")) + + assertEquals(front, result) + coVerify(exactly = 1) { + api.updateFront("f1", FrontUpdate(memberIds = listOf("a", "b"))) + } + } + + @Test fun `falls back on 405 too`() = runTest { + val api = mockk() + coEvery { api.replaceFront(any(), any()) } throws httpError(405) + coEvery { api.updateFront(any(), any()) } returns front + + api.replaceFrontMembers("f1", listOf("a")) + + coVerify(exactly = 1) { api.updateFront(any(), any()) } + } + + @Test fun `does not swallow other server errors`() = runTest { + // A 409 means the change itself was rejected (duplicate member set). + // Retrying it as a PATCH would either fail again or, worse, quietly + // apply something the server just refused. + val api = mockk() + coEvery { api.replaceFront(any(), any()) } throws httpError(409) + + assertFailsWith { api.replaceFrontMembers("f1", listOf("a")) } + coVerify(exactly = 0) { api.updateFront(any(), any()) } + } + + @Test fun `does not swallow auth failures`() = runTest { + val api = mockk() + coEvery { api.replaceFront(any(), any()) } throws httpError(401) + + assertFailsWith { api.replaceFrontMembers("f1", listOf("a")) } + coVerify(exactly = 0) { api.updateFront(any(), any()) } + } + + @Test fun `passes the boundary timestamp through to the replace call`() = runTest { + // The offline queue replays a removal with the time it was made, so the + // history boundary lands where the user actually made the change. + val api = mockk() + coEvery { api.replaceFront(any(), any()) } returns front + + api.replaceFrontMembers("f1", listOf("a"), startedAt = "2026-08-01T12:00:00Z") + + coVerify { + api.replaceFront( + "f1", + FrontReplace(memberIds = listOf("a"), startedAt = "2026-08-01T12:00:00Z"), + ) + } + } +}