From 63d1b3b5cf40fb2007d51b3e83273de77b054481 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Thu, 25 Jul 2024 15:54:00 -0600 Subject: [PATCH 1/6] Add approver token to the Vue MR List filtered search token list --- app/assets/javascripts/issues/list/constants.js | 11 +++++++++++ .../list/components/merge_requests_list_app.vue | 16 ++++++++++++++++ .../queries/get_merge_requests.query.graphql | 2 ++ .../components/filtered_search_bar/constants.js | 2 ++ 4 files changed, 31 insertions(+) diff --git a/app/assets/javascripts/issues/list/constants.js b/app/assets/javascripts/issues/list/constants.js index 181fb580d22664..0d434a10851bb2 100644 --- a/app/assets/javascripts/issues/list/constants.js +++ b/app/assets/javascripts/issues/list/constants.js @@ -12,6 +12,7 @@ import { OPERATOR_AFTER, OPERATOR_BEFORE, TOKEN_TYPE_APPROVED_BY, + TOKEN_TYPE_APPROVER, TOKEN_TYPE_ASSIGNEE, TOKEN_TYPE_REVIEWER, TOKEN_TYPE_AUTHOR, @@ -166,6 +167,16 @@ export const filtersMap = { }, }, }, + [TOKEN_TYPE_APPROVER]: { + [API_PARAM]: { + [NORMAL_FILTER]: 'approver', + }, + [URL_PARAM]: { + [OPERATOR_IS]: { + [NORMAL_FILTER]: 'approver', + }, + }, + }, [TOKEN_TYPE_AUTHOR]: { [API_PARAM]: { [NORMAL_FILTER]: 'authorUsername', diff --git a/app/assets/javascripts/merge_requests/list/components/merge_requests_list_app.vue b/app/assets/javascripts/merge_requests/list/components/merge_requests_list_app.vue index ecc868a27da973..985699d869860a 100644 --- a/app/assets/javascripts/merge_requests/list/components/merge_requests_list_app.vue +++ b/app/assets/javascripts/merge_requests/list/components/merge_requests_list_app.vue @@ -19,6 +19,8 @@ import { OPERATORS_IS_NOT, TOKEN_TITLE_APPROVED_BY, TOKEN_TYPE_APPROVED_BY, + TOKEN_TITLE_APPROVER, + TOKEN_TYPE_APPROVER, TOKEN_TITLE_AUTHOR, TOKEN_TYPE_AUTHOR, TOKEN_TITLE_DRAFT, @@ -221,6 +223,20 @@ export default { preloadedUsers, multiSelect: false, }, + { + type: TOKEN_TYPE_APPROVER, + title: TOKEN_TITLE_APPROVER, + icon: 'approval', + token: UserToken, + dataType: 'user', + operators: OPERATORS_IS, + fullPath: this.fullPath, + isProject: true, + recentSuggestionsStorageKey: `${this.fullPath}-merge_requests-recent-tokens-approvers`, + preloadedUsers, + multiSelect: false, + unique: true, + }, { type: TOKEN_TYPE_ASSIGNEE, title: TOKEN_TITLE_ASSIGNEE, diff --git a/app/assets/javascripts/merge_requests/list/queries/get_merge_requests.query.graphql b/app/assets/javascripts/merge_requests/list/queries/get_merge_requests.query.graphql index b386a7e8cf3061..5e44c857690630 100644 --- a/app/assets/javascripts/merge_requests/list/queries/get_merge_requests.query.graphql +++ b/app/assets/javascripts/merge_requests/list/queries/get_merge_requests.query.graphql @@ -8,6 +8,7 @@ query getMergeRequests( $sort: MergeRequestSort $state: MergeRequestState $approvedBy: [String!] + $approver: String $assigneeUsernames: String $assigneeWildcardId: AssigneeWildcardId $reviewerUsername: String @@ -36,6 +37,7 @@ query getMergeRequests( sort: $sort state: $state approvedBy: $approvedBy + approver: $approver assigneeUsername: $assigneeUsernames assigneeWildcardId: $assigneeWildcardId reviewerUsername: $reviewerUsername diff --git a/app/assets/javascripts/vue_shared/components/filtered_search_bar/constants.js b/app/assets/javascripts/vue_shared/components/filtered_search_bar/constants.js index 620e0698972494..846a377ac2892c 100644 --- a/app/assets/javascripts/vue_shared/components/filtered_search_bar/constants.js +++ b/app/assets/javascripts/vue_shared/components/filtered_search_bar/constants.js @@ -63,6 +63,7 @@ export const TOKEN_EMPTY_SEARCH_TERM = { export const TOKEN_TITLE_APPROVED_BY = __('Approved-By'); export const TOKEN_TITLE_MERGE_USER = __('Merged-By'); +export const TOKEN_TITLE_APPROVER = __('Approver'); export const TOKEN_TITLE_ASSIGNEE = s__('SearchToken|Assignee'); export const TOKEN_TITLE_AUTHOR = __('Author'); export const TOKEN_TITLE_CONFIDENTIAL = __('Confidential'); @@ -90,6 +91,7 @@ export const TOKEN_TITLE_DEPLOYED_BEFORE = __('Deployed-before'); export const TOKEN_TITLE_DEPLOYED_AFTER = __('Deployed-after'); export const TOKEN_TITLE_ASSIGNED_SEAT = __('Assigned seat'); +export const TOKEN_TYPE_APPROVER = __('approver'); export const TOKEN_TYPE_APPROVED_BY = 'approved-by'; export const TOKEN_TYPE_MERGE_USER = 'merge-user'; export const TOKEN_TYPE_ASSIGNEE = 'assignee'; -- GitLab From cbd4f84c4329dbf46a165f1bdcdfc3e1d745a89c Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Thu, 25 Jul 2024 15:57:42 -0600 Subject: [PATCH 2/6] Add tests for the approver search token --- .../list/components/merge_requests_list_app_spec.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/spec/frontend/merge_requests/list/components/merge_requests_list_app_spec.js b/spec/frontend/merge_requests/list/components/merge_requests_list_app_spec.js index 576349328994ef..880e6232f10fef 100644 --- a/spec/frontend/merge_requests/list/components/merge_requests_list_app_spec.js +++ b/spec/frontend/merge_requests/list/components/merge_requests_list_app_spec.js @@ -15,6 +15,7 @@ import { TYPENAME_USER } from '~/graphql_shared/constants'; import { convertToGraphQLId } from '~/graphql_shared/utils'; import { TOKEN_TYPE_APPROVED_BY, + TOKEN_TYPE_APPROVER, TOKEN_TYPE_AUTHOR, TOKEN_TYPE_DRAFT, TOKEN_TYPE_LABEL, @@ -151,6 +152,7 @@ describe('Merge requests list app', () => { it('does not have preloaded users when gon.current_user_id does not exist', () => { expect(findIssuableList().props('searchTokens')).toMatchObject([ { type: TOKEN_TYPE_APPROVED_BY, preloadedUsers: [] }, + { type: TOKEN_TYPE_APPROVER, preloadedUsers: [] }, { type: TOKEN_TYPE_ASSIGNEE }, { type: TOKEN_TYPE_REVIEWER, preloadedUsers: [] }, { type: TOKEN_TYPE_AUTHOR, preloadedUsers: [] }, @@ -171,6 +173,7 @@ describe('Merge requests list app', () => { describe('when all tokens are available', () => { const urlParams = { 'approved_by_usernames[]': 'anthony', + approver: 'angus', assignee_username: 'bob', reviewer_username: 'bill', draft: 'yes', @@ -208,6 +211,7 @@ describe('Merge requests list app', () => { expect(findIssuableList().props('searchTokens')).toMatchObject([ { type: TOKEN_TYPE_APPROVED_BY, preloadedUsers }, + { type: TOKEN_TYPE_APPROVER, preloadedUsers }, { type: TOKEN_TYPE_ASSIGNEE }, { type: TOKEN_TYPE_REVIEWER, preloadedUsers }, { type: TOKEN_TYPE_AUTHOR, preloadedUsers }, @@ -227,6 +231,7 @@ describe('Merge requests list app', () => { it('pre-displays tokens that are in the url search parameters', () => { expect(findIssuableList().props('initialFilterValue')).toMatchObject([ { type: TOKEN_TYPE_APPROVED_BY }, + { type: TOKEN_TYPE_APPROVER }, { type: TOKEN_TYPE_ASSIGNEE }, { type: TOKEN_TYPE_REVIEWER }, { type: TOKEN_TYPE_DRAFT }, -- GitLab From 1d31a481767aa67cfd7a80db4db62c95c44afc9a Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Wed, 4 Sep 2024 12:09:09 -0600 Subject: [PATCH 3/6] Update to use the back end name --- .../list/queries/get_merge_requests.query.graphql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/merge_requests/list/queries/get_merge_requests.query.graphql b/app/assets/javascripts/merge_requests/list/queries/get_merge_requests.query.graphql index 5e44c857690630..c8938c34c89afb 100644 --- a/app/assets/javascripts/merge_requests/list/queries/get_merge_requests.query.graphql +++ b/app/assets/javascripts/merge_requests/list/queries/get_merge_requests.query.graphql @@ -37,7 +37,7 @@ query getMergeRequests( sort: $sort state: $state approvedBy: $approvedBy - approver: $approver + approverUsernames: $approver assigneeUsername: $assigneeUsernames assigneeWildcardId: $assigneeWildcardId reviewerUsername: $reviewerUsername -- GitLab From 8d6c0fc469dcc0cbe1e9ed82cf16aacb6db2b27a Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Wed, 4 Sep 2024 12:10:11 -0600 Subject: [PATCH 4/6] Remove unnecessary localization --- .../vue_shared/components/filtered_search_bar/constants.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/vue_shared/components/filtered_search_bar/constants.js b/app/assets/javascripts/vue_shared/components/filtered_search_bar/constants.js index 846a377ac2892c..a9a59f240d9176 100644 --- a/app/assets/javascripts/vue_shared/components/filtered_search_bar/constants.js +++ b/app/assets/javascripts/vue_shared/components/filtered_search_bar/constants.js @@ -91,7 +91,7 @@ export const TOKEN_TITLE_DEPLOYED_BEFORE = __('Deployed-before'); export const TOKEN_TITLE_DEPLOYED_AFTER = __('Deployed-after'); export const TOKEN_TITLE_ASSIGNED_SEAT = __('Assigned seat'); -export const TOKEN_TYPE_APPROVER = __('approver'); +export const TOKEN_TYPE_APPROVER = 'approver'; export const TOKEN_TYPE_APPROVED_BY = 'approved-by'; export const TOKEN_TYPE_MERGE_USER = 'merge-user'; export const TOKEN_TYPE_ASSIGNEE = 'assignee'; -- GitLab From e7a61d9ccde803f111514c2178ba8d5802ffb394 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Wed, 4 Sep 2024 12:38:32 -0600 Subject: [PATCH 5/6] Fix GQL parameter name and value identifier (array, no nulls) --- .../list/queries/get_merge_requests.query.graphql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/merge_requests/list/queries/get_merge_requests.query.graphql b/app/assets/javascripts/merge_requests/list/queries/get_merge_requests.query.graphql index c8938c34c89afb..26fbc1ddb80727 100644 --- a/app/assets/javascripts/merge_requests/list/queries/get_merge_requests.query.graphql +++ b/app/assets/javascripts/merge_requests/list/queries/get_merge_requests.query.graphql @@ -8,7 +8,7 @@ query getMergeRequests( $sort: MergeRequestSort $state: MergeRequestState $approvedBy: [String!] - $approver: String + $approver: [String!] $assigneeUsernames: String $assigneeWildcardId: AssigneeWildcardId $reviewerUsername: String @@ -37,7 +37,7 @@ query getMergeRequests( sort: $sort state: $state approvedBy: $approvedBy - approverUsernames: $approver + approver: $approver assigneeUsername: $assigneeUsernames assigneeWildcardId: $assigneeWildcardId reviewerUsername: $reviewerUsername -- GitLab From 8c51863636ddb0a4df267d86a7e656673ae43552 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Tue, 24 Sep 2024 16:16:18 -0600 Subject: [PATCH 6/6] Make the Approver filter non-unique --- app/assets/javascripts/issues/list/constants.js | 2 +- .../merge_requests/list/components/merge_requests_list_app.vue | 1 - .../list/components/merge_requests_list_app_spec.js | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/issues/list/constants.js b/app/assets/javascripts/issues/list/constants.js index 0d434a10851bb2..ebeb21f3791068 100644 --- a/app/assets/javascripts/issues/list/constants.js +++ b/app/assets/javascripts/issues/list/constants.js @@ -173,7 +173,7 @@ export const filtersMap = { }, [URL_PARAM]: { [OPERATOR_IS]: { - [NORMAL_FILTER]: 'approver', + [NORMAL_FILTER]: 'approver[]', }, }, }, diff --git a/app/assets/javascripts/merge_requests/list/components/merge_requests_list_app.vue b/app/assets/javascripts/merge_requests/list/components/merge_requests_list_app.vue index 985699d869860a..2c869a2609a5bf 100644 --- a/app/assets/javascripts/merge_requests/list/components/merge_requests_list_app.vue +++ b/app/assets/javascripts/merge_requests/list/components/merge_requests_list_app.vue @@ -235,7 +235,6 @@ export default { recentSuggestionsStorageKey: `${this.fullPath}-merge_requests-recent-tokens-approvers`, preloadedUsers, multiSelect: false, - unique: true, }, { type: TOKEN_TYPE_ASSIGNEE, diff --git a/spec/frontend/merge_requests/list/components/merge_requests_list_app_spec.js b/spec/frontend/merge_requests/list/components/merge_requests_list_app_spec.js index 880e6232f10fef..5ad5b8af29a507 100644 --- a/spec/frontend/merge_requests/list/components/merge_requests_list_app_spec.js +++ b/spec/frontend/merge_requests/list/components/merge_requests_list_app_spec.js @@ -173,7 +173,7 @@ describe('Merge requests list app', () => { describe('when all tokens are available', () => { const urlParams = { 'approved_by_usernames[]': 'anthony', - approver: 'angus', + 'approver[]': 'angus', assignee_username: 'bob', reviewer_username: 'bill', draft: 'yes', -- GitLab