[MI-2088]: Review fixes of mattermost#872 (#10)

* [MI-2088]:Add Support for DM subscriptions for watchers and reporters

* [MI-2088]: Review Fixes
1. Improved the name of certain variable
2. Improved spacing

* [MI-2088]:Revieww fixes
1.Changes the names of few variables
2.Improved code complexity

* [MI-2088]: Review fixes done
1.Changed the name of few variables

* [MI-2088]:fixed review fixes

* [MI-2088]:fixed review fixes

* [MI-2088]:fixed review fixes

* [MI-2088]:Review fixes done
1.Changed the way to store assignee, mention, reporter and watching in struct so that the code can be easily tested

* [MI-2088]: Review fixes done
1. Changed the name of various variables
2. Used constants everywhere

* [MI-2088]: Review fixes done
1. Added Test cases
2. Used booleans instead of pointer in RolesForDMNotification

* [MI-2088]: Review fixes done
1. Modified test cases

* [MI-2088]: Review fixes done
1. Self reviewed

* [MI-2088]: Review fixes done
1. Changed the formatting
2. Changed the name of few variables
3. Improved code quality

* [MI-2088]: Fixes CI errors

* [MI-2088]: Review fixes done
1. Improved code quality

* [MI-2088]: Review fixes done
1. Improved code quality

* [MI-2088]: Review fixes done
1. Improved code quality

* [MI-2088]: Fixed CI errors

* [MI-2088]: Review fixes done
1. Solved a bug of getting null pointer on frontend.
2. Improved code quality

* [MI-2088]: Fixed CI errors

* [MI-2088]: Review fixes done
1. Improved code quality

* [MI-2088]: Reivew fixes done
1. Improved code quality

Co-authored-by: Kshitij Katiyar <kshitij.katiyar@brightscout.com>

server/command.go

