From 4802fb0b83c59b8849649a000d15c9c32242d6ca Mon Sep 17 00:00:00 2001 From: Nityanand Rai <107465508+Nityanand13@users.noreply.github.com> Date: Mon, 28 Nov 2022 19:36:20 +0530 Subject: [PATCH 1/9] [MI-2337]: Supported filtering on comment visibility for subscriptions (#18) * [MI-2337]: Supported filtering on comment visibility for subscriptions * [MI-2337]: Review fixes done 1. Improved code quality 2. Improved quality of comments * [MI-2337]: Review fixes done 1. Improved code quality 2. Changed few names * [MI-2337]: Review fixes done 1. Improved code quality * [MI-2337]: Review fixes done 1. Improved code quality 2. Improved comments quality * [MI-2337]: Review fixes done 1. Improved code quality * [MI-2384]: Converted class component to functional component of Comment visibility filter (#20) * [MI-2384]: Converted class component to functional component of comment visibility filter * [MI-2384]: Review fixes done 1. Improved code quality 2. Fixed lint errors * [MI-2384]: Review fixes done 1. Improved code quality * [MI-2384]: Logged the error properly --- server/client.go | 26 +++++++++ server/http.go | 3 + server/issue.go | 57 +++++++++++++++++-- server/subscribe.go | 13 ++++- server/subscribe_test.go | 2 +- server/webhook_worker.go | 27 ++++++++- webapp/src/actions/index.ts | 7 +++ .../jira_commentvisibility_selector/index.ts | 13 +++++ .../jira_commentvisibility_selector.tsx | 54 ++++++++++++++++++ .../channel_subscription_filter.tsx | 14 ++++- webapp/src/utils/jira_issue_metadata.tsx | 18 +++++- 11 files changed, 220 insertions(+), 14 deletions(-) create mode 100644 webapp/src/components/data_selectors/jira_commentvisibility_selector/index.ts create mode 100644 webapp/src/components/data_selectors/jira_commentvisibility_selector/jira_commentvisibility_selector.tsx diff --git a/server/client.go b/server/client.go index 7efe93bdf..ded57b3b4 100644 --- a/server/client.go +++ b/server/client.go @@ -26,8 +26,10 @@ import ( ) const autocompleteSearchRoute = "2/jql/autocompletedata/suggestions" +const commentVisibilityRoute = "2/user" const userSearchRoute = "2/user/assignable/search" const unrecognizedEndpoint = "_unrecognized" +const visibleToAllUsers = "visible-to-all-users" // Client is the combined interface for all upstream APIs and convenience methods. type Client interface { @@ -64,6 +66,7 @@ type SearchService interface { SearchUsersAssignableToIssue(issueKey, query string, maxResults int) ([]jira.User, error) SearchUsersAssignableInProject(projectKey, query string, maxResults int) ([]jira.User, error) SearchAutoCompleteFields(params map[string]string) (*AutoCompleteResult, error) + SearchCommentVisibilityFields(params map[string]string) (*CommentVisibilityResult, error) } // IssueService is the interface for issue-related APIs. @@ -252,6 +255,18 @@ type AutoCompleteResult struct { Results []Result `json:"results"` } +type Item struct { + Name string `json:"name"` +} + +type Group struct { + Items []Item `json:"items"` +} + +type CommentVisibilityResult struct { + Groups Group `json:"groups"` +} + // SearchAutoCompleteFields searches fieldValue specified in the params and returns autocomplete suggestions // for that fieldValue func (client JiraClient) SearchAutoCompleteFields(params map[string]string) (*AutoCompleteResult, error) { @@ -264,6 +279,17 @@ func (client JiraClient) SearchAutoCompleteFields(params map[string]string) (*Au return result, nil } +// SearchCommentVisibilityFields searches fieldValue specified in the params and returns the comment visibility suggestions +// for that fieldValue +func (client JiraClient) SearchCommentVisibilityFields(params map[string]string) (*CommentVisibilityResult, error) { + result := &CommentVisibilityResult{} + if err := client.RESTGet(commentVisibilityRoute, params, result); err != nil { + return nil, err + } + result.Groups.Items = append(result.Groups.Items, Item{visibleToAllUsers}) + return result, nil +} + // DoTransition executes a transition on an issue. func (client JiraClient) DoTransition(issueKey, transitionID string) error { resp, err := client.Jira.Issue.DoTransition(issueKey, transitionID) diff --git a/server/http.go b/server/http.go index a0189ebda..50a3583e1 100644 --- a/server/http.go +++ b/server/http.go @@ -32,6 +32,7 @@ const ( routeAPIGetJiraProjectMetadata = "/api/v2/get-jira-project-metadata" routeAPIGetSearchIssues = "/api/v2/get-search-issues" routeAPIGetAutoCompleteFields = "/api/v2/get-search-autocomplete-fields" + routeAPIGetCommentVisibilityFields = "/api/v2/get-comment-visibility-fields" routeAPIGetSearchUsers = "/api/v2/get-search-users" routeAPIAttachCommentToIssue = "/api/v2/attach-comment-to-issue" routeAPIUserInfo = "/api/v2/userinfo" @@ -126,6 +127,8 @@ func (p *Plugin) serveHTTP(c *plugin.Context, w http.ResponseWriter, r *http.Req return p.httpGetSearchIssues(w, r) case routeAPIGetAutoCompleteFields: return p.httpGetAutoCompleteFields(w, r) + case routeAPIGetCommentVisibilityFields: + return p.httpGetCommentVisibilityFields(w, r) case routeAPIGetSearchUsers: return p.httpGetSearchUsers(w, r) case routeAPIAttachCommentToIssue: diff --git a/server/issue.go b/server/issue.go index dc784d39c..098bc10ee 100644 --- a/server/issue.go +++ b/server/issue.go @@ -22,12 +22,16 @@ import ( ) const ( - labelsField = "labels" - statusField = "status" - reporterField = "reporter" - priorityField = "priority" - descriptionField = "description" - resolutionField = "resolution" + labelsField = "labels" + statusField = "status" + reporterField = "reporter" + priorityField = "priority" + descriptionField = "description" + resolutionField = "resolution" + headerMattermostUserID = "Mattermost-User-ID" + instanceIDQueryParam = "instance_id" + fieldValueQueryParam = "fieldValue" + expandQueryParam = "expand" ) func makePost(userID, channelID, message string) *model.Post { @@ -404,6 +408,47 @@ func (p *Plugin) GetCreateIssueMetadataForProjects(instanceID, mattermostUserID }) } +func (p *Plugin) httpGetCommentVisibilityFields(w http.ResponseWriter, r *http.Request) (int, error) { + if r.Method != http.MethodGet { + return http.StatusMethodNotAllowed, fmt.Errorf("Request: " + r.Method + " is not allowed, must be GET") + } + + mattermostUserID := r.Header.Get(headerMattermostUserID) + if mattermostUserID == "" { + return http.StatusUnauthorized, errors.New("not authorized") + } + + instanceID := r.FormValue(instanceIDQueryParam) + client, _, connection, err := p.getClient(types.ID(instanceID), types.ID(mattermostUserID)) + if err != nil { + return http.StatusInternalServerError, err + } + + params := map[string]string{ + "fieldValue": r.FormValue(fieldValueQueryParam), + "expand": r.FormValue(expandQueryParam), + "accountId": connection.AccountID, + } + response, err := client.SearchCommentVisibilityFields(params) + if err != nil { + return http.StatusInternalServerError, err + } + if response == nil { + return http.StatusInternalServerError, errors.New("failed to return the response") + } + + jsonResponse, err := json.Marshal(response) + if err != nil { + return http.StatusInternalServerError, errors.WithMessage(err, "failed to marshal the response") + } + + w.Header().Set("Content-Type", "application/json") + if _, err = w.Write(jsonResponse); err != nil { + return http.StatusInternalServerError, errors.WithMessage(err, "failed to write the response") + } + return http.StatusOK, nil +} + func (p *Plugin) httpGetSearchIssues(w http.ResponseWriter, r *http.Request) (int, error) { if r.Method != http.MethodGet { return respondErr(w, http.StatusMethodNotAllowed, diff --git a/server/subscribe.go b/server/subscribe.go index 0792f9048..bec1e23cd 100644 --- a/server/subscribe.go +++ b/server/subscribe.go @@ -31,6 +31,7 @@ const ( FilterEmpty = "empty" MaxSubscriptionNameLength = 100 + CommentVisibility = "commentVisibility" ) type FieldFilter struct { @@ -123,7 +124,7 @@ func (p *Plugin) getUserID() string { return p.getConfig().botUserID } -func (p *Plugin) matchesSubsciptionFilters(wh *webhook, filters SubscriptionFilters) bool { +func (p *Plugin) matchesSubsciptionFilters(wh *webhook, filters SubscriptionFilters, visibilityAttribute string) bool { webhookEvents := wh.Events() foundEvent := false eventTypes := filters.Events @@ -159,6 +160,12 @@ func (p *Plugin) matchesSubsciptionFilters(wh *webhook, filters SubscriptionFilt } value := getIssueFieldValue(&wh.JiraWebhook.Issue, field.Key) + if visibilityAttribute != "" { + value[visibilityAttribute] = true + } else if field.Key == CommentVisibility { + value[visibleToAllUsers] = true + } + containsAny := value.ContainsAny(field.Values.Elems()...) containsAll := value.ContainsAll(field.Values.Elems()...) @@ -174,7 +181,7 @@ func (p *Plugin) matchesSubsciptionFilters(wh *webhook, filters SubscriptionFilt return validFilter } -func (p *Plugin) getChannelsSubscribed(wh *webhook, instanceID types.ID) ([]ChannelSubscription, error) { +func (p *Plugin) getChannelsSubscribed(wh *webhook, instanceID types.ID, visibilityAttribute string) ([]ChannelSubscription, error) { subs, err := p.getSubscriptions(instanceID) if err != nil { return nil, err @@ -183,7 +190,7 @@ func (p *Plugin) getChannelsSubscribed(wh *webhook, instanceID types.ID) ([]Chan var channelSubscriptions []ChannelSubscription subIds := subs.Channel.ByID for _, sub := range subIds { - if p.matchesSubsciptionFilters(wh, sub.Filters) { + if p.matchesSubsciptionFilters(wh, sub.Filters, visibilityAttribute) { channelSubscriptions = append(channelSubscriptions, sub) } } diff --git a/server/subscribe_test.go b/server/subscribe_test.go index c9ce769a4..1cb80620e 100644 --- a/server/subscribe_test.go +++ b/server/subscribe_test.go @@ -1385,7 +1385,7 @@ func TestGetChannelsSubscribed(t *testing.T) { wh, err := ParseWebhook(bb) assert.Nil(t, err) - actual, err := p.getChannelsSubscribed(wh.(*webhook), testInstance1.InstanceID) + actual, err := p.getChannelsSubscribed(wh.(*webhook), testInstance1.InstanceID, "") assert.Nil(t, err) assert.Equal(t, len(tc.ChannelSubscriptions), len(actual)) actualChannelIDs := NewStringSet() diff --git a/server/webhook_worker.go b/server/webhook_worker.go index a008e665f..43c97702f 100644 --- a/server/webhook_worker.go +++ b/server/webhook_worker.go @@ -4,6 +4,8 @@ package main import ( + jira "github.com/andygrunwald/go-jira" + "github.com/mattermost/mattermost-plugin-jira/server/utils/types" ) @@ -49,7 +51,30 @@ func (ww webhookWorker) process(msg *webhookMessage) (err error) { return err } - channelsSubscribed, err := ww.p.getChannelsSubscribed(v, msg.InstanceID) + // To check if this is a comment-related webhook payload + isCommentEvent := wh.Events().Intersection(commentEvents).Len() > 0 + visibilityAttribute := "" + if isCommentEvent { + mattermostUserID, er := ww.p.userStore.LoadMattermostUserID(msg.InstanceID, v.JiraWebhook.Comment.Author.AccountID) + if er != nil { + ww.p.API.LogInfo("Commentator is not connected with the mattermost", "Error", er.Error()) + return er + } + + client, _, _, er := ww.p.getClient(msg.InstanceID, mattermostUserID) + if er != nil { + return er + } + + comment := jira.Comment{} + if er = client.RESTGet(v.JiraWebhook.Comment.Self, nil, &comment); er != nil { + return er + } + + visibilityAttribute = comment.Visibility.Value + } + + channelsSubscribed, err := ww.p.getChannelsSubscribed(v, msg.InstanceID, visibilityAttribute) if err != nil { return err } diff --git a/webapp/src/actions/index.ts b/webapp/src/actions/index.ts index c1a58d8f1..055d449d6 100644 --- a/webapp/src/actions/index.ts +++ b/webapp/src/actions/index.ts @@ -157,6 +157,13 @@ export const searchAutoCompleteFields = (params) => { }; }; +export const searchCommentVisibilityFields = (params) => { + return async (dispatch, getState) => { + const url = `${getPluginServerRoute(getState())}/api/v2/get-comment-visibility-fields`; + return doFetchWithResponse(`${url}${buildQueryString(params)}`); + }; +}; + export const searchUsers = (params) => { return async (dispatch, getState) => { const url = getPluginServerRoute(getState()) + '/api/v2/get-search-users'; diff --git a/webapp/src/components/data_selectors/jira_commentvisibility_selector/index.ts b/webapp/src/components/data_selectors/jira_commentvisibility_selector/index.ts new file mode 100644 index 000000000..94fef6257 --- /dev/null +++ b/webapp/src/components/data_selectors/jira_commentvisibility_selector/index.ts @@ -0,0 +1,13 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {connect} from 'react-redux'; +import {bindActionCreators} from 'redux'; + +import {searchCommentVisibilityFields} from 'actions'; + +import JiraCommentVisibilitySelector from './jira_commentvisibility_selector'; + +const mapDispatchToProps = (dispatch) => bindActionCreators({searchCommentVisibilityFields}, dispatch); + +export default connect(null, mapDispatchToProps)(JiraCommentVisibilitySelector); diff --git a/webapp/src/components/data_selectors/jira_commentvisibility_selector/jira_commentvisibility_selector.tsx b/webapp/src/components/data_selectors/jira_commentvisibility_selector/jira_commentvisibility_selector.tsx new file mode 100644 index 000000000..c23019ac3 --- /dev/null +++ b/webapp/src/components/data_selectors/jira_commentvisibility_selector/jira_commentvisibility_selector.tsx @@ -0,0 +1,54 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import React from 'react'; + +import {ReactSelectOption} from 'types/model'; + +import BackendSelector, {Props as BackendSelectorProps} from '../backend_selector'; + +const stripHTML = (text: string) => { + if (!text) { + return text; + } + + const doc = new DOMParser().parseFromString(text, 'text/html'); + return doc.body.textContent || ''; +}; + +type Props = BackendSelectorProps & { + searchCommentVisibilityFields: (params: {fieldValue: string}) => ( + Promise<{data: {groups: {items: {name: string}[]}}; error?: Error}> + ); + fieldName: string; +}; + +const JiraCommentVisibilitySelector = (props: Props) => { + const {value, isMulti, instanceID, searchCommentVisibilityFields} = props; + const fetchInitialSelectedValues = async (): Promise => + ((!value || (isMulti && !value.length)) ? [] : commentVisibilityFields('')); + + const commentVisibilityFields = async (inputValue: string): Promise => { + const params = { + fieldValue: inputValue, + instance_id: instanceID, + expand: 'groups', + }; + return searchCommentVisibilityFields(params).then(({data}) => { + return data.groups.items.map((suggestion) => ({ + value: suggestion.name, + label: stripHTML(suggestion.name), + })); + }); + }; + + return ( + + ); +}; + +export default JiraCommentVisibilitySelector; diff --git a/webapp/src/components/modals/channel_subscriptions/channel_subscription_filter.tsx b/webapp/src/components/modals/channel_subscriptions/channel_subscription_filter.tsx index 9abdf12e6..8d5f92a57 100644 --- a/webapp/src/components/modals/channel_subscriptions/channel_subscription_filter.tsx +++ b/webapp/src/components/modals/channel_subscriptions/channel_subscription_filter.tsx @@ -5,10 +5,11 @@ import {Theme} from 'mattermost-redux/types/preferences'; import ReactSelectSetting from 'components/react_select_setting'; import JiraEpicSelector from 'components/data_selectors/jira_epic_selector'; -import {isEpicLinkField, isMultiSelectField, isLabelField} from 'utils/jira_issue_metadata'; +import {isEpicLinkField, isMultiSelectField, isLabelField, isCommentVisibilityField} from 'utils/jira_issue_metadata'; import {FilterField, FilterValue, ReactSelectOption, IssueMetadata, IssueType, FilterFieldInclusion} from 'types/model'; import ConfirmModal from 'components/confirm_modal'; import JiraAutoCompleteSelector from 'components/data_selectors/jira_autocomplete_selector'; +import JiraCommentVisibilitySelector from 'components/data_selectors/jira_commentvisibility_selector'; export type Props = { fields: FilterField[]; @@ -244,7 +245,16 @@ export default class ChannelSubscriptionFilter extends React.PureComponent + ); + } else if (isEpicLinkField(this.props.field)) { valueSelector = ( { + return { + key: 'commentVisibility', + name: 'Comment Visibility', + schema: { + type: 'commentVisibility', + }, + values: [], + issueTypes: field.validIssueTypes, + } as FilterField; + }); + + const result = populatedFields.concat(userDefinedFields, commentVisibility); const epicLinkField = fields.find(isEpicLinkField); if (epicLinkField) { result.unshift({ @@ -264,6 +276,10 @@ export function isLabelField(field: JiraField | FilterField): boolean { return field.schema.system === 'labels' || field.schema.custom === 'com.atlassian.jira.plugin.system.customfieldtypes:labels'; } +export function isCommentVisibilityField(field: JiraField | FilterField): boolean { + return field.key === 'commentVisibility'; +} + export function isEpicIssueType(issueType: IssueType): boolean { return issueType.name === 'Epic'; } From 444352c0f0bef0725e664762812b3c3cb11600e4 Mon Sep 17 00:00:00 2001 From: Nityanand Rai <107465508+Nityanand13@users.noreply.github.com> Date: Wed, 30 Nov 2022 18:57:42 +0530 Subject: [PATCH 2/9] Done the review fixes of PR #894 (#21) * [MI-2337]: Supported filtering on comment visibility for subscriptions * [MI-2337]: Review fixes done 1. Improved code quality 2. Improved quality of comments * [MI-2337]: Review fixes done 1. Improved code quality 2. Changed few names * [MI-2337]: Review fixes done 1. Improved code quality * [MI-2337]: Review fixes done 1. Improved code quality 2. Improved comments quality * [MI-2337]: Review fixes done 1. Improved code quality * [MI-2384]: Converted class component to functional component of Comment visibility filter (#20) * [MI-2384]: Converted class component to functional component of comment visibility filter * [MI-2384]: Review fixes done 1. Improved code quality 2. Fixed lint errors * [MI-2384]: Review fixes done 1. Improved code quality * [MI-2384]: Logged the error properly * [MI-2337]: Done the review fixes of PR #894 1. Made a constant 2. Improved code quality 3. Changed few names --- server/client.go | 10 +++--- server/webhook_worker.go | 46 ++++++++++++++---------- webapp/src/utils/jira_issue_metadata.tsx | 6 ++-- 3 files changed, 36 insertions(+), 26 deletions(-) diff --git a/server/client.go b/server/client.go index ded57b3b4..be01c6d39 100644 --- a/server/client.go +++ b/server/client.go @@ -255,16 +255,16 @@ type AutoCompleteResult struct { Results []Result `json:"results"` } -type Item struct { +type GroupItem struct { Name string `json:"name"` } -type Group struct { - Items []Item `json:"items"` +type GroupsInfo struct { + GroupsItem []GroupItem `json:"items"` } type CommentVisibilityResult struct { - Groups Group `json:"groups"` + Groups GroupsInfo `json:"groups"` } // SearchAutoCompleteFields searches fieldValue specified in the params and returns autocomplete suggestions @@ -286,7 +286,7 @@ func (client JiraClient) SearchCommentVisibilityFields(params map[string]string) if err := client.RESTGet(commentVisibilityRoute, params, result); err != nil { return nil, err } - result.Groups.Items = append(result.Groups.Items, Item{visibleToAllUsers}) + result.Groups.GroupsItem = append(result.Groups.GroupsItem, GroupItem{visibleToAllUsers}) return result, nil } diff --git a/server/webhook_worker.go b/server/webhook_worker.go index 43c97702f..bb3ceae15 100644 --- a/server/webhook_worker.go +++ b/server/webhook_worker.go @@ -29,6 +29,30 @@ func (ww webhookWorker) work() { } } +func isCommentRelatedWebhook(wh Webhook) bool { + return wh.Events().Intersection(commentEvents).Len() > 0 +} + +func (ww webhookWorker) getVisibilityAttribute(msg *webhookMessage, v *webhook) (string, error) { + mattermostUserID, err := ww.p.userStore.LoadMattermostUserID(msg.InstanceID, v.JiraWebhook.Comment.Author.AccountID) + if err != nil { + ww.p.API.LogInfo("Commentator is not connected with the mattermost", "Error", err.Error()) + return "", err + } + + client, _, _, err := ww.p.getClient(msg.InstanceID, mattermostUserID) + if err != nil { + return "", err + } + + comment := jira.Comment{} + if err = client.RESTGet(v.JiraWebhook.Comment.Self, nil, &comment); err != nil { + return "", err + } + + return comment.Visibility.Value, nil +} + func (ww webhookWorker) process(msg *webhookMessage) (err error) { defer func() { if err == ErrWebhookIgnored { @@ -51,27 +75,11 @@ func (ww webhookWorker) process(msg *webhookMessage) (err error) { return err } - // To check if this is a comment-related webhook payload - isCommentEvent := wh.Events().Intersection(commentEvents).Len() > 0 visibilityAttribute := "" - if isCommentEvent { - mattermostUserID, er := ww.p.userStore.LoadMattermostUserID(msg.InstanceID, v.JiraWebhook.Comment.Author.AccountID) - if er != nil { - ww.p.API.LogInfo("Commentator is not connected with the mattermost", "Error", er.Error()) - return er - } - - client, _, _, er := ww.p.getClient(msg.InstanceID, mattermostUserID) - if er != nil { - return er - } - - comment := jira.Comment{} - if er = client.RESTGet(v.JiraWebhook.Comment.Self, nil, &comment); er != nil { - return er + if isCommentRelatedWebhook(wh) { + if visibilityAttribute, err = ww.getVisibilityAttribute(msg, v); err != nil { + return err } - - visibilityAttribute = comment.Visibility.Value } channelsSubscribed, err := ww.p.getChannelsSubscribed(v, msg.InstanceID, visibilityAttribute) diff --git a/webapp/src/utils/jira_issue_metadata.tsx b/webapp/src/utils/jira_issue_metadata.tsx index 02d48363e..016e25718 100644 --- a/webapp/src/utils/jira_issue_metadata.tsx +++ b/webapp/src/utils/jira_issue_metadata.tsx @@ -23,6 +23,8 @@ type FieldWithInfo = JiraField & { issueTypeMeta: IssueTypeIdentifier; } +const commentVisibilityFieldKey = 'commentVisibility'; + // This is a replacement for the Array.flat() function which will be polyfilled by Babel // in our 5.16 release. Remove this and replace with .flat() then. const flatten = (arr: any[]) => { @@ -212,7 +214,7 @@ export function getCustomFieldFiltersForProjects(metadata: IssueMetadata | null, const commentVisibility = selectFields.map((field) => { return { - key: 'commentVisibility', + key: commentVisibilityFieldKey, name: 'Comment Visibility', schema: { type: 'commentVisibility', @@ -277,7 +279,7 @@ export function isLabelField(field: JiraField | FilterField): boolean { } export function isCommentVisibilityField(field: JiraField | FilterField): boolean { - return field.key === 'commentVisibility'; + return field.key === commentVisibilityFieldKey; } export function isEpicIssueType(issueType: IssueType): boolean { From 10649689809b8306d1b80a4b1d0f7e8d0ffe78da Mon Sep 17 00:00:00 2001 From: Nityanand Rai <107465508+Nityanand13@users.noreply.github.com> Date: Tue, 13 Dec 2022 17:02:42 +0530 Subject: [PATCH 3/9] [MI 2337] Fixed CI errors and review comments of PR #894 (#24) * [MI-2337]: Supported filtering on comment visibility for subscriptions * [MI-2337]: Review fixes done 1. Improved code quality 2. Improved quality of comments * [MI-2337]: Review fixes done 1. Improved code quality 2. Changed few names * [MI-2337]: Review fixes done 1. Improved code quality * [MI-2337]: Review fixes done 1. Improved code quality 2. Improved comments quality * [MI-2337]: Review fixes done 1. Improved code quality * [MI-2384]: Converted class component to functional component of Comment visibility filter (#20) * [MI-2384]: Converted class component to functional component of comment visibility filter * [MI-2384]: Review fixes done 1. Improved code quality 2. Fixed lint errors * [MI-2384]: Review fixes done 1. Improved code quality * [MI-2384]: Logged the error properly * [MI-2337]: Done the review fixes of PR #894 1. Made a constant 2. Improved code quality 3. Changed few names * [MI-2337]: Fixed CI errors and few review comments of PR #894 * [MI-2337]: Review fixes done 1. Improved code quality --- server/client.go | 10 +- .../channel_subscription_filter.test.tsx.snap | 28 +++ ...channel_subscription_filters.test.tsx.snap | 28 +++ .../edit_channel_subscription.test.tsx.snap | 170 ++++++++++++++++++ webapp/src/utils/jira_issue_metadata.test.tsx | 65 ++++--- 5 files changed, 271 insertions(+), 30 deletions(-) diff --git a/server/client.go b/server/client.go index be01c6d39..b7491254a 100644 --- a/server/client.go +++ b/server/client.go @@ -255,16 +255,16 @@ type AutoCompleteResult struct { Results []Result `json:"results"` } -type GroupItem struct { +type JiraUserGroup struct { Name string `json:"name"` } -type GroupsInfo struct { - GroupsItem []GroupItem `json:"items"` +type JiraUserGroupCollection struct { + JiraUserGroups []*JiraUserGroup `json:"items"` } type CommentVisibilityResult struct { - Groups GroupsInfo `json:"groups"` + Groups *JiraUserGroupCollection `json:"groups"` } // SearchAutoCompleteFields searches fieldValue specified in the params and returns autocomplete suggestions @@ -286,7 +286,7 @@ func (client JiraClient) SearchCommentVisibilityFields(params map[string]string) if err := client.RESTGet(commentVisibilityRoute, params, result); err != nil { return nil, err } - result.Groups.GroupsItem = append(result.Groups.GroupsItem, GroupItem{visibleToAllUsers}) + result.Groups.JiraUserGroups = append(result.Groups.JiraUserGroups, &JiraUserGroup{visibleToAllUsers}) return result, nil } diff --git a/webapp/src/components/modals/channel_subscriptions/__snapshots__/channel_subscription_filter.test.tsx.snap b/webapp/src/components/modals/channel_subscriptions/__snapshots__/channel_subscription_filter.test.tsx.snap index 8cd36dee5..fee5b34ec 100644 --- a/webapp/src/components/modals/channel_subscriptions/__snapshots__/channel_subscription_filter.test.tsx.snap +++ b/webapp/src/components/modals/channel_subscriptions/__snapshots__/channel_subscription_filter.test.tsx.snap @@ -50,6 +50,34 @@ exports[`components/ChannelSubscriptionFilter should match snapshot 1`] = ` "label": "Affects versions", "value": "versions", }, + Object { + "label": "Comment Visibility", + "value": "commentVisibility", + }, + Object { + "label": "Comment Visibility", + "value": "commentVisibility", + }, + Object { + "label": "Comment Visibility", + "value": "commentVisibility", + }, + Object { + "label": "Comment Visibility", + "value": "commentVisibility", + }, + Object { + "label": "Comment Visibility", + "value": "commentVisibility", + }, + Object { + "label": "Comment Visibility", + "value": "commentVisibility", + }, + Object { + "label": "Comment Visibility", + "value": "commentVisibility", + }, Object { "label": "Epic Link", "value": "customfield_10014", diff --git a/webapp/src/components/modals/channel_subscriptions/__snapshots__/channel_subscription_filters.test.tsx.snap b/webapp/src/components/modals/channel_subscriptions/__snapshots__/channel_subscription_filters.test.tsx.snap index ef1cfd844..ace7afebc 100644 --- a/webapp/src/components/modals/channel_subscriptions/__snapshots__/channel_subscription_filters.test.tsx.snap +++ b/webapp/src/components/modals/channel_subscriptions/__snapshots__/channel_subscription_filters.test.tsx.snap @@ -60,6 +60,20 @@ exports[`components/ChannelSubscriptionFilters should match snapshot 1`] = ` } fields={ Array [ + Object { + "issueTypes": Array [ + Object { + "id": "10001", + "name": "Bug", + }, + ], + "key": "commentVisibility", + "name": "Comment Visibility", + "schema": Object { + "type": "commentVisibility", + }, + "values": Array [], + }, Object { "issueTypes": Array [ Object { @@ -177,6 +191,20 @@ exports[`components/ChannelSubscriptionFilters should match snapshot 1`] = ` } fields={ Array [ + Object { + "issueTypes": Array [ + Object { + "id": "10001", + "name": "Bug", + }, + ], + "key": "commentVisibility", + "name": "Comment Visibility", + "schema": Object { + "type": "commentVisibility", + }, + "values": Array [], + }, Object { "issueTypes": Array [ Object { diff --git a/webapp/src/components/modals/channel_subscriptions/__snapshots__/edit_channel_subscription.test.tsx.snap b/webapp/src/components/modals/channel_subscriptions/__snapshots__/edit_channel_subscription.test.tsx.snap index 9ff6146e3..d6bbae96e 100644 --- a/webapp/src/components/modals/channel_subscriptions/__snapshots__/edit_channel_subscription.test.tsx.snap +++ b/webapp/src/components/modals/channel_subscriptions/__snapshots__/edit_channel_subscription.test.tsx.snap @@ -521,6 +521,176 @@ exports[`components/EditChannelSubscription should match snapshot after fetching }, ], }, + Object { + "issueTypes": Array [ + Object { + "id": "10004", + "name": "Bug", + }, + ], + "key": "commentVisibility", + "name": "Comment Visibility", + "schema": Object { + "type": "commentVisibility", + }, + "values": Array [], + }, + Object { + "issueTypes": Array [ + Object { + "id": "10002", + "name": "Task", + }, + Object { + "id": "10001", + "name": "Story", + }, + Object { + "id": "10004", + "name": "Bug", + }, + Object { + "id": "10000", + "name": "Epic", + }, + ], + "key": "commentVisibility", + "name": "Comment Visibility", + "schema": Object { + "type": "commentVisibility", + }, + "values": Array [], + }, + Object { + "issueTypes": Array [ + Object { + "id": "10002", + "name": "Task", + }, + Object { + "id": "10001", + "name": "Story", + }, + Object { + "id": "10004", + "name": "Bug", + }, + Object { + "id": "10000", + "name": "Epic", + }, + ], + "key": "commentVisibility", + "name": "Comment Visibility", + "schema": Object { + "type": "commentVisibility", + }, + "values": Array [], + }, + Object { + "issueTypes": Array [ + Object { + "id": "10002", + "name": "Task", + }, + Object { + "id": "10001", + "name": "Story", + }, + Object { + "id": "10004", + "name": "Bug", + }, + Object { + "id": "10000", + "name": "Epic", + }, + ], + "key": "commentVisibility", + "name": "Comment Visibility", + "schema": Object { + "type": "commentVisibility", + }, + "values": Array [], + }, + Object { + "issueTypes": Array [ + Object { + "id": "10002", + "name": "Task", + }, + Object { + "id": "10001", + "name": "Story", + }, + Object { + "id": "10004", + "name": "Bug", + }, + Object { + "id": "10000", + "name": "Epic", + }, + ], + "key": "commentVisibility", + "name": "Comment Visibility", + "schema": Object { + "type": "commentVisibility", + }, + "values": Array [], + }, + Object { + "issueTypes": Array [ + Object { + "id": "10002", + "name": "Task", + }, + Object { + "id": "10001", + "name": "Story", + }, + Object { + "id": "10004", + "name": "Bug", + }, + Object { + "id": "10000", + "name": "Epic", + }, + ], + "key": "commentVisibility", + "name": "Comment Visibility", + "schema": Object { + "type": "commentVisibility", + }, + "values": Array [], + }, + Object { + "issueTypes": Array [ + Object { + "id": "10002", + "name": "Task", + }, + Object { + "id": "10001", + "name": "Story", + }, + Object { + "id": "10004", + "name": "Bug", + }, + Object { + "id": "10000", + "name": "Epic", + }, + ], + "key": "commentVisibility", + "name": "Comment Visibility", + "schema": Object { + "type": "commentVisibility", + }, + "values": Array [], + }, Object { "issueTypes": Array [ Object { diff --git a/webapp/src/utils/jira_issue_metadata.test.tsx b/webapp/src/utils/jira_issue_metadata.test.tsx index a69cbdd96..f202db464 100644 --- a/webapp/src/utils/jira_issue_metadata.test.tsx +++ b/webapp/src/utils/jira_issue_metadata.test.tsx @@ -92,11 +92,14 @@ describe('utils/jira_issue_metadata', () => { const actual = getCustomFieldFiltersForProjects(metadata, [projectKey]); expect(actual).not.toBe(null); - expect(actual.length).toBe(1); - - expect(actual[0].key).toEqual('custom1'); - expect(actual[0].name).toEqual('MJK - Checkbox'); - expect(actual[0].values).toEqual([{value: '10033', label: '1'}, {value: '10034', label: '2'}]); + expect(actual.length).toBe(2); + + expect(actual[0].key).toEqual('commentVisibility'); + expect(actual[0].name).toEqual('Comment Visibility'); + expect(actual[0].values).toEqual([]); + expect(actual[1].key).toEqual('custom1'); + expect(actual[1].name).toEqual('MJK - Checkbox'); + expect(actual[1].values).toEqual([{value: '10033', label: '1'}, {value: '10034', label: '2'}]); }); test('should return options for single-select options', () => { @@ -132,11 +135,14 @@ describe('utils/jira_issue_metadata', () => { const actual = getCustomFieldFiltersForProjects(metadata, [projectKey]); expect(actual).not.toBe(null); - expect(actual.length).toBe(1); - - expect(actual[0].key).toEqual('custom1'); - expect(actual[0].name).toEqual('MJK - Radio Buttons'); - expect(actual[0].values).toEqual([{value: '10035', label: '1'}, {value: '10036', label: '2'}]); + expect(actual.length).toBe(2); + + expect(actual[0].key).toEqual('commentVisibility'); + expect(actual[0].name).toEqual('Comment Visibility'); + expect(actual[0].values).toEqual([]); + expect(actual[1].key).toEqual('custom1'); + expect(actual[1].name).toEqual('MJK - Radio Buttons'); + expect(actual[1].values).toEqual([{value: '10035', label: '1'}, {value: '10036', label: '2'}]); }); test('should return options for priority', () => { @@ -197,11 +203,14 @@ describe('utils/jira_issue_metadata', () => { const actual = getCustomFieldFiltersForProjects(metadata, [projectKey]); expect(actual).not.toBe(null); - expect(actual.length).toBe(1); - - expect(actual[0].key).toEqual('priority'); - expect(actual[0].name).toEqual('Priority'); - expect(actual[0].values).toEqual([{value: '1', label: 'Highest'}, {value: '2', label: 'High'}, {value: '3', label: 'Medium'}, {value: '4', label: 'Low'}, {value: '5', label: 'Lowest'}]); + expect(actual.length).toBe(2); + + expect(actual[0].key).toEqual('commentVisibility'); + expect(actual[0].name).toEqual('Comment Visibility'); + expect(actual[0].values).toEqual([]); + expect(actual[1].key).toEqual('priority'); + expect(actual[1].name).toEqual('Priority'); + expect(actual[1].values).toEqual([{value: '1', label: 'Highest'}, {value: '2', label: 'High'}, {value: '3', label: 'Medium'}, {value: '4', label: 'Low'}, {value: '5', label: 'Lowest'}]); }); test('should return options for fix version', () => { @@ -231,11 +240,14 @@ describe('utils/jira_issue_metadata', () => { const actual = getCustomFieldFiltersForProjects(metadata, [projectKey]); expect(actual).not.toBe(null); - expect(actual.length).toBe(1); - - expect(actual[0].key).toEqual('fixVersions'); - expect(actual[0].name).toEqual('Fix versions'); - expect(actual[0].values).toEqual([{value: '10000', label: '5.14 (August 2019)'}]); + expect(actual.length).toBe(2); + + expect(actual[0].key).toEqual('commentVisibility'); + expect(actual[0].name).toEqual('Comment Visibility'); + expect(actual[0].values).toEqual([]); + expect(actual[1].key).toEqual('fixVersions'); + expect(actual[1].name).toEqual('Fix versions'); + expect(actual[1].values).toEqual([{value: '10000', label: '5.14 (August 2019)'}]); }); test('should return options for security level', () => { @@ -284,11 +296,14 @@ describe('utils/jira_issue_metadata', () => { const actual = getCustomFieldFiltersForProjects(metadata, [projectKey]); expect(actual).not.toBe(null); - expect(actual.length).toBe(1); - - expect(actual[0].key).toEqual('security'); - expect(actual[0].name).toEqual('Security Level'); - expect(actual[0].values).toEqual([{value: '10001', label: 'Admin only'}, {value: '10000', label: 'Everyone'}, {value: '10002', label: 'Staff'}]); + expect(actual.length).toBe(2); + + expect(actual[0].key).toEqual('commentVisibility'); + expect(actual[0].name).toEqual('Comment Visibility'); + expect(actual[0].values).toEqual([]); + expect(actual[1].key).toEqual('security'); + expect(actual[1].name).toEqual('Security Level'); + expect(actual[1].values).toEqual([{value: '10001', label: 'Admin only'}, {value: '10000', label: 'Everyone'}, {value: '10002', label: 'Staff'}]); }); test('should return options with a `userDefined` flag for array of strings', () => { From 649d5202b4b6448f52df297dfd004487c36b944d Mon Sep 17 00:00:00 2001 From: Kshitij Katiyar Date: Mon, 11 Sep 2023 16:08:22 +0530 Subject: [PATCH 4/9] [MM-626]:Fixed a endpoint --- server/http.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/http.go b/server/http.go index 2ace70439..0ec0c29df 100644 --- a/server/http.go +++ b/server/http.go @@ -24,7 +24,7 @@ import ( ) const ( - routeAPIGetCommentVisibilityFields = "/api/v2/get-comment-visibility-fields" + routeAPIGetCommentVisibilityFields = "/get-comment-visibility-fields" routeAutocomplete = "/autocomplete" routeAutocompleteConnect = "/connect" routeAutocompleteUserInstance = "/user-instance" From 279ed3df29a223f7cc06395d0aa9f70657c7cffb Mon Sep 17 00:00:00 2001 From: raghavaggarwal2308 Date: Mon, 13 May 2024 19:26:48 +0530 Subject: [PATCH 5/9] Fixed: 1. Path of API call to get commen data 2. Testcases --- server/webhook_worker.go | 6 ++--- webapp/src/utils/jira_issue_metadata.test.tsx | 26 +++++++++---------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/server/webhook_worker.go b/server/webhook_worker.go index 49f32df2f..8678e9aca 100644 --- a/server/webhook_worker.go +++ b/server/webhook_worker.go @@ -4,10 +4,10 @@ package main import ( + "fmt" jira "github.com/andygrunwald/go-jira" - "github.com/pkg/errors" - "github.com/mattermost/mattermost-plugin-jira/server/utils/types" + "github.com/pkg/errors" ) type webhookWorker struct { @@ -51,7 +51,7 @@ func (ww webhookWorker) getVisibilityAttribute(msg *webhookMessage, v *webhook) } comment := jira.Comment{} - if err = client.RESTGet(v.JiraWebhook.Comment.Self, nil, &comment); err != nil { + if err = client.RESTGet(fmt.Sprintf("2/issue/%s/comment/%s", v.JiraWebhook.Issue.ID, v.JiraWebhook.Comment.ID), nil, &comment); err != nil { return "", err } diff --git a/webapp/src/utils/jira_issue_metadata.test.tsx b/webapp/src/utils/jira_issue_metadata.test.tsx index 8055ee5af..9f2f42a06 100644 --- a/webapp/src/utils/jira_issue_metadata.test.tsx +++ b/webapp/src/utils/jira_issue_metadata.test.tsx @@ -95,7 +95,7 @@ describe('utils/jira_issue_metadata', () => { const actual = getCustomFieldFiltersForProjects(metadata, [projectKey], []); expect(actual).not.toBe(null); - expect(actual.length).toBe(2); + expect(actual.length).toBe(3); expect(actual[0].key).toEqual('commentVisibility'); expect(actual[0].name).toEqual('Comment Visibility'); @@ -103,7 +103,7 @@ describe('utils/jira_issue_metadata', () => { expect(actual[1].key).toEqual('custom1'); expect(actual[1].name).toEqual('MJK - Checkbox'); expect(actual[1].values).toEqual([{value: '10033', label: '1'}, {value: '10034', label: '2'}]); - expect(actual[1].name).toBe('Status'); + expect(actual[2].name).toBe('Status'); }); test('should return options for single-select options', () => { @@ -139,7 +139,7 @@ describe('utils/jira_issue_metadata', () => { const actual = getCustomFieldFiltersForProjects(metadata, [projectKey], []); expect(actual).not.toBe(null); - expect(actual.length).toBe(2); + expect(actual.length).toBe(3); expect(actual[0].key).toEqual('commentVisibility'); expect(actual[0].name).toEqual('Comment Visibility'); @@ -147,7 +147,7 @@ describe('utils/jira_issue_metadata', () => { expect(actual[1].key).toEqual('custom1'); expect(actual[1].name).toEqual('MJK - Radio Buttons'); expect(actual[1].values).toEqual([{value: '10035', label: '1'}, {value: '10036', label: '2'}]); - expect(actual[1].name).toBe('Status'); + expect(actual[2].name).toBe('Status'); }); test('should return options for priority', () => { @@ -208,7 +208,7 @@ describe('utils/jira_issue_metadata', () => { const actual = getCustomFieldFiltersForProjects(metadata, [projectKey], []); expect(actual).not.toBe(null); - expect(actual.length).toBe(2); + expect(actual.length).toBe(3); expect(actual[0].key).toEqual('commentVisibility'); expect(actual[0].name).toEqual('Comment Visibility'); @@ -216,7 +216,7 @@ describe('utils/jira_issue_metadata', () => { expect(actual[1].key).toEqual('priority'); expect(actual[1].name).toEqual('Priority'); expect(actual[1].values).toEqual([{value: '1', label: 'Highest'}, {value: '2', label: 'High'}, {value: '3', label: 'Medium'}, {value: '4', label: 'Low'}, {value: '5', label: 'Lowest'}]); - expect(actual[1].name).toBe('Status'); + expect(actual[2].name).toBe('Status'); }); test('should return options for fix version', () => { @@ -246,7 +246,7 @@ describe('utils/jira_issue_metadata', () => { const actual = getCustomFieldFiltersForProjects(metadata, [projectKey], []); expect(actual).not.toBe(null); - expect(actual.length).toBe(2); + expect(actual.length).toBe(3); expect(actual[0].key).toEqual('commentVisibility'); expect(actual[0].name).toEqual('Comment Visibility'); @@ -254,7 +254,7 @@ describe('utils/jira_issue_metadata', () => { expect(actual[1].key).toEqual('fixVersions'); expect(actual[1].name).toEqual('Fix versions'); expect(actual[1].values).toEqual([{value: '10000', label: '5.14 (August 2019)'}]); - expect(actual[1].name).toBe('Status'); + expect(actual[2].name).toBe('Status'); }); test('should return options for security level', () => { @@ -303,15 +303,15 @@ describe('utils/jira_issue_metadata', () => { const actual = getCustomFieldFiltersForProjects(metadata, [projectKey], []); expect(actual).not.toBe(null); - expect(actual.length).toBe(2); + expect(actual.length).toBe(3); expect(actual[0].key).toEqual('commentVisibility'); expect(actual[0].name).toEqual('Comment Visibility'); expect(actual[0].values).toEqual([]); - expect(actual[0].key).toEqual('security'); - expect(actual[0].name).toEqual('Security Level'); - expect(actual[0].values).toEqual([{value: '10001', label: 'Admin only'}, {value: '10000', label: 'Everyone'}, {value: '10002', label: 'Staff'}]); - expect(actual[1].name).toBe('Status'); + expect(actual[1].key).toEqual('security'); + expect(actual[1].name).toEqual('Security Level'); + expect(actual[1].values).toEqual([{value: '10001', label: 'Admin only'}, {value: '10000', label: 'Everyone'}, {value: '10002', label: 'Staff'}]); + expect(actual[2].name).toBe('Status'); }); test('getStatusField should return options for statuses for selected issue types only', () => { From 8929dfe1057d638634ca0d05db1ce05388e33969 Mon Sep 17 00:00:00 2001 From: raghavaggarwal2308 Date: Wed, 15 May 2024 13:36:34 +0530 Subject: [PATCH 6/9] Review fixes --- server/webhook_worker.go | 12 +- .../channel_subscription_filter.test.tsx.snap | 24 --- .../edit_channel_subscription.test.tsx.snap | 144 +----------------- webapp/src/utils/jira_issue_metadata.test.tsx | 18 ++- webapp/src/utils/jira_issue_metadata.tsx | 31 ++-- 5 files changed, 37 insertions(+), 192 deletions(-) diff --git a/server/webhook_worker.go b/server/webhook_worker.go index 8678e9aca..a2692bcd4 100644 --- a/server/webhook_worker.go +++ b/server/webhook_worker.go @@ -4,10 +4,12 @@ package main import ( - "fmt" jira "github.com/andygrunwald/go-jira" - "github.com/mattermost/mattermost-plugin-jira/server/utils/types" "github.com/pkg/errors" + + "github.com/mattermost/mattermost-plugin-jira/server/utils/types" + + "fmt" ) type webhookWorker struct { @@ -38,10 +40,10 @@ func isCommentRelatedWebhook(wh Webhook) bool { return wh.Events().Intersection(commentEvents).Len() > 0 } -func (ww webhookWorker) getVisibilityAttribute(msg *webhookMessage, v *webhook) (string, error) { +func (ww webhookWorker) getCommentVisibility(msg *webhookMessage, v *webhook) (string, error) { mattermostUserID, err := ww.p.userStore.LoadMattermostUserID(msg.InstanceID, v.JiraWebhook.Comment.Author.AccountID) if err != nil { - ww.p.API.LogInfo("Commentator is not connected with the mattermost", "Error", err.Error()) + ww.p.API.LogDebug("Comment author is not connected with Mattermost", "Error", err.Error()) return "", err } @@ -82,7 +84,7 @@ func (ww webhookWorker) process(msg *webhookMessage) (err error) { visibilityAttribute := "" if isCommentRelatedWebhook(wh) { - if visibilityAttribute, err = ww.getVisibilityAttribute(msg, v); err != nil { + if visibilityAttribute, err = ww.getCommentVisibility(msg, v); err != nil { return err } } diff --git a/webapp/src/components/modals/channel_subscriptions/__snapshots__/channel_subscription_filter.test.tsx.snap b/webapp/src/components/modals/channel_subscriptions/__snapshots__/channel_subscription_filter.test.tsx.snap index 8e1d7d842..c0c379544 100644 --- a/webapp/src/components/modals/channel_subscriptions/__snapshots__/channel_subscription_filter.test.tsx.snap +++ b/webapp/src/components/modals/channel_subscriptions/__snapshots__/channel_subscription_filter.test.tsx.snap @@ -54,30 +54,6 @@ exports[`components/ChannelSubscriptionFilter should match snapshot 1`] = ` "label": "Comment Visibility", "value": "commentVisibility", }, - Object { - "label": "Comment Visibility", - "value": "commentVisibility", - }, - Object { - "label": "Comment Visibility", - "value": "commentVisibility", - }, - Object { - "label": "Comment Visibility", - "value": "commentVisibility", - }, - Object { - "label": "Comment Visibility", - "value": "commentVisibility", - }, - Object { - "label": "Comment Visibility", - "value": "commentVisibility", - }, - Object { - "label": "Comment Visibility", - "value": "commentVisibility", - }, Object { "label": "Epic Link", "value": "customfield_10014", diff --git a/webapp/src/components/modals/channel_subscriptions/__snapshots__/edit_channel_subscription.test.tsx.snap b/webapp/src/components/modals/channel_subscriptions/__snapshots__/edit_channel_subscription.test.tsx.snap index 73a542f1d..fd81d16be 100644 --- a/webapp/src/components/modals/channel_subscriptions/__snapshots__/edit_channel_subscription.test.tsx.snap +++ b/webapp/src/components/modals/channel_subscriptions/__snapshots__/edit_channel_subscription.test.tsx.snap @@ -521,124 +521,6 @@ exports[`components/EditChannelSubscription should match snapshot after fetching }, ], }, - Object { - "issueTypes": Array [ - Object { - "id": "10004", - "name": "Bug", - }, - ], - "key": "commentVisibility", - "name": "Comment Visibility", - "schema": Object { - "type": "commentVisibility", - }, - "values": Array [], - }, - Object { - "issueTypes": Array [ - Object { - "id": "10002", - "name": "Task", - }, - Object { - "id": "10001", - "name": "Story", - }, - Object { - "id": "10004", - "name": "Bug", - }, - Object { - "id": "10000", - "name": "Epic", - }, - ], - "key": "commentVisibility", - "name": "Comment Visibility", - "schema": Object { - "type": "commentVisibility", - }, - "values": Array [], - }, - Object { - "issueTypes": Array [ - Object { - "id": "10002", - "name": "Task", - }, - Object { - "id": "10001", - "name": "Story", - }, - Object { - "id": "10004", - "name": "Bug", - }, - Object { - "id": "10000", - "name": "Epic", - }, - ], - "key": "commentVisibility", - "name": "Comment Visibility", - "schema": Object { - "type": "commentVisibility", - }, - "values": Array [], - }, - Object { - "issueTypes": Array [ - Object { - "id": "10002", - "name": "Task", - }, - Object { - "id": "10001", - "name": "Story", - }, - Object { - "id": "10004", - "name": "Bug", - }, - Object { - "id": "10000", - "name": "Epic", - }, - ], - "key": "commentVisibility", - "name": "Comment Visibility", - "schema": Object { - "type": "commentVisibility", - }, - "values": Array [], - }, - Object { - "issueTypes": Array [ - Object { - "id": "10002", - "name": "Task", - }, - Object { - "id": "10001", - "name": "Story", - }, - Object { - "id": "10004", - "name": "Bug", - }, - Object { - "id": "10000", - "name": "Epic", - }, - ], - "key": "commentVisibility", - "name": "Comment Visibility", - "schema": Object { - "type": "commentVisibility", - }, - "values": Array [], - }, Object { "issueTypes": Array [ Object { @@ -646,30 +528,8 @@ exports[`components/EditChannelSubscription should match snapshot after fetching "name": "Task", }, Object { - "id": "10001", - "name": "Story", - }, - Object { - "id": "10004", - "name": "Bug", - }, - Object { - "id": "10000", - "name": "Epic", - }, - ], - "key": "commentVisibility", - "name": "Comment Visibility", - "schema": Object { - "type": "commentVisibility", - }, - "values": Array [], - }, - Object { - "issueTypes": Array [ - Object { - "id": "10002", - "name": "Task", + "id": "10003", + "name": "Sub-task", }, Object { "id": "10001", diff --git a/webapp/src/utils/jira_issue_metadata.test.tsx b/webapp/src/utils/jira_issue_metadata.test.tsx index 9f2f42a06..aa67282f1 100644 --- a/webapp/src/utils/jira_issue_metadata.test.tsx +++ b/webapp/src/utils/jira_issue_metadata.test.tsx @@ -37,7 +37,7 @@ describe('utils/jira_issue_metadata', () => { expect(actual.length).toBeGreaterThan(0); }); - test('should return only the status field if there are no available values', () => { + test('should return only the status and comment visibility field if there are no available values', () => { const field = { hasDefaultValue: false, key: 'customfield_10021', @@ -59,8 +59,9 @@ describe('utils/jira_issue_metadata', () => { const actual = getCustomFieldFiltersForProjects(metadata, [projectKey], []); expect(actual).not.toBe(null); - expect(actual.length).toBe(1); - expect(actual[0].name).toBe('Status'); + expect(actual.length).toBe(2); + expect(actual[0].name).toBe('Comment Visibility'); + expect(actual[1].name).toBe('Status'); }); test('should return options for multi-select options', () => { @@ -376,12 +377,13 @@ describe('utils/jira_issue_metadata', () => { const actual = getCustomFieldFiltersForProjects(metadata, [projectKey], []); expect(actual).not.toBe(null); - expect(actual.length).toBe(2); + expect(actual.length).toBe(3); - expect(actual[0].key).toEqual('custom1'); - expect(actual[0].name).toEqual('MJK - Labels'); - expect(actual[0].userDefined).toEqual(true); - expect(actual[1].name).toBe('Status'); + expect(actual[0].name).toBe('Comment Visibility'); + expect(actual[1].key).toEqual('custom1'); + expect(actual[1].name).toEqual('MJK - Labels'); + expect(actual[1].userDefined).toEqual(true); + expect(actual[2].name).toBe('Status'); }); test('getConflictingFields should return a list of fields with conflicts', () => { diff --git a/webapp/src/utils/jira_issue_metadata.tsx b/webapp/src/utils/jira_issue_metadata.tsx index 5a7223761..25040078e 100644 --- a/webapp/src/utils/jira_issue_metadata.tsx +++ b/webapp/src/utils/jira_issue_metadata.tsx @@ -259,19 +259,7 @@ export function getCustomFieldFiltersForProjects(metadata: IssueMetadata | null, } as FilterField; }); - const commentVisibility = selectFields.map((field) => { - return { - key: commentVisibilityFieldKey, - name: 'Comment Visibility', - schema: { - type: 'commentVisibility', - }, - values: [], - issueTypes: field.validIssueTypes, - } as FilterField; - }); - - const result = populatedFields.concat(userDefinedFields, commentVisibility); + const result = populatedFields.concat(userDefinedFields); const epicLinkField = fields.find(isEpicLinkField); if (epicLinkField) { result.unshift({ @@ -283,6 +271,23 @@ export function getCustomFieldFiltersForProjects(metadata: IssueMetadata | null, } as FilterField); } + const commentVisibilityField = { + key: commentVisibilityFieldKey, + name: 'Comment Visibility', + schema: { + type: 'commentVisibility', + }, + values: [], + issueTypes: metadata && metadata.issue_types_with_statuses.map((type) => { + return { + id: type.id, + name: type.name, + }; + }), + } as FilterField; + + result.push(commentVisibilityField); + const statusField = getStatusField(metadata, issueTypes); if (statusField) { result.push(statusField); From 1d1b82ad2b6a6a9d64ede8534c2d0269e58ed298 Mon Sep 17 00:00:00 2001 From: raghavaggarwal2308 Date: Mon, 20 May 2024 13:57:27 +0530 Subject: [PATCH 7/9] Review fixes --- server/issue.go | 5 +++-- .../jira_commentvisibility_selector.tsx | 1 - .../channel_subscription_filter.tsx | 10 +++++----- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/server/issue.go b/server/issue.go index 296950b68..000ee5982 100644 --- a/server/issue.go +++ b/server/issue.go @@ -32,10 +32,11 @@ const ( headerMattermostUserID = "Mattermost-User-ID" instanceIDQueryParam = "instance_id" fieldValueQueryParam = "fieldValue" - expandQueryParam = "expand" QueryParamInstanceID = "instance_id" QueryParamProjectID = "project_id" + + expandValueGroups = "groups" ) type CreateMetaInfo struct { @@ -429,7 +430,7 @@ func (p *Plugin) httpGetCommentVisibilityFields(w http.ResponseWriter, r *http.R params := map[string]string{ "fieldValue": r.FormValue(fieldValueQueryParam), - "expand": r.FormValue(expandQueryParam), + "expand": expandValueGroups, "accountId": connection.AccountID, } response, err := client.SearchCommentVisibilityFields(params) diff --git a/webapp/src/components/data_selectors/jira_commentvisibility_selector/jira_commentvisibility_selector.tsx b/webapp/src/components/data_selectors/jira_commentvisibility_selector/jira_commentvisibility_selector.tsx index c23019ac3..43961e56a 100644 --- a/webapp/src/components/data_selectors/jira_commentvisibility_selector/jira_commentvisibility_selector.tsx +++ b/webapp/src/components/data_selectors/jira_commentvisibility_selector/jira_commentvisibility_selector.tsx @@ -32,7 +32,6 @@ const JiraCommentVisibilitySelector = (props: Props) => { const params = { fieldValue: inputValue, instance_id: instanceID, - expand: 'groups', }; return searchCommentVisibilityFields(params).then(({data}) => { return data.groups.items.map((suggestion) => ({ diff --git a/webapp/src/components/modals/channel_subscriptions/channel_subscription_filter.tsx b/webapp/src/components/modals/channel_subscriptions/channel_subscription_filter.tsx index 412a4dc1b..ad2fb8102 100644 --- a/webapp/src/components/modals/channel_subscriptions/channel_subscription_filter.tsx +++ b/webapp/src/components/modals/channel_subscriptions/channel_subscription_filter.tsx @@ -66,7 +66,7 @@ export default class ChannelSubscriptionFilter extends React.PureComponent { + handleValueChangeWithoutName = (values: string[]): void => { const {onChange, value} = this.props; const newValues = values || []; @@ -283,7 +283,7 @@ export default class ChannelSubscriptionFilter extends React.PureComponent ); } else if (isEpicLinkField(this.props.field)) { @@ -292,7 +292,7 @@ export default class ChannelSubscriptionFilter extends React.PureComponent ); } else if (isLabelField(field)) { @@ -301,7 +301,7 @@ export default class ChannelSubscriptionFilter extends React.PureComponent ); } else { @@ -386,7 +386,7 @@ export default class ChannelSubscriptionFilter extends React.PureComponent void; From 70efc7e231f07008e4041577a7f4113fc44f729d Mon Sep 17 00:00:00 2001 From: raghavaggarwal2308 Date: Mon, 1 Jul 2024 12:27:52 +0530 Subject: [PATCH 8/9] [MM-537] Review fixes: 1. Removed extra API call for getting comment visibility details 2. Removed unnecessary commentVisibility attribute --- server/subscribe.go | 16 +++++---- server/subscribe_test.go | 2 +- server/webhook_worker.go | 36 +------------------ .../jira_commentvisibility_selector.tsx | 4 +-- .../channel_subscription_filter.tsx | 2 +- 5 files changed, 14 insertions(+), 46 deletions(-) diff --git a/server/subscribe.go b/server/subscribe.go index f0812f531..c3727d033 100644 --- a/server/subscribe.go +++ b/server/subscribe.go @@ -125,7 +125,7 @@ func (p *Plugin) getUserID() string { return p.getConfig().botUserID } -func (p *Plugin) matchesSubsciptionFilters(wh *webhook, filters SubscriptionFilters, visibilityAttribute string) bool { +func (p *Plugin) matchesSubsciptionFilters(wh *webhook, filters SubscriptionFilters) bool { webhookEvents := wh.Events() foundEvent := false eventTypes := filters.Events @@ -171,10 +171,12 @@ func (p *Plugin) matchesSubsciptionFilters(wh *webhook, filters SubscriptionFilt } value := getIssueFieldValue(issue, field.Key) - if visibilityAttribute != "" { - value[visibilityAttribute] = true - } else if field.Key == CommentVisibility { - value[visibleToAllUsers] = true + if field.Key == CommentVisibility { + if wh.Comment.Visibility.Value != "" { + value = value.Add(wh.Comment.Visibility.Value) + } else { + value = value.Add(visibleToAllUsers) + } } if !isValidFieldInclusion(field, value, inclusion) { @@ -206,7 +208,7 @@ func isValidFieldInclusion(field FieldFilter, value StringSet, inclusion string) return true } -func (p *Plugin) getChannelsSubscribed(wh *webhook, instanceID types.ID, visibilityAttribute string) ([]ChannelSubscription, error) { +func (p *Plugin) getChannelsSubscribed(wh *webhook, instanceID types.ID) ([]ChannelSubscription, error) { subs, err := p.getSubscriptions(instanceID) if err != nil { return nil, err @@ -215,7 +217,7 @@ func (p *Plugin) getChannelsSubscribed(wh *webhook, instanceID types.ID, visibil var channelSubscriptions []ChannelSubscription subIds := subs.Channel.ByID for _, sub := range subIds { - if p.matchesSubsciptionFilters(wh, sub.Filters, visibilityAttribute) { + if p.matchesSubsciptionFilters(wh, sub.Filters) { channelSubscriptions = append(channelSubscriptions, sub) } } diff --git a/server/subscribe_test.go b/server/subscribe_test.go index 871419fcb..9ebb12bc7 100644 --- a/server/subscribe_test.go +++ b/server/subscribe_test.go @@ -1662,7 +1662,7 @@ func TestGetChannelsSubscribed(t *testing.T) { wh, err := ParseWebhook(bb) assert.Nil(t, err) - actual, err := p.getChannelsSubscribed(wh.(*webhook), testInstance1.InstanceID, "") + actual, err := p.getChannelsSubscribed(wh.(*webhook), testInstance1.InstanceID) assert.Nil(t, err) assert.Equal(t, len(tc.ChannelSubscriptions), len(actual)) actualChannelIDs := NewStringSet() diff --git a/server/webhook_worker.go b/server/webhook_worker.go index a2692bcd4..859cc6bcc 100644 --- a/server/webhook_worker.go +++ b/server/webhook_worker.go @@ -4,12 +4,9 @@ package main import ( - jira "github.com/andygrunwald/go-jira" "github.com/pkg/errors" "github.com/mattermost/mattermost-plugin-jira/server/utils/types" - - "fmt" ) type webhookWorker struct { @@ -36,30 +33,6 @@ func (ww webhookWorker) work() { } } -func isCommentRelatedWebhook(wh Webhook) bool { - return wh.Events().Intersection(commentEvents).Len() > 0 -} - -func (ww webhookWorker) getCommentVisibility(msg *webhookMessage, v *webhook) (string, error) { - mattermostUserID, err := ww.p.userStore.LoadMattermostUserID(msg.InstanceID, v.JiraWebhook.Comment.Author.AccountID) - if err != nil { - ww.p.API.LogDebug("Comment author is not connected with Mattermost", "Error", err.Error()) - return "", err - } - - client, _, _, err := ww.p.getClient(msg.InstanceID, mattermostUserID) - if err != nil { - return "", err - } - - comment := jira.Comment{} - if err = client.RESTGet(fmt.Sprintf("2/issue/%s/comment/%s", v.JiraWebhook.Issue.ID, v.JiraWebhook.Comment.ID), nil, &comment); err != nil { - return "", err - } - - return comment.Visibility.Value, nil -} - func (ww webhookWorker) process(msg *webhookMessage) (err error) { defer func() { if err == ErrWebhookIgnored { @@ -82,14 +55,7 @@ func (ww webhookWorker) process(msg *webhookMessage) (err error) { return err } - visibilityAttribute := "" - if isCommentRelatedWebhook(wh) { - if visibilityAttribute, err = ww.getCommentVisibility(msg, v); err != nil { - return err - } - } - - channelsSubscribed, err := ww.p.getChannelsSubscribed(v, msg.InstanceID, visibilityAttribute) + channelsSubscribed, err := ww.p.getChannelsSubscribed(v, msg.InstanceID) if err != nil { return err } diff --git a/webapp/src/components/data_selectors/jira_commentvisibility_selector/jira_commentvisibility_selector.tsx b/webapp/src/components/data_selectors/jira_commentvisibility_selector/jira_commentvisibility_selector.tsx index 43961e56a..831778921 100644 --- a/webapp/src/components/data_selectors/jira_commentvisibility_selector/jira_commentvisibility_selector.tsx +++ b/webapp/src/components/data_selectors/jira_commentvisibility_selector/jira_commentvisibility_selector.tsx @@ -25,8 +25,6 @@ type Props = BackendSelectorProps & { const JiraCommentVisibilitySelector = (props: Props) => { const {value, isMulti, instanceID, searchCommentVisibilityFields} = props; - const fetchInitialSelectedValues = async (): Promise => - ((!value || (isMulti && !value.length)) ? [] : commentVisibilityFields('')); const commentVisibilityFields = async (inputValue: string): Promise => { const params = { @@ -41,6 +39,8 @@ const JiraCommentVisibilitySelector = (props: Props) => { }); }; + const fetchInitialSelectedValues = async (): Promise => (value?.length ? commentVisibilityFields('') : []); + return ( Date: Mon, 1 Jul 2024 14:08:55 +0530 Subject: [PATCH 9/9] [MM-537] Fix lint error --- server/webhook_http.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/webhook_http.go b/server/webhook_http.go index ee97dc06f..3fe651b7f 100644 --- a/server/webhook_http.go +++ b/server/webhook_http.go @@ -29,7 +29,7 @@ var eventParamMasks = map[string]StringSet{ "updated_description": NewStringSet(eventUpdatedDescription), // issue description edited "updated_labels": NewStringSet(eventUpdatedLabels), // updated labels "updated_prioity": NewStringSet(eventUpdatedPriority), // changes in priority - "updated_rank": NewStringSet(eventUpdatedRank), // ranked higher or lower + "updated_rank": NewStringSet(eventUpdatedRank), // ranked higher or lower "updated_sprint": NewStringSet(eventUpdatedSprint), // assigned to a different sprint "updated_status": NewStringSet(eventUpdatedStatus), // transitions like Done, In Progress "updated_summary": NewStringSet(eventUpdatedSummary), // issue renamed