[Notifier] [Bridge] Fix missed messageId for SendMessage object in slack notifier #40955
Conversation
3fae769 to
374788d
Compare
| @@ -91,6 +91,9 @@ protected function doSend(MessageInterface $message): SentMessage | |||
| throw new TransportException(sprintf('Unable to post the Slack message: "%s".', $result['error']), $response); | |||
| } | |||
|
|
|||
| return new SentMessage($message, (string) $this); | |||
| $sentMessage = new SentMessage($message, (string) $this); | |||
| $sentMessage->setMessageId($result['ts']); | |||
There was a problem hiding this comment.
Shall we check if the key exists in the array?
There was a problem hiding this comment.
We already check for "ok" status. I had looked to another transports/bridges, they did not have even this checks.
Which behavior do you expect, if by some unknown reason slack return response with ok status, but with no messageid specified?
There was a problem hiding this comment.
Yes, but ok let's keep it
src/Symfony/Component/Notifier/Bridge/Slack/Tests/SlackTransportTest.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Notifier/Bridge/Slack/Tests/SlackTransportTest.php
Outdated
Show resolved
Hide resolved
|
I would consider this a feature, it is working without the ID and must wait for |
374788d to
838f36b
Compare
I though to put this change as usual to 5.x branch, but checked "Maintenance" page (https://symfony.com/doc/current/contributing/code/maintenance.html) and it's sounds like it matches the rules. |
|
Thank you @WaylandAce. |
There are missed messageId property for SendMessage object in slack notifier.
Regarding slack's documentation: https://api.slack.com/messaging/sending#publishing