feat: Add option to add recurring entries (expenses/incomes/payments) - #342
Open
DennisBauer wants to merge 7 commits into
Open
feat: Add option to add recurring entries (expenses/incomes/payments)#342DennisBauer wants to merge 7 commits into
DennisBauer wants to merge 7 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis change adds recurring-series models, persistence, synchronization, scheduled-entry projection, REST operations, WebSocket handling, Compose editing and detail screens, settlement entries, localized errors, and related tests. ChangesRecurring entries and schedules
Estimated code review effort: 5 (Critical) | ~120 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: 11
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)
240-245: 🎯 Functional Correctness | 🔴 Critical | 🏗️ Heavy liftA series edit loses the template splits.
existingSplitsis derived only fromexisting, which is null when the screen edits a series.splitsByParticipantis therefore empty, andsplitInputssetsincluded = if (isEditing) split != null else true(Line 323), which evaluates tofalsefor every participant becauseisEditingis true. The split editor opens with nobody selected, and saving an expense or income series fails withadd_entry_error_no_splits. The loaded amounts and split type fromseries.rule.splitsare also discarded.Map
series?.rule?.splitsinto the same shape as the entry splits, and select rows from the series when the screen edits a series.Also applies to: 285-295
🤖 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 240 - 245, Update the split-loading logic around existingSplits and splitsByParticipant to use series?.rule?.splits when editing a series, mapping those rule splits into the same participant-keyed shape as entry splits and preserving their amounts and split type. Ensure splitInputs selects participants based on the mapped series splits while retaining the existing entry behavior.
🧹 Nitpick comments (18)
features/tabgroup/domain/src/commonMain/kotlin/de/tabmates/features/tabgroup/domain/recurring/ScheduledLedger.kt (1)
21-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCorrect the deleted-entry contract.
This documentation says that soft-deleted entries remain in the result.
ScheduledEntryProjector.ktat Lines 28-31 andRecurringSeriesRepository.ktat Lines 24-28 state that persistence removes these entries and retains only their claimed slots.Document the persistence behavior and the claimed-slot requirement. Do not require implementations to expose rows that persistence removes.
Proposed documentation update
- * Every entry of [groupId], soft-deleted ones included, followed by a placeholder for each - * occurrence that is due and unwritten. + * Every persisted entry of [groupId], followed by a placeholder for each occurrence that is + * due and unwritten. * - * Deleted entries are kept in the result because callers filter them themselves, and because - * dropping them here would hide the slots they occupy from the projection. + * Soft-deleted recurring entries can be absent from persistence. Implementations must retain + * their claimed slots so projection does not recreate those occurrences.🤖 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/domain/src/commonMain/kotlin/de/tabmates/features/tabgroup/domain/recurring/ScheduledLedger.kt` around lines 21 - 25, Update the documentation for ScheduledLedger to state that persistence removes soft-deleted entries from the result while retaining their claimed slots, and that implementations must preserve those claimed slots when projecting due unwritten occurrences. Remove the requirement that deleted entry rows remain exposed to callers.features/tabgroup/domain/src/commonMain/kotlin/de/tabmates/features/tabgroup/domain/models/TabEntry.kt (1)
37-56: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftMake recurring provenance atomic.
The constructors accept
recurringSeriesId,recurringOccurrenceDate, andisScheduledPlaceholderindependently. They permit a recurring entry with an incomplete slot identity or a placeholder without a slot.
ScheduledEntryProjector.projectat Lines 51-53 ignores an existing entry when either identity property is null. The projector can then add a duplicate placeholder and double-count the occurrence.Represent these properties with one nullable provenance value, or enforce the invariant in every subtype.
Also applies to: 80-82, 104-106, 128-130
🤖 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/domain/src/commonMain/kotlin/de/tabmates/features/tabgroup/domain/models/TabEntry.kt` around lines 37 - 56, The recurring provenance fields in TabEntry and every affected subtype constructor are independently nullable, allowing incomplete identities and invalid placeholders. Make recurringSeriesId and recurringOccurrenceDate a single nullable provenance value, or enforce that they are both set together and require isScheduledPlaceholder to have valid provenance; update ScheduledEntryProjector.project to match entries using the atomic provenance so it cannot create duplicate placeholders.features/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/mappers/RecurringSeriesMappers.kt (2)
153-174: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReturn a typed failure instead of throwing for a settlement template without a receiver.
requireNotNullthrowsIllegalArgumentExceptionfrom the mapping layer. The coding guidelines require expected operation errors to be represented withResult.FailureorEmptyResult. Validate the receiver where the template is built, or maketoDto()return aResultso the caller can map the error toUiText.As per coding guidelines: "Always represent expected operation errors with
Result.FailureorEmptyResult".🤖 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/mappers/RecurringSeriesMappers.kt` around lines 153 - 174, Replace the requireNotNull call in the settlement branch of the recurring template mapper with typed failure handling using Result.Failure or EmptyResult. Propagate the failure through the toDto() caller, or validate receivedByUserId before building the DTO, so the missing receiver can be mapped to UiText without throwing IllegalArgumentException.Source: Coding guidelines
84-91: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused
unresolvedParticipantIds()helper.The repository has no call sites.
RecurringSeriesLocalWriter.ensureParticipantsExistaccepts domain models and recomputes the participant IDs independently.🤖 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/mappers/RecurringSeriesMappers.kt` around lines 84 - 91, Remove the unused RecurringSeriesDto.unresolvedParticipantIds() helper, including its associated documentation, while leaving referencedParticipants() and RecurringSeriesLocalWriter.ensureParticipantsExist unchanged.features/tabgroup/data/src/desktopTest/kotlin/de/tabmates/features/tabgroup/data/sync/OfflineFirstSyncRepositoryTest.kt (1)
37-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the recorded series refreshes.
The fake records
refreshedGroupIds, but no test reads it. The new behavior inOfflineFirstSyncRepository.backfillNewAndPendingGroupsrefreshes recurring series for every newly known and pending group. Pass the fake explicitly indeltaSyncBackfillsEntriesForNewlyKnownGroupand assert the recorded ids, so a regression in that loop fails a test.💚 Proposed assertion in `deltaSyncBackfillsEntriesForNewlyKnownGroup`
val pendingStore = FakePendingTabEntryBackfillStore() + val recurringSeriesRepository = FakeRecurringSeriesRepository() - repository(service, cursorStore, tabEntryService, pendingStore).sync() + repository( + service, + cursorStore, + tabEntryService, + pendingStore, + recurringSeriesRepository = recurringSeriesRepository, + ).sync() assertEquals(listOf("g2"), tabEntryService.receivedGroupIds) + assertEquals(listOf("g2"), recurringSeriesRepository.refreshedGroupIds) assertEquals(setOf("e9"), localEntryIds()) assertTrue(pendingStore.getAll().isEmpty())Consider also covering full-sync series pruning and slot-claim recording once
snapshot(...)accepts recurring fixtures.Also applies to: 47-48
🤖 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/sync/OfflineFirstSyncRepositoryTest.kt` at line 37, Update deltaSyncBackfillsEntriesForNewlyKnownGroup to create and pass an explicit FakeRecurringSeriesRepository, then assert its refreshedGroupIds contains the newly known group and any pending group expected by the backfill. Use the fake’s recorded values to verify backfillNewAndPendingGroups refreshes every relevant group.features/tabgroup/data/src/desktopTest/kotlin/de/tabmates/features/tabgroup/data/tabentry/TabEntryRealtimeSyncTest.kt (1)
13-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the new realtime branches.
The wiring is correct: the real
RecurringSeriesLocalWriterruns against the in-memory database. Two new behaviors inTabEntryRealtimeSyncstay untested here.
RECURRING_SERIES_CHANGEDdecodes aRecurringSeriesDtoand persists one series. A field-name or shape mismatch in that payload would only surface at runtime.handleUpsertrecords a recurring slot claim before the soft-delete branch. A test that emits an ack for a soft-deleted generated entry would lock in that the claim row survives.Add one frame test per behavior with a
RecurringSeriesDtofixture.Also applies to: 180-180
🤖 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` at line 13, Add two frame-based tests in TabEntryRealtimeSyncTest: one emitting a RECURRING_SERIES_CHANGED payload using a RecurringSeriesDto fixture and asserting the series is persisted, and another emitting an acknowledged upsert for a soft-deleted generated entry and asserting its recurring slot claim remains. Reuse the existing in-memory database, real RecurringSeriesLocalWriter wiring, and established frame-test helpers.features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/AddEntryScreen.kt (1)
807-823: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLet
paidByDisplaydelegate toparticipantDisplay.
participantDisplayandpaidByDisplay(Lines 825-835) resolve the same three cases against the same map. Keep one implementation so the removed-member and current-user labels cannot diverge.♻️ Proposed refactor
`@Composable` -private fun paidByDisplay(state: AddEntryState): String { - // Resolved through the wider map, not the member list: the payer of an edited entry may have - // been removed from the group since, and their real name is still known. - val paidBy = state.participantsById[state.paidByUserId] - return when { - paidBy == null -> stringResource(Res.string.expense_detail_removed_member) - paidBy.userId == state.currentUserId -> stringResource(Res.string.add_entry_paid_by_you) - else -> paidBy.username - } -} +private fun paidByDisplay(state: AddEntryState): String = + participantDisplay(state, state.paidByUserId)🤖 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/AddEntryScreen.kt` around lines 807 - 823, Update paidByDisplay to delegate to participantDisplay using the same state and user ID inputs, and remove its duplicate participant-resolution logic. Preserve the existing removed-member, current-user, and username results through the shared participantDisplay implementation.features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/MainGraph.kt (1)
292-301: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGuard the migration against a non-top
EditSettlemententry.The migration entry renders no content.
entry<EditSettlement>only composes when the key is the top of the back stack, so a persisted stack that holdsEditSettlementbelow another entry keeps the deprecated key until the user navigates back to it. At that pointremoveAll { it is EditSettlement }plusaddalso drops any entries that sat above it. Consider running the same conversion once at restore time on the whole back stack instead of inside the entry body.🤖 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/MainGraph.kt` around lines 292 - 301, The EditSettlement migration currently runs only when that entry reaches the top and can remove entries above it. Move the conversion out of the entry<EditSettlement> body and into the back-stack restoration flow, converting every persisted EditSettlement entry in place to EditEntry while preserving all other entries and their ordering; retain the deprecated route only as needed for deserialization.features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/RepeatEditorScreen.kt (2)
227-235: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
selectablesemantics for the radio rows.The
Rowis clickable and theRadioButtonalso has its ownonClick. Accessibility services then expose two separate targets for one option, and the row is not announced as a radio button. UseModifier.selectablewithrole = Role.RadioButtonand passonClick = nullto theRadioButton.♻️ Proposed refactor
Row( verticalAlignment = Alignment.CenterVertically, modifier = Modifier .fillMaxWidth() - .clickable(onClick = onClick) + .selectable( + selected = selected, + role = Role.RadioButton, + onClick = onClick, + ) .padding(horizontal = 8.dp), ) { - RadioButton(selected = selected, onClick = onClick) + RadioButton(selected = selected, onClick = null)Add the imports:
import androidx.compose.foundation.selection.selectable import androidx.compose.ui.semantics.Role🤖 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/RepeatEditorScreen.kt` around lines 227 - 235, Update the radio-option Row modifier to use selectable with Role.RadioButton instead of clickable, and set RadioButton’s onClick to null so each option exposes a single accessible radio target. Add the required selectable and Role imports.
192-200: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueCache the occurrence calculation across recompositions.
RecurringOccurrenceCalculator.upcomingOccurrencesruns on every recomposition ofRepeatPreview. The inputs change only when the repeat config changes. Wrap the call inremember.♻️ Proposed refactor
- val dates = - RecurringOccurrenceCalculator.upcomingOccurrences( - rule = repeat.toPreviewRule(), - after = state.repeatStartDate.minus(1, DateTimeUnit.DAY), - limit = PREVIEW_COUNT, - ) + val dates = + remember(repeat) { + RecurringOccurrenceCalculator.upcomingOccurrences( + rule = repeat.toPreviewRule(), + after = repeat.startDate.minus(1, DateTimeUnit.DAY), + limit = PREVIEW_COUNT, + ) + }🤖 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/RepeatEditorScreen.kt` around lines 192 - 200, Cache the upcoming occurrence calculation in RepeatPreview by wrapping the RecurringOccurrenceCalculator.upcomingOccurrences call in remember, keyed by the repeat configuration and state.repeatStartDate so it recomputes only when those inputs change while preserving the existing rule, anchor date, and limit.features/tabgroup/presentation/src/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/testing/FakeRecurringSeriesRepository.kt (2)
39-39: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
getClaimedSlotsForGroupignoresgroupId.The fake returns every claim regardless of group.
FakeScheduledLedgerfeeds this set intoScheduledEntryProjector.project, so in a test with two groups the claims of one group suppress occurrences in the other. Filter the claims through the series that belong to the requested group.♻️ Proposed refactor
- override fun getClaimedSlotsForGroup(groupId: String): Flow<Set<RecurringSlot>> = claimedSlots + override fun getClaimedSlotsForGroup(groupId: String): Flow<Set<RecurringSlot>> = + combine(series, claimedSlots) { all, claims -> + val seriesIds = all.filter { it.groupId == groupId }.mapTo(mutableSetOf()) { it.seriesId } + claims.filterTo(mutableSetOf()) { it.seriesId in seriesIds } + }🤖 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/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/testing/FakeRecurringSeriesRepository.kt` at line 39, Update FakeRecurringSeriesRepository.getClaimedSlotsForGroup to filter claimedSlots by the recurring series belonging to the requested groupId before returning them, rather than returning all claims. Preserve the Flow<Set<RecurringSlot>> contract and ensure claims from other groups cannot reach FakeScheduledLedger or ScheduledEntryProjector.project.
22-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename the result and centralize the feature fake.
- Rename
createResulttowriteResult;updateSeriesalso returns it.- Add
features/tabgroup/testingfor the shared fake. Preserve the data fake’s fail-fast write behavior and the presentation fake’s configurable state and writes when consolidating them.🤖 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/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/testing/FakeRecurringSeriesRepository.kt` around lines 22 - 23, Rename createResult to writeResult in FakeRecurringSeriesRepository and update both createSeries and updateSeries to use it. Consolidate the fake into the shared features/tabgroup/testing location, preserving the data fake’s fail-fast write behavior while retaining the presentation fake’s configurable state and write handling.Source: Coding guidelines
features/tabgroup/presentation/src/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/testing/FakeScheduledLedger.kt (1)
22-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExpose the backing fakes.
Both repositories are
private val. A test that uses the defaultrecurringSeriesRepositorycannot add series or claims to it, so it must construct and hold its own instance. Change both tovalso callers can drive them through the ledger.♻️ Proposed refactor
class FakeScheduledLedger( - private val tabEntryRepository: FakeTabEntryRepository = FakeTabEntryRepository(), - private val recurringSeriesRepository: FakeRecurringSeriesRepository = FakeRecurringSeriesRepository(), + val tabEntryRepository: FakeTabEntryRepository = FakeTabEntryRepository(), + val recurringSeriesRepository: FakeRecurringSeriesRepository = FakeRecurringSeriesRepository(),🤖 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/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/testing/FakeScheduledLedger.kt` around lines 22 - 30, Expose the backing repositories in FakeScheduledLedger by changing tabEntryRepository and recurringSeriesRepository from private val to public val, while preserving their default instances and existing behavior.features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupoverview/GroupDetailPane.kt (2)
1329-1360: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffOptional: render the recurring list lazily.
RecurringTabbuilds every row in aColumn. A group with many schedules composes all rows at once. The file already importsLazyColumnandrememberLazyListState. ConsiderLazyColumnwithstickyHeaderoritemsection labels.🤖 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/groupoverview/GroupDetailPane.kt` around lines 1329 - 1360, Optionally replace the eager Column rendering in RecurringTab with a LazyColumn using the existing LazyColumn and rememberLazyListState imports. Render the active and ended section labels as list items or sticky headers, and each RecurringRow as a lazy item while preserving ordering, spacing, callbacks, and the existing bottom clearance.
1373-1384: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
RecurringOccurrenceCalculator.upcomingOccurrencesis called during composition withoutremember. Both call sites re-run the date walk on every recomposition although the inputs change rarely.
features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupoverview/GroupDetailPane.kt#L1373-L1384: wrap the next-occurrence calculation inremember(series, today); this runs once per rendered schedule row.features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/RepeatEditorScreen.kt#L192-L200: wrap the preview calculation inremember(repeat).🤖 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/groupoverview/GroupDetailPane.kt` around lines 1373 - 1384, The next-occurrence calculation in GroupDetailPane.kt lines 1373-1384 should be wrapped with remember(series, today), preserving the existing active/attention checks and calculation. The preview calculation in RepeatEditorScreen.kt lines 192-200 should likewise be wrapped with remember(repeat); both sites must retain their current results while avoiding repeated date walks during recomposition.features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupoverview/GroupOverviewViewModel.kt (1)
60-77: 🚀 Performance & Scalability | 🔵 TrivialWatch the observer count on the overview screen.
ScheduledLedger.observeEntriesForGroupcombines three database flows per group (DefaultScheduledLedger.kt:22-24). This screen calls it once per group, so the number of active Room observers is now three times the group count, and each emission re-runsScheduledEntryProjector.project. Confirm this stays acceptable for accounts with many groups. If not, add a ledger API that observes all groups at once.🤖 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/groupoverview/GroupOverviewViewModel.kt` around lines 60 - 77, Review enrichWithStats and ScheduledLedger.observeEntriesForGroup for overview screens with many groups; avoid creating three database observers and rerunning ScheduledEntryProjector.project once per group. If the current observer count is not acceptable, add and use a ledger API that observes entries for all requested groups in one combined flow while preserving the existing per-item enrichment behavior.features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupdetail/GroupDetailViewModel.kt (1)
109-117: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueOptional: the group's series are observed twice.
recurringSeriesRepository.getSeriesForGroup(groupId)is collected here, andDefaultScheduledLedgercollects the same flow for the projection (features/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/recurring/DefaultScheduledLedger.kt:22-24). Two Room observers run for one screen. If the query becomes expensive, expose the used series from the ledger instead so one reader serves both.🤖 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/groupdetail/GroupDetailViewModel.kt` around lines 109 - 117, Optionally eliminate the duplicate series observation by exposing the series collected by DefaultScheduledLedger and reusing that stream in GroupDetailViewModel’s recurringSeries/sideInputs flow. Preserve the existing empty-list startup behavior and ensure both scheduledLedger projections and sideInputs consume the same series source.features/tabgroup/presentation/src/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/AddEntryRecurringTest.kt (1)
216-219: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse Turbine to collect ViewModel state.
These tests manually manage
StateFlowsubscriptions. Use Turbine so each test owns collection lifetime and assertions.
features/tabgroup/presentation/src/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/AddEntryRecurringTest.kt#L216-L219: Replace the background collector helper with Turbine collection in each state-dependent test.features/tabgroup/presentation/src/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupdetail/GroupDetailScheduledEntriesTest.kt#L65-L65: Replace activation throughfirst()with a Turbine collection that awaits the loaded state.🤖 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/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/AddEntryRecurringTest.kt` around lines 216 - 219, Replace the manual background collector in AddEntryRecurringTest.kt (lines 216-219) with Turbine collection scoped within each state-dependent test, removing the activate helper while preserving idle/initial-state handling and assertions. In GroupDetailScheduledEntriesTest.kt (line 65), replace first()-based activation with Turbine and await the loaded state before continuing; update both sites to own collection lifetimes per test.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/mappers/RecurringSeriesMappers.kt`:
- Around line 311-328: Update the end mapping in the recurring-series mapper so
incomplete UNTIL or COUNT rows never become RecurringEnd.Never. When the
required endUntilDate or endCount is missing, use the existing stop-generation
fallback if available; otherwise mark the row unreadable and log it, while
preserving normal NEVER, UNTIL, and COUNT mappings.
In
`@features/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/recurring/DefaultScheduledLedger.kt`:
- Around line 20-41: Update DefaultScheduledLedger.observeEntriesForGroup so
projection also refreshes at each UTC date boundary, not only when repository
flows emit. Add a date-boundary flow or ticker to the combined inputs, derive
today from an injected Clock rather than directly from Clock.System.now(), and
preserve the existing UTC-date behavior while enabling deterministic testing.
In
`@features/tabgroup/domain/src/commonMain/kotlin/de/tabmates/features/tabgroup/domain/recurring/RecurringOccurrenceCalculator.kt`:
- Line 24: The MAX_SLOT_SCAN limit in walkSlots causes valid recurring series to
be silently truncated, affecting dueOccurrences, upcomingOccurrences, and
isOccurrenceDate. Replace the truncation with an initial-slot or
resumable-position calculation, or enforce a domain limit that makes the limit
unreachable, while preserving complete results for valid series; add a
regression test covering a series beyond MAX_SLOT_SCAN.
In
`@features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/AddEntryViewModel.kt`:
- Around line 775-779: Update todayLocal() to convert the current instant using
TimeZone.UTC instead of TimeZone.currentSystemDefault(), keeping the returned
LocalDate behavior unchanged so form date calculations align with the
scheduler’s UTC day.
- Around line 89-96: The series-edit flow incorrectly reuses entry loading and
split selection. In AddEntryViewModel.kt lines 89-96, add an isEditingEntry flag
based only on entryId and gate tabEntryRepository.getTabEntryById with it; in
lines 240-295, derive existingSplits and splitsByParticipant from
series?.rule?.splits when editing a series, then use those values to select the
corresponding rows in splitInputs.
- Around line 431-433: Update onRepeatEndChange so a RecurringEnd.Until date
earlier than the current repeatStartDate is clamped to repeatStartDate before
updating state; preserve non-date end variants and continue hiding the picker.
In
`@features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/RepeatEditorScreen.kt`:
- Around line 299-313: Update StepperRow so both IconButton modifiers provide
localized accessibility content descriptions through semantics, using Res.string
resources that describe decrement and increment actions. Add a maxValue
parameter, disable the increment button when value reaches that limit, and
coerce incremented values to maxValue; update repeatInterval and
RecurringEnd.Count call sites with appropriate bounds.
In
`@features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupdetail/GroupDetailViewModel.kt`:
- Around line 84-86: Remove the unused tabEntryRepository property and import
from GroupDetailViewModel, then remove its constructor argument from
GroupDetailViewModelTest and GroupDetailScheduledEntriesTest while retaining the
repository argument passed to FakeScheduledLedger.
In
`@features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupoverview/GroupDetailPane.kt`:
- Around line 504-554: In the Box around the entry rows, move ScheduledRowChip
after the when(entry) block so it is drawn above ExpenseRow, SettlementRow, and
IncomeRow. Update the row click handling so scheduled entries receive no click
handler (or otherwise omit Modifier.clickable), rather than only guarding
callbacks; preserve normal click behavior for non-scheduled rows.
- Around line 267-287: In the GroupDetailPane tab-rendering block, compute
recurring-series attention once before DetailTab.entries.forEach and reuse that
value when determining needsAttention. Add localized accessibility semantics
with a contentDescription to the attention-dot Box, describing that a schedule
needs attention, while preserving the existing visual indicator.
In
`@features/tabgroup/presentation/src/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupdetail/GroupDetailScheduledEntriesTest.kt`:
- Line 146: Update the LocalDate.plusOneYear extension in
GroupDetailScheduledEntriesTest to use calendar-aware year addition that clamps
invalid dates to the target month’s last valid day, preserving February 29 to
February 28 when the following year is not a leap year.
---
Outside diff comments:
In
`@features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/AddEntryViewModel.kt`:
- Around line 240-245: Update the split-loading logic around existingSplits and
splitsByParticipant to use series?.rule?.splits when editing a series, mapping
those rule splits into the same participant-keyed shape as entry splits and
preserving their amounts and split type. Ensure splitInputs selects participants
based on the mapped series splits while retaining the existing entry behavior.
---
Nitpick comments:
In
`@features/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/mappers/RecurringSeriesMappers.kt`:
- Around line 153-174: Replace the requireNotNull call in the settlement branch
of the recurring template mapper with typed failure handling using
Result.Failure or EmptyResult. Propagate the failure through the toDto() caller,
or validate receivedByUserId before building the DTO, so the missing receiver
can be mapped to UiText without throwing IllegalArgumentException.
- Around line 84-91: Remove the unused
RecurringSeriesDto.unresolvedParticipantIds() helper, including its associated
documentation, while leaving referencedParticipants() and
RecurringSeriesLocalWriter.ensureParticipantsExist unchanged.
In
`@features/tabgroup/data/src/desktopTest/kotlin/de/tabmates/features/tabgroup/data/sync/OfflineFirstSyncRepositoryTest.kt`:
- Line 37: Update deltaSyncBackfillsEntriesForNewlyKnownGroup to create and pass
an explicit FakeRecurringSeriesRepository, then assert its refreshedGroupIds
contains the newly known group and any pending group expected by the backfill.
Use the fake’s recorded values to verify backfillNewAndPendingGroups refreshes
every relevant group.
In
`@features/tabgroup/data/src/desktopTest/kotlin/de/tabmates/features/tabgroup/data/tabentry/TabEntryRealtimeSyncTest.kt`:
- Line 13: Add two frame-based tests in TabEntryRealtimeSyncTest: one emitting a
RECURRING_SERIES_CHANGED payload using a RecurringSeriesDto fixture and
asserting the series is persisted, and another emitting an acknowledged upsert
for a soft-deleted generated entry and asserting its recurring slot claim
remains. Reuse the existing in-memory database, real RecurringSeriesLocalWriter
wiring, and established frame-test helpers.
In
`@features/tabgroup/domain/src/commonMain/kotlin/de/tabmates/features/tabgroup/domain/models/TabEntry.kt`:
- Around line 37-56: The recurring provenance fields in TabEntry and every
affected subtype constructor are independently nullable, allowing incomplete
identities and invalid placeholders. Make recurringSeriesId and
recurringOccurrenceDate a single nullable provenance value, or enforce that they
are both set together and require isScheduledPlaceholder to have valid
provenance; update ScheduledEntryProjector.project to match entries using the
atomic provenance so it cannot create duplicate placeholders.
In
`@features/tabgroup/domain/src/commonMain/kotlin/de/tabmates/features/tabgroup/domain/recurring/ScheduledLedger.kt`:
- Around line 21-25: Update the documentation for ScheduledLedger to state that
persistence removes soft-deleted entries from the result while retaining their
claimed slots, and that implementations must preserve those claimed slots when
projecting due unwritten occurrences. Remove the requirement that deleted entry
rows remain exposed to callers.
In
`@features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/AddEntryScreen.kt`:
- Around line 807-823: Update paidByDisplay to delegate to participantDisplay
using the same state and user ID inputs, and remove its duplicate
participant-resolution logic. Preserve the existing removed-member,
current-user, and username results through the shared participantDisplay
implementation.
In
`@features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/RepeatEditorScreen.kt`:
- Around line 227-235: Update the radio-option Row modifier to use selectable
with Role.RadioButton instead of clickable, and set RadioButton’s onClick to
null so each option exposes a single accessible radio target. Add the required
selectable and Role imports.
- Around line 192-200: Cache the upcoming occurrence calculation in
RepeatPreview by wrapping the RecurringOccurrenceCalculator.upcomingOccurrences
call in remember, keyed by the repeat configuration and state.repeatStartDate so
it recomputes only when those inputs change while preserving the existing rule,
anchor date, and limit.
In
`@features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupdetail/GroupDetailViewModel.kt`:
- Around line 109-117: Optionally eliminate the duplicate series observation by
exposing the series collected by DefaultScheduledLedger and reusing that stream
in GroupDetailViewModel’s recurringSeries/sideInputs flow. Preserve the existing
empty-list startup behavior and ensure both scheduledLedger projections and
sideInputs consume the same series source.
In
`@features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupoverview/GroupDetailPane.kt`:
- Around line 1329-1360: Optionally replace the eager Column rendering in
RecurringTab with a LazyColumn using the existing LazyColumn and
rememberLazyListState imports. Render the active and ended section labels as
list items or sticky headers, and each RecurringRow as a lazy item while
preserving ordering, spacing, callbacks, and the existing bottom clearance.
- Around line 1373-1384: The next-occurrence calculation in GroupDetailPane.kt
lines 1373-1384 should be wrapped with remember(series, today), preserving the
existing active/attention checks and calculation. The preview calculation in
RepeatEditorScreen.kt lines 192-200 should likewise be wrapped with
remember(repeat); both sites must retain their current results while avoiding
repeated date walks during recomposition.
In
`@features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupoverview/GroupOverviewViewModel.kt`:
- Around line 60-77: Review enrichWithStats and
ScheduledLedger.observeEntriesForGroup for overview screens with many groups;
avoid creating three database observers and rerunning
ScheduledEntryProjector.project once per group. If the current observer count is
not acceptable, add and use a ledger API that observes entries for all requested
groups in one combined flow while preserving the existing per-item enrichment
behavior.
In
`@features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/MainGraph.kt`:
- Around line 292-301: The EditSettlement migration currently runs only when
that entry reaches the top and can remove entries above it. Move the conversion
out of the entry<EditSettlement> body and into the back-stack restoration flow,
converting every persisted EditSettlement entry in place to EditEntry while
preserving all other entries and their ordering; retain the deprecated route
only as needed for deserialization.
In
`@features/tabgroup/presentation/src/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/AddEntryRecurringTest.kt`:
- Around line 216-219: Replace the manual background collector in
AddEntryRecurringTest.kt (lines 216-219) with Turbine collection scoped within
each state-dependent test, removing the activate helper while preserving
idle/initial-state handling and assertions. In
GroupDetailScheduledEntriesTest.kt (line 65), replace first()-based activation
with Turbine and await the loaded state before continuing; update both sites to
own collection lifetimes per test.
In
`@features/tabgroup/presentation/src/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/testing/FakeRecurringSeriesRepository.kt`:
- Line 39: Update FakeRecurringSeriesRepository.getClaimedSlotsForGroup to
filter claimedSlots by the recurring series belonging to the requested groupId
before returning them, rather than returning all claims. Preserve the
Flow<Set<RecurringSlot>> contract and ensure claims from other groups cannot
reach FakeScheduledLedger or ScheduledEntryProjector.project.
- Around line 22-23: Rename createResult to writeResult in
FakeRecurringSeriesRepository and update both createSeries and updateSeries to
use it. Consolidate the fake into the shared features/tabgroup/testing location,
preserving the data fake’s fail-fast write behavior while retaining the
presentation fake’s configurable state and write handling.
In
`@features/tabgroup/presentation/src/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/testing/FakeScheduledLedger.kt`:
- Around line 22-30: Expose the backing repositories in FakeScheduledLedger by
changing tabEntryRepository and recurringSeriesRepository from private val to
public val, while preserving their default instances and existing behavior.
🪄 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: 6e349e12-f58f-4a70-a871-aaddbf252d63
📒 Files selected for processing (84)
core/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/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/dto/RecurringSeriesDto.ktfeatures/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/dto/RecurringSeriesRequestDtos.ktfeatures/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/dto/SyncResponseDto.ktfeatures/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/dto/TabEntryDto.ktfeatures/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/mappers/RecurringSeriesMappers.ktfeatures/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/mappers/SyncMappers.ktfeatures/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/mappers/TabEntryMappers.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/recurring/DefaultScheduledLedger.ktfeatures/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/recurring/KtorRecurringSeriesService.ktfeatures/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/recurring/OfflineFirstRecurringSeriesRepository.ktfeatures/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/recurring/RecurringSeriesService.ktfeatures/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/sync/OfflineFirstSyncRepository.ktfeatures/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/sync/RecurringSeriesLocalWriter.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/desktopTest/kotlin/de/tabmates/features/tabgroup/data/sync/FakeRecurringSeriesRepository.ktfeatures/tabgroup/data/src/desktopTest/kotlin/de/tabmates/features/tabgroup/data/sync/OfflineFirstSyncRepositoryTest.ktfeatures/tabgroup/data/src/desktopTest/kotlin/de/tabmates/features/tabgroup/data/tabentry/TabEntryRealtimeSyncTest.ktfeatures/tabgroup/database/schemas/de.tabmates.features.tabgroup.database.TabMatesDatabase/8.jsonfeatures/tabgroup/database/src/commonMain/kotlin/de/tabmates/features/tabgroup/database/TabMatesDatabase.ktfeatures/tabgroup/database/src/commonMain/kotlin/de/tabmates/features/tabgroup/database/dao/RecurringSeriesDao.ktfeatures/tabgroup/database/src/commonMain/kotlin/de/tabmates/features/tabgroup/database/dao/RecurringSlotClaimDao.ktfeatures/tabgroup/database/src/commonMain/kotlin/de/tabmates/features/tabgroup/database/entities/RecurringExceptionEntity.ktfeatures/tabgroup/database/src/commonMain/kotlin/de/tabmates/features/tabgroup/database/entities/RecurringSeriesEntity.ktfeatures/tabgroup/database/src/commonMain/kotlin/de/tabmates/features/tabgroup/database/entities/RecurringSeriesWithDetails.ktfeatures/tabgroup/database/src/commonMain/kotlin/de/tabmates/features/tabgroup/database/entities/RecurringSlotClaimEntity.ktfeatures/tabgroup/database/src/commonMain/kotlin/de/tabmates/features/tabgroup/database/entities/RecurringTemplateSplitEntity.ktfeatures/tabgroup/database/src/commonMain/kotlin/de/tabmates/features/tabgroup/database/entities/TabEntryEntity.ktfeatures/tabgroup/database/src/commonMain/kotlin/de/tabmates/features/tabgroup/database/entities/types/RecurrenceFrequencyDatabase.ktfeatures/tabgroup/database/src/commonMain/kotlin/de/tabmates/features/tabgroup/database/entities/types/RecurringEndTypeDatabase.ktfeatures/tabgroup/domain/src/commonMain/kotlin/de/tabmates/features/tabgroup/domain/models/SyncSnapshot.ktfeatures/tabgroup/domain/src/commonMain/kotlin/de/tabmates/features/tabgroup/domain/models/TabEntry.ktfeatures/tabgroup/domain/src/commonMain/kotlin/de/tabmates/features/tabgroup/domain/recurring/RecurrenceFrequency.ktfeatures/tabgroup/domain/src/commonMain/kotlin/de/tabmates/features/tabgroup/domain/recurring/RecurringEnd.ktfeatures/tabgroup/domain/src/commonMain/kotlin/de/tabmates/features/tabgroup/domain/recurring/RecurringOccurrenceCalculator.ktfeatures/tabgroup/domain/src/commonMain/kotlin/de/tabmates/features/tabgroup/domain/recurring/RecurringSeries.ktfeatures/tabgroup/domain/src/commonMain/kotlin/de/tabmates/features/tabgroup/domain/recurring/RecurringSeriesRepository.ktfeatures/tabgroup/domain/src/commonMain/kotlin/de/tabmates/features/tabgroup/domain/recurring/ScheduledEntryProjector.ktfeatures/tabgroup/domain/src/commonMain/kotlin/de/tabmates/features/tabgroup/domain/recurring/ScheduledLedger.ktfeatures/tabgroup/domain/src/commonTest/kotlin/de/tabmates/features/tabgroup/domain/recurring/RecurringOccurrenceCalculatorTest.ktfeatures/tabgroup/domain/src/commonTest/kotlin/de/tabmates/features/tabgroup/domain/recurring/ScheduledEntryProjectorTest.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/EntryKind.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/RepeatConfig.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/RepeatEditorScreen.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/editsettlement/EditSettlementEvent.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/editsettlement/EditSettlementRoot.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/editsettlement/EditSettlementState.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/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/groupoverview/GroupOverviewViewModel.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/home/HomeViewModel.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/recurringdetail/RecurringSeriesDetailEvent.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/recurringdetail/RecurringSeriesDetailRoot.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/recurringdetail/RecurringSeriesDetailState.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/recurringdetail/RecurringSeriesDetailViewModel.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/recurringdetail/ScheduleSummary.ktfeatures/tabgroup/presentation/src/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/AddEntryRecurringTest.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/editsettlement/EditSettlementViewModelTest.ktfeatures/tabgroup/presentation/src/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupdetail/GroupDetailScheduledEntriesTest.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/groupoverview/GroupOverviewViewModelTest.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/home/HomeViewModelTest.ktfeatures/tabgroup/presentation/src/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/testing/FakeConnectionStatusRepository.ktfeatures/tabgroup/presentation/src/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/testing/FakeRecurringSeriesRepository.ktfeatures/tabgroup/presentation/src/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/testing/FakeScheduledLedger.ktfeatures/tabgroup/presentation/src/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/testing/RecurringFixtures.kt
💤 Files with no reviewable changes (5)
- features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/editsettlement/EditSettlementEvent.kt
- features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/editsettlement/EditSettlementRoot.kt
- features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/editsettlement/EditSettlementViewModel.kt
- features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/editsettlement/EditSettlementState.kt
- features/tabgroup/presentation/src/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/navigation/editsettlement/EditSettlementViewModelTest.kt
DennisBauer
force-pushed
the
feat/recurring-entries
branch
from
August 11, 2026 14:21
09a3254 to
f5fef85
Compare
RecurringOccurrenceCalculator is a port of the server's generator and has to stay one: the server decides which occurrences get written, this decides which ones the client previews. Its test suite is ported case-for-case, including the Jan-31 clamp-and-recover regression, because a divergence books money on the wrong day with no error anywhere. ScheduledEntryProjector builds placeholders as ordinary TabEntry values carrying isScheduledPlaceholder, so every existing balance calculator consumes them unchanged.
Four new tables plus two nullable columns on tab entries, all additive, so Room derives the 7->8 migration. The rule is flattened into the series row: the server only ever ships the newest revision, so a separate table buys nothing. recurring_slot_claim carries no foreign key on purpose. A generated entry can reach this device before the schedule that produced it does, and an FK would reject exactly that claim. It is also never deleted with its entry: the server keeps a slot claimed whatever happens to the row in it, so without the record a deliberately deleted occurrence reappears as a placeholder on every projection.
Schedules are managed over REST with no outbox behind them. A schedule is a standing instruction to write into other people's ledgers; one queued offline would fire days later against a group that has moved on. Every write needs a connection and reports its own failure. The delta sync filters series by the cursor, so a just-joined group arrives without the schedules it already had -- same gap the tab-entry backfiller already covers, extended to series. INVALID_RECURRING_RULE and RECURRING_ENTRIES_DISABLED are mapped by code: the 503 means the feature is off, not that the server is struggling, and the same request can succeed later untouched.
An occurrence that is due but unwritten still moves the ledger, so it has to move the numbers too -- otherwise every balance jumps when the server's sweep lands, and offline the group looks settled when it is not. ScheduledLedger is the one place that answers "what is this group's balance". Projecting in some screens and not others is how the same group ends up owing two different amounts on home and on its own screen. Settle-up deliberately keeps reading TabEntryRepository: it turns balances into real settlement writes and must only act on entries that exist.
One form for all three entry types and for schedules. Settlements gain a receiver picker and drop the split editor; the kind toggle locks once bound to something that exists, since entry type is fixed server-side and a series' type is fixed for its whole life. Setting a repeat saves a schedule *instead of* an entry. The server writes the first occurrence itself, so saving both books the same thing twice in everybody's ledger with no error raised. The repeat editor is a full screen, not a sheet: a frequency list, an interval stepper, a start date and three end options do not fit one without nesting scrolls, and the date preview is what makes a monthly schedule anchored on the 31st legible before it is saved. Its fields are held separately from the assembled config so flipping through "Never" does not reset a schedule the user already tuned.
A read-only tab beside the others, and a detail screen carrying the three things a member can do: skip a future occurrence, edit from a future occurrence onwards, end it. needsAttention means one thing -- a member the template names has left, so nothing is being created -- and it is surfaced loudly, naming them, because nothing else on the screen would say so. Placeholder rows are faded, chipped and not clickable. They count in the balances above but have no server id to open, and every action lives on the schedule instead. EditSettlement is retired now that the add form handles settlements. Its NavKey stays registered and redirects: dropping a polymorphic subclass outright fails to deserialize a back stack persisted by an older build and takes the whole restored stack down with it. Safe to delete a release from now.
DennisBauer
force-pushed
the
feat/recurring-entries
branch
from
August 11, 2026 15:05
f5fef85 to
02356c7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupoverview/GroupDetailPane.kt`:
- Around line 1336-1343: Update the today state in the RecurringTab composition
so it refreshes when the UTC calendar date changes, rather than remaining fixed
by the no-key remember block. Use boundary-aware state or the existing ViewModel
date source, while preserving the current UTC date calculation and
next-occurrence behavior.
🪄 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: 59ce1619-f53d-4bd7-bb9e-e2f8d54191da
📒 Files selected for processing (18)
features/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/mappers/RecurringSeriesMappers.ktfeatures/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/recurring/DefaultScheduledLedger.ktfeatures/tabgroup/data/src/desktopTest/kotlin/de/tabmates/features/tabgroup/data/recurring/DefaultScheduledLedgerTest.ktfeatures/tabgroup/data/src/desktopTest/kotlin/de/tabmates/features/tabgroup/data/sync/OfflineFirstSyncRepositoryTest.ktfeatures/tabgroup/data/src/desktopTest/kotlin/de/tabmates/features/tabgroup/data/tabentry/TabEntryRealtimeSyncTest.ktfeatures/tabgroup/domain/src/commonMain/kotlin/de/tabmates/features/tabgroup/domain/recurring/ScheduledLedger.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/addentry/AddEntryViewModel.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/RepeatEditorScreen.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/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/AddEntryRecurringTest.ktfeatures/tabgroup/presentation/src/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupdetail/GroupDetailScheduledEntriesTest.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/testing/FakeRecurringSeriesRepository.kt
🚧 Files skipped from review as they are similar to previous changes (10)
- features/tabgroup/domain/src/commonMain/kotlin/de/tabmates/features/tabgroup/domain/recurring/ScheduledLedger.kt
- features/tabgroup/data/src/desktopTest/kotlin/de/tabmates/features/tabgroup/data/sync/OfflineFirstSyncRepositoryTest.kt
- features/tabgroup/presentation/src/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupdetail/GroupDetailViewModelTest.kt
- features/tabgroup/presentation/src/commonMain/composeResources/values-de/string.xml
- features/tabgroup/presentation/src/commonMain/composeResources/values/string.xml
- features/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/recurring/DefaultScheduledLedger.kt
- features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/RepeatEditorScreen.kt
- features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupdetail/GroupDetailViewModel.kt
- features/tabgroup/data/src/commonMain/kotlin/de/tabmates/features/tabgroup/data/mappers/RecurringSeriesMappers.kt
- features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/AddEntryViewModel.kt
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupschedules/GroupSchedulesScreen.kt (1)
236-236: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
DEFAULT_CURRENCY_DECIMALSinstead of a local constant.
GroupDetailPane.ktimportsde.tabmates.core.presentation.format.DEFAULT_CURRENCY_DECIMALSfor the same fallback. A second private constant with the same meaning can drift.♻️ Proposed refactor
+import de.tabmates.core.presentation.format.DEFAULT_CURRENCY_DECIMALS- currency?.decimalDigits ?: DEFAULT_DECIMALS, + currency?.decimalDigits ?: DEFAULT_CURRENCY_DECIMALS,- -private const val DEFAULT_DECIMALS = 2Also applies to: 285-285
🤖 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/groupschedules/GroupSchedulesScreen.kt` at line 236, Replace the local decimal fallback used by GroupSchedulesScreen with the shared DEFAULT_CURRENCY_DECIMALS symbol, including both occurrences referenced in the diff. Import and reuse the existing format constant consistently, removing the duplicate local constant if present.features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupoverview/EntryIcon.kt (1)
23-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a
modifierparameter, and consider moving the file to the shared components package.Two points on this shared composable:
EntryIconemits UI but accepts nomodifier. Callers cannot add padding, size, or semantics. Addmodifier: Modifier = Modifieras the first optional parameter and apply it to the rootBox.- The doc comment states the badge is shared.
GroupSchedulesScreen.ktimports it from thegroupoverviewpackage.DetailHeroalready lives inpresentation.components. MovingEntryIconthere would keep shared components in one place.♻️ Proposed refactor for the modifier parameter
`@Composable` internal fun EntryIcon( icon: DrawableResource, + modifier: Modifier = Modifier, containerColor: Color = MaterialTheme.colorScheme.surfaceVariant, contentColor: Color = MaterialTheme.colorScheme.onSurfaceVariant, ) { Box( modifier = - Modifier + modifier .size(40.dp) .background(containerColor, RoundedCornerShape(10.dp)),🤖 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/groupoverview/EntryIcon.kt` around lines 23 - 43, Update EntryIcon to accept modifier: Modifier = Modifier as its first optional parameter and apply it to the root Box so callers can customize the emitted UI. Move EntryIcon from the groupoverview package to presentation.components, then update GroupSchedulesScreen and any other references/imports to use the new shared-components location.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/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupoverview/UpcomingSchedule.kt`:
- Around line 22-55: Update RecurringOccurrenceCalculator to jump directly to
the first slot after the cutoff instead of scanning up to MAX_SLOT_SCAN, then
compute occurrence projections in GroupDetailViewModel and
GroupSchedulesViewModel before composition. Pass those precomputed results into
UpcomingSchedule/upcomingSchedules and the GroupSchedulesScreen composable,
removing the occurrence scans from both remember blocks; the overview projection
belongs in GroupDetailViewModel, not GroupOverviewViewModel. Affected sites:
features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupoverview/UpcomingSchedule.kt:22-55
must consume the projected results, and
features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupschedules/GroupSchedulesScreen.kt:176-189
must likewise consume ViewModel-provided projections.
In
`@features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupschedules/GroupSchedulesScreen.kt`:
- Around line 276-283: Update rememberTodayUtc so its date refreshes across UTC
midnight instead of being permanently cached by an unkeyed remember. Prefer
exposing and collecting the current date from GroupSchedulesViewModel, reusing
DefaultScheduledLedger’s boundary-driven emissions; otherwise use state
scheduled to update at the next UTC midnight, and ensure ScheduleRow receives
the refreshed cutoff.
---
Nitpick comments:
In
`@features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupoverview/EntryIcon.kt`:
- Around line 23-43: Update EntryIcon to accept modifier: Modifier = Modifier as
its first optional parameter and apply it to the root Box so callers can
customize the emitted UI. Move EntryIcon from the groupoverview package to
presentation.components, then update GroupSchedulesScreen and any other
references/imports to use the new shared-components location.
In
`@features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupschedules/GroupSchedulesScreen.kt`:
- Line 236: Replace the local decimal fallback used by GroupSchedulesScreen with
the shared DEFAULT_CURRENCY_DECIMALS symbol, including both occurrences
referenced in the diff. Import and reuse the existing format constant
consistently, removing the duplicate local constant if present.
🪄 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: d01e4257-6322-48f6-b316-48eca7484026
📒 Files selected for processing (19)
features/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/components/DetailHero.ktfeatures/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/RepeatEditorScreen.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/entrydetail/EntryDetailRoot.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/groupoverview/EntryIcon.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/groupoverview/UpcomingSchedule.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupschedules/GroupSchedulesScreen.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupschedules/GroupSchedulesState.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupschedules/GroupSchedulesViewModel.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupsettings/GroupSettingsScreen.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/recurringdetail/RecurringSeriesDetailRoot.ktfeatures/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/settlementdetail/SettlementDetailRoot.ktfeatures/tabgroup/presentation/src/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupoverview/UpcomingSchedulesTest.ktfeatures/tabgroup/presentation/src/commonTest/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupschedules/GroupSchedulesViewModelTest.kt
🚧 Files skipped from review as they are similar to previous changes (5)
- features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/addentry/RepeatEditorScreen.kt
- features/tabgroup/presentation/src/commonMain/composeResources/values/string.xml
- features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/recurringdetail/RecurringSeriesDetailRoot.kt
- features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/MainGraph.kt
- features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupdetail/GroupDetailRoot.kt
Comment on lines
+22
to
+55
| internal fun upcomingSchedules( | ||
| series: List<RecurringSeries>, | ||
| today: LocalDate, | ||
| ): List<UpcomingSchedule> = | ||
| series | ||
| .filter { it.isActive } | ||
| .map { candidate -> | ||
| UpcomingSchedule( | ||
| series = candidate, | ||
| // A parked schedule promises no date: the server writes nothing for it until a | ||
| // member repairs the template. | ||
| nextDate = | ||
| if (candidate.needsAttention) { | ||
| null | ||
| } else { | ||
| RecurringOccurrenceCalculator | ||
| .upcomingOccurrences( | ||
| rule = candidate.rule, | ||
| after = today, | ||
| limit = 1, | ||
| skippedDates = candidate.skippedOccurrenceDates, | ||
| ).firstOrNull() | ||
| }, | ||
| ) | ||
| } | ||
| // An active schedule whose dates have run out has nothing upcoming to promise, so it belongs | ||
| // on the schedules screen and not in a section named for what is coming. | ||
| .filter { it.series.needsAttention || it.nextDate != null } | ||
| // Parked first: they are the only rows asking for something, and the peek limit could | ||
| // otherwise bury the one schedule that is silently producing nothing. | ||
| .sortedWith( | ||
| compareByDescending<UpcomingSchedule> { it.series.needsAttention } | ||
| .thenBy(nullsLast()) { it.nextDate }, | ||
| ) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -e kt 'RecurringOccurrenceCalculator' --exec cat -n {}Repository: TabMates/app
Length of output: 24606
🏁 Script executed:
#!/bin/bash
set -euo pipefail
files=(
"features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupoverview/UpcomingSchedule.kt"
"features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupschedules/GroupSchedulesScreen.kt"
)
for file in "${files[@]}"; do
echo "===== $file ====="
wc -l "$file"
sed -n '1,90p' "$file"
sed -n '140,215p' "$file"
done
echo "===== related symbols and ViewModels ====="
rg -n -t kt \
"upcomingSchedules|UpcomingSchedule|RecurringOccurrenceCalculator|class .*ViewModel|fun .*ViewModel|remember\\(" \
features/tabgroup/presentation/src/commonMain/kotlinRepository: TabMates/app
Length of output: 10240
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "===== Kotlin files ====="
fd -e kt . features/tabgroup/presentation/src/commonMain/kotlin | sort
echo "===== calculator and projection call sites ====="
rg -n --glob '*.kt' \
"upcomingSchedules|UpcomingSchedule|RecurringOccurrenceCalculator|GroupSchedulesViewModel|GroupOverviewViewModel|ScheduleRow\\(" \
features/tabgroup/presentation/src/commonMain/kotlin
echo "===== GroupSchedulesViewModel candidates ====="
fd -e kt . | while read -r file; do
if rg -q "class GroupSchedulesViewModel|GroupSchedulesViewModel" "$file"; then
echo "--- $file ---"
cat -n "$file"
fi
done
echo "===== GroupOverviewViewModel candidates ====="
fd -e kt . | while read -r file; do
if rg -q "class GroupOverviewViewModel|GroupOverviewViewModel" "$file"; then
echo "--- $file ---"
cat -n "$file"
fi
doneRepository: TabMates/app
Length of output: 50368
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "===== GroupDetailPane occurrence projection ====="
sed -n '450,525p' \
features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupoverview/GroupDetailPane.kt
sed -n '1335,1370p' \
features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupoverview/GroupDetailPane.kt
echo "===== GroupDetailViewModel and state ====="
sed -n '1,220p' \
features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupdetail/GroupDetailViewModel.kt
rg -n --glob '*.kt' \
"recurringSeries|GroupDetailState|GroupDetailPane\\(" \
features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupdetail \
features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupoverview
echo "===== GroupSchedulesState ====="
cat -n \
features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupschedules/GroupSchedulesState.kt
echo "===== Calculator call-site summary ====="
rg -n --glob '*.kt' \
"RecurringOccurrenceCalculator\\.upcomingOccurrences|upcomingSchedules\\(" \
features/tabgroup/presentation/src/commonMain/kotlinRepository: TabMates/app
Length of output: 22043
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "===== RecurringRule validation and construction ====="
rg -n --glob '*.kt' \
"data class RecurringRule|class RecurringRule|RecurringRule\\(|startDate|MAX_SLOT_SCAN|interval must be" \
features/tabgroup core
echo "===== Relevant ViewModel state construction ====="
sed -n '100,205p' \
features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupdetail/GroupDetailViewModel.kt
sed -n '20,55p' \
features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupschedules/GroupSchedulesViewModel.kt
echo "===== Standalone iteration and cutoff probe ====="
python3 - <<'PY'
from datetime import date, timedelta
MAX_SLOT_SCAN = 10_000
def scan_daily(start, after, interval=1, limit=1):
taken = 0
visited = 0
result = []
while visited < MAX_SLOT_SCAN:
current = start + timedelta(days=interval * visited)
visited += 1
if taken >= limit:
break
if current <= after:
continue
taken += 1
result.append(current)
return visited, result
for start, after in [
(date(2020, 1, 1), date(2026, 8, 1)),
(date(1970, 1, 1), date(2026, 8, 1)),
]:
visited, result = scan_daily(start, after)
print(f"start={start} after={after} visited={visited} result={result}")
PYRepository: TabMates/app
Length of output: 23380
Fix occurrence projection before merge. upcomingOccurrences scans one slot at a time and stops after MAX_SLOT_SCAN = 10_000. A daily rule that started more than 10,000 days before today returns no next date, even when a valid occurrence exists. Both remember blocks also perform this scan during composition. Make the calculator jump to the first slot after the cutoff, then compute the projections in GroupDetailViewModel and GroupSchedulesViewModel and pass the results to the composables. The overview projection is currently owned by GroupDetailViewModel, not GroupOverviewViewModel.
features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupoverview/UpcomingSchedule.kt#L22-L55features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupschedules/GroupSchedulesScreen.kt#L176-L189
📍 Affects 2 files
features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupoverview/UpcomingSchedule.kt#L22-L55(this comment)features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupschedules/GroupSchedulesScreen.kt#L176-L189
🤖 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/groupoverview/UpcomingSchedule.kt`
around lines 22 - 55, Update RecurringOccurrenceCalculator to jump directly to
the first slot after the cutoff instead of scanning up to MAX_SLOT_SCAN, then
compute occurrence projections in GroupDetailViewModel and
GroupSchedulesViewModel before composition. Pass those precomputed results into
UpcomingSchedule/upcomingSchedules and the GroupSchedulesScreen composable,
removing the occurrence scans from both remember blocks; the overview projection
belongs in GroupDetailViewModel, not GroupOverviewViewModel. Affected sites:
features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupoverview/UpcomingSchedule.kt:22-55
must consume the projected results, and
features/tabgroup/presentation/src/commonMain/kotlin/de/tabmates/features/tabgroup/presentation/navigation/groupschedules/GroupSchedulesScreen.kt:176-189
must likewise consume ViewModel-provided projections.
Comment on lines
+276
to
+283
| @Composable | ||
| private fun rememberTodayUtc(): LocalDate = | ||
| remember { | ||
| Clock.System | ||
| .now() | ||
| .toLocalDateTime(TimeZone.UTC) | ||
| .date | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
rememberTodayUtc never refreshes.
The remember call has no key, so the value stays fixed for the lifetime of the composition. If the screen stays open across a UTC day boundary, ScheduleRow keeps the old cutoff and can show an occurrence that already passed. Expose the current date from GroupSchedulesViewModel, or use state that updates at the next UTC midnight. DefaultScheduledLedger already re-emits at that boundary and can serve as the source.
🤖 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/groupschedules/GroupSchedulesScreen.kt`
around lines 276 - 283, Update rememberTodayUtc so its date refreshes across UTC
midnight instead of being permanently cached by an unkeyed remember. Prefer
exposing and collecting the current date from GroupSchedulesViewModel, reusing
DefaultScheduledLedger’s boundary-driven emissions; otherwise use state
scheduled to update at the next UTC midnight, and ensure ScheduleRow receives
the refreshed cutoff.
Four tabs was one too many, and two of them overlapped: due occurrences were already faded rows in Transactions while the rules behind them sat in Repeating, so the same rent read twice. Upcoming is now a section above the ledger carrying the schedules about to produce something, and the full list -- ended ones included -- moves to its own screen behind Manage and a group settings row. The section is a boundary, not a merge. A due occurrence has moved the balance already and belongs in the ledger at its own date; an upcoming one has not, so its amount is muted and never reaches the stat cards. Editing a schedule was unreachable. onEdit had a single call site -- the banner only a parked schedule shows -- so a healthy one could not be edited at all. The pencil and the end action now sit in the top bar the way an entry's do. HeroSection existed twice, byte for byte, in entry and settlement detail; it is one DetailHero serving all three now, and EntryIcon moves out of GroupDetailPane for the same reason. A schedule that changes shape on the way to the screen listing them reads as a different kind of thing. Rails were the rest of it. The transactions tab hangs off 24dp and the new rows sat on 16; the repeat editor's root column had no padding at all, so FieldRow drew its outline against both screen edges. Its radio rows were also under Material's 48dp minimum touch target -- the row is the selectable, and RadioButton only applies that floor when it is itself clickable, so onClick = null dropped it and nothing put it back.
DennisBauer
force-pushed
the
feat/recurring-entries
branch
from
August 11, 2026 18:22
ca027e6 to
a8eaafa
Compare
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