Skip to content
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-717] Implement subscription notification for pull request "reopened" event #718

Merged
merged 3 commits into from
Dec 5, 2023

Conversation

raghavaggarwal2308
Copy link
Contributor

Summary

  • Added logic to handle the case for pull request reopened webhook event.

Screenshot:

image

What to test?

  • Subscription notification for pull request reopened event is working properly.
Steps to reproduce:
  1. Connect your GitHub account.
  2. Create a subscription of event type "pulls" in a channel.
  3. Reopen a closed PR on GitHub for the project you have created a subscription for.
Expected behavior:
  • You should get a notification in the channel with the text similar to the above screenshot.
Environment:

MM version: v9.2.2
Node version: 14.18.0
Go version: 1.20.11

Ticket Link

Fixes #717

@codecov-commenter
Copy link

codecov-commenter commented Dec 1, 2023

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (8cf783f) 15.49% compared to head (881d2be) 15.79%.
Report is 1 commits behind head on master.

Files Patch % Lines
server/plugin/webhook.go 0.00% 10 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #718      +/-   ##
==========================================
+ Coverage   15.49%   15.79%   +0.30%     
==========================================
  Files          15       15              
  Lines        5737     5767      +30     
==========================================
+ Hits          889      911      +22     
- Misses       4806     4814       +8     
  Partials       42       42              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hanzei hanzei added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Dec 1, 2023
@mickmister mickmister changed the title [GH-717] Fix issue: Subscription notification not working for pull request "reopened" event. [GH-717] Implement subscription notification for pull request "reopened" event Dec 4, 2023
Copy link
Contributor

@mickmister mickmister left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, one non-blocking suggestion to clean up the existing code a bit with a switch statement

server/plugin/webhook.go Outdated Show resolved Hide resolved
server/plugin/webhook.go Show resolved Hide resolved
@mickmister mickmister mentioned this pull request Dec 4, 2023
Copy link
Contributor

@hanzei hanzei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work. 👍

@avas27JTG avas27JTG added this to the v2.2.0 milestone Dec 4, 2023
@raghavaggarwal2308
Copy link
Contributor Author

@mickmister FIxed the review comments mentioned by you

@hanzei hanzei removed the 2: Dev Review Requires review by a core committer label Dec 4, 2023
Copy link

@AayushChaudhary0001 AayushChaudhary0001 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and approved
Working fine for reopened pull requests, LGTM.

@avas27JTG avas27JTG merged commit d8fb001 into mattermost:master Dec 5, 2023
9 checks passed
@avas27JTG avas27JTG deleted the MM-717 branch December 5, 2023 12:22
@hanzei hanzei added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Dec 5, 2023
mickmister pushed a commit that referenced this pull request Dec 12, 2023
…ed" event (#718)

* [MI-3808] Fix issue: Subscription notification not working for pull request "reopened" event.

* [MI-3808] Added test case for new template

* [MI-3808] Review fix
@mickmister mickmister mentioned this pull request Dec 12, 2023
mickmister added a commit that referenced this pull request Dec 13, 2023
* [GH-717] Implement subscription notification for pull request "reopened" event (#718)

* [MI-3808] Fix issue: Subscription notification not working for pull request "reopened" event.

* [MI-3808] Added test case for new template

* [MI-3808] Review fix

* bump version to 2.1.8

---------

Co-authored-by: Raghav Aggarwal <raghav.aggarwal@brightscout.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Subscriptions not creating posts for reopening PRs
6 participants