robert/todo

Watch

Fork 0

#25 — Activity Feed per List #25

New issue

Closed

opened 2026-08-18 13:12:55 +02:00 by lena · 3 comments

lena commented

2026-08-18 13:12:55 +02:00

(Migrated from git.butzei.de)

Story: Activity Feed per List

As a list member,
I want to see a chronological log of what has happened in a list,
so that I can catch up on changes since I last looked without asking others.

Acceptance criteria:

Each list has an "Activity" tab or panel showing a paginated feed of events (newest first)
The following events are recorded and displayed:
- Todo created (title, author)
- Todo completed (title, author)
- Todo reopened (title, author)
- Todo deleted (title, author)
- Member joined the list
- Member removed from the list
- List renamed (old name → new name)
Each event shows the acting user's name and a relative timestamp ("2 hours ago") with an absolute tooltip on hover
The feed is paginated; older events load on scroll or via a "Load more" button
The feed is read-only
All list members (including regular members) can view the feed

Out of scope for this story:

Filtering the feed by event type or user
Editing or deleting events
Notifications based on feed events (that is #26)

Blockers: None

Priority: Should — answers "what changed?" without requiring real-time notifications.

# Story: Activity Feed per List **As a** list member, **I want to** see a chronological log of what has happened in a list, **so that** I can catch up on changes since I last looked without asking others. **Acceptance criteria:** - [ ] Each list has an "Activity" tab or panel showing a paginated feed of events (newest first) - [ ] The following events are recorded and displayed: - Todo created (title, author) - Todo completed (title, author) - Todo reopened (title, author) - Todo deleted (title, author) - Member joined the list - Member removed from the list - List renamed (old name → new name) - [ ] Each event shows the acting user's name and a relative timestamp ("2 hours ago") with an absolute tooltip on hover - [ ] The feed is paginated; older events load on scroll or via a "Load more" button - [ ] The feed is read-only - [ ] All list members (including regular members) can view the feed **Out of scope for this story:** - Filtering the feed by event type or user - Editing or deleting events - Notifications based on feed events (that is `#26`) **Blockers:** None **Priority:** Should — answers "what changed?" without requiring real-time notifications.

lena commented

2026-08-18 13:12:55 +02:00

(Migrated from git.butzei.de)

security (25_activity_feed_security.md)

Security Pre-Review: Story `#25` — Activity Feed per List

Reviewer: Security Agent
Date: 2026-07-11
Design reviewed: 25_activity_feed_design.md

Checklist

Authorization present: GetActivityFeedForListQuery declares AuthorizeTodoListAccessForCurrentUserQuery
via ConfigureDi. CreateActivityEventCommand is internal, never HTTP-mapped, and every call site is already
gated by that call site's own authorization (todo handlers via AuthorizeTodoListAccessForCurrentUserQuery,
rename/remove via AuthorizeTodoListOwnerAccessForCurrentUserQuery) — same accepted pattern as
CreateNotificationCommand.
Resource ownership verified: the query is scoped by TodoListId and requires current-user membership —
matches the established pattern; no bypass.
No sensitive data leaked: ActivityEventDto carries Kind, Body, ActorUserId, ActorDisplayName,
TodoNr, CreatedAt, Id only. No password hash/salt/session data. Exposing which member did what to other
members is the explicit intent of the story (AC: "All list members ... can view the feed") — not a leak.
Mass assignment safe: CreateActivityEventCommand is never constructed from client input; TodoListId
and ActorUserId are always server-derived in the calling handler, never taken verbatim from the request DTO
(e.g. ActorUserId for RemoveTodoListMemberCommandHandler is the acting owner's id from
GetCurrentUserIdQuery, not request.MemberId).
Input validation: Body is built server-side from already-validated Vogen value objects (TodoTitle,
TodoListTitle) — no new unconstrained string surface.

Findings

1. Stored-XSS risk if the frontend ever renders Body/ActorDisplayName via dangerouslySetInnerHTML.
TodoTitle allows arbitrary user text (no HTML-stripping), so a title like <img src=x onerror=...> ends up
verbatim inside Body. This is not a new risk introduced by this feature — TodoItem already displays the same
titles today — but it must be carried forward correctly here.
Requirement (not a blocker): ActivityEventItem.tsx must render Body and ActorDisplayName as plain React
children ({...}), never dangerouslySetInnerHTML. React's default escaping is sufficient. Flagging for the Frontend
Engineer and for final security review to confirm.

2. ActorUserId: ON DELETE SET NULL — reviewed and approved.
This is a correct, deliberate departure from NotificationEntity's cascade behavior (see design doc §2.1's
rationale). Approved: it avoids silently deleting other members' shared history when one past contributor deletes
their own account, and the SET NULL + generic fallback string means no lingering FK-joinable PII about the
deleted user remains.

