-
Notifications
You must be signed in to change notification settings - Fork 189
feat(slack): add template functions for user/channel/group mentions #411
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: master
Are you sure you want to change the base?
feat(slack): add template functions for user/channel/group mentions #411
Conversation
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.
Pull Request Overview
This pull request adds template functions for mentioning Slack users, channels, and user groups by their human-readable identifiers (email addresses, channel names, and group handles) rather than requiring Slack IDs. The implementation uses placeholder markers that are replaced with actual Slack IDs via API lookups, with in-memory caching to minimize API calls.
- Introduces three new template functions:
slackUserByEmail,slackChannel, andslackUserGroup - Implements in-memory caching for lookup results to reduce API calls
- Updates Slack SDK from v0.16.0 to v0.17.3
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/templates/service.go | Adds three template functions that generate placeholder markers for Slack mentions |
| pkg/services/slack.go | Implements lookup functions for users/channels/groups with caching, regex patterns for marker detection, and mention processing logic |
| pkg/services/slack_test.go | Adds comprehensive tests for pattern matching, mention processing, caching behavior, and end-to-end Slack message sending |
| go.mod | Updates slack-go/slack to v0.17.3 and gorilla/websocket to v1.5.3 |
| go.sum | Updates checksums for dependency version changes |
| docs/services/slack.md | Documents the new mention features with examples and required Slack OAuth scopes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Pull Request Overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ee859a1 to
e7d1817
Compare
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.
Pull Request Overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #411 +/- ##
==========================================
- Coverage 55.41% 55.04% -0.38%
==========================================
Files 46 46
Lines 4125 4309 +184
==========================================
+ Hits 2286 2372 +86
- Misses 1511 1580 +69
- Partials 328 357 +29 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Add ability to mention Slack users by email, channels by name, and user groups by handle instead of requiring Slack IDs. This enables notifying commit authors and other users without prior knowledge of their Slack IDs. Introduces three template functions: - slackUserByEmail: lookup user ID by email address - slackChannel: lookup channel ID by channel name - slackUserGroup: lookup user group ID by handle/name Lookups are cached in-memory to minimize API calls. Failed lookups are logged as warnings and preserve the original text in the message. Fixes argoproj#230 Fixes argoproj#253 Signed-off-by: Willian Paixao <[email protected]>
Co-authored-by: Copilot <[email protected]>
Signed-off-by: Willian Paixao <[email protected]>
The fix prevents the "thundering herd" problem where multiple concurrent requests for the same user/channel/group would all make redundant API calls. Signed-off-by: Willian Paixao <[email protected]>
Address review feedback from Copilot PR review:
- Fix race condition in lookup functions using double-check locking
pattern to prevent thundering herd problem where multiple concurrent
requests could make redundant API calls
- Add opportunistic caching for channels: cache all returned channels
from GetConversations API call, not just the requested one, to
minimize future API calls
- Add opportunistic caching for user groups: cache all returned groups
by both handle and name for flexible lookup and improved performance
- Fix documentation example to use correct ArgoCD template variable
syntax: replace invalid `.author.email` with proper
`(call .repo.GetCommitMetadata .app.status.sync.revision).Author`
These changes improve performance in workspaces with many channels/groups
and prevent duplicate API calls under concurrent load.
Signed-off-by: Willian Paixao <[email protected]>
ec2822b to
21cb364
Compare
|
Hey @pasha-codefresh can I get a review please? |
Add ability to mention Slack users by email, channels by name, and user
groups by handle instead of requiring Slack IDs. This enables notifying
commit authors and other users without prior knowledge of their Slack IDs.
Introduces three template functions:
Lookups are cached in-memory to minimize API calls. Failed lookups are
logged as warnings and preserve the original text in the message.
Fixes #230
Fixes #253