Retention for finished messages and completed cards, plus overview pages #191

Merged

robbertbos merged 1 commit from retention-and-overview-pages into main 2026-07-26 20:41:46 +00:00

Conversation 0 Commits 1 Files changed 77 +4174 -265

robbertbos commented 2026-07-25 21:38:38 +00:00

Owner

Copy link

Waggle kept everything forever. This adds automatic cleanup, under one rule:

Waggle never deletes what you added yourself or have not finished.

What is cleaned up, and what never is

Two per-user settings, each off / 30 / 90 / 180 / 365 days, defaulting to 90 for new accounts.

Cleaned up	Kept, always
Sent, cancelled and failed outgoing messages, with their attachments	Drafts
Completed and read cards	Cards carrying a note

A completed card is also spared while it still has a live outgoing message attached, or while it still owes Mattermost an unsave (cleanup_status pending/running/failed). That last guard matters: deleting such a card would leave the message flagged "saved" in Mattermost with nothing left in Waggle to un-flag it.

What disappears is Waggle's own copy. The Mattermost messages stay where they are. Only a message that was never sent is genuinely lost, and the settings copy says so.

Accounts predating the migration start switched off. Otherwise the first nightly sweep would have deleted years of archive before anyone had seen the setting. Those users get a one-time banner offering to switch it on at 90 days.

Three pages for what is kept

Reachable from the menu, rendered with the app's own card components rather than a bespoke list:

• Concepten (/concept-berichten) - a draft lived in no list at all before this, because both scheduled-post endpoints filter drafts out. A half-written reply was unreachable without opening the exact card it hung on.
• Uitgaande post (/uitgaande-post) - everything outgoing in three sections, failures first. Outgoing messages previously lived as sections inside the card lists, and sent messages were visible nowhere.
• Notities (/notities) - the cards that are never cleaned up.

Two defects that were not in the issue

Both found during review, both closed, each pinned by a test that fails against the bug:

1. Cancelled and failed rows shielded their attachments from the 7-day orphan sweep indefinitely. That sweep built its referenced set from select(OutgoingPost.files) with no state filter, so a dead row kept its attachment alive. Nothing cleaned up on cancel or on a failed send; only the manual "Gelezen, verberg" button did.
2. Deleting a completed card took its drafts with it through the FK cascade, including their attachment bytes, because the ref query had no state filter either. Reachable in practice: you can write a reply on a card you completed months ago.

Attachment bytes are released only after the deleting transaction commits, and only through _delete_orphan_refs, because a ref can legitimately be held by a live draft - editing a scheduled post cancels the row and re-hangs its files on the new draft.

Also here

The terms are re-readable on /voorwaarden, with the date they were accepted, from a single shared component so the retention sentence cannot drift between the two places. /welkom drops its connect-Mattermost step: a server is linked from Instellingen, and a new user without one gets a banner pointing there.

Accepted trade-offs

• A user can switch cleanup off entirely, so an operator cannot treat storage limitation as a system guarantee. Stated plainly in docs/deployment.md rather than left implicit.
• Existing accounts get no storage limitation until they act. The alternative was an archive deleted on deploy night.
• No warning before a card is removed. The user ticked the card off themselves, set the period themselves, and everything they added is spared regardless.

Issue #146

Closed with a different solution than it asked for. The issue wanted a maximum age on draft attachments; drafts are kept instead, and answered with a findable list. docs/deployment.md is updated accordingly.

Tested

Backend 1610 tests at 100% branch coverage, on SQLite and Postgres. Frontend 570 unit tests, 63 e2e. The migration's data step - the line that decides whether a deploy destroys data - has its own tests, because alembic/versions/ sits outside the coverage gate.

Separately filed: #205, our universal CSS reset strips the padding from NLDD components that style their host, and MinBZK/storybook#160 as the upstream hardening suggestion.

