Commit 999c39d6 authored by Alexandru Croitor's avatar Alexandru Croitor Committed by Markus Koller

Prepare templates fetching code for templates inheritance

Refactor the code around template fetching to make it reusable and
ready to be adjusted for inheritable issue and merge requests templates
parent 58acfc19
...@@ -24,10 +24,8 @@ class Projects::TemplatesController < Projects::ApplicationController ...@@ -24,10 +24,8 @@ class Projects::TemplatesController < Projects::ApplicationController
end end
def names def names
templates = @template_type.dropdown_names(project)
respond_to do |format| respond_to do |format|
format.json { render json: templates } format.json { render json: TemplateFinder.all_template_names_array(project, params[:template_type].to_s.pluralize) }
end end
end end
......
...@@ -28,6 +28,10 @@ class LicenseTemplateFinder ...@@ -28,6 +28,10 @@ class LicenseTemplateFinder
end end
end end
def template_names
::Gitlab::Template::BaseTemplate.template_names_by_category(vendored_licenses)
end
private private
def vendored_licenses def vendored_licenses
......
...@@ -21,6 +21,18 @@ class TemplateFinder ...@@ -21,6 +21,18 @@ class TemplateFinder
new(type, project, params) new(type, project, params)
end end
end end
def all_template_names(project, type)
return {} if !VENDORED_TEMPLATES.key?(type.to_s) && type.to_s != 'licenses'
build(type, project).template_names
end
# This is issues and merge requests description templates only.
# This will be removed once we introduce group level inherited templates
def all_template_names_array(project, type)
all_template_names(project, type).values.flatten.uniq
end
end end
attr_reader :type, :project, :params attr_reader :type, :project, :params
...@@ -43,6 +55,10 @@ class TemplateFinder ...@@ -43,6 +55,10 @@ class TemplateFinder
vendored_templates.all(project) vendored_templates.all(project)
end end
end end
def template_names
vendored_templates.template_names(project)
end
end end
TemplateFinder.prepend_if_ee('::EE::TemplateFinder') TemplateFinder.prepend_if_ee('::EE::TemplateFinder')
...@@ -194,40 +194,28 @@ module BlobHelper ...@@ -194,40 +194,28 @@ module BlobHelper
@ref_project ||= @target_project || @project @ref_project ||= @target_project || @project
end end
def template_dropdown_names(items)
grouped = items.group_by(&:category)
categories = grouped.keys
categories.each_with_object({}) do |category, hash|
hash[category] = grouped[category].map do |item|
{ name: item.name, id: item.key }
end
end
end
private :template_dropdown_names
def licenses_for_select(project) def licenses_for_select(project)
@licenses_for_select ||= template_dropdown_names(TemplateFinder.build(:licenses, project).execute) @licenses_for_select ||= TemplateFinder.all_template_names(project, :licenses)
end end
def gitignore_names(project) def gitignore_names(project)
@gitignore_names ||= template_dropdown_names(TemplateFinder.build(:gitignores, project).execute) @gitignore_names ||= TemplateFinder.all_template_names(project, :gitignores)
end end
def gitlab_ci_ymls(project) def gitlab_ci_ymls(project)
@gitlab_ci_ymls ||= template_dropdown_names(TemplateFinder.build(:gitlab_ci_ymls, project).execute) @gitlab_ci_ymls ||= TemplateFinder.all_template_names(project, :gitlab_ci_ymls)
end end
def gitlab_ci_syntax_ymls(project) def gitlab_ci_syntax_ymls(project)
@gitlab_ci_syntax_ymls ||= template_dropdown_names(TemplateFinder.build(:gitlab_ci_syntax_ymls, project).execute) @gitlab_ci_syntax_ymls ||= TemplateFinder.all_template_names(project, :gitlab_ci_syntax_ymls)
end end
def metrics_dashboard_ymls(project) def metrics_dashboard_ymls(project)
@metrics_dashboard_ymls ||= template_dropdown_names(TemplateFinder.build(:metrics_dashboard_ymls, project).execute) @metrics_dashboard_ymls ||= TemplateFinder.all_template_names(project, :metrics_dashboard_ymls)
end end
def dockerfile_names(project) def dockerfile_names(project)
@dockerfile_names ||= template_dropdown_names(TemplateFinder.build(:dockerfiles, project).execute) @dockerfile_names ||= TemplateFinder.all_template_names(project, :dockerfiles)
end end
def blob_editor_paths(project) def blob_editor_paths(project)
......
# frozen_string_literal: true
module IssuablesDescriptionTemplatesHelper
include Gitlab::Utils::StrongMemoize
include GitlabRoutingHelper
def template_dropdown_tag(issuable, &block)
title = selected_template(issuable) || "Choose a template"
options = {
toggle_class: 'js-issuable-selector',
title: title,
filter: true,
placeholder: 'Filter',
footer_content: true,
data: {
data: issuable_templates(ref_project, issuable.to_ability_name),
field_name: 'issuable_template',
selected: selected_template(issuable),
project_id: ref_project.id,
project_path: ref_project.path,
namespace_path: ref_project.namespace.full_path
}
}
dropdown_tag(title, options: options) do
capture(&block)
end
end
def issuable_templates(project, issuable_type)
@template_types ||= {}
@template_types[project.id] ||= {}
@template_types[project.id][issuable_type] ||= TemplateFinder.all_template_names_array(project, issuable_type.pluralize)
end
def issuable_templates_names(issuable)
issuable_templates(ref_project, issuable.to_ability_name).map { |template| template[:name] }
end
def selected_template(issuable)
params[:issuable_template] if issuable_templates(ref_project, issuable.to_ability_name).any? { |template| template[:name] == params[:issuable_template] }
end
def template_names_path(parent, issuable)
return '' unless parent.is_a?(Project)
project_template_names_path(parent, template_type: issuable.to_ability_name)
end
end
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
module IssuablesHelper module IssuablesHelper
include GitlabRoutingHelper include GitlabRoutingHelper
include IssuablesDescriptionTemplatesHelper
def sidebar_gutter_toggle_icon def sidebar_gutter_toggle_icon
content_tag(:span, class: 'js-sidebar-toggle-container', data: { is_expanded: !sidebar_gutter_collapsed? }) do content_tag(:span, class: 'js-sidebar-toggle-container', data: { is_expanded: !sidebar_gutter_collapsed? }) do
...@@ -75,28 +76,6 @@ module IssuablesHelper ...@@ -75,28 +76,6 @@ module IssuablesHelper
.to_json .to_json
end end
def template_dropdown_tag(issuable, &block)
title = selected_template(issuable) || "Choose a template"
options = {
toggle_class: 'js-issuable-selector',
title: title,
filter: true,
placeholder: 'Filter',
footer_content: true,
data: {
data: issuable_templates(issuable),
field_name: 'issuable_template',
selected: selected_template(issuable),
project_path: ref_project.path,
namespace_path: ref_project.namespace.full_path
}
}
dropdown_tag(title, options: options) do
capture(&block)
end
end
def users_dropdown_label(selected_users) def users_dropdown_label(selected_users)
case selected_users.length case selected_users.length
when 0 when 0
...@@ -282,6 +261,7 @@ module IssuablesHelper ...@@ -282,6 +261,7 @@ module IssuablesHelper
{ {
projectPath: ref_project.path, projectPath: ref_project.path,
projectId: ref_project.id,
projectNamespace: ref_project.namespace.full_path projectNamespace: ref_project.namespace.full_path
} }
end end
...@@ -358,24 +338,6 @@ module IssuablesHelper ...@@ -358,24 +338,6 @@ module IssuablesHelper
cookies[:collapsed_gutter] == 'true' cookies[:collapsed_gutter] == 'true'
end end
def issuable_templates(issuable)
@issuable_templates ||=
case issuable
when Issue
ref_project.repository.issue_template_names
when MergeRequest
ref_project.repository.merge_request_template_names
end
end
def issuable_templates_names(issuable)
issuable_templates(issuable).map { |template| template[:name] }
end
def selected_template(issuable)
params[:issuable_template] if issuable_templates(issuable).any? { |template| template[:name] == params[:issuable_template] }
end
def issuable_todo_button_data(issuable, is_collapsed) def issuable_todo_button_data(issuable, is_collapsed)
{ {
todo_text: _('Add a to do'), todo_text: _('Add a to do'),
...@@ -413,12 +375,6 @@ module IssuablesHelper ...@@ -413,12 +375,6 @@ module IssuablesHelper
end end
end end
def template_names_path(parent, issuable)
return '' unless parent.is_a?(Project)
project_template_names_path(parent, template_type: issuable.class.name.underscore)
end
def issuable_sidebar_options(issuable) def issuable_sidebar_options(issuable)
{ {
endpoint: "#{issuable[:issuable_json_path]}?serializer=sidebar_extras", endpoint: "#{issuable[:issuable_json_path]}?serializer=sidebar_extras",
......
...@@ -43,7 +43,7 @@ class Repository ...@@ -43,7 +43,7 @@ class Repository
changelog license_blob license_key gitignore changelog license_blob license_key gitignore
gitlab_ci_yml branch_names tag_names branch_count gitlab_ci_yml branch_names tag_names branch_count
tag_count avatar exists? root_ref merged_branch_names tag_count avatar exists? root_ref merged_branch_names
has_visible_content? issue_template_names merge_request_template_names has_visible_content? issue_template_names_by_category merge_request_template_names_by_category
user_defined_metrics_dashboard_paths xcode_project? has_ambiguous_refs?).freeze user_defined_metrics_dashboard_paths xcode_project? has_ambiguous_refs?).freeze
# Methods that use cache_method but only memoize the value # Methods that use cache_method but only memoize the value
...@@ -60,8 +60,8 @@ class Repository ...@@ -60,8 +60,8 @@ class Repository
gitignore: :gitignore, gitignore: :gitignore,
gitlab_ci: :gitlab_ci_yml, gitlab_ci: :gitlab_ci_yml,
avatar: :avatar, avatar: :avatar,
issue_template: :issue_template_names, issue_template: :issue_template_names_by_category,
merge_request_template: :merge_request_template_names, merge_request_template: :merge_request_template_names_by_category,
metrics_dashboard: :user_defined_metrics_dashboard_paths, metrics_dashboard: :user_defined_metrics_dashboard_paths,
xcode_config: :xcode_project? xcode_config: :xcode_project?
}.freeze }.freeze
...@@ -572,15 +572,16 @@ class Repository ...@@ -572,15 +572,16 @@ class Repository
end end
cache_method :avatar cache_method :avatar
def issue_template_names # store issue_template_names as hash
Gitlab::Template::IssueTemplate.dropdown_names(project) def issue_template_names_by_category
Gitlab::Template::IssueTemplate.repository_template_names(project)
end end
cache_method :issue_template_names, fallback: [] cache_method :issue_template_names_by_category, fallback: {}
def merge_request_template_names def merge_request_template_names_by_category
Gitlab::Template::MergeRequestTemplate.dropdown_names(project) Gitlab::Template::MergeRequestTemplate.repository_template_names(project)
end end
cache_method :merge_request_template_names, fallback: [] cache_method :merge_request_template_names_by_category, fallback: {}
def user_defined_metrics_dashboard_paths def user_defined_metrics_dashboard_paths
Gitlab::Metrics::Dashboard::RepoDashboardFinder.list_dashboards(project) Gitlab::Metrics::Dashboard::RepoDashboardFinder.list_dashboards(project)
......
- issuable = local_assigns.fetch(:issuable, nil) - issuable = local_assigns.fetch(:issuable, nil)
- return unless issuable && issuable_templates(issuable).any? - return unless issuable && issuable_templates(ref_project, issuable.to_ability_name).any?
.issuable-form-select-holder.selectbox.form-group .issuable-form-select-holder.selectbox.form-group
.js-issuable-selector-wrap{ data: { issuable_type: issuable.to_ability_name, qa_selector: 'template_dropdown' } } .js-issuable-selector-wrap{ data: { issuable_type: issuable.to_ability_name, qa_selector: 'template_dropdown' } }
......
- issuable = local_assigns.fetch(:issuable) - issuable = local_assigns.fetch(:issuable)
- has_wip_commits = local_assigns.fetch(:has_wip_commits) - has_wip_commits = local_assigns.fetch(:has_wip_commits)
- form = local_assigns.fetch(:form) - form = local_assigns.fetch(:form)
- no_issuable_templates = issuable_templates(issuable).empty? - no_issuable_templates = issuable_templates(ref_project, issuable.to_ability_name).empty?
- div_class = no_issuable_templates ? 'col-sm-10' : 'col-sm-7 col-lg-8' - div_class = no_issuable_templates ? 'col-sm-10' : 'col-sm-7 col-lg-8'
- toggle_wip_link_start = '<a href="" class="js-toggle-wip">' - toggle_wip_link_start = '<a href="" class="js-toggle-wip">'
- toggle_wip_link_end = '</a>' - toggle_wip_link_end = '</a>'
......
...@@ -25,6 +25,13 @@ module EE ...@@ -25,6 +25,13 @@ module EE
end end
end end
override :template_names
def template_names
return super unless custom_templates?
custom_templates.all_template_names.merge(super)
end
private private
def custom_templates? def custom_templates?
......
...@@ -33,5 +33,14 @@ module EE ...@@ -33,5 +33,14 @@ module EE
custom_templates.all + super custom_templates.all + super
end end
end end
override :template_names
def template_names
return super if custom_templates.nil? || !custom_templates.enabled?
# on custom templates we do want to fetch all template names as this will iterate through inherited templates
# from ancestor group levels
custom_templates.all_template_names.merge(super)
end
end end
end end
...@@ -15,6 +15,19 @@ module Gitlab ...@@ -15,6 +15,19 @@ module Gitlab
instance_enabled? || namespace_enabled? instance_enabled? || namespace_enabled?
end end
def all_template_names
template_names = {}
namespace_template_projects_hash.flat_map do |namespace, project|
template_names[category_for(namespace)] = template_names_for(project).values.flatten
end
if instance_enabled?
template_names[_('Instance')] = template_names_for(instance_template_project).values.flatten
end
template_names
end
def all def all
by_namespace = namespace_template_projects_hash.flat_map do |namespace, project| by_namespace = namespace_template_projects_hash.flat_map do |namespace, project|
templates_for(project, category_for(namespace)) templates_for(project, category_for(namespace))
...@@ -22,7 +35,7 @@ module Gitlab ...@@ -22,7 +35,7 @@ module Gitlab
by_instance = by_instance =
if instance_enabled? if instance_enabled?
templates_for(instance_template_project, 'Instance') templates_for(instance_template_project, _('Instance'))
else else
[] []
end end
...@@ -36,7 +49,7 @@ module Gitlab ...@@ -36,7 +49,7 @@ module Gitlab
return found if found return found if found
end end
template_for(instance_template_project, name, 'Instance') template_for(instance_template_project, name, _('Instance'))
end end
private private
...@@ -76,6 +89,12 @@ module Gitlab ...@@ -76,6 +89,12 @@ module Gitlab
end end
end end
def template_names_for(project)
return [] unless project
finder.template_names(project)
end
def templates_for(project, category) def templates_for(project, category)
return [] unless project return [] unless project
......
...@@ -70,4 +70,49 @@ RSpec.describe TemplateFinder do ...@@ -70,4 +70,49 @@ RSpec.describe TemplateFinder do
end end
end end
end end
describe '#template_names' do
let_it_be(:template_files) do
{
"Dockerfile/project_dockerfiles_template.dockerfile" => "project_dockerfiles_template content",
"gitignore/project_gitignores_template.gitignore" => "project_gitignores_template content",
"gitlab-ci/project_gitlab_ci_ymls_template.yml" => "project_gitlab_ci_ymls_template content",
"metrics-dashboards/project_metrics_dashboard_ymls_template.yml" => "project_metrics_dashboard_ymls_template content",
".gitlab/issue_templates/project_issues_template.md" => "project_issues_template content",
".gitlab/merge_request_templates/project_merge_requests_template.md" => "project_merge_requests_template content"
}
end
let_it_be(:group, reload: true) { create(:group) }
let_it_be(:project, reload: true) { create(:project, group: group) }
let_it_be(:group_template_project, reload: true) { create(:project, :custom_repo, group: group, files: template_files) }
where(:type, :custom_name) do
:dockerfiles | 'project_dockerfiles_template'
:gitignores | 'project_gitignores_template'
:gitlab_ci_ymls | 'project_gitlab_ci_ymls_template'
:metrics_dashboard_ymls | 'project_metrics_dashboard_ymls_template'
end
before do
stub_licensed_features(custom_file_templates: true, custom_file_templates_for_namespace: true)
group.update_columns(file_template_project_id: group_template_project.id)
end
subject(:result) { described_class.new(type, project, params).template_names.values.flatten.map { |el| OpenStruct.new(el) } }
with_them do
context 'when project has a repository' do
it 'returns all custom templates' do
expect(result).to include(have_attributes(name: custom_name))
end
end
context 'template names hash keys' do
it 'has all the expected keys' do
expect(result.first.to_h.keys).to match_array(%i(id key name project_id))
end
end
end
end
end end
...@@ -6,10 +6,10 @@ RSpec.describe BlobHelper do ...@@ -6,10 +6,10 @@ RSpec.describe BlobHelper do
include TreeHelper include TreeHelper
describe '#licenses_for_select' do describe '#licenses_for_select' do
let(:group) { create(:group) } let_it_be(:group) { create(:group) }
let(:project) { create(:project, namespace: group) } let_it_be(:project) { create(:project, :repository, namespace: group) }
let_it_be(:group_category) { "Group #{group.full_name}" }
let(:group_category) { "Group #{group.full_name}" }
let(:categories) { result.keys } let(:categories) { result.keys }
let(:by_group) { result[group_category] } let(:by_group) { result[group_category] }
let(:by_instance) { result['Instance'] } let(:by_instance) { result['Instance'] }
...@@ -32,7 +32,7 @@ RSpec.describe BlobHelper do ...@@ -32,7 +32,7 @@ RSpec.describe BlobHelper do
.and_return([OpenStruct.new(key: 'name', name: 'Name')]) .and_return([OpenStruct.new(key: 'name', name: 'Name')])
expect(categories).to contain_exactly(:Popular, :Other, group_category) expect(categories).to contain_exactly(:Popular, :Other, group_category)
expect(by_group).to contain_exactly({ id: 'name', name: 'Name' }) expect(by_group).to contain_exactly({ id: 'name', name: 'Name', key: 'name', project_id: nil })
expect(by_popular).to be_present expect(by_popular).to be_present
expect(by_other).to be_present expect(by_other).to be_present
end end
...@@ -46,7 +46,7 @@ RSpec.describe BlobHelper do ...@@ -46,7 +46,7 @@ RSpec.describe BlobHelper do
.and_return([OpenStruct.new(key: 'name', name: 'Name')]) .and_return([OpenStruct.new(key: 'name', name: 'Name')])
expect(categories).to contain_exactly(:Popular, :Other, 'Instance') expect(categories).to contain_exactly(:Popular, :Other, 'Instance')
expect(by_instance).to contain_exactly({ id: 'name', name: 'Name' }) expect(by_instance).to contain_exactly({ id: 'name', name: 'Name', key: 'name', project_id: nil })
expect(by_popular).to be_present expect(by_popular).to be_present
expect(by_other).to be_present expect(by_other).to be_present
end end
......
...@@ -184,4 +184,68 @@ RSpec.describe Gitlab::CustomFileTemplates do ...@@ -184,4 +184,68 @@ RSpec.describe Gitlab::CustomFileTemplates do
end end
end end
end end
describe '#all_template_names' do
where(:template_finder, :type) do
Gitlab::Template::CustomDockerfileTemplate | :dockerfile
Gitlab::Template::CustomGitignoreTemplate | :gitignore
Gitlab::Template::CustomGitlabCiYmlTemplate | :gitlab_ci_yml
Gitlab::Template::CustomLicenseTemplate | :license
end
with_them do
let(:result) { templates.all_template_names }
let(:template_file_names) { result.values.flatten.map { |el| el[:name] } }
let(:template_categories) { result.keys }
before do
stub_ee_application_setting(file_template_project: instance_template_project)
group.update_columns(file_template_project_id: group_template_project.id)
end
context 'unlicensed' do
let(:target_project) { project }
it { expect(result).to be_empty }
end
context 'licensed' do
before do
stub_licensed_features(custom_file_templates: true, custom_file_templates_for_namespace: true)
end
context 'in a toplevel group' do
let(:target_project) { project }
it 'has the group names and instance as category' do
expect(template_categories).to eq(["Group #{group.full_name}", "Instance"])
end
it 'orders results from most specific to least specific' do
expect(template_file_names).to eq(["group_#{type}", "instance_#{type}"])
end
end
context 'in a subgroup' do
let_it_be(:subgroup) { create(:group, parent: group) }
let_it_be(:subproject) { create(:project, namespace: subgroup) }
let_it_be(:subgroup_template_project) { create(:project, :custom_repo, namespace: subgroup, files: template_files('subgroup')) }
let(:target_project) { subproject }
before do
subgroup.update_columns(file_template_project_id: subgroup_template_project.id)
end
it 'has the group names and instance as category' do
expect(template_categories).to eq(["Group #{subgroup.full_name}", "Group #{group.full_name}", "Instance"])
end
it 'orders results from most specific to least specific' do
expect(template_file_names).to eq(["subgroup_#{type}", "group_#{type}", "instance_#{type}"])
end
end
end
end
end
end end
...@@ -26,9 +26,7 @@ module API ...@@ -26,9 +26,7 @@ module API
use :pagination use :pagination
end end
get ':id/templates/:type' do get ':id/templates/:type' do
templates = TemplateFinder templates = TemplateFinder.all_template_names_array(user_project, params[:type])
.build(params[:type], user_project)
.execute
present paginate(::Kaminari.paginate_array(templates)), with: Entities::TemplatesList present paginate(::Kaminari.paginate_array(templates)), with: Entities::TemplatesList
end end
......
...@@ -95,19 +95,29 @@ module Gitlab ...@@ -95,19 +95,29 @@ module Gitlab
File.join(base_dir, categories[category]) File.join(base_dir, categories[category])
end end
# If template is organized by category it returns { category_name: [{ name: template_name }, { name: template2_name }] } # `repository_template_names` - reads through Gitaly the actual templates names within a
# If no category is present returns [{ name: template_name }, { name: template2_name}] # given project's repository. This is only used by issue and merge request templates,
def dropdown_names(project = nil) # that need to call this once and then cache the returned value.
return [] if project && !project.repository.exists? #
# `template_names` - is an alias to `repository_template_names`. It would read through
if categories.any? # Gitaly the actual template names within a given project's repository for all file templates
categories.keys.map do |category| # other than `issue` and `merge request` description templates, which would instead
files = self.by_category(category, project) # overwrite the `template_names` method to return a redis cached version, by reading cached values
[category, files.map { |t| { name: t.name } }] # from `repository.issue_template_names_by_category` and `repository.merge_request_template_names_by_category`
end.to_h # methods.
else def repository_template_names(project)
files = self.all(project) template_names_by_category(self.all(project))
files.map { |t| { name: t.name } } end
alias_method :template_names, :repository_template_names
def template_names_by_category(items)
grouped = items.group_by(&:category)
categories = grouped.keys
categories.each_with_object({}) do |category, hash|
hash[category] = grouped[category].map do |item|
{ name: item.name, id: item.key, key: item.key, project_id: item.try(:project_id) }
end
end end
end end
......
...@@ -11,8 +11,8 @@ module Gitlab ...@@ -11,8 +11,8 @@ module Gitlab
def initialize(project, base_dir, extension, categories = {}) def initialize(project, base_dir, extension, categories = {})
@categories = categories @categories = categories
@extension = extension @extension = extension
@repository = project.repository @repository = project&.repository
@commit = @repository.head_commit if @repository.exists? @commit = @repository.head_commit if @repository&.exists?
super(base_dir) super(base_dir)
end end
...@@ -51,7 +51,7 @@ module Gitlab ...@@ -51,7 +51,7 @@ module Gitlab
private private
def select_directory(file_name) def select_directory(file_name)
return [] unless @commit return unless @commit
# Insert root as directory # Insert root as directory
directories = ["", *@categories.keys] directories = ["", *@categories.keys]
......
...@@ -15,6 +15,16 @@ module Gitlab ...@@ -15,6 +15,16 @@ module Gitlab
def finder(project) def finder(project)
Gitlab::Template::Finders::RepoTemplateFinder.new(project, self.base_dir, self.extension, self.categories) Gitlab::Template::Finders::RepoTemplateFinder.new(project, self.base_dir, self.extension, self.categories)
end end
def template_names(project)
return {} unless project&.repository&.exists?
# here we rely on project.repository caching mechanism. Ideally we would want the template finder to have its
# own caching mechanism to avoid the back and forth call jumps between finder and model.
#
# follow-up issue: https://gitlab.com/gitlab-org/gitlab/-/issues/300279
project.repository.issue_template_names_by_category
end
end end
end end
end end
......
...@@ -15,6 +15,16 @@ module Gitlab ...@@ -15,6 +15,16 @@ module Gitlab
def finder(project) def finder(project)
Gitlab::Template::Finders::RepoTemplateFinder.new(project, self.base_dir, self.extension, self.categories) Gitlab::Template::Finders::RepoTemplateFinder.new(project, self.base_dir, self.extension, self.categories)
end end
def template_names(project)
return {} unless project&.repository&.exists?
# here we rely on project.repository caching mechanism. Ideally we would want the template finder to have its
# own caching mechanism to avoid the back and forth call jumps between finder and model.
#
# follow-up issue: https://gitlab.com/gitlab-org/gitlab/-/issues/300279
project.repository.merge_request_template_names_by_category
end
end end
end end
end end
......
...@@ -165,7 +165,8 @@ RSpec.describe Projects::TemplatesController do ...@@ -165,7 +165,8 @@ RSpec.describe Projects::TemplatesController do
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.size).to eq(2)
expect(json_response).to match(expected_template_names) expect(json_response.size).to eq(2)
expect(json_response.map { |x| x.slice('name') }).to match(expected_template_names)
end end
it 'fails for user with no access' do it 'fails for user with no access' do
......
...@@ -3,12 +3,7 @@ ...@@ -3,12 +3,7 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe LicenseTemplateFinder do RSpec.describe LicenseTemplateFinder do
describe '#execute' do RSpec.shared_examples 'filters by popular category' do
subject(:result) { described_class.new(nil, params).execute }
let(:categories) { categorised_licenses.keys }
let(:categorised_licenses) { result.group_by(&:category) }
context 'popular: true' do context 'popular: true' do
let(:params) { { popular: true } } let(:params) { { popular: true } }
...@@ -26,6 +21,15 @@ RSpec.describe LicenseTemplateFinder do ...@@ -26,6 +21,15 @@ RSpec.describe LicenseTemplateFinder do
expect(categorised_licenses[:Other]).to be_present expect(categorised_licenses[:Other]).to be_present
end end
end end
end
describe '#execute' do
subject(:result) { described_class.new(nil, params).execute }
let(:categories) { categorised_licenses.keys }
let(:categorised_licenses) { result.group_by(&:category) }
it_behaves_like 'filters by popular category'
context 'popular: nil' do context 'popular: nil' do
let(:params) { { popular: nil } } let(:params) { { popular: nil } }
...@@ -48,4 +52,31 @@ RSpec.describe LicenseTemplateFinder do ...@@ -48,4 +52,31 @@ RSpec.describe LicenseTemplateFinder do
end end
end end
end end
describe '#template_names' do
let(:params) { {} }
subject(:template_names) { described_class.new(nil, params).template_names }
let(:categories) { categorised_licenses.keys }
let(:categorised_licenses) { template_names }
it_behaves_like 'filters by popular category'
context 'popular: nil' do
let(:params) { { popular: nil } }
it 'returns all licenses known by the Licensee gem' do
from_licensee = Licensee::License.all.map { |l| l.key }
expect(template_names.values.flatten.map { |x| x[:key] }).to match_array(from_licensee)
end
end
context 'template names hash keys' do
it 'has all the expected keys' do
expect(template_names.values.flatten.first.keys).to match_array(%i(id key name project_id))
end
end
end
end end
...@@ -5,6 +5,102 @@ require 'spec_helper' ...@@ -5,6 +5,102 @@ require 'spec_helper'
RSpec.describe TemplateFinder do RSpec.describe TemplateFinder do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
let_it_be(:template_files) do
{
"Dockerfile/project_dockerfiles_template.dockerfile" => "project_dockerfiles_template content",
"gitignore/project_gitignores_template.gitignore" => "project_gitignores_template content",
"gitlab-ci/project_gitlab_ci_ymls_template.yml" => "project_gitlab_ci_ymls_template content",
".gitlab/issue_templates/project_issues_template.md" => "project_issues_template content",
".gitlab/merge_request_templates/project_merge_requests_template.md" => "project_merge_requests_template content"
}
end
RSpec.shared_examples 'fetches predefined vendor templates' do
where(:type, :vendored_name) do
:dockerfiles | 'Binary'
:gitignores | 'Actionscript'
:gitlab_ci_ymls | 'Android'
:metrics_dashboard_ymls | 'Default'
:gitlab_ci_syntax_ymls | 'Artifacts example'
end
with_them do
it 'returns all vendored templates when no name is specified' do
expect(result).to include(have_attributes(name: vendored_name))
end
context 'with name param' do
let(:params) { { name: vendored_name } }
it 'returns only the specified vendored template when a name is specified' do
expect(result).to have_attributes(name: vendored_name)
end
context 'with mistaken name param' do
let(:params) { { name: 'unknown' } }
it 'returns nil when an unknown name is specified' do
expect(result).to be_nil
end
end
end
end
end
RSpec.shared_examples 'no issues and merge requests templates available' do
context 'with issue and merge request templates' do
where(:type, :vendored_name) do
:issues | nil
:merge_requests | nil
end
with_them do
context 'when fetching all templates' do
it 'returns empty array' do
expect(result).to eq([])
end
end
context 'when looking for specific template by name' do
let(:params) { { name: 'anything' } }
it 'raises an error' do
expect { result }.to raise_exception(Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError)
end
end
end
end
end
RSpec.shared_examples 'fetches issues and merge requests templates' do
where(:type, :template_name) do
:issues | 'project_issues_template'
:merge_requests | 'project_merge_requests_template'
end
with_them do
it 'returns all repository template files for issues and merge requests' do
expect(result).to include(have_attributes(name: template_name))
end
context 'with name param' do
let(:params) { { name: template_name } }
it 'returns only the specified vendored template when a name is specified' do
expect(result).to have_attributes(name: template_name)
end
context 'with mistaken name param' do
let(:params) { { name: 'unknown' } }
it 'raises an error when an unknown name is specified' do
expect { result }.to raise_exception(Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError)
end
end
end
end
end
describe '#build' do describe '#build' do
let(:project) { build_stubbed(:project) } let(:project) { build_stubbed(:project) }
...@@ -15,6 +111,8 @@ RSpec.describe TemplateFinder do ...@@ -15,6 +111,8 @@ RSpec.describe TemplateFinder do
:licenses | ::LicenseTemplateFinder :licenses | ::LicenseTemplateFinder
:metrics_dashboard_ymls | described_class :metrics_dashboard_ymls | described_class
:gitlab_ci_syntax_ymls | described_class :gitlab_ci_syntax_ymls | described_class
:issues | described_class
:merge_requests | described_class
end end
with_them do with_them do
...@@ -26,6 +124,37 @@ RSpec.describe TemplateFinder do ...@@ -26,6 +124,37 @@ RSpec.describe TemplateFinder do
end end
describe '#execute' do describe '#execute' do
let_it_be(:project) { nil }
let(:params) { {} }
subject(:result) { described_class.new(type, project, params).execute }
context 'when no project is passed in' do
it_behaves_like 'fetches predefined vendor templates'
it_behaves_like 'no issues and merge requests templates available'
end
context 'when project has no repository' do
let_it_be(:project) { create(:project) }
it_behaves_like 'fetches predefined vendor templates'
it_behaves_like 'no issues and merge requests templates available'
end
context 'when project has a repository' do
let_it_be(:project) { create(:project, :custom_repo, files: template_files) }
it_behaves_like 'fetches predefined vendor templates'
it_behaves_like 'fetches issues and merge requests templates'
end
end
describe '#template_names' do
let_it_be(:project) { nil }
let(:params) { {} }
subject(:result) { described_class.new(type, project, params).template_names.values.flatten.map { |el| OpenStruct.new(el) } }
where(:type, :vendored_name) do where(:type, :vendored_name) do
:dockerfiles | 'Binary' :dockerfiles | 'Binary'
:gitignores | 'Actionscript' :gitignores | 'Actionscript'
...@@ -35,22 +164,67 @@ RSpec.describe TemplateFinder do ...@@ -35,22 +164,67 @@ RSpec.describe TemplateFinder do
end end
with_them do with_them do
context 'when no project is passed in' do
it 'returns all vendored templates when no name is specified' do it 'returns all vendored templates when no name is specified' do
result = described_class.new(type, nil).execute expect(result).to include(have_attributes(name: vendored_name))
end
end
context 'when project has no repository' do
let_it_be(:project) { create(:project) }
it 'returns all vendored templates when no name is specified' do
expect(result).to include(have_attributes(name: vendored_name)) expect(result).to include(have_attributes(name: vendored_name))
end end
end
it 'returns only the specified vendored template when a name is specified' do context 'when project has a repository' do
result = described_class.new(type, nil, name: vendored_name).execute let_it_be(:project) { create(:project, :custom_repo, files: template_files) }
expect(result).to have_attributes(name: vendored_name) it 'returns all vendored templates when no name is specified' do
expect(result).to include(have_attributes(name: vendored_name))
end
end end
it 'returns nil when an unknown name is specified' do context 'template names hash keys' do
result = described_class.new(type, nil, name: 'unknown').execute it 'has all the expected keys' do
expect(result.first.to_h.keys).to match_array(%i(id key name project_id))
end
end
end
expect(result).to be_nil where(:type, :template_name) do
:issues | 'project_issues_template'
:merge_requests | 'project_merge_requests_template'
end
with_them do
context 'when no project is passed in' do
it 'returns all vendored templates when no name is specified' do
expect(result).to eq([])
end
end
context 'when project has no repository' do
let_it_be(:project) { create(:project) }
it 'returns all vendored templates when no name is specified' do
expect(result).to eq([])
end
end
context 'when project has a repository' do
let_it_be(:project) { create(:project, :custom_repo, files: template_files) }
it 'returns all vendored templates when no name is specified' do
expect(result).to include(have_attributes(name: template_name))
end
context 'template names hash keys' do
it 'has all the expected keys' do
expect(result.first.to_h.keys).to match_array(%i(id key name project_id))
end
end
end end
end end
end end
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe IssuablesDescriptionTemplatesHelper, :clean_gitlab_redis_cache do
include_context 'project issuable templates context'
describe '#issuable_templates' do
let_it_be(:inherited_from) { nil }
let_it_be(:user) { create(:user) }
let_it_be(:parent_group, reload: true) { create(:group) }
let_it_be(:project, reload: true) { create(:project, :custom_repo, files: issuable_template_files) }
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) }
it 'returns empty hash when template type does not exist' do
expect(helper.issuable_templates(build(:project), 'non-existent-template-type')).to eq([])
end
context 'with cached issuable templates' do
before do
allow(Gitlab::Template::IssueTemplate).to receive(:template_names).and_return({})
allow(Gitlab::Template::MergeRequestTemplate).to receive(:template_names).and_return({})
helper.issuable_templates(project, 'issues')
helper.issuable_templates(project, 'merge_request')
end
it 'does not call TemplateFinder' do
expect(Gitlab::Template::IssueTemplate).not_to receive(:template_names)
expect(Gitlab::Template::MergeRequestTemplate).not_to receive(:template_names)
helper.issuable_templates(project, 'issues')
helper.issuable_templates(project, 'merge_request')
end
end
context 'when project has no parent group' do
it_behaves_like 'project issuable templates'
end
context 'when project has parent group' do
before do
project.update!(group: parent_group)
end
context 'when project parent group does not have a file template project' do
it_behaves_like 'project issuable templates'
end
context 'when project parent group has a file template project' do
let_it_be(:file_template_project) { create(:project, :custom_repo, group: parent_group, files: issuable_template_files) }
let_it_be(:group, reload: true) { create(:group, parent: parent_group) }
let_it_be(:project, reload: true) { create(:project, :custom_repo, group: group, files: issuable_template_files) }
before do
project.update!(group: group)
parent_group.update_columns(file_template_project_id: file_template_project.id)
end
it_behaves_like 'project issuable templates'
end
end
end
describe '#issuable_templates_names' do
let(:project) { double(Project, id: 21) }
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 }
]
end
it 'returns project templates only' do
allow(helper).to receive(:ref_project).and_return(project)
allow(helper).to receive(:issuable_templates).and_return(templates)
expect(helper.issuable_templates_names(Issue.new)).to eq(%w[another_issue_template custom_issue_template])
end
context 'when there are not templates in the project' do
let(:templates) { {} }
it 'returns empty array' do
allow(helper).to receive(:ref_project).and_return(project)
allow(helper).to receive(:issuable_templates).and_return(templates)
expect(helper.issuable_templates_names(Issue.new)).to eq([])
end
end
end
end
...@@ -1949,8 +1949,8 @@ RSpec.describe Repository do ...@@ -1949,8 +1949,8 @@ RSpec.describe Repository do
:root_ref, :root_ref,
:merged_branch_names, :merged_branch_names,
:has_visible_content?, :has_visible_content?,
:issue_template_names, :issue_template_names_by_category,
:merge_request_template_names, :merge_request_template_names_by_category,
:user_defined_metrics_dashboard_paths, :user_defined_metrics_dashboard_paths,
:xcode_project?, :xcode_project?,
:has_ambiguous_refs? :has_ambiguous_refs?
......
# frozen_string_literal: true
RSpec.shared_context 'project issuable templates context' do
let_it_be(:issuable_template_files) do
{
'.gitlab/issue_templates/issue-bar.md' => 'Issue Template Bar',
'.gitlab/issue_templates/issue-foo.md' => 'Issue Template Foo',
'.gitlab/issue_templates/issue-bad.txt' => 'Issue Template Bad',
'.gitlab/issue_templates/issue-baz.xyz' => 'Issue Template Baz',
'.gitlab/merge_request_templates/merge_request-bar.md' => 'Merge Request Template Bar',
'.gitlab/merge_request_templates/merge_request-foo.md' => 'Merge Request Template Foo',
'.gitlab/merge_request_templates/merge_request-bad.txt' => 'Merge Request Template Bad',
'.gitlab/merge_request_templates/merge_request-baz.xyz' => 'Merge Request Template Baz'
}
end
end
RSpec.shared_examples 'project issuable templates' do
context 'issuable templates' do
before do
allow(helper).to receive(:current_user).and_return(user)
end
it 'returns only md files as issue templates' do
expect(helper.issuable_templates(project, 'issue')).to eq(templates('issue', nil))
end
it 'returns only md files as merge_request templates' do
expect(helper.issuable_templates(project, 'merge_request')).to eq(templates('merge_request', nil))
end
end
def expected_templates(issuable_type)
expectation = {}
expectation["Project Templates"] = templates(issuable_type, project)
expectation["Group #{inherited_from.namespace.full_name}"] = templates(issuable_type, inherited_from) if inherited_from.present?
expectation
end
def templates(issuable_type, inherited_from)
[
{ id: "#{issuable_type}-bar", key: "#{issuable_type}-bar", name: "#{issuable_type}-bar", project_id: inherited_from&.id },
{ id: "#{issuable_type}-foo", key: "#{issuable_type}-foo", name: "#{issuable_type}-foo", project_id: inherited_from&.id }
]
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