Commit c45aef05 authored by Markus Koller's avatar Markus Koller

Merge branch 'file-templates-fetching-refactor' into 'master'

File templates fetching refactor [RUN AS-IF-FOSS] [RUN ALL RSPEC]

See merge request gitlab-org/gitlab!52222
parents 2c5cf14f 999c39d6
...@@ -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
# Gitaly the actual template names within a given project's repository for all file templates
# other than `issue` and `merge request` description templates, which would instead
# overwrite the `template_names` method to return a redis cached version, by reading cached values
# from `repository.issue_template_names_by_category` and `repository.merge_request_template_names_by_category`
# methods.
def repository_template_names(project)
template_names_by_category(self.all(project))
end
alias_method :template_names, :repository_template_names
if categories.any? def template_names_by_category(items)
categories.keys.map do |category| grouped = items.group_by(&:category)
files = self.by_category(category, project) categories = grouped.keys
[category, files.map { |t| { name: t.name } }]
end.to_h categories.each_with_object({}) do |category, hash|
else hash[category] = grouped[category].map do |item|
files = self.all(project) { name: item.name, id: item.key, key: item.key, project_id: item.try(:project_id) }
files.map { |t| { name: t.name } } 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
it 'returns all vendored templates when no name is specified' do context 'when no project is passed in' do
result = described_class.new(type, nil).execute it 'returns all vendored templates when no name is specified' do
expect(result).to include(have_attributes(name: vendored_name))
end
end
expect(result).to include(have_attributes(name: vendored_name)) 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))
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
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
expect(result).to be_nil 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