@@ -259,16 +259,16 @@ func createSettingsCommand(optInstance bool) *model.AutocompleteData {
 	notifications := model.NewAutocompleteData(
 		"notifications", "[assignee|mention|reporter|watching]", "manage notifications")
 
-	assigneeNotifications := model.NewAutocompleteData(subCommandAssignee, "", "manage assignee notifications")
+	assigneeNotifications := model.NewAutocompleteData(assigneeRole, "", "manage assignee notifications")
 	assigneeNotifications.AddStaticListArgument("value", true, setting)
 
-	mentionNotifications := model.NewAutocompleteData(subCommandMention, "", "manage mention notifications")
+	mentionNotifications := model.NewAutocompleteData(mentionRole, "", "manage mention notifications")
 	mentionNotifications.AddStaticListArgument("value", true, setting)
 
-	reporterNotifications := model.NewAutocompleteData(subCommandReporter, "", "manage reporter notifications")
+	reporterNotifications := model.NewAutocompleteData(reporterRole, "", "manage reporter notifications")
 	reporterNotifications.AddStaticListArgument("value", true, setting)
 
-	watchingNotifications := model.NewAutocompleteData(subCommandWatching, "", "manage watching notifications")
+	watchingNotifications := model.NewAutocompleteData(watchingRole, "", "manage watching notifications")
 	reporterNotifications.AddStaticListArgument("value", true, setting)
 	notifications.AddCommand(assigneeNotifications)
 	notifications.AddCommand(mentionNotifications)

server/command_test.go

@@ -73,10 +73,10 @@ func getMockUserStoreKV() mockUserStoreKV {
 	withNotifications.Settings = &ConnectionSettings{
 		Notifications: true,
 		RolesForDMNotification: map[string]bool{
-			subCommandAssignee: true,
-			subCommandMention:  true,
-			subCommandReporter: true,
-			subCommandWatching: true,
+			assigneeRole: true,
+			mentionRole:  true,
+			reporterRole: true,
+			watchingRole: true,
 		},
 	}
 

server/issue.go

@@ -529,15 +529,15 @@ func (p *Plugin) httpGetJiraProjectMetadata(w http.ResponseWriter, r *http.Reque
 			Value: project.Key,
 			Label: project.Name,
 		}
-		issueTypes := make([]option, len(project.IssueTypes))
-		for index, issueType := range project.IssueTypes {
+		issueTypes := []option{}
+		for _, issueType := range project.IssueTypes {
 			if issueType.Subtasks {
 				continue
 			}
-			issueTypes[index] = option{
+			issueTypes = append(issueTypes, option{
 				Value: issueType.Id,
 				Label: issueType.Name,
-			}
+			})
 		}
 		issues[project.Key] = issueTypes
 	}

server/issue_test.go

@@ -272,26 +272,26 @@ func TestRouteShareIssuePublicly(t *testing.T) {
 func TestShouldReceiveNotification(t *testing.T) {
 	cs := ConnectionSettings{}
 	cs.RolesForDMNotification = make(map[string]bool)
-	cs.RolesForDMNotification[subCommandAssignee] = true
-	cs.RolesForDMNotification[subCommandMention] = true
+	cs.RolesForDMNotification[assigneeRole] = true
+	cs.RolesForDMNotification[mentionRole] = true
 	for name, tt := range map[string]struct {
 		role         string
 		notification bool
 	}{
-		subCommandAssignee: {
-			role:         subCommandAssignee,
+		assigneeRole: {
+			role:         assigneeRole,
 			notification: true,
 		},
-		subCommandMention: {
-			role:         subCommandMention,
+		mentionRole: {
+			role:         mentionRole,
 			notification: true,
 		},
-		subCommandReporter: {
-			role:         subCommandReporter,
+		reporterRole: {
+			role:         reporterRole,
 			notification: false,
 		},
-		subCommandWatching: {
-			role:         subCommandWatching,
+		watchingRole: {
+			role:         watchingRole,
 			notification: false,
 		},
 	} {
@@ -319,10 +319,10 @@ func TestFetchConnectedUser(t *testing.T) {
 				Settings: &ConnectionSettings{
 					Notifications: true,
 					RolesForDMNotification: map[string]bool{
-						subCommandAssignee: true,
-						subCommandMention:  true,
-						subCommandReporter: true,
-						subCommandWatching: true,
+						assigneeRole: true,
+						mentionRole:  true,
+						reporterRole: true,
+						watchingRole: true,
 					},
 				},
 				User: jira.User{
@@ -427,7 +427,6 @@ func TestApplyReporterNotification(t *testing.T) {
 			}
 		})
 	}
-
 }
 
 func TestGetUserSetting(t *testing.T) {
@@ -450,10 +449,10 @@ func TestGetUserSetting(t *testing.T) {
 				Settings: &ConnectionSettings{
 					Notifications: true,
 					RolesForDMNotification: (map[string]bool{
-						subCommandAssignee: true,
-						subCommandMention:  true,
-						subCommandReporter: true,
-						subCommandWatching: true,
+						assigneeRole: true,
+						mentionRole:  true,
+						reporterRole: true,
+						watchingRole: true,
 					}),
 				},
 			},

server/settings.go

@@ -1,9 +1,9 @@
 package main
 
 import (
-	"strings"
-
 	"github.com/mattermost/mattermost-server/v6/model"
+	"golang.org/x/text/cases"
+	"golang.org/x/text/language"
 
 	"github.com/mattermost/mattermost-plugin-jira/server/utils/types"
 )
@@ -14,14 +14,15 @@ const (
 
 	errStoreNewSettings = "Could not store new settings. Please contact your system administrator. Error: %v"
 	errConnectToJira    = "Your account is not connected to Jira. Please type `/jira connect`. %v"
-	subCommandAssignee  = "assignee"
-	subCommandMention   = "mention"
-	subCommandReporter  = "reporter"
-	subCommandWatching  = "watching"
+
+	assigneeRole = "assignee"
+	mentionRole  = "mention"
+	reporterRole = "reporter"
+	watchingRole = "watching"
 )
 
-func (connection *Connection) sendNotification(role string, hasNotification bool) bool {
-	if role != subCommandAssignee && role != subCommandMention && role != subCommandReporter && role != subCommandWatching {
+func (connection *Connection) updateRolesForDMNotification(role string, hasNotification bool) bool {
+	if role != assigneeRole && role != mentionRole && role != reporterRole && role != watchingRole {
 		return false
 	}
 	if connection.Settings.RolesForDMNotification == nil {
@@ -50,7 +51,7 @@ func (p *Plugin) settingsNotifications(header *model.CommandArgs, instanceID, ma
 	if connection.Settings == nil {
 		connection.Settings = &ConnectionSettings{}
 	}
-	if !connection.sendNotification(args[1], value) {
+	if !connection.updateRolesForDMNotification(args[1], value) {
 		return p.responsef(header, helpText)
 	}
 
@@ -69,5 +70,5 @@ func (p *Plugin) settingsNotifications(header *model.CommandArgs, instanceID, ma
 		notifications = settingOn
 	}
 
-	return p.responsef(header, "Settings updated.\n\t%s notifications %s.", strings.Title(args[1]), notifications)
+	return p.responsef(header, "Settings updated.\n\t%s notifications %s.", cases.Title(language.Und, cases.NoLower).String(args[1]), notifications)
 }

server/subscribe_test.go

@@ -1380,6 +1380,7 @@ func TestGetChannelsSubscribed(t *testing.T) {
 
 			r := bytes.NewReader(data)
 			bb, err := ioutil.ReadAll(r)
+
 			require.Nil(t, err)
 			wh, err := ParseWebhook(bb)
 			assert.Nil(t, err)

server/user.go

@@ -53,19 +53,19 @@ func (s *ConnectionSettings) String() string {
 	reporterNotifications := "Notifications for reporter: off"
 	watchingNotifications := "Notifications for watching: off"
 
-	if s != nil && s.ShouldReceiveNotification(subCommandAssignee) {
+	if s != nil && s.ShouldReceiveNotification(assigneeRole) {
 		assigneeNotifications = "Notifications for assignee: on"
 	}
 
-	if s != nil && s.ShouldReceiveNotification(subCommandMention) {
+	if s != nil && s.ShouldReceiveNotification(mentionRole) {
 		mentionNotifications = "Notifications for mention: on"
 	}
 
-	if s != nil && s.ShouldReceiveNotification(subCommandReporter) {
+	if s != nil && s.ShouldReceiveNotification(reporterRole) {
 		reporterNotifications = "Notifications for reporter: on"
 	}
 
-	if s != nil && s.ShouldReceiveNotification(subCommandWatching) {
+	if s != nil && s.ShouldReceiveNotification(watchingRole) {
 		watchingNotifications = "Notifications for watching: on"
 	}
 

server/user_cloud.go

@@ -104,10 +104,10 @@ func (p *Plugin) httpACUserInteractive(w http.ResponseWriter, r *http.Request, i
 		Settings: &ConnectionSettings{
 			Notifications: true,
 			RolesForDMNotification: map[string]bool{
-				subCommandMention:  true,
-				subCommandAssignee: true,
-				subCommandReporter: true,
-				subCommandWatching: true,
+				mentionRole:  true,
+				assigneeRole: true,
+				reporterRole: true,
+				watchingRole: true,
 			},
 		},
 	}

server/user_server.go

@@ -106,10 +106,10 @@ func (p *Plugin) httpOAuth1aComplete(w http.ResponseWriter, r *http.Request, ins
 	connection.Settings = &ConnectionSettings{
 		Notifications: true,
 		RolesForDMNotification: map[string]bool{
-			subCommandMention:  true,
-			subCommandAssignee: true,
-			subCommandReporter: true,
-			subCommandWatching: true,
+			mentionRole:  true,
+			assigneeRole: true,
+			reporterRole: true,
+			watchingRole: true,
 		},
 	}
 

server/user_test.go

@@ -19,10 +19,10 @@ func TestUserSettings_String(t *testing.T) {
 			settings: ConnectionSettings{
 				Notifications: true,
 				RolesForDMNotification: map[string]bool{
-					subCommandAssignee: true,
-					subCommandMention:  true,
-					subCommandReporter: true,
-					subCommandWatching: true,
+					assigneeRole: true,
+					mentionRole:  true,
+					reporterRole: true,
+					watchingRole: true,
 				},
 			},
 			expectedOutput: "\t- Notifications for assignee: on \n\t- Notifications for mention: on \n\t- Notifications for reporter: on \n\t- Notifications for watching: on",
@@ -31,10 +31,10 @@ func TestUserSettings_String(t *testing.T) {
 			settings: ConnectionSettings{
 				Notifications: false,
 				RolesForDMNotification: map[string]bool{
-					subCommandAssignee: false,
-					subCommandMention:  false,
-					subCommandReporter: false,
-					subCommandWatching: false,
+					assigneeRole: false,
+					mentionRole:  false,
+					reporterRole: false,
+					watchingRole: false,
 				},
 			},
 			expectedOutput: "\t- Notifications for assignee: off \n\t- Notifications for mention: off \n\t- Notifications for reporter: off \n\t- Notifications for watching: off",

server/webhook_worker.go

@@ -43,6 +43,7 @@ func (ww webhookWorker) process(msg *webhookMessage) (err error) {
 	if err = v.JiraWebhook.expandIssue(ww.p, msg.InstanceID); err != nil {
 		return err
 	}
+
 	ww.p.checkIssueWatchers(v, msg.InstanceID)
 	ww.p.applyReporterNotification(v, msg.InstanceID, v.Issue.Fields.Reporter)