From 242132330a22f992b3033915b2bcaed56d8335a5 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 21 Oct 2026 14:08:06 +0200 Subject: [PATCH 01/12] feat(core,db): AuditLink.for_model and independent_session (#523) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- CHANGELOG.md | 6 +++ CLAUDE.md | 5 ++- docs/database/sessions.md | 15 +++++++ docs/framework/lifecycle.md | 6 +-- .../core/simple_module_core/audit_links.py | 27 +++++++++++++ framework/core/simple_module_core/module.py | 4 +- framework/core/tests/test_audit_links.py | 30 ++++++++++++++ framework/db/simple_module_db/__init__.py | 2 + framework/db/simple_module_db/independent.py | 31 +++++++++++++++ .../db/tests/test_independent_session.py | 39 +++++++++++++++++++ modules/api_tokens/api_tokens/audit.py | 5 +-- modules/api_tokens/api_tokens/resolver.py | 5 +-- .../background_tasks/module.py | 7 +--- modules/feature_flags/feature_flags/module.py | 7 +--- modules/file_storage/file_storage/module.py | 6 +-- modules/permissions/permissions/module.py | 11 ++---- modules/settings/settings/module.py | 7 +--- modules/users/users/audit.py | 8 +--- 18 files changed, 176 insertions(+), 45 deletions(-) create mode 100644 framework/db/simple_module_db/independent.py create mode 100644 framework/db/tests/test_independent_session.py diff --git a/CHANGELOG.md b/CHANGELOG.md index a8cd80048..325738715 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,12 @@ All notable changes to this project are documented in this file. The format is b ## [Unreleased] ### Added +- `AuditLink.for_model(Model, url_template=..., ...)` fills `entity_type` (class + name) and `table_name` from the model; every module's `register_audit_links` + now uses it. `simple_module_db.independent_session(app)` is an async context + manager for out-of-request sessions that commits on success and rolls back on + error (`permissions` startup replay and `api_tokens` last-used write use it) + (#523). - New `mail` module (`simple_module_mail`) owns outgoing mail: the SMTP/console transport, its settings (`/admin/settings/`, package `mail`) and the on-demand `mail.mailer` health check behind "Test mailer connection". Modules send through diff --git a/CLAUDE.md b/CLAUDE.md index d499391e1..3b7b9786e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -362,8 +362,9 @@ development and testing (`session_cookie_secure`). durable handlers, which have no app). The `mail` module owns SMTP and its settings; without it the sender only logs (`delivers_email` is false). Tests use the `captured_mail` fixture. -- **Audit links / design packs**: `register_audit_links` takes - `AuditLink(entity_type=Model.__name__, …)` (class name, not table name, or it +- **Audit links / design packs**: `register_audit_links` builds + links with `AuditLink.for_model(Model, url_template=…, …)` (it fills + `entity_type=Model.__name__` and `table_name`; a table-name `entity_type` silently never matches); `register_design_packs` slugs must match `^[a-z0-9][a-z0-9-]*$` and the host must still import the CSS. - **i18n.** Ship `/locales/.json`, declare in `locale_dirs()` with diff --git a/docs/database/sessions.md b/docs/database/sessions.md index 2cea3ea2f..5b1b65bf9 100644 --- a/docs/database/sessions.md +++ b/docs/database/sessions.md @@ -133,6 +133,21 @@ async def worker(db: DatabaseState): The `async with session.begin()` pattern opens a transaction you control. Use it in code that runs **outside** a request scope. +### `independent_session(app)` + +For the common case, a short unit of work outside the request that should commit when it succeeds and roll back when it fails, use `independent_session`: + +```python +from simple_module_db import independent_session + + +async def on_startup(self, app): + async with independent_session(app) as db: + await replay_rows(db) # committed on clean exit, rolled back on error +``` + +It opens a session from `app.state.sm.db.session_factory`, commits when the block exits cleanly, and rolls back and re-raises otherwise. Do not call `commit()` inside the block. Keep using `session_factory()` directly when the block is read-only, must commit mid-way (for example to do follow-up work after the commit, as the branding image reaper does), or runs against another engine. + ## Sessions and `MultiTenantMixin` The active tenant is carried in the `current_tenant_id` contextvar (set by `TenantMiddleware`, not on the session). The tenant listeners read that contextvar to filter SELECTs and stamp INSERTs. Sessions stay **request-scoped** so they always run under the request's tenant context — sharing one across tenants would silently leak data. diff --git a/docs/framework/lifecycle.md b/docs/framework/lifecycle.md index 7e7b7ca07..a7b076dcf 100644 --- a/docs/framework/lifecycle.md +++ b/docs/framework/lifecycle.md @@ -272,8 +272,8 @@ def register_audit_links(self, registry: AuditLinkRegistry) -> None: from users.models import User registry.register( - AuditLink( - entity_type=User.__name__, # class name — NOT __tablename__ + AuditLink.for_model( + User, # fills entity_type=User.__name__ (NOT __tablename__) and table_name url_template="/admin/users/{id}", label="User", label_key="users.audit.user", @@ -281,7 +281,7 @@ def register_audit_links(self, registry: AuditLinkRegistry) -> None: ) ``` -`entity_type` matches `AuditEntry.entity_type`, which `snapshot_changes` writes as `type(obj).__name__`. Keying it off `__tablename__` (`"users_user"`) produces a link that never matches — and nothing errors: an unmatched lookup falls back to rendering `entity_type` as a plain label, so the row still shows text while silently never becoming a link. Using `Model.__name__` rather than a string literal makes a rename impossible to get wrong. +`entity_type` matches `AuditEntry.entity_type`, which `snapshot_changes` writes as `type(obj).__name__`. Keying it off `__tablename__` (`"users_user"`) produces a link that never matches — and nothing errors: an unmatched lookup falls back to rendering `entity_type` as a plain label, so the row still shows text while silently never becoming a link. `AuditLink.for_model(Model, ...)` fills `entity_type` from `Model.__name__` and `table_name` from `Model.__tablename__`, so a rename or a table-name mix-up is impossible; the plain `AuditLink(...)` constructor stays available for links without a model class. `url_template` must contain the literal `{id}` placeholder, substituted with the entity id. A template without it raises at boot rather than pointing every row at the same page. diff --git a/framework/core/simple_module_core/audit_links.py b/framework/core/simple_module_core/audit_links.py index 47a561727..bcc6935b0 100644 --- a/framework/core/simple_module_core/audit_links.py +++ b/framework/core/simple_module_core/audit_links.py @@ -108,6 +108,33 @@ def __post_init__(self) -> None: f"every row would link to the same page" ) + @classmethod + def for_model( + cls, + model: type, + *, + url_template: str, + label: str = "", + label_key: str = "", + label_resolver: LabelResolver | None = None, + label_permission: str = "", + ) -> AuditLink: + """Build the link for a SQLModel table class. + + Fills ``entity_type`` from ``model.__name__`` (the class name, never the + table name) and ``table_name`` from ``model.__tablename__``, so a caller + cannot key it off the wrong one. ``model`` must be a table model. + """ + return cls( + entity_type=model.__name__, + url_template=url_template, + label=label, + label_key=label_key, + table_name=model.__tablename__, + label_resolver=label_resolver, + label_permission=label_permission, + ) + def url_for(self, entity_id: str) -> str | None: """The row's page, or ``None`` when this kind of row has no screen.""" if not self.url_template: diff --git a/framework/core/simple_module_core/module.py b/framework/core/simple_module_core/module.py index 115834250..1ca54c1d7 100644 --- a/framework/core/simple_module_core/module.py +++ b/framework/core/simple_module_core/module.py @@ -214,8 +214,8 @@ def register_audit_links(self, registry: AuditLinkRegistry) -> None: def register_audit_links(self, registry): registry.register( - AuditLink( - entity_type=User.__name__, + AuditLink.for_model( + User, url_template="/admin/users/{id}/edit", label="User", ) diff --git a/framework/core/tests/test_audit_links.py b/framework/core/tests/test_audit_links.py index e6b150311..4695afe79 100644 --- a/framework/core/tests/test_audit_links.py +++ b/framework/core/tests/test_audit_links.py @@ -164,3 +164,33 @@ def test_an_empty_template_is_accepted(self) -> None: def test_a_non_empty_template_still_has_to_carry_the_id(self) -> None: with pytest.raises(ValueError, match="contains no"): AuditLink(entity_type="Thing", url_template="/things") + + +class TestForModel: + def test_equals_hand_built_link(self) -> None: + class Widget: + __tablename__ = "widgets_widget" + + built = AuditLink.for_model( + Widget, + url_template="/w/{id}", + label="Widget", + label_key="w.audit", + label_permission="p", + ) + assert built == AuditLink( + entity_type="Widget", + url_template="/w/{id}", + label="Widget", + label_key="w.audit", + table_name="widgets_widget", + label_permission="p", + ) + + def test_entity_type_is_class_name_not_table(self) -> None: + class Widget: + __tablename__ = "widgets_widget" + + link = AuditLink.for_model(Widget, url_template="") + assert link.entity_type == "Widget" + assert link.table_name == "widgets_widget" diff --git a/framework/db/simple_module_db/__init__.py b/framework/db/simple_module_db/__init__.py index 41af536df..6cfc9a3d2 100644 --- a/framework/db/simple_module_db/__init__.py +++ b/framework/db/simple_module_db/__init__.py @@ -5,6 +5,7 @@ from simple_module_db.callbacks import OnCommitCallback from simple_module_db.deps import get_db from simple_module_db.extensions import UnsupportedDatabaseError, ensure_db_extensions +from simple_module_db.independent import independent_session from simple_module_db.migration_portability import drop_enums_if_postgres from simple_module_db.migrations import ( build_module_metadata, @@ -67,6 +68,7 @@ "finalize_session", "get_db", "hard_delete", + "independent_session", "init_db", "is_valid_tenant_id", "like_contains_pattern", diff --git a/framework/db/simple_module_db/independent.py b/framework/db/simple_module_db/independent.py new file mode 100644 index 000000000..6eb9668a2 --- /dev/null +++ b/framework/db/simple_module_db/independent.py @@ -0,0 +1,31 @@ +"""A session opened outside the request, with the commit convention in one place.""" + +from __future__ import annotations + +from collections.abc import AsyncGenerator +from contextlib import asynccontextmanager +from typing import Any + +from sqlalchemy.ext.asyncio import AsyncSession + + +@asynccontextmanager +async def independent_session(app: Any) -> AsyncGenerator[AsyncSession]: + """Open a session from ``app``'s factory, outside any request. + + Commits when the block exits cleanly and rolls back (then re-raises) when it + raises, so startup hooks, background cleanups and out-of-band writes do not + each hand-roll the pair. The session is closed either way. + + Request code must not use this: the request session (``get_db``) commits in + ``CommitBeforeResponseMiddleware``, and a second session cannot see its + uncommitted rows. Code that must commit mid-block, never commit, or run on a + different engine keeps using ``session_factory()`` directly. + """ + async with app.state.sm.db.session_factory() as session: + try: + yield session + await session.commit() + except BaseException: + await session.rollback() + raise diff --git a/framework/db/tests/test_independent_session.py b/framework/db/tests/test_independent_session.py new file mode 100644 index 000000000..04176d27d --- /dev/null +++ b/framework/db/tests/test_independent_session.py @@ -0,0 +1,39 @@ +from __future__ import annotations + +from types import SimpleNamespace + +import pytest +from simple_module_db import independent_session +from sqlalchemy import text + + +@pytest.fixture +def app(db_state): + return SimpleNamespace(state=SimpleNamespace(sm=SimpleNamespace(db=db_state))) + + +async def _count(db_state) -> int: + async with db_state.session_factory() as s: + return (await s.execute(text("SELECT count(*) FROM t_indep"))).scalar_one() + + +async def _make_table(db_state) -> None: + async with db_state.session_factory() as s: + await s.execute(text("CREATE TABLE t_indep (id integer)")) + await s.commit() + + +async def test_commits_on_success(app, db_state): + await _make_table(db_state) + async with independent_session(app) as s: + await s.execute(text("INSERT INTO t_indep VALUES (1)")) + assert await _count(db_state) == 1 + + +async def test_rolls_back_on_error(app, db_state): + await _make_table(db_state) + with pytest.raises(RuntimeError): + async with independent_session(app) as s: + await s.execute(text("INSERT INTO t_indep VALUES (1)")) + raise RuntimeError("boom") + assert await _count(db_state) == 0 diff --git a/modules/api_tokens/api_tokens/audit.py b/modules/api_tokens/api_tokens/audit.py index 999cdf178..d8fd64dbe 100644 --- a/modules/api_tokens/api_tokens/audit.py +++ b/modules/api_tokens/api_tokens/audit.py @@ -42,11 +42,10 @@ async def resolve_token_labels(db: AsyncSession, ids: list[str]) -> dict[str, st def build_token_audit_link() -> AuditLink: - return AuditLink( - entity_type=ApiToken.__name__, + return AuditLink.for_model( + ApiToken, url_template=f"{ADMIN_URL}?{QP_TOKEN}={{id}}", label=AUDIT_LINK_LABEL, label_key=AUDIT_LINK_LABEL_KEY, - table_name=ApiToken.__tablename__, label_resolver=resolve_token_labels, ) diff --git a/modules/api_tokens/api_tokens/resolver.py b/modules/api_tokens/api_tokens/resolver.py index fbeb69fb5..8e93b0a91 100644 --- a/modules/api_tokens/api_tokens/resolver.py +++ b/modules/api_tokens/api_tokens/resolver.py @@ -20,7 +20,7 @@ from auth.contracts.schemas import UserContext from auth.contracts.tokens import AUTH_KIND_PAT, AUTH_KIND_SERVICE, is_api_token, parse_bearer from simple_module_core.tenancy import is_tenant_role -from simple_module_db import ALL_TENANTS_OPTION +from simple_module_db import ALL_TENANTS_OPTION, independent_session from sqlalchemy import or_, select, update from sqlalchemy.orm import noload, selectinload from starlette.requests import Request @@ -116,9 +116,8 @@ async def touch_last_used(app: Any, token: ApiToken, now: datetime) -> None: .execution_options(**{ALL_TENANTS_OPTION: True}) ) try: - async with app.state.sm.db.session_factory() as db: + async with independent_session(app) as db: await db.execute(stmt) - await db.commit() except Exception: logger.warning("api_tokens: could not record last use of %s", token.id, exc_info=True) diff --git a/modules/background_tasks/background_tasks/module.py b/modules/background_tasks/background_tasks/module.py index 088c83b12..8620be979 100644 --- a/modules/background_tasks/background_tasks/module.py +++ b/modules/background_tasks/background_tasks/module.py @@ -98,14 +98,11 @@ def register_audit_links(self, registry: AuditLinkRegistry) -> None: from background_tasks.models import TaskExecution registry.register( - AuditLink( - # Class name, not __tablename__ — see AuditLink.entity_type. - # The table name travels alongside as the audit log's type tag. - entity_type=TaskExecution.__name__, + AuditLink.for_model( + TaskExecution, url_template=f"{VIEW_PREFIX}/{{id}}", label="Task execution", label_key="background_tasks.audit.task_execution", - table_name=TaskExecution.__tablename__, label_resolver=_resolve_execution_labels, ) ) diff --git a/modules/feature_flags/feature_flags/module.py b/modules/feature_flags/feature_flags/module.py index 5598215bd..d4e785f29 100644 --- a/modules/feature_flags/feature_flags/module.py +++ b/modules/feature_flags/feature_flags/module.py @@ -103,14 +103,11 @@ def register_audit_links(self, registry: AuditLinkRegistry) -> None: from feature_flags.models import FeatureFlagOverride registry.register( - AuditLink( - # Class name, not __tablename__ — see AuditLink.entity_type. - # The table name travels alongside as the audit log's type tag. - entity_type=FeatureFlagOverride.__name__, + AuditLink.for_model( + FeatureFlagOverride, url_template=f"{MENU_URL}?{QP_OVERRIDE}={{id}}", label=AUDIT_LINK_LABEL, label_key=AUDIT_LINK_LABEL_KEY, - table_name=FeatureFlagOverride.__tablename__, label_resolver=_resolve_override_labels, ) ) diff --git a/modules/file_storage/file_storage/module.py b/modules/file_storage/file_storage/module.py index 14e7900da..642f52130 100644 --- a/modules/file_storage/file_storage/module.py +++ b/modules/file_storage/file_storage/module.py @@ -124,13 +124,11 @@ def register_audit_links(self, registry: AuditLinkRegistry) -> None: from file_storage.models import StoredFile registry.register( - AuditLink( - # Class name, not __tablename__ — see AuditLink.entity_type. - entity_type=StoredFile.__name__, + AuditLink.for_model( + StoredFile, url_template="", label="File", label_key=f"{constants.LOCALE_NAMESPACE}.audit.file", - table_name=constants.TABLE_STORED_FILE, label_resolver=_resolve_file_labels, ) ) diff --git a/modules/permissions/permissions/module.py b/modules/permissions/permissions/module.py index 2968bb02e..f00c65c52 100644 --- a/modules/permissions/permissions/module.py +++ b/modules/permissions/permissions/module.py @@ -11,6 +11,7 @@ from simple_module_core.audit_links import AuditLink, AuditLinkRegistry from simple_module_core.module import ModuleBase, ModuleMeta from simple_module_core.permissions import PermissionRegistry +from simple_module_db import independent_session from permissions.constants import _MODULE_AUTH, _MODULE_USERS @@ -66,13 +67,11 @@ def register_audit_links(self, registry: AuditLinkRegistry) -> None: from permissions.models import RolePermission registry.register( - AuditLink( - # Class name, not __tablename__ — see AuditLink.entity_type. - entity_type=RolePermission.__name__, + AuditLink.for_model( + RolePermission, url_template="", label="Role permission", label_key="permissions.audit.role_permission", - table_name=RolePermission.__tablename__, label_resolver=_resolve_role_permission_labels, ) ) @@ -103,10 +102,8 @@ async def on_startup(self, app: FastAPI) -> None: """Replay persisted role→permission rows into the live registry.""" from permissions.service import PermissionService - db_state = app.state.sm.db registry = app.state.sm.permissions - async with db_state.session_factory() as db: + async with independent_session(app) as db: service = PermissionService(db, registry) await service.load_all_into_registry() await service.sync_admin_all_permissions() - await db.commit() diff --git a/modules/settings/settings/module.py b/modules/settings/settings/module.py index abbda9876..1ec458e69 100644 --- a/modules/settings/settings/module.py +++ b/modules/settings/settings/module.py @@ -128,14 +128,11 @@ def register_audit_links(self, registry: AuditLinkRegistry) -> None: from simple_module_db.stored_settings import Setting registry.register( - AuditLink( - # Class name, not __tablename__ — see AuditLink.entity_type. - # The table name travels alongside as the audit log's type tag. - entity_type=Setting.__name__, + AuditLink.for_model( + Setting, url_template=f"{VIEW_PREFIX}/{{id}}/edit", label="Setting", label_key="settings.audit.setting", - table_name=Setting.__tablename__, label_resolver=_resolve_setting_labels, ) ) diff --git a/modules/users/users/audit.py b/modules/users/users/audit.py index 54804245c..55403f1ce 100644 --- a/modules/users/users/audit.py +++ b/modules/users/users/audit.py @@ -56,15 +56,11 @@ async def resolve_user_labels(db: AsyncSession, ids: list[str]) -> dict[str, str def build_user_audit_link(admin_url_prefix: str) -> AuditLink: """The registry entry for user rows, rooted at the module's admin route.""" - return AuditLink( - # The model class name — what snapshot_changes records. Keying this - # off __tablename__ ("users_user") silently never matches; the table - # name travels alongside, as the tag shown beside the row's name. - entity_type=User.__name__, + return AuditLink.for_model( + User, url_template=f"{admin_url_prefix}{{id}}", label=_LABEL, label_key=_LABEL_KEY, - table_name=User.__tablename__, label_resolver=resolve_user_labels, # A name here is the same disclosure the user list makes, so it takes # the same permission. Without it, a role holding `audit_log.view` and From ea604dce42d190751d95bdaf410888a35f44b709 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 21 Oct 2026 14:09:26 +0200 Subject: [PATCH 02/12] chore(deps): bound FastAPI below 0.157 with a private-API contract test (#516); file_storage tasks extra and transport-neutral durable docs (#502) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- CHANGELOG.md | 5 + docs/framework/events.md | 4 +- docs/modules/file_storage.md | 12 ++- framework/core/pyproject.toml | 2 +- .../core/simple_module_core/durable_events.py | 10 +- framework/core/simple_module_core/events.py | 8 +- framework/hosting/pyproject.toml | 2 +- .../simple_module_hosting/_route_guard.py | 3 + .../test_route_guard_fastapi_contract.py | 94 +++++++++++++++++++ framework/inertia/pyproject.toml | 2 +- framework/testing/pyproject.toml | 2 +- modules/file_storage/README.md | 9 ++ modules/file_storage/pyproject.toml | 2 + ...t_file_storage_without_background_tasks.py | 40 ++++++++ uv.lock | 14 ++- 15 files changed, 189 insertions(+), 20 deletions(-) create mode 100644 framework/hosting/tests/test_route_guard_fastapi_contract.py create mode 100644 modules/file_storage/tests/test_file_storage_without_background_tasks.py diff --git a/CHANGELOG.md b/CHANGELOG.md index a8cd80048..cfc368fa9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,11 @@ All notable changes to this project are documented in this file. The format is b - `captured_mail` pytest fixture in `simple_module_test` records sent mail. ### Changed +- Bound FastAPI to `>=0.140,<0.157` in framework and module dependencies; a + contract test guards the route guard's private FastAPI/Starlette APIs (#516). +- Declare and document the optional `file_storage[tasks]` extra for the lockstep + background-tasks package; document `DurableTransport` as the durable-event + extension point and verify uploads without background tasks (#502). - The module-settings mechanism moved from the `settings` plugin into the framework (#500): `register_module_settings` / `ModuleSettingsRegistry` to `simple_module_core.module_settings`, hydration (`hydrate_settings`, diff --git a/docs/framework/events.md b/docs/framework/events.md index 4447825ab..95608b9aa 100644 --- a/docs/framework/events.md +++ b/docs/framework/events.md @@ -1,6 +1,6 @@ # Events -`EventBus` is an **in-process** pub/sub mechanism. `publish` is not a message broker — no persistence, no retries, no cross-process delivery. For events that must be handled even across a crash, use [durable delivery](#durable-delivery) (`subscribe_durable` / `publish_durable`), which the `background_tasks` module backs with an outbox. To tell *other worker processes* that something they cached is stale, use the [invalidation bus](/framework/invalidation) instead — same shape, deliberately narrower contract. +`EventBus` is an **in-process** pub/sub mechanism. `publish` is not a message broker — no persistence, no retries, no cross-process delivery. For events that must be handled even across a crash, use [durable delivery](#durable-delivery) (`subscribe_durable` / `publish_durable`), through the `DurableTransport` protocol extension point. The framework ships an outbox implementation in `background_tasks`. To tell *other worker processes* that something they cached is stale, use the [invalidation bus](/framework/invalidation) instead — same shape, deliberately narrower contract. Events are the preferred way for modules to react to **other modules'** actions without creating a direct code dependency. @@ -186,7 +186,7 @@ await bus.publish_durable(OrderPlaced(order_id=order.id, ...), db) `register_event_handlers(bus, None)` — subscribe durably *before* any early `return` taken when `app is None`. -`DurableTransport` is a protocol (`async enqueue(event, handler_names, db)`), so +`DurableTransport` (`simple_module_core.durable_events`) is the extension point: a protocol (`async enqueue(event, handler_names, db)`), so another module could provide one; `background_tasks` is the shipped implementation — see [its docs](/modules/background_tasks#durable-events-outbox) for the outbox, retries, the admin page and the worker side. diff --git a/docs/modules/file_storage.md b/docs/modules/file_storage.md index 2f56bd429..05cafbf51 100644 --- a/docs/modules/file_storage.md +++ b/docs/modules/file_storage.md @@ -2,6 +2,14 @@ Pluggable file storage with two shipped backends — local filesystem and S3-compatible — plus a browse / upload / download admin UI. Backend selection is per-deployment (DB-backed setting), so you can switch from `filesystem` (dev / single-node) to `s3` (prod) without code changes. +Install `simple_module_file_storage[tasks]` to include the matching +`simple_module_background_tasks` package, then enable `background_tasks` and run +its worker for asynchronous multipart-upload checksums. Run its beat scheduler +as well for upload cleanup jobs. +The extras combine as `simple_module_file_storage[s3,tasks]`. Without +`background_tasks`, uploads still work; completed multipart uploads keep +`checksum_sha256` null. Streamed uploads compute their checksum inline. + ## ModuleMeta | Field | Value | @@ -250,8 +258,8 @@ The audit-log label resolver names files across tenants on purpose: the audit log is a platform screen over every tenant's entries, each of which already records the filename. -The module has no background jobs, sweeps or CLI commands. Anything added -later that touches `StoredFile` outside a request must run under +The optional checksum task and upload reaper use the background-tasks worker. +Any task that touches `StoredFile` outside a request must run under `tenant_context(row.tenant_id)` for per-tenant work, or `all_tenants()` for a deliberate platform-wide sweep. diff --git a/framework/core/pyproject.toml b/framework/core/pyproject.toml index c59b1f746..c20d8f6d8 100644 --- a/framework/core/pyproject.toml +++ b/framework/core/pyproject.toml @@ -22,7 +22,7 @@ classifiers = [ ] dependencies = [ "babel>=2.14", - "fastapi>=0.115", + "fastapi>=0.140,<0.157", "packaging>=23.0", "pydantic>=2.0", "pydantic-settings>=2.0", diff --git a/framework/core/simple_module_core/durable_events.py b/framework/core/simple_module_core/durable_events.py index 911c3087a..9dc6f0c13 100644 --- a/framework/core/simple_module_core/durable_events.py +++ b/framework/core/simple_module_core/durable_events.py @@ -5,7 +5,8 @@ written to an outbox in the publisher's own transaction and a worker runs the handler later, retrying until it succeeds. The framework only defines the contract — the subscription registry, the event (de)serialisation and the -transport protocol. ``background_tasks`` ships the transport. +``DurableTransport`` protocol, the extension point for delivery implementations. +``background_tasks`` ships the framework's implementation. Handlers are called as ``await handler(event, ctx)`` where ``ctx`` is a :class:`DurableContext`. Delivery is at-least-once; a handler that writes only @@ -36,9 +37,10 @@ class DurableDeliveryUnavailable(RuntimeError): # noqa: N818 - name fixed by the design """``publish_durable`` was called but no durable transport is installed. - Durable delivery needs an outbox and a worker; the ``background_tasks`` - module installs both. Raised instead of silently degrading to in-process - delivery, which would drop the guarantee the caller asked for. + Durable delivery needs a ``DurableTransport`` implementation; the shipped + ``background_tasks`` module provides an outbox and a worker. Raised instead + of silently degrading to in-process delivery, which would drop the + guarantee the caller asked for. """ diff --git a/framework/core/simple_module_core/events.py b/framework/core/simple_module_core/events.py index e115128f0..a906c2e06 100644 --- a/framework/core/simple_module_core/events.py +++ b/framework/core/simple_module_core/events.py @@ -45,14 +45,15 @@ class EventBus: * ``publish_nowait`` — fire-and-forget via pyee's event-loop scheduling. * ``publish_durable`` — written to an outbox in the caller's transaction and delivered at-least-once by the installed :class:`DurableTransport` - (``background_tasks``) to handlers registered with ``subscribe_durable``. + to handlers registered with ``subscribe_durable``. The framework ships + an implementation in ``background_tasks``. """ def __init__(self) -> None: self._emitter = AsyncIOEventEmitter() self._emitter.on("error", self._on_emitter_error) self.durable = DurableRegistry() - # Installed by the module that owns the outbox (background_tasks). + # DurableTransport is the extension point; background_tasks ships an implementation. self.durable_transport: DurableTransport | None = None @staticmethod @@ -127,7 +128,8 @@ async def publish_durable(self, event: Event, db: Any) -> None: Nothing is delivered unless that transaction commits. In-process ``subscribe`` handlers are **not** called; publish separately if you want both. Raises :class:`DurableDeliveryUnavailable` when no transport - is installed (``background_tasks`` not installed or not started). + is installed. Implement :class:`DurableTransport` or enable the shipped + ``background_tasks`` implementation. """ transport = self.durable_transport if transport is None: diff --git a/framework/hosting/pyproject.toml b/framework/hosting/pyproject.toml index 47138d3b1..235dc836e 100644 --- a/framework/hosting/pyproject.toml +++ b/framework/hosting/pyproject.toml @@ -22,7 +22,7 @@ classifiers = [ ] dependencies = [ "click>=8.1", - "fastapi>=0.115", + "fastapi>=0.140,<0.157", "httpx>=0.27", # SessionMiddleware (starlette.middleware.sessions) imports itsdangerous at # module import time; starlette treats it as an optional extra, so declare diff --git a/framework/hosting/simple_module_hosting/_route_guard.py b/framework/hosting/simple_module_hosting/_route_guard.py index 58a74410a..59ee63027 100644 --- a/framework/hosting/simple_module_hosting/_route_guard.py +++ b/framework/hosting/simple_module_hosting/_route_guard.py @@ -29,6 +29,9 @@ from typing import Any from fastapi import FastAPI + +# Private APIs: see tests/test_route_guard_fastapi_contract.py before widening +# the fastapi>=0.140,<0.157 dependency bound. from starlette._utils import get_route_path from starlette.routing import Match from starlette.types import Scope diff --git a/framework/hosting/tests/test_route_guard_fastapi_contract.py b/framework/hosting/tests/test_route_guard_fastapi_contract.py new file mode 100644 index 000000000..e73978e88 --- /dev/null +++ b/framework/hosting/tests/test_route_guard_fastapi_contract.py @@ -0,0 +1,94 @@ +"""Upgrade contract for the private APIs used by simple_module_hosting._route_guard.""" + +import inspect + +import fastapi.routing as routing +import starlette._utils as starlette_utils +from fastapi import APIRouter, FastAPI +from starlette.routing import Match + +GUARD = "framework/hosting/simple_module_hosting/_route_guard.py" + + +def _accepts(fn, name, *args): + message = f"{name} must be callable with {len(args)} positional arguments; update {GUARD}" + assert callable(fn), message + try: + inspect.signature(fn).bind(*args) + except (TypeError, ValueError) as exc: + raise AssertionError(message) from exc + return fn + + +def test_private_routing_contract(): + included_type = getattr(routing, "_IncludedRouter", None) + assert inspect.isclass(included_type), ( + f"fastapi.routing._IncludedRouter must be a class; update {GUARD}" + ) + get_route_path = getattr(starlette_utils, "get_route_path", None) + scope = {"type": "http", "path": "/api/item", "root_path": "", "method": "GET"} + get_route_path = _accepts(get_route_path, "starlette._utils.get_route_path(scope)", scope) + assert get_route_path(scope) == "/api/item", ( + f"get_route_path must return the route path string; update {GUARD}" + ) + + app = FastAPI() + router = APIRouter() + + @router.get("/api/item") + async def item(): + return {} + + app.include_router(router) + included = next( + (route for route in app.router.routes if isinstance(route, included_type)), None + ) + assert included is not None, f"include_router must create an _IncludedRouter; update {GUARD}" + assert getattr(included, "original_router", None) is router, ( + f"_IncludedRouter.original_router must retain the APIRouter; update {GUARD}" + ) + version = getattr(router, "_get_routes_version", None) + version = _accepts(version, "original_router._get_routes_version()") + assert isinstance(version(), int), f"_get_routes_version() must return an int; update {GUARD}" + + contexts = getattr(included, "effective_route_contexts", None) + contexts = _accepts(contexts, "_IncludedRouter.effective_route_contexts()") + resolved = list(contexts()) + assert resolved, f"effective_route_contexts() must yield route contexts; update {GUARD}" + for ctx in resolved: + assert hasattr(ctx, "starlette_route"), ( + f"route context must expose starlette_route; update {GUARD}" + ) + assert isinstance(getattr(ctx, "path", None), str), ( + f"route context must expose a string path; update {GUARD}" + ) + if ctx.starlette_route is not None: + assert isinstance(getattr(ctx.starlette_route, "path", None), str), ( + f"context.starlette_route must expose a string path; update {GUARD}" + ) + + matches = getattr(included, "matches", None) + matches = _accepts(matches, "_IncludedRouter.matches(scope)", scope) + result = matches(scope) + assert isinstance(result, tuple) and len(result) == 2, ( + f"matches(scope) must return (Match, Scope); update {GUARD}" + ) + assert isinstance(result[0], Match) and isinstance(result[1], dict), ( + f"matches(scope) must return (Match, Scope); update {GUARD}" + ) + + def replacement(scope): + return Match.NONE, {} + + try: + included.matches = replacement + except (AttributeError, TypeError) as exc: + raise AssertionError( + f"_IncludedRouter.matches must allow instance assignment; update {GUARD}" + ) from exc + assert included.matches is replacement, ( + f"_IncludedRouter.matches must retain the assigned callable; update {GUARD}" + ) + assert included.matches(scope) == (Match.NONE, {}), ( + f"assigned matches(scope) must be callable; update {GUARD}" + ) diff --git a/framework/inertia/pyproject.toml b/framework/inertia/pyproject.toml index 1a932e0c5..9f446bb2b 100644 --- a/framework/inertia/pyproject.toml +++ b/framework/inertia/pyproject.toml @@ -19,7 +19,7 @@ classifiers = [ "Typing :: Typed", ] dependencies = [ - "fastapi>=0.115", + "fastapi>=0.140,<0.157", "jinja2>=3.1", "starlette>=0.44", ] diff --git a/framework/testing/pyproject.toml b/framework/testing/pyproject.toml index f9af127bc..70c1ea4db 100644 --- a/framework/testing/pyproject.toml +++ b/framework/testing/pyproject.toml @@ -21,7 +21,7 @@ classifiers = [ "Typing :: Typed", ] dependencies = [ - "fastapi>=0.115", + "fastapi>=0.140,<0.157", "httpx>=0.27", "pytest>=8.0", "pytest-asyncio>=0.24", diff --git a/modules/file_storage/README.md b/modules/file_storage/README.md index b6cca0951..19384b8f2 100644 --- a/modules/file_storage/README.md +++ b/modules/file_storage/README.md @@ -12,6 +12,14 @@ pip install simple_module_file_storage pip install "simple_module_file_storage[s3]" ``` +Install `simple_module_file_storage[tasks]` to include the matching +`simple_module_background_tasks` package, then enable `background_tasks` and run +its worker for asynchronous multipart-upload checksums. Run its beat scheduler +as well for upload cleanup jobs. +The extras combine as `simple_module_file_storage[s3,tasks]`. Without +`background_tasks`, uploads still work; completed multipart uploads keep +`checksum_sha256` null. Streamed uploads compute their checksum inline. + ## What it provides - `POST /api/file-storage/upload` multipart upload endpoint. @@ -45,6 +53,7 @@ Backend and S3 credentials are configured from the admin UI at **/settings/modul - `simple_module_core`, `simple_module_db`, `simple_module_hosting`, `simple_module_settings` - `aiofiles` - Optional: `aioboto3` (install the `[s3]` extra) +- Optional: `simple_module_background_tasks` (install the `[tasks]` extra) ## License diff --git a/modules/file_storage/pyproject.toml b/modules/file_storage/pyproject.toml index da50b180b..8a179dc28 100644 --- a/modules/file_storage/pyproject.toml +++ b/modules/file_storage/pyproject.toml @@ -31,6 +31,7 @@ dependencies = [ [project.optional-dependencies] s3 = ["aioboto3>=13"] +tasks = ["simple_module_background_tasks==0.0.37"] [project.entry-points.simple_module] file_storage = "file_storage.module:FileStorageModule" @@ -58,3 +59,4 @@ simple_module_core = { workspace = true } simple_module_db = { workspace = true } simple_module_hosting = { workspace = true } simple_module_settings = { workspace = true } +simple_module_background_tasks = { workspace = true } diff --git a/modules/file_storage/tests/test_file_storage_without_background_tasks.py b/modules/file_storage/tests/test_file_storage_without_background_tasks.py new file mode 100644 index 000000000..7c63f99e9 --- /dev/null +++ b/modules/file_storage/tests/test_file_storage_without_background_tasks.py @@ -0,0 +1,40 @@ +"""Multipart uploads remain usable without the optional background_tasks integration.""" + +import builtins + +from file_storage_uploads_helpers import API, open_session, upload_all + + +async def test_upload_without_background_tasks(app, authenticated_client, storage, monkeypatch): + monkeypatch.delattr(app.state, "background_tasks", raising=False) + original_import = builtins.__import__ + attempted = [] + + def without_background_tasks(name, *args, **kwargs): + if name == "background_tasks" or name.startswith("background_tasks."): + attempted.append(name) + raise ImportError("background_tasks is not installed") + return original_import(name, *args, **kwargs) + + monkeypatch.setattr(builtins, "__import__", without_background_tasks) + data = b"upload without a worker" + created = await open_session(authenticated_client, total=len(data)) + assert created.status_code == 201, created.text + upload_id = created.json()["id"] + etags = await upload_all(authenticated_client, upload_id, data) + done = await authenticated_client.post( + f"{API}/uploads/{upload_id}/complete", + json={"parts": [{"n": n, "etag": e} for n, e in etags.items()]}, + ) + assert done.status_code == 200, done.text + file = done.json()["file"] + assert file["checksum_sha256"] is None + metadata = await authenticated_client.get(f"{API}/files/{file['id']}") + assert metadata.status_code == 200, metadata.text + assert metadata.json()["checksum_sha256"] is None + download = await authenticated_client.get(f"{API}/files/{file['id']}/download") + assert download.status_code == 200 + assert download.content == data + assert attempted == [], ( + "uploads without app.state.background_tasks must not import background_tasks" + ) diff --git a/uv.lock b/uv.lock index 12d8fe50b..230c56f15 100644 --- a/uv.lock +++ b/uv.lock @@ -3168,7 +3168,7 @@ dependencies = [ [package.metadata] requires-dist = [ { name = "babel", specifier = ">=2.14" }, - { name = "fastapi", specifier = ">=0.115" }, + { name = "fastapi", specifier = ">=0.140,<0.157" }, { name = "packaging", specifier = ">=23.0" }, { name = "pydantic", specifier = ">=2.0" }, { name = "pydantic-settings", specifier = ">=2.0" }, @@ -3253,18 +3253,22 @@ dependencies = [ s3 = [ { name = "aioboto3" }, ] +tasks = [ + { name = "simple-module-background-tasks" }, +] [package.metadata] requires-dist = [ { name = "aioboto3", marker = "extra == 's3'", specifier = ">=13" }, { name = "aiofiles", specifier = ">=23" }, { name = "pillow", specifier = ">=10" }, + { name = "simple-module-background-tasks", marker = "extra == 'tasks'", editable = "modules/background_tasks" }, { name = "simple-module-core", editable = "framework/core" }, { name = "simple-module-db", editable = "framework/db" }, { name = "simple-module-hosting", editable = "framework/hosting" }, { name = "simple-module-settings", editable = "modules/settings" }, ] -provides-extras = ["s3"] +provides-extras = ["s3", "tasks"] [[package]] name = "simple-module-host" @@ -3333,7 +3337,7 @@ redis = [ [package.metadata] requires-dist = [ { name = "click", specifier = ">=8.1" }, - { name = "fastapi", specifier = ">=0.115" }, + { name = "fastapi", specifier = ">=0.140,<0.157" }, { name = "httpx", specifier = ">=0.27" }, { name = "itsdangerous", specifier = ">=2.0" }, { name = "jinja2", specifier = ">=3.1" }, @@ -3359,7 +3363,7 @@ dependencies = [ [package.metadata] requires-dist = [ - { name = "fastapi", specifier = ">=0.115" }, + { name = "fastapi", specifier = ">=0.140,<0.157" }, { name = "jinja2", specifier = ">=3.1" }, { name = "starlette", specifier = ">=0.44" }, ] @@ -3558,7 +3562,7 @@ dependencies = [ [package.metadata] requires-dist = [ - { name = "fastapi", specifier = ">=0.115" }, + { name = "fastapi", specifier = ">=0.140,<0.157" }, { name = "httpx", specifier = ">=0.27" }, { name = "pytest", specifier = ">=8.0" }, { name = "pytest-asyncio", specifier = ">=0.24" }, From 83d6c18d66a4fec57366fd7fd3dc86e08e263fb3 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 21 Oct 2026 14:12:43 +0200 Subject: [PATCH 03/12] feat(pagination): PageParams + paginate helper, PaginationBar, migrate list call sites (#513) Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- CHANGELOG.md | 14 +++ docs/.vitepress/config.ts | 1 + docs/database/pagination.md | 36 ++++++++ framework/db/simple_module_db/__init__.py | 12 +++ framework/db/simple_module_db/pagination.py | 89 +++++++++++++++++++ framework/db/tests/test_db_pagination.py | 75 ++++++++++++++++ .../simple_module_hosting/pagination.py | 60 +++++++++++++ .../simple_module_hosting/ui_locales/en.json | 5 ++ .../simple_module_hosting/ui_locales/es.json | 5 ++ .../tests/test_hosting_pagination_deps.py | 42 +++++++++ .../audit_log/audit_log/endpoints/views.py | 8 +- modules/audit_log/audit_log/locales/en.json | 5 +- modules/audit_log/audit_log/pages/Browse.tsx | 8 +- .../audit_log/pages/components/Pager.tsx | 48 ---------- .../background_tasks/endpoints/views.py | 6 +- .../background_tasks/locales/en.json | 3 - .../background_tasks/outbox_service.py | 28 +++--- .../pages/components/ExecutionsTable.tsx | 35 +------- .../pages/components/OutboxTable.tsx | 33 ++----- .../file_storage/endpoints/views.py | 19 ++-- modules/file_storage/file_storage/queries.py | 4 +- modules/settings/settings/endpoints/views.py | 4 +- modules/tenants/tenants/endpoints/admin.py | 13 ++- modules/users/users/admin/queries.py | 6 +- modules/users/users/admin/views.py | 19 ++-- packages/i18n/src/generated-resources.ts | 9 +- packages/i18n/src/keys.generated.ts | 11 ++- .../ui/src/components/PaginationBar.test.tsx | 57 ++++++++++++ packages/ui/src/components/PaginationBar.tsx | 66 ++++++++++++++ packages/ui/src/index.ts | 1 + 30 files changed, 539 insertions(+), 183 deletions(-) create mode 100644 docs/database/pagination.md create mode 100644 framework/db/simple_module_db/pagination.py create mode 100644 framework/db/tests/test_db_pagination.py create mode 100644 framework/hosting/simple_module_hosting/pagination.py create mode 100644 framework/hosting/tests/test_hosting_pagination_deps.py delete mode 100644 modules/audit_log/audit_log/pages/components/Pager.tsx create mode 100644 packages/ui/src/components/PaginationBar.test.tsx create mode 100644 packages/ui/src/components/PaginationBar.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index a8cd80048..7c51b1f83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,12 @@ All notable changes to this project are documented in this file. The format is b ## [Unreleased] ### Added +- Framework pagination helper (#513): `simple_module_db.PageParams`, `Page` and + `async paginate(session, stmt, params)` (clamps to the last page, never a + negative offset, counts through soft-delete/tenant filters), `clamp_page` / + `total_pages`; `simple_module_hosting.pagination` adds the strict + `page_params(n)` and clamping `lenient_page_params(n)` dependencies; and a + `PaginationBar` component in `@simple-module-py/ui` (`ui.pagination.*` keys). - New `mail` module (`simple_module_mail`) owns outgoing mail: the SMTP/console transport, its settings (`/admin/settings/`, package `mail`) and the on-demand `mail.mailer` health check behind "Test mailer connection". Modules send through @@ -25,6 +31,14 @@ All notable changes to this project are documented in this file. The format is b - `captured_mail` pytest fixture in `simple_module_test` records sent mail. ### Changed +- Users, file_storage, background_tasks (executions and outbox), audit_log, + settings and tenants lists now share the pagination helpers instead of + per-module copies (#513). Browser view routes (`background_tasks`, + `file_storage`, `tenants` admin) still clamp `?page=0`/negative to page 1 + rather than reject; JSON APIs keep `ge=1` and answer 422. The outbox API now + reports the clamped page when asked past the end. The audit log, executions + and outbox tables use `PaginationBar`; their private `showing/previous/next` + locale keys are removed in favour of `ui.pagination.*`. - The module-settings mechanism moved from the `settings` plugin into the framework (#500): `register_module_settings` / `ModuleSettingsRegistry` to `simple_module_core.module_settings`, hydration (`hydrate_settings`, diff --git a/docs/.vitepress/config.ts b/docs/.vitepress/config.ts index c0bbebb25..75b462ca5 100644 --- a/docs/.vitepress/config.ts +++ b/docs/.vitepress/config.ts @@ -141,6 +141,7 @@ export default defineConfig({ { text: "Per-module Base", link: "/database/per-module-base" }, { text: "Mixins", link: "/database/mixins" }, { text: "Session lifecycle", link: "/database/sessions" }, + { text: "Pagination", link: "/database/pagination" }, { text: "Migrations", link: "/database/migrations" }, { text: "Extensions and column types", link: "/database/extensions" }, ], diff --git a/docs/database/pagination.md b/docs/database/pagination.md new file mode 100644 index 000000000..6799813f4 --- /dev/null +++ b/docs/database/pagination.md @@ -0,0 +1,36 @@ +# Pagination + +One helper clamps, counts and slices, so every list agrees on what `?page=0` +or a page past the end means. + +```python +from simple_module_db import PageParams, paginate +from simple_module_hosting.pagination import LenientPageParamsDep, page_params + + +# JSON API: page < 1 or per_page outside 1..200 is a 422. +@router.get("/", response_model=None) +async def list_orders( + params: PageParams = Depends(page_params(default_per_page=50)), + db: AsyncSession = Depends(get_db), +): + page = await paginate(db, select(Order).order_by(Order.id), params) + return page # items, total, page, per_page, pages +``` + +- `PageParams(page, per_page)` lives in `simple_module_db` (no FastAPI import). + It normalises on construction (`page >= 1`, `1 <= per_page <= 200`), so + `.offset` is never negative. `simple_module_hosting.pagination` builds it from + the query string: `page_params(n)` is strict (422), `lenient_page_params(n)` / + `LenientPageParamsDep` clamp garbage instead, for Inertia views reached from + bookmarked URLs. +- `paginate(session, stmt, params, scalars=True)` counts `stmt` as a subquery, + so soft-delete and tenant filters apply to the count exactly as to the rows. + `stmt` needs its own `ORDER BY`. A page past the end is clamped to the last + page and the returned `page` is the one served. Pass `scalars=False` for + multi-column selects (items are rows). +- Queries that cannot use `paginate` (a window-function total, a service that + counts separately) still share `clamp_page(page, total, per_page)`, + `total_pages(total, per_page)` and `PageParams.offset`. +- Frontend: `PaginationBar` from `@simple-module-py/ui/components/PaginationBar` + takes `page`, `perPage`, `total`, `onPageChange` (the `Page` shape). diff --git a/framework/db/simple_module_db/__init__.py b/framework/db/simple_module_db/__init__.py index 41af536df..814fecf85 100644 --- a/framework/db/simple_module_db/__init__.py +++ b/framework/db/simple_module_db/__init__.py @@ -13,6 +13,13 @@ render_item, ) from simple_module_db.mixins import AuditMixin, MultiTenantMixin, SoftDeleteMixin, VersionedMixin +from simple_module_db.pagination import ( + Page, + PageParams, + clamp_page, + paginate, + total_pages, +) from simple_module_db.provider import DatabaseProvider, detect_provider from simple_module_db.search import LIKE_ESCAPE_CHAR, like_contains_pattern, like_prefix_pattern from simple_module_db.session import DatabaseState, RequestSession, init_db @@ -50,6 +57,8 @@ "MissingTenantError", "MultiTenantMixin", "OnCommitCallback", + "Page", + "PageParams", "RequestSession", "SoftDeleteMixin", "TSVector", @@ -59,6 +68,7 @@ "all_tenants", "bind_current_tenant", "build_module_metadata", + "clamp_page", "create_module_base", "current_tenant_id", "detect_provider", @@ -75,6 +85,8 @@ "make_process_revision_directives", "mark_written", "no_tenant", + "paginate", "render_item", "tenant_context", + "total_pages", ] diff --git a/framework/db/simple_module_db/pagination.py b/framework/db/simple_module_db/pagination.py new file mode 100644 index 000000000..19a793cf1 --- /dev/null +++ b/framework/db/simple_module_db/pagination.py @@ -0,0 +1,89 @@ +"""Offset pagination: one place that clamps, counts and slices. + +``PageParams`` is a plain value (no FastAPI import, so the DB package stays +web-free); ``simple_module_hosting.pagination`` builds it from query strings. +``paginate`` counts the *same statement* as a subquery, so soft-delete and +tenant filters apply to the count exactly as they do to the rows. +""" + +from __future__ import annotations + +import math +from dataclasses import dataclass +from typing import TYPE_CHECKING, Any + +from sqlalchemy import func, select +from sqlmodel import SQLModel + +if TYPE_CHECKING: + from sqlalchemy.ext.asyncio import AsyncSession + from sqlalchemy.sql import Select + + +DEFAULT_PER_PAGE = 25 +MAX_PER_PAGE = 200 + + +def total_pages(total: int, per_page: int) -> int: + """Number of pages for ``total`` rows; always at least 1 (an empty list is page 1).""" + return max(1, math.ceil(max(total, 0) / max(per_page, 1))) + + +def clamp_page(page: int, total: int, per_page: int) -> int: + """``page`` limited to ``1..total_pages`` so a stale link lands on the last real page.""" + return min(max(page, 1), total_pages(total, per_page)) + + +@dataclass(frozen=True, slots=True) +class PageParams: + """A requested page. Values are normalised on construction, never trusted.""" + + page: int = 1 + per_page: int = DEFAULT_PER_PAGE + + def __post_init__(self) -> None: + object.__setattr__(self, "page", max(int(self.page), 1)) + object.__setattr__(self, "per_page", max(1, min(int(self.per_page), MAX_PER_PAGE))) + + @property + def offset(self) -> int: + """SQL OFFSET for the requested page; never negative.""" + return (self.page - 1) * self.per_page + + +class Page[T](SQLModel): + """One page of results. ``page`` is the page actually served (after clamping).""" + + items: list[T] + total: int + page: int + per_page: int + pages: int + + +async def paginate( + session: AsyncSession, + stmt: Select[Any], + params: PageParams, + *, + scalars: bool = True, +) -> Page[Any]: + """Run ``stmt`` one page at a time. + + ``stmt`` must carry its own ``ORDER BY``. A page past the end is clamped to + the last page, so the client never gets an empty list with a non-zero + total. ``scalars=False`` returns row objects for multi-column selects. + """ + count_stmt = select(func.count()).select_from(stmt.order_by(None).subquery()) + total = int((await session.execute(count_stmt)).scalar_one()) + page = clamp_page(params.page, total, params.per_page) + paged = stmt.offset((page - 1) * params.per_page).limit(params.per_page) + result = await session.execute(paged) + items = list(result.scalars().all() if scalars else result.all()) + return Page( + items=items, + total=total, + page=page, + per_page=params.per_page, + pages=total_pages(total, params.per_page), + ) diff --git a/framework/db/tests/test_db_pagination.py b/framework/db/tests/test_db_pagination.py new file mode 100644 index 000000000..bb800f357 --- /dev/null +++ b/framework/db/tests/test_db_pagination.py @@ -0,0 +1,75 @@ +"""paginate / PageParams: clamping, counting through filters, no negative offset.""" + +from __future__ import annotations + +import pytest +from simple_module_db import PageParams, clamp_page, paginate, total_pages +from sqlmodel import Field, SQLModel, select + + +class _Item(SQLModel, table=True): + __tablename__ = "pagination_test_item" + __table_args__ = {"extend_existing": True} + id: int | None = Field(default=None, primary_key=True) + n: int = 0 + + +def test_params_never_negative_offset(): + assert PageParams(page=0).offset == 0 + assert PageParams(page=-5, per_page=10).offset == 0 + assert PageParams(page=3, per_page=10).offset == 20 + + +def test_params_clamp_per_page(): + assert PageParams(per_page=0).per_page == 1 + assert PageParams(per_page=10_000).per_page == 200 + + +def test_total_pages_and_clamp(): + assert total_pages(0, 10) == 1 + assert total_pages(21, 10) == 3 + assert clamp_page(99, 21, 10) == 3 + assert clamp_page(0, 21, 10) == 1 + + +@pytest.fixture +async def seeded(db_session): + from sqlalchemy import text # noqa: F401 + + conn = await db_session.connection() + await conn.run_sync(lambda c: _Item.__table__.create(c, checkfirst=True)) + db_session.add_all([_Item(n=i) for i in range(23)]) + await db_session.flush() + return db_session + + +async def test_paginate_slices_and_counts(seeded): + stmt = select(_Item).order_by(_Item.n) + page = await paginate(seeded, stmt, PageParams(page=2, per_page=10)) + assert (page.total, page.page, page.pages, page.per_page) == (23, 2, 3, 10) + assert [i.n for i in page.items] == list(range(10, 20)) + + +async def test_paginate_clamps_past_the_end(seeded): + stmt = select(_Item).order_by(_Item.n) + page = await paginate(seeded, stmt, PageParams(page=50, per_page=10)) + assert page.page == 3 + assert [i.n for i in page.items] == [20, 21, 22] + + +async def test_paginate_count_follows_filters(seeded): + stmt = select(_Item).where(_Item.n < 5).order_by(_Item.n) + page = await paginate(seeded, stmt, PageParams(per_page=2)) + assert (page.total, page.pages, len(page.items)) == (5, 3, 2) + + +async def test_paginate_empty(seeded): + stmt = select(_Item).where(_Item.n > 1000).order_by(_Item.n) + page = await paginate(seeded, stmt, PageParams(page=4)) + assert (page.total, page.page, page.pages, page.items) == (0, 1, 1, []) + + +async def test_paginate_rows(seeded): + stmt = select(_Item.n).order_by(_Item.n) + page = await paginate(seeded, stmt, PageParams(per_page=3), scalars=False) + assert [r.n for r in page.items] == [0, 1, 2] diff --git a/framework/hosting/simple_module_hosting/pagination.py b/framework/hosting/simple_module_hosting/pagination.py new file mode 100644 index 000000000..c134083b8 --- /dev/null +++ b/framework/hosting/simple_module_hosting/pagination.py @@ -0,0 +1,60 @@ +"""Query-string dependencies that build a ``simple_module_db.PageParams``. + +Two flavours, because API and browser routes fail differently: + +* ``PageParamsDep`` — strict. ``page < 1`` or ``per_page`` out of range is a + 422, for JSON APIs whose callers should fix their request. +* ``LenientPageParamsDep`` — for Inertia views reached from URLs people edit + and bookmark. Garbage and out-of-range values clamp instead of rejecting. +""" + +from __future__ import annotations + +from typing import Annotated + +from fastapi import Depends, Query +from simple_module_db.pagination import DEFAULT_PER_PAGE, MAX_PER_PAGE, PageParams + + +def page_params(default_per_page: int = DEFAULT_PER_PAGE): + """Strict dependency factory: ``Depends(page_params(50))`` for a 50-row default.""" + + def dependency( + page: int = Query(1, ge=1), + per_page: int = Query(default_per_page, ge=1, le=MAX_PER_PAGE), + ) -> PageParams: + return PageParams(page=page, per_page=per_page) + + return dependency + + +def _lenient_int(raw: str | None, fallback: int) -> int: + try: + return int(raw) if raw is not None else fallback + except ValueError: + return fallback + + +def lenient_page_params(default_per_page: int = DEFAULT_PER_PAGE): + """Clamping dependency factory for browser-facing view routes.""" + + def dependency( + page: str | None = Query(None), + per_page: str | None = Query(None), + ) -> PageParams: + return PageParams( + page=_lenient_int(page, 1), per_page=_lenient_int(per_page, default_per_page) + ) + + return dependency + + +PageParamsDep = Annotated[PageParams, Depends(page_params())] +LenientPageParamsDep = Annotated[PageParams, Depends(lenient_page_params())] + +__all__ = [ + "LenientPageParamsDep", + "PageParamsDep", + "lenient_page_params", + "page_params", +] diff --git a/framework/hosting/simple_module_hosting/ui_locales/en.json b/framework/hosting/simple_module_hosting/ui_locales/en.json index bbc3b2f95..9fabe32d4 100644 --- a/framework/hosting/simple_module_hosting/ui_locales/en.json +++ b/framework/hosting/simple_module_hosting/ui_locales/en.json @@ -101,5 +101,10 @@ "unauthorized_title": "Sign in required", "unavailable_description": "The service is temporarily unavailable. Please try again shortly.", "unavailable_title": "Service unavailable" + }, + "pagination": { + "showing": "Showing {from}–{to} of {total}", + "previous": "Previous", + "next": "Next" } } diff --git a/framework/hosting/simple_module_hosting/ui_locales/es.json b/framework/hosting/simple_module_hosting/ui_locales/es.json index 211dde46a..0ade328df 100644 --- a/framework/hosting/simple_module_hosting/ui_locales/es.json +++ b/framework/hosting/simple_module_hosting/ui_locales/es.json @@ -101,5 +101,10 @@ "unauthorized_title": "Inicio de sesión requerido", "unavailable_description": "El servicio no está disponible temporalmente. Inténtalo de nuevo en unos momentos.", "unavailable_title": "Servicio no disponible" + }, + "pagination": { + "showing": "Mostrando {from}–{to} de {total}", + "previous": "Anterior", + "next": "Siguiente" } } diff --git a/framework/hosting/tests/test_hosting_pagination_deps.py b/framework/hosting/tests/test_hosting_pagination_deps.py new file mode 100644 index 000000000..dfe56d031 --- /dev/null +++ b/framework/hosting/tests/test_hosting_pagination_deps.py @@ -0,0 +1,42 @@ +"""Strict (422) and lenient (clamp) page-param dependencies.""" + +from __future__ import annotations + +from fastapi import Depends, FastAPI +from fastapi.testclient import TestClient +from simple_module_db import PageParams +from simple_module_hosting.pagination import lenient_page_params, page_params + +app = FastAPI() + + +@app.get("/strict") +def strict(p: PageParams = Depends(page_params(50))) -> dict[str, int]: + return {"page": p.page, "per_page": p.per_page, "offset": p.offset} + + +@app.get("/lenient") +def lenient(p: PageParams = Depends(lenient_page_params(20))) -> dict[str, int]: + return {"page": p.page, "per_page": p.per_page, "offset": p.offset} + + +client = TestClient(app) + + +def test_strict_defaults_and_factory_page_size(): + assert client.get("/strict").json() == {"page": 1, "per_page": 50, "offset": 0} + + +def test_strict_rejects_out_of_range(): + for q in ("page=0", "page=-1", "per_page=0", "per_page=201", "page=abc"): + assert client.get(f"/strict?{q}").status_code == 422, q + + +def test_lenient_clamps_instead_of_rejecting(): + assert client.get("/lenient?page=-4&per_page=9999").json() == { + "page": 1, + "per_page": 200, + "offset": 0, + } + assert client.get("/lenient?page=banana").json()["page"] == 1 + assert client.get("/lenient").json()["per_page"] == 20 diff --git a/modules/audit_log/audit_log/endpoints/views.py b/modules/audit_log/audit_log/endpoints/views.py index 0c546f686..93b3826b2 100644 --- a/modules/audit_log/audit_log/endpoints/views.py +++ b/modules/audit_log/audit_log/endpoints/views.py @@ -2,12 +2,12 @@ from __future__ import annotations -import math from datetime import datetime from fastapi import APIRouter, Depends, Query, Request from simple_module_core.audit_links import AuditLinkRegistry from simple_module_db.deps import get_db +from simple_module_db.pagination import clamp_page from simple_module_hosting.i18n_deps import TranslatorDep from simple_module_hosting.inertia_deps import InertiaDep from simple_module_hosting.permissions import RequiresPermission, resolved_permissions_for @@ -108,9 +108,9 @@ async def browse( # and re-queried — otherwise the client gets an empty `items` list with a # nonzero `total`, which renders the correlation banner ("N entries") right # above an empty-state saying nothing matches. - total_pages = max(1, math.ceil(result.total / page_size_int)) - if page_int > total_pages: - page_int = total_pages + clamped = clamp_page(page_int, result.total, page_size_int) + if clamped != page_int: + page_int = clamped result = await service.list_filtered(filters, page=page_int, page_size=page_size_int) # Resolve ids for display only — the stored row keeps the bare id, which diff --git a/modules/audit_log/audit_log/locales/en.json b/modules/audit_log/audit_log/locales/en.json index 1becc13f8..1f5d7cff7 100644 --- a/modules/audit_log/audit_log/locales/en.json +++ b/modules/audit_log/audit_log/locales/en.json @@ -7,10 +7,7 @@ "empty_description": "Changes to entities will appear here automatically.", "no_match_title": "No entries match these filters", "no_match_description": "Entries exist outside the current filters.", - "clear_filters": "Clear filters", - "showing": "Showing {from}–{to} of {total}", - "previous": "Previous", - "next": "Next" + "clear_filters": "Clear filters" }, "filters": { "entity_type_label": "Entity type", diff --git a/modules/audit_log/audit_log/pages/Browse.tsx b/modules/audit_log/audit_log/pages/Browse.tsx index cffb5620c..c32325eeb 100644 --- a/modules/audit_log/audit_log/pages/Browse.tsx +++ b/modules/audit_log/audit_log/pages/Browse.tsx @@ -1,6 +1,7 @@ import { Head, router, usePage } from '@inertiajs/react'; import { keys, useT } from '@simple-module-py/i18n'; import { PageShell } from '@simple-module-py/ui/components/PageShell'; +import { PaginationBar } from '@simple-module-py/ui/components/PaginationBar'; import { Button } from '@simple-module-py/ui/components/ui/button'; import { Card } from '@simple-module-py/ui/components/ui/card'; import { AdminLayout } from '@simple-module-py/ui/layouts/AdminLayout'; @@ -16,7 +17,6 @@ import { FilterBar, type FilterState, } from './components/FilterBar'; -import { Pager } from './components/Pager'; interface Props { items: AuditEntryRead[]; @@ -174,11 +174,11 @@ function Browse() { /> )} - navigate(state, next)} + onPageChange={(next) => navigate(state, next)} /> diff --git a/modules/audit_log/audit_log/pages/components/Pager.tsx b/modules/audit_log/audit_log/pages/components/Pager.tsx deleted file mode 100644 index 86df607d9..000000000 --- a/modules/audit_log/audit_log/pages/components/Pager.tsx +++ /dev/null @@ -1,48 +0,0 @@ -import { keys, useT } from '@simple-module-py/i18n'; -import { Button } from '@simple-module-py/ui/components/ui/button'; - -interface Props { - page: number; - pageSize: number; - total: number; - onPage: (page: number) => void; -} - -/** The table footer: which slice is on screen, and the way to the next one. - * - * Always rendered, one page or forty: the range is how a reader checks the - * filter matched what they expected, and "Showing 0–0 of 0" is the honest - * answer when it matched nothing. - */ -export function Pager({ page, pageSize, total, onPage }: Props) { - const { t } = useT(); - const totalPages = Math.max(1, Math.ceil(total / pageSize)); - const from = total === 0 ? 0 : (page - 1) * pageSize + 1; - const to = Math.min(page * pageSize, total); - - return ( -
- {t(keys.audit_log.browse.showing, { from, to, total: total.toLocaleString() })} -
- - -
-
- ); -} diff --git a/modules/background_tasks/background_tasks/endpoints/views.py b/modules/background_tasks/background_tasks/endpoints/views.py index faddc7e78..909ae0ed3 100644 --- a/modules/background_tasks/background_tasks/endpoints/views.py +++ b/modules/background_tasks/background_tasks/endpoints/views.py @@ -6,7 +6,9 @@ import uuid from fastapi import APIRouter, Depends, HTTPException, Query, Request +from simple_module_db.pagination import PageParams from simple_module_hosting.inertia_deps import InertiaDep +from simple_module_hosting.pagination import lenient_page_params from simple_module_hosting.permissions import RequiresPermission from simple_module_inertia import InertiaResponse @@ -56,7 +58,7 @@ async def index( task_name: str = Query(default="", alias="q"), queue: str = Query(default=""), tenant_id: str = Query(default="", alias="tenant"), - page: int = Query(default=1), + params: PageParams = Depends(lenient_page_params(PER_PAGE)), service: BackgroundTaskService = Depends(get_background_task_service), ) -> InertiaResponse: response = await service.list( @@ -64,7 +66,7 @@ async def index( task_name=task_name or None, queue=queue or None, tenant_id=tenant_id or None, - page=page, + page=params.page, per_page=PER_PAGE, ) # A filtered-empty list says nothing about the fleet, so don't pay to poll diff --git a/modules/background_tasks/background_tasks/locales/en.json b/modules/background_tasks/background_tasks/locales/en.json index 814789c7e..9f8118f21 100644 --- a/modules/background_tasks/background_tasks/locales/en.json +++ b/modules/background_tasks/background_tasks/locales/en.json @@ -5,9 +5,6 @@ "search_placeholder": "Search by task name…", "workers_button": "Workers", "retry_all_button": "Retry all failed", - "previous": "Previous", - "next": "Next", - "showing": "Showing {from}–{to} of {total}", "outbox_button": "Outbox" }, "strip": { diff --git a/modules/background_tasks/background_tasks/outbox_service.py b/modules/background_tasks/background_tasks/outbox_service.py index c9b5367ec..28137b0a5 100644 --- a/modules/background_tasks/background_tasks/outbox_service.py +++ b/modules/background_tasks/background_tasks/outbox_service.py @@ -7,6 +7,7 @@ from typing import TYPE_CHECKING, Any from fastapi import HTTPException, status +from simple_module_db.pagination import PageParams, paginate from sqlalchemy import delete, false, func, select, update from background_tasks.constants import OutboxStatus @@ -61,23 +62,18 @@ async def list( ) -> OutboxListResponse: statuses = [status_filter] if status_filter else list(OPEN_STATUSES) where = [OutboxMessage.status.in_(statuses), *self.scope.conditions()] - total = ( - await self.db.execute(select(func.count()).select_from(OutboxMessage).where(*where)) - ).scalar_one() - rows = ( - await self.db.execute( - select(OutboxMessage) - .where(*where) - .order_by(OutboxMessage.created_at.desc(), OutboxMessage.id) # ty: ignore[invalid-argument-type] - .offset((page - 1) * per_page) - .limit(per_page) - ) - ).scalars() + found = await paginate( + self.db, + select(OutboxMessage) + .where(*where) + .order_by(OutboxMessage.created_at.desc(), OutboxMessage.id), # ty: ignore[invalid-argument-type] + PageParams(page=page, per_page=per_page), + ) return OutboxListResponse( - items=[OutboxItem.model_validate(r) for r in rows], - total=total, - page=page, - per_page=per_page, + items=[OutboxItem.model_validate(r) for r in found.items], + total=found.total, + page=found.page, + per_page=found.per_page, counts=await self.counts(), ) diff --git a/modules/background_tasks/background_tasks/pages/components/ExecutionsTable.tsx b/modules/background_tasks/background_tasks/pages/components/ExecutionsTable.tsx index 7583cd83d..06346de96 100644 --- a/modules/background_tasks/background_tasks/pages/components/ExecutionsTable.tsx +++ b/modules/background_tasks/background_tasks/pages/components/ExecutionsTable.tsx @@ -1,5 +1,5 @@ import { keys, useT } from '@simple-module-py/i18n'; -import { Button } from '@simple-module-py/ui/components/ui/button'; +import { PaginationBar } from '@simple-module-py/ui/components/PaginationBar'; import { Card } from '@simple-module-py/ui/components/ui/card'; import { Table, @@ -49,9 +49,6 @@ export function ExecutionsTable({ empty, }: Props) { const { t } = useT(); - const totalPages = Math.max(1, Math.ceil(total / perPage)); - const from = total === 0 ? 0 : (page - 1) * perPage + 1; - const to = Math.min(page * perPage, total); return ( @@ -89,35 +86,7 @@ export function ExecutionsTable({ {executions.length === 0 && empty} -
- - {t(keys.background_tasks.index.showing, { - from, - to, - total: total.toLocaleString(), - })} - -
- - -
-
+ ); } diff --git a/modules/background_tasks/background_tasks/pages/components/OutboxTable.tsx b/modules/background_tasks/background_tasks/pages/components/OutboxTable.tsx index 79677fe6b..84c24ea77 100644 --- a/modules/background_tasks/background_tasks/pages/components/OutboxTable.tsx +++ b/modules/background_tasks/background_tasks/pages/components/OutboxTable.tsx @@ -1,4 +1,5 @@ import { keys, useT } from '@simple-module-py/i18n'; +import { PaginationBar } from '@simple-module-py/ui/components/PaginationBar'; import { Button } from '@simple-module-py/ui/components/ui/button'; import { Card } from '@simple-module-py/ui/components/ui/card'; import { @@ -43,9 +44,6 @@ function errorSummary(error: string | null): string { export function OutboxTable(props: Props) { const { messages, canManage, busyId, onRetry, onDiscard, page, perPage, total } = props; const { t } = useT(); - const totalPages = Math.max(1, Math.ceil(total / perPage)); - const from = total === 0 ? 0 : (page - 1) * perPage + 1; - const to = Math.min(page * perPage, total); return ( @@ -145,29 +143,12 @@ export function OutboxTable(props: Props) { )} -
- - {t(keys.background_tasks.index.showing, { from, to, total: total.toLocaleString() })} - -
- - -
-
+ ); } diff --git a/modules/file_storage/file_storage/endpoints/views.py b/modules/file_storage/file_storage/endpoints/views.py index 1b63a9ed9..a1853e8f2 100644 --- a/modules/file_storage/file_storage/endpoints/views.py +++ b/modules/file_storage/file_storage/endpoints/views.py @@ -2,11 +2,11 @@ from __future__ import annotations -import math - -from fastapi import APIRouter, Depends, Query +from fastapi import APIRouter, Depends from simple_module_db.deps import get_db +from simple_module_db.pagination import PageParams, clamp_page from simple_module_hosting.inertia_deps import InertiaDep +from simple_module_hosting.pagination import lenient_page_params from simple_module_hosting.permissions import RequiresPermission from simple_module_inertia import InertiaResponse from sqlalchemy.ext.asyncio import AsyncSession @@ -30,7 +30,7 @@ ) async def browse( inertia: InertiaDep, - page: int = Query(default=1), + params: PageParams = Depends(lenient_page_params(_PER_PAGE)), q: str = "", content_type: str = "", uploaded_by: str = "", @@ -38,11 +38,9 @@ async def browse( service: FileStorageService = Depends(get_file_storage_service), owner: str | None = OwnerScope, ) -> InertiaResponse: - # A user-supplied page below 1 (0, negative, a hand-edited ?page=) must - # clamp to the first page rather than reject — mirrors the past-the-end - # clamp below so no value of ?page= can produce a raw validation-error - # page for a browser-facing route. - page = max(page, 1) + # ``lenient_page_params`` clamps a page below 1 (0, negative, a hand-edited + # ?page=) to the first page, so no ?page= value can produce a raw + # validation-error page for a browser-facing route. filters = { "search": q or None, "content_type": content_type or None, @@ -58,8 +56,7 @@ async def browse( # for the rows first meant fetching, and throwing away, a page nobody would # ever see; the total is what decides which page to fetch, so it goes first. total = await service.count_files(**filters) - total_pages = max(1, math.ceil(total / _PER_PAGE)) - page = min(page, total_pages) + page = clamp_page(params.page, total, _PER_PAGE) items = await service.page_of_files(page=page, per_page=_PER_PAGE, **filters) # The bucket-wide numbers — usage and both facet lists — come from one # scan, cached with a short TTL. They ignore the active filters so the diff --git a/modules/file_storage/file_storage/queries.py b/modules/file_storage/file_storage/queries.py index 4d035025c..c522661fc 100644 --- a/modules/file_storage/file_storage/queries.py +++ b/modules/file_storage/file_storage/queries.py @@ -16,6 +16,7 @@ from urllib.parse import quote from simple_module_db import LIKE_ESCAPE_CHAR, like_contains_pattern, like_prefix_pattern +from simple_module_db.pagination import PageParams from sqlalchemy import Select, func, select from sqlalchemy.ext.asyncio import AsyncSession @@ -116,8 +117,9 @@ async def page_of_files( query = select(StoredFile) for clause in filter_clauses(created_by=created_by, search=search, content_type=content_type): query = query.where(clause) + params = PageParams(page=page, per_page=per_page) result = await db.execute( - query.order_by(*order_clauses(sort)).offset((page - 1) * per_page).limit(per_page) + query.order_by(*order_clauses(sort)).offset(params.offset).limit(params.per_page) ) return [StoredFileOut.model_validate(to_out_dict(r)) for r in result.scalars().all()] diff --git a/modules/settings/settings/endpoints/views.py b/modules/settings/settings/endpoints/views.py index 55b60d1f4..6b5c35599 100644 --- a/modules/settings/settings/endpoints/views.py +++ b/modules/settings/settings/endpoints/views.py @@ -10,10 +10,10 @@ from __future__ import annotations import asyncio -import math from fastapi import APIRouter, Depends, HTTPException, Request from pydantic import ValidationError +from simple_module_db.pagination import clamp_page from simple_module_hosting.i18n_deps import TranslatorDep from simple_module_hosting.inertia_deps import InertiaDep from simple_module_hosting.inertia_utils import redirect_back_with_errors, validation_errors_to_dict @@ -107,7 +107,7 @@ async def browse( # A page past the end (stale ?page= link, or the last row on it was just # deleted) is clamped and re-queried — otherwise the client gets an empty # list with a nonzero total and renders a blank table. - number = min(query.page, max(1, math.ceil(total / query.per_page))) + number = clamp_page(query.page, total, query.per_page) if number != query.page: items, total = await service.list_filtered( query.scope_filter, query.q, number, query.per_page diff --git a/modules/tenants/tenants/endpoints/admin.py b/modules/tenants/tenants/endpoints/admin.py index 4acacc5f5..2b092392c 100644 --- a/modules/tenants/tenants/endpoints/admin.py +++ b/modules/tenants/tenants/endpoints/admin.py @@ -7,7 +7,9 @@ from __future__ import annotations from fastapi import APIRouter, Depends +from simple_module_db.pagination import PageParams from simple_module_hosting.inertia_deps import InertiaDep +from simple_module_hosting.pagination import lenient_page_params from simple_module_hosting.permissions import RequiresPermission from simple_module_inertia import InertiaResponse @@ -25,10 +27,13 @@ @views.get("/", response_model=None, dependencies=_VIEW) async def browse( - inertia: InertiaDep, service: TenantServiceDep, q: str = "", page: int = 1 + inertia: InertiaDep, + service: TenantServiceDep, + q: str = "", + params: PageParams = Depends(lenient_page_params(_PAGE_SIZE)), ) -> InertiaResponse: - page = max(page, 1) - tenants = await service.list_all(search=q, limit=_PAGE_SIZE, offset=(page - 1) * _PAGE_SIZE) + page = params.page + tenants = await service.list_all(search=q, limit=params.per_page, offset=params.offset) counts = await service.member_counts([t.id for t in tenants]) return await inertia.render( PAGE_ADMIN, @@ -42,7 +47,7 @@ async def browse( ], "q": q, "page": page, - "has_more": len(tenants) == _PAGE_SIZE, + "has_more": len(tenants) == params.per_page, }, ) diff --git a/modules/users/users/admin/queries.py b/modules/users/users/admin/queries.py index 79db143bc..1dc494244 100644 --- a/modules/users/users/admin/queries.py +++ b/modules/users/users/admin/queries.py @@ -7,6 +7,7 @@ from typing import Any, cast from simple_module_db import LIKE_ESCAPE_CHAR, like_contains_pattern +from simple_module_db.pagination import PageParams from sqlalchemy import func, or_, select from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.orm import selectinload @@ -132,8 +133,7 @@ async def list_users( """ # Clamp here, not in the endpoints: the admin view takes these params # raw, and a page<=0 would reach SQL as a negative OFFSET. - page = max(page, 1) - per_page = max(1, min(per_page, 200)) + params = PageParams(page=page, per_page=per_page) stmt = select( User.id, # ty: ignore[invalid-argument-type] User.email, # ty: ignore[invalid-argument-type] @@ -197,7 +197,7 @@ async def list_users( else: order_clause = sort_col.desc() if order == "desc" else sort_col.asc() - stmt = stmt.order_by(order_clause).offset((page - 1) * per_page).limit(per_page) + stmt = stmt.order_by(order_clause).offset(params.offset).limit(params.per_page) rows = (await self._db.execute(stmt)).all() # One batched roles query, grouped in Python — DB-agnostic (no diff --git a/modules/users/users/admin/views.py b/modules/users/users/admin/views.py index c964a708a..33f16e423 100644 --- a/modules/users/users/admin/views.py +++ b/modules/users/users/admin/views.py @@ -2,11 +2,11 @@ from __future__ import annotations -import math import uuid from fastapi import APIRouter, Depends, HTTPException, Request from simple_module_db.deps import get_db +from simple_module_db.pagination import PageParams, clamp_page from simple_module_hosting.i18n_deps import TranslatorDep from simple_module_hosting.inertia_deps import InertiaDep from simple_module_hosting.permissions import RequiresPermission @@ -59,11 +59,11 @@ async def admin_index( clean_status, clean_verified, clean_sort, clean_order = sanitize_list_filters( status, verified, sort, order ) - # Clamp before building filters, with the same bounds the service applies - # internally — the pagination prop must echo the values the query actually - # ran with, or ?page=0 renders page-1 rows labelled "Page 0". - page = max(page, 1) - per_page = max(1, min(per_page, 200)) + # Normalise before building filters, with the same bounds the service + # applies internally — the pagination prop must echo the values the query + # actually ran with, or ?page=0 renders page-1 rows labelled "Page 0". + params = PageParams(page=page, per_page=per_page) + page, per_page = params.page, params.per_page filters = { "per_page": per_page, "search": q, @@ -78,10 +78,9 @@ async def admin_index( # that page just got deleted) must be clamped and re-queried — otherwise # the client gets an empty `users` list with a nonzero `total`, which # renders a blank table instead of the last real page. - # per_page is already clamped to >= 1 above, so this can't divide by zero. - total_pages = max(1, math.ceil(total / per_page)) - if page > total_pages: - page = total_pages + clamped = clamp_page(page, total, per_page) + if clamped != page: + page = clamped users, total = await service.list_users(page=page, **filters) roles = await service.list_roles() aggregates = await service.count_user_states() diff --git a/packages/i18n/src/generated-resources.ts b/packages/i18n/src/generated-resources.ts index c11dfa611..61ee34058 100644 --- a/packages/i18n/src/generated-resources.ts +++ b/packages/i18n/src/generated-resources.ts @@ -87,11 +87,8 @@ export default { 'audit_log.browse.empty_description': '', 'audit_log.browse.empty_title': '', 'audit_log.browse.export_csv': '', - 'audit_log.browse.next': '', 'audit_log.browse.no_match_description': '', 'audit_log.browse.no_match_title': '', - 'audit_log.browse.previous': '', - 'audit_log.browse.showing': '', 'audit_log.browse.title': '', 'audit_log.changes.fields_set': '', 'audit_log.changes.fields_set_one': '', @@ -168,12 +165,9 @@ export default { 'background_tasks.filters.tenant_label': '', 'background_tasks.filters.tenant_platform': '', 'background_tasks.index.description': '', - 'background_tasks.index.next': '', 'background_tasks.index.outbox_button': '', - 'background_tasks.index.previous': '', 'background_tasks.index.retry_all_button': '', 'background_tasks.index.search_placeholder': '', - 'background_tasks.index.showing': '', 'background_tasks.index.title': '', 'background_tasks.index.workers_button': '', 'background_tasks.nav.background_tasks': '', @@ -995,6 +989,9 @@ export default { 'ui.nav_groups.content': '', 'ui.nav_groups.navigation': '', 'ui.nav_groups.system': '', + 'ui.pagination.next': '', + 'ui.pagination.previous': '', + 'ui.pagination.showing': '', 'ui.public_nav.close_menu': '', 'ui.public_nav.docs': '', 'ui.public_nav.github': '', diff --git a/packages/i18n/src/keys.generated.ts b/packages/i18n/src/keys.generated.ts index 9c08f1c56..fab915734 100644 --- a/packages/i18n/src/keys.generated.ts +++ b/packages/i18n/src/keys.generated.ts @@ -122,11 +122,8 @@ export const keys = { empty_description: 'audit_log.browse.empty_description', empty_title: 'audit_log.browse.empty_title', export_csv: 'audit_log.browse.export_csv', - next: 'audit_log.browse.next', no_match_description: 'audit_log.browse.no_match_description', no_match_title: 'audit_log.browse.no_match_title', - previous: 'audit_log.browse.previous', - showing: 'audit_log.browse.showing', title: 'audit_log.browse.title', }, changes: { @@ -228,12 +225,9 @@ export const keys = { }, index: { description: 'background_tasks.index.description', - next: 'background_tasks.index.next', outbox_button: 'background_tasks.index.outbox_button', - previous: 'background_tasks.index.previous', retry_all_button: 'background_tasks.index.retry_all_button', search_placeholder: 'background_tasks.index.search_placeholder', - showing: 'background_tasks.index.showing', title: 'background_tasks.index.title', workers_button: 'background_tasks.index.workers_button', }, @@ -1271,6 +1265,11 @@ export const keys = { navigation: 'ui.nav_groups.navigation', system: 'ui.nav_groups.system', }, + pagination: { + next: 'ui.pagination.next', + previous: 'ui.pagination.previous', + showing: 'ui.pagination.showing', + }, public_nav: { close_menu: 'ui.public_nav.close_menu', docs: 'ui.public_nav.docs', diff --git a/packages/ui/src/components/PaginationBar.test.tsx b/packages/ui/src/components/PaginationBar.test.tsx new file mode 100644 index 000000000..57d7dcf7e --- /dev/null +++ b/packages/ui/src/components/PaginationBar.test.tsx @@ -0,0 +1,57 @@ +import { fireEvent, render, screen } from '@testing-library/react'; +import { describe, expect, test, vi } from 'vitest'; + +vi.mock('@simple-module-py/i18n', () => ({ + useT: () => ({ + t: (key: string, vars?: Record) => + key === 'ui.pagination.showing' + ? `Showing ${vars?.from}–${vars?.to} of ${vars?.total}` + : ({ 'ui.pagination.previous': 'Previous', 'ui.pagination.next': 'Next' }[key] ?? key), + }), + keys: { + ui: { + pagination: { + showing: 'ui.pagination.showing', + previous: 'ui.pagination.previous', + next: 'ui.pagination.next', + }, + }, + }, +})); + +import { PaginationBar } from './PaginationBar'; + +describe('PaginationBar', () => { + test('shows the slice and pages forward and back', () => { + const onPageChange = vi.fn(); + render(); + expect(screen.getByText('Showing 11–20 of 25')).toBeTruthy(); + fireEvent.click(screen.getByText('Next')); + fireEvent.click(screen.getByText('Previous')); + expect(onPageChange).toHaveBeenNthCalledWith(1, 3); + expect(onPageChange).toHaveBeenNthCalledWith(2, 1); + }); + + test('disables Previous on the first page and Next on the last', () => { + const { rerender } = render( + {}} />, + ); + expect((screen.getByText('Previous') as HTMLButtonElement).disabled).toBe(true); + rerender( {}} />); + expect((screen.getByText('Next') as HTMLButtonElement).disabled).toBe(true); + expect(screen.getByText('Showing 21–25 of 25')).toBeTruthy(); + }); + + test('an empty list reads 0–0 and Next is disabled', () => { + render( {}} />); + expect(screen.getByText('Showing 0–0 of 0')).toBeTruthy(); + expect((screen.getByText('Next') as HTMLButtonElement).disabled).toBe(true); + }); + + test('hideWhenSinglePage renders nothing for one page', () => { + const { container } = render( + {}} />, + ); + expect(container.firstChild).toBeNull(); + }); +}); diff --git a/packages/ui/src/components/PaginationBar.tsx b/packages/ui/src/components/PaginationBar.tsx new file mode 100644 index 000000000..16de2d84d --- /dev/null +++ b/packages/ui/src/components/PaginationBar.tsx @@ -0,0 +1,66 @@ +import { keys, useT } from '@simple-module-py/i18n'; +import { Button } from '@simple-module-py/ui/components/ui/button'; +import { cn } from '@simple-module-py/ui/lib/utils'; + +interface PaginationBarProps { + /** The page being shown (1-based) — echo the server's value, not the URL's. */ + page: number; + perPage: number; + total: number; + onPageChange: (page: number) => void; + /** Render nothing when everything fits on one page. Off by default: the + * "Showing 0–0 of 0" line is how a reader checks a filter matched. */ + hideWhenSinglePage?: boolean; + className?: string; +} + +/** + * The table footer for any list backed by `simple_module_db.paginate`'s + * `Page` shape (`page`, `per_page`, `total`): which slice is on screen, plus + * Previous / Next. + */ +export function PaginationBar({ + page, + perPage, + total, + onPageChange, + hideWhenSinglePage = false, + className, +}: PaginationBarProps) { + const { t } = useT(); + const pages = Math.max(1, Math.ceil(total / Math.max(perPage, 1))); + if (hideWhenSinglePage && pages <= 1) return null; + const from = total === 0 ? 0 : (page - 1) * perPage + 1; + const to = Math.min(page * perPage, total); + + return ( +
+ {t(keys.ui.pagination.showing, { from, to, total: total.toLocaleString() })} +
+ + +
+
+ ); +} diff --git a/packages/ui/src/index.ts b/packages/ui/src/index.ts index f81b64703..7716a407c 100644 --- a/packages/ui/src/index.ts +++ b/packages/ui/src/index.ts @@ -6,6 +6,7 @@ export { InterpolatedText } from './components/InterpolatedText'; export { NavIcon } from './components/NavIcon'; export { OfflineBanner } from './components/OfflineBanner'; export { PageShell } from './components/PageShell'; +export { PaginationBar } from './components/PaginationBar'; export { PasswordInput } from './components/PasswordInput'; export { PasswordStrength, From 147edf804275eae386fa95ae6e5107e916402ab8 Mon Sep 17 00:00:00 2001 From: Anto Subash Date: Wed, 21 Oct 2026 14:17:43 +0200 Subject: [PATCH 04/12] refactor(scaffold): one module scaffolder; make new-module uses the CLI templates (#509) make new-module now calls create_module(framework_repo=True) from the CLI instead of ~980 lines of f-string templates. Both scaffolders share templates/module/ and emit constants.py, models, contracts, service, deps, guarded endpoints, Browse/Create/Edit pages, menu item with label_key, permissions and locales, so a fresh module passes make lint. The in-repo overlay adds workspace-sourced pyproject metadata, LICENSE, README and i18n-keyed pages; framework_repo.wire_module wires host/root pyproject and the publish-pypi matrix. Scaffold tests consolidated in framework/cli/tests. Claude-Session: https://claude.ai/code/session_01M9neheZZEe3sVpDi2S3zT4 --- CHANGELOG.md | 12 + CLAUDE.md | 48 +-- Makefile | 2 + README.md | 2 +- docs/framework-conventions.md | 4 +- framework/cli/simple_module_cli/case.py | 12 + .../cli/simple_module_cli/framework_repo.py | 127 ++++++++ .../cli/simple_module_cli/scaffolding.py | 24 ++ .../module/__PACKAGE__/constants.py.tpl | 36 +++ .../__PACKAGE__/contracts/__init__.py.tpl | 13 + .../__PACKAGE__/contracts/schemas.py.tpl | 38 +++ .../templates/module/__PACKAGE__/deps.py.tpl | 15 + .../module/__PACKAGE__/endpoints/api.py.tpl | 92 +++++- .../module/__PACKAGE__/endpoints/views.py.tpl | 89 +++++- .../module/__PACKAGE__/locales/en.json.tpl | 25 ++ .../module/__PACKAGE__/models.py.tpl | 26 ++ .../module/__PACKAGE__/module.py.tpl | 69 ++++- .../module/__PACKAGE__/pages/Browse.tsx.tpl | 37 +++ .../module/__PACKAGE__/pages/Create.tsx.tpl | 43 +++ .../module/__PACKAGE__/pages/Edit.tsx.tpl | 50 +++ .../module/__PACKAGE__/pages/Index.tsx.tpl | 13 - .../templates/module/__PACKAGE__/py.typed | 0 .../module/__PACKAGE__/service.py.tpl | 62 ++++ .../module/_optional/framework_repo/LICENSE | 21 ++ .../_optional/framework_repo/README.md.tpl | 34 +++ .../__PACKAGE__/pages/Browse.tsx.tpl | 48 +++ .../__PACKAGE__/pages/Create.tsx.tpl | 45 +++ .../__PACKAGE__/pages/Edit.tsx.tpl | 52 ++++ .../framework_repo/pyproject.toml.tpl | 44 +++ .../module/tests/test___PACKAGE__.py.tpl | 110 ++++++- .../cli/tests/test_cli_new_regressions.py | 4 +- .../cli/tests/test_framework_repo_wiring.py | 125 ++++++++ .../tests/test_scaffolding_framework_repo.py | 110 +++++++ .../cli/tests/test_scaffolding_module.py | 2 +- .../cli/tests/test_scaffolding_module_js.py | 11 +- scripts/_templates_contracts.py | 71 ----- scripts/_templates_endpoints.py | 125 -------- scripts/_templates_js.py | 41 --- scripts/_templates_module.py | 139 --------- scripts/_templates_py.py | 288 ------------------ scripts/_templates_tests.py | 181 ----------- scripts/_templates_tsx.py | 145 --------- scripts/new_module.py | 257 ++-------------- scripts/tests/conftest.py | 46 +-- scripts/tests/test_cli.py | 29 -- scripts/tests/test_generated.py | 145 --------- scripts/tests/test_helpers.py | 111 ------- scripts/tests/test_scaffold.py | 236 -------------- skills/simple-module-creating/SKILL.md | 28 +- skills/simple-module-testing/SKILL.md | 2 +- 50 files changed, 1421 insertions(+), 1868 deletions(-) create mode 100644 framework/cli/simple_module_cli/framework_repo.py create mode 100644 framework/cli/simple_module_cli/templates/module/__PACKAGE__/constants.py.tpl create mode 100644 framework/cli/simple_module_cli/templates/module/__PACKAGE__/contracts/__init__.py.tpl create mode 100644 framework/cli/simple_module_cli/templates/module/__PACKAGE__/contracts/schemas.py.tpl create mode 100644 framework/cli/simple_module_cli/templates/module/__PACKAGE__/deps.py.tpl create mode 100644 framework/cli/simple_module_cli/templates/module/__PACKAGE__/locales/en.json.tpl create mode 100644 framework/cli/simple_module_cli/templates/module/__PACKAGE__/models.py.tpl create mode 100644 framework/cli/simple_module_cli/templates/module/__PACKAGE__/pages/Browse.tsx.tpl create mode 100644 framework/cli/simple_module_cli/templates/module/__PACKAGE__/pages/Create.tsx.tpl create mode 100644 framework/cli/simple_module_cli/templates/module/__PACKAGE__/pages/Edit.tsx.tpl delete mode 100644 framework/cli/simple_module_cli/templates/module/__PACKAGE__/pages/Index.tsx.tpl create mode 100644 framework/cli/simple_module_cli/templates/module/__PACKAGE__/py.typed create mode 100644 framework/cli/simple_module_cli/templates/module/__PACKAGE__/service.py.tpl create mode 100644 framework/cli/simple_module_cli/templates/module/_optional/framework_repo/LICENSE create mode 100644 framework/cli/simple_module_cli/templates/module/_optional/framework_repo/README.md.tpl create mode 100644 framework/cli/simple_module_cli/templates/module/_optional/framework_repo/__PACKAGE__/pages/Browse.tsx.tpl create mode 100644 framework/cli/simple_module_cli/templates/module/_optional/framework_repo/__PACKAGE__/pages/Create.tsx.tpl create mode 100644 framework/cli/simple_module_cli/templates/module/_optional/framework_repo/__PACKAGE__/pages/Edit.tsx.tpl create mode 100644 framework/cli/simple_module_cli/templates/module/_optional/framework_repo/pyproject.toml.tpl create mode 100644 framework/cli/tests/test_framework_repo_wiring.py create mode 100644 framework/cli/tests/test_scaffolding_framework_repo.py delete mode 100644 scripts/_templates_contracts.py delete mode 100644 scripts/_templates_endpoints.py delete mode 100644 scripts/_templates_js.py delete mode 100644 scripts/_templates_module.py delete mode 100644 scripts/_templates_py.py delete mode 100644 scripts/_templates_tests.py delete mode 100644 scripts/_templates_tsx.py delete mode 100644 scripts/tests/test_cli.py delete mode 100644 scripts/tests/test_generated.py delete mode 100644 scripts/tests/test_helpers.py delete mode 100644 scripts/tests/test_scaffold.py diff --git a/CHANGELOG.md b/CHANGELOG.md index a8cd80048..ff4fdb788 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,18 @@ All notable changes to this project are documented in this file. The format is b - `captured_mail` pytest fixture in `simple_module_test` records sent mail. ### Changed +- One module scaffolder (#509). `make new-module` now calls the CLI's + `create_module` (`framework_repo=True` overlay) instead of ~980 lines of + f-string templates in `scripts/_templates_*.py`, which are deleted, so + `smpy create-module` / `smpy new` and `make new-module` share one set of + `templates/module/` files. Both now emit `constants.py`, `models.py`, + `contracts/`, `service.py`, `deps.py`, permission-guarded REST + Inertia + endpoints, Browse/Create/Edit pages, a menu item with `label_key`, + `register_permissions`, `locale_dirs` and `locales/en.json` (the CLI sample + page `Index.tsx` is replaced by `Browse.tsx`). `make new-module` also adds + the module to the `publish-pypi` matrix and runs `npm install` and + `make gen-i18n`; a fresh module passes `make lint`. The scaffold tests live in + `framework/cli/tests/` only. - The module-settings mechanism moved from the `settings` plugin into the framework (#500): `register_module_settings` / `ModuleSettingsRegistry` to `simple_module_core.module_settings`, hydration (`hydrate_settings`, diff --git a/CLAUDE.md b/CLAUDE.md index d499391e1..e17044582 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -128,13 +128,13 @@ The Python package name (`feature_flags`) keys the i18n namespace, the namespace is the meta name. **Module layout** (`make new-module` scaffolds the files below except -`constants.py`, `tasks.py` and the CSS, plus `pyproject.toml`, `README.md`, -`LICENSE`, `package.json`, `tsconfig.json`, `tests/`): +`tasks.py` and the CSS, plus `pyproject.toml`, `README.md`, `LICENSE`, +`package.json`, `tsconfig.json`, `tests/`): ``` modules/// ├── module.py # ModuleBase subclass, meta = ModuleMeta(...) -├── constants.py # add it: PERM_*, ROLE_*, _PAGE_*, _MODULE_* — see § Conventions +├── constants.py # PERM_*, PAGE_*, MENU_*, ROLE_*, _MODULE_* — see § Conventions (scaffolded) ├── models.py # SQLModel tables (table=True) ├── contracts/ # SQLModel DTOs — the public surface; add a Protocol only for real extension points ├── settings.py # DbBackedSettings subclass @@ -398,23 +398,35 @@ development and testing (`session_cookie_secure`). ## Adding a module or package -`make new-module name=` scaffolds the package, wires it into -`host/pyproject.toml` and the root `testpaths`/`extra-paths`, and runs -`uv sync`. Expect `make lint` to fail right after: the scaffold still emits a -literal `inertia.render("/Create")` and permission and menu strings -without constants or `label_key`. Fix those first. It also does **not**: - -1. Run `npm install`, so regenerate and commit `package-lock.json` (CI uses - `npm ci`). Commit the `uv.lock` change the scaffold's - `uv sync` makes too (CI uses `uv sync --locked`). -2. Add `simple_module_` to the `publish-pypi` matrix in - `.github/workflows/release.yml`; a test and `check_metadata.py` fail until - you do. `pyproject.toml` needs description, `readme`, `license = "MIT"`, - keyword `simple-module`, `urls.Repository`; `README.md` needs ≥ 500 bytes, an - H1 and Install/Usage sections. +There is **one scaffolder**: `make new-module name=` runs +`scripts/new_module.py`, a thin wrapper over the CLI's +`simple_module_cli.scaffolding.create_module(..., framework_repo=True)`, the same +templates `smpy create-module` / `smpy new` use +(`framework/cli/simple_module_cli/templates/module/`). Change a convention there, +once. The `framework_repo` variant is a template overlay +(`_optional/framework_repo/`: workspace-sourced `pyproject.toml` with the +`check_metadata.py` fields, `LICENSE`, `README.md`, i18n-keyed pages). + +It generates a CRUD sample (model, DTOs, service, REST + Inertia endpoints, +Browse/Create/Edit pages, `locales/en.json`, tests) whose permissions, page keys +and menu item come from `constants.py` (`label_key` set, routes guarded), so a +fresh module passes `make lint`. Then +`simple_module_cli.framework_repo.wire_module` adds it to `host/pyproject.toml`, +the root `testpaths`/`extra-paths` and the `publish-pypi` matrix in +`release.yml`, and the target runs `uv sync --all-packages`, `npm install` and +`make gen-i18n`. Commit the resulting `uv.lock`, `package-lock.json` and +`packages/i18n/src/{keys.generated,generated-resources}.ts` changes (CI uses +`--locked` / `npm ci` and diffs the i18n files). It does **not**: + +1. Create a migration for the sample table: edit `models.py`, then follow + § Database (`--head