Commit b5ef6d226864d3ea132d2c6e97b74b51f2b64a6f
1 parent
cce35b6d
Exists in
master
and in
4 other branches
API: refactored and simplified error handling in merge requests API
Showing
2 changed files
with
8 additions
and
18 deletions
Show diff stats
lib/api/merge_requests.rb
| @@ -4,21 +4,12 @@ module Gitlab | @@ -4,21 +4,12 @@ module Gitlab | ||
| 4 | before { authenticate! } | 4 | before { authenticate! } |
| 5 | 5 | ||
| 6 | resource :projects do | 6 | resource :projects do |
| 7 | - | ||
| 8 | helpers do | 7 | helpers do |
| 9 | - # If an error occurred this helper method provides an appropriate status code | ||
| 10 | - # | ||
| 11 | - # Parameters: | ||
| 12 | - # merge_request_errors (required) - The errors collection of MR | ||
| 13 | - # | ||
| 14 | - def handle_merge_request_error(merge_request_errors) | ||
| 15 | - if merge_request_errors[:target_branch].any? | ||
| 16 | - bad_request!(:target_branch) | ||
| 17 | - elsif merge_request_errors[:source_branch].any? | ||
| 18 | - bad_request!(:source_branch) | ||
| 19 | - elsif merge_request_errors[:base].any? | ||
| 20 | - error!(merge_request_errors[:base], 422) | 8 | + def handle_merge_request_errors!(errors) |
| 9 | + if errors[:project_access].any? | ||
| 10 | + error!(errors[:project_access], 422) | ||
| 21 | end | 11 | end |
| 12 | + not_found! | ||
| 22 | end | 13 | end |
| 23 | end | 14 | end |
| 24 | 15 | ||
| @@ -78,8 +69,7 @@ module Gitlab | @@ -78,8 +69,7 @@ module Gitlab | ||
| 78 | merge_request.reload_code | 69 | merge_request.reload_code |
| 79 | present merge_request, with: Entities::MergeRequest | 70 | present merge_request, with: Entities::MergeRequest |
| 80 | else | 71 | else |
| 81 | - handle_merge_request_error(merge_request.errors) | ||
| 82 | - not_found! | 72 | + handle_merge_request_errors! merge_request.errors |
| 83 | end | 73 | end |
| 84 | end | 74 | end |
| 85 | 75 | ||
| @@ -107,8 +97,7 @@ module Gitlab | @@ -107,8 +97,7 @@ module Gitlab | ||
| 107 | merge_request.mark_as_unchecked | 97 | merge_request.mark_as_unchecked |
| 108 | present merge_request, with: Entities::MergeRequest | 98 | present merge_request, with: Entities::MergeRequest |
| 109 | else | 99 | else |
| 110 | - handle_merge_request_error(merge_request.errors) | ||
| 111 | - not_found! | 100 | + handle_merge_request_errors! merge_request.errors |
| 112 | end | 101 | end |
| 113 | end | 102 | end |
| 114 | 103 |
lib/api/projects.rb
| @@ -233,7 +233,7 @@ module Gitlab | @@ -233,7 +233,7 @@ module Gitlab | ||
| 233 | end | 233 | end |
| 234 | end | 234 | end |
| 235 | 235 | ||
| 236 | - # Delete project hook | 236 | + # Deletes project hook. This is an idempotent function. |
| 237 | # | 237 | # |
| 238 | # Parameters: | 238 | # Parameters: |
| 239 | # id (required) - The ID of a project | 239 | # id (required) - The ID of a project |
| @@ -248,6 +248,7 @@ module Gitlab | @@ -248,6 +248,7 @@ module Gitlab | ||
| 248 | @hook = ProjectHook.find(params[:hook_id]) | 248 | @hook = ProjectHook.find(params[:hook_id]) |
| 249 | @hook.destroy | 249 | @hook.destroy |
| 250 | rescue | 250 | rescue |
| 251 | + # ProjectHook can raise Error if hook_id not found | ||
| 251 | end | 252 | end |
| 252 | end | 253 | end |
| 253 | 254 |