[GH-558] Fix issue: /jira assign should accept @username

server/command.go

@@ -7,6 +7,7 @@ import (
 	"sort"
 	"strings"
 
+	jira "github.com/andygrunwald/go-jira"
 	"github.com/pkg/errors"
 
 	"github.com/mattermost/mattermost-plugin-api/experimental/command"
@@ -935,8 +936,15 @@ func executeAssign(p *Plugin, c *plugin.Context, header *model.CommandArgs, args
 	}
 	issueKey := strings.ToUpper(args[0])
 	userSearch := strings.Join(args[1:], " ")
+	var assignee *jira.User
+	if strings.HasPrefix(userSearch, "@") {
+		assignee, err = p.GetJiraUserFromMentions(instance.GetID(), header.UserMentions, userSearch)
+		if err != nil {
+			return p.responsef(header, "%v", err)
+		}
+	}
 
-	msg, err := p.AssignIssue(instance, types.ID(header.UserId), issueKey, userSearch)
+	msg, err := p.AssignIssue(instance, types.ID(header.UserId), issueKey, userSearch, assignee)
 	if err != nil {
 		return p.responsef(header, "%v", err)
 	}

server/command_test.go

@@ -457,3 +457,85 @@ func TestPlugin_ExecuteCommand_Uninstall(t *testing.T) {
 		})
 	}
 }
+
+func TestPlugin_ExecuteCommand_Assign(t *testing.T) {
+	p := &Plugin{}
+	tc := TestConfiguration{}
+	p.updateConfig(func(conf *config) {
+		conf.Secret = tc.Secret
+		conf.mattermostSiteURL = mattermostSiteURL
+	})
+
+	tests := map[string]struct {
+		commandArgs       *model.CommandArgs
+		expectedMsgPrefix string
+		SetupAPI          func(api *plugintest.API)
+	}{
+		"assign with no issue": {
+			commandArgs: &model.CommandArgs{
+				Command: "/jira assign",
+				UserId:  mockUserIDWithNotifications,
+			},
+			expectedMsgPrefix: "Please specify an issue key and an assignee search string, in the form `/jira assign <issue-key> <assignee>`",
+			SetupAPI:          func(api *plugintest.API) {},
+		},
+		"assign to valid issue but no user": {
+			commandArgs: &model.CommandArgs{
+				Command: "/jira assign INVALID",
+				UserId:  mockUserIDWithNotifications,
+			},
+			expectedMsgPrefix: "Please specify an issue key and an assignee search string, in the form `/jira assign <issue-key> <assignee>`",
+			SetupAPI:          func(api *plugintest.API) {},
+		},
+		"assign the valid issue to a non-existing user": {
+			commandArgs: &model.CommandArgs{
+				Command: "/jira assign VALID @unknownUser",
+				UserId:  mockUserIDWithNotifications,
+			},
+			expectedMsgPrefix: "the mentioned user was not found",
+			SetupAPI:          func(api *plugintest.API) {},
+		},
+		"assign the valid issue to a non-connected user": {
+			commandArgs: &model.CommandArgs{
+				Command: "/jira assign VALID @non_connected_user",
+				UserId:  mockUserIDWithNotifications,
+				UserMentions: model.UserMentionMap{
+					"non_connected_user": "non_connected_user",
+				},
+			},
+			expectedMsgPrefix: "the mentioned user is not connected to Jira",
+			SetupAPI: func(api *plugintest.API) {
+				api.On("LogWarn", mockAnythingOfTypeBatch("string", 5)...).Return(nil)
+			},
+		},
+	}
+
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			api := &plugintest.API{}
+			defer api.AssertExpectations(t)
+
+			tt.SetupAPI(api)
+			isSendEphemeralPostCalled := false
+			api.On("SendEphemeralPost", mock.AnythingOfType("string"), mock.AnythingOfType("*model.Post")).Run(func(args mock.Arguments) {
+				isSendEphemeralPostCalled = true
+
+				post := args.Get(1).(*model.Post)
+				actual := strings.TrimSpace(post.Message)
+				assert.True(
+					t,
+					strings.HasPrefix(actual, tt.expectedMsgPrefix),
+					"Expected returned message to start with: \n %s\nActual:\n%s", tt.expectedMsgPrefix, actual)
+			}).Once().Return(&model.Post{})
+
+			p.SetAPI(api)
+			p.instanceStore = p.getMockInstanceStoreKV(1)
+			p.userStore = getMockUserStoreKV()
+
+			cmdResponse, appError := p.ExecuteCommand(&plugin.Context{}, tt.commandArgs)
+			require.Nil(t, appError)
+			require.NotNil(t, cmdResponse)
+			assert.True(t, isSendEphemeralPostCalled)
+		})
+	}
+}

server/issue.go

