Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ import de.tabmates.features.tabgroup.presentation.navigation.GroupDetail
import de.tabmates.features.tabgroup.presentation.navigation.Home
import de.tabmates.features.tabgroup.presentation.navigation.JoinGroup
import de.tabmates.features.tabgroup.presentation.navigation.Profile
import de.tabmates.features.tabgroup.presentation.navigation.ObserveGroupRemovals
import de.tabmates.features.tabgroup.presentation.navigation.mainGraph
import de.tabmates.features.tabgroup.presentation.navigation.mainSerializersModule
import kotlinx.coroutines.Dispatchers
Expand Down Expand Up @@ -259,6 +260,9 @@ fun App() {
val topLevelTabs = remember { listOf(Home, Activity, Group, Profile) }
val snackbarHostState = remember { SnackbarHostState() }
val appScope = rememberCoroutineScope()
// Above both adaptive layouts on purpose: each builds its own mainGraph, and observing
// per layout would announce the same removal twice.
ObserveGroupRemovals(backStack = backStack, snackbarHostState = snackbarHostState)
val isAndroidBrowser = remember { isAndroidBrowser() }

if (currentKey is LoggedIn) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,12 @@ suspend inline fun <reified Response : Any> HttpClient.get(
suspend inline fun <reified Response : Any> HttpClient.delete(
route: String,
queryParams: Map<String, Any> = mapOf(),
// See [post]: inspected before the generic status handling, for calls whose failures can only
// be told apart by the response body (e.g. removing a group participant).
noinline mapKnownError: (suspend (HttpResponse) -> DataError.Remote?)? = null,
crossinline builder: HttpRequestBuilder.() -> Unit = {},
): Result<Response, DataError.Remote> {
return safeCall {
return safeCall(mapKnownError = mapKnownError) {
delete {
url(routeForRequest(route))
queryParams.forEach { (key, value) ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,15 @@ sealed interface DataError : Error {
// help — the only way out is a new build, so it is surfaced as the update prompt rather
// than a generic error. See UpgradeRequiredNotifier.
UPGRADE_REQUIRED,

// 400 with body { "code": "CANNOT_REMOVE_SELF" } when removing a group participant. Not
// reachable through the UI — your own row offers no remove — but mapped so a mistake
// reads as itself rather than a generic bad request. Leaving is a separate endpoint.
CANNOT_REMOVE_SELF,

// 403 with body { "code": "CANNOT_REMOVE_GROUP_CREATOR" }: the group's creator can only
// leave voluntarily. Distinguished from FORBIDDEN, which means the caller is not a member.
CANNOT_REMOVE_GROUP_CREATOR,
UNKNOWN,
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,6 @@
<string name="error_message_send_failed">Nachricht konnte nicht gesendet werden</string>
<string name="error_turnstile_retry">Verifizierung fehlgeschlagen. Bitte versuche es erneut.</string>
<string name="error_upgrade_required">Diese TabMates-Version wird nicht mehr unterstützt. Bitte aktualisiere die App.</string>
<string name="error_cannot_remove_self">Du kannst dich nicht selbst entfernen. Verlasse die Gruppe stattdessen in den Einstellungen.</string>
<string name="error_cannot_remove_group_creator">Die Person, die diese Gruppe erstellt hat, kann nicht entfernt werden.</string>
</resources>
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,6 @@
<string name="error_message_send_failed">Failed to send message</string>
<string name="error_turnstile_retry">Couldn't verify you're human. Please try again.</string>
<string name="error_upgrade_required">This version of TabMates is no longer supported. Please update to continue.</string>
<string name="error_cannot_remove_self">You cannot remove yourself. Leave the group from its settings instead.</string>
<string name="error_cannot_remove_group_creator">The person who created this group cannot be removed.</string>
</resources>
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package de.tabmates.core.presentation.util
import de.tabmates.core.domain.util.DataError
import tabmatesapp.core.presentation.generated.resources.Res
import tabmatesapp.core.presentation.generated.resources.error_bad_request
import tabmatesapp.core.presentation.generated.resources.error_cannot_remove_group_creator
import tabmatesapp.core.presentation.generated.resources.error_cannot_remove_self
import tabmatesapp.core.presentation.generated.resources.error_conflict
import tabmatesapp.core.presentation.generated.resources.error_disk_full
import tabmatesapp.core.presentation.generated.resources.error_forbidden
Expand Down Expand Up @@ -40,6 +42,8 @@ fun DataError.toUiText(): UiText {
DataError.Remote.SERIALIZATION -> Res.string.error_serialization
DataError.Remote.TURNSTILE_FAILED -> Res.string.error_turnstile_retry
DataError.Remote.UPGRADE_REQUIRED -> Res.string.error_upgrade_required
DataError.Remote.CANNOT_REMOVE_SELF -> Res.string.error_cannot_remove_self
DataError.Remote.CANNOT_REMOVE_GROUP_CREATOR -> Res.string.error_cannot_remove_group_creator
DataError.Remote.UNKNOWN -> Res.string.error_unknown
DataError.Connection.NOT_CONNECTED -> Res.string.error_no_internet
DataError.Connection.MESSAGE_SEND_FAILED -> Res.string.error_message_send_failed
Expand Down
1 change: 1 addition & 0 deletions features/tabgroup/data/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ kotlin {
dependencies {
implementation(libs.kotlin.test)
implementation(libs.kotlinx.coroutines.test)
implementation(libs.ktor.client.mock)
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
package de.tabmates.features.tabgroup.data.di

import de.tabmates.features.tabgroup.domain.group.GroupRemovalNotifier
import org.koin.core.annotation.ComponentScan
import org.koin.core.annotation.Configuration
import org.koin.core.annotation.Module
import org.koin.core.annotation.Single

@Module
@Configuration
@ComponentScan("de.tabmates.features.tabgroup.data")
class TabgroupDataModule
class TabgroupDataModule {
// Lives in the domain module, which carries no Koin annotations — same arrangement as
// UpgradeRequiredNotifier, which CoreDataModule provides for the same reason.
@Single
fun provideGroupRemovalNotifier(): GroupRemovalNotifier = GroupRemovalNotifier()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package de.tabmates.features.tabgroup.data.dto

import de.tabmates.core.domain.util.DataError
import io.ktor.client.call.body
import io.ktor.client.statement.HttpResponse
import kotlinx.coroutines.CancellationException
import kotlinx.serialization.Serializable

@Serializable
internal data class RemoveParticipantErrorDto(val code: String? = null)
Comment thread
DennisBauer marked this conversation as resolved.

/**
* Maps the two removal refusals the server states by code.
*
* Everything else — including the `403` a non-member gets and the `404` covering both an unknown
* group and an unknown target — returns `null` and falls through to the generic status handling.
* The catch tolerates non-JSON bodies; the shared `Json` already sets `ignoreUnknownKeys`. It stops
* short of cancellation, which has to reach the caller rather than be answered with a plain
* `BAD_REQUEST` from the generic handling below.
*/
internal suspend fun HttpResponse.removeParticipantErrorOrNull(): DataError.Remote? {
if (status.value != 400 && status.value != 403) return null
val code =
try {
body<RemoveParticipantErrorDto>().code
} catch (e: CancellationException) {
throw e
} catch (e: Exception) {
null
}
return when (code) {
"CANNOT_REMOVE_SELF" -> DataError.Remote.CANNOT_REMOVE_SELF
"CANNOT_REMOVE_GROUP_CREATOR" -> DataError.Remote.CANNOT_REMOVE_GROUP_CREATOR
else -> null
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import de.tabmates.core.domain.util.asEmptyResult
import de.tabmates.core.domain.util.map
import de.tabmates.features.tabgroup.data.dto.GroupDto
import de.tabmates.features.tabgroup.data.dto.GroupInvitePreviewDto
import de.tabmates.features.tabgroup.data.dto.removeParticipantErrorOrNull
import de.tabmates.features.tabgroup.data.dto.request.AddNewParticipantToGroupRequest
import de.tabmates.features.tabgroup.data.dto.request.CreateGroupRequest
import de.tabmates.features.tabgroup.data.dto.request.JoinGroupRequest
Expand Down Expand Up @@ -88,6 +89,17 @@ class KtorGroupService(
).asEmptyResult()
}

override suspend fun removeParticipant(
groupId: String,
userId: String,
): EmptyResult<DataError.Remote> {
return httpClient
.delete<Unit>(
route = "/api/group/$groupId/participants/$userId",
mapKnownError = { it.removeParticipantErrorOrNull() },
).asEmptyResult()
}

override suspend fun updateGroup(
groupId: String,
title: String,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,22 @@ class OfflineFirstGroupRepository(
}
}

override suspend fun removeParticipant(
groupId: String,
userId: String,
): EmptyResult<DataError.Remote> {
return groupService
.removeParticipant(groupId, userId)
.onSuccess {
// The endpoint answers with an empty body, so the local mirror is refreshed the
// long way round: fetchGroupById syncs the participant cross-refs, which is what
// flips the removed person to inactive. Failing that refresh (offline right after
// the server accepted) is not an error — the same GROUP_METADATA_CHANGED frame the
// other members get, or the next sync, converges anyway.
fetchGroupById(groupId)
}
}

override suspend fun addParticipantsToGroup(
groupId: String,
userIds: Set<String>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ object WsMessageType {
const val ACK = "ACK"
const val TAB_ENTRY_DELETED = "TAB_ENTRY_DELETED"
const val GROUP_METADATA_CHANGED = "GROUP_METADATA_CHANGED"

/**
* Unicast to the person just removed from a group, and sent before the server stops routing to
* them so it always lands. Someone who left on their own is not told — they already know.
*/
const val REMOVED_FROM_GROUP = "REMOVED_FROM_GROUP"
const val ACTIVITY_EVENT = "ACTIVITY_EVENT"
const val ERROR = "ERROR"
}
Expand Down Expand Up @@ -124,6 +130,15 @@ data class GroupMetadataChangedWsPayload(
val groupId: String,
)

/**
* Mirrors server `RemovedFromGroupDto`. Deliberately carries no title: the server has no reason to
* repeat what the client already has, so the name for the snackbar comes from the local mirror.
*/
@Serializable
data class RemovedFromGroupWsPayload(
val groupId: String,
)

/** Mirrors server `ErrorDto`. */
@Serializable
data class WsErrorPayload(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -660,8 +660,11 @@ class TabEntryOutbox(
DataError.Remote.PAYLOAD_TOO_LARGE,
DataError.Remote.SERIALIZATION,
// Turnstile only gates the auth endpoints, never this delete; classify as
// permanent for exhaustiveness (it would never clear on retry anyway).
// permanent for exhaustiveness (it would never clear on retry anyway). The
// same goes for the two participant-removal refusals.
DataError.Remote.TURNSTILE_FAILED,
DataError.Remote.CANNOT_REMOVE_SELF,
DataError.Remote.CANNOT_REMOVE_GROUP_CREATOR,
-> DispatchResult.Permanent(result.error.name.lowercase())
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,18 @@ import de.tabmates.features.tabgroup.data.mappers.toDomain
import de.tabmates.features.tabgroup.data.mappers.toEntity
import de.tabmates.features.tabgroup.data.network.WebSocketChannel
import de.tabmates.features.tabgroup.data.network.dto.GroupMetadataChangedWsPayload
import de.tabmates.features.tabgroup.data.network.dto.RemovedFromGroupWsPayload
import de.tabmates.features.tabgroup.data.network.dto.TabEntryDeletedWsPayload
import de.tabmates.features.tabgroup.data.network.dto.WebSocketMessageDto
import de.tabmates.features.tabgroup.data.network.dto.WsErrorPayload
import de.tabmates.features.tabgroup.data.network.dto.WsMessageType
import de.tabmates.features.tabgroup.database.TabMatesDatabase
import de.tabmates.features.tabgroup.domain.group.GroupRemovalNotifier
import de.tabmates.features.tabgroup.domain.group.GroupRepository
import de.tabmates.features.tabgroup.domain.models.TabEntry
import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
import kotlinx.serialization.json.Json
Expand All @@ -34,6 +38,7 @@ class TabEntryRealtimeSync(
webSocketConnector: WebSocketChannel,
private val database: TabMatesDatabase,
private val groupRepository: GroupRepository,
private val groupRemovalNotifier: GroupRemovalNotifier,
private val json: Json,
private val logger: TabMatesLogger,
@Named(APPLICATION_SCOPE) private val applicationScope: CoroutineScope,
Expand Down Expand Up @@ -62,13 +67,19 @@ class TabEntryRealtimeSync(

WsMessageType.GROUP_METADATA_CHANGED -> handleGroupMetadataChanged(message.payload)

WsMessageType.REMOVED_FROM_GROUP -> handleRemovedFromGroup(message.payload)

Comment thread
coderabbitai[bot] marked this conversation as resolved.
// Owned by ActivityRealtimeSync; named here only to keep it out of the unknown-type log.
WsMessageType.ACTIVITY_EVENT -> Unit

WsMessageType.ERROR -> handleError(message.payload)

else -> logger.warning(TAG, "Unknown WS message type=${message.type}")
}
// Every branch above suspends, so a cancelled scope surfaces here as an exception like
// any other — let it through rather than log it as a handler failure and carry on.
} catch (e: CancellationException) {
throw e
} catch (e: Throwable) {
logger.error(TAG, "Failed to handle WS message of type ${message.type}", e)
}
Expand Down Expand Up @@ -132,6 +143,24 @@ class TabEntryRealtimeSync(
}
}

private suspend fun handleRemovedFromGroup(payload: String) {
val event = json.decodeFromString(RemovedFromGroupWsPayload.serializer(), payload)
// Read before the delete: the payload carries no title, and the local row is the only place
// the group's name still exists once it is gone.
val title =
groupRepository
.getGroups()
.first()
.firstOrNull { it.id == event.groupId }
?.title
logger.debug(TAG, "Removed from group ${event.groupId}")
// Told before the delete too, so the shell can pop this group's screens rather than let
// them re-render against a group that no longer exists.
groupRemovalNotifier.notifyRemoved(groupId = event.groupId, title = title)
// Cascades the cross-refs, entries and activity rows that belong to it.
database.groupDao.deleteGroupById(event.groupId)
}

// Diagnostic only — TabEntryOutbox owns what an error does to the write that caused it.
private fun handleError(payload: String) {
val error = json.decodeFromString(WsErrorPayload.serializer(), payload)
Expand Down
Loading