Skip to content

Commit 4e5a02a

Browse files
authored
feat(audit_log): add audit log module with two-phase capture (#185)
* docs: add audit log module design spec Covers data model, capture mechanism (SQLAlchemy before_flush callback), module structure, REST API, admin Browse page, and testing strategy. * docs: add audit log module implementation plan 13-task TDD plan covering framework changes (AuditRecord, callback wiring), module scaffold, model, service, API, Inertia Browse page, tests, and migration. * feat(db): add AuditRecord dataclass and collect_audit_records diff logic * feat(db): wire audit callback into DatabaseState and before_flush listener * feat(audit_log): scaffold module package with constants and host wiring * feat(audit_log): add AuditEntry model and DTO schemas * feat(audit_log): add capture callback * feat(audit_log): add service layer and deps * feat(audit_log): add API and Inertia view endpoints * feat(audit_log): add AuditLogModule with lifecycle hooks * feat(audit_log): add i18n locale strings * feat(audit_log): add Browse.tsx page with filters, pagination, and change diffs * test(audit_log): add integration tests for capture, API filtering, and recursion guard Also fix AuditEntryRead.id type from str to uuid.UUID to match the AuditEntry model (surfaced by the new tests). * migration(audit_log): add audit_log_audit_entry table * chore: regenerate i18n and package-lock with audit_log module * fix(audit_log): use datetime-local inputs and preserve page_size in navigation * fix(test): ensure test_filter_by_action is non-vacuous * fix(audit_log): deterministic pagination, error-resilient capture, entity_id filter, non-null created_at * fix(db): correctly classify soft-deleted entities and exclude SoftDeleteMixin fields from audit diffs * fix(audit_log): gracefully handle invalid query params in view endpoint 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. * fix(db): two-phase audit capture resolves DB-assigned integer PKs 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. * chore(audit_log): format files, fix frozenset typing, add e2e specs - 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) * chore: add QA report and verification screenshots * style(audit_log): align Browse page layout with other module pages * fix(audit_log): satisfy CI checks for the layout pass - 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 * fix(audit_log): inline useT in ChangesList to satisfy TS strict typing 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. * fix(ci): unblock PR checks - 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. * ci(e2e): include AuditLog in SM_MODULES_ENABLED allowlist 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.
1 parent 66bb303 commit 4e5a02a

42 files changed

