diff options
author | Douwe Maan <douwe@selenight.nl> | 2017-03-14 01:13:12 +0300 |
---|---|---|
committer | Douwe Maan <douwe@selenight.nl> | 2017-03-21 07:51:46 +0300 |
commit | 5c7647397a08ac9dd72822cb527c5d17f74a48dc (patch) | |
tree | 3b5dd9752d8ca17e15077d05de25907b49f576b0 | |
parent | df206b9ba72435fe050a78e98cb79301eefb8c3e (diff) |
Fix some specs
-rw-r--r-- | app/assets/javascripts/notes.js | 4 | ||||
-rw-r--r-- | app/controllers/projects/notes_controller.rb | 2 | ||||
-rw-r--r-- | app/helpers/notes_helper.rb | 4 | ||||
-rw-r--r-- | app/views/projects/notes/_note.html.haml | 2 | ||||
-rw-r--r-- | app/views/projects/notes/_notes.html.haml | 2 | ||||
-rw-r--r-- | features/steps/project/merge_requests.rb | 3 | ||||
-rw-r--r-- | features/steps/shared/diff_note.rb | 4 | ||||
-rw-r--r-- | features/steps/shared/markdown.rb | 2 | ||||
-rw-r--r-- | spec/controllers/projects/notes_controller_spec.rb | 3 | ||||
-rw-r--r-- | spec/factories/notes.rb | 15 | ||||
-rw-r--r-- | spec/models/diff_discussion_spec.rb | 8 | ||||
-rw-r--r-- | spec/models/merge_request_spec.rb | 20 | ||||
-rw-r--r-- | spec/models/note_spec.rb | 4 | ||||
-rw-r--r-- | spec/requests/api/issues_spec.rb | 2 |
14 files changed, 49 insertions, 26 deletions
diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index af53d7b0925..87bfc0085ce 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/_note.html.haml b/app/views/projects/notes/_note.html.haml index cc22a4ff3ba..097dd17c094 100644 --- a/app/views/projects/notes/_note.html.haml +++ b/app/views/projects/notes/_note.html.haml @@ -37,7 +37,7 @@ ":can-resolve" => can_resolve, ":author-name" => "'#{j(note.author.name)}'", "author-avatar" => note.author.avatar_url, - ":note-truncated" => "'#{truncate(note.note, length: 17)}'", + ":note-truncated" => "'#{j(truncate(note.note, length: 17))}'", ":resolved-by" => "'#{j(note.resolved_by.try(:name))}'", "v-show" => "#{can_resolve || note.resolved?}", "inline-template" => true, 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 9f0057cace7..d9912e84641 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/features/steps/shared/diff_note.rb b/features/steps/shared/diff_note.rb index 11fa85ed2fe..071aa2e3eff 100644 --- a/features/steps/shared/diff_note.rb +++ b/features/steps/shared/diff_note.rb @@ -196,7 +196,7 @@ module SharedDiffNote step 'The diff comment preview tab should display rendered Markdown' do page.within(diff_file_selector) do find('.js-md-preview-button').click - expect(find('.js-md-preview')).to have_css('img.emoji', visible: true) + expect(find('.js-md-preview')).to have_css('gl-emoji', visible: true) end end @@ -210,7 +210,7 @@ module SharedDiffNote step 'I should see a diff comment with an emoji image' do page.within("#{diff_file_selector} .note") do - expect(page).to have_xpath("//img[@alt=':smile:']") + expect(page).to have_xpath("//gl-emoji[@data-name='smile']") end end diff --git a/features/steps/shared/markdown.rb b/features/steps/shared/markdown.rb index a036d9b884f..875d27d9383 100644 --- a/features/steps/shared/markdown.rb +++ b/features/steps/shared/markdown.rb @@ -40,7 +40,7 @@ module SharedMarkdown step 'The Markdown preview tab should display rendered Markdown' do page.within('.gfm-form') do find('.js-md-preview-button').click - expect(find('.js-md-preview')).to have_css('img.emoji', visible: true) + expect(find('.js-md-preview')).to have_css('gl-emoji', visible: true) end end 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 e7738ca3034..6a3864cfc54 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -939,7 +939,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 } |