From f17f168f56c6e77391d22f900b9b9b2759388770 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 7 Mar 2016 19:04:00 -0500 Subject: Ensure a Note's st_diff attribute is properly set First, for reasons unknown, `diff_for_line_code` wasn't being called for MergeRequest noteables. So we removed that check. Second, when it _was_ being called, it didn't do a good enough job of returning a diff -- it could return a `Note` record that didn't even have a diff itself! So if a user posted a reply to an outdated diff discussion, the new note's `st_diff` would be `nil`, and the frontend wouldn't know how to display it ("It's got a line code but no diff? :boom:"). Now the reply gets a diff from its "parent" note (if one's available), and everyone's happy. --- app/models/note.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/models/note.rb b/app/models/note.rb index 3b20d5d22b6..68c6d20b120 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -70,6 +70,7 @@ class Note < ActiveRecord::Base scope :fresh, ->{ order(created_at: :asc, id: :asc) } scope :inc_author_project, ->{ includes(:project, :author) } scope :inc_author, ->{ includes(:author) } + scope :with_diff, -> { where('st_diff IS NOT NULL') } scope :with_associations, -> do includes(:author, :noteable, :updated_by, @@ -146,10 +147,7 @@ class Note < ActiveRecord::Base end def set_diff - # First lets find notes with same diff - # before iterating over all mr diffs - diff = diff_for_line_code unless for_merge_request? - diff ||= find_diff + diff = diff_for_line_code || find_diff self.st_diff = diff.to_hash if diff end @@ -159,7 +157,9 @@ class Note < ActiveRecord::Base end def diff_for_line_code - Note.where(noteable_id: noteable_id, noteable_type: noteable_type, line_code: line_code).last.try(:diff) + self.class. + where(noteable: noteable, line_code: line_code). + with_diff.last.try(:diff) end # Check if such line of code exists in merge request diff -- cgit v1.2.1