Merge branch '52123-issuable-actions-notesfinder-pderichs' into 'master'

Use NotesFinder to fetch notes on API and Controllers

Closes #52123

See merge request gitlab-org/gitlab-ce!31300
This commit is contained in:
Stan Hu 2019-08-01 16:27:02 +00:00
commit dbc6c24403
15 changed files with 316 additions and 78 deletions

View file

@ -98,13 +98,12 @@ module IssuableActions
render json: { notice: "#{quantity} #{resource_name.pluralize(quantity)} updated" } render json: { notice: "#{quantity} #{resource_name.pluralize(quantity)} updated" }
end end
# rubocop: disable CodeReuse/ActiveRecord # rubocop:disable CodeReuse/ActiveRecord
def discussions def discussions
notes = issuable.discussion_notes notes = NotesFinder.new(current_user, finder_params_for_issuable).execute
.inc_relations_for_view .inc_relations_for_view
.with_notes_filter(notes_filter) .includes(:noteable)
.includes(:noteable) .fresh
.fresh
if notes_filter != UserPreference::NOTES_FILTERS[:only_comments] if notes_filter != UserPreference::NOTES_FILTERS[:only_comments]
notes = ResourceEvents::MergeIntoNotesService.new(issuable, current_user).execute(notes) notes = ResourceEvents::MergeIntoNotesService.new(issuable, current_user).execute(notes)
@ -117,7 +116,7 @@ module IssuableActions
render json: discussion_serializer.represent(discussions, context: self) render json: discussion_serializer.represent(discussions, context: self)
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop:enable CodeReuse/ActiveRecord
private private
@ -222,4 +221,13 @@ module IssuableActions
def parent def parent
@project || @group # rubocop:disable Gitlab/ModuleWithInstanceVariables @project || @group # rubocop:disable Gitlab/ModuleWithInstanceVariables
end end
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def finder_params_for_issuable
{
target: @issuable,
notes_filter: notes_filter
}.tap { |new_params| new_params[:project] = project if respond_to?(:project, true) }
end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
end end

View file

@ -243,7 +243,7 @@ module NotesActions
end end
def notes_finder def notes_finder
@notes_finder ||= NotesFinder.new(project, current_user, finder_params) @notes_finder ||= NotesFinder.new(current_user, finder_params)
end end
def note_serializer def note_serializer

View file

@ -68,7 +68,7 @@ class Projects::NotesController < Projects::ApplicationController
alias_method :awardable, :note alias_method :awardable, :note
def finder_params def finder_params
params.merge(last_fetched_at: last_fetched_at, notes_filter: notes_filter) params.merge(project: project, last_fetched_at: last_fetched_at, notes_filter: notes_filter)
end end
def authorize_admin_note! def authorize_admin_note!

View file

@ -27,7 +27,9 @@ class Snippets::NotesController < ApplicationController
alias_method :noteable, :snippet alias_method :noteable, :snippet
def finder_params def finder_params
params.merge(last_fetched_at: last_fetched_at, target_id: snippet.id, target_type: 'personal_snippet') params.merge(last_fetched_at: last_fetched_at, target_id: snippet.id, target_type: 'personal_snippet').tap do |merged_params|
merged_params[:project] = project if respond_to?(:project)
end
end end
def authorize_read_snippet! def authorize_read_snippet!

View file

