Skip to content

Notifications Decisions

Distilled from 6 DECISIONS.md files (KD-0174/0175/0288/0293 had no DECISIONS.md — bug-fix tasks). Each entry is a real fork in the road — what we chose, what we passed on, and why. Implementation details that aren't a tradeoff are NOT here.


Comment notifications: creator+assignee now, recipient-collection seam for later

  • Chose: Refactor NotifyCommentCreatedAction to loop over a collected recipient set (matching NotifyLaneChangeAction).
  • Rejected: Build a full subscribers/watchers system (issue_subscribers pivot, subscribe UI, ~20-25 files).
  • Why: Watchers were explicitly out of scope; refactoring to a recipient set creates the seam where a future resolver plugs in without rewriting any notify action.
  • Source: KD-0289

Comment self-notification: rely on existing dedup, no new guard

  • Chose: Trust CreateCommentAction's existing [$data->userId] initial dedup list.
  • Rejected: Adding an explicit "skip commenter" check.
  • Why: The existing $alreadyNotifiedUserIds mechanism already covers self-exclusion whether the commenter is creator, assignee, or both.
  • Source: KD-0289

Notifiable column: polymorphic morphs replacing issue_id

  • Chose: Drop issue_id, add notifiable_id + notifiable_type (NOT NULL).
  • Rejected: Make issue_id nullable + add separate nullable team_id/project_id columns.
  • Why: Sparse-row design with multiple nullable FKs has no clean morphTo relation; polymorphic morphs are extensible to any model.
  • Source: KD-0291

Type representation: extend enum, not refactor to STI

  • Chose: Add ProjectMembership = 5, TeamMembership = 6 cases to NotificationTypeEnum.
  • Rejected: Single Table Inheritance with a class per notification type.
  • Why: STI is the architecturally superior long-term foundation but the refactor cost is unjustified for two new types right now.
  • Source: KD-0291

Morph nullability: NOT NULL, with data backfill

  • Chose: morphs() (NOT NULL) with migration copying issue_idnotifiable_id before dropping the column.
  • Rejected: nullableMorphs() to ease migration.
  • Why: No valid notification exists without something to point to; nullable would invite invalid rows.
  • Source: KD-0291

v1 channel scope: in-app only, prefs columns added pre-emptively

  • Chose: Ship in-app for new types; add notify_project_membership / notify_team_membership columns now but don't read them yet.
  • Rejected: Defer the preference columns until email is built.
  • Why: Schema adds are cheap upfront; users can configure preferences before email exists, no double migration.
  • Source: KD-0291

Team membership click target: non-clickable, with tooltip

  • Chose: Render no link; team notifications show a title tooltip only.
  • Rejected: Link to a team page or settings.
  • Why: No actionable destination exists for a newly-added team member; revisit when a team detail page exists.
  • Source: KD-0291

Self-notification on project/team add: no guard

  • Chose: A user adding themselves still gets the notification.
  • Rejected: Suppress self-notifications on membership changes.
  • Why: Consistent with existing NotifyAssignmentChangeAction (no guard); behavioral consistency outweighs the edge-case UX win.
  • Source: KD-0291

Orphaned-assignment confirm UX: differentiated team vs member

  • Chose: Count-only modal for team removal; grouped issue list for direct member removal.
  • Rejected: One unified UI for both flows.
  • Why: Team removal can affect dozens of users with hundreds of issues — a list would overwhelm; member removal benefits from inspection.
  • Source: KD-0292

Cancel orphan-confirmation: block the entire save

  • Chose: Membership change does not persist unless the user confirms unassignment.
  • Rejected: Save the membership change anyway and skip unassignment.
  • Why: "Save anyway" silently leaves orphaned assignments — defeats the entire feature.
  • Source: KD-0292

"Done" lane detection: last lane by order, no new column

  • Chose: Treat the highest-order lane as Done.
  • Rejected: Adding an is_done boolean to the lanes table.
  • Why: EpicBoard.vue already uses this convention; an explicit flag would need migration + management UI for no real benefit.
  • Source: KD-0292

Preview endpoint: one shared, frontend decides display

  • Chose: Single POST /preview-orphaned-assignments returning grouped data + total count.
  • Rejected: Separate /preview-orphaned-teams and /preview-orphaned-members endpoints.
  • Why: Frontend uses count for teams and full list for members from the same payload; one Action, one route, no duplicated logic.
  • Source: KD-0292

Orphan-finding logic: shared Action, not duplicated

  • Chose: New FindOrphanedAssignmentsAction consumed by both preview and update paths.
  • Rejected: Duplicate computation in preview vs update actions.
  • Why: Two implementations of the same logic would drift; one source of truth is testable in isolation.
  • Source: KD-0292

Unassign mechanics: inside UpdateProjectAction transaction

  • Chose: Add unassignOrphaned: bool to UpdateProjectData; do the unassign inside the same transaction.
  • Rejected: Separate endpoint called by the frontend after the update succeeds.
  • Why: Two-call flow is non-atomic — an unassign failure leaves the membership change applied without cleanup.
  • Source: KD-0292

