Skip to content
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

Refactor MS-Teams notification to use AdaptiveCards #4538

Merged

Conversation

taschenuhr
Copy link
Contributor

@taschenuhr taschenuhr commented Feb 28, 2024

⚠️⚠️⚠️ Since we do not accept all types of pull requests and do not want to waste your time. Please be sure that you have read pull request rules:
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]:

  • I have read and understand the pull request rules.

Description

Fixes #4457
Fixes #4464 (likely)

This PR changes the payload of the MS-Teams notification to use AdaptiveCards. The template is based on https://adaptivecards.io/designer. This solves the two issues mentioned in #4457:

  1. The newer AdaptiveCard layout is used instead of the MessageCard.
  2. With the AdaptiveCard, the message is not collapsed anymore.

Type of change

Please delete any options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • My code follows the style guidelines of this project
  • I ran ESLint and other linters for modified files
  • I have performed a self-review of my own code and tested it
  • I have commented my code, particularly in hard-to-understand areas (including JSDoc for methods)
  • My changes generates no new warnings
  • My code needed automated testing. I have added them (this is optional task)

Screenshots (if any)

The new layout in the PR looks like this:

Event before after
UP oldUp newUp
DOWN oldDown newDown
Cert-expiry oldCert newCert
Testing oldTest newTest

CommanderStorm

This comment was marked as resolved.

@CommanderStorm CommanderStorm marked this pull request as draft February 28, 2024 10:59
CommanderStorm

This comment was marked as resolved.

@taschenuhr
Copy link
Contributor Author

Hi @CommanderStorm, did you have time to review my latest changes? Did those resolve all open ideas/issues?

Copy link
Collaborator

@CommanderStorm CommanderStorm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@taschenuhr
Copy link
Contributor Author

That moment, when you're flooding your Teams with test notifications and achieve to screenshot the exact wrong one 🤦‍♂️
I've created new screenshots (including the mini fix for the test notification). Crossing fingers, now everything works :)

@CommanderStorm CommanderStorm marked this pull request as ready for review March 7, 2024 14:29
Copy link
Collaborator

@CommanderStorm CommanderStorm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the bugfix/enhancement!

All changes in this PR are small (all cases [even those which are "nice to have"] are covered, only affects a previously broken provider) and uncontroversial ⇒ merging with junior maintainer approval

@CommanderStorm CommanderStorm merged commit 6eef219 into louislam:master Mar 7, 2024
14 of 17 checks passed
@taschenuhr taschenuhr deleted the feature/msteams-adaptivecards branch March 8, 2024 09:33
@taschenuhr
Copy link
Contributor Author

Hi @louislam, would this PR be worth putting into an upcoming 1.23.x release?

@CommanderStorm
Copy link
Collaborator

CommanderStorm commented Apr 5, 2024

We are only backporting bugfixes to 1.23.x, as only those can be shipped in patch releases.

See the (now more hidden) changelog of 1.23.0 or the equivalent part of the contributing guide:

🐻 It should be the last minor version of Uptime Kuma v1. I will focus on the development of v2. Stay tuned!

Update: The last minor version (1.23.x) means that there won't be any new features introduced in v1. However, bug fixes will still be provided. You can expect to see 1.23.1, 1.23.2 and so on.

@CommanderStorm CommanderStorm added this to the 2.0.0 milestone Apr 7, 2024
@A-Matt A-Matt mentioned this pull request Jul 8, 2024
@lucas-alcantara
Copy link

I have been playing around with Power Automate to post a message in Teams since #4922, as I have other custom applications that uses Teams webhooks as well. Apparently, I cannot just send a POST request to the webhook generated by Workflows if the channel I configured on the flow is private. Was this code tested on a private channel as well?

@taschenuhr
Copy link
Contributor Author

Hi @lucas-alcantara, unfortunately sending messages to private channels using the PowerAutomate workflow is not supported by Microsoft (maybe because the bot doesn't have access to the private channel). See https://learn.microsoft.com/en-us/power-automate/teams/send-a-message-in-teams#known-issues-and-limitations

@lucas-alcantara
Copy link

Thanks! I could not find that page among the zillions of redundant/deprecated documentation pages Microsoft has. Is it worth mentioning that limitation on uptime kuma's docs as well?

@CommanderStorm
Copy link
Collaborator

I don't know how popular this PowerAutomate thing is. Since we have a teams and not a power automate provider and this is a PA issue, I don't think this is nessesary to document

We try to bake everything nessesary to set up a notification provider into the UI.
If you want, you can imrove the docs via our contribution guide here:

https://github.com/louislam/uptime-kuma/blob/master/src/components/notifications/Teams.vue

@alie2n
Copy link

alie2n commented Jul 15, 2024

Hi,
as Microsoft decided to deprecate the incoming webhooks next month (2024-08-15 according to https://devblogs.microsoft.com/microsoft365dev/retirement-of-office-365-connectors-within-microsoft-teams/) and the proposed solution (please use Workflows) does not support the MessageCard content type. It would be very helpful to accept this PR to the 1.23.x branch as it now becomes a bugfix...

@CommanderStorm
Copy link
Collaborator

Using Workflows instead of webhooks also means changing the bits of documentation surrounding that.
If you want to rebase this change, the contribution guide is here

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants