From 626719d00191386ef963c0663da4a143f43ee6da Mon Sep 17 00:00:00 2001 From: Vicky Kurniawan Date: Thu, 19 Aug 2021 08:51:59 +0700 Subject: [PATCH] Jira's autolink should support issue links that contain a comment link in the URL (#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 | 6 +++--- server/settings.go | 25 +++++++++---------------- server/user.go | 4 ++-- server/webhook.go | 15 ++++++++++----- server/webhook_worker.go | 2 ++ 5 files changed, 26 insertions(+), 26 deletions(-) diff --git a/server/client.go b/server/client.go index 7c1dec504..67bc9efa4 100644 --- a/server/client.go +++ b/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. diff --git a/server/settings.go b/server/settings.go index a0acd8f0f..401062d87 100644 --- a/server/settings.go +++ b/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) } diff --git a/server/user.go b/server/user.go index e6734e331..a52c031e5 100644 --- a/server/user.go +++ b/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 { diff --git a/server/webhook.go b/server/webhook.go index 3f4464939..70c99a80a 100644 --- a/server/webhook.go +++ b/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 } diff --git a/server/webhook_worker.go b/server/webhook_worker.go index c30cc36d9..b6907e015 100644 --- a/server/webhook_worker.go +++ b/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) }