Skip to content

Commit

Permalink
[MI-3424]:Fixed issue mattermost#321 'Wrong notification in case of a…
Browse files Browse the repository at this point in the history
…ssigned prs' (#40)

* [MI-3424]:Fixed issue mattermost#321 'Wrong notification in case of assigned prs'

* [MI-3424]:Fixed review comments

* [MI-3424]:Fixed review comments

* [MI-3424]:Fixed test cases
  • Loading branch information
Kshitij-Katiyar authored Aug 23, 2023
1 parent 8638c32 commit 8c7070d
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 2 deletions.
28 changes: 26 additions & 2 deletions server/webhook/merge_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ func (w *webhook) handleDMMergeRequest(event *gitlab.MergeEvent) ([]*HandleWebho
authorGitlabUsername := w.gitlabRetreiver.GetUsernameByID(event.ObjectAttributes.AuthorID)
senderGitlabUsername := event.User.Username

toUsers := []string{authorGitlabUsername}
for _, assigneeID := range event.ObjectAttributes.AssigneeIDs {
toUsers = append(toUsers, w.gitlabRetreiver.GetUsernameByID(assigneeID))
}

message := ""

switch event.ObjectAttributes.State {
Expand All @@ -33,7 +38,26 @@ func (w *webhook) handleDMMergeRequest(event *gitlab.MergeEvent) ([]*HandleWebho
case actionReopen:
message = fmt.Sprintf("[%s](%s) reopen your merge request [%s!%v](%s)", senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername), event.ObjectAttributes.Target.PathWithNamespace, event.ObjectAttributes.IID, event.ObjectAttributes.URL)
case actionUpdate:
// TODO not enough check (opened/update) to say assigned to you...
toUsers = []string{}

// Not going to show notification in case of commit push.
if event.ObjectAttributes.OldRev != "" {
break
}

for _, currentAssigneeID := range event.ObjectAttributes.AssigneeIDs {
assignedInPrevious := false
for _, previousAssignee := range event.Changes.Assignees.Previous {
if previousAssignee.ID == currentAssigneeID {
assignedInPrevious = true
break
}
}
if !assignedInPrevious {
toUsers = append(toUsers, w.gitlabRetreiver.GetUsernameByID(currentAssigneeID))
}
}

message = fmt.Sprintf("[%s](%s) assigned you to merge request [%s!%v](%s)", senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername), event.ObjectAttributes.Target.PathWithNamespace, event.ObjectAttributes.IID, event.ObjectAttributes.URL)
case actionApproved:
message = fmt.Sprintf("[%s](%s) approved your merge request [%s!%v](%s)", senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername), event.ObjectAttributes.Target.PathWithNamespace, event.ObjectAttributes.IID, event.ObjectAttributes.URL)
Expand All @@ -51,7 +75,7 @@ func (w *webhook) handleDMMergeRequest(event *gitlab.MergeEvent) ([]*HandleWebho
if len(message) > 0 {
handlers := []*HandleWebhook{{
Message: message,
ToUsers: []string{w.gitlabRetreiver.GetUsernameByID(event.ObjectAttributes.AssigneeID), authorGitlabUsername},
ToUsers: toUsers,
ToChannels: []string{},
From: senderGitlabUsername,
}}
Expand Down
6 changes: 6 additions & 0 deletions server/webhook/merge_request_fixture_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ const OpenMergeRequest = `{
"total_time_spent":0,
"human_total_time_spent":null,
"human_time_estimate":null,
"assignee_ids": [
50
],
"action":"open"
},
"labels":[],
Expand Down Expand Up @@ -650,6 +653,9 @@ const AssigneeMergeRequest = `{
"total_time_spent":0,
"human_total_time_spent":null,
"human_time_estimate":null,
"assignee_ids": [
50
],
"action":"update"
},
"labels":[],
Expand Down

0 comments on commit 8c7070d

Please sign in to comment.