diff options
author | Annabel Dunstone <annabel.dunstone@gmail.com> | 2016-08-16 15:23:36 -0500 |
---|---|---|
committer | Annabel Dunstone <annabel.dunstone@gmail.com> | 2016-08-16 15:23:36 -0500 |
commit | 30d578f6345c3807158e16eb73b28f290e416d16 (patch) | |
tree | d699ad98ea5284771eaed67365376551980a3df4 | |
parent | dc42d52a17bef5bdaa91f5d0e4f213664d23af49 (diff) | |
parent | f7e08ad0de580e9f27ad5301542f2913c6bb2bff (diff) | |
download | gitlab-ce-30d578f6345c3807158e16eb73b28f290e416d16.tar.gz |
Merge branch 'master' of gitlab.com:gitlab-org/gitlab-ce into 18681-pipelines-merge-request
54 files changed, 1012 insertions, 215 deletions
diff --git a/CHANGELOG b/CHANGELOG index 9299639a3ab..aececed9add 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -40,6 +40,7 @@ v 8.11.0 (unreleased) - Various redundant database indexes have been removed - Update `timeago` plugin to use multiple string/locale settings - Remove unused images (ClemMakesApps) + - Get issue and merge request description templates from repositories - Limit git rev-list output count to one in forced push check - Show deployment status on merge requests with external URLs - Clean up unused routes (Josef Strzibny) diff --git a/app/assets/javascripts/api.js b/app/assets/javascripts/api.js index 49c2ac0dac3..84b292e59c6 100644 --- a/app/assets/javascripts/api.js +++ b/app/assets/javascripts/api.js @@ -9,10 +9,11 @@ licensePath: "/api/:version/licenses/:key", gitignorePath: "/api/:version/gitignores/:key", gitlabCiYmlPath: "/api/:version/gitlab_ci_ymls/:key", + issuableTemplatePath: "/:namespace_path/:project_path/templates/:type/:key", + group: function(group_id, callback) { - var url; - url = Api.buildUrl(Api.groupPath); - url = url.replace(':id', group_id); + var url = Api.buildUrl(Api.groupPath) + .replace(':id', group_id); return $.ajax({ url: url, data: { @@ -24,8 +25,7 @@ }); }, groups: function(query, skip_ldap, callback) { - var url; - url = Api.buildUrl(Api.groupsPath); + var url = Api.buildUrl(Api.groupsPath); return $.ajax({ url: url, data: { @@ -39,8 +39,7 @@ }); }, namespaces: function(query, callback) { - var url; - url = Api.buildUrl(Api.namespacesPath); + var url = Api.buildUrl(Api.namespacesPath); return $.ajax({ url: url, data: { @@ -54,8 +53,7 @@ }); }, projects: function(query, order, callback) { - var url; - url = Api.buildUrl(Api.projectsPath); + var url = Api.buildUrl(Api.projectsPath); return $.ajax({ url: url, data: { @@ -70,9 +68,8 @@ }); }, newLabel: function(project_id, data, callback) { - var url; - url = Api.buildUrl(Api.labelsPath); - url = url.replace(':id', project_id); + var url = Api.buildUrl(Api.labelsPath) + .replace(':id', project_id); data.private_token = gon.api_token; return $.ajax({ url: url, @@ -86,9 +83,8 @@ }); }, groupProjects: function(group_id, query, callback) { - var url; - url = Api.buildUrl(Api.groupProjectsPath); - url = url.replace(':id', group_id); + var url = Api.buildUrl(Api.groupProjectsPath) + .replace(':id', group_id); return $.ajax({ url: url, data: { @@ -102,8 +98,8 @@ }); }, licenseText: function(key, data, callback) { - var url; - url = Api.buildUrl(Api.licensePath).replace(':key', key); + var url = Api.buildUrl(Api.licensePath) + .replace(':key', key); return $.ajax({ url: url, data: data @@ -112,19 +108,32 @@ }); }, gitignoreText: function(key, callback) { - var url; - url = Api.buildUrl(Api.gitignorePath).replace(':key', key); + var url = Api.buildUrl(Api.gitignorePath) + .replace(':key', key); return $.get(url, function(gitignore) { return callback(gitignore); }); }, gitlabCiYml: function(key, callback) { - var url; - url = Api.buildUrl(Api.gitlabCiYmlPath).replace(':key', key); + var url = Api.buildUrl(Api.gitlabCiYmlPath) + .replace(':key', key); return $.get(url, function(file) { return callback(file); }); }, + issueTemplate: function(namespacePath, projectPath, key, type, callback) { + var url = Api.buildUrl(Api.issuableTemplatePath) + .replace(':key', key) + .replace(':type', type) + .replace(':project_path', projectPath) + .replace(':namespace_path', namespacePath); + $.ajax({ + url: url, + dataType: 'json' + }).done(function(file) { + callback(null, file); + }).error(callback); + }, buildUrl: function(url) { if (gon.relative_url_root != null) { url = gon.relative_url_root + url; diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index f1aab067351..e596b98603b 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -41,6 +41,7 @@ /*= require date.format */ /*= require_directory ./behaviors */ /*= require_directory ./blob */ +/*= require_directory ./templates */ /*= require_directory ./commit */ /*= require_directory ./extensions */ /*= require_directory ./lib/utils */ diff --git a/app/assets/javascripts/blob/template_selector.js b/app/assets/javascripts/blob/template_selector.js index 2cf0a6631b8..b0a37ef0e0a 100644 --- a/app/assets/javascripts/blob/template_selector.js +++ b/app/assets/javascripts/blob/template_selector.js @@ -9,6 +9,7 @@ } this.onClick = bind(this.onClick, this); this.dropdown = opts.dropdown, this.data = opts.data, this.pattern = opts.pattern, this.wrapper = opts.wrapper, this.editor = opts.editor, this.fileEndpoint = opts.fileEndpoint, this.$input = (ref = opts.$input) != null ? ref : $('#file_name'); + this.dropdownIcon = $('.fa-chevron-down', this.dropdown); this.buildDropdown(); this.bindEvents(); this.onFilenameUpdate(); @@ -60,11 +61,26 @@ return this.requestFile(item); }; - TemplateSelector.prototype.requestFile = function(item) {}; + TemplateSelector.prototype.requestFile = function(item) { + // This `requestFile` method is an abstract method that should + // be added by all subclasses. + }; - TemplateSelector.prototype.requestFileSuccess = function(file) { + TemplateSelector.prototype.requestFileSuccess = function(file, skipFocus) { this.editor.setValue(file.content, 1); - return this.editor.focus(); + if (!skipFocus) this.editor.focus(); + }; + + TemplateSelector.prototype.startLoadingSpinner = function() { + this.dropdownIcon + .addClass('fa-spinner fa-spin') + .removeClass('fa-chevron-down'); + }; + + TemplateSelector.prototype.stopLoadingSpinner = function() { + this.dropdownIcon + .addClass('fa-chevron-down') + .removeClass('fa-spinner fa-spin'); }; return TemplateSelector; diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js index 3946e861976..7160fa71ce5 100644 --- a/app/assets/javascripts/dispatcher.js +++ b/app/assets/javascripts/dispatcher.js @@ -55,6 +55,7 @@ shortcut_handler = new ShortcutsNavigation(); new GLForm($('.issue-form')); new IssuableForm($('.issue-form')); + new IssuableTemplateSelectors(); break; case 'projects:merge_requests:new': case 'projects:merge_requests:edit': @@ -62,6 +63,7 @@ shortcut_handler = new ShortcutsNavigation(); new GLForm($('.merge-request-form')); new IssuableForm($('.merge-request-form')); + new IssuableTemplateSelectors(); break; case 'projects:tags:new': new ZenMode(); diff --git a/app/assets/javascripts/protected_branch_create.js.es6 b/app/assets/javascripts/protected_branch_create.js.es6 index 00e20a03b04..2efca2414dc 100644 --- a/app/assets/javascripts/protected_branch_create.js.es6 +++ b/app/assets/javascripts/protected_branch_create.js.es6 @@ -44,8 +44,8 @@ // Enable submit button const $branchInput = this.$wrap.find('input[name="protected_branch[name]"]'); - const $allowedToMergeInput = this.$wrap.find('input[name="protected_branch[merge_access_level_attributes][access_level]"]'); - const $allowedToPushInput = this.$wrap.find('input[name="protected_branch[push_access_level_attributes][access_level]"]'); + const $allowedToMergeInput = this.$wrap.find('input[name="protected_branch[merge_access_levels_attributes][0][access_level]"]'); + const $allowedToPushInput = this.$wrap.find('input[name="protected_branch[push_access_levels_attributes][0][access_level]"]'); if ($branchInput.val() && $allowedToMergeInput.val() && $allowedToPushInput.val()){ this.$form.find('input[type="submit"]').removeAttr('disabled'); diff --git a/app/assets/javascripts/protected_branch_edit.js.es6 b/app/assets/javascripts/protected_branch_edit.js.es6 index 8d42e268ebc..a59fcbfa082 100644 --- a/app/assets/javascripts/protected_branch_edit.js.es6 +++ b/app/assets/javascripts/protected_branch_edit.js.es6 @@ -39,12 +39,14 @@ _method: 'PATCH', id: this.$wrap.data('banchId'), protected_branch: { - merge_access_level_attributes: { + merge_access_levels_attributes: [{ + id: this.$allowedToMergeDropdown.data('access-level-id'), access_level: $allowedToMergeInput.val() - }, - push_access_level_attributes: { + }], + push_access_levels_attributes: [{ + id: this.$allowedToPushDropdown.data('access-level-id'), access_level: $allowedToPushInput.val() - } + }] } }, success: () => { diff --git a/app/assets/javascripts/templates/issuable_template_selector.js.es6 b/app/assets/javascripts/templates/issuable_template_selector.js.es6 new file mode 100644 index 00000000000..c32ddf80219 --- /dev/null +++ b/app/assets/javascripts/templates/issuable_template_selector.js.es6 @@ -0,0 +1,51 @@ +/*= require ../blob/template_selector */ + +((global) => { + class IssuableTemplateSelector extends TemplateSelector { + constructor(...args) { + super(...args); + this.projectPath = this.dropdown.data('project-path'); + this.namespacePath = this.dropdown.data('namespace-path'); + this.issuableType = this.wrapper.data('issuable-type'); + this.titleInput = $(`#${this.issuableType}_title`); + + let initialQuery = { + name: this.dropdown.data('selected') + }; + + if (initialQuery.name) this.requestFile(initialQuery); + + $('.reset-template', this.dropdown.parent()).on('click', () => { + if (this.currentTemplate) this.setInputValueToTemplateContent(); + }); + } + + requestFile(query) { + this.startLoadingSpinner(); + Api.issueTemplate(this.namespacePath, this.projectPath, query.name, this.issuableType, (err, currentTemplate) => { + this.currentTemplate = currentTemplate; + if (err) return; // Error handled by global AJAX error handler + this.stopLoadingSpinner(); + this.setInputValueToTemplateContent(); + }); + return; + } + + setInputValueToTemplateContent() { + // `this.requestFileSuccess` sets the value of the description input field + // to the content of the template selected. + if (this.titleInput.val() === '') { + // If the title has not yet been set, focus the title input and + // skip focusing the description input by setting `true` as the 2nd + // argument to `requestFileSuccess`. + this.requestFileSuccess(this.currentTemplate, true); + this.titleInput.focus(); + } else { + this.requestFileSuccess(this.currentTemplate); + } + return; + } + } + + global.IssuableTemplateSelector = IssuableTemplateSelector; +})(window); diff --git a/app/assets/javascripts/templates/issuable_template_selectors.js.es6 b/app/assets/javascripts/templates/issuable_template_selectors.js.es6 new file mode 100644 index 00000000000..bd8cdde033e --- /dev/null +++ b/app/assets/javascripts/templates/issuable_template_selectors.js.es6 @@ -0,0 +1,29 @@ +((global) => { + class IssuableTemplateSelectors { + constructor(opts = {}) { + this.$dropdowns = opts.$dropdowns || $('.js-issuable-selector'); + this.editor = opts.editor || this.initEditor(); + + this.$dropdowns.each((i, dropdown) => { + let $dropdown = $(dropdown); + new IssuableTemplateSelector({ + pattern: /(\.md)/, + data: $dropdown.data('data'), + wrapper: $dropdown.closest('.js-issuable-selector-wrap'), + dropdown: $dropdown, + editor: this.editor + }); + }); + } + + initEditor() { + let editor = $('.markdown-area'); + // Proxy ace-editor's .setValue to jQuery's .val + editor.setValue = editor.val; + editor.getValue = editor.val; + return editor; + } + } + + global.IssuableTemplateSelectors = IssuableTemplateSelectors; +})(window); diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index e8eafa15899..f1635a53763 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -56,9 +56,13 @@ position: absolute; top: 50%; right: 6px; - margin-top: -4px; + margin-top: -6px; color: $dropdown-toggle-icon-color; font-size: 10px; + &.fa-spinner { + font-size: 16px; + margin-top: -8px; + } } &:hover, { @@ -406,6 +410,7 @@ font-size: 14px; a { + cursor: pointer; padding-left: 10px; } } diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index 7a50bc9c832..46c4a11aa2e 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -395,3 +395,12 @@ display: inline-block; line-height: 18px; } + +.js-issuable-selector-wrap { + .js-issuable-selector { + width: 100%; + } + @media (max-width: $screen-sm-max) { + margin-bottom: $gl-padding; + } +} diff --git a/app/controllers/autocomplete_controller.rb b/app/controllers/autocomplete_controller.rb index d828d163c28..e1641ba6265 100644 --- a/app/controllers/autocomplete_controller.rb +++ b/app/controllers/autocomplete_controller.rb @@ -1,5 +1,6 @@ class AutocompleteController < ApplicationController skip_before_action :authenticate_user!, only: [:users] + before_action :load_project, only: [:users] before_action :find_users, only: [:users] def users @@ -55,11 +56,8 @@ class AutocompleteController < ApplicationController def find_users @users = - if params[:project_id].present? - project = Project.find(params[:project_id]) - return render_404 unless can?(current_user, :read_project, project) - - project.team.users + if @project + @project.team.users elsif params[:group_id].present? group = Group.find(params[:group_id]) return render_404 unless can?(current_user, :read_group, group) @@ -71,4 +69,14 @@ class AutocompleteController < ApplicationController User.none end end + + def load_project + @project ||= begin + if params[:project_id].present? + project = Project.find(params[:project_id]) + return render_404 unless can?(current_user, :read_project, project) + project + end + end + end end diff --git a/app/controllers/projects/protected_branches_controller.rb b/app/controllers/projects/protected_branches_controller.rb index d28ec6e2eac..9a438d5512c 100644 --- a/app/controllers/projects/protected_branches_controller.rb +++ b/app/controllers/projects/protected_branches_controller.rb @@ -9,16 +9,16 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController def index @protected_branch = @project.protected_branches.new - load_protected_branches_gon_variables + load_gon_index end def create - @protected_branch = ProtectedBranches::CreateService.new(@project, current_user, protected_branch_params).execute + @protected_branch = ::ProtectedBranches::CreateService.new(@project, current_user, protected_branch_params).execute if @protected_branch.persisted? redirect_to namespace_project_protected_branches_path(@project.namespace, @project) else load_protected_branches - load_protected_branches_gon_variables + load_gon_index render :index end end @@ -28,7 +28,7 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController end def update - @protected_branch = ProtectedBranches::UpdateService.new(@project, current_user, protected_branch_params).execute(@protected_branch) + @protected_branch = ::ProtectedBranches::UpdateService.new(@project, current_user, protected_branch_params).execute(@protected_branch) if @protected_branch.valid? respond_to do |format| @@ -58,17 +58,23 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController def protected_branch_params params.require(:protected_branch).permit(:name, - merge_access_level_attributes: [:access_level], - push_access_level_attributes: [:access_level]) + merge_access_levels_attributes: [:access_level, :id], + push_access_levels_attributes: [:access_level, :id]) end def load_protected_branches @protected_branches = @project.protected_branches.order(:name).page(params[:page]) end - def load_protected_branches_gon_variables - gon.push({ open_branches: @project.open_branches.map { |br| { text: br.name, id: br.name, title: br.name } }, - push_access_levels: ProtectedBranch::PushAccessLevel.human_access_levels.map { |id, text| { id: id, text: text } }, - merge_access_levels: ProtectedBranch::MergeAccessLevel.human_access_levels.map { |id, text| { id: id, text: text } } }) + def access_levels_options + { + push_access_levels: ProtectedBranch::PushAccessLevel.human_access_levels.map { |id, text| { id: id, text: text, before_divider: true } }, + merge_access_levels: ProtectedBranch::MergeAccessLevel.human_access_levels.map { |id, text| { id: id, text: text, before_divider: true } } + } + end + + def load_gon_index + params = { open_branches: @project.open_branches.map { |br| { text: br.name, id: br.name, title: br.name } } } + gon.push(params.merge(access_levels_options)) end end diff --git a/app/controllers/projects/templates_controller.rb b/app/controllers/projects/templates_controller.rb new file mode 100644 index 00000000000..694b468c8d3 --- /dev/null +++ b/app/controllers/projects/templates_controller.rb @@ -0,0 +1,19 @@ +class Projects::TemplatesController < Projects::ApplicationController + before_action :authenticate_user!, :get_template_class + + def show + template = @template_type.find(params[:key], project) + + respond_to do |format| + format.json { render json: template.to_json } + end + end + + private + + def get_template_class + template_types = { issue: Gitlab::Template::IssueTemplate, merge_request: Gitlab::Template::MergeRequestTemplate }.with_indifferent_access + @template_type = template_types[params[:template_type]] + render json: [], status: 404 unless @template_type + end +end diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index 48c27828219..1cb5d847626 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -182,17 +182,42 @@ module BlobHelper } end + def selected_template(issuable) + templates = issuable_templates(issuable) + params[:issuable_template] if templates.include?(params[:issuable_template]) + end + + def can_add_template?(issuable) + names = issuable_templates(issuable) + names.empty? && can?(current_user, :push_code, @project) && !@project.private? + end + + def merge_request_template_names + @merge_request_templates ||= Gitlab::Template::MergeRequestTemplate.dropdown_names(ref_project) + end + + def issue_template_names + @issue_templates ||= Gitlab::Template::IssueTemplate.dropdown_names(ref_project) + end + + def issuable_templates(issuable) + @issuable_templates ||= + if issuable.is_a?(Issue) + issue_template_names + elsif issuable.is_a?(MergeRequest) + merge_request_template_names + end + end + + def ref_project + @ref_project ||= @target_project || @project + end + def gitignore_names - @gitignore_names ||= - Gitlab::Template::Gitignore.categories.keys.map do |k| - [k, Gitlab::Template::Gitignore.by_category(k).map { |t| { name: t.name } }] - end.to_h + @gitignore_names ||= Gitlab::Template::GitignoreTemplate.dropdown_names end def gitlab_ci_ymls - @gitlab_ci_ymls ||= - Gitlab::Template::GitlabCiYml.categories.keys.map do |k| - [k, Gitlab::Template::GitlabCiYml.by_category(k).map { |t| { name: t.name } }] - end.to_h + @gitlab_ci_ymls ||= Gitlab::Template::GitlabCiYmlTemplate.dropdown_names end end diff --git a/app/models/concerns/protected_branch_access.rb b/app/models/concerns/protected_branch_access.rb new file mode 100644 index 00000000000..5a7b36070e7 --- /dev/null +++ b/app/models/concerns/protected_branch_access.rb @@ -0,0 +1,7 @@ +module ProtectedBranchAccess + extend ActiveSupport::Concern + + def humanize + self.class.human_access_levels[self.access_level] + end +end diff --git a/app/models/protected_branch.rb b/app/models/protected_branch.rb index 226b3f54342..6240912a6e1 100644 --- a/app/models/protected_branch.rb +++ b/app/models/protected_branch.rb @@ -5,11 +5,14 @@ class ProtectedBranch < ActiveRecord::Base validates :name, presence: true validates :project, presence: true - has_one :merge_access_level, dependent: :destroy - has_one :push_access_level, dependent: :destroy + has_many :merge_access_levels, dependent: :destroy + has_many :push_access_levels, dependent: :destroy - accepts_nested_attributes_for :push_access_level - accepts_nested_attributes_for :merge_access_level + validates_length_of :merge_access_levels, is: 1, message: "are restricted to a single instance per protected branch." + validates_length_of :push_access_levels, is: 1, message: "are restricted to a single instance per protected branch." + + accepts_nested_attributes_for :push_access_levels + accepts_nested_attributes_for :merge_access_levels def commit project.commit(self.name) diff --git a/app/models/protected_branch/merge_access_level.rb b/app/models/protected_branch/merge_access_level.rb index b1112ee737d..806b3ccd275 100644 --- a/app/models/protected_branch/merge_access_level.rb +++ b/app/models/protected_branch/merge_access_level.rb @@ -1,4 +1,6 @@ class ProtectedBranch::MergeAccessLevel < ActiveRecord::Base + include ProtectedBranchAccess + belongs_to :protected_branch delegate :project, to: :protected_branch @@ -17,8 +19,4 @@ class ProtectedBranch::MergeAccessLevel < ActiveRecord::Base project.team.max_member_access(user.id) >= access_level end - - def humanize - self.class.human_access_levels[self.access_level] - end end diff --git a/app/models/protected_branch/push_access_level.rb b/app/models/protected_branch/push_access_level.rb index 6a5e49cf453..92e9c51d883 100644 --- a/app/models/protected_branch/push_access_level.rb +++ b/app/models/protected_branch/push_access_level.rb @@ -1,4 +1,6 @@ class ProtectedBranch::PushAccessLevel < ActiveRecord::Base + include ProtectedBranchAccess + belongs_to :protected_branch delegate :project, to: :protected_branch @@ -20,8 +22,4 @@ class ProtectedBranch::PushAccessLevel < ActiveRecord::Base project.team.max_member_access(user.id) >= access_level end - - def humanize - self.class.human_access_levels[self.access_level] - end end diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index 6f521462cf3..d5fb2018d24 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -91,12 +91,12 @@ class GitPushService < BaseService params = { name: @project.default_branch, - push_access_level_attributes: { + push_access_levels_attributes: [{ access_level: current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_PUSH ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER - }, - merge_access_level_attributes: { + }], + merge_access_levels_attributes: [{ access_level: current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_MERGE ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER - } + }] } ProtectedBranches::CreateService.new(@project, current_user, params).execute diff --git a/app/services/protected_branches/create_service.rb b/app/services/protected_branches/create_service.rb index 6150a2a83c9..a84e335340d 100644 --- a/app/services/protected_branches/create_service.rb +++ b/app/services/protected_branches/create_service.rb @@ -5,23 +5,7 @@ module ProtectedBranches def execute raise Gitlab::Access::AccessDeniedError unless can?(current_user, :admin_project, project) - protected_branch = project.protected_branches.new(params) - - ProtectedBranch.transaction do - protected_branch.save! - - if protected_branch.push_access_level.blank? - protected_branch.create_push_access_level!(access_level: Gitlab::Access::MASTER) - end - - if protected_branch.merge_access_level.blank? - protected_branch.create_merge_access_level!(access_level: Gitlab::Access::MASTER) - end - end - - protected_branch - rescue ActiveRecord::RecordInvalid - protected_branch + project.protected_branches.create(params) end end end diff --git a/app/views/projects/protected_branches/_create_protected_branch.html.haml b/app/views/projects/protected_branches/_create_protected_branch.html.haml index 85d0c494ba8..d4c6fa24768 100644 --- a/app/views/projects/protected_branches/_create_protected_branch.html.haml +++ b/app/views/projects/protected_branches/_create_protected_branch.html.haml @@ -5,6 +5,7 @@ Protect a branch .panel-body .form-horizontal + = form_errors(@protected_branch) .form-group = f.label :name, class: 'col-md-2 text-right' do Branch: @@ -18,19 +19,19 @@ %code production/* are supported .form-group - %label.col-md-2.text-right{ for: 'merge_access_level_attributes' } + %label.col-md-2.text-right{ for: 'merge_access_levels_attributes' } Allowed to merge: .col-md-10 = dropdown_tag('Select', options: { toggle_class: 'js-allowed-to-merge wide', - data: { field_name: 'protected_branch[merge_access_level_attributes][access_level]', input_id: 'merge_access_level_attributes' }}) + data: { field_name: 'protected_branch[merge_access_levels_attributes][0][access_level]', input_id: 'merge_access_levels_attributes' }}) .form-group - %label.col-md-2.text-right{ for: 'push_access_level_attributes' } + %label.col-md-2.text-right{ for: 'push_access_levels_attributes' } Allowed to push: .col-md-10 = dropdown_tag('Select', options: { toggle_class: 'js-allowed-to-push wide', - data: { field_name: 'protected_branch[push_access_level_attributes][access_level]', input_id: 'push_access_level_attributes' }}) + data: { field_name: 'protected_branch[push_access_levels_attributes][0][access_level]', input_id: 'push_access_levels_attributes' }}) .panel-footer = f.submit 'Protect', class: 'btn-create btn', disabled: true diff --git a/app/views/projects/protected_branches/_protected_branch.html.haml b/app/views/projects/protected_branches/_protected_branch.html.haml index e2e01ee78f8..0628134b1bb 100644 --- a/app/views/projects/protected_branches/_protected_branch.html.haml +++ b/app/views/projects/protected_branches/_protected_branch.html.haml @@ -13,16 +13,9 @@ = time_ago_with_tooltip(commit.committed_date) - else (branch was removed from repository) - %td - = hidden_field_tag "allowed_to_merge_#{protected_branch.id}", protected_branch.merge_access_level.access_level - = dropdown_tag( (protected_branch.merge_access_level.humanize || 'Select') , - options: { toggle_class: 'js-allowed-to-merge', dropdown_class: 'dropdown-menu-selectable js-allowed-to-merge-container', - data: { field_name: "allowed_to_merge_#{protected_branch.id}" }}) - %td - = hidden_field_tag "allowed_to_push_#{protected_branch.id}", protected_branch.push_access_level.access_level - = dropdown_tag( (protected_branch.push_access_level.humanize || 'Select') , - options: { toggle_class: 'js-allowed-to-push', dropdown_class: 'dropdown-menu-selectable js-allowed-to-push-container', - data: { field_name: "allowed_to_push_#{protected_branch.id}" }}) + + = render partial: 'update_protected_branch', locals: { protected_branch: protected_branch } + - if can_admin_project %td = link_to 'Unprotect', [@project.namespace.becomes(Namespace), @project, protected_branch], data: { confirm: 'Branch will be writable for developers. Are you sure?' }, method: :delete, class: 'btn btn-warning' diff --git a/app/views/projects/protected_branches/_update_protected_branch.html.haml b/app/views/projects/protected_branches/_update_protected_branch.html.haml new file mode 100644 index 00000000000..d6044aacaec --- /dev/null +++ b/app/views/projects/protected_branches/_update_protected_branch.html.haml @@ -0,0 +1,10 @@ +%td + = hidden_field_tag "allowed_to_merge_#{protected_branch.id}", protected_branch.merge_access_levels.first.access_level + = dropdown_tag( (protected_branch.merge_access_levels.first.humanize || 'Select') , + options: { toggle_class: 'js-allowed-to-merge', dropdown_class: 'dropdown-menu-selectable js-allowed-to-merge-container', + data: { field_name: "allowed_to_merge_#{protected_branch.id}", access_level_id: protected_branch.merge_access_levels.first.id }}) +%td + = hidden_field_tag "allowed_to_push_#{protected_branch.id}", protected_branch.push_access_levels.first.access_level + = dropdown_tag( (protected_branch.push_access_levels.first.humanize || 'Select') , + options: { toggle_class: 'js-allowed-to-push', dropdown_class: 'dropdown-menu-selectable js-allowed-to-push-container', + data: { field_name: "allowed_to_push_#{protected_branch.id}", access_level_id: protected_branch.push_access_levels.first.id }}) diff --git a/app/views/shared/issuable/_filter.html.haml b/app/views/shared/issuable/_filter.html.haml index 0b7fa8c7d06..c957cd84479 100644 --- a/app/views/shared/issuable/_filter.html.haml +++ b/app/views/shared/issuable/_filter.html.haml @@ -45,7 +45,7 @@ .filter-item.inline = dropdown_tag("Milestone", options: { title: "Assign milestone", toggle_class: 'js-milestone-select js-extra-options js-filter-submit js-filter-bulk-update', filter: true, dropdown_class: "dropdown-menu-selectable dropdown-menu-milestone", placeholder: "Search milestones", data: { show_no: true, field_name: "update[milestone_id]", project_id: @project.id, milestones: namespace_project_milestones_path(@project.namespace, @project, :json), use_id: true } }) .filter-item.inline.labels-filter - = render "shared/issuable/label_dropdown", classes: ['js-filter-bulk-update', 'js-multiselect'], show_create: false, show_footer: false, extra_options: false, filter_submit: false, show_footer: false, data_options: { persist_when_hide: "true", field_name: "update[label_ids][]", show_no: false, show_any: false, use_id: true } + = render "shared/issuable/label_dropdown", classes: ['js-filter-bulk-update', 'js-multiselect'], show_create: false, show_footer: false, extra_options: false, filter_submit: false, data_options: { persist_when_hide: "true", field_name: "update[label_ids][]", show_no: false, show_any: false, use_id: true } .filter-item.inline = dropdown_tag("Subscription", options: { toggle_class: "js-subscription-event", title: "Change subscription", dropdown_class: "dropdown-menu-selectable", data: { field_name: "update[subscription_event]" } } ) do %ul diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml index c30bdb0ae91..210b43c7e0b 100644 --- a/app/views/shared/issuable/_form.html.haml +++ b/app/views/shared/issuable/_form.html.haml @@ -2,7 +2,22 @@ .form-group = f.label :title, class: 'control-label' - .col-sm-10 + + - issuable_template_names = issuable_templates(issuable) + + - if issuable_template_names.any? + .col-sm-3.col-lg-2 + .js-issuable-selector-wrap{ data: { issuable_type: issuable.class.to_s.underscore.downcase } } + - title = selected_template(issuable) || "Choose a template" + + = dropdown_tag(title, options: { toggle_class: 'js-issuable-selector', + title: title, filter: true, placeholder: 'Filter', footer_content: true, + data: { data: issuable_template_names, field_name: 'issuable_template', selected: selected_template(issuable), project_path: @project.path, namespace_path: @project.namespace.path } } ) do + %ul.dropdown-footer-list + %li + %a.reset-template + Reset template + %div{ class: issuable_template_names.any? ? 'col-sm-7 col-lg-8' : 'col-sm-10' } = f.text_field :title, maxlength: 255, autofocus: true, autocomplete: 'off', class: 'form-control pad', required: true @@ -23,6 +38,13 @@ to prevent a %strong Work In Progress merge request from being merged before it's ready. + + - if can_add_template?(issuable) + %p.help-block + Add + = link_to "issuable templates", help_page_path('workflow/description_templates') + to help your contributors communicate effectively! + .form-group.detail-page-description = f.label :description, 'Description', class: 'control-label' .col-sm-10 diff --git a/config/routes.rb b/config/routes.rb index 634ca35b5b4..b57d42ee846 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -528,6 +528,11 @@ Rails.application.routes.draw do put '/update/*id', to: 'blob#update', constraints: { id: /.+/ }, as: 'update_blob' post '/preview/*id', to: 'blob#preview', constraints: { id: /.+/ }, as: 'preview_blob' + # + # Templates + # + get '/templates/:template_type/:key' => 'templates#show', as: :template + scope do get( '/blob/*id/diff', diff --git a/doc/workflow/README.md b/doc/workflow/README.md index 49dec613716..993349e5b46 100644 --- a/doc/workflow/README.md +++ b/doc/workflow/README.md @@ -17,6 +17,7 @@ - [Share projects with other groups](share_projects_with_other_groups.md) - [Web Editor](web_editor.md) - [Releases](releases.md) +- [Issuable Templates](issuable_templates.md) - [Milestones](milestones.md) - [Merge Requests](merge_requests.md) - [Revert changes](revert_changes.md) diff --git a/doc/workflow/description_templates.md b/doc/workflow/description_templates.md new file mode 100644 index 00000000000..9514564af02 --- /dev/null +++ b/doc/workflow/description_templates.md @@ -0,0 +1,12 @@ +# Description templates + +Description templates allow you to define context-specific templates for issue and merge request description fields for your project. When in use, users that create a new issue or merge request can select a description template to help them communicate with other contributors effectively. + +Every GitLab project can define its own set of description templates as they are added to the root directory of a GitLab project's repository. + +Description templates are written in markdown _(`.md`)_ and stored in your projects repository under the `/.gitlab/issue_templates/` and `/.gitlab/merge_request_templates/` directories. + +![Description templates](img/description_templates.png) + +_Example:_ +`/.gitlab/issue_templates/bug.md` will enable the `bug` dropdown option for new issues. When `bug` is selected, the content from the `bug.md` template file will be copied to the issue description field. diff --git a/doc/workflow/img/description_templates.png b/doc/workflow/img/description_templates.png Binary files differnew file mode 100644 index 00000000000..af2e9403826 --- /dev/null +++ b/doc/workflow/img/description_templates.png diff --git a/lib/api/branches.rb b/lib/api/branches.rb index a77afe634f6..b615703df93 100644 --- a/lib/api/branches.rb +++ b/lib/api/branches.rb @@ -61,22 +61,27 @@ module API name: @branch.name } - unless developers_can_merge.nil? - protected_branch_params.merge!({ - merge_access_level_attributes: { - access_level: developers_can_merge ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER - } - }) + # If `developers_can_merge` is switched off, _all_ `DEVELOPER` + # merge_access_levels need to be deleted. + if developers_can_merge == false + protected_branch.merge_access_levels.where(access_level: Gitlab::Access::DEVELOPER).destroy_all end - unless developers_can_push.nil? - protected_branch_params.merge!({ - push_access_level_attributes: { - access_level: developers_can_push ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER - } - }) + # If `developers_can_push` is switched off, _all_ `DEVELOPER` + # push_access_levels need to be deleted. + if developers_can_push == false + protected_branch.push_access_levels.where(access_level: Gitlab::Access::DEVELOPER).destroy_all end + protected_branch_params.merge!( + merge_access_levels_attributes: [{ + access_level: developers_can_merge ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER + }], + push_access_levels_attributes: [{ + access_level: developers_can_push ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER + }] + ) + if protected_branch service = ProtectedBranches::UpdateService.new(user_project, current_user, protected_branch_params) service.execute(protected_branch) diff --git a/lib/api/entities.rb b/lib/api/entities.rb index ae74d14a4bb..ec455e67329 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -129,12 +129,14 @@ module API expose :developers_can_push do |repo_branch, options| project = options[:project] - project.protected_branches.matching(repo_branch.name).any? { |protected_branch| protected_branch.push_access_level.access_level == Gitlab::Access::DEVELOPER } + access_levels = project.protected_branches.matching(repo_branch.name).map(&:push_access_levels).flatten + access_levels.any? { |access_level| access_level.access_level == Gitlab::Access::DEVELOPER } end expose :developers_can_merge do |repo_branch, options| project = options[:project] - project.protected_branches.matching(repo_branch.name).any? { |protected_branch| protected_branch.merge_access_level.access_level == Gitlab::Access::DEVELOPER } + access_levels = project.protected_branches.matching(repo_branch.name).map(&:merge_access_levels).flatten + access_levels.any? { |access_level| access_level.access_level == Gitlab::Access::DEVELOPER } end end diff --git a/lib/api/templates.rb b/lib/api/templates.rb index 18408797756..b9e718147e1 100644 --- a/lib/api/templates.rb +++ b/lib/api/templates.rb @@ -1,21 +1,28 @@ module API class Templates < Grape::API - TEMPLATE_TYPES = { - gitignores: Gitlab::Template::Gitignore, - gitlab_ci_ymls: Gitlab::Template::GitlabCiYml + GLOBAL_TEMPLATE_TYPES = { + gitignores: Gitlab::Template::GitignoreTemplate, + gitlab_ci_ymls: Gitlab::Template::GitlabCiYmlTemplate }.freeze - TEMPLATE_TYPES.each do |template, klass| + helpers do + def render_response(template_type, template) + not_found!(template_type.to_s.singularize) unless template + present template, with: Entities::Template + end + end + + GLOBAL_TEMPLATE_TYPES.each do |template_type, klass| # Get the list of the available template # # Example Request: # GET /gitignores # GET /gitlab_ci_ymls - get template.to_s do + get template_type.to_s do present klass.all, with: Entities::TemplatesList end - # Get the text for a specific template + # Get the text for a specific template present in local filesystem # # Parameters: # name (required) - The name of a template @@ -23,13 +30,10 @@ module API # Example Request: # GET /gitignores/Elixir # GET /gitlab_ci_ymls/Ruby - get "#{template}/:name" do + get "#{template_type}/:name" do required_attributes! [:name] - new_template = klass.find(params[:name]) - not_found!(template.to_s.singularize) unless new_template - - present new_template, with: Entities::Template + render_response(template_type, new_template) end end end diff --git a/lib/gitlab/template/base_template.rb b/lib/gitlab/template/base_template.rb index 760ff3e614a..7ebec8e2cff 100644 --- a/lib/gitlab/template/base_template.rb +++ b/lib/gitlab/template/base_template.rb @@ -1,8 +1,9 @@ module Gitlab module Template class BaseTemplate - def initialize(path) + def initialize(path, project = nil) @path = path + @finder = self.class.finder(project) end def name @@ -10,23 +11,32 @@ module Gitlab end def content - File.read(@path) + @finder.read(@path) + end + + def to_json + { name: name, content: content } end class << self - def all - self.categories.keys.flat_map { |cat| by_category(cat) } + def all(project = nil) + if categories.any? + categories.keys.flat_map { |cat| by_category(cat, project) } + else + by_category("", project) + end end - def find(key) - file_name = "#{key}#{self.extension}" - - directory = select_directory(file_name) - directory ? new(File.join(category_directory(directory), file_name)) : nil + def find(key, project = nil) + path = self.finder(project).find(key) + path.present? ? new(path, project) : nil end + # Set categories as sub directories + # Example: { "category_name_1" => "directory_path_1", "category_name_2" => "directory_name_2" } + # Default is no category with all files in base dir of each class def categories - raise NotImplementedError + {} end def extension @@ -37,29 +47,40 @@ module Gitlab raise NotImplementedError end - def by_category(category) - templates_for_directory(category_directory(category)) + # Defines which strategy will be used to get templates files + # RepoTemplateFinder - Finds templates on project repository, templates are filtered perproject + # GlobalTemplateFinder - Finds templates on gitlab installation source, templates can be used in all projects + def finder(project = nil) + raise NotImplementedError end - def category_directory(category) - File.join(base_dir, categories[category]) + def by_category(category, project = nil) + directory = category_directory(category) + files = finder(project).list_files_for(directory) + + files.map { |f| new(f, project) } end - private + def category_directory(category) + return base_dir unless category.present? - def select_directory(file_name) - categories.keys.find do |category| - File.exist?(File.join(category_directory(category), file_name)) - end + File.join(base_dir, categories[category]) end - def templates_for_directory(dir) - dir << '/' unless dir.end_with?('/') - Dir.glob(File.join(dir, "*#{self.extension}")).select { |f| f =~ filter_regex }.map { |f| new(f) } - end + # If template is organized by category it returns { category_name: [{ name: template_name }, { name: template2_name }] } + # If no category is present returns [{ name: template_name }, { name: template2_name}] + def dropdown_names(project = nil) + return [] if project && !project.repository.exists? - def filter_regex - @filter_reges ||= /#{Regexp.escape(extension)}\z/ + if categories.any? + categories.keys.map do |category| + files = self.by_category(category, project) + [category, files.map { |t| { name: t.name } }] + end.to_h + else + files = self.all(project) + files.map { |t| { name: t.name } } + end end end end diff --git a/lib/gitlab/template/finders/base_template_finder.rb b/lib/gitlab/template/finders/base_template_finder.rb new file mode 100644 index 00000000000..473b05257c6 --- /dev/null +++ b/lib/gitlab/template/finders/base_template_finder.rb @@ -0,0 +1,35 @@ +module Gitlab + module Template + module Finders + class BaseTemplateFinder + def initialize(base_dir) + @base_dir = base_dir + end + + def list_files_for + raise NotImplementedError + end + + def read + raise NotImplementedError + end + + def find + raise NotImplementedError + end + + def category_directory(category) + return @base_dir unless category.present? + + @base_dir + @categories[category] + end + + class << self + def filter_regex(extension) + /#{Regexp.escape(extension)}\z/ + end + end + end + end + end +end diff --git a/lib/gitlab/template/finders/global_template_finder.rb b/lib/gitlab/template/finders/global_template_finder.rb new file mode 100644 index 00000000000..831da45191f --- /dev/null +++ b/lib/gitlab/template/finders/global_template_finder.rb @@ -0,0 +1,38 @@ +# Searches and reads file present on Gitlab installation directory +module Gitlab + module Template + module Finders + class GlobalTemplateFinder < BaseTemplateFinder + def initialize(base_dir, extension, categories = {}) + @categories = categories + @extension = extension + super(base_dir) + end + + def read(path) + File.read(path) + end + + def find(key) + file_name = "#{key}#{@extension}" + + directory = select_directory(file_name) + directory ? File.join(category_directory(directory), file_name) : nil + end + + def list_files_for(dir) + dir << '/' unless dir.end_with?('/') + Dir.glob(File.join(dir, "*#{@extension}")).select { |f| f =~ self.class.filter_regex(@extension) } + end + + private + + def select_directory(file_name) + @categories.keys.find do |category| + File.exist?(File.join(category_directory(category), file_name)) + end + end + end + end + end +end diff --git a/lib/gitlab/template/finders/repo_template_finder.rb b/lib/gitlab/template/finders/repo_template_finder.rb new file mode 100644 index 00000000000..22c39436cb2 --- /dev/null +++ b/lib/gitlab/template/finders/repo_template_finder.rb @@ -0,0 +1,59 @@ +# Searches and reads files present on each Gitlab project repository +module Gitlab + module Template + module Finders + class RepoTemplateFinder < BaseTemplateFinder + # Raised when file is not found + class FileNotFoundError < StandardError; end + + def initialize(project, base_dir, extension, categories = {}) + @categories = categories + @extension = extension + @repository = project.repository + @commit = @repository.head_commit if @repository.exists? + + super(base_dir) + end + + def read(path) + blob = @repository.blob_at(@commit.id, path) if @commit + raise FileNotFoundError if blob.nil? + blob.data + end + + def find(key) + file_name = "#{key}#{@extension}" + directory = select_directory(file_name) + raise FileNotFoundError if directory.nil? + + category_directory(directory) + file_name + end + + def list_files_for(dir) + return [] unless @commit + + dir << '/' unless dir.end_with?('/') + + entries = @repository.tree(:head, dir).entries + + names = entries.map(&:name) + names.select { |f| f =~ self.class.filter_regex(@extension) } + end + + private + + def select_directory(file_name) + return [] unless @commit + + # Insert root as directory + directories = ["", @categories.keys] + + directories.find do |category| + path = category_directory(category) + file_name + @repository.blob_at(@commit.id, path) + end + end + end + end + end +end diff --git a/lib/gitlab/template/gitignore.rb b/lib/gitlab/template/gitignore_template.rb index 964fbfd4de3..8d2a9d2305c 100644 --- a/lib/gitlab/template/gitignore.rb +++ b/lib/gitlab/template/gitignore_template.rb @@ -1,6 +1,6 @@ module Gitlab module Template - class Gitignore < BaseTemplate + class GitignoreTemplate < BaseTemplate class << self def extension '.gitignore' @@ -16,6 +16,10 @@ module Gitlab def base_dir Rails.root.join('vendor/gitignore') end + + def finder(project = nil) + Gitlab::Template::Finders::GlobalTemplateFinder.new(self.base_dir, self.extension, self.categories) + end end end end diff --git a/lib/gitlab/template/gitlab_ci_yml.rb b/lib/gitlab/template/gitlab_ci_yml_template.rb index 7f480fe33c0..8d1a1ed54c9 100644 --- a/lib/gitlab/template/gitlab_ci_yml.rb +++ b/lib/gitlab/template/gitlab_ci_yml_template.rb @@ -1,6 +1,6 @@ module Gitlab module Template - class GitlabCiYml < BaseTemplate + class GitlabCiYmlTemplate < BaseTemplate def content explanation = "# This file is a template, and might need editing before it works on your project." [explanation, super].join("\n") @@ -21,6 +21,10 @@ module Gitlab def base_dir Rails.root.join('vendor/gitlab-ci-yml') end + + def finder(project = nil) + Gitlab::Template::Finders::GlobalTemplateFinder.new(self.base_dir, self.extension, self.categories) + end end end end diff --git a/lib/gitlab/template/issue_template.rb b/lib/gitlab/template/issue_template.rb new file mode 100644 index 00000000000..c6fa8d3eafc --- /dev/null +++ b/lib/gitlab/template/issue_template.rb @@ -0,0 +1,19 @@ +module Gitlab + module Template + class IssueTemplate < BaseTemplate + class << self + def extension + '.md' + end + + def base_dir + '.gitlab/issue_templates/' + end + + def finder(project) + Gitlab::Template::Finders::RepoTemplateFinder.new(project, self.base_dir, self.extension, self.categories) + end + end + end + end +end diff --git a/lib/gitlab/template/merge_request_template.rb b/lib/gitlab/template/merge_request_template.rb new file mode 100644 index 00000000000..f826c02f3b5 --- /dev/null +++ b/lib/gitlab/template/merge_request_template.rb @@ -0,0 +1,19 @@ +module Gitlab + module Template + class MergeRequestTemplate < BaseTemplate + class << self + def extension + '.md' + end + + def base_dir + '.gitlab/merge_request_templates/' + end + + def finder(project) + Gitlab::Template::Finders::RepoTemplateFinder.new(project, self.base_dir, self.extension, self.categories) + end + end + end + end +end diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb index c55a7fc4d3d..9858d2e7d83 100644 --- a/lib/gitlab/user_access.rb +++ b/lib/gitlab/user_access.rb @@ -32,7 +32,7 @@ module Gitlab if project.protected_branch?(ref) return true if project.empty_repo? && project.user_can_push_to_empty_repo?(user) - access_levels = project.protected_branches.matching(ref).map(&:push_access_level) + access_levels = project.protected_branches.matching(ref).map(&:push_access_levels).flatten access_levels.any? { |access_level| access_level.check_access(user) } else user.can?(:push_code, project) @@ -43,7 +43,7 @@ module Gitlab return false unless user if project.protected_branch?(ref) - access_levels = project.protected_branches.matching(ref).map(&:merge_access_level) + access_levels = project.protected_branches.matching(ref).map(&:merge_access_levels).flatten access_levels.any? { |access_level| access_level.check_access(user) } else user.can?(:push_code, project) diff --git a/spec/controllers/projects/templates_controller_spec.rb b/spec/controllers/projects/templates_controller_spec.rb new file mode 100644 index 00000000000..7b3a26d7ca7 --- /dev/null +++ b/spec/controllers/projects/templates_controller_spec.rb @@ -0,0 +1,48 @@ +require 'spec_helper' + +describe Projects::TemplatesController do + let(:project) { create(:project) } + let(:user) { create(:user) } + let(:user2) { create(:user) } + let(:file_path_1) { '.gitlab/issue_templates/bug.md' } + let(:body) { JSON.parse(response.body) } + + before do + project.team << [user, :developer] + sign_in(user) + end + + before do + project.team.add_user(user, Gitlab::Access::MASTER) + project.repository.commit_file(user, file_path_1, "something valid", "test 3", "master", false) + end + + describe '#show' do + it 'renders template name and content as json' do + get(:show, namespace_id: project.namespace.to_param, template_type: "issue", key: "bug", project_id: project.path, format: :json) + + expect(response.status).to eq(200) + expect(body["name"]).to eq("bug") + expect(body["content"]).to eq("something valid") + end + + it 'renders 404 when unauthorized' do + sign_in(user2) + get(:show, namespace_id: project.namespace.to_param, template_type: "issue", key: "bug", project_id: project.path, format: :json) + + expect(response.status).to eq(404) + end + + it 'renders 404 when template type is not found' do + sign_in(user) + get(:show, namespace_id: project.namespace.to_param, template_type: "dont_exist", key: "bug", project_id: project.path, format: :json) + + expect(response.status).to eq(404) + end + + it 'renders 404 without errors' do + sign_in(user) + expect { get(:show, namespace_id: project.namespace.to_param, template_type: "dont_exist", key: "bug", project_id: project.path, format: :json) }.not_to raise_error + end + end +end diff --git a/spec/factories/protected_branches.rb b/spec/factories/protected_branches.rb index 5575852c2d7..b2695e0482a 100644 --- a/spec/factories/protected_branches.rb +++ b/spec/factories/protected_branches.rb @@ -3,26 +3,26 @@ FactoryGirl.define do name project - after(:create) do |protected_branch| - protected_branch.create_push_access_level!(access_level: Gitlab::Access::MASTER) - protected_branch.create_merge_access_level!(access_level: Gitlab::Access::MASTER) + after(:build) do |protected_branch| + protected_branch.push_access_levels.new(access_level: Gitlab::Access::MASTER) + protected_branch.merge_access_levels.new(access_level: Gitlab::Access::MASTER) end trait :developers_can_push do after(:create) do |protected_branch| - protected_branch.push_access_level.update!(access_level: Gitlab::Access::DEVELOPER) + protected_branch.push_access_levels.first.update!(access_level: Gitlab::Access::DEVELOPER) end end trait :developers_can_merge do after(:create) do |protected_branch| - protected_branch.merge_access_level.update!(access_level: Gitlab::Access::DEVELOPER) + protected_branch.merge_access_levels.first.update!(access_level: Gitlab::Access::DEVELOPER) end end trait :no_one_can_push do after(:create) do |protected_branch| - protected_branch.push_access_level.update!(access_level: Gitlab::Access::NO_ACCESS) + protected_branch.push_access_levels.first.update!(access_level: Gitlab::Access::NO_ACCESS) end end end diff --git a/spec/features/projects/issuable_templates_spec.rb b/spec/features/projects/issuable_templates_spec.rb new file mode 100644 index 00000000000..4a83740621a --- /dev/null +++ b/spec/features/projects/issuable_templates_spec.rb @@ -0,0 +1,89 @@ +require 'spec_helper' + +feature 'issuable templates', feature: true, js: true do + include WaitForAjax + + let(:user) { create(:user) } + let(:project) { create(:project, :public) } + + before do + project.team << [user, :master] + login_as user + end + + context 'user creates an issue using templates' do + let(:template_content) { 'this is a test "bug" template' } + let(:issue) { create(:issue, author: user, assignee: user, project: project) } + + background do + project.repository.commit_file(user, '.gitlab/issue_templates/bug.md', template_content, 'added issue template', 'master', false) + visit edit_namespace_project_issue_path project.namespace, project, issue + fill_in :'issue[title]', with: 'test issue title' + end + + scenario 'user selects "bug" template' do + select_template 'bug' + wait_for_ajax + preview_template + save_changes + end + end + + context 'user creates a merge request using templates' do + let(:template_content) { 'this is a test "feature-proposal" template' } + let(:merge_request) { create(:merge_request, :with_diffs, source_project: project) } + + background do + project.repository.commit_file(user, '.gitlab/merge_request_templates/feature-proposal.md', template_content, 'added merge request template', 'master', false) + visit edit_namespace_project_merge_request_path project.namespace, project, merge_request + fill_in :'merge_request[title]', with: 'test merge request title' + end + + scenario 'user selects "feature-proposal" template' do + select_template 'feature-proposal' + wait_for_ajax + preview_template + save_changes + end + end + + context 'user creates a merge request from a forked project using templates' do + let(:template_content) { 'this is a test "feature-proposal" template' } + let(:fork_user) { create(:user) } + let(:fork_project) { create(:project, :public) } + let(:merge_request) { create(:merge_request, :with_diffs, source_project: fork_project) } + + background do + logout + project.team << [fork_user, :developer] + fork_project.team << [fork_user, :master] + create(:forked_project_link, forked_to_project: fork_project, forked_from_project: project) + login_as fork_user + fork_project.repository.commit_file(fork_user, '.gitlab/merge_request_templates/feature-proposal.md', template_content, 'added merge request template', 'master', false) + visit edit_namespace_project_merge_request_path fork_project.namespace, fork_project, merge_request + fill_in :'merge_request[title]', with: 'test merge request title' + end + + scenario 'user selects "feature-proposal" template' do + select_template 'feature-proposal' + wait_for_ajax + preview_template + save_changes + end + end + + def preview_template + click_link 'Preview' + expect(page).to have_content template_content + end + + def save_changes + click_button "Save changes" + expect(page).to have_content template_content + end + + def select_template(name) + first('.js-issuable-selector').click + first('.js-issuable-selector-wrap .dropdown-content a', text: name).click + end +end diff --git a/spec/features/protected_branches_spec.rb b/spec/features/protected_branches_spec.rb index 3499460c84d..a0ee6cab7ec 100644 --- a/spec/features/protected_branches_spec.rb +++ b/spec/features/protected_branches_spec.rb @@ -71,7 +71,10 @@ feature 'Projected Branches', feature: true, js: true do project.repository.add_branch(user, 'production-stable', 'master') project.repository.add_branch(user, 'staging-stable', 'master') project.repository.add_branch(user, 'development', 'master') - create(:protected_branch, project: project, name: "*-stable") + + visit namespace_project_protected_branches_path(project.namespace, project) + set_protected_branch_name('*-stable') + click_on "Protect" visit namespace_project_protected_branches_path(project.namespace, project) click_on "2 matching branches" @@ -90,13 +93,17 @@ feature 'Projected Branches', feature: true, js: true do visit namespace_project_protected_branches_path(project.namespace, project) set_protected_branch_name('master') within('.new_protected_branch') do - find(".js-allowed-to-push").click - within(".dropdown.open .dropdown-menu") { click_on access_type_name } + allowed_to_push_button = find(".js-allowed-to-push") + + unless allowed_to_push_button.text == access_type_name + allowed_to_push_button.click + within(".dropdown.open .dropdown-menu") { click_on access_type_name } + end end click_on "Protect" expect(ProtectedBranch.count).to eq(1) - expect(ProtectedBranch.last.push_access_level.access_level).to eq(access_type_id) + expect(ProtectedBranch.last.push_access_levels.map(&:access_level)).to eq([access_type_id]) end it "allows updating protected branches so that #{access_type_name} can push to them" do @@ -112,7 +119,7 @@ feature 'Projected Branches', feature: true, js: true do end wait_for_ajax - expect(ProtectedBranch.last.push_access_level.access_level).to eq(access_type_id) + expect(ProtectedBranch.last.push_access_levels.map(&:access_level)).to include(access_type_id) end end @@ -121,13 +128,17 @@ feature 'Projected Branches', feature: true, js: true do visit namespace_project_protected_branches_path(project.namespace, project) set_protected_branch_name('master') within('.new_protected_branch') do - find(".js-allowed-to-merge").click - within(".dropdown.open .dropdown-menu") { click_on access_type_name } + allowed_to_merge_button = find(".js-allowed-to-merge") + + unless allowed_to_merge_button.text == access_type_name + allowed_to_merge_button.click + within(".dropdown.open .dropdown-menu") { click_on access_type_name } + end end click_on "Protect" expect(ProtectedBranch.count).to eq(1) - expect(ProtectedBranch.last.merge_access_level.access_level).to eq(access_type_id) + expect(ProtectedBranch.last.merge_access_levels.map(&:access_level)).to eq([access_type_id]) end it "allows updating protected branches so that #{access_type_name} can merge to them" do @@ -143,7 +154,7 @@ feature 'Projected Branches', feature: true, js: true do end wait_for_ajax - expect(ProtectedBranch.last.merge_access_level.access_level).to eq(access_type_id) + expect(ProtectedBranch.last.merge_access_levels.map(&:access_level)).to include(access_type_id) end end end diff --git a/spec/lib/gitlab/template/gitignore_spec.rb b/spec/lib/gitlab/template/gitignore_template_spec.rb index bc0ec9325cc..9750a012e22 100644 --- a/spec/lib/gitlab/template/gitignore_spec.rb +++ b/spec/lib/gitlab/template/gitignore_template_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Gitlab::Template::Gitignore do +describe Gitlab::Template::GitignoreTemplate do subject { described_class } describe '.all' do @@ -24,7 +24,7 @@ describe Gitlab::Template::Gitignore do it 'returns the Gitignore object of a valid file' do ruby = subject.find('Ruby') - expect(ruby).to be_a Gitlab::Template::Gitignore + expect(ruby).to be_a Gitlab::Template::GitignoreTemplate expect(ruby.name).to eq('Ruby') end end diff --git a/spec/lib/gitlab/template/gitlab_ci_yml_template_spec.rb b/spec/lib/gitlab/template/gitlab_ci_yml_template_spec.rb new file mode 100644 index 00000000000..e3b8321eda3 --- /dev/null +++ b/spec/lib/gitlab/template/gitlab_ci_yml_template_spec.rb @@ -0,0 +1,41 @@ +require 'spec_helper' + +describe Gitlab::Template::GitlabCiYmlTemplate do + subject { described_class } + + describe '.all' do + it 'strips the gitlab-ci suffix' do + expect(subject.all.first.name).not_to end_with('.gitlab-ci.yml') + end + + it 'combines the globals and rest' do + all = subject.all.map(&:name) + + expect(all).to include('Elixir') + expect(all).to include('Docker') + expect(all).to include('Ruby') + end + end + + describe '.find' do + it 'returns nil if the file does not exist' do + expect(subject.find('mepmep-yadida')).to be nil + end + + it 'returns the GitlabCiYml object of a valid file' do + ruby = subject.find('Ruby') + + expect(ruby).to be_a Gitlab::Template::GitlabCiYmlTemplate + expect(ruby.name).to eq('Ruby') + end + end + + describe '#content' do + it 'loads the full file' do + gitignore = subject.new(Rails.root.join('vendor/gitlab-ci-yml/Ruby.gitlab-ci.yml')) + + expect(gitignore.name).to eq 'Ruby' + expect(gitignore.content).to start_with('#') + end + end +end diff --git a/spec/lib/gitlab/template/issue_template_spec.rb b/spec/lib/gitlab/template/issue_template_spec.rb new file mode 100644 index 00000000000..f770857e958 --- /dev/null +++ b/spec/lib/gitlab/template/issue_template_spec.rb @@ -0,0 +1,89 @@ +require 'spec_helper' + +describe Gitlab::Template::IssueTemplate do + subject { described_class } + + let(:user) { create(:user) } + let(:project) { create(:project) } + let(:file_path_1) { '.gitlab/issue_templates/bug.md' } + let(:file_path_2) { '.gitlab/issue_templates/template_test.md' } + let(:file_path_3) { '.gitlab/issue_templates/feature_proposal.md' } + + before do + project.team.add_user(user, Gitlab::Access::MASTER) + project.repository.commit_file(user, file_path_1, "something valid", "test 3", "master", false) + project.repository.commit_file(user, file_path_2, "template_test", "test 1", "master", false) + project.repository.commit_file(user, file_path_3, "feature_proposal", "test 2", "master", false) + end + + describe '.all' do + it 'strips the md suffix' do + expect(subject.all(project).first.name).not_to end_with('.issue_template') + end + + it 'combines the globals and rest' do + all = subject.all(project).map(&:name) + + expect(all).to include('bug') + expect(all).to include('feature_proposal') + expect(all).to include('template_test') + end + end + + describe '.find' do + it 'returns nil if the file does not exist' do + expect { subject.find('mepmep-yadida', project) }.to raise_error(Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError) + end + + it 'returns the issue object of a valid file' do + ruby = subject.find('bug', project) + + expect(ruby).to be_a Gitlab::Template::IssueTemplate + expect(ruby.name).to eq('bug') + end + end + + describe '.by_category' do + it 'return array of templates' do + all = subject.by_category('', project).map(&:name) + expect(all).to include('bug') + expect(all).to include('feature_proposal') + expect(all).to include('template_test') + end + + context 'when repo is bare or empty' do + let(:empty_project) { create(:empty_project) } + before { empty_project.team.add_user(user, Gitlab::Access::MASTER) } + + it "returns empty array" do + templates = subject.by_category('', empty_project) + expect(templates).to be_empty + end + end + end + + describe '#content' do + it 'loads the full file' do + issue_template = subject.new('.gitlab/issue_templates/bug.md', project) + + expect(issue_template.name).to eq 'bug' + expect(issue_template.content).to eq('something valid') + end + + it 'raises error when file is not found' do + issue_template = subject.new('.gitlab/issue_templates/bugnot.md', project) + expect { issue_template.content }.to raise_error(Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError) + end + + context "when repo is empty" do + let(:empty_project) { create(:empty_project) } + + before { empty_project.team.add_user(user, Gitlab::Access::MASTER) } + + it "raises file not found" do + issue_template = subject.new('.gitlab/issue_templates/not_existent.md', empty_project) + expect { issue_template.content }.to raise_error(Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError) + end + end + end +end diff --git a/spec/lib/gitlab/template/merge_request_template_spec.rb b/spec/lib/gitlab/template/merge_request_template_spec.rb new file mode 100644 index 00000000000..bb0f68043fa --- /dev/null +++ b/spec/lib/gitlab/template/merge_request_template_spec.rb @@ -0,0 +1,89 @@ +require 'spec_helper' + +describe Gitlab::Template::MergeRequestTemplate do + subject { described_class } + + let(:user) { create(:user) } + let(:project) { create(:project) } + let(:file_path_1) { '.gitlab/merge_request_templates/bug.md' } + let(:file_path_2) { '.gitlab/merge_request_templates/template_test.md' } + let(:file_path_3) { '.gitlab/merge_request_templates/feature_proposal.md' } + + before do + project.team.add_user(user, Gitlab::Access::MASTER) + project.repository.commit_file(user, file_path_1, "something valid", "test 3", "master", false) + project.repository.commit_file(user, file_path_2, "template_test", "test 1", "master", false) + project.repository.commit_file(user, file_path_3, "feature_proposal", "test 2", "master", false) + end + + describe '.all' do + it 'strips the md suffix' do + expect(subject.all(project).first.name).not_to end_with('.issue_template') + end + + it 'combines the globals and rest' do + all = subject.all(project).map(&:name) + + expect(all).to include('bug') + expect(all).to include('feature_proposal') + expect(all).to include('template_test') + end + end + + describe '.find' do + it 'returns nil if the file does not exist' do + expect { subject.find('mepmep-yadida', project) }.to raise_error(Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError) + end + + it 'returns the merge request object of a valid file' do + ruby = subject.find('bug', project) + + expect(ruby).to be_a Gitlab::Template::MergeRequestTemplate + expect(ruby.name).to eq('bug') + end + end + + describe '.by_category' do + it 'return array of templates' do + all = subject.by_category('', project).map(&:name) + expect(all).to include('bug') + expect(all).to include('feature_proposal') + expect(all).to include('template_test') + end + + context 'when repo is bare or empty' do + let(:empty_project) { create(:empty_project) } + before { empty_project.team.add_user(user, Gitlab::Access::MASTER) } + + it "returns empty array" do + templates = subject.by_category('', empty_project) + expect(templates).to be_empty + end + end + end + + describe '#content' do + it 'loads the full file' do + issue_template = subject.new('.gitlab/merge_request_templates/bug.md', project) + + expect(issue_template.name).to eq 'bug' + expect(issue_template.content).to eq('something valid') + end + + it 'raises error when file is not found' do + issue_template = subject.new('.gitlab/merge_request_templates/bugnot.md', project) + expect { issue_template.content }.to raise_error(Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError) + end + + context "when repo is empty" do + let(:empty_project) { create(:empty_project) } + + before { empty_project.team.add_user(user, Gitlab::Access::MASTER) } + + it "raises file not found" do + issue_template = subject.new('.gitlab/merge_request_templates/not_existent.md', empty_project) + expect { issue_template.content }.to raise_error(Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError) + end + end + end +end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 9c3b4712cab..0a32a486703 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1089,13 +1089,13 @@ describe Project, models: true do let(:project) { create(:project) } it 'returns true when the branch matches a protected branch via direct match' do - project.protected_branches.create!(name: 'foo') + create(:protected_branch, project: project, name: "foo") expect(project.protected_branch?('foo')).to eq(true) end it 'returns true when the branch matches a protected branch via wildcard match' do - project.protected_branches.create!(name: 'production/*') + create(:protected_branch, project: project, name: "production/*") expect(project.protected_branch?('production/some-branch')).to eq(true) end @@ -1105,7 +1105,7 @@ describe Project, models: true do end it 'returns false when the branch does not match a protected branch via wildcard match' do - project.protected_branches.create!(name: 'production/*') + create(:protected_branch, project: project, name: "production/*") expect(project.protected_branch?('staging/some-branch')).to eq(false) end diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb index 9444138f93d..3fd989dd7a6 100644 --- a/spec/requests/api/branches_spec.rb +++ b/spec/requests/api/branches_spec.rb @@ -243,7 +243,7 @@ describe API::API, api: true do end it "removes protected branch" do - project.protected_branches.create(name: branch_name) + create(:protected_branch, project: project, name: branch_name) delete api("/projects/#{project.id}/repository/branches/#{branch_name}", user) expect(response).to have_http_status(405) expect(json_response['message']).to eq('Protected branch cant be removed') diff --git a/spec/requests/api/templates_spec.rb b/spec/requests/api/templates_spec.rb index 68d0f41b489..5bd5b861792 100644 --- a/spec/requests/api/templates_spec.rb +++ b/spec/requests/api/templates_spec.rb @@ -3,50 +3,53 @@ require 'spec_helper' describe API::Templates, api: true do include ApiHelpers - describe 'the Template Entity' do - before { get api('/gitignores/Ruby') } + context 'global templates' do + describe 'the Template Entity' do + before { get api('/gitignores/Ruby') } - it { expect(json_response['name']).to eq('Ruby') } - it { expect(json_response['content']).to include('*.gem') } - end + it { expect(json_response['name']).to eq('Ruby') } + it { expect(json_response['content']).to include('*.gem') } + end - describe 'the TemplateList Entity' do - before { get api('/gitignores') } + describe 'the TemplateList Entity' do + before { get api('/gitignores') } - it { expect(json_response.first['name']).not_to be_nil } - it { expect(json_response.first['content']).to be_nil } - end + it { expect(json_response.first['name']).not_to be_nil } + it { expect(json_response.first['content']).to be_nil } + end - context 'requesting gitignores' do - describe 'GET /gitignores' do - it 'returns a list of available gitignore templates' do - get api('/gitignores') + context 'requesting gitignores' do + describe 'GET /gitignores' do + it 'returns a list of available gitignore templates' do + get api('/gitignores') - expect(response).to have_http_status(200) - expect(json_response).to be_an Array - expect(json_response.size).to be > 15 + expect(response.status).to eq(200) + expect(json_response).to be_an Array + expect(json_response.size).to be > 15 + end end end - end - context 'requesting gitlab-ci-ymls' do - describe 'GET /gitlab_ci_ymls' do - it 'returns a list of available gitlab_ci_ymls' do - get api('/gitlab_ci_ymls') + context 'requesting gitlab-ci-ymls' do + describe 'GET /gitlab_ci_ymls' do + it 'returns a list of available gitlab_ci_ymls' do + get api('/gitlab_ci_ymls') - expect(response).to have_http_status(200) - expect(json_response).to be_an Array - expect(json_response.first['name']).not_to be_nil + expect(response.status).to eq(200) + expect(json_response).to be_an Array + expect(json_response.first['name']).not_to be_nil + end end end - end - describe 'GET /gitlab_ci_ymls/Ruby' do - it 'adds a disclaimer on the top' do - get api('/gitlab_ci_ymls/Ruby') + describe 'GET /gitlab_ci_ymls/Ruby' do + it 'adds a disclaimer on the top' do + get api('/gitlab_ci_ymls/Ruby') - expect(response).to have_http_status(200) - expect(json_response['content']).to start_with("# This file is a template,") + expect(response).to have_http_status(200) + expect(json_response['name']).not_to be_nil + expect(json_response['content']).to start_with("# This file is a template,") + end end end end diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 80f6ebac86c..6ac1fa8f182 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -227,8 +227,8 @@ describe GitPushService, services: true do expect(project.default_branch).to eq("master") execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' ) expect(project.protected_branches).not_to be_empty - expect(project.protected_branches.first.push_access_level.access_level).to eq(Gitlab::Access::MASTER) - expect(project.protected_branches.first.merge_access_level.access_level).to eq(Gitlab::Access::MASTER) + expect(project.protected_branches.first.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::MASTER]) + expect(project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MASTER]) end it "when pushing a branch for the first time with default branch protection disabled" do @@ -249,8 +249,8 @@ describe GitPushService, services: true do execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' ) expect(project.protected_branches).not_to be_empty - expect(project.protected_branches.last.push_access_level.access_level).to eq(Gitlab::Access::DEVELOPER) - expect(project.protected_branches.last.merge_access_level.access_level).to eq(Gitlab::Access::MASTER) + expect(project.protected_branches.last.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) + expect(project.protected_branches.last.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MASTER]) end it "when pushing a branch for the first time with default branch protection set to 'developers can merge'" do @@ -260,8 +260,8 @@ describe GitPushService, services: true do expect(project.default_branch).to eq("master") execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' ) expect(project.protected_branches).not_to be_empty - expect(project.protected_branches.first.push_access_level.access_level).to eq(Gitlab::Access::MASTER) - expect(project.protected_branches.first.merge_access_level.access_level).to eq(Gitlab::Access::DEVELOPER) + expect(project.protected_branches.first.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::MASTER]) + expect(project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) end it "when pushing new commits to existing branch" do |