From 9fe5d8030d331d7c05524da7744b344a70317ad2 Mon Sep 17 00:00:00 2001 From: manojmj Date: Tue, 2 Jul 2019 11:38:44 +0530 Subject: [PATCH] Allow subgroups to use their parent group's custom project templates This change allows subgroups to use their parent group's custom project templates. --- doc/user/group/custom_project_templates.md | 2 +- ee/app/models/ee/group.rb | 4 +- ee/app/services/ee/projects/create_service.rb | 8 +- ...archy-of-the-group-owning-the-template.yml | 5 + ee/spec/models/group_spec.rb | 15 ++- .../create_from_template_service_spec.rb | 91 +++++++++++++++---- locale/gitlab.pot | 6 +- 7 files changed, 93 insertions(+), 38 deletions(-) create mode 100644 ee/changelogs/unreleased/9768-allow-a-template-in-a-namespace-which-is-out-of-the-hierarchy-of-the-group-owning-the-template.yml diff --git a/doc/user/group/custom_project_templates.md b/doc/user/group/custom_project_templates.md index 8a85c375490821..7cdba8cf2b53c5 100644 --- a/doc/user/group/custom_project_templates.md +++ b/doc/user/group/custom_project_templates.md @@ -24,7 +24,7 @@ project in the group will be available to every logged in user. However, private projects will be available only if the user is a member of the project. NOTE: **Note:** -Projects of nested subgroups of a selected template source cannot be used. +Only direct subgroups can be set as the template source. Projects of nested subgroups of a selected template source cannot be used. Repository and database information that are copied over to each new project are identical to the data exported with [GitLab's Project Import/Export](../project/settings/import_export.md). diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb index 61f70c88b605f2..a343a949e820b6 100644 --- a/ee/app/models/ee/group.rb +++ b/ee/app/models/ee/group.rb @@ -217,9 +217,9 @@ def dependency_proxy_feature_available? def custom_project_templates_group_allowed return if custom_project_templates_group_id.blank? - return if descendants.exists?(id: custom_project_templates_group_id) + return if children.exists?(id: custom_project_templates_group_id) - errors.add(:custom_project_templates_group_id, "has to be a descendant of the group") + errors.add(:custom_project_templates_group_id, "has to be a subgroup of the group") end end end diff --git a/ee/app/services/ee/projects/create_service.rb b/ee/app/services/ee/projects/create_service.rb index 92cdd4dc6065e0..8fb1cdde7bf67b 100644 --- a/ee/app/services/ee/projects/create_service.rb +++ b/ee/app/services/ee/projects/create_service.rb @@ -65,17 +65,17 @@ def create_predefined_push_rule end # When using a project template from a Group, the new project can only be created - # under the top level group or any subgroup + # under the template owner's group or subgroups def validate_namespace_used_with_template(project, group_with_project_templates_id) return unless project.group subgroup_with_templates_id = group_with_project_templates_id || params[:group_with_project_templates_id] return if subgroup_with_templates_id.blank? - templates_owner = ::Group.find(subgroup_with_templates_id) + templates_owner = ::Group.find(subgroup_with_templates_id).parent - unless templates_owner.self_and_hierarchy.exists?(id: project.namespace_id) - project.errors.add(:namespace, _("is out of the hierarchy of the Group owning the template")) + unless templates_owner.self_and_descendants.exists?(id: project.namespace_id) + project.errors.add(:namespace, _("is not a descendant of the Group owning the template")) end end # rubocop: enable CodeReuse/ActiveRecord diff --git a/ee/changelogs/unreleased/9768-allow-a-template-in-a-namespace-which-is-out-of-the-hierarchy-of-the-group-owning-the-template.yml b/ee/changelogs/unreleased/9768-allow-a-template-in-a-namespace-which-is-out-of-the-hierarchy-of-the-group-owning-the-template.yml new file mode 100644 index 00000000000000..8534198c1a162b --- /dev/null +++ b/ee/changelogs/unreleased/9768-allow-a-template-in-a-namespace-which-is-out-of-the-hierarchy-of-the-group-owning-the-template.yml @@ -0,0 +1,5 @@ +--- +title: Allow subgroups to use their parent group's custom project templates +merge_request: 14499 +author: +type: fixed diff --git a/ee/spec/models/group_spec.rb b/ee/spec/models/group_spec.rb index efc29195a23740..f2e923cb9ecdf6 100644 --- a/ee/spec/models/group_spec.rb +++ b/ee/spec/models/group_spec.rb @@ -49,25 +49,24 @@ context 'validates if custom_project_templates_group_id is allowed' do let(:subgroup_1) { create(:group, parent: group) } - it 'rejects change if the assigned group is not a descendant' do + it 'rejects change if the assigned group is not a subgroup' do group.custom_project_templates_group_id = create(:group).id expect(group).not_to be_valid - expect(group.errors.messages[:custom_project_templates_group_id]).to eq ['has to be a descendant of the group'] + expect(group.errors.messages[:custom_project_templates_group_id]).to eq ['has to be a subgroup of the group'] end - it 'allows value if the current group is a top parent and the value is from a descendant' do - subgroup = create(:group, parent: group) - group.custom_project_templates_group_id = subgroup.id + it 'allows value if the assigned value is from a subgroup' do + group.custom_project_templates_group_id = subgroup_1.id expect(group).to be_valid end - it 'allows value if the current group is a subgroup and the value is from a descendant' do + it 'rejects change if the assigned value is from a subgroup\'s descendant group' do subgroup_1_1 = create(:group, parent: subgroup_1) - subgroup_1.custom_project_templates_group_id = subgroup_1_1.id + group.custom_project_templates_group_id = subgroup_1_1.id - expect(group).to be_valid + expect(group).not_to be_valid end it 'allows value when it is blank' do diff --git a/ee/spec/services/projects/create_from_template_service_spec.rb b/ee/spec/services/projects/create_from_template_service_spec.rb index 5a580d42efd3a1..8b2d3bfee7a34d 100644 --- a/ee/spec/services/projects/create_from_template_service_spec.rb +++ b/ee/spec/services/projects/create_from_template_service_spec.rb @@ -1,5 +1,18 @@ require 'spec_helper' +# Group Hierarchy: + +# Group +# Subgroup 1 +# Subgroup 1_1 +# Subgroup 1_1_1 +# Subgroup 1_2 (the group with the template) +# project_template +# Subgroup_1_2_1 +# Subgroup 2 +# Subgroup 2_1 +# Group 2 + describe Projects::CreateFromTemplateService do let(:group) { create(:group) } let(:project) { create(:project, :public, namespace: group) } @@ -9,7 +22,12 @@ let(:subgroup_1) { create(:group, parent: group) } let(:subgroup_1_1) { create(:group, parent: subgroup_1) } - let(:project_template) { create(:project, :public, namespace: subgroup_1) } + let(:subgroup_1_1_1) { create(:group, parent: subgroup_1_1) } + let(:subgroup_1_2) { create(:group, parent: subgroup_1) } + let(:subgroup_1_2_1) { create(:group, parent: subgroup_1_2) } + let(:subgroup_2) { create(:group, parent: group) } + let(:subgroup_2_1) { create(:group, parent: subgroup_2) } + let(:project_template) { create(:project, :public, namespace: subgroup_1_2) } let(:namespace_id) { nil } let(:group_with_project_templates_id) { nil } @@ -97,17 +115,25 @@ describe 'creating project from a Group project template', :postgresql do let(:project_name) { project_template.name } - let(:group_with_project_templates_id) { subgroup_1.id } + let(:group_with_project_templates_id) { subgroup_1_2.id } let(:group2) { create(:group) } before do + subgroup_1.update!(custom_project_templates_group_id: subgroup_1_2.id) group.add_maintainer(user) group2.add_maintainer(user) end - context "when the namespace is out of the hierarchy of the Group's owning the template" do - let(:namespace_id) { group2.id } + shared_examples 'a persisted project' do + it "is persisted" do + project = subject.execute + + expect(project).to be_saved + expect(project.import_scheduled?).to be(true) + end + end + shared_examples 'a project that isn\'t persisted' do it "isn't persisted" do project = subject.execute @@ -116,31 +142,56 @@ end end - shared_examples 'a persisted project' do - it "is persisted" do - project = subject.execute + context 'when the namespace is not a descendant of the Group owning the template' do + context 'when project is created under a group that is outside the hierarchy its root ancestor group' do + let(:namespace_id) { group2.id } - expect(project).to be_saved - expect(project.import_scheduled?).to be(true) + it_behaves_like 'a project that isn\'t persisted' end - end - context 'when project is created under a top level group' do - let(:namespace_id) { group.id } + context 'when project is created under a group that is a descendant of its root ancestor group' do + let(:namespace_id) { subgroup_2.id } - it_behaves_like 'a persisted project' - end + it_behaves_like 'a project that isn\'t persisted' + end - context 'when project is created under a subgroup' do - let(:namespace_id) { subgroup_1.id } + context 'when project is created under a subgroup that is a descendant of its root ancestor group' do + let(:namespace_id) { subgroup_2_1.id } - it_behaves_like 'a persisted project' + it_behaves_like 'a project that isn\'t persisted' + end end - context 'when project is created under a nested subgroup' do - let(:namespace_id) { subgroup_1_1.id } + context 'when the namespace is inside the hierarchy of the Group owning the template' do + context 'when project is created under its parent group' do + let(:namespace_id) { subgroup_1.id } + + it_behaves_like 'a persisted project' + end + + context 'when project is created under the same group' do + let(:namespace_id) { subgroup_1_2.id } + + it_behaves_like 'a persisted project' + end + + context 'when project is created under its descendant group' do + let(:namespace_id) { subgroup_1_2_1.id } - it_behaves_like 'a persisted project' + it_behaves_like 'a persisted project' + end + + context 'when project is created under a group that is a descendant of its parent group' do + let(:namespace_id) { subgroup_1_1.id } + + it_behaves_like 'a persisted project' + end + + context 'when project is created under a subgroup that is a descendant of its parent group' do + let(:namespace_id) { subgroup_1_1_1.id } + + it_behaves_like 'a persisted project' + end end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 4db5fc227a4283..2929f12eca99ee 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -16980,13 +16980,13 @@ msgstr "" msgid "is invalid because there is upstream lock" msgstr "" -msgid "is not a valid X509 certificate." +msgid "is not a descendant of the Group owning the template" msgstr "" -msgid "is not an email you own" +msgid "is not a valid X509 certificate." msgstr "" -msgid "is out of the hierarchy of the Group owning the template" +msgid "is not an email you own" msgstr "" msgid "issue" -- GitLab