-
Notifications
You must be signed in to change notification settings - Fork 57
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
fix for issue #68: send embed
and components
only if filled
#69
fix for issue #68: send embed
and components
only if filled
#69
Conversation
note: discord have no documented `embed` attribute, it is called `embeds` and should be send as an array. Maybe in the older days it was called `embed`. I go with the current documentation here, see https://discord.com/developers/docs/resources/channel#create-message
embed
and components
only if filledembed
and components
only if filled
api v10 takes the newer array "embeds", |
ok quick test shows the if I foce v6 this works ok |
I have tested it, if nothing is specified the newer array "embeds" is working correctly. |
src/DiscordChannel.php
Outdated
@@ -31,16 +31,24 @@ public function __construct(Discord $discord) | |||
*/ | |||
public function send($notifiable, Notification $notification) | |||
{ | |||
if (! $channel = $notifiable->routeNotificationFor('discord', $notification)) { | |||
if (!$channel = $notifiable->routeNotificationFor('discord', $notification)) { |
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.
suspect this change will fail StyleCI check since the default in Laravel is with the space
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.
I have fixed it with another commit. I personally don't use StyleCI nor I have an integration in my IDE for that.
I'm waiting for someone to approve this pull request. |
Thx for fixing. I'm waiting for merging this pr. |
1 similar comment
Thx for fixing. I'm waiting for merging this pr. |
For everyone who's waiting for this PR, maybe you can implement a workaround for your stuff. With a filled |
I also would like to see this PR merged please |
Can this please be merged? |
Why has this not been merged? Can someone please escalate and merge. 🙏 |
I have send an email to the maintainer @atymic I hope he responds quickly. Sadly only the maintainer can approve PRs. I have also done some research, maybe @codyphobe or @arcdigital can merge this PR too. |
I can take a look later today! |
11ab1bf
into
laravel-notification-channels:master
Sorry guys, I didn't see the emails from github. Thanks @deto1986 for emailing me. If anyone is interested in taking over maintenance of this channel please comment and we can have a chat. |
I'm interested, I use this channel quite often. |
note: discord have no documented
embed
attribute, it is calledembeds
and should be send as an array. Maybe in the older days it was calledembed
. I go with the current documentation here, see https://discord.com/developers/docs/resources/channel#create-message