bug: REPLY notifications not surfaced in Chronik "Für dich" feed #295

New Issue

marcel · 2026-04-20T19:45:37+02:00

marcel commented

2026-04-20 19:45:37 +02:00

Problem

When user A replies to a comment thread that user B started (or participated in), user B receives a REPLY notification in the bell. However, the corresponding COMMENT_ADDED event never appears in the Chronik activity feed under the "Für dich" filter — even after the notification has been read/dismissed.

The "Für dich" box at the top correctly hides read notifications (that is expected behaviour). But the activity timeline below it should surface the event regardless of read status.

Root Cause

The "fuer-dich" filter on the timeline is:

// +page.svelte:101-102
case 'fuer-dich':
    return merged.filter((i) => i.kind === 'MENTION_CREATED' || i.youMentioned);

youMentioned is computed in SQL (AuditLogQueryRepository.java:72-73) as:

BOOL_OR(s.kind = 'MENTION_CREATED'
    AND s.payload->>'mentionedUserId' = :currentUserId) AS you_mentioned

Because youMentioned is derived exclusively from MENTION_CREATED rows, it is structurally impossible for a COMMENT_ADDED row to ever have youMentioned = true. The filter is therefore equivalent to kind === 'MENTION_CREATED' only.

A reply to your comment creates a COMMENT_ADDED audit event (CommentService.java:184). That event never qualifies for the "Für dich" filter, so it silently disappears from the user's personalised view.

Inconsistency

System	Definition of "for you"
Notification bell	REPLY (thread participant) + MENTION (explicit @)
Chronik "Für dich" filter	MENTION only (effectively)

Proposed Fix

Add a youParticipated flag to ActivityFeedItemDTO, computed in the activity-feed SQL query via a correlated subquery against the notifications table:

EXISTS(
    SELECT 1 FROM notifications n
    WHERE n.type = 'REPLY'
      AND n.recipient_id::text = :currentUserId
      AND n.reference_id::text = (s.payload->>'commentId')
) AS you_participated

Then widen the frontend filter:

case 'fuer-dich':
    return merged.filter(
        (i) => i.kind === 'MENTION_CREATED' || i.youMentioned || i.youParticipated
    );

This keeps the SQL cheap (index on notifications.recipient_id + notifications.reference_id) and avoids any schema changes.

Affected files

backend/src/main/java/org/raddatz/familienarchiv/audit/AuditLogQueryRepository.java — add you_participated CTE/subquery
backend/src/main/java/org/raddatz/familienarchiv/dashboard/DashboardService.java — map new column
frontend/src/lib/generated/api.ts — regen after schema change
frontend/src/routes/chronik/+page.svelte — widen filter predicate

## Problem When user A replies to a comment thread that user B started (or participated in), user B receives a **REPLY** notification in the bell. However, the corresponding `COMMENT_ADDED` event **never appears** in the Chronik activity feed under the "Für dich" filter — even after the notification has been read/dismissed. The "Für dich" box at the top correctly hides read notifications (that is expected behaviour). But the activity timeline below it should surface the event regardless of read status. ## Root Cause The `"fuer-dich"` filter on the timeline is: ```typescript // +page.svelte:101-102 case 'fuer-dich': return merged.filter((i) => i.kind === 'MENTION_CREATED' || i.youMentioned); ``` `youMentioned` is computed in SQL (`AuditLogQueryRepository.java:72-73`) as: ```sql BOOL_OR(s.kind = 'MENTION_CREATED' AND s.payload->>'mentionedUserId' = :currentUserId) AS you_mentioned ``` Because `youMentioned` is derived exclusively from `MENTION_CREATED` rows, it is **structurally impossible** for a `COMMENT_ADDED` row to ever have `youMentioned = true`. The filter is therefore equivalent to `kind === 'MENTION_CREATED'` only. A reply to your comment creates a `COMMENT_ADDED` audit event (`CommentService.java:184`). That event never qualifies for the "Für dich" filter, so it silently disappears from the user's personalised view. ## Inconsistency | System | Definition of "for you" | |---|---| | Notification bell | REPLY (thread participant) + MENTION (explicit @) | | Chronik "Für dich" filter | MENTION only (effectively) | ## Proposed Fix Add a `youParticipated` flag to `ActivityFeedItemDTO`, computed in the activity-feed SQL query via a correlated subquery against the `notifications` table: ```sql EXISTS( SELECT 1 FROM notifications n WHERE n.type = 'REPLY' AND n.recipient_id::text = :currentUserId AND n.reference_id::text = (s.payload->>'commentId') ) AS you_participated ``` Then widen the frontend filter: ```typescript case 'fuer-dich': return merged.filter( (i) => i.kind === 'MENTION_CREATED' || i.youMentioned || i.youParticipated ); ``` This keeps the SQL cheap (index on `notifications.recipient_id` + `notifications.reference_id`) and avoids any schema changes. ## Affected files - `backend/src/main/java/org/raddatz/familienarchiv/audit/AuditLogQueryRepository.java` — add `you_participated` CTE/subquery - `backend/src/main/java/org/raddatz/familienarchiv/dashboard/DashboardService.java` — map new column - `frontend/src/lib/generated/api.ts` — regen after schema change - `frontend/src/routes/chronik/+page.svelte` — widen filter predicate

