Merge branch 'fix-mr-commit-comment' into 'master'

Fix MR commenting system when new commits pushed

Fixes #1608

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`

See merge request !1108

2 files changed, 67 insertions, 3 deletions

diff --git a/app/models/project.rb b/app/models/project.rb
index 0adedaa8dcd..7edc15b7782 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 21800ab98ff..524cab2b925 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