From 00c24aa96b3c556d0aff92987118ea32fbc91fe5 Mon Sep 17 00:00:00 2001 From: Adron Hall Date: Wed, 16 Sep 2026 13:33:40 -0700 Subject: [PATCH] feat(organizations): join and leave organizations Android could browse and create organizations but never join or leave one. Both contracts were captured against the live API before implementing: - join is POST /api/user/organizations with {"organizationId": "..."} -> 201 {message, membership}; 404 for an unknown org, 409 when already a member, 403 for a private org. - there is no dedicated leave route: leaving is DELETE /api/organizations/{id}/members/{userId} with your own user id -> 200. - membership is reported as `role` on the index and `userRole` on the detail endpoint, and is simply absent/null when you are not a member, so the existing index already lists joinable public organizations. - a sole owner is refused with 400 "Cannot remove the last owner". The index card now offers Join for public organizations the user has not joined and shows their role otherwise; the detail screen shows a join prompt for non-members (and skips the members request, which is members-only and 403s) and a Leave entry with a confirmation for members. The only owner is told why they cannot leave before any request is made, and the server's rejection is surfaced with the same explanation as a backstop rather than a generic error. Leaving pops back to the index; the repository re-reads the organization so the Room cache reflects the new membership, dropping a private organization that is no longer visible. Tests: repository round-trips asserting the real paths and bodies for join and leave (including the 409, the last-owner 400, and the missing-session guard), error-message mapping, view-model membership/join/leave behaviour, and Compose coverage for member vs non-member rendering. Closes #81 --- .../navigation/InterlinedListNavHost.kt | 2 + .../ui/detail/OrganizationDetailScreenTest.kt | 105 +++++++++++ .../ui/list/OrganizationsScreenTest.kt | 50 ++++++ .../data/CurrentUserIdProvider.kt | 12 ++ .../data/DefaultOrganizationsRepository.kt | 43 +++++ .../organizations/data/OrganizationMapper.kt | 2 +- .../data/OrganizationsRepository.kt | 13 ++ .../data/remote/OrganizationsApi.kt | 13 ++ .../data/remote/dto/OrganizationDtos.kt | 21 ++- .../organizations/di/OrganizationsModule.kt | 8 + .../organizations/domain/Organization.kt | 10 ++ .../ui/OrganizationsErrorMessages.kt | 34 ++++ .../ui/detail/OrganizationDetailScreen.kt | 163 +++++++++++++++--- .../ui/detail/OrganizationDetailViewModel.kt | 96 +++++++++-- .../ui/list/OrganizationsScreen.kt | 40 ++++- .../ui/list/OrganizationsViewModel.kt | 22 +++ .../FakeOrganizationsRepository.kt | 29 ++++ .../DefaultOrganizationsRepositoryTest.kt | 131 +++++++++++++- .../ui/OrganizationsErrorMessagesTest.kt | 51 ++++++ .../detail/OrganizationDetailViewModelTest.kt | 160 +++++++++++++++++ .../ui/list/OrganizationsViewModelTest.kt | 55 ++++++ 21 files changed, 1023 insertions(+), 37 deletions(-) create mode 100644 feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/CurrentUserIdProvider.kt create mode 100644 feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/ui/OrganizationsErrorMessagesTest.kt diff --git a/app/src/main/java/com/interlinedlist/android/navigation/InterlinedListNavHost.kt b/app/src/main/java/com/interlinedlist/android/navigation/InterlinedListNavHost.kt index 3d9827b..11bf611 100644 --- a/app/src/main/java/com/interlinedlist/android/navigation/InterlinedListNavHost.kt +++ b/app/src/main/java/com/interlinedlist/android/navigation/InterlinedListNavHost.kt @@ -607,6 +607,8 @@ private fun MainShell( OrganizationDetailRoute( onBack = { tabNav.popBackStack() }, onDeleted = { tabNav.popBackStack() }, + // Leaving drops access to the org, so return to the index. + onLeft = { tabNav.popBackStack() }, ) } 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 22587e8..421f728 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 @@ -28,6 +28,8 @@ class OrganizationDetailScreenTest { state: OrganizationDetailUiState, onRemoveMember: (OrgMember) -> Unit = {}, onDelete: () -> Unit = {}, + onJoin: () -> Unit = {}, + onLeave: () -> Unit = {}, ) { composeRule.setContent { InterlinedListTheme { @@ -40,6 +42,8 @@ class OrganizationDetailScreenTest { onRemoveMember = onRemoveMember, onSaveEdit = { _, _, _ -> }, onDelete = onDelete, + onJoin = onJoin, + onLeave = onLeave, ) } } @@ -97,4 +101,105 @@ class OrganizationDetailScreenTest { 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, + ), + ) + + composeRule.onNodeWithTag(OrganizationDetailTestTags.JOIN_PROMPT).assertIsDisplayed() + composeRule.onNodeWithTag(OrganizationDetailTestTags.JOIN).assertIsDisplayed() + composeRule.onNodeWithTag(OrganizationDetailTestTags.SEARCH).assertDoesNotExist() + } + + @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 }, + ) + + composeRule.onNodeWithTag(OrganizationDetailTestTags.JOIN).performClick() + assert(joined) + } + + @Test + fun nonMemberOfPrivateOrg_isNotOfferedJoin() { + setScreen( + state = OrganizationDetailUiState( + organization = Organization("o1", "Acme", null, null, false, 2, null, null), + isLoading = false, + ), + ) + + composeRule.onNodeWithTag(OrganizationDetailTestTags.JOIN_PROMPT).assertIsDisplayed() + composeRule.onNodeWithTag(OrganizationDetailTestTags.JOIN).assertDoesNotExist() + } + + @Test + 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, + ), + onLeave = { left = true }, + ) + + composeRule.onNodeWithTag(OrganizationDetailTestTags.OVERFLOW).performClick() + composeRule.onNodeWithTag(OrganizationDetailTestTags.LEAVE).performClick() + composeRule.onNodeWithTag(OrganizationDetailTestTags.LEAVE_DIALOG).assertIsDisplayed() + composeRule.onNodeWithTag(OrganizationDetailTestTags.LEAVE_CONFIRM).performClick() + assert(left) + } + + @Test + 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, + ), + onLeave = { left = true }, + ) + + composeRule.onNodeWithTag(OrganizationDetailTestTags.OVERFLOW).performClick() + composeRule.onNodeWithTag(OrganizationDetailTestTags.LEAVE).performClick() + // The dialog explains why, and offers no destructive confirm at all. + composeRule.onNodeWithTag(OrganizationDetailTestTags.LAST_OWNER_NOTICE).assertIsDisplayed() + composeRule.onNodeWithTag(OrganizationDetailTestTags.LEAVE_CONFIRM).assertDoesNotExist() + assert(!left) + } + + @Test + fun nonMember_isNotOfferedLeave() { + setScreen( + state = OrganizationDetailUiState( + organization = Organization("o1", "Metals", null, null, true, 1, null, null), + isLoading = false, + ), + ) + + composeRule.onNodeWithTag(OrganizationDetailTestTags.OVERFLOW).performClick() + composeRule.onNodeWithTag(OrganizationDetailTestTags.LEAVE).assertDoesNotExist() + } } diff --git a/feature/organizations/src/androidTest/kotlin/com/interlinedlist/android/feature/organizations/ui/list/OrganizationsScreenTest.kt b/feature/organizations/src/androidTest/kotlin/com/interlinedlist/android/feature/organizations/ui/list/OrganizationsScreenTest.kt index 094111f..2ecae2d 100644 --- a/feature/organizations/src/androidTest/kotlin/com/interlinedlist/android/feature/organizations/ui/list/OrganizationsScreenTest.kt +++ b/feature/organizations/src/androidTest/kotlin/com/interlinedlist/android/feature/organizations/ui/list/OrganizationsScreenTest.kt @@ -6,6 +6,7 @@ import androidx.compose.ui.test.onNodeWithTag import androidx.compose.ui.test.performClick import androidx.test.ext.junit.runners.AndroidJUnit4 import com.interlinedlist.android.core.designsystem.theme.InterlinedListTheme +import com.interlinedlist.android.feature.organizations.domain.OrgRole import com.interlinedlist.android.feature.organizations.domain.Organization import org.junit.Rule import org.junit.Test @@ -25,6 +26,7 @@ class OrganizationsScreenTest { private fun setScreen( state: OrganizationsUiState, onOpenOrg: (String) -> Unit = {}, + onJoinOrg: (String) -> Unit = {}, ) { composeRule.setContent { InterlinedListTheme { @@ -33,6 +35,7 @@ class OrganizationsScreenTest { onOpenOrg = onOpenOrg, onBack = {}, onLoadMore = {}, + onJoinOrg = onJoinOrg, onCreateOrganization = { _, _, _ -> }, ) } @@ -86,4 +89,51 @@ class OrganizationsScreenTest { composeRule.onNodeWithTag(OrganizationsTestTags.CREATE_FAB).performClick() composeRule.onNodeWithTag(OrganizationsTestTags.CREATE_NAME).assertIsDisplayed() } + + @Test + fun rendersMembershipState_joinForNonMembers_roleForMembers() { + setScreen( + state = OrganizationsUiState( + organizations = listOf( + // Public, no role -> not a member: offer Join. + Organization("1", "Metals", null, null, true, 1, null, null), + // A membership reports a role: show it, never offer Join. + Organization("2", "Bikey Life", null, null, true, 3, OrgRole.MEMBER, null), + ), + isRefreshing = false, + ), + ) + + composeRule.onNodeWithTag(OrganizationsTestTags.join("1")).assertIsDisplayed() + composeRule.onNodeWithTag(OrganizationsTestTags.membership("1")).assertDoesNotExist() + composeRule.onNodeWithTag(OrganizationsTestTags.membership("2")).assertIsDisplayed() + composeRule.onNodeWithTag(OrganizationsTestTags.join("2")).assertDoesNotExist() + } + + @Test + fun joinButton_reportsTheOrgId() { + var joined: String? = null + setScreen( + state = OrganizationsUiState( + organizations = listOf(Organization("1", "Metals", null, null, true, 1, null, null)), + isRefreshing = false, + ), + onJoinOrg = { joined = it }, + ) + + composeRule.onNodeWithTag(OrganizationsTestTags.join("1")).performClick() + assert(joined == "1") + } + + @Test + fun privateOrgsTheUserIsNotIn_offerNoJoin() { + setScreen( + state = OrganizationsUiState( + organizations = listOf(Organization("1", "Acme", null, null, false, 2, null, null)), + isRefreshing = false, + ), + ) + + composeRule.onNodeWithTag(OrganizationsTestTags.join("1")).assertDoesNotExist() + } } diff --git a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/CurrentUserIdProvider.kt b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/CurrentUserIdProvider.kt new file mode 100644 index 0000000..f844446 --- /dev/null +++ b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/CurrentUserIdProvider.kt @@ -0,0 +1,12 @@ +package com.interlinedlist.android.feature.organizations.data + +/** + * Supplies the signed-in user's id. Leaving an organization is expressed by the + * API as deleting your own membership row + * (`DELETE /api/organizations/{id}/members/{userId}`), so the repository needs it. + * Abstracted from `SessionStore` (which is Android-backed) so the repository stays + * unit-testable on the plain JVM. + */ +fun interface CurrentUserIdProvider { + fun currentUserId(): String? +} 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 88fc0c9..0f7e61f 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 @@ -9,6 +9,7 @@ import com.interlinedlist.android.feature.organizations.data.local.OrganizationD import com.interlinedlist.android.feature.organizations.data.remote.OrganizationsApi import com.interlinedlist.android.feature.organizations.data.remote.dto.AddMemberRequest import com.interlinedlist.android.feature.organizations.data.remote.dto.CreateOrganizationRequest +import com.interlinedlist.android.feature.organizations.data.remote.dto.JoinOrganizationRequest import com.interlinedlist.android.feature.organizations.data.remote.dto.OrganizationsResponse import com.interlinedlist.android.feature.organizations.data.remote.dto.UpdateMemberRequest import com.interlinedlist.android.feature.organizations.data.remote.dto.UpdateOrganizationRequest @@ -32,6 +33,7 @@ class DefaultOrganizationsRepository @Inject constructor( private val api: OrganizationsApi, private val dao: OrganizationDao, private val json: kotlinx.serialization.json.Json, + private val currentUserId: CurrentUserIdProvider, private val dispatchers: DispatcherProvider, ) : OrganizationsRepository { @@ -142,6 +144,47 @@ class DefaultOrganizationsRepository @Inject constructor( } } + override suspend fun joinOrganization(orgId: String): ApiResult = + withContext(dispatchers.io) { + when (val result = safeApiCall(json) { api.joinOrganization(JoinOrganizationRequest(orgId)) }) { + // Re-read the org so the cached row carries the new role and member + // count; the index then renders "Member" instead of "Join". + is ApiResult.Success -> { + getOrganization(orgId) + ApiResult.Success(Unit) + } + is ApiResult.Failure -> result + } + } + + override suspend fun leaveOrganization(orgId: String): ApiResult = + withContext(dispatchers.io) { + val userId = currentUserId.currentUserId()?.takeIf { it.isNotBlank() } + ?: return@withContext ApiResult.Failure( + AppError.Unauthorized("We couldn't confirm who you're signed in as. Sign in again and retry."), + ) + when (val result = safeApiCall(json) { api.removeMember(orgId, userId) }) { + is ApiResult.Success -> { + refreshAfterLeaving(orgId) + ApiResult.Success(Unit) + } + is ApiResult.Failure -> result + } + } + + /** + * Re-reads an org just left so the cache drops the membership. A private org is + * invisible to a non-member, so a 403/404 means it should leave the cache too. + */ + private suspend fun refreshAfterLeaving(orgId: String) { + val refreshed = getOrganization(orgId) + if (refreshed is ApiResult.Failure && + (refreshed.error is AppError.Forbidden || refreshed.error is AppError.NotFound) + ) { + dao.deleteById(orgId) + } + } + override suspend fun getMembers(orgId: String, limit: Int): ApiResult> = withContext(dispatchers.io) { safeApiCall(json) { api.getMembers(orgId, limit = limit, offset = 0) } 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 5c78a31..0b95ba0 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 @@ -15,7 +15,7 @@ object OrganizationMapper { avatarUrl = dto.resolvedAvatar, isPublic = dto.resolvedPublic, memberCount = dto.resolvedMemberCount, - role = dto.role?.let(OrgRole::fromApi), + role = dto.resolvedRole?.let(OrgRole::fromApi), updatedAt = dto.updatedAt, ) diff --git a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/OrganizationsRepository.kt b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/OrganizationsRepository.kt index 6d54d95..41fe286 100644 --- a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/OrganizationsRepository.kt +++ b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/OrganizationsRepository.kt @@ -46,6 +46,19 @@ interface OrganizationsRepository { /** Deletes an organization and evicts it from the cache. */ suspend fun deleteOrganization(id: String): ApiResult + /** + * Joins a public organization (`POST /api/user/organizations`). On success the + * cached row is re-read so it carries the new role and member count. + */ + suspend fun joinOrganization(orgId: String): ApiResult + + /** + * Leaves an organization by removing the signed-in user's own membership. The + * server refuses to orphan an organization: the last owner gets a 400 + * ("Cannot remove the last owner"), which is reported as a failure. + */ + suspend fun leaveOrganization(orgId: String): ApiResult + /** Members of an organization (users granted access), with their roles. */ suspend fun getMembers(orgId: String, limit: Int = DEFAULT_PAGE_SIZE): ApiResult> diff --git a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/remote/OrganizationsApi.kt b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/remote/OrganizationsApi.kt index 15624af..6be810f 100644 --- a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/remote/OrganizationsApi.kt +++ b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/data/remote/OrganizationsApi.kt @@ -2,6 +2,7 @@ package com.interlinedlist.android.feature.organizations.data.remote import com.interlinedlist.android.feature.organizations.data.remote.dto.AddMemberRequest import com.interlinedlist.android.feature.organizations.data.remote.dto.CreateOrganizationRequest +import com.interlinedlist.android.feature.organizations.data.remote.dto.JoinOrganizationRequest import com.interlinedlist.android.feature.organizations.data.remote.dto.MembersResponse import com.interlinedlist.android.feature.organizations.data.remote.dto.OrgUsersResponse import com.interlinedlist.android.feature.organizations.data.remote.dto.OrganizationEnvelope @@ -33,6 +34,14 @@ interface OrganizationsApi { @GET("api/user/organizations") suspend fun getUserOrganizations(): OrganizationsResponse + /** + * Joins a public organization. Confirmed live: the body key is + * `organizationId`, the response is 201 `{ message, membership }`, a private + * org answers 403 and an existing membership answers 409. + */ + @POST("api/user/organizations") + suspend fun joinOrganization(@Body body: JoinOrganizationRequest) + @POST("api/organizations") suspend fun createOrganization(@Body body: CreateOrganizationRequest): OrganizationEnvelope @@ -68,6 +77,10 @@ interface OrganizationsApi { @Body body: UpdateMemberRequest, ) + /** + * Removes a membership. Used both for removing someone else and for *leaving* + * (passing the signed-in user's own id) — the API has no separate leave route. + */ @DELETE("api/organizations/{id}/members/{userId}") suspend fun removeMember( @Path("id") id: String, 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 20ec90e..a7aa5d1 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 @@ -30,8 +30,11 @@ data class OrganizationDto( val memberCount: Int? = null, val membersCount: Int? = null, val members: Int? = null, - // The current user's role in this org, when the endpoint includes it. + // The current user's role in this org, when the endpoint includes it. The + // index reports it as `role`, the detail endpoint as `userRole` (explicitly + // null for a non-member), so both names are read. val role: String? = null, + val userRole: String? = null, val updatedAt: String? = null, ) { /** The avatar URL under whichever field name the API used. */ @@ -40,6 +43,12 @@ data class OrganizationDto( val resolvedPublic: Boolean get() = isPublic ?: public ?: false /** Member count under whichever name the API used, defaulting to zero. */ val resolvedMemberCount: Int get() = memberCount ?: membersCount ?: members ?: 0 + + /** + * The caller's role under whichever name the endpoint used. `null` means the + * caller is not a member — that is how the API signals non-membership. + */ + val resolvedRole: String? get() = role ?: userRole } /** Pagination block shared by list endpoints. */ @@ -84,6 +93,16 @@ data class CreateOrganizationRequest( val isPublic: String? = null, ) +/** + * Body for `POST /api/user/organizations` — joining a public organization. + * The server requires the key `organizationId` (it 400s with + * `{"error":"Organization ID is required"}` otherwise). + */ +@Serializable +data class JoinOrganizationRequest( + val organizationId: String, +) + /** Body for `PUT /api/organizations/{id}` — partial metadata updates. */ @Serializable data class UpdateOrganizationRequest( diff --git a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/di/OrganizationsModule.kt b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/di/OrganizationsModule.kt index 550e80f..0b43ad4 100644 --- a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/di/OrganizationsModule.kt +++ b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/di/OrganizationsModule.kt @@ -2,6 +2,8 @@ package com.interlinedlist.android.feature.organizations.di import android.content.Context import androidx.room.Room +import com.interlinedlist.android.core.datastore.SessionStore +import com.interlinedlist.android.feature.organizations.data.CurrentUserIdProvider import com.interlinedlist.android.feature.organizations.data.DefaultOrganizationsRepository import com.interlinedlist.android.feature.organizations.data.OrganizationsRepository import com.interlinedlist.android.feature.organizations.data.local.OrganizationDao @@ -50,4 +52,10 @@ object OrganizationsDataModule { @Provides fun provideOrganizationDao(db: OrganizationsDatabase): OrganizationDao = db.organizationDao() + + /** Adapts the Android-backed [SessionStore] to the module's id contract. */ + @Provides + @Singleton + fun provideCurrentUserIdProvider(sessionStore: SessionStore): CurrentUserIdProvider = + CurrentUserIdProvider { sessionStore.userId } } 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 1416a00..3c3716a 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 @@ -19,6 +19,16 @@ data class Organization( ) { /** Best label for a card: the name, falling back to a placeholder. */ val displayName: String get() = name.ifBlank { "Untitled organization" } + + /** + * 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 + + /** A public organization the user has not joined can be joined from the UI. */ + val canJoin: Boolean get() = isPublic && !isMember } /** 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 7f09967..6543274 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 @@ -15,3 +15,37 @@ fun AppError.toUserMessage(): String = when (this) { /** True when the error is the subscriber-only gate, so the UI can show an upsell. */ 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. + */ +val AppError.isLastOwnerRejection: Boolean + get() = message?.contains("last owner", ignoreCase = true) == true + +/** Explains why a join was refused instead of showing a bare API string. */ +fun AppError.toJoinMessage(): String = when (this) { + is AppError.Conflict -> "You're already a member of this organization." + is AppError.Forbidden -> "This organization is private. Ask an owner or admin to add you." + is AppError.NotFound -> "That organization could not be found." + else -> toUserMessage() +} + +/** + * Explains why a leave was refused. The last-owner rejection is spelled out — the + * organization would be left without an owner — so the user knows what to do + * instead of seeing a generic failure. + */ +fun AppError.toLeaveMessage(): String = when { + isLastOwnerRejection -> LAST_OWNER_EXPLANATION + else -> toUserMessage() +} + +/** + * Shown both before the attempt (when the UI can see you are the only owner) and + * after the server refuses, so the two paths read the same. + */ +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." 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 0e058d6..f2aefe7 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 @@ -14,6 +14,7 @@ import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.items import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.filled.ArrowBack +import androidx.compose.material.icons.automirrored.filled.Logout import androidx.compose.material.icons.filled.Add import androidx.compose.material.icons.filled.Close import androidx.compose.material.icons.filled.Delete @@ -57,6 +58,7 @@ 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_EXPLANATION /** Stable test tags for the organization detail screen. */ object OrganizationDetailTestTags { @@ -72,6 +74,12 @@ object OrganizationDetailTestTags { const val EDIT_DIALOG = "orgDetailEditDialog" const val DELETE_DIALOG = "orgDetailDeleteDialog" const val DELETE_CONFIRM = "orgDetailDeleteConfirm" + const val JOIN = "orgDetailJoin" + const val JOIN_PROMPT = "orgDetailJoinPrompt" + const val LEAVE = "orgDetailLeave" + const val LEAVE_DIALOG = "orgDetailLeaveDialog" + const val LEAVE_CONFIRM = "orgDetailLeaveConfirm" + const val LAST_OWNER_NOTICE = "orgDetailLastOwnerNotice" fun member(userId: String) = "orgMember_$userId" fun remove(userId: String) = "orgMemberRemove_$userId" fun candidate(userId: String) = "orgCandidate_$userId" @@ -79,13 +87,14 @@ object OrganizationDetailTestTags { /** * Hilt-wired entry for a single organization. Reads its `orgId` from the nav - * SavedStateHandle (see [ORG_ID_ARG]); [onBack] and [onDeleted] let the app pop - * navigation after viewing or deleting the org. + * SavedStateHandle (see [ORG_ID_ARG]); [onBack], [onDeleted] and [onLeft] let the + * app pop navigation after viewing, deleting, or leaving the org. */ @Composable fun OrganizationDetailRoute( onBack: () -> Unit, onDeleted: () -> Unit, + onLeft: () -> Unit, modifier: Modifier = Modifier, viewModel: OrganizationDetailViewModel = hiltViewModel(), ) { @@ -99,6 +108,8 @@ fun OrganizationDetailRoute( onRemoveMember = viewModel::removeMember, onSaveEdit = { name, description, isPublic -> viewModel.updateOrganization(name, description, isPublic) }, onDelete = { viewModel.deleteOrganization(onDeleted) }, + onJoin = viewModel::join, + onLeave = { viewModel.leave(onLeft) }, modifier = modifier, ) } @@ -115,11 +126,14 @@ fun OrganizationDetailScreen( onRemoveMember: (OrgMember) -> Unit, onSaveEdit: (name: String?, description: String?, isPublic: Boolean?) -> Unit, onDelete: () -> Unit, + onJoin: () -> Unit, + onLeave: () -> Unit, modifier: Modifier = Modifier, ) { var menuOpen by remember { mutableStateOf(false) } var showEdit by remember { mutableStateOf(false) } var showDeleteConfirm by remember { mutableStateOf(false) } + var showLeaveConfirm by remember { mutableStateOf(false) } Scaffold( modifier = modifier.fillMaxSize(), @@ -149,6 +163,16 @@ fun OrganizationDetailScreen( 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), + ) + } } }, ) @@ -186,26 +210,32 @@ fun OrganizationDetailScreen( ) } - 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), - ) + // 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), + ) - MemberList( - members = state.members, - candidates = state.candidates, - isEmpty = state.isEmpty, - onAddCandidate = onAddCandidate, - onChangeRole = onChangeRole, - onRemoveMember = onRemoveMember, - ) + MemberList( + members = state.members, + candidates = state.candidates, + isEmpty = state.isEmpty, + onAddCandidate = onAddCandidate, + onChangeRole = onChangeRole, + onRemoveMember = onRemoveMember, + ) + } else { + JoinPrompt(canJoin = state.canJoin, isJoining = state.isJoining, onJoin = onJoin) + } } } } @@ -221,6 +251,15 @@ fun OrganizationDetailScreen( ) } + if (showLeaveConfirm) { + LeaveOrganizationDialog( + organizationName = state.title, + isLastOwner = state.isLastOwner, + onDismiss = { showLeaveConfirm = false }, + onConfirm = { showLeaveConfirm = false; onLeave() }, + ) + } + if (showDeleteConfirm) { AlertDialog( onDismissRequest = { showDeleteConfirm = false }, @@ -240,6 +279,86 @@ fun OrganizationDetailScreen( } } +/** + * Confirmation before leaving. A sole owner is told why they cannot leave (the + * organization would be orphaned, and the server refuses with 400) and is offered + * no destructive action — only a way out of the dialog. + */ +@Composable +private fun LeaveOrganizationDialog( + organizationName: String, + isLastOwner: Boolean, + onDismiss: () -> Unit, + onConfirm: () -> Unit, +) { + AlertDialog( + onDismissRequest = onDismiss, + modifier = Modifier.testTag(OrganizationDetailTestTags.LEAVE_DIALOG), + title = { Text(if (isLastOwner) "You're the last owner" else "Leave organization?") }, + text = { + if (isLastOwner) { + Text( + text = LAST_OWNER_EXPLANATION, + modifier = Modifier.testTag(OrganizationDetailTestTags.LAST_OWNER_NOTICE), + ) + } else { + Text( + "You'll lose access to \"$organizationName\". " + + "You can join again while it stays public.", + ) + } + }, + confirmButton = { + if (isLastOwner) { + TextButton(onClick = onDismiss) { Text("Got it") } + } else { + TextButton( + onClick = onConfirm, + modifier = Modifier.testTag(OrganizationDetailTestTags.LEAVE_CONFIRM), + ) { Text("Leave") } + } + }, + dismissButton = { + if (!isLastOwner) TextButton(onClick = onDismiss) { Text("Cancel") } + }, + ) +} + +/** Shown to a non-member: join a public org, or explain that a private one needs an invite. */ +@Composable +private fun JoinPrompt(canJoin: Boolean, isJoining: Boolean, onJoin: () -> Unit) { + Column( + modifier = Modifier + .fillMaxWidth() + .padding(24.dp) + .testTag(OrganizationDetailTestTags.JOIN_PROMPT), + horizontalAlignment = Alignment.CenterHorizontally, + ) { + Text( + text = if (canJoin) "You're not a member yet" else "Members only", + style = MaterialTheme.typography.titleMedium, + ) + Spacer(Modifier.height(4.dp)) + Text( + text = if (canJoin) { + "Join to see its members and take part." + } else { + "This organization is private. Ask an owner or admin to add you." + }, + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + if (canJoin) { + Spacer(Modifier.height(12.dp)) + Button( + onClick = onJoin, + enabled = !isJoining, + modifier = Modifier.testTag(OrganizationDetailTestTags.JOIN), + ) { Text(if (isJoining) "Joining…" else "Join") } + } + } +} + @Composable private fun OrganizationHeader(org: Organization) { Column(Modifier.padding(horizontal = 16.dp, vertical = 8.dp)) { @@ -480,6 +599,8 @@ private fun OrganizationDetailScreenPreview() { onRemoveMember = {}, onSaveEdit = { _, _, _ -> }, onDelete = {}, + onJoin = {}, + onLeave = {}, ) } } 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 30078ad..7e3b423 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 @@ -9,7 +9,10 @@ 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_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.toUserMessage import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.MutableStateFlow @@ -31,13 +34,32 @@ data class OrganizationDetailUiState( val errorMessage: String? = null, val subscriptionRequired: Boolean = false, val deleted: Boolean = false, + // Membership (join / leave) in flight. + val isJoining: Boolean = false, + val isLeaving: Boolean = false, // Member search / add. val searchQuery: String = "", val candidates: List = emptyList(), val isSearching: Boolean = false, ) { val title: String get() = organization?.displayName.orEmpty() - val isEmpty: Boolean get() = members.isEmpty() && !isLoading && errorMessage == null + val isEmpty: Boolean get() = members.isEmpty() && !isLoading && errorMessage == null && isMember + + /** Whether the signed-in user belongs to this organization. */ + val isMember: Boolean get() = organization?.isMember == true + + /** A public organization the user has not joined can be joined from here. */ + val canJoin: Boolean get() = organization?.canJoin == true + + /** + * True when the user is this organization's only owner. Leaving would orphan + * the organization, and the server refuses it (400 "Cannot remove the last + * 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 } @HiltViewModel @@ -60,17 +82,28 @@ class OrganizationDetailViewModel @Inject constructor( fun load() { _uiState.update { it.copy(isLoading = true, errorMessage = null, subscriptionRequired = false) } viewModelScope.launch { - when (val result = repository.getOrganization(orgId)) { - is ApiResult.Success -> _uiState.update { it.copy(organization = result.data, isLoading = false) } - is ApiResult.Failure -> _uiState.update { - it.copy( - isLoading = false, - errorMessage = result.error.toUserMessage(), - subscriptionRequired = result.error.isSubscriptionGate, - ) + val organization = when (val result = repository.getOrganization(orgId)) { + is ApiResult.Success -> { + _uiState.update { it.copy(organization = result.data, isLoading = false) } + result.data + } + is ApiResult.Failure -> { + _uiState.update { + it.copy( + isLoading = false, + errorMessage = result.error.toUserMessage(), + subscriptionRequired = result.error.isSubscriptionGate, + ) + } + null } } - // Members are loaded after metadata; a failure surfaces but keeps the header. + // 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) { + _uiState.update { it.copy(members = emptyList()) } + return@launch + } when (val members = repository.getMembers(orgId)) { is ApiResult.Success -> _uiState.update { it.copy(members = members.data) } is ApiResult.Failure -> _uiState.update { @@ -80,6 +113,49 @@ class OrganizationDetailViewModel @Inject constructor( } } + /** Joins this (public) organization, then reloads so membership state is server-truth. */ + fun join() { + if (_uiState.value.isJoining) return + _uiState.update { it.copy(isJoining = true, errorMessage = null) } + viewModelScope.launch { + when (val result = repository.joinOrganization(orgId)) { + is ApiResult.Success -> { + _uiState.update { it.copy(isJoining = false) } + load() + } + is ApiResult.Failure -> _uiState.update { + it.copy(isJoining = false, errorMessage = result.error.toJoinMessage()) + } + } + } + } + + /** + * Leaves this organization. A sole owner is stopped with an explanation rather + * than a failed request; if the server refuses anyway (the member list may not + * have loaded) that rejection is explained the same way. + */ + fun leave(onLeft: () -> Unit = {}) { + val state = _uiState.value + if (state.isLeaving) return + if (state.isLastOwner) { + _uiState.update { it.copy(errorMessage = LAST_OWNER_EXPLANATION) } + return + } + _uiState.update { it.copy(isLeaving = true, errorMessage = null) } + viewModelScope.launch { + when (val result = repository.leaveOrganization(orgId)) { + is ApiResult.Success -> { + _uiState.update { it.copy(isLeaving = false) } + onLeft() + } + is ApiResult.Failure -> _uiState.update { + it.copy(isLeaving = false, errorMessage = result.error.toLeaveMessage()) + } + } + } + } + fun updateOrganization( name: String?, description: String?, diff --git a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/ui/list/OrganizationsScreen.kt b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/ui/list/OrganizationsScreen.kt index 755356d..d96eeb0 100644 --- a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/ui/list/OrganizationsScreen.kt +++ b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/ui/list/OrganizationsScreen.kt @@ -59,6 +59,8 @@ object OrganizationsTestTags { const val CREATE_NAME = "organizationsCreateName" const val CREATE_CONFIRM = "organizationsCreateConfirm" fun row(id: String) = "organizationRow_$id" + fun join(id: String) = "organizationJoin_$id" + fun membership(id: String) = "organizationMembership_$id" } /** @@ -79,6 +81,7 @@ fun OrganizationsRoute( onOpenOrg = onOpenOrg, onBack = onBack, onLoadMore = viewModel::loadMore, + onJoinOrg = viewModel::joinOrganization, onCreateOrganization = { name, description, isPublic -> viewModel.createOrganization(name, description, isPublic, onCreated = { onOpenOrg(it.id) }) }, @@ -94,6 +97,7 @@ fun OrganizationsScreen( onOpenOrg: (String) -> Unit, onBack: () -> Unit, onLoadMore: () -> Unit, + onJoinOrg: (String) -> Unit, onCreateOrganization: (name: String, description: String?, isPublic: Boolean) -> Unit, modifier: Modifier = Modifier, ) { @@ -157,8 +161,10 @@ fun OrganizationsScreen( organizations = state.organizations, isLoadingMore = state.isLoadingMore, hasMore = state.hasMore, + joiningOrgIds = state.joiningOrgIds, onOpenOrg = onOpenOrg, onLoadMore = onLoadMore, + onJoinOrg = onJoinOrg, ) } } @@ -181,8 +187,10 @@ private fun OrganizationsList( organizations: List, isLoadingMore: Boolean, hasMore: Boolean, + joiningOrgIds: Set, onOpenOrg: (String) -> Unit, onLoadMore: () -> Unit, + onJoinOrg: (String) -> Unit, ) { LazyColumn( modifier = Modifier @@ -192,7 +200,12 @@ private fun OrganizationsList( verticalArrangement = Arrangement.spacedBy(12.dp), ) { items(organizations, key = { it.id }) { org -> - OrganizationCard(org = org, onClick = { onOpenOrg(org.id) }) + OrganizationCard( + org = org, + isJoining = org.id in joiningOrgIds, + onClick = { onOpenOrg(org.id) }, + onJoin = { onJoinOrg(org.id) }, + ) } if (hasMore) { item { @@ -213,7 +226,12 @@ private fun LaunchedLoadMore(onLoadMore: () -> Unit) { @OptIn(ExperimentalMaterial3Api::class) @Composable -private fun OrganizationCard(org: Organization, onClick: () -> Unit) { +private fun OrganizationCard( + org: Organization, + isJoining: Boolean, + onClick: () -> Unit, + onJoin: () -> Unit, +) { Card( onClick = onClick, modifier = Modifier @@ -238,7 +256,11 @@ private fun OrganizationCard(org: Organization, onClick: () -> Unit) { ) } Spacer(Modifier.height(8.dp)) - Row(horizontalArrangement = Arrangement.spacedBy(8.dp)) { + Row( + modifier = Modifier.fillMaxWidth(), + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.spacedBy(8.dp), + ) { Text( text = "${org.memberCount} ${if (org.memberCount == 1) "member" else "members"}", style = MaterialTheme.typography.labelMedium, @@ -251,13 +273,24 @@ private fun OrganizationCard(org: Organization, onClick: () -> Unit) { color = MaterialTheme.colorScheme.secondary, ) } + Spacer(Modifier.weight(1f)) + // Membership drives the affordance: members see their role, while a + // public org they have not joined offers Join. org.role?.let { role -> Text( text = role.label, style = MaterialTheme.typography.labelMedium, color = MaterialTheme.colorScheme.tertiary, + modifier = Modifier.testTag(OrganizationsTestTags.membership(org.id)), ) } + if (org.canJoin) { + TextButton( + onClick = onJoin, + enabled = !isJoining, + modifier = Modifier.testTag(OrganizationsTestTags.join(org.id)), + ) { Text(if (isJoining) "Joining…" else "Join") } + } } } } @@ -372,6 +405,7 @@ private fun OrganizationsScreenPreview() { onOpenOrg = {}, onBack = {}, onLoadMore = {}, + onJoinOrg = {}, onCreateOrganization = { _, _, _ -> }, ) } diff --git a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/ui/list/OrganizationsViewModel.kt b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/ui/list/OrganizationsViewModel.kt index 205bc2e..6498774 100644 --- a/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/ui/list/OrganizationsViewModel.kt +++ b/feature/organizations/src/main/kotlin/com/interlinedlist/android/feature/organizations/ui/list/OrganizationsViewModel.kt @@ -6,6 +6,7 @@ import com.interlinedlist.android.core.common.result.ApiResult import com.interlinedlist.android.feature.organizations.data.OrganizationsRepository import com.interlinedlist.android.feature.organizations.domain.Organization import com.interlinedlist.android.feature.organizations.ui.isSubscriptionGate +import com.interlinedlist.android.feature.organizations.ui.toJoinMessage import com.interlinedlist.android.feature.organizations.ui.toUserMessage import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.MutableStateFlow @@ -27,6 +28,8 @@ data class OrganizationsUiState( val nextOffset: Int = 0, val errorMessage: String? = null, val subscriptionRequired: Boolean = false, + /** Ids of organizations whose join request is in flight. */ + val joiningOrgIds: Set = emptySet(), ) { val isEmpty: Boolean get() = organizations.isEmpty() && !isRefreshing && errorMessage == null && !subscriptionRequired @@ -130,5 +133,24 @@ class OrganizationsViewModel @Inject constructor( } } + /** + * Joins a public organization listed in the index. The repository refreshes the + * cached row on success, so the card flips to its member state through the Room + * stream without a full reload. + */ + fun joinOrganization(orgId: String) { + if (orgId in _uiState.value.joiningOrgIds) return + _uiState.update { it.copy(joiningOrgIds = it.joiningOrgIds + orgId, errorMessage = null) } + viewModelScope.launch { + val result = repository.joinOrganization(orgId) + _uiState.update { state -> + state.copy( + joiningOrgIds = state.joiningOrgIds - orgId, + errorMessage = (result as? ApiResult.Failure)?.error?.toJoinMessage(), + ) + } + } + } + fun clearError() = _uiState.update { it.copy(errorMessage = null) } } 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 9be7885..4682ecd 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,11 +32,15 @@ class FakeOrganizationsRepository : OrganizationsRepository { var addMemberResult: ApiResult = ApiResult.Success(Unit) var updateRoleResult: ApiResult = ApiResult.Success(Unit) var removeMemberResult: ApiResult = ApiResult.Success(Unit) + var joinResult: ApiResult = ApiResult.Success(Unit) + var leaveResult: ApiResult = ApiResult.Success(Unit) var refreshCount = 0 var loadMoreCount = 0 var addMemberCount = 0 var removeMemberCount = 0 + var joinedOrgIds = mutableListOf() + var leftOrgIds = mutableListOf() var lastMemberSearch: String? = null var lastUpdate: Triple? = null @@ -84,6 +88,27 @@ class FakeOrganizationsRepository : OrganizationsRepository { return deleteResult } + override suspend fun joinOrganization(orgId: String): ApiResult { + joinedOrgIds += orgId + if (joinResult is ApiResult.Success) { + // Mirrors the repository: the cached row gains the caller's membership. + cache.value = cache.value.map { org -> + if (org.id == orgId) org.copy(role = OrgRole.MEMBER, memberCount = org.memberCount + 1) else org + } + } + return joinResult + } + + override suspend fun leaveOrganization(orgId: String): ApiResult { + leftOrgIds += orgId + if (leaveResult is ApiResult.Success) { + cache.value = cache.value.map { org -> + if (org.id == orgId) org.copy(role = null, memberCount = (org.memberCount - 1).coerceAtLeast(0)) else org + } + } + return leaveResult + } + override suspend fun getMembers(orgId: String, limit: Int): ApiResult> = membersResult override suspend fun searchMemberCandidates( @@ -111,5 +136,9 @@ class FakeOrganizationsRepository : OrganizationsRepository { companion object { fun subscriptionFailure(): ApiResult.Failure = ApiResult.Failure(AppError.SubscriptionRequired("Organizations require an active subscription")) + + /** 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")) } } 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 0558266..6e236b2 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 @@ -39,6 +39,10 @@ class DefaultOrganizationsRepositoryTest { private lateinit var api: OrganizationsApi private lateinit var dao: FakeOrganizationDao private lateinit var repository: DefaultOrganizationsRepository + private lateinit var currentUserId: CurrentUserIdProvider + + /** The signed-in user the leave call must address; null models a lost session. */ + private var signedInUserId: String? = "me-1" private val json = Json { ignoreUnknownKeys = true; explicitNulls = false } private val dispatcher = StandardTestDispatcher() @@ -58,7 +62,8 @@ class DefaultOrganizationsRepositoryTest { .build() .create(OrganizationsApi::class.java) dao = FakeOrganizationDao() - repository = DefaultOrganizationsRepository(api, dao, json, testDispatchers) + currentUserId = CurrentUserIdProvider { signedInUserId } + repository = DefaultOrganizationsRepository(api, dao, json, currentUserId, testDispatchers) } @After @@ -229,6 +234,130 @@ class DefaultOrganizationsRepositoryTest { assertThat(request.body.readUtf8()).contains("\"role\":\"owner\"") } + @Test + fun `joinOrganization posts the organization id and caches the new membership`() = runTest(dispatcher) { + // Live contract: POST /api/user/organizations { "organizationId": "..." } -> 201. + server.enqueue( + MockResponse().setResponseCode(201).setBody( + """{ "message": "Joined organization successfully", "membership": { "id": "m1", "role": "member" } }""", + ), + ) + // The repository re-reads the org so the cached row carries the new role. + server.enqueue( + MockResponse().setBody( + """{ "organization": { "id": "o1", "name": "Bikey Life", "userRole": "member", "memberCount": 4 } }""", + ), + ) + + val result = repository.joinOrganization("o1") + + assertThat(result).isInstanceOf(ApiResult.Success::class.java) + val join = server.takeRequest() + assertThat(join.method).isEqualTo("POST") + assertThat(join.path).isEqualTo("/api/user/organizations") + assertThat(join.body.readUtf8()).isEqualTo("""{"organizationId":"o1"}""") + assertThat(server.takeRequest().path).isEqualTo("/api/organizations/o1") + // Membership state is now cached: the index flips from "Join" to "Member". + val cached = dao.observeOrganizations().first().single() + assertThat(cached.role).isEqualTo("member") + assertThat(cached.memberCount).isEqualTo(4) + } + + @Test + fun `joinOrganization surfaces the already-a-member conflict`() = runTest(dispatcher) { + server.enqueue( + MockResponse().setResponseCode(409) + .setBody("""{ "error": "User is already a member of this organization", "code": "conflict" }"""), + ) + + val result = repository.joinOrganization("o1") + + assertThat(result).isInstanceOf(ApiResult.Failure::class.java) + val error = (result as ApiResult.Failure).error + assertThat(error).isInstanceOf(AppError.Conflict::class.java) + assertThat(error.message).isEqualTo("User is already a member of this organization") + } + + @Test + fun `leaveOrganization deletes the signed-in user's membership and refreshes the org`() = + runTest(dispatcher) { + dao.upsert(CachedOrganizationEntity("o1", "Bikey Life", null, null, true, 3, "member", null)) + server.enqueue(MockResponse().setBody("""{ "message": "Member removed from organization successfully" }""")) + server.enqueue( + MockResponse().setBody( + """{ "organization": { "id": "o1", "name": "Bikey Life", "userRole": null, "memberCount": 2 } }""", + ), + ) + + val result = repository.leaveOrganization("o1") + + assertThat(result).isInstanceOf(ApiResult.Success::class.java) + val leave = server.takeRequest() + assertThat(leave.method).isEqualTo("DELETE") + // Leaving is removing your own membership row. + assertThat(leave.path).isEqualTo("/api/organizations/o1/members/me-1") + // The cached row keeps the org but drops the membership, so the UI offers "Join". + val cached = dao.observeOrganizations().first().single() + assertThat(cached.role).isNull() + assertThat(cached.memberCount).isEqualTo(2) + } + + @Test + fun `leaveOrganization evicts an org that is no longer visible after leaving`() = runTest(dispatcher) { + dao.upsert(CachedOrganizationEntity("o1", "Private Co", null, null, false, 3, "member", null)) + server.enqueue(MockResponse().setBody("""{ "message": "Member removed from organization successfully" }""")) + // A private org is invisible once you are no longer a member. + server.enqueue(MockResponse().setResponseCode(403).setBody("""{ "error": "Forbidden", "code": "forbidden" }""")) + + val result = repository.leaveOrganization("o1") + + assertThat(result).isInstanceOf(ApiResult.Success::class.java) + assertThat(dao.observeOrganizations().first()).isEmpty() + } + + @Test + fun `leaveOrganization surfaces the last-owner rejection and keeps the membership cached`() = + runTest(dispatcher) { + dao.upsert(CachedOrganizationEntity("o1", "Acme", null, null, false, 1, "owner", null)) + // Captured live: DELETE .../members/{me} as the sole owner -> 400. + server.enqueue( + MockResponse().setResponseCode(400) + .setBody("""{ "error": "Cannot remove the last owner", "code": "bad_request" }"""), + ) + + val result = repository.leaveOrganization("o1") + + assertThat(result).isInstanceOf(ApiResult.Failure::class.java) + assertThat((result as ApiResult.Failure).error.message).isEqualTo("Cannot remove the last owner") + // Still a member: the cache must not pretend the leave happened. + assertThat(dao.observeOrganizations().first().single().role).isEqualTo("owner") + } + + @Test + fun `leaveOrganization fails without calling the API when the user id is unknown`() = runTest(dispatcher) { + signedInUserId = null + + val result = repository.leaveOrganization("o1") + + assertThat(result).isInstanceOf(ApiResult.Failure::class.java) + assertThat((result as ApiResult.Failure).error).isInstanceOf(AppError.Unauthorized::class.java) + assertThat(server.requestCount).isEqualTo(0) + } + + @Test + fun `getOrganization reads membership from the detail endpoint's userRole`() = runTest(dispatcher) { + // The detail endpoint reports membership as `userRole` (null for non-members). + server.enqueue( + MockResponse().setBody("""{ "organization": { "id": "o1", "name": "Metals", "userRole": null } }"""), + ) + + val result = repository.getOrganization("o1") + + val org = (result as ApiResult.Success).data + assertThat(org.role).isNull() + assertThat(org.isMember).isFalse() + } + @Test fun `removeMember deletes the membership`() = runTest(dispatcher) { server.enqueue(MockResponse().setResponseCode(200).setBody("{}")) 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 new file mode 100644 index 0000000..78c83b7 --- /dev/null +++ b/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/ui/OrganizationsErrorMessagesTest.kt @@ -0,0 +1,51 @@ +package com.interlinedlist.android.feature.organizations.ui + +import com.google.common.truth.Truth.assertThat +import com.interlinedlist.android.core.common.result.AppError +import org.junit.Test + +/** + * The join/leave failures the live API actually returns, turned into explanations. + * Messages captured against interlinedlist.com: 409 "User is already a member of + * this organization", 403 for a private org, and 400 "Cannot remove the last owner". + */ +class OrganizationsErrorMessagesTest { + + @Test + fun `last-owner rejection is explained, not echoed as a generic error`() { + val error = AppError.Unknown("Cannot remove the last owner") + + assertThat(error.isLastOwnerRejection).isTrue() + assertThat(error.toLeaveMessage()).isEqualTo(LAST_OWNER_EXPLANATION) + assertThat(error.toLeaveMessage()).contains("Make another member an owner") + // The generic mapping would have leaked the raw API string. + assertThat(error.toLeaveMessage()).isNotEqualTo(error.toUserMessage()) + } + + @Test + fun `other leave failures fall back to the shared mapping`() { + assertThat(AppError.Network(null).toLeaveMessage()) + .isEqualTo(AppError.Network(null).toUserMessage()) + } + + @Test + fun `joining an org you already belong to reads as a conflict`() { + val error = AppError.Conflict("User is already a member of this organization") + + assertThat(error.toJoinMessage()).isEqualTo("You're already a member of this organization.") + } + + @Test + fun `joining a private org explains it cannot be self-joined`() { + val error = AppError.Forbidden("Organization is private") + + assertThat(error.toJoinMessage()).contains("private") + assertThat(error.toJoinMessage()).contains("Ask an owner or admin") + } + + @Test + fun `joining a missing org reports it as not found`() { + assertThat(AppError.NotFound("Organization not found").toJoinMessage()) + .isEqualTo("That organization could not be found.") + } +} 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 f7557e3..ad19570 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 @@ -3,11 +3,13 @@ package com.interlinedlist.android.feature.organizations.ui.detail import androidx.lifecycle.SavedStateHandle import com.google.common.truth.Truth.assertThat import com.interlinedlist.android.core.common.result.ApiResult +import com.interlinedlist.android.core.common.result.AppError import com.interlinedlist.android.feature.organizations.FakeOrganizationsRepository 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_EXPLANATION import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.StandardTestDispatcher @@ -174,4 +176,162 @@ class OrganizationDetailViewModelTest { assertThat(repo.removeMemberCount).isEqualTo(1) assertThat(vm.uiState.value.members.map { it.userId }).containsExactly("u2") } + + @Test + fun `a non-member sees the join affordance and no member list request`() = runTest(dispatcher) { + val repo = FakeOrganizationsRepository().apply { + // No role means "not a member" — how the API reports it. + getResult = ApiResult.Success(Organization("o1", "Metals", null, null, true, 1, null, null)) + membersResult = ApiResult.Success(listOf(member("u1", OrgRole.OWNER))) + } + val vm = vmFor(repo) + advanceUntilIdle() + + val state = vm.uiState.value + assertThat(state.isMember).isFalse() + assertThat(state.canJoin).isTrue() + // Members are members-only on the server, so they are not requested or shown. + assertThat(state.members).isEmpty() + assertThat(state.errorMessage).isNull() + } + + @Test + fun `a member sees membership state and the loaded member list`() = runTest(dispatcher) { + val repo = FakeOrganizationsRepository().apply { + getResult = ApiResult.Success( + Organization("o1", "Bikey Life", null, null, true, 3, OrgRole.MEMBER, null), + ) + membersResult = ApiResult.Success(listOf(member("u1", OrgRole.OWNER), member("u2"))) + } + val vm = vmFor(repo) + advanceUntilIdle() + + val state = vm.uiState.value + assertThat(state.isMember).isTrue() + assertThat(state.canJoin).isFalse() + assertThat(state.members).hasSize(2) + } + + @Test + fun `join reloads so membership state reflects the server`() = runTest(dispatcher) { + val repo = FakeOrganizationsRepository().apply { + getResult = ApiResult.Success(Organization("o1", "Metals", null, null, true, 1, null, null)) + } + val vm = vmFor(repo) + advanceUntilIdle() + + // After joining, the org reads back as a membership. + repo.getResult = ApiResult.Success( + Organization("o1", "Metals", null, null, true, 2, OrgRole.MEMBER, null), + ) + repo.membersResult = ApiResult.Success(listOf(member("u1", OrgRole.OWNER), member("me"))) + vm.join() + advanceUntilIdle() + + assertThat(repo.joinedOrgIds).containsExactly("o1") + val state = vm.uiState.value + assertThat(state.isJoining).isFalse() + assertThat(state.isMember).isTrue() + assertThat(state.members.map { it.userId }).containsExactly("u1", "me").inOrder() + } + + @Test + fun `join failure explains an existing membership`() = runTest(dispatcher) { + val repo = FakeOrganizationsRepository().apply { + getResult = ApiResult.Success(Organization("o1", "Metals", null, null, true, 1, null, null)) + joinResult = ApiResult.Failure(AppError.Conflict("User is already a member of this organization")) + } + val vm = vmFor(repo) + advanceUntilIdle() + + vm.join() + advanceUntilIdle() + + assertThat(vm.uiState.value.isJoining).isFalse() + assertThat(vm.uiState.value.errorMessage).isEqualTo("You're already a member of this organization.") + } + + @Test + fun `leave removes the membership and hands back to the caller`() = runTest(dispatcher) { + val repo = FakeOrganizationsRepository().apply { + getResult = ApiResult.Success( + Organization("o1", "Bikey Life", null, null, true, 3, OrgRole.MEMBER, null), + ) + membersResult = ApiResult.Success(listOf(member("u1", OrgRole.OWNER), member("me"))) + } + val vm = vmFor(repo) + advanceUntilIdle() + + var left = false + vm.leave(onLeft = { left = true }) + advanceUntilIdle() + + assertThat(repo.leftOrgIds).containsExactly("o1") + assertThat(left).isTrue() + assertThat(vm.uiState.value.errorMessage).isNull() + } + + @Test + fun `the only owner is stopped with an explanation before any request`() = runTest(dispatcher) { + val repo = FakeOrganizationsRepository().apply { + getResult = ApiResult.Success( + Organization("o1", "Acme", null, null, false, 2, OrgRole.OWNER, null), + ) + membersResult = ApiResult.Success(listOf(member("me", OrgRole.OWNER), member("u2"))) + } + val vm = vmFor(repo) + advanceUntilIdle() + + assertThat(vm.uiState.value.isLastOwner).isTrue() + + var left = false + vm.leave(onLeft = { left = true }) + advanceUntilIdle() + + assertThat(repo.leftOrgIds).isEmpty() + assertThat(left).isFalse() + assertThat(vm.uiState.value.errorMessage).isEqualTo(LAST_OWNER_EXPLANATION) + } + + @Test + fun `an owner alongside another owner may leave`() = runTest(dispatcher) { + val repo = FakeOrganizationsRepository().apply { + getResult = ApiResult.Success( + Organization("o1", "Acme", null, null, false, 2, OrgRole.OWNER, null), + ) + membersResult = ApiResult.Success(listOf(member("me", OrgRole.OWNER), member("u2", OrgRole.OWNER))) + } + val vm = vmFor(repo) + advanceUntilIdle() + + assertThat(vm.uiState.value.isLastOwner).isFalse() + + vm.leave() + advanceUntilIdle() + + assertThat(repo.leftOrgIds).containsExactly("o1") + } + + @Test + fun `a server last-owner rejection is surfaced as the same explanation`() = runTest(dispatcher) { + val repo = FakeOrganizationsRepository().apply { + getResult = ApiResult.Success( + Organization("o1", "Acme", null, null, false, 1, OrgRole.OWNER, null), + ) + // The member list did not load, so the client-side guard cannot fire. + membersResult = ApiResult.Failure(AppError.Network(null)) + leaveResult = FakeOrganizationsRepository.lastOwnerFailure() + } + val vm = vmFor(repo) + advanceUntilIdle() + vm.clearError() + + var left = false + vm.leave(onLeft = { left = true }) + advanceUntilIdle() + + assertThat(left).isFalse() + assertThat(vm.uiState.value.isLeaving).isFalse() + assertThat(vm.uiState.value.errorMessage).isEqualTo(LAST_OWNER_EXPLANATION) + } } diff --git a/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/ui/list/OrganizationsViewModelTest.kt b/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/ui/list/OrganizationsViewModelTest.kt index f7de7ae..9f75a66 100644 --- a/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/ui/list/OrganizationsViewModelTest.kt +++ b/feature/organizations/src/test/kotlin/com/interlinedlist/android/feature/organizations/ui/list/OrganizationsViewModelTest.kt @@ -139,4 +139,59 @@ class OrganizationsViewModelTest { assertThat(vm.uiState.value.subscriptionRequired).isTrue() } + + @Test + fun `joining a public org flips the cached row to a membership`() = runTest(dispatcher) { + val public = Organization("o1", "Metals", null, null, true, 1, null, null) + val repo = FakeOrganizationsRepository().apply { + refreshResult = ApiResult.Success(Paged(listOf(public), hasMore = false, total = 1, offset = 1)) + } + val vm = OrganizationsViewModel(repo) + // Keep the combined Room stream active so the cache reaches the UI state. + backgroundScope.launch { vm.uiState.collect { } } + advanceUntilIdle() + + // Before joining the card offers Join. + assertThat(vm.uiState.value.organizations.single().canJoin).isTrue() + + vm.joinOrganization("o1") + advanceUntilIdle() + + assertThat(repo.joinedOrgIds).containsExactly("o1") + val joined = vm.uiState.value.organizations.single() + assertThat(joined.isMember).isTrue() + assertThat(joined.canJoin).isFalse() + assertThat(joined.memberCount).isEqualTo(2) + assertThat(vm.transientState.value.joiningOrgIds).isEmpty() + assertThat(vm.transientState.value.errorMessage).isNull() + } + + @Test + fun `a refused join surfaces an explanation and clears the in-flight flag`() = runTest(dispatcher) { + val repo = FakeOrganizationsRepository().apply { + joinResult = ApiResult.Failure(AppError.Forbidden("Organization is private")) + } + val vm = OrganizationsViewModel(repo) + advanceUntilIdle() + + vm.joinOrganization("o1") + advanceUntilIdle() + + val state = vm.transientState.value + assertThat(state.joiningOrgIds).isEmpty() + assertThat(state.errorMessage).contains("private") + } + + @Test + fun `a second join for the same org is ignored while one is in flight`() = runTest(dispatcher) { + val repo = FakeOrganizationsRepository() + val vm = OrganizationsViewModel(repo) + advanceUntilIdle() + + vm.joinOrganization("o1") + vm.joinOrganization("o1") + advanceUntilIdle() + + assertThat(repo.joinedOrgIds).containsExactly("o1") + } }