Add MS Teams webhook handler #2002
Conversation
|
|
||
| if ($this->includeContextAndExtra) { | ||
| $this->normalizerFormatter = new NormalizerFormatter(); | ||
| } |
There was a problem hiding this comment.
This is already done in ->includeContextAndExtra()
Seb33300
force-pushed
the
ms-teams-handler
branch
from
1e74014 to
953482c
Compare
| return $this->teamsRecord; | ||
| } | ||
|
|
||
| public function getWebhookUrl(): string |
There was a problem hiding this comment.
I don't think those getters should exist
| * @author Sébastien Alfaiate <[email protected]> | ||
| * @see https://learn.microsoft.com/adaptive-cards/authoring-cards/getting-started | ||
| */ | ||
| class TeamsRecord |
There was a problem hiding this comment.
This class should probably be @internal
| /** | ||
| * @return $this | ||
| */ | ||
| public function includeContextAndExtra(bool $includeContextAndExtra = false): self |
There was a problem hiding this comment.
No need to create public methods for that. The constructor should assign its config directly.
| public function getFormatter(): FormatterInterface | ||
| { | ||
| $formatter = parent::getFormatter(); | ||
| $this->teamsRecord->setFormatter($formatter); |
There was a problem hiding this comment.
Instead of those hacks to synchronize the formatter in the TeamsRecord helper, I would suggest passing the formatter of the handler as a parameter in getAdaptiveCardPayload instead.
| * @author Sébastien Alfaiate <[email protected]> | ||
| * @see https://learn.microsoft.com/adaptive-cards/authoring-cards/getting-started | ||
| */ | ||
| class TeamsRecord |
There was a problem hiding this comment.
This class name is weird to me. This helper is a service creating the payload for the Teams API. It is not the representation of a record (creating confusion with the LogRecord class of the public Monolog API)
| /** | ||
| * Returns MS Teams container style associated with provided level. | ||
| */ | ||
| public function getContainerStyle(Level $level): string |
There was a problem hiding this comment.
Those methods should be private as they are only used internally.
Seb33300
force-pushed
the
ms-teams-handler
branch
4 times, most recently
from
1744888 to
c093448
Compare
Seb33300
force-pushed
the
ms-teams-handler
branch
from
c093448 to
b7697f8
Compare
|
Hi @stof, Thanks for your review. I think all your comments are fixed now. Please note that my proposal was based on the Slack code, so most of your comments are also applicable to the Slack implementation. |
2 participants
We are migrating from Slack to Microsoft Teams and we would like to continue receiving log notifications there, as this feature has been extremely useful for our workflow.
I am also planning to submit a PR on Laravel to support this handler if this PR is approved (Laravel already supports the slack handler).
The implementation is inspired from the existing
SlackWebhookHandler.This handler support an additional parameter
toggleFieldswhere we can specify the list of field we want to display using a toggle button (eg: useful for stacktrace)Here is how it looks:
(Note the username and avatar has to be configured when creating the webhook URL)
When we click on the toggle button: