feat: error log admin diagnostics + multi-role dean login fix (#397)#398
Merged
Merged
Conversation
Two related changes bundled together — the dean login 500 was the catalyst for the diagnostic page, so both ship in the same PR. ## Error log admin diagnostics Captures unhandled 5xx exceptions so they're visible on the admin console instead of vanishing into a generic 500 response. - new `error_log` table (entity + migration) — never soft-deleted, indexed on status_code / path / user_id / error_name / acknowledged_at / occurred_at for filter performance - `SystemErrorsModule` (`src/modules/system-errors/`) mirrors `AuditModule` shape: `ErrorLogService.Emit()` async-enqueues via BullMQ, `ErrorLogProcessor` persists, `ErrorLogQueryService` handles paginated reads + acknowledge flow, `ErrorLogController` exposes `/error-logs` endpoints behind a `@UseJwtGuard(SUPER_ADMIN)` - `ErrorCaptureFilter` global exception filter: catches everything, only persists ≥500, redacts sensitive keys (`password`/`token`/`refreshToken`/`authorization` etc.) and caps payload depth/breadth/string length, then defers to `BaseExceptionFilter` so the wire response is byte-identical to today - `ErrorLogCleanupJob` daily at 04:00 UTC — 90-day retention via `nativeDelete` on `occurred_at` - 12 new unit tests covering sanitizer behaviour and filter capture paths (4xx skipped, 5xx captured, non-http hosts skipped, capture failures swallowed without re-throwing) ## Multi-role dean login fix (ucmn-t-67092) Adds defensive guards to `MoodleUserHydrationService.resolveInstitutionalRoles` so users with both DEAN and CHAIRPERSON institutional roles can log in when the populated `moodleCategory` relation is null (soft-delete filter or drift after Moodle restructure). Seven previously-unguarded `ir.moodleCategory.moodleCategoryId` dereferences now optional-chain + filter; orphaned auto rows are now removed instead of crashing. Regression tests cover both DEAN-orphan and CHAIRPERSON-orphan shapes. Single-role deans skip the (DEAN ∧ CHAIRPERSON) cleanup branch, which is why only the intersection user tripped the TypeError. ## Migration scope (verified safe on staging) This migration also adds `analysis_pipeline_trigger_check` — `@Enum(() => PipelineTrigger)` is declared on the entity but the CHECK was missing from the DB. Verified: 0 violating rows, no name collision. Does NOT include the CLI-suggested `drop index uq_analysis_pipeline_active_scope`. That's the FAC-132 partial unique index — MikroORM decorators can't represent it (see new gotcha entry in `src/entities/CLAUDE.md`) and dropping it would reintroduce the duplicate-active-pipeline bug it fixed. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two related changes bundled together — the dean login 500 was the catalyst for the diagnostic page, so both ship in the same PR.
Error log admin diagnostics
Captures unhandled 5xx exceptions so they're visible on the admin console instead of vanishing into a generic 500 response.
error_logtable (entity + migration) — never soft-deleted, indexed on status_code / path / user_id / error_name / acknowledged_at / occurred_at for filter performanceSystemErrorsModule(src/modules/system-errors/) mirrorsAuditModuleshape:ErrorLogService.Emit()async-enqueues via BullMQ,ErrorLogProcessorpersists,ErrorLogQueryServicehandles paginated reads + acknowledge flow,ErrorLogControllerexposes/error-logsendpoints behind a@UseJwtGuard(SUPER_ADMIN)ErrorCaptureFilterglobal exception filter: catches everything, only persists ≥500, redacts sensitive keys (password/token/refreshToken/authorizationetc.) and caps payload depth/breadth/string length, then defers toBaseExceptionFilterso the wire response is byte-identical to todayErrorLogCleanupJobdaily at 04:00 UTC — 90-day retention vianativeDeleteonoccurred_atMulti-role dean login fix (ucmn-t-67092)
Adds defensive guards to
MoodleUserHydrationService.resolveInstitutionalRolesso users with both DEAN and CHAIRPERSON institutional roles can log in when the populatedmoodleCategoryrelation is null (soft-delete filter or drift after Moodle restructure). Seven previously-unguardedir.moodleCategory.moodleCategoryIddereferences now optional-chainSingle-role deans skip the (DEAN ∧ CHAIRPERSON) cleanup branch, which is why only the intersection user tripped the TypeError.
Migration scope (verified safe on staging)
This migration also adds
analysis_pipeline_trigger_check—@Enum(() => PipelineTrigger)is declared on the entity but the CHECK was missing from the DB. Verified: 0 violating rows, no name collision.Does NOT include the CLI-suggested
drop index uq_analysis_pipeline_active_scope. That's the FAC-132 partial unique index — MikroORM decorators can't represent it (see new gotcha entry insrc/entities/CLAUDE.md) and dropping it would reintroduce the duplicate-active-pipeline bug it fixed.