Merge branch 'master' of dev.gitlab.org:gitlab/gitlabhq into ce-to-ee

CHANGELOG

@@ -91,6 +91,7 @@ v 7.9.0 (unreleased)
   - Allow user to choose a public email to show on public profile
   - Remove truncation from issue titles on milestone page (Jason Blanchard)
   - Fix stuck Merge Request merging events from old installations (Ben Bodenmiller)
+  - Fix merge request comments on files with multiple commits
 
 v 7.9.3
   - Contains no changes

app/helpers/diff_helper.rb

@@ -101,7 +101,7 @@ module DiffHelper
   end
 
   def line_comments
-    @line_comments ||= @line_notes.group_by(&:line_code)
+    @line_comments ||= @line_notes.select(&:active?).group_by(&:line_code)
   end
 
   def organize_comments(type_left, type_right, line_code_left, line_code_right)

app/models/note.rb

@@ -359,7 +359,7 @@ class Note < ActiveRecord::Base
   def set_diff
     # First lets find notes with same diff
     # before iterating over all mr diffs
-    diff = Note.where(noteable_id: self.noteable_id, noteable_type: self.noteable_type, line_code: self.line_code).last.try(:diff)
+    diff = diff_for_line_code unless for_merge_request?
     diff ||= find_diff
 
     self.st_diff = diff.to_hash if diff
@@ -369,6 +369,10 @@ class Note < ActiveRecord::Base
     @diff ||= Gitlab::Git::Diff.new(st_diff) if st_diff.respond_to?(:map)
   end
 
+  def diff_for_line_code
+    Note.where(noteable_id: noteable_id, noteable_type: noteable_type, line_code: line_code).last.try(:diff)
+  end
+
   # Check if such line of code exists in merge request diff
   # If exists - its active discussion
   # If not - its outdated diff

app/services/notification_service.rb

@@ -130,22 +130,25 @@ class NotificationService
     if note.commit_id.present?
       recipients << note.commit_author
     end
-
-    # Get users who left comment in thread
-    recipients = recipients.concat(noteable_commenters(note))
+    
+    # Add all users participating in the thread (author, assignee, comment authors)
+    participants = 
+      if target.respond_to?(:participants)
+        target.participants
+      elsif target.is_a?(Commit)
+        author_ids = Note.for_commit_id(target.id).pluck(:author_id).uniq
+        User.where(id: author_ids)
+      else
+        note.mentioned_users
+      end
+    recipients = recipients.concat(participants)
 
     # Merge project watchers
     recipients = recipients.concat(project_watchers(note.project)).compact.uniq
 
-    # Reject users with Mention notification level
-    recipients = reject_mention_users(recipients, note.project)
-
-    # Add explicitly mentioned users
-    if target.respond_to?(:participants)
-      recipients = recipients.concat(target.participants)
-    else
-      recipients = recipients.concat(note.mentioned_users)
-    end
+    # Reject users with Mention notification level, except those mentioned in _this_ note.
+    recipients = reject_mention_users(recipients - note.mentioned_users, note.project)
+    recipients = recipients + note.mentioned_users
 
     # Reject mutes users
     recipients = reject_muted_users(recipients, note.project)
@@ -216,18 +219,6 @@ class NotificationService
 
   protected
 
-  def noteable_commenters(note)
-    opts = { noteable_type: note.noteable_type, project_id: note.project_id }
-
-    if note.commit_id.present?
-      opts.merge!(commit_id: note.commit_id)
-    else
-      opts.merge!(noteable_id: note.noteable_id)
-    end
-
-    User.where(id: Note.where(opts).pluck(:author_id))
-  end
-
   # Get project users with WATCH notification level
   def project_watchers(project)
     project_members = project_member_notification(project)

app/views/projects/diffs/_text_file.html.haml

@@ -23,7 +23,7 @@
         %td.line_content{class: "noteable_line #{type} #{line_code}", "line_code" => line_code}= raw diff_line_content(line.text)
 
     - if @reply_allowed
-      - comments = @line_notes.select { |n| n.line_code == line_code }.sort_by(&:created_at)
+      - comments = @line_notes.select { |n| n.line_code == line_code && n.active? }.sort_by(&:created_at)
       - unless comments.empty?
         = render "projects/notes/diff_notes_with_reply", notes: comments, line: line.text
 

app/views/projects/notes/_discussion.html.haml

@@ -6,9 +6,8 @@
         = image_tag avatar_icon(note.author_email), class: "avatar s40"
     .timeline-content
       - if note.for_merge_request?
