feat(Guild): add message search#11459
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
📝 WalkthroughWalkthroughAdded a new public Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Guild
participant Validation
participant REST as REST Client
participant API as Discord API
participant Cache as Message Cache
User->>Guild: searchMessages(options)
Guild->>Validation: validate options (content, authorType, has, etc.)
alt Invalid Input
Validation-->>Guild: throw DiscordjsTypeError
Guild-->>User: error
end
Validation-->>Guild: options valid
Guild->>REST: buildQuery & GET /channels/{id}/messages/search
REST->>API: HTTP GET request
API-->>REST: { messages, total_results }
REST-->>Guild: response data
loop For each message in response
Guild->>Cache: check cached message
alt Message cached
Cache-->>Guild: return cached Message
else Message not cached
Guild->>Guild: create new Message instance
end
end
Guild->>Guild: build Collection from messages
Guild-->>User: { totalResults, messages: Collection }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can disable the changed files summary in the walkthrough.Disable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/discord.js/src/structures/Guild.js`:
- Around line 1184-1197: The numeric validations in Guild.js for limit, offset
and slop only check type and range but not integer-ness; update the validation
in the block that handles limit, offset and slop (the checks around the
variables limit, offset, slop in the Guild class) to also require integers by
using Number.isInteger(value) (or equivalent) after the typeof check and before
the range check, and throw the same DiscordjsTypeError(ErrorCodes.InvalidType,
'<name>', 'integer') (or a matching error code/message) when the value is not an
integer so fractional values like 1.5 are rejected.
- Around line 1219-1249: Add client-side length validation for the search array
parameters in the same function that currently validates element values (where
variables like channelId, authorId, mentions, mentionsRoleId, repliedToUserId,
repliedToMessageId, embedProvider, linkHostname, attachmentFilename,
attachmentExtension are handled): create an object mapping each param to its
documented max (e.g., channelId:500, authorId:100, mentions:100,
mentionsRoleId:100, repliedToUserId:100, repliedToMessageId:100,
embedProvider:100, linkHostname:100, attachmentFilename:100,
attachmentExtension:100) and iterate it, checking if the corresponding
variable's length exceeds the max and throwing new
DiscordjsTypeError(ErrorCodes.ClientInvalidOption, param, `at most ${max}
elements`) when it does; place these checks alongside the existing validations
for authorType/has/embedType so they run before the API call.
- Around line 1090-1091: Fix the JSDoc indentation for the block whose
description is "Options used to search messages in a guild." by removing the
extra leading spaces before the opening /** so it lines up with other JSDoc
blocks in the file (the comment immediately preceding the guild message search
options definition in the Guild class). Ensure the opening /** indentation
matches adjacent JSDoc blocks to keep formatting consistent.
- Around line 1280-1283: The code in Guild.js uses the Message constructor as a
fallback in the messages reduction (new Message(this.client, message)) but
Message is not imported, causing a runtime ReferenceError; add an import for the
Message class alongside the other structure imports near the top of the file so
that Message is available when computing messages (the reduction referencing
this.client.channels.cache.get, channel?.messages._add, and new Message(...)).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e28e2b08-78ff-4d1a-9309-0a4d8ffbf056
📒 Files selected for processing (1)
packages/discord.js/src/structures/Guild.js
📜 Review details
🔇 Additional comments (3)
packages/discord.js/src/structures/Guild.js (3)
6-15: LGTM!The new imports for message search types are correctly added and align with the validation logic in
searchMessages.
1153-1179: LGTM on the method signature and parameter destructuring.The destructuring pattern with default values for
cache = trueis consistent with other methods in the codebase. The parameter list comprehensively covers the Discord API's search options.
1199-1217: LGTM on enum and boolean validations.The validation for
sortBy,sortOrder, and boolean parameters (mentionEveryone,pinned,includeNsfw) is correct and provides clear error messages.
| } | ||
| } | ||
|
|
||
| const data = await this.client.rest.get(Routes.guildMessagesSearch(this.id), { query }); |
There was a problem hiding this comment.
This is not handling the 202 Accepted response.
TBD if we want to handle retries automatically here or just throw
| offset, | ||
| slop, | ||
| } = {}) { | ||
| if (content !== undefined && typeof content !== 'string') { |
There was a problem hiding this comment.
Do we really need all these checks here? personally, I think this should just be left to the api to return the error, unless the error is too broad (i have not checked).
Perhaps we can even delegate most of these to a MessagsSearchParamBuilder, which I think makes sense to have for this.
Ig it depends on what others will say 🤷
There was a problem hiding this comment.
Removed validation. Not sure about the builder though, will wait for input from other reviewers.
| embedProvider?: readonly string[]; | ||
| embedType?: readonly MessageSearchEmbedType[]; | ||
| has?: readonly MessageSearchHasType[]; | ||
| includeNsfw?: boolean; |
There was a problem hiding this comment.
I brought up this topic here #11393 (comment) but didn't get a response. I also voiced my opinion there
There was a problem hiding this comment.
@discordjs/core thoughts? I lean more towards includeNSFW
5 participants
Add guild messages search functionality.