-
Notifications
You must be signed in to change notification settings - Fork 152
Fixed Teams notification #491
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe changes introduce a new conditional branch in the Teams notification logic to handle two providers. The "webhookb2" branch behavior is maintained while a new branch for the "workflows" provider has been added. For the "workflows" provider, messages are formatted as HTML and sent as a JSON payload via an HTTP POST request, with error handling for both the sending process and response status. Additional packages ( Changes
Sequence Diagram(s)sequenceDiagram
participant Sender as Notification Sender
participant Teams as Teams Provider
participant HTTP as HTTP Endpoint
Sender->>Teams: Call Send() with provider info
alt Provider is webhookb2
Teams->>Teams: Parse webhook URL and construct shoutrrr URL
Teams-->>Sender: Send notification using shoutrrr
else Provider is workflows
Teams->>Teams: Format message as HTML and build JSON payload
Teams->>HTTP: Send HTTP POST request with payload
HTTP-->>Teams: Return response status
Teams-->>Sender: Return notification result and handle errors
end
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pkg/providers/teams/teams.go (1)
48-49
: Provider extraction logic may be brittle.The method to determine provider type relies on a fixed position in the URL path. This approach could be fragile if the URL structure changes.
Consider using a more robust approach, such as:
- provider := strings.Split(pr.TeamsWebHookURL, "/")[3] + urlParts := strings.Split(pr.TeamsWebHookURL, "/") + provider := "" + if len(urlParts) > 3 { + provider = urlParts[3] + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/providers/teams/teams.go
(2 hunks)
🔇 Additional comments (4)
pkg/providers/teams/teams.go (4)
4-6
: Appropriate imports for the new HTTP functionality.The added imports for
bytes
andnet/http
packages are necessary for the new HTTP POST implementation used with the Power Automate method.
50-66
: Good preservation of backward compatibility.The implementation maintains support for the deprecated webhookb2 method while properly documenting it as deprecated, ensuring users can migrate at their own pace.
71-77
: Good HTTP request implementation and resource management.The HTTP request implementation correctly handles the response body with a deferred close operation, which prevents resource leaks.
78-82
:✅ Verification successful
Verify the expected status code for Power Automate.
The code checks for
http.StatusAccepted
(202), which should be confirmed against Microsoft's documentation for Power Automate workflows.
🌐 Web query:
What status code does Microsoft Power Automate return for successful webhook requests?
💡 Result:
By default, Microsoft Power Automate returns HTTP 202 Accepted for successful webhook requests, indicating asynchronous processing[1][4][8]. This means the request has been accepted but processing may not yet be completed.
Key Details:
- Asynchronous Execution: Power Automate processes webhook-triggered flows asynchronously, immediately returning 202 after accepting the payload[1][6].
- Customization Option: You can override the default behavior by adding an HTTP Response action at the end of your flow to return different status codes (e.g., 200 OK)[1][5].
- Error Handling: Even if subsequent steps fail (e.g., large payloads or JSON errors), the initial response remains 202 Accepted[8]. Errors appear in flow runs but aren’t reflected in the HTTP response code[8].
Example Use Case:
Webhook Trigger → Flow Processing → HTTP 202 (Default) ↳ Add "HTTP Response" action → Returns 200 OK (Custom)
For integrations requiring immediate 200 OK responses (e.g., third-party plugins), explicit use of the HTTP Response action is necessary[1][10].
Citations:
- 1: https://community.dynamics.com/blogs/post/?postid=d914a6af-bef3-417f-a02b-0571610e3a05
- 2: https://community.powerplatform.com/forums/thread/details/?threadid=ce358662-c0c0-40de-8754-347019f886ad
- 3: https://learn.microsoft.com/en-us/connectors/custom-connectors/create-webhook-trigger
- 4: https://community.powerplatform.com/forums/thread/details/?threadid=9be7d198-cdd2-46a8-b52c-2d1d35117ad6
- 5: https://aps.autodesk.com/blog/use-webhooks-power-automate
- 6: https://learn.microsoft.com/en-us/power-apps/developer/data-platform/use-webhooks
- 7: https://learn.microsoft.com/en-us/graph/change-notifications-delivery-webhooks
- 8: https://learn.microsoft.com/en-ie/answers/questions/1820278/how-to-retrieve-specific-error-details-from-power
- 9: https://answers.microsoft.com/en-us/msoffice/forum/all/webhook-sharepoint-power-automate/6e56a7e0-0879-401e-a69f-516d7d5877e2
- 10: jenkinsci/office-365-connector-plugin#356
- 11: https://answers.microsoft.com/en-us/msteams/forum/all/teams-workflow/35cc17b1-ae25-4d95-8692-c1aff2414fe1
- 12: https://forum.newrelic.com/s/hubtopic/aAX8W0000008bqiWAA
- 13: https://dev.thebotplatform.com/docs/how-to-set-up-and-use-webhooks-in-power-automate
Status Code Verification: Microsoft Power Automate Behavior Confirmed
After reviewing the documentation, Microsoft Power Automate returns HTTP 202 Accepted by default for successful webhook requests. The current implementation in
pkg/providers/teams/teams.go
that checks forhttp.StatusAccepted
is correct for default flows. If you later decide to implement a custom HTTP Response action that returns a different status code (such as 200 OK), you'll need to update this logic accordingly.
// New Power Automate method | ||
} else if provider == "workflows" { | ||
htmlMessage := strings.ReplaceAll(msg, "\n", "<br>") | ||
payload := fmt.Sprintf(`{"text": "%s"}`, htmlMessage) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improve JSON payload creation for better security.
The current implementation has two potential issues:
- Simple replacement of newlines with HTML breaks might not handle other HTML special characters
- Using string formatting to construct JSON can lead to malformed JSON if the message contains quotes or special characters
- htmlMessage := strings.ReplaceAll(msg, "\n", "<br>")
- payload := fmt.Sprintf(`{"text": "%s"}`, htmlMessage)
+ import "encoding/json"
+ import "html"
+
+ // HTML encode message to escape special characters
+ htmlMessage := html.EscapeString(strings.ReplaceAll(msg, "\n", "<br>"))
+
+ // Create payload using proper JSON marshaling
+ type teamsPayload struct {
+ Text string `json:"text"`
+ }
+ payloadObj := teamsPayload{Text: htmlMessage}
+ payloadBytes, err := json.Marshal(payloadObj)
+ if err != nil {
+ err = errors.Wrap(err, fmt.Sprintf("failed to create JSON payload for teams notification id: %s", pr.ID))
+ TeamsErr = multierr.Append(TeamsErr, err)
+ continue
+ }
Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dogancanbakir Thanks for confirming! I'm glad you found the suggestion helpful. Let me know if you need any further assistance with the implementation!
(_/)
(^.^)
/>🎯 Bullseye!
// New Power Automate method | ||
} else if provider == "workflows" { | ||
htmlMessage := strings.ReplaceAll(msg, "\n", "<br>") | ||
payload := fmt.Sprintf(`{"text": "%s"}`, htmlMessage) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
The Teams provider was previously using the webhook API from the Connectors method, which was deprecated in July 2024 and officially reached end-of-service in January 2025.
Reference: Microsoft 365 Dev Blog: Retirement of Office 365 Connectors within Microsoft Teams
To maintain Teams notification functionality, it must migrate to the new Power Automate method. This approach involves sending an HTML-based message to Power Automate, which then processes the notification via its Workflows mechanism.
Implementation Reference:
Teams Incoming Webhook Deprecated – Use Power Automate Instead
This pull request updates the implementation to align with new recommended approach, ensuring continued support for Teams notifications, while also maintaining the old method in case of anyone that still uses it.
Summary by CodeRabbit