Commit c80f5e0a authored by Douwe Maan's avatar Douwe Maan

Add specs for new Note and DiffNote methods.

parent 6073b281
...@@ -107,6 +107,9 @@ class DiffNote < Note ...@@ -107,6 +107,9 @@ class DiffNote < Note
return unless resolvable? return unless resolvable?
discussion_notes = self.noteable.notes.fresh.select { |n| n.discussion_id == self.discussion_id } discussion_notes = self.noteable.notes.fresh.select { |n| n.discussion_id == self.discussion_id }
return if discussion_notes.empty?
Discussion.new(discussion_notes) Discussion.new(discussion_notes)
end end
......
...@@ -188,4 +188,250 @@ describe DiffNote, models: true do ...@@ -188,4 +188,250 @@ describe DiffNote, models: true do
end end
end end
end end
describe "#resolvable?" do
context "when noteable is a commit" do
subject { create(:diff_note_on_commit, project: project, position: position) }
it "returns false" do
expect(subject.resolvable?).to be false
end
end
context "when noteable is a merge request" do
context "when a system note" do
before do
subject.system = true
end
it "returns false" do
expect(subject.resolvable?).to be false
end
end
context "when a regular note" do
it "returns true" do
expect(subject.resolvable?).to be true
end
end
end
end
describe "#to_be_resolved?" do
context "when not resolvable" do
before do
allow(subject).to receive(:resolvable?).and_return(false)
end
it "returns false" do
expect(subject.to_be_resolved?).to be false
end
end
context "when resolvable" do
before do
allow(subject).to receive(:resolvable?).and_return(true)
end
context "when resolved" do
before do
allow(subject).to receive(:resolved?).and_return(true)
end
it "returns false" do
expect(subject.to_be_resolved?).to be false
end
end
context "when not resolved" do
before do
allow(subject).to receive(:resolved?).and_return(false)
end
it "returns true" do
expect(subject.to_be_resolved?).to be true
end
end
end
end
describe "#resolve!" do
let(:current_user) { create(:user) }
context "when not resolvable" do
before do
allow(subject).to receive(:resolvable?).and_return(false)
end
it "returns nil" do
expect(subject.resolve!(current_user)).to be_nil
end
it "doesn't set resolved_at" do
subject.resolve!(current_user)
expect(subject.resolved_at).to be_nil
end
it "doesn't set resolved_by" do
subject.resolve!(current_user)
expect(subject.resolved_by).to be_nil
end
it "doesn't mark as resolved" do
subject.resolve!(current_user)
expect(subject.resolved?).to be false
end
end
context "when resolvable" do
before do
allow(subject).to receive(:resolvable?).and_return(true)
end
context "when already resolved" do
let(:user) { create(:user) }
before do
subject.resolve!(user)
end
it "returns nil" do
expect(subject.resolve!(current_user)).to be_nil
end
it "doesn't change resolved_at" do
expect(subject.resolved_at).not_to be_nil
expect { subject.resolve!(current_user) }.not_to change { subject.resolved_at }
end
it "doesn't change resolved_by" do
expect(subject.resolved_by).to eq(user)
expect { subject.resolve!(current_user) }.not_to change { subject.resolved_by }
end
it "doesn't change resolved status" do
expect(subject.resolved?).to be true
expect { subject.resolve!(current_user) }.not_to change { subject.resolved? }
end
end
context "when not yet resolved" do
it "returns true" do
expect(subject.resolve!(current_user)).to be true
end
it "sets resolved_at" do
subject.resolve!(current_user)
expect(subject.resolved_at).not_to be_nil
end
it "sets resolved_by" do
subject.resolve!(current_user)
expect(subject.resolved_by).to eq(current_user)
end
it "marks as resolved" do
subject.resolve!(current_user)
expect(subject.resolved?).to be true
end
end
end
end
describe "#unresolve!" do
context "when not resolvable" do
before do
allow(subject).to receive(:resolvable?).and_return(false)
end
it "returns nil" do
expect(subject.unresolve!).to be_nil
end
end
context "when resolvable" do
before do
allow(subject).to receive(:resolvable?).and_return(true)
end
context "when resolved" do
let(:user) { create(:user) }
before do
subject.resolve!(user)
end
it "returns true" do
expect(subject.unresolve!).to be true
end
it "unsets resolved_at" do
subject.unresolve!
expect(subject.resolved_at).to be_nil
end
it "unsets resolved_by" do
subject.unresolve!
expect(subject.resolved_by).to be_nil
end
it "unmarks as resolved" do
subject.unresolve!
expect(subject.resolved?).to be false
end
end
context "when not resolved" do
it "returns nil" do
expect(subject.unresolve!).to be_nil
end
end
end
end
describe "#discussion" do
context "when not resolvable" do
before do
allow(subject).to receive(:resolvable?).and_return(false)
end
it "returns nil" do
expect(subject.discussion).to be_nil
end
end
context "when resolvable" do
let!(:diff_note2) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: subject.position) }
let!(:diff_note3) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: active_position2) }
let(:active_position2) do
Gitlab::Diff::Position.new(
old_path: "files/ruby/popen.rb",
new_path: "files/ruby/popen.rb",
old_line: 16,
new_line: 22,
diff_refs: merge_request.diff_refs
)
end
it "returns the discussion this note is in" do
discussion = subject.discussion
expect(discussion.id).to eq(subject.discussion_id)
expect(discussion.notes).to eq([subject, diff_note2])
end
end
end
end end
require 'spec_helper' require 'spec_helper'
describe Note, models: true do describe Note, models: true do
include RepoHelpers
describe 'associations' do describe 'associations' do
it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:project) }
it { is_expected.to belong_to(:noteable).touch(true) } it { is_expected.to belong_to(:noteable).touch(true) }
...@@ -267,4 +269,56 @@ describe Note, models: true do ...@@ -267,4 +269,56 @@ describe Note, models: true do
expect(note.participants).to include(note.author) expect(note.participants).to include(note.author)
end end
end end
describe ".grouped_diff_discussions" do
let!(:merge_request) { create(:merge_request) }
let(:project) { merge_request.project }
let!(:active_diff_note1) { create(:diff_note_on_merge_request, project: project, noteable: merge_request) }
let!(:active_diff_note2) { create(:diff_note_on_merge_request, project: project, noteable: merge_request) }
let!(:active_diff_note3) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: active_position2) }
let!(:outdated_diff_note1) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: outdated_position) }
let!(:outdated_diff_note2) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: outdated_position) }
let(:active_position2) do
Gitlab::Diff::Position.new(
old_path: "files/ruby/popen.rb",
new_path: "files/ruby/popen.rb",
old_line: 16,
new_line: 22,
diff_refs: merge_request.diff_refs
)
end
let(:outdated_position) do
Gitlab::Diff::Position.new(
old_path: "files/ruby/popen.rb",
new_path: "files/ruby/popen.rb",
old_line: nil,
new_line: 9,
diff_refs: project.commit("874797c3a73b60d2187ed6e2fcabd289ff75171e").diff_refs
)
end
subject { merge_request.notes.grouped_diff_discussions }
it "includes active discussions" do
discussions = subject.values
expect(discussions.count).to eq(2)
expect(discussions.map(&:id)).to eq([active_diff_note1.discussion_id, active_diff_note3.discussion_id])
expect(discussions.all?(&:active?)).to be true
expect(discussions.first.notes).to eq([active_diff_note1, active_diff_note2])
expect(discussions.last.notes).to eq([active_diff_note3])
end
it "doesn't include outdated discussions" do
expect(subject.values.map(&:id)).not_to include(outdated_diff_note1.discussion_id)
end
it "groups the discussions by line code" do
expect(subject[active_diff_note1.line_code].id).to eq(active_diff_note1.discussion_id)
expect(subject[active_diff_note3.line_code].id).to eq(active_diff_note3.discussion_id)
end
end
end end
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