@@ -853,7 +853,7 @@ func (p *Plugin) UnassignIssue(instance Instance, mattermostUserID types.ID, iss
 
 const MinUserSearchQueryLength = 3
 
-func (p *Plugin) AssignIssue(instance Instance, mattermostUserID types.ID, issueKey, userSearch string) (string, error) {
+func (p *Plugin) AssignIssue(instance Instance, mattermostUserID types.ID, issueKey, userSearch string, assignee *jira.User) (string, error) {
 	connection, err := p.userStore.LoadConnection(instance.GetID(), mattermostUserID)
 	if err != nil {
 		return "", err
@@ -877,12 +877,17 @@ func (p *Plugin) AssignIssue(instance Instance, mattermostUserID types.ID, issue
 	}
 
 	// Get list of assignable users
-	jiraUsers, err := client.SearchUsersAssignableToIssue(issueKey, userSearch, 10)
-	if StatusCode(err) == 401 {
-		return "You do not have the appropriate permissions to perform this action. Please contact your Jira administrator.", nil
-	}
-	if err != nil {
-		return "", err
+	jiraUsers := []jira.User{}
+	if assignee != nil {
+		jiraUsers = append(jiraUsers, *assignee)
+	} else {
+		jiraUsers, err = client.SearchUsersAssignableToIssue(issueKey, userSearch, 10)
+		if StatusCode(err) == http.StatusUnauthorized {
+			return "You do not have the appropriate permissions to perform this action. Please contact your Jira administrator.", nil
+		}
+		if err != nil {
+			return "", err
+		}
 	}
 
 	// handle number of returned jira users

server/user.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"strings"
 
 	jira "github.com/andygrunwald/go-jira"
 	"github.com/pkg/errors"
@@ -281,3 +282,23 @@ func (p *Plugin) disconnectUser(instance Instance, user *User) (*Connection, err
 
 	return conn, nil
 }
+
+func (p *Plugin) GetJiraUserFromMentions(instanceID types.ID, mentions model.UserMentionMap, userKey string) (*jira.User, error) {
+	userKey = strings.TrimPrefix(userKey, "@")
+	mentionUser, found := mentions[userKey]
+	if !found {
+		return nil, errors.New("the mentioned user was not found")
+	}
+
+	connection, err := p.userStore.LoadConnection(instanceID, types.ID(mentionUser))
+	if err != nil {
+		p.client.Log.Warn("Error occurred while loading connection", "User", mentionUser, "Error", err.Error())
+		return nil, errors.New("the mentioned user is not connected to Jira")
+	}
+
+	if connection.AccountID != "" {
+		return &connection.User, nil
+	}
+
+	return nil, errors.New("the mentioned user is not connected to Jira")
+}

server/user_test.go

@@ -5,6 +5,9 @@ import (
 	"net/http/httptest"
 	"testing"
 
+	jira "github.com/andygrunwald/go-jira"
+	pluginapi "github.com/mattermost/mattermost-plugin-api"
+	"github.com/mattermost/mattermost-server/v6/model"
 	"github.com/mattermost/mattermost-server/v6/plugin"
 	"github.com/mattermost/mattermost-server/v6/plugin/plugintest"
 	"github.com/stretchr/testify/assert"
@@ -62,3 +65,64 @@ func TestRouteUserStart(t *testing.T) {
 		})
 	}
 }
+
+func TestGetJiraUserFromMentions(t *testing.T) {
+	p := Plugin{}
+	p.userStore = getMockUserStoreKV()
+	p.instanceStore = p.getMockInstanceStoreKV(1)
+	testUser, err := p.userStore.LoadUser("connected_user")
+	assert.Nil(t, err)
+
+	tests := map[string]struct {
+		mentions       *model.UserMentionMap
+		userSearch     string
+		expectedResult *jira.User
+		expectedError  string
+		SetupAPI       func(api *plugintest.API)
+	}{
+		"if no mentions, no users are returned": {
+			mentions:      &model.UserMentionMap{},
+			userSearch:    "join",
+			expectedError: "the mentioned user was not found",
+			SetupAPI:      func(api *plugintest.API) {},
+		},
+		"non connected user won't appear when mentioned": {
+			mentions: &model.UserMentionMap{
+				"non_connected_user": "non_connected_user",
+			},
+			userSearch:    "non_connected_user",
+			expectedError: "the mentioned user is not connected to Jira",
+			SetupAPI: func(api *plugintest.API) {
+				api.On("LogWarn", mockAnythingOfTypeBatch("string", 5)...)
+			},
+		},
+		"Connected users are shown and returned as Jira Users, when mentioned": {
+			mentions: &model.UserMentionMap{
+				"connected_user": string(testUser.MattermostUserID)},
+			userSearch:     "connected_user",
+			expectedResult: &jira.User{AccountID: "test"},
+			SetupAPI:       func(api *plugintest.API) {},
+		},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			api := &plugintest.API{}
+			defer api.AssertExpectations(t)
+
+			tc.SetupAPI(api)
+			p.SetAPI(api)
+			p.client = pluginapi.NewClient(api, p.Driver)
+
+			user, err := p.GetJiraUserFromMentions(testInstance1.InstanceID, *tc.mentions, tc.userSearch)
+			if tc.expectedError != "" {
+				assert.Equal(t, tc.expectedError, err.Error())
+				assert.Nil(t, user)
+				return
+			}
+
+			assert.Equal(t, tc.expectedResult, user)
+			assert.Nil(t, err)
+		})
+	}
+}