Waggle kept everything forever. This adds automatic cleanup, under one rule: > **Waggle never deletes what you added yourself or have not finished.** ## What is cleaned up, and what never is Two per-user settings, each off / 30 / 90 / 180 / 365 days, defaulting to 90 for new accounts. | Cleaned up | Kept, always | | --- | --- | | Sent, cancelled and failed outgoing messages, with their attachments | Drafts | | Completed and read cards | Cards carrying a note | A completed card is also spared while it still has a live outgoing message attached, or while it still owes Mattermost an unsave (`cleanup_status` pending/running/failed). That last guard matters: deleting such a card would leave the message flagged "saved" in Mattermost with nothing left in Waggle to un-flag it. What disappears is Waggle's own copy. The Mattermost messages stay where they are. Only a message that was never sent is genuinely lost, and the settings copy says so. **Accounts predating the migration start switched off.** Otherwise the first nightly sweep would have deleted years of archive before anyone had seen the setting. Those users get a one-time banner offering to switch it on at 90 days. ## Three pages for what is kept Reachable from the menu, rendered with the app's own card components rather than a bespoke list: - **Concepten** (`/concept-berichten`) - a draft lived in no list at all before this, because both scheduled-post endpoints filter drafts out. A half-written reply was unreachable without opening the exact card it hung on. - **Uitgaande post** (`/uitgaande-post`) - everything outgoing in three sections, failures first. Outgoing messages previously lived as sections inside the card lists, and sent messages were visible nowhere. - **Notities** (`/notities`) - the cards that are never cleaned up. ## Two defects that were not in the issue Both found during review, both closed, each pinned by a test that fails against the bug: 1. **Cancelled and failed rows shielded their attachments from the 7-day orphan sweep indefinitely.** That sweep built its `referenced` set from `select(OutgoingPost.files)` with no state filter, so a dead row kept its attachment alive. Nothing cleaned up on cancel or on a failed send; only the manual "Gelezen, verberg" button did. 2. **Deleting a completed card took its drafts with it** through the FK cascade, including their attachment bytes, because the ref query had no state filter either. Reachable in practice: you can write a reply on a card you completed months ago. Attachment bytes are released only after the deleting transaction commits, and only through `_delete_orphan_refs`, because a ref can legitimately be held by a live draft - editing a scheduled post cancels the row and re-hangs its files on the new draft. ## Also here The terms are re-readable on `/voorwaarden`, with the date they were accepted, from a single shared component so the retention sentence cannot drift between the two places. `/welkom` drops its connect-Mattermost step: a server is linked from Instellingen, and a new user without one gets a banner pointing there. ## Accepted trade-offs - A user can switch cleanup off entirely, so an operator cannot treat storage limitation as a system guarantee. Stated plainly in `docs/deployment.md` rather than left implicit. - Existing accounts get no storage limitation until they act. The alternative was an archive deleted on deploy night. - No warning before a card is removed. The user ticked the card off themselves, set the period themselves, and everything they added is spared regardless. ## Issue #146 Closed with a different solution than it asked for. The issue wanted a maximum age on draft attachments; drafts are kept instead, and answered with a findable list. `docs/deployment.md` is updated accordingly. ## Tested Backend 1610 tests at 100% branch coverage, on SQLite and Postgres. Frontend 570 unit tests, 63 e2e. The migration's data step - the line that decides whether a deploy destroys data - has its own tests, because `alembic/versions/` sits outside the coverage gate. Separately filed: [#205](https://code.overheid.nl/robbertbos/waggle/issues/205), our universal CSS reset strips the padding from NLDD components that style their host, and [MinBZK/storybook#160](https://github.com/MinBZK/storybook/issues/160) as the upstream hardening suggestion.

robbertbos added 22 commits 2026-07-25 21:38:38 +00:00

Document NLDD components added between 0.8.58 and 0.8.70 ... 2ef3fc8ccb

nldd-banner is a real alert component with title/body/dismiss/actions slot;
nldd-list + nldd-list-item ship a built-in empty state and link rows; no new
plain select landed. Also records the four icon names later tasks need.

Add per-user retention settings columns ... 1fc385f5d4

NULL means the category is not cleaned up. New rows default to 90 days;
the migration switches existing accounts off so a deploy never deletes
anything the user has not opted into.

Fix retention migration to use dialect-neutral SQL, tighten helper type ... 965c719ee0

The Core-selectable UPDATE compiled to bound parameters, which breaks
alembic upgrade head --sql (offline mode has no literal_binds). Replace
it with plain SQL text that works identically online and offline.

Also tighten the _retention test helper's return type to match the
surrounding style in test_migrations.py.

Expose retention settings over the settings API ... 69a735fb32

A KEEP sentinel separates 'field not supplied' from 'set to null', which
for these two columns means 'do not clean up' rather than 'unchanged'.

Cover retention_finished_posts_days route branch, clean up test decorators ... 301ea961c0

Add the missing integration test for the "supplied vs null" route branch
on retention_finished_posts_days - previously only exercised at the
service layer, so a regression in the route's model_fields_set check
would not have failed any test. Also drop four redundant
@pytest.mark.asyncio decorators now that asyncio_mode is auto.

