Commit 3b31d854 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Merge branch 'remove_inherited_issuable_templates_ff' into 'master'

Remove inherited_issuable_templates feature flag [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!56747
parents 933ce128 f8abac34
...@@ -25,7 +25,7 @@ class Projects::TemplatesController < Projects::ApplicationController ...@@ -25,7 +25,7 @@ class Projects::TemplatesController < Projects::ApplicationController
def names def names
respond_to do |format| respond_to do |format|
format.json { render json: TemplateFinder.all_template_names_hash_or_array(project, params[:template_type].to_s) } format.json { render json: TemplateFinder.all_template_names(project, params[:template_type].to_s.pluralize) }
end end
end end
......
...@@ -21,27 +21,11 @@ class TemplateFinder ...@@ -21,27 +21,11 @@ class TemplateFinder
end end
end end
# This is temporary and will be removed once we introduce group level inherited templates and
# remove the inherited_issuable_templates FF
def all_template_names_hash_or_array(project, issuable_type)
if project.inherited_issuable_templates_enabled?
all_template_names(project, issuable_type.pluralize)
else
all_template_names_array(project, issuable_type.pluralize)
end
end
def all_template_names(project, type) def all_template_names(project, type)
return {} if !VENDORED_TEMPLATES.key?(type.to_s) && type.to_s != 'licenses' return {} if !VENDORED_TEMPLATES.key?(type.to_s) && type.to_s != 'licenses'
build(type, project).template_names build(type, project).template_names
end end
# This is for issues and merge requests description templates only.
# This will be removed once we introduce group level inherited templates and remove the inherited_issuable_templates FF
def all_template_names_array(project, type)
all_template_names(project, type).values.flatten.select { |tmpl| tmpl[:project_id] == project.id }.compact.uniq
end
end end
attr_reader :type, :project, :params attr_reader :type, :project, :params
......
...@@ -29,17 +29,12 @@ module IssuablesDescriptionTemplatesHelper ...@@ -29,17 +29,12 @@ module IssuablesDescriptionTemplatesHelper
def issuable_templates(project, issuable_type) def issuable_templates(project, issuable_type)
@template_types ||= {} @template_types ||= {}
@template_types[project.id] ||= {} @template_types[project.id] ||= {}
@template_types[project.id][issuable_type] ||= TemplateFinder.all_template_names_hash_or_array(project, issuable_type) @template_types[project.id][issuable_type] ||= TemplateFinder.all_template_names(project, issuable_type.pluralize)
end end
def issuable_templates_names(issuable) def issuable_templates_names(issuable)
all_templates = issuable_templates(ref_project, issuable.to_ability_name) all_templates = issuable_templates(ref_project, issuable.to_ability_name)
all_templates.values.flatten.map { |tpl| tpl[:name] if tpl[:project_id] == ref_project.id }.compact.uniq
if ref_project.inherited_issuable_templates_enabled?
all_templates.values.flatten.map { |tpl| tpl[:name] if tpl[:project_id] == ref_project.id }.compact.uniq
else
all_templates.map { |template| template[:name] }
end
end end
def selected_template(issuable) def selected_template(issuable)
......
...@@ -2592,10 +2592,6 @@ class Project < ApplicationRecord ...@@ -2592,10 +2592,6 @@ class Project < ApplicationRecord
Projects::GitGarbageCollectWorker Projects::GitGarbageCollectWorker
end end
def inherited_issuable_templates_enabled?
Feature.enabled?(:inherited_issuable_templates, self, default_enabled: :yaml)
end
def activity_path def activity_path
Gitlab::Routing.url_helpers.activity_project_path(self) Gitlab::Routing.url_helpers.activity_project_path(self)
end end
......
---
name: inherited_issuable_templates
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/52360
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/321247
milestone: '13.9'
type: development
group: group::project management
default_enabled: true
...@@ -106,11 +106,7 @@ instance or the project's parent groups. ...@@ -106,11 +106,7 @@ instance or the project's parent groups.
### Set instance-level description templates **(PREMIUM SELF)** ### Set instance-level description templates **(PREMIUM SELF)**
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/52360) in GitLab 13.9. > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/52360) in GitLab 13.9.
> - [Deployed behind a feature flag](../feature_flags.md), disabled by default. > - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/321247) in GitLab 14.0.
> - [Enabled by default](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56737) in GitLab 13.11.
> - Enabled by default on GitLab.com.
> - Recommended for production use.
> - For GitLab self-managed instances, GitLab administrators can opt to [disable it](#enable-or-disable-issue-and-merge-request-description-templates-at-group-and-instance-level). **(PREMIUM SELF)**
You can set a description template at the **instance level** for issues You can set a description template at the **instance level** for issues
and merge requests. and merge requests.
...@@ -132,11 +128,7 @@ Learn more about [instance template repository](../admin_area/settings/instance_ ...@@ -132,11 +128,7 @@ Learn more about [instance template repository](../admin_area/settings/instance_
### Set group-level description templates **(PREMIUM)** ### Set group-level description templates **(PREMIUM)**
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/52360) in GitLab 13.9. > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/52360) in GitLab 13.9.
> - [Deployed behind a feature flag](../feature_flags.md), disabled by default. > - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/321247) in GitLab 14.0.
> - [Enabled by default](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56737) in GitLab 13.11.
> - Enabled by default on GitLab.com.
> - Recommended for production use.
> - For GitLab self-managed instances, GitLab administrators can opt to [disable it](#enable-or-disable-issue-and-merge-request-description-templates-at-group-and-instance-level). **(PREMIUM SELF)**
With **group-level** description templates, you can store your templates in a single repository and With **group-level** description templates, you can store your templates in a single repository and
configure the group file templates setting to point to that repository. configure the group file templates setting to point to that repository.
...@@ -231,28 +223,3 @@ it's very hard to read otherwise.) ...@@ -231,28 +223,3 @@ it's very hard to read otherwise.)
/cc @project-manager /cc @project-manager
/assign @qa-tester /assign @qa-tester
``` ```
## Enable or disable issue and merge request description templates at group and instance level **(PREMIUM SELF)**
Setting issue and merge request description templates at group and instance levels
is under development but ready for production use. It is deployed behind a
feature flag that is **enabled by default**.
[GitLab administrators with access to the GitLab Rails console](../../administration/feature_flags.md)
can disable it.
To disable it:
```ruby
Feature.disable(:inherited_issuable_templates)
```
To enable it:
```ruby
Feature.enable(:inherited_issuable_templates)
```
The feature flag affects these features:
- Setting a templates project as issue and merge request description templates source at group level.
- Setting a templates project as issue and merge request description templates source at instance level.
...@@ -8,7 +8,9 @@ module EE ...@@ -8,7 +8,9 @@ module EE
dockerfiles: ::Gitlab::Template::CustomDockerfileTemplate, dockerfiles: ::Gitlab::Template::CustomDockerfileTemplate,
gitignores: ::Gitlab::Template::CustomGitignoreTemplate, gitignores: ::Gitlab::Template::CustomGitignoreTemplate,
gitlab_ci_ymls: ::Gitlab::Template::CustomGitlabCiYmlTemplate, gitlab_ci_ymls: ::Gitlab::Template::CustomGitlabCiYmlTemplate,
metrics_dashboard_ymls: ::Gitlab::Template::CustomMetricsDashboardYmlTemplate metrics_dashboard_ymls: ::Gitlab::Template::CustomMetricsDashboardYmlTemplate,
issues: ::Gitlab::Template::IssueTemplate,
merge_requests: ::Gitlab::Template::MergeRequestTemplate
).freeze ).freeze
attr_reader :custom_templates attr_reader :custom_templates
...@@ -17,8 +19,8 @@ module EE ...@@ -17,8 +19,8 @@ module EE
def initialize(type, project, *args, &blk) def initialize(type, project, *args, &blk)
super super
if custom_templates_mapping.key?(type) if CUSTOM_TEMPLATES.key?(type)
finder = custom_templates_mapping.fetch(type) finder = CUSTOM_TEMPLATES.fetch(type)
@custom_templates = ::Gitlab::CustomFileTemplates.new(finder, project) @custom_templates = ::Gitlab::CustomFileTemplates.new(finder, project)
end end
end end
...@@ -42,18 +44,5 @@ module EE ...@@ -42,18 +44,5 @@ module EE
# from ancestor group levels # from ancestor group levels
custom_templates.all_template_names.merge(super) custom_templates.all_template_names.merge(super)
end end
# This method is going to be removed once we remove the `inherited_issuable_templates` FF and
# issues and merge_requests entries will go into CUSTOM_TEMPLATES
def custom_templates_mapping
if project&.inherited_issuable_templates_enabled?
CUSTOM_TEMPLATES.merge(
issues: ::Gitlab::Template::IssueTemplate,
merge_requests: ::Gitlab::Template::MergeRequestTemplate
)
else
CUSTOM_TEMPLATES
end
end
end end
end end
...@@ -40,23 +40,8 @@ RSpec.describe 'Service Desk Setting', :js, :clean_gitlab_redis_cache do ...@@ -40,23 +40,8 @@ RSpec.describe 'Service Desk Setting', :js, :clean_gitlab_redis_cache do
end end
group.update_columns(file_template_project_id: group_template_repo.id) group.update_columns(file_template_project_id: group_template_repo.id)
visit edit_project_path(project)
end end
context 'when inherited_issuable_templates enabled' do it_behaves_like 'issue description templates from current project only'
before do
stub_feature_flags(inherited_issuable_templates: true)
visit edit_project_path(project)
end
it_behaves_like 'issue description templates from current project only'
end
context 'when inherited_issuable_templates disabled' do
before do
stub_feature_flags(inherited_issuable_templates: false)
visit edit_project_path(project)
end
it_behaves_like 'issue description templates from current project only'
end
end end
...@@ -26,7 +26,7 @@ module API ...@@ -26,7 +26,7 @@ module API
use :pagination use :pagination
end end
get ':id/templates/:type' do get ':id/templates/:type' do
templates = TemplateFinder.all_template_names_array(user_project, params[:type]) templates = TemplateFinder.all_template_names(user_project, params[:type]).values.flatten
present paginate(::Kaminari.paginate_array(templates)), with: Entities::TemplatesList present paginate(::Kaminari.paginate_array(templates)), with: Entities::TemplatesList
end end
......
...@@ -160,28 +160,12 @@ RSpec.describe Projects::TemplatesController do ...@@ -160,28 +160,12 @@ RSpec.describe Projects::TemplatesController do
end end
shared_examples 'template names request' do shared_examples 'template names request' do
context 'when feature flag enabled' do it 'returns the template names', :aggregate_failures do
it 'returns the template names', :aggregate_failures do get(:names, params: { namespace_id: project.namespace, template_type: template_type, project_id: project }, format: :json)
get(:names, params: { namespace_id: project.namespace, template_type: template_type, project_id: project }, format: :json)
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['Project Templates'].size).to eq(2)
expect(json_response['Project Templates'].map { |x| x.slice('name') }).to match(expected_template_names)
end
end
context 'when feature flag disabled' do
before do
stub_feature_flags(inherited_issuable_templates: false)
end
it 'returns the template names', :aggregate_failures do
get(:names, params: { namespace_id: project.namespace, template_type: template_type, project_id: project }, format: :json)
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(json_response.size).to eq(2) expect(json_response['Project Templates'].size).to eq(2)
expect(json_response.map { |x| x.slice('name') }).to match(expected_template_names) expect(json_response['Project Templates'].map { |x| x.slice('name') }).to match(expected_template_names)
end
end end
it 'fails for user with no access' do it 'fails for user with no access' do
......
...@@ -89,25 +89,10 @@ RSpec.describe 'Service Desk Setting', :js, :clean_gitlab_redis_cache do ...@@ -89,25 +89,10 @@ RSpec.describe 'Service Desk Setting', :js, :clean_gitlab_redis_cache do
before do before do
stub_licensed_features(custom_file_templates_for_namespace: false, custom_file_templates: false) stub_licensed_features(custom_file_templates_for_namespace: false, custom_file_templates: false)
group.update_columns(file_template_project_id: group_template_repo.id) group.update_columns(file_template_project_id: group_template_repo.id)
visit edit_project_path(project)
end end
context 'when inherited_issuable_templates enabled' do it_behaves_like 'issue description templates from current project only'
before do
stub_feature_flags(inherited_issuable_templates: true)
visit edit_project_path(project)
end
it_behaves_like 'issue description templates from current project only'
end
context 'when inherited_issuable_templates disabled' do
before do
stub_feature_flags(inherited_issuable_templates: false)
visit edit_project_path(project)
end
it_behaves_like 'issue description templates from current project only'
end
end end
end end
end end
...@@ -13,24 +13,8 @@ RSpec.describe IssuablesDescriptionTemplatesHelper, :clean_gitlab_redis_cache do ...@@ -13,24 +13,8 @@ RSpec.describe IssuablesDescriptionTemplatesHelper, :clean_gitlab_redis_cache do
let_it_be(:group_member) { create(:group_member, :developer, group: parent_group, user: user) } let_it_be(:group_member) { create(:group_member, :developer, group: parent_group, user: user) }
let_it_be(:project_member) { create(:project_member, :developer, user: user, project: project) } let_it_be(:project_member) { create(:project_member, :developer, user: user, project: project) }
context 'when feature flag disabled' do it 'returns empty hash when template type does not exist' do
before do expect(helper.issuable_templates(build(:project), 'non-existent-template-type')).to eq({})
stub_feature_flags(inherited_issuable_templates: false)
end
it 'returns empty array when template type does not exist' do
expect(helper.issuable_templates(project, 'non-existent-template-type')).to eq([])
end
end
context 'when feature flag enabled' do
before do
stub_feature_flags(inherited_issuable_templates: true)
end
it 'returns empty hash when template type does not exist' do
expect(helper.issuable_templates(build(:project), 'non-existent-template-type')).to eq({})
end
end end
context 'with cached issuable templates' do context 'with cached issuable templates' do
...@@ -81,16 +65,18 @@ RSpec.describe IssuablesDescriptionTemplatesHelper, :clean_gitlab_redis_cache do ...@@ -81,16 +65,18 @@ RSpec.describe IssuablesDescriptionTemplatesHelper, :clean_gitlab_redis_cache do
allow(helper).to receive(:issuable_templates).and_return(templates) allow(helper).to receive(:issuable_templates).and_return(templates)
end end
context 'when feature flag disabled' do context 'with matching project templates' do
let(:templates) do let(:templates) do
[ {
{ name: "another_issue_template", id: "another_issue_template", project_id: project.id }, "" => [
{ name: "custom_issue_template", id: "custom_issue_template", project_id: project.id } { name: "another_issue_template", id: "another_issue_template", project_id: project.id },
] { name: "custom_issue_template", id: "custom_issue_template", project_id: project.id }
end ],
"Instance" => [
before do { name: "first_issue_issue_template", id: "first_issue_issue_template", project_id: non_existing_record_id },
stub_feature_flags(inherited_issuable_templates: false) { name: "second_instance_issue_template", id: "second_instance_issue_template", project_id: non_existing_record_id }
]
}
end end
it 'returns project templates only' do it 'returns project templates only' do
...@@ -98,47 +84,22 @@ RSpec.describe IssuablesDescriptionTemplatesHelper, :clean_gitlab_redis_cache do ...@@ -98,47 +84,22 @@ RSpec.describe IssuablesDescriptionTemplatesHelper, :clean_gitlab_redis_cache do
end end
end end
context 'when feature flag enabled' do context 'without matching project templates' do
before do let(:templates) do
stub_feature_flags(inherited_issuable_templates: true) {
end "Project Templates" => [
{ name: "another_issue_template", id: "another_issue_template", project_id: non_existing_record_id },
context 'with matching project templates' do { name: "custom_issue_template", id: "custom_issue_template", project_id: non_existing_record_id }
let(:templates) do ],
{ "Instance" => [
"" => [ { name: "first_issue_issue_template", id: "first_issue_issue_template", project_id: non_existing_record_id },
{ name: "another_issue_template", id: "another_issue_template", project_id: project.id }, { name: "second_instance_issue_template", id: "second_instance_issue_template", project_id: non_existing_record_id }
{ name: "custom_issue_template", id: "custom_issue_template", project_id: project.id } ]
], }
"Instance" => [
{ name: "first_issue_issue_template", id: "first_issue_issue_template", project_id: non_existing_record_id },
{ name: "second_instance_issue_template", id: "second_instance_issue_template", project_id: non_existing_record_id }
]
}
end
it 'returns project templates only' do
expect(helper.issuable_templates_names(Issue.new)).to eq(%w[another_issue_template custom_issue_template])
end
end end
context 'without matching project templates' do it 'returns empty array' do
let(:templates) do expect(helper.issuable_templates_names(Issue.new)).to eq([])
{
"Project Templates" => [
{ name: "another_issue_template", id: "another_issue_template", project_id: non_existing_record_id },
{ name: "custom_issue_template", id: "custom_issue_template", project_id: non_existing_record_id }
],
"Instance" => [
{ name: "first_issue_issue_template", id: "first_issue_issue_template", project_id: non_existing_record_id },
{ name: "second_instance_issue_template", id: "second_instance_issue_template", project_id: non_existing_record_id }
]
}
end
it 'returns empty array' do
expect(helper.issuable_templates_names(Issue.new)).to eq([])
end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment