summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/assets/javascripts/notes.js4
-rw-r--r--app/controllers/projects/notes_controller.rb2
-rw-r--r--app/helpers/notes_helper.rb4
-rw-r--r--app/views/projects/notes/_notes.html.haml2
-rw-r--r--features/steps/project/merge_requests.rb3
-rw-r--r--spec/controllers/projects/notes_controller_spec.rb3
-rw-r--r--spec/factories/notes.rb15
-rw-r--r--spec/models/diff_discussion_spec.rb8
-rw-r--r--spec/models/merge_request_spec.rb20
-rw-r--r--spec/models/note_spec.rb4
-rw-r--r--spec/requests/api/issues_spec.rb2
11 files changed, 45 insertions, 22 deletions
diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js
index 71c03c89314..226029ba8f3 100644
--- a/app/assets/javascripts/notes.js
+++ b/app/assets/javascripts/notes.js
@@ -192,7 +192,7 @@ require('./task_list');
};
Notes.prototype.refresh = function() {
- if (!document.hidden && document.URL.indexOf(this.noteable_url) === 0) {
+ if (!document.hidden) {
return this.getContent();
}
};
@@ -371,7 +371,7 @@ require('./task_list');
discussionContainer.append(note_html);
}
- if (typeof gl.diffNotesCompileComponents !== 'undefined' && note.discussion_id) {
+ if (typeof gl.diffNotesCompileComponents !== 'undefined' && note.discussion_resolvable) {
gl.diffNotesCompileComponents();
this.renderDiscussionAvatar(diffAvatarContainer, note);
}
diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb
index f80313bbee0..186098c67b7 100644
--- a/app/controllers/projects/notes_controller.rb
+++ b/app/controllers/projects/notes_controller.rb
@@ -169,6 +169,8 @@ class Projects::NotesController < Projects::ApplicationController
discussion = note.to_discussion(noteable)
unless discussion.render_as_individual_notes?
attrs.merge!(
+ discussion_resolvable: discussion.resolvable?,
+
diff_discussion_html: diff_discussion_html(discussion),
discussion_html: discussion_html(discussion),
diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb
index 01ecac983cf..e2fa9905e86 100644
--- a/app/helpers/notes_helper.rb
+++ b/app/helpers/notes_helper.rb
@@ -54,17 +54,15 @@ module NotesHelper
if use_legacy_diff_note
new_note = LegacyDiffNote.new(@new_diff_note_attrs.merge(line_code: line_code))
- discussion_id = new_note.discussion_id
else
new_note = DiffNote.new(@new_diff_note_attrs.merge(position: position))
- discussion_id = new_note.discussion_id
data[:position] = position.to_json
end
data.merge(
note_type: new_note.type,
- discussion_id: discussion_id
+ discussion_id: new_note.discussion_class.discussion_id(new_note)
)
end
diff --git a/app/views/projects/notes/_notes.html.haml b/app/views/projects/notes/_notes.html.haml
index 50809068453..d619f7ac262 100644
--- a/app/views/projects/notes/_notes.html.haml
+++ b/app/views/projects/notes/_notes.html.haml
@@ -1,4 +1,4 @@
-- if @discussions.present?
+- if defined?(@discussions)
- @discussions.each do |discussion|
- if discussion.render_as_individual_notes?
= render partial: "projects/notes/note", collection: discussion.notes, as: :note
diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb
index 5510c65265a..a67b3222af3 100644
--- a/features/steps/project/merge_requests.rb
+++ b/features/steps/project/merge_requests.rb
@@ -347,6 +347,9 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
end
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.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
diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb
index 87084806568..a3b0e8a252e 100644
--- a/spec/controllers/projects/notes_controller_spec.rb
+++ b/spec/controllers/projects/notes_controller_spec.rb
@@ -58,7 +58,8 @@ describe Projects::NotesController do
noteable_id: merge_request.id.to_s,
noteable_type: 'MergeRequest',
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))
diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb
index 755c6d7e031..0ad7034e96d 100644
--- a/spec/factories/notes.rb
+++ b/spec/factories/notes.rb
@@ -18,7 +18,7 @@ FactoryGirl.define do
factory :discussion_note_on_merge_request, traits: [:on_merge_request], class: DiscussionNote do
association :project, :repository
-
+
trait :resolved do
resolved_at { Time.now }
resolved_by { create(:user) }
@@ -117,5 +117,18 @@ FactoryGirl.define do
trait :with_svg_attachment do
attachment { fixture_file_upload(Rails.root + "spec/fixtures/unsanitized.svg", "image/svg+xml") }
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
diff --git a/spec/models/diff_discussion_spec.rb b/spec/models/diff_discussion_spec.rb
index 8f3a4a8cc49..6109d77f27c 100644
--- a/spec/models/diff_discussion_spec.rb
+++ b/spec/models/diff_discussion_spec.rb
@@ -1,7 +1,13 @@
require 'spec_helper'
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
let(:truncated_lines) { subject.truncated_diff_lines }
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index faa89ff0507..3d26049b54a 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -1234,15 +1234,7 @@ describe MergeRequest, models: true do
end
describe '#resolvable_discussions' do
- 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.resolvable_discussions).to eq([first_discussion])
- end
+ # TODO: Test
end
describe "#discussions_resolvable?" do
@@ -1372,7 +1364,15 @@ describe MergeRequest, models: true do
end
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
describe '#discussions_can_be_resolved_by?' do
diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index c5e4a639f06..7a85c5f22ff 100644
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -414,7 +414,7 @@ describe Note, models: true do
describe '#to_discussion' do
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
discussion = subject.to_discussion
@@ -429,7 +429,7 @@ describe Note, models: true do
let!(:note2) { create(:diff_note_on_merge_request, project: note1.project, noteable: note1.noteable) }
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
discussion = subject.discussion
diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index 4729adba11c..504ff19a17f 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/spec/requests/api/issues_spec.rb
@@ -953,7 +953,7 @@ describe API::Issues, api: true do
end
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(:project) { merge_request.source_project }
View Lorry Controller Status