diff options
author | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2014-11-11 16:49:26 +0200 |
---|---|---|
committer | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2014-11-11 16:49:26 +0200 |
commit | 2139e3519b1f1023478bec087cf94f2ec237c0c7 (patch) | |
tree | 15ddc95dd2008c4949b8835de4280f6e7d226264 | |
parent | ccd842c3a7742d1d99a13a32ea725032810f2690 (diff) | |
download | gitlab-ce-2139e3519b1f1023478bec087cf94f2ec237c0c7.tar.gz |
Refactor merge request refresh logic on push
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
-rw-r--r-- | app/models/project.rb | 39 | ||||
-rw-r--r-- | app/services/merge_requests/refresh_service.rb | 67 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 57 | ||||
-rw-r--r-- | spec/services/merge_requests/refresh_service_spec.rb | 98 |
4 files changed, 167 insertions, 94 deletions
diff --git a/app/models/project.rb b/app/models/project.rb index 1383bf3c46e..d2576bb85d0 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -402,43 +402,8 @@ class Project < ActiveRecord::Base end def update_merge_requests(oldrev, newrev, ref, user) - return true unless ref =~ /heads/ - branch_name = ref.gsub("refs/heads/", "") - commits = self.repository.commits_between(oldrev, newrev) - c_ids = commits.map(&:id) - - # Close merge requests - mrs = self.merge_requests.opened.where(target_branch: branch_name).to_a - mrs = mrs.select(&:last_commit).select { |mr| c_ids.include?(mr.last_commit.id) } - - mrs.uniq.select(&:source_project).each do |merge_request| - MergeRequests::MergeService.new.execute(merge_request, user, nil) - end - - # Update code for merge requests into project between project branches - mrs = self.merge_requests.opened.by_branch(branch_name).to_a - # Update code for merge requests between project and project fork - mrs += self.fork_merge_requests.opened.by_branch(branch_name).to_a - - mrs.uniq.select(&:source_project).each do |merge_request| - merge_request.reload_code - merge_request.mark_as_unchecked - end - - # Add comment about pushing new commits to merge requests - comment_mr_with_commits(branch_name, commits, user) - - true - end - - def comment_mr_with_commits(branch_name, commits, user) - mrs = self.origin_merge_requests.opened.where(source_branch: branch_name).to_a - mrs += self.fork_merge_requests.opened.where(source_branch: branch_name).to_a - - mrs.uniq.select(&:source_project).each do |merge_request| - Note.create_new_commits_note(merge_request, merge_request.project, - user, commits) - end + MergeRequests::RefreshService.new(self, user). + execute(oldrev, newrev, ref) end def valid_repo? diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb new file mode 100644 index 00000000000..74448998ddd --- /dev/null +++ b/app/services/merge_requests/refresh_service.rb @@ -0,0 +1,67 @@ +module MergeRequests + class RefreshService < MergeRequests::BaseService + def execute(oldrev, newrev, ref) + return true unless ref =~ /heads/ + + @branch_name = ref.gsub("refs/heads/", "") + @fork_merge_requests = @project.fork_merge_requests.opened + @commits = @project.repository.commits_between(oldrev, newrev) + + close_merge_requests + reload_merge_requests + comment_mr_with_commits + + true + end + + private + + # 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 + commit_ids = @commits.map(&:id) + merge_requests = @project.merge_requests.opened.where(target_branch: @branch_name).to_a + merge_requests = merge_requests.select(&:last_commit) + + merge_requests = merge_requests.select do |merge_request| + commit_ids.include?(merge_request.last_commit.id) + end + + + merge_requests.uniq.select(&:source_project).each do |merge_request| + MergeRequests::MergeService.new.execute(merge_request, @current_user, nil) + end + end + + # Refresh merge request diff if we push to source or target branch of merge request + # Note: we should update merge requests from forks too + def reload_merge_requests + merge_requests = @project.merge_requests.opened.by_branch(@branch_name).to_a + merge_requests += @fork_merge_requests.by_branch(@branch_name).to_a + merge_requests = filter_merge_requests(merge_requests) + + merge_requests.each do |merge_request| + merge_request.reload_code + merge_request.mark_as_unchecked + end + end + + # Add comment about pushing new commits to merge requests + def comment_mr_with_commits + merge_requests = @project.origin_merge_requests.opened.where(source_branch: @branch_name).to_a + merge_requests += @fork_merge_requests.where(source_branch: @branch_name).to_a + merge_requests = filter_merge_requests(merge_requests) + + merge_requests.each do |merge_request| + Note.create_new_commits_note(merge_request, merge_request.project, + @current_user, @commits) + end + end + + def filter_merge_requests(merge_requests) + merge_requests.uniq.select(&:source_project) + end + end +end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 48b58400a1e..70a15cac1a8 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -145,63 +145,6 @@ describe Project do end end - describe 'comment merge requests with commits' do - before do - @user = create(:user) - group = create(:group) - group.add_owner(@user) - - @project = create(:project, namespace: group) - @fork_project = Projects::ForkService.new(@project, @user).execute - @merge_request = create(:merge_request, source_project: @project, - source_branch: 'master', - target_branch: 'feature', - target_project: @project) - @fork_merge_request = create(:merge_request, source_project: @fork_project, - source_branch: 'master', - target_branch: 'feature', - target_project: @project) - - @commits = @merge_request.commits - end - - context 'push to origin repo source branch' do - before do - @project.comment_mr_with_commits('master', @commits, @user) - end - - it { @merge_request.notes.should_not be_empty } - it { @fork_merge_request.notes.should be_empty } - end - - context 'push to origin repo target branch' do - before do - @project.comment_mr_with_commits('feature', @commits, @user) - end - - it { @merge_request.notes.should be_empty } - it { @fork_merge_request.notes.should be_empty } - end - - context 'push to fork repo source branch' do - before do - @fork_project.comment_mr_with_commits('master', @commits, @user) - end - - it { @merge_request.notes.should be_empty } - it { @fork_merge_request.notes.should_not be_empty } - end - - context 'push to fork repo target branch' do - before do - @fork_project.comment_mr_with_commits('feature', @commits, @user) - end - - it { @merge_request.notes.should be_empty } - it { @fork_merge_request.notes.should be_empty } - end - end - describe :find_with_namespace do context 'with namespace' do before do diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb new file mode 100644 index 00000000000..9f294152053 --- /dev/null +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -0,0 +1,98 @@ +require 'spec_helper' + +describe MergeRequests::RefreshService do + let(:project) { create(:project) } + let(:user) { create(:user) } + let(:service) { MergeRequests::RefreshService } + + describe :execute do + before do + @user = create(:user) + group = create(:group) + group.add_owner(@user) + + @project = create(:project, namespace: group) + @fork_project = Projects::ForkService.new(@project, @user).execute + @merge_request = create(:merge_request, source_project: @project, + source_branch: 'master', + target_branch: 'feature', + target_project: @project) + + @fork_merge_request = create(:merge_request, source_project: @fork_project, + source_branch: 'master', + target_branch: 'feature', + target_project: @project) + + @commits = @merge_request.commits + + @oldrev = @commits.last.id + @newrev = @commits.first.id + end + + context 'push to origin repo source branch' do + before do + service.new(@project, @user).execute(@oldrev, @newrev, 'refs/heads/master') + reload_mrs + end + + it { @merge_request.notes.should_not be_empty } + it { @merge_request.should be_open } + it { @fork_merge_request.should be_open } + it { @fork_merge_request.notes.should be_empty } + end + + context 'push to origin repo target branch' do + before do + service.new(@project, @user).execute(@oldrev, @newrev, 'refs/heads/feature') + reload_mrs + end + + it { @merge_request.notes.should be_empty } + it { @merge_request.should be_merged } + it { @fork_merge_request.should be_merged } + it { @fork_merge_request.notes.should be_empty } + end + + context 'push to fork repo source branch' do + before do + service.new(@fork_project, @user).execute(@oldrev, @newrev, 'refs/heads/master') + reload_mrs + end + + it { @merge_request.notes.should be_empty } + it { @merge_request.should be_open } + it { @fork_merge_request.notes.should_not be_empty } + it { @fork_merge_request.should be_open } + end + + context 'push to fork repo target branch' do + before do + service.new(@fork_project, @user).execute(@oldrev, @newrev, 'refs/heads/feature') + reload_mrs + end + + it { @merge_request.notes.should be_empty } + it { @merge_request.should be_open } + it { @fork_merge_request.notes.should be_empty } + it { @fork_merge_request.should be_open } + end + + context 'push to origin repo target branch after fork project was removed' do + before do + @fork_project.destroy + service.new(@project, @user).execute(@oldrev, @newrev, 'refs/heads/feature') + reload_mrs + end + + it { @merge_request.notes.should be_empty } + it { @merge_request.should be_merged } + it { @fork_merge_request.should be_open } + it { @fork_merge_request.notes.should be_empty } + end + + def reload_mrs + @merge_request.reload + @fork_merge_request.reload + end + end +end |