Skip to content

Conversation

@willianpaixao
Copy link
Contributor

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 #230
Fixes #253

Copy link

Copilot AI left a 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, and slackUserGroup
  • 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.

Copy link

Copilot AI left a 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.

Copy link

Copilot AI left a 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
Copy link

codecov bot commented Nov 18, 2025

Codecov Report

❌ Patch coverage is 65.29412% with 59 lines in your changes missing coverage. Please review.
✅ Project coverage is 55.04%. Comparing base (da04400) to head (ec2822b).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
pkg/services/slack.go 68.75% 36 Missing and 14 partials ⚠️
pkg/templates/service.go 10.00% 6 Missing and 3 partials ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

willianpaixao and others added 5 commits December 3, 2025 16:50
  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]>
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]>
@willianpaixao willianpaixao force-pushed the willianpaixao-issue-230 branch from ec2822b to 21cb364 Compare December 3, 2025 15:51
@willianpaixao
Copy link
Contributor Author

Hey @pasha-codefresh can I get a review please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Capture author information for multisource applications Slack: Add ability to mention user by username/email

1 participant