Fix Teams workflow webhook #4957
Conversation
This commit backports the Adaptive-Templeate. The version of the template uses is from commit 2c31f3a.
downgrade version, otherwise the error "We're sorry, this card couldn't be displayed" will be displayed when using flowbot, which is default
CommanderStorm
added
area:notifications
There was a problem hiding this comment.
There are lots of changes between your version and the version in master.
Yes, that version is also not perfect, but given how hard it is to review backported versions, please stick to master as close as possible.
There are tons of changes in this PR (compared to master). Most of those don't quite seem intentional or are at least not explained.
This is not a backport, this is a backport with changes in the same commit. Reviewing these changes is pretty hard, given that I now don't have a diff and creates future merge conflicts.
Because of how hard this is to review otherwise:
- Please start with master,
- and then (in different commits) change what is needed only.
That way, reviewing becomes possible again.
I have tried to go through your changes and have commented on the things which are most unclear.
| }); | ||
| } | ||
|
|
||
| // if (heartbeatJSON?.localDateTime) { |
There was a problem hiding this comment.
why did you comment this out?
There was a problem hiding this comment.
I modified the passed paramters to match the old function.
Currently there is no heartbeatJSON in that function.
| const payload = { | ||
| "type": "message", | ||
| // message with status prefix as notification text | ||
| "summary": this._statusMessageFactory(status, monitorName, true), |
There was a problem hiding this comment.
No thrid parameter exists in your modified version of _statusMessageFactory ^^
| * @param {string} monitorUrl URL of monitor affected | ||
| * @returns {Object} | ||
| */ | ||
| _notificationPayloadFactoryLegacy = ({ |
There was a problem hiding this comment.
Please remove the legacy code (including where this function is used)
If microsoft decides to no longer support the webhooks, it is not worth adding code to support them.
Since the feature would be broken otherwise I think this is still a semver patch without this.
Currently, this just produces a bigger merge conflict ^^
There was a problem hiding this comment.
Sure, I can do that.
My thinking was, that i want to keep the old Webhooks running until Mircosoft gets rid of them.
Thought that breaking the old webhooks before that would result in this being classified as a breaking change.
Normally I would have removed that code in a second PR after this one.
If this is a hard requirement to get this merged, i get rid of it now.
CommanderStorm
added
pr:please address review comments
|
Hi @Tabisch In case you don't have time to follow up, I created a PR for your branch based on @CommanderStorm 's requirement. Please feel free to have a look. |
Fix and remove legacy code
|
@tpai Totally forgot that this was still open. |
CommanderStorm
added
pr:needs review
|
@CommanderStorm Can you please review this PR again 🤗 |
https://github.com/louislam/uptime-kuma/blob/master/CONTRIBUTING.md#can-i-create-a-pull-request-for-uptime-kuma
Tick the checkbox if you understand [x]:
Description
This commit backports a slightly modified version the Adaptive-Templeate found in the Master-branch.
The version of the template used is from commit 2c31f3a.
All previously created connector webhook will keep working until Microsoft removes this functionality October 1st 2024.
https://devblogs.microsoft.com/microsoft365dev/retirement-of-office-365-connectors-within-microsoft-teams/
All new webhooks set up via workflows will use the backported version.
Line 13 in Teams.vue might need to be modified later. Teams.vue
I was not able to find any Microsoft documentation that can replace that link that this point in time.
Fixes #4934
Fixes #4922
Type of change
Checklist
Screenshots (if any)
Connector webhook (legacy)
Workflow webhook