marcel added the bug notification labels 2026-04-20 19:45:41 +02:00

marcel commented

2026-04-20 19:47:09 +02:00

🏗️ Markus Keller — Senior Application Architect

Observations

The proposed fix crosses a domain boundary: AuditLogQueryRepository lives in the audit package and the fix adds a correlated subquery against the notifications table. These are two separate domains. This is a pragmatic choice for a monolith, but it should be a conscious decision, not an accident.
The affected files list omits ActivityFeedRow.java — this interface in audit/ActivityFeedRow.java needs a new isYouParticipated() getter before anything else in the chain can work.
The fix is structurally correct: the COMMENT_ADDED payload already stores commentId (set in CommentService.java:184), and notifications.reference_id is that same commentId (per V16 comment: "commentId that triggered this notification"). The join point is solid.

Recommendations

Add ActivityFeedRow.java to the affected files list — it's the missing link between the SQL projection and the DTO mapper in DashboardService.getActivity(). Without isYouParticipated() on the interface, the new SQL column has nowhere to land.

Accept the cross-domain query, but name it explicitly. In the SQL, alias the subquery clearly:

EXISTS(
    SELECT 1 FROM notifications n
    WHERE n.recipient_id = :currentUserId::uuid   -- cast HERE, not on the column
      AND n.type = 'REPLY'
      AND n.reference_id = (s.payload->>'commentId')::uuid
) AS you_participated

Casting :currentUserId::uuid rather than recipient_id::text lets the query planner use the existing index on recipient_id.

The ActivityFeedItemDTO record needs the new field added with @Schema(requiredMode = REQUIRED) to drive TypeScript codegen — consistent with how youMentioned is currently declared at line 17.

Open Decisions

Cross-domain query vs. application-layer join: the alternative to the correlated subquery is having DashboardService call NotificationService.findReplyCommentIds(currentUserId) and pass the result set into the SQL query as an IN (:ids) list. This keeps domain ownership cleaner (notification data fetched via the notification service, not queried from the audit repo) at the cost of a second round-trip. For a family archive with tens of notifications this is noise; at thousands it becomes a real question. Decide which direction you want the boundary to go before implementation.

## 🏗️ Markus Keller — Senior Application Architect ### Observations - The proposed fix crosses a domain boundary: `AuditLogQueryRepository` lives in the `audit` package and the fix adds a correlated subquery against the `notifications` table. These are two separate domains. This is a pragmatic choice for a monolith, but it should be a conscious decision, not an accident. - The affected files list omits `ActivityFeedRow.java` — this interface in `audit/ActivityFeedRow.java` needs a new `isYouParticipated()` getter before anything else in the chain can work. - The fix is structurally correct: the `COMMENT_ADDED` payload already stores `commentId` (set in `CommentService.java:184`), and `notifications.reference_id` is that same `commentId` (per V16 comment: "commentId that triggered this notification"). The join point is solid. ### Recommendations - **Add `ActivityFeedRow.java` to the affected files list** — it's the missing link between the SQL projection and the DTO mapper in `DashboardService.getActivity()`. Without `isYouParticipated()` on the interface, the new SQL column has nowhere to land. - **Accept the cross-domain query, but name it explicitly.** In the SQL, alias the subquery clearly: ```sql EXISTS( SELECT 1 FROM notifications n WHERE n.recipient_id = :currentUserId::uuid -- cast HERE, not on the column AND n.type = 'REPLY' AND n.reference_id = (s.payload->>'commentId')::uuid ) AS you_participated ``` Casting `:currentUserId::uuid` rather than `recipient_id::text` lets the query planner use the existing index on `recipient_id`. - **The `ActivityFeedItemDTO` record** needs the new field added with `@Schema(requiredMode = REQUIRED)` to drive TypeScript codegen — consistent with how `youMentioned` is currently declared at line 17. ### Open Decisions - **Cross-domain query vs. application-layer join**: the alternative to the correlated subquery is having `DashboardService` call `NotificationService.findReplyCommentIds(currentUserId)` and pass the result set into the SQL query as an `IN (:ids)` list. This keeps domain ownership cleaner (notification data fetched via the notification service, not queried from the audit repo) at the cost of a second round-trip. For a family archive with tens of notifications this is noise; at thousands it becomes a real question. Decide which direction you want the boundary to go before implementation.

marcel commented

