diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 6bd8111c..becb3bce 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -55,6 +55,12 @@ module Gitlab render_api_error!('403 Forbidden', 403) end + def bad_request!(attribute) + message = ["400 (Bad request)"] + message << "\"" + attribute.to_s + "\" not given" + render_api_error!(message.join(' '), 400) + end + def not_found!(resource = nil) message = ["404"] message << resource if resource diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index a0ca3026..0c18ece3 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -13,9 +13,9 @@ module Gitlab # def handle_merge_request_error(merge_request_errors) if merge_request_errors[:target_branch].any? - error!(merge_request_errors[:target_branch], 400) + bad_request!(:target_branch) elsif merge_request_errors[:source_branch].any? - error!(merge_request_errors[:source_branch], 400) + bad_request!(:source_branch) elsif merge_request_errors[:base].any? error!(merge_request_errors[:base], 422) end @@ -129,7 +129,7 @@ module Gitlab present note, with: Entities::MRNote else if note.errors[:note].any? - error!(note.errors[:note], 400) + bad_request!(:note) end not_found! end diff --git a/lib/api/milestones.rb b/lib/api/milestones.rb index 1f7d0876..cdb0e146 100644 --- a/lib/api/milestones.rb +++ b/lib/api/milestones.rb @@ -13,7 +13,7 @@ module Gitlab # def handle_milestone_errors(milestone_errors) if milestone_errors[:title].any? - error!(milestone_errors[:title], 400) + bad_request!(:title) end end end diff --git a/lib/api/notes.rb b/lib/api/notes.rb index 47dead9d..56de6e09 100644 --- a/lib/api/notes.rb +++ b/lib/api/notes.rb @@ -44,7 +44,7 @@ module Gitlab present @note, with: Entities::Note else # :note is exposed as :body, but :note is set on error - error!(@note.errors[:note], 400) if @note.errors[:note].any? + bad_request!(:note) if @note.errors[:note].any? not_found! end end diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 24761cd5..ecd3401f 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -45,7 +45,7 @@ module Gitlab # Example Request # POST /projects post do - error!("Name is required", 400) if !params.has_key? :name + bad_request!(:name) if !params.has_key? :name attrs = attributes_for_keys [:name, :description, :default_branch, @@ -101,8 +101,8 @@ module Gitlab post ":id/members" do authorize! :admin_project, user_project - error!("User id not given", 400) if !params.has_key? :user_id - error!("Access level not given", 400) if !params.has_key? :access_level + bad_request!(:user_id) if !params.has_key? :user_id + bad_request!(:access_level) if !params.has_key? :access_level # either the user is already a team member or a new one team_member = user_project.team_member_by_id(params[:user_id]) @@ -133,8 +133,8 @@ module Gitlab authorize! :admin_project, user_project team_member = user_project.users_projects.find_by_user_id(params[:user_id]) - error!("Access level not given", 400) if !params.has_key? :access_level - error!("User can not be found", 404) if team_member.nil? + bad_request!(:access_level) if !params.has_key? :access_level + not_found!("User can not be found") if team_member.nil? if team_member.update_attributes(project_access: params[:access_level]) @member = team_member.user @@ -196,7 +196,7 @@ module Gitlab post ":id/hooks" do authorize! :admin_project, user_project - error!("Url not given", 400) unless params.has_key? :url + bad_request!(:url) unless params.has_key? :url @hook = user_project.hooks.new({"url" => params[:url]}) if @hook.save @@ -218,7 +218,7 @@ module Gitlab @hook = user_project.hooks.find(params[:hook_id]) authorize! :admin_project, user_project - error!("Url not given", 400) unless params.has_key? :url + bad_request!(:url) unless params.has_key? :url attrs = attributes_for_keys [:url] if @hook.update_attributes attrs @@ -237,7 +237,7 @@ module Gitlab # DELETE /projects/:id/hooks delete ":id/hooks" do authorize! :admin_project, user_project - error!("Hook id not given", 400) unless params.has_key? :hook_id + bad_request!(:hook_id) unless params.has_key? :hook_id begin @hook = ProjectHook.find(params[:hook_id]) @@ -368,9 +368,9 @@ module Gitlab post ":id/snippets" do authorize! :write_snippet, user_project - error!("Title not given", 400) if !params[:title].present? - error!("Filename not given", 400) if !params[:file_name].present? - error!("Code not given", 400) if !params[:code].present? + bad_request!(:title) if !params[:title].present? + bad_request!(:file_name) if !params[:file_name].present? + bad_request!(:code) if !params[:code].present? attrs = attributes_for_keys [:title, :file_name] attrs[:expires_at] = params[:lifetime] if params[:lifetime].present? @@ -451,7 +451,7 @@ module Gitlab get ":id/repository/commits/:sha/blob" do authorize! :download_code, user_project - error!("Filepath must be specified", 400) if !params.has_key? :filepath + bad_request!(:filepath) if !params.has_key? :filepath ref = params[:sha] diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 4e7a84df..431aae7c 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -32,6 +32,11 @@ describe Gitlab::API do response.status.should == 200 json_response['title'].should == merge_request.title end + + it "should return a 404 error if merge_request_id not found" do + get api("/projects/#{project.id}/merge_request/999", user) + response.status.should == 404 + end end describe "POST /projects/:id/merge_requests" do