Skip to content

Conversation

norkunas
Copy link
Contributor

@norkunas norkunas commented Apr 15, 2025

Q A
Branch? 7.4
Bug fix? no
New feature? yes
Deprecations? no
Issues N/A
License MIT

Currently Discord transport is limited because it allows to use only server webhook which sends to a single channel.
I propose to add DiscordBotTransport with a discord+bot scheme, so we can send notifications to multiple channels/as direct messages.

Our use case that we have a channel where all notifications must be sent and then more important ones also to people as direct messages.

@norkunas norkunas requested a review from OskarStark as a code owner April 15, 2025 07:46
@carsonbot carsonbot added this to the 7.3 milestone Apr 15, 2025
@carsonbot carsonbot changed the title [Notifier][Discord] Add DiscordBotTransport [Notifier] [Discord] Add DiscordBotTransport Apr 15, 2025
@norkunas norkunas force-pushed the discord-bot-transport branch from 9016866 to b213dbf Compare April 15, 2025 08:38
@norkunas norkunas force-pushed the discord-bot-transport branch from b213dbf to c7f59ba Compare April 15, 2025 09:14
@fabpot fabpot modified the milestones: 7.3, 7.4 May 26, 2025
@norkunas norkunas force-pushed the discord-bot-transport branch 2 times, most recently from f1999a2 to d502376 Compare May 27, 2025 05:57
@OskarStark OskarStark changed the title [Notifier] [Discord] Add DiscordBotTransport [Notifier][Discord] Add DiscordBotTransport May 27, 2025
@norkunas norkunas force-pushed the discord-bot-transport branch from d502376 to 4815173 Compare June 27, 2025 04:32
@norkunas
Copy link
Contributor Author

@OskarStark maybe you could approve here? :)

@norkunas norkunas force-pushed the discord-bot-transport branch from 4815173 to f4a9afb Compare August 19, 2025 06:19
@norkunas norkunas force-pushed the discord-bot-transport branch from f4a9afb to ee429bf Compare August 19, 2025 06:21
$options = $message->getOptions()?->toArray() ?? [];
$options['content'] = $message->getSubject();

if (mb_strlen($options['content'], 'UTF-8') > self::SUBJECT_LIMIT) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove this username check, if the api responds with an actionable error message. Is this the case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then it should be removed from both transports. Can it be done in a follow-up pr?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes

@OskarStark OskarStark requested a review from fabpot August 19, 2025 13:51
@fabpot
Copy link
Member

fabpot commented Aug 19, 2025

Thank you @norkunas.

@fabpot fabpot merged commit b317ccd into symfony:7.4 Aug 19, 2025
4 of 12 checks passed
@norkunas norkunas deleted the discord-bot-transport branch August 20, 2025 06:18
nicolas-grekas added a commit that referenced this pull request Aug 20, 2025
This PR was merged into the 7.4 branch.

Discussion
----------

[Notifier][Discord] Remove length checks

| Q             | A
| ------------- | ---
| Branch?       | 7.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #60218 (comment) #60269 (comment)
| License       | MIT

Commits
-------

2780296 [Notifier][Discord] Remove length checks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants