diff options
author | Alex Sanford <alex.sanford1@gmail.com> | 2016-10-07 04:09:15 +0300 |
---|---|---|
committer | Alex Sanford <alex.sanford1@gmail.com> | 2016-11-01 13:28:22 +0300 |
commit | 3c2f40cd39cab63d6bf33e156123cc74aeb0b4a9 (patch) | |
tree | 5fc65223a251cab6fdd85789d86f88f9ea881c16 /app/services | |
parent | 458fa667f5273c3a91906c20cb9e580fb6cfaff4 (diff) |
Add validation errors to Merge Request form
If source branch or target branch doesn't exist in the project, display
form validation errors. Previously, this caused a 500 error code.
Diffstat (limited to 'app/services')
-rw-r--r-- | app/services/merge_requests/build_service.rb | 50 |
1 files changed, 34 insertions, 16 deletions
diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index 404f75616b5..f415244068b 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -13,20 +13,8 @@ module MergeRequests merge_request.target_project ||= (project.forked_from_project || project) merge_request.target_branch ||= merge_request.target_project.default_branch - if merge_request.target_branch.blank? || merge_request.source_branch.blank? - message = - if params[:source_branch] || params[:target_branch] - "You must select source and target branch" - end - - return build_failed(merge_request, message) - end - - if merge_request.source_project == merge_request.target_project && - merge_request.target_branch == merge_request.source_branch - - return build_failed(merge_request, 'You must select different branches') - end + messages = validate_branches(merge_request) + return build_failed(merge_request, messages) unless messages.empty? compare = CompareService.new.execute( merge_request.source_project, @@ -43,6 +31,34 @@ module MergeRequests private + def validate_branches(merge_request) + messages = [] + + if merge_request.target_branch.blank? || merge_request.source_branch.blank? + messages << + if params[:source_branch] || params[:target_branch] + "You must select source and target branch" + end + end + + if merge_request.source_project == merge_request.target_project && + merge_request.target_branch == merge_request.source_branch + + messages << 'You must select different branches' + end + + # See if source and target branches exist + unless merge_request.source_project.commit(merge_request.source_branch) + messages << "Source branch \"#{merge_request.source_branch}\" does not exist" + end + + unless merge_request.target_project.commit(merge_request.target_branch) + messages << "Target branch \"#{merge_request.target_branch}\" does not exist" + end + + messages + end + # When your branch name starts with an iid followed by a dash this pattern will be # interpreted as the user wants to close that issue on this project. # @@ -91,8 +107,10 @@ module MergeRequests merge_request end - def build_failed(merge_request, message) - merge_request.errors.add(:base, message) unless message.nil? + def build_failed(merge_request, messages) + messages.compact.each do |message| + merge_request.errors.add(:base, message) + end merge_request.compare_commits = [] merge_request.can_be_created = false merge_request |