diff options
| -rw-r--r-- | app/controllers/projects/merge_requests_controller.rb | 30 | ||||
| -rw-r--r-- | app/models/merge_request.rb | 15 | ||||
| -rw-r--r-- | app/models/project.rb | 4 | ||||
| -rw-r--r-- | app/views/projects/merge_requests/_form.html.haml | 2 | ||||
| -rw-r--r-- | lib/api/merge_requests.rb | 58 |
5 files changed, 42 insertions, 67 deletions
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index e6edceb1fbc..c090dd917c2 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -76,10 +76,10 @@ class Projects::MergeRequestsController < Projects::ApplicationController end def create - @merge_request = MergeRequest.new(params[:merge_request]) - @merge_request.author = current_user @target_branches ||= [] - if @merge_request.save + @merge_request = MergeRequests::CreateService.new(project, current_user, params[:merge_request]).execute + + if @merge_request.valid? redirect_to [@merge_request.target_project, @merge_request], notice: 'Merge request was successfully created.' else @source_project = @merge_request.source_project @@ -89,29 +89,9 @@ class Projects::MergeRequestsController < Projects::ApplicationController end def update - # If we close MergeRequest we want to ignore validation - # so we can close broken one (Ex. fork project removed) - if params[:merge_request] == {"state_event"=>"close"} - @merge_request.allow_broken = true - - if @merge_request.close - opts = { notice: 'Merge request was successfully closed.' } - else - opts = { alert: 'Failed to close merge request.' } - end - - redirect_to [@merge_request.target_project, @merge_request], opts - return - end - - # We dont allow change of source/target projects - # after merge request was created - params[:merge_request].delete(:source_project_id) - params[:merge_request].delete(:target_project_id) - - if @merge_request.update_attributes(params[:merge_request]) - @merge_request.reset_events_cache + @merge_request = MergeRequests::UpdateService.new(project, current_user, params[:merge_request]).execute(@merge_request) + if @merge_request.valid? respond_to do |format| format.js format.html do diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 5c2b0656d40..ffed03307ec 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -97,6 +97,7 @@ class MergeRequest < ActiveRecord::Base validates :target_project, presence: true validates :target_branch, presence: true validate :validate_branches + validate :validate_fork scope :of_group, ->(group) { where("source_project_id in (:group_project_ids) OR target_project_id in (:group_project_ids)", group_project_ids: group.project_ids) } scope :of_user_team, ->(team) { where("(source_project_id in (:team_project_ids) OR target_project_id in (:team_project_ids) AND assignee_id in (:team_member_ids))", team_project_ids: team.project_ids, team_member_ids: team.member_ids) } @@ -125,6 +126,20 @@ class MergeRequest < ActiveRecord::Base end end + def validate_fork + if target_projet == source_project + true + else + # If source and target projects are different + # we should check if source project is actually a fork of target project + if source_project.forked_from?(target_project) + true + else + errors.add :base, "Source project is not a fork of target project" + end + end + end + def update_merge_request_diff if source_branch_changed? || target_branch_changed? reload_code diff --git a/app/models/project.rb b/app/models/project.rb index 769ab217625..79066e1c54a 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -552,4 +552,8 @@ class Project < ActiveRecord::Base gitlab_shell.update_repository_head(self.path_with_namespace, branch) reload_default_branch end + + def forked_from?(project) + forked? && project == forked_from_project + end end diff --git a/app/views/projects/merge_requests/_form.html.haml b/app/views/projects/merge_requests/_form.html.haml index 22502760e50..0fe2d1d9801 100644 --- a/app/views/projects/merge_requests/_form.html.haml +++ b/app/views/projects/merge_requests/_form.html.haml @@ -33,7 +33,7 @@ .col-sm-10 .clearfix .pull-left - - projects = @project.forked_from_project.nil? ? [@project] : [ @project,@project.forked_from_project] + - projects = @project.forked_from_project.nil? ? [@project] : [@project, @project.forked_from_project] = f.select(:target_project_id, options_from_collection_for_select(projects, 'id', 'path_with_namespace', f.object.target_project_id), {}, { class: 'target_project select2 span3', disabled: @merge_request.persisted? }) .pull-left diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 3a1a00d0719..e2d2d034444 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -13,14 +13,6 @@ module API end not_found! end - - def not_fork?(target_project_id, user_project) - target_project_id.nil? || target_project_id == user_project.id.to_s - end - - def target_matches_fork(target_project_id,user_project) - user_project.forked? && user_project.forked_from_project.id.to_s == target_project_id - end end # List merge requests @@ -70,29 +62,15 @@ module API # POST /projects/:id/merge_requests # post ":id/merge_requests" do - set_current_user_for_thread do - authorize! :write_merge_request, user_project - required_attributes! [:source_branch, :target_branch, :title] - attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :target_project_id, :description] - merge_request = user_project.merge_requests.new(attrs) - merge_request.author = current_user - merge_request.source_project = user_project - target_project_id = attrs[:target_project_id] - if not_fork?(target_project_id, user_project) - merge_request.target_project = user_project - else - if target_matches_fork(target_project_id,user_project) - merge_request.target_project = Project.find_by(id: attrs[:target_project_id]) - else - render_api_error!('(Bad Request) Specified target project that is not the source project, or the source fork of the project.', 400) - end - end - - if merge_request.save - present merge_request, with: Entities::MergeRequest - else - handle_merge_request_errors! merge_request.errors - end + authorize! :write_merge_request, user_project + required_attributes! [:source_branch, :target_branch, :title] + attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :target_project_id, :description] + merge_request = ::MergeRequests::CreateService.new(user_project, current_user, attrs).execute + + if merge_request.valid? + present merge_request, with: Entities::MergeRequest + else + handle_merge_request_errors! merge_request.errors end end @@ -111,17 +89,15 @@ module API # PUT /projects/:id/merge_request/:merge_request_id # put ":id/merge_request/:merge_request_id" do - set_current_user_for_thread do - attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :state_event, :description] - merge_request = user_project.merge_requests.find(params[:merge_request_id]) - - authorize! :modify_merge_request, merge_request + attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :state_event, :description] + merge_request = user_project.merge_requests.find(params[:merge_request_id]) + authorize! :modify_merge_request, merge_request + merge_request = ::MergeRequests::UpdateService.new(user_project, current_user, attrs).execute(merge_request) - if merge_request.update_attributes attrs - present merge_request, with: Entities::MergeRequest - else - handle_merge_request_errors! merge_request.errors - end + if merge_request.valid? + present merge_request, with: Entities::MergeRequest + else + handle_merge_request_errors! merge_request.errors end end |