Add retention queries for finished posts and completed cards ... 0a51433d72

Drafts and cards carrying a note are never purged, and a card that still
owes Mattermost an unsave is skipped so the obligation is not dropped.

Never purge a completed card that still carries a live outgoing post ... 9cdc8febe3

The FK on outgoing_posts.card_id is ondelete=CASCADE, so deleting a
completed card removed every outgoing post on it - a draft the user was
still writing included, plus any scheduled or sending row. This
contradicts the rule the retention design is built on: a draft is never
deleted, at any age.

Byte loss followed the row loss. The refs query had no state filter, so
the draft's attachment refs were returned as "freed"; the caller then
asked _delete_orphan_refs whether a live row still held them, found none
(that row had just cascaded away) and deleted the files off disk. It is
reachable: upsert_draft has no completed_at guard, so a user can open a
card completed months ago and attach a file to a reply.

Two layers. The candidate query gets a correlated NOT EXISTS against
OutgoingPost on the non-terminal states, so such a card is not a
candidate at all; kept in SQL because the candidate set is unbounded and
the predicate is cheap. The refs query is additionally restricted to
terminal rows, so a live row's bytes can never be reported as freed even
if one slips past the guard.

Also prove the tenant scoping. Both functions already filtered on
user_id, but every test used the same user_id fixture, so deleting either
filter turned nothing red - on a function whose only job is permanent
deletion, that is the one guard whose failure is both unrecoverable and
invisible. Two tests now seed an old, otherwise purgeable row owned by a
second user. Verified by mutation: with the predicates removed only the
new tests fail, the other 18 pass.

The refs query gains a user_id predicate as well, matching the convention
outgoing_post_service already states - an explicit invariant rather than
a bet on uuid collisions.

Wire the retention sweep into the lifespan ... a397fb6d81

Runs daily and, like the audit purge, also in dry-run: deleting data on a
retention schedule is a privacy obligation, not a Mattermost write.

The sweep reads the settings rows that exist rather than going through
get_settings_row(), which would create one per user; a failing user is
logged and skipped so it cannot park the obligation for everyone else;
and freed attachment bytes are released after the delete commits, via
_delete_orphan_refs, because a live draft can hold the same ref.

Both purge queries take a batch bound (RETENTION_BATCH_SIZE) so the card
purge cannot blow past the bind-parameter cap on a first sweep; the card
batch is capped after the notes filter, since a SQL LIMIT would keep
handing back the same spared rows.

Pin down attachment handling in the retention sweep ... 419dbd1bc8

A ref held by a live draft must survive the purge of the dead row that
shares it.

Add list endpoints for drafts, the full outbox and noted cards ... a7f2c9e84d

Drafts had no list endpoint at all: both scheduled-post routes filter them
out, which is why a forgotten draft is invisible today.

Pin has_notes/stage priority behavior with a regression test ... 7a7c74fa21

GET /api/cards?has_notes=true silently ignores stage: the intended
behavior for the only caller that exists, but invisible outside the
service docstring. Add a test seeding a completed and a non-completed
noted card and asserting both return under ?stage=voltooid&has_notes=true,
so a refactor that makes the filters compose has to fail this test and
be a deliberate decision. Document the priority rule on the route and
in the OpenAPI schema.

Add query hooks for drafts, the outbox and noted cards ... fc6b8a329a

Types and TanStack Query hooks for the retention feature's remaining
frontend work: Settings gains the two retention fields, plus
useDrafts/useOutbox/useNotedCards for the upcoming list pages.

Add the retention settings panel daa84d3097

Revise retention panel copy and fix doubled spacing ... 691c4d92c1

PO copy revisions after seeing the panel: the intro now states that cleanup
is on by default (90 days) instead of presenting it as opt-in, and no longer
claims cleanup is irreversible - only Waggle's own copy is removed; the one
real loss is a never-sent outgoing message. Each field's label and
supporting-label are collapsed into a single sentence, and the export hint
paragraph is dropped.

From review: Settings.vue applies a flex gap to every direct child of
nldd-card, and margins on this panel's top-level paragraphs added to that
gap (measured 32px around the intro vs the intended 16px). Top-level text
now uses margin: 0 like the other settings panels.

Offer existing accounts a one-time nudge to switch cleanup on ... 8c72f14e1d

Rides on the existing hints_seen bookkeeping; the primary button sets both
categories to 90 days, which also makes the banner disappear on its own.

