feat(tabgroup): remove people from a group & handle being removed from a group - #339
Merged
Conversation
Screens resolved participants from the active member list, so anyone removed from a group vanished from the maths even though their expenses stay. Worst case: editing an expense dropped a non-member's split on save, deleting it for good. Union active members with the participants the entries reference, and resolve names through the global participant table.
📝 WalkthroughWalkthroughThe change adds participant removal with known error handling, real-time group-removal notifications, navigation cleanup, and former-member support across group data, settlement, entry, and presentation flows. ChangesGroup participant removal
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/AddEntryViewModel.kt (1)
180-231: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPreserve unresolved former split IDs before enabling save.
If
getAllParticipants()does not return a referenced former participant, Lines 191-199 omit that participant fromsplitParticipants. Lines 205-206 also omit the participant fromparticipantsById.
buildSplits()uses onlystate.splitInputs. Saving the edited entry then deletes the unresolved participant's existing split.paidByDisplay()already treats a missing participant record as a valid historical state.Keep unresolved split participants in state with a removed-member placeholder, or block saving until their identity data resolves. Do not build an update request from an incomplete participant lookup.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/AddEntryViewModel.kt` around lines 180 - 231, Preserve every referenced former participant with an existing split when loading the edit state, even if getAllParticipants() cannot resolve its identity. Update the flow around formerParticipants, splitParticipants, participantsById, and buildSplits() to retain unresolved IDs via a removed-member placeholder or prevent saving until all identities resolve; do not construct an update request from incomplete participant data.
🧹 Nitpick comments (1)
features/tabgroup/data/src/desktopTest/kotlin/de/tabmates/features/tabgroup/data/tabentry/TabEntryRealtimeSyncTest.kt (1)
84-107: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse Turbine to collect the removal event.
Line 91 starts a manual background collector and stores events in mutable state. Collect
removalNotifier.removalswith Turbine and assert theRemovedFromGroupvalue withawaitItem().This makes event delivery explicit and removes collector lifecycle handling from the test.
As per coding guidelines,
**/*Test.ktrequires kotlin-test, Turbine, and hand-written fakes; do not use a mocking library.Also applies to: 165-190
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@features/tabgroup/data/src/desktopTest/kotlin/de/tabmates/features/tabgroup/data/tabentry/TabEntryRealtimeSyncTest.kt` around lines 84 - 107, Replace the manual background collector and mutable removals list in the test `being removed reports the group by its local name and then deletes it` with Turbine collection on `removalNotifier.removals`. Use Turbine’s `awaitItem()` to assert the expected `RemovedFromGroup` value after emitting the message, while retaining the existing deletion assertion and hand-written fake repository.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@features/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/dto/RemoveParticipantErrorDto.kt`:
- Around line 8-9: Rename RemoveParticipantErrorDto to
RemoveParticipantErrorResponse and rename its file accordingly, preserving the
nullable code field and serialization. Update the response decode at the
referenced usage to use RemoveParticipantErrorResponse, and adjust all
references to the old type name.
- Around line 18-20: Update removeParticipantErrorOrNull so
CancellationException thrown while decoding RemoveParticipantErrorDto is
rethrown instead of converted to null by runCatching. Preserve the existing null
fallback for other decoding failures and the current status handling.
In
`@features/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/tabentry/TabEntryRealtimeSync.kt`:
- Around line 69-70: Update the handle flow surrounding handleRemovedFromGroup
so CancellationException is caught before the broad Throwable handler and
rethrown unchanged. Preserve the existing logging and handling for
non-cancellation failures.
In
`@features/tabgroup/domain/src/commonMain/kotlin/de/tabmates/features/tabgroup/domain/group/GroupRemovalNotifier.kt`:
- Around line 13-24: The GroupRemovalNotifier currently drops removals emitted
before the app shell begins collecting. Update GroupRemovalNotifier to retain
pending RemovedFromGroup events until ObserveAsEvents consumes or acknowledges
them, while preserving non-suspending notification behavior; add a regression
test that emits a removal before collection starts and verifies the event is
later delivered.
In
`@features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/grouppeople/GroupPeopleScreen.kt`:
- Around line 259-271: Update the AlertDialog dismissal handling in
GroupPeopleScreen so both onDismissRequest and the dismissButton are disabled
while isRemoving is true. Preserve normal dismissal behavior once removal
completes, while preventing removeTarget from being cleared during the active
removal request.
---
Outside diff comments:
In
`@features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/AddEntryViewModel.kt`:
- Around line 180-231: Preserve every referenced former participant with an
existing split when loading the edit state, even if getAllParticipants() cannot
resolve its identity. Update the flow around formerParticipants,
splitParticipants, participantsById, and buildSplits() to retain unresolved IDs
via a removed-member placeholder or prevent saving until all identities resolve;
do not construct an update request from incomplete participant data.
---
Nitpick comments:
In
`@features/tabgroup/data/src/desktopTest/kotlin/de/tabmates/features/tabgroup/data/tabentry/TabEntryRealtimeSyncTest.kt`:
- Around line 84-107: Replace the manual background collector and mutable
removals list in the test `being removed reports the group by its local name and
then deletes it` with Turbine collection on `removalNotifier.removals`. Use
Turbine’s `awaitItem()` to assert the expected `RemovedFromGroup` value after
emitting the message, while retaining the existing deletion assertion and
hand-written fake repository.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 40113f97-b979-4b8a-8f15-ff7886c7cb5e
📒 Files selected for processing (49)
composeApp/src/commonMain/kotlin/de/tabmates/composeapp/App.ktcore/data/src/commonMain/kotlin/de/tabmates/core/data/networking/HttpClientExt.ktcore/domain/src/commonMain/kotlin/de/tabmates/core/domain/util/DataError.ktcore/presentation/src/commonMain/composeResources/values-de/string.xmlcore/presentation/src/commonMain/composeResources/values/string.xmlcore/presentation/src/commonMain/kotlin/de/tabmates/core/presentation/util/DataErrorToUiText.ktfeatures/tabgroup/data/build.gradle.ktsfeatures/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/di/TabgroupDataModule.ktfeatures/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/dto/RemoveParticipantErrorDto.ktfeatures/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/group/KtorGroupService.ktfeatures/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/group/OfflineFirstGroupRepository.ktfeatures/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/network/dto/TabEntryWsMessages.ktfeatures/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/tabentry/TabEntryOutbox.ktfeatures/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/tabentry/TabEntryRealtimeSync.ktfeatures/tabgroup/data/src/commonTest/kotlin/de/tabmates/features/tabgroup/data/group/KtorGroupServiceTest.ktfeatures/tabgroup/data/src/desktopTest/kotlin/de/tabmates/features/tabgroup/data/group/FakeGroupService.ktfeatures/tabgroup/data/src/desktopTest/kotlin/de/tabmates/features/tabgroup/data/tabentry/TabEntryRealtimeSyncTest.ktfeatures/tabgroup/domain/src/commonMain/kotlin/de/tabmates/features/tabgroup/domain/activity/ActivityEventType.ktfeatures/tabgroup/domain/src/commonMain/kotlin/de/tabmates/features/tabgroup/domain/group/GroupRemovalNotifier.ktfeatures/tabgroup/domain/src/commonMain/kotlin/de/tabmates/features/tabgroup/domain/group/GroupRepository.ktfeatures/tabgroup/domain/src/commonMain/kotlin/de/tabmates/features/tabgroup/domain/group/GroupService.ktfeatures/tabgroup/domain/src/commonMain/kotlin/de/tabmates/features/tabgroup/domain/models/ReferencedParticipants.ktfeatures/tabgroup/presentation/src/commonMain/composeResources/values-de/string.xmlfeatures/tabgroup/presentation/src/commonMain/composeResources/values/string.xmlfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/MainGraph.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/MainNavKeys.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/AddEntryScreen.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/AddEntryState.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/AddEntryViewModel.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/SplitEditorScreen.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/editsettlement/EditSettlementViewModel.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/entrydetail/EntryDetailViewModel.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupdetail/GroupDetailRoot.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupdetail/GroupDetailViewModel.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupoverview/GroupDetailPane.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/grouppeople/GroupPeopleScreen.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/grouppeople/GroupPeopleState.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/grouppeople/GroupPeopleViewModel.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/settlementdetail/SettlementDetailViewModel.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/settleup/SettleUpScreen.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/settleup/SettleUpState.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/settleup/SettleUpViewModel.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/util/GroupWithParticipants.ktfeatures/tabgroup/presentation/src/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/AddEntryViewModelTest.ktfeatures/tabgroup/presentation/src/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/navigation/creategroup/FakeGroupRepository.ktfeatures/tabgroup/presentation/src/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/navigation/entrydetail/EntryDetailViewModelTest.ktfeatures/tabgroup/presentation/src/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupdetail/GroupDetailViewModelTest.ktfeatures/tabgroup/presentation/src/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/navigation/grouppeople/GroupPeopleViewModelTest.ktfeatures/tabgroup/presentation/src/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/navigation/settleup/SettleUpViewModelTest.kt
There was a problem hiding this comment.
🧹 Nitpick comments (1)
features/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/dto/RemoveParticipantErrorDto.kt (1)
28-28: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMake the intentionally ignored exception explicit.
Because this fallback accepts malformed or non-JSON error bodies, keep the broad catch and rename
eto_. Keep theCancellationExceptionrethrow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@features/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/dto/RemoveParticipantErrorDto.kt` at line 28, In the exception fallback around RemoveParticipantErrorDto parsing, rename the intentionally ignored catch parameter from e to _ while preserving the broad Exception catch and the existing CancellationException rethrow.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@features/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/dto/RemoveParticipantErrorDto.kt`:
- Line 28: In the exception fallback around RemoveParticipantErrorDto parsing,
rename the intentionally ignored catch parameter from e to _ while preserving
the broad Exception catch and the existing CancellationException rethrow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9b9e3ea9-108e-46c2-926c-ca01d73acb94
📒 Files selected for processing (4)
features/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/dto/RemoveParticipantErrorDto.ktfeatures/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/tabentry/TabEntryRealtimeSync.ktfeatures/tabgroup/domain/src/commonMain/kotlin/de/tabmates/features/tabgroup/domain/group/GroupRemovalNotifier.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/grouppeople/GroupPeopleScreen.kt
🚧 Files skipped from review as they are similar to previous changes (3)
- features/tabgroup/domain/src/commonMain/kotlin/de/tabmates/features/tabgroup/domain/group/GroupRemovalNotifier.kt
- features/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/tabentry/TabEntryRealtimeSync.kt
- features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/grouppeople/GroupPeopleScreen.kt
DELETE /api/group/{groupId}/participants/{userId} answers with an empty
body, so success refetches the group to sync the participant cross-refs.
The server states two refusals by code, each sharing a status with a
different meaning: 400 CANNOT_REMOVE_SELF and 403
CANNOT_REMOVE_GROUP_CREATOR. Reading them needs mapKnownError on delete,
which only post had.
A trailing X on every row but your own and the creator's, behind a confirm dialog. Membership is the only permission, so placeholders and members read the same. The dialog names an unsettled balance without blocking on it: removal drops membership only, and the money stays exactly where it was.
The server unicasts REMOVED_FROM_GROUP to whoever it just removed, with the group id alone — so the title for the snackbar is read from the local mirror before the group is deleted. Popping that group's screens now goes through a GroupScoped marker rather than a hand-listed predicate: leaving can only happen from the group settings, being removed can land on any of the nine.
GroupDetailRoot rendered the same placeholder while loading and when the group is not there locally, so a push deep link for a group you were removed from — or one that was deleted — looked like an endless spinner.
DennisBauer
force-pushed
the
feat/remove-group-participant
branch
from
August 7, 2026 14:04
e2afdc8 to
48348c2
Compare
DennisBauer
enabled auto-merge
August 7, 2026 14:09
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit