diff --git a/ee/app/helpers/ee/projects_helper.rb b/ee/app/helpers/ee/projects_helper.rb index 69418622ceb0ec62decdba3f54ebab2ee8b06d0c..200158524915a467a5c8c1f24c31c33c2f6f774d 100644 --- a/ee/app/helpers/ee/projects_helper.rb +++ b/ee/app/helpers/ee/projects_helper.rb @@ -167,9 +167,15 @@ def membership_locked? end def group_project_templates_count(group_id) - allowed_subgroups = current_user.available_subgroups_with_custom_project_templates(group_id) + projects_not_aimed_for_deletions_for(group_id).count do |project| + can?(current_user, :download_code, project) + end + end - ::Project.in_namespace(allowed_subgroups).not_aimed_for_deletion.count + def group_project_templates(group_id) + projects_not_aimed_for_deletions_for(group_id).select do |project| + can?(current_user, :download_code, project) + end end def project_security_dashboard_config(project) @@ -330,5 +336,13 @@ def security_dashboard_pipeline_data(project) pipelines end + + def allowed_subgroups(group_id) + current_user.available_subgroups_with_custom_project_templates(group_id) + end + + def projects_not_aimed_for_deletions_for(group_id) + ::Project.in_namespace(allowed_subgroups(group_id)).not_aimed_for_deletion + end end end diff --git a/ee/app/models/ee/user.rb b/ee/app/models/ee/user.rb index f44861a65d3e1c8a3d6bcffdd19a4e38c07469af..9c7f13305d4b93641813899a179688fcbc2b9452 100644 --- a/ee/app/models/ee/user.rb +++ b/ee/app/models/ee/user.rb @@ -294,7 +294,7 @@ def search_membership_ancestry def available_subgroups_with_custom_project_templates(group_id = nil) found_groups = GroupsWithTemplatesFinder.new(self, group_id).execute - ::GroupsFinder.new(self, min_access_level: ::Gitlab::Access::DEVELOPER) + ::GroupsFinder.new(self) .execute .where(id: found_groups.select(:custom_project_templates_group_id)) .preload(:projects) diff --git a/ee/app/views/users/available_group_templates.html.haml b/ee/app/views/users/available_group_templates.html.haml index 413d2dea16119b715eb0d0f5e4128d6f499e9226..94e818252f5b75d181e82990c4ef28730bf4aefc 100644 --- a/ee/app/views/users/available_group_templates.html.haml +++ b/ee/app/views/users/available_group_templates.html.haml @@ -1,7 +1,7 @@ .custom-project-templates - if @groups_with_project_templates.present? - @groups_with_project_templates.each do |group| - - projects = group.projects.not_aimed_for_deletion + - projects = group_project_templates(group.id) .template-group-options.js-template-group-options{ class: ('expanded border-top-0' if @groups_with_project_templates.first == group) } .template-header.d-flex.align-items-center .template-subgroup.d-flex.flex-fill.align-items-center diff --git a/ee/spec/helpers/projects_helper_spec.rb b/ee/spec/helpers/projects_helper_spec.rb index e4d19fcf63ba9ff6888fda329e28d4a408157ac7..067e8aac3c85242264d7884ce7e7b77cb5af5d22 100644 --- a/ee/spec/helpers/projects_helper_spec.rb +++ b/ee/spec/helpers/projects_helper_spec.rb @@ -147,6 +147,7 @@ before do allow(helper).to receive(:current_user).and_return(user) + allow(helper).to receive(:can?).and_return(true) end specify do @@ -162,6 +163,57 @@ expect(helper.group_project_templates_count(parent_group.id)).to eq 0 end end + + context 'when project is not visible to user' do + before do + allow(helper).to receive(:can?).with(user, :download_code, template_project).and_return(false) + end + + specify do + expect(helper.group_project_templates_count(parent_group.id)).to eq 0 + end + end + end + + describe '#group_project_templates' do + let_it_be(:user) { create(:user) } + let_it_be(:parent_group) { create(:group, name: 'parent-group') } + let_it_be(:template_group) { create(:group, parent: parent_group, name: 'template-group') } + let_it_be(:template_project) { create(:project, group: template_group, name: 'template-project') } + + before_all do + parent_group.update!(custom_project_templates_group_id: template_group.id) + parent_group.add_owner(user) + end + + before do + allow(helper).to receive(:current_user).and_return(user) + allow(helper).to receive(:can?).and_return(true) + end + + specify do + expect(helper.group_project_templates(parent_group)).to match_array([template_project]) + end + + context 'when template project is pending deletion' do + before do + template_project.update!(marked_for_deletion_at: Date.current) + end + + specify do + expect(helper.group_project_templates(parent_group)).to be_empty + end + end + + context 'when project is not visible to user' do + before do + allow(helper).to receive(:can?).and_return(false) + end + + specify do + expect(helper.group_project_templates(parent_group)).to be_empty + end + end end describe '#project_security_dashboard_config' do diff --git a/ee/spec/models/ee/user_spec.rb b/ee/spec/models/ee/user_spec.rb index b9dfc7ee8df6da74ec61b3869968502441219840..7ee31ee09f8a17fd4e9e4fc57d5e107b219c6eee 100644 --- a/ee/spec/models/ee/user_spec.rb +++ b/ee/spec/models/ee/user_spec.rb @@ -758,12 +758,12 @@ context 'with Groups with custom project templates' do let!(:group_1) { create(:group, name: 'group-1') } - let!(:group_2) { create(:group, name: 'group-2') } + let!(:group_2) { create(:group, :private, name: 'group-2') } let!(:group_3) { create(:group, name: 'group-3') } let!(:group_4) { create(:group, name: 'group-4') } let!(:subgroup_1) { create(:group, parent: group_1, name: 'subgroup-1') } - let!(:subgroup_2) { create(:group, parent: group_2, name: 'subgroup-2') } + let!(:subgroup_2) { create(:group, :private, parent: group_2, name: 'subgroup-2') } let!(:subgroup_3) { create(:group, parent: group_3, name: 'subgroup-3') } let!(:subgroup_4) { create(:group, parent: group_4, name: 'subgroup-4') } @@ -779,19 +779,38 @@ subgroup_4.update!(custom_project_templates_group_id: subsubgroup_4.id) create(:project, namespace: subgroup_1) - create(:project, namespace: subgroup_2) + create(:project, :private, namespace: subgroup_2) create(:project, namespace: subsubgroup_1) create(:project, namespace: subsubgroup_4) end - context 'when the access level of the user is below the required one' do - before do - group_1.add_reporter(user) + context 'when a user is not a member of the groups' do + subject(:available_subgroups) { user.available_subgroups_with_custom_project_templates } + + it 'only templates in publicly visible groups with projects are available' do + expect(available_subgroups).to match_array([subgroup_1, subsubgroup_1, subsubgroup_4]) + end + end + + context 'when a user is a member of the groups' do + subject(:available_subgroups) { user.available_subgroups_with_custom_project_templates } + + where(:access_level) do + [:guest, :reporter, :developer, :maintainer, :owner] end - it 'returns an empty collection' do - expect(user.available_subgroups_with_custom_project_templates).to be_empty + with_them do + before do + group_1.add_member(user, access_level) + group_2.add_member(user, access_level) + group_3.add_member(user, access_level) + group_4.add_member(user, access_level) + end + + it 'the templates in groups with projects are available' do + expect(available_subgroups).to match_array([subgroup_1, subgroup_2, subsubgroup_1, subsubgroup_4]) + end end end