feat: add AuditLogManager for dashboard settings audit logging#188
feat: add AuditLogManager for dashboard settings audit logging#188RedStar071 wants to merge 1 commit intomainfrom
Conversation
- Add AuditLog Prisma model with migration - Add AuditLogManager following PermissionNodeManager conventions - Add AuditLogChange type and PrismaJson augmentation - Integrate into SettingsContext with constructor/onPatch pattern - Wire audit logging into settings.patch.ts with structuredClone snapshot - Add GET /guilds/:guild/audit-logs endpoint with pagination - Add 18 unit tests for AuditLogManager
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Introduces dashboard settings audit logging by adding a new AuditLog Prisma model plus a settings-side AuditLogManager, wiring it into the settings PATCH flow, and exposing a paginated API endpoint to read audit log entries.
Changes:
- Add
AuditLogPrisma model + migration (tableaudit_log, index on(guild_id, created_at DESC)). - Add
AuditLogManagerand integrate it intoSettingsContext, plus exportedreadSettingsAuditLog()helper. - Add
GET /guilds/:guild/audit-logsendpoint and add audit-log writes toPATCH /guilds/:guild/settings, plus unit tests forAuditLogManager.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/lib/database/settings/structures/AuditLogManager.test.ts | Adds unit tests for audit log manager behavior (constructor/onPatch/update/add/remove + section/value derivation). |
| src/routes/guilds/[guild]/settings.patch.ts | Adds fire-and-forget audit logging on settings mutation (currently has a snapshot correctness issue). |
| src/routes/guilds/[guild]/audit-logs.get.ts | Adds paginated endpoint to fetch audit logs for a guild (minor consistency issue vs other routes). |
| src/lib/types/Augments.d.ts | Adds PrismaJson augmentation type AuditLogChanges. |
| src/lib/database/settings/types.ts | Introduces AuditLogChange type. |
| src/lib/database/settings/structures/AuditLogManager.ts | Implements AuditLogManager with update/add/remove/write + section/value helpers. |
| src/lib/database/settings/index.ts | Re-exports AuditLogManager. |
| src/lib/database/settings/functions.ts | Adds readSettingsAuditLog() helper (currently returns cached manager, affecting snapshot use-cases). |
| src/lib/database/settings/context/SettingsContext.ts | Adds auditLog manager to SettingsContext and updates it on patch. |
| prisma/schema.prisma | Adds AuditLog model and Guild.auditLogs relation. |
| prisma/migrations/20260416120000_add_dashboard_audit_log/migration.sql | Creates audit_log table, index, and FK to guilds. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Capture current settings for audit log before mutation | ||
| const auditLog = readSettingsAuditLog(structuredClone(trx.settings)); | ||
|
|
||
| await trx.write(settingsData).submit(); | ||
|
|
||
| // Fire-and-forget audit log write | ||
| auditLog.update(request.auth!.id, settingsData).catch(() => null); |
There was a problem hiding this comment.
structuredClone(trx.settings) does not actually create an independent audit-log snapshot here. readSettingsAuditLog() goes through getSettingsContext(settings) which is cached by settings.id, so it returns the existing per-guild AuditLogManager. After trx.write(...).submit(), updateSettingsContext() calls SettingsContext.update() which onPatch()es the same manager with the now-mutated settings object, causing oldValue to reflect the new settings (and often match newValue). To preserve pre-mutation values, create a fresh AuditLogManager instance from the cloned settings (bypassing the context cache), or change readSettingsAuditLog to support returning a non-cached manager for snapshots.
| export function readSettingsAuditLog(settings: ReadonlyGuildData) { | ||
| return getSettingsContext(settings).auditLog; | ||
| } |
There was a problem hiding this comment.
readSettingsAuditLog() returns the cached SettingsContext.auditLog instance keyed by settings.id. This makes it unsuitable for callers that expect a snapshot-based manager (e.g., passing a cloned settings object), because it will still return the shared per-guild instance whose internal #settings reference is updated on every settings patch. Consider returning a new AuditLogManager(settings) here (or adding a separate helper like createSettingsAuditLogSnapshot for non-cached instances) so audit logs can reliably compare against the provided settings object.
| import { authenticated, canManage, ratelimit } from '#lib/api/utils'; | ||
| import { seconds } from '#utils/common'; | ||
| import { container } from '@sapphire/framework'; | ||
| import { HttpCodes, Route } from '@sapphire/plugin-api'; |
There was a problem hiding this comment.
This route uses the global container import instead of the this.container instance property used by the other guild routes in this folder. For consistency (and to make testing/mocking easier), prefer this.container.client / this.container.prisma here as in channels.get.ts, roles.get.ts, etc.
| test('GIVEN new settings THEN updates internal state', () => { | ||
| const newEntity = Object.assign(Object.create(null), getDefaultGuildSettings(), { id: '987654321' }) as ReadonlyGuildData; | ||
| manager.onPatch(newEntity); | ||
|
|
||
| // Verify by calling write which uses the guildId | ||
| manager.write('user1', { action: 'test', section: 'general', changes: [] }); |
There was a problem hiding this comment.
The test calls manager.write(...) without awaiting the returned promise. Although the mock is invoked synchronously before the first await, not awaiting can still lead to unhandled-rejection noise if the implementation changes or the mock rejects. Await the call (or return the promise) to keep the test robust.
| test('GIVEN new settings THEN updates internal state', () => { | |
| const newEntity = Object.assign(Object.create(null), getDefaultGuildSettings(), { id: '987654321' }) as ReadonlyGuildData; | |
| manager.onPatch(newEntity); | |
| // Verify by calling write which uses the guildId | |
| manager.write('user1', { action: 'test', section: 'general', changes: [] }); | |
| test('GIVEN new settings THEN updates internal state', async () => { | |
| const newEntity = Object.assign(Object.create(null), getDefaultGuildSettings(), { id: '987654321' }) as ReadonlyGuildData; | |
| manager.onPatch(newEntity); | |
| // Verify by calling write which uses the guildId | |
| await manager.write('user1', { action: 'test', section: 'general', changes: [] }); |
Summary
Adds an AuditLogManager to track dashboard settings changes, adapted from wolfstar.rocks PR #144 and following the same conventions as `PermissionNodeManager` and other existing managers.
Changes
Database
Core
Routes
Tests