From 355a2df560dea2719a027ada718a923ab7876ffe Mon Sep 17 00:00:00 2001 From: Patrick Derichs Date: Fri, 26 Jul 2019 09:10:27 +0200 Subject: Use NotesFinder in module IssuableActions Add spec for concern IssuableActions Add shared samples for discussions endpoint Add schema validations for discussions Fix rubocop style issue Make target assignable Use new possibility to provide target --- spec/controllers/concerns/issuable_actions_spec.rb | 69 ++++++++++++++++++++++ .../controllers/projects/issues_controller_spec.rb | 22 +++++++ .../projects/merge_requests_controller_spec.rb | 16 +++++ spec/finders/notes_finder_spec.rb | 31 +++++++--- spec/fixtures/api/schemas/entities/discussion.json | 67 +++++++++++++++++++++ .../fixtures/api/schemas/entities/discussions.json | 4 ++ .../discussions_provider_shared_examples.rb | 15 +++++ 7 files changed, 217 insertions(+), 7 deletions(-) create mode 100644 spec/controllers/concerns/issuable_actions_spec.rb create mode 100644 spec/fixtures/api/schemas/entities/discussion.json create mode 100644 spec/fixtures/api/schemas/entities/discussions.json create mode 100644 spec/support/shared_examples/discussions_provider_shared_examples.rb (limited to 'spec') diff --git a/spec/controllers/concerns/issuable_actions_spec.rb b/spec/controllers/concerns/issuable_actions_spec.rb new file mode 100644 index 00000000000..3281fd4a2ef --- /dev/null +++ b/spec/controllers/concerns/issuable_actions_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe IssuableActions do + let(:project) { double('project') } + let(:user) { double('user') } + let(:issuable) { double('issuable') } + let(:finder_params_for_issuable) { {} } + let(:notes_result) { double('notes_result') } + let(:discussion_serializer) { double('discussion_serializer') } + + let(:controller) do + klass = Class.new do + attr_reader :current_user, :project, :issuable + + def self.before_action(action, params = nil) + end + + include IssuableActions + + def initialize(issuable, project, user, finder_params) + @issuable = issuable + @project = project + @current_user = user + @finder_params = finder_params + end + + def finder_params_for_issuable + @finder_params + end + + def params + { + notes_filter: 1 + } + end + + def prepare_notes_for_rendering(notes) + [] + end + + def render(options) + end + end + + klass.new(issuable, project, user, finder_params_for_issuable) + end + + describe '#discussions' do + before do + allow(user).to receive(:set_notes_filter) + allow(user).to receive(:user_preference) + allow(discussion_serializer).to receive(:represent) + end + + it 'instantiates and calls NotesFinder as expected' do + expect(Discussion).to receive(:build_collection).and_return([]) + expect(DiscussionSerializer).to receive(:new).and_return(discussion_serializer) + expect(NotesFinder).to receive(:new).with(project, user, finder_params_for_issuable).and_call_original + + expect_any_instance_of(NotesFinder).to receive(:execute).and_return(notes_result) + + expect(notes_result).to receive_messages(inc_relations_for_view: notes_result, includes: notes_result, fresh: notes_result) + + controller.discussions + end + end +end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 32d14dce936..0f885d776e1 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -1260,6 +1260,28 @@ describe Projects::IssuesController do sign_in(user) end + context do + it_behaves_like 'discussions provider' do + let!(:author) { create(:user) } + let!(:project) { create(:project) } + + let!(:issue) { create(:issue, project: project, author: user) } + + let!(:note_on_issue1) { create(:discussion_note_on_issue, noteable: issue, project: issue.project, author: create(:user)) } + let!(:note_on_issue2) { create(:discussion_note_on_issue, noteable: issue, project: issue.project, author: create(:user)) } + + let(:requested_iid) { issue.iid } + let(:expected_discussion_count) { 3 } + let(:expected_discussion_ids) do + [ + issue.notes.first.discussion_id, + note_on_issue1.discussion_id, + note_on_issue2.discussion_id + ] + end + end + end + it 'returns discussion json' do get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index f11880122b1..b777fa7f816 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -1121,6 +1121,22 @@ describe Projects::MergeRequestsController do end end end + + context do + it_behaves_like 'discussions provider' do + let!(:author) { create(:user) } + let!(:project) { create(:project) } + + let!(:merge_request) { create(:merge_request, source_project: project) } + + let!(:mr_note1) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) } + let!(:mr_note2) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) } + + let(:requested_iid) { merge_request.iid } + let(:expected_discussion_count) { 2 } + let(:expected_discussion_ids) { [mr_note1.discussion_id, mr_note2.discussion_id] } + end + end end describe 'GET edit' do diff --git a/spec/finders/notes_finder_spec.rb b/spec/finders/notes_finder_spec.rb index 87bde4ca2f6..677fde0d8ce 100644 --- a/spec/finders/notes_finder_spec.rb +++ b/spec/finders/notes_finder_spec.rb @@ -118,16 +118,11 @@ describe NotesFinder do context 'for target' do let(:project) { create(:project, :repository) } - let(:note1) { create :note_on_commit, project: project } - let(:note2) { create :note_on_commit, project: project } + let!(:note1) { create :note_on_commit, project: project } + let!(:note2) { create :note_on_commit, project: project } let(:commit) { note1.noteable } let(:params) { { target_id: commit.id, target_type: 'commit', last_fetched_at: 1.hour.ago.to_i } } - before do - note1 - note2 - end - it 'finds all notes' do notes = described_class.new(project, user, params).execute expect(notes.size).to eq(2) @@ -194,6 +189,28 @@ describe NotesFinder do end end end + + context 'for explicit target' do + let(:project) { create(:project, :repository) } + let!(:note1) { create :note_on_commit, project: project, created_at: 1.day.ago, updated_at: 2.hours.ago } + let!(:note2) { create :note_on_commit, project: project } + let(:commit) { note1.noteable } + let(:params) { { target: commit } } + + it 'returns the expected notes' do + expect(described_class.new(project, user, params).execute).to eq([note1, note2]) + end + + it 'returns the expected notes when last_fetched_at is given' do + params = { target: commit, last_fetched_at: 1.hour.ago.to_i } + expect(described_class.new(project, user, params).execute).to eq([note2]) + end + + it 'fails when nil is provided' do + params = { target: nil } + expect { described_class.new(project, user, params).execute }.to raise_error(RuntimeError) + end + end end describe '.search' do diff --git a/spec/fixtures/api/schemas/entities/discussion.json b/spec/fixtures/api/schemas/entities/discussion.json new file mode 100644 index 00000000000..bcc1db79e83 --- /dev/null +++ b/spec/fixtures/api/schemas/entities/discussion.json @@ -0,0 +1,67 @@ +{ + "type": "object", + "required" : [ + "id", + "notes", + "individual_note" + ], + "properties" : { + "id": { "type": "string" }, + "individual_note": { "type": "boolean" }, + "notes": { + "type": "array", + "items": { + "type": "object", + "properties" : { + "id": { "type": "string" }, + "type": { "type": ["string", "null"] }, + "body": { "type": "string" }, + "attachment": { "type": ["string", "null"]}, + "award_emoji": { "type": "array" }, + "author": { + "type": "object", + "properties": { + "name": { "type": "string" }, + "username": { "type": "string" }, + "id": { "type": "integer" }, + "state": { "type": "string" }, + "avatar_url": { "type": "uri" }, + "web_url": { "type": "uri" }, + "status_tooltip_html": { "type": ["string", "null"] }, + "path": { "type": "string" } + }, + "additionalProperties": false + }, + "created_at": { "type": "date" }, + "updated_at": { "type": "date" }, + "system": { "type": "boolean" }, + "noteable_id": { "type": "integer" }, + "noteable_iid": { "type": "integer" }, + "noteable_type": { "type": "string" }, + "resolved": { "type": "boolean" }, + "resolvable": { "type": "boolean" }, + "resolved_by": { "type": ["string", "null"] }, + "note": { "type": "string" }, + "note_html": { "type": "string" }, + "current_user": { "type": "object" }, + "suggestions": { "type": "array" }, + "discussion_id": { "type": "string" }, + "emoji_awardable": { "type": "boolean" }, + "report_abuse_path": { "type": "string" }, + "noteable_note_url": { "type": "string" }, + "resolve_path": { "type": "string" }, + "resolve_with_issue_path": { "type": "string" }, + "cached_markdown_version": { "type": "integer" }, + "human_access": { "type": ["string", "null"] }, + "toggle_award_path": { "type": "string" }, + "path": { "type": "string" } + }, + "required": [ + "id", "attachment", "author", "created_at", "updated_at", + "system", "noteable_id", "noteable_type" + ], + "additionalProperties": false + } + } + } +} diff --git a/spec/fixtures/api/schemas/entities/discussions.json b/spec/fixtures/api/schemas/entities/discussions.json new file mode 100644 index 00000000000..5a837429776 --- /dev/null +++ b/spec/fixtures/api/schemas/entities/discussions.json @@ -0,0 +1,4 @@ +{ + "type": "array", + "items": { "$ref": "discussion.json" } +} diff --git a/spec/support/shared_examples/discussions_provider_shared_examples.rb b/spec/support/shared_examples/discussions_provider_shared_examples.rb new file mode 100644 index 00000000000..77cf1ac3f51 --- /dev/null +++ b/spec/support/shared_examples/discussions_provider_shared_examples.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'spec_helper' + +shared_examples 'discussions provider' do + it 'returns the expected discussions' do + get :discussions, params: { namespace_id: project.namespace, project_id: project, id: requested_iid } + + expect(response).to have_gitlab_http_status(200) + expect(response).to match_response_schema('entities/discussions') + + expect(json_response.size).to eq(expected_discussion_count) + expect(json_response.pluck('id')).to eq(expected_discussion_ids) + end +end -- cgit v1.2.3