diff options
| -rw-r--r-- | app/controllers/merge_requests_controller.rb | 2 | ||||
| -rw-r--r-- | app/models/merge_request.rb | 25 | ||||
| -rw-r--r-- | app/processors/merge_request_git_backend_processor.rb | 20 | ||||
| -rw-r--r-- | test/functional/merge_requests_controller_test.rb | 6 | ||||
| -rw-r--r-- | test/unit/merge_request_test.rb | 33 | ||||
| -rw-r--r-- | test/unit/processors/merge_request_git_backend_processor_test.rb | 25 |
6 files changed, 67 insertions, 44 deletions
diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index 7b2c24b..077b44b 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -207,7 +207,7 @@ class MergeRequestsController < ApplicationController end def destroy - @merge_request.soft_delete + @merge_request.destroy flash[:success] = I18n.t "merge_requests_controller.destroy_success" redirect_to [@owner, @repository] end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index b7b53e8..42d4acf 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -31,7 +31,7 @@ class MergeRequest < ActiveRecord::Base has_many :versions, :class_name => 'MergeRequestVersion', :order => 'version' before_destroy :nullify_messages - + after_destroy :delete_tracking_branches is_indexed :fields => ["proposal", {:field => "status_tag", :as => "status"}], :include => [{ @@ -497,18 +497,25 @@ class MergeRequest < ActiveRecord::Base user, "new version #{current_version_number}") end - def delete_target_repository_ref - source_repository.git.git.push({},target_repository.full_repository_path, ":#{merge_branch_name}") - end - - # Since we'll be deleting the ref in the backend, this will be handled in the message queue - def soft_delete - msg = {:merge_request_id => to_param, :action => "delete"} + # Since we'll be deleting the ref in the backend, this will be + # handled in the message queue + def delete_tracking_branches + msg = { + :merge_request_id => to_param, + :action => "delete", + :target_path => target_repository.full_repository_path, + :target_name => target_repository.url_path, + :merge_branch_name => merge_branch_name, + :source_repository_id => source_repository.id, + :target_repository_id => target_repository.id, + } publish :merge_request_backend_updates, msg.to_json end def tracking_repository - target_repository.create_tracking_repository unless target_repository.has_tracking_repository? + unless target_repository.has_tracking_repository? + target_repository.create_tracking_repository + end target_repository.tracking_repository end diff --git a/app/processors/merge_request_git_backend_processor.rb b/app/processors/merge_request_git_backend_processor.rb index 56dc977..7f557b3 100644 --- a/app/processors/merge_request_git_backend_processor.rb +++ b/app/processors/merge_request_git_backend_processor.rb @@ -28,18 +28,28 @@ class MergeRequestGitBackendProcessor < ApplicationProcessor @body["action"].to_sym end - def merge_request - @merge_request ||= MergeRequest.find(@body["merge_request_id"]) + def source_repository + @source_repository ||= Repository.find(@body["source_repository_id"]) + end + + def target_repository + @target_repository ||= Repository.find(@body["target_repository_id"]) + end + + def delete_target_repository_ref + source_repository.git.git.push({:timeout => false}, + target_repository.full_repository_path, + ":#{@body['merge_branch_name']}") end private def do_delete - logger.info("Deleting tracking branch #{merge_request.merge_branch_name} for merge request in target repository #{merge_request.target_repository.id}") + logger.info("Deleting tracking branch #{@body['merge_branch_name']} for merge request " + + "in target repository #{@body['target_name']}") begin - merge_request.delete_target_repository_ref + delete_target_repository_ref rescue Grit::NoSuchPathError => e logger.error "Could not find Git path. Message is #{e.message}" end - merge_request.destroy end end diff --git a/test/functional/merge_requests_controller_test.rb b/test/functional/merge_requests_controller_test.rb index b84aced..5f8e0b4 100644 --- a/test/functional/merge_requests_controller_test.rb +++ b/test/functional/merge_requests_controller_test.rb @@ -643,9 +643,9 @@ class MergeRequestsControllerTest < ActionController::TestCase should "soft-delete the record" do login_as :johan - MergeRequest.stubs(:find).returns(@merge_request) - @merge_request.expects(:soft_delete) - do_delete + assert_difference("@target_repository.merge_requests.open.count", -1) do + do_delete + end assert_redirected_to(project_repository_path(@project, @target_repository)) assert_match(/merge request was retracted/i, flash[:success]) end diff --git a/test/unit/merge_request_test.rb b/test/unit/merge_request_test.rb index 74350cc..166c25b 100644 --- a/test/unit/merge_request_test.rb +++ b/test/unit/merge_request_test.rb @@ -645,29 +645,24 @@ class MergeRequestTest < ActiveSupport::TestCase end end - context "Deletion of merge requests in the target git repository" do - setup do - @merge_request = merge_requests(:moes_to_johans) - @source_git_repo = mock - @source_git = mock - @source_git_repo.stubs(:git).returns(@source_git) - @merge_request.source_repository.stubs(:git).returns(@source_git_repo) - end - - should "push a deletion to the target repository" do - @source_git.expects(:push).with({}, @merge_request.target_repository.full_repository_path, ":#{@merge_request.merge_branch_name}") - @merge_request.delete_target_repository_ref - end - end - context "Soft deletion of merge requests" do setup do @merge_request = merge_requests(:moes_to_johans_open) end - should "send a message when being soft deleted" do - p = proc {@merge_request.soft_delete} - message = find_message_with_queue_and_regexp('/queue/GitoriousMergeRequestBackend', /.*/) {p.call} - assert_equal({'merge_request_id' => @merge_request.id.to_s, "action" => "delete"}, message) + + should "send a message when being destroyed" do + deletion_proc = proc{ @merge_request.destroy } + msg = find_message_with_queue_and_regexp('/queue/GitoriousMergeRequestBackend', /.*/) { + deletion_proc.call + } + assert_nil MergeRequest.find_by_id(@merge_request.id) + assert_equal @merge_request.id.to_s, msg["merge_request_id"] + assert_equal "delete", msg["action"] + assert_equal @merge_request.target_repository.full_repository_path, msg["target_path"] + assert_equal @merge_request.target_repository.url_path, msg["target_name"] + assert_equal @merge_request.merge_branch_name, msg["merge_branch_name"] + assert_equal @merge_request.target_repository.id, msg["target_repository_id"] + assert_equal @merge_request.source_repository.id, msg["source_repository_id"] end end end diff --git a/test/unit/processors/merge_request_git_backend_processor_test.rb b/test/unit/processors/merge_request_git_backend_processor_test.rb index c6e0288..ee8a99e 100644 --- a/test/unit/processors/merge_request_git_backend_processor_test.rb +++ b/test/unit/processors/merge_request_git_backend_processor_test.rb @@ -33,18 +33,30 @@ class MergeRequestGitBackendProcessorTest < ActiveSupport::TestCase context "Deleting the merge request and its tracking branch" do setup do - @msg = {:merge_request_id => @merge_request.to_param, :action => "delete"} - @processor.instance_variable_set("@merge_request", @merge_request) + @source_git_repo = mock + @source_git = mock + @source_git_repo.stubs(:git).returns(@source_git) + @merge_request.source_repository.stubs(:git).returns(@source_git_repo) + @msg = { + :merge_request_id => @merge_request.to_param, + :action => "delete", + :target_path => @merge_request.target_repository.full_repository_path, + :target_name => @merge_request.target_repository.url_path, + :merge_branch_name => @merge_request.merge_branch_name, + :source_repository_id => @merge_request.source_repository.id, + :target_repository_id => @merge_request.target_repository.id, + } end - should "delete the tracking repo and the merge request itself" do - @merge_request.expects(:delete_target_repository_ref).once - @merge_request.expects(:destroy).once + should "push to delete the tracking branch" do + @processor.stubs(:source_repository).returns(@merge_request.source_repository) + @source_git.expects(:push).with({:timeout => false}, + @merge_request.target_repository.full_repository_path, + ":#{@merge_request.merge_branch_name}") @processor.on_message(@msg.to_json) end should "handle non-existing target gits" do - @merge_request.expects(:destroy).once assert_nothing_raised do @processor.on_message(@msg.to_json) end @@ -57,7 +69,6 @@ class MergeRequestGitBackendProcessorTest < ActiveSupport::TestCase @processor.expects(:do_delete).once @processor.on_message(msg.to_json) assert_equal :delete, @processor.action - assert_equal @merge_request, @processor.merge_request end should "understand the close command" do |