[MM-340] Fix issue of receiving incorrect notification for various ev…

…ents in DM (#471) * [MM-340] Fix issue of receiving incorrect notification for various events in DM * [MM-340] Fix flaky tests * [MM-340] Update list of users to notify during multiple events update * [MM-340] Update function name * [MM-340] Remove additional events notification received from gitlab * [MM-340] Break comment in multiple lines * [MM-340] Update multi line comment to single line comment
mattermost · Jul 10, 2024 · 01b5e6c · 01b5e6c
1 parent bef60dd
commit 01b5e6c
Show file tree

Hide file tree

Showing 4 changed files with 590 additions and 172 deletions.
diff --git a/server/webhook/merge_request.go b/server/webhook/merge_request.go
@@ -29,57 +29,91 @@ func (w *webhook) handleDMMergeRequest(event *gitlab.MergeEvent) ([]*HandleWebho
 	}
 
 	message := ""
-
+	handlers := []*HandleWebhook{}
 	switch event.ObjectAttributes.State {
 	case stateOpened:
 		switch event.ObjectAttributes.Action {
 		case actionOpen:
 			message = fmt.Sprintf("[%s](%s) requested your review on [%s!%v](%s)", senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername), event.ObjectAttributes.Target.PathWithNamespace, event.ObjectAttributes.IID, event.ObjectAttributes.URL)
+			handlers = []*HandleWebhook{{
+				Message: message,
+				ToUsers: toUsers,
+				From:    senderGitlabUsername,
+			}}
 		case actionReopen:
 			message = fmt.Sprintf("[%s](%s) reopened your merge request [%s!%v](%s)", senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername), event.ObjectAttributes.Target.PathWithNamespace, event.ObjectAttributes.IID, event.ObjectAttributes.URL)
+			handlers = []*HandleWebhook{{
+				Message: message,
+				ToUsers: toUsers,
+				From:    senderGitlabUsername,
+			}}
 		case actionUpdate:
-			toUsers = []string{authorGitlabUsername}
-
 			// 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))
+			// Handle change in assignees
+			if event.Changes.Assignees.Current != nil || event.Changes.Assignees.Previous != nil {
+				updatedCurrentUsers := w.calculateUserDiffs(event.Changes.Assignees.Previous, event.Changes.Assignees.Current)
+
+				if len(updatedCurrentUsers) != 0 {
+					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)
+					handlers = append(handlers, &HandleWebhook{
+						Message: message,
+						ToUsers: updatedCurrentUsers,
+						From:    senderGitlabUsername,
+					})
 				}
 			}
 
-			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)
+			// Handle change in reviewers
+			if event.Changes.Reviewers.Current != nil || event.Changes.Reviewers.Previous != nil {
+				updatedCurrentUsers := w.calculateUserDiffs(event.Changes.Reviewers.Previous, event.Changes.Reviewers.Current)
+
+				if len(updatedCurrentUsers) != 0 {
+					message = fmt.Sprintf("[%s](%s) requested your review on merge request [%s!%v](%s)", senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername), event.ObjectAttributes.Target.PathWithNamespace, event.ObjectAttributes.IID, event.ObjectAttributes.URL)
+					handlers = append(handlers, &HandleWebhook{
+						Message: message,
+						ToUsers: updatedCurrentUsers,
+						From:    senderGitlabUsername,
+					})
+				}
+			}
 		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)
+			handlers = []*HandleWebhook{{
+				Message: message,
+				ToUsers: toUsers,
+				From:    senderGitlabUsername,
+			}}
 		case actionUnapproved:
 			message = fmt.Sprintf("[%s](%s) requested changes to your merge request [%s!%v](%s)", senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername), event.ObjectAttributes.Target.PathWithNamespace, event.ObjectAttributes.IID, event.ObjectAttributes.URL)
+			handlers = []*HandleWebhook{{
+				Message: message,
+				ToUsers: toUsers,
+				From:    senderGitlabUsername,
+			}}
 		}
 	case stateClosed:
 		message = fmt.Sprintf("[%s](%s) closed your merge request [%s!%v](%s)", senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername), event.ObjectAttributes.Target.PathWithNamespace, event.ObjectAttributes.IID, event.ObjectAttributes.URL)
+		handlers = []*HandleWebhook{{
+			Message: message,
+			ToUsers: toUsers,
+			From:    senderGitlabUsername,
+		}}
 	case stateMerged:
 		if event.ObjectAttributes.Action == actionMerge {
 			message = fmt.Sprintf("[%s](%s) merged your merge request [%s!%v](%s)", senderGitlabUsername, w.gitlabRetreiver.GetUserURL(senderGitlabUsername), event.ObjectAttributes.Target.PathWithNamespace, event.ObjectAttributes.IID, event.ObjectAttributes.URL)
+			handlers = []*HandleWebhook{{
+				Message: message,
+				ToUsers: toUsers,
+				From:    senderGitlabUsername,
+			}}
 		}
 	}
 
-	if len(message) > 0 {
-		handlers := []*HandleWebhook{{
-			Message:    message,
-			ToUsers:    toUsers,
-			ToChannels: []string{},
-			From:       senderGitlabUsername,
-		}}
-
+	if len(handlers) > 0 {
 		if mention := w.handleMention(mentionDetails{
 			senderUsername:    senderGitlabUsername,
 			pathWithNamespace: event.Project.PathWithNamespace,
@@ -148,3 +182,21 @@ func (w *webhook) handleChannelMergeRequest(ctx context.Context, event *gitlab.M
 
 	return res, nil
 }
+
+// calculateUserDiffs function takes previousUsers and currentUsers of an event,
+// finds the change in the user list, and returns the updated current user list.
+func (w *webhook) calculateUserDiffs(previousUsers, currentUsers []*gitlab.EventUser) []string {
+	mapPreviousUsers := map[int]*gitlab.EventUser{}
+	updatedCurrentUsers := []string{}
+	for _, previousUser := range previousUsers {
+		mapPreviousUsers[previousUser.ID] = previousUser
+	}
+
+	for _, currentUser := range currentUsers {
+		if _, exist := mapPreviousUsers[currentUser.ID]; !exist {
+			updatedCurrentUsers = append(updatedCurrentUsers, w.gitlabRetreiver.GetUsernameByID(currentUser.ID))
+		}
+	}
+
+	return updatedCurrentUsers
+}