Skip to content

refactor(audit-log): cache fetched user in AuditLogManager#216

Merged
RedStar071 merged 2 commits into
mainfrom
refactor/audit-log-cache-fetched-user
Jun 24, 2026
Merged

refactor(audit-log): cache fetched user in AuditLogManager#216
RedStar071 merged 2 commits into
mainfrom
refactor/audit-log-cache-fetched-user

Conversation

@RedStar071

Copy link
Copy Markdown
Member

Summary

  • Adds a #user private field to AuditLogManager to memoize the result of container.client.users.fetch
  • Replaces the verbose try/catch block in #fetchUser with a single ??= assignment for lazy caching
  • Removes the minimal fallback user object — errors from the fetch now propagate to callers

Type of Change

  • Refactor (no functional change, improves code structure)

Test Plan

  • Existing audit log events still post correctly in a test guild
  • #fetchUser is not called more than once per AuditLogManager instance when the same user triggers multiple log entries

Introduce a #user field to memoize the result of container.client.users.fetch,
avoiding repeated network calls within the same AuditLogManager instance.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Audit log entries now more reliably display the correct acting user in command and settings change events.
    • Improved channel log rendering so embeds are less likely to fail and now show cleaner, more consistent command and channel details.
    • Access-denied settings messages now include the provided reason more reliably.
    • Settings change audit logs now handle change details more consistently, helping ensure key updates are still shown.

Walkthrough

AuditLogManager is refactored so that #emitChannelLog fetches the acting Discord user once and passes the resulting actor object into #buildCommandExecuteEmbed and #buildSettingsChangeEmbed, making both builders synchronous and removing their internal user-fetch logic. #fetchUser is updated to standard async method syntax. Tests add a createUser stub and update assertions to match the revised embed structure.

Changes

AuditLogManager actor fetch hoisting

Layer / File(s) Summary
Actor fetch hoisted; embed builders made synchronous
src/lib/database/settings/structures/AuditLogManager.ts
#emitChannelLog calls #fetchUser(params.actorId) once before branching, then passes the resolved actor into #buildCommandExecuteEmbed(t, actor, payload) and #buildSettingsChangeEmbed(t, actor, payload). Both builders remove their internal actorId-based fetch and become synchronous. #fetchUser is updated to standard async #fetchUser(userId) method syntax.
Test stubs and assertion updates
tests/lib/database/settings/structures/AuditLogManager.test.ts
Imports createUser and stubs container.client.users.fetch and container.client.user in the channel-emit fan-out setup. Updates #buildCommandExecuteEmbed assertions to check data.description for backtick-formatted command names and slash mentions instead of embed field values. Relaxes the settings-change diff-field minimum from 2 to 1, and changes the access-denied description check to toContain.

Possibly related PRs

  • wolfstar-project/wolfstar#211: Directly overlaps with this PR's refactor of #buildCommandExecuteEmbed and #buildSettingsChangeEmbed to accept a pre-fetched actor and adjust embed structure.
  • wolfstar-project/wolfstar#215: Also modifies AuditLogManager embed construction for the same builders, including author/description/footer behaviour and i18n field usage.

Suggested reviewers

  • lorypelli
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: refactoring AuditLogManager to cache the fetched user.
Description check ✅ Passed The description is related to the changeset, even though some implementation details differ from the actual patch.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/audit-log-cache-fetched-user

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

…nchronous

#buildCommandExecuteEmbed and #buildSettingsChangeEmbed are now sync
methods that receive a pre-fetched User, keeping the makeMessage factory
synchronous as the GuildMessageLog event system expects.

Also restores the try/catch fallback in #fetchUser that was lost in the
previous commit, and updates tests to reflect the description-based
embed format introduced in PR #215.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
tests/lib/database/settings/structures/AuditLogManager.test.ts (1)

7-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use configured path aliases instead of deep relative import

Line 7 should use the configured alias form (for example via #root/*) rather than ../../../../... to stay consistent with the TS import standard in this repo.
As per coding guidelines, "Use path aliases: #lib/, #utils/, #generated/prisma, #root/*, #languages".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/lib/database/settings/structures/AuditLogManager.test.ts` at line 7,
The test import in AuditLogManager.test.ts is using a deep relative path instead
of the repo’s configured alias style. Update the createUser import to use the
appropriate path alias (for example `#root/`*) rather than traversing with
../../../../, and keep the import consistent with the alias conventions used
elsewhere in the test suite.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/lib/database/settings/structures/AuditLogManager.test.ts`:
- Line 7: The test import in AuditLogManager.test.ts is using a deep relative
path instead of the repo’s configured alias style. Update the createUser import
to use the appropriate path alias (for example `#root/`*) rather than traversing
with ../../../../, and keep the import consistent with the alias conventions
used elsewhere in the test suite.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: fa3aba36-2971-4fe9-83ac-7c3b44e18646

📥 Commits

Reviewing files that changed from the base of the PR and between f0f6799 and a67debb.

📒 Files selected for processing (2)
  • src/lib/database/settings/structures/AuditLogManager.ts
  • tests/lib/database/settings/structures/AuditLogManager.test.ts

@RedStar071 RedStar071 merged commit 6816f79 into main Jun 24, 2026
9 of 10 checks passed
@RedStar071 RedStar071 deleted the refactor/audit-log-cache-fetched-user branch June 24, 2026 18:11
@github-project-automation github-project-automation Bot moved this from Todo to Done in Wolfstar Jun 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant