diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 517e454862498b8ee8ccb539da74e6f5ac189456..0a5fe24b5af5c57dca858ac0a8253c56baff7db8 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 515ed447f005a90afa4df2687974978c46cfb463..a511b7940edeb24a4770453608f7648e6e291bdd 100644
--- a/app/models/label.rb
+++ b/app/models/label.rb
@@ -1,4 +1,6 @@
class Label < ActiveRecord::Base
+ DEFAULT_COLOR = '#82C5FF'
+
belongs_to :project
has_many :label_links, dependent: :destroy
has_many :issues, through: :label_links, source: :target, source_type: 'Issue'
diff --git a/doc/api/issues.md b/doc/api/issues.md
index f775d502a6df27af3a2d3b12135479e21df8e22d..a4b3b3e991885656fb8c9140651786f7da626b44 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/merge_requests.md b/doc/api/merge_requests.md
index a46472a0812a7dd9c49f7e7f766e59c29d02ca04..f56e968e7c2c547821741913a543cd234a20451c 100644
--- a/doc/api/merge_requests.md
+++ b/doc/api/merge_requests.md
@@ -136,6 +136,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.
@@ -183,15 +186,18 @@ 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 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
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index 8189e433789541d6647fb2074c659df3a83b7e83..d36b29a00b1c8dd4dd16c1525ca8daf4b69c5b02 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 b29118b2fd828c2c7e42adbd55e650d9a9669f34..055529ccbd89a10ff8f42697f3f1741ac336acb2 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/merge_requests.rb b/lib/api/merge_requests.rb
index acca7cb6bad3611e33189ab2f7a8c08e20fd7912..0d765f9280eda65a55801bfe023b1981299b8488 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/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index dff7f20cb325cda83862b27263b7137f253c641b..d8e8e4f5035e54958b3a6680c98487e11c40467e 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/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 3611d9d6dc3317d05252bb61b5782eca5c096e6d..58cf7f139dc92f63462d6ff896e84cfc70b87279 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