Skip to content
Projects
Groups
Snippets
Help
Loading...
Help
Support
Keyboard shortcuts
?
Submit feedback
Contribute to GitLab
Sign in / Register
Toggle navigation
G
gitlab-ce
Project overview
Project overview
Details
Activity
Releases
Repository
Repository
Files
Commits
Branches
Tags
Contributors
Graph
Compare
Issues
0
Issues
0
List
Boards
Labels
Milestones
Merge Requests
0
Merge Requests
0
Analytics
Analytics
Repository
Value Stream
Wiki
Wiki
Snippets
Snippets
Members
Members
Collapse sidebar
Close sidebar
Activity
Graph
Create a new issue
Commits
Issue Boards
Open sidebar
Boxiang Sun
gitlab-ce
Commits
39464857
Commit
39464857
authored
10 years ago
by
Jacob Vosmaer
Browse files
Options
Browse Files
Download
Plain Diff
Merge pull request #7526 from jubianchi/api/issue-api-validate-labels
API: Labels validation in issues API
parents
f5b23bc2
9d5c6901
No related merge requests found
Changes
7
Hide whitespace changes
Inline
Side-by-side
Showing
7 changed files
with
107 additions
and
15 deletions
+107
-15
app/models/concerns/issuable.rb
app/models/concerns/issuable.rb
+4
-0
app/models/label.rb
app/models/label.rb
+2
-2
lib/api/helpers.rb
lib/api/helpers.rb
+6
-2
lib/api/issues.rb
lib/api/issues.rb
+6
-5
spec/models/label_spec.rb
spec/models/label_spec.rb
+23
-0
spec/requests/api/issues_spec.rb
spec/requests/api/issues_spec.rb
+50
-6
spec/requests/api/labels_spec.rb
spec/requests/api/labels_spec.rb
+16
-0
No files found.
app/models/concerns/issuable.rb
View file @
39464857
...
...
@@ -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
(
...
...
This diff is collapsed.
Click to expand it.
app/models/label.rb
View file @
39464857
...
...
@@ -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"
)
}
...
...
This diff is collapsed.
Click to expand it.
lib/api/helpers.rb
View file @
39464857
...
...
@@ -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
...
...
This diff is collapsed.
Click to expand it.
lib/api/issues.rb
View file @
39464857
...
...
@@ -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
)
re
turn
render_api_error!
(
'Label names invalid'
,
405
)
if
(
errors
=
validate_label_params
(
params
)).
any?
re
nder_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
)
re
turn
render_api_error!
(
'Label names invalid'
,
405
)
if
(
errors
=
validate_label_params
(
params
)).
any?
re
nder_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
...
...
This diff is collapsed.
Click to expand it.
spec/models/label_spec.rb
View file @
39464857
...
...
@@ -5,4 +5,27 @@ describe Label do
it
{
label
.
should
be_valid
}
it
{
should
belong_to
(
:project
)
}
describe
'Validation'
do
it
'should validate color code'
do
build
(
:label
,
color:
'G-ITLAB'
).
should_not
be_valid
build
(
:label
,
color:
'AABBCC'
).
should_not
be_valid
build
(
:label
,
color:
'#AABBCCEE'
).
should_not
be_valid
build
(
:label
,
color:
'#GGHHII'
).
should_not
be_valid
build
(
:label
,
color:
'#'
).
should_not
be_valid
build
(
:label
,
color:
''
).
should_not
be_valid
build
(
:label
,
color:
'#AABBCC'
).
should
be_valid
end
it
'should validate title'
do
build
(
:label
,
title:
'G,ITLAB'
).
should_not
be_valid
build
(
:label
,
title:
'G?ITLAB'
).
should_not
be_valid
build
(
:label
,
title:
'G&ITLAB'
).
should_not
be_valid
build
(
:label
,
title:
''
).
should_not
be_valid
build
(
:label
,
title:
'GITLAB'
).
should
be_valid
build
(
:label
,
title:
'gitlab'
).
should
be_valid
end
end
end
This diff is collapsed.
Click to expand it.
spec/requests/api/issues_spec.rb
View file @
39464857
...
...
@@ -73,12 +73,12 @@ describe API::API, api: true do
response
.
status
.
should
==
400
end
it
'should return 40
5
on invalid label names'
do
it
'should return 40
0
on invalid label names'
do
post
api
(
"/projects/
#{
project
.
id
}
/issues"
,
user
),
title:
'new issue'
,
labels:
'label, ?'
response
.
status
.
should
==
40
5
json_response
[
'message'
]
.
should
==
'Label names invalid'
response
.
status
.
should
==
40
0
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 40
5
on invalid label names'
do
it
'should return 40
0
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
...
...
This diff is collapsed.
Click to expand it.
spec/requests/api/labels_spec.rb
View file @
39464857
...
...
@@ -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
This diff is collapsed.
Click to expand it.
Write
Preview
Markdown
is supported
0%
Try again
or
attach a new file
Attach a file
Cancel
You are about to add
0
people
to the discussion. Proceed with caution.
Finish editing this message first!
Cancel
Please
register
or
sign in
to comment