Commit ea1b24cb authored by Robert Speicher's avatar Robert Speicher

Merge branch 'pderichs-52123' into 'master'

Use NotesFinder to get Noteable

See merge request gitlab-org/gitlab-ce!28205
parents 9fa2b657 5469d21d
......@@ -34,8 +34,10 @@ class NotesFinder
target_type = @params[:target_type]
target_id = @params[:target_id]
target_iid = @params[:target_iid]
return @target = nil unless target_type && target_id
return @target = nil unless target_type
return @target = nil unless target_id || target_iid
@target =
if target_type == "commit"
......@@ -43,12 +45,22 @@ class NotesFinder
@project.commit(target_id)
end
else
noteables_for_type(target_type).find(target_id)
noteables_for_type_by_id(target_type, target_id, target_iid)
end
end
private
def noteables_for_type_by_id(type, id, iid)
query = if id
{ id: id }
else
{ iid: iid }
end
noteables_for_type(type).find_by!(query) # rubocop: disable CodeReuse/ActiveRecord
end
def init_collection
if target
notes_on_target
......
......@@ -25,7 +25,7 @@ module API
end
# rubocop: disable CodeReuse/ActiveRecord
get ":id/#{noteables_path}/:noteable_id/discussions" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
notes = noteable.notes
.inc_relations_for_view
......@@ -47,7 +47,7 @@ module API
requires :noteable_id, types: [Integer, String], desc: 'The ID of the noteable'
end
get ":id/#{noteables_path}/:noteable_id/discussions/:discussion_id" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
notes = readable_discussion_notes(noteable, params[:discussion_id])
if notes.empty?
......@@ -82,7 +82,7 @@ module API
end
end
post ":id/#{noteables_path}/:noteable_id/discussions" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
type = params[:position] ? 'DiffNote' : 'DiscussionNote'
id_key = noteable.is_a?(Commit) ? :commit_id : :noteable_id
......@@ -112,7 +112,7 @@ module API
requires :noteable_id, types: [Integer, String], desc: 'The ID of the noteable'
end
get ":id/#{noteables_path}/:noteable_id/discussions/:discussion_id/notes" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
notes = readable_discussion_notes(noteable, params[:discussion_id])
if notes.empty?
......@@ -132,7 +132,7 @@ module API
optional :created_at, type: String, desc: 'The creation date of the note'
end
post ":id/#{noteables_path}/:noteable_id/discussions/:discussion_id/notes" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
notes = readable_discussion_notes(noteable, params[:discussion_id])
first_note = notes.first
......@@ -166,7 +166,7 @@ module API
requires :note_id, type: Integer, desc: 'The ID of a note'
end
get ":id/#{noteables_path}/:noteable_id/discussions/:discussion_id/notes/:note_id" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
get_note(noteable, params[:note_id])
end
......@@ -183,7 +183,7 @@ module API
exactly_one_of :body, :resolved
end
put ":id/#{noteables_path}/:noteable_id/discussions/:discussion_id/notes/:note_id" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
if params[:resolved].nil?
update_note(noteable, params[:note_id])
......@@ -201,7 +201,7 @@ module API
requires :note_id, type: Integer, desc: 'The ID of a note'
end
delete ":id/#{noteables_path}/:noteable_id/discussions/:discussion_id/notes/:note_id" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
delete_note(noteable, params[:note_id])
end
......@@ -216,7 +216,7 @@ module API
requires :resolved, type: Boolean, desc: 'Mark discussion resolved/unresolved'
end
put ":id/#{noteables_path}/:noteable_id/discussions/:discussion_id" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
resolve_discussion(noteable, params[:discussion_id], params[:resolved])
end
......
......@@ -183,11 +183,6 @@ module API
user_project.commit_by(oid: id)
end
def find_project_snippet(id)
finder_params = { project: user_project }
SnippetsFinder.new(current_user, finder_params).find(id)
end
# rubocop: disable CodeReuse/ActiveRecord
def find_merge_request_with_access(iid, access_level = :read_merge_request)
merge_request = user_project.merge_requests.find_by!(iid: iid)
......
......@@ -73,14 +73,23 @@ module API
"read_#{noteable.class.to_s.underscore}".to_sym
end
def find_noteable(parent, noteables_str, noteable_id)
noteable = public_send("find_#{parent}_#{noteables_str.singularize}", noteable_id) # rubocop:disable GitlabSecurity/PublicSend
def find_noteable(parent_type, parent_id, noteable_type, noteable_id)
params = params_by_noteable_type_and_id(noteable_type, noteable_id)
readable = can?(current_user, noteable_read_ability_name(noteable), noteable)
return not_found!(noteables_str) unless readable
noteable = NotesFinder.new(user_project, current_user, params).target
noteable = nil unless can?(current_user, noteable_read_ability_name(noteable), noteable)
noteable || not_found!(noteable_type)
end
noteable
def params_by_noteable_type_and_id(type, id)
target_type = type.name.underscore
{ target_type: target_type }.tap do |h|
if %w(issue merge_request).include?(target_type)
h[:target_iid] = id
else
h[:target_id] = id
end
end
end
def noteable_parent(noteable)
......
......@@ -15,8 +15,6 @@ module API
requires :id, type: String, desc: "The ID of a #{parent_type}"
end
resource parent_type.pluralize.to_sym, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
noteables_str = noteable_type.to_s.underscore.pluralize
desc "Get a list of #{noteable_type.to_s.downcase} notes" do
success Entities::Note
end
......@@ -30,7 +28,7 @@ module API
end
# rubocop: disable CodeReuse/ActiveRecord
get ":id/#{noteables_str}/:noteable_id/notes" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
# We exclude notes that are cross-references and that cannot be viewed
# by the current user. By doing this exclusion at this level and not
......@@ -56,7 +54,7 @@ module API
requires :noteable_id, type: Integer, desc: 'The ID of the noteable'
end
get ":id/#{noteables_str}/:noteable_id/notes/:note_id" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
get_note(noteable, params[:note_id])
end
......@@ -69,7 +67,7 @@ module API
optional :created_at, type: String, desc: 'The creation date of the note'
end
post ":id/#{noteables_str}/:noteable_id/notes" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
opts = {
note: params[:body],
......@@ -96,7 +94,7 @@ module API
requires :body, type: String, desc: 'The content of a note'
end
put ":id/#{noteables_str}/:noteable_id/notes/:note_id" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
update_note(noteable, params[:note_id])
end
......@@ -109,7 +107,7 @@ module API
requires :note_id, type: Integer, desc: 'The ID of a note'
end
delete ":id/#{noteables_str}/:noteable_id/notes/:note_id" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
noteable = find_noteable(parent_type, params[:id], noteable_type, params[:noteable_id])
delete_note(noteable, params[:note_id])
end
......
......@@ -26,7 +26,7 @@ module API
# rubocop: disable CodeReuse/ActiveRecord
get ":id/#{eventables_str}/:eventable_id/resource_label_events" do
eventable = find_noteable(parent_type, eventables_str, params[:eventable_id])
eventable = find_noteable(parent_type, params[:id], eventable_type, params[:eventable_id])
events = eventable.resource_label_events.includes(:label, :user)
present paginate(events), with: Entities::ResourceLabelEvent
......@@ -42,7 +42,7 @@ module API
requires :eventable_id, types: [Integer, String], desc: 'The ID of the eventable'
end
get ":id/#{eventables_str}/:eventable_id/resource_label_events/:event_id" do
eventable = find_noteable(parent_type, eventables_str, params[:eventable_id])
eventable = find_noteable(parent_type, params[:id], eventable_type, params[:eventable_id])
event = eventable.resource_label_events.find(params[:event_id])
present event, with: Entities::ResourceLabelEvent
......
......@@ -292,5 +292,31 @@ describe NotesFinder do
expect(subject.target).to eq(commit)
end
end
context 'target_iid' do
let(:issue) { create(:issue, project: project) }
let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
it 'finds issues by iid' do
iid_params = { target_type: 'issue', target_iid: issue.iid }
expect(described_class.new(project, user, iid_params).target).to eq(issue)
end
it 'finds merge requests by iid' do
iid_params = { target_type: 'merge_request', target_iid: merge_request.iid }
expect(described_class.new(project, user, iid_params).target).to eq(merge_request)
end
it 'returns nil if both target_id and target_iid are not given' do
params_without_any_id = { target_type: 'issue' }
expect(described_class.new(project, user, params_without_any_id).target).to be_nil
end
it 'prioritizes target_id over target_iid' do
issue2 = create(:issue, project: project)
iid_params = { target_type: 'issue', target_id: issue2.id, target_iid: issue.iid }
expect(described_class.new(project, user, iid_params).target).to eq(issue2)
end
end
end
end
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