-        - if note.outdated?
-          = render "projects/notes/discussions/outdated", discussion_notes: discussion_notes
-        - else
-          = render "projects/notes/discussions/active", discussion_notes: discussion_notes
+        - (active_notes, outdated_notes) = discussion_notes.partition(&:active?)
+        = render "projects/notes/discussions/active", discussion_notes: active_notes if active_notes.length > 0
+        = render "projects/notes/discussions/outdated", discussion_notes: outdated_notes if outdated_notes.length > 0
       - else
         = render "projects/notes/discussions/commit", discussion_notes: discussion_notes

lib/gitlab/gitorious_import/client.rb

@@ -27,37 +27,5 @@ module Gitlab
         repo_list.to_s.split(',').map(&:strip).reject(&:blank?)
       end
     end
-
-    Repository = Struct.new(:full_name) do
-      def id
-        Digest::SHA1.hexdigest(full_name)
-      end
-
-      def namespace
-        segments.first
-      end
-
-      def path
-        segments.last
-      end
-
-      def name
-        path.titleize
-      end
-
-      def description
-        ""
-      end
-
-      def import_url
-        "#{GITORIOUS_HOST}/#{full_name}.git"
-      end
-
-      private
-
-      def segments
-        full_name.split('/')
-      end
-    end
   end
 end

lib/gitlab/gitorious_import/repository.rb

+module Gitlab
+  module GitoriousImport
+    GITORIOUS_HOST = "https://gitorious.org"
+    
+    Repository = Struct.new(:full_name) do
+      def id
+        Digest::SHA1.hexdigest(full_name)
+      end
+
+      def namespace
+        segments.first
+      end
+
+      def path
+        segments.last
+      end
+
+      def name
+        path.titleize
+      end
+
+      def description
+        ""
+      end
+
+      def import_url
+        "#{GITORIOUS_HOST}/#{full_name}.git"
+      end
+
+      private
+
+      def segments
+        full_name.split('/')
+      end
+    end
+  end
+end

spec/lib/gitlab/bitbucket_import/project_creator_spec.rb

@@ -8,8 +8,12 @@ describe Gitlab::BitbucketImport::ProjectCreator do
     is_private: true,
     owner: "asd"}.with_indifferent_access
   }
-  let(:namespace){ create(:namespace) }
+  let(:namespace){ create(:group, owner: user) }
 
+  before do
+    namespace.add_owner(user)
+  end
+  
   it 'creates project' do
     allow_any_instance_of(Project).to receive(:add_import_job)
     

spec/lib/gitlab/github_import/project_creator_spec.rb

@@ -10,7 +10,11 @@ describe Gitlab::GithubImport::ProjectCreator do
     clone_url: "https://gitlab.com/asd/vim.git",
     owner: OpenStruct.new(login: "john"))
   }
-  let(:namespace){ create(:namespace) }
+  let(:namespace){ create(:group, owner: user) }
+
+  before do
+    namespace.add_owner(user)
+  end
 
   it 'creates project' do
     allow_any_instance_of(Project).to receive(:add_import_job)

spec/lib/gitlab/gitlab_import/project_creator_spec.rb

@@ -10,7 +10,11 @@ describe Gitlab::GitlabImport::ProjectCreator do
     http_url_to_repo: "https://gitlab.com/asd/vim.git",
     owner: {name: "john"}}.with_indifferent_access
   }
-  let(:namespace){ create(:namespace) }
+  let(:namespace){ create(:group, owner: user) }
+
+  before do
+    namespace.add_owner(user)
+  end
 
   it 'creates project' do
     allow_any_instance_of(Project).to receive(:add_import_job)

spec/lib/gitlab/gitorious_import/project_creator_spec.rb

@@ -3,7 +3,11 @@ require 'spec_helper'
 describe Gitlab::GitoriousImport::ProjectCreator do
   let(:user) { create(:user) }
   let(:repo) { Gitlab::GitoriousImport::Repository.new('foo/bar-baz-qux') }
-  let(:namespace){ create(:namespace) }
+  let(:namespace){ create(:group, owner: user) }
+
+  before do
+    namespace.add_owner(user)
+  end
 
   it 'creates project' do
     allow_any_instance_of(Project).to receive(:add_import_job)

spec/lib/gitlab/google_code_import/project_creator_spec.rb

@@ -9,7 +9,11 @@ describe Gitlab::GoogleCodeImport::ProjectCreator do
       "repositoryUrls"  => [ "https://vim.googlecode.com/git/" ]
     )
   }
-  let(:namespace) { create(:namespace) }
+  let(:namespace){ create(:group, owner: user) }
+
+  before do
+    namespace.add_owner(user)
+  end
 
   it 'creates project' do
     allow_any_instance_of(Project).to receive(:add_import_job)