2026-04-20 19:47:20 +02:00

👨‍💻 Felix Brandt — Senior Fullstack Developer

Observations

The affected files list is missing ActivityFeedRow.java (audit/ActivityFeedRow.java). This interface is the projection interface for the native SQL query result. It currently has isYouMentioned() and needs a new isYouParticipated() getter — without it the new SQL column you_participated has no target and Spring Data JPA will throw at startup.
The proposed SQL uses ::text casts on UUID columns — recipient_id::text = :currentUserId and reference_id::text = (s.payload->>'commentId'). Casting the column to text kills index usage. The correct form casts the parameter: recipient_id = :currentUserId::uuid.
DashboardService.getActivity() (line 136) constructs ActivityFeedItemDTO — once youParticipated is added to the record, the constructor call there needs updating too. It's not in the affected files list.

Recommendations

SQL cast fix — invert the cast direction so the index on recipient_id can be used:

EXISTS(
    SELECT 1 FROM notifications n
    WHERE n.type = 'REPLY'
      AND n.recipient_id = :currentUserId::uuid
      AND n.reference_id = (s.payload->>'commentId')::uuid
) AS you_participated

Complete affected files list:
1. audit/ActivityFeedRow.java — add boolean isYouParticipated()
2. dashboard/ActivityFeedItemDTO.java — add @Schema(requiredMode = REQUIRED) boolean youParticipated
3. audit/AuditLogQueryRepository.java — add you_participated subquery
4. dashboard/DashboardService.java — pass row.isYouParticipated() in the DTO constructor at line 136
5. frontend/src/lib/generated/api.ts — regen (already listed)
6. frontend/src/routes/chronik/+page.svelte — widen filter (already listed)
Frontend filter — the widened predicate is clean as proposed:
```
case 'fuer-dich':
    return merged.filter(
        (i) => i.kind === 'MENTION_CREATED' || i.youMentioned || i.youParticipated
    );
```
No further split needed; the three conditions are semantically distinct and naming each in a $derived would be overkill here.

## 👨‍💻 Felix Brandt — Senior Fullstack Developer ### Observations - The affected files list is **missing `ActivityFeedRow.java`** (`audit/ActivityFeedRow.java`). This interface is the projection interface for the native SQL query result. It currently has `isYouMentioned()` and needs a new `isYouParticipated()` getter — without it the new SQL column `you_participated` has no target and Spring Data JPA will throw at startup. - The proposed SQL uses `::text` casts on UUID columns — `recipient_id::text = :currentUserId` and `reference_id::text = (s.payload->>'commentId')`. Casting the column to text kills index usage. The correct form casts the parameter: `recipient_id = :currentUserId::uuid`. - `DashboardService.getActivity()` (line 136) constructs `ActivityFeedItemDTO` — once `youParticipated` is added to the record, the constructor call there needs updating too. It's not in the affected files list. ### Recommendations - **SQL cast fix** — invert the cast direction so the index on `recipient_id` can be used: ```sql EXISTS( SELECT 1 FROM notifications n WHERE n.type = 'REPLY' AND n.recipient_id = :currentUserId::uuid AND n.reference_id = (s.payload->>'commentId')::uuid ) AS you_participated ``` - **Complete affected files list:** 1. `audit/ActivityFeedRow.java` — add `boolean isYouParticipated()` 2. `dashboard/ActivityFeedItemDTO.java` — add `@Schema(requiredMode = REQUIRED) boolean youParticipated` 3. `audit/AuditLogQueryRepository.java` — add `you_participated` subquery 4. `dashboard/DashboardService.java` — pass `row.isYouParticipated()` in the DTO constructor at line 136 5. `frontend/src/lib/generated/api.ts` — regen (already listed) 6. `frontend/src/routes/chronik/+page.svelte` — widen filter (already listed) - **Frontend filter** — the widened predicate is clean as proposed: ```typescript case 'fuer-dich': return merged.filter( (i) => i.kind === 'MENTION_CREATED' || i.youMentioned || i.youParticipated ); ``` No further split needed; the three conditions are semantically distinct and naming each in a `$derived` would be overkill here.

marcel commented

2026-04-20 19:47:29 +02:00

🔒 Nora Steiner — Application Security Engineer

Observations

