Skip to content

feat: Add Banners Table and API - #337

Open
camielvs wants to merge 1 commit into
TangleML:masterfrom
camielvs:cvs/banners
Open

feat: Add Banners Table and API#337
camielvs wants to merge 1 commit into
TangleML:masterfrom
camielvs:cvs/banners

Conversation

@camielvs

@camielvs camielvs commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

What this adds

A generic, site-wide announcement banner: a dedicated banner table, a public read endpoint for the frontend, and admin CRUD endpoints for managing banners. API only — there is no admin UI in this phase.

Design objective

Give operators a way to show a short, timed message to every user of the app (planned maintenance, degraded functionality, a link to a status page) without a deploy, and give the frontend a single cheap endpoint it can poll to render whatever is currently live. The banner content is deliberately generic — title, body, severity variant, optional link — with no assumptions about who is producing it or why.

API

Method Path Auth Purpose
GET /api/banners/active any user Banners that are live right now. Returns only display fields, sends Cache-Control: no-store.
GET /api/admin/banners admin All banners, newest first. ?include_deleted=true also returns soft-deleted ones.
POST /api/admin/banners admin Create.
GET /api/admin/banners/{id} admin Read one, including admin-only fields.
PATCH /api/admin/banners/{id} admin Partial update.
DELETE /api/admin/banners/{id} admin Soft delete.

A banner is active when deleted_at IS NULL AND is_enabled = true AND (starts_at IS NULL OR starts_at <= now) AND (ends_at IS NULL OR ends_at > now). Active banners are sorted by starts_at descending with un-scheduled banners last, then created_at descending.

You can exercise all of this from the /docs route.

image

Key decisions

A dedicated banner table instead of UserSettings. UserSettings is a per-user key/value JSON blob keyed by user_id; a banner is a global object with its own lifecycle, so it fits neither the key nor the shape. Storing banners there would mean either duplicating a banner into every user's settings row or inventing a magic pseudo-user to hold the global ones, and in both cases the scheduling window and the enabled/deleted flags would live inside opaque JSON — not queryable, not indexable, not constrainable. A table gives us a real WHERE clause for the active lookup, real indexes, and per-row audit columns.

Soft delete only. DELETE sets deleted_at and never removes the row, so a banner that was shown to users stays auditable and an accidental delete is recoverable. deleted_at IS NULL is part of both the active query and the default admin list; ?include_deleted=true opts back in. Delete is idempotent — deleting an already-deleted banner leaves the original timestamp alone.

Two response shapes rather than one. /api/banners/active returns 11 display fields; the admin endpoints add is_enabled, created_by, updated_by and deleted_at. Keeping them as separate response types (BannerResponse / AdminBannerResponse) means the public endpoint cannot leak operator identities by accident. This is also why the banner routes don't use the router's default_config: that config strips null fields, and the frontend wants url: null present rather than absent.

Reusing errors.ApiValidationError for validation failures. Invalid input (bad URL, over-length title/body, url_text without a url, ends_at <= starts_at) raises the existing ApiValidationError, which the existing handler maps to 422. No new error type and no new exception handler were added. An unknown variant is rejected by FastAPI request validation, so it also returns 422 — one status code for all bad input.

variant as a str enum column. BannerVariant (info / warning / success / error) is mapped onto the column with values_callable, matching how ContainerExecutionStatus is handled, so the DB stores "warning" rather than "WARNING" and the valid set shows up in the OpenAPI schema instead of living in a hand-written validator.

Client datetimes are normalized to UTC on the way in. The DB stores naive UTC (see UtcDateTime), which means a starts_at of 2026-01-01T12:00:00+02:00 would otherwise be stored as if 12:00 were UTC. It is converted to 10:00Z before being stored, and cross-field comparison normalizes both sides so a request-supplied aware datetime can be compared against a DB-loaded one.

Two indexes, matching the two access paths. (is_enabled, deleted_at, starts_at, ends_at) serves the active lookup that the frontend hits on every page load; created_at DESC serves the admin list.

Partial update semantics. PATCH follows the existing convention in this codebase (PublishedComponentService.update): a field that is null/absent is left unchanged. The trade-off is that PATCH cannot currently clear a nullable field back to null — {"url": null} is a no-op, not a clear. Worth revisiting if that turns out to matter, but it would need a departure from the convention (a sentinel or exclude_unset).

Schema migration

Purely additive: a new table plus its two indexes, created by the existing metadata.create_all. No existing table, column or index is touched, so no migrate_db step is needed. Verified by building a DB with the code at master, inserting a row, then running create_db_engine_and_migrate_db with this branch: banner and its indexes appear, every other table is unchanged, and the pre-existing row survives.

Testing

tests/test_banners_api.py adds 28 tests against a real TestClient app over an in-memory SQLite DB, covering: empty active list and the no-store header; create and read-back through the admin endpoints; the active window (enabled/in-window included, disabled/future/expired excluded); the public response exposing exactly the 11 display fields; partial update including trimming, updated_at advancing, and untouched fields staying put; soft delete removing the banner from both the active list and the default admin list while the row remains fetchable; 403 for a non-admin on every admin route while the public read still works; 422 for every invalid-input case; 404 for unknown ids; active sort order; and UTC conversion of offset datetimes.

Also verified by hand against a running server: create → active → disable → active empty → delete → row still present in SQLite.

@camielvs
camielvs requested a review from a team August 18, 2026 00:24
@camielvs
camielvs requested a review from Ark-kun as a code owner August 18, 2026 00:24
@camielvs
camielvs marked this pull request as draft August 18, 2026 00:25
@camielvs
camielvs marked this pull request as ready for review August 18, 2026 00:56
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.

1 participant