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

[MM-21762] Refactor jira issue search #442

Merged
merged 3 commits into from
Jan 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 0 additions & 3 deletions server/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ const (
routeAPIGetCreateIssueMetadata = "/api/v2/get-create-issue-metadata-for-project"
routeAPIGetJiraProjectMetadata = "/api/v2/get-jira-project-metadata"
routeAPIGetSearchIssues = "/api/v2/get-search-issues"
routeAPIGetSearchEpics = "/api/v2/get-search-epics"
routeAPIAttachCommentToIssue = "/api/v2/attach-comment-to-issue"
routeAPIUserInfo = "/api/v2/userinfo"
routeAPISubscribeWebhook = "/api/v2/webhook"
Expand Down Expand Up @@ -73,8 +72,6 @@ func handleHTTPRequest(p *Plugin, w http.ResponseWriter, r *http.Request) (int,
return withInstance(p.currentInstanceStore, w, r, httpAPIGetJiraProjectMetadata)
case routeAPIGetSearchIssues:
return withInstance(p.currentInstanceStore, w, r, httpAPIGetSearchIssues)
case routeAPIGetSearchEpics:
return withInstance(p.currentInstanceStore, w, r, httpAPIGetSearchEpics)
case routeAPIAttachCommentToIssue:
return withInstance(p.currentInstanceStore, w, r, httpAPIAttachCommentToIssue)

Expand Down
129 changes: 28 additions & 101 deletions server/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"net/http"
"regexp"
"strconv"
"strings"
"sync"

Expand Down Expand Up @@ -265,7 +266,7 @@ func httpAPIGetCreateIssueMetadataForProjects(ji Instance, w http.ResponseWriter
return http.StatusOK, nil
}

func httpAPIGetSearchEpics(ji Instance, w http.ResponseWriter, r *http.Request) (int, error) {
func httpAPIGetSearchIssues(ji Instance, w http.ResponseWriter, r *http.Request) (int, error) {
if r.Method != http.MethodGet {
return http.StatusMethodNotAllowed,
errors.New("Request: " + r.Method + " is not allowed, must be GET")
Expand All @@ -286,23 +287,35 @@ func httpAPIGetSearchEpics(ji Instance, w http.ResponseWriter, r *http.Request)
return http.StatusInternalServerError, err
}

epicNameTypeID := r.FormValue("epic_name_type_id")
jqlString := r.FormValue("jql")
q := r.FormValue("q")
jqlString := r.FormValue("jql")
fieldsStr := r.FormValue("fields")
limitStr := r.FormValue("limit")

if len(epicNameTypeID) == 0 {
return http.StatusBadRequest, errors.New("epic_name_type_id query param is required")
if len(fieldsStr) == 0 {
fieldsStr = "key,summary"
}
if len(jqlString) == 0 {
return http.StatusBadRequest, errors.New("jql query param is required")
escaped := strings.ReplaceAll(q, `"`, `\"`)
jqlString = fmt.Sprintf(`text ~ "%s" OR text ~ "%s*"`, escaped, escaped)
}

limit := 50
if len(limitStr) > 0 {
parsedLimit, parseErr := strconv.Atoi(limitStr)
if parseErr == nil {
limit = parsedLimit
}
}

fields := strings.Split(fieldsStr, ",")

var exact *jira.Issue
var wg sync.WaitGroup
if reJiraIssueKey.MatchString(q) {
wg.Add(1)
go func() {
exact, _ = client.GetIssue(q, nil)
exact, _ = client.GetIssue(q, &jira.GetQueryOptions{Fields: fieldsStr})
wg.Done()
}()
}
Expand All @@ -311,34 +324,22 @@ func httpAPIGetSearchEpics(ji Instance, w http.ResponseWriter, r *http.Request)
wg.Add(1)
go func() {
found, _ = client.SearchIssues(jqlString, &jira.SearchOptions{
MaxResults: 50,
Fields: []string{epicNameTypeID},
MaxResults: limit,
Fields: fields,
})

wg.Done()
}()

wg.Wait()

result := []utils.ReactSelectOption{}
result := []jira.Issue{}
if exact != nil {
name, _ := exact.Fields.Unknowns.String(epicNameTypeID)
if name != "" {
label := fmt.Sprintf("%s: %s", exact.Key, name)
result = append(result, utils.ReactSelectOption{
Label: label,
Value: exact.Key,
})
}
result = append(result, *exact)
}
for _, epic := range found {
name, _ := epic.Fields.Unknowns.String(epicNameTypeID)
if name != "" {
label := fmt.Sprintf("%s: %s", epic.Key, name)
result = append(result, utils.ReactSelectOption{
Label: label,
Value: epic.Key,
})
}

for _, issue := range found {
result = append(result, issue)
}

bb, err := json.Marshal(result)
Expand Down Expand Up @@ -437,80 +438,6 @@ func httpAPIGetJiraProjectMetadata(ji Instance, w http.ResponseWriter, r *http.R

var reJiraIssueKey = regexp.MustCompile(`^([[:alpha:]]+)-([[:digit:]]+)$`)

func httpAPIGetSearchIssues(ji Instance, w http.ResponseWriter, r *http.Request) (int, error) {
if r.Method != http.MethodGet {
return http.StatusMethodNotAllowed,
errors.New("Request: " + r.Method + " is not allowed, must be GET")
}

mattermostUserId := r.Header.Get("Mattermost-User-Id")
if mattermostUserId == "" {
return http.StatusUnauthorized, errors.New("not authorized")
}

jiraUser, err := ji.GetPlugin().userStore.LoadJIRAUser(ji, mattermostUserId)
if err != nil {
return http.StatusInternalServerError, err
}

client, err := ji.GetClient(jiraUser)
if err != nil {
return http.StatusInternalServerError, err
}

q := r.FormValue("q")
var exact *jira.Issue
var wg sync.WaitGroup
if reJiraIssueKey.MatchString(q) {
wg.Add(1)
go func() {
exact, _ = client.GetIssue(q, nil)
wg.Done()
}()
}

jqlString := `text ~ "` + strings.ReplaceAll(q, `"`, `\"`) + `"`
var found []jira.Issue
wg.Add(1)
go func() {
found, _ = client.SearchIssues(jqlString, &jira.SearchOptions{
MaxResults: 50,
Fields: []string{"key", "summary"},
})
wg.Done()
}()

wg.Wait()

var result []utils.ReactSelectOption
if exact != nil {
result = append(result, utils.ReactSelectOption{
Value: exact.Key,
Label: exact.Key + ": " + exact.Fields.Summary,
})
}
for _, issue := range found {
result = append(result, utils.ReactSelectOption{
Value: issue.Key,
Label: issue.Key + ": " + issue.Fields.Summary,
})
}

w.Header().Set("Content-Type", "application/json")
b, err := json.Marshal(result)
if err != nil {
return http.StatusInternalServerError,
errors.WithMessage(err, "failed to marshal response")
}
_, err = w.Write(b)
if err != nil {
return http.StatusInternalServerError,
errors.WithMessage(err, "failed to write response")
}

return http.StatusOK, nil
}

func httpAPIAttachCommentToIssue(ji Instance, w http.ResponseWriter, r *http.Request) (int, error) {
if r.Method != http.MethodPost {
return http.StatusMethodNotAllowed,
Expand Down
4 changes: 2 additions & 2 deletions webapp/src/actions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,9 @@ export const fetchJiraProjectMetadata = () => {
};
};

export const fetchEpicsWithParams = (params) => {
export const searchIssues = (params) => {
return async (dispatch, getState) => {
const url = getPluginServerRoute(getState()) + '/api/v2/get-search-epics';
const url = getPluginServerRoute(getState()) + '/api/v2/get-search-issues';
return doFetchWithResponse(`${url}${buildQueryString(params)}`);
};
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,16 @@ exports[`components/JiraEpicSelector should match snapshot 1`] = `
],
}
}
fetchEpicsWithParams={
inputId="epic"
isMulti={true}
onChange={[MockFunction]}
removeValidate={[MockFunction]}
searchIssues={
[MockFunction] {
"calls": Array [
Array [
Object {
"epic_name_type_id": "customfield_10011",
"fields": "customfield_10011",
"jql": "project=KT and issuetype=10000 and id IN (KT-17, KT-20) ORDER BY updated DESC",
"q": "",
},
Expand All @@ -36,10 +40,6 @@ exports[`components/JiraEpicSelector should match snapshot 1`] = `
],
}
}
inputId="epic"
isMulti={true}
onChange={[MockFunction]}
removeValidate={[MockFunction]}
theme={
Object {
"awayIndicator": "#ffbc42",
Expand Down Expand Up @@ -93,12 +93,20 @@ exports[`components/JiraEpicSelector should match snapshot 1`] = `
}
cacheOptions={false}
defaultOptions={true}
fetchEpicsWithParams={
filterOption={null}
isMulti={true}
loadOptions={[Function]}
menuPlacement="auto"
menuPortalTarget={<body />}
name="epic"
onChange={[Function]}
removeValidate={[MockFunction]}
searchIssues={
[MockFunction] {
"calls": Array [
Array [
Object {
"epic_name_type_id": "customfield_10011",
"fields": "customfield_10011",
"jql": "project=KT and issuetype=10000 and id IN (KT-17, KT-20) ORDER BY updated DESC",
"q": "",
},
Expand All @@ -112,14 +120,6 @@ exports[`components/JiraEpicSelector should match snapshot 1`] = `
],
}
}
filterOption={null}
isMulti={true}
loadOptions={[Function]}
menuPlacement="auto"
menuPortalTarget={<body />}
name="epic"
onChange={[Function]}
removeValidate={[MockFunction]}
styles={
Object {
"clearIndicator": [Function],
Expand Down
4 changes: 2 additions & 2 deletions webapp/src/components/jira_epic_selector/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@
import {connect} from 'react-redux';
import {bindActionCreators} from 'redux';

import {fetchEpicsWithParams} from 'actions';
import {searchIssues} from 'actions';

import JiraEpicSelector from './jira_epic_selector';

const mapDispatchToProps = (dispatch) => bindActionCreators({
fetchEpicsWithParams,
searchIssues,
}, dispatch);

export default connect(null, mapDispatchToProps)(JiraEpicSelector);
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import JiraEpicSelector from './jira_epic_selector';

describe('components/JiraEpicSelector', () => {
const baseProps = {
fetchEpicsWithParams: jest.fn().mockResolvedValue({}),
searchIssues: jest.fn().mockResolvedValue({}),
issueMetadata: issueMetadata as IssueMetadata,
theme: Preferences.THEMES.default,
isMulti: true,
Expand All @@ -32,46 +32,46 @@ describe('components/JiraEpicSelector', () => {
expect(wrapper).toMatchSnapshot();
});

test('should call fetchEpicsWithParams on mount if values are present', () => {
test('should call searchIssues on mount if values are present', () => {
const props = {...baseProps};
const wrapper = shallow<JiraEpicSelector>(
<JiraEpicSelector {...props}/>
);
expect(props.fetchEpicsWithParams).toHaveBeenCalledWith({
epic_name_type_id: 'customfield_10011',
expect(props.searchIssues).toHaveBeenCalledWith({
fields: 'customfield_10011',
jql: 'project=KT and issuetype=10000 and id IN (KT-17, KT-20) ORDER BY updated DESC',
q: '',
});
});

test('should not call fetchEpicsWithParams on mount if no values are present', () => {
test('should not call searchIssues on mount if no values are present', () => {
const props = {...baseProps, value: []};
const wrapper = shallow<JiraEpicSelector>(
<JiraEpicSelector {...props}/>
);
expect(props.fetchEpicsWithParams).not.toHaveBeenCalled();
expect(props.searchIssues).not.toHaveBeenCalled();
});

test('#searchIssues should call fetchEpicsWithParams', () => {
test('#searchIssues should call searchIssues', () => {
const props = {...baseProps};
const wrapper = shallow<JiraEpicSelector>(
<JiraEpicSelector {...props}/>
);

wrapper.instance().searchIssues('');

let args = props.fetchEpicsWithParams.mock.calls[1][0];
let args = props.searchIssues.mock.calls[1][0];
expect(args).toEqual({
epic_name_type_id: 'customfield_10011',
fields: 'customfield_10011',
jql: 'project=KT and issuetype=10000 ORDER BY updated DESC',
q: '',
});

wrapper.instance().searchIssues('some input');

args = props.fetchEpicsWithParams.mock.calls[2][0];
args = props.searchIssues.mock.calls[2][0];
expect(args).toEqual({
epic_name_type_id: 'customfield_10011',
fields: 'customfield_10011',
jql: 'project=KT and issuetype=10000 and ("Epic Name"~"some input" or "Epic Name"~"some input*") ORDER BY updated DESC',
q: 'some input',
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import AsyncSelect from 'react-select/async';

import {getStyleForReactSelect} from 'utils/styles';
import {isEpicNameField, isEpicIssueType} from 'utils/jira_issue_metadata';
import {IssueMetadata, ReactSelectOption} from 'types/model';
import {IssueMetadata, ReactSelectOption, JiraIssue} from 'types/model';

import Setting from 'components/setting';

Expand All @@ -18,7 +18,7 @@ const searchDebounceDelay = 400;
type Props = {
required?: boolean;
hideRequiredStar?: boolean;
fetchEpicsWithParams: (params: object) => Promise<{data: ReactSelectOption[]}>;
searchIssues: (params: object) => Promise<{data: JiraIssue[]}>;
theme: object;
isMulti?: boolean;
onChange: (values: string[]) => void;
Expand Down Expand Up @@ -128,12 +128,15 @@ export default class JiraEpicSelector extends React.PureComponent<Props, State>

const params = {
jql: fullJql,
epic_name_type_id: epicNameTypeId,
fields: epicNameTypeId,
q: userInput,
};

return this.props.fetchEpicsWithParams(params).then(({data}) => {
return data;
return this.props.searchIssues(params).then(({data}: {data: JiraIssue[]}) => {
return data.map((issue) => ({
value: issue.key,
label: `${issue.key}: ${issue.fields[epicNameTypeId]}`,
}));
}).catch((e) => {
this.setState({error: e});
return [];
Expand Down
Loading