Files
sciagent/docs/ADMIN_APPLICANT_NOTIFICATION_SYSTEM_ANALYSIS.md
Thinh Lam 688fac73e9
CI/CD / backend (push) Failing after 2m8s
CI/CD / frontend (push) Failing after 1m40s
CI/CD / deploy (push) Has been skipped
sciagent code + Gitea Actions CI/CD
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-30 09:38:30 +07:00

10 KiB
Raw Permalink Blame History

Analysis: notification system for admin → applicant (status & feedback)

This document describes how the stack works today, what is missing for true notifications, and a concrete v1 path aligned with current repo complexity (fe0 / be0 / PostgreSQL). It incorporates a review of the refined draft (ADMIN_APPLICANT_NOTIFICATION_SYSTEM_ANALYSIS.md from review) and adjusts a few points for this codebase.

It complements assets/APPLICANT_STATUS_NOTIFICATIONS_PLAN.md (council + broader product) by anchoring on application_admin_results and PUT /api/applications/{applicationId}/admin-result.


0. Evaluation of the reviewed draft (summary)

The reviewed version improves the original repo doc in several ways; adopt these:

Theme Verdict
Locked v1 scope In-app inbox only; ~60s polling + refetchOnWindowFocus; no email, MinIO PDF, WebSocket, or council unification in v1. Reduces scope and matches current team capacity.
Append-on-every-save Explicit product choice: new row per admin save; optional UI-only collapsing of consecutive rows. Clear and simple.
Schema pragmatism v1 type with TEXT + CHECK; omit JSONB until a second notification type exists. Fewer columns to maintain.
Indexes created_at DESC inbox index + partial index on (recipient_user_id) WHERE read_at IS NULL for unread count. Appropriate.
Security PATCH .../read returns 404 for foreign rows (same as missing id) to avoid user enumeration. Good default.
Helper surface notification_service.create_admin_decision_notification(...) keeps the admin route thin and future council hook consistent.

Adjust for this repository (critical):

  1. application_id type — Public application identifiers in this project are strings (e.g. sub-{hex}, case codes), not UUIDs. The notification table should use application_id TEXT NOT NULL (or VARCHAR) for deep links, matching ApplicationItem.id and API paths—not UUID.

  2. “try/except without rolling back the decision” — With get_session(), everything runs in one transaction that commits once at context exit. If the notification INSERT fails after the upsert has flushed, the entire transaction—including the decision—rolls back on exception, unless you:

    • use a begin_nested() savepoint around the notification insert (rollback to savepoint on failure, then continue), or
    • perform the notification insert in a separate short session after the admin-result transaction commits (best-effort second transaction).

    The reviewed drafts intent (“decision is sacred; notification best-effort”) is right; the implementation must use one of the patterns above, not a bare try/except in the same flat transaction.


1. v1 scope (locked)

  • In-app notifications only: PostgreSQL table + applicant GET / PATCH + /dashboard/notifications + optional header bell.
  • Delivery: React Query refetchInterval: 60_000 and refetchOnWindowFocus: true (no SSE/WebSocket in v1).
  • Write trigger: successful admin-result upsert (same product semantics as todays AdminStaffReadonlyReviewDialog / ResultManager).
  • Deferred to v2: email + outbox worker, MinIO/PDF letter, council localStorage → API unification, notification preferences, i18n beyond Vietnamese, retention jobs.

2. Current state (baseline)

2.1 Data and decisions

Layer What exists
PostgreSQL initiatives.statusapplication_admin_results on PUT …/admin-result (idempotent upsert).
Applicant reads GET /api/applications/mine, GET /api/applications/{id} — status and nhan_xet can reflect admin feedback after submission enrichment.
Admin Decided list lifecycle=decided; React Query invalidation on save.
Council Some flows still use localStorage; not applicant-visible until server-backed (v2; see assets plan).

Gap: No user_notifications (or equivalent); applicants only discover changes by refetching lists/detail.

2.2 Frontend

  • Sonner: admin-only affordance at save time.
  • React Query: applications, applications-mine, application-detail, etc.—no notification queries yet.
  • /dashboard/notifications: linked from sidebars; no page implementation observed.

2.3 Backend

  • FastAPI + transactional get_session(); natural hook: after admin-result body returns, or inside handler with savepoint / post-commit insert (see §0).

2.4 MinIO (v2 only for notifications)

Evidence/exports buckets are not the source of truth for notification text. Optional v2 PDF letter remains separate from v1.


3. Target architecture (v1)