@ -3,6 +3,8 @@
class NotesFinder class NotesFinder
FETCH_OVERLAP = 5.seconds FETCH_OVERLAP = 5.seconds
attr_reader :target_type
# Used to filter Notes # Used to filter Notes
# When used with target_type and target_id this returns notes specifically for the controller # When used with target_type and target_id this returns notes specifically for the controller
# #
@ -10,15 +12,17 @@ class NotesFinder
# current_user - which user check authorizations with # current_user - which user check authorizations with
# project - which project to look for notes on # project - which project to look for notes on
# params: # params:
# target: noteable
# target_type: string # target_type: string
# target_id: integer # target_id: integer
# last_fetched_at: time # last_fetched_at: time
# search: string # search: string
# #
def initialize(project, current_user, params = {}) def initialize(current_user, params = {})
@project = project @project = params[:project]
@current_user = current_user @current_user = current_user
@params = params @params = params.dup
@target_type = @params[:target_type]
end end
def execute def execute
@ -32,7 +36,27 @@ class NotesFinder
def target def target
return @target if defined?(@target) return @target if defined?(@target)
target_type = @params[:target_type] if target_given?
use_explicit_target
else
find_target_by_type_and_ids
end
end
private
def target_given?
@params.key?(:target)
end
def use_explicit_target
@target = @params[:target]
@target_type = @target.class.name.underscore
@target
end
def find_target_by_type_and_ids
target_id = @params[:target_id] target_id = @params[:target_id]
target_iid = @params[:target_iid] target_iid = @params[:target_iid]
@ -45,13 +69,11 @@ class NotesFinder
@project.commit(target_id) @project.commit(target_id)
end end
else else
noteables_for_type_by_id(target_type, target_id, target_iid) noteable_for_type_by_id(target_type, target_id, target_iid)
end end
end end
private def noteable_for_type_by_id(type, id, iid)
def noteables_for_type_by_id(type, id, iid)
query = if id query = if id
{ id: id } { id: id }
else else
@ -77,10 +99,6 @@ class NotesFinder
search(notes) search(notes)
end end
def target_type
@params[:target_type]
end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def notes_of_any_type def notes_of_any_type
types = %w(commit issue merge_request snippet) types = %w(commit issue merge_request snippet)

View file

@ -76,7 +76,7 @@ module API
def find_noteable(parent_type, parent_id, noteable_type, noteable_id) def find_noteable(parent_type, parent_id, noteable_type, noteable_id)
params = params_by_noteable_type_and_id(noteable_type, noteable_id) params = params_by_noteable_type_and_id(noteable_type, noteable_id)
noteable = NotesFinder.new(user_project, current_user, params).target noteable = NotesFinder.new(current_user, params.merge(project: user_project)).target
noteable = nil unless can?(current_user, noteable_read_ability_name(noteable), noteable) noteable = nil unless can?(current_user, noteable_read_ability_name(noteable), noteable)
noteable || not_found!(noteable_type) noteable || not_found!(noteable_type)
end end

View file

@ -108,7 +108,7 @@ module Gitlab
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def notes_finder(type) def notes_finder(type)
NotesFinder.new(project, @current_user, search: query, target_type: type).execute.user.order('updated_at DESC') NotesFinder.new(@current_user, search: query, target_type: type, project: project).execute.user.order('updated_at DESC')
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord

View file

@ -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(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

View file

@ -1260,6 +1260,28 @@ describe Projects::IssuesController do
sign_in(user) sign_in(user)
end 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 it 'returns discussion json' do
get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid }

View file

@ -1210,6 +1210,22 @@ describe Projects::MergeRequestsController do
end end
end 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 end
describe 'GET edit' do describe 'GET edit' do

View file

@ -43,7 +43,7 @@ describe Projects::NotesController do
request.headers['X-Last-Fetched-At'] = last_fetched_at request.headers['X-Last-Fetched-At'] = last_fetched_at
expect(NotesFinder).to receive(:new) expect(NotesFinder).to receive(:new)
.with(anything, anything, hash_including(last_fetched_at: last_fetched_at)) .with(anything, hash_including(last_fetched_at: last_fetched_at))
.and_call_original .and_call_original
get :index, params: request_params get :index, params: request_params

View file

