-
Notifications
You must be signed in to change notification settings - Fork 148
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
[GH-733] Updated the "issue_creations" subscription to post notification when an issue is labeled/reopened #736
Conversation
…ion when a issue is labelled
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.
Thanks for this @raghavaggarwal2308 👍
Wondering if you would be up for also changing the string of the "person opened an issue", to include the string "with label x" when it has a label that matches a subscription in the channel
I think that "with label x" may need to be applied in multiple other places as well |
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.
LGTM 👍
One nit pick below
server/plugin/webhook.go
Outdated
if sub.IssueCreations() && action != actionOpened { | ||
if sub.IssueCreations() && action == actionClosed { |
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.
2/5 I like the previous code as it ensures that even when new actions are added, the code is only expected for the action open.
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.
@hanzei Updated
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #736 +/- ##
=======================================
Coverage 15.78% 15.78%
=======================================
Files 15 15
Lines 5770 5770
=======================================
Hits 911 911
Misses 4817 4817
Partials 42 42 ☔ View full report in Codecov by Sentry. |
@mickmister Sure, I will make the change |
@raghavaggarwal2308 We can do the above change in a separate PR. Created an issue for this here #742 |
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.
This PR has been tested for the following scenarios:-
- Notification sent to the user in the subscribed channel for desired label added on an issue.
- Notification sent to the user in the subscribed channel when an issue is opened for "issue_creation" subscription.
The PR was working fine for both the above conditions, LGTM. Approved.
Summary
Screenshots
What to test:
issue_creations,label:"label-name"
issue_creations
Steps to test:
issue_creations,label:"label-name"
is a channel.Ticket Link
Fixes #733