diff options
52 files changed, 823 insertions, 198 deletions
diff --git a/CHANGELOG b/CHANGELOG index 50dc6b8945b..c19826bc5c1 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -11,6 +11,7 @@ v 7.2.0 - Fix bug when MR download patch return invalid diff - Test gitlab-shell integration - Repository import timeout increased from 2 to 4 minutes allowing larger repos to be imported + - API for labels (Robert Schilling) v 7.1.1 - Fix cpu usage issue in Firefox @@ -178,6 +178,7 @@ gem "gitlab_emoji", "~> 0.0.1.1" gem "gon", '~> 5.0.0' gem 'nprogress-rails' gem 'request_store' +gem "virtus" group :development do gem "annotate", "~> 2.6.0.beta2" diff --git a/Gemfile.lock b/Gemfile.lock index 1b94f063845..500e80ce4ee 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -692,5 +692,6 @@ DEPENDENCIES unicorn (~> 4.6.3) unicorn-worker-killer version_sorter + virtus webmock wikicloth (= 0.8.1) diff --git a/PROCESS.md b/PROCESS.md index a6ff62a9a69..c986013e2f2 100644 --- a/PROCESS.md +++ b/PROCESS.md @@ -34,7 +34,7 @@ The most important thing is making sure valid issues receive feedback from the d ## Workflow labels -Workflow labels are purposely not very detailed since that would be hard to keep updated as you would need to reevaluate them after every comment. We optionally use functional labels on demand when want to group related issues to get an overview (for example all issues related to RVM, to tackle them in one go) and to add details to the issue. +Workflow labels are purposely not very detailed since that would be hard to keep updated as you would need to re-evaluate them after every comment. We optionally use functional labels on demand when want to group related issues to get an overview (for example all issues related to RVM, to tackle them in one go) and to add details to the issue. - *Awaiting feedback*: Feedback pending from the reporter - *Awaiting confirmation of fix*: The issue should already be solved in **master** (generally you can avoid this workflow item and just close the issue right away) @@ -61,7 +61,7 @@ If an issue is complex and needs the attention of a specific person, assignment ## Be kind -Be kind to people trying to contribute. Be aware that people can be a non-native or a native English speaker, they might not understand thing or they might be very sensitive to how your word things. Use emoji to express your feelings (heart, star, smile, etc.). Some good tips about giving feedback to merge requests is in the [Thoughtbot code review guide](https://github.com/thoughtbot/guides/tree/master/code-review). +Be kind to people trying to contribute. Be aware that people may be a non-native English speaker, they might not understand things or they might be very sensitive as to how you word things. Use Emoji to express your feelings (heart, star, smile, etc.). Some good tips about giving feedback to merge requests is in the [Thoughtbot code review guide](https://github.com/thoughtbot/guides/tree/master/code-review). ## Copy & paste responses diff --git a/app/assets/javascripts/application.js.coffee b/app/assets/javascripts/application.js.coffee index 82a810b6551..1960479321c 100644 --- a/app/assets/javascripts/application.js.coffee +++ b/app/assets/javascripts/application.js.coffee @@ -53,15 +53,40 @@ window.split = (val) -> window.extractLast = (term) -> return split( term ).pop() +window.rstrip = (val) -> + return val.replace(/\s+$/, '') + # Disable button if text field is empty window.disableButtonIfEmptyField = (field_selector, button_selector) -> field = $(field_selector) - closest_submit = field.closest("form").find(button_selector) + closest_submit = field.closest('form').find(button_selector) + + closest_submit.disable() if rstrip(field.val()) is "" + + field.on 'input', -> + if rstrip($(@).val()) is "" + closest_submit.disable() + else + closest_submit.enable() + +# Disable button if any input field with given selector is empty +window.disableButtonIfAnyEmptyField = (form, form_selector, button_selector) -> + closest_submit = form.find(button_selector) + empty = false + form.find('input').filter(form_selector).each -> + empty = true if rstrip($(this).val()) is "" + + if empty + closest_submit.disable() + else + closest_submit.enable() - closest_submit.disable() if field.val().replace(/\s+$/, "") is "" + form.keyup -> + empty = false + form.find('input').filter(form_selector).each -> + empty = true if rstrip($(this).val()) is "" - field.on "input", -> - if $(@).val().replace(/\s+$/, "") is "" + if empty closest_submit.disable() else closest_submit.enable() diff --git a/app/assets/javascripts/diff.js.coffee b/app/assets/javascripts/diff.js.coffee new file mode 100644 index 00000000000..dbe00c487dc --- /dev/null +++ b/app/assets/javascripts/diff.js.coffee @@ -0,0 +1,46 @@ +class Diff + UNFOLD_COUNT = 20 + constructor: -> + $(document).on('click', '.js-unfold', (event) => + target = $(event.target) + unfoldBottom = target.hasClass('js-unfold-bottom') + unfold = true + + [old_line, line_number] = @lineNumbers(target.parent()) + offset = line_number - old_line + + if unfoldBottom + line_number += 1 + since = line_number + to = line_number + UNFOLD_COUNT + else + [prev_old_line, prev_new_line] = @lineNumbers(target.parent().prev()) + line_number -= 1 + to = line_number + if line_number - UNFOLD_COUNT > prev_new_line + 1 + since = line_number - UNFOLD_COUNT + else + since = prev_new_line + 1 + unfold = false + + link = target.parents('.diff-file').attr('data-blob-diff-path') + params = + since: since + to: to + bottom: unfoldBottom + offset: offset + unfold: unfold + + $.get(link, params, (response) => + target.parent().replaceWith(response) + ) + ) + + lineNumbers: (line) -> + return ([0, 0]) unless line.children().length + lines = line.children().slice(0, 2) + line_numbers = ($(l).attr('data-linenumber') for l in lines) + (parseInt(line_number) for line_number in line_numbers) + + +@Diff = Diff diff --git a/app/assets/javascripts/dispatcher.js.coffee b/app/assets/javascripts/dispatcher.js.coffee index ff68b520ad6..a463a2eb194 100644 --- a/app/assets/javascripts/dispatcher.js.coffee +++ b/app/assets/javascripts/dispatcher.js.coffee @@ -23,13 +23,21 @@ class Dispatcher new Issue() when 'projects:milestones:show' new Milestone() - when 'projects:issues:new', 'projects:merge_requests:new' + when 'projects:issues:new' GitLab.GfmAutoComplete.setup() + when 'projects:merge_requests:new' + GitLab.GfmAutoComplete.setup() + new Diff() + when 'projects:merge_requests:show' + new Diff() + when "projects:merge_requests:diffs" + new Diff() when 'dashboard:show' new Dashboard() new Activities() when 'projects:commit:show' new Commit() + new Diff() when 'groups:show', 'projects:show' new Activities() when 'projects:new', 'projects:edit' @@ -42,6 +50,8 @@ class Dispatcher new TreeView() when 'projects:blob:show' new BlobView() + when 'projects:labels:new' + new Labels() switch path.first() when 'admin' then new Admin() diff --git a/app/assets/javascripts/labels.js.coffee b/app/assets/javascripts/labels.js.coffee new file mode 100644 index 00000000000..8e53d6929df --- /dev/null +++ b/app/assets/javascripts/labels.js.coffee @@ -0,0 +1,35 @@ +class Labels + constructor: -> + form = $('.label-form') + @setupLabelForm(form) + @cleanBinding() + @addBinding() + @updateColorPreview + + addBinding: -> + $(document).on 'click', '.suggest-colors a', @setSuggestedColor + $(document).on 'input', 'input#label_color', @updateColorPreview + + cleanBinding: -> + $(document).off 'click', '.suggest-colors a' + $(document).off 'input', 'input#label_color' + + # Initializes the form to disable the save button if no color or title is entered + setupLabelForm: (form) -> + disableButtonIfAnyEmptyField form, '.form-control', form.find('.js-save-button') + + # Updates the the preview color with the hex-color input + updateColorPreview: => + previewColor = $('input#label_color').val() + $('div.label-color-preview').css('background-color', previewColor) + + # Updates the preview color with a click on a suggested color + setSuggestedColor: (e) => + color = $(e.currentTarget).data('color') + $('input#label_color').val(color) + @updateColorPreview() + # Notify the form, that color has changed + $('.label-form').trigger('keyup') + e.preventDefault() + +@Labels = Labels diff --git a/app/assets/stylesheets/sections/diff.scss b/app/assets/stylesheets/sections/diff.scss index 88b188dbe8d..488d06919b0 100644 --- a/app/assets/stylesheets/sections/diff.scss +++ b/app/assets/stylesheets/sections/diff.scss @@ -48,6 +48,9 @@ background-color: #8F8; } } + .unfold { + cursor: pointer; + } .file-mode-changed { padding: 10px; diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index db3d173b98d..7009e3b1bc8 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/app/controllers/projects/blob_controller.rb @@ -25,6 +25,21 @@ class Projects::BlobController < Projects::ApplicationController end end + def diff + @form = UnfoldForm.new(params) + @lines = @blob.data.lines[@form.since - 1..@form.to - 1] + + if @form.bottom? + @match_line = '' + else + lines_length = @lines.length - 1 + line = [@form.since, lines_length].join(',') + @match_line = "@@ -#{line}+#{line} @@" + end + + render layout: false + end + private def blob diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index 9a8b3928bf4..f61aa259154 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -232,4 +232,16 @@ module CommitsHelper def diff_file_mode_changed?(diff) diff.a_mode && diff.b_mode && diff.a_mode != diff.b_mode end + + def unfold_bottom_class(bottom) + (bottom) ? 'js-unfold-bottom' : '' + end + + def view_file_btn(commit_sha, diff, project) + link_to project_blob_path(project, tree_join(commit_sha, diff.new_path)), + class: 'btn btn-small view-file js-view-file' do + raw('View file @') + content_tag(:span, commit_sha[0..6], + class: 'commit-short-id') + end + end end diff --git a/app/helpers/labels_helper.rb b/app/helpers/labels_helper.rb index 37f3832e54f..5bfba4f14f2 100644 --- a/app/helpers/labels_helper.rb +++ b/app/helpers/labels_helper.rb @@ -4,7 +4,7 @@ module LabelsHelper end def render_colored_label(label) - label_color = label.color || "#428bca" + label_color = label.color || Label::DEFAULT_COLOR text_color = text_color_for_bg(label_color) content_tag :span, class: 'label color-label', style: "background:#{label_color};color:#{text_color}" do diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 517e4548624..0a5fe24b5af 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -140,7 +140,8 @@ module Issuable def add_labels_by_names(label_names) label_names.each do |label_name| - label = project.labels.find_or_create_by(title: label_name.strip) + label = project.labels.create_with( + color: Label::DEFAULT_COLOR).find_or_create_by(title: label_name.strip) self.labels << label end end diff --git a/app/models/label.rb b/app/models/label.rb index ce982579675..819d6cefa41 100644 --- a/app/models/label.rb +++ b/app/models/label.rb @@ -1,19 +1,24 @@ class Label < ActiveRecord::Base + DEFAULT_COLOR = '#428bca' + belongs_to :project has_many :label_links, dependent: :destroy has_many :issues, through: :label_links, source: :target, source_type: 'Issue' - validates :color, format: { with: /\A\#[0-9A-Fa-f]{6}+\Z/ }, allow_blank: true + validates :color, + format: { with: /\A\#[0-9A-Fa-f]{6}+\Z/ }, + allow_blank: false validates :project, presence: true - # Dont allow '?', '&', and ',' for label titles - validates :title, presence: true, format: { with: /\A[^&\?,&]*\z/ } + # Don't allow '?', '&', and ',' for label titles + validates :title, + presence: true, + format: { with: /\A[^&\?,&]*\z/ }, + uniqueness: { scope: :project_id } scope :order_by_name, -> { reorder("labels.title ASC") } - def name - title - end + alias_attribute :name, :title def open_issues_count issues.opened.count diff --git a/app/models/project.rb b/app/models/project.rb index a24eae7d26b..7f6aa6d4249 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -577,4 +577,8 @@ class Project < ActiveRecord::Base def forks_count ForkedProjectLink.where(forked_from_project_id: self.id).count end + + def find_label(name) + labels.find_by(name: name) + end end diff --git a/app/views/projects/blob/diff.html.haml b/app/views/projects/blob/diff.html.haml new file mode 100644 index 00000000000..cfb91d6568a --- /dev/null +++ b/app/views/projects/blob/diff.html.haml @@ -0,0 +1,19 @@ +- if @lines.present? + - if @form.unfold? && @form.since != 1 && !@form.bottom? + %tr.line_holder{ id: @form.since } + = render "projects/commits/diffs/match_line", {line: @match_line, + line_old: @form.since, line_new: @form.since, bottom: false} + + - @lines.each_with_index do |line, index| + - line_new = index + @form.since + - line_old = line_new - @form.offset + %tr.line_holder + %td.old_line.diff-line-num{data: {linenumber: line_old}} + = link_to raw(line_old), "#" + %td.new_line= link_to raw(line_new) , "#" + %td.line_content.noteable_line= line + + - if @form.unfold? && @form.bottom? && @form.to < @blob.loc + %tr.line_holder{ id: @form.to } + = render "projects/commits/diffs/match_line", {line: @match_line, + line_old: @form.to, line_new: @form.to, bottom: true} diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml index f2b0699f136..2bc9048b2ad 100644 --- a/app/views/projects/commit/_commit_box.html.haml +++ b/app/views/projects/commit/_commit_box.html.haml @@ -55,4 +55,4 @@ = gfm escape_once(@commit.title) - if @commit.description.present? %pre.commit-description - = gfm escape_once(@commit.description) + = preserve(gfm(escape_once(@commit.description))) diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index abe0d4cff46..8e73663939f 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -23,7 +23,7 @@ - if commit.description? .commit-row-description.js-toggle-content %pre - = commit.description + = preserve(gfm(escape_once(commit.description))) .commit-row-info = commit_author_link(commit, avatar: true, size: 16) diff --git a/app/views/projects/commits/_diff_file.html.haml b/app/views/projects/commits/_diff_file.html.haml index 9cbcb84aead..6e6107c8849 100644 --- a/app/views/projects/commits/_diff_file.html.haml +++ b/app/views/projects/commits/_diff_file.html.haml @@ -1,15 +1,15 @@ - file = project.repository.blob_for_diff(@commit, diff) - return unless file -.diff-file{id: "diff-#{i}"} +- blob_diff_path = diff_project_blob_path(project, + tree_join(@commit.id, diff.new_path)) +.diff-file{id: "diff-#{i}", data: {blob_diff_path: blob_diff_path }} .diff-header{id: "file-path-#{hexdigest(diff.new_path || diff.old_path)}"} - if diff.deleted_file %span= diff.old_path .diff-btn-group - if @commit.parent_ids.present? - = link_to project_blob_path(project, tree_join(@commit.parent_id, diff.new_path)), { class: 'btn btn-small view-file' } do - View file @ - %span.commit-short-id= @commit.short_id(6) + = view_file_btn(@commit.parent_id, diff, project) - else %span= diff.new_path - if diff_file_mode_changed?(diff) @@ -26,10 +26,7 @@ Edit - = link_to project_blob_path(project, tree_join(@commit.id, diff.new_path)), { class: 'btn btn-small view-file' } do - View file @ - %span.commit-short-id= @commit.short_id(6) - + = view_file_btn(@commit.id, diff, project) .diff-content -# Skipp all non non-supported blobs diff --git a/app/views/projects/commits/_text_file.html.haml b/app/views/projects/commits/_text_file.html.haml index f5b0d711416..756481c1b21 100644 --- a/app/views/projects/commits/_text_file.html.haml +++ b/app/views/projects/commits/_text_file.html.haml @@ -3,18 +3,20 @@ %a.supp_diff_link Changes suppressed. Click to show %table.text-file{class: "#{'hide' if too_big}"} + - last_line = 0 - each_diff_line(diff, index) do |line, type, line_code, line_new, line_old, raw_line| + - last_line = line_new %tr.line_holder{ id: line_code, class: "#{type}" } - if type == "match" - %td.old_line= "..." - %td.new_line= "..." - %td.line_content.matched= line + = render "projects/commits/diffs/match_line", {line: line, + line_old: line_old, line_new: line_new, bottom: false} - else %td.old_line = link_to raw(type == "new" ? " " : line_old), "##{line_code}", id: line_code - if @comments_allowed = link_to_new_diff_note(line_code) - %td.new_line= link_to raw(type == "old" ? " " : line_new) , "##{line_code}", id: line_code + %td.new_line{data: {linenumber: line_new}} + = link_to raw(type == "old" ? " " : line_new) , "##{line_code}", id: line_code %td.line_content{class: "noteable_line #{type} #{line_code}", "line_code" => line_code}= raw diff_line_content(line) - if @reply_allowed @@ -22,6 +24,10 @@ - unless comments.empty? = render "projects/notes/diff_notes_with_reply", notes: comments, line: line + - if last_line > 0 + = render "projects/commits/diffs/match_line", {line: "", + line_old: last_line, line_new: last_line, bottom: true} + - if diff.diff.blank? && diff_file_mode_changed?(diff) .file-mode-changed File mode changed diff --git a/app/views/projects/commits/diffs/_match_line.html.haml b/app/views/projects/commits/diffs/_match_line.html.haml new file mode 100644 index 00000000000..4ebe3379733 --- /dev/null +++ b/app/views/projects/commits/diffs/_match_line.html.haml @@ -0,0 +1,7 @@ +%td.old_line.diff-line-num.unfold.js-unfold{data: {linenumber: line_old}, + class: unfold_bottom_class(bottom)} + \... +%td.new_line.diff-line-num.unfold.js-unfold{data: {linenumber: line_new}, + class: unfold_bottom_class(bottom)} + \... +%td.line_content.matched= line diff --git a/app/views/projects/labels/_form.html.haml b/app/views/projects/labels/_form.html.haml index 2a5c907febe..72a01e1c271 100644 --- a/app/views/projects/labels/_form.html.haml +++ b/app/views/projects/labels/_form.html.haml @@ -28,22 +28,6 @@ .form-actions - = f.submit 'Save', class: 'btn btn-save' + = f.submit 'Save', class: 'btn btn-save js-save-button' = link_to "Cancel", project_labels_path(@project), class: 'btn btn-cancel' - -:coffeescript - updateColorPreview = -> - previewColor = $('input#label_color').val() - $('div.label-color-preview').css('background-color', previewColor) - - $('.suggest-colors a').on 'click', (e) -> - color = $(this).data("color") - $('input#label_color').val(color) - updateColorPreview() - e.preventDefault() - - $('input#label_color').on 'input', -> - updateColorPreview() - - updateColorPreview() diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 49e35d5bb68..136622c65a2 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -133,7 +133,7 @@ Settings.backup['path'] = File.expand_path(Settings.backup['path'] || "t # Git # Settings['git'] ||= Settingslogic.new({}) -Settings.git['max_size'] ||= 5242880 # 5.megabytes +Settings.git['max_size'] ||= 20971520 # 20.megabytes Settings.git['bin_path'] ||= '/usr/bin/git' Settings.git['timeout'] ||= 10 diff --git a/config/routes.rb b/config/routes.rb index 261fbb50e38..ce66ea99951 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -193,7 +193,9 @@ Gitlab::Application.routes.draw do end scope module: :projects do - resources :blob, only: [:show, :destroy], constraints: {id: /.+/} + resources :blob, only: [:show, :destroy], constraints: { id: /.+/ } do + get :diff, on: :member + end resources :raw, only: [:show], constraints: {id: /.+/} resources :tree, only: [:show], constraints: {id: /.+/, format: /(html|js)/ } resources :edit_tree, only: [:show, :update], constraints: { id: /.+/ }, path: 'edit' do diff --git a/doc/api/README.md b/doc/api/README.md index a0a9ba6f4b6..ababb7b6999 100644 --- a/doc/api/README.md +++ b/doc/api/README.md @@ -12,6 +12,7 @@ - [Branches](branches.md) - [Merge Requests](merge_requests.md) - [Issues](issues.md) +- [Labels](labels.md) - [Milestones](milestones.md) - [Notes](notes.md) (comments) - [Deploy Keys](deploy_keys.md) @@ -30,7 +31,7 @@ ## Introduction -All API requests require authentication. You need to pass a `private_token` parameter by url or header. If passed as header, the header name must be "PRIVATE-TOKEN" (capital and with dash instead of underscore). You can find or reset your private token in your profile. +All API requests require authentication. You need to pass a `private_token` parameter by URL or header. If passed as header, the header name must be "PRIVATE-TOKEN" (capital and with dash instead of underscore). You can find or reset your private token in your profile. If no, or an invalid, `private_token` is provided then an error message will be returned with status code 401: @@ -64,14 +65,14 @@ API request types: - `GET` requests access one or more resources and return the result as JSON - `POST` requests return `201 Created` if the resource is successfully created and return the newly created resource as JSON -- `GET`, `PUT` and `DELETE` return `200 Ok` if the resource is accessed, modified or deleted successfully, the (modified) result is returned as JSON -- `DELETE` requests are designed to be idempotent, meaning a request a resource still returns `200 Ok` even it was deleted before or is not available. The reasoning behind it is the user is not really interested if the resource existed before or not. +- `GET`, `PUT` and `DELETE` return `200 OK` if the resource is accessed, modified or deleted successfully, the (modified) result is returned as JSON +- `DELETE` requests are designed to be idempotent, meaning a request a resource still returns `200 OK` even it was deleted before or is not available. The reasoning behind it is the user is not really interested if the resource existed before or not. The following list shows the possible return codes for API requests. Return values: -- `200 Ok` - The `GET`, `PUT` or `DELETE` request was successful, the resource(s) itself is returned as JSON +- `200 OK` - The `GET`, `PUT` or `DELETE` request was successful, the resource(s) itself is returned as JSON - `201 Created` - The `POST` request was successful and the resource is returned as JSON - `400 Bad Request` - A required attribute of the API request is missing, e.g. the title of an issue is not given - `401 Unauthorized` - The user is not authenticated, a valid user token is necessary, see above diff --git a/doc/api/branches.md b/doc/api/branches.md index 5c460965182..31469b6fe97 100644 --- a/doc/api/branches.md +++ b/doc/api/branches.md @@ -84,7 +84,7 @@ Parameters: ## Protect repository branch Protects a single project repository branch. This is an idempotent function, protecting an already -protected repository branch still returns a `200 Ok` status code. +protected repository branch still returns a `200 OK` status code. ``` PUT /projects/:id/repository/branches/:branch/protect @@ -125,7 +125,7 @@ Parameters: ## Unprotect repository branch Unprotects a single project repository branch. This is an idempotent function, unprotecting an already -unprotected repository branch still returns a `200 Ok` status code. +unprotected repository branch still returns a `200 OK` status code. ``` PUT /projects/:id/repository/branches/:branch/unprotect diff --git a/doc/api/issues.md b/doc/api/issues.md index f775d502a6d..a4b3b3e9918 100644 --- a/doc/api/issues.md +++ b/doc/api/issues.md @@ -157,6 +157,9 @@ Parameters: - `milestone_id` (optional) - The ID of a milestone to assign issue - `labels` (optional) - Comma-separated label names for an issue +If the operation is successful, 200 and the newly created issue is returned. +If an error occurs, an error number and a message explaining the reason is returned. + ## Edit issue Updates an existing project issue. This function is also used to mark an issue as closed. @@ -176,6 +179,9 @@ Parameters: - `labels` (optional) - Comma-separated label names for an issue - `state_event` (optional) - The state event of an issue ('close' to close issue and 'reopen' to reopen it) +If the operation is successful, 200 and the updated issue is returned. +If an error occurs, an error number and a message explaining the reason is returned. + ## Delete existing issue (**Deprecated**) The function is deprecated and returns a `405 Method Not Allowed` error if called. An issue gets now closed and is done by calling `PUT /projects/:id/issues/:issue_id` with parameter `closed` set to 1. diff --git a/doc/api/labels.md b/doc/api/labels.md new file mode 100644 index 00000000000..95fd4e84119 --- /dev/null +++ b/doc/api/labels.md @@ -0,0 +1,85 @@ +# Labels + +## List labels + +Get all labels for given project. + +``` +GET /projects/:id/labels +``` + +```json +[ + { + "name": "Awesome", + "color": "#DD10AA" + }, + { + "name": "Documentation", + "color": "#1E80DD" + }, + { + "name": "Feature", + "color": "#11FF22" + }, + { + "name": "Bug", + "color": "#EE1122" + } +] +``` + +## Create a new label + +Creates a new label for given repository with given name and color. + +``` +POST /projects/:id/labels +``` + +Parameters: + +- `id` (required) - The ID of a project +- `name` (required) - The name of the label +- `color` (required) - Color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB) + +It returns 200 and the newly created label, if the operation succeeds. +If the label already exists, 409 and an error message is returned. +If label parameters are invalid, 405 and an explaining error message is returned. + +## Delete a label + +Deletes a label given by its name. + +``` +DELETE /projects/:id/labels +``` + +- `id` (required) - The ID of a project +- `name` (required) - The name of the label to be deleted + +It returns 200 if the label successfully was deleted, 404 for wrong parameters +and 400 if the label does not exist. +In case of an error, additionally an error message is returned. + +## Edit an existing label + +Updates an existing label with new name or now color. At least one parameter +is required, to update the label. + +``` +PUT /projects/:id/labels +``` + +Parameters: + +- `id` (required) - The ID of a project +- `name` (required) - The name of the existing label +- `new_name` (optional) - The new name of the label +- `color` (optional) - New color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB) + +On success, this method returns 200 with the updated label. +If required parameters are missing, 400 is returned. +If the label to be updated is missing, 404 is returned. +If parameters are invalid, 405 is returned. In case of an error, +additionally an error message is returned. diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index 794d46ed7c9..3616e29ef7c 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -139,6 +139,9 @@ Parameters: } ``` +If the operation is successful, 200 and the newly created merge request is returned. +If an error occurs, an error number and a message explaining the reason is returned. + ## Update MR Updates an existing merge request. You can change branches, title, or even close the MR. @@ -186,17 +189,20 @@ Parameters: } ``` +If the operation is successful, 200 and the updated merge request is returned. +If an error occurs, an error number and a message explaining the reason is returned. + ## Accept MR -Merge changes submitted with MR usign this API. +Merge changes submitted with MR using this API. -If merge success you get 200 OK. +If merge success you get `200 OK`. If it has some conflicts and can not be merged - you get 405 and error message 'Branch cannot be merged' -If merge request is already merged or closed - you get 405 and error message 'Method Not Allowed' +If merge request is already merged or closed - you get 405 and error message 'Method Not Allowed' -If you dont have permissions to accept this merge request - you get 401 +If you don't have permissions to accept this merge request - you'll get a 401 ``` PUT /projects/:id/merge_request/:merge_request_id/merge diff --git a/doc/api/project_snippets.md b/doc/api/project_snippets.md index 47c81b6446c..50e134847c0 100644 --- a/doc/api/project_snippets.md +++ b/doc/api/project_snippets.md @@ -78,7 +78,7 @@ Parameters: ## Delete snippet Deletes an existing project snippet. This is an idempotent function and deleting a non-existent -snippet still returns a `200 Ok` status code. +snippet still returns a `200 OK` status code. ``` DELETE /projects/:id/snippets/:snippet_id diff --git a/doc/api/projects.md b/doc/api/projects.md index b8876e8e104..8995551b9ea 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -1,6 +1,6 @@ # Projects -### List projects +## List projects Get a list of projects accessible by the authenticated user. @@ -10,7 +10,7 @@ GET /projects Parameters: -+ `archived` (optional) - if passed, limit by archived status +- `archived` (optional) - if passed, limit by archived status ```json [ @@ -87,8 +87,7 @@ Parameters: ] ``` - -#### List owned projects +### List owned projects Get a list of projects which are owned by the authenticated user. @@ -96,7 +95,7 @@ Get a list of projects which are owned by the authenticated user. GET /projects/owned ``` -#### List ALL projects +### List ALL projects Get a list of all GitLab projects (admin only). @@ -115,7 +114,7 @@ GET /projects/:id Parameters: -+ `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project +- `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project ```json { @@ -176,7 +175,7 @@ GET /projects/:id/events Parameters: -+ `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project +- `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project ```json [ @@ -238,7 +237,6 @@ Parameters: ] ``` - ### Create project Creates a new project owned by the authenticated user. @@ -249,17 +247,16 @@ POST /projects Parameters: -+ `name` (required) - new project name -+ `namespace_id` (optional) - namespace for the new project (defaults to user) -+ `description` (optional) - short project description -+ `issues_enabled` (optional) -+ `merge_requests_enabled` (optional) -+ `wiki_enabled` (optional) -+ `snippets_enabled` (optional) -+ `public` (optional) - if `true` same as setting visibility_level = 20 -+ `visibility_level` (optional) -* `import_url` (optional) - +- `name` (required) - new project name +- `namespace_id` (optional) - namespace for the new project (defaults to user) +- `description` (optional) - short project description +- `issues_enabled` (optional) +- `merge_requests_enabled` (optional) +- `wiki_enabled` (optional) +- `snippets_enabled` (optional) +- `public` (optional) - if `true` same as setting visibility_level = 20 +- `visibility_level` (optional) +- `import_url` (optional) ### Create project for user @@ -271,20 +268,19 @@ POST /projects/user/:user_id Parameters: -+ `user_id` (required) - user_id of owner -+ `name` (required) - new project name -+ `description` (optional) - short project description -+ `default_branch` (optional) - 'master' by default -+ `issues_enabled` (optional) -+ `merge_requests_enabled` (optional) -+ `wiki_enabled` (optional) -+ `snippets_enabled` (optional) -+ `public` (optional) - if `true` same as setting visibility_level = 20 -+ `visibility_level` (optional) -* `import_url` (optional) +- `user_id` (required) - user_id of owner +- `name` (required) - new project name +- `description` (optional) - short project description +- `default_branch` (optional) - 'master' by default +- `issues_enabled` (optional) +- `merge_requests_enabled` (optional) +- `wiki_enabled` (optional) +- `snippets_enabled` (optional) +- `public` (optional) - if `true` same as setting visibility_level = 20 +- `visibility_level` (optional) +- `import_url` (optional) - -## Remove project +### Remove project Removes a project including all associated resources (issues, merge requests etc.) @@ -294,8 +290,7 @@ DELETE /projects/:id Parameters: -+ `id` (required) - The ID of a project - +- `id` (required) - The ID of a project ## Team members @@ -309,9 +304,8 @@ GET /projects/:id/members Parameters: -+ `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project -+ `query` (optional) - Query string to search for members - +- `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project +- `query` (optional) - Query string to search for members ### Get project team member @@ -323,8 +317,8 @@ GET /projects/:id/members/:user_id Parameters: -+ `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project -+ `user_id` (required) - The ID of a user +- `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project +- `user_id` (required) - The ID of a user ```json { @@ -338,7 +332,6 @@ Parameters: } ``` - ### Add project team member Adds a user to a project team. This is an idempotent method and can be called multiple times @@ -351,10 +344,9 @@ POST /projects/:id/members Parameters: -+ `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project -+ `user_id` (required) - The ID of a user to add -+ `access_level` (required) - Project access level - +- `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project +- `user_id` (required) - The ID of a user to add +- `access_level` (required) - Project access level ### Edit project team member @@ -366,10 +358,9 @@ PUT /projects/:id/members/:user_id Parameters: -+ `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project -+ `user_id` (required) - The ID of a team member -+ `access_level` (required) - Project access level - +- `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project +- `user_id` (required) - The ID of a team member +- `access_level` (required) - Project access level ### Remove project team member @@ -381,15 +372,14 @@ DELETE /projects/:id/members/:user_id Parameters: -+ `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project -+ `user_id` (required) - The ID of a team member +- `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project +- `user_id` (required) - The ID of a team member This method is idempotent and can be called multiple times with the same parameters. Revoking team membership for a user who is not currently a team member is considered success. Please note that the returned JSON currently differs slightly. Thus you should not rely on the returned JSON structure. - ## Hooks ### List project hooks @@ -402,8 +392,7 @@ GET /projects/:id/hooks Parameters: -+ `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project - +- `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project ### Get project hook @@ -415,8 +404,8 @@ GET /projects/:id/hooks/:hook_id Parameters: -+ `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project -+ `hook_id` (required) - The ID of a project hook +- `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project +- `hook_id` (required) - The ID of a project hook ```json { @@ -430,7 +419,6 @@ Parameters: } ``` - ### Add project hook Adds a hook to a specified project. @@ -441,12 +429,11 @@ POST /projects/:id/hooks Parameters: -+ `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project -+ `url` (required) - The hook URL -+ `push_events` - Trigger hook on push events -+ `issues_events` - Trigger hook on issues events -+ `merge_requests_events` - Trigger hook on merge_requests events - +- `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project +- `url` (required) - The hook URL +- `push_events` - Trigger hook on push events +- `issues_events` - Trigger hook on issues events +- `merge_requests_events` - Trigger hook on merge_requests events ### Edit project hook @@ -458,13 +445,12 @@ PUT /projects/:id/hooks/:hook_id Parameters: -+ `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project -+ `hook_id` (required) - The ID of a project hook -+ `url` (required) - The hook URL -+ `push_events` - Trigger hook on push events -+ `issues_events` - Trigger hook on issues events -+ `merge_requests_events` - Trigger hook on merge_requests events - +- `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project +- `hook_id` (required) - The ID of a project hook +- `url` (required) - The hook URL +- `push_events` - Trigger hook on push events +- `issues_events` - Trigger hook on issues events +- `merge_requests_events` - Trigger hook on merge_requests events ### Delete project hook @@ -477,13 +463,12 @@ DELETE /projects/:id/hooks/:hook_id Parameters: -+ `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project -+ `hook_id` (required) - The ID of hook to delete +- `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project +- `hook_id` (required) - The ID of hook to delete Note the JSON response differs if the hook is available or not. If the project hook is available before it is returned in the JSON response or an empty response is returned. - ## Branches ### List branches @@ -496,7 +481,7 @@ GET /projects/:id/repository/branches Parameters: -+ `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project +- `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project ```json [ @@ -561,9 +546,8 @@ GET /projects/:id/repository/branches/:branch Parameters: -+ `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project -+ `branch` (required) - The name of the branch. - +- `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project +- `branch` (required) - The name of the branch. ### Protect single branch @@ -575,9 +559,8 @@ PUT /projects/:id/repository/branches/:branch/protect Parameters: -+ `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project -+ `branch` (required) - The name of the branch. - +- `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project +- `branch` (required) - The name of the branch. ### Unprotect single branch @@ -589,9 +572,8 @@ PUT /projects/:id/repository/branches/:branch/unprotect Parameters: -+ `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project -+ `branch` (required) - The name of the branch. - +- `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project +- `branch` (required) - The name of the branch. ## Admin fork relation @@ -605,8 +587,8 @@ POST /projects/:id/fork/:forked_from_id Parameters: -+ `id` (required) - The ID of the project -+ `forked_from_id:` (required) - The ID of the project that was forked from +- `id` (required) - The ID of the project +- `forked_from_id:` (required) - The ID of the project that was forked from ### Delete an existing forked from relationship @@ -616,8 +598,7 @@ DELETE /projects/:id/fork Parameter: -+ `id` (required) - The ID of the project - +- `id` (required) - The ID of the project ## Search for projects by name @@ -629,32 +610,6 @@ GET /projects/search/:query Parameters: -+ query (required) - A string contained in the project name -+ per_page (optional) - number of projects to return per page -+ page (optional) - the page to retrieve - - -## Labels - -### List project labels - -Get a list of project labels. - -``` -GET /projects/:id/labels -``` - -Parameters: - -+ `id` (required) - The ID or NAMESPACE/PROJECT_NAME of a project - -```json -[ - { - "name": "feature" - }, - { - "name": "bug" - } -] -``` +- query (required) - A string contained in the project name +- per_page (optional) - number of projects to return per page +- page (optional) - the page to retrieve diff --git a/doc/api/system_hooks.md b/doc/api/system_hooks.md index 6483a73c7ec..f9637d8a6c4 100644 --- a/doc/api/system_hooks.md +++ b/doc/api/system_hooks.md @@ -59,7 +59,7 @@ Parameters: ## Delete system hook -Deletes a system hook. This is an idempotent API function and returns `200 Ok` even if the hook is not available. If the hook is deleted it is also returned as JSON. +Deletes a system hook. This is an idempotent API function and returns `200 OK` even if the hook is not available. If the hook is deleted it is also returned as JSON. ``` DELETE /hooks/:id diff --git a/doc/api/users.md b/doc/api/users.md index 57078353fd0..3fdd3a75e88 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -202,7 +202,7 @@ e.g. when renaming the email address to some existing one. Deletes a user. Available only for administrators. This is an idempotent function, calling this function for a non-existent user id -still returns a status code `200 Ok`. +still returns a status code `200 OK`. The JSON response differs if the user was actually deleted or not. In the former the user is returned and in the latter not. @@ -336,7 +336,7 @@ Will return created key with status `201 Created` on success, or `404 Not found` Deletes key owned by currently authenticated user. This is an idempotent function and calling it on a key that is already deleted -or not available results in `200 Ok`. +or not available results in `200 OK`. ``` DELETE /user/keys/:id @@ -359,4 +359,4 @@ Parameters: - `uid` (required) - id of specified user - `id` (required) - SSH key ID -Will return `200 Ok` on success, or `404 Not found` if either user or key cannot be found. +Will return `200 OK` on success, or `404 Not found` if either user or key cannot be found. diff --git a/doc/permissions/permissions.md b/doc/permissions/permissions.md index 29fe521b4d1..db22b7dbe56 100644 --- a/doc/permissions/permissions.md +++ b/doc/permissions/permissions.md @@ -23,6 +23,7 @@ If a user is a GitLab administrator they receive all permissions. | Add tags | | | ✓ | ✓ | ✓ | | Write a wiki | | | ✓ | ✓ | ✓ | | Manage issue tracker | | | ✓ | ✓ | ✓ | +| Manage labels | | | ✓ | ✓ | ✓ | | Create new milestones | | | | ✓ | ✓ | | Add new team members | | | | ✓ | ✓ | | Push to protected branches | | | | ✓ | ✓ | diff --git a/features/project/issues/labels.feature b/features/project/issues/labels.feature index 4a37b6dc9fa..29cf5307271 100644 --- a/features/project/issues/labels.feature +++ b/features/project/issues/labels.feature @@ -10,7 +10,7 @@ Feature: Project Labels And I should see label "feature" Scenario: I create new label - Given I visit new label page + Given I visit project "Shop" new label page When I submit new label 'support' Then I should see label 'support' @@ -23,3 +23,21 @@ Feature: Project Labels Scenario: I remove label When I remove label 'bug' Then I should not see label 'bug' + + Scenario: I create a label with invalid color + Given I visit project "Shop" new label page + When I submit new label with invalid color + Then I should see label color error message + + Scenario: I create a label that already exists + Given I visit project "Shop" new label page + When I submit new label 'bug' + Then I should see label label exist error message + + Scenario: I create the same label on another project + Given I own project "Forum" + And I visit project "Forum" labels page + And I visit project "Forum" new label page + When I submit new label 'bug' + Then I should see label 'bug' + diff --git a/features/project/merge_requests.feature b/features/project/merge_requests.feature index d4c71ba336e..8b6c296dfe6 100644 --- a/features/project/merge_requests.feature +++ b/features/project/merge_requests.feature @@ -139,3 +139,11 @@ Feature: Project Merge Requests And I click link "Show inline discussion" of the second file Then I should see a comment like "Line is wrong" in the second file And I should still see a comment like "Line is correct" in the first file + + @javascript + Scenario: I unfold diff + Given project "Shop" have "Bug NS-05" open merge request with diffs inside + And I visit merge request page "Bug NS-05" + And I switch to the diff tab + And I unfold diff + Then I should see additional file lines diff --git a/features/steps/project/labels.rb b/features/steps/project/labels.rb index 3d9aa29299c..8320405e096 100644 --- a/features/steps/project/labels.rb +++ b/features/steps/project/labels.rb @@ -31,6 +31,36 @@ class ProjectLabels < Spinach::FeatureSteps click_button 'Save' end + step 'I submit new label \'bug\'' do + fill_in 'Title', with: 'bug' + fill_in 'Background Color', with: '#F95610' + click_button 'Save' + end + + step 'I submit new label with invalid color' do + fill_in 'Title', with: 'support' + fill_in 'Background Color', with: '#12' + click_button 'Save' + end + + step 'I should see label label exist error message' do + within '.label-form' do + page.should have_content 'Title has already been taken' + end + end + + step 'I should see label color error message' do + within '.label-form' do + page.should have_content 'Color is invalid' + end + end + + step 'I should see label \'bug\'' do + within '.manage-labels-list' do + page.should have_content 'bug' + end + end + step 'I should not see label \'bug\'' do within '.manage-labels-list' do page.should_not have_content 'bug' diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index f0007a039e4..05d3e5067c5 100644 --- a/features/steps/project/merge_requests.rb +++ b/features/steps/project/merge_requests.rb @@ -242,6 +242,14 @@ class ProjectMergeRequests < Spinach::FeatureSteps end end + step 'I unfold diff' do + first('.js-unfold').click + end + + step 'I should see additional file lines' do + expect(first('.text-file')).to have_content('.bundle') + end + def project @project ||= Project.find_by!(name: "Shop") end diff --git a/features/steps/shared/paths.rb b/features/steps/shared/paths.rb index 4e97dba20b3..0d06383509f 100644 --- a/features/steps/shared/paths.rb +++ b/features/steps/shared/paths.rb @@ -287,10 +287,22 @@ module SharedPaths end step 'I visit project "Shop" labels page' do + project = Project.find_by(name: 'Shop') visit project_labels_path(project) end - step 'I visit new label page' do + step 'I visit project "Forum" labels page' do + project = Project.find_by(name: 'Forum') + visit project_labels_path(project) + end + + step 'I visit project "Shop" new label page' do + project = Project.find_by(name: 'Shop') + visit new_project_label_path(project) + end + + step 'I visit project "Forum" new label page' do + project = Project.find_by(name: 'Forum') visit new_project_label_path(project) end diff --git a/lib/api/api.rb b/lib/api/api.rb index ce4cc8b34f7..2c7cd9038c3 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -46,5 +46,6 @@ module API mount Commits mount Namespaces mount Branches + mount Labels end end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index fc7d391fd30..74fdef93543 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -194,7 +194,7 @@ module API end class Label < Grape::Entity - expose :name + expose :name, :color end class RepoDiff < Grape::Entity diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 8189e433789..d36b29a00b1 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -112,6 +112,21 @@ module API ActionController::Parameters.new(attrs).permit! end + # Helper method for validating all labels against its names + def validate_label_params(params) + if params[:labels].present? + params[:labels].split(',').each do |label_name| + label = user_project.labels.create_with( + color: Label::DEFAULT_COLOR).find_or_initialize_by( + title: label_name.strip) + if label.invalid? + return true + end + end + end + false + end + # error helpers def forbidden! diff --git a/lib/api/issues.rb b/lib/api/issues.rb index b29118b2fd8..055529ccbd8 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -51,12 +51,18 @@ module API required_attributes! [:title] attrs = attributes_for_keys [:title, :description, :assignee_id, :milestone_id] + # Validate label names in advance + if validate_label_params(params) + return render_api_error!('Label names invalid', 405) + end + issue = ::Issues::CreateService.new(user_project, current_user, attrs).execute if issue.valid? - # Find or create labels and attach to issue + # Find or create labels and attach to issue. Labels are valid because + # we already checked its name, so there can't be an error here if params[:labels].present? - issue.add_labels_by_names(params[:labels].split(",")) + issue.add_labels_by_names(params[:labels].split(',')) end present issue, with: Entities::Issue @@ -83,12 +89,19 @@ module API authorize! :modify_issue, issue attrs = attributes_for_keys [:title, :description, :assignee_id, :milestone_id, :state_event] + # Validate label names in advance + if validate_label_params(params) + return render_api_error!('Label names invalid', 405) + end + issue = ::Issues::UpdateService.new(user_project, current_user, attrs).execute(issue) if issue.valid? - # Find or create labels and attach to issue + # Find or create labels and attach to issue. Labels are valid because + # we already checked its name, so there can't be an error here if params[:labels].present? - issue.add_labels_by_names(params[:labels].split(",")) + # Create and add labels to the new created issue + issue.add_labels_by_names(params[:labels].split(',')) end present issue, with: Entities::Issue diff --git a/lib/api/labels.rb b/lib/api/labels.rb new file mode 100644 index 00000000000..d1684b2293c --- /dev/null +++ b/lib/api/labels.rb @@ -0,0 +1,103 @@ +module API + # Labels API + class Labels < Grape::API + before { authenticate! } + + resource :projects do + # Get all labels of the project + # + # Parameters: + # id (required) - The ID of a project + # Example Request: + # GET /projects/:id/labels + get ':id/labels' do + present user_project.labels, with: Entities::Label + end + + # Creates a new label + # + # Parameters: + # id (required) - The ID of a project + # name (required) - The name of the label to be deleted + # color (required) - Color of the label given in 6-digit hex + # notation with leading '#' sign (e.g. #FFAABB) + # Example Request: + # POST /projects/:id/labels + post ':id/labels' do + authorize! :admin_label, user_project + required_attributes! [:name, :color] + + attrs = attributes_for_keys [:name, :color] + label = user_project.find_label(attrs[:name]) + + if label + return render_api_error!('Label already exists', 409) + end + + label = user_project.labels.create(attrs) + + if label.valid? + present label, with: Entities::Label + else + render_api_error!(label.errors.full_messages.join(', '), 405) + end + end + + # Deletes an existing label + # + # Parameters: + # id (required) - The ID of a project + # name (required) - The name of the label to be deleted + # + # Example Request: + # DELETE /projects/:id/labels + delete ':id/labels' do + authorize! :admin_label, user_project + required_attributes! [:name] + + label = user_project.find_label(params[:name]) + if !label + return render_api_error!('Label not found', 404) + end + + label.destroy + end + + # Updates an existing label. At least one optional parameter is required. + # + # Parameters: + # id (required) - The ID of a project + # name (optional) - The name of the label to be deleted + # color (optional) - Color of the label given in 6-digit hex + # notation with leading '#' sign (e.g. #FFAABB) + # Example Request: + # PUT /projects/:id/labels + put ':id/labels' do + authorize! :admin_label, user_project + required_attributes! [:name] + + label = user_project.find_label(params[:name]) + if !label + return render_api_error!('Label not found', 404) + end + + attrs = attributes_for_keys [:new_name, :color] + + if attrs.empty? + return render_api_error!('Required parameters "name" or "color" ' \ + 'missing', + 400) + end + + # Rename new name to the actual label attribute name + attrs[:name] = attrs.delete(:new_name) if attrs.key?(:new_name) + + if label.update(attrs) + present label, with: Entities::Label + else + render_api_error!(label.errors.full_messages.join(', '), 405) + end + end + end + end +end diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index acca7cb6bad..0d765f9280e 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -76,6 +76,12 @@ module API authorize! :write_merge_request, user_project required_attributes! [:source_branch, :target_branch, :title] attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :target_project_id, :description] + + # Validate label names in advance + if validate_label_params(params) + return render_api_error!('Label names invalid', 405) + end + merge_request = ::MergeRequests::CreateService.new(user_project, current_user, attrs).execute if merge_request.valid? @@ -109,6 +115,12 @@ module API attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :state_event, :description] merge_request = user_project.merge_requests.find(params[:merge_request_id]) authorize! :modify_merge_request, merge_request + + # Validate label names in advance + if validate_label_params(params) + return render_api_error!('Label names invalid', 405) + end + merge_request = ::MergeRequests::UpdateService.new(user_project, current_user, attrs).execute(merge_request) if merge_request.valid? diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 149678e6803..55f7975bbf7 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -224,17 +224,6 @@ module API @users = paginate @users present @users, with: Entities::UserBasic end - - # Get a project labels - # - # Parameters: - # id (required) - The ID of a project - # Example Request: - # GET /projects/:id/labels - get ':id/labels' do - @labels = user_project.labels - present @labels, with: Entities::Label - end end end end diff --git a/lib/gitlab/diff_parser.rb b/lib/gitlab/diff_parser.rb index 14bbb328637..b244295027e 100644 --- a/lib/gitlab/diff_parser.rb +++ b/lib/gitlab/diff_parser.rb @@ -30,7 +30,7 @@ module Gitlab line_new = line.match(/\+[0-9]*/)[0].to_i.abs rescue 0 next if line_old == 1 && line_new == 1 #top of file - yield(full_line, type, nil, nil, nil) + yield(full_line, type, nil, line_new, line_old) next else type = identification_type(line) diff --git a/lib/gt_one_coercion.rb b/lib/gt_one_coercion.rb new file mode 100644 index 00000000000..ef2dc09767c --- /dev/null +++ b/lib/gt_one_coercion.rb @@ -0,0 +1,5 @@ +class GtOneCoercion < Virtus::Attribute + def coerce(value) + [1, value.to_i].max + end +end diff --git a/lib/unfold_form.rb b/lib/unfold_form.rb new file mode 100644 index 00000000000..46b12beeaaf --- /dev/null +++ b/lib/unfold_form.rb @@ -0,0 +1,11 @@ +require_relative 'gt_one_coercion' + +class UnfoldForm + include Virtus.model + + attribute :since, GtOneCoercion + attribute :to, GtOneCoercion + attribute :bottom, Boolean + attribute :unfold, Boolean, default: true + attribute :offset, Integer +end diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index dff7f20cb32..d8e8e4f5035 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -5,6 +5,10 @@ describe API::API, api: true do let(:user) { create(:user) } let!(:project) { create(:project, namespace: user.namespace ) } let!(:issue) { create(:issue, author: user, assignee: user, project: project) } + let!(:label) do + create(:label, title: 'label', color: '#FFAABB', project: project) + end + before { project.team << [user, :reporter] } describe "GET /issues" do @@ -68,6 +72,14 @@ describe API::API, api: true do post api("/projects/#{project.id}/issues", user), labels: 'label, label2' response.status.should == 400 end + + it 'should return 405 on invalid label names' do + post api("/projects/#{project.id}/issues", user), + title: 'new issue', + labels: 'label, ?' + response.status.should == 405 + json_response['message'].should == 'Label names invalid' + end end describe "PUT /projects/:id/issues/:issue_id to update only title" do @@ -84,6 +96,14 @@ describe API::API, api: true do title: 'updated title' response.status.should == 404 end + + it 'should return 405 on invalid label names' do + put api("/projects/#{project.id}/issues/#{issue.id}", user), + title: 'updated title', + labels: 'label, ?' + response.status.should == 405 + json_response['message'].should == 'Label names invalid' + end end describe "PUT /projects/:id/issues/:issue_id to update state and label" do diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb index d40c2c21cec..b06b353333d 100644 --- a/spec/requests/api/labels_spec.rb +++ b/spec/requests/api/labels_spec.rb @@ -21,4 +21,131 @@ describe API::API, api: true do json_response.first['name'].should == label1.name end end + + describe 'POST /projects/:id/labels' do + it 'should return created label' do + post api("/projects/#{project.id}/labels", user), + name: 'Foo', + color: '#FFAABB' + response.status.should == 201 + json_response['name'].should == 'Foo' + json_response['color'].should == '#FFAABB' + end + + it 'should return a 400 bad request if name not given' do + post api("/projects/#{project.id}/labels", user), color: '#FFAABB' + response.status.should == 400 + end + + it 'should return a 400 bad request if color not given' do + post api("/projects/#{project.id}/labels", user), name: 'Foobar' + response.status.should == 400 + end + + it 'should return 405 for invalid color' do + post api("/projects/#{project.id}/labels", user), + name: 'Foo', + color: '#FFAA' + response.status.should == 405 + json_response['message'].should == 'Color is invalid' + end + + it 'should return 405 for invalid name' do + post api("/projects/#{project.id}/labels", user), + name: '?', + color: '#FFAABB' + response.status.should == 405 + json_response['message'].should == 'Title is invalid' + end + + it 'should return 409 if label already exists' do + post api("/projects/#{project.id}/labels", user), + name: 'label1', + color: '#FFAABB' + response.status.should == 409 + json_response['message'].should == 'Label already exists' + end + end + + describe 'DELETE /projects/:id/labels' do + it 'should return 200 for existing label' do + delete api("/projects/#{project.id}/labels", user), name: 'label1' + response.status.should == 200 + end + + it 'should return 404 for non existing label' do + delete api("/projects/#{project.id}/labels", user), name: 'label2' + response.status.should == 404 + json_response['message'].should == 'Label not found' + end + + it 'should return 400 for wrong parameters' do + delete api("/projects/#{project.id}/labels", user) + response.status.should == 400 + end + end + + describe 'PUT /projects/:id/labels' do + it 'should return 200 if name and colors are changed' do + put api("/projects/#{project.id}/labels", user), + name: 'label1', + new_name: 'New Label', + color: '#FFFFFF' + response.status.should == 200 + json_response['name'].should == 'New Label' + json_response['color'].should == '#FFFFFF' + end + + it 'should return 200 if name is changed' do + put api("/projects/#{project.id}/labels", user), + name: 'label1', + new_name: 'New Label' + response.status.should == 200 + json_response['name'].should == 'New Label' + json_response['color'].should == label1.color + end + + it 'should return 200 if colors is changed' do + put api("/projects/#{project.id}/labels", user), + name: 'label1', + color: '#FFFFFF' + response.status.should == 200 + json_response['name'].should == label1.name + json_response['color'].should == '#FFFFFF' + end + + it 'should return 404 if label does not exist' do + put api("/projects/#{project.id}/labels", user), + name: 'label2', + new_name: 'label3' + response.status.should == 404 + end + + it 'should return 400 if no label name given' do + put api("/projects/#{project.id}/labels", user), new_name: 'label2' + response.status.should == 400 + end + + it 'should return 400 if no new parameters given' do + put api("/projects/#{project.id}/labels", user), name: 'label1' + response.status.should == 400 + end + + it 'should return 405 for invalid name' do + put api("/projects/#{project.id}/labels", user), + name: 'label1', + new_name: '?', + color: '#FFFFFF' + response.status.should == 405 + json_response['message'].should == 'Title is invalid' + end + + it 'should return 405 for invalid name' do + put api("/projects/#{project.id}/labels", user), + name: 'label1', + color: '#FF' + response.status.should == 405 + json_response['message'].should == 'Color is invalid' + end + end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 3611d9d6dc3..58cf7f139dc 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -78,9 +78,14 @@ describe API::API, api: true do context 'between branches projects' do it "should return merge_request" do post api("/projects/#{project.id}/merge_requests", user), - title: 'Test merge_request', source_branch: "stable", target_branch: "master", author: user + title: 'Test merge_request', + source_branch: 'stable', + target_branch: 'master', + author: user, + labels: 'label, label2' response.status.should == 201 json_response['title'].should == 'Test merge_request' + json_response['labels'].should == ['label', 'label2'] end it "should return 422 when source_branch equals target_branch" do @@ -106,6 +111,17 @@ describe API::API, api: true do target_branch: 'master', source_branch: 'stable' response.status.should == 400 end + + it 'should return 405 on invalid label names' do + post api("/projects/#{project.id}/merge_requests", user), + title: 'Test merge_request', + source_branch: 'stable', + target_branch: 'master', + author: user, + labels: 'label, ?' + response.status.should == 405 + json_response['message'].should == 'Label names invalid' + end end context 'forked projects' do @@ -235,6 +251,15 @@ describe API::API, api: true do response.status.should == 200 json_response['target_branch'].should == 'wiki' end + + it 'should return 405 on invalid label names' do + put api("/projects/#{project.id}/merge_request/#{merge_request.id}", + user), + title: 'new issue', + labels: 'label, ?' + response.status.should == 405 + json_response['message'].should == 'Label names invalid' + end end describe "POST /projects/:id/merge_request/:merge_request_id/comments" do |