Lines changed: 4494 additions & 2 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.github/workflows/pr.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ jobs:
149149
SM_USERS_BOOTSTRAP_PASSWORD: admin
150150
# Exclude Keycloak module — SM020 prevents both users and keycloak
151151
# from running simultaneously. E2E tests use the users module.
152-
SM_MODULES_ENABLED: '["Auth","Users","Dashboard","Permissions","Settings","BackgroundTasks","FileStorage","FeatureFlags"]'
152+
SM_MODULES_ENABLED: '["Auth","Users","Dashboard","Permissions","Settings","BackgroundTasks","FileStorage","FeatureFlags","AuditLog"]'
153153
E2E_BASE_URL: http://localhost:8000
154154
steps:
155155
- uses: actions/checkout@v6
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
# QA Report: Audit Log Module
2+
**Date:** 2026-05-28
3+
**Tester:** Claude QA (Senior)
4+
**Target:** http://localhost:8000/audit_log
5+
**Depth:** normal
6+
**Iteration:** 1 of 3
7+
8+
## Summary
9+
| Category | Passed | Failed | Skipped |
10+
|----------|--------|--------|---------|
11+
| Happy Path | 8 | 2 | 0 |
12+
| Form Validation | — | — | — (agent timed out) |
13+
| Error States | 11 | 6 | 0 |
14+
| **Total** | **19** | **8** | **0** |
15+
16+
## Critical Issues (P0)
17+
None.
18+
19+
## Major Issues (P1)
20+
21+
### BUG-001: Invalid query params render raw JSON validation errors
22+
- **Severity:** P1 (4 instances: ES-005, ES-006, ES-007, ES-008)
23+
- **Steps to reproduce:** Navigate to /audit_log?page_size=0 or /audit_log?page=-1 or /audit_log?page=abc or /audit_log?page_size=500
24+
- **Expected:** Graceful fallback — clamp to defaults or show user-friendly error page
25+
- **Actual:** Raw FastAPI JSON validation error shown as entire page content: `{"detail":[{"type":"greater_than_equal",...}]}`
26+
- **Root cause:** Inertia view endpoint uses `Query(ge=1, le=200)` constraints which raise `RequestValidationError` — no handler converts these to Inertia-friendly responses
27+
- **Fix hint:** Add a `RequestValidationError` exception handler in the view that either clamps to defaults or renders an Inertia error page. Could be framework-level (benefits all modules).
28+
29+
## Minor Issues (P2)
30+
31+
### BUG-002: Setting entity_id is empty string for integer-PK entities
32+
- **Severity:** P2
33+
- **Steps to reproduce:** Create a Setting via POST /api/settings/, then check the audit log for that Setting's "Created" entry
34+
- **Expected:** entity_id shows the Setting's actual ID (e.g., "1")
35+
- **Actual:** entity_id is "" (empty string) — the entity column shows "Setting" with no ID
36+
- **Root cause:** Known limitation — `_entity_pk_str` runs during before_flush when integer PKs haven't been assigned yet. UUID PKs work fine.
37+
- **Fix hint:** Architectural change needed — move to after_flush or accept as known limitation for integer-PK models.
38+
39+
## Observations (P3)
40+
41+
### OBS-001: Sidebar "Audit Log" link has no active/highlighted state
42+
- **Severity:** P3
43+
- **Details:** All sidebar links share identical CSS classes regardless of current page. No `aria-current="page"` is set. This is a framework-wide issue affecting all modules, not specific to Audit Log.
44+
45+
### OBS-002: Entity Type dropdown opens on Tab focus
46+
- **Severity:** P3
47+
- **Details:** The Radix Select component's Entity Type dropdown auto-opens when receiving Tab focus, potentially trapping keyboard navigation. This is a known Radix UI behavior.
48+
49+
### OBS-003: Out-of-range page shows empty state without context
50+
- **Severity:** P3
51+
- **Details:** /audit_log?page=999 shows "No audit entries" empty state. Could show "Page out of range" or redirect to last valid page.
52+
53+
### OBS-004: Form validation agent timed out
54+
- **Details:** The form validation agent stalled while testing datetime-local inputs (likely Playwright interaction complexity with date pickers). Core form validation was partially covered by other agents.
55+
56+
## Passed Tests
57+
58+
<details>
59+
<summary>Click to expand (19 tests passed)</summary>
60+
61+
| # | Category | Scenario | Result |
62+
|---|----------|----------|--------|
63+
| 1 | Happy Path | Page loads with data | PASS |
64+
| 2 | Happy Path | Filter by Entity Type | PASS |
65+
| 3 | Happy Path | Filter by Action | PASS |
66+
| 4 | Happy Path | Filter by User ID | PASS |
67+
| 5 | Happy Path | Clear filters | PASS |
68+
| 6 | Happy Path | Pagination (Next/Previous) | PASS |
69+
| 7 | Happy Path | New entity generates audit entry | PASS |
70+
| 8 | Happy Path | Empty state display | PASS |
71+
| 9 | Error States | Empty state for zero results | PASS |
72+
| 10 | Error States | URL-based filter preselection | PASS |
73+
| 11 | Error States | page_size=5 via URL | PASS |
74+
| 12 | Error States | Browser back preserves state | PASS |
75+
| 13 | Error States | Page refresh preserves filters | PASS |
76+
| 14 | Error States | Enter key submits filter form | PASS |
77+
| 15 | Error States | Rapid Apply clicks (5x) | PASS |
78+
| 16 | Error States | Unauthenticated redirect to login | PASS |
79+
| 17 | Error States | API endpoint returns JSON | PASS |
80+
| 18 | Error States | API returns 401 unauthenticated | PASS |
81+
| 19 | Error States | XSS in params safely handled | PASS |
82+
83+
</details>
192 KB
Loading
47.7 KB
Loading

‎.verify/screenshot.png‎

-44.4 KB
Loading

0 commit comments

Comments
 (0)