diff --git a/CHANGELOG.md b/CHANGELOG.md index 9cc64a0ea..8be69ed05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ All notable changes to this project are documented in this file. The format is b installed, otherwise run the durable handlers inline in `db`'s transaction (#583). ### Changed +- Frontend: every page that posts through the Inertia router now shows a toast on failure (sharing revoke, dashboard Doctor rerun included); `useLeaveGuard` lives in `@simple-module-py/ui`, the background-tasks outbox confirms through the shared dialog instead of `window.confirm`, and `reportClientError()` is the single hook the error boundary reports through (#601). - Users: lockout, registration and bulk-invite writes that must land before the request ends now commit through `simple_module_db.independent_session` (its own short-lived session) instead of committing the request session. Failed or unverified password logins now roll @@ -71,6 +72,9 @@ All notable changes to this project are documented in this file. The format is b `file_storage.reconcile_checksums` beat task that re-enqueues checksums for files left without one for 30 minutes, up to 500 per run (#590). +### Removed +- Frontend: `@simple-module-py/ui` no longer depends on `react-hook-form`/`@hookform/resolvers`, and the unused shadcn `form` component is removed; `host/client_app` drops the unused `zod` and `date-fns` (#601). **Behaviour change** for anyone importing `@simple-module-py/ui/components/ui/form`. + ## [0.0.38] — 2026-10-23 ### Fixed diff --git a/docs/frontend/inertia.md b/docs/frontend/inertia.md index 6baf20240..cbee069e4 100644 --- a/docs/frontend/inertia.md +++ b/docs/frontend/inertia.md @@ -39,25 +39,30 @@ Breakdown: ## Responding with JSON vs. Inertia -- **`endpoints/api.py`** — returns JSON. Called by background/non-Inertia code, or by Inertia's `router.get/post` on the client (which sends `X-Inertia: true` and receives a full Inertia response, **not** raw JSON). -- **`endpoints/views.py`** — returns Inertia pages. Routed at `view_prefix`, rendered as full HTML on the first request and as JSON page updates on client-side navigation. - -The split is convention: keep GET-for-page-data calls in `views.py`, keep POST/PATCH/DELETE data mutations in `api.py` even if triggered from Inertia forms. This way JSON APIs are usable from scripts and Inertia pages route cleanly. +- **`endpoints/api.py`** returns JSON. Use the typed `createApi` client wrapped in + `apiResult` for JSON actions, and toast failed results (including network failures). +- **`endpoints/views.py`** returns Inertia pages or redirects. View-route forms use + Inertia `useForm` for data, validation errors and processing state. Small actions + without form fields may use `router.post/delete` with an `onError` toast. + Handle `onHttpException` and `onNetworkError` too when an action should toast + HTTP or connection failures; returning `false` suppresses Inertia's default handler. ## Inertia form submission -From a page, use Inertia's `router.post`: +Submit to the module's view route: ```tsx -import { router } from "@inertiajs/react"; +import { useForm } from "@inertiajs/react"; function CreateOrder() { - const [data, setData] = useState({ customer_email: "", total: "" }); + const form = useForm({ customer_email: "", total: "" }); return (
{ e.preventDefault(); - router.post("/api/orders", data); + form.post("/orders", { + onError: () => toast.error(t(keys.orders.create.failed)), + }); }} > ... @@ -97,8 +102,9 @@ There is no `flash` shared prop. For "created successfully" toasts, fire `sonner ```tsx import { toast } from "sonner"; -router.post("/api/orders", data, { - onSuccess: () => toast.success("Order created"), +router.post("/orders", data, { + onSuccess: () => toast.success(t(keys.orders.create.saved)), + onError: () => toast.error(t(keys.orders.create.failed)), }); ``` diff --git a/framework/db/tests/_db_shape_matrix.py b/framework/db/tests/_db_shape_matrix.py new file mode 100644 index 000000000..1f2cd0081 --- /dev/null +++ b/framework/db/tests/_db_shape_matrix.py @@ -0,0 +1,44 @@ +"""Combined tenant/soft-delete probes shared by the statement families (#597).""" + +import pytest +from _models import _tenant_session +from simple_module_db.base import create_module_base +from simple_module_db.mixins import MultiTenantMixin, SoftDeleteMixin +from simple_module_db.tenancy import tenant_context +from sqlmodel import Field + +Base = create_module_base("shape_matrix") + + +class MatrixRow(Base, MultiTenantMixin, SoftDeleteMixin, table=True): # ty: ignore[unsupported-base] + __tablename__ = "shape_matrix_row" + id: int = Field(primary_key=True) + name: str = "original" + + +class MatrixAnchor(Base, table=True): # ty: ignore[unsupported-base] + __tablename__ = "shape_matrix_anchor" + id: int = Field(primary_key=True) + + +row, anchor = MatrixRow.__table__, MatrixAnchor.__table__ + + +async def seed(session): + session.add_all([MatrixAnchor(id=i) for i in range(1, 6)]) + for tenant, ids in (("A", (1, 2)), ("B", (3, 4))): + with tenant_context(tenant): + session.add_all([MatrixRow(id=ids[0]), MatrixRow(id=ids[1], is_deleted=True)]) + await session.flush() + await session.commit() + + +@pytest.fixture(params=[False, True], ids=["tenant-off", "tenant-on"]) +async def matrix_session(request): + async for session in _tenant_session(Base.metadata, strict=request.param, seed=seed): + yield session + + +def visible_ids(tenant, include_deleted): + ids = {"A": [1, 2], "B": [3, 4], None: [1, 2, 3, 4]}[tenant] + return ids if include_deleted else [i for i in ids if i % 2] diff --git a/framework/db/tests/test_db_shape_matrix_dml.py b/framework/db/tests/test_db_shape_matrix_dml.py new file mode 100644 index 000000000..1ff024ca9 --- /dev/null +++ b/framework/db/tests/test_db_shape_matrix_dml.py @@ -0,0 +1,35 @@ +"""Bulk DML scopes tenants but intentionally includes trashed targets (#597).""" + +import pytest +from _db_shape_matrix import matrix_session, row, visible_ids +from simple_module_db.query_filter import strict_configured +from simple_module_db.tenancy import TenantIsolationError, no_tenant, tenant_context +from sqlalchemy import delete, select, update + +__all__ = ["matrix_session"] + + +@pytest.mark.parametrize("operation", ["update", "delete"]) +@pytest.mark.parametrize("tenant", ["A", "B", None]) +@pytest.mark.parametrize("include_deleted", [False, True]) +async def test_dml_matrix(matrix_session, operation, tenant, include_deleted): + stmt = update(row).values(name="changed") if operation == "update" else delete(row) + stmt = stmt.execution_options(include_deleted=include_deleted) + with no_tenant() if tenant is None else tenant_context(tenant): + if tenant is None and strict_configured(matrix_session.sync_session): + with pytest.raises(TenantIsolationError): + await matrix_session.execute(stmt) + affected = [] + else: + result = await matrix_session.execute(stmt) + affected = visible_ids(tenant, True) + assert result.rowcount == len(affected) + # Bypass both filters to prove *which* rows changed, including foreign/deleted rows. + inspect = select(row.c.id, row.c.name).execution_options(all_tenants=True, include_deleted=True) + actual = sorted((await matrix_session.execute(inspect)).all()) + expected = [ + (i, "changed" if i in affected else "original") + for i in range(1, 5) + if operation == "update" or i not in affected + ] + assert actual == expected diff --git a/framework/db/tests/test_db_shape_matrix_joins.py b/framework/db/tests/test_db_shape_matrix_joins.py new file mode 100644 index 000000000..164755b6e --- /dev/null +++ b/framework/db/tests/test_db_shape_matrix_joins.py @@ -0,0 +1,55 @@ +"""Join predicates must filter nullable rows without losing their anchors (#597).""" + +import pytest +from _db_shape_matrix import anchor, matrix_session, row, visible_ids +from simple_module_db._core_joins import scope_joins +from simple_module_db.query_filter import strict_configured +from simple_module_db.tenancy import TenantIsolationError, no_tenant, tenant_context +from sqlalchemy import select + +__all__ = ["matrix_session"] + + +@pytest.mark.parametrize("outer", [False, True], ids=["join", "outerjoin"]) +@pytest.mark.parametrize("setup", [False, True], ids=["explicit", "setup"]) +@pytest.mark.parametrize("tenant", ["A", "B", None]) +@pytest.mark.parametrize("include_deleted", [False, True]) +async def test_join_matrix(matrix_session, outer, setup, tenant, include_deleted): + on = row.c.id == anchor.c.id + stmt = select(anchor.c.id, row.c.id) + if setup: + stmt = stmt.select_from(anchor).join(row, on, isouter=outer) + else: + stmt = stmt.select_from(anchor.join(row, on, isouter=outer)) + stmt = stmt.execution_options(include_deleted=include_deleted) + with no_tenant() if tenant is None else tenant_context(tenant): + if tenant is None and strict_configured(matrix_session.sync_session): + with pytest.raises(TenantIsolationError): + await matrix_session.execute(stmt) + else: + visible = visible_ids(tenant, include_deleted) + expected = ( + [(i, i if i in visible else None) for i in range(1, 6)] + if outer + else [(i, i) for i in visible] + ) + assert sorted((await matrix_session.execute(stmt)).all()) == expected + + +@pytest.mark.parametrize("outer", [False, True]) +def test_scope_joins_places_nullable_predicate_in_on(outer): + stmt = select(anchor.c.id, row.c.id).select_from( + anchor.join(row, anchor.c.id == row.c.id, isouter=outer) + ) + scoped = scope_joins( + stmt, lambda table: [table.c.is_deleted.is_(False)] if table is row else [] + ) + assert scoped.covered == [row] + assert bool(scoped.where) is (not outer) + rewritten = stmt._generate() + rewritten._from_obj = scoped.from_obj + rewritten = rewritten.where(*scoped.where) + sql = str(rewritten) + before_where, _, where = sql.partition("WHERE") + assert ("is_deleted" in before_where) is outer + assert ("is_deleted" in where) is (not outer) diff --git a/framework/db/tests/test_db_shape_matrix_selects.py b/framework/db/tests/test_db_shape_matrix_selects.py new file mode 100644 index 000000000..66d13be67 --- /dev/null +++ b/framework/db/tests/test_db_shape_matrix_selects.py @@ -0,0 +1,58 @@ +"""Select/walk matrix, using the same fixture on SQLite and Postgres (#597).""" + +import pytest +from _db_shape_matrix import MatrixRow, anchor, matrix_session, row, visible_ids +from simple_module_db._core_walk import references_plain +from simple_module_db.query_filter import strict_configured +from simple_module_db.tenancy import TenantIsolationError, no_tenant, tenant_context +from sqlalchemy import delete, exists, select, union, update + +__all__ = ["matrix_session"] + + +def statement(shape): + if shape == "orm": + return select(MatrixRow.id) + if shape == "core": + return select(row.c.id) + if shape == "select_from": + return select(anchor.c.id).select_from(anchor, row).where(anchor.c.id == row.c.id) + if shape == "subquery": + return select(select(row.c.id).subquery().c.id) + if shape == "cte": + return select(select(row.c.id).cte("scoped").c.id) + if shape == "union": + return union(select(row.c.id).where(row.c.id <= 2), select(row.c.id).where(row.c.id > 2)) + if shape == "exists": + return select(anchor.c.id).where(exists().where(row.c.id == anchor.c.id)) + raise AssertionError(shape) + + +SHAPES = ["orm", "core", "select_from", "subquery", "cte", "union", "exists"] + + +@pytest.mark.parametrize("shape", SHAPES) +@pytest.mark.parametrize("tenant", ["A", "B", None], ids=["own", "other", "unbound"]) +@pytest.mark.parametrize("include_deleted", [False, True]) +async def test_select_matrix(matrix_session, shape, tenant, include_deleted): + stmt = statement(shape).execution_options(include_deleted=include_deleted) + with no_tenant() if tenant is None else tenant_context(tenant): + if tenant is None and strict_configured(matrix_session.sync_session): + with pytest.raises(TenantIsolationError): + await matrix_session.execute(stmt) + else: + ids = (await matrix_session.execute(stmt)).scalars().all() + assert sorted(ids) == visible_ids(tenant, include_deleted) + + +@pytest.mark.parametrize("shape", [*SHAPES, "update", "delete"]) +def test_walk_detects_only_plain_scoped_references(shape): + stmt = ( + update(row).values(name="changed") + if shape == "update" + else delete(row) + if shape == "delete" + else statement(shape) + ) + assert references_plain(stmt, {row.name}) is (shape != "orm") + assert references_plain(stmt, {"unrelated_table"}) is False diff --git a/host/client_app/package.json b/host/client_app/package.json index 9042739d5..8be09cf96 100644 --- a/host/client_app/package.json +++ b/host/client_app/package.json @@ -8,15 +8,12 @@ "preview": "vite preview" }, "dependencies": { - "@hookform/resolvers": "^5.9.1", "@inertiajs/react": "^3.7.0", "@simple-module-py/i18n": "*", "@simple-module-py/ui": "*", - "date-fns": "^4.4.0", "lucide-react": "^1.43.0", "react": "^19.2.8", - "react-dom": "^19.2.8", - "zod": "^4.5.4" + "react-dom": "^19.2.8" }, "devDependencies": { "@simple-module-py/tsconfig": "*", diff --git a/modules/audit_log/tests-js/audit_log_Browse.test.tsx b/modules/audit_log/tests-js/audit_log_Browse.test.tsx new file mode 100644 index 000000000..04a87017f --- /dev/null +++ b/modules/audit_log/tests-js/audit_log_Browse.test.tsx @@ -0,0 +1,81 @@ +import { configureI18n, keys } from '@simple-module-py/i18n'; +import { render, screen } from '@testing-library/react'; +import type { ComponentProps } from 'react'; +import { describe, expect, it, vi } from 'vitest'; + +configureI18n({ locale: 'en', messages: {} }); +vi.mock('@inertiajs/react', () => ({ + Head: () => null, + Link: ({ href, children }: { href: string; children: React.ReactNode }) => ( + {children} + ), + router: { visit: vi.fn() }, + usePage: () => ({ props: {} }), +})); + +import Browse from '../audit_log/pages/Browse'; + +const props: ComponentProps = { + items: [], + total: 0, + page: 1, + page_size: 50, + entity_types: [], + tenant_ids: [], + platform_tenant_value: '__platform__', + export_url: '/api/audit_log/export', + filters: { + entity_type: null, + action: null, + tenant_id: null, + user_id: null, + correlation_id: null, + from_date: null, + to_date: null, + }, +}; + +describe('audit_log Browse', () => { + it('mounts the filter form, empty state, and export link', () => { + render(); + expect(screen.getByRole('heading', { name: keys.audit_log.browse.title })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: keys.audit_log.filters.apply })).toBeInTheDocument(); + expect(screen.getByRole('link', { name: keys.audit_log.browse.export_csv })).toHaveAttribute( + 'download', + ); + expect(screen.getByRole('button', { name: keys.ui.pagination.next })).toBeDisabled(); + }); + + it('mounts a populated entry with its resolved actor and resource', () => { + render( + , + ); + expect(screen.getByText('Alice')).toBeInTheDocument(); + expect(screen.getByText('Meeting notes')).toBeInTheDocument(); + expect(screen.getByRole('table')).toBeInTheDocument(); + }); +}); diff --git a/modules/background_tasks/background_tasks/pages/Outbox.tsx b/modules/background_tasks/background_tasks/pages/Outbox.tsx index 0c747d282..cb4956499 100644 --- a/modules/background_tasks/background_tasks/pages/Outbox.tsx +++ b/modules/background_tasks/background_tasks/pages/Outbox.tsx @@ -1,11 +1,12 @@ import { Head, Link, router } from '@inertiajs/react'; import { keys, useT } from '@simple-module-py/i18n'; +import { ConfirmActionDialog } from '@simple-module-py/ui/components/ConfirmActionDialog'; import { PageShell } from '@simple-module-py/ui/components/PageShell'; import { Button } from '@simple-module-py/ui/components/ui/button'; import { usePermissions } from '@simple-module-py/ui/hooks/use-permissions'; import { AdminLayout } from '@simple-module-py/ui/layouts/AdminLayout'; import { cn } from '@simple-module-py/ui/lib/utils'; -import { ArrowLeft } from 'lucide-react'; +import { ArrowLeft, Trash2 } from 'lucide-react'; import { useState } from 'react'; import { OutboxTable } from './components/OutboxTable'; import { VIEW_BASE } from './constants'; @@ -31,6 +32,7 @@ function Outbox({ messages, counts, pagination, filters }: Props) { const { t } = useT(); const { can } = usePermissions(); const canManage = can('background_tasks.manage'); + const [pendingDiscard, setPendingDiscard] = useState(null); const [busyId, setBusyId] = useState(null); function visit(status: string, page: number) { @@ -51,13 +53,6 @@ function Outbox({ messages, counts, pagination, filters }: Props) { } } - function discard(message: OutboxMessage) { - const prompt = t(keys.background_tasks.outbox.discard_confirm, { - handler: message.handler_name, - }); - if (window.confirm(prompt)) void act(message, discardOutboxMessage); - } - const segments = [ { value: '', label: t(keys.background_tasks.outbox.filter_all), count: null }, ...OPEN_OUTBOX_STATUSES.map((s) => ({ @@ -105,13 +100,30 @@ function Outbox({ messages, counts, pagination, filters }: Props) { canManage={canManage} busyId={busyId} onRetry={(m) => void act(m, retryOutboxMessage)} - onDiscard={discard} + onDiscard={setPendingDiscard} page={pagination.page} perPage={pagination.per_page} total={pagination.total} onPageChange={(page) => visit(filters.status, page)} /> + !open && setPendingDiscard(null)} + icon={Trash2} + title={t(keys.background_tasks.outbox.discard)} + description={t(keys.background_tasks.outbox.discard_confirm, { + handler: pendingDiscard?.handler_name ?? '', + })} + confirmLabel={t(keys.background_tasks.outbox.discard)} + cancelLabel={t(keys.background_tasks.retry_dialog.cancel)} + busy={busyId !== null} + onConfirm={() => { + if (pendingDiscard) { + void act(pendingDiscard, discardOutboxMessage).finally(() => setPendingDiscard(null)); + } + }} + /> ); } diff --git a/modules/background_tasks/tests-js/background_tasks_OutboxPage.test.tsx b/modules/background_tasks/tests-js/background_tasks_OutboxPage.test.tsx index a289d7fd1..a3f80ca92 100644 --- a/modules/background_tasks/tests-js/background_tasks_OutboxPage.test.tsx +++ b/modules/background_tasks/tests-js/background_tasks_OutboxPage.test.tsx @@ -1,6 +1,6 @@ import '@testing-library/jest-dom/vitest'; import { configureI18n, keys } from '@simple-module-py/i18n'; -import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { fireEvent, render, screen, waitFor, within } from '@testing-library/react'; import type { ComponentProps } from 'react'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; @@ -134,21 +134,27 @@ describe('background_tasks Outbox page', () => { it('asks before discarding and calls nothing when the operator declines', () => { const fetchMock = stubFetch(204); - const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(false); render()} />); fireEvent.click(screen.getByRole('button', { name: ob.discard })); - expect(confirmSpy).toHaveBeenCalledTimes(1); + const dialog = screen.getByRole('alertdialog'); + fireEvent.click( + within(dialog).getByRole('button', { name: keys.background_tasks.retry_dialog.cancel }), + ); + expect(screen.queryByRole('alertdialog')).toBeNull(); expect(fetchMock).not.toHaveBeenCalled(); expect(reload).not.toHaveBeenCalled(); }); it('discards with a DELETE once confirmed', async () => { const fetchMock = stubFetch(204); - vi.spyOn(window, 'confirm').mockReturnValue(true); render()} />); fireEvent.click(screen.getByRole('button', { name: ob.discard })); + expect(fetchMock).not.toHaveBeenCalled(); + fireEvent.click( + within(screen.getByRole('alertdialog')).getByRole('button', { name: ob.discard }), + ); await waitFor(() => expect(toast.success).toHaveBeenCalledWith(ob.discarded)); expect(requestsOf(fetchMock)).toContainEqual({ url: '/api/background_tasks/admin/outbox/m1', @@ -159,9 +165,11 @@ describe('background_tasks Outbox page', () => { it('toasts a discard failure', async () => { stubFetch(500); - vi.spyOn(window, 'confirm').mockReturnValue(true); render()} />); fireEvent.click(screen.getByRole('button', { name: ob.discard })); + fireEvent.click( + within(screen.getByRole('alertdialog')).getByRole('button', { name: ob.discard }), + ); await waitFor(() => expect(toast.error).toHaveBeenCalledWith(ob.discard_failed)); }); diff --git a/modules/dashboard/dashboard/locales/de.json b/modules/dashboard/dashboard/locales/de.json index c3fbedee4..9294a20ff 100644 --- a/modules/dashboard/dashboard/locales/de.json +++ b/modules/dashboard/dashboard/locales/de.json @@ -79,7 +79,8 @@ "report_checks": "Prüfungen", "report_checks_unavailable": "nicht verfügbar (Diagnose läuft nur in der Entwicklungsumgebung)", "report_migrations": "Migrationen", - "report_dev_server": "Entwicklungsserver" + "report_dev_server": "Entwicklungsserver", + "rerun_failed": "Die Diagnose konnte nicht erneut ausgeführt werden" }, "nav": { "dashboard": "Dashboard", diff --git a/modules/dashboard/dashboard/locales/en.json b/modules/dashboard/dashboard/locales/en.json index 39194916e..a8a784406 100644 --- a/modules/dashboard/dashboard/locales/en.json +++ b/modules/dashboard/dashboard/locales/en.json @@ -79,7 +79,8 @@ "report_checks": "Checks", "report_checks_unavailable": "not available (diagnostics run in development only)", "report_migrations": "Migrations", - "report_dev_server": "Dev server" + "report_dev_server": "Dev server", + "rerun_failed": "Failed to rerun diagnostics" }, "nav": { "dashboard": "Dashboard", diff --git a/modules/dashboard/dashboard/locales/es.json b/modules/dashboard/dashboard/locales/es.json index a2349884e..80b9c32ac 100644 --- a/modules/dashboard/dashboard/locales/es.json +++ b/modules/dashboard/dashboard/locales/es.json @@ -79,7 +79,8 @@ "report_checks": "Comprobaciones", "report_checks_unavailable": "no disponible (las comprobaciones solo se ejecutan en desarrollo)", "report_migrations": "Migraciones", - "report_dev_server": "Servidor de desarrollo" + "report_dev_server": "Servidor de desarrollo", + "rerun_failed": "No se pudo volver a ejecutar el diagnóstico" }, "nav": { "dashboard": "Panel", diff --git a/modules/dashboard/dashboard/pages/Doctor.tsx b/modules/dashboard/dashboard/pages/Doctor.tsx index b3ec88ac5..904b2bac7 100644 --- a/modules/dashboard/dashboard/pages/Doctor.tsx +++ b/modules/dashboard/dashboard/pages/Doctor.tsx @@ -20,6 +20,10 @@ const RERUN_URL = '/admin/doctor/rerun'; function Doctor(props: DoctorProps) { const { t } = useT(); const checkLabels = useCheckLabels(); + const rerunFailed = () => { + toast.error(t(keys.dashboard.doctor.rerun_failed)); + return false; + }; async function copy(text: string, successMessage: string) { if (await copyToClipboard(text)) { @@ -69,7 +73,17 @@ function Doctor(props: DoctorProps) {