Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(GuildAuditLogEntry)!: Fix some incorrect types and runtime logic #10591

Open
wants to merge 22 commits into
base: main
Choose a base branch
from

Conversation

cobaltt7
Copy link
Contributor

@cobaltt7 cobaltt7 commented Nov 5, 2024

Please describe the changes this PR makes and why it should be merged:

Fixes some audit log types and runtime logic.

  • The target of MessageBulkDelete is a channel, not a guild
    • Runtime logic already resolved it as a channel
    • Added a typings test
    • BREAKING CHANGE: It also doesn't have a options.channel_id, so I stopped .extra.channel from being set to { id: undefined }
  • AutoModeration trigger logs have the violating User as a target, not the AutoModerationRule
    • BREAKING CHANGE: Fixed both types and runtime logic here, it previously created a broken AutoModerationRule
  • GuildOnboardingCreate and GuildOnboardingUpdate do not have any target
    • BREAKING CHANGE: Removed Targets.GuildOnboarding, it will fallback to Targets.Unknown and generate a placeholder target from the changes
    • Added a note to the types mentioning them and other events that do not have targets
  • Handle SoundboardSound as much as possible right now, add placeholders for after feat: add soundboard #10590
  • Properly resolve the target type of AutoModerationRule rules
  • Include PartialUser in the types where a partial user is possible
  • Properly type the values of the GuildAuditLogsEntry.Target static object
    • Previously was typed as:
      {
        Guild: "Guild" | "Channel" | "User" | ...;
        Channel: "Guild" | "Channel" | "User" | ...;
        User: "Guild" | "Channel" | "User" | ...;
        ...
      }
      Instead of:
      {
        Guild: "Guild";
        Channel: "Channel";
        User: "User";
        ...
      }
  • Hardcode Role and Emoji targets at runtime
    • They were already hardcoded in the types, although they were missing the { id: Snowflake } alternative for if they were uncached

Status and versioning classification:

  • Code changes have been tested against the Discord API, or there are no code changes
  • I know how to update typings and have done so, or typings don't need updating
  • This PR includes breaking changes (methods removed or renamed, parameters moved or removed)

There are some breaking changes, but they all are minor things that never worked properly.

@cobaltt7 cobaltt7 requested a review from a team as a code owner November 5, 2024 22:00
Copy link

vercel bot commented Nov 5, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
discord-js ⬜️ Ignored (Inspect) Visit Preview Dec 27, 2024 2:38pm
discord-js-guide ⬜️ Ignored (Inspect) Visit Preview Dec 27, 2024 2:38pm

@cobaltt7 cobaltt7 changed the title types(GuildAuditLogEntry): Fix some incorrect types fix(GuildAuditLogEntry): Fix some incorrect types and runtime logic Nov 6, 2024
@Jiralite Jiralite added this to the discord.js 15.0.0 milestone Nov 13, 2024
@Jiralite Jiralite changed the title fix(GuildAuditLogEntry): Fix some incorrect types and runtime logic fix(GuildAuditLogEntry)!: Fix some incorrect types and runtime logic Nov 13, 2024
@Qjuh
Copy link
Contributor

Qjuh commented Nov 13, 2024

#10521 already covers half of these changes.

@Jiralite
Copy link
Member

Please resolve conflicts!

@cobaltt7
Copy link
Contributor Author

Resolved conflicts and removed the overlapping changes. Sorry, I hadn't realized that PR existed!

@vladfrangu
Copy link
Member

Hmm, some of these breaking changes sound like we should patch in existing 14 version too... @discordjs/core thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

4 participants