Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 24 commits into from
Dec 1, 2023
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
6ebca97
Jira assign working with AccountID
javaguirre Jul 31, 2022
f32f93a
Fix naming, joining test cases for jira user mentions
javaguirre Aug 16, 2022
ae33f3b
Fix linting
javaguirre Aug 16, 2022
5311780
Remove @ from user mention if any
javaguirre Sep 15, 2022
48ab69f
add errors when mention not found or in Jira
javaguirre Sep 15, 2022
c0532ba
Fix linting
javaguirre Sep 15, 2022
9e1c4cd
Fix Get mentions func definition format
javaguirre Sep 19, 2022
c687107
Renaming jiraUser to assignee
javaguirre Sep 16, 2022
5ee65c3
remove space on func definition
javaguirre Sep 19, 2022
782dcd1
Adding failed executeAssign test cases
javaguirre Sep 19, 2022
fbb9202
Merge branch 'master' into feature/issue-558
mattermost-build Feb 22, 2023
699d301
Merge branch 'feature/issue-558' of github.com:javaguirre/mattermost-…
raghavaggarwal2308 May 2, 2023
45f154b
[MI-3051] Fix issues:
raghavaggarwal2308 May 2, 2023
4db58e1
[MI-3051] Minor code refactoring
raghavaggarwal2308 May 2, 2023
8076b5c
[MI-3051] Review fixes
raghavaggarwal2308 May 2, 2023
8483d01
[MI-3051] Review fixes
raghavaggarwal2308 May 2, 2023
3db575f
[MI-3051] Review fixes
raghavaggarwal2308 May 2, 2023
ff90b47
[MI-3051] Fix failing tests
raghavaggarwal2308 May 2, 2023
e2e781c
[MI-3051] Review fixes
raghavaggarwal2308 May 4, 2023
32b980f
[MI-3051] Fix failing tests
raghavaggarwal2308 May 4, 2023
668f4e4
[MI-3051] Review fixes
raghavaggarwal2308 May 4, 2023
9f03f03
Merge pull request #48 from Brightscout/MI-3051
raghavaggarwal2308 May 4, 2023
3d8cdb1
[GH-558] Replaced p.API.LogWarn with p.API.Log.Warn
raghavaggarwal2308 May 5, 2023
b9979fb
Merge branch 'master' of github.com:mattermost/mattermost-plugin-jira…
Kshitij-Katiyar Sep 25, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion server/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -886,8 +887,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)
}
Expand Down
82 changes: 82 additions & 0 deletions server/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}
19 changes: 12 additions & 7 deletions server/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -854,7 +854,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
Expand All @@ -878,12 +878,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
Expand Down
21 changes: 21 additions & 0 deletions server/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"io"
"net/http"
"strings"

jira "github.com/andygrunwald/go-jira"
"github.com/pkg/errors"
Expand Down Expand Up @@ -275,3 +276,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.API.LogWarn("Error occurred while loading connection", "User", mentionUser, "Error", err.Error())
raghavaggarwal2308 marked this conversation as resolved.
Show resolved Hide resolved
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")
}
62 changes: 62 additions & 0 deletions server/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"net/http/httptest"
"testing"

jira "github.com/andygrunwald/go-jira"
"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"
Expand Down Expand Up @@ -62,3 +64,63 @@ 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)
raghavaggarwal2308 marked this conversation as resolved.
Show resolved Hide resolved

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)
})
}
}