Commit 4f25e317 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'legacy-diff-notes-are-dumb' into 'master'

Don't fail when a LegacyDiffNote didn't store the right diff

Fixes https://sentry.gitlap.com/gitlab/gitlabcom/issues/8865/ and https://sentry.gitlap.com/gitlab/gitlabcom/issues/8754/

See merge request !5287
parents c676283b 34653c1e
...@@ -38,7 +38,7 @@ class LegacyDiffNote < Note ...@@ -38,7 +38,7 @@ class LegacyDiffNote < Note
end end
def diff_line def diff_line
@diff_line ||= diff_file.line_for_line_code(self.line_code) @diff_line ||= diff_file.line_for_line_code(self.line_code) if diff_file
end end
def for_line?(line) def for_line?(line)
...@@ -55,7 +55,7 @@ class LegacyDiffNote < Note ...@@ -55,7 +55,7 @@ class LegacyDiffNote < Note
def active? def active?
return @active if defined?(@active) return @active if defined?(@active)
return true if for_commit? return true if for_commit?
return true unless self.diff return true unless diff_line
return false unless noteable return false unless noteable
noteable_diff = find_noteable_diff noteable_diff = find_noteable_diff
......
- if @note.diff_note? - if @note.diff_note? && @note.diff_file
%p.details %p.details
New comment on diff for New comment on diff for
= link_to @note.diff_file.file_path, @target_url = link_to @note.diff_file.file_path, @target_url
......
...@@ -16,10 +16,10 @@ describe LegacyDiffNote, models: true do ...@@ -16,10 +16,10 @@ describe LegacyDiffNote, models: true do
end end
describe '#active?' do describe '#active?' do
it 'is always true when the note has no associated diff' do it 'is always true when the note has no associated diff line' do
note = build(:legacy_diff_note_on_merge_request) note = build(:legacy_diff_note_on_merge_request)
expect(note).to receive(:diff).and_return(nil) expect(note).to receive(:diff_line).and_return(nil)
expect(note).to be_active expect(note).to be_active
end end
...@@ -27,7 +27,7 @@ describe LegacyDiffNote, models: true do ...@@ -27,7 +27,7 @@ describe LegacyDiffNote, models: true do
it 'is never true when the note has no noteable associated' do it 'is never true when the note has no noteable associated' do
note = build(:legacy_diff_note_on_merge_request) note = build(:legacy_diff_note_on_merge_request)
expect(note).to receive(:diff).and_return(double) expect(note).to receive(:diff_line).and_return(double)
expect(note).to receive(:noteable).and_return(nil) expect(note).to receive(:noteable).and_return(nil)
expect(note).not_to be_active expect(note).not_to be_active
...@@ -47,7 +47,7 @@ describe LegacyDiffNote, models: true do ...@@ -47,7 +47,7 @@ describe LegacyDiffNote, models: true do
merge = build_stubbed(:merge_request, :simple) merge = build_stubbed(:merge_request, :simple)
note = build(:legacy_diff_note_on_merge_request, noteable: merge) note = build(:legacy_diff_note_on_merge_request, noteable: merge)
allow(note).to receive(:diff).and_return(double) allow(note).to receive(:diff_line).and_return(double)
expect(note).to receive(:find_noteable_diff).and_return(nil) expect(note).to receive(:find_noteable_diff).and_return(nil)
expect(note).not_to be_active expect(note).not_to be_active
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment