Jira's autolink should support issue links that contain a comment lin…

…k in the URL (mattermost#773)

* change variable name from issue to watcher in server/client.go
* change variable type from bool to pointer bool in server/user.go
* add constants for error store new settings and error connect to Jira
* move CheckWatcherUser before PostNotifications (server/webhook_worker.go)
* remove LoadConnection after StoreConnection (server/settings.go)

server/client.go

@@ -196,13 +196,13 @@ func (client JiraClient) GetIssue(key string, options *jira.GetQueryOptions) (*j
 	return issue, nil
 }
 
-// GetWatchers returns an Issue watchers by issueKey.
+// GetWatchers returns an array of Jira watchers for a given issue.
 func (client JiraClient) GetWatchers(issueKey string) (*[]jira.User, error) {
-	issue, resp, err := client.Jira.Issue.GetWatchers(issueKey)
+	watchers, resp, err := client.Jira.Issue.GetWatchers(issueKey)
 	if err != nil {
 		return nil, userFriendlyJiraError(resp, err)
 	}
-	return issue, nil
+	return watchers, nil
 }
 
 // GetTransitions returns transitions for an issue with issueKey.

server/settings.go

@@ -9,6 +9,9 @@ import (
 const (
 	settingOn  = "on"
 	settingOff = "off"
+
+	errStoreNewSettings = "Could not store new settings. Please contact your system administrator. error: %v"
+	errConnectToJira    = "Your username is not connected to Jira. Please type `/jira connect`. %v"
 )
 
 func (p *Plugin) settingsNotifications(header *model.CommandArgs, instanceID, mattermostUserID types.ID, connection *Connection, args []string) *model.CommandResponse {
@@ -34,13 +37,13 @@ func (p *Plugin) settingsNotifications(header *model.CommandArgs, instanceID, ma
 	connection.Settings.Notifications = value
 	if err := p.userStore.StoreConnection(instanceID, mattermostUserID, connection); err != nil {
 		p.errorf("settingsNotifications, err: %v", err)
-		p.responsef(header, "Could not store new settings. Please contact your system administrator. error: %v", err)
+		p.responsef(header, errStoreNewSettings, err)
 	}
 
 	// send back the actual value
 	updatedConnection, err := p.userStore.LoadConnection(instanceID, mattermostUserID)
 	if err != nil {
-		return p.responsef(header, "Your username is not connected to Jira. Please type `jira connect`. %v", err)
+		return p.responsef(header, errConnectToJira, err)
 	}
 	notifications := settingOff
 	if updatedConnection.Settings.Notifications {
@@ -51,7 +54,7 @@ func (p *Plugin) settingsNotifications(header *model.CommandArgs, instanceID, ma
 }
 
 func (p *Plugin) settingsWatching(header *model.CommandArgs, instanceID, mattermostUserID types.ID, connection *Connection, args []string) *model.CommandResponse {
-	const helpText = "`/jira watching watching [value]`\n* Invalid value. Accepted values are: `on` or `off`."
+	const helpText = "`/jira watching [value]`\n* Invalid value. Accepted values are: `on` or `off`."
 
 	if len(args) != 2 {
 		return p.responsef(header, helpText)
@@ -70,21 +73,11 @@ func (p *Plugin) settingsWatching(header *model.CommandArgs, instanceID, matterm
 	if connection.Settings == nil {
 		connection.Settings = &ConnectionSettings{}
 	}
-	connection.Settings.Watching = value
+	connection.Settings.Watching = &value
 	if err := p.userStore.StoreConnection(instanceID, mattermostUserID, connection); err != nil {
 		p.errorf("settingsWatching, err: %v", err)
-		p.responsef(header, "Could not store new settings. Please contact your system administrator. error: %v", err)
-	}
-
-	// send back the actual value
-	updatedConnection, err := p.userStore.LoadConnection(instanceID, mattermostUserID)
-	if err != nil {
-		return p.responsef(header, "Your username is not connected to Jira. Please type `jira connect`. %v", err)
-	}
-	watching := settingOff
-	if updatedConnection.Settings.Watching {
-		watching = settingOn
+		p.responsef(header, errStoreNewSettings, err)
 	}
 
-	return p.responsef(header, "Settings updated. Watching %s.", watching)
+	return p.responsef(header, "Settings updated. Watching %s.", value)
 }

server/user.go

@@ -43,8 +43,8 @@ func (c *Connection) JiraAccountID() types.ID {
 }
 
 type ConnectionSettings struct {
-	Notifications bool `json:"notifications"`
-	Watching      bool `json:"watching"`
+	Notifications bool  `json:"notifications"`
+	Watching      *bool `json:"watching"`
 }
 
 func (s *ConnectionSettings) String() string {

server/webhook.go

@@ -27,6 +27,7 @@ type Webhook interface {
 	Events() StringSet
 	PostToChannel(p *Plugin, instanceID types.ID, channelID, fromUserID, subscriptionName string) (*model.Post, int, error)
 	PostNotifications(p *Plugin, instanceID types.ID) ([]*model.Post, int, error)
+	CheckWatcherUser(p *Plugin, instanceID types.ID)
 }
 
 type webhookField struct {
@@ -110,8 +111,6 @@ func (wh *webhook) PostNotifications(p *Plugin, instanceID types.ID) ([]*model.P
 		return nil, http.StatusOK, nil
 	}
 
-	wh.checkWatcherUser(p, instance)
-
 	posts := []*model.Post{}
 	for _, notification := range wh.notifications {
 		var mattermostUserID types.ID
@@ -226,7 +225,13 @@ func (wh *webhook) getConnection(p *Plugin, instance Instance, notification webh
 	return
 }
 
-func (wh *webhook) checkWatcherUser(p *Plugin, instance Instance) {
+func (wh *webhook) CheckWatcherUser(p *Plugin, instanceID types.ID) {
+	instance, err := p.instanceStore.LoadInstance(instanceID)
+	if err != nil {
+		// This isn't an internal server error. There's just no instance installed.
+		return
+	}
+
 	watcherUsers := &[]jira.User{}
 	for _, notification := range wh.notifications {
 		c, err := wh.getConnection(p, instance, notification)
@@ -261,7 +266,7 @@ func (wh *webhook) checkWatcherUser(p *Plugin, instance Instance) {
 				jiraAccountID: watcherUser.AccountID,
 				message:       message,
 				postType:      postType,
-				commentSelf:   watcherUser.Self,
+				commentSelf:   wh.JiraWebhook.Comment.Self,
 			}
 
 			c, err := wh.getConnection(p, instance, *whUserNotification)
@@ -270,7 +275,7 @@ func (wh *webhook) checkWatcherUser(p *Plugin, instance Instance) {
 			}
 
 			// if setting watching value is false don't put into webhookUserNotification
-			if c.Settings == nil || !c.Settings.Watching || err != nil {
+			if err != nil || c.Settings == nil || (c.Settings.Watching != nil && !*c.Settings.Watching) {
 				continue
 			}
 

server/webhook_worker.go

@@ -57,6 +57,8 @@ func (ww webhookWorker) process(msg *webhookMessage) (err error) {
 		return err
 	}
 
+	wh.CheckWatcherUser(ww.p, msg.InstanceID)
+
 	if _, _, err = wh.PostNotifications(ww.p, msg.InstanceID); err != nil {
 		ww.p.errorf("WebhookWorker id: %d, error posting notifications, err: %v", ww.id, err)
 	}