Team-member removal: separate access-check Action, shared issue query

  • Chose: New FindOrphanedAssignmentsForTeamAction with its own access-check; share only the non-Done-issues query.
  • Rejected: Force the per-project Action to accept a "team membership override" parameter.
  • Why: "Who loses access" is fundamentally different per entry point; muddying the per-project action with a single-caller param is wrong abstraction.
  • Source: KD-0292
  • Chose: Inline the warning inside ProjectTeamsMembersModal/ProjectDirectMembersModal; keep the standalone modal for team-edit page.
  • Rejected: Inline everywhere or modal-on-modal everywhere.
  • Why: Team-edit page is not a modal-on-modal situation, so the inline change would force a UX rework with no upside.
  • Source: KD-0292

Retention period: 6 months, hardcoded

  • Chose: 180-day TTL as a constant.
  • Rejected: 3-month aggressive, 12-month conservative, per-tenant configurable.
  • Why: Volume analysis shows the table is small even at large team sizes — period choice is a UX call, not a perf one; configurability is premature.
  • Source: KD-0294

Notification deletion: hard delete, no soft delete

  • Chose: Permanent row removal for both user-initiated and retention cleanup.
  • Rejected: Add deleted_at column with later purge job.
  • Why: Notifications are pointers, not content; only User uses soft deletes in the codebase, and even DeleteUserAction hard-deletes notifications.
  • Source: KD-0294

Deletion paths: both user-initiated and scheduled cleanup

  • Chose: Ship per-row delete UI + scheduled retention command.
  • Rejected: Cleanup-only (no user delete) or user-only (no auto-cleanup).
  • Why: They serve different purposes — UX inbox management vs unbounded-growth safety net for inactive users; both are cheap given hard-delete.
  • Source: KD-0294

Cleanup mechanism: scheduled artisan command, daily, tenant-iterating

  • Chose: app:cleanup-notifications registered with ->daily(), iterating tenants via TenantSwitcher.
  • Rejected: Queued database job, model-event observer, or single-tenant command (the ReapStaleAiRunsCommand gap).
  • Why: A daily DELETE is sub-millisecond per tenant; queue infra is overengineered, observers couple cleanup to user activity, and skipping tenant iteration is a known existing bug.
  • Source: KD-0294

Delete confirmation UX: no modal, no undo

  • Chose: Single-click instant delete (GitHub-style "done").
  • Rejected: Confirmation modal (matches existing destructive-action pattern), or undo toast.
  • Why: Notifications contain no unique content; the app's confirmation pattern exists for high-value entities, and undo would require a new pattern not used elsewhere.
  • Source: KD-0294

Bulk delete API: single bulk endpoint for both single and multi

  • Chose: POST /notifications/bulk-delete accepts an array (length 1 or N).
  • Rejected: Separate DELETE /notifications/{id} plus bulk endpoint.
  • Why: Frontend always sends an array; one endpoint, one Action, minimal API surface — same pattern as BulkDeleteIssuesAction.
  • Source: KD-0346

Optimistic UI: yes, with rollback

  • Chose: Remove notifications from state immediately, restore on error.
  • Rejected: Wait for server confirmation.
  • Why: Existing notification store already does optimistic updates for read/unread; consistency with surrounding patterns.
  • Source: KD-0346
  • Chose: Refactor row to stretched-link pattern with relative z-1 on interactive cells.
  • Rejected: Keep @click on <tr> and add stop-propagation on the new checkbox/actions cells.
  • Why: Recent table overhaul standardised on stretched-link everywhere except inbox (which had no other interactive elements at the time); adding checkboxes triggers the same pattern shift.
  • Source: KD-0346

Read endpoints: collapse to bulk, drop singular + read-all

  • Chose: Three bulk endpoints (bulk-read/bulk-unread/bulk-delete) replace {id}/read, {id}/unread, read-all.
  • Rejected: Keep all four endpoints alongside the new bulk ones.
  • Why: With select-all checkbox, "all" is just bulk-with-N; per-row mark-as-read sends [id]; cleaner API surface, fewer Actions.
  • Source: KD-0346

Selection state: in Overview.vue, not store

  • Chose: Local ref<number[]> in the page.
  • Rejected: Adding selection state to the notification store.
  • Why: Selection is UI state, not domain state; should reset on navigation/filter change, which is automatic when local.
  • Source: KD-0346

Notification model: Linear (subscriptions), not Jira (admin schemes)

  • Chose: User-owned channel × event matrix with contextual subscribe/unsubscribe.
  • Rejected: Admin-configured per-project notification schemes.
  • Why: Schemes are notoriously confusing and require admin UI; Linear's model fits a user-driven product better at this scale.
  • Source: KD-0362

Channel split: in-app columns originally added, then dropped (D14 reversal)

Chose (final): Pivot membership is the in-app gate; drop all 6 notify_*_app columns. Rejected: Independent in-app + email toggles per event type (the 12-column matrix from D2). Why: A user could have a pivot row (watching) but notify_comment_app=false (silent) — the bell would lie. Pivot becomes single source of truth for in-app delivery. Source: KD-0362

