summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/services/merge_requests/refresh_service.rb16
-rw-r--r--changelogs/unreleased/osw-fix-lost-diffs-when-source-branch-deleted.yml5
-rw-r--r--spec/models/merge_request_spec.rb2
-rw-r--r--spec/services/merge_requests/refresh_service_spec.rb25
4 files changed, 43 insertions, 5 deletions
diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb
index 9f05535d4d4..262622f8bd0 100644
--- a/app/services/merge_requests/refresh_service.rb
+++ b/app/services/merge_requests/refresh_service.rb
@@ -9,7 +9,8 @@ module MergeRequests
Gitlab::GitalyClient.allow_n_plus_1_calls(&method(:find_new_commits))
# Be sure to close outstanding MRs before reloading them to avoid generating an
# empty diff during a manual merge
- close_merge_requests
+ close_upon_missing_source_branch_ref
+ post_merge_manually_merged
reload_merge_requests
reset_merge_when_pipeline_succeeds
mark_pending_todos_done
@@ -29,11 +30,22 @@ module MergeRequests
private
+ def close_upon_missing_source_branch_ref
+ # MergeRequest#reload_diff ignores not opened MRs. This means it won't
+ # create an `empty` diff for `closed` MRs without a source branch, keeping
+ # the latest diff state as the last _valid_ one.
+ merge_requests_for_source_branch.reject(&:source_branch_exists?).each do |mr|
+ MergeRequests::CloseService
+ .new(mr.target_project, @current_user)
+ .execute(mr)
+ end
+ end
+
# Collect open merge requests that target same branch we push into
# and close if push to master include last commit from merge request
# We need this to close(as merged) merge requests that were merged into
# target branch manually
- def close_merge_requests
+ def post_merge_manually_merged
commit_ids = @commits.map(&:id)
merge_requests = @project.merge_requests.preload(:latest_merge_request_diff).opened.where(target_branch: @branch_name).to_a
merge_requests = merge_requests.select(&:diff_head_commit)
diff --git a/changelogs/unreleased/osw-fix-lost-diffs-when-source-branch-deleted.yml b/changelogs/unreleased/osw-fix-lost-diffs-when-source-branch-deleted.yml
new file mode 100644
index 00000000000..1cffb213f23
--- /dev/null
+++ b/changelogs/unreleased/osw-fix-lost-diffs-when-source-branch-deleted.yml
@@ -0,0 +1,5 @@
+---
+title: Close and do not reload MR diffs when source branch is deleted
+merge_request:
+author:
+type: fixed
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 15f9c68e400..eb9690df313 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -1539,7 +1539,7 @@ describe MergeRequest do
expect { subject.reload_diff }.to change { subject.merge_request_diffs.count }.by(1)
end
- it "executs diff cache service" do
+ it "executes diff cache service" do
expect_any_instance_of(MergeRequests::MergeRequestDiffCacheService).to receive(:execute).with(subject)
subject.reload_diff
diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb
index 7a01d3dd698..7c3374c6113 100644
--- a/spec/services/merge_requests/refresh_service_spec.rb
+++ b/spec/services/merge_requests/refresh_service_spec.rb
@@ -55,11 +55,12 @@ describe MergeRequests::RefreshService do
before do
allow(refresh_service).to receive(:execute_hooks)
- refresh_service.execute(@oldrev, @newrev, 'refs/heads/master')
- reload_mrs
end
it 'executes hooks with update action' do
+ refresh_service.execute(@oldrev, @newrev, 'refs/heads/master')
+ reload_mrs
+
expect(refresh_service).to have_received(:execute_hooks)
.with(@merge_request, 'update', old_rev: @oldrev)
@@ -72,6 +73,26 @@ describe MergeRequests::RefreshService do
expect(@build_failed_todo).to be_done
expect(@fork_build_failed_todo).to be_done
end
+
+ context 'when source branch ref does not exists' do
+ before do
+ DeleteBranchService.new(@project, @user).execute(@merge_request.source_branch)
+ end
+
+ it 'closes MRs without source branch ref' do
+ expect { refresh_service.execute(@oldrev, @newrev, 'refs/heads/master') }
+ .to change { @merge_request.reload.state }
+ .from('opened')
+ .to('closed')
+
+ expect(@fork_merge_request.reload).to be_open
+ end
+
+ it 'does not change the merge request diff' do
+ expect { refresh_service.execute(@oldrev, @newrev, 'refs/heads/master') }
+ .not_to change { @merge_request.reload.merge_request_diff }
+ end
+ end
end
context 'when pipeline exists for the source branch' do