diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 10bd76b1c35b8dfce053d5a87f11432a4275b48a..4894c6176743c27c4104b079b13e470ccd4ebb3f 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -122,9 +122,11 @@ class MergeRequest < ActiveRecord::Base 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 = similar_mrs.where('id not in (?)', self.id) if self.id - 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 @@ -140,7 +142,8 @@ class MergeRequest < ActiveRecord::Base if source_project.forked_from?(target_project) true 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 diff --git a/doc/api/README.md b/doc/api/README.md index ababb7b6999a87199f54038398cab5ad4101c3bc..f76a253083f1ee962ec85fe8a5526619327fda04 100644 --- a/doc/api/README.md +++ b/doc/api/README.md @@ -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 - `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 +- `422 Unprocessable` - The entity could not be processed - `500 Server Error` - While handling the request something went wrong on the server side ## Sudo @@ -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. 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>", + ... + ], + } + } + } diff --git a/lib/api/deploy_keys.rb b/lib/api/deploy_keys.rb index 7f5a125038cdb1bf5bb76c0e75dc9b17c26bced3..06eb7756841fe4ef5540bfef81480cb6fd2ddda5 100644 --- a/lib/api/deploy_keys.rb +++ b/lib/api/deploy_keys.rb @@ -58,7 +58,7 @@ module API if key.valid? && user_project.deploy_keys << key present key, with: Entities::SSHKey else - not_found! + render_validation_error!(key) end end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 6af0f6d1b252b3a296bb289779b771511dee55ca..3a619169ecabf130a5513a5dacc19ccab79e8e5d 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -155,7 +155,17 @@ module API end 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 def render_api_error!(message, status) diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 5369149cdfccefa55e63a9568041c1806b7636bf..30170c657bafeda9977d60881b0beea3bda39869 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -109,7 +109,7 @@ module API present issue, with: Entities::Issue else - not_found! + render_validation_error!(issue) end end @@ -149,7 +149,7 @@ module API present issue, with: Entities::Issue else - not_found! + render_validation_error!(issue) end end diff --git a/lib/api/labels.rb b/lib/api/labels.rb index 2fdf53ffec21f530febb635c9b2cf69bbb166268..78ca58ad0d13462b83473724cdb169eef4673e09 100644 --- a/lib/api/labels.rb +++ b/lib/api/labels.rb @@ -30,16 +30,14 @@ module API attrs = attributes_for_keys [:name, :color] label = user_project.find_label(attrs[:name]) - if label - return render_api_error!('Label already exists', 409) - end + conflict!('Label already exists') if label label = user_project.labels.create(attrs) if label.valid? present label, with: Entities::Label else - render_api_error!(label.errors.full_messages.join(', '), 400) + render_validation_error!(label) end end @@ -56,9 +54,7 @@ module API required_attributes! [:name] label = user_project.find_label(params[:name]) - if !label - return render_api_error!('Label not found', 404) - end + not_found!('Label') unless label label.destroy end @@ -66,10 +62,11 @@ module API # 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) + # id (required) - The ID of a project + # 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 + # notation with leading '#' sign (e.g. #FFAABB) # Example Request: # PUT /projects/:id/labels put ':id/labels' do @@ -77,16 +74,14 @@ module API required_attributes! [:name] label = user_project.find_label(params[:name]) - if !label - return render_api_error!('Label not found', 404) - end + not_found!('Label not found') unless label attrs = attributes_for_keys [:new_name, :color] if attrs.empty? - return render_api_error!('Required parameters "name" or "color" ' \ - 'missing', - 400) + render_api_error!('Required parameters "new_name" or "color" ' \ + 'missing', + 400) end # Rename new name to the actual label attribute name @@ -95,7 +90,7 @@ module API if label.update(attrs) present label, with: Entities::Label else - render_api_error!(label.errors.full_messages.join(', '), 400) + render_validation_error!(label) end end end diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 172c8504d0f67af98f928631688b9c1c93eead67..a365f1db00f6f07ad4c6fba2fd75a8725e89a3db 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -10,8 +10,13 @@ module API error!(errors[:project_access], 422) elsif errors[:branch_conflict].any? 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 - not_found! + + render_api_error!(errors, 400) end end @@ -228,7 +233,7 @@ module API if note.save present note, with: Entities::MRNote else - not_found! + render_validation_error!(note) end end end diff --git a/lib/api/project_snippets.rb b/lib/api/project_snippets.rb index 8e09fff68436f30d5bbca94c75b0300b59d89d3d..0c2d282f785134734a7d212eed004abb9972716f 100644 --- a/lib/api/project_snippets.rb +++ b/lib/api/project_snippets.rb @@ -56,7 +56,7 @@ module API if @snippet.save present @snippet, with: Entities::ProjectSnippet else - not_found! + render_validation_error!(@snippet) end end @@ -80,7 +80,7 @@ module API if @snippet.update_attributes attrs present @snippet, with: Entities::ProjectSnippet else - not_found! + render_validation_error!(@snippet) end end @@ -97,6 +97,7 @@ module API authorize! :modify_project_snippet, @snippet @snippet.destroy rescue + not_found!('Snippet') end end diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 55f7975bbf776f80a3dd0280c9ce4a92441729d8..f555819df1bf2d385478c58d3a7845cc55e88c51 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -111,7 +111,7 @@ module API if @project.errors[:limit_reached].present? error!(@project.errors[:limit_reached], 403) end - not_found! + render_validation_error!(@project) end end @@ -149,7 +149,7 @@ module API if @project.saved? present @project, with: Entities::Project else - not_found! + render_validation_error!(@project) end end diff --git a/lib/api/users.rb b/lib/api/users.rb index 69553f163978d04d67c70abdc23a033943f049db..d07815a8a9774577d4abe5d9af78666d887dc334 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -42,7 +42,8 @@ module API # Parameters: # email (required) - Email # password (required) - Password - # name - Name + # name (required) - Name + # username (required) - Name # skype - Skype ID # linkedin - Linkedin # twitter - Twitter account @@ -65,7 +66,15 @@ module API if user.save present user, with: Entities::UserFull 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 @@ -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] user = User.find(params[:id]) - not_found!("User not found") unless user + not_found!('User') unless user admin = attrs.delete(:admin) 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) present user, with: Entities::UserFull else - not_found! + render_validation_error!(user) end end @@ -113,13 +131,15 @@ module API # POST /users/:id/keys post ":id/keys" do authenticated_as_admin! + required_attributes! [:title, :key] + user = User.find(params[:id]) attrs = attributes_for_keys [:title, :key] key = user.keys.new attrs if key.save present key, with: Entities::SSHKey else - not_found! + render_validation_error!(key) end end @@ -132,11 +152,9 @@ module API get ':uid/keys' do authenticated_as_admin! user = User.find_by(id: params[:uid]) - if user - present user.keys, with: Entities::SSHKey - else - not_found! - end + not_found!('User') unless user + + present user.keys, with: Entities::SSHKey end # Delete existing ssh key of a specified user. Only available to admin @@ -150,15 +168,13 @@ module API delete ':uid/keys/:id' do authenticated_as_admin! user = User.find_by(id: params[:uid]) - if user - begin - key = user.keys.find params[:id] - key.destroy - rescue ActiveRecord::RecordNotFound - not_found! - end - else - not_found! + not_found!('User') unless user + + begin + key = user.keys.find params[:id] + key.destroy + rescue ActiveRecord::RecordNotFound + not_found!('Key') end end @@ -173,7 +189,7 @@ module API if user user.destroy else - not_found! + not_found!('User') end end end @@ -219,7 +235,7 @@ module API if key.save present key, with: Entities::SSHKey else - not_found! + render_validation_error!(key) end end diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index e8eebda95b40943b24fcd94e6821081561b128f3..9876452f81df656a909667ed69325ecde09ebe43 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -169,6 +169,15 @@ describe API::API, api: true do response.status.should == 400 json_response['message']['labels']['?']['title'].should == ['is invalid'] 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 describe "PUT /projects/:id/issues/:issue_id to update only title" 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' 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 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 ee9088933a1cf609fda1864779d8be4bb284c0d6..dbddc8a7da40438390a29ebd1f1a958c60aca1e9 100644 --- a/spec/requests/api/labels_spec.rb +++ b/spec/requests/api/labels_spec.rb @@ -47,7 +47,7 @@ describe API::API, api: true do name: 'Foo', color: '#FFAA' response.status.should == 400 - json_response['message'].should == 'Color is invalid' + json_response['message']['color'].should == ['is invalid'] end it 'should return 400 for too long color code' do @@ -55,7 +55,7 @@ describe API::API, api: true do name: 'Foo', color: '#FFAAFFFF' response.status.should == 400 - json_response['message'].should == 'Color is invalid' + json_response['message']['color'].should == ['is invalid'] end it 'should return 400 for invalid name' do @@ -63,7 +63,7 @@ describe API::API, api: true do name: '?', color: '#FFAABB' response.status.should == 400 - json_response['message'].should == 'Title is invalid' + json_response['message']['title'].should == ['is invalid'] end it 'should return 409 if label already exists' do @@ -84,7 +84,7 @@ describe API::API, api: true do 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' + json_response['message'].should == '404 Label Not Found' end it 'should return 400 for wrong parameters' do @@ -132,11 +132,14 @@ describe API::API, api: true do it 'should return 400 if no label name given' do put api("/projects/#{project.id}/labels", user), new_name: 'label2' response.status.should == 400 + json_response['message'].should == '400 (Bad request) "name" not given' end it 'should return 400 if no new parameters given' do put api("/projects/#{project.id}/labels", user), name: 'label1' response.status.should == 400 + json_response['message'].should == 'Required parameters '\ + '"new_name" or "color" missing' end it 'should return 400 for invalid name' do @@ -145,7 +148,7 @@ describe API::API, api: true do new_name: '?', color: '#FFFFFF' response.status.should == 400 - json_response['message'].should == 'Title is invalid' + json_response['message']['title'].should == ['is invalid'] end it 'should return 400 for invalid name' do @@ -153,7 +156,7 @@ describe API::API, api: true do name: 'label1', color: '#FF' response.status.should == 400 - json_response['message'].should == 'Color is invalid' + json_response['message']['color'].should == ['is invalid'] end it 'should return 400 for too long color code' do @@ -161,7 +164,7 @@ describe API::API, api: true do name: 'Foo', color: '#FFAAFFFF' response.status.should == 400 - json_response['message'].should == 'Color is invalid' + json_response['message']['color'].should == ['is invalid'] end end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index d39962670b1ade65d45f560bc463faa77d722675..5ba3a33099154e4d6c917489103b9f96d9c91e54 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -163,6 +163,28 @@ describe API::API, api: true do json_response['message']['labels']['?']['title'].should == ['is invalid'] 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 context 'forked projects' do @@ -210,16 +232,26 @@ describe API::API, api: true do response.status.should == 400 end - it "should return 404 when target_branch is specified and not a forked project" do - 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 - response.status.should == 404 - end + 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), + title: 'Test merge_request', + target_branch: 'master', + source_branch: 'stable', + author: user, + target_project_id: fork_project.id + response.status.should == 422 + end - it "should return 404 when target_branch is specified and for a different fork" do - 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 - response.status.should == 404 + it 'should return 422 if targeting a different fork' do + 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 + response.status.should == 422 + end end it "should return 201 when target_branch is specified and for the same project" do @@ -256,7 +288,7 @@ describe API::API, api: true do merge_request.close put api("/projects/#{project.id}/merge_request/#{merge_request.id}/merge", user) response.status.should == 405 - json_response['message'].should == 'Method Not Allowed' + json_response['message'].should == '405 Method Not Allowed' end it "should return 401 if user has no permissions to merge" do @@ -316,7 +348,8 @@ describe API::API, api: true do end 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 end end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 5575da86c2e558d640ab2c9150a1ece2a37b184e..571d85062779e7fdfb3779ff8c8abb20aff0e51d 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -188,9 +188,24 @@ describe API::API, api: true do response.status.should == 201 end - it "should respond with 404 on failure" do + it 'should respond with 400 on failure' do 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 it "should assign attributes to project" do @@ -410,9 +425,9 @@ describe API::API, api: true do response.status.should == 200 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) - response.status.should == 200 + response.status.should == 404 end end @@ -459,7 +474,21 @@ describe API::API, api: true do describe "POST /projects/:id/keys" do it "should not create an invalid ssh key" do 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 it "should create new ssh key" do diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 8bbe9b5b736d8faa7a3c75ed9b97ff88452e4147..b0752ebe43c004328f4c27371a1db7ceca4dcf15 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -51,6 +51,7 @@ describe API::API, api: true do it "should return a 404 error if user id not found" do get api("/users/9999", user) response.status.should == 404 + json_response['message'].should == '404 Not found' end end @@ -98,18 +99,47 @@ describe API::API, api: true do end 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 end - it "should return 400 error if password not given" do - post api("/users", admin), { email: 'test@example.com' } + it 'should return 400 error if name not given' do + 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 end 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 + 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 it "shouldn't available for non admin users" do @@ -117,21 +147,37 @@ describe API::API, api: true do response.status.should == 403 end - context "with existing user" do - before { post api("/users", admin), { email: 'test@example.com', password: 'password', username: 'test' } } + context 'with existing user' do + 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 { - 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) + response.status.should == 409 + json_response['message'].should == 'Email has already been taken' end - it "should return 409 conflict error if user with email exists" do - post api("/users", admin), { email: 'test@example.com', password: 'password' } - end - - it "should return 409 conflict error if same username exists" do - post api("/users", admin), { email: 'foo@example.com', password: 'pass', username: 'test' } + it 'should return 409 conflict error if same username exists' do + expect do + post api('/users', admin), + name: 'foo', + email: 'foo@example.com', + 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 @@ -173,6 +219,20 @@ describe API::API, api: true do user.reload.bio.should == 'new test bio' 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 put api("/users/#{user.id}", admin), {admin: true} response.status.should == 200 @@ -190,7 +250,7 @@ describe API::API, api: true do it "should not allow invalid update" do put api("/users/#{user.id}", admin), {email: 'invalid email'} - response.status.should == 404 + response.status.should == 400 user.reload.email.should_not == 'invalid email' end @@ -202,25 +262,49 @@ describe API::API, api: true do it "should return 404 for non-existing user" do put api("/users/999999", admin), {bio: 'update should fail'} 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 context "with existing user" do before { 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' } - @user_id = User.all.last.id + @user = User.all.last } -# it "should return 409 conflict error if email address exists" do -# put api("/users/#{@user_id}", admin), { email: 'test@example.com' } -# response.status.should == 409 -# end -# -# it "should return 409 conflict error if username taken" do -# @user_id = User.all.last.id -# put api("/users/#{@user_id}", admin), { username: 'test' } -# response.status.should == 409 -# end + it 'should return 409 conflict error if email address exists' do + put api("/users/#{@user.id}", admin), email: 'test@example.com' + response.status.should == 409 + @user.reload.email.should == @user.email + end + + it 'should return 409 conflict error if username taken' do + @user_id = User.all.last.id + put api("/users/#{@user.id}", admin), username: 'test' + response.status.should == 409 + @user.reload.username.should == @user.username + end end end @@ -229,7 +313,14 @@ describe API::API, api: true do it "should not create invalid ssh key" do 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 it "should create ssh key" do @@ -254,6 +345,7 @@ describe API::API, api: true do it 'should return 404 for non-existing user' do get api('/users/999999/keys', admin) response.status.should == 404 + json_response['message'].should == '404 User Not Found' end it 'should return array of ssh keys' do @@ -292,11 +384,13 @@ describe API::API, api: true do user.save delete api("/users/999999/keys/#{key.id}", admin) response.status.should == 404 + json_response['message'].should == '404 User Not Found' end it 'should return 404 error if key not foud' do delete api("/users/#{user.id}/keys/42", admin) response.status.should == 404 + json_response['message'].should == '404 Key Not Found' end end end @@ -324,6 +418,7 @@ describe API::API, api: true do it "should return 404 for non-existing user" do delete api("/users/999999", admin) response.status.should == 404 + json_response['message'].should == '404 User Not Found' end end @@ -375,6 +470,7 @@ describe API::API, api: true do it "should return 404 Not Found within invalid ID" do get api("/user/keys/42", user) response.status.should == 404 + json_response['message'].should == '404 Not found' end it "should return 404 error if admin accesses user's ssh key" do @@ -383,6 +479,7 @@ describe API::API, api: true do admin get api("/user/keys/#{key.id}", admin) response.status.should == 404 + json_response['message'].should == '404 Not found' end end @@ -403,6 +500,13 @@ describe API::API, api: true do it "should not create ssh key without key" do post api("/user/keys", user), title: 'title' 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 it "should not create ssh key without title" do