From 6a3225dabc36065ddcbed76b624a74fe1ef9a487 Mon Sep 17 00:00:00 2001 From: SiteRelEnby <125829806+SiteRelEnby@users.noreply.github.com> Date: Tue, 25 Aug 2026 09:40:46 -0400 Subject: [PATCH] feat(members): apply tags to a member Tags could be created, renamed and deleted under Settings > System > Tags, but there was no way to put one on a member: the member screens had no tag UI, and the API layer only carried the tag CRUD endpoints, not the member-tag one. An Android-only user could build a whole tag vocabulary and never use it. Adds GET and PUT /v1/members/{id}/tags and a chip picker, following the relationships editor on the same screens: editable in the member editor, read-only on the profile, and rendering nothing in read-only mode when the member has no tags, so it drops in unconditionally. Both are inside the existing "existing members only" guard, since a member being created has no id to attach tags to yet. A tap applies immediately rather than waiting for the editor's Save. The tag set is its own endpoint, not part of the member body that Save flushes, and the relationships editor beside it already behaves this way. The endpoint takes the member's complete tag set rather than a delta, so a toggle that sent only the changed tag would silently strip every other tag off the member. That is the main thing the tests pin, along with the rollback when a write is refused: leaving a chip filled after a rejected save would report a tag as stuck when it is not. A system with no tags defined gets a pointer to where they are created instead of an empty card. Listing the tag vocabulary is best-effort, so a viewer who can read a member's tags but not list all of them still sees what is set. Web shows this on the member profile only; on Android it is editable in the editor and read-only on the profile, matching how relationships already work here. --- .../lupine/sheaf/data/api/SheafApiService.kt | 13 ++ .../systems/lupine/sheaf/data/model/Models.kt | 6 + .../sheaf/ui/members/MemberTagsEditor.kt | 132 +++++++++++++ .../ui/members/MemberTagsEditorViewModel.kt | 94 +++++++++ .../lupine/sheaf/ui/members/MembersScreen.kt | 7 +- .../members/MemberTagsEditorViewModelTest.kt | 182 ++++++++++++++++++ 6 files changed, 433 insertions(+), 1 deletion(-) create mode 100644 sheaf/app/src/main/java/systems/lupine/sheaf/ui/members/MemberTagsEditor.kt create mode 100644 sheaf/app/src/main/java/systems/lupine/sheaf/ui/members/MemberTagsEditorViewModel.kt create mode 100644 sheaf/app/src/test/java/systems/lupine/sheaf/ui/members/MemberTagsEditorViewModelTest.kt 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 e3058c6..159af0d 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 @@ -343,6 +343,19 @@ interface SheafApiService { @GET("/v1/tags") suspend fun listTags(): List + @GET("/v1/members/{id}/tags") + suspend fun getMemberTags(@Path("id") id: String): List + + /** + * Sets the member's complete tag set, not a delta: send every tag the + * member should end up with. Requires the tags:write scope. + */ + @PUT("/v1/members/{id}/tags") + suspend fun setMemberTags( + @Path("id") id: String, + @Body body: MemberTagUpdate, + ): List + @POST("/v1/tags") suspend fun createTag(@Body body: TagCreate): TagRead 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 d02c7b7..c557904 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 @@ -601,6 +601,12 @@ data class GroupMemberUpdate( // ── Tags ────────────────────────────────────────────────────────────────────── +/** Body for `PUT /v1/members/{id}/tags`: the member's complete tag set. */ +@JsonClass(generateAdapter = true) +data class MemberTagUpdate( + @Json(name = "tag_ids") val tagIds: List, +) + @JsonClass(generateAdapter = true) data class TagRead( val id: String, diff --git a/sheaf/app/src/main/java/systems/lupine/sheaf/ui/members/MemberTagsEditor.kt b/sheaf/app/src/main/java/systems/lupine/sheaf/ui/members/MemberTagsEditor.kt new file mode 100644 index 0000000..4082628 --- /dev/null +++ b/sheaf/app/src/main/java/systems/lupine/sheaf/ui/members/MemberTagsEditor.kt @@ -0,0 +1,132 @@ +package systems.lupine.sheaf.ui.members + +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.ExperimentalLayoutApi +import androidx.compose.foundation.layout.FlowRow +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.filled.Check +import androidx.compose.material3.AssistChip +import androidx.compose.material3.Card +import androidx.compose.material3.CardDefaults +import androidx.compose.material3.FilterChip +import androidx.compose.material3.Icon +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.collectAsState +import androidx.compose.runtime.getValue +import androidx.compose.ui.Modifier +import androidx.compose.ui.unit.dp +import androidx.hilt.navigation.compose.hiltViewModel +import systems.lupine.sheaf.ui.components.ColorSwatch +import systems.lupine.sheaf.ui.components.ErrorBanner + +/** + * The tags on one member: a chip per tag in the system, filled when applied. + * + * Mirrors [systems.lupine.sheaf.ui.relationships.RelationshipsEditor]: editable + * on the member editor, read-only on the profile, and rendering nothing at all + * in read-only mode when the member has no tags, so it can be dropped in + * unconditionally. + * + * Changes apply on tap rather than waiting for the editor's Save. The tag set + * is its own endpoint, not part of the member body that Save flushes, and the + * relationships editor on the same screen already works this way. + */ +@OptIn(ExperimentalLayoutApi::class) +@Composable +fun MemberTagsEditor( + memberId: String, + modifier: Modifier = Modifier, + readOnly: Boolean = false, + viewModel: MemberTagsEditorViewModel = hiltViewModel(), +) { + LaunchedEffect(memberId) { viewModel.load(memberId) } + val state by viewModel.state.collectAsState() + + val applied = state.allTags.filter { it.id in state.selected } + if (readOnly && (state.isLoading || applied.isEmpty())) return + // Nothing to show and nothing to pick from: a system with no tags defined + // gets a pointer to where they're made rather than an empty card. + if (!readOnly && !state.isLoading && state.allTags.isEmpty()) { + Card( + modifier = modifier.fillMaxWidth(), + colors = CardDefaults.cardColors( + containerColor = MaterialTheme.colorScheme.surfaceVariant, + ), + ) { + Column(modifier = Modifier.padding(16.dp)) { + Text("Tags", style = MaterialTheme.typography.labelMedium, + color = MaterialTheme.colorScheme.primary) + Text( + "No tags yet. Create them in Settings > System > Tags, then " + + "come back to apply them here.", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } + } + return + } + + Card( + modifier = modifier.fillMaxWidth(), + colors = CardDefaults.cardColors( + containerColor = MaterialTheme.colorScheme.surfaceVariant, + ), + ) { + Column(modifier = Modifier.padding(16.dp)) { + Text( + "Tags", + style = MaterialTheme.typography.labelMedium, + color = MaterialTheme.colorScheme.primary, + ) + state.error?.let { ErrorBanner(it, modifier = Modifier.padding(top = 8.dp)) } + + FlowRow( + modifier = Modifier.padding(top = 8.dp), + horizontalArrangement = Arrangement.spacedBy(8.dp), + verticalArrangement = Arrangement.spacedBy(4.dp), + ) { + if (readOnly) { + applied.forEach { tag -> + AssistChip( + onClick = {}, + label = { Text(tag.name) }, + leadingIcon = { ColorSwatch(tag.color ?: DEFAULT_TAG_COLOR, size = 14.dp) }, + ) + } + } else { + state.allTags.forEach { tag -> + val on = tag.id in state.selected + FilterChip( + selected = on, + // Chips stay tappable while a save is in flight: + // the change is optimistic and rolls back on + // failure, so blocking here would only add lag. + onClick = { viewModel.toggle(tag.id) }, + label = { Text(tag.name) }, + leadingIcon = if (on) ({ + Icon( + Icons.Default.Check, + contentDescription = null, + modifier = Modifier.size(16.dp), + ) + }) else ({ + ColorSwatch(tag.color ?: DEFAULT_TAG_COLOR, size = 14.dp) + }), + ) + } + } + } + } + } +} + +// Matches the swatch the tags manager falls back to for a colourless tag. +private const val DEFAULT_TAG_COLOR = "#10B981" diff --git a/sheaf/app/src/main/java/systems/lupine/sheaf/ui/members/MemberTagsEditorViewModel.kt b/sheaf/app/src/main/java/systems/lupine/sheaf/ui/members/MemberTagsEditorViewModel.kt new file mode 100644 index 0000000..bf6e5a7 --- /dev/null +++ b/sheaf/app/src/main/java/systems/lupine/sheaf/ui/members/MemberTagsEditorViewModel.kt @@ -0,0 +1,94 @@ +package systems.lupine.sheaf.ui.members + +import androidx.lifecycle.ViewModel +import androidx.lifecycle.viewModelScope +import dagger.hilt.android.lifecycle.HiltViewModel +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.update +import kotlinx.coroutines.launch +import systems.lupine.sheaf.data.api.SheafApiService +import systems.lupine.sheaf.data.model.MemberTagUpdate +import systems.lupine.sheaf.data.model.TagRead +import systems.lupine.sheaf.util.toUserMessage +import javax.inject.Inject + +data class MemberTagsUiState( + /** Every tag the system has, the pool to choose from. */ + val allTags: List = emptyList(), + /** Ids currently on this member. */ + val selected: Set = emptySet(), + val isLoading: Boolean = true, + val isSaving: Boolean = false, + val error: String? = null, +) + +@HiltViewModel +class MemberTagsEditorViewModel @Inject constructor( + private val api: SheafApiService, +) : ViewModel() { + + private val _state = MutableStateFlow(MemberTagsUiState()) + val state: StateFlow = _state.asStateFlow() + + private var memberId: String? = null + + fun load(memberId: String) { + // Guard against the LaunchedEffect re-firing on recomposition: a reload + // mid-edit would stomp a toggle the user just made. + if (this.memberId == memberId && !_state.value.isLoading) return + this.memberId = memberId + viewModelScope.launch { + _state.update { it.copy(isLoading = true, error = null) } + // The tag vocabulary is best-effort: a viewer who can read this + // member's tags but not list all of them still gets to see what is + // set, just with nothing to add from. + val all = runCatching { api.listTags() }.getOrDefault(emptyList()) + runCatching { api.getMemberTags(memberId) } + .onSuccess { mine -> + _state.update { + it.copy( + allTags = all, + selected = mine.mapTo(mutableSetOf()) { t -> t.id }, + isLoading = false, + ) + } + } + .onFailure { e -> + _state.update { it.copy(isLoading = false, error = e.toUserMessage()) } + } + } + } + + /** + * Add or remove one tag. The endpoint takes the member's whole tag set + * rather than a delta, so send the result of the toggle. + * + * Applies optimistically and rolls back on failure: this is a chip the user + * taps, and leaving it un-filled until a round-trip completes makes the tap + * feel broken on a slow connection. + */ + fun toggle(tagId: String) { + val id = memberId ?: return + val before = _state.value.selected + val after = if (tagId in before) before - tagId else before + tagId + _state.update { it.copy(selected = after, isSaving = true, error = null) } + viewModelScope.launch { + runCatching { api.setMemberTags(id, MemberTagUpdate(tagIds = after.toList())) } + .onSuccess { saved -> + _state.update { + it.copy( + selected = saved.mapTo(mutableSetOf()) { t -> t.id }, + isSaving = false, + ) + } + } + .onFailure { e -> + _state.update { + it.copy(selected = before, isSaving = false, error = e.toUserMessage()) + } + } + } + } +} diff --git a/sheaf/app/src/main/java/systems/lupine/sheaf/ui/members/MembersScreen.kt b/sheaf/app/src/main/java/systems/lupine/sheaf/ui/members/MembersScreen.kt index 92b020e..0845a83 100644 --- a/sheaf/app/src/main/java/systems/lupine/sheaf/ui/members/MembersScreen.kt +++ b/sheaf/app/src/main/java/systems/lupine/sheaf/ui/members/MembersScreen.kt @@ -818,9 +818,12 @@ fun MemberDetailScreen( } } - // Relationships (existing members only; a new member has no id yet). + // Tags and relationships, for existing members only: both attach to a + // member id, and a member being created does not have one yet. if (!viewModel.isNewMember) { Spacer(Modifier.height(16.dp)) + MemberTagsEditor(memberId = memberId) + Spacer(Modifier.height(12.dp)) RelationshipsEditor(scope = REL_SCOPE_MEMBER, nodeId = memberId) } @@ -1151,6 +1154,8 @@ fun MemberProfileScreen( } // Relationships (read-only; renders nothing when there are none). + MemberTagsEditor(memberId = member.id, readOnly = true) + RelationshipsEditor( scope = REL_SCOPE_MEMBER, nodeId = member.id, diff --git a/sheaf/app/src/test/java/systems/lupine/sheaf/ui/members/MemberTagsEditorViewModelTest.kt b/sheaf/app/src/test/java/systems/lupine/sheaf/ui/members/MemberTagsEditorViewModelTest.kt new file mode 100644 index 0000000..2070038 --- /dev/null +++ b/sheaf/app/src/test/java/systems/lupine/sheaf/ui/members/MemberTagsEditorViewModelTest.kt @@ -0,0 +1,182 @@ +package systems.lupine.sheaf.ui.members + +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.mockk +import io.mockk.slot +import kotlinx.coroutines.test.advanceUntilIdle +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.MainDispatcherRule +import systems.lupine.sheaf.data.api.SheafApiService +import systems.lupine.sheaf.data.model.MemberTagUpdate +import systems.lupine.sheaf.data.model.TagRead +import org.junit.Rule +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull + +/** + * The tag endpoint takes a member's whole tag set rather than a delta, so a + * toggle that sends the wrong thing silently strips every other tag off the + * member. That is the failure this covers. + */ +class MemberTagsEditorViewModelTest { + + @get:Rule val dispatcher = MainDispatcherRule() + + private fun tag(id: String, name: String = id) = TagRead( + id = id, + systemId = "s1", + name = name, + color = null, + createdAt = "2026-01-01T00:00:00Z", + updatedAt = "2026-01-01T00:00:00Z", + ) + + private val all = listOf(tag("a"), tag("b"), tag("c")) + + private fun api(mine: List): SheafApiService = mockk { + coEvery { listTags() } returns all + coEvery { getMemberTags("m1") } returns mine + } + + @Test fun `loads the system's tags and the member's current set`() = runTest { + val vm = MemberTagsEditorViewModel(api(listOf(tag("b")))) + vm.load("m1") + advanceUntilIdle() + + assertEquals(all, vm.state.value.allTags) + assertEquals(setOf("b"), vm.state.value.selected) + } + + @Test fun `adding a tag sends the whole resulting set, not just the new one`() = runTest { + val service = api(listOf(tag("b"))) + val body = slot() + coEvery { service.setMemberTags("m1", capture(body)) } answers { + body.captured.tagIds.map { tag(it) } + } + val vm = MemberTagsEditorViewModel(service) + vm.load("m1") + advanceUntilIdle() + + vm.toggle("a") + + advanceUntilIdle() + + // Sending only "a" here would drop "b" from the member entirely. + assertEquals(setOf("a", "b"), body.captured.tagIds.toSet()) + assertEquals(setOf("a", "b"), vm.state.value.selected) + } + + @Test fun `removing a tag sends the remainder`() = runTest { + val service = api(listOf(tag("a"), tag("b"))) + val body = slot() + coEvery { service.setMemberTags("m1", capture(body)) } answers { + body.captured.tagIds.map { tag(it) } + } + val vm = MemberTagsEditorViewModel(service) + vm.load("m1") + advanceUntilIdle() + + vm.toggle("a") + + advanceUntilIdle() + + assertEquals(listOf("b"), body.captured.tagIds) + assertEquals(setOf("b"), vm.state.value.selected) + } + + @Test fun `removing the last tag sends an empty set rather than skipping the call`() = runTest { + val service = api(listOf(tag("a"))) + val body = slot() + coEvery { service.setMemberTags("m1", capture(body)) } returns emptyList() + val vm = MemberTagsEditorViewModel(service) + vm.load("m1") + advanceUntilIdle() + + vm.toggle("a") + + advanceUntilIdle() + + assertEquals(emptyList(), body.captured.tagIds) + assertEquals(emptySet(), vm.state.value.selected) + } + + @Test fun `the selection reconciles with what the server reports back`() = runTest { + // The server is the authority on the resulting set; if it disagrees + // with our optimistic guess, its answer wins. + val service = api(listOf(tag("a"))) + coEvery { service.setMemberTags("m1", any()) } returns listOf(tag("a"), tag("c")) + val vm = MemberTagsEditorViewModel(service) + vm.load("m1") + advanceUntilIdle() + + vm.toggle("b") + + advanceUntilIdle() + + assertEquals(setOf("a", "c"), vm.state.value.selected) + } + + @Test fun `a failed toggle rolls back and surfaces the error`() = runTest { + val service = api(listOf(tag("a"))) + coEvery { service.setMemberTags("m1", any()) } throws HttpException( + Response.error(403, "".toResponseBody("application/json".toMediaType())), + ) + val vm = MemberTagsEditorViewModel(service) + vm.load("m1") + advanceUntilIdle() + + vm.toggle("b") + + advanceUntilIdle() + + // Leaving the chip filled after a refused write would tell the user + // the tag stuck when it did not. + assertEquals(setOf("a"), vm.state.value.selected) + assertNotNull(vm.state.value.error) + assertEquals(false, vm.state.value.isSaving) + } + + @Test fun `a missing tag vocabulary still shows what the member has`() = runTest { + // listTags is best-effort: a viewer without it should still see the + // member's own tags rather than an error. + val service: SheafApiService = mockk { + coEvery { listTags() } throws HttpException( + Response.error(403, "".toResponseBody("application/json".toMediaType())), + ) + coEvery { getMemberTags("m1") } returns listOf(tag("a")) + } + val vm = MemberTagsEditorViewModel(service) + vm.load("m1") + advanceUntilIdle() + + assertEquals(emptyList(), vm.state.value.allTags) + assertEquals(setOf("a"), vm.state.value.selected) + assertNull(vm.state.value.error) + } + + @Test fun `reloading the same member mid-edit does not stomp the selection`() = runTest { + // The composable's LaunchedEffect can re-fire; a reload that reset the + // selection would undo a toggle the user just made. + val service = api(listOf(tag("a"))) + coEvery { service.setMemberTags("m1", any()) } returns listOf(tag("a"), tag("b")) + val vm = MemberTagsEditorViewModel(service) + vm.load("m1") + advanceUntilIdle() + vm.toggle("b") + advanceUntilIdle() + + vm.load("m1") + + advanceUntilIdle() + + assertEquals(setOf("a", "b"), vm.state.value.selected) + coVerify(exactly = 1) { service.getMemberTags("m1") } + } +}