From 71ea84e06fa0f37ab7caa2ff259f87b81cb6934c Mon Sep 17 00:00:00 2001 From: Coung Ngo <cngo@gitlab.com> Date: Tue, 4 Mar 2025 09:28:52 +0000 Subject: [PATCH] Fix group filter on work items list When filtering by group on the work items list, the UI currently shows all items (epics, issues, etc) on the selected group as well as its subgroups. This behaviour is incorrect. This commit fixes this behaviour so that the UI only shows items directly associated with the group (epics) and the group only (not its subgroups) Changelog: fixed --- .../get_work_item_state_counts.query.graphql | 4 +- .../graphql/list/get_work_items.query.graphql | 7 +-- .../work_items/pages/work_items_list_app.vue | 5 ++- .../work_items_list_filters_spec.rb | 36 +++++++++++++++- .../work_items_list_filters_spec.rb | 13 +----- .../components/work_items_list_app_spec.js | 43 +++++++++++++++++++ 6 files changed, 88 insertions(+), 20 deletions(-) diff --git a/app/assets/javascripts/work_items/graphql/list/get_work_item_state_counts.query.graphql b/app/assets/javascripts/work_items/graphql/list/get_work_item_state_counts.query.graphql index fdb4b9d00f557..0f842a4777a6b 100644 --- a/app/assets/javascripts/work_items/graphql/list/get_work_item_state_counts.query.graphql +++ b/app/assets/javascripts/work_items/graphql/list/get_work_item_state_counts.query.graphql @@ -1,5 +1,7 @@ query getWorkItemStateCounts( + $excludeProjects: Boolean = false $includeDescendants: Boolean = true + $isGroup: Boolean = true $fullPath: ID! $search: String $sort: WorkItemSort @@ -16,11 +18,11 @@ query getWorkItemStateCounts( $in: [IssuableSearchableField!] $not: NegatedWorkItemFilterInput $or: UnionedWorkItemFilterInput - $isGroup: Boolean = true ) { group(fullPath: $fullPath) @include(if: $isGroup) { id workItemStateCounts( + excludeProjects: $excludeProjects includeDescendants: $includeDescendants search: $search sort: $sort diff --git a/app/assets/javascripts/work_items/graphql/list/get_work_items.query.graphql b/app/assets/javascripts/work_items/graphql/list/get_work_items.query.graphql index 5a76a0f8c0a72..1f2b0f98c5d82 100644 --- a/app/assets/javascripts/work_items/graphql/list/get_work_items.query.graphql +++ b/app/assets/javascripts/work_items/graphql/list/get_work_items.query.graphql @@ -2,6 +2,9 @@ #import "ee_else_ce/work_items/graphql/list/work_item_widgets.fragment.graphql" query getWorkItems( + $excludeProjects: Boolean = false + $includeDescendants: Boolean = true + $isGroup: Boolean = true $fullPath: ID! $search: String $sort: WorkItemSort @@ -22,15 +25,13 @@ query getWorkItems( $beforeCursor: String $firstPageSize: Int $lastPageSize: Int - $isGroup: Boolean = true - $excludeProjects: Boolean ) { group(fullPath: $fullPath) @include(if: $isGroup) { id name workItems( excludeProjects: $excludeProjects - includeDescendants: true + includeDescendants: $includeDescendants search: $search sort: $sort state: $state diff --git a/app/assets/javascripts/work_items/pages/work_items_list_app.vue b/app/assets/javascripts/work_items/pages/work_items_list_app.vue index 4e5a5e2e7eaec..8e703bdf75461 100644 --- a/app/assets/javascripts/work_items/pages/work_items_list_app.vue +++ b/app/assets/javascripts/work_items/pages/work_items_list_app.vue @@ -260,6 +260,7 @@ export default { return this.isGroup ? WORKSPACE_GROUP : WORKSPACE_PROJECT; }, queryVariables() { + const hasGroupFilter = Boolean(this.urlFilterParams.group_path); return { fullPath: this.fullPath, sort: this.sortKey, @@ -267,8 +268,8 @@ export default { search: this.searchQuery, ...this.apiFilterParams, ...this.pageParams, - excludeProjects: this.isEpicsList, - includeDescendants: !this.apiFilterParams.fullPath, + excludeProjects: hasGroupFilter || this.isEpicsList, + includeDescendants: !hasGroupFilter, types: this.apiFilterParams.types || this.workItemType || this.defaultWorkItemTypes, isGroup: this.isGroup, }; diff --git a/ee/spec/features/work_items/work_items_list_filters_spec.rb b/ee/spec/features/work_items/work_items_list_filters_spec.rb index a54284edbeda9..67e5854b2fb58 100644 --- a/ee/spec/features/work_items/work_items_list_filters_spec.rb +++ b/ee/spec/features/work_items/work_items_list_filters_spec.rb @@ -9,8 +9,16 @@ let_it_be(:user) { create(:user) } let_it_be(:group) { create(:group) } + let_it_be(:sub_group) { create(:group, parent: group) } + let_it_be(:sub_group_project) { create(:project, :public, group: sub_group, developers: user) } + let_it_be(:sub_sub_group) { create(:group, parent: sub_group) } let_it_be(:project) { create(:project, :public, group: group, developers: user) } + let_it_be(:epic) { create(:work_item, :epic_with_legacy_epic, namespace: group) } + let_it_be(:sub_epic) { create(:work_item, :epic_with_legacy_epic, namespace: sub_group) } + let_it_be(:sub_issue) { create(:issue, project: sub_group_project) } + let_it_be(:sub_sub_epic) { create(:work_item, :epic_with_legacy_epic, namespace: sub_sub_group) } + let_it_be(:incident) { create(:incident, project: project) } let_it_be(:issue) { create(:issue, project: project) } let_it_be(:task) { create(:work_item, :task, project: project) } @@ -18,19 +26,43 @@ context 'for signed in user' do before do - stub_licensed_features(quality_management: true) + stub_licensed_features(epics: true, quality_management: true, subepics: true) sign_in(user) visit group_work_items_path(group) close_work_item_feedback_popover_if_present end + describe 'group' do + it 'filters', :aggregate_failures do + select_tokens 'Group', group.name, submit: true + + expect(page).to have_css('.issue', count: 1) + expect(page).to have_link(epic.title) + + click_button 'Clear' + + select_tokens 'Group', sub_group.name, submit: true + + expect(page).to have_css('.issue', count: 1) + expect(page).to have_link(sub_epic.title) + + click_button 'Clear' + + select_tokens 'Group', sub_sub_group.name, submit: true + + expect(page).to have_css('.issue', count: 1) + expect(page).to have_link(sub_sub_epic.title) + end + end + describe 'type' do it 'filters', :aggregate_failures do select_tokens 'Type', 'Issue', submit: true - expect(page).to have_css('.issue', count: 1) + expect(page).to have_css('.issue', count: 2) expect(page).to have_link(issue.title) + expect(page).to have_link(sub_issue.title) click_button 'Clear' diff --git a/spec/features/work_items/work_items_list_filters_spec.rb b/spec/features/work_items/work_items_list_filters_spec.rb index 262826a6c21b6..383556b6518e0 100644 --- a/spec/features/work_items/work_items_list_filters_spec.rb +++ b/spec/features/work_items/work_items_list_filters_spec.rb @@ -10,9 +10,7 @@ let_it_be(:user2) { create(:user) } let_it_be(:group) { create(:group) } - let_it_be(:sub_group) { create(:group, parent: group) } let_it_be(:project) { create(:project, :public, group: group, developers: [user1, user2]) } - let_it_be(:sub_group_project) { create(:project, :public, group: sub_group, developers: [user1, user2]) } let_it_be(:label1) { create(:label, project: project) } let_it_be(:label2) { create(:label, project: project) } @@ -37,7 +35,7 @@ end let_it_be(:task) do - create(:work_item, :task, project: sub_group_project, + create(:work_item, :task, project: project, assignees: [user2], author: user2, confidential: true, @@ -137,15 +135,6 @@ end end - describe 'group' do - it 'filters', :aggregate_failures do - select_tokens 'Group', sub_group.name, submit: true - - expect(page).to have_css('.issue', count: 1) - expect(page).to have_link(task.title) - end - end - describe 'label' do it 'filters', :aggregate_failures do select_tokens 'Label', '=', label1.title, submit: true diff --git a/spec/frontend/work_items/list/components/work_items_list_app_spec.js b/spec/frontend/work_items/list/components/work_items_list_app_spec.js index 55752f9c421b2..edeee4a3bad4c 100644 --- a/spec/frontend/work_items/list/components/work_items_list_app_spec.js +++ b/spec/frontend/work_items/list/components/work_items_list_app_spec.js @@ -749,4 +749,47 @@ describeSkipVue3(skipReason, () => { expect(findIssuableList().props('issuables')).toEqual([]); }); }); + + describe('group filter', () => { + describe('filtering by group', () => { + it('query excludes descendants and excludes projects', async () => { + mountComponent(); + await waitForPromises(); + + findIssuableList().vm.$emit('filter', [ + { + type: TOKEN_TYPE_GROUP, + value: { data: 'path/to/another/group', operator: OPERATOR_IS }, + }, + ]); + await nextTick(); + + expect(defaultQueryHandler).toHaveBeenCalledWith( + expect.objectContaining({ + excludeProjects: true, + includeDescendants: false, + }), + ); + }); + }); + + describe('not filtering by group', () => { + it('query includes descendants and includes projects', async () => { + mountComponent(); + await waitForPromises(); + + findIssuableList().vm.$emit('filter', [ + { type: TOKEN_TYPE_AUTHOR, value: { data: 'homer', operator: OPERATOR_IS } }, + ]); + await nextTick(); + + expect(defaultQueryHandler).toHaveBeenCalledWith( + expect.objectContaining({ + excludeProjects: false, + includeDescendants: true, + }), + ); + }); + }); + }); }); -- GitLab