From 7fa80b5bd01caff61c08c70b052c9965893cce5a Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Tue, 30 Dec 2014 13:36:13 +0100 Subject: [PATCH 1/8] Update branch api not found messages to 'Branch not found'. --- lib/api/branches.rb | 9 +++++---- lib/api/helpers.rb | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/api/branches.rb b/lib/api/branches.rb index 6ec1a753a69..b52d786e020 100644 --- a/lib/api/branches.rb +++ b/lib/api/branches.rb @@ -14,7 +14,8 @@ module API # Example Request: # GET /projects/:id/repository/branches get ":id/repository/branches" do - present user_project.repository.branches.sort_by(&:name), with: Entities::RepoObject, project: user_project + branches = user_project.repository.branches.sort_by(&:name) + present branches, with: Entities::RepoObject, project: user_project end # Get a single branch @@ -26,7 +27,7 @@ module API # GET /projects/:id/repository/branches/:branch get ':id/repository/branches/:branch', requirements: { branch: /.*/ } do @branch = user_project.repository.branches.find { |item| item.name == params[:branch] } - not_found!("Branch does not exist") if @branch.nil? + not_found!("Branch") unless @branch present @branch, with: Entities::RepoObject, project: user_project end @@ -43,7 +44,7 @@ module API authorize_admin_project @branch = user_project.repository.find_branch(params[:branch]) - not_found! unless @branch + not_found!("Branch") unless @branch protected_branch = user_project.protected_branches.find_by(name: @branch.name) user_project.protected_branches.create(name: @branch.name) unless protected_branch @@ -63,7 +64,7 @@ module API authorize_admin_project @branch = user_project.repository.find_branch(params[:branch]) - not_found! unless @branch + not_found!("Branch does not exist") unless @branch protected_branch = user_project.protected_branches.find_by(name: @branch.name) protected_branch.destroy if protected_branch diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 2f2342840fd..62c26ef76ce 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -42,7 +42,7 @@ module API def user_project @project ||= find_project(params[:id]) - @project || not_found! + @project || not_found!("Project") end def find_project(id) From d4b613ded728bbd45e5d67e5af555d661581ecf0 Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Tue, 30 Dec 2014 14:00:07 +0100 Subject: [PATCH 2/8] Clearer message if adding comment to commit via api fails. --- lib/api/commits.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/commits.rb b/lib/api/commits.rb index 6c5391b98c8..1aea6943000 100644 --- a/lib/api/commits.rb +++ b/lib/api/commits.rb @@ -108,7 +108,7 @@ module API if note.save present note, with: Entities::CommitNote else - not_found! + error!("Failed to save note", 422) end end end From ed464edabeb62e35363ebadd0a5bb5ff394b6781 Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Tue, 30 Dec 2014 14:29:55 +0100 Subject: [PATCH 3/8] Message for api files and groups. --- lib/api/commits.rb | 2 +- lib/api/files.rb | 4 ++-- lib/api/groups.rb | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/api/commits.rb b/lib/api/commits.rb index 1aea6943000..8e528e266bf 100644 --- a/lib/api/commits.rb +++ b/lib/api/commits.rb @@ -108,7 +108,7 @@ module API if note.save present note, with: Entities::CommitNote else - error!("Failed to save note", 422) + render_api_error!("Failed to save note #{note.errors.messages}", 422) end end end diff --git a/lib/api/files.rb b/lib/api/files.rb index 84e1d311781..e6e71bac367 100644 --- a/lib/api/files.rb +++ b/lib/api/files.rb @@ -35,7 +35,7 @@ module API file_path = attrs.delete(:file_path) commit = user_project.repository.commit(ref) - not_found! "Commit" unless commit + not_found! 'Commit' unless commit blob = user_project.repository.blob_at(commit.sha, file_path) @@ -53,7 +53,7 @@ module API commit_id: commit.id, } else - render_api_error!('File not found', 404) + not_found! 'File' end end diff --git a/lib/api/groups.rb b/lib/api/groups.rb index a2d915a7eca..cee51c82ad5 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -54,7 +54,7 @@ module API if @group.save present @group, with: Entities::Group else - not_found! + render_api_error!("Failed to save group #{@group.errors.messages}", 422) end end @@ -97,7 +97,7 @@ module API if result present group else - not_found! + render_api_error!("Failed to transfer project #{project.errors.messages}", 422) end end end From 7240150c8986c7aa21d0eb3140ac9a4c7674a4d2 Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Tue, 30 Dec 2014 15:17:46 +0100 Subject: [PATCH 4/8] Forward the messages in api response. --- lib/api/milestones.rb | 4 ++-- lib/api/notes.rb | 2 +- lib/api/project_hooks.rb | 4 ++-- lib/api/project_members.rb | 2 +- lib/api/projects.rb | 2 +- lib/api/repositories.rb | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/api/milestones.rb b/lib/api/milestones.rb index a4fdb752d69..4d79f5a69ae 100644 --- a/lib/api/milestones.rb +++ b/lib/api/milestones.rb @@ -48,7 +48,7 @@ module API if milestone.valid? present milestone, with: Entities::Milestone else - not_found! + not_found!("Milestone #{milestone.errors.messages}") end end @@ -72,7 +72,7 @@ module API if milestone.valid? present milestone, with: Entities::Milestone else - not_found! + not_found!("Milestone #{milestone.errors.messages}") end end end diff --git a/lib/api/notes.rb b/lib/api/notes.rb index b29c054a044..b04d623c695 100644 --- a/lib/api/notes.rb +++ b/lib/api/notes.rb @@ -61,7 +61,7 @@ module API if @note.valid? present @note, with: Entities::Note else - not_found! + not_found!("Note #{@note.errors.messages}") end end diff --git a/lib/api/project_hooks.rb b/lib/api/project_hooks.rb index 7d056b9bf58..be9850367b9 100644 --- a/lib/api/project_hooks.rb +++ b/lib/api/project_hooks.rb @@ -53,7 +53,7 @@ module API if @hook.errors[:url].present? error!("Invalid url given", 422) end - not_found! + not_found!("Project hook #{@hook.errors.messages}") end end @@ -82,7 +82,7 @@ module API if @hook.errors[:url].present? error!("Invalid url given", 422) end - not_found! + not_found!("Project hook #{@hook.errors.messages}") end end diff --git a/lib/api/project_members.rb b/lib/api/project_members.rb index 1595ed0bc36..8e32f124ea5 100644 --- a/lib/api/project_members.rb +++ b/lib/api/project_members.rb @@ -9,7 +9,7 @@ module API if errors[:access_level].any? error!(errors[:access_level], 422) end - not_found! + not_found!(errors) end end diff --git a/lib/api/projects.rb b/lib/api/projects.rb index d6dd03656a6..e1cc2348865 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -227,7 +227,7 @@ module API render_api_error!("Project already forked", 409) end else - not_found! + not_found!("Source Project") end end diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb index a1a7721b288..03a556a2c55 100644 --- a/lib/api/repositories.rb +++ b/lib/api/repositories.rb @@ -133,7 +133,7 @@ module API env['api.format'] = :binary present data else - not_found! + not_found!('File') end end From 0da5154b5a71216a9bbff861561636906ca8c167 Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Tue, 30 Dec 2014 15:40:11 +0100 Subject: [PATCH 5/8] Fix api tests. --- spec/requests/api/fork_spec.rb | 4 ++-- spec/requests/api/groups_spec.rb | 3 ++- spec/requests/api/projects_spec.rb | 4 ++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/spec/requests/api/fork_spec.rb b/spec/requests/api/fork_spec.rb index cbbd1e7de5a..5921b3e0698 100644 --- a/spec/requests/api/fork_spec.rb +++ b/spec/requests/api/fork_spec.rb @@ -44,7 +44,7 @@ describe API::API, api: true do it 'should fail on missing project access for the project to fork' do post api("/projects/fork/#{project.id}", user3) response.status.should == 404 - json_response['message'].should == '404 Not Found' + json_response['message'].should == '404 Project Not Found' end it 'should fail if forked project exists in the user namespace' do @@ -58,7 +58,7 @@ describe API::API, api: true do it 'should fail if project to fork from does not exist' do post api('/projects/fork/424242', user) response.status.should == 404 - json_response['message'].should == '404 Not Found' + json_response['message'].should == '404 Project Not Found' end end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 8dfd2cd650e..a5aade06cba 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -91,7 +91,8 @@ describe API::API, api: true do it "should not create group, duplicate" do post api("/groups", admin), {name: "Duplicate Test", path: group2.path} - response.status.should == 404 + response.status.should == 422 + response.message.should == "Unprocessable Entity" end it "should return 400 bad request error if name not given" do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index f8c5d40b9bf..79865f15f06 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -289,7 +289,7 @@ describe API::API, api: true do it "should return a 404 error if not found" do get api("/projects/42", user) response.status.should == 404 - json_response['message'].should == '404 Not Found' + json_response['message'].should == '404 Project Not Found' end it "should return a 404 error if user is not a member" do @@ -340,7 +340,7 @@ describe API::API, api: true do it "should return a 404 error if not found" do get api("/projects/42/events", user) response.status.should == 404 - json_response['message'].should == '404 Not Found' + json_response['message'].should == '404 Project Not Found' end it "should return a 404 error if user is not a member" do From cd0aed3d54fc01d0c361a8cf282d2de48297f66a Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Wed, 7 Jan 2015 10:46:00 +0100 Subject: [PATCH 6/8] Add a message when unable to save an object through api. --- lib/api/commits.rb | 2 +- lib/api/deploy_keys.rb | 2 +- lib/api/groups.rb | 4 ++-- lib/api/issues.rb | 8 ++++---- lib/api/labels.rb | 4 ++-- lib/api/merge_requests.rb | 4 ++-- lib/api/milestones.rb | 4 ++-- lib/api/notes.rb | 2 +- 8 files changed, 15 insertions(+), 15 deletions(-) diff --git a/lib/api/commits.rb b/lib/api/commits.rb index 8e528e266bf..0de4e720ffe 100644 --- a/lib/api/commits.rb +++ b/lib/api/commits.rb @@ -108,7 +108,7 @@ module API if note.save present note, with: Entities::CommitNote else - render_api_error!("Failed to save note #{note.errors.messages}", 422) + render_api_error!("Failed to save note #{note.errors.messages}", 400) end end end diff --git a/lib/api/deploy_keys.rb b/lib/api/deploy_keys.rb index 06eb7756841..dd4b761feb2 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 - render_validation_error!(key) + render_api_error!("Failed to add key #{key.errors.messages}", 400) end end diff --git a/lib/api/groups.rb b/lib/api/groups.rb index cee51c82ad5..bda60b3b7d5 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -54,7 +54,7 @@ module API if @group.save present @group, with: Entities::Group else - render_api_error!("Failed to save group #{@group.errors.messages}", 422) + render_api_error!("Failed to save group #{@group.errors.messages}", 400) end end @@ -97,7 +97,7 @@ module API if result present group else - render_api_error!("Failed to transfer project #{project.errors.messages}", 422) + render_api_error!("Failed to transfer project #{project.errors.messages}", 400) end end end diff --git a/lib/api/issues.rb b/lib/api/issues.rb index d2828b24c36..01496c39955 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -104,7 +104,7 @@ module API # Validate label names in advance if (errors = validate_label_params(params)).any? - render_api_error!({ labels: errors }, 400) + render_api_error!("Unable to validate label: #{errors}"}, 400) end issue = ::Issues::CreateService.new(user_project, current_user, attrs).execute @@ -118,7 +118,7 @@ module API present issue, with: Entities::Issue else - render_validation_error!(issue) + render_api_error!("Unable to create issue #{issue.errors.messages}", 400) end end @@ -142,7 +142,7 @@ module API # Validate label names in advance if (errors = validate_label_params(params)).any? - render_api_error!({ labels: errors }, 400) + render_api_error!("Unable to validate label: #{errors}"}, 400) end issue = ::Issues::UpdateService.new(user_project, current_user, attrs).execute(issue) @@ -158,7 +158,7 @@ module API present issue, with: Entities::Issue else - render_validation_error!(issue) + render_api_error!("Unable to update issue #{issue.errors.messages}", 400) end end diff --git a/lib/api/labels.rb b/lib/api/labels.rb index 78ca58ad0d1..e8ded662253 100644 --- a/lib/api/labels.rb +++ b/lib/api/labels.rb @@ -37,7 +37,7 @@ module API if label.valid? present label, with: Entities::Label else - render_validation_error!(label) + render_api_error!("Unable to create label #{label.errors.messages}", 400) end end @@ -90,7 +90,7 @@ module API if label.update(attrs) present label, with: Entities::Label else - render_validation_error!(label) + render_api_error!("Unable to create label #{label.errors.messages}", 400) end end end diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index a365f1db00f..1a73c4943b8 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -137,7 +137,7 @@ module API # Validate label names in advance if (errors = validate_label_params(params)).any? - render_api_error!({ labels: errors }, 400) + render_api_error!("Unable to validate label: #{errors}"}, 400) end merge_request = ::MergeRequests::UpdateService.new(user_project, current_user, attrs).execute(merge_request) @@ -233,7 +233,7 @@ module API if note.save present note, with: Entities::MRNote else - render_validation_error!(note) + render_api_error!("Failed to save note #{note.errors.messages}", 400) end end end diff --git a/lib/api/milestones.rb b/lib/api/milestones.rb index 4d79f5a69ae..2ea49359df0 100644 --- a/lib/api/milestones.rb +++ b/lib/api/milestones.rb @@ -48,7 +48,7 @@ module API if milestone.valid? present milestone, with: Entities::Milestone else - not_found!("Milestone #{milestone.errors.messages}") + render_api_error!("Failed to create milestone #{milestone.errors.messages}", 400) end end @@ -72,7 +72,7 @@ module API if milestone.valid? present milestone, with: Entities::Milestone else - not_found!("Milestone #{milestone.errors.messages}") + render_api_error!("Failed to update milestone #{milestone.errors.messages}", 400) end end end diff --git a/lib/api/notes.rb b/lib/api/notes.rb index b04d623c695..3726be7c537 100644 --- a/lib/api/notes.rb +++ b/lib/api/notes.rb @@ -93,7 +93,7 @@ module API if @note.valid? present @note, with: Entities::Note else - bad_request!('Invalid note') + render_api_error!("Failed to save note #{note.errors.messages}", 400) end end From 8dd672776ee72990fd41b37559a8ba102595d6ca Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Wed, 7 Jan 2015 11:39:20 +0100 Subject: [PATCH 7/8] Fix failing tests due to updates on the return messages. --- lib/api/deploy_keys.rb | 2 +- lib/api/issues.rb | 8 ++++---- lib/api/labels.rb | 4 ++-- lib/api/merge_requests.rb | 2 +- spec/requests/api/groups_spec.rb | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/api/deploy_keys.rb b/lib/api/deploy_keys.rb index dd4b761feb2..06eb7756841 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 - render_api_error!("Failed to add key #{key.errors.messages}", 400) + render_validation_error!(key) end end diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 01496c39955..d2828b24c36 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -104,7 +104,7 @@ module API # Validate label names in advance if (errors = validate_label_params(params)).any? - render_api_error!("Unable to validate label: #{errors}"}, 400) + render_api_error!({ labels: errors }, 400) end issue = ::Issues::CreateService.new(user_project, current_user, attrs).execute @@ -118,7 +118,7 @@ module API present issue, with: Entities::Issue else - render_api_error!("Unable to create issue #{issue.errors.messages}", 400) + render_validation_error!(issue) end end @@ -142,7 +142,7 @@ module API # Validate label names in advance if (errors = validate_label_params(params)).any? - render_api_error!("Unable to validate label: #{errors}"}, 400) + render_api_error!({ labels: errors }, 400) end issue = ::Issues::UpdateService.new(user_project, current_user, attrs).execute(issue) @@ -158,7 +158,7 @@ module API present issue, with: Entities::Issue else - render_api_error!("Unable to update issue #{issue.errors.messages}", 400) + render_validation_error!(issue) end end diff --git a/lib/api/labels.rb b/lib/api/labels.rb index e8ded662253..78ca58ad0d1 100644 --- a/lib/api/labels.rb +++ b/lib/api/labels.rb @@ -37,7 +37,7 @@ module API if label.valid? present label, with: Entities::Label else - render_api_error!("Unable to create label #{label.errors.messages}", 400) + render_validation_error!(label) end end @@ -90,7 +90,7 @@ module API if label.update(attrs) present label, with: Entities::Label else - render_api_error!("Unable to create label #{label.errors.messages}", 400) + render_validation_error!(label) end end end diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 1a73c4943b8..81038d05f12 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -137,7 +137,7 @@ module API # Validate label names in advance if (errors = validate_label_params(params)).any? - render_api_error!("Unable to validate label: #{errors}"}, 400) + render_api_error!({ labels: errors }, 400) end merge_request = ::MergeRequests::UpdateService.new(user_project, current_user, attrs).execute(merge_request) diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index a5aade06cba..95f82463367 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -91,8 +91,8 @@ describe API::API, api: true do it "should not create group, duplicate" do post api("/groups", admin), {name: "Duplicate Test", path: group2.path} - response.status.should == 422 - response.message.should == "Unprocessable Entity" + response.status.should == 400 + response.message.should == "Bad Request" end it "should return 400 bad request error if name not given" do From fd100e381a1b9f36830813d7b4549cbbb2562773 Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Wed, 7 Jan 2015 11:39:43 +0100 Subject: [PATCH 8/8] Add returned API messages updates to changelog. --- CHANGELOG | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 9fafbbba673..b87d8a2cada 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -14,7 +14,7 @@ v 7.7.0 - - - New side navigation - - + - Updates to the messages returned by API (sponsored by O'Reilly Media) - - - Add alert message in case of outdated browser (IE < 10)