Load issue templates from repository
part of #18656 (closed)
Does this MR meet the acceptance criteria?
-
CHANGELOG entry added -
Documentation created/updated -
API support added - Tests
-
Added for this feature/bug -
All builds are passing
-
-
Conform by the style guides -
Branch has no merge conflicts with master
(if you do - rebase it please) -
Squashed related commits together
Merge request reports
Activity
@DouweM Please take a look. I used a kind of built in strategy pattern inside
BaseTemplate
class.
By reusing this code we can re-use a lot of front-end code, including the dropdown code. Also we will be able to extend it for Merge Request templates.Edited by Felipe Cardozo@grzesiek Can you please take a look at this merge request, provide input on the direction of the implementation, and help @felipe_artur in getting it ready as development continues? It's not ready for review yet, just for a look to see if this is the way we want to go.
Reassigned to @grzesiek
@DouweM Sure!
- lib/gitlab/template/issue_template.rb 0 → 100644
1 module Gitlab 2 module Template 3 class IssueTemplate < BaseTemplate @DouweM decided to keep
IssueTemplate
instead ofIssue
since the object is not an issue.
@felipe_artur Do you think we can benefit from having some unit tests for templates-related classes?
@felipe_artur Just a few questions
!Reassigned to @felipe_artur
10 11 end 11 12 12 13 def content 13 File.read(@path) 14 self.class.finder(@project_id).read(@path) 14 15 end 15 16 16 17 class << self 17 def all 18 self.categories.keys.flat_map { |cat| by_category(cat) } 18 def finder(project_id = nil) 19 if repo_template? 20 Gitlab::Template::RepoTemplateFinder.new(project_id, base_dir, extension, categories) 21 else 22 Gitlab::Template::LocalTemplateFinder.new(base_dir, extension, categories) 23 end What do you think about instantiating finder in a concrete class? Using a common interface for each finder (constructor in particular) may allow us to just create a
def self.finder
method that will return a class of a finder for a concrete class, and initialize it in constructor. With this approach it would be possible to removerepo_template?
. I would be great to simplify API of template classes a little, so for a small refactoring here.
mentioned in merge request !5031 (closed)
Marked the task CHANGELOG entry added as completed
Marked the task Documentation created/updated as completed
10 13 end 11 14 12 15 def content 13 File.read(@path) 16 @finder.read(@path) 14 17 end 15 18 16 19 class << self I just wonder if since we have a template that depends on the state (issue template depends on specific project), what do you think about completely getting rid of class methods in templates? Those class methods seem to belong to object, as we do use the finder that actually encapsulates the state. We can still fabricate finder on the instance level, and move all those methods to instance. What do you think about this design change? If you have other ideas, just let me know!
Added 2 commits:
- f8591b15 - Add controller to get issue templates, code fixes and documentation improvements
- 5c99863b - Merge branch 'issue_18656' of https://gitlab.com/gitlab-org/gitlab-ce into issue_18656
@felipe_artur My latest push finishes hooking up the dropdown, now its a matter of clearing build failures?
@felipe_artur And that latest push allows users to visit
/issues/new?issue_template=[template_name]
to load the template on page load, but this could probably be improved by handling it with ruby before render.Edited by Luke Bennett1 1 module API We dont have API documentation about templates.
Edited by Felipe Cardozo