Unverified Commit 9c30b0e9 authored by Douwe Maan's avatar Douwe Maan Committed by Luke "Jared" Bennett

Fix some specs

parent 336016fa
...@@ -192,7 +192,7 @@ require('./task_list'); ...@@ -192,7 +192,7 @@ require('./task_list');
}; };
Notes.prototype.refresh = function() { Notes.prototype.refresh = function() {
if (!document.hidden && document.URL.indexOf(this.noteable_url) === 0) { if (!document.hidden) {
return this.getContent(); return this.getContent();
} }
}; };
...@@ -371,7 +371,7 @@ require('./task_list'); ...@@ -371,7 +371,7 @@ require('./task_list');
discussionContainer.append(note_html); discussionContainer.append(note_html);
} }
if (typeof gl.diffNotesCompileComponents !== 'undefined' && note.discussion_id) { if (typeof gl.diffNotesCompileComponents !== 'undefined' && note.discussion_resolvable) {
gl.diffNotesCompileComponents(); gl.diffNotesCompileComponents();
this.renderDiscussionAvatar(diffAvatarContainer, note); this.renderDiscussionAvatar(diffAvatarContainer, note);
} }
......
...@@ -169,6 +169,8 @@ class Projects::NotesController < Projects::ApplicationController ...@@ -169,6 +169,8 @@ class Projects::NotesController < Projects::ApplicationController
discussion = note.to_discussion(noteable) discussion = note.to_discussion(noteable)
unless discussion.render_as_individual_notes? unless discussion.render_as_individual_notes?
attrs.merge!( attrs.merge!(
discussion_resolvable: discussion.resolvable?,
diff_discussion_html: diff_discussion_html(discussion), diff_discussion_html: diff_discussion_html(discussion),
discussion_html: discussion_html(discussion), discussion_html: discussion_html(discussion),
......
...@@ -54,17 +54,15 @@ module NotesHelper ...@@ -54,17 +54,15 @@ module NotesHelper
if use_legacy_diff_note if use_legacy_diff_note
new_note = LegacyDiffNote.new(@new_diff_note_attrs.merge(line_code: line_code)) new_note = LegacyDiffNote.new(@new_diff_note_attrs.merge(line_code: line_code))
discussion_id = new_note.discussion_id
else else
new_note = DiffNote.new(@new_diff_note_attrs.merge(position: position)) new_note = DiffNote.new(@new_diff_note_attrs.merge(position: position))
discussion_id = new_note.discussion_id
data[:position] = position.to_json data[:position] = position.to_json
end end
data.merge( data.merge(
note_type: new_note.type, note_type: new_note.type,
discussion_id: discussion_id discussion_id: new_note.discussion_class.discussion_id(new_note)
) )
end end
......
- if @discussions.present? - if defined?(@discussions)
- @discussions.each do |discussion| - @discussions.each do |discussion|
- if discussion.render_as_individual_notes? - if discussion.render_as_individual_notes?
= render partial: "projects/notes/note", collection: discussion.notes, as: :note = render partial: "projects/notes/note", collection: discussion.notes, as: :note
......
...@@ -347,6 +347,9 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps ...@@ -347,6 +347,9 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
end end
step 'I should see a discussion by user "John Doe" has started on diff' do step 'I should see a discussion by user "John Doe" has started on diff' do
# Trigger a refresh of notes
execute_script("$(document).trigger('visibilitychange');")
wait_for_ajax
page.within(".notes .discussion") do page.within(".notes .discussion") do
page.should have_content "#{user_exists("John Doe").name} #{user_exists("John Doe").to_reference} started a discussion" page.should have_content "#{user_exists("John Doe").name} #{user_exists("John Doe").to_reference} started a discussion"
page.should have_content sample_commit.line_code_path page.should have_content sample_commit.line_code_path
......
...@@ -58,7 +58,8 @@ describe Projects::NotesController do ...@@ -58,7 +58,8 @@ describe Projects::NotesController do
noteable_id: merge_request.id.to_s, noteable_id: merge_request.id.to_s,
noteable_type: 'MergeRequest', noteable_type: 'MergeRequest',
merge_request_diff_head_sha: 'sha', merge_request_diff_head_sha: 'sha',
in_reply_to_discussion_id: nil in_reply_to_discussion_id: nil,
new_discussion: nil
} }
expect(Notes::CreateService).to receive(:new).with(project, user, service_params).and_return(double(execute: true)) expect(Notes::CreateService).to receive(:new).with(project, user, service_params).and_return(double(execute: true))
......
...@@ -18,7 +18,7 @@ FactoryGirl.define do ...@@ -18,7 +18,7 @@ FactoryGirl.define do
factory :discussion_note_on_merge_request, traits: [:on_merge_request], class: DiscussionNote do factory :discussion_note_on_merge_request, traits: [:on_merge_request], class: DiscussionNote do
association :project, :repository association :project, :repository
trait :resolved do trait :resolved do
resolved_at { Time.now } resolved_at { Time.now }
resolved_by { create(:user) } resolved_by { create(:user) }
...@@ -117,5 +117,18 @@ FactoryGirl.define do ...@@ -117,5 +117,18 @@ FactoryGirl.define do
trait :with_svg_attachment do trait :with_svg_attachment do
attachment { fixture_file_upload(Rails.root + "spec/fixtures/unsanitized.svg", "image/svg+xml") } attachment { fixture_file_upload(Rails.root + "spec/fixtures/unsanitized.svg", "image/svg+xml") }
end end
transient do
in_reply_to nil
end
before(:create) do |note, evaluator|
discussion = evaluator.in_reply_to
next unless discussion
discussion = discussion.to_discussion if discussion.is_a?(Note)
next unless discussion
note.assign_attributes(discussion.reply_attributes)
end
end end
end end
require 'spec_helper' require 'spec_helper'
describe DiffDiscussion, model: true do describe DiffDiscussion, model: true do
# TODO: Test subject { described_class.new([first_note, second_note, third_note]) }
let(:first_note) { create(:diff_note_on_merge_request) }
let(:second_note) { create(:diff_note_on_merge_request) }
let(:third_note) { create(:diff_note_on_merge_request) }
# TODO: Test
describe "#truncated_diff_lines" do describe "#truncated_diff_lines" do
let(:truncated_lines) { subject.truncated_diff_lines } let(:truncated_lines) { subject.truncated_diff_lines }
......
...@@ -1234,15 +1234,7 @@ describe MergeRequest, models: true do ...@@ -1234,15 +1234,7 @@ describe MergeRequest, models: true do
end end
describe '#resolvable_discussions' do describe '#resolvable_discussions' do
before do # TODO: Test
allow(first_discussion).to receive(:to_be_resolved?).and_return(true)
allow(second_discussion).to receive(:to_be_resolved?).and_return(false)
allow(third_discussion).to receive(:to_be_resolved?).and_return(false)
end
it 'includes only discussions that need to be resolved' do
expect(subject.resolvable_discussions).to eq([first_discussion])
end
end end
describe "#discussions_resolvable?" do describe "#discussions_resolvable?" do
...@@ -1372,7 +1364,15 @@ describe MergeRequest, models: true do ...@@ -1372,7 +1364,15 @@ describe MergeRequest, models: true do
end end
describe "#discussions_to_be_resolved" do describe "#discussions_to_be_resolved" do
# TODO: Test before do
allow(first_discussion).to receive(:to_be_resolved?).and_return(true)
allow(second_discussion).to receive(:to_be_resolved?).and_return(false)
allow(third_discussion).to receive(:to_be_resolved?).and_return(false)
end
it 'includes only discussions that need to be resolved' do
expect(subject.discussions_to_be_resolved).to eq([first_discussion])
end
end end
describe '#discussions_can_be_resolved_by?' do describe '#discussions_can_be_resolved_by?' do
......
...@@ -414,7 +414,7 @@ describe Note, models: true do ...@@ -414,7 +414,7 @@ describe Note, models: true do
describe '#to_discussion' do describe '#to_discussion' do
subject { create(:discussion_note_on_merge_request) } subject { create(:discussion_note_on_merge_request) }
let!(:note2) { create(:discussion_note_on_merge_request, project: subject.project, noteable: subject.noteable, in_reply_to_discussion_id: subject.discussion_id) } let!(:note2) { create(:discussion_note_on_merge_request, project: subject.project, noteable: subject.noteable, in_reply_to: subject) }
it "returns a discussion with just this note" do it "returns a discussion with just this note" do
discussion = subject.to_discussion discussion = subject.to_discussion
...@@ -429,7 +429,7 @@ describe Note, models: true do ...@@ -429,7 +429,7 @@ describe Note, models: true do
let!(:note2) { create(:diff_note_on_merge_request, project: note1.project, noteable: note1.noteable) } let!(:note2) { create(:diff_note_on_merge_request, project: note1.project, noteable: note1.noteable) }
context 'when the note is part of a discussion' do context 'when the note is part of a discussion' do
subject { create(:discussion_note_on_merge_request, project: note1.project, noteable: note1.noteable, in_reply_to_discussion_id: note1.discussion_id) } subject { create(:discussion_note_on_merge_request, project: note1.project, noteable: note1.noteable, in_reply_to: note1) }
it "returns the discussion this note is in" do it "returns the discussion this note is in" do
discussion = subject.discussion discussion = subject.discussion
......
...@@ -953,7 +953,7 @@ describe API::Issues, api: true do ...@@ -953,7 +953,7 @@ describe API::Issues, api: true do
end end
context 'resolving discussions' do context 'resolving discussions' do
let(:discussion) { Discussion.for_diff_notes([create(:diff_note_on_merge_request)]).first } let(:discussion) { create(:diff_note_on_merge_request).to_discussion }
let(:merge_request) { discussion.noteable } let(:merge_request) { discussion.noteable }
let(:project) { merge_request.source_project } let(:project) { merge_request.source_project }
......
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