From 90196e282840ecc70fd9ff562482d845909af372 Mon Sep 17 00:00:00 2001 From: Adron Hall Date: Wed, 16 Sep 2026 13:53:08 -0700 Subject: [PATCH] feat(organizations): gate actions on role and surface visibility MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The detail screen offered Edit and Delete to everyone, including non-members (#81 flagged this), and offered every role chip and a Remove button on every member row regardless of what the signed-in user could actually do. Calls that the server would refuse were simply sent. Model the documented role rules once, in OrgPermissions, quoting /help/organizations rather than inventing a hierarchy: - owner: full control; the only role that may delete the organization - admin: add/remove members and change roles, except an owner's, and may not hand out ownership - member: basic access — the roster is visible but read-only - non-member: nothing; the members endpoint is members-only (403) - a system organization ("The Public") can be neither left nor deleted The detail screen now renders only the permitted affordances: the overflow disappears entirely when nothing is available, the member picker is owners/admins only, role chips are limited to the roles the viewer may assign, and Remove is hidden where it would be refused. The server stays authoritative — hiding is UX, not enforcement — so every rejection still surfaces, and the last-owner rules are explained before and after the request for demote and remove, matching how #81 handled leave. Visibility is stated on the header (with what public/private mean) and remains editable by the roles permitted to edit the organization. Three wire bugs found while probing the live API, all fixed: - isPublic was serialised as a string on both create and update; the server answers 500 to a string, so visibility never round-tripped - the PUT echo carries no userRole/memberCount, so mapping it straight through made the editor look like a non-member and hid the actions they had just used; the repository re-reads instead - isSystem was not modelled at all, so "The Public" offered Leave Tests: a role x action matrix in OrgPermissionsTest, per-role view model coverage including a non-member seeing no Edit/Delete, last-owner demote and remove explained both before the call and on the server's rejection, visibility read and round-tripped, plus Compose coverage of the same rules on the rendered screen. Closes #83 --- .../ui/detail/OrganizationDetailScreenTest.kt | 226 +++++++++++----- .../data/DefaultOrganizationsRepository.kt | 20 +- .../organizations/data/OrganizationMapper.kt | 3 + .../data/local/CachedOrganizationEntity.kt | 2 + .../data/local/OrganizationsDatabase.kt | 3 +- .../data/remote/dto/OrganizationDtos.kt | 24 +- .../organizations/domain/OrgPermissions.kt | 96 +++++++ .../organizations/domain/Organization.kt | 13 +- .../ui/OrganizationsErrorMessages.kt | 36 ++- .../ui/detail/OrganizationDetailScreen.kt | 245 +++++++++++++----- .../ui/detail/OrganizationDetailViewModel.kt | 93 +++++-- .../FakeOrganizationsRepository.kt | 18 +- .../DefaultOrganizationsRepositoryTest.kt | 45 +++- .../data/OrganizationMapperTest.kt | 13 + .../domain/OrgPermissionsTest.kt | 186 +++++++++++++ .../ui/OrganizationsErrorMessagesTest.kt | 28 ++ .../detail/OrganizationDetailViewModelTest.kt | 239 +++++++++++++++++ 17 files changed, 1120 insertions(+), 170 deletions(-) create mode 100644 feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/domain/OrgPermissions.kt create mode 100644 feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/domain/OrgPermissionsTest.kt diff --git a/feature/organizations/src/androidTest/kotlin/com/interlinedlist/android/feature/organizations/ui/detail/OrganizationDetailScreenTest.kt b/feature/organizations/src/androidTest/kotlin/com/interlinedlist/android/feature/organizations/ui/detail/OrganizationDetailScreenTest.kt index 421f728..01bbae1 100644 --- a/feature/organizations/src/androidTest/kotlin/com/interlinedlist/android/feature/organizations/ui/detail/OrganizationDetailScreenTest.kt +++ b/feature/organizations/src/androidTest/kotlin/com/interlinedlist/android/feature/organizations/ui/detail/OrganizationDetailScreenTest.kt @@ -1,8 +1,10 @@ package com.interlinedlist.android.feature.organizations.ui.detail import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.assertTextContains import androidx.compose.ui.test.junit4.createComposeRule import androidx.compose.ui.test.onNodeWithTag +import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick import androidx.test.ext.junit.runners.AndroidJUnit4 import com.interlinedlist.android.core.designsystem.theme.InterlinedListTheme @@ -17,6 +19,10 @@ import org.junit.runner.RunWith * Compose UI coverage for the stateless [OrganizationDetailScreen]. Runs on-device; * the orchestrator executes instrumented tests after merge, so this is written to * compile and be correct. + * + * The role × action expectations mirror + * `https://interlinedlist.com/help/organizations`, and are asserted in plain JVM + * form by `OrgPermissionsTest`; here they are checked at the rendering level. */ @RunWith(AndroidJUnit4::class) class OrganizationDetailScreenTest { @@ -26,7 +32,9 @@ class OrganizationDetailScreenTest { private fun setScreen( state: OrganizationDetailUiState, + onChangeRole: (OrgMember, OrgRole) -> Unit = { _, _ -> }, onRemoveMember: (OrgMember) -> Unit = {}, + onSaveEdit: (String?, String?, Boolean?) -> Unit = { _, _, _ -> }, onDelete: () -> Unit = {}, onJoin: () -> Unit = {}, onLeave: () -> Unit = {}, @@ -38,9 +46,9 @@ class OrganizationDetailScreenTest { onBack = {}, onSearchQueryChange = {}, onAddCandidate = {}, - onChangeRole = { _, _ -> }, + onChangeRole = onChangeRole, onRemoveMember = onRemoveMember, - onSaveEdit = { _, _, _ -> }, + onSaveEdit = onSaveEdit, onDelete = onDelete, onJoin = onJoin, onLeave = onLeave, @@ -49,15 +57,33 @@ class OrganizationDetailScreenTest { } } - private fun loaded() = OrganizationDetailUiState( - organization = Organization("o1", "Acme Corp", "Makers", null, false, 2, OrgRole.OWNER, null), - members = listOf( - OrgMember("u1", "ada", "Ada", null, OrgRole.OWNER, active = true), - OrgMember("u2", "grace", null, null, OrgRole.MEMBER, active = true), + private fun state( + role: OrgRole?, + members: List = emptyList(), + isPublic: Boolean = false, + isSystem: Boolean = false, + ) = OrganizationDetailUiState( + organization = Organization( + id = "o1", + name = "Acme Corp", + description = "Makers", + avatarUrl = null, + isPublic = isPublic, + memberCount = members.size, + role = role, + updatedAt = null, + isSystem = isSystem, ), + members = members, isLoading = false, ) + private val ada = OrgMember("u1", "ada", "Ada", null, OrgRole.OWNER, active = true) + private val secondOwner = OrgMember("u3", "linus", null, null, OrgRole.OWNER, active = true) + private val grace = OrgMember("u2", "grace", null, null, OrgRole.MEMBER, active = true) + + private fun loaded() = state(OrgRole.OWNER, listOf(ada, secondOwner, grace)) + @Test fun rendersMembers_andSearchField() { setScreen(state = loaded()) @@ -72,8 +98,8 @@ class OrganizationDetailScreenTest { var removed: OrgMember? = null setScreen(state = loaded(), onRemoveMember = { removed = it }) - composeRule.onNodeWithTag(OrganizationDetailTestTags.remove("u1")).performClick() - assert(removed?.userId == "u1") + composeRule.onNodeWithTag(OrganizationDetailTestTags.remove("u2")).performClick() + assert(removed?.userId == "u2") } @Test @@ -91,26 +117,14 @@ class OrganizationDetailScreenTest { @Test fun showsEmptyState_whenNoMembers() { - setScreen( - state = OrganizationDetailUiState( - organization = Organization("o1", "Acme", null, null, false, 0, OrgRole.OWNER, null), - members = emptyList(), - isLoading = false, - ), - ) + setScreen(state = state(OrgRole.OWNER)) composeRule.onNodeWithTag(OrganizationDetailTestTags.EMPTY).assertIsDisplayed() } @Test fun nonMember_seesJoinPrompt_andNoMemberTools() { - setScreen( - state = OrganizationDetailUiState( - // No role: the API reports membership only for the caller's own orgs. - organization = Organization("o1", "Metals", null, null, true, 1, null, null), - isLoading = false, - ), - ) + setScreen(state = state(role = null, isPublic = true)) composeRule.onNodeWithTag(OrganizationDetailTestTags.JOIN_PROMPT).assertIsDisplayed() composeRule.onNodeWithTag(OrganizationDetailTestTags.JOIN).assertIsDisplayed() @@ -120,13 +134,7 @@ class OrganizationDetailScreenTest { @Test fun nonMember_join_reportsTheAction() { var joined = false - setScreen( - state = OrganizationDetailUiState( - organization = Organization("o1", "Metals", null, null, true, 1, null, null), - isLoading = false, - ), - onJoin = { joined = true }, - ) + setScreen(state = state(role = null, isPublic = true), onJoin = { joined = true }) composeRule.onNodeWithTag(OrganizationDetailTestTags.JOIN).performClick() assert(joined) @@ -134,12 +142,7 @@ class OrganizationDetailScreenTest { @Test fun nonMemberOfPrivateOrg_isNotOfferedJoin() { - setScreen( - state = OrganizationDetailUiState( - organization = Organization("o1", "Acme", null, null, false, 2, null, null), - isLoading = false, - ), - ) + setScreen(state = state(role = null, isPublic = false)) composeRule.onNodeWithTag(OrganizationDetailTestTags.JOIN_PROMPT).assertIsDisplayed() composeRule.onNodeWithTag(OrganizationDetailTestTags.JOIN).assertDoesNotExist() @@ -149,14 +152,7 @@ class OrganizationDetailScreenTest { fun member_leaveFlow_confirmsBeforeLeaving() { var left = false setScreen( - state = OrganizationDetailUiState( - organization = Organization("o1", "Bikey Life", null, null, true, 3, OrgRole.MEMBER, null), - members = listOf( - OrgMember("u1", "ada", "Ada", null, OrgRole.OWNER, active = true), - OrgMember("me", "me", null, null, OrgRole.MEMBER, active = true), - ), - isLoading = false, - ), + state = state(OrgRole.MEMBER, listOf(ada, grace), isPublic = true), onLeave = { left = true }, ) @@ -171,14 +167,7 @@ class OrganizationDetailScreenTest { fun soleOwner_isExplainedInsteadOfBeingAllowedToLeave() { var left = false setScreen( - state = OrganizationDetailUiState( - organization = Organization("o1", "Acme", null, null, false, 2, OrgRole.OWNER, null), - members = listOf( - OrgMember("me", "me", null, null, OrgRole.OWNER, active = true), - OrgMember("u2", "grace", null, null, OrgRole.MEMBER, active = true), - ), - isLoading = false, - ), + state = state(OrgRole.OWNER, listOf(ada, grace)), onLeave = { left = true }, ) @@ -190,16 +179,137 @@ class OrganizationDetailScreenTest { assert(!left) } + // ---- Role-gated affordances -------------------------------------------- + + @Test + fun nonMember_isOfferedNoOverflowAtAll() { + // #81 left Edit and Delete exposed to non-members; with nothing permitted + // the menu itself is gone, so neither can be reached. + setScreen(state = state(role = null, isPublic = true)) + + composeRule.onNodeWithTag(OrganizationDetailTestTags.OVERFLOW).assertDoesNotExist() + composeRule.onNodeWithTag(OrganizationDetailTestTags.EDIT).assertDoesNotExist() + composeRule.onNodeWithTag(OrganizationDetailTestTags.DELETE).assertDoesNotExist() + composeRule.onNodeWithTag(OrganizationDetailTestTags.LEAVE).assertDoesNotExist() + } + + @Test + fun member_seesLeaveButNeitherEditNorDelete() { + setScreen(state = state(OrgRole.MEMBER, listOf(ada, grace), isPublic = true)) + + composeRule.onNodeWithTag(OrganizationDetailTestTags.OVERFLOW).performClick() + composeRule.onNodeWithTag(OrganizationDetailTestTags.LEAVE).assertIsDisplayed() + composeRule.onNodeWithTag(OrganizationDetailTestTags.EDIT).assertDoesNotExist() + composeRule.onNodeWithTag(OrganizationDetailTestTags.DELETE).assertDoesNotExist() + } + + @Test + fun member_getsNoMemberManagementControls() { + setScreen(state = state(OrgRole.MEMBER, listOf(ada, grace), isPublic = true)) + + // "Member: Basic access" — the roster is visible, read-only. + composeRule.onNodeWithTag(OrganizationDetailTestTags.SEARCH).assertDoesNotExist() + composeRule.onNodeWithTag(OrganizationDetailTestTags.remove("u2")).assertDoesNotExist() + composeRule.onNodeWithTag(OrganizationDetailTestTags.roleChip("u2", OrgRole.ADMIN)) + .assertDoesNotExist() + composeRule.onNodeWithTag(OrganizationDetailTestTags.roleLabel("u2")).assertIsDisplayed() + } + + @Test + fun admin_mayEditButNotDelete() { + setScreen(state = state(OrgRole.ADMIN, listOf(ada, grace), isPublic = true)) + + composeRule.onNodeWithTag(OrganizationDetailTestTags.OVERFLOW).performClick() + composeRule.onNodeWithTag(OrganizationDetailTestTags.EDIT).assertIsDisplayed() + // "Owner: … can delete the org" — an admin cannot. + composeRule.onNodeWithTag(OrganizationDetailTestTags.DELETE).assertDoesNotExist() + } + @Test - fun nonMember_isNotOfferedLeave() { + fun admin_mayNotManageAnOwner_butMayManageOthers() { + setScreen(state = state(OrgRole.ADMIN, listOf(ada, grace), isPublic = true)) + + // "Admin: Can add and remove members and change roles (except owner)" + composeRule.onNodeWithTag(OrganizationDetailTestTags.remove("u1")).assertDoesNotExist() + composeRule.onNodeWithTag(OrganizationDetailTestTags.roleLabel("u1")).assertIsDisplayed() + + composeRule.onNodeWithTag(OrganizationDetailTestTags.remove("u2")).assertIsDisplayed() + composeRule.onNodeWithTag(OrganizationDetailTestTags.roleChip("u2", OrgRole.ADMIN)) + .assertIsDisplayed() + // An admin cannot hand out ownership. + composeRule.onNodeWithTag(OrganizationDetailTestTags.roleChip("u2", OrgRole.OWNER)) + .assertDoesNotExist() + } + + @Test + fun owner_mayGrantOwnership() { + setScreen(state = loaded()) + + composeRule.onNodeWithTag(OrganizationDetailTestTags.roleChip("u2", OrgRole.OWNER)) + .assertIsDisplayed() + } + + @Test + fun theOnlyOwner_isOfferedNeitherDemotionNorRemoval() { + // Ada is the single owner: the server refuses both, so neither is offered. + setScreen(state = state(OrgRole.OWNER, listOf(ada, grace))) + + composeRule.onNodeWithTag(OrganizationDetailTestTags.remove("u1")).assertDoesNotExist() + composeRule.onNodeWithTag(OrganizationDetailTestTags.roleChip("u1", OrgRole.MEMBER)) + .assertDoesNotExist() + composeRule.onNodeWithTag(OrganizationDetailTestTags.roleChip("u1", OrgRole.OWNER)) + .assertIsDisplayed() + composeRule.onNodeWithTag(OrganizationDetailTestTags.MEMBER_LAST_OWNER_NOTICE) + .assertIsDisplayed() + } + + @Test + fun systemOrganization_offersNoLeaveOrDelete() { + // "You cannot leave the system \"The Public\" organization." + setScreen(state = state(OrgRole.OWNER, listOf(ada, secondOwner), isPublic = true, isSystem = true)) + + composeRule.onNodeWithTag(OrganizationDetailTestTags.SYSTEM).assertIsDisplayed() + composeRule.onNodeWithTag(OrganizationDetailTestTags.OVERFLOW).performClick() + composeRule.onNodeWithTag(OrganizationDetailTestTags.LEAVE).assertDoesNotExist() + composeRule.onNodeWithTag(OrganizationDetailTestTags.DELETE).assertDoesNotExist() + // Editing is still a role question, and an owner may. + composeRule.onNodeWithTag(OrganizationDetailTestTags.EDIT).assertIsDisplayed() + } + + // ---- Visibility --------------------------------------------------------- + + @Test + fun visibility_isShownOnTheHeader() { + setScreen(state = state(OrgRole.MEMBER, listOf(ada, grace), isPublic = true)) + + composeRule.onNodeWithTag(OrganizationDetailTestTags.VISIBILITY) + .assertTextContains("Public · anyone can see and join") + composeRule.onNodeWithTag(OrganizationDetailTestTags.VIEWER_ROLE) + .assertTextContains("Your role: Member") + } + + @Test + fun privateVisibility_isShownOnTheHeader() { + setScreen(state = state(OrgRole.MEMBER, listOf(ada, grace), isPublic = false)) + + composeRule.onNodeWithTag(OrganizationDetailTestTags.VISIBILITY) + .assertTextContains("Private · invite-only; an owner or admin adds members") + } + + @Test + fun owner_canToggleVisibility_andItRoundTripsToTheSave() { + var saved: Triple? = null setScreen( - state = OrganizationDetailUiState( - organization = Organization("o1", "Metals", null, null, true, 1, null, null), - isLoading = false, - ), + state = state(OrgRole.OWNER, listOf(ada, secondOwner), isPublic = false), + onSaveEdit = { name, description, isPublic -> saved = Triple(name, description, isPublic) }, ) composeRule.onNodeWithTag(OrganizationDetailTestTags.OVERFLOW).performClick() - composeRule.onNodeWithTag(OrganizationDetailTestTags.LEAVE).assertDoesNotExist() + composeRule.onNodeWithTag(OrganizationDetailTestTags.EDIT).performClick() + composeRule.onNodeWithTag(OrganizationDetailTestTags.EDIT_DIALOG).assertIsDisplayed() + composeRule.onNodeWithTag(OrganizationDetailTestTags.EDIT_VISIBILITY).performClick() + composeRule.onNodeWithText("Save").performClick() + + assert(saved?.third == true) { "expected the toggled visibility to reach the save, got $saved" } } } diff --git a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/DefaultOrganizationsRepository.kt b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/DefaultOrganizationsRepository.kt index 0f7e61f..0f56608 100644 --- a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/DefaultOrganizationsRepository.kt +++ b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/DefaultOrganizationsRepository.kt @@ -73,7 +73,7 @@ class DefaultOrganizationsRepository @Inject constructor( val body = CreateOrganizationRequest( name = name, description = description, - isPublic = isPublic.toString(), + isPublic = isPublic, ) when (val result = safeApiCall(json) { api.createOrganization(body) }) { is ApiResult.Success -> { @@ -115,20 +115,14 @@ class DefaultOrganizationsRepository @Inject constructor( val body = UpdateOrganizationRequest( name = name?.trim()?.ifBlank { null }, description = description?.trim(), - isPublic = isPublic?.toString(), + isPublic = isPublic, ) when (val result = safeApiCall(json) { api.updateOrganization(id, body) }) { - is ApiResult.Success -> { - // The API may echo the updated org; if not, re-fetch it for a fresh cache. - val dto = result.data.org - if (dto != null) { - val org = OrganizationMapper.fromDto(dto) - dao.upsert(OrganizationMapper.toEntity(org)) - ApiResult.Success(org) - } else { - getOrganization(id) - } - } + // The PUT echo omits `userRole` and `memberCount` (verified live), so + // mapping it straight through would make the editor look like a + // non-member and hide the very actions they just used. Re-read instead, + // which is authoritative and refreshes the cache. + is ApiResult.Success -> getOrganization(id) is ApiResult.Failure -> result } } diff --git a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/OrganizationMapper.kt b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/OrganizationMapper.kt index 0b95ba0..61a011b 100644 --- a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/OrganizationMapper.kt +++ b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/OrganizationMapper.kt @@ -17,6 +17,7 @@ object OrganizationMapper { memberCount = dto.resolvedMemberCount, role = dto.resolvedRole?.let(OrgRole::fromApi), updatedAt = dto.updatedAt, + isSystem = dto.isSystem ?: false, ) fun toEntity(org: Organization): CachedOrganizationEntity = CachedOrganizationEntity( @@ -28,6 +29,7 @@ object OrganizationMapper { memberCount = org.memberCount, role = org.role?.apiValue, updatedAt = org.updatedAt, + isSystem = org.isSystem, ) fun fromEntity(entity: CachedOrganizationEntity): Organization = Organization( @@ -39,5 +41,6 @@ object OrganizationMapper { memberCount = entity.memberCount, role = entity.role?.let(OrgRole::fromApi), updatedAt = entity.updatedAt, + isSystem = entity.isSystem, ) } diff --git a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/local/CachedOrganizationEntity.kt b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/local/CachedOrganizationEntity.kt index bccd174..dc2ddc4 100644 --- a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/local/CachedOrganizationEntity.kt +++ b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/local/CachedOrganizationEntity.kt @@ -18,4 +18,6 @@ data class CachedOrganizationEntity( val memberCount: Int, val role: String?, val updatedAt: String?, + /** Built-in organization ("The Public"): not leavable, not deletable. */ + val isSystem: Boolean = false, ) diff --git a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/local/OrganizationsDatabase.kt b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/local/OrganizationsDatabase.kt index 2c62e1f..b2b4c1d 100644 --- a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/local/OrganizationsDatabase.kt +++ b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/local/OrganizationsDatabase.kt @@ -10,7 +10,8 @@ import androidx.room.RoomDatabase */ @Database( entities = [CachedOrganizationEntity::class], - version = 1, + // v2 adds `isSystem`; the cache is disposable (destructive migration). + version = 2, exportSchema = false, ) abstract class OrganizationsDatabase : RoomDatabase() { diff --git a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/remote/dto/OrganizationDtos.kt b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/remote/dto/OrganizationDtos.kt index a7aa5d1..84623d3 100644 --- a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/remote/dto/OrganizationDtos.kt +++ b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/remote/dto/OrganizationDtos.kt @@ -26,6 +26,9 @@ data class OrganizationDto( val isPublic: Boolean? = null, @Serializable(with = FlexibleBooleanSerializer::class) val public: Boolean? = null, + // Built-in organizations ("The Public"); present on every index and detail row. + @Serializable(with = FlexibleBooleanSerializer::class) + val isSystem: Boolean? = null, // The API reports the member count under a few different names. val memberCount: Int? = null, val membersCount: Int? = null, @@ -84,13 +87,19 @@ data class OrganizationEnvelope( val org: OrganizationDto? get() = organization ?: data } -/** Body for `POST /api/organizations`. `isPublic` is serialised as a string per the API. */ +/** + * Body for `POST /api/organizations`. + * + * `isPublic` must be a JSON **boolean**. Verified live: sending it as a string + * (`"false"`) answers `500 {"error":"Internal server error"}`, so the visibility + * chosen at creation never reached the server. + */ @Serializable data class CreateOrganizationRequest( val name: String, val description: String? = null, val avatar: String? = null, - val isPublic: String? = null, + val isPublic: Boolean? = null, ) /** @@ -103,11 +112,18 @@ data class JoinOrganizationRequest( val organizationId: String, ) -/** Body for `PUT /api/organizations/{id}` — partial metadata updates. */ +/** + * Body for `PUT /api/organizations/{id}` — partial metadata updates. + * + * `isPublic` must be a JSON **boolean**, exactly as for create: a string answers + * `500`. Verified live against `PUT /api/organizations/{id}`, which echoes the + * updated organization back under `organization` — but *without* `userRole` or + * `memberCount`, so the caller has to re-read to keep its membership state. + */ @Serializable data class UpdateOrganizationRequest( val name: String? = null, val description: String? = null, val avatar: String? = null, - val isPublic: String? = null, + val isPublic: Boolean? = null, ) diff --git a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/domain/OrgPermissions.kt b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/domain/OrgPermissions.kt new file mode 100644 index 0000000..31ecb66 --- /dev/null +++ b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/domain/OrgPermissions.kt @@ -0,0 +1,96 @@ +package com.interlinedlist.android.feature.organizations.domain + +/** + * What the signed-in user may do in one organization, derived from the role the API + * reports for them. The rules are taken verbatim from the help centre + * (`https://interlinedlist.com/help/organizations`) — nothing here is inferred: + * + * - "Owner: Full control; can delete the org and manage all members" + * - "Admin: Can add and remove members and change roles (except owner)" + * - "Member: Basic access" + * - "Private: Invite-only; members must be added by an owner or admin" + * - "Public: Anyone can see and join" + * - "You cannot leave the system \"The Public\" organization." + * + * Membership itself is signalled by the presence of a role: the API omits `role` for + * organizations the caller does not belong to, and reports `userRole: null` on the + * detail endpoint. + * + * This gates the UI only. Hiding an action is a usability improvement, not a + * security boundary — every mutation still goes to the server, which stays + * authoritative and whose rejections are surfaced to the user. + */ +data class OrgPermissions( + /** The signed-in user's role, or `null` when they are not a member. */ + val viewerRole: OrgRole?, + val isPublic: Boolean, + /** A built-in organization such as "The Public"; it cannot be left or deleted. */ + val isSystem: Boolean, +) { + + val isMember: Boolean get() = viewerRole != null + + /** + * Owners and admins administer the organization; the help centre addresses both + * as "an owner or admin" everywhere it describes management. + */ + private val administers: Boolean + get() = viewerRole == OrgRole.OWNER || viewerRole == OrgRole.ADMIN + + /** `GET /api/organizations/{id}/members` is members-only (403 otherwise). */ + val canViewMembers: Boolean get() = isMember + + val canAddMember: Boolean get() = administers + + /** Name, description and avatar — "an owner or admin" manages the organization. */ + val canEditOrganization: Boolean get() = administers + + /** Visibility rides on the same `PUT`, so it follows the edit rule exactly. */ + val canEditVisibility: Boolean get() = canEditOrganization + + /** "Owner: … can delete the org" — and a system organization is never deletable. */ + val canDeleteOrganization: Boolean get() = viewerRole == OrgRole.OWNER && !isSystem + + /** Any member may leave, except from a system organization. */ + val canLeave: Boolean get() = isMember && !isSystem + + /** "Public: Anyone can see and join"; joining a private org answers 403. */ + val canJoin: Boolean get() = !isMember && isPublic + + /** True when any organization-level action is available, so the menu is worth showing. */ + val hasAnyOrganizationAction: Boolean + get() = canEditOrganization || canDeleteOrganization || canLeave + + /** An admin may not act on an owner: "change roles (except owner)". */ + private fun canManage(targetRole: OrgRole): Boolean = when (viewerRole) { + OrgRole.OWNER -> true + OrgRole.ADMIN -> targetRole != OrgRole.OWNER + else -> false + } + + /** Whether the role of a member currently holding [targetRole] may be changed. */ + fun canChangeRoleOf(targetRole: OrgRole): Boolean = canManage(targetRole) + + /** Whether a member currently holding [targetRole] may be removed. */ + fun canRemove(targetRole: OrgRole): Boolean = canManage(targetRole) + + /** + * The roles this viewer may assign to a member currently holding [targetRole], + * least- to most-privileged. Empty when the member is out of reach. An admin + * cannot hand out ownership, so `owner` is withheld from them. + */ + fun assignableRolesFor(targetRole: OrgRole): List = when { + !canManage(targetRole) -> emptyList() + viewerRole == OrgRole.OWNER -> OrgRole.entries + else -> OrgRole.entries.filterNot { it == OrgRole.OWNER } + } + + companion object { + /** Permits nothing — the state before an organization has loaded. */ + val NONE = OrgPermissions(viewerRole = null, isPublic = false, isSystem = false) + + fun of(organization: Organization?): OrgPermissions = organization?.let { + OrgPermissions(viewerRole = it.role, isPublic = it.isPublic, isSystem = it.isSystem) + } ?: NONE + } +} diff --git a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/domain/Organization.kt b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/domain/Organization.kt index 3c3716a..af0e089 100644 --- a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/domain/Organization.kt +++ b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/domain/Organization.kt @@ -16,19 +16,28 @@ data class Organization( /** The current user's role in this org, when the API reports it. */ val role: OrgRole?, val updatedAt: String?, + /** + * A built-in organization such as "The Public", which every account belongs to. + * The help centre states it cannot be left; it is not deletable either. + * Declared last with a default so existing call sites stay positional. + */ + val isSystem: Boolean = false, ) { /** Best label for a card: the name, falling back to a placeholder. */ val displayName: String get() = name.ifBlank { "Untitled organization" } + /** What the signed-in user may do here, per the documented role model. */ + val permissions: OrgPermissions get() = OrgPermissions.of(this) + /** * Whether the signed-in user belongs to this organization. The API reports a * role only for the caller's own memberships, so an absent role means "not a * member" — that drives the Join/Leave affordance. */ - val isMember: Boolean get() = role != null + val isMember: Boolean get() = permissions.isMember /** A public organization the user has not joined can be joined from the UI. */ - val canJoin: Boolean get() = isPublic && !isMember + val canJoin: Boolean get() = permissions.canJoin } /** diff --git a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/ui/OrganizationsErrorMessages.kt b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/ui/OrganizationsErrorMessages.kt index 6543274..6a93971 100644 --- a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/ui/OrganizationsErrorMessages.kt +++ b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/ui/OrganizationsErrorMessages.kt @@ -17,9 +17,11 @@ fun AppError.toUserMessage(): String = when (this) { val AppError.isSubscriptionGate: Boolean get() = this is AppError.SubscriptionRequired /** - * The server's guard against orphaning an organization: a sole owner may not leave - * (400 `{"error":"Cannot remove the last owner"}`). Detected on the message because - * the status code is a plain bad request. + * The server's guard against orphaning an organization. Both halves are plain 400s + * with `code: "bad_request"`, so they are detected on the message (verified live): + * + * - `{"error":"Cannot remove the last owner"}` — removing, or leaving as, the sole owner + * - `{"error":"Cannot demote the last owner"}` — changing the sole owner's role */ val AppError.isLastOwnerRejection: Boolean get() = message?.contains("last owner", ignoreCase = true) == true @@ -49,3 +51,31 @@ fun AppError.toLeaveMessage(): String = when { const val LAST_OWNER_EXPLANATION: String = "You're the only owner of this organization. Make another member an owner, " + "or delete the organization, before you leave." + +/** + * Explains why a role change was refused. The server refuses to demote the only + * owner, which would leave the organization without one. + */ +fun AppError.toRoleChangeMessage(): String = when { + isLastOwnerRejection -> LAST_OWNER_DEMOTE_EXPLANATION + else -> toUserMessage() +} + +/** Explains why removing a member was refused — most often the last-owner guard. */ +fun AppError.toRemoveMemberMessage(): String = when { + isLastOwnerRejection -> LAST_OWNER_REMOVE_EXPLANATION + else -> toUserMessage() +} + +/** + * Shown both before the attempt (the UI can see there is only one owner) and after + * the server refuses with "Cannot demote the last owner", so the paths read alike. + */ +const val LAST_OWNER_DEMOTE_EXPLANATION: String = + "This is the organization's only owner. Make someone else an owner first, " + + "then you can change this role." + +/** The removal counterpart of [LAST_OWNER_DEMOTE_EXPLANATION]. */ +const val LAST_OWNER_REMOVE_EXPLANATION: String = + "This is the organization's only owner. Make someone else an owner first, " + + "then you can remove them." diff --git a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/ui/detail/OrganizationDetailScreen.kt b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/ui/detail/OrganizationDetailScreen.kt index f2aefe7..2b043df 100644 --- a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/ui/detail/OrganizationDetailScreen.kt +++ b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/ui/detail/OrganizationDetailScreen.kt @@ -80,9 +80,20 @@ object OrganizationDetailTestTags { const val LEAVE_DIALOG = "orgDetailLeaveDialog" const val LEAVE_CONFIRM = "orgDetailLeaveConfirm" const val LAST_OWNER_NOTICE = "orgDetailLastOwnerNotice" + const val MEMBER_LAST_OWNER_NOTICE = "orgMemberLastOwnerNotice" + const val VISIBILITY = "orgDetailVisibility" + const val VIEWER_ROLE = "orgDetailViewerRole" + const val SYSTEM = "orgDetailSystem" + const val EDIT_VISIBILITY = "orgDetailEditVisibility" fun member(userId: String) = "orgMember_$userId" fun remove(userId: String) = "orgMemberRemove_$userId" fun candidate(userId: String) = "orgCandidate_$userId" + + /** A tappable role chip on a member row; absent when that role is not assignable. */ + fun roleChip(userId: String, role: OrgRole) = "orgMemberRole_${userId}_${role.apiValue}" + + /** The read-only role label shown when the viewer may not change this member's role. */ + fun roleLabel(userId: String) = "orgMemberRoleLabel_$userId" } /** @@ -134,6 +145,7 @@ fun OrganizationDetailScreen( var showEdit by remember { mutableStateOf(false) } var showDeleteConfirm by remember { mutableStateOf(false) } var showLeaveConfirm by remember { mutableStateOf(false) } + val permissions = state.permissions Scaffold( modifier = modifier.fillMaxSize(), @@ -146,32 +158,40 @@ fun OrganizationDetailScreen( } }, actions = { - IconButton( - onClick = { menuOpen = true }, - modifier = Modifier.testTag(OrganizationDetailTestTags.OVERFLOW), - ) { Icon(Icons.Default.MoreVert, contentDescription = "More actions") } - DropdownMenu(expanded = menuOpen, onDismissRequest = { menuOpen = false }) { - DropdownMenuItem( - text = { Text("Edit") }, - leadingIcon = { Icon(Icons.Default.Edit, contentDescription = null) }, - onClick = { menuOpen = false; showEdit = true }, - modifier = Modifier.testTag(OrganizationDetailTestTags.EDIT), - ) - DropdownMenuItem( - text = { Text("Delete") }, - leadingIcon = { Icon(Icons.Default.Delete, contentDescription = null) }, - onClick = { menuOpen = false; showDeleteConfirm = true }, - modifier = Modifier.testTag(OrganizationDetailTestTags.DELETE), - ) - if (state.isMember) { - DropdownMenuItem( - text = { Text("Leave organization") }, - leadingIcon = { - Icon(Icons.AutoMirrored.Filled.Logout, contentDescription = null) - }, - onClick = { menuOpen = false; showLeaveConfirm = true }, - modifier = Modifier.testTag(OrganizationDetailTestTags.LEAVE), - ) + // Only the actions this role may take are offered; with none + // left there is nothing to open, so the menu itself is dropped. + if (permissions.hasAnyOrganizationAction) { + IconButton( + onClick = { menuOpen = true }, + modifier = Modifier.testTag(OrganizationDetailTestTags.OVERFLOW), + ) { Icon(Icons.Default.MoreVert, contentDescription = "More actions") } + DropdownMenu(expanded = menuOpen, onDismissRequest = { menuOpen = false }) { + if (permissions.canEditOrganization) { + DropdownMenuItem( + text = { Text("Edit") }, + leadingIcon = { Icon(Icons.Default.Edit, contentDescription = null) }, + onClick = { menuOpen = false; showEdit = true }, + modifier = Modifier.testTag(OrganizationDetailTestTags.EDIT), + ) + } + if (permissions.canDeleteOrganization) { + DropdownMenuItem( + text = { Text("Delete") }, + leadingIcon = { Icon(Icons.Default.Delete, contentDescription = null) }, + onClick = { menuOpen = false; showDeleteConfirm = true }, + modifier = Modifier.testTag(OrganizationDetailTestTags.DELETE), + ) + } + if (permissions.canLeave) { + DropdownMenuItem( + text = { Text("Leave organization") }, + leadingIcon = { + Icon(Icons.AutoMirrored.Filled.Logout, contentDescription = null) + }, + onClick = { menuOpen = false; showLeaveConfirm = true }, + modifier = Modifier.testTag(OrganizationDetailTestTags.LEAVE), + ) + } } } }, @@ -212,23 +232,25 @@ fun OrganizationDetailScreen( // The members endpoint is members-only, so a non-member is offered // the join action instead of member management. - if (state.isMember) { - OutlinedTextField( - value = state.searchQuery, - onValueChange = onSearchQueryChange, - label = { Text("Add a member") }, - singleLine = true, - leadingIcon = { Icon(Icons.Default.Search, contentDescription = null) }, - modifier = Modifier - .fillMaxWidth() - .padding(horizontal = 16.dp, vertical = 8.dp) - .testTag(OrganizationDetailTestTags.SEARCH), - ) + if (permissions.canViewMembers) { + // Only owners and admins may add a member, so only they get the + // picker; everyone else sees the roster read-only. + if (permissions.canAddMember) { + OutlinedTextField( + value = state.searchQuery, + onValueChange = onSearchQueryChange, + label = { Text("Add a member") }, + singleLine = true, + leadingIcon = { Icon(Icons.Default.Search, contentDescription = null) }, + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = 16.dp, vertical = 8.dp) + .testTag(OrganizationDetailTestTags.SEARCH), + ) + } MemberList( - members = state.members, - candidates = state.candidates, - isEmpty = state.isEmpty, + state = state, onAddCandidate = onAddCandidate, onChangeRole = onChangeRole, onRemoveMember = onRemoveMember, @@ -240,7 +262,7 @@ fun OrganizationDetailScreen( } } - if (showEdit && state.organization != null) { + if (showEdit && state.organization != null && permissions.canEditOrganization) { EditOrganizationDialog( organization = state.organization, onDismiss = { showEdit = false }, @@ -359,6 +381,12 @@ private fun JoinPrompt(canJoin: Boolean, isJoining: Boolean, onJoin: () -> Unit) } } +/** + * Metadata header. Visibility is stated outright, with the help centre's own + * wording for what it means ("Public: Anyone can see and join" / "Private: + * Invite-only; members must be added by an owner or admin"), because it decides + * who can find and join the organization. + */ @Composable private fun OrganizationHeader(org: Organization) { Column(Modifier.padding(horizontal = 16.dp, vertical = 8.dp)) { @@ -376,10 +404,32 @@ private fun OrganizationHeader(org: Organization) { style = MaterialTheme.typography.labelMedium, color = MaterialTheme.colorScheme.primary, ) + org.role?.let { role -> + Text( + text = "Your role: ${role.label}", + style = MaterialTheme.typography.labelMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.testTag(OrganizationDetailTestTags.VIEWER_ROLE), + ) + } + } + Spacer(Modifier.height(4.dp)) + Text( + text = if (org.isPublic) { + "Public · anyone can see and join" + } else { + "Private · invite-only; an owner or admin adds members" + }, + style = MaterialTheme.typography.labelMedium, + color = MaterialTheme.colorScheme.secondary, + modifier = Modifier.testTag(OrganizationDetailTestTags.VISIBILITY), + ) + if (org.isSystem) { Text( - text = if (org.isPublic) "Public" else "Private", + text = "Built-in organization · everyone belongs to it", style = MaterialTheme.typography.labelMedium, - color = MaterialTheme.colorScheme.secondary, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.testTag(OrganizationDetailTestTags.SYSTEM), ) } } @@ -387,9 +437,7 @@ private fun OrganizationHeader(org: Organization) { @Composable private fun MemberList( - members: List, - candidates: List, - isEmpty: Boolean, + state: OrganizationDetailUiState, onAddCandidate: (MemberCandidate) -> Unit, onChangeRole: (OrgMember, OrgRole) -> Unit, onRemoveMember: (OrgMember) -> Unit, @@ -401,19 +449,22 @@ private fun MemberList( contentPadding = PaddingValues(16.dp), verticalArrangement = Arrangement.spacedBy(12.dp), ) { - if (candidates.isNotEmpty()) { + if (state.candidates.isNotEmpty()) { item { Text("Suggestions", style = MaterialTheme.typography.labelLarge) } - items(candidates, key = { "candidate-${it.userId}" }) { candidate -> + items(state.candidates, key = { "candidate-${it.userId}" }) { candidate -> CandidateRow(candidate = candidate, onAdd = { onAddCandidate(candidate) }) } } - if (isEmpty && candidates.isEmpty()) { - item { EmptyState() } + if (state.isEmpty && state.candidates.isEmpty()) { + item { EmptyState(canAddMember = state.permissions.canAddMember) } } else { - items(members, key = { it.userId }) { member -> + items(state.members, key = { it.userId }) { member -> MemberRow( member = member, + assignableRoles = state.assignableRolesFor(member), + canRemove = state.canRemove(member) && !state.isOnlyOwner(member), + isOnlyOwner = state.isOnlyOwner(member), onChangeRole = { onChangeRole(member, it) }, onRemove = { onRemoveMember(member) }, ) @@ -422,10 +473,19 @@ private fun MemberList( } } +/** + * One member. The role chips are limited to what the viewer may actually assign — + * an admin cannot touch an owner or hand out ownership — and the organization's + * only owner is offered no demotion at all, because the server refuses it. When the + * viewer may change nothing, the role is shown as a plain label instead. + */ @OptIn(ExperimentalMaterial3Api::class) @Composable private fun MemberRow( member: OrgMember, + assignableRoles: List, + canRemove: Boolean, + isOnlyOwner: Boolean, onChangeRole: (OrgRole) -> Unit, onRemove: () -> Unit, ) { @@ -452,21 +512,44 @@ private fun MemberRow( color = MaterialTheme.colorScheme.onSurfaceVariant, ) } - IconButton( - onClick = onRemove, - modifier = Modifier.testTag(OrganizationDetailTestTags.remove(member.userId)), - ) { Icon(Icons.Default.Close, contentDescription = "Remove member") } + if (canRemove) { + IconButton( + onClick = onRemove, + modifier = Modifier.testTag(OrganizationDetailTestTags.remove(member.userId)), + ) { Icon(Icons.Default.Close, contentDescription = "Remove member") } + } } Spacer(Modifier.height(8.dp)) - Row(horizontalArrangement = Arrangement.spacedBy(8.dp)) { - OrgRole.entries.forEach { role -> - FilterChip( - selected = member.role == role, - onClick = { onChangeRole(role) }, - label = { Text(role.label) }, - ) + if (assignableRoles.isEmpty()) { + Text( + text = member.role.label, + style = MaterialTheme.typography.labelLarge, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.testTag(OrganizationDetailTestTags.roleLabel(member.userId)), + ) + } else { + Row(horizontalArrangement = Arrangement.spacedBy(8.dp)) { + assignableRoles.forEach { role -> + FilterChip( + selected = member.role == role, + onClick = { onChangeRole(role) }, + label = { Text(role.label) }, + modifier = Modifier.testTag( + OrganizationDetailTestTags.roleChip(member.userId, role), + ), + ) + } } } + if (isOnlyOwner) { + Spacer(Modifier.height(4.dp)) + Text( + text = "The only owner can't be demoted or removed.", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.testTag(OrganizationDetailTestTags.MEMBER_LAST_OWNER_NOTICE), + ) + } } } } @@ -501,6 +584,12 @@ private fun CandidateRow(candidate: MemberCandidate, onAdd: () -> Unit) { } } +/** + * Edits name, description and visibility. Visibility travels on the same + * `PUT /api/organizations/{id}` as the rest, so + * [com.interlinedlist.android.feature.organizations.domain.OrgPermissions.canEditVisibility] + * matches `canEditOrganization` and this dialog only opens for roles that hold it. + */ @Composable private fun EditOrganizationDialog( organization: Organization, @@ -533,8 +622,23 @@ private fun EditOrganizationDialog( verticalAlignment = Alignment.CenterVertically, horizontalArrangement = Arrangement.SpaceBetween, ) { - Text("Public", style = MaterialTheme.typography.bodyLarge) - Switch(checked = isPublic, onCheckedChange = { isPublic = it }) + Column(Modifier.weight(1f)) { + Text("Public", style = MaterialTheme.typography.bodyLarge) + Text( + text = if (isPublic) { + "Anyone can see and join." + } else { + "Invite-only; an owner or admin adds members." + }, + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } + Switch( + checked = isPublic, + onCheckedChange = { isPublic = it }, + modifier = Modifier.testTag(OrganizationDetailTestTags.EDIT_VISIBILITY), + ) } Row( modifier = Modifier.fillMaxWidth(), @@ -552,7 +656,7 @@ private fun EditOrganizationDialog( } @Composable -private fun EmptyState() { +private fun EmptyState(canAddMember: Boolean) { Box( modifier = Modifier .fillMaxWidth() @@ -564,7 +668,11 @@ private fun EmptyState() { Text("No members yet", style = MaterialTheme.typography.titleMedium) Spacer(Modifier.height(4.dp)) Text( - "Search above to add someone.", + text = if (canAddMember) { + "Search above to add someone." + } else { + "Only an owner or admin can add members." + }, style = MaterialTheme.typography.bodyMedium, color = MaterialTheme.colorScheme.onSurfaceVariant, ) @@ -585,9 +693,10 @@ private fun OrganizationDetailScreenPreview() { InterlinedListTheme { OrganizationDetailScreen( state = OrganizationDetailUiState( - organization = Organization("1", "Acme Corp", "We make everything", null, false, 2, OrgRole.OWNER, null), + organization = Organization("1", "Acme Corp", "We make everything", null, false, 3, OrgRole.OWNER, null), members = listOf( OrgMember("u1", "ada", "Ada Lovelace", null, OrgRole.OWNER, active = true), + OrgMember("u3", "linus", null, null, OrgRole.OWNER, active = true), OrgMember("u2", "grace", null, null, OrgRole.MEMBER, active = true), ), isLoading = false, diff --git a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/ui/detail/OrganizationDetailViewModel.kt b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/ui/detail/OrganizationDetailViewModel.kt index 7e3b423..d050661 100644 --- a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/ui/detail/OrganizationDetailViewModel.kt +++ b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/ui/detail/OrganizationDetailViewModel.kt @@ -7,12 +7,17 @@ import com.interlinedlist.android.core.common.result.ApiResult import com.interlinedlist.android.feature.organizations.data.OrganizationsRepository import com.interlinedlist.android.feature.organizations.domain.MemberCandidate import com.interlinedlist.android.feature.organizations.domain.OrgMember +import com.interlinedlist.android.feature.organizations.domain.OrgPermissions import com.interlinedlist.android.feature.organizations.domain.OrgRole import com.interlinedlist.android.feature.organizations.domain.Organization +import com.interlinedlist.android.feature.organizations.ui.LAST_OWNER_DEMOTE_EXPLANATION import com.interlinedlist.android.feature.organizations.ui.LAST_OWNER_EXPLANATION +import com.interlinedlist.android.feature.organizations.ui.LAST_OWNER_REMOVE_EXPLANATION import com.interlinedlist.android.feature.organizations.ui.isSubscriptionGate import com.interlinedlist.android.feature.organizations.ui.toJoinMessage import com.interlinedlist.android.feature.organizations.ui.toLeaveMessage +import com.interlinedlist.android.feature.organizations.ui.toRemoveMemberMessage +import com.interlinedlist.android.feature.organizations.ui.toRoleChangeMessage import com.interlinedlist.android.feature.organizations.ui.toUserMessage import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.MutableStateFlow @@ -45,11 +50,20 @@ data class OrganizationDetailUiState( val title: String get() = organization?.displayName.orEmpty() val isEmpty: Boolean get() = members.isEmpty() && !isLoading && errorMessage == null && isMember + /** + * What the signed-in user's role permits here. Drives which affordances render; + * the server remains authoritative for every mutation. + */ + val permissions: OrgPermissions get() = OrgPermissions.of(organization) + /** Whether the signed-in user belongs to this organization. */ - val isMember: Boolean get() = organization?.isMember == true + val isMember: Boolean get() = permissions.isMember /** A public organization the user has not joined can be joined from here. */ - val canJoin: Boolean get() = organization?.canJoin == true + val canJoin: Boolean get() = permissions.canJoin + + /** How many owners the loaded member list holds; the server protects the last one. */ + private val ownerCount: Int get() = members.count { it.role == OrgRole.OWNER } /** * True when the user is this organization's only owner. Leaving would orphan @@ -57,9 +71,29 @@ data class OrganizationDetailUiState( * owner"), so the UI explains it up front instead of failing. Requires a loaded * member list; without one the server's rejection is the backstop. */ - val isLastOwner: Boolean - get() = organization?.role == OrgRole.OWNER && - members.count { it.role == OrgRole.OWNER } == 1 + val isLastOwner: Boolean get() = organization?.role == OrgRole.OWNER && ownerCount == 1 + + /** + * True when [member] is the organization's only owner, so demoting or removing + * them is refused by the server ("Cannot demote/remove the last owner"). + */ + fun isOnlyOwner(member: OrgMember): Boolean = + member.role == OrgRole.OWNER && ownerCount == 1 + + /** Whether [member]'s role may be changed *and* the change would be accepted. */ + fun canChangeRoleOf(member: OrgMember): Boolean = permissions.canChangeRoleOf(member.role) + + /** Whether [member] may be removed by the signed-in user. */ + fun canRemove(member: OrgMember): Boolean = permissions.canRemove(member.role) + + /** + * Roles offered for [member]. The only owner may not be demoted, so they are + * offered `owner` alone rather than chips that would be rejected. + */ + fun assignableRolesFor(member: OrgMember): List { + val assignable = permissions.assignableRolesFor(member.role) + return if (isOnlyOwner(member)) assignable.filter { it == OrgRole.OWNER } else assignable + } } @HiltViewModel @@ -100,7 +134,7 @@ class OrganizationDetailViewModel @Inject constructor( } // Members are members-only on the server (403 otherwise), so a // non-member sees the join prompt rather than a permission error. - if (organization?.isMember != true) { + if (!OrgPermissions.of(organization).canViewMembers) { _uiState.update { it.copy(members = emptyList()) } return@launch } @@ -115,7 +149,7 @@ class OrganizationDetailViewModel @Inject constructor( /** Joins this (public) organization, then reloads so membership state is server-truth. */ fun join() { - if (_uiState.value.isJoining) return + if (_uiState.value.isJoining || !_uiState.value.canJoin) return _uiState.update { it.copy(isJoining = true, errorMessage = null) } viewModelScope.launch { when (val result = repository.joinOrganization(orgId)) { @@ -137,7 +171,7 @@ class OrganizationDetailViewModel @Inject constructor( */ fun leave(onLeft: () -> Unit = {}) { val state = _uiState.value - if (state.isLeaving) return + if (state.isLeaving || !state.permissions.canLeave) return if (state.isLastOwner) { _uiState.update { it.copy(errorMessage = LAST_OWNER_EXPLANATION) } return @@ -156,12 +190,17 @@ class OrganizationDetailViewModel @Inject constructor( } } + /** + * Saves name, description and visibility. Visibility is sent on the same `PUT`, + * so it is gated by the same permission. + */ fun updateOrganization( name: String?, description: String?, isPublic: Boolean?, onDone: () -> Unit = {}, ) { + if (!_uiState.value.permissions.canEditOrganization) return _uiState.update { it.copy(isSaving = true) } viewModelScope.launch { when (val result = repository.updateOrganization(orgId, name, description, isPublic)) { @@ -177,6 +216,7 @@ class OrganizationDetailViewModel @Inject constructor( } fun deleteOrganization(onDeleted: () -> Unit = {}) { + if (!_uiState.value.permissions.canDeleteOrganization) return viewModelScope.launch { when (val result = repository.deleteOrganization(orgId)) { is ApiResult.Success -> { @@ -208,6 +248,7 @@ class OrganizationDetailViewModel @Inject constructor( } fun addMember(candidate: MemberCandidate, role: OrgRole = OrgRole.MEMBER) { + if (!_uiState.value.permissions.canAddMember) return viewModelScope.launch { when (val result = repository.addMember(orgId, candidate.userId, role)) { is ApiResult.Success -> { @@ -220,29 +261,51 @@ class OrganizationDetailViewModel @Inject constructor( } } + /** + * Changes a member's role. Demoting the organization's only owner is refused by + * the server (400 "Cannot demote the last owner"), so it is explained up front; + * a server rejection is explained the same way when the guard cannot see it. + */ fun changeRole(member: OrgMember, role: OrgRole) { if (member.role == role) return + val state = _uiState.value + if (!state.canChangeRoleOf(member)) return + if (role != OrgRole.OWNER && state.isOnlyOwner(member)) { + _uiState.update { it.copy(errorMessage = LAST_OWNER_DEMOTE_EXPLANATION) } + return + } viewModelScope.launch { when (val result = repository.updateMemberRole(orgId, member.userId, role)) { - is ApiResult.Success -> _uiState.update { state -> - state.copy( - members = state.members.map { + is ApiResult.Success -> _uiState.update { current -> + current.copy( + members = current.members.map { if (it.userId == member.userId) it.copy(role = role) else it }, ) } - is ApiResult.Failure -> _uiState.update { it.copy(errorMessage = result.error.toUserMessage()) } + is ApiResult.Failure -> _uiState.update { + it.copy(errorMessage = result.error.toRoleChangeMessage()) + } } } } + /** Removes a member. The only owner is protected exactly as [changeRole] is. */ fun removeMember(member: OrgMember) { + val state = _uiState.value + if (!state.canRemove(member)) return + if (state.isOnlyOwner(member)) { + _uiState.update { it.copy(errorMessage = LAST_OWNER_REMOVE_EXPLANATION) } + return + } viewModelScope.launch { when (val result = repository.removeMember(orgId, member.userId)) { - is ApiResult.Success -> _uiState.update { state -> - state.copy(members = state.members.filterNot { it.userId == member.userId }) + is ApiResult.Success -> _uiState.update { current -> + current.copy(members = current.members.filterNot { it.userId == member.userId }) + } + is ApiResult.Failure -> _uiState.update { + it.copy(errorMessage = result.error.toRemoveMemberMessage()) } - is ApiResult.Failure -> _uiState.update { it.copy(errorMessage = result.error.toUserMessage()) } } } } diff --git a/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/FakeOrganizationsRepository.kt b/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/FakeOrganizationsRepository.kt index 4682ecd..0be976f 100644 --- a/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/FakeOrganizationsRepository.kt +++ b/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/FakeOrganizationsRepository.kt @@ -32,6 +32,7 @@ class FakeOrganizationsRepository : OrganizationsRepository { var addMemberResult: ApiResult = ApiResult.Success(Unit) var updateRoleResult: ApiResult = ApiResult.Success(Unit) var removeMemberResult: ApiResult = ApiResult.Success(Unit) + var updateRoleCount = 0 var joinResult: ApiResult = ApiResult.Success(Unit) var leaveResult: ApiResult = ApiResult.Success(Unit) @@ -66,9 +67,14 @@ class FakeOrganizationsRepository : OrganizationsRepository { Organization("new", name, description, null, isPublic, 1, OrgRole.OWNER, null), ) + /** + * Defaults to an organization the caller owns, so management tests exercise the + * mutation rather than the permission gate. Tests that care about a narrower + * role (or about non-membership) set [getResult] explicitly. + */ override suspend fun getOrganization(id: String): ApiResult = getResult ?: ApiResult.Success( - Organization(id, "Org $id", null, null, false, 0, OrgRole.MEMBER, null), + Organization(id, "Org $id", null, null, false, 0, OrgRole.OWNER, null), ) override suspend fun updateOrganization( @@ -125,8 +131,10 @@ class FakeOrganizationsRepository : OrganizationsRepository { return addMemberResult } - override suspend fun updateMemberRole(orgId: String, userId: String, role: OrgRole): ApiResult = - updateRoleResult + override suspend fun updateMemberRole(orgId: String, userId: String, role: OrgRole): ApiResult { + updateRoleCount++ + return updateRoleResult + } override suspend fun removeMember(orgId: String, userId: String): ApiResult { removeMemberCount++ @@ -140,5 +148,9 @@ class FakeOrganizationsRepository : OrganizationsRepository { /** The live 400 the server returns when the only owner tries to leave. */ fun lastOwnerFailure(): ApiResult.Failure = ApiResult.Failure(AppError.Unknown("Cannot remove the last owner")) + + /** The live 400 the server returns when the only owner would be demoted. */ + fun lastOwnerDemoteFailure(): ApiResult.Failure = + ApiResult.Failure(AppError.Unknown("Cannot demote the last owner")) } } diff --git a/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/data/DefaultOrganizationsRepositoryTest.kt b/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/data/DefaultOrganizationsRepositoryTest.kt index 6e236b2..9d90a85 100644 --- a/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/data/DefaultOrganizationsRepositoryTest.kt +++ b/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/data/DefaultOrganizationsRepositoryTest.kt @@ -133,8 +133,8 @@ class DefaultOrganizationsRepositoryTest { assertThat(request.path).isEqualTo("/api/organizations") val body = request.body.readUtf8() assertThat(body).contains("\"name\":\"Newco\"") - // isPublic is serialised as a string per the API contract. - assertThat(body).contains("\"isPublic\":\"true\"") + // isPublic must be a JSON boolean: the server answers 500 to a string. + assertThat(body).contains("\"isPublic\":true") } @Test @@ -154,9 +154,16 @@ class DefaultOrganizationsRepositoryTest { @Test fun `updateOrganization sends the changed fields and refreshes the cache`() = runTest(dispatcher) { + // The PUT echo, then the re-read the repository performs afterwards. server.enqueue( MockResponse().setBody("""{ "organization": { "id": "o1", "name": "Renamed", "isPublic": false } }"""), ) + server.enqueue( + MockResponse().setBody( + """{ "organization": { "id": "o1", "name": "Renamed", "isPublic": false, + "userRole": "owner", "memberCount": 4 } }""", + ), + ) val result = repository.updateOrganization("o1", name = "Renamed", description = null, isPublic = false) @@ -168,9 +175,41 @@ class DefaultOrganizationsRepositoryTest { assertThat(request.path).isEqualTo("/api/organizations/o1") val body = request.body.readUtf8() assertThat(body).contains("\"name\":\"Renamed\"") - assertThat(body).contains("\"isPublic\":\"false\"") + // A JSON boolean, not a string — a string answers 500 (verified live). + assertThat(body).contains("\"isPublic\":false") } + @Test + fun `updateOrganization re-reads so the editor keeps their role and member count`() = + runTest(dispatcher) { + // Verified live: the PUT echo carries no `userRole`/`memberCount`, so + // trusting it would make the owner who just edited look like a + // non-member and hide Edit/Delete from them. + server.enqueue( + MockResponse().setBody( + """{ "organization": { "id": "o1", "name": "Renamed", "isPublic": true } }""", + ), + ) + server.enqueue( + MockResponse().setBody( + """{ "organization": { "id": "o1", "name": "Renamed", "isPublic": true, + "userRole": "owner", "memberCount": 4 } }""", + ), + ) + + val result = repository.updateOrganization("o1", name = "Renamed", description = null, isPublic = true) + + val org = (result as ApiResult.Success).data + assertThat(org.role).isEqualTo(OrgRole.OWNER) + assertThat(org.memberCount).isEqualTo(4) + assertThat(org.isPublic).isTrue() + + server.takeRequest() // the PUT + val reread = server.takeRequest() + assertThat(reread.method).isEqualTo("GET") + assertThat(reread.path).isEqualTo("/api/organizations/o1") + } + @Test fun `deleteOrganization evicts from cache on success`() = runTest(dispatcher) { dao.upsert(CachedOrganizationEntity("gone", "X", null, null, false, 0, null, null)) diff --git a/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/data/OrganizationMapperTest.kt b/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/data/OrganizationMapperTest.kt index b1c68f4..3c54566 100644 --- a/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/data/OrganizationMapperTest.kt +++ b/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/data/OrganizationMapperTest.kt @@ -33,6 +33,18 @@ class OrganizationMapperTest { assertThat(org.role).isEqualTo(OrgRole.ADMIN) } + @Test + fun `maps the system flag so built-in organizations are recognisable`() { + val system = OrganizationMapper.fromDto( + OrganizationDto(id = "sys", name = "The Public", isPublic = true, isSystem = true), + ) + val ordinary = OrganizationMapper.fromDto(OrganizationDto(id = "o1", name = "Acme")) + + assertThat(system.isSystem).isTrue() + // Absent means "not a system organization". + assertThat(ordinary.isSystem).isFalse() + } + @Test fun `defaults an absent public flag to private and a missing count to zero`() { val org = OrganizationMapper.fromDto(OrganizationDto(id = "o2", name = "Nameless")) @@ -53,6 +65,7 @@ class OrganizationMapperTest { memberCount = 3, role = OrgRole.OWNER, updatedAt = "2026-01-01", + isSystem = true, ) val restored = OrganizationMapper.fromEntity(OrganizationMapper.toEntity(org)) diff --git a/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/domain/OrgPermissionsTest.kt b/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/domain/OrgPermissionsTest.kt new file mode 100644 index 0000000..720eb47 --- /dev/null +++ b/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/domain/OrgPermissionsTest.kt @@ -0,0 +1,186 @@ +package com.interlinedlist.android.feature.organizations.domain + +import com.google.common.truth.Truth.assertThat +import org.junit.Test + +/** + * The role × action matrix documented at `https://interlinedlist.com/help/organizations`: + * + * - "Owner: Full control; can delete the org and manage all members" + * - "Admin: Can add and remove members and change roles (except owner)" + * - "Member: Basic access" + * - "Private: Invite-only; members must be added by an owner or admin" + * - "You cannot leave the system \"The Public\" organization." + * + * One case per role per action, so a future change to the matrix has to be deliberate. + */ +class OrgPermissionsTest { + + private fun org( + role: OrgRole?, + isPublic: Boolean = true, + isSystem: Boolean = false, + ) = Organization( + id = "o1", + name = "Acme", + description = null, + avatarUrl = null, + isPublic = isPublic, + memberCount = 3, + role = role, + updatedAt = null, + isSystem = isSystem, + ) + + private fun permissionsFor( + role: OrgRole?, + isPublic: Boolean = true, + isSystem: Boolean = false, + ) = OrgPermissions.of(org(role, isPublic, isSystem)) + + // ---- View members ------------------------------------------------------- + // `GET /api/organizations/{id}/members` is members-only (403 otherwise). + + @Test + fun `owner admin and member may view members but a non-member may not`() { + assertThat(permissionsFor(OrgRole.OWNER).canViewMembers).isTrue() + assertThat(permissionsFor(OrgRole.ADMIN).canViewMembers).isTrue() + assertThat(permissionsFor(OrgRole.MEMBER).canViewMembers).isTrue() + assertThat(permissionsFor(null).canViewMembers).isFalse() + } + + // ---- Add a member ------------------------------------------------------- + + @Test + fun `only an owner or admin may add a member`() { + assertThat(permissionsFor(OrgRole.OWNER).canAddMember).isTrue() + assertThat(permissionsFor(OrgRole.ADMIN).canAddMember).isTrue() + assertThat(permissionsFor(OrgRole.MEMBER).canAddMember).isFalse() + assertThat(permissionsFor(null).canAddMember).isFalse() + } + + // ---- Change a member's role -------------------------------------------- + + @Test + fun `an owner may change any member's role`() { + val owner = permissionsFor(OrgRole.OWNER) + assertThat(owner.canChangeRoleOf(OrgRole.OWNER)).isTrue() + assertThat(owner.canChangeRoleOf(OrgRole.ADMIN)).isTrue() + assertThat(owner.canChangeRoleOf(OrgRole.MEMBER)).isTrue() + } + + @Test + fun `an admin may change roles except an owner's`() { + val admin = permissionsFor(OrgRole.ADMIN) + assertThat(admin.canChangeRoleOf(OrgRole.OWNER)).isFalse() + assertThat(admin.canChangeRoleOf(OrgRole.ADMIN)).isTrue() + assertThat(admin.canChangeRoleOf(OrgRole.MEMBER)).isTrue() + } + + @Test + fun `a member and a non-member may not change any role`() { + for (permissions in listOf(permissionsFor(OrgRole.MEMBER), permissionsFor(null))) { + OrgRole.entries.forEach { target -> + assertThat(permissions.canChangeRoleOf(target)).isFalse() + } + } + } + + @Test + fun `an owner may grant owner but an admin may not`() { + assertThat(permissionsFor(OrgRole.OWNER).assignableRolesFor(OrgRole.MEMBER)) + .containsExactly(OrgRole.MEMBER, OrgRole.ADMIN, OrgRole.OWNER) + // "change roles (except owner)" — an admin cannot hand out ownership. + assertThat(permissionsFor(OrgRole.ADMIN).assignableRolesFor(OrgRole.MEMBER)) + .containsExactly(OrgRole.MEMBER, OrgRole.ADMIN) + assertThat(permissionsFor(OrgRole.ADMIN).assignableRolesFor(OrgRole.OWNER)).isEmpty() + assertThat(permissionsFor(OrgRole.MEMBER).assignableRolesFor(OrgRole.MEMBER)).isEmpty() + } + + // ---- Remove a member ---------------------------------------------------- + + @Test + fun `an owner may remove anyone and an admin anyone but an owner`() { + val owner = permissionsFor(OrgRole.OWNER) + OrgRole.entries.forEach { assertThat(owner.canRemove(it)).isTrue() } + + val admin = permissionsFor(OrgRole.ADMIN) + assertThat(admin.canRemove(OrgRole.OWNER)).isFalse() + assertThat(admin.canRemove(OrgRole.ADMIN)).isTrue() + assertThat(admin.canRemove(OrgRole.MEMBER)).isTrue() + } + + @Test + fun `a member and a non-member may not remove anyone`() { + for (permissions in listOf(permissionsFor(OrgRole.MEMBER), permissionsFor(null))) { + OrgRole.entries.forEach { assertThat(permissions.canRemove(it)).isFalse() } + } + } + + // ---- Edit the organization --------------------------------------------- + + @Test + fun `only an owner or admin may edit the organization`() { + assertThat(permissionsFor(OrgRole.OWNER).canEditOrganization).isTrue() + assertThat(permissionsFor(OrgRole.ADMIN).canEditOrganization).isTrue() + assertThat(permissionsFor(OrgRole.MEMBER).canEditOrganization).isFalse() + assertThat(permissionsFor(null).canEditOrganization).isFalse() + } + + @Test + fun `visibility is editable exactly when the organization is`() { + // Visibility rides on the same PUT, so it follows the same rule. + OrgRole.entries.plus(null).forEach { role -> + val permissions = permissionsFor(role) + assertThat(permissions.canEditVisibility).isEqualTo(permissions.canEditOrganization) + } + } + + // ---- Delete the organization ------------------------------------------- + + @Test + fun `only an owner may delete the organization`() { + assertThat(permissionsFor(OrgRole.OWNER).canDeleteOrganization).isTrue() + assertThat(permissionsFor(OrgRole.ADMIN).canDeleteOrganization).isFalse() + assertThat(permissionsFor(OrgRole.MEMBER).canDeleteOrganization).isFalse() + assertThat(permissionsFor(null).canDeleteOrganization).isFalse() + } + + // ---- Join / leave ------------------------------------------------------- + + @Test + fun `only a non-member of a public organization may join`() { + assertThat(permissionsFor(null, isPublic = true).canJoin).isTrue() + assertThat(permissionsFor(null, isPublic = false).canJoin).isFalse() + OrgRole.entries.forEach { assertThat(permissionsFor(it).canJoin).isFalse() } + } + + @Test + fun `any member may leave but a non-member has nothing to leave`() { + OrgRole.entries.forEach { assertThat(permissionsFor(it).canLeave).isTrue() } + assertThat(permissionsFor(null).canLeave).isFalse() + } + + // ---- System organizations ---------------------------------------------- + + @Test + fun `a system organization cannot be left or deleted by anyone`() { + // "You cannot leave the system \"The Public\" organization." + OrgRole.entries.forEach { role -> + val permissions = permissionsFor(role, isSystem = true) + assertThat(permissions.canLeave).isFalse() + assertThat(permissions.canDeleteOrganization).isFalse() + } + } + + @Test + fun `an absent organization permits nothing`() { + val none = OrgPermissions.of(null) + assertThat(none.canViewMembers).isFalse() + assertThat(none.canAddMember).isFalse() + assertThat(none.canEditOrganization).isFalse() + assertThat(none.canDeleteOrganization).isFalse() + assertThat(none.canLeave).isFalse() + assertThat(none.canJoin).isFalse() + } +} diff --git a/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/ui/OrganizationsErrorMessagesTest.kt b/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/ui/OrganizationsErrorMessagesTest.kt index 78c83b7..2d155d1 100644 --- a/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/ui/OrganizationsErrorMessagesTest.kt +++ b/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/ui/OrganizationsErrorMessagesTest.kt @@ -48,4 +48,32 @@ class OrganizationsErrorMessagesTest { assertThat(AppError.NotFound("Organization not found").toJoinMessage()) .isEqualTo("That organization could not be found.") } + + @Test + fun `a last-owner demote rejection is explained in role-change terms`() { + // Live 400: {"error":"Cannot demote the last owner","code":"bad_request"} + val error = AppError.Unknown("Cannot demote the last owner") + + assertThat(error.isLastOwnerRejection).isTrue() + assertThat(error.toRoleChangeMessage()).isEqualTo(LAST_OWNER_DEMOTE_EXPLANATION) + assertThat(error.toRoleChangeMessage()).contains("Make someone else an owner first") + assertThat(error.toRoleChangeMessage()).isNotEqualTo(error.toUserMessage()) + } + + @Test + fun `a last-owner remove rejection is explained in removal terms`() { + // Live 400: {"error":"Cannot remove the last owner","code":"bad_request"} + val error = AppError.Unknown("Cannot remove the last owner") + + assertThat(error.toRemoveMemberMessage()).isEqualTo(LAST_OWNER_REMOVE_EXPLANATION) + assertThat(error.toRemoveMemberMessage()).contains("remove them") + } + + @Test + fun `other member-management failures fall back to the shared mapping`() { + val forbidden = AppError.Forbidden("Nope") + + assertThat(forbidden.toRoleChangeMessage()).isEqualTo(forbidden.toUserMessage()) + assertThat(forbidden.toRemoveMemberMessage()).isEqualTo(forbidden.toUserMessage()) + } } diff --git a/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/ui/detail/OrganizationDetailViewModelTest.kt b/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/ui/detail/OrganizationDetailViewModelTest.kt index ad19570..b07ec26 100644 --- a/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/ui/detail/OrganizationDetailViewModelTest.kt +++ b/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/ui/detail/OrganizationDetailViewModelTest.kt @@ -9,7 +9,9 @@ import com.interlinedlist.android.feature.organizations.domain.MemberCandidate import com.interlinedlist.android.feature.organizations.domain.OrgMember import com.interlinedlist.android.feature.organizations.domain.OrgRole import com.interlinedlist.android.feature.organizations.domain.Organization +import com.interlinedlist.android.feature.organizations.ui.LAST_OWNER_DEMOTE_EXPLANATION import com.interlinedlist.android.feature.organizations.ui.LAST_OWNER_EXPLANATION +import com.interlinedlist.android.feature.organizations.ui.LAST_OWNER_REMOVE_EXPLANATION import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.StandardTestDispatcher @@ -334,4 +336,241 @@ class OrganizationDetailViewModelTest { assertThat(vm.uiState.value.isLeaving).isFalse() assertThat(vm.uiState.value.errorMessage).isEqualTo(LAST_OWNER_EXPLANATION) } + + // ---- Role-gated actions ------------------------------------------------ + + private fun repoWith(role: OrgRole?, vararg members: OrgMember) = FakeOrganizationsRepository().apply { + getResult = ApiResult.Success(Organization("o1", "Acme", null, null, true, 3, role, null)) + membersResult = ApiResult.Success(members.toList()) + } + + @Test + fun `a member may not add remove or promote, and is offered no edit or delete`() = runTest(dispatcher) { + val repo = repoWith(OrgRole.MEMBER, member("u1", OrgRole.OWNER), member("u2")) + val vm = vmFor(repo) + advanceUntilIdle() + + val state = vm.uiState.value + assertThat(state.permissions.canAddMember).isFalse() + assertThat(state.permissions.canEditOrganization).isFalse() + assertThat(state.permissions.canDeleteOrganization).isFalse() + assertThat(state.canChangeRoleOf(state.members.last())).isFalse() + assertThat(state.canRemove(state.members.last())).isFalse() + // A plain member still sees the roster. + assertThat(state.permissions.canViewMembers).isTrue() + } + + @Test + fun `an admin manages members but may not touch an owner or delete the org`() = runTest(dispatcher) { + val repo = repoWith(OrgRole.ADMIN, member("u1", OrgRole.OWNER), member("u2")) + val vm = vmFor(repo) + advanceUntilIdle() + + val state = vm.uiState.value + val owner = state.members.first { it.userId == "u1" } + val plain = state.members.first { it.userId == "u2" } + + assertThat(state.permissions.canAddMember).isTrue() + assertThat(state.permissions.canEditOrganization).isTrue() + assertThat(state.permissions.canDeleteOrganization).isFalse() + // "change roles (except owner)" + assertThat(state.canChangeRoleOf(owner)).isFalse() + assertThat(state.canRemove(owner)).isFalse() + assertThat(state.canChangeRoleOf(plain)).isTrue() + assertThat(state.assignableRolesFor(plain)).containsExactly(OrgRole.MEMBER, OrgRole.ADMIN) + } + + @Test + fun `an owner may manage every member and delete the org`() = runTest(dispatcher) { + val repo = repoWith(OrgRole.OWNER, member("u1", OrgRole.OWNER), member("u2", OrgRole.OWNER)) + val vm = vmFor(repo) + advanceUntilIdle() + + val state = vm.uiState.value + assertThat(state.permissions.canDeleteOrganization).isTrue() + state.members.forEach { + assertThat(state.canChangeRoleOf(it)).isTrue() + assertThat(state.canRemove(it)).isTrue() + } + assertThat(state.assignableRolesFor(state.members.first())) + .containsExactly(OrgRole.MEMBER, OrgRole.ADMIN, OrgRole.OWNER) + } + + @Test + fun `a non-member is offered neither edit nor delete`() = runTest(dispatcher) { + // No role: the API omits `role` (and reports `userRole: null`) for non-members. + val repo = repoWith(null) + val vm = vmFor(repo) + advanceUntilIdle() + + val permissions = vm.uiState.value.permissions + assertThat(permissions.canEditOrganization).isFalse() + assertThat(permissions.canDeleteOrganization).isFalse() + assertThat(permissions.canAddMember).isFalse() + assertThat(permissions.hasAnyOrganizationAction).isFalse() + } + + @Test + fun `a non-member's edit and delete are refused even if invoked`() = runTest(dispatcher) { + val repo = repoWith(null) + val vm = vmFor(repo) + advanceUntilIdle() + + var edited = false + var deleted = false + vm.updateOrganization("Hijacked", null, isPublic = true) { edited = true } + vm.deleteOrganization { deleted = true } + advanceUntilIdle() + + assertThat(edited).isFalse() + assertThat(deleted).isFalse() + assertThat(repo.lastUpdate).isNull() + assertThat(vm.uiState.value.organization?.name).isEqualTo("Acme") + } + + @Test + fun `a system organization offers no leave and no delete`() = runTest(dispatcher) { + val repo = FakeOrganizationsRepository().apply { + // "You cannot leave the system \"The Public\" organization." + getResult = ApiResult.Success( + Organization("o1", "The Public", null, null, true, 33, OrgRole.OWNER, null, isSystem = true), + ) + membersResult = ApiResult.Success(listOf(member("u1", OrgRole.OWNER), member("me"))) + } + val vm = vmFor(repo) + advanceUntilIdle() + + assertThat(vm.uiState.value.permissions.canLeave).isFalse() + assertThat(vm.uiState.value.permissions.canDeleteOrganization).isFalse() + + vm.leave() + advanceUntilIdle() + assertThat(repo.leftOrgIds).isEmpty() + } + + // ---- Last-owner protection on demote and remove ------------------------- + + @Test + fun `demoting the only owner is explained instead of being sent`() = runTest(dispatcher) { + val repo = repoWith(OrgRole.OWNER, member("u1", OrgRole.OWNER), member("u2")) + val vm = vmFor(repo) + advanceUntilIdle() + + val onlyOwner = vm.uiState.value.members.first { it.userId == "u1" } + assertThat(vm.uiState.value.isOnlyOwner(onlyOwner)).isTrue() + // They are offered no demotion at all, only the role they already hold. + assertThat(vm.uiState.value.assignableRolesFor(onlyOwner)).containsExactly(OrgRole.OWNER) + + vm.changeRole(onlyOwner, OrgRole.MEMBER) + advanceUntilIdle() + + assertThat(repo.updateRoleCount).isEqualTo(0) + assertThat(vm.uiState.value.errorMessage).isEqualTo(LAST_OWNER_DEMOTE_EXPLANATION) + assertThat(vm.uiState.value.members.first { it.userId == "u1" }.role).isEqualTo(OrgRole.OWNER) + } + + @Test + fun `a server last-owner demote rejection is surfaced as the same explanation`() = runTest(dispatcher) { + // Two owners locally, so the client guard cannot fire; the server still refuses. + val repo = repoWith(OrgRole.OWNER, member("u1", OrgRole.OWNER), member("u2", OrgRole.OWNER)).apply { + updateRoleResult = FakeOrganizationsRepository.lastOwnerDemoteFailure() + } + val vm = vmFor(repo) + advanceUntilIdle() + + vm.changeRole(vm.uiState.value.members.first { it.userId == "u1" }, OrgRole.MEMBER) + advanceUntilIdle() + + assertThat(repo.updateRoleCount).isEqualTo(1) + assertThat(vm.uiState.value.errorMessage).isEqualTo(LAST_OWNER_DEMOTE_EXPLANATION) + // The local list is untouched, so it still matches the server. + assertThat(vm.uiState.value.members.first { it.userId == "u1" }.role).isEqualTo(OrgRole.OWNER) + } + + @Test + fun `removing the only owner is explained instead of being sent`() = runTest(dispatcher) { + val repo = repoWith(OrgRole.OWNER, member("u1", OrgRole.OWNER), member("u2")) + val vm = vmFor(repo) + advanceUntilIdle() + + vm.removeMember(vm.uiState.value.members.first { it.userId == "u1" }) + advanceUntilIdle() + + assertThat(repo.removeMemberCount).isEqualTo(0) + assertThat(vm.uiState.value.errorMessage).isEqualTo(LAST_OWNER_REMOVE_EXPLANATION) + assertThat(vm.uiState.value.members.map { it.userId }).containsExactly("u1", "u2") + } + + @Test + fun `a server last-owner remove rejection is surfaced as the same explanation`() = runTest(dispatcher) { + val repo = repoWith(OrgRole.OWNER, member("u1", OrgRole.OWNER), member("u2", OrgRole.OWNER)).apply { + removeMemberResult = FakeOrganizationsRepository.lastOwnerFailure() + } + val vm = vmFor(repo) + advanceUntilIdle() + + vm.removeMember(vm.uiState.value.members.first { it.userId == "u1" }) + advanceUntilIdle() + + assertThat(repo.removeMemberCount).isEqualTo(1) + assertThat(vm.uiState.value.errorMessage).isEqualTo(LAST_OWNER_REMOVE_EXPLANATION) + assertThat(vm.uiState.value.members).hasSize(2) + } + + @Test + fun `an admin's attempt to manage an owner is not sent`() = runTest(dispatcher) { + val repo = repoWith(OrgRole.ADMIN, member("u1", OrgRole.OWNER), member("u2", OrgRole.OWNER)) + val vm = vmFor(repo) + advanceUntilIdle() + + val owner = vm.uiState.value.members.first { it.userId == "u1" } + vm.changeRole(owner, OrgRole.MEMBER) + vm.removeMember(owner) + advanceUntilIdle() + + assertThat(repo.updateRoleCount).isEqualTo(0) + assertThat(repo.removeMemberCount).isEqualTo(0) + } + + // ---- Visibility --------------------------------------------------------- + + @Test + fun `visibility is readable and round-trips for a permitted role`() = runTest(dispatcher) { + val repo = repoWith(OrgRole.OWNER, member("u1", OrgRole.OWNER), member("u2", OrgRole.OWNER)).apply { + updateResult = ApiResult.Success( + Organization("o1", "Acme", null, null, false, 3, OrgRole.OWNER, null), + ) + } + val vm = vmFor(repo) + advanceUntilIdle() + + assertThat(vm.uiState.value.organization?.isPublic).isTrue() + assertThat(vm.uiState.value.permissions.canEditVisibility).isTrue() + + vm.updateOrganization(name = null, description = null, isPublic = false) + advanceUntilIdle() + + assertThat(repo.lastUpdate).isEqualTo(Triple(null, null, false)) + // The re-read result is what the header now shows. + assertThat(vm.uiState.value.organization?.isPublic).isFalse() + // Role and membership survive the edit. + assertThat(vm.uiState.value.isMember).isTrue() + assertThat(vm.uiState.value.permissions.canDeleteOrganization).isTrue() + } + + @Test + fun `a member may read visibility but not change it`() = runTest(dispatcher) { + val repo = repoWith(OrgRole.MEMBER, member("u1", OrgRole.OWNER)) + val vm = vmFor(repo) + advanceUntilIdle() + + assertThat(vm.uiState.value.organization?.isPublic).isTrue() + assertThat(vm.uiState.value.permissions.canEditVisibility).isFalse() + + vm.updateOrganization(name = null, description = null, isPublic = false) + advanceUntilIdle() + + assertThat(repo.lastUpdate).isNull() + assertThat(vm.uiState.value.organization?.isPublic).isTrue() + } }