fix: Enforce name limit of 2 again for custom emojis#11375
fix: Enforce name limit of 2 again for custom emojis#11375
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #11375 +/- ##
==========================================
+ Coverage 32.43% 32.45% +0.01%
==========================================
Files 369 369
Lines 13616 13620 +4
Branches 1068 1069 +1
==========================================
+ Hits 4416 4420 +4
Misses 9065 9065
Partials 135 135
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
📝 WalkthroughWalkthroughUpdates emoji test cases in button and select menu builders to use emoji character objects and adjust validation. Modifies emojiPredicate in Assertions.ts to enforce that custom emoji names must be at least 2 characters when both id and name fields are provided. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
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.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
packages/builders/__tests__/components/button.test.tspackages/builders/__tests__/components/selectMenu.test.tspackages/builders/src/components/Assertions.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/builders/__tests__/components/button.test.ts (2)
packages/builders/src/components/button/CustomIdButton.ts (2)
PrimaryButtonBuilder(41-45)DangerButtonBuilder(68-72)packages/builders/src/components/button/LinkButton.ts (1)
LinkButtonBuilder(17-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Tests
- GitHub Check: Tests
🔇 Additional comments (9)
packages/builders/__tests__/components/button.test.ts (6)
24-32: LGTM! Unicode emoji test correctly updated.The test now uses an emoji character object
{ name: '🩵' }instead of a string, and the addition ofbutton.toJSON()ensures the emoji validation is exercised during serialization. This aligns with the updated validation logic.
34-42: LGTM! Custom emoji validation test correctly added.This test case properly validates custom emojis with both
idandnamefields. The name 'test' has 4 characters, which satisfies the new minimum length requirement of 2 characters for custom emoji names.
49-52: LGTM! Unicode emoji test updated correctly.The test correctly uses an emoji character object for a unicode emoji on a danger button.
59-62: LGTM! Link button emoji test updated correctly.The test correctly uses an emoji character object for a unicode emoji on a link button.
77-91: LGTM! Critical test case for custom emoji name length validation.This test segment correctly validates the PR's main objective:
- Line 79-81: Properly expects an error when passing an emoji string instead of an object
- Line 88: Critically tests the new constraint - a custom emoji (with
id) having a name of length 1 should throw an error, enforcing the minimum 2-character requirement for custom emoji names
109-109: LGTM! Invalid emoji format test.Correctly expects an error when passing an emoji string instead of an object.
packages/builders/src/components/Assertions.ts (2)
11-13: LGTM! Improved error message for mandatory field validation.The updated error message with proper punctuation improves readability. However, note that this refine only checks for
undefinedand doesn't prevent empty strings (see comment on line 8).
14-26: LGTM! Correctly enforces minimum name length for custom emojis.This cross-field validation perfectly addresses the PR objective by enforcing that custom emojis (those with an
id) must have names of at least 2 characters when thenamefield is provided. The validation logic is correct:
- When both
idandnameare defined: enforcesname.length >= 2- When only
idOR onlynameis defined: validation passes (returnstrue)This allows unicode emojis (no
id) to have single-character names like{ name: '🩵' }while preventing custom emojis from having single-character names like{ id: '123', name: 'a' }.packages/builders/__tests__/components/selectMenu.test.ts (1)
83-83: LGTM! Select menu emoji test updated consistently.The change from
{ name: 'test' }to{ name: '🩵' }aligns with the emoji handling updates across the test suite. This correctly tests unicode emoji usage in select menu options, consistent with the changes made to button tests.
| .setCustomId('custom') | ||
| .setLabel('test') | ||
| .setDisabled(true) | ||
| .setEmoji({ id: '1234567890123456', name: '1' }); |
There was a problem hiding this comment.
For custom emojis, does discord actually validate their name? Because when sending a message, we can still do <:_:[id]> no?
There was a problem hiding this comment.
They don't validate, but (some) clients viewing the emote will grab the name from cache.
Still, they are documented as 2-32 long in name, you cannot name them as single chars, and our previous hacky workaround of ignoring this and using _ would break markdown in places.
|
FYI the |
|
Did you try uploading an emoji? {
"message": "Invalid Form Body",
"code": 50035,
"errors": {
"name": {
"_errors": [
{
"code": "BASE_TYPE_BAD_LENGTH",
"message": "Must be between 2 and 32 in length."
}
]
}
}
} |
|
I sent a primary button component with a custom emoji, which is one of the predicates this pull request is modifying |
|
Right, and how did you get that custom emoji? It was uploaded somewhere. |
|
@Jiralite i think they're referring to the API validating the emoji name when creating the message, not the emoji structure itself... |
Fixes a regression introduced with #11371 that allowed custom emojis to have a name of length 1.
Also fixes the dishonest emoji tests where a name was specified with a string instead of actual emojis.