summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorOswaldo Ferreira <oswaldo@gitlab.com>2017-01-13 11:35:33 -0200
committerOswaldo Ferreira <oswaldo@gitlab.com>2017-01-23 12:45:53 -0200
commit4eb39e43a855cdf0e60b1d450a99bbae2e963b36 (patch)
tree715b6f1ae424d32edd90d39eb9bde68b79ec33f4
parent4b999ccd57c45ea8f30317c1ebde4623d67bf6f0 (diff)
downloadgitlab-ce-4eb39e43a855cdf0e60b1d450a99bbae2e963b36.tar.gz
Backport EE changes on approvals reset for closed MRsbackport-ee-1051-approvals-reset-on-closed-mr
-rw-r--r--app/models/project.rb2
-rw-r--r--app/services/merge_requests/base_service.rb16
-rw-r--r--app/services/merge_requests/refresh_service.rb22
-rw-r--r--spec/services/merge_requests/refresh_service_spec.rb51
4 files changed, 52 insertions, 39 deletions
diff --git a/app/models/project.rb b/app/models/project.rb
index 1630975b0d3..cd35601d76b 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -121,8 +121,6 @@ class Project < ActiveRecord::Base
# Merge Requests for target project should be removed with it
has_many :merge_requests, dependent: :destroy, foreign_key: 'target_project_id'
- # Merge requests from source project should be kept when source project was removed
- has_many :fork_merge_requests, foreign_key: 'source_project_id', class_name: 'MergeRequest'
has_many :issues, dependent: :destroy
has_many :labels, dependent: :destroy, class_name: 'ProjectLabel'
has_many :services, dependent: :destroy
diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb
index 70e25956dc7..5a53b973059 100644
--- a/app/services/merge_requests/base_service.rb
+++ b/app/services/merge_requests/base_service.rb
@@ -38,15 +38,13 @@ module MergeRequests
private
- def merge_requests_for(branch)
- origin_merge_requests = @project.origin_merge_requests
- .opened.where(source_branch: branch).to_a
-
- fork_merge_requests = @project.fork_merge_requests
- .opened.where(source_branch: branch).to_a
-
- (origin_merge_requests + fork_merge_requests)
- .uniq.select(&:source_project)
+ # Returns all origin and fork merge requests from `@project` satisfying passed arguments.
+ def merge_requests_for(source_branch, mr_states: [:opened])
+ MergeRequest
+ .with_state(mr_states)
+ .where(source_branch: source_branch, source_project_id: @project.id)
+ .preload(:source_project) # we don't need a #includes since we're just preloading for the #select
+ .select(&:source_project)
end
def pipeline_merge_requests(pipeline)
diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb
index 51d5d7563fc..b4bfb0e5e8c 100644
--- a/app/services/merge_requests/refresh_service.rb
+++ b/app/services/merge_requests/refresh_service.rb
@@ -42,7 +42,7 @@ module MergeRequests
commit_ids.include?(merge_request.diff_head_sha)
end
- merge_requests.uniq.select(&:source_project).each do |merge_request|
+ filter_merge_requests(merge_requests).each do |merge_request|
MergeRequests::PostMergeService.
new(merge_request.target_project, @current_user).
execute(merge_request)
@@ -58,10 +58,13 @@ module MergeRequests
def reload_merge_requests
merge_requests = @project.merge_requests.opened.
by_source_or_target_branch(@branch_name).to_a
- merge_requests += fork_merge_requests
- merge_requests = filter_merge_requests(merge_requests)
- merge_requests.each do |merge_request|
+ # Fork merge requests
+ merge_requests += MergeRequest.opened
+ .where(source_branch: @branch_name, source_project: @project)
+ .where.not(target_project: @project).to_a
+
+ filter_merge_requests(merge_requests).each do |merge_request|
if merge_request.source_branch == @branch_name || force_push?
merge_request.reload_diff
else
@@ -175,16 +178,7 @@ module MergeRequests
end
def merge_requests_for_source_branch
- @source_merge_requests ||= begin
- merge_requests = @project.origin_merge_requests.opened.where(source_branch: @branch_name).to_a
- merge_requests += fork_merge_requests
- filter_merge_requests(merge_requests)
- end
- end
-
- def fork_merge_requests
- @fork_merge_requests ||= @project.fork_merge_requests.opened.
- where(source_branch: @branch_name).to_a
+ @source_merge_requests ||= merge_requests_for(@branch_name)
end
def branch_added?
diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb
index 00d0e20f47c..314ea670a71 100644
--- a/spec/services/merge_requests/refresh_service_spec.rb
+++ b/spec/services/merge_requests/refresh_service_spec.rb
@@ -106,23 +106,46 @@ describe MergeRequests::RefreshService, services: true do
context 'push to fork repo source branch' do
let(:refresh_service) { service.new(@fork_project, @user) }
- 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
- expect(refresh_service).to have_received(:execute_hooks).
- with(@fork_merge_request, 'update', @oldrev)
+ context 'open fork merge request' 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
+ expect(refresh_service).to have_received(:execute_hooks).
+ with(@fork_merge_request, 'update', @oldrev)
+ end
+
+ it { expect(@merge_request.notes).to be_empty }
+ it { expect(@merge_request).to be_open }
+ it { expect(@fork_merge_request.notes.last.note).to include('added 28 commits') }
+ it { expect(@fork_merge_request).to be_open }
+ it { expect(@build_failed_todo).to be_pending }
+ it { expect(@fork_build_failed_todo).to be_pending }
end
- it { expect(@merge_request.notes).to be_empty }
- it { expect(@merge_request).to be_open }
- it { expect(@fork_merge_request.notes.last.note).to include('added 28 commits') }
- it { expect(@fork_merge_request).to be_open }
- it { expect(@build_failed_todo).to be_pending }
- it { expect(@fork_build_failed_todo).to be_pending }
+ context 'closed fork merge request' do
+ before do
+ @fork_merge_request.close!
+ allow(refresh_service).to receive(:execute_hooks)
+ refresh_service.execute(@oldrev, @newrev, 'refs/heads/master')
+ reload_mrs
+ end
+
+ it 'do not execute hooks with update action' do
+ expect(refresh_service).not_to have_received(:execute_hooks)
+ end
+
+ it { expect(@merge_request.notes).to be_empty }
+ it { expect(@merge_request).to be_open }
+ it { expect(@fork_merge_request.notes).to be_empty }
+ it { expect(@fork_merge_request).to be_closed }
+ it { expect(@build_failed_todo).to be_pending }
+ it { expect(@fork_build_failed_todo).to be_pending }
+ end
end
context 'push to fork repo target branch' do