[MM-340] Fix issue of receiving incorrect notification for various events in DM #471
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #471 +/- ##
==========================================
+ Coverage 34.02% 35.67% +1.65%
==========================================
Files 22 22
Lines 4021 4146 +125
==========================================
+ Hits 1368 1479 +111
- Misses 2515 2524 +9
- Partials 138 143 +5 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Nice work on this @ayusht2810 👍 I have some comments for discussion
|
@mickmister updated the function name. Other comments are pending on @hanzei and your reply. |
server/webhook/merge_request.go
Outdated
| func (w *webhook) checkForMultipleEvents(event *gitlab.MergeEvent) bool { | ||
| eventCount := 0 | ||
| if event.Changes.Labels.Current != nil || event.Changes.Labels.Previous != nil { | ||
| eventCount++ |
There was a problem hiding this comment.
Can the code simply return with return true in this case?
There was a problem hiding this comment.
Removed the above function in the updated logic as there was no need for it.
|
@hanzei Fixed the review fixes. Please re-review. |
There was a problem hiding this comment.
The above PR has been tested for the following scenarios:
- Tested the notification when MR is assigned to you as a reviewer or assignee
- Tested the notification when MR is assigned to other user as a reviewer or assignee
The PR was working fine for both the conditions, LGTM. Approved
Summary
Ticket Link
Checklist
make testRan test cases and ensured they are passingmake check-styleRan style check and ensured both webapp and server pass the checksWhat to test?