Merge branch '36329-discussion-start-of-discussion' into 'master'

Replace Discussion#new_discussion? with more reliable Note#start_of_discussion?

See merge request gitlab-org/gitlab!20306

app/models/diff_note.rb

@@ -88,10 +88,6 @@ class DiffNote < Note
     line&.suggestible?
   end
 
-  def discussion_first_note?
-    self == discussion.first_note
-  end
-
   def banzai_render_context(field)
     super.merge(suggestions_filter_enabled: true)
   end
@@ -108,7 +104,7 @@ class DiffNote < Note
   end
 
   def should_create_diff_file?
-    on_text? && note_diff_file.nil? && discussion_first_note?
+    on_text? && note_diff_file.nil? && start_of_discussion?
   end
 
   def fetch_diff_file

app/models/discussion.rb

@@ -139,10 +139,6 @@ class Discussion
     false
   end
 
-  def new_discussion?
-    notes.length == 1
-  end
-
   def last_note
     @last_note ||= notes.last
   end

app/models/note.rb

@@ -409,6 +409,10 @@ class Note < ApplicationRecord
     full_discussion || to_discussion
   end
 
+  def start_of_discussion?
+    discussion.first_note == self
+  end
+
   def part_of_discussion?
     !to_discussion.individual_note?
   end

app/services/notes/base_service.rb

@@ -4,7 +4,7 @@ module Notes
   class BaseService < ::BaseService
     def clear_noteable_diffs_cache(note)
       if note.is_a?(DiffNote) &&
-          note.discussion_first_note? &&
+          note.start_of_discussion? &&
           note.position.unfolded_diff?(project.repository)
         note.noteable.diffs.clear_cache
       end

app/views/notify/_note_email.html.haml

@@ -11,7 +11,7 @@
     - if discussion.nil?
       commented
     - else
-      - if discussion.new_discussion?
+      - if note.start_of_discussion?
         started a new
       - else
         commented on a

app/views/notify/_note_email.text.erb

@@ -7,7 +7,7 @@
 <%  if discussion.nil? -%>
  <%= 'commented' -%>:
 <%  else -%>
-<%    if discussion.new_discussion? -%>
+<%    if note.start_of_discussion? -%>
  <%=    'started a new discussion' -%>
 <%    else -%>
  <%=    'commented on a discussion' -%>

ee/app/services/ee/notes/post_process_service.rb

@@ -17,7 +17,7 @@ module EE
       private
 
       def create_design_discussion_system_note?
-        note && note.for_design? && note.discussion.new_discussion?
+        note && note.for_design? && note.start_of_discussion?
       end
     end
   end

spec/mailers/notify_spec.rb

@@ -990,7 +990,8 @@ describe Notify do
         end
 
         context 'when a comment on an existing discussion' do
-          let!(:second_note) { create(model, author: note_author, noteable: nil, in_reply_to: note) }
+          let(:first_note) { create_note }
+          let(:note) { create(model, author: note_author, noteable: nil, in_reply_to: first_note) }
 
           it 'contains an introduction' do
             is_expected.to have_body_text 'commented on a'
@@ -1000,7 +1001,11 @@ describe Notify do
 
       describe 'on a commit' do
         let(:commit) { project.commit }
-        let(:note) { create(:discussion_note_on_commit, commit_id: commit.id, project: project, author: note_author) }
+        let(:note) { create_note }
+
+        def create_note
+          create(:discussion_note_on_commit, commit_id: commit.id, project: project, author: note_author)
+        end
 
         before do
           allow(note).to receive(:noteable).and_return(commit)
@@ -1027,9 +1032,13 @@ describe Notify do
       end
 
       describe 'on a merge request' do
-        let(:note) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project, author: note_author) }
+        let(:note) { create_note }
         let(:note_on_merge_request_path) { project_merge_request_path(project, merge_request, anchor: "note_#{note.id}") }
 
+        def create_note
+          create(:discussion_note_on_merge_request, noteable: merge_request, project: project, author: note_author)
+        end
+
         before do
           allow(note).to receive(:noteable).and_return(merge_request)
         end
@@ -1055,9 +1064,13 @@ describe Notify do
       end
 
       describe 'on an issue' do
-        let(:note) { create(:discussion_note_on_issue, noteable: issue, project: project, author: note_author) }
+        let(:note) { create_note }
         let(:note_on_issue_path) { project_issue_path(project, issue, anchor: "note_#{note.id}") }
 
+        def create_note
+          create(:discussion_note_on_issue, noteable: issue, project: project, author: note_author)
+        end
+
         before do
           allow(note).to receive(:noteable).and_return(issue)
         end
@@ -1134,7 +1147,8 @@ describe Notify do
         end
 
         context 'when a comment on an existing discussion' do
-          let!(:second_note) { create(model, author: note_author, noteable: nil, in_reply_to: note) }
+          let(:first_note) { create(model) }
+          let(:note) { create(model, author: note_author, noteable: nil, in_reply_to: first_note) }
 
           it 'contains an introduction' do
             is_expected.to have_body_text 'commented on a discussion on'

spec/models/note_spec.rb

@@ -456,6 +456,19 @@ describe Note do
     end
   end
 
+  describe '#start_of_discussion?' do
+    let_it_be(:note) { create(:discussion_note_on_merge_request) }
+    let_it_be(:reply) { create(:discussion_note_on_merge_request, in_reply_to: note) }
+
+    it 'returns true when note is the start of a discussion' do
+      expect(note).to be_start_of_discussion
+    end
+
+    it 'returns false when note is a reply' do
+      expect(reply).not_to be_start_of_discussion
+    end
+  end
+
   describe '.find_discussion' do
     let!(:note) { create(:discussion_note_on_merge_request) }
     let!(:note2) { create(:discussion_note_on_merge_request, in_reply_to: note) }