3. Known/accepted limitation — freeform names baked into Body are not scrubbed on later account deletion.
E.g. removed Sam from the list keeps "Sam" as a literal string even if Sam later deletes their entire account.
This mirrors the existing, already-accepted behavior of NotificationEntity.Body (which already bakes in list
titles that aren't scrubbed either), and "GDPR compliance tooling" is explicitly listed as out of scope for this
WG-household app in 05_security_agent.md. Accepted, not a blocker — noted here for the record rather than
silently overlooked.

Verdict

Approved to proceed to implementation, with finding #1 carried forward as a concrete implementation
requirement to verify in the final security review (§ QA/Security final pass) rather than a design blocker.

**security** (`25_activity_feed_security.md`) # Security Pre-Review: Story `#25` — Activity Feed per List **Reviewer:** Security Agent **Date:** 2026-07-11 **Design reviewed:** `25_activity_feed_design.md` ## Checklist - [x] **Authorization present:** `GetActivityFeedForListQuery` declares `AuthorizeTodoListAccessForCurrentUserQuery` via `ConfigureDi`. `CreateActivityEventCommand` is `internal`, never HTTP-mapped, and every call site is already gated by that call site's own authorization (todo handlers via `AuthorizeTodoListAccessForCurrentUserQuery`, rename/remove via `AuthorizeTodoListOwnerAccessForCurrentUserQuery`) — same accepted pattern as `CreateNotificationCommand`. - [x] **Resource ownership verified:** the query is scoped by `TodoListId` and requires current-user membership — matches the established pattern; no bypass. - [x] **No sensitive data leaked:** `ActivityEventDto` carries `Kind`, `Body`, `ActorUserId`, `ActorDisplayName`, `TodoNr`, `CreatedAt`, `Id` only. No password hash/salt/session data. Exposing which member did what to other members is the explicit intent of the story (AC: "All list members ... can view the feed") — not a leak. - [x] **Mass assignment safe:** `CreateActivityEventCommand` is never constructed from client input; `TodoListId` and `ActorUserId` are always server-derived in the calling handler, never taken verbatim from the request DTO (e.g. `ActorUserId` for `RemoveTodoListMemberCommandHandler` is the *acting* owner's id from `GetCurrentUserIdQuery`, not `request.MemberId`). - [x] **Input validation:** `Body` is built server-side from already-validated Vogen value objects (`TodoTitle`, `TodoListTitle`) — no new unconstrained string surface. ## Findings **1. Stored-XSS risk if the frontend ever renders `Body`/`ActorDisplayName` via `dangerouslySetInnerHTML`.** `TodoTitle` allows arbitrary user text (no HTML-stripping), so a title like `<img src=x onerror=...>` ends up verbatim inside `Body`. This is not a new risk introduced by this feature — `TodoItem` already displays the same titles today — but it must be carried forward correctly here. **Requirement (not a blocker):** `ActivityEventItem.tsx` must render `Body` and `ActorDisplayName` as plain React children (`{...}`), never `dangerouslySetInnerHTML`. React's default escaping is sufficient. Flagging for the Frontend Engineer and for final security review to confirm. **2. `ActorUserId: ON DELETE SET NULL` — reviewed and approved.** This is a correct, deliberate departure from `NotificationEntity`'s cascade behavior (see design doc §2.1's rationale). Approved: it avoids silently deleting other members' shared history when one past contributor deletes their own account, and the `SET NULL` + generic fallback string means no lingering FK-joinable PII about the deleted user remains. **3. Known/accepted limitation — freeform names baked into `Body` are not scrubbed on later account deletion.** E.g. `removed Sam from the list` keeps "Sam" as a literal string even if Sam later deletes their entire account. This mirrors the existing, already-accepted behavior of `NotificationEntity.Body` (which already bakes in list titles that aren't scrubbed either), and "GDPR compliance tooling" is explicitly listed as out of scope for this WG-household app in `05_security_agent.md`. **Accepted, not a blocker** — noted here for the record rather than silently overlooked. ## Verdict **Approved to proceed to implementation**, with finding `#1` carried forward as a concrete implementation requirement to verify in the final security review (§ QA/Security final pass) rather than a design blocker.

lena commented

2026-08-18 13:12:55 +02:00

(Migrated from git.butzei.de)

design (25_activity_feed_design.md)

Design: Story `#25` — Activity Feed per List

Status: Ready for Security pre-review
Author: Software Architect
Date: 2026-07-11

1. Summary

A new ActivityEventEntity table records an append-only, list-scoped log of seven event kinds. Events are recorded
as a side effect inside the seven existing command handlers that cause them (todo create/complete/reopen/delete,
member joined/removed, list renamed) — there is no generic domain-event bus in this codebase (confirmed against
the Notification feature, #26), so each handler must inject and call a new internal CreateActivityEventCommand
the same way existing handlers inject CreateNotificationCommand.

The feed is read-only and pull-based: the frontend fetches a page when the "Activity" tab is opened and again
on "Load more". No WebSocket channel is added — the acceptance criteria don't require live updates (that's
explicitly out of scope, deferred to #26-style notifications), and adding one would be premature for a feature
whose only interaction is "open tab, scroll".

Trigger	Kind	Body template	TodoNr
Todo created	`TodoCreated`	`created '{title}'`	set
Todo completed	`TodoCompleted`	`completed '{title}'`	set
Todo reopened	`TodoReopened`	`reopened '{title}'`	set
Todo deleted	`TodoDeleted`	`deleted '{title}'`	null (todo no longer exists)
Member joins list	`MemberJoined`	`joined the list`	null
Member removed	`MemberRemoved`	`removed {removedMemberName} from the list`	null
List renamed	`ListRenamed`	`renamed the list from '{old}' to '{new}'`	null

Body never embeds the actor's own name — the frontend prepends the actor's display name as a distinct UI
element (per AC: "shows the acting user's name" as its own thing, so it can be styled/linked separately from the
action text), e.g. {ActorDisplayName} {Body}.

2. DB Schema Changes

2.1 New table: `ActivityEventEntity`

CREATE TABLE "ActivityEventEntity" (
    "Id"          integer GENERATED ALWAYS AS IDENTITY PRIMARY KEY,
    "TodoListId"  integer NOT NULL REFERENCES "TodoListEntity"("Id") ON DELETE CASCADE,
    "ActorUserId" integer NULL REFERENCES "UserEntity"("Id") ON DELETE SET NULL,
    "Kind"        text    NOT NULL,
    "Body"        text    NOT NULL,
    "TodoNr"      integer NULL,
    "CreatedAt"   timestamptz NOT NULL DEFAULT now()
);

CREATE INDEX "IX_ActivityEventEntity_TodoListId_CreatedAt"
    ON "ActivityEventEntity" ("TodoListId", "CreatedAt" DESC);

TodoListId: ON DELETE CASCADE — same as NotificationEntity. A deleted list's history is meaningless.

ActorUserId: ON DELETE SET NULL — deliberately not cascade, unlike NotificationEntity.RecipientId.
This is the one place this design departs from the #26 precedent, and it's worth spelling out why: a
NotificationEntity row is private to its single recipient, so cascading it away when that user deletes their
account is correct — the data has no other viewer. An ActivityEventEntity row is shared, multi-user history.
If Alice created ten todos over a list's lifetime and later deletes her account (#10, GDPR deletion), cascading on
ActorUserId would silently erase those ten rows from every other member's feed — the shared history changes
underneath the remaining members for a reason that has nothing to do with the list. SET NULL keeps the row; the
query layer (§5.1) falls back to a fixed string ("A former member") when ActorUserId is null. No other data
identifies who Alice was once her account is gone, which is the intended GDPR effect.

The removed/target member's name in MemberRemoved (and any other name baked into Body) is a plain string
literal written at event-creation time, not a live join — so it always reads correctly regardless of what happens
to that user's account afterward, matching how NotificationEntity.Body already works.

No stored ActorDisplayName column. Unlike this paragraph's first draft assumption, NotificationDto's
TodoListTitle is not a denormalized column on NotificationEntity — it's a live EF join
(x.TodoList.Title) done in the query projection. ActorDisplayName follows the same convention: joined from
ActorUser.Name at read time, with the SET NULL fallback above.

2.2 EF Core entity

// CqsTodo/Entities/ActivityEventEntity.cs
namespace CqsTodo.Entities;

public class ActivityEventEntity : IEntity<ActivityEventEntity>
{
    public ActivityEventId Id { get; set; } = ActivityEventId.From(0);
    public TodoListId TodoListId { get; set; }
    public TodoListEntity TodoList { get; set; } = null!;
    public UserId? ActorUserId { get; set; }
    public UserEntity? ActorUser { get; set; }
    public ActivityEventKind Kind { get; set; }
    public string Body { get; set; } = string.Empty;
    public TodoNr? TodoNr { get; set; }
    public DateTimeOffset CreatedAt { get; set; }

    public void Configure(EntityTypeBuilder<ActivityEventEntity> builder)
    {
        builder.HasKey(x => x.Id);
        builder.Property(x => x.Id).HasIdentityVogenColumn();
        builder.HasOne(x => x.TodoList)
               .WithMany()
               .HasForeignKey(x => x.TodoListId)
               .OnDelete(DeleteBehavior.Cascade);
        builder.HasOne(x => x.ActorUser)
               .WithMany()
               .HasForeignKey(x => x.ActorUserId)
               .OnDelete(DeleteBehavior.SetNull);
        builder.Property(x => x.Kind).HasConversion<string>();
        builder.Property(x => x.Body).HasMaxLength(500);
        builder.HasIndex(x => new { x.TodoListId, x.CreatedAt });
    }
}

2.3 New enum

// CqsTodo/Entities/ActivityEventKind.cs
namespace CqsTodo.Entities;

public enum ActivityEventKind
{
    TodoCreated,
    TodoCompleted,
    TodoReopened,
    TodoDeleted,
    MemberJoined,
    MemberRemoved,
    ListRenamed,
}

2.4 New Vogen value object

// Common/Types/ActivityEventId.cs
using Vogen;

namespace Common.Types;

[ValueObject<int>]
public partial struct ActivityEventId : IValueObject<ActivityEventId, int>
{
    public static Validation Validate(int value)
        => value >= 0 ? Validation.Ok : Validation.Invalid("Must be 0 or positive value.");
}

2.5 Migration name

AddActivityEventEntity

3. DTO

// Common/Dtos/ActivityEventDto.cs
public record ActivityEventDto(
    ActivityEventId Id,
    ActivityEventKind Kind,
    string Body,
    UserId? ActorUserId,
    string ActorDisplayName,
    TodoNr? TodoNr,
    DateTimeOffset CreatedAt);

public record ActivityFeedPageDto(
    IReadOnlyCollection<ActivityEventDto> Events,
    bool HasMore);

4. Internal command: `CreateActivityEventCommand`

Not exposed as an API endpoint (same convention as CreateNotificationCommand — internal record, MapRequests
only maps public types, no ConfigureDi/authorization since callers already enforce it).

// CqsTodo/Features/Activity/CreateActivityEventCommandHandler.cs
internal record CreateActivityEventCommand(
    TodoListId TodoListId,
    UserId ActorUserId,
    ActivityEventKind Kind,
    string Body,
    TodoNr? TodoNr = null)
    : IRequest<ActivityEventDto>;

Handler logic (mirrors CreateNotificationCommandHandler):

Insert ActivityEventEntity with CreatedAt = DateTimeOffset.UtcNow.
Re-read via a fresh DbContext, left-joining ActorUser.Name (fallback "A former member" if null), project
to ActivityEventDto.
Return the DTO. No subject/WS publish (feed is pull-based, see §1).

4.1 Hook points — seven existing handlers each gain one injected dependency

Each handler injects IHandler<CreateActivityEventCommand, ActivityEventDto> createActivityEventHandler (and
IHandler<GetCurrentUserIdQuery, UserId?> where not already present) and calls it after the primary write
succeeds, exactly where CreateNotificationCommand is called in AssignTodoCommandHandler /
AcceptListInvitationCommandHandler today:

Handler	Actor	Notes
`CreateTodoCommandHandler`	current user	`TodoNr = entity.Nr`
`CheckTodoCommandHandler`	current user	`TodoNr = request.TodoNr`
`UncheckTodoCommandHandler`	current user	`TodoNr = request.TodoNr`
`DeleteTodoCommandHandler`	current user	capture `entity.Title.Value` before `dbContext.Remove(entity)` — the row is gone after
`AcceptListInvitationCommandHandler`	joining user (`userId`)	only inside the existing `!isAlreadyMember` branch
`RemoveTodoListMemberCommandHandler`	current user (the owner performing removal)	capture `membership.User.Name` (requires `.Include(x => x.User)` or a small projection) before `dbContext.Remove(membership)`
`RenameTodoListCommandHandler`	current user	must read the old title before `ExecuteUpdateAsync` — the handler currently goes straight to the update with no prior read; add `var oldTitle = await dbContext.Set<TodoListEntity>().Where(x => x.Id == request.Id).Select(x => x.Title).SingleOrDefaultAsync(...)`, throw `EntityNotFoundException` if null (replaces the existing `rows == 0` check)

CheckTodoCommandHandler / UncheckTodoCommandHandler don't currently inject GetCurrentUserIdQuery — add it.
RemoveTodoListMemberCommandHandler / RenameTodoListCommandHandler likewise need GetCurrentUserIdQuery added.

5. Query: `GetActivityFeedForListQuery`

// CqsTodo/Features/Activity/GetActivityFeedForListQueryHandler.cs
public record GetActivityFeedForListQuery(
    TodoListId TodoListId,
    int Skip = 0,
    int Take = 20)
    : IRequest<ActivityFeedPageDto>;

5.1 Handler logic

Clamp Take to [1, 100], clamp Skip to >= 0 (DoS guard, same rationale as Notification's Limit clamp).
Query ordered CreatedAt DESC, Skip(skip).Take(take + 1) to detect whether more rows exist.

Project to ActivityEventDto:

.Select(x => new ActivityEventDto(
    x.Id,
    x.Kind,
    x.Body,
    x.ActorUserId,
    x.ActorUser != null ? x.ActorUser.Name.Value : "A former member",
    x.TodoNr,
    x.CreatedAt))

HasMore = rows.Count > take; return only the first take rows plus HasMore.

Authorization:

public static void ConfigureDi(IHandlerRegistrationConfig<GetActivityFeedForListQuery> config)
    => config.WithAuthorization(x => new AuthorizeTodoListAccessForCurrentUserQuery(x.TodoListId));

Any list member (owner or regular member) passes this check — matches AC "All list members can view the feed."

6. Frontend

New ActivityFeedPanel.tsx component, rendered behind an "Activity" tab alongside the existing todo list view
(reuses the Tabs UI primitive already in src/components/ui/tabs.tsx, used elsewhere e.g. SettingsModal).
New ActivityEventItem.tsx: renders {ActorDisplayName} {Body} + relative time (reuse
src/lib/relativeTime.ts, already used by NotificationItem.tsx) with a title attribute (native browser
tooltip) holding the absolute timestamp — same pattern as NotificationItem.
Fetch via a new getActivityFeed(listId, skip, take) call in src/api/api.tsx, mapped to
POST api/GetActivityFeedForListQuery per the MapRequests convention (all queries are POST).
Pagination: "Load more" button appends the next page to local state; disabled/hidden when HasMore === false.
No infinite-scroll observer for this iteration — a button is simpler and satisfies the AC ("via a 'Load more'
button" is explicitly listed as an acceptable option).
No store/WebSocket wiring needed — pull-based per §1.

7. Out of scope (per story)

Filtering by event type/user, editing/deleting events, real-time push — all explicitly excluded in the story. Not
revisited here.

**design** (`25_activity_feed_design.md`) # Design: Story `#25` — Activity Feed per List **Status:** Ready for Security pre-review **Author:** Software Architect **Date:** 2026-07-11 --- ## 1. Summary A new `ActivityEventEntity` table records an append-only, list-scoped log of seven event kinds. Events are recorded as a side effect inside the seven existing command handlers that cause them (todo create/complete/reopen/delete, member joined/removed, list renamed) — there is no generic domain-event bus in this codebase (confirmed against the Notification feature, `#26`), so each handler must inject and call a new internal `CreateActivityEventCommand` the same way existing handlers inject `CreateNotificationCommand`. The feed is **read-only and pull-based**: the frontend fetches a page when the "Activity" tab is opened and again on "Load more". No WebSocket channel is added — the acceptance criteria don't require live updates (that's explicitly out of scope, deferred to `#26`-style notifications), and adding one would be premature for a feature whose only interaction is "open tab, scroll". | Trigger | Kind | Body template | TodoNr | |---|---|---|---| | Todo created | `TodoCreated` | `created '{title}'` | set | | Todo completed | `TodoCompleted` | `completed '{title}'` | set | | Todo reopened | `TodoReopened` | `reopened '{title}'` | set | | Todo deleted | `TodoDeleted` | `deleted '{title}'` | null (todo no longer exists) | | Member joins list | `MemberJoined` | `joined the list` | null | | Member removed | `MemberRemoved` | `removed {removedMemberName} from the list` | null | | List renamed | `ListRenamed` | `renamed the list from '{old}' to '{new}'` | null | `Body` never embeds the actor's own name — the frontend prepends the actor's display name as a distinct UI element (per AC: "shows the acting user's name" as its own thing, so it can be styled/linked separately from the action text), e.g. `{ActorDisplayName} {Body}`. --- ## 2. DB Schema Changes ### 2.1 New table: `ActivityEventEntity` ```sql CREATE TABLE "ActivityEventEntity" ( "Id" integer GENERATED ALWAYS AS IDENTITY PRIMARY KEY, "TodoListId" integer NOT NULL REFERENCES "TodoListEntity"("Id") ON DELETE CASCADE, "ActorUserId" integer NULL REFERENCES "UserEntity"("Id") ON DELETE SET NULL, "Kind" text NOT NULL, "Body" text NOT NULL, "TodoNr" integer NULL, "CreatedAt" timestamptz NOT NULL DEFAULT now() ); CREATE INDEX "IX_ActivityEventEntity_TodoListId_CreatedAt" ON "ActivityEventEntity" ("TodoListId", "CreatedAt" DESC); ``` **`TodoListId`: `ON DELETE CASCADE`** — same as `NotificationEntity`. A deleted list's history is meaningless. **`ActorUserId`: `ON DELETE SET NULL` — deliberately *not* cascade, unlike `NotificationEntity.RecipientId`.** This is the one place this design departs from the `#26` precedent, and it's worth spelling out why: a `NotificationEntity` row is private to its single recipient, so cascading it away when that user deletes their account is correct — the data has no other viewer. An `ActivityEventEntity` row is **shared, multi-user history**. If Alice created ten todos over a list's lifetime and later deletes her account (`#10`, GDPR deletion), cascading on `ActorUserId` would silently erase those ten rows from every other member's feed — the shared history changes underneath the remaining members for a reason that has nothing to do with the list. `SET NULL` keeps the row; the query layer (§5.1) falls back to a fixed string (`"A former member"`) when `ActorUserId` is null. No other data identifies who Alice was once her account is gone, which is the intended GDPR effect. The removed/target member's name in `MemberRemoved` (and any other name baked into `Body`) is a plain string literal written at event-creation time, not a live join — so it always reads correctly regardless of what happens to that user's account afterward, matching how `NotificationEntity.Body` already works. **No stored `ActorDisplayName` column.** Unlike this paragraph's first draft assumption, `NotificationDto`'s `TodoListTitle` is *not* a denormalized column on `NotificationEntity` — it's a live EF join (`x.TodoList.Title`) done in the query projection. `ActorDisplayName` follows the same convention: joined from `ActorUser.Name` at read time, with the `SET NULL` fallback above. ### 2.2 EF Core entity ```csharp // CqsTodo/Entities/ActivityEventEntity.cs namespace CqsTodo.Entities; public class ActivityEventEntity : IEntity<ActivityEventEntity> { public ActivityEventId Id { get; set; } = ActivityEventId.From(0); public TodoListId TodoListId { get; set; } public TodoListEntity TodoList { get; set; } = null!; public UserId? ActorUserId { get; set; } public UserEntity? ActorUser { get; set; } public ActivityEventKind Kind { get; set; } public string Body { get; set; } = string.Empty; public TodoNr? TodoNr { get; set; } public DateTimeOffset CreatedAt { get; set; } public void Configure(EntityTypeBuilder<ActivityEventEntity> builder) { builder.HasKey(x => x.Id); builder.Property(x => x.Id).HasIdentityVogenColumn(); builder.HasOne(x => x.TodoList) .WithMany() .HasForeignKey(x => x.TodoListId) .OnDelete(DeleteBehavior.Cascade); builder.HasOne(x => x.ActorUser) .WithMany() .HasForeignKey(x => x.ActorUserId) .OnDelete(DeleteBehavior.SetNull); builder.Property(x => x.Kind).HasConversion<string>(); builder.Property(x => x.Body).HasMaxLength(500); builder.HasIndex(x => new { x.TodoListId, x.CreatedAt }); } } ``` ### 2.3 New enum ```csharp // CqsTodo/Entities/ActivityEventKind.cs namespace CqsTodo.Entities; public enum ActivityEventKind { TodoCreated, TodoCompleted, TodoReopened, TodoDeleted, MemberJoined, MemberRemoved, ListRenamed, } ``` ### 2.4 New Vogen value object ```csharp // Common/Types/ActivityEventId.cs using Vogen; namespace Common.Types; [ValueObject<int>] public partial struct ActivityEventId : IValueObject<ActivityEventId, int> { public static Validation Validate(int value) => value >= 0 ? Validation.Ok : Validation.Invalid("Must be 0 or positive value."); } ``` Register in `CqsTodo/DbContext/VogenEfCoreConverters.cs`: `[EfCoreConverter<ActivityEventId>]`. ### 2.5 Migration name `AddActivityEventEntity` --- ## 3. DTO ```csharp // Common/Dtos/ActivityEventDto.cs public record ActivityEventDto( ActivityEventId Id, ActivityEventKind Kind, string Body, UserId? ActorUserId, string ActorDisplayName, TodoNr? TodoNr, DateTimeOffset CreatedAt); public record ActivityFeedPageDto( IReadOnlyCollection<ActivityEventDto> Events, bool HasMore); ``` --- ## 4. Internal command: `CreateActivityEventCommand` Not exposed as an API endpoint (same convention as `CreateNotificationCommand` — `internal` record, `MapRequests` only maps `public` types, no `ConfigureDi`/authorization since callers already enforce it). ```csharp // CqsTodo/Features/Activity/CreateActivityEventCommandHandler.cs internal record CreateActivityEventCommand( TodoListId TodoListId, UserId ActorUserId, ActivityEventKind Kind, string Body, TodoNr? TodoNr = null) : IRequest<ActivityEventDto>; ``` **Handler logic** (mirrors `CreateNotificationCommandHandler`): 1. Insert `ActivityEventEntity` with `CreatedAt = DateTimeOffset.UtcNow`. 2. Re-read via a fresh `DbContext`, left-joining `ActorUser.Name` (fallback `"A former member"` if null), project to `ActivityEventDto`. 3. Return the DTO. No subject/WS publish (feed is pull-based, see §1). ### 4.1 Hook points — seven existing handlers each gain one injected dependency Each handler injects `IHandler<CreateActivityEventCommand, ActivityEventDto> createActivityEventHandler` (and `IHandler<GetCurrentUserIdQuery, UserId?>` where not already present) and calls it after the primary write succeeds, exactly where `CreateNotificationCommand` is called in `AssignTodoCommandHandler` / `AcceptListInvitationCommandHandler` today: | Handler | Actor | Notes | |---|---|---| | `CreateTodoCommandHandler` | current user | `TodoNr = entity.Nr` | | `CheckTodoCommandHandler` | current user | `TodoNr = request.TodoNr` | | `UncheckTodoCommandHandler` | current user | `TodoNr = request.TodoNr` | | `DeleteTodoCommandHandler` | current user | capture `entity.Title.Value` **before** `dbContext.Remove(entity)` — the row is gone after | | `AcceptListInvitationCommandHandler` | joining user (`userId`) | only inside the existing `!isAlreadyMember` branch | | `RemoveTodoListMemberCommandHandler` | current user (the owner performing removal) | capture `membership.User.Name` (requires `.Include(x => x.User)` or a small projection) **before** `dbContext.Remove(membership)` | | `RenameTodoListCommandHandler` | current user | must read the **old** title before `ExecuteUpdateAsync` — the handler currently goes straight to the update with no prior read; add `var oldTitle = await dbContext.Set<TodoListEntity>().Where(x => x.Id == request.Id).Select(x => x.Title).SingleOrDefaultAsync(...)`, throw `EntityNotFoundException` if null (replaces the existing `rows == 0` check) | `CheckTodoCommandHandler` / `UncheckTodoCommandHandler` don't currently inject `GetCurrentUserIdQuery` — add it. `RemoveTodoListMemberCommandHandler` / `RenameTodoListCommandHandler` likewise need `GetCurrentUserIdQuery` added. --- ## 5. Query: `GetActivityFeedForListQuery` ```csharp // CqsTodo/Features/Activity/GetActivityFeedForListQueryHandler.cs public record GetActivityFeedForListQuery( TodoListId TodoListId, int Skip = 0, int Take = 20) : IRequest<ActivityFeedPageDto>; ``` ### 5.1 Handler logic 1. Clamp `Take` to `[1, 100]`, clamp `Skip` to `>= 0` (DoS guard, same rationale as Notification's `Limit` clamp). 2. Query ordered `CreatedAt DESC`, `Skip(skip).Take(take + 1)` to detect whether more rows exist. 3. Project to `ActivityEventDto`: ```csharp .Select(x => new ActivityEventDto( x.Id, x.Kind, x.Body, x.ActorUserId, x.ActorUser != null ? x.ActorUser.Name.Value : "A former member", x.TodoNr, x.CreatedAt)) ``` 4. `HasMore = rows.Count > take`; return only the first `take` rows plus `HasMore`. **Authorization:** ```csharp public static void ConfigureDi(IHandlerRegistrationConfig<GetActivityFeedForListQuery> config) => config.WithAuthorization(x => new AuthorizeTodoListAccessForCurrentUserQuery(x.TodoListId)); ``` Any list member (owner or regular member) passes this check — matches AC "All list members can view the feed." --- ## 6. Frontend - New `ActivityFeedPanel.tsx` component, rendered behind an "Activity" tab alongside the existing todo list view (reuses the `Tabs` UI primitive already in `src/components/ui/tabs.tsx`, used elsewhere e.g. `SettingsModal`). - New `ActivityEventItem.tsx`: renders `{ActorDisplayName} {Body}` + relative time (reuse `src/lib/relativeTime.ts`, already used by `NotificationItem.tsx`) with a `title` attribute (native browser tooltip) holding the absolute timestamp — same pattern as `NotificationItem`. - Fetch via a new `getActivityFeed(listId, skip, take)` call in `src/api/api.tsx`, mapped to `POST api/GetActivityFeedForListQuery` per the `MapRequests` convention (all queries are POST). - Pagination: "Load more" button appends the next page to local state; disabled/hidden when `HasMore === false`. No infinite-scroll observer for this iteration — a button is simpler and satisfies the AC ("via a 'Load more' button" is explicitly listed as an acceptable option). - No store/WebSocket wiring needed — pull-based per §1. --- ## 7. Out of scope (per story) Filtering by event type/user, editing/deleting events, real-time push — all explicitly excluded in the story. Not revisited here.

lena commented

2026-08-18 13:12:55 +02:00

(Migrated from git.butzei.de)

security_final (25_activity_feed_security_final.md)

Security Final Review: Story `#25` — Activity Feed per List

Reviewer: Security Agent
Date: 2026-07-11
Reviewed against: 061f084 (backend), 4c67108 (backend tests), aba1617 (frontend)

Verification of the pre-review requirement

Finding #1 from pre-review (stored-XSS via todo titles baked into Body) — verified fixed.
ActivityEventItem.tsx renders event.actorDisplayName and event.body as plain JSX children
({event.actorDisplayName} / {event.body}); no dangerouslySetInnerHTML anywhere in the new components
(confirmed by grep). Added ActivityEventItem.test.tsx test
renders todo titles containing HTML-like text safely as text, not markup asserts a body containing
<img src=x onerror=...> renders as literal text and no <img> element is created in the DOM.

Re-run of the backend checklist against the actual implementation

Authorization: GetActivityFeedForListQueryHandler.ConfigureDi declares
AuthorizeTodoListAccessForCurrentUserQuery(x.TodoListId) — any list member can read, matching the AC.
CreateActivityEventCommand stays internal/unmapped; every call site is already behind its own
authorization (AuthorizeTodoListAccessForCurrentUserQuery for the four todo hooks and member-joined,
AuthorizeTodoListOwnerAccessForCurrentUserQuery for rename/member-removal).
Mass assignment: confirmed in the actual diff — RemoveTodoListMemberCommandHandler passes
currentUserId!.Value (the authenticated owner) as the actor, never request.MemberId (the removed member).
Same pattern holds in all seven hook sites.
No sensitive data leaked: ActivityEventDto unchanged from design — no password/session fields.
FK behavior: migration 20260711105003_AddActivityEventEntity.cs creates
FK_ActivityEventEntity_UserEntity_ActorUserId with onDelete: ReferentialAction.SetNull, matching the
design decision and the pre-existing note left in NotificationEntity.cs anticipating exactly this
("Any future ActorId FK must NOT use ON DELETE CASCADE"). Verified with a DB-level test
(Activity_event_survives_with_null_actor_when_actor_account_is_deleted) that actually deletes the
UserEntity row and asserts the event row survives with ActorUserId == null.
Input validation: GetActivityFeedForListQueryHandler throws ArgumentException for Take <= 0,
Take > 100, or Skip < 0 — matches the GetNotificationsForCurrentUserQuery precedent exactly (tests
Throws_ArgumentException_when_Take_is_out_of_range / ..._Skip_is_negative).

New observation (informational, not a blocker)

DeleteTodoCommandHandler now records the todo's title in Body after the row has already been removed
from the DbContext's change tracker (title captured into a local string before dbContext.Remove(entity)).
This is correct and intentional — confirmed the title snapshot happens before deletion, and the corresponding
test (Creates_activity_event_with_title_snapshot_after_todo_is_gone) exercises this. No action needed.

Verdict

Approved. The one concrete requirement carried over from the design review is verified fixed with a
regression test. No new findings block this feature from moving to done/.

**security_final** (`25_activity_feed_security_final.md`) # Security Final Review: Story `#25` — Activity Feed per List **Reviewer:** Security Agent **Date:** 2026-07-11 **Reviewed against:** `061f084` (backend), `4c67108` (backend tests), `aba1617` (frontend) ## Verification of the pre-review requirement **Finding `#1` from pre-review (stored-XSS via todo titles baked into `Body`)** — verified fixed. `ActivityEventItem.tsx` renders `event.actorDisplayName` and `event.body` as plain JSX children (`{event.actorDisplayName}` / `{event.body}`); no `dangerouslySetInnerHTML` anywhere in the new components (confirmed by grep). Added `ActivityEventItem.test.tsx` test `renders todo titles containing HTML-like text safely as text, not markup` asserts a body containing `<img src=x onerror=...>` renders as literal text and no `<img>` element is created in the DOM. ## Re-run of the backend checklist against the actual implementation - [x] **Authorization:** `GetActivityFeedForListQueryHandler.ConfigureDi` declares `AuthorizeTodoListAccessForCurrentUserQuery(x.TodoListId)` — any list member can read, matching the AC. `CreateActivityEventCommand` stays `internal`/unmapped; every call site is already behind its own authorization (`AuthorizeTodoListAccessForCurrentUserQuery` for the four todo hooks and member-joined, `AuthorizeTodoListOwnerAccessForCurrentUserQuery` for rename/member-removal). - [x] **Mass assignment:** confirmed in the actual diff — `RemoveTodoListMemberCommandHandler` passes `currentUserId!.Value` (the authenticated owner) as the actor, never `request.MemberId` (the removed member). Same pattern holds in all seven hook sites. - [x] **No sensitive data leaked:** `ActivityEventDto` unchanged from design — no password/session fields. - [x] **FK behavior:** migration `20260711105003_AddActivityEventEntity.cs` creates `FK_ActivityEventEntity_UserEntity_ActorUserId` with `onDelete: ReferentialAction.SetNull`, matching the design decision and the pre-existing note left in `NotificationEntity.cs` anticipating exactly this ("Any future ActorId FK must NOT use ON DELETE CASCADE"). Verified with a DB-level test (`Activity_event_survives_with_null_actor_when_actor_account_is_deleted`) that actually deletes the `UserEntity` row and asserts the event row survives with `ActorUserId == null`. - [x] **Input validation:** `GetActivityFeedForListQueryHandler` throws `ArgumentException` for `Take <= 0`, `Take > 100`, or `Skip < 0` — matches the `GetNotificationsForCurrentUserQuery` precedent exactly (tests `Throws_ArgumentException_when_Take_is_out_of_range` / `..._Skip_is_negative`). ## New observation (informational, not a blocker) `DeleteTodoCommandHandler` now records the todo's title in `Body` *after* the row has already been removed from the `DbContext`'s change tracker (title captured into a local `string` before `dbContext.Remove(entity)`). This is correct and intentional — confirmed the title snapshot happens before deletion, and the corresponding test (`Creates_activity_event_with_title_snapshot_after_todo_is_gone`) exercises this. No action needed. ## Verdict **Approved.** The one concrete requirement carried over from the design review is verified fixed with a regression test. No new findings block this feature from moving to `done/`.