No IDOR risk: the correlated subquery is always scoped to recipient_id = :currentUserId, so a user can only ever see feed items as "participated" if they themselves have a REPLY notification for that comment. Good.
The ::text cast on recipient_id (as written in the issue) is not a security vulnerability, but it is a correctness smell: comparing UUIDs via text representations has different collation semantics and disables index scans. It should be fixed (see Felix's comment) but it is not a security issue.
Implicit payload contract dependency: the subquery relies on s.payload->>'commentId' matching notifications.reference_id. This is documented in V16 ("commentId that triggered this notification") and in AuditKind.java:24 ("Payload: {"commentId": "uuid"}"). The contract is explicit in two places. If the payload key were ever renamed, the subquery would silently return false for all rows rather than failing loudly. This is an operational risk, not an exploitable one.

Recommendations

No security blockers. The fix is well-scoped to the current user and uses parameterized queries throughout.
Add a comment on the payload contract in the SQL to make the implicit dependency explicit:
```
-- payload->>'commentId' matches notifications.reference_id per AuditKind.COMMENT_ADDED contract
AND n.reference_id = (s.payload->>'commentId')::uuid
```
This is the kind of "why" comment that prevents a future developer from renaming the payload key without realizing the impact.
Regression test: write a test that verifies youParticipated = false when queried as a user who has NO reply notification for that comment (i.e., someone who was not a thread participant). This proves the subquery is properly scoped and doesn't bleed across users.

## 🔒 Nora Steiner — Application Security Engineer ### Observations - No IDOR risk: the correlated subquery is always scoped to `recipient_id = :currentUserId`, so a user can only ever see feed items as "participated" if they themselves have a REPLY notification for that comment. Good. - The `::text` cast on `recipient_id` (as written in the issue) is not a security vulnerability, but it is a correctness smell: comparing UUIDs via text representations has different collation semantics and disables index scans. It should be fixed (see Felix's comment) but it is not a security issue. - **Implicit payload contract dependency**: the subquery relies on `s.payload->>'commentId'` matching `notifications.reference_id`. This is documented in V16 ("commentId that triggered this notification") and in `AuditKind.java:24` ("Payload: `{"commentId": "uuid"}`"). The contract is explicit in two places. If the payload key were ever renamed, the subquery would silently return `false` for all rows rather than failing loudly. This is an operational risk, not an exploitable one. ### Recommendations - **No security blockers.** The fix is well-scoped to the current user and uses parameterized queries throughout. - **Add a comment on the payload contract** in the SQL to make the implicit dependency explicit: ```sql -- payload->>'commentId' matches notifications.reference_id per AuditKind.COMMENT_ADDED contract AND n.reference_id = (s.payload->>'commentId')::uuid ``` This is the kind of "why" comment that prevents a future developer from renaming the payload key without realizing the impact. - **Regression test**: write a test that verifies `youParticipated = false` when queried as a user who has NO reply notification for that comment (i.e., someone who was not a thread participant). This proves the subquery is properly scoped and doesn't bleed across users.

marcel commented

2026-04-20 19:47:43 +02:00

🧪 Sara Holt — QA Engineer & Test Strategist

Observations

Good existing test infrastructure. AuditLogQueryRepositoryRolledUpTest uses @DataJpaTest + @Import(PostgresContainerConfig.class) with a real Postgres container. It already has insertUserAndDocs() and insertAuditEvent() helpers. This is the exact right class to add youParticipated tests to.
No existing tests for youMentioned. A grep across the test directory finds zero assertions against youMentioned in the test suite. This means the new youParticipated behaviour has no model test to follow, but it also means we should write both together.
The issue proposes no test plan. For a bug this size (SQL query + interface + DTO + service + frontend filter), each layer needs coverage.

Recommendations

Layer 1 — SQL / Repository (integration test in AuditLogQueryRepositoryRolledUpTest):

@Test
void youParticipated_is_true_when_user_has_reply_notification_for_comment() {
    insertUserAndDocs();
    UUID commentId = UUID.randomUUID();
    insertAuditEvent(OTHER_USER_ID, DOC_ID, "COMMENT_ADDED",
        Instant.now(), Map.of("commentId", commentId.toString()));
    insertReplyNotification(USER_ID, DOC_ID, commentId);  // helper to insert into notifications

    List<ActivityFeedRow> rows = auditLogQueryRepository
        .findRolledUpActivityFeed(USER_ID.toString(), 40);

    assertThat(rows).anySatisfy(r ->
        assertThat(r.isYouParticipated()).isTrue()
    );
}

@Test
void youParticipated_is_false_for_comment_with_no_reply_notification() {
    insertUserAndDocs();
    insertAuditEvent(OTHER_USER_ID, DOC_ID, "COMMENT_ADDED",
        Instant.now(), Map.of("commentId", UUID.randomUUID().toString()));
    // no notification inserted

    List<ActivityFeedRow> rows = auditLogQueryRepository
        .findRolledUpActivityFeed(USER_ID.toString(), 40);

    assertThat(rows).allSatisfy(r ->
        assertThat(r.isYouParticipated()).isFalse()
    );
}

Layer 2 — Load function (Vitest, import +page.server.ts directly):

Existing test for the Chronik load function (if any) should verify that unreadNotifications is loaded regardless of the filter param.

Layer 3 — Frontend filter (+page.svelte):

Unit-test the displayFeed derivation: given a feed item with youParticipated=true and kind='COMMENT_ADDED', it should appear in 'fuer-dich' and 'kommentare' but not 'hochgeladen' or 'transkription'.

Also: add a insertAuditEvent overload (or update the existing one) that accepts a Map<String, String> payload — the current helper in AuditLogQueryRepositoryRolledUpTest inserts events without payload, which won't work for the COMMENT_ADDED test that relies on payload->>'commentId'.

## 🧪 Sara Holt — QA Engineer & Test Strategist ### Observations - **Good existing test infrastructure.** `AuditLogQueryRepositoryRolledUpTest` uses `@DataJpaTest` + `@Import(PostgresContainerConfig.class)` with a real Postgres container. It already has `insertUserAndDocs()` and `insertAuditEvent()` helpers. This is the exact right class to add `youParticipated` tests to. - **No existing tests for `youMentioned`.** A grep across the test directory finds zero assertions against `youMentioned` in the test suite. This means the new `youParticipated` behaviour has no model test to follow, but it also means we should write both together. - The issue proposes no test plan. For a bug this size (SQL query + interface + DTO + service + frontend filter), each layer needs coverage. ### Recommendations **Layer 1 — SQL / Repository (integration test in `AuditLogQueryRepositoryRolledUpTest`):** ```java @Test void youParticipated_is_true_when_user_has_reply_notification_for_comment() { insertUserAndDocs(); UUID commentId = UUID.randomUUID(); insertAuditEvent(OTHER_USER_ID, DOC_ID, "COMMENT_ADDED", Instant.now(), Map.of("commentId", commentId.toString())); insertReplyNotification(USER_ID, DOC_ID, commentId); // helper to insert into notifications List<ActivityFeedRow> rows = auditLogQueryRepository .findRolledUpActivityFeed(USER_ID.toString(), 40); assertThat(rows).anySatisfy(r -> assertThat(r.isYouParticipated()).isTrue() ); } @Test void youParticipated_is_false_for_comment_with_no_reply_notification() { insertUserAndDocs(); insertAuditEvent(OTHER_USER_ID, DOC_ID, "COMMENT_ADDED", Instant.now(), Map.of("commentId", UUID.randomUUID().toString())); // no notification inserted List<ActivityFeedRow> rows = auditLogQueryRepository .findRolledUpActivityFeed(USER_ID.toString(), 40); assertThat(rows).allSatisfy(r -> assertThat(r.isYouParticipated()).isFalse() ); } ``` **Layer 2 — Load function (Vitest, import `+page.server.ts` directly):** - Existing test for the Chronik load function (if any) should verify that `unreadNotifications` is loaded regardless of the `filter` param. **Layer 3 — Frontend filter (`+page.svelte`):** - Unit-test the `displayFeed` derivation: given a feed item with `youParticipated=true` and `kind='COMMENT_ADDED'`, it should appear in `'fuer-dich'` and `'kommentare'` but not `'hochgeladen'` or `'transkription'`. **Also:** add a `insertAuditEvent` overload (or update the existing one) that accepts a `Map<String, String>` payload — the current helper in `AuditLogQueryRepositoryRolledUpTest` inserts events without payload, which won't work for the `COMMENT_ADDED` test that relies on `payload->>'commentId'`.

marcel commented

2026-04-20 19:47:51 +02:00

🎨 Leonie Voss — UX Designer & Accessibility Strategist

Observations

This fix is purely a data correctness issue — no UI components are being added or changed. From a design perspective, the fix is correct: once youParticipated is wired up, COMMENT_ADDED events will appear in the "Für dich" timeline, which is the right place for them.
The existing ChronikTimeline component already renders COMMENT_ADDED events (they appear in the "alle" and "kommentare" filters). The only change is that they now also appear in "fuer-dich". No new visual treatment is needed — the existing comment event row design is appropriate.
One minor UX consideration: the "Für dich" filter currently surfaces only mention events. After the fix, it will also include reply events. These two event types already have the same visual treatment (actor avatar + document link + timestamp). No differentiation is needed at this stage.

Recommendations

No visual or interaction design changes required for this fix.
Verify the existing COMMENT_ADDED row renders correctly at 320px in the "fuer-dich" context — not a new concern, but since this event type will now appear in a new context, it's worth a quick manual check that the document title doesn't overflow on small screens.
When this feature is complete, the "Für dich" filter label is accurate: it now truly means "notifications addressed to you" (mentions) AND "conversations you participated in" (replies). No label change needed.

## 🎨 Leonie Voss — UX Designer & Accessibility Strategist ### Observations - This fix is purely a data correctness issue — no UI components are being added or changed. From a design perspective, the fix is correct: once `youParticipated` is wired up, `COMMENT_ADDED` events will appear in the "Für dich" timeline, which is the right place for them. - The existing `ChronikTimeline` component already renders `COMMENT_ADDED` events (they appear in the "alle" and "kommentare" filters). The only change is that they now also appear in "fuer-dich". No new visual treatment is needed — the existing comment event row design is appropriate. - One minor UX consideration: the "Für dich" filter currently surfaces only mention events. After the fix, it will also include reply events. These two event types already have the same visual treatment (actor avatar + document link + timestamp). No differentiation is needed at this stage. ### Recommendations - No visual or interaction design changes required for this fix. - **Verify the existing COMMENT_ADDED row renders correctly at 320px** in the "fuer-dich" context — not a new concern, but since this event type will now appear in a new context, it's worth a quick manual check that the document title doesn't overflow on small screens. - When this feature is complete, the "Für dich" filter label is accurate: it now truly means "notifications addressed to you" (mentions) AND "conversations you participated in" (replies). No label change needed.

marcel commented

2026-04-20 19:48:00 +02:00

⚙️ Tobias Wendt — DevOps & Platform Engineer

Observations

No infrastructure, Docker Compose, or CI pipeline changes required for this fix. All changes are confined to the application layer and a potential new Flyway migration.
Index concern: the proposed correlated subquery filters notifications by recipient_id and reference_id. The existing index from V16 is:
```
CREATE INDEX idx_notifications_recipient ON notifications(recipient_id, read, created_at DESC);
```
reference_id is NOT in this index. At family archive scale (tens to low hundreds of notifications per user), a sequential scan within the user's notification rows is fine — the index on recipient_id reduces the scan to one user's rows. At thousands of notifications, adding reference_id to the index would help.
No new Flyway migration is needed for the fix itself (it's a pure query change). If the team decides to add a reference_id index later, that would be V50.

Recommendations

No action required from DevOps. The fix is self-contained in application code.
Future index (not blocking): if notification volume grows beyond ~500 per user, consider:
```
-- V50__add_notifications_reference_id_index.sql
CREATE INDEX idx_notifications_reference_id ON notifications(recipient_id, reference_id)
    WHERE type = 'REPLY';
```
The partial index on type = 'REPLY' keeps it lean — only the rows the subquery actually cares about.
OpenAPI regen (already noted in the issue) requires the backend to be running with --spring.profiles.active=dev to expose /v3/api-docs. Make sure the dev Docker Compose stack is up before running npm run generate:api in frontend/.

## ⚙️ Tobias Wendt — DevOps & Platform Engineer ### Observations - No infrastructure, Docker Compose, or CI pipeline changes required for this fix. All changes are confined to the application layer and a potential new Flyway migration. - **Index concern**: the proposed correlated subquery filters `notifications` by `recipient_id` and `reference_id`. The existing index from V16 is: ```sql CREATE INDEX idx_notifications_recipient ON notifications(recipient_id, read, created_at DESC); ``` `reference_id` is NOT in this index. At family archive scale (tens to low hundreds of notifications per user), a sequential scan within the user's notification rows is fine — the index on `recipient_id` reduces the scan to one user's rows. At thousands of notifications, adding `reference_id` to the index would help. - **No new Flyway migration is needed** for the fix itself (it's a pure query change). If the team decides to add a `reference_id` index later, that would be V50. ### Recommendations - **No action required from DevOps.** The fix is self-contained in application code. - **Future index** (not blocking): if notification volume grows beyond ~500 per user, consider: ```sql -- V50__add_notifications_reference_id_index.sql CREATE INDEX idx_notifications_reference_id ON notifications(recipient_id, reference_id) WHERE type = 'REPLY'; ``` The partial index on `type = 'REPLY'` keeps it lean — only the rows the subquery actually cares about. - **OpenAPI regen** (already noted in the issue) requires the backend to be running with `--spring.profiles.active=dev` to expose `/v3/api-docs`. Make sure the dev Docker Compose stack is up before running `npm run generate:api` in `frontend/`.

marcel commented

2026-04-20 19:48:11 +02:00

🗳️ Decision Queue — Action Required

1 decision needs your input before implementation starts.

Architecture

Cross-domain query vs. application-layer join — The proposed fix queries the notifications table directly from AuditLogQueryRepository (in the audit domain). The alternative is: DashboardService calls NotificationService.findReplyCommentIds(currentUserId) first, then passes the resulting UUID set into the SQL query as IN (:ids). The correlated subquery is simpler to implement and performs identically at family archive scale; the service-layer join is cleaner architecturally (each domain fetches its own data) but adds a second DB round-trip. (Raised by: Markus)

## 🗳️ Decision Queue — Action Required _1 decision needs your input before implementation starts._ ### Architecture - **Cross-domain query vs. application-layer join** — The proposed fix queries the `notifications` table directly from `AuditLogQueryRepository` (in the `audit` domain). The alternative is: `DashboardService` calls `NotificationService.findReplyCommentIds(currentUserId)` first, then passes the resulting UUID set into the SQL query as `IN (:ids)`. The correlated subquery is simpler to implement and performs identically at family archive scale; the service-layer join is cleaner architecturally (each domain fetches its own data) but adds a second DB round-trip. _(Raised by: Markus)_

marcel commented

2026-04-20 20:57:10 +02:00

🏗️ Markus Keller — Architect Discussion Summary

One open architectural decision worked through with @marcel.

✅ Resolved

Cross-domain query: keep the correlated subquery — AuditLogQueryRepository querying the notifications table directly is a conscious, deliberate decision. YAGNI applies: a NotificationService.findReplyCommentIds() abstraction only earns its keep when notification data is needed from audit/dashboard in multiple other places. That hasn't happened yet. Accept the cross-domain read, name it explicitly in a SQL comment (as Nora recommended), and revisit the boundary if it recurs.

Overall read

The fix is well-scoped and the team's pre-review caught every structural gap (missing ActivityFeedRow.java, SQL cast direction, DTO constructor, test coverage). The one architectural question was the domain boundary — pragmatism wins here. Ready to implement.

## 🏗️ Markus Keller — Architect Discussion Summary _One open architectural decision worked through with @marcel._ ### ✅ Resolved - **Cross-domain query: keep the correlated subquery** — `AuditLogQueryRepository` querying the `notifications` table directly is a conscious, deliberate decision. YAGNI applies: a `NotificationService.findReplyCommentIds()` abstraction only earns its keep when notification data is needed from audit/dashboard in multiple other places. That hasn't happened yet. Accept the cross-domain read, name it explicitly in a SQL comment (as Nora recommended), and revisit the boundary if it recurs. ### Overall read The fix is well-scoped and the team's pre-review caught every structural gap (missing `ActivityFeedRow.java`, SQL cast direction, DTO constructor, test coverage). The one architectural question was the domain boundary — pragmatism wins here. Ready to implement.

marcel commented

2026-04-20 21:07:32 +02:00

👨‍💻 Felix Brandt — Developer Discussion Summary

Three implementation decisions worked through with @marcel before coding starts.

✅ Resolved

insertReplyNotification helper — use JdbcTemplate — The test needs to pass in the same commentId UUID generated in the test body (to link the COMMENT_ADDED audit event to the notification). A @Sql fixture file would require string interpolation to inject it. JdbcTemplate inline is consistent with insertAuditEvent and insertUserAndDocs, keeps the UUID wiring visible, and reads top-to-bottom without jumping to a fixture file.
youMentioned retroactive tests — write alongside youParticipated — Zero existing assertions against youMentioned. While the test infrastructure is warm and the helpers are in place, the cost to close that gap is near zero. Both get covered in the same pass.
Frontend filter test — extract predicate to chronik/feedFilters.ts, Vitest unit test — No existing Vitest file for the chronik route. Rather than build a full component harness for +page.svelte, extract the filter logic to a pure filterFeed(items, filter) function. The $derived in the page just calls it. The function is trivially testable with no DOM setup, and it keeps business logic out of the template — consistent with the code style guide.

Overall read

Implementation is fully specified. TDD order: failing repository test first (isYouParticipated() on ActivityFeedRow), then SQL, then DTO, then service, then extract and test feedFilters.ts, then widen the $derived predicate. No open decisions remain.

## 👨‍💻 Felix Brandt — Developer Discussion Summary _Three implementation decisions worked through with @marcel before coding starts._ ### ✅ Resolved - **`insertReplyNotification` helper — use `JdbcTemplate`** — The test needs to pass in the same `commentId` UUID generated in the test body (to link the `COMMENT_ADDED` audit event to the notification). A `@Sql` fixture file would require string interpolation to inject it. `JdbcTemplate` inline is consistent with `insertAuditEvent` and `insertUserAndDocs`, keeps the UUID wiring visible, and reads top-to-bottom without jumping to a fixture file. - **`youMentioned` retroactive tests — write alongside `youParticipated`** — Zero existing assertions against `youMentioned`. While the test infrastructure is warm and the helpers are in place, the cost to close that gap is near zero. Both get covered in the same pass. - **Frontend filter test — extract predicate to `chronik/feedFilters.ts`, Vitest unit test** — No existing Vitest file for the chronik route. Rather than build a full component harness for `+page.svelte`, extract the filter logic to a pure `filterFeed(items, filter)` function. The `$derived` in the page just calls it. The function is trivially testable with no DOM setup, and it keeps business logic out of the template — consistent with the code style guide. ### Overall read Implementation is fully specified. TDD order: failing repository test first (`isYouParticipated()` on `ActivityFeedRow`), then SQL, then DTO, then service, then extract and test `feedFilters.ts`, then widen the `$derived` predicate. No open decisions remain.

marcel commented

2026-04-20 21:55:00 +02:00

✅ Implementation Complete

All tasks from the plan have been implemented on branch feat/issue-295-you-participated.

Commits

ef333d80 test(audit): add youParticipated and youMentioned repository tests
— Added isYouParticipated() to ActivityFeedRow interface; 4 new tests covering the positive/negative cases for both youParticipated and youMentioned (retroactive); insertAuditEvent overload with payload; insertReplyNotification JdbcTemplate helper.
64058caf feat(audit): surface youParticipated via REPLY notification subquery
— Added you_participated correlated EXISTS subquery to findRolledUpActivityFeed. Carries payload through the aggregated CTE via MIN(payload::text)::jsonb. Uses CAST(:currentUserId AS uuid) to avoid Spring Data JPA misparse of :: cast syntax as parameter name.
9dd0dbda feat(dashboard): expose youParticipated in ActivityFeedItemDTO
— Added @Schema(requiredMode = REQUIRED) boolean youParticipated to the DTO record and wired row.isYouParticipated() through DashboardService.getActivity().
8b7cedb7 feat(chronik): extract feedFilters.ts with youParticipated in fuer-dich
— Extracted filterFeed(items, filter) pure function; widened fuer-dich branch to kind === 'MENTION_CREATED' || youMentioned || youParticipated; 13 unit tests covering all 5 filters and edge cases; manually synced ActivityFeedItemDTO generated type.
7429db4f refactor(chronik): replace inline filter switch with filterFeed()
— Replaced 20-line inline switch in +page.svelte with a single filterFeed() call.

Test results

Backend: 1196 tests, 0 failures
Frontend (server project): 318 tests, 0 failures

SQL note

Following Markus's recommendation, the cross-domain correlated subquery is deliberate — the SQL comment names the payload->>'commentId' / notifications.reference_id contract per AuditKind.COMMENT_ADDED. Nora's scoping concern is satisfied: the recipient_id = CAST(:currentUserId AS uuid) predicate ensures only the querying user's own notifications are matched.

## ✅ Implementation Complete All tasks from the plan have been implemented on branch `feat/issue-295-you-participated`. ### Commits 1. **`ef333d80`** `test(audit): add youParticipated and youMentioned repository tests` — Added `isYouParticipated()` to `ActivityFeedRow` interface; 4 new tests covering the positive/negative cases for both `youParticipated` and `youMentioned` (retroactive); `insertAuditEvent` overload with payload; `insertReplyNotification` JdbcTemplate helper. 2. **`64058caf`** `feat(audit): surface youParticipated via REPLY notification subquery` — Added `you_participated` correlated EXISTS subquery to `findRolledUpActivityFeed`. Carries `payload` through the `aggregated` CTE via `MIN(payload::text)::jsonb`. Uses `CAST(:currentUserId AS uuid)` to avoid Spring Data JPA misparse of `::` cast syntax as parameter name. 3. **`9dd0dbda`** `feat(dashboard): expose youParticipated in ActivityFeedItemDTO` — Added `@Schema(requiredMode = REQUIRED) boolean youParticipated` to the DTO record and wired `row.isYouParticipated()` through `DashboardService.getActivity()`. 4. **`8b7cedb7`** `feat(chronik): extract feedFilters.ts with youParticipated in fuer-dich` — Extracted `filterFeed(items, filter)` pure function; widened `fuer-dich` branch to `kind === 'MENTION_CREATED' || youMentioned || youParticipated`; 13 unit tests covering all 5 filters and edge cases; manually synced `ActivityFeedItemDTO` generated type. 5. **`7429db4f`** `refactor(chronik): replace inline filter switch with filterFeed()` — Replaced 20-line inline switch in `+page.svelte` with a single `filterFeed()` call. ### Test results - Backend: 1196 tests, 0 failures - Frontend (server project): 318 tests, 0 failures ### SQL note Following Markus's recommendation, the cross-domain correlated subquery is deliberate — the SQL comment names the `payload->>'commentId'` / `notifications.reference_id` contract per `AuditKind.COMMENT_ADDED`. Nora's scoping concern is satisfied: the `recipient_id = CAST(:currentUserId AS uuid)` predicate ensures only the querying user's own notifications are matched.

marcel referenced a pull request that will close this issue

2026-04-20 21:55:12 +02:00

fix(chronik): surface REPLY events in Für-dich feed via youParticipated #297

marcel referenced this issue

2026-04-20 21:57:24 +02:00

fix(chronik): surface REPLY events in Für-dich feed via youParticipated #297

marcel referenced this issue

2026-04-20 21:58:17 +02:00

fix(chronik): surface REPLY events in Für-dich feed via youParticipated #297

marcel closed this issue

2026-04-21 09:00:26 +02:00

marcel referenced this issue from a commit