Per-project subscriptions: dropped from scope

  • Chose: Per-issue subscriptions only.
  • Rejected: Per-project bell on ProjectLayout creating a Global > Project > Issue hierarchy.
  • Why: Three layers without proportional value at current scale; per-issue granularity is sufficient.
  • Source: KD-0362

Lane-change filtering: removed entirely (D4 reversed by reviewers)

Chose (final): No allowlist; lane-change notifications gated solely by notify_lane_change toggle. Rejected: JSON column notify_lane_allowlist with NULL = all, [] = none. Why: Project-rule violation (no JSON columns); string-name matching is fundamentally broken — no project scoping, breaks on lane rename, semantics unrecoverable across projects. Source: KD-0362

Watchers data model: pure M2M pivot (D5 revised)

  • Chose: issue_watchers row exists ⇔ user is watching; no subscribed flag, no implicit/explicit distinction.
  • Rejected: "Implicit + override" model where creator/assignee/commenters were implicitly subscribed and the table only stored unsubscribe overrides.
  • Why: PR reviewer flagged the override model as too clever; explicit pivot rows are simpler to reason about and query.
  • Source: KD-0362

Mentions don't auto-watch (D18 revises D5b)

  • Chose: NotifyMentionedUsersAction sends the mention but does not write a pivot row.
  • Rejected: Auto-attach mentioned users to the watcher pivot.
  • Why: A mention is a one-shot summons, not a subscription; auto-attach silently subscribes users to all future activity, which is the noise we want to avoid.
  • Source: KD-0362

Reassignment: don't detach old assignee (D19 revises D13)

  • Chose: Reassignment leaves the old assignee's pivot row alone; "unassigned" notification still fires.
  • Rejected: Auto-detach old assignee from the pivot when the assignee changes.
  • Why: The pivot can't tell why a row exists — old assignee may also be creator/commenter/manual subscriber; detaching strips unrelated watch state.
  • Source: KD-0362

Migration cutover banner: dropped pre-merge (D17 reversed)

Chose (final): Release-notes mention only; no in-app banner. Rejected: One-time dismissable banner backed by seen_watcher_cutover_banner column + dedicated endpoint. Why: Footprint (column + Action + controller + Vue + tests) was disproportionate to a one-time UX nicety; assignees self-correct quickly via the discoverable bell. Source: KD-0362

Read-state visual: bold-text + dot, drop envelope icons

  • Chose: Gmail pattern — bold = unread, regular = read; existing red dot kept for unread.
  • Rejected: Envelope icons that double as state indicator AND clickable mark-as-read action.
  • Why: The double role was persistently confusing in user testing; bold/regular is universally understood; per-row mark-as-read remains via bulk-toolbar.
  • Source: KD-0362

Plain @Name resolution: rewrite to wire-format at write-time, not resolve at notify-time

  • Chose: A resolver Action rewrites @Name[@ id="N"] before save, at the top of the three write paths; the persisted content carries the canonical wire-format.
  • Rejected: Resolve names only inside NotifyMentionedUsersAction, leaving stored content as plain @Name.
  • Why: Persisting the wire-format means everything downstream works unchanged — FE chip rendering, mail rendering, notify dedupe, and the update-mention diff — as one transformation with a single source of truth; notify-time resolution produces no UI chip and re-runs on every render, drifting as project membership changes.
  • Source: KD-0732 D1

Mention matching policy: unique full name → unique first name, case-insensitive, ambiguity stays literal

  • Chose: @Jane Doe matches a full name; @Jane matches only if exactly one project member has that first name; candidates are tried longest-first and any ambiguity (duplicate first or full names) leaves the text literal.
  • Rejected: Full-name-only matching (most real @Jane usage would silently stay literal); full → first → last (more reach, more false positives in prose).
  • Why: Agents naturally write @Jane, so first-name matching is needed for the feature to fire, but failing safe to literal on any ambiguity avoids guessing wrong — the cost of a missed mention is lower than a misdirected one.
  • Source: KD-0732 D2

Mention resolution applies to all channels, not just MCP

  • Chose: Resolve plain @Name in the shared Actions for every author — a human declining the FE picker and typing @Jane resolves too.
  • Rejected: Thread an MCP-origin flag through CreateCommentData/CreateIssueData so only agent-authored content resolves.
  • Why: A plainly-typed @Jane almost certainly meant a mention regardless of who wrote it, and the ambiguity→literal safeguard makes uniform resolution safe — an origin flag is plumbing that exists only to preserve accidental non-behavior.
  • Source: KD-0732 D3

Mention member set: team/direct project membership, not accessibleBy

  • Chose: Match against team-or-direct project members — the same whereHas('teams.projects')->orWhereHas('directProjects') shape the agent sees in prepare-project-context's members[].
  • Rejected: ProjectBuilder::accessibleBy, which additionally includes all-projects admins.
  • Why: It matches AC2's "project members" literally and keeps the agent's mental model and the resolver in agreement; every match is still a strict subset of the notify path's accessibleBy gate, so no resolved mention is ever dropped for access reasons.
  • Source: KD-0732 D5