diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 0a5fe24b5af5c57dca858ac0a8253c56baff7db8..5c9b44812be654dbb1ffa3eb42a91ca7e839f0de 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -138,6 +138,10 @@ module Issuable
labels.order('title ASC').pluck(:title)
end
+ def remove_labels
+ labels.delete_all
+ end
+
def add_labels_by_names(label_names)
label_names.each do |label_name|
label = project.labels.create_with(
diff --git a/app/models/label.rb b/app/models/label.rb
index 3ff52416c24f8ae2770b4f3eba214b016173618e..233f477444fb0f05ffffed041f31b8e30c2fc9d9 100644
--- a/app/models/label.rb
+++ b/app/models/label.rb
@@ -6,14 +6,14 @@ class Label < ActiveRecord::Base
has_many :issues, through: :label_links, source: :target, source_type: 'Issue'
validates :color,
- format: { with: /\A\#[0-9A-Fa-f]{6}+\Z/ },
+ format: { with: /\A#[0-9A-Fa-f]{6}\Z/ },
allow_blank: false
validates :project, presence: true
# Don't allow '?', '&', and ',' for label titles
validates :title,
presence: true,
- format: { with: /\A[^&\?,&]*\z/ },
+ format: { with: /\A[^&\?,&]+\z/ },
uniqueness: { scope: :project_id }
scope :order_by_name, -> { reorder("labels.title ASC") }
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index d36b29a00b1c8dd4dd16c1525ca8daf4b69c5b02..6af0f6d1b252b3a296bb289779b771511dee55ca 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -114,17 +114,21 @@ module API
# Helper method for validating all labels against its names
def validate_label_params(params)
+ errors = {}
+
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
+ errors[label.title] = label.errors
end
end
end
- false
+
+ errors
end
# error helpers
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index 055529ccbd89a10ff8f42697f3f1741ac336acb2..eb6a74cd2bc21be5944d090ab5f57e9dd95635c4 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -52,8 +52,8 @@ module API
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)
+ if (errors = validate_label_params(params)).any?
+ render_api_error!({ labels: errors }, 400)
end
issue = ::Issues::CreateService.new(user_project, current_user, attrs).execute
@@ -90,8 +90,8 @@ module API
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)
+ if (errors = validate_label_params(params)).any?
+ render_api_error!({ labels: errors }, 400)
end
issue = ::Issues::UpdateService.new(user_project, current_user, attrs).execute(issue)
@@ -99,7 +99,8 @@ module API
if issue.valid?
# 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?
+ unless params[:labels].nil?
+ issue.remove_labels
# Create and add labels to the new created issue
issue.add_labels_by_names(params[:labels].split(','))
end
diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index d8e8e4f5035e54958b3a6680c98487e11c40467e..08dc94ebdf3f4994e18b5ef6f46c43fe277114cd 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/spec/requests/api/issues_spec.rb
@@ -73,12 +73,12 @@ describe API::API, api: true do
response.status.should == 400
end
- it 'should return 405 on invalid label names' do
+ it 'should return 400 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'
+ response.status.should == 400
+ json_response['message']['labels']['?']['title'].should == ['is invalid']
end
end
@@ -97,12 +97,56 @@ describe API::API, api: true do
response.status.should == 404
end
- it 'should return 405 on invalid label names' do
+ it 'should return 400 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'
+ response.status.should == 400
+ json_response['message']['labels']['?']['title'].should == ['is invalid']
+ end
+ end
+
+ describe 'PUT /projects/:id/issues/:issue_id to update labels' do
+ let!(:label) { create(:label, title: 'dummy', project: project) }
+ let!(:label_link) { create(:label_link, label: label, target: issue) }
+
+ it 'should not update labels if not present' do
+ put api("/projects/#{project.id}/issues/#{issue.id}", user),
+ title: 'updated title'
+ response.status.should == 200
+ json_response['labels'].should == [label.title]
+ end
+
+ it 'should remove all labels' do
+ put api("/projects/#{project.id}/issues/#{issue.id}", user),
+ labels: ''
+ response.status.should == 200
+ json_response['labels'].should == []
+ end
+
+ it 'should update labels' do
+ put api("/projects/#{project.id}/issues/#{issue.id}", user),
+ labels: 'foo,bar'
+ response.status.should == 200
+ json_response['labels'].should include 'foo'
+ json_response['labels'].should include 'bar'
+ end
+
+ it 'should return 400 on invalid label names' do
+ put api("/projects/#{project.id}/issues/#{issue.id}", user),
+ labels: 'label, ?'
+ response.status.should == 400
+ json_response['message']['labels']['?']['title'].should == ['is invalid']
+ end
+
+ it 'should allow special label names' do
+ put api("/projects/#{project.id}/issues/#{issue.id}", user),
+ labels: 'label:foo, label-bar,label_bar,label/bar'
+ response.status.should == 200
+ json_response['labels'].should include 'label:foo'
+ json_response['labels'].should include 'label-bar'
+ json_response['labels'].should include 'label_bar'
+ json_response['labels'].should include 'label/bar'
end
end
diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb
index cd1b84c53c9eda6189a42d1d3930414b3d6b0175..ee9088933a1cf609fda1864779d8be4bb284c0d6 100644
--- a/spec/requests/api/labels_spec.rb
+++ b/spec/requests/api/labels_spec.rb
@@ -50,6 +50,14 @@ describe API::API, api: true do
json_response['message'].should == 'Color is invalid'
end
+ it 'should return 400 for too long color code' do
+ post api("/projects/#{project.id}/labels", user),
+ name: 'Foo',
+ color: '#FFAAFFFF'
+ response.status.should == 400
+ json_response['message'].should == 'Color is invalid'
+ end
+
it 'should return 400 for invalid name' do
post api("/projects/#{project.id}/labels", user),
name: '?',
@@ -147,5 +155,13 @@ describe API::API, api: true do
response.status.should == 400
json_response['message'].should == 'Color is invalid'
end
+
+ it 'should return 400 for too long color code' do
+ post api("/projects/#{project.id}/labels", user),
+ name: 'Foo',
+ color: '#FFAAFFFF'
+ response.status.should == 400
+ json_response['message'].should == 'Color is invalid'
+ end
end
end