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

Finish base webhooks #1261

Closed

Conversation

thedudedies21
Copy link

In this update, me and Mathium worked on finishing the updates required to fully reimplement the api for webhooks in such a way that matched expected behaviors across the board

Comment on lines +159 to +166
const blockedContains = ["discord", "clyde", "spacebar"];
for (const word of blockedContains) {
if (body.username.toLowerCase().includes(word)) {
return res.status(400).json({
username: [`Username cannot contain "${word}"`],
});
}
}
Copy link
Member

Choose a reason for hiding this comment

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

should this not be a config option? this code is repeated in src/api/routes/channels/#channel_id/webhooks.ts which I'm not a fan of

Copy link
Contributor

Choose a reason for hiding this comment

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

this is how the code was before in spacebar, this isn't new, though I get wanting it to be an option

throw DiscordApiErrors.UNKNOWN_WEBHOOK;

if (!body.name && !body.avatar && !body.channel_id) {
throw new HTTPError("Empty messages are not allowed", 50006);
Copy link
Member

Choose a reason for hiding this comment

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

slightly misleading error message?

@MathMan05
Copy link
Contributor

hey, just to note, you might wanna do what the failed test says to do

@thedudedies21
Copy link
Author

Im gonna close this as I added in some new stuff to complete and merge with another pull request

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants