Skip to content

Backend/feature: make dashboard reminders dismissable#8513

Closed
aapeliv wants to merge 2 commits into
developfrom
backend/feature/dismiss-reminders
Closed

Backend/feature: make dashboard reminders dismissable#8513
aapeliv wants to merge 2 commits into
developfrom
backend/feature/dismiss-reminders

Conversation

@aapeliv

@aapeliv aapeliv commented May 10, 2026

Copy link
Copy Markdown
Member

Adds a backend for dismissing the four dashboard reminders surfaced by Account.GetReminders. Today the reminders are derived purely from state and can't be dismissed; users have asked for a way to defer or silence them.

  • New ReminderDismissal model with partial unique indexes for the global vs entity cases and a CHECK constraint enforcing host_request_id is set iff the type is per-entity.
  • New DismissReminder RPC with a flat ReminderType enum + optional host_request_id.
  • GetReminders now filters dismissed reminders: global types (complete_profile, complete_verification) are hidden for a 90-day cooldown; per-entity types (respond_to_host_request, write_reference) are hidden permanently for that host_request_id.
  • Re-dismissing a global reminder refreshes the cooldown; re-dismissing an entity reminder is a harmless no-op.

Migration is intentionally not included — autogenerated in CI per project convention.

Frontend wiring (dismiss button on ReminderItem, optimistic React Query update) is out of scope for this PR.

Testing

  • make format and make mypy clean (the one mypy error in tests/fixtures/misc.py is pre-existing and unrelated).
  • New tests in test_account.py (all passing) covering:
    • Dismissing each reminder type and confirming it disappears from GetReminders.
    • Per-host-request isolation: dismissing one host request reminder leaves the others intact.
    • 90-day cooldown expiry — reminder reappears once dismissed_at is mutated past the cooldown.
    • Re-dismiss refreshes the cooldown timestamp.
    • Validation: global type with host_request_id set / entity type without it / UNSPECIFIED all return INVALID_ARGUMENT.
    • Idempotence: re-dismissing the same reminder doesn't raise and doesn't create duplicate rows.
  • pytest src/tests/test_account.py src/tests/test_references.py — 58 tests pass.

Backend checklist

  • Added tests for any new code or added a regression test if fixing a bug
  • Run the backend locally and it works
  • Added migrations if there are any database changes, rebased onto develop if necessary for linear migration history (CI autogenerates)

For maintainers

  • Maintainers can push commits to my branch
  • Maintainers can merge this PR for me

This PR was created with the Couchers PR skill.

@aapeliv
aapeliv requested a review from tristanlabelle as a code owner May 10, 2026 02:05
@vercel

vercel Bot commented May 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
couchers Ready Ready Preview May 10, 2026 2:26am

Request Review

@couchersbot

couchersbot Bot commented May 10, 2026

Copy link
Copy Markdown
Contributor

Claude finished @CouchersBot[bot]'s task in 53s —— View job


Migration Review

  • Fetched PR diff
  • Reviewed 0149_backend_feature_add_dismissable_.py
  • Cross-checked against app/backend/src/couchers/models/reminder.py
  • Verified down-revision chain (0148 → 0149)
  • Posted migration review comment

Result: SAFE — pure additive migration on a new empty table. No NULL/backfill, type cast, FK orphan, or unique-constraint risks against existing data. New remindertype enum (no ADD VALUE to an existing one). Server default uses now() per project convention. Downgrade is complete.

See full review: migration review comment
· Branch: backend/feature/dismiss-reminders

@couchersbot

couchersbot Bot commented May 10, 2026

Copy link
Copy Markdown
Contributor

Migration Review

One of our most common issues is migrations that don't work with existing data in production. This is very hard to catch with CI, so we have this bot to check for potential problems.

Overall Risk: SAFE

Files Reviewed

  • 0149_backend_feature_add_dismissable_.py — creates new reminder_dismissals table with associated remindertype enum, FK references to users and host_requests, two partial unique indexes, one regular lookup index, and a CHECK constraint enforcing the global-vs-entity invariant.

Findings

No issues found.

The migration only adds a brand-new table, so there is no risk of NULL backfill, type cast, or constraint violations against existing data. The remindertype enum is new (no ADD VALUE to existing enums). FKs to users.id and host_requests.id cannot orphan since the table starts empty. The dismissed_at server default uses now() consistent with project convention. Down-revision chain is correct (0148 → 0149). Downgrade properly drops the indexes and table (the remindertype enum is dropped automatically with the column on table drop). No CONCURRENTLY needed since the table is empty at migration time.

Summary

Pure additive migration on a new empty table — safe to run against the production database.

aapeliv and others added 2 commits May 9, 2026 22:22
Adds a ReminderDismissal model and DismissReminder RPC. Global reminders
(complete profile/verification) hide for a 90-day cooldown; per-host-request
reminders (respond to host request, write reference) are dismissed permanently.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@aapeliv
aapeliv force-pushed the backend/feature/dismiss-reminders branch from f61a1b7 to 18a453a Compare May 10, 2026 02:22
@aapeliv aapeliv changed the title Backend/feature: Add dismissable dashboard reminders Backend/feature: make dashboard reminders dismissable May 10, 2026
@aapeliv
aapeliv marked this pull request as draft May 10, 2026 03:25
@nabramow

Copy link
Copy Markdown
Member

@aapeliv We added this functionality in the frontend via localStorage, do we still need this PR?

@nabramow

nabramow commented Jul 6, 2026

Copy link
Copy Markdown
Member

Closing as we added this to the frontend via localStorage for now. Feel free to reopen later if we decide we prefer backend.

@nabramow nabramow closed this Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants