-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[Notifier][Discord] Add DiscordBotTransport
#60218
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
Conversation
DiscordBotTransport
DiscordBotTransport
9016866
to
b213dbf
Compare
src/Symfony/Component/Notifier/Bridge/Discord/DiscordTransportFactory.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Notifier/Bridge/Discord/DiscordOptions.php
Outdated
Show resolved
Hide resolved
b213dbf
to
c7f59ba
Compare
f1999a2
to
d502376
Compare
DiscordBotTransport
DiscordBotTransport
d502376
to
4815173
Compare
@OskarStark maybe you could approve here? :) |
4815173
to
f4a9afb
Compare
f4a9afb
to
ee429bf
Compare
$options = $message->getOptions()?->toArray() ?? []; | ||
$options['content'] = $message->getSubject(); | ||
|
||
if (mb_strlen($options['content'], 'UTF-8') > self::SUBJECT_LIMIT) { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
Thank you @norkunas. |
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
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 adiscord+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.