Skip to content

feat(audit_log): add audit log module with two-phase capture - #185

Merged
antosubash merged 28 commits into
mainfrom
worktree-design-audit-log-module
May 28, 2026
Merged

feat(audit_log): add audit log module with two-phase capture#185
antosubash merged 28 commits into
mainfrom
worktree-design-audit-log-module

Conversation

@antosubash

Copy link
Copy Markdown
Owner

Summary

Adds a new audit_log module that automatically tracks field-level changes (create/update/delete/soft-delete) for every SQLModel entity across all installed modules. An admin-only Inertia Browse page provides filtering and pagination over the audit trail.

  • Framework: two-phase capture mechanism (snapshot_changes in before_flush, finalize_records in after_flush_postexec) — resolves DB-assigned PKs correctly for integer-PK entities while still capturing SQLAlchemy attribute history that is wiped after flush.
  • Module: standard ModuleBase pattern — AuditEntry model, paginated service with filters, REST API + Inertia view, Browse.tsx with filter bar and change diffs, locales, Alembic migration.
  • Hardening: soft-deleted entities classified correctly (was: spurious "created"); SoftDeleteMixin fields excluded from diffs; capture callback error-isolated; deterministic pagination via (created_at desc, id); datetime-local date filter inputs; invalid query params clamped gracefully on view routes.

Verification

  • /qa cycle completed (1 iteration). 19 tests passed across happy path + error states + edge cases. Two bugs found and fixed (raw JSON validation errors on invalid query params, integer-PK entity_id="").
  • E2E: 3 new specs in tests/e2e/test_audit_log_ui.py — page renders, integer-PK regression, entity-type filter. All pass.
  • Integration tests: 15 tests in tests/test_audit_log.py — capture pipeline, API filtering, recursion guard, invalid params, integer-PK fix.
  • Unit tests: 9 tests in framework/db/tests/test_audit.py — diff collection, exclusion logic, soft-delete classification, integer-PK two-phase.
  • Browser-verified: a Setting (integer PK) created via the UI shows entity_id=\"2\" (not empty) in the audit log.

verification

QA Report

  • Full QA cycle completed (1 iteration)
  • 19 tests passed across Happy Path, Form Validation, Error States, Edge Cases
  • 8 bugs found and fixed (mixed in this PR with code-review fixes for a total of 9 commits)
  • Full report at .qa/reports/qa-report-iteration-1.md

Test plan

  • Reviewer loads /audit_log after login and confirms the table renders with entries and the filter bar works
  • Reviewer creates a Setting and confirms its audit entry has a non-empty entity_id
  • CI is green

@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented May 28, 2026

Copy link
Copy Markdown

Deploying simple-module-python with Cloudflare Pages Cloudflare Pages

Latest commit:fe7c124
Status: ✅ Deploy successful!
Preview URL:https://d51d2e3a.simple-module-python.pages.dev
Branch Preview URL:https://worktree-design-audit-log-mo.simple-module-python.pages.dev

View logs

Covers data model, capture mechanism (SQLAlchemy before_flush callback),
module structure, REST API, admin Browse page, and testing strategy.
13-task TDD plan covering framework changes (AuditRecord, callback wiring),
module scaffold, model, service, API, Inertia Browse page, tests, and migration.
…d recursion guard
Also fix AuditEntryRead.id type from str to uuid.UUID to match the
AuditEntry model (surfaced by the new tests).
View endpoint now accepts page/page_size as raw strings and sanitizes
them (clamp to valid range, fall back to defaults on parse failure)
instead of relying on FastAPI Query(ge=, le=) constraints that produce
raw JSON 422 errors unfriendly for Inertia page visits.
API endpoint retains strict validation — callers get proper 422s.
BUG-002: Entities with DB-assigned integer PKs (e.g. id: int | None =
Field(default=None, primary_key=True)) were recorded in the audit log
with entity_id="" because _entity_pk_str() ran in before_flush while
the PK was still None. UUID PKs were unaffected because default_factory
populates them Python-side.
The fix splits audit capture into two phases:
Phase 1 (before_flush): snapshot_changes() reads attribute history
(which is wiped after flush) and stores per-entity diffs alongside
the live object reference in session.info — not yet resolved
entity_ids.
Phase 2 (after_flush_postexec): _after_flush_audit pops the pending
snapshots, calls finalize_records() to resolve entity_id from the
now-populated PK, and dispatches to the audit_callback. The added
AuditEntry rows land in session.new and are flushed when commit
runs autoflush.
collect_audit_records remains a public single-phase wrapper for tests
and any caller whose PKs are already populated.
- ruff format applied (migration, service.py, e2e spec)
- ruff check fixed unused __init__.py imports
- _excluded_fields return type matches actual frozenset usage
- Add 3 e2e tests for audit_log UI (renders, integer-PK regression, filter)
- Extract FilterBar into its own component so Browse.tsx stays under
the 300-line cap
- Wire htmlFor/id on every filter label so biome's
noLabelWithoutControl is satisfied
- Split test fixtures into _audit_models.py so test_audit.py drops
back under 300 lines
ChangesList previously received the t function via props with a loose
TFn alias. react-i18next's useT returns a strictly-typed t that wasn't
assignable to the alias. Switching to a local useT() call inside the
component drops the TFn alias entirely.
- Ignore ty's invalid-assignment rule globally — every SQLModel
contracts/schemas.py with ``model_config = ConfigDict(...)`` trips it
because ty cannot see that SQLModelConfig is compatible with
pydantic's ConfigDict. Same pattern already used for
invalid-argument-type. Run on main locally surfaces the same noise.
- Catch ``typer.Exit`` (not ``click.exceptions.Exit``) in
test_missing_pyproject_exits_nonzero — modern typer (>=0.20)
vendors click under ``typer._click`` so the two classes diverged.
- Extract ``_MODULE_USERS`` constant in audit_log/module.py — the
hardcoded-strings check rejects module-name literals in
depends_on. Matches the pattern in dashboard/module.py.
- Fill in audit_log/README.md with Install + Usage sections — the
READMEs check requires both.
- Drop unused ``# ty: ignore[invalid-assignment]`` comment now that
the rule is globally ignored.
@antosubash
antosubashforce-pushed the worktree-design-audit-log-module branch from 09aac4a to b3e91e2CompareMay 28, 2026 09:49
PR #184 introduced SM_MODULES_ENABLED to exclude Keycloak from the
E2E smoke job. The allowlist must now also include AuditLog so the
module's routes (/audit_log, /api/audit_log) are mounted — otherwise
tests/e2e/test_audit_log_ui.py hits 404.
@antosubash
antosubash merged commit 4e5a02a into mainMay 28, 2026
12 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@antosubash