Skip to content
Snippets Groups Projects

Load issue templates from repository

Merged Felipe Cardozo requested to merge issue_18656 into master

part of #18656 (closed)

Does this MR meet the acceptance criteria?

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • 1 module Gitlab
    2 module Template
    3 class IssueTemplate < BaseTemplate
  • @felipe_artur Do you think we can benefit from having some unit tests for templates-related classes?

  • @felipe_artur Just a few questions :tada: !

  • 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 remove repo_template?. I would be great to simplify API of template classes a little, so :thumbsup: for a small refactoring here.

    • Outdated comment i think. We did this right?

  • Felipe Cardozo Added 1 commit:

    Added 1 commit:

    • db334308 - Load issue templates from repository
  • Luke Bennett mentioned in merge request !5031 (closed)

    mentioned in merge request !5031 (closed)

  • Felipe Cardozo Marked the task CHANGELOG entry added as completed

    Marked the task CHANGELOG entry added as completed

  • Felipe Cardozo Added 1 commit:

    Added 1 commit:

    • 3c1e253f - Load issue templates from repository
  • Felipe Cardozo Marked the task API support added as completed

    Marked the task API support added as completed

  • Marked the task Documentation created/updated as completed

  • Felipe Cardozo Marked the task Added for this feature/bug as completed

    Marked the task Added for this feature/bug 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!

    • Seems right, but looks like the previous idea was to create objects from class methods like find or all like active record does, creating instances or collections of itself.

      If i remove these class methods we will lose that feature.

    • Makes sense, since we also define a finder strategy on class level, we can leave this as is :thumbsup:.

  • Luke Bennett Added 1 commit:

    Added 1 commit:

    • 354bed6b - reformatted cs and removed inline js
  • Felipe Cardozo Added 2 commits:

    Added 2 commits:

  • Luke Bennett Added 3 commits:

    Added 3 commits:

    • 641a37ca - Updated docs, dropdown position and added reset template button
    • 8edf8a4e - hooked up project ID
    • a7a06fc2 - Added reset template functionality
  • @felipe_artur My latest push finishes hooking up the dropdown, now its a matter of clearing build failures?

  • Luke Bennett Added 1 commit:

    Added 1 commit:

    • 091c9e37 - Added initial value handling
  • @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 Bennett
  • 1 1 module API
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading