Add unit specs for `Note#active?`
This commit is contained in:
parent
491d0ae1c3
commit
a63eba9a2b
|
@ -172,26 +172,29 @@ class Note < ActiveRecord::Base
|
||||||
Note.where(noteable_id: noteable_id, noteable_type: noteable_type, line_code: line_code).last.try(:diff)
|
Note.where(noteable_id: noteable_id, noteable_type: noteable_type, line_code: line_code).last.try(:diff)
|
||||||
end
|
end
|
||||||
|
|
||||||
# Check if such line of code exists in merge request diff
|
# Check if this note is part of an "active" discussion
|
||||||
# If exists - its active discussion
|
#
|
||||||
# If not - its outdated diff
|
# This will always return true for anything except MergeRequest noteables,
|
||||||
|
# which have special logic.
|
||||||
|
#
|
||||||
|
# If the note's current diff cannot be matched in the MergeRequest's current
|
||||||
|
# diff, it's considered inactive.
|
||||||
def active?
|
def active?
|
||||||
return true unless self.diff
|
return true unless self.diff
|
||||||
return false unless noteable
|
return false unless noteable
|
||||||
return @active if defined?(@active)
|
return @active if defined?(@active)
|
||||||
|
|
||||||
diffs = noteable.diffs(Commit.max_diff_options)
|
noteable_diff = find_noteable_diff
|
||||||
notable_diff = diffs.find { |d| d.new_path == self.diff.new_path }
|
|
||||||
|
|
||||||
return @active = false if notable_diff.nil?
|
if noteable_diff
|
||||||
|
parsed_lines = Gitlab::Diff::Parser.new.parse(noteable_diff.diff.each_line)
|
||||||
|
|
||||||
parsed_lines = Gitlab::Diff::Parser.new.parse(notable_diff.diff.each_line)
|
@active = parsed_lines.any? { |line_obj| line_obj.text == diff_line }
|
||||||
# We cannot use ||= because @active may be false
|
else
|
||||||
@active = parsed_lines.any? { |line_obj| line_obj.text == diff_line }
|
@active = false
|
||||||
end
|
end
|
||||||
|
|
||||||
def outdated?
|
@active
|
||||||
!active?
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def diff_file_index
|
def diff_file_index
|
||||||
|
@ -375,6 +378,12 @@ class Note < ActiveRecord::Base
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
|
# Find the diff on noteable that matches our own
|
||||||
|
def find_noteable_diff
|
||||||
|
diffs = noteable.diffs(Commit.max_diff_options)
|
||||||
|
diffs.find { |d| d.new_path == self.diff.new_path }
|
||||||
|
end
|
||||||
|
|
||||||
def awards_supported?
|
def awards_supported?
|
||||||
(for_issue? || for_merge_request?) && !for_diff_line?
|
(for_issue? || for_merge_request?) && !for_diff_line?
|
||||||
end
|
end
|
||||||
|
|
|
@ -152,7 +152,7 @@ describe Note, models: true do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe :grouped_awards do
|
describe '.grouped_awards' do
|
||||||
before do
|
before do
|
||||||
create :note, note: "smile", is_award: true
|
create :note, note: "smile", is_award: true
|
||||||
create :note, note: "smile", is_award: true
|
create :note, note: "smile", is_award: true
|
||||||
|
@ -169,6 +169,66 @@ describe Note, models: true do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe '#active?' do
|
||||||
|
it 'is always true when the note has no associated diff' do
|
||||||
|
note = build(:note)
|
||||||
|
|
||||||
|
expect(note).to receive(:diff).and_return(nil)
|
||||||
|
|
||||||
|
expect(note).to be_active
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'is never true when the note has no noteable associated' do
|
||||||
|
note = build(:note)
|
||||||
|
|
||||||
|
expect(note).to receive(:diff).and_return(double)
|
||||||
|
expect(note).to receive(:noteable).and_return(nil)
|
||||||
|
|
||||||
|
expect(note).not_to be_active
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'returns the memoized value if defined' do
|
||||||
|
note = build(:note)
|
||||||
|
|
||||||
|
expect(note).to receive(:diff).and_return(double)
|
||||||
|
expect(note).to receive(:noteable).and_return(double)
|
||||||
|
|
||||||
|
note.instance_variable_set(:@active, 'foo')
|
||||||
|
expect(note).not_to receive(:find_noteable_diff)
|
||||||
|
|
||||||
|
expect(note.active?).to eq 'foo'
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'for a merge request noteable' do
|
||||||
|
it 'is false when noteable has no matching diff' do
|
||||||
|
merge = build_stubbed(:merge_request, :simple)
|
||||||
|
note = build(:note, noteable: merge)
|
||||||
|
|
||||||
|
allow(note).to receive(:diff).and_return(double)
|
||||||
|
expect(note).to receive(:find_noteable_diff).and_return(nil)
|
||||||
|
|
||||||
|
expect(note).not_to be_active
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'is true when noteable has a matching diff' do
|
||||||
|
merge = create(:merge_request, :simple)
|
||||||
|
|
||||||
|
# Generate a real line_code value so we know it will match. We use a
|
||||||
|
# random line from a random diff just for funsies.
|
||||||
|
diff = merge.diffs.to_a.sample
|
||||||
|
line = Gitlab::Diff::Parser.new.parse(diff.diff.each_line).to_a.sample
|
||||||
|
code = Gitlab::Diff::LineCode.generate(diff.new_path, line.new_pos, line.old_pos)
|
||||||
|
|
||||||
|
# We're persisting in order to trigger the set_diff callback
|
||||||
|
note = create(:note, noteable: merge, line_code: code)
|
||||||
|
|
||||||
|
# Make sure we don't get a false positive from a guard clause
|
||||||
|
expect(note).to receive(:find_noteable_diff).and_call_original
|
||||||
|
expect(note).to be_active
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
describe "editable?" do
|
describe "editable?" do
|
||||||
it "returns true" do
|
it "returns true" do
|
||||||
note = build(:note)
|
note = build(:note)
|
||||||
|
|
Loading…
Reference in New Issue