Merge branch...

Merge branch '9768-allow-a-template-in-a-namespace-which-is-out-of-the-hierarchy-of-the-group-owning-the-template' into 'master'

Resolve "Allow a template in a namespace which is out of the hierarchy of the Group owning the template"

Closes #9768

See merge request gitlab-org/gitlab-ee!14499

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).

ee/app/models/ee/group.rb

@@ -217,9 +217,9 @@ module EE
 
     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

ee/app/services/ee/projects/create_service.rb

@@ -65,17 +65,17 @@ module EE
       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

ee/changelogs/unreleased/9768-allow-a-template-in-a-namespace-which-is-out-of-the-hierarchy-of-the-group-owning-the-template.yml

+---
+title: Allow subgroups to use their parent group's custom project templates
+merge_request: 14499
+author:
+type: fixed

ee/spec/models/group_spec.rb

@@ -49,25 +49,24 @@ describe Group do
     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

ee/spec/services/projects/create_from_template_service_spec.rb

 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 @@ describe Projects::CreateFromTemplateService do
 
   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 Projects::CreateFromTemplateService do
 
     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 @@ describe Projects::CreateFromTemplateService do
         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

locale/gitlab.pot

@@ -16992,13 +16992,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"