From 5a903149e75465e4025f154977597aeef94b618c Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Wed, 11 Oct 2017 10:17:24 +0200 Subject: Handle archived projects in the `GroupDescendantsFinder` --- app/finders/group_descendants_finder.rb | 37 +++++++++++++++++------ app/models/concerns/loaded_in_group_list.rb | 32 +++++++++++++------- spec/finders/group_descendants_finder_spec.rb | 32 ++++++++++++++++++++ spec/models/concerns/loaded_in_group_list_spec.rb | 33 ++++++++++++++++---- 4 files changed, 108 insertions(+), 26 deletions(-) diff --git a/app/finders/group_descendants_finder.rb b/app/finders/group_descendants_finder.rb index ea7974eb882..7ceede3a31a 100644 --- a/app/finders/group_descendants_finder.rb +++ b/app/finders/group_descendants_finder.rb @@ -1,10 +1,29 @@ +# GroupDescendantsFinder +# +# Used to find and filter all subgroups and projects of a passed parent group +# visible to a specified user. +# +# When passing a `filter` param, the search is performed over all nested levels +# of the `parent_group`. All ancestors for a search result are loaded +# +# Arguments: +# current_user: The user for which the children should be visible +# parent_group: The group to find children of +# params: +# Supports all params that the `ProjectsFinder` and `GroupProjectsFinder` +# support. +# +# filter: string - is aliased to `search` for consistency with the frontend +# archived: string - `only` or `true`. +# `non_archived` is passed to the `ProjectFinder`s if none +# was given. class GroupDescendantsFinder attr_reader :current_user, :parent_group, :params def initialize(current_user: nil, parent_group:, params: {}) @current_user = current_user @parent_group = parent_group - @params = params.reverse_merge(non_archived: true) + @params = params.reverse_merge(non_archived: params[:archived].blank?) end def execute @@ -83,7 +102,7 @@ class GroupDescendantsFinder projects_to_load_ancestors_of = projects.where.not(namespace: parent_group) groups_to_load_ancestors_of = Group.where(id: projects_to_load_ancestors_of.select(:namespace_id)) ancestors_for_groups(groups_to_load_ancestors_of) - .with_selects_for_list + .with_selects_for_list(params[:archived]) end def subgroups @@ -96,7 +115,7 @@ class GroupDescendantsFinder else direct_child_groups end - groups.with_selects_for_list.order_by(sort) + groups.with_selects_for_list(params[:archived]).order_by(sort) end def direct_child_projects @@ -104,15 +123,15 @@ class GroupDescendantsFinder .execute end - def projects_for_user - Project.public_or_visible_to_user(current_user).non_archived - end - # Finds all projects nested under `parent_group` or any of its descendant # groups def projects_matching_filter - projects_for_user.search(params[:filter]) - .where(namespace_id: hierarchy_for_parent.base_and_descendants.select(:id)) + projects_nested_in_group = Project.where(namespace_id: hierarchy_for_parent.base_and_descendants.select(:id)) + params_with_search = params.merge(search: params[:filter]) + + ProjectsFinder.new(params: params_with_search, + current_user: current_user, + project_ids_relation: projects_nested_in_group).execute end def projects diff --git a/app/models/concerns/loaded_in_group_list.rb b/app/models/concerns/loaded_in_group_list.rb index e73ddcfb567..8b519d742c5 100644 --- a/app/models/concerns/loaded_in_group_list.rb +++ b/app/models/concerns/loaded_in_group_list.rb @@ -2,15 +2,14 @@ module LoadedInGroupList extend ActiveSupport::Concern PROJECT_COUNT_SQL = <<~PROJECTCOUNT.freeze - (SELECT COUNT(*) AS preloaded_project_count - FROM projects - WHERE projects.namespace_id = namespaces.id - AND projects.archived IS NOT true) + SELECT COUNT(*) AS preloaded_project_count + FROM projects + WHERE projects.namespace_id = namespaces.id PROJECTCOUNT SUBGROUP_COUNT_SQL = <<~SUBGROUPCOUNT.freeze (SELECT COUNT(*) AS preloaded_subgroup_count - FROM namespaces children - WHERE children.parent_id = namespaces.id) + FROM namespaces children + WHERE children.parent_id = namespaces.id) SUBGROUPCOUNT MEMBER_COUNT_SQL = <<~MEMBERCOUNT.freeze (SELECT COUNT(*) AS preloaded_member_count @@ -21,17 +20,28 @@ module LoadedInGroupList MEMBERCOUNT COUNT_SELECTS = ['namespaces.*', - PROJECT_COUNT_SQL, SUBGROUP_COUNT_SQL, MEMBER_COUNT_SQL].freeze module ClassMethods - def with_counts - select(COUNT_SELECTS) + def with_counts(archived = nil) + selects = COUNT_SELECTS.dup << project_count(archived) + select(selects) end - def with_selects_for_list - with_route.with_counts + def with_selects_for_list(archived = nil) + with_route.with_counts(archived) + end + + def project_count(archived) + project_count = if archived == 'only' + PROJECT_COUNT_SQL + 'AND projects.archived IS true' + elsif Gitlab::Utils.to_boolean(archived) + PROJECT_COUNT_SQL + else + PROJECT_COUNT_SQL + 'AND projects.archived IS NOT true' + end + "(#{project_count})" end end diff --git a/spec/finders/group_descendants_finder_spec.rb b/spec/finders/group_descendants_finder_spec.rb index 4a5bdd84508..074914420a1 100644 --- a/spec/finders/group_descendants_finder_spec.rb +++ b/spec/finders/group_descendants_finder_spec.rb @@ -35,6 +35,28 @@ describe GroupDescendantsFinder do expect(finder.execute).to contain_exactly(project) end + context 'when archived is `true`' do + let(:params) { { archived: 'true' } } + + it 'includes archived projects' do + archived_project = create(:project, namespace: group, archived: true) + project = create(:project, namespace: group) + + expect(finder.execute).to contain_exactly(archived_project, project) + end + end + + context 'when archived is `only`' do + let(:params) { { archived: 'only' } } + + it 'includes only archived projects' do + archived_project = create(:project, namespace: group, archived: true) + _project = create(:project, namespace: group) + + expect(finder.execute).to contain_exactly(archived_project) + end + end + it 'does not include archived projects' do _archived_project = create(:project, :archived, namespace: group) @@ -84,6 +106,16 @@ describe GroupDescendantsFinder do expect(finder.execute).to contain_exactly(public_subgroup) end + context 'when archived is `true`' do + let(:params) { { archived: 'true' } } + + it 'includes archived projects in the count of subgroups' do + create(:project, namespace: subgroup, archived: true) + + expect(finder.execute.first.preloaded_project_count).to eq(1) + end + end + context 'with a filter' do let(:params) { { filter: 'test' } } diff --git a/spec/models/concerns/loaded_in_group_list_spec.rb b/spec/models/concerns/loaded_in_group_list_spec.rb index d64b288aa0c..bf5bfaa76de 100644 --- a/spec/models/concerns/loaded_in_group_list_spec.rb +++ b/spec/models/concerns/loaded_in_group_list_spec.rb @@ -4,24 +4,45 @@ describe LoadedInGroupList do let(:parent) { create(:group) } subject(:found_group) { Group.with_selects_for_list.find_by(id: parent.id) } - before do - create(:group, parent: parent) - create(:project, namespace: parent) - parent.add_developer(create(:user)) - end - describe '.with_selects_for_list' do it 'includes the preloaded counts for groups' do + create(:group, parent: parent) + create(:project, namespace: parent) + parent.add_developer(create(:user)) + found_group = Group.with_selects_for_list.find_by(id: parent.id) expect(found_group.preloaded_project_count).to eq(1) expect(found_group.preloaded_subgroup_count).to eq(1) expect(found_group.preloaded_member_count).to eq(1) end + + context 'with archived projects' do + it 'counts including archived projects when `true` is passed' do + create(:project, namespace: parent, archived: true) + create(:project, namespace: parent) + + found_group = Group.with_selects_for_list('true').find_by(id: parent.id) + + expect(found_group.preloaded_project_count).to eq(2) + end + + it 'counts only archived projects when `only` is passed' do + create_list(:project, 2, namespace: parent, archived: true) + create(:project, namespace: parent) + + found_group = Group.with_selects_for_list('only').find_by(id: parent.id) + + expect(found_group.preloaded_project_count).to eq(2) + end + end end describe '#children_count' do it 'counts groups and projects' do + create(:group, parent: parent) + create(:project, namespace: parent) + expect(found_group.children_count).to eq(2) end end -- cgit v1.2.1