@ -14,7 +14,7 @@ describe NotesFinder do
let!(:system_note) { create(:note_on_issue, project: project, system: true) } let!(:system_note) { create(:note_on_issue, project: project, system: true) }
it 'returns only user notes when using only_comments filter' do it 'returns only user notes when using only_comments filter' do
finder = described_class.new(project, user, notes_filter: UserPreference::NOTES_FILTERS[:only_comments]) finder = described_class.new(user, project: project, notes_filter: UserPreference::NOTES_FILTERS[:only_comments])
notes = finder.execute notes = finder.execute
@ -22,7 +22,7 @@ describe NotesFinder do
end end
it 'returns only system notes when using only_activity filters' do it 'returns only system notes when using only_activity filters' do
finder = described_class.new(project, user, notes_filter: UserPreference::NOTES_FILTERS[:only_activity]) finder = described_class.new(user, project: project, notes_filter: UserPreference::NOTES_FILTERS[:only_activity])
notes = finder.execute notes = finder.execute
@ -30,7 +30,7 @@ describe NotesFinder do
end end
it 'gets all notes' do it 'gets all notes' do
finder = described_class.new(project, user, notes_filter: UserPreference::NOTES_FILTERS[:all_activity]) finder = described_class.new(user, project: project, notes_filter: UserPreference::NOTES_FILTERS[:all_activity])
notes = finder.execute notes = finder.execute
@ -41,7 +41,7 @@ describe NotesFinder do
it 'finds notes on merge requests' do it 'finds notes on merge requests' do
create(:note_on_merge_request, project: project) create(:note_on_merge_request, project: project)
notes = described_class.new(project, user).execute notes = described_class.new(user, project: project).execute
expect(notes.count).to eq(1) expect(notes.count).to eq(1)
end end
@ -49,7 +49,7 @@ describe NotesFinder do
it 'finds notes on snippets' do it 'finds notes on snippets' do
create(:note_on_project_snippet, project: project) create(:note_on_project_snippet, project: project)
notes = described_class.new(project, user).execute notes = described_class.new(user, project: project).execute
expect(notes.count).to eq(1) expect(notes.count).to eq(1)
end end
@ -59,13 +59,13 @@ describe NotesFinder do
note = create(:note_on_commit, project: project) note = create(:note_on_commit, project: project)
params = { target_type: 'commit', target_id: note.noteable.id } params = { target_type: 'commit', target_id: note.noteable.id }
notes = described_class.new(project, create(:user), params).execute notes = described_class.new(create(:user), params).execute
expect(notes.count).to eq(0) expect(notes.count).to eq(0)
end end
it 'succeeds when no notes found' do it 'succeeds when no notes found' do
notes = described_class.new(project, create(:user)).execute notes = described_class.new(create(:user), project: project).execute
expect(notes.count).to eq(0) expect(notes.count).to eq(0)
end end
@ -82,7 +82,7 @@ describe NotesFinder do
it 'publicly excludes notes on merge requests' do it 'publicly excludes notes on merge requests' do
create(:note_on_merge_request, project: project) create(:note_on_merge_request, project: project)
notes = described_class.new(project, create(:user)).execute notes = described_class.new(create(:user), project: project).execute
expect(notes.count).to eq(0) expect(notes.count).to eq(0)
end end
@ -90,7 +90,7 @@ describe NotesFinder do
it 'publicly excludes notes on issues' do it 'publicly excludes notes on issues' do
create(:note_on_issue, project: project) create(:note_on_issue, project: project)
notes = described_class.new(project, create(:user)).execute notes = described_class.new(create(:user), project: project).execute
expect(notes.count).to eq(0) expect(notes.count).to eq(0)
end end
@ -98,7 +98,7 @@ describe NotesFinder do
it 'publicly excludes notes on snippets' do it 'publicly excludes notes on snippets' do
create(:note_on_project_snippet, project: project) create(:note_on_project_snippet, project: project)
notes = described_class.new(project, create(:user)).execute notes = described_class.new(create(:user), project: project).execute
expect(notes.count).to eq(0) expect(notes.count).to eq(0)
end end
@ -110,7 +110,7 @@ describe NotesFinder do
let!(:note2) { create :note_on_commit, project: project } let!(:note2) { create :note_on_commit, project: project }
it 'finds only notes for the selected type' do it 'finds only notes for the selected type' do
notes = described_class.new(project, user, target_type: 'issue').execute notes = described_class.new(user, project: project, target_type: 'issue').execute
expect(notes).to eq([note1]) expect(notes).to eq([note1])
end end
@ -118,56 +118,51 @@ describe NotesFinder do
context 'for target' do context 'for target' do
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
let(:note1) { create :note_on_commit, project: project } let!(:note1) { create :note_on_commit, project: project }
let(:note2) { create :note_on_commit, project: project } let!(:note2) { create :note_on_commit, project: project }
let(:commit) { note1.noteable } let(:commit) { note1.noteable }
let(:params) { { target_id: commit.id, target_type: 'commit', last_fetched_at: 1.hour.ago.to_i } } let(:params) { { project: project, 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 it 'finds all notes' do
notes = described_class.new(project, user, params).execute notes = described_class.new(user, params).execute
expect(notes.size).to eq(2) expect(notes.size).to eq(2)
end end
it 'finds notes on merge requests' do it 'finds notes on merge requests' do
note = create(:note_on_merge_request, project: project) note = create(:note_on_merge_request, project: project)
params = { target_type: 'merge_request', target_id: note.noteable.id } params = { project: project, target_type: 'merge_request', target_id: note.noteable.id }
notes = described_class.new(project, user, params).execute notes = described_class.new(user, params).execute
expect(notes).to include(note) expect(notes).to include(note)
end end
it 'finds notes on snippets' do it 'finds notes on snippets' do
note = create(:note_on_project_snippet, project: project) note = create(:note_on_project_snippet, project: project)
params = { target_type: 'snippet', target_id: note.noteable.id } params = { project: project, target_type: 'snippet', target_id: note.noteable.id }
notes = described_class.new(project, user, params).execute notes = described_class.new(user, params).execute
expect(notes.count).to eq(1) expect(notes.count).to eq(1)
end end
it 'finds notes on personal snippets' do it 'finds notes on personal snippets' do
note = create(:note_on_personal_snippet) note = create(:note_on_personal_snippet)
params = { target_type: 'personal_snippet', target_id: note.noteable_id } params = { project: project, target_type: 'personal_snippet', target_id: note.noteable_id }
notes = described_class.new(project, user, params).execute notes = described_class.new(user, params).execute
expect(notes.count).to eq(1) expect(notes.count).to eq(1)
end end
it 'raises an exception for an invalid target_type' do it 'raises an exception for an invalid target_type' do
params[:target_type] = 'invalid' params[:target_type] = 'invalid'
expect { described_class.new(project, user, params).execute }.to raise_error("invalid target_type '#{params[:target_type]}'") expect { described_class.new(user, params).execute }.to raise_error("invalid target_type '#{params[:target_type]}'")
end end
it 'filters out old notes' do it 'filters out old notes' do
note2.update_attribute(:updated_at, 2.hours.ago) note2.update_attribute(:updated_at, 2.hours.ago)
notes = described_class.new(project, user, params).execute notes = described_class.new(user, params).execute
expect(notes).to eq([note1]) expect(notes).to eq([note1])
end end
@ -175,25 +170,47 @@ describe NotesFinder do
let(:confidential_issue) { create(:issue, :confidential, project: project, author: user) } let(:confidential_issue) { create(:issue, :confidential, project: project, author: user) }
let!(:confidential_note) { create(:note, noteable: confidential_issue, project: confidential_issue.project) } let!(:confidential_note) { create(:note, noteable: confidential_issue, project: confidential_issue.project) }
let(:params) { { target_id: confidential_issue.id, target_type: 'issue', last_fetched_at: 1.hour.ago.to_i } } let(:params) { { project: confidential_issue.project, target_id: confidential_issue.id, target_type: 'issue', last_fetched_at: 1.hour.ago.to_i } }
it 'returns notes if user can see the issue' do it 'returns notes if user can see the issue' do
expect(described_class.new(project, user, params).execute).to eq([confidential_note]) expect(described_class.new(user, params).execute).to eq([confidential_note])
end end
it 'raises an error if user can not see the issue' do it 'raises an error if user can not see the issue' do
user = create(:user) user = create(:user)
expect { described_class.new(project, user, params).execute }.to raise_error(ActiveRecord::RecordNotFound) expect { described_class.new(user, params).execute }.to raise_error(ActiveRecord::RecordNotFound)
end end
it 'raises an error for project members with guest role' do it 'raises an error for project members with guest role' do
user = create(:user) user = create(:user)
project.add_guest(user) project.add_guest(user)
expect { described_class.new(project, user, params).execute }.to raise_error(ActiveRecord::RecordNotFound) expect { described_class.new(user, params).execute }.to raise_error(ActiveRecord::RecordNotFound)
end end
end 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) { { project: project, target: commit } }
it 'returns the expected notes' do
expect(described_class.new(user, params).execute).to eq([note1, note2])
end
it 'returns the expected notes when last_fetched_at is given' do
params = { project: project, target: commit, last_fetched_at: 1.hour.ago.to_i }
expect(described_class.new(user, params).execute).to eq([note2])
end
it 'fails when nil is provided' do
params = { project: project, target: nil }
expect { described_class.new(user, params).execute }.to raise_error(RuntimeError)
end
end
end end
describe '.search' do describe '.search' do
@ -201,17 +218,17 @@ describe NotesFinder do
let(:note) { create(:note_on_issue, note: 'WoW', project: project) } let(:note) { create(:note_on_issue, note: 'WoW', project: project) }
it 'returns notes with matching content' do it 'returns notes with matching content' do
expect(described_class.new(note.project, nil, search: note.note).execute).to eq([note]) expect(described_class.new(nil, project: note.project, search: note.note).execute).to eq([note])
end end
it 'returns notes with matching content regardless of the casing' do it 'returns notes with matching content regardless of the casing' do
expect(described_class.new(note.project, nil, search: 'WOW').execute).to eq([note]) expect(described_class.new(nil, project: note.project, search: 'WOW').execute).to eq([note])
end end
it 'returns commit notes user can access' do it 'returns commit notes user can access' do
note = create(:note_on_commit, project: project) note = create(:note_on_commit, project: project)
expect(described_class.new(note.project, create(:user), search: note.note).execute).to eq([note]) expect(described_class.new(create(:user), project: note.project, search: note.note).execute).to eq([note])
end end
context "confidential issues" do context "confidential issues" do
@ -220,27 +237,27 @@ describe NotesFinder do
let(:confidential_note) { create(:note, note: "Random", noteable: confidential_issue, project: confidential_issue.project) } let(:confidential_note) { create(:note, note: "Random", noteable: confidential_issue, project: confidential_issue.project) }
it "returns notes with matching content if user can see the issue" do it "returns notes with matching content if user can see the issue" do
expect(described_class.new(confidential_note.project, user, search: confidential_note.note).execute).to eq([confidential_note]) expect(described_class.new(user, project: confidential_note.project, search: confidential_note.note).execute).to eq([confidential_note])
end end
it "does not return notes with matching content if user can not see the issue" do it "does not return notes with matching content if user can not see the issue" do
user = create(:user) user = create(:user)
expect(described_class.new(confidential_note.project, user, search: confidential_note.note).execute).to be_empty expect(described_class.new(user, project: confidential_note.project, search: confidential_note.note).execute).to be_empty
end end
it "does not return notes with matching content for project members with guest role" do it "does not return notes with matching content for project members with guest role" do
user = create(:user) user = create(:user)
project.add_guest(user) project.add_guest(user)
expect(described_class.new(confidential_note.project, user, search: confidential_note.note).execute).to be_empty expect(described_class.new(user, project: confidential_note.project, search: confidential_note.note).execute).to be_empty
end end
it "does not return notes with matching content for unauthenticated users" do it "does not return notes with matching content for unauthenticated users" do
expect(described_class.new(confidential_note.project, nil, search: confidential_note.note).execute).to be_empty expect(described_class.new(nil, project: confidential_note.project, search: confidential_note.note).execute).to be_empty
end end
end end
context 'inlines SQL filters on subqueries for performance' do context 'inlines SQL filters on subqueries for performance' do
let(:sql) { described_class.new(note.project, nil, search: note.note).execute.to_sql } let(:sql) { described_class.new(nil, project: note.project, search: note.note).execute.to_sql }
let(:number_of_noteable_types) { 4 } let(:number_of_noteable_types) { 4 }
specify 'project_id check' do specify 'project_id check' do
@ -254,11 +271,11 @@ describe NotesFinder do
end end
describe '#target' do describe '#target' do
subject { described_class.new(project, user, params) } subject { described_class.new(user, params) }
context 'for a issue target' do context 'for a issue target' do
let(:issue) { create(:issue, project: project) } let(:issue) { create(:issue, project: project) }
let(:params) { { target_type: 'issue', target_id: issue.id } } let(:params) { { project: project, target_type: 'issue', target_id: issue.id } }
it 'returns the issue' do it 'returns the issue' do
expect(subject.target).to eq(issue) expect(subject.target).to eq(issue)
@ -267,7 +284,7 @@ describe NotesFinder do
context 'for a merge request target' do context 'for a merge request target' do
let(:merge_request) { create(:merge_request, source_project: project) } let(:merge_request) { create(:merge_request, source_project: project) }
let(:params) { { target_type: 'merge_request', target_id: merge_request.id } } let(:params) { { project: project, target_type: 'merge_request', target_id: merge_request.id } }
it 'returns the merge_request' do it 'returns the merge_request' do
expect(subject.target).to eq(merge_request) expect(subject.target).to eq(merge_request)
@ -276,7 +293,7 @@ describe NotesFinder do
context 'for a snippet target' do context 'for a snippet target' do
let(:snippet) { create(:project_snippet, project: project) } let(:snippet) { create(:project_snippet, project: project) }
let(:params) { { target_type: 'snippet', target_id: snippet.id } } let(:params) { { project: project, target_type: 'snippet', target_id: snippet.id } }
it 'returns the snippet' do it 'returns the snippet' do
expect(subject.target).to eq(snippet) expect(subject.target).to eq(snippet)
@ -286,7 +303,7 @@ describe NotesFinder do
context 'for a commit target' do context 'for a commit target' do
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
let(:commit) { project.commit } let(:commit) { project.commit }
let(:params) { { target_type: 'commit', target_id: commit.id } } let(:params) { { project: project, target_type: 'commit', target_id: commit.id } }
it 'returns the commit' do it 'returns the commit' do
expect(subject.target).to eq(commit) expect(subject.target).to eq(commit)
@ -298,24 +315,24 @@ describe NotesFinder do
let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
it 'finds issues by iid' do it 'finds issues by iid' do
iid_params = { target_type: 'issue', target_iid: issue.iid } iid_params = { project: project, target_type: 'issue', target_iid: issue.iid }
expect(described_class.new(project, user, iid_params).target).to eq(issue) expect(described_class.new(user, iid_params).target).to eq(issue)
end end
it 'finds merge requests by iid' do it 'finds merge requests by iid' do
iid_params = { target_type: 'merge_request', target_iid: merge_request.iid } iid_params = { project: project, target_type: 'merge_request', target_iid: merge_request.iid }
expect(described_class.new(project, user, iid_params).target).to eq(merge_request) expect(described_class.new(user, iid_params).target).to eq(merge_request)
end end
it 'returns nil if both target_id and target_iid are not given' do it 'returns nil if both target_id and target_iid are not given' do
params_without_any_id = { target_type: 'issue' } params_without_any_id = { project: project, target_type: 'issue' }
expect(described_class.new(project, user, params_without_any_id).target).to be_nil expect(described_class.new(user, params_without_any_id).target).to be_nil
end end
it 'prioritizes target_id over target_iid' do it 'prioritizes target_id over target_iid' do
issue2 = create(:issue, project: project) issue2 = create(:issue, project: project)
iid_params = { target_type: 'issue', target_id: issue2.id, target_iid: issue.iid } iid_params = { project: project, target_type: 'issue', target_id: issue2.id, target_iid: issue.iid }
expect(described_class.new(project, user, iid_params).target).to eq(issue2) expect(described_class.new(user, iid_params).target).to eq(issue2)
end end
end end
end end

View file

@ -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
}
}
}
}

View file

@ -0,0 +1,4 @@
{
"type": "array",
"items": { "$ref": "discussion.json" }
}

View file

@ -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