diff options
author | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2014-09-23 14:20:21 +0300 |
---|---|---|
committer | Marin Jankovski <marin@gitlab.com> | 2014-09-23 17:12:36 +0200 |
commit | 8ccae3c38528643bae9372857f0c85ec708c68ba (patch) | |
tree | f5058d97f1e2589587c235ad3e67631c73aa1eaf | |
parent | 0e8ab213be093097f61f6bbbdf916ff8d5a3c140 (diff) | |
download | gitlab-ce-8ccae3c38528643bae9372857f0c85ec708c68ba.tar.gz |
Fix MR commenting system when new commits pushed
This conmmit fixes wierd behaviour when you see wrong comments about new
commits in merge requests.
Short explanation of a bug:
When you push new commits to project we updated `project.merge_requests`
with comment about push. But `project.merge_requests` includes also
merge requests from forks to origin project. So when you push to master
branch it commented on all merge requests from forks to origin where
source_branch was `master`
-rw-r--r-- | app/models/project.rb | 14 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 56 |
2 files changed, 67 insertions, 3 deletions
diff --git a/app/models/project.rb b/app/models/project.rb index 114e40983f8..725b1f83630 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -422,15 +422,19 @@ class Project < ActiveRecord::Base end # Add comment about pushing new commits to merge requests - mrs = self.merge_requests.opened.where(source_branch: branch_name).to_a + 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.each do |merge_request| Note.create_new_commits_note(merge_request, merge_request.project, user, commits) end - - true end def valid_repo? @@ -599,4 +603,8 @@ class Project < ActiveRecord::Base def find_label(name) labels.find_by(name: name) end + + def origin_merge_requests + merge_requests.where(source_project_id: self.id) + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 1c11ac39567..c53ac820a9f 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -144,6 +144,62 @@ 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 |