Conversation
tb102122
requested review from
Br0wnHammer,
Owaiseimdad,
ajhollid and
karenvicent
as code owners
server/src/service/infrastructure/notificationProviders/teams.ts
Outdated
Show resolved
Hide resolved
server/src/service/infrastructure/notificationProviders/teams.ts
Outdated
Show resolved
Hide resolved
…om/tb102122/Checkmate into feature/teams-notification-channel
|
@Br0wnHammer the review comments are done. Sorry what do you mean with the failing checks? I don't see any check in the PR. |
|
@tb102122 I have approved the workflows to run. Now you can see the checks that are failing. |
|
done sorry that I missed that part |
|
@tb102122 There is also one more check that is failing. Can you please take a look at it? |
|
@Br0wnHammer Done hope that is the correct way. I haven't touched that file. |
ajhollid
left a comment
ajhollid
left a comment
There was a problem hiding this comment.
In general, it looks great! There's a couple of minor nitpicks with regards to types and some validation I don't think we need, but a nice PR overall.
Thanks for your hard work here, looking forward to revisions!
| homeserverUrl: z.union([z.string(), z.literal("")]).optional(), | ||
| roomId: z.union([z.string(), z.literal("")]).optional(), | ||
| accessToken: z.union([z.string(), z.literal("")]).optional(), |
There was a problem hiding this comment.
These fields are for the Matrix notification type I believe? I don't think teams need them 🤔
| actions.push({ | ||
| type: "Action.OpenUrl", | ||
| title: "View Incident", | ||
| url: `${message.clientHost}/infrastructure/${message.monitor.id}`, |
There was a problem hiding this comment.
This should point to /incidents/{id}, I realize this is a bug in other providers as well.
| }; | ||
| const color = colorMap[message.severity] || "default"; | ||
|
|
||
| const body: any[] = []; |
There was a problem hiding this comment.
Let's type this properly and avoid using any
| /** | ||
| * Build an Adaptive Card from NotificationMessage | ||
| */ | ||
| private buildAdaptiveCard(message: NotificationMessage): object { |
There was a problem hiding this comment.
Lets type the return here properly, object type has no type safety
3 participants
(Please remove this line only before submitting your PR. Ensure that all relevant items are checked before submission.)
Describe your changes
adding Team's as a notification channel.
Write your issue number after "Fixes "
Fixes #3363
Please ensure all items are checked off before requesting a review. "Checked off" means you need to add an "x" character between brackets so they turn into checkmarks.
<div>Add</div>, use):npm run formatin server and client directories, which automatically formats your code.