From ae86a1b9d3c9ca4ce592fa89085acd059ffc09a0 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin <godfat@godfat.org> Date: Fri, 6 Jan 2017 02:11:27 +0800 Subject: [PATCH] Just trust set_commit_variables to set everything! Removing those weird setup in assign_change_commit_vars fixed all the failures in the tests. I still cannot say why but clearly we need to have better names. It's so confusing right now. We should seriously stop fiddling those instance variables. --- app/controllers/concerns/creates_commit.rb | 4 +--- app/controllers/projects/commit_controller.rb | 8 +++----- app/services/commits/change_service.rb | 3 ++- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/app/controllers/concerns/creates_commit.rb b/app/controllers/concerns/creates_commit.rb index 025fca088bf..eafaed8a3d0 100644 --- a/app/controllers/concerns/creates_commit.rb +++ b/app/controllers/concerns/creates_commit.rb @@ -4,11 +4,9 @@ module CreatesCommit def create_commit(service, success_path:, failure_path:, failure_view: nil, success_notice: nil) set_commit_variables - source_branch = @ref if - @ref && @mr_target_project.repository.branch_exists?(@ref) commit_params = @commit_params.merge( source_project: @mr_target_project, - source_branch: source_branch, + source_branch: @mr_target_branch, target_branch: @mr_source_branch ) diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index 791ed88db30..44f34006049 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -40,7 +40,7 @@ class Projects::CommitController < Projects::ApplicationController end def revert - assign_change_commit_vars(@commit.revert_branch_name) + assign_change_commit_vars return render_404 if @target_branch.blank? @@ -49,7 +49,7 @@ class Projects::CommitController < Projects::ApplicationController end def cherry_pick - assign_change_commit_vars(@commit.cherry_pick_branch_name) + assign_change_commit_vars return render_404 if @target_branch.blank? @@ -110,11 +110,9 @@ class Projects::CommitController < Projects::ApplicationController @ci_pipelines = project.pipelines.where(sha: commit.sha) end - def assign_change_commit_vars(mr_source_branch) + def assign_change_commit_vars @commit = project.commit(params[:id]) @target_branch = params[:target_branch] - @mr_source_branch = mr_source_branch - @mr_target_branch = @target_branch @commit_params = { commit: @commit, create_merge_request: params[:create_merge_request].present? || different_project? diff --git a/app/services/commits/change_service.rb b/app/services/commits/change_service.rb index 9b241aa8b04..60bd59a5d9f 100644 --- a/app/services/commits/change_service.rb +++ b/app/services/commits/change_service.rb @@ -5,6 +5,7 @@ module Commits def execute @source_project = params[:source_project] || @project + @source_branch = params[:source_branch] @target_branch = params[:target_branch] @commit = params[:commit] @create_merge_request = params[:create_merge_request].present? @@ -38,7 +39,7 @@ module Commits into, tree_id, source_project: @source_project, - source_branch_name: @target_branch) + source_branch_name: @source_branch) success else -- GitLab