Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ Standard mixins in `simple_module_db.mixins`: `AuditMixin`, `SoftDeleteMixin` (b

## Diagnostic codes

Meaningful codes when reading `make doctor` output: `SM001` missing meta (error), `SM003` orphan page / `SM004` phantom render (warn), `SM007` module overrides no hooks (info), `SM008` duplicate name (error), `SM009` framework→plugin import (error), `SM010` DB revision behind head (error), `SM011` module table not in migration history (warn), `SM012` `register_settings` overridden but nothing on `app.state.<module>` (warn, fires at dev boot only), `SM013`–`SM016` locale issues, `SM017` module ships `.tsx` pages but is missing `package.json`/`tsconfig.json` (warn). In production, errors fail boot.
Meaningful codes when reading `make doctor` output: `SM001` missing meta (error), `SM003` orphan page / `SM004` phantom render (warn), `SM007` module overrides no hooks (info), `SM008` duplicate name (error), `SM009` framework→plugin import (error), `SM010` DB revision behind head (error), `SM011` module table not in migration history (warn), `SM012` `register_settings` overridden but nothing on `app.state.<module>` (warn, fires at dev boot only), `SM013`–`SM016` locale issues, `SM017` module ships `.tsx` pages but is missing `package.json`/`tsconfig.json` (warn), `SM018` Inertia `router.{post,patch,put,delete}()` in a page targets a JSON `/api/*` endpoint (warn — Inertia rejects non-Inertia responses). In production, errors fail boot.

## Tests & fixtures

Expand Down
73 changes: 73 additions & 0 deletions framework/core/simple_module_core/diagnostics/_inertia_api.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
"""SM018: flag Inertia ``router.{post,patch,put,delete}('/api/...')`` calls.

Inertia's client-side router (``@inertiajs/react``'s ``router.*``) expects
Inertia-shaped responses or redirects — a raw JSON body from the REST
API layer triggers ``All Inertia requests must receive a valid Inertia
response``. The fix is to point the call at a module **view** endpoint
that returns ``RedirectResponse(..., status_code=303)``.

This check regex-scans each module's ``pages/**/*.tsx`` files for the
anti-pattern. It is textual (not AST) because a full TSX parser in
Python is not worth the dependency — the pattern is unambiguous enough
that string matching catches it reliably.
"""

from __future__ import annotations

import re
from pathlib import Path
from typing import TYPE_CHECKING

from simple_module_core.diagnostics._types import Diagnostic, DiagnosticLevel

if TYPE_CHECKING:
from simple_module_core.module import ModuleBase

# Matches calls like:
# router.post('/api/datasets/', ...)
# router.patch(`/api/datasets/${id}`, ...)
# router.delete("/api/datasets/" + id)
# The quote character is captured so we only match string-literal URLs
# (backtick, single, or double) — variable URLs don't count.
_ROUTER_API_CALL = re.compile(
r"router\.(?P<method>post|patch|put|delete)\s*\(\s*[`'\"]/api/",
)


def check_inertia_api_calls(mod: ModuleBase, src_dir: Path) -> list[Diagnostic]:
"""Warn when a page uses Inertia's router against a JSON API endpoint."""
pages_dir = src_dir / "pages"
if not pages_dir.exists():
return []

diags: list[Diagnostic] = []
for tsx in sorted(pages_dir.rglob("*.tsx")):
try:
source = tsx.read_text()
except OSError:
continue
for lineno, line in enumerate(source.splitlines(), start=1):
match = _ROUTER_API_CALL.search(line)
if not match:
continue
method = match.group("method").upper()
diags.append(
Diagnostic(
level=DiagnosticLevel.WARNING,
code="SM018",
message=(
f"router.{method.lower()}() targets a JSON API endpoint — "
"Inertia will reject the response"
),
module_name=mod.meta.name,
file=f"{tsx}:{lineno}",
suggestion=(
"Point the call at a view endpoint (e.g. '/"
f"{mod.meta.name.lower()}/...') that returns "
"RedirectResponse(..., status_code=303). Or, if you "
"really need the JSON payload, use fetch() instead of "
"Inertia's router."
),
)
)
return diags
2 changes: 2 additions & 0 deletions framework/core/simple_module_core/diagnostics/_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from typing import TYPE_CHECKING

from simple_module_core.diagnostics._coupling import check_framework_module_coupling
from simple_module_core.diagnostics._inertia_api import check_inertia_api_calls
from simple_module_core.diagnostics._js_workspace import check_js_workspace_files
from simple_module_core.diagnostics._types import Diagnostic, DiagnosticLevel

Expand Down Expand Up @@ -62,6 +63,7 @@ def run(self, modules: list[ModuleBase]) -> list[Diagnostic]:
diagnostics.extend(self._check_orphan_pages(mod, src_dir, rendered_pages))
diagnostics.extend(self._check_phantom_renders(mod, src_dir, rendered_pages))
diagnostics.extend(check_js_workspace_files(mod, src_dir))
diagnostics.extend(check_inertia_api_calls(mod, src_dir))

return diagnostics

Expand Down
70 changes: 70 additions & 0 deletions framework/core/tests/test_module_diagnostics.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from dataclasses import dataclass
from pathlib import Path

from simple_module_core.diagnostics._inertia_api import check_inertia_api_calls
from simple_module_core.diagnostics._js_workspace import check_js_workspace_files
from simple_module_core.diagnostics._types import DiagnosticLevel

Expand Down Expand Up @@ -74,3 +75,72 @@ async def test_fires_only_for_missing_file(self, tmp_path: Path):
assert len(results) == 1
assert results[0].code == "SM017"
assert "tsconfig.json" in (results[0].file or "")


def _mk_page(src_dir: Path, filename: str, body: str) -> Path:
pages = src_dir / "pages"
pages.mkdir(parents=True, exist_ok=True)
page = pages / filename
page.write_text(body)
return page


class TestSM018InertiaApiCalls:
async def test_flags_router_post_to_api_path(self, tmp_path: Path):
src_dir = tmp_path / "datasets" / "datasets"
_mk_page(src_dir, "Create.tsx", "router.post('/api/datasets/', data, {})")
mod = _FakeModule(meta=_FakeMeta(name="Datasets"))

results = check_inertia_api_calls(mod, src_dir) # pyright: ignore[reportArgumentType]

assert len(results) == 1
assert results[0].code == "SM018"
assert results[0].level == DiagnosticLevel.WARNING
assert "router.post()" in results[0].message
assert "Create.tsx:1" in (results[0].file or "")

async def test_flags_all_mutating_methods(self, tmp_path: Path):
src_dir = tmp_path / "m" / "m"
body = "\n".join(
[
"router.post('/api/a/', d)",
"router.patch('/api/a/1', d)",
"router.put(`/api/a/${id}`, d)",
'router.delete("/api/a/1")',
]
)
_mk_page(src_dir, "X.tsx", body)
mod = _FakeModule(meta=_FakeMeta(name="M"))

results = check_inertia_api_calls(mod, src_dir) # pyright: ignore[reportArgumentType]

methods = sorted(r.message.split("()")[0].split(".")[-1] for r in results)
assert methods == ["delete", "patch", "post", "put"]
assert all(r.code == "SM018" for r in results)

async def test_silent_on_view_path(self, tmp_path: Path):
src_dir = tmp_path / "m" / "m"
_mk_page(src_dir, "Create.tsx", "router.post('/datasets/', data)")
mod = _FakeModule(meta=_FakeMeta(name="Datasets"))

results = check_inertia_api_calls(mod, src_dir) # pyright: ignore[reportArgumentType]

assert results == []

async def test_silent_on_router_get(self, tmp_path: Path):
src_dir = tmp_path / "m" / "m"
_mk_page(src_dir, "Browse.tsx", "router.get('/api/search', params)")
mod = _FakeModule(meta=_FakeMeta(name="M"))

results = check_inertia_api_calls(mod, src_dir) # pyright: ignore[reportArgumentType]

assert results == []

async def test_silent_when_no_pages_dir(self, tmp_path: Path):
src_dir = tmp_path / "backend_only" / "backend_only"
src_dir.mkdir(parents=True)
mod = _FakeModule(meta=_FakeMeta(name="BackendOnly"))

results = check_inertia_api_calls(mod, src_dir) # pyright: ignore[reportArgumentType]

assert results == []
46 changes: 31 additions & 15 deletions modules/datasets/datasets/endpoints/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,22 +101,16 @@ async def download_dataset(
)


@router.post(
"/",
response_model=DatasetOut,
status_code=201,
dependencies=[Depends(RequiresPermission(constants.PERM_DATASETS_UPLOAD))],
)
async def upload_dataset(
async def perform_upload(
request: Request,
name: str = Form(..., min_length=1, max_length=200),
description: str | None = Form(default=None, max_length=2000),
kind: str | None = Form(default=None),
file: UploadFile = File(...),
service: DatasetService = Depends(get_dataset_service),
bus: EventBus = Depends(get_event_bus),
celery: Celery = Depends(get_celery),
max_upload_bytes: int = Depends(get_max_upload_bytes),
name: str,
description: str | None,
kind: str | None,
file: UploadFile,
service: DatasetService,
bus: EventBus,
celery: Celery,
max_upload_bytes: int,
) -> DatasetOut:
if kind is not None and kind not in constants.ALL_KINDS:
raise HTTPException(status_code=422, detail=f"Unknown kind: {kind}")
Expand Down Expand Up @@ -187,6 +181,28 @@ async def upload_dataset(
return dataset


@router.post(
"/",
response_model=DatasetOut,
status_code=201,
dependencies=[Depends(RequiresPermission(constants.PERM_DATASETS_UPLOAD))],
)
async def upload_dataset(
request: Request,
name: str = Form(..., min_length=1, max_length=200),
description: str | None = Form(default=None, max_length=2000),
kind: str | None = Form(default=None),
file: UploadFile = File(...),
service: DatasetService = Depends(get_dataset_service),
bus: EventBus = Depends(get_event_bus),
celery: Celery = Depends(get_celery),
max_upload_bytes: int = Depends(get_max_upload_bytes),
) -> DatasetOut:
return await perform_upload(
request, name, description, kind, file, service, bus, celery, max_upload_bytes
)


@router.patch(
constants.PATH_DATASET,
response_model=DatasetOut,
Expand Down
74 changes: 72 additions & 2 deletions modules/datasets/datasets/endpoints/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,24 @@

from __future__ import annotations

from fastapi import APIRouter, Depends
from celery import Celery
from fastapi import APIRouter, Depends, File, Form, HTTPException, Request, UploadFile
from inertia import InertiaResponse
from simple_module_core.events import EventBus
from simple_module_hosting.inertia_deps import InertiaDep
from simple_module_hosting.permissions import RequiresPermission
from starlette.responses import RedirectResponse

from datasets import constants
from datasets.deps import get_dataset_service
from datasets.contracts.events import DatasetDeleted
from datasets.contracts.schemas import DatasetUpdate
from datasets.deps import (
get_celery,
get_dataset_service,
get_event_bus,
get_max_upload_bytes,
)
from datasets.endpoints.api import perform_upload
from datasets.service import DatasetService

# Module-local Inertia page identifiers. These must be Name-only literal
Expand Down Expand Up @@ -44,6 +55,31 @@ async def create_view(inertia: InertiaDep) -> InertiaResponse:
return await inertia.render(_PAGE_CREATE)


@router.post(
"/",
response_model=None,
dependencies=[Depends(RequiresPermission(constants.PERM_DATASETS_UPLOAD))],
)
async def upload_view(
request: Request,
name: str = Form(..., min_length=1, max_length=200),
description: str | None = Form(default=None, max_length=2000),
kind: str | None = Form(default=None),
file: UploadFile = File(...),
service: DatasetService = Depends(get_dataset_service),
bus: EventBus = Depends(get_event_bus),
celery: Celery = Depends(get_celery),
max_upload_bytes: int = Depends(get_max_upload_bytes),
) -> RedirectResponse:
# Inertia's client-side router expects a redirect after POST. Return 303
# so it re-issues a GET against the browse page, which replies with a
# full Inertia response.
await perform_upload(
request, name, description, kind, file, service, bus, celery, max_upload_bytes
)
return RedirectResponse(constants.REDIRECT_BROWSE, status_code=303)


@router.get("/{dataset_id}", response_model=None)
async def show_view(
dataset_id: int,
Expand Down Expand Up @@ -76,3 +112,37 @@ async def edit_view(
{"datasets": [], "error": "Dataset not found"},
)
return await inertia.render(_PAGE_EDIT, {"dataset": item.model_dump(mode="json")})


@router.patch(
"/{dataset_id}",
response_model=None,
dependencies=[Depends(RequiresPermission(constants.PERM_DATASETS_EDIT))],
)
async def update_view(
dataset_id: int,
data: DatasetUpdate,
service: DatasetService = Depends(get_dataset_service),
) -> RedirectResponse:
item = await service.update(dataset_id, data)
if item is None:
raise HTTPException(status_code=404, detail="Dataset not found")
return RedirectResponse(constants.REDIRECT_BROWSE, status_code=303)


@router.delete(
"/{dataset_id}",
response_model=None,
dependencies=[Depends(RequiresPermission(constants.PERM_DATASETS_DELETE))],
)
async def delete_view(
dataset_id: int,
service: DatasetService = Depends(get_dataset_service),
bus: EventBus = Depends(get_event_bus),
) -> RedirectResponse:
existing = await service.get_by_id(dataset_id)
if existing is None:
raise HTTPException(status_code=404, detail="Dataset not found")
await service.delete(dataset_id)
await bus.publish(DatasetDeleted(dataset_id=dataset_id, slug=existing.slug))
return RedirectResponse(constants.REDIRECT_BROWSE, status_code=303)
2 changes: 1 addition & 1 deletion modules/datasets/datasets/pages/Browse.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ function Browse() {
const canDelete = can('datasets.delete');

function handleDelete(dataset: Dataset) {
router.delete(`/api/datasets/${dataset.id}`, { preserveScroll: true });
router.delete(`/datasets/${dataset.id}`, { preserveScroll: true });
}

return (
Expand Down
2 changes: 1 addition & 1 deletion modules/datasets/datasets/pages/Create.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ function Create() {
data.append('file', file);

setSubmitting(true);
router.post('/api/datasets/', data, {
router.post('/datasets/', data, {
forceFormData: true,
onSuccess: () => toast.success(t(keys.datasets.toasts.created)),
onError: (errs) => {
Expand Down
2 changes: 1 addition & 1 deletion modules/datasets/datasets/pages/Edit.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function Edit() {
e.preventDefault();
setSubmitting(true);
router.patch(
`/api/datasets/${dataset.id}`,
`/datasets/${dataset.id}`,
{ name, description: description || null, kind, crs: crs || null },
{
onSuccess: () => {
Expand Down
Loading
Loading