Commit 998cd3cb authored by jubianchi's avatar jubianchi

Improve error reporting on users API

* users (#6878, #3526, #4209): Validation error messages are now exposed through 400 responses, 409 response are sent in case of duplicate email or username
* MRs (#5335): 409 responses are sent in case of duplicate merge request (source/target branches), 422 responses are sent when submiting MR fo/from unrelated forks
* issues
* labels
* projects
parent 892371bc
...@@ -122,9 +122,11 @@ class MergeRequest < ActiveRecord::Base ...@@ -122,9 +122,11 @@ class MergeRequest < ActiveRecord::Base
if opened? || reopened? if opened? || reopened?
similar_mrs = self.target_project.merge_requests.where(source_branch: source_branch, target_branch: target_branch, source_project_id: source_project.id).opened similar_mrs = self.target_project.merge_requests.where(source_branch: source_branch, target_branch: target_branch, source_project_id: source_project.id).opened
similar_mrs = similar_mrs.where('id not in (?)', self.id) if self.id similar_mrs = similar_mrs.where('id not in (?)', self.id) if self.id
if similar_mrs.any? if similar_mrs.any?
errors.add :base, "Cannot Create: This merge request already exists: #{similar_mrs.pluck(:title)}" errors.add :validate_branches,
"Cannot Create: This merge request already exists: #{
similar_mrs.pluck(:title)
}"
end end
end end
end end
...@@ -140,7 +142,8 @@ class MergeRequest < ActiveRecord::Base ...@@ -140,7 +142,8 @@ class MergeRequest < ActiveRecord::Base
if source_project.forked_from?(target_project) if source_project.forked_from?(target_project)
true true
else else
errors.add :base, "Source project is not a fork of target project" errors.add :validate_fork,
'Source project is not a fork of target project'
end end
end end
end end
......
...@@ -80,6 +80,7 @@ Return values: ...@@ -80,6 +80,7 @@ Return values:
- `404 Not Found` - A resource could not be accessed, e.g. an ID for a resource could not be found - `404 Not Found` - A resource could not be accessed, e.g. an ID for a resource could not be found
- `405 Method Not Allowed` - The request is not supported - `405 Method Not Allowed` - The request is not supported
- `409 Conflict` - A conflicting resource already exists, e.g. creating a project with a name that already exists - `409 Conflict` - A conflicting resource already exists, e.g. creating a project with a name that already exists
- `422 Unprocessable` - The entity could not be processed
- `500 Server Error` - While handling the request something went wrong on the server side - `500 Server Error` - While handling the request something went wrong on the server side
## Sudo ## Sudo
...@@ -144,3 +145,52 @@ Issue: ...@@ -144,3 +145,52 @@ Issue:
- iid - is unique only in scope of a single project. When you browse issues or merge requests with Web UI, you see iid. - iid - is unique only in scope of a single project. When you browse issues or merge requests with Web UI, you see iid.
So if you want to get issue with api you use `http://host/api/v3/.../issues/:id.json`. But when you want to create a link to web page - use `http:://host/project/issues/:iid.json` So if you want to get issue with api you use `http://host/api/v3/.../issues/:id.json`. But when you want to create a link to web page - use `http:://host/project/issues/:iid.json`
## Data validation and error reporting
When working with the API you may encounter validation errors. In such case, the API will answer with an HTTP `400` status.
Such errors appear in two cases:
* A required attribute of the API request is missing, e.g. the title of an issue is not given
* An attribute did not pass the validation, e.g. user bio is too long
When an attribute is missing, you will get something like:
HTTP/1.1 400 Bad Request
Content-Type: application/json
{
"message":"400 (Bad request) \"title\" not given"
}
When a validation error occurs, error messages will be different. They will hold all details of validation errors:
HTTP/1.1 400 Bad Request
Content-Type: application/json
{
"message": {
"bio": [
"is too long (maximum is 255 characters)"
]
}
}
This makes error messages more machine-readable. The format can be described as follow:
{
"message": {
"<property-name>": [
"<error-message>",
"<error-message>",
...
],
"<embed-entity>": {
"<property-name>": [
"<error-message>",
"<error-message>",
...
],
}
}
}
...@@ -58,7 +58,7 @@ module API ...@@ -58,7 +58,7 @@ module API
if key.valid? && user_project.deploy_keys << key if key.valid? && user_project.deploy_keys << key
present key, with: Entities::SSHKey present key, with: Entities::SSHKey
else else
not_found! render_validation_error!(key)
end end
end end
......
...@@ -155,7 +155,17 @@ module API ...@@ -155,7 +155,17 @@ module API
end end
def not_allowed! def not_allowed!
render_api_error!('Method Not Allowed', 405) render_api_error!('405 Method Not Allowed', 405)
end
def conflict!(message = nil)
render_api_error!(message || '409 Conflict', 409)
end
def render_validation_error!(model)
unless model.valid?
render_api_error!(model.errors.messages || '400 Bad Request', 400)
end
end end
def render_api_error!(message, status) def render_api_error!(message, status)
......
...@@ -109,7 +109,7 @@ module API ...@@ -109,7 +109,7 @@ module API
present issue, with: Entities::Issue present issue, with: Entities::Issue
else else
not_found! render_validation_error!(issue)
end end
end end
...@@ -149,7 +149,7 @@ module API ...@@ -149,7 +149,7 @@ module API
present issue, with: Entities::Issue present issue, with: Entities::Issue
else else
not_found! render_validation_error!(issue)
end end
end end
......
...@@ -30,16 +30,14 @@ module API ...@@ -30,16 +30,14 @@ module API
attrs = attributes_for_keys [:name, :color] attrs = attributes_for_keys [:name, :color]
label = user_project.find_label(attrs[:name]) label = user_project.find_label(attrs[:name])
if label conflict!('Label already exists') if label
return render_api_error!('Label already exists', 409)
end
label = user_project.labels.create(attrs) label = user_project.labels.create(attrs)
if label.valid? if label.valid?
present label, with: Entities::Label present label, with: Entities::Label
else else
render_api_error!(label.errors.full_messages.join(', '), 400) render_validation_error!(label)
end end
end end
...@@ -56,9 +54,7 @@ module API ...@@ -56,9 +54,7 @@ module API
required_attributes! [:name] required_attributes! [:name]
label = user_project.find_label(params[:name]) label = user_project.find_label(params[:name])
if !label not_found!('Label') unless label
return render_api_error!('Label not found', 404)
end
label.destroy label.destroy
end end
...@@ -67,7 +63,8 @@ module API ...@@ -67,7 +63,8 @@ module API
# #
# Parameters: # Parameters:
# id (required) - The ID of a project # id (required) - The ID of a project
# name (optional) - The name of the label to be deleted # name (required) - The name of the label to be deleted
# new_name (optional) - The new name of the label
# color (optional) - Color of the label given in 6-digit hex # color (optional) - Color of the label given in 6-digit hex
# notation with leading '#' sign (e.g. #FFAABB) # notation with leading '#' sign (e.g. #FFAABB)
# Example Request: # Example Request:
...@@ -77,14 +74,12 @@ module API ...@@ -77,14 +74,12 @@ module API
required_attributes! [:name] required_attributes! [:name]
label = user_project.find_label(params[:name]) label = user_project.find_label(params[:name])
if !label not_found!('Label not found') unless label
return render_api_error!('Label not found', 404)
end
attrs = attributes_for_keys [:new_name, :color] attrs = attributes_for_keys [:new_name, :color]
if attrs.empty? if attrs.empty?
return render_api_error!('Required parameters "name" or "color" ' \ render_api_error!('Required parameters "new_name" or "color" ' \
'missing', 'missing',
400) 400)
end end
...@@ -95,7 +90,7 @@ module API ...@@ -95,7 +90,7 @@ module API
if label.update(attrs) if label.update(attrs)
present label, with: Entities::Label present label, with: Entities::Label
else else
render_api_error!(label.errors.full_messages.join(', '), 400) render_validation_error!(label)
end end
end end
end end
......
...@@ -10,8 +10,13 @@ module API ...@@ -10,8 +10,13 @@ module API
error!(errors[:project_access], 422) error!(errors[:project_access], 422)
elsif errors[:branch_conflict].any? elsif errors[:branch_conflict].any?
error!(errors[:branch_conflict], 422) error!(errors[:branch_conflict], 422)
elsif errors[:validate_fork].any?
error!(errors[:validate_fork], 422)
elsif errors[:validate_branches].any?
conflict!(errors[:validate_branches])
end end
not_found!
render_api_error!(errors, 400)
end end
end end
...@@ -214,7 +219,7 @@ module API ...@@ -214,7 +219,7 @@ module API
if note.save if note.save
present note, with: Entities::MRNote present note, with: Entities::MRNote
else else
not_found! render_validation_error!(note)
end end
end end
end end
......
...@@ -56,7 +56,7 @@ module API ...@@ -56,7 +56,7 @@ module API
if @snippet.save if @snippet.save
present @snippet, with: Entities::ProjectSnippet present @snippet, with: Entities::ProjectSnippet
else else
not_found! render_validation_error!(@snippet)
end end
end end
...@@ -80,7 +80,7 @@ module API ...@@ -80,7 +80,7 @@ module API
if @snippet.update_attributes attrs if @snippet.update_attributes attrs
present @snippet, with: Entities::ProjectSnippet present @snippet, with: Entities::ProjectSnippet
else else
not_found! render_validation_error!(@snippet)
end end
end end
...@@ -97,6 +97,7 @@ module API ...@@ -97,6 +97,7 @@ module API
authorize! :modify_project_snippet, @snippet authorize! :modify_project_snippet, @snippet
@snippet.destroy @snippet.destroy
rescue rescue
not_found!('Snippet')
end end
end end
......
...@@ -111,7 +111,7 @@ module API ...@@ -111,7 +111,7 @@ module API
if @project.errors[:limit_reached].present? if @project.errors[:limit_reached].present?
error!(@project.errors[:limit_reached], 403) error!(@project.errors[:limit_reached], 403)
end end
not_found! render_validation_error!(@project)
end end
end end
...@@ -149,7 +149,7 @@ module API ...@@ -149,7 +149,7 @@ module API
if @project.saved? if @project.saved?
present @project, with: Entities::Project present @project, with: Entities::Project
else else
not_found! render_validation_error!(@project)
end end
end end
......
...@@ -42,7 +42,8 @@ module API ...@@ -42,7 +42,8 @@ module API
# Parameters: # Parameters:
# email (required) - Email # email (required) - Email
# password (required) - Password # password (required) - Password
# name - Name # name (required) - Name
# username (required) - Name
# skype - Skype ID # skype - Skype ID
# linkedin - Linkedin # linkedin - Linkedin
# twitter - Twitter account # twitter - Twitter account
...@@ -65,7 +66,15 @@ module API ...@@ -65,7 +66,15 @@ module API
if user.save if user.save
present user, with: Entities::UserFull present user, with: Entities::UserFull
else else
not_found! conflict!('Email has already been taken') if User.
where(email: user.email).
count > 0
conflict!('Username has already been taken') if User.
where(username: user.username).
count > 0
render_validation_error!(user)
end end
end end
...@@ -92,14 +101,23 @@ module API ...@@ -92,14 +101,23 @@ module API
attrs = attributes_for_keys [:email, :name, :password, :skype, :linkedin, :twitter, :website_url, :projects_limit, :username, :extern_uid, :provider, :bio, :can_create_group, :admin] attrs = attributes_for_keys [:email, :name, :password, :skype, :linkedin, :twitter, :website_url, :projects_limit, :username, :extern_uid, :provider, :bio, :can_create_group, :admin]
user = User.find(params[:id]) user = User.find(params[:id])
not_found!("User not found") unless user not_found!('User') unless user
admin = attrs.delete(:admin) admin = attrs.delete(:admin)
user.admin = admin unless admin.nil? user.admin = admin unless admin.nil?
conflict!('Email has already been taken') if attrs[:email] &&
User.where(email: attrs[:email]).
where.not(id: user.id).count > 0
conflict!('Username has already been taken') if attrs[:username] &&
User.where(username: attrs[:username]).
where.not(id: user.id).count > 0
if user.update_attributes(attrs) if user.update_attributes(attrs)
present user, with: Entities::UserFull present user, with: Entities::UserFull
else else
not_found! render_validation_error!(user)
end end
end end
...@@ -113,13 +131,15 @@ module API ...@@ -113,13 +131,15 @@ module API
# POST /users/:id/keys # POST /users/:id/keys
post ":id/keys" do post ":id/keys" do
authenticated_as_admin! authenticated_as_admin!
required_attributes! [:title, :key]
user = User.find(params[:id]) user = User.find(params[:id])
attrs = attributes_for_keys [:title, :key] attrs = attributes_for_keys [:title, :key]
key = user.keys.new attrs key = user.keys.new attrs
if key.save if key.save
present key, with: Entities::SSHKey present key, with: Entities::SSHKey
else else
not_found! render_validation_error!(key)
end end
end end
...@@ -132,11 +152,9 @@ module API ...@@ -132,11 +152,9 @@ module API
get ':uid/keys' do get ':uid/keys' do
authenticated_as_admin! authenticated_as_admin!
user = User.find_by(id: params[:uid]) user = User.find_by(id: params[:uid])
if user not_found!('User') unless user
present user.keys, with: Entities::SSHKey present user.keys, with: Entities::SSHKey
else
not_found!
end
end end
# Delete existing ssh key of a specified user. Only available to admin # Delete existing ssh key of a specified user. Only available to admin
...@@ -150,15 +168,13 @@ module API ...@@ -150,15 +168,13 @@ module API
delete ':uid/keys/:id' do delete ':uid/keys/:id' do
authenticated_as_admin! authenticated_as_admin!
user = User.find_by(id: params[:uid]) user = User.find_by(id: params[:uid])
if user not_found!('User') unless user
begin begin
key = user.keys.find params[:id] key = user.keys.find params[:id]
key.destroy key.destroy
rescue ActiveRecord::RecordNotFound rescue ActiveRecord::RecordNotFound
not_found! not_found!('Key')
end
else
not_found!
end end
end end
...@@ -173,7 +189,7 @@ module API ...@@ -173,7 +189,7 @@ module API
if user if user
user.destroy user.destroy
else else
not_found! not_found!('User')
end end
end end
end end
...@@ -219,7 +235,7 @@ module API ...@@ -219,7 +235,7 @@ module API
if key.save if key.save
present key, with: Entities::SSHKey present key, with: Entities::SSHKey
else else
not_found! render_validation_error!(key)
end end
end end
......
...@@ -169,6 +169,15 @@ describe API::API, api: true do ...@@ -169,6 +169,15 @@ describe API::API, api: true do
response.status.should == 400 response.status.should == 400
json_response['message']['labels']['?']['title'].should == ['is invalid'] json_response['message']['labels']['?']['title'].should == ['is invalid']
end end
it 'should return 400 if title is too long' do
post api("/projects/#{project.id}/issues", user),
title: 'g' * 256
response.status.should == 400
json_response['message']['title'].should == [
'is too long (maximum is 255 characters)'
]
end
end end
describe "PUT /projects/:id/issues/:issue_id to update only title" do describe "PUT /projects/:id/issues/:issue_id to update only title" do
...@@ -237,6 +246,15 @@ describe API::API, api: true do ...@@ -237,6 +246,15 @@ describe API::API, api: true do
json_response['labels'].should include 'label_bar' json_response['labels'].should include 'label_bar'
json_response['labels'].should include 'label/bar' json_response['labels'].should include 'label/bar'
end end
it 'should return 400 if title is too long' do
put api("/projects/#{project.id}/issues/#{issue.id}", user),
title: 'g' * 256
response.status.should == 400
json_response['message']['title'].should == [
'is too long (maximum is 255 characters)'
]
end
end end
describe "PUT /projects/:id/issues/:issue_id to update state and label" do describe "PUT /projects/:id/issues/:issue_id to update state and label" do
......
...@@ -47,7 +47,7 @@ describe API::API, api: true do ...@@ -47,7 +47,7 @@ describe API::API, api: true do
name: 'Foo', name: 'Foo',
color: '#FFAA' color: '#FFAA'
response.status.should == 400 response.status.should == 400
json_response['message'].should == 'Color is invalid' json_response['message']['color'].should == ['is invalid']
end end
it 'should return 400 for too long color code' do it 'should return 400 for too long color code' do
...@@ -55,7 +55,7 @@ describe API::API, api: true do ...@@ -55,7 +55,7 @@ describe API::API, api: true do
name: 'Foo', name: 'Foo',
color: '#FFAAFFFF' color: '#FFAAFFFF'
response.status.should == 400 response.status.should == 400
json_response['message'].should == 'Color is invalid' json_response['message']['color'].should == ['is invalid']
end end
it 'should return 400 for invalid name' do it 'should return 400 for invalid name' do
...@@ -63,7 +63,7 @@ describe API::API, api: true do ...@@ -63,7 +63,7 @@ describe API::API, api: true do
name: '?', name: '?',
color: '#FFAABB' color: '#FFAABB'
response.status.should == 400 response.status.should == 400
json_response['message'].should == 'Title is invalid' json_response['message']['title'].should == ['is invalid']
end end
it 'should return 409 if label already exists' do it 'should return 409 if label already exists' do
...@@ -84,7 +84,7 @@ describe API::API, api: true do ...@@ -84,7 +84,7 @@ describe API::API, api: true do
it 'should return 404 for non existing label' do it 'should return 404 for non existing label' do
delete api("/projects/#{project.id}/labels", user), name: 'label2' delete api("/projects/#{project.id}/labels", user), name: 'label2'
response.status.should == 404 response.status.should == 404
json_response['message'].should == 'Label not found' json_response['message'].should == '404 Label Not Found'
end end
it 'should return 400 for wrong parameters' do it 'should return 400 for wrong parameters' do
...@@ -132,11 +132,14 @@ describe API::API, api: true do ...@@ -132,11 +132,14 @@ describe API::API, api: true do
it 'should return 400 if no label name given' do it 'should return 400 if no label name given' do
put api("/projects/#{project.id}/labels", user), new_name: 'label2' put api("/projects/#{project.id}/labels", user), new_name: 'label2'
response.status.should == 400 response.status.should == 400
json_response['message'].should == '400 (Bad request) "name" not given'
end end
it 'should return 400 if no new parameters given' do it 'should return 400 if no new parameters given' do
put api("/projects/#{project.id}/labels", user), name: 'label1' put api("/projects/#{project.id}/labels", user), name: 'label1'
response.status.should == 400 response.status.should == 400
json_response['message'].should == 'Required parameters '\
'"new_name" or "color" missing'
end end
it 'should return 400 for invalid name' do it 'should return 400 for invalid name' do
...@@ -145,7 +148,7 @@ describe API::API, api: true do ...@@ -145,7 +148,7 @@ describe API::API, api: true do
new_name: '?', new_name: '?',
color: '#FFFFFF' color: '#FFFFFF'
response.status.should == 400 response.status.should == 400
json_response['message'].should == 'Title is invalid' json_response['message']['title'].should == ['is invalid']
end end
it 'should return 400 for invalid name' do it 'should return 400 for invalid name' do
...@@ -153,7 +156,7 @@ describe API::API, api: true do ...@@ -153,7 +156,7 @@ describe API::API, api: true do
name: 'label1', name: 'label1',
color: '#FF' color: '#FF'
response.status.should == 400 response.status.should == 400
json_response['message'].should == 'Color is invalid' json_response['message']['color'].should == ['is invalid']
end end
it 'should return 400 for too long color code' do it 'should return 400 for too long color code' do
...@@ -161,7 +164,7 @@ describe API::API, api: true do ...@@ -161,7 +164,7 @@ describe API::API, api: true do
name: 'Foo', name: 'Foo',
color: '#FFAAFFFF' color: '#FFAAFFFF'
response.status.should == 400 response.status.should == 400
json_response['message'].should == 'Color is invalid' json_response['message']['color'].should == ['is invalid']
end end
end end
end end
...@@ -123,6 +123,28 @@ describe API::API, api: true do ...@@ -123,6 +123,28 @@ describe API::API, api: true do
json_response['message']['labels']['?']['title'].should == json_response['message']['labels']['?']['title'].should ==
['is invalid'] ['is invalid']
end end
context 'with existing MR' do
before do
post api("/projects/#{project.id}/merge_requests", user),
title: 'Test merge_request',
source_branch: 'stable',
target_branch: 'master',
author: user
@mr = MergeRequest.all.last
end
it 'should return 409 when MR already exists for source/target' do
expect do
post api("/projects/#{project.id}/merge_requests", user),
title: 'New test merge_request',
source_branch: 'stable',
target_branch: 'master',
author: user
end.to change { MergeRequest.count }.by(0)
response.status.should == 409
end
end
end end
context 'forked projects' do context 'forked projects' do
...@@ -170,16 +192,26 @@ describe API::API, api: true do ...@@ -170,16 +192,26 @@ describe API::API, api: true do
response.status.should == 400 response.status.should == 400
end end
it "should return 404 when target_branch is specified and not a forked project" do context 'when target_branch is specified' do
it 'should return 422 if not a forked project' do
post api("/projects/#{project.id}/merge_requests", user), post api("/projects/#{project.id}/merge_requests", user),
title: 'Test merge_request', target_branch: 'master', source_branch: 'stable', author: user, target_project_id: fork_project.id title: 'Test merge_request',
response.status.should == 404 target_branch: 'master',
source_branch: 'stable',
author: user,
target_project_id: fork_project.id
response.status.should == 422
end end
it "should return 404 when target_branch is specified and for a different fork" do it 'should return 422 if targeting a different fork' do
post api("/projects/#{fork_project.id}/merge_requests", user2), post api("/projects/#{fork_project.id}/merge_requests", user2),
title: 'Test merge_request', target_branch: 'master', source_branch: 'stable', author: user2, target_project_id: unrelated_project.id title: 'Test merge_request',
response.status.should == 404 target_branch: 'master',
source_branch: 'stable',
author: user2,
target_project_id: unrelated_project.id
response.status.should == 422
end
end end
it "should return 201 when target_branch is specified and for the same project" do it "should return 201 when target_branch is specified and for the same project" do
...@@ -216,7 +248,7 @@ describe API::API, api: true do ...@@ -216,7 +248,7 @@ describe API::API, api: true do
merge_request.close merge_request.close
put api("/projects/#{project.id}/merge_request/#{merge_request.id}/merge", user) put api("/projects/#{project.id}/merge_request/#{merge_request.id}/merge", user)
response.status.should == 405 response.status.should == 405
json_response['message'].should == 'Method Not Allowed' json_response['message'].should == '405 Method Not Allowed'
end end
it "should return 401 if user has no permissions to merge" do it "should return 401 if user has no permissions to merge" do
...@@ -276,7 +308,8 @@ describe API::API, api: true do ...@@ -276,7 +308,8 @@ describe API::API, api: true do
end end
it "should return 404 if note is attached to non existent merge request" do it "should return 404 if note is attached to non existent merge request" do
post api("/projects/#{project.id}/merge_request/111/comments", user), note: "My comment" post api("/projects/#{project.id}/merge_request/404/comments", user),
note: 'My comment'
response.status.should == 404 response.status.should == 404
end end
end end
......
...@@ -188,9 +188,24 @@ describe API::API, api: true do ...@@ -188,9 +188,24 @@ describe API::API, api: true do
response.status.should == 201 response.status.should == 201
end end
it "should respond with 404 on failure" do it 'should respond with 400 on failure' do
post api("/projects/user/#{user.id}", admin) post api("/projects/user/#{user.id}", admin)
response.status.should == 404 response.status.should == 400
json_response['message']['creator'].should == ['can\'t be blank']
json_response['message']['namespace'].should == ['can\'t be blank']
json_response['message']['name'].should == [
'can\'t be blank',
'is too short (minimum is 0 characters)',
'can contain only letters, digits, \'_\', \'-\' and \'.\' and '\
'space. It must start with letter, digit or \'_\'.'
]
json_response['message']['path'].should == [
'can\'t be blank',
'is too short (minimum is 0 characters)',
'can contain only letters, digits, \'_\', \'-\' and \'.\'. It must '\
'start with letter, digit or \'_\', optionally preceeded by \'.\'. '\
'It must not end in \'.git\'.'
]
end end
it "should assign attributes to project" do it "should assign attributes to project" do
...@@ -407,9 +422,9 @@ describe API::API, api: true do ...@@ -407,9 +422,9 @@ describe API::API, api: true do
response.status.should == 200 response.status.should == 200
end end
it "should return success when deleting unknown snippet id" do it 'should return 404 when deleting unknown snippet id' do
delete api("/projects/#{project.id}/snippets/1234", user) delete api("/projects/#{project.id}/snippets/1234", user)
response.status.should == 200 response.status.should == 404
end end
end end
...@@ -456,7 +471,21 @@ describe API::API, api: true do ...@@ -456,7 +471,21 @@ describe API::API, api: true do
describe "POST /projects/:id/keys" do describe "POST /projects/:id/keys" do
it "should not create an invalid ssh key" do it "should not create an invalid ssh key" do
post api("/projects/#{project.id}/keys", user), { title: "invalid key" } post api("/projects/#{project.id}/keys", user), { title: "invalid key" }
response.status.should == 404 response.status.should == 400
json_response['message']['key'].should == [
'can\'t be blank',
'is too short (minimum is 0 characters)',
'is invalid'
]
end
it 'should not create a key without title' do
post api("/projects/#{project.id}/keys", user), key: 'some key'
response.status.should == 400
json_response['message']['title'].should == [
'can\'t be blank',
'is too short (minimum is 0 characters)'
]
end end
it "should create new ssh key" do it "should create new ssh key" do
......
...@@ -51,6 +51,7 @@ describe API::API, api: true do ...@@ -51,6 +51,7 @@ describe API::API, api: true do
it "should return a 404 error if user id not found" do it "should return a 404 error if user id not found" do
get api("/users/9999", user) get api("/users/9999", user)
response.status.should == 404 response.status.should == 404
json_response['message'].should == '404 Not found'
end end
end end
...@@ -98,18 +99,47 @@ describe API::API, api: true do ...@@ -98,18 +99,47 @@ describe API::API, api: true do
end end
it "should not create user with invalid email" do it "should not create user with invalid email" do
post api("/users", admin), { email: "invalid email", password: 'password' } post api('/users', admin),
email: 'invalid email',
password: 'password',
name: 'test'
response.status.should == 400 response.status.should == 400
end end
it "should return 400 error if password not given" do it 'should return 400 error if name not given' do
post api("/users", admin), { email: 'test@example.com' } post api('/users', admin), email: 'test@example.com', password: 'pass1234'
response.status.should == 400
end
it 'should return 400 error if password not given' do
post api('/users', admin), email: 'test@example.com', name: 'test'
response.status.should == 400 response.status.should == 400
end end
it "should return 400 error if email not given" do it "should return 400 error if email not given" do
post api("/users", admin), { password: 'pass1234' } post api('/users', admin), password: 'pass1234', name: 'test'
response.status.should == 400
end
it 'should return 400 error if user does not validate' do
post api('/users', admin),
password: 'pass',
email: 'test@example.com',
username: 'test!',
name: 'test',
bio: 'g' * 256,
projects_limit: -1
response.status.should == 400 response.status.should == 400
json_response['message']['password'].
should == ['is too short (minimum is 8 characters)']
json_response['message']['bio'].
should == ['is too long (maximum is 255 characters)']
json_response['message']['projects_limit'].
should == ['must be greater than or equal to 0']
json_response['message']['username'].
should == ['can contain only letters, digits, '\
'\'_\', \'-\' and \'.\'. It must start with letter, digit or '\
'\'_\', optionally preceeded by \'.\'. It must not end in \'.git\'.']
end end
it "shouldn't available for non admin users" do it "shouldn't available for non admin users" do
...@@ -117,21 +147,37 @@ describe API::API, api: true do ...@@ -117,21 +147,37 @@ describe API::API, api: true do
response.status.should == 403 response.status.should == 403
end end
context "with existing user" do context 'with existing user' do
before { post api("/users", admin), { email: 'test@example.com', password: 'password', username: 'test' } } before do
post api('/users', admin),
email: 'test@example.com',
password: 'password',
username: 'test',
name: 'foo'
end
it "should not create user with same email" do it 'should return 409 conflict error if user with same email exists' do
expect { expect {
post api("/users", admin), { email: 'test@example.com', password: 'password' } post api('/users', admin),
name: 'foo',
email: 'test@example.com',
password: 'password',
username: 'foo'
}.to change { User.count }.by(0) }.to change { User.count }.by(0)
response.status.should == 409
json_response['message'].should == 'Email has already been taken'
end end
it "should return 409 conflict error if user with email exists" do it 'should return 409 conflict error if same username exists' do
post api("/users", admin), { email: 'test@example.com', password: 'password' } expect do
end post api('/users', admin),
name: 'foo',
it "should return 409 conflict error if same username exists" do email: 'foo@example.com',
post api("/users", admin), { email: 'foo@example.com', password: 'pass', username: 'test' } password: 'password',
username: 'test'
end.to change { User.count }.by(0)
response.status.should == 409
json_response['message'].should == 'Username has already been taken'
end end
end end
end end
...@@ -173,6 +219,20 @@ describe API::API, api: true do ...@@ -173,6 +219,20 @@ describe API::API, api: true do
user.reload.bio.should == 'new test bio' user.reload.bio.should == 'new test bio'
end end
it 'should update user with his own email' do
put api("/users/#{user.id}", admin), email: user.email
response.status.should == 200
json_response['email'].should == user.email
user.reload.email.should == user.email
end
it 'should update user with his own username' do
put api("/users/#{user.id}", admin), username: user.username
response.status.should == 200
json_response['username'].should == user.username
user.reload.username.should == user.username
end
it "should update admin status" do it "should update admin status" do
put api("/users/#{user.id}", admin), {admin: true} put api("/users/#{user.id}", admin), {admin: true}
response.status.should == 200 response.status.should == 200
...@@ -190,7 +250,7 @@ describe API::API, api: true do ...@@ -190,7 +250,7 @@ describe API::API, api: true do
it "should not allow invalid update" do it "should not allow invalid update" do
put api("/users/#{user.id}", admin), {email: 'invalid email'} put api("/users/#{user.id}", admin), {email: 'invalid email'}
response.status.should == 404 response.status.should == 400
user.reload.email.should_not == 'invalid email' user.reload.email.should_not == 'invalid email'
end end
...@@ -202,25 +262,49 @@ describe API::API, api: true do ...@@ -202,25 +262,49 @@ describe API::API, api: true do
it "should return 404 for non-existing user" do it "should return 404 for non-existing user" do
put api("/users/999999", admin), {bio: 'update should fail'} put api("/users/999999", admin), {bio: 'update should fail'}
response.status.should == 404 response.status.should == 404
json_response['message'].should == '404 Not found'
end
it 'should return 400 error if user does not validate' do
put api("/users/#{user.id}", admin),
password: 'pass',
email: 'test@example.com',
username: 'test!',
name: 'test',
bio: 'g' * 256,
projects_limit: -1
response.status.should == 400
json_response['message']['password'].
should == ['is too short (minimum is 8 characters)']
json_response['message']['bio'].
should == ['is too long (maximum is 255 characters)']
json_response['message']['projects_limit'].
should == ['must be greater than or equal to 0']
json_response['message']['username'].
should == ['can contain only letters, digits, '\
'\'_\', \'-\' and \'.\'. It must start with letter, digit or '\
'\'_\', optionally preceeded by \'.\'. It must not end in \'.git\'.']
end end
context "with existing user" do context "with existing user" do
before { before {
post api("/users", admin), { email: 'test@example.com', password: 'password', username: 'test', name: 'test' } post api("/users", admin), { email: 'test@example.com', password: 'password', username: 'test', name: 'test' }
post api("/users", admin), { email: 'foo@bar.com', password: 'password', username: 'john', name: 'john' } post api("/users", admin), { email: 'foo@bar.com', password: 'password', username: 'john', name: 'john' }
@user_id = User.all.last.id @user = User.all.last
} }
# it "should return 409 conflict error if email address exists" do it 'should return 409 conflict error if email address exists' do
# put api("/users/#{@user_id}", admin), { email: 'test@example.com' } put api("/users/#{@user.id}", admin), email: 'test@example.com'
# response.status.should == 409 response.status.should == 409
# end @user.reload.email.should == @user.email
# end
# it "should return 409 conflict error if username taken" do
# @user_id = User.all.last.id it 'should return 409 conflict error if username taken' do
# put api("/users/#{@user_id}", admin), { username: 'test' } @user_id = User.all.last.id
# response.status.should == 409 put api("/users/#{@user.id}", admin), username: 'test'
# end response.status.should == 409
@user.reload.username.should == @user.username
end
end end
end end
...@@ -229,7 +313,14 @@ describe API::API, api: true do ...@@ -229,7 +313,14 @@ describe API::API, api: true do
it "should not create invalid ssh key" do it "should not create invalid ssh key" do
post api("/users/#{user.id}/keys", admin), { title: "invalid key" } post api("/users/#{user.id}/keys", admin), { title: "invalid key" }
response.status.should == 404 response.status.should == 400
json_response['message'].should == '400 (Bad request) "key" not given'
end
it 'should not create key without title' do
post api("/users/#{user.id}/keys", admin), key: 'some key'
response.status.should == 400
json_response['message'].should == '400 (Bad request) "title" not given'
end end
it "should create ssh key" do it "should create ssh key" do
...@@ -254,6 +345,7 @@ describe API::API, api: true do ...@@ -254,6 +345,7 @@ describe API::API, api: true do
it 'should return 404 for non-existing user' do it 'should return 404 for non-existing user' do
get api('/users/999999/keys', admin) get api('/users/999999/keys', admin)
response.status.should == 404 response.status.should == 404
json_response['message'].should == '404 User Not Found'
end end
it 'should return array of ssh keys' do it 'should return array of ssh keys' do
...@@ -292,11 +384,13 @@ describe API::API, api: true do ...@@ -292,11 +384,13 @@ describe API::API, api: true do
user.save user.save
delete api("/users/999999/keys/#{key.id}", admin) delete api("/users/999999/keys/#{key.id}", admin)
response.status.should == 404 response.status.should == 404
json_response['message'].should == '404 User Not Found'
end end
it 'should return 404 error if key not foud' do it 'should return 404 error if key not foud' do
delete api("/users/#{user.id}/keys/42", admin) delete api("/users/#{user.id}/keys/42", admin)
response.status.should == 404 response.status.should == 404
json_response['message'].should == '404 Key Not Found'
end end
end end
end end
...@@ -324,6 +418,7 @@ describe API::API, api: true do ...@@ -324,6 +418,7 @@ describe API::API, api: true do
it "should return 404 for non-existing user" do it "should return 404 for non-existing user" do
delete api("/users/999999", admin) delete api("/users/999999", admin)
response.status.should == 404 response.status.should == 404
json_response['message'].should == '404 User Not Found'
end end
end end
...@@ -375,6 +470,7 @@ describe API::API, api: true do ...@@ -375,6 +470,7 @@ describe API::API, api: true do
it "should return 404 Not Found within invalid ID" do it "should return 404 Not Found within invalid ID" do
get api("/user/keys/42", user) get api("/user/keys/42", user)
response.status.should == 404 response.status.should == 404
json_response['message'].should == '404 Not found'
end end
it "should return 404 error if admin accesses user's ssh key" do it "should return 404 error if admin accesses user's ssh key" do
...@@ -383,6 +479,7 @@ describe API::API, api: true do ...@@ -383,6 +479,7 @@ describe API::API, api: true do
admin admin
get api("/user/keys/#{key.id}", admin) get api("/user/keys/#{key.id}", admin)
response.status.should == 404 response.status.should == 404
json_response['message'].should == '404 Not found'
end end
end end
...@@ -403,6 +500,13 @@ describe API::API, api: true do ...@@ -403,6 +500,13 @@ describe API::API, api: true do
it "should not create ssh key without key" do it "should not create ssh key without key" do
post api("/user/keys", user), title: 'title' post api("/user/keys", user), title: 'title'
response.status.should == 400 response.status.should == 400
json_response['message'].should == '400 (Bad request) "key" not given'
end
it 'should not create ssh key without title' do
post api('/user/keys', user), key: 'some key'
response.status.should == 400
json_response['message'].should == '400 (Bad request) "title" not given'
end end
it "should not create ssh key without title" do it "should not create ssh key without title" do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment