Changed the way to store assignee, mention, reporter and watching in …

…struct so that the code can be easily tested (#6)

* [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

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

server/command_test.go

@@ -72,11 +72,13 @@ func getMockUserStoreKV() mockUserStoreKV {
 	trueValue := true
 	withNotifications := connection // copy
 	withNotifications.Settings = &ConnectionSettings{
-		Notifications:                trueValue,
-		SendNotificationsForMention:  &trueValue,
-		SendNotificationsForAssignee: &trueValue,
-		SendNotificationsForReporter: &trueValue,
-		SendNotificationsForWatching: &trueValue,
+		Notifications: trueValue,
+		RolesForDMNotification: map[string]*bool{
+			subCommandAssignee: &trueValue,
+			subCommandMention:  &trueValue,
+			subCommandReporter: &trueValue,
+			subCommandWatching: &trueValue,
+		},
 	}
 
 	return mockUserStoreKV{

server/issue.go

@@ -1094,7 +1094,7 @@ func (p *Plugin) applyReporterNotification(wh *webhook, instanceID types.ID, rep
 	commentMessage := fmt.Sprintf("%s **commented** on %s:\n> %s", commentAuthor, jwhook.mdKeySummaryLink(), jwhook.Comment.Body)
 
 	connection, err := p.GetUserSetting(wh, instanceID, reporter.Name, reporter.AccountID)
-	if err != nil || connection.Settings == nil || !connection.Settings.ShouldReceiveNotificationsForReporter() {
+	if err != nil || connection.Settings == nil || !connection.Settings.ShouldReceiveNotification(notificationTypeReporter) {
 		return
 	}
 
@@ -1131,39 +1131,15 @@ func (p *Plugin) GetUserSetting(wh *webhook, instanceID types.ID, jiraAccountID,
 	return connection, nil
 }
 
-func (s *ConnectionSettings) ShouldReceiveNotificationsForAssignee() bool {
-	if s.SendNotificationsForAssignee != nil {
-		return *s.SendNotificationsForAssignee
+func (s *ConnectionSettings) ShouldReceiveNotification(role string) bool {
+	if val, ok := s.RolesForDMNotification[role]; ok {
+		return *val
 	}
 
 	// Check old setting for backwards compatibility
 	return s.Notifications
 }
 
-func (s *ConnectionSettings) ShouldReceiveNotificationsForReporter() bool {
-	if s.SendNotificationsForReporter != nil {
-		return *s.SendNotificationsForReporter
-	}
-
-	return s.Notifications
-}
-
-func (s *ConnectionSettings) ShouldReceiveNotificationsForMention() bool {
-	if s.SendNotificationsForMention != nil {
-		return *s.SendNotificationsForMention
-	}
-
-	return s.Notifications
-}
-
-func (s *ConnectionSettings) ShouldReceiveNotificationsForWatching() bool {
-	if s.SendNotificationsForWatching != nil {
-		return *s.SendNotificationsForWatching
-	}
-
-	return s.Notifications
-}
-
 func (wh *webhook) fetchConnectedUser(p *Plugin, instanceID types.ID) (Client, *Connection, error) {
 	var accountInformation []map[string]string
 

server/settings.go

@@ -20,6 +20,13 @@ const (
 	subCommandWatching  = "watching"
 )
 
+func (connection *Connection) sendNotification(role string, hasNotification bool) bool {
+	if role != subCommandAssignee && role != subCommandMention && role != subCommandReporter && role != subCommandWatching {
+		return false
+	}
+	connection.Settings.RolesForDMNotification[role] = &hasNotification
+	return true
+}
 func (p *Plugin) settingsNotifications(header *model.CommandArgs, instanceID, mattermostUserID types.ID, connection *Connection, args []string) *model.CommandResponse {
 	const helpText = "`/jira settings notifications [assignee|mention|reporter|watching] [value]`\n* Invalid value. Accepted values are: `on` or `off`."
 
@@ -40,16 +47,7 @@ func (p *Plugin) settingsNotifications(header *model.CommandArgs, instanceID, ma
 	if connection.Settings == nil {
 		connection.Settings = &ConnectionSettings{}
 	}
-	switch args[1] {
-	case subCommandAssignee:
-		connection.Settings.SendNotificationsForAssignee = &value
-	case subCommandMention:
-		connection.Settings.SendNotificationsForMention = &value
-	case subCommandReporter:
-		connection.Settings.SendNotificationsForReporter = &value
-	case subCommandWatching:
-		connection.Settings.SendNotificationsForWatching = &value
-	default:
+	if !connection.sendNotification(args[1], value) {
 		return p.responsef(header, helpText)
 	}
 
@@ -64,23 +62,8 @@ func (p *Plugin) settingsNotifications(header *model.CommandArgs, instanceID, ma
 		return p.responsef(header, errConnectToJira, err)
 	}
 	notifications := settingOff
-	switch args[1] {
-	case subCommandAssignee:
-		if *updatedConnection.Settings.SendNotificationsForAssignee {
-			notifications = settingOn
-		}
-	case subCommandMention:
-		if *updatedConnection.Settings.SendNotificationsForMention {
-			notifications = settingOn
-		}
-	case subCommandReporter:
-		if *updatedConnection.Settings.SendNotificationsForReporter {
-			notifications = settingOn
-		}
-	case subCommandWatching:
-		if *updatedConnection.Settings.SendNotificationsForWatching {
-			notifications = settingOn
-		}
+	if *updatedConnection.Settings.RolesForDMNotification[args[1]] {
+		notifications = settingOn
 	}
 
 	return p.responsef(header, "Settings updated.\n\t%s notifications %s.", strings.Title(args[1]), notifications)

server/user.go

@@ -43,11 +43,8 @@ func (c *Connection) JiraAccountID() types.ID {
 }
 
 type ConnectionSettings struct {
-	Notifications                bool  `json:"notifications"`
-	SendNotificationsForMention  *bool `json:"send_notifications_for_mention"`
-	SendNotificationsForAssignee *bool `json:"send_notifications_for_assignee"`
-	SendNotificationsForReporter *bool `json:"send_notifications_for_reporter"`
-	SendNotificationsForWatching *bool `json:"send_notifications_for_watching"`
+	Notifications          bool `json:"notifications"`
+	RolesForDMNotification map[string]*bool
 }
 
 func (s *ConnectionSettings) String() string {
@@ -56,19 +53,19 @@ func (s *ConnectionSettings) String() string {
 	reporterNotifications := "Notifications for reporter: off"
 	watchingNotifications := "Notifications for watching: off"
 
-	if s != nil && s.ShouldReceiveNotificationsForAssignee() {
+	if s != nil && s.ShouldReceiveNotification(subCommandAssignee) {
 		assigneeNotifications = "Notifications for assignee: on"
 	}
 
-	if s != nil && s.ShouldReceiveNotificationsForMention() {
+	if s != nil && s.ShouldReceiveNotification(subCommandMention) {
 		mentionNotifications = "Notifications for mention: on"
 	}
 
-	if s != nil && s.ShouldReceiveNotificationsForReporter() {
+	if s != nil && s.ShouldReceiveNotification(subCommandReporter) {
 		reporterNotifications = "Notifications for reporter: on"
 	}
 
-	if s != nil && s.ShouldReceiveNotificationsForWatching() {
+	if s != nil && s.ShouldReceiveNotification(subCommandWatching) {
 		watchingNotifications = "Notifications for watching: on"
 	}
 

server/user_cloud.go

@@ -103,11 +103,13 @@ func (p *Plugin) httpACUserInteractive(w http.ResponseWriter, r *http.Request, i
 		},
 		// Set default settings the first time a user connects
 		Settings: &ConnectionSettings{
-			Notifications:                trueValue,
-			SendNotificationsForWatching: &trueValue,
-			SendNotificationsForMention:  &trueValue,
-			SendNotificationsForAssignee: &trueValue,
-			SendNotificationsForReporter: &trueValue,
+			Notifications: trueValue,
+			RolesForDMNotification: map[string]*bool{
+				subCommandMention:  &trueValue,
+				subCommandAssignee: &trueValue,
+				subCommandReporter: &trueValue,
+				subCommandWatching: &trueValue,
+			},
 		},
 	}
 

server/user_server.go

@@ -105,11 +105,13 @@ func (p *Plugin) httpOAuth1aComplete(w http.ResponseWriter, r *http.Request, ins
 	trueValue := true
 	// Set default settings the first time a user connects
 	connection.Settings = &ConnectionSettings{
-		Notifications:                trueValue,
-		SendNotificationsForWatching: &trueValue,
-		SendNotificationsForMention:  &trueValue,
-		SendNotificationsForAssignee: &trueValue,
-		SendNotificationsForReporter: &trueValue,
+		Notifications: trueValue,
+		RolesForDMNotification: map[string]*bool{
+			subCommandMention:  &trueValue,
+			subCommandAssignee: &trueValue,
+			subCommandReporter: &trueValue,
+			subCommandWatching: &trueValue,
+		},
 	}
 
 	err = p.connectUser(instance, types.ID(mattermostUserID), connection)

server/user_test.go

@@ -19,21 +19,25 @@ func TestUserSettings_String(t *testing.T) {
 	}{
 		"notifications on": {
 			settings: ConnectionSettings{
-				Notifications:                valueTrue,
-				SendNotificationsForMention:  nil,
-				SendNotificationsForAssignee: nil,
-				SendNotificationsForReporter: nil,
-				SendNotificationsForWatching: nil,
+				Notifications: valueTrue,
+				RolesForDMNotification: map[string]*bool{
+					subCommandAssignee: nil,
+					subCommandMention:  nil,
+					subCommandReporter: nil,
+					subCommandWatching: nil,
+				},
 			},
 			expectedOutput: "\t- Notifications for assignee: on \n\t- Notifications for mention: on \n\t- Notifications for reporter: on \n\t- Notifications for watching: on",
 		},
 		"notifications off": {
 			settings: ConnectionSettings{
-				Notifications:                valueFalse,
-				SendNotificationsForMention:  nil,
-				SendNotificationsForAssignee: nil,
-				SendNotificationsForReporter: nil,
-				SendNotificationsForWatching: nil,
+				Notifications: valueFalse,
+				RolesForDMNotification: map[string]*bool{
+					subCommandAssignee: nil,
+					subCommandMention:  nil,
+					subCommandReporter: nil,
+					subCommandWatching: nil,
+				},
 			},
 			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.go

@@ -138,23 +138,8 @@ func (wh *webhook) PostNotifications(p *Plugin, instanceID types.ID) ([]*model.P
 		}
 		// If this is a comment-related webhook, we need to check if they have permissions to read that.
 		// Otherwise, check if they can view the issue.
-		switch notification.notificationType {
-		case subCommandAssignee:
-			if !c.Settings.ShouldReceiveNotificationsForAssignee() {
-				continue
-			}
-		case subCommandMention:
-			if !c.Settings.ShouldReceiveNotificationsForMention() {
-				continue
-			}
-		case subCommandReporter:
-			if !c.Settings.ShouldReceiveNotificationsForReporter() {
-				continue
-			}
-		case subCommandWatching:
-			if !c.Settings.ShouldReceiveNotificationsForWatching() {
-				continue
-			}
+		if !c.Settings.ShouldReceiveNotification(notification.notificationType) {
+			continue
 		}
 		if _, ok := mapForNotification[mattermostUserID]; ok {
 			continue