Commit dcdfa04b authored by Jan Provaznik's avatar Jan Provaznik

Add discussion API

* adds basic discussions API for issues and snippets
* reorganizes notes specs (so same tests can be used for all noteable types - issues, MRs, snippets)
parent 8a0052c0
...@@ -222,6 +222,10 @@ module Issuable ...@@ -222,6 +222,10 @@ module Issuable
def to_ability_name def to_ability_name
model_name.singular model_name.singular
end end
def parent_class
::Project
end
end end
def today? def today?
......
...@@ -81,7 +81,7 @@ class Note < ActiveRecord::Base ...@@ -81,7 +81,7 @@ class Note < ActiveRecord::Base
validates :author, presence: true validates :author, presence: true
validates :discussion_id, presence: true, format: { with: /\A\h{40}\z/ } validates :discussion_id, presence: true, format: { with: /\A\h{40}\z/ }
validate unless: [:for_commit?, :importing?, :for_personal_snippet?] do |note| validate unless: [:for_commit?, :importing?, :skip_project_check?] do |note|
unless note.noteable.try(:project) == note.project unless note.noteable.try(:project) == note.project
errors.add(:project, 'does not match noteable project') errors.add(:project, 'does not match noteable project')
end end
...@@ -228,7 +228,7 @@ class Note < ActiveRecord::Base ...@@ -228,7 +228,7 @@ class Note < ActiveRecord::Base
end end
def skip_project_check? def skip_project_check?
for_personal_snippet? !for_project_noteable?
end end
def commit def commit
...@@ -308,6 +308,11 @@ class Note < ActiveRecord::Base ...@@ -308,6 +308,11 @@ class Note < ActiveRecord::Base
self.noteable.supports_discussions? && !part_of_discussion? self.noteable.supports_discussions? && !part_of_discussion?
end end
def can_create_todo?
# Skip system notes, and notes on project snippet
!system? && !for_snippet?
end
def discussion_class(noteable = nil) def discussion_class(noteable = nil)
# When commit notes are rendered on an MR's Discussion page, they are # When commit notes are rendered on an MR's Discussion page, they are
# displayed in one discussion instead of individually. # displayed in one discussion instead of individually.
......
...@@ -168,5 +168,9 @@ class Snippet < ActiveRecord::Base ...@@ -168,5 +168,9 @@ class Snippet < ActiveRecord::Base
def search_code(query) def search_code(query)
fuzzy_search(query, [:content]) fuzzy_search(query, [:content])
end end
def parent_class
::Project
end
end end
end end
...@@ -26,14 +26,19 @@ module Notes ...@@ -26,14 +26,19 @@ module Notes
if project if project
project.notes.find_discussion(discussion_id) project.notes.find_discussion(discussion_id)
else else
# only PersonalSnippets can have discussions without project association
discussion = Note.find_discussion(discussion_id) discussion = Note.find_discussion(discussion_id)
noteable = discussion.noteable noteable = discussion.noteable
return nil unless noteable.is_a?(PersonalSnippet) && can?(current_user, :comment_personal_snippet, noteable) return nil unless noteable_without_project?(noteable)
discussion discussion
end end
end end
def noteable_without_project?(noteable)
return true if noteable.is_a?(PersonalSnippet) && can?(current_user, :comment_personal_snippet, noteable)
false
end
end end
end end
...@@ -11,7 +11,7 @@ module Notes ...@@ -11,7 +11,7 @@ module Notes
unless @note.system? unless @note.system?
EventCreateService.new.leave_note(@note, @note.author) EventCreateService.new.leave_note(@note, @note.author)
return if @note.for_personal_snippet? return unless @note.for_project_noteable?
@note.create_cross_references! @note.create_cross_references!
execute_note_hooks execute_note_hooks
......
...@@ -280,7 +280,7 @@ module NotificationRecipientService ...@@ -280,7 +280,7 @@ module NotificationRecipientService
add_participants(note.author) add_participants(note.author)
add_mentions(note.author, target: note) add_mentions(note.author, target: note)
unless note.for_personal_snippet? if note.for_project_noteable?
# Merge project watchers # Merge project watchers
add_project_watchers add_project_watchers
......
...@@ -241,8 +241,7 @@ class TodoService ...@@ -241,8 +241,7 @@ class TodoService
end end
def handle_note(note, author, skip_users = []) def handle_note(note, author, skip_users = [])
# Skip system notes, and notes on project snippet return unless note.can_create_todo?
return if note.system? || note.for_snippet?
project = note.project project = note.project
target = note.noteable target = note.noteable
......
---
title: Add discussions API for Issues and Snippets
merge_request:
author:
type: added
...@@ -35,6 +35,7 @@ following locations: ...@@ -35,6 +35,7 @@ following locations:
- [Group milestones](group_milestones.md) - [Group milestones](group_milestones.md)
- [Namespaces](namespaces.md) - [Namespaces](namespaces.md)
- [Notes](notes.md) (comments) - [Notes](notes.md) (comments)
- [Threaded comments](discussions.md)
- [Notification settings](notification_settings.md) - [Notification settings](notification_settings.md)
- [Open source license templates](templates/licenses.md) - [Open source license templates](templates/licenses.md)
- [Pages Domains](pages_domains.md) - [Pages Domains](pages_domains.md)
......
This diff is collapsed.
This diff is collapsed.
...@@ -134,6 +134,7 @@ module API ...@@ -134,6 +134,7 @@ module API
mount ::API::MergeRequests mount ::API::MergeRequests
mount ::API::Namespaces mount ::API::Namespaces
mount ::API::Notes mount ::API::Notes
mount ::API::Discussions
mount ::API::NotificationSettings mount ::API::NotificationSettings
mount ::API::PagesDomains mount ::API::PagesDomains
mount ::API::Pipelines mount ::API::Pipelines
......
module API
class Discussions < Grape::API
include PaginationParams
helpers ::API::Helpers::NotesHelpers
before { authenticate! }
NOTEABLE_TYPES = [Issue, Snippet].freeze
NOTEABLE_TYPES.each do |noteable_type|
parent_type = noteable_type.parent_class.to_s.underscore
noteables_str = noteable_type.to_s.underscore.pluralize
params do
requires :id, type: String, desc: "The ID of a #{parent_type}"
end
resource parent_type.pluralize.to_sym, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do
desc "Get a list of #{noteable_type.to_s.downcase} discussions" do
success Entities::Discussion
end
params do
requires :noteable_id, type: Integer, desc: 'The ID of the noteable'
use :pagination
end
get ":id/#{noteables_str}/:noteable_id/discussions" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
return not_found!("Discussions") unless can?(current_user, noteable_read_ability_name(noteable), noteable)
notes = noteable.notes
.inc_relations_for_view
.includes(:noteable)
.fresh
notes = notes.reject { |n| n.cross_reference_not_visible_for?(current_user) }
discussions = Kaminari.paginate_array(Discussion.build_collection(notes, noteable))
present paginate(discussions), with: Entities::Discussion
end
desc "Get a single #{noteable_type.to_s.downcase} discussion" do
success Entities::Discussion
end
params do
requires :discussion_id, type: String, desc: 'The ID of a discussion'
requires :noteable_id, type: Integer, desc: 'The ID of the noteable'
end
get ":id/#{noteables_str}/:noteable_id/discussions/:discussion_id" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
notes = readable_discussion_notes(noteable, params[:discussion_id])
if notes.empty? || !can?(current_user, noteable_read_ability_name(noteable), noteable)
return not_found!("Discussion")
end
discussion = Discussion.build(notes, noteable)
present discussion, with: Entities::Discussion
end
desc "Create a new #{noteable_type.to_s.downcase} discussion" do
success Entities::Discussion
end
params do
requires :noteable_id, type: Integer, desc: 'The ID of the noteable'
requires :body, type: String, desc: 'The content of a note'
optional :created_at, type: String, desc: 'The creation date of the note'
end
post ":id/#{noteables_str}/:noteable_id/discussions" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
opts = {
note: params[:body],
created_at: params[:created_at],
type: 'DiscussionNote',
noteable_type: noteables_str.classify,
noteable_id: noteable.id
}
note = create_note(noteable, opts)
if note.valid?
present note.discussion, with: Entities::Discussion
else
bad_request!("Note #{note.errors.messages}")
end
end
desc "Get comments in a single #{noteable_type.to_s.downcase} discussion" do
success Entities::Discussion
end
params do
requires :discussion_id, type: String, desc: 'The ID of a discussion'
requires :noteable_id, type: Integer, desc: 'The ID of the noteable'
end
get ":id/#{noteables_str}/:noteable_id/discussions/:discussion_id/notes" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
notes = readable_discussion_notes(noteable, params[:discussion_id])
if notes.empty? || !can?(current_user, noteable_read_ability_name(noteable), noteable)
return not_found!("Notes")
end
present notes, with: Entities::Note
end
desc "Add a comment to a #{noteable_type.to_s.downcase} discussion" do
success Entities::Note
end
params do
requires :noteable_id, type: Integer, desc: 'The ID of the noteable'
requires :discussion_id, type: String, desc: 'The ID of a discussion'
requires :body, type: String, desc: 'The content of a note'
optional :created_at, type: String, desc: 'The creation date of the note'
end
post ":id/#{noteables_str}/:noteable_id/discussions/:discussion_id/notes" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
notes = readable_discussion_notes(noteable, params[:discussion_id])
return not_found!("Discussion") if notes.empty?
return bad_request!("Discussion is an individual note.") unless notes.first.part_of_discussion?
opts = {
note: params[:body],
type: 'DiscussionNote',
in_reply_to_discussion_id: params[:discussion_id],
created_at: params[:created_at]
}
note = create_note(noteable, opts)
if note.valid?
present note, with: Entities::Note
else
bad_request!("Note #{note.errors.messages}")
end
end
desc "Get a comment in a #{noteable_type.to_s.downcase} discussion" do
success Entities::Note
end
params do
requires :noteable_id, type: Integer, desc: 'The ID of the noteable'
requires :discussion_id, type: String, desc: 'The ID of a discussion'
requires :note_id, type: Integer, desc: 'The ID of a note'
end
get ":id/#{noteables_str}/:noteable_id/discussions/:discussion_id/notes/:note_id" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
get_note(noteable, params[:note_id])
end
desc "Edit a comment in a #{noteable_type.to_s.downcase} discussion" do
success Entities::Note
end
params do
requires :noteable_id, type: Integer, desc: 'The ID of the noteable'
requires :discussion_id, type: String, desc: 'The ID of a discussion'
requires :note_id, type: Integer, desc: 'The ID of a note'
requires :body, type: String, desc: 'The content of a note'
end
put ":id/#{noteables_str}/:noteable_id/discussions/:discussion_id/notes/:note_id" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
update_note(noteable, params[:note_id])
end
desc "Delete a comment in a #{noteable_type.to_s.downcase} discussion" do
success Entities::Note
end
params do
requires :noteable_id, type: Integer, desc: 'The ID of the noteable'
requires :discussion_id, type: String, desc: 'The ID of a discussion'
requires :note_id, type: Integer, desc: 'The ID of a note'
end
delete ":id/#{noteables_str}/:noteable_id/discussions/:discussion_id/notes/:note_id" do
noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
delete_note(noteable, params[:note_id])
end
end
end
helpers do
def readable_discussion_notes(noteable, discussion_id)
notes = noteable.notes
.where(discussion_id: discussion_id)
.inc_relations_for_view
.includes(:noteable)
.fresh
notes.reject { |n| n.cross_reference_not_visible_for?(current_user) }
end
end
end
end
...@@ -629,6 +629,7 @@ module API ...@@ -629,6 +629,7 @@ module API
NOTEABLE_TYPES_WITH_IID = %w(Issue MergeRequest).freeze NOTEABLE_TYPES_WITH_IID = %w(Issue MergeRequest).freeze
expose :id expose :id
expose :type
expose :note, as: :body expose :note, as: :body
expose :attachment_identifier, as: :attachment expose :attachment_identifier, as: :attachment
expose :author, using: Entities::UserBasic expose :author, using: Entities::UserBasic
...@@ -640,6 +641,12 @@ module API ...@@ -640,6 +641,12 @@ module API
expose(:noteable_iid) { |note| note.noteable.iid if NOTEABLE_TYPES_WITH_IID.include?(note.noteable_type) } expose(:noteable_iid) { |note| note.noteable.iid if NOTEABLE_TYPES_WITH_IID.include?(note.noteable_type) }
end end
class Discussion < Grape::Entity
expose :id
expose :individual_note?, as: :individual_note
expose :notes, using: Entities::Note
end
class AwardEmoji < Grape::Entity class AwardEmoji < Grape::Entity
expose :id expose :id
expose :name expose :name
......
module API
module Helpers
module NotesHelpers
def update_note(noteable, note_id)
note = noteable.notes.find(params[:note_id])
authorize! :admin_note, note
opts = {
note: params[:body]
}
parent = noteable_parent(noteable)
project = parent if parent.is_a?(Project)
note = ::Notes::UpdateService.new(project, current_user, opts).execute(note)
if note.valid?
present note, with: Entities::Note
else
bad_request!("Failed to save note #{note.errors.messages}")
end
end
def delete_note(noteable, note_id)
note = noteable.notes.find(note_id)
authorize! :admin_note, note
parent = noteable_parent(noteable)
project = parent if parent.is_a?(Project)
destroy_conditionally!(note) do |note|
::Notes::DestroyService.new(project, current_user).execute(note)
end
end
def get_note(noteable, note_id)
note = noteable.notes.with_metadata.find(params[:note_id])
can_read_note = can?(current_user, noteable_read_ability_name(noteable), noteable) && !note.cross_reference_not_visible_for?(current_user)
if can_read_note
present note, with: Entities::Note
else
not_found!("Note")
end
end
def noteable_read_ability_name(noteable)
"read_#{noteable.class.to_s.underscore}".to_sym
end
def find_noteable(parent, noteables_str, noteable_id)
public_send("find_#{parent}_#{noteables_str.singularize}", noteable_id) # rubocop:disable GitlabSecurity/PublicSend
end
def noteable_parent(noteable)
public_send("user_#{noteable.class.parent_class.to_s.underscore}") # rubocop:disable GitlabSecurity/PublicSend
end
def create_note(noteable, opts)
noteables_str = noteable.model_name.to_s.underscore.pluralize
return not_found!(noteables_str) unless can?(current_user, noteable_read_ability_name(noteable), noteable)
authorize! :create_note, noteable
parent = noteable_parent(noteable)
if opts[:created_at]
opts.delete(:created_at) unless current_user.admin? || parent.owner == current_user
end
project = parent if parent.is_a?(Project)
::Notes::CreateService.new(project, current_user, opts).execute
end
end
end
end
module API module API
class Notes < Grape::API class Notes < Grape::API
include PaginationParams include PaginationParams
helpers ::API::Helpers::NotesHelpers
before { authenticate! } before { authenticate! }
NOTEABLE_TYPES = [Issue, MergeRequest, Snippet].freeze NOTEABLE_TYPES = [Issue, MergeRequest, Snippet].freeze
params do NOTEABLE_TYPES.each do |noteable_type|
requires :id, type: String, desc: 'The ID of a project' parent_type = noteable_type.parent_class.to_s.underscore
end noteables_str = noteable_type.to_s.underscore.pluralize
resource :projects, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do
NOTEABLE_TYPES.each do |noteable_type| params do
requires :id, type: String, desc: "The ID of a #{parent_type}"
end
resource parent_type.pluralize.to_sym, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do
noteables_str = noteable_type.to_s.underscore.pluralize noteables_str = noteable_type.to_s.underscore.pluralize
desc 'Get a list of project +noteable+ notes' do desc "Get a list of #{noteable_type.to_s.downcase} notes" do
success Entities::Note success Entities::Note
end end
params do params do
...@@ -25,7 +29,7 @@ module API ...@@ -25,7 +29,7 @@ module API
use :pagination use :pagination
end end
get ":id/#{noteables_str}/:noteable_id/notes" do get ":id/#{noteables_str}/:noteable_id/notes" do
noteable = find_project_noteable(noteables_str, params[:noteable_id]) noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
if can?(current_user, noteable_read_ability_name(noteable), noteable) if can?(current_user, noteable_read_ability_name(noteable), noteable)
# We exclude notes that are cross-references and that cannot be viewed # We exclude notes that are cross-references and that cannot be viewed
...@@ -46,7 +50,7 @@ module API ...@@ -46,7 +50,7 @@ module API
end end
end end
desc 'Get a single +noteable+ note' do desc "Get a single #{noteable_type.to_s.downcase} note" do
success Entities::Note success Entities::Note
end end
params do params do
...@@ -54,18 +58,11 @@ module API ...@@ -54,18 +58,11 @@ module API
requires :noteable_id, type: Integer, desc: 'The ID of the noteable' requires :noteable_id, type: Integer, desc: 'The ID of the noteable'
end end
get ":id/#{noteables_str}/:noteable_id/notes/:note_id" do get ":id/#{noteables_str}/:noteable_id/notes/:note_id" do
noteable = find_project_noteable(noteables_str, params[:noteable_id]) noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
note = noteable.notes.with_metadata.find(params[:note_id]) get_note(noteable, params[:note_id])
can_read_note = can?(current_user, noteable_read_ability_name(noteable), noteable) && !note.cross_reference_not_visible_for?(current_user)
if can_read_note
present note, with: Entities::Note
else
not_found!("Note")
end
end end
desc 'Create a new +noteable+ note' do desc "Create a new #{noteable_type.to_s.downcase} note" do
success Entities::Note success Entities::Note
end end
params do params do
...@@ -74,34 +71,25 @@ module API ...@@ -74,34 +71,25 @@ module API
optional :created_at, type: String, desc: 'The creation date of the note' optional :created_at, type: String, desc: 'The creation date of the note'
end end
post ":id/#{noteables_str}/:noteable_id/notes" do post ":id/#{noteables_str}/:noteable_id/notes" do
noteable = find_project_noteable(noteables_str, params[:noteable_id]) noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
opts = { opts = {
note: params[:body], note: params[:body],
noteable_type: noteables_str.classify, noteable_type: noteables_str.classify,
noteable_id: noteable.id noteable_id: noteable.id,
created_at: params[:created_at]
} }
if can?(current_user, noteable_read_ability_name(noteable), noteable) note = create_note(noteable, opts)
authorize! :create_note, noteable
if params[:created_at] && (current_user.admin? || user_project.owner == current_user) if note.valid?
opts[:created_at] = params[:created_at] present note, with: Entities.const_get(note.class.name)
end
note = ::Notes::CreateService.new(user_project, current_user, opts).execute
if note.valid?
present note, with: Entities.const_get(note.class.name)
else
not_found!("Note #{note.errors.messages}")
end
else else
not_found!("Note") bad_request!("Note #{note.errors.messages}")
end end
end end
desc 'Update an existing +noteable+ note' do desc "Update an existing #{noteable_type.to_s.downcase} note" do
success Entities::Note success Entities::Note
end end
params do params do
...@@ -110,24 +98,12 @@ module API ...@@ -110,24 +98,12 @@ module API
requires :body, type: String, desc: 'The content of a note' requires :body, type: String, desc: 'The content of a note'
end end
put ":id/#{noteables_str}/:noteable_id/notes/:note_id" do put ":id/#{noteables_str}/:noteable_id/notes/:note_id" do
note = user_project.notes.find(params[:note_id]) noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
authorize! :admin_note, note update_note(noteable, params[:note_id])
opts = {
note: params[:body]
}
note = ::Notes::UpdateService.new(user_project, current_user, opts).execute(note)
if note.valid?
present note, with: Entities::Note
else
render_api_error!("Failed to save note #{note.errors.messages}", 400)
end
end end
desc 'Delete a +noteable+ note' do desc "Delete a #{noteable_type.to_s.downcase} note" do
success Entities::Note success Entities::Note
end end
params do params do
...@@ -135,25 +111,11 @@ module API ...@@ -135,25 +111,11 @@ module API
requires :note_id, type: Integer, desc: 'The ID of a note' requires :note_id, type: Integer, desc: 'The ID of a note'
end end
delete ":id/#{noteables_str}/:noteable_id/notes/:note_id" do delete ":id/#{noteables_str}/:noteable_id/notes/:note_id" do
note = user_project.notes.find(params[:note_id]) noteable = find_noteable(parent_type, noteables_str, params[:noteable_id])
authorize! :admin_note, note
destroy_conditionally!(note) do |note| delete_note(noteable, params[:note_id])
::Notes::DestroyService.new(user_project, current_user).execute(note)
end
end end
end end
end end
helpers do
def find_project_noteable(noteables_str, noteable_id)
public_send("find_project_#{noteables_str.singularize}", noteable_id) # rubocop:disable GitlabSecurity/PublicSend
end
def noteable_read_ability_name(noteable)
"read_#{noteable.class.to_s.underscore}".to_sym
end
end
end end
end end
...@@ -16,6 +16,8 @@ FactoryBot.define do ...@@ -16,6 +16,8 @@ FactoryBot.define do
factory :note_on_personal_snippet, traits: [:on_personal_snippet] factory :note_on_personal_snippet, traits: [:on_personal_snippet]
factory :system_note, traits: [:system] factory :system_note, traits: [:system]
factory :discussion_note, class: DiscussionNote
factory :discussion_note_on_merge_request, traits: [:on_merge_request], class: DiscussionNote do factory :discussion_note_on_merge_request, traits: [:on_merge_request], class: DiscussionNote do
association :project, :repository association :project, :repository
...@@ -31,6 +33,8 @@ FactoryBot.define do ...@@ -31,6 +33,8 @@ FactoryBot.define do
factory :discussion_note_on_personal_snippet, traits: [:on_personal_snippet], class: DiscussionNote factory :discussion_note_on_personal_snippet, traits: [:on_personal_snippet], class: DiscussionNote
factory :discussion_note_on_snippet, traits: [:on_snippet], class: DiscussionNote
factory :legacy_diff_note_on_commit, traits: [:on_commit, :legacy_diff_note], class: LegacyDiffNote factory :legacy_diff_note_on_commit, traits: [:on_commit, :legacy_diff_note], class: LegacyDiffNote
factory :legacy_diff_note_on_merge_request, traits: [:on_merge_request, :legacy_diff_note], class: LegacyDiffNote do factory :legacy_diff_note_on_merge_request, traits: [:on_merge_request, :legacy_diff_note], class: LegacyDiffNote do
...@@ -96,6 +100,10 @@ FactoryBot.define do ...@@ -96,6 +100,10 @@ FactoryBot.define do
noteable { create(:issue, project: project) } noteable { create(:issue, project: project) }
end end
trait :on_snippet do
noteable { create(:snippet, project: project) }
end
trait :on_merge_request do trait :on_merge_request do
noteable { create(:merge_request, source_project: project) } noteable { create(:merge_request, source_project: project) }
end end
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
"type": "object", "type": "object",
"properties" : { "properties" : {
"id": { "type": "integer" }, "id": { "type": "integer" },
"type": { "type": ["string", "null"] },
"body": { "type": "string" }, "body": { "type": "string" },
"attachment": { "type": ["string", "null"] }, "attachment": { "type": ["string", "null"] },
"author": { "author": {
......
require 'spec_helper'
describe API::Discussions do
let(:user) { create(:user) }
let!(:project) { create(:project, :public, namespace: user.namespace) }
let(:private_user) { create(:user) }
before do
project.add_reporter(user)
end
context "when noteable is an Issue" do
let!(:issue) { create(:issue, project: project, author: user) }
let!(:issue_note) { create(:discussion_note_on_issue, noteable: issue, project: project, author: user) }
it_behaves_like "discussions API", 'projects', 'issues', 'iid' do
let(:parent) { project }
let(:noteable) { issue }
let(:note) { issue_note }
end
end
context "when noteable is a Snippet" do
let!(:snippet) { create(:project_snippet, project: project, author: user) }
let!(:snippet_note) { create(:discussion_note_on_snippet, noteable: snippet, project: project, author: user) }
it_behaves_like "discussions API", 'projects', 'snippets', 'id' do
let(:parent) { project }
let(:noteable) { snippet }
let(:note) { snippet_note }
end
end
end
This diff is collapsed.
shared_examples 'discussions API' do |parent_type, noteable_type, id_name|
describe "GET /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions" do
it "returns an array of discussions" do
get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", user)
expect(response).to have_gitlab_http_status(200)
expect(response).to include_pagination_headers
expect(json_response).to be_an Array
expect(json_response.first['id']).to eq(note.discussion_id)
end
it "returns a 404 error when noteable id not found" do
get api("/#{parent_type}/#{parent.id}/#{noteable_type}/12345/discussions", user)
expect(response).to have_gitlab_http_status(404)
end
it "returns 404 when not authorized" do
parent.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", private_user)
expect(response).to have_gitlab_http_status(404)
end
end
describe "GET /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id" do
it "returns a discussion by id" do
get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions/#{note.discussion_id}", user)
expect(response).to have_gitlab_http_status(200)
expect(json_response['id']).to eq(note.discussion_id)
expect(json_response['notes'].first['body']).to eq(note.note)
end
it "returns a 404 error if discussion not found" do
get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions/12345", user)
expect(response).to have_gitlab_http_status(404)
end
end
describe "POST /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions" do
it "creates a new note" do
post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", user), body: 'hi!'
expect(response).to have_gitlab_http_status(201)
expect(json_response['notes'].first['body']).to eq('hi!')
expect(json_response['notes'].first['author']['username']).to eq(user.username)
end
it "returns a 400 bad request error if body not given" do
post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", user)
expect(response).to have_gitlab_http_status(400)
end
it "returns a 401 unauthorized error if user not authenticated" do
post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions"), body: 'hi!'
expect(response).to have_gitlab_http_status(401)
end
context 'when an admin or owner makes the request' do
it 'accepts the creation date to be set' do
creation_time = 2.weeks.ago
post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", user),
body: 'hi!', created_at: creation_time
expect(response).to have_gitlab_http_status(201)
expect(json_response['notes'].first['body']).to eq('hi!')
expect(json_response['notes'].first['author']['username']).to eq(user.username)
expect(Time.parse(json_response['notes'].first['created_at'])).to be_like_time(creation_time)
end
end
context 'when user does not have access to read the discussion' do
before do
parent.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
end
it 'responds with 404' do
post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/discussions", private_user),
body: 'Foo'
expect(response).to have_gitlab_http_status(404)
end
end
end
describe "POST /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id/notes" do
it 'adds a new note to the discussion' do
post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
"discussions/#{note.discussion_id}/notes", user), body: 'Hello!'
expect(response).to have_gitlab_http_status(201)
expect(json_response['body']).to eq('Hello!')
expect(json_response['type']).to eq('DiscussionNote')
end
it 'returns a 400 bad request error if body not given' do
post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
"discussions/#{note.discussion_id}/notes", user)
expect(response).to have_gitlab_http_status(400)
end
it "returns a 400 bad request error if discussion is individual note" do
note.update_attribute(:type, nil)
post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
"discussions/#{note.discussion_id}/notes", user), body: 'hi!'
expect(response).to have_gitlab_http_status(400)
end
end
describe "PUT /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id/notes/:note_id" do
it 'returns modified note' do
put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
"discussions/#{note.discussion_id}/notes/#{note.id}", user), body: 'Hello!'
expect(response).to have_gitlab_http_status(200)
expect(json_response['body']).to eq('Hello!')
end
it 'returns a 404 error when note id not found' do
put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
"discussions/#{note.discussion_id}/notes/12345", user),
body: 'Hello!'
expect(response).to have_gitlab_http_status(404)
end
it 'returns a 400 bad request error if body not given' do
put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
"discussions/#{note.discussion_id}/notes/#{note.id}", user)
expect(response).to have_gitlab_http_status(400)
end
end
describe "DELETE /#{parent_type}/:id/#{noteable_type}/:noteable_id/discussions/:discussion_id/notes/:note_id" do
it 'deletes a note' do
delete api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
"discussions/#{note.discussion_id}/notes/#{note.id}", user)
expect(response).to have_gitlab_http_status(204)
# Check if note is really deleted
delete api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
"discussions/#{note.discussion_id}/notes/#{note.id}", user)
expect(response).to have_gitlab_http_status(404)
end
it 'returns a 404 error when note id not found' do
delete api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
"discussions/#{note.discussion_id}/notes/12345", user)
expect(response).to have_gitlab_http_status(404)
end
it_behaves_like '412 response' do
let(:request) do
api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
"discussions/#{note.discussion_id}/notes/#{note.id}", user)
end
end
end
end
shared_examples 'noteable API' do |parent_type, noteable_type, id_name|
describe "GET /#{parent_type}/:id/#{noteable_type}/:noteable_id/notes" do
context 'sorting' do
before do
params = { noteable: noteable, author: user }
params[:project] = parent if parent.is_a?(Project)
create_list(:note, 3, params)
end
it 'sorts by created_at in descending order by default' do
get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user)
response_dates = json_response.map { |note| note['created_at'] }
expect(json_response.length).to eq(4)
expect(response_dates).to eq(response_dates.sort.reverse)
end
it 'sorts by ascending order when requested' do
get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes?sort=asc", user)
response_dates = json_response.map { |note| note['created_at'] }
expect(json_response.length).to eq(4)
expect(response_dates).to eq(response_dates.sort)
end
it 'sorts by updated_at in descending order when requested' do
get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes?order_by=updated_at", user)
response_dates = json_response.map { |note| note['updated_at'] }
expect(json_response.length).to eq(4)
expect(response_dates).to eq(response_dates.sort.reverse)
end
it 'sorts by updated_at in ascending order when requested' do
get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes?order_by=updated_at&sort=asc", user)
response_dates = json_response.map { |note| note['updated_at'] }
expect(json_response.length).to eq(4)
expect(response_dates).to eq(response_dates.sort)
end
end
it "returns an array of notes" do
get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user)
expect(response).to have_gitlab_http_status(200)
expect(response).to include_pagination_headers
expect(json_response).to be_an Array
expect(json_response.first['body']).to eq(note.note)
end
it "returns a 404 error when noteable id not found" do
get api("/#{parent_type}/#{parent.id}/#{noteable_type}/12345/notes", user)
expect(response).to have_gitlab_http_status(404)
end
it "returns 404 when not authorized" do
parent.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", private_user)
expect(response).to have_gitlab_http_status(404)
end
end
describe "GET /#{parent_type}/:id/#{noteable_type}/:noteable_id/notes/:note_id" do
it "returns a note by id" do
get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes/#{note.id}", user)
expect(response).to have_gitlab_http_status(200)
expect(json_response['body']).to eq(note.note)
end
it "returns a 404 error if note not found" do
get api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes/12345", user)
expect(response).to have_gitlab_http_status(404)
end
end
describe "POST /#{parent_type}/:id/#{noteable_type}/:noteable_id/notes" do
it "creates a new note" do
post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user), body: 'hi!'
expect(response).to have_gitlab_http_status(201)
expect(json_response['body']).to eq('hi!')
expect(json_response['author']['username']).to eq(user.username)
end
it "returns a 400 bad request error if body not given" do
post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user)
expect(response).to have_gitlab_http_status(400)
end
it "returns a 401 unauthorized error if user not authenticated" do
post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes"), body: 'hi!'
expect(response).to have_gitlab_http_status(401)
end
it "creates an activity event when a note is created" do
expect(Event).to receive(:create!)
post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user), body: 'hi!'
end
context 'when an admin or owner makes the request' do
it 'accepts the creation date to be set' do
creation_time = 2.weeks.ago
post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user),
body: 'hi!', created_at: creation_time
expect(response).to have_gitlab_http_status(201)
expect(json_response['body']).to eq('hi!')
expect(json_response['author']['username']).to eq(user.username)
expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time)
end
end
context 'when the user is posting an award emoji on a noteable created by someone else' do
it 'creates a new note' do
parent.add_developer(private_user)
post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", private_user), body: ':+1:'
expect(response).to have_gitlab_http_status(201)
expect(json_response['body']).to eq(':+1:')
end
end
context 'when the user is posting an award emoji on his/her own noteable' do
it 'creates a new note' do
post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", user), body: ':+1:'
expect(response).to have_gitlab_http_status(201)
expect(json_response['body']).to eq(':+1:')
end
end
context 'when user does not have access to read the noteable' do
before do
parent.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
end
it 'responds with 404' do
post api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes", private_user),
body: 'Foo'
expect(response).to have_gitlab_http_status(404)
end
end
end
describe "PUT /#{parent_type}/:id/#{noteable_type}/:noteable_id/notes/:note_id" do
it 'returns modified note' do
put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
"notes/#{note.id}", user), body: 'Hello!'
expect(response).to have_gitlab_http_status(200)
expect(json_response['body']).to eq('Hello!')
end
it 'returns a 404 error when note id not found' do
put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes/12345", user),
body: 'Hello!'
expect(response).to have_gitlab_http_status(404)
end
it 'returns a 400 bad request error if body not given' do
put api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
"notes/#{note.id}", user)
expect(response).to have_gitlab_http_status(400)
end
end
describe "DELETE /#{parent_type}/:id/#{noteable_type}/:noteable_id/notes/:note_id" do
it 'deletes a note' do
delete api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
"notes/#{note.id}", user)
expect(response).to have_gitlab_http_status(204)
# Check if note is really deleted
delete api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/"\
"notes/#{note.id}", user)
expect(response).to have_gitlab_http_status(404)
end
it 'returns a 404 error when note id not found' do
delete api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes/12345", user)
expect(response).to have_gitlab_http_status(404)
end
it_behaves_like '412 response' do
let(:request) { api("/#{parent_type}/#{parent.id}/#{noteable_type}/#{noteable[id_name]}/notes/#{note.id}", user) }
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