Dismiss retention hint only after the settings write succeeds ... 5f67d6c23e

Review fixes for 8c72f14:
- enable() chained hint.dismiss() off the mutation's onSuccess instead of
  calling it unconditionally; a failed PUT no longer marks the hint seen
  while cleanup stays off
- drop the nested nldd-button-group: nldd-banner already wraps its actions
  slot in one
- widen visibility tests (mixed retention state, non-banner route,
  /leeslijst) and add a failure-path test asserting no dismiss on a failed
  mutation

Add pages for drafts, the outbox and noted cards ... 84eed98fbb

Outgoing posts had no home of their own: they lived as sections inside the
card lists, and sent messages were not visible anywhere.

Clear ?kaart= on modal close and include reading-list cards in Notities ... 563d9bdd71

Closing a card modal only reset in-memory selection; the kaart query
param stayed in the URL and forced the card open again on reload. Both
Inbox and ReadingList now strip it via router.replace on close.

GET /api/cards?has_notes=true was scoped to track=inbox, so a noted
reading-list card never showed on /notities even though the retention
sweep spares noted cards on both tracks. Drop the track filter for
that path.

Add e2e coverage for the retention intro and the overview pages ... 0371087b81

Covers the full retention-intro path (existing-account banner state,
enabling cleanup, hint dismissal surviving a reload) and the three
overview pages (/concept-berichten, /uitgaande-post, /notities),
including link-through from a row to the underlying card.

Document the retention policy 370ce346ce

Open cards outside the list window, stop the retention banner arguing back ... 86b8ca0cd7

GET /api/cards returns active cards plus only the 50 most recent completed
ones, while the overview pages link to a card of any age via /?kaart=<id>. A
noted, long-completed card therefore changed the URL and rendered nothing.
Add GET /api/cards/{card_id} (user-scoped, 404 for someone else's card) and
let useSelectedCard fall back to it when the loaded list misses.

PUT /api/settings now marks hint.retention.v1 seen whenever a retention field
is supplied. Setting both categories to "Niet opruimen" leaves both columns
null, which is the intro banner's own trigger, so the banner came back to
offer switching on what the user had just switched off.

Review nits: accurate docstring, "Gelezen" tag, honest retention copy ... 7892758a64

- cleanup_runner's docstring described only the unsave race-guard; the module
  now holds five background loops.
- A finished reading-list card is "gelezen" in Waggle's vocabulary, not
  "voltooid".
- The retention panel promised a 90-day default straight above two dropdowns
  reading "Niet opruimen"; that default only applies to new accounts.
- The :key comment cited an import-replace, which never writes the retention
  columns.
- docs/deployment.md now notes the retention sweep runs under WAGGLE_DRY_RUN=1
  as well.

robbertbos force-pushed retention-and-overview-pages from 7892758a64 to 2aaad960a8 2026-07-25 21:50:01 +00:00 Compare

robbertbos force-pushed retention-and-overview-pages from 2aaad960a8 to f950032cfb 2026-07-26 06:58:46 +00:00 Compare

robbertbos force-pushed retention-and-overview-pages from 1839e06718 to b84b34b176 2026-07-26 10:08:32 +00:00 Compare

robbertbos force-pushed retention-and-overview-pages from a97ee477fc to b326d83f75 2026-07-26 19:18:09 +00:00 Compare

robbertbos force-pushed retention-and-overview-pages from b326d83f75 to 9b94083420 2026-07-26 20:11:10 +00:00 Compare

robbertbos force-pushed retention-and-overview-pages from 9b94083420 to 20e73883b1 2026-07-26 20:17:43 +00:00 Compare

robbertbos force-pushed retention-and-overview-pages from 20e73883b1 to 54bfffd2ca 2026-07-26 20:24:54 +00:00 Compare

robbertbos force-pushed retention-and-overview-pages from 54bfffd2ca to c4f334409b 2026-07-26 20:28:08 +00:00 Compare

robbertbos merged commit c4f334409b into main 2026-07-26 20:41:46 +00:00

robbertbos deleted branch retention-and-overview-pages 2026-07-26 20:41:46 +00:00

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

bug

Something is not working

  

duplicate

This issue or pull request already exists

  

enhancement

New feature

  

help wanted

Need some help

  

invalid

Something is wrong

  

nldd ds

  

question

More information is needed

  

wontfix

This won't be fixed

No labels

bug

duplicate

enhancement

help wanted

invalid

nldd ds

question

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

robbertbos/waggle!191

No description provided.