flowchart LR
  subgraph admin [Admin UI]
    A[Confirm / ResultManager]
  end
  subgraph be [be0]
    B[PUT admin-result]
    C[Notification helper]
  end
  subgraph db [PostgreSQL]
    D[application_admin_results]
    E[initiatives]
    F[user_notifications]
  end
  subgraph fe [Applicant FE]
    H[Polling + onFocus]
    I[Inbox + bell]
  end
  A --> B
  B --> D
  B --> E
  B --> C
  C --> F
  F --> H
  H --> I

4. Database design (v1)

4.1 Principles

  • application_admin_results remains canonical for full feedback/rationale.
  • Notification rows are summaries + pointer to the public application_id string and optional FKs for audit.

4.2 Table: user_notifications

Column Type Notes
id UUID PK
recipient_user_id UUID FK → users.id (ON DELETE CASCADE) From initiatives.owner_id at insert.
type TEXT NOT NULL v1: CHECK (type IN ('admin_application_decision')).
title TEXT NOT NULL e.g. “Kết quả duyệt hồ sơ”.
body TEXT NOT NULL Decision label + ~280 chars feedback (newline-stripped).
application_id TEXT NOT NULL Public id (sub-… / case-shaped), matches API list/detail.
related_initiative_id UUID FK → initiatives.id (ON DELETE SET NULL)
source_admin_result_id UUID FK → application_admin_results.id (ON DELETE SET NULL)
read_at TIMESTAMPTZ nullable
created_at TIMESTAMPTZ NOT NULL DEFAULT now()

v1: no payload JSONB; add when a second type needs extra fields.

4.3 Indexes

CREATE INDEX user_notifications_inbox_idx
  ON user_notifications (recipient_user_id, created_at DESC);

CREATE INDEX user_notifications_unread_idx
  ON user_notifications (recipient_user_id)
  WHERE read_at IS NULL;

4.4 Insertion semantics

  • Product: append one row per successful admin-result save (including typo fixes).
  • Technical: implement best-effort notification with savepoint (session.begin_nested()) or post-commit insert so a notification failure never rolls back the adjudication. Document the chosen pattern in code comments next to the handler.

5. Backend API (be0)

5.1 Write path

After upsert_admin_result succeeds, resolve owner_id, build title/body, insert user_notifications via helper using the patterns in §4.4.

Sketch:

notification_service.create_admin_decision_notification(
    session, *, initiative, admin_result_row, application_id_public: str, decision_label: str
) -> UserNotification | None

5.2 Read paths (applicant-only in v1)

Method Purpose
GET /api/notifications Paginated; recipient_user_id = current user; fields: id, type, title, body, read_at, created_at, application_id, related_initiative_id.
GET /api/notifications/unread-count Count unread; benefits from partial index.
PATCH /api/notifications/{id}/read Set read_at = now() only if row belongs to user.

Authorization: for PATCH, return 404 if row missing or not owned (same body as missing).

5.3 Relationship to applications API

Notifications complement GET /api/applications/{id} (status + feedback); they do not replace it.


6. Frontend (fe0)

  • Page: implement /dashboard/notifications.
  • React Query: ["notifications", { page }] and ["notifications-unread-count"].
  • Polling: refetchInterval: 60_000, refetchOnWindowFocus: true.
  • UX: row click → PATCH .../read (optimistic unread decrement optional) → navigate using application_id string to existing applicant routes.
  • Bell: subscribe to unread-count query; same polling cadence.
  • Admin UI: no change required for v1 unless product adds “dont notify” toggle later.

7. Security and privacy

  • Scope all reads/patch to authenticated recipient.
  • Do not log full notification bodies in verbose HTTP logs in production.
  • recipient_user_id snapshot at insert time: historical rows stay with original recipient if ownership changes.

8. Rollout order (v1)

  1. Migration: table + indexes.
  2. SQLAlchemy model + notification_service helper (with savepoint or post-commit).
  3. Wire admin-result handler.
  4. Applicant GET list, GET unread-count, PATCH read.
  5. FE: inbox page + bell.
  6. Tests: notification appears after PUT (applicant token); PATCH read; PATCH foreign id → 404; notification failure does not undo admin-result (integration test with forced insert error).

9. v2 candidates (out of scope for v1)

Item Notes
Email Outbox + worker; user_notifications stays canonical.
MinIO PDF Generate on save; store artifact key; optional payload JSONB for metadata.
Council New type + CHECK extension + second writer from council API.
Preferences / retention Add when volume or compliance requires.

10. Relation to other docs

  • assets/APPLICANT_STATUS_NOTIFICATIONS_PLAN.md — council outcomes, workflow, broader audit. This file is the admin-first, v1-scoped implementation companion.

Update when migrations land, when the transaction strategy (savepoint vs post-commit) is fixed in code, or when v2 scope is agreed.