diff options
| -rw-r--r-- | app/models/concerns/issuable.rb | 4 | ||||
| -rw-r--r-- | app/models/note.rb | 9 | ||||
| -rw-r--r-- | app/models/snippet.rb | 4 | ||||
| -rw-r--r-- | app/services/notes/build_service.rb | 9 | ||||
| -rw-r--r-- | app/services/notes/post_process_service.rb | 2 | ||||
| -rw-r--r-- | app/services/notification_recipient_service.rb | 2 | ||||
| -rw-r--r-- | app/services/todo_service.rb | 3 | ||||
| -rw-r--r-- | changelogs/unreleased/discussions-api.yml | 5 | ||||
| -rw-r--r-- | doc/api/README.md | 1 | ||||
| -rw-r--r-- | doc/api/discussions.md | 411 | ||||
| -rw-r--r-- | doc/api/notes.md | 77 | ||||
| -rw-r--r-- | lib/api/api.rb | 1 | ||||
| -rw-r--r-- | lib/api/discussions.rb | 195 | ||||
| -rw-r--r-- | lib/api/entities.rb | 7 | ||||
| -rw-r--r-- | lib/api/helpers/notes_helpers.rb | 76 | ||||
| -rw-r--r-- | lib/api/notes.rb | 94 | ||||
| -rw-r--r-- | spec/factories/notes.rb | 8 | ||||
| -rw-r--r-- | spec/fixtures/api/schemas/public_api/v4/notes.json | 1 | ||||
| -rw-r--r-- | spec/requests/api/discussions_spec.rb | 33 | ||||
| -rw-r--r-- | spec/requests/api/notes_spec.rb | 590 | ||||
| -rw-r--r-- | spec/support/shared_examples/requests/api/discussions.rb | 169 | ||||
| -rw-r--r-- | spec/support/shared_examples/requests/api/notes.rb | 206 |
22 files changed, 1311 insertions, 596 deletions
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 7049f340c9d..52c85c7c22f 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -222,6 +222,10 @@ module Issuable def to_ability_name model_name.singular end + + def parent_class + ::Project + end end def today? diff --git a/app/models/note.rb b/app/models/note.rb index d7a67ec277c..787a80f0196 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -81,7 +81,7 @@ class Note < ActiveRecord::Base validates :author, presence: true 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 errors.add(:project, 'does not match noteable project') end @@ -228,7 +228,7 @@ class Note < ActiveRecord::Base end def skip_project_check? - for_personal_snippet? + !for_project_noteable? end def commit @@ -308,6 +308,11 @@ class Note < ActiveRecord::Base self.noteable.supports_discussions? && !part_of_discussion? end + def can_create_todo? + # Skip system notes, and notes on project snippet + !system? && !for_snippet? + end + def discussion_class(noteable = nil) # When commit notes are rendered on an MR's Discussion page, they are # displayed in one discussion instead of individually. diff --git a/app/models/snippet.rb b/app/models/snippet.rb index a58c208279e..644120453cf 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -168,5 +168,9 @@ class Snippet < ActiveRecord::Base def search_code(query) fuzzy_search(query, [:content]) end + + def parent_class + ::Project + end end end diff --git a/app/services/notes/build_service.rb b/app/services/notes/build_service.rb index abf25bb778b..77e7b8a5ea7 100644 --- a/app/services/notes/build_service.rb +++ b/app/services/notes/build_service.rb @@ -26,14 +26,19 @@ module Notes if project project.notes.find_discussion(discussion_id) else - # only PersonalSnippets can have discussions without project association discussion = Note.find_discussion(discussion_id) 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 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 diff --git a/app/services/notes/post_process_service.rb b/app/services/notes/post_process_service.rb index 6a10e172483..ad3dcc5010b 100644 --- a/app/services/notes/post_process_service.rb +++ b/app/services/notes/post_process_service.rb @@ -11,7 +11,7 @@ module Notes unless @note.system? EventCreateService.new.leave_note(@note, @note.author) - return if @note.for_personal_snippet? + return unless @note.for_project_noteable? @note.create_cross_references! execute_note_hooks diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb index 6835b14648b..e4be953e810 100644 --- a/app/services/notification_recipient_service.rb +++ b/app/services/notification_recipient_service.rb @@ -280,7 +280,7 @@ module NotificationRecipientService add_participants(note.author) add_mentions(note.author, target: note) - unless note.for_personal_snippet? + if note.for_project_noteable? # Merge project watchers add_project_watchers diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index c2ca404b179..ffd48e842c2 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -241,8 +241,7 @@ class TodoService end def handle_note(note, author, skip_users = []) - # Skip system notes, and notes on project snippet - return if note.system? || note.for_snippet? + return unless note.can_create_todo? project = note.project target = note.noteable diff --git a/changelogs/unreleased/discussions-api.yml b/changelogs/unreleased/discussions-api.yml new file mode 100644 index 00000000000..110df3aa414 --- /dev/null +++ b/changelogs/unreleased/discussions-api.yml @@ -0,0 +1,5 @@ +--- +title: Add discussions API for Issues and Snippets +merge_request: +author: +type: added diff --git a/doc/api/README.md b/doc/api/README.md index b193ef4ab7f..a08baf86ef1 100644 --- a/doc/api/README.md +++ b/doc/api/README.md @@ -35,6 +35,7 @@ following locations: - [Group milestones](group_milestones.md) - [Namespaces](namespaces.md) - [Notes](notes.md) (comments) +- [Threaded comments](discussions.md) - [Notification settings](notification_settings.md) - [Open source license templates](templates/licenses.md) - [Pages Domains](pages_domains.md) diff --git a/doc/api/discussions.md b/doc/api/discussions.md new file mode 100644 index 00000000000..07837d7d4c3 --- /dev/null +++ b/doc/api/discussions.md @@ -0,0 +1,411 @@ +# Discussions API + +Discussions are set of related notes on snippets, issues or epics. + +## Issues + +### List project issue discussions + +Gets a list of all discussions for a single issue. + +``` +GET /projects/:id/issues/:issue_iid/discussions +``` + +| Attribute | Type | Required | Description | +| ------------------- | ---------------- | ---------- | ------------ | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | +| `issue_iid` | integer | yes | The IID of an issue | + +```json +[ + { + "id": "6a9c1750b37d513a43987b574953fceb50b03ce7", + "individual_note": false, + "notes": [ + { + "id": 1126, + "type": "DiscussionNote", + "body": "discussion text", + "attachment": null, + "author": { + "id": 1, + "name": "root", + "username": "root", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/00afb8fb6ab07c3ee3e9c1f38777e2f4?s=80&d=identicon", + "web_url": "http://localhost:3000/root" + }, + "created_at": "2018-03-03T21:54:39.668Z", + "updated_at": "2018-03-03T21:54:39.668Z", + "system": false, + "noteable_id": 3, + "noteable_type": "Issue", + "noteable_iid": null + }, + { + "id": 1129, + "type": "DiscussionNote", + "body": "reply to the discussion", + "attachment": null, + "author": { + "id": 1, + "name": "root", + "username": "root", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/00afb8fb6ab07c3ee3e9c1f38777e2f4?s=80&d=identicon", + "web_url": "http://localhost:3000/root" + }, + "created_at": "2018-03-04T13:38:02.127Z", + "updated_at": "2018-03-04T13:38:02.127Z", + "system": false, + "noteable_id": 3, + "noteable_type": "Issue", + "noteable_iid": null + } + ] + }, + { + "id": "87805b7c09016a7058e91bdbe7b29d1f284a39e6", + "individual_note": true, + "notes": [ + { + "id": 1128, + "type": null, + "body": "a single comment", + "attachment": null, + "author": { + "id": 1, + "name": "root", + "username": "root", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/00afb8fb6ab07c3ee3e9c1f38777e2f4?s=80&d=identicon", + "web_url": "http://localhost:3000/root" + }, + "created_at": "2018-03-04T09:17:22.520Z", + "updated_at": "2018-03-04T09:17:22.520Z", + "system": false, + "noteable_id": 3, + "noteable_type": "Issue", + "noteable_iid": null + } + ] + } +] +``` + +```bash +curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/11/discussions +``` + +### Get single issue discussion + +Returns a single discussion for a specific project issue + +``` +GET /projects/:id/issues/:issue_iid/discussions/:discussion_id +``` + +Parameters: + +| Attribute | Type | Required | Description | +| --------------- | -------------- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | +| `issue_iid` | integer | yes | The IID of an issue | +| `discussion_id` | integer | yes | The ID of a discussion | + +```bash +curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/11/discussions/6a9c1750b37d513a43987b574953fceb50b03ce7 +``` + +### Create new issue discussion + +Creates a new discussion to a single project issue. This is similar to creating +a note but but another comments (replies) can be added to it later. + +``` +POST /projects/:id/issues/:issue_iid/discussions +``` + +Parameters: + +| Attribute | Type | Required | Description | +| --------------- | -------------- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | +| `issue_iid` | integer | yes | The IID of an issue | +| `body` | string | yes | The content of a discussion | +| `created_at` | string | no | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z | + +```bash +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/11/discussions?body=comment +``` + +### Add note to existing issue discussion + +Adds a new note to the discussion. + +``` +POST /projects/:id/issues/:issue_iid/discussions/:discussion_id/notes +``` + +Parameters: + +| Attribute | Type | Required | Description | +| --------------- | -------------- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | +| `issue_iid` | integer | yes | The IID of an issue | +| `discussion_id` | integer | yes | The ID of a discussion | +| `note_id` | integer | yes | The ID of a discussion note | +| `body` | string | yes | The content of a discussion | +| `created_at` | string | no | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z | + +```bash +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/11/discussions/6a9c1750b37d513a43987b574953fceb50b03ce7/notes?body=comment +``` + +### Modify existing issue discussion note + +Modify existing discussion note of an issue. + +``` +PUT /projects/:id/issues/:issue_iid/discussions/:discussion_id/notes/:note_id +``` + +Parameters: + +| Attribute | Type | Required | Description | +| --------------- | -------------- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | +| `issue_iid` | integer | yes | The IID of an issue | +| `discussion_id` | integer | yes | The ID of a discussion | +| `note_id` | integer | yes | The ID of a discussion note | +| `body` | string | yes | The content of a discussion | + +```bash +curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/11/discussions/6a9c1750b37d513a43987b574953fceb50b03ce7/notes/1108?body=comment +``` + +### Delete an issue discussion note + +Deletes an existing discussion note of an issue. + +``` +DELETE /projects/:id/issues/:issue_iid/discussions/:discussion_id/notes/:note_id +``` + +Parameters: + +| Attribute | Type | Required | Description | +| --------------- | -------------- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | +| `issue_iid` | integer | yes | The IID of an issue | +| `discussion_id` | integer | yes | The ID of a discussion | +| `note_id` | integer | yes | The ID of a discussion note | + +```bash +curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/11/discussions/636 +``` + +## Snippets + +### List project snippet discussions + +Gets a list of all discussions for a single snippet. + +``` +GET /projects/:id/snippets/:snippet_id/discussions +``` + +| Attribute | Type | Required | Description | +| ------------------- | ---------------- | ---------- | ------------| +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | +| `snippet_id` | integer | yes | The ID of an snippet | + +```json +[ + { + "id": "6a9c1750b37d513a43987b574953fceb50b03ce7", + "individual_note": false, + "notes": [ + { + "id": 1126, + "type": "DiscussionNote", + "body": "discussion text", + "attachment": null, + "author": { + "id": 1, + "name": "root", + "username": "root", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/00afb8fb6ab07c3ee3e9c1f38777e2f4?s=80&d=identicon", + "web_url": "http://localhost:3000/root" + }, + "created_at": "2018-03-03T21:54:39.668Z", + "updated_at": "2018-03-03T21:54:39.668Z", + "system": false, + "noteable_id": 3, + "noteable_type": "Snippet", + "noteable_id": null + }, + { + "id": 1129, + "type": "DiscussionNote", + "body": "reply to the discussion", + "attachment": null, + "author": { + "id": 1, + "name": "root", + "username": "root", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/00afb8fb6ab07c3ee3e9c1f38777e2f4?s=80&d=identicon", + "web_url": "http://localhost:3000/root" + }, + "created_at": "2018-03-04T13:38:02.127Z", + "updated_at": "2018-03-04T13:38:02.127Z", + "system": false, + "noteable_id": 3, + "noteable_type": "Snippet", + "noteable_id": null + } + ] + }, + { + "id": "87805b7c09016a7058e91bdbe7b29d1f284a39e6", + "individual_note": true, + "notes": [ + { + "id": 1128, + "type": null, + "body": "a single comment", + "attachment": null, + "author": { + "id": 1, + "name": "root", + "username": "root", + "state": "active", + "avatar_url": "https://www.gravatar.com/avatar/00afb8fb6ab07c3ee3e9c1f38777e2f4?s=80&d=identicon", + "web_url": "http://localhost:3000/root" + }, + "created_at": "2018-03-04T09:17:22.520Z", + "updated_at": "2018-03-04T09:17:22.520Z", + "system": false, + "noteable_id": 3, + "noteable_type": "Snippet", + "noteable_id": null + } + ] + } +] +``` + +```bash +curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/snippets/11/discussions +``` + +### Get single snippet discussion + +Returns a single discussion for a specific project snippet + +``` +GET /projects/:id/snippets/:snippet_id/discussions/:discussion_id +``` + +Parameters: + +| Attribute | Type | Required | Description | +| --------------- | -------------- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | +| `snippet_id` | integer | yes | The ID of an snippet | +| `discussion_id` | integer | yes | The ID of a discussion | + +```bash +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/snippets/11/discussions/6a9c1750b37d513a43987b574953fceb50b03ce7 +``` + +### Create new snippet discussion + +Creates a new discussion to a single project snippet. This is similar to creating +a note but but another comments (replies) can be added to it later. + +``` +POST /projects/:id/snippets/:snippet_id/discussions +``` + +Parameters: + +| Attribute | Type | Required | Description | +| --------------- | -------------- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | +| `snippet_id` | integer | yes | The ID of an snippet | +| `body` | string | yes | The content of a discussion | +| `created_at` | string | no | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z | + +```bash +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/snippets/11/discussions?body=comment +``` + +### Add note to existing snippet discussion + +Adds a new note to the discussion. + +``` +POST /projects/:id/snippets/:snippet_id/discussions/:discussion_id/notes +``` + +Parameters: + +| Attribute | Type | Required | Description | +| --------------- | -------------- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | +| `snippet_id` | integer | yes | The ID of an snippet | +| `discussion_id` | integer | yes | The ID of a discussion | +| `note_id` | integer | yes | The ID of a discussion note | +| `body` | string | yes | The content of a discussion | +| `created_at` | string | no | Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z | + +```bash +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/snippets/11/discussions/6a9c1750b37d513a43987b574953fceb50b03ce7/notes?body=comment +``` + +### Modify existing snippet discussion note + +Modify existing discussion note of an snippet. + +``` +PUT /projects/:id/snippets/:snippet_id/discussions/:discussion_id/notes/:note_id +``` + +Parameters: + +| Attribute | Type | Required | Description | +| --------------- | -------------- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | +| `snippet_id` | integer | yes | The ID of an snippet | +| `discussion_id` | integer | yes | The ID of a discussion | +| `note_id` | integer | yes | The ID of a discussion note | +| `body` | string | yes | The content of a discussion | + +```bash +curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/snippets/11/discussions/6a9c1750b37d513a43987b574953fceb50b03ce7/notes/1108?body=comment +``` + +### Delete an snippet discussion note + +Deletes an existing discussion note of an snippet. + +``` +DELETE /projects/:id/snippets/:snippet_id/discussions/:discussion_id/notes/:note_id +``` + +Parameters: + +| Attribute | Type | Required | Description | +| --------------- | -------------- | -------- | ----------- | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | +| `snippet_id` | integer | yes | The ID of an snippet | +| `discussion_id` | integer | yes | The ID of a discussion | +| `note_id` | integer | yes | The ID of a discussion note | + +```bash +curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/snippets/11/discussions/636 +``` diff --git a/doc/api/notes.md b/doc/api/notes.md index 1b68bd99ce2..aa38d22845c 100644 --- a/doc/api/notes.md +++ b/doc/api/notes.md @@ -15,7 +15,7 @@ GET /projects/:id/issues/:issue_iid/notes?sort=asc&order_by=updated_at | Attribute | Type | Required | Description | | ------------------- | ---------------- | ---------- | --------------------------------------------------------------------------------------------------------------------------------------------------- | -| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | `issue_iid` | integer | yes | The IID of an issue | `sort` | string | no | Return issue notes sorted in `asc` or `desc` order. Default is `desc` | `order_by` | string | no | Return issue notes ordered by `created_at` or `updated_at` fields. Default is `created_at` @@ -63,6 +63,10 @@ GET /projects/:id/issues/:issue_iid/notes?sort=asc&order_by=updated_at ] ``` +```bash +curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/11/notes +``` + ### Get single issue note Returns a single note for a specific project issue @@ -73,14 +77,17 @@ GET /projects/:id/issues/:issue_iid/notes/:note_id Parameters: -- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user +- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) - `issue_iid` (required) - The IID of a project issue - `note_id` (required) - The ID of an issue note +```bash +curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/11/notes/1 +``` + ### Create new issue note -Creates a new note to a single project issue. If you create a note where the body -only contains an Award Emoji, you'll receive this object back. +Creates a new note to a single project issue. ``` POST /projects/:id/issues/:issue_iid/notes @@ -88,11 +95,15 @@ POST /projects/:id/issues/:issue_iid/notes Parameters: -- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user +- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) - `issue_id` (required) - The IID of an issue - `body` (required) - The content of a note - `created_at` (optional) - Date time string, ISO 8601 formatted, e.g. 2016-03-11T03:45:40Z +```bash +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/11/notes?body=note +``` + ### Modify existing issue note Modify existing note of an issue. @@ -103,11 +114,15 @@ PUT /projects/:id/issues/:issue_iid/notes/:note_id Parameters: -- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user +- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) - `issue_iid` (required) - The IID of an issue - `note_id` (required) - The ID of a note - `body` (required) - The content of a note +```bash +curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/11/notes?body=note +``` + ### Delete an issue note Deletes an existing note of an issue. @@ -120,7 +135,7 @@ Parameters: | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | | `issue_iid` | integer | yes | The IID of an issue | | `note_id` | integer | yes | The ID of a note | @@ -141,11 +156,15 @@ GET /projects/:id/snippets/:snippet_id/notes?sort=asc&order_by=updated_at | Attribute | Type | Required | Description | | ------------------- | ---------------- | ---------- | --------------------------------------------------------------------------------------------------------------------------------------------------- | -| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | `snippet_id` | integer | yes | The ID of a project snippet | `sort` | string | no | Return snippet notes sorted in `asc` or `desc` order. Default is `desc` | `order_by` | string | no | Return snippet notes ordered by `created_at` or `updated_at` fields. Default is `created_at` +```bash +curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/snippets/11/notes +``` + ### Get single snippet note Returns a single note for a given snippet. @@ -156,7 +175,7 @@ GET /projects/:id/snippets/:snippet_id/notes/:note_id Parameters: -- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user +- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) - `snippet_id` (required) - The ID of a project snippet - `note_id` (required) - The ID of a snippet note @@ -179,6 +198,10 @@ Parameters: } ``` +```bash +curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/snippets/11/notes/11 +``` + ### Create new snippet note Creates a new note for a single snippet. Snippet notes are comments users can post to a snippet. @@ -190,10 +213,14 @@ POST /projects/:id/snippets/:snippet_id/notes Parameters: -- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user +- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) - `snippet_id` (required) - The ID of a snippet - `body` (required) - The content of a note +```bash +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/snippet/11/notes?body=note +``` + ### Modify existing snippet note Modify existing note of a snippet. @@ -204,11 +231,15 @@ PUT /projects/:id/snippets/:snippet_id/notes/:note_id Parameters: -- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user +- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) - `snippet_id` (required) - The ID of a snippet - `note_id` (required) - The ID of a note - `body` (required) - The content of a note +```bash +curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/snippets/11/notes?body=note +``` + ### Delete a snippet note Deletes an existing note of a snippet. @@ -221,7 +252,7 @@ Parameters: | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | | `snippet_id` | integer | yes | The ID of a snippet | | `note_id` | integer | yes | The ID of a note | @@ -242,11 +273,15 @@ GET /projects/:id/merge_requests/:merge_request_iid/notes?sort=asc&order_by=upda | Attribute | Type | Required | Description | | ------------------- | ---------------- | ---------- | --------------------------------------------------------------------------------------------------------------------------------------------------- | -| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | `merge_request_iid` | integer | yes | The IID of a project merge request | `sort` | string | no | Return merge request notes sorted in `asc` or `desc` order. Default is `desc` | `order_by` | string | no | Return merge request notes ordered by `created_at` or `updated_at` fields. Default is `created_at` +```bash +curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/merge_requests/11/notes +``` + ### Get single merge request note Returns a single note for a given merge request. @@ -257,7 +292,7 @@ GET /projects/:id/merge_requests/:merge_request_iid/notes/:note_id Parameters: -- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user +- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) - `merge_request_iid` (required) - The IID of a project merge request - `note_id` (required) - The ID of a merge request note @@ -283,6 +318,10 @@ Parameters: } ``` +```bash +curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/merge_requests/11/notes/1 +``` + ### Create new merge request note Creates a new note for a single merge request. @@ -295,7 +334,7 @@ POST /projects/:id/merge_requests/:merge_request_iid/notes Parameters: -- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user +- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) - `merge_request_iid` (required) - The IID of a merge request - `body` (required) - The content of a note @@ -309,11 +348,15 @@ PUT /projects/:id/merge_requests/:merge_request_iid/notes/:note_id Parameters: -- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user +- `id` (required) - The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) - `merge_request_iid` (required) - The IID of a merge request - `note_id` (required) - The ID of a note - `body` (required) - The content of a note +```bash +curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/merge_requests/11/notes?body=note +``` + ### Delete a merge request note Deletes an existing note of a merge request. @@ -326,7 +369,7 @@ Parameters: | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | | `merge_request_iid` | integer | yes | The IID of a merge request | | `note_id` | integer | yes | The ID of a note | diff --git a/lib/api/api.rb b/lib/api/api.rb index 754549f72f0..8d8332c889a 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -134,6 +134,7 @@ module API mount ::API::MergeRequests mount ::API::Namespaces mount ::API::Notes + mount ::API::Discussions mount ::API::NotificationSettings mount ::API::PagesDomains mount ::API::Pipelines diff --git a/lib/api/discussions.rb b/lib/api/discussions.rb new file mode 100644 index 00000000000..6abd575b6ad --- /dev/null +++ b/lib/api/discussions.rb @@ -0,0 +1,195 @@ +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 diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 0c8ec7dd5f5..aa96033898b 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -629,6 +629,7 @@ module API NOTEABLE_TYPES_WITH_IID = %w(Issue MergeRequest).freeze expose :id + expose :type expose :note, as: :body expose :attachment_identifier, as: :attachment expose :author, using: Entities::UserBasic @@ -640,6 +641,12 @@ module API expose(:noteable_iid) { |note| note.noteable.iid if NOTEABLE_TYPES_WITH_IID.include?(note.noteable_type) } end + class Discussion < Grape::Entity + expose :id + expose :individual_note?, as: :individual_note + expose :notes, using: Entities::Note + end + class AwardEmoji < Grape::Entity expose :id expose :name diff --git a/lib/api/helpers/notes_helpers.rb b/lib/api/helpers/notes_helpers.rb new file mode 100644 index 00000000000..cd91df1ecd8 --- /dev/null +++ b/lib/api/helpers/notes_helpers.rb @@ -0,0 +1,76 @@ +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 diff --git a/lib/api/notes.rb b/lib/api/notes.rb index 3588dc85c9e..69f1df6b341 100644 --- a/lib/api/notes.rb +++ b/lib/api/notes.rb @@ -1,19 +1,23 @@ module API class Notes < Grape::API include PaginationParams + helpers ::API::Helpers::NotesHelpers before { authenticate! } NOTEABLE_TYPES = [Issue, MergeRequest, Snippet].freeze - params do - requires :id, type: String, desc: 'The ID of a project' - end - resource :projects, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do - NOTEABLE_TYPES.each do |noteable_type| + 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 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 end params do @@ -25,7 +29,7 @@ module API use :pagination end 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) # We exclude notes that are cross-references and that cannot be viewed @@ -46,7 +50,7 @@ module API end end - desc 'Get a single +noteable+ note' do + desc "Get a single #{noteable_type.to_s.downcase} note" do success Entities::Note end params do @@ -54,18 +58,11 @@ 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_project_noteable(noteables_str, params[:noteable_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 + noteable = find_noteable(parent_type, noteables_str, params[:noteable_id]) + get_note(noteable, params[:note_id]) end - desc 'Create a new +noteable+ note' do + desc "Create a new #{noteable_type.to_s.downcase} note" do success Entities::Note end params do @@ -74,34 +71,25 @@ 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_project_noteable(noteables_str, params[:noteable_id]) + noteable = find_noteable(parent_type, noteables_str, params[:noteable_id]) opts = { note: params[:body], 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) - authorize! :create_note, noteable + note = create_note(noteable, opts) - if params[:created_at] && (current_user.admin? || user_project.owner == current_user) - opts[:created_at] = params[:created_at] - 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 + if note.valid? + present note, with: Entities.const_get(note.class.name) else - not_found!("Note") + bad_request!("Note #{note.errors.messages}") end end - desc 'Update an existing +noteable+ note' do + desc "Update an existing #{noteable_type.to_s.downcase} note" do success Entities::Note end params do @@ -110,24 +98,12 @@ module API requires :body, type: String, desc: 'The content of a note' end 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 - - 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 + update_note(noteable, params[:note_id]) end - desc 'Delete a +noteable+ note' do + desc "Delete a #{noteable_type.to_s.downcase} note" do success Entities::Note end params do @@ -135,25 +111,11 @@ module API requires :note_id, type: Integer, desc: 'The ID of a note' end delete ":id/#{noteables_str}/:noteable_id/notes/:note_id" do - note = user_project.notes.find(params[:note_id]) - - authorize! :admin_note, note + noteable = find_noteable(parent_type, noteables_str, params[:noteable_id]) - destroy_conditionally!(note) do |note| - ::Notes::DestroyService.new(user_project, current_user).execute(note) - end + delete_note(noteable, params[:note_id]) 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 diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb index 3f4e408b3a6..857333f222d 100644 --- a/spec/factories/notes.rb +++ b/spec/factories/notes.rb @@ -16,6 +16,8 @@ FactoryBot.define do factory :note_on_personal_snippet, traits: [:on_personal_snippet] factory :system_note, traits: [:system] + factory :discussion_note, class: DiscussionNote + factory :discussion_note_on_merge_request, traits: [:on_merge_request], class: DiscussionNote do association :project, :repository @@ -31,6 +33,8 @@ FactoryBot.define do 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_merge_request, traits: [:on_merge_request, :legacy_diff_note], class: LegacyDiffNote do @@ -96,6 +100,10 @@ FactoryBot.define do noteable { create(:issue, project: project) } end + trait :on_snippet do + noteable { create(:snippet, project: project) } + end + trait :on_merge_request do noteable { create(:merge_request, source_project: project) } end diff --git a/spec/fixtures/api/schemas/public_api/v4/notes.json b/spec/fixtures/api/schemas/public_api/v4/notes.json index 6525f7c2c80..4c4ca3b582f 100644 --- a/spec/fixtures/api/schemas/public_api/v4/notes.json +++ b/spec/fixtures/api/schemas/public_api/v4/notes.json @@ -4,6 +4,7 @@ "type": "object", "properties" : { "id": { "type": "integer" }, + "type": { "type": ["string", "null"] }, "body": { "type": "string" }, "attachment": { "type": ["string", "null"] }, "author": { diff --git a/spec/requests/api/discussions_spec.rb b/spec/requests/api/discussions_spec.rb new file mode 100644 index 00000000000..4a44b219a67 --- /dev/null +++ b/spec/requests/api/discussions_spec.rb @@ -0,0 +1,33 @@ +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 diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index 981c9c27325..dd568c24c72 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -3,117 +3,86 @@ require 'spec_helper' describe API::Notes do let(:user) { create(:user) } let!(:project) { create(:project, :public, namespace: user.namespace) } - let!(:issue) { create(:issue, project: project, author: user) } - let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: user) } - let!(:snippet) { create(:project_snippet, project: project, author: user) } - let!(:issue_note) { create(:note, noteable: issue, project: project, author: user) } - let!(:merge_request_note) { create(:note, noteable: merge_request, project: project, author: user) } - let!(:snippet_note) { create(:note, noteable: snippet, project: project, author: user) } - - # For testing the cross-reference of a private issue in a public issue let(:private_user) { create(:user) } - let(:private_project) do - create(:project, namespace: private_user.namespace) - .tap { |p| p.add_master(private_user) } - end - let(:private_issue) { create(:issue, project: private_project) } - - let(:ext_proj) { create(:project, :public) } - let(:ext_issue) { create(:issue, project: ext_proj) } - - let!(:cross_reference_note) do - create :note, - noteable: ext_issue, project: ext_proj, - note: "mentioned in issue #{private_issue.to_reference(ext_proj)}", - system: true - end before do project.add_reporter(user) end - describe "GET /projects/:id/noteable/:noteable_id/notes" do - context "when noteable is an Issue" do - context 'sorting' do - before do - create_list(:note, 3, noteable: issue, project: project, author: user) - end - - it 'sorts by created_at in descending order by default' do - get api("/projects/#{project.id}/issues/#{issue.iid}/notes", user) - - response_dates = json_response.map { |noteable| noteable['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("/projects/#{project.id}/issues/#{issue.iid}/notes?sort=asc", user) - - response_dates = json_response.map { |noteable| noteable['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("/projects/#{project.id}/issues/#{issue.iid}/notes?order_by=updated_at", user) - - response_dates = json_response.map { |noteable| noteable['updated_at'] } + context "when noteable is an Issue" do + let!(:issue) { create(:issue, project: project, author: user) } + let!(:issue_note) { create(:note, noteable: issue, project: project, author: user) } - expect(json_response.length).to eq(4) - expect(response_dates).to eq(response_dates.sort.reverse) - end + it_behaves_like "noteable API", 'projects', 'issues', 'iid' do + let(:parent) { project } + let(:noteable) { issue } + let(:note) { issue_note } + end - it 'sorts by updated_at in ascending order when requested' do - get api("/projects/#{project.id}/issues/#{issue.iid}/notes??order_by=updated_at&sort=asc", user) + context 'when user does not have access to create noteable' do + let(:private_issue) { create(:issue, project: create(:project, :private)) } - response_dates = json_response.map { |noteable| noteable['updated_at'] } + ## + # We are posting to project user has access to, but we use issue id + # from a different project, see #15577 + # + before do + post api("/projects/#{private_issue.project.id}/issues/#{private_issue.iid}/notes", user), + body: 'Hi!' + end - expect(json_response.length).to eq(4) - expect(response_dates).to eq(response_dates.sort) - end + it 'responds with resource not found error' do + expect(response.status).to eq 404 end - it "returns an array of issue notes" do - get api("/projects/#{project.id}/issues/#{issue.iid}/notes", user) + it 'does not create new note' do + expect(private_issue.notes.reload).to be_empty + end + end - 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(issue_note.note) + context "when referencing other project" do + # For testing the cross-reference of a private issue in a public project + let(:private_project) do + create(:project, namespace: private_user.namespace) + .tap { |p| p.add_master(private_user) } end + let(:private_issue) { create(:issue, project: private_project) } - it "returns a 404 error when issue id not found" do - get api("/projects/#{project.id}/issues/12345/notes", user) + let(:ext_proj) { create(:project, :public) } + let(:ext_issue) { create(:issue, project: ext_proj) } - expect(response).to have_gitlab_http_status(404) + let!(:cross_reference_note) do + create :note, + noteable: ext_issue, project: ext_proj, + note: "mentioned in issue #{private_issue.to_reference(ext_proj)}", + system: true end - context "and current user cannot view the notes" do - it "returns an empty array" do - get api("/projects/#{ext_proj.id}/issues/#{ext_issue.iid}/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).to be_empty - end + describe "GET /projects/:id/noteable/:noteable_id/notes" do + context "current user cannot view the notes" do + it "returns an empty array" do + get api("/projects/#{ext_proj.id}/issues/#{ext_issue.iid}/notes", user) - context "and issue is confidential" do - before do - ext_issue.update_attributes(confidential: true) + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response).to be_empty end - it "returns 404" do - get api("/projects/#{ext_proj.id}/issues/#{ext_issue.iid}/notes", user) + context "issue is confidential" do + before do + ext_issue.update_attributes(confidential: true) + end - expect(response).to have_gitlab_http_status(404) + it "returns 404" do + get api("/projects/#{ext_proj.id}/issues/#{ext_issue.iid}/notes", user) + + expect(response).to have_gitlab_http_status(404) + end end end - context "and current user can view the note" do + context "current user can view the note" do it "returns an empty array" do get api("/projects/#{ext_proj.id}/issues/#{ext_issue.iid}/notes", private_user) @@ -124,172 +93,29 @@ describe API::Notes do end end end - end - - context "when noteable is a Snippet" do - context 'sorting' do - before do - create_list(:note, 3, noteable: snippet, project: project, author: user) - end - - it 'sorts by created_at in descending order by default' do - get api("/projects/#{project.id}/snippets/#{snippet.id}/notes", user) - - response_dates = json_response.map { |noteable| noteable['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("/projects/#{project.id}/snippets/#{snippet.id}/notes?sort=asc", user) - - response_dates = json_response.map { |noteable| noteable['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("/projects/#{project.id}/snippets/#{snippet.id}/notes?order_by=updated_at", user) - - response_dates = json_response.map { |noteable| noteable['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("/projects/#{project.id}/snippets/#{snippet.id}/notes??order_by=updated_at&sort=asc", user) + describe "GET /projects/:id/noteable/:noteable_id/notes/:note_id" do + context "current user cannot view the notes" do + it "returns a 404 error" do + get api("/projects/#{ext_proj.id}/issues/#{ext_issue.iid}/notes/#{cross_reference_note.id}", user) - response_dates = json_response.map { |noteable| noteable['updated_at'] } - - expect(json_response.length).to eq(4) - expect(response_dates).to eq(response_dates.sort) - end - end - it "returns an array of snippet notes" do - get api("/projects/#{project.id}/snippets/#{snippet.id}/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(snippet_note.note) - end - - it "returns a 404 error when snippet id not found" do - get api("/projects/#{project.id}/snippets/42/notes", user) - - expect(response).to have_gitlab_http_status(404) - end - - it "returns 404 when not authorized" do - get api("/projects/#{project.id}/snippets/#{snippet.id}/notes", private_user) - - expect(response).to have_gitlab_http_status(404) - end - end - - context "when noteable is a Merge Request" do - context 'sorting' do - before do - create_list(:note, 3, noteable: merge_request, project: project, author: user) - end - - it 'sorts by created_at in descending order by default' do - get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes", user) - - response_dates = json_response.map { |noteable| noteable['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("/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes?sort=asc", user) - - response_dates = json_response.map { |noteable| noteable['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("/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes?order_by=updated_at", user) - - response_dates = json_response.map { |noteable| noteable['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("/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes??order_by=updated_at&sort=asc", user) - - response_dates = json_response.map { |noteable| noteable['updated_at'] } - - expect(json_response.length).to eq(4) - expect(response_dates).to eq(response_dates.sort) - end - end - it "returns an array of merge_requests notes" do - get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/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(merge_request_note.note) - end - - it "returns a 404 error if merge request id not found" do - get api("/projects/#{project.id}/merge_requests/4444/notes", user) - - expect(response).to have_gitlab_http_status(404) - end - - it "returns 404 when not authorized" do - get api("/projects/#{project.id}/merge_requests/4444/notes", private_user) - - expect(response).to have_gitlab_http_status(404) - end - end - end - - describe "GET /projects/:id/noteable/:noteable_id/notes/:note_id" do - context "when noteable is an Issue" do - it "returns an issue note by id" do - get api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{issue_note.id}", user) - - expect(response).to have_gitlab_http_status(200) - expect(json_response['body']).to eq(issue_note.note) - end - - it "returns a 404 error if issue note not found" do - get api("/projects/#{project.id}/issues/#{issue.iid}/notes/12345", user) - - expect(response).to have_gitlab_http_status(404) - end - - context "and current user cannot view the note" do - it "returns a 404 error" do - get api("/projects/#{ext_proj.id}/issues/#{ext_issue.iid}/notes/#{cross_reference_note.id}", user) - - expect(response).to have_gitlab_http_status(404) - end - - context "when issue is confidential" do - before do - issue.update_attributes(confidential: true) + expect(response).to have_gitlab_http_status(404) end - it "returns 404" do - get api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{issue_note.id}", private_user) + context "when issue is confidential" do + before do + issue.update_attributes(confidential: true) + end - expect(response).to have_gitlab_http_status(404) + it "returns 404" do + get api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{issue_note.id}", private_user) + + expect(response).to have_gitlab_http_status(404) + end end end - context "and current user can view the note" do + context "current user can view the note" do it "returns an issue note by id" do get api("/projects/#{ext_proj.id}/issues/#{ext_issue.iid}/notes/#{cross_reference_note.id}", private_user) @@ -299,132 +125,27 @@ describe API::Notes do end end end - - context "when noteable is a Snippet" do - it "returns a snippet note by id" do - get api("/projects/#{project.id}/snippets/#{snippet.id}/notes/#{snippet_note.id}", user) - - expect(response).to have_gitlab_http_status(200) - expect(json_response['body']).to eq(snippet_note.note) - end - - it "returns a 404 error if snippet note not found" do - get api("/projects/#{project.id}/snippets/#{snippet.id}/notes/12345", user) - - expect(response).to have_gitlab_http_status(404) - end - end end - describe "POST /projects/:id/noteable/:noteable_id/notes" do - context "when noteable is an Issue" do - it "creates a new issue note" do - post api("/projects/#{project.id}/issues/#{issue.iid}/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("/projects/#{project.id}/issues/#{issue.iid}/notes", user) - - expect(response).to have_gitlab_http_status(400) - end - - it "returns a 401 unauthorized error if user not authenticated" do - post api("/projects/#{project.id}/issues/#{issue.iid}/notes"), 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("/projects/#{project.id}/issues/#{issue.iid}/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 an issue created by someone else' do - let(:issue2) { create(:issue, project: project) } - - it 'creates a new issue note' do - post api("/projects/#{project.id}/issues/#{issue2.iid}/notes", 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 issue' do - it 'creates a new issue note' do - post api("/projects/#{project.id}/issues/#{issue.iid}/notes", user), body: ':+1:' - - expect(response).to have_gitlab_http_status(201) - expect(json_response['body']).to eq(':+1:') - end - end - end - - context "when noteable is a Snippet" do - it "creates a new snippet note" do - post api("/projects/#{project.id}/snippets/#{snippet.id}/notes", user), body: 'hi!' + context "when noteable is a Snippet" do + let!(:snippet) { create(:project_snippet, project: project, author: user) } + let!(:snippet_note) { create(:note, noteable: snippet, project: project, author: user) } - 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("/projects/#{project.id}/snippets/#{snippet.id}/notes", user) - - expect(response).to have_gitlab_http_status(400) - end - - it "returns a 401 unauthorized error if user not authenticated" do - post api("/projects/#{project.id}/snippets/#{snippet.id}/notes"), body: 'hi!' - - expect(response).to have_gitlab_http_status(401) - end + it_behaves_like "noteable API", 'projects', 'snippets', 'id' do + let(:parent) { project } + let(:noteable) { snippet } + let(:note) { snippet_note } end + end - context 'when user does not have access to read the noteable' do - it 'responds with 404' do - project = create(:project, :private) { |p| p.add_guest(user) } - issue = create(:issue, :confidential, project: project) - - post api("/projects/#{project.id}/issues/#{issue.iid}/notes", user), - body: 'Foo' - - expect(response).to have_gitlab_http_status(404) - end - end - - context 'when user does not have access to create noteable' do - let(:private_issue) { create(:issue, project: create(:project, :private)) } - - ## - # We are posting to project user has access to, but we use issue id - # from a different project, see #15577 - # - before do - post api("/projects/#{private_issue.project.id}/issues/#{private_issue.iid}/notes", user), - body: 'Hi!' - end - - it 'responds with resource not found error' do - expect(response.status).to eq 404 - end + context "when noteable is a Merge Request" do + let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: user) } + let!(:merge_request_note) { create(:note, noteable: merge_request, project: project, author: user) } - it 'does not create new note' do - expect(private_issue.notes.reload).to be_empty - end + it_behaves_like "noteable API", 'projects', 'merge_requests', 'iid' do + let(:parent) { project } + let(:noteable) { merge_request } + let(:note) { merge_request_note } end context 'when the merge request discussion is locked' do @@ -461,145 +182,4 @@ describe API::Notes do end end end - - describe "POST /projects/:id/noteable/:noteable_id/notes to test observer on create" do - it "creates an activity event when an issue note is created" do - expect(Event).to receive(:create!) - - post api("/projects/#{project.id}/issues/#{issue.iid}/notes", user), body: 'hi!' - end - end - - describe 'PUT /projects/:id/noteable/:noteable_id/notes/:note_id' do - context 'when noteable is an Issue' do - it 'returns modified note' do - put api("/projects/#{project.id}/issues/#{issue.iid}/"\ - "notes/#{issue_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("/projects/#{project.id}/issues/#{issue.iid}/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("/projects/#{project.id}/issues/#{issue.iid}/"\ - "notes/#{issue_note.id}", user) - - expect(response).to have_gitlab_http_status(400) - end - end - - context 'when noteable is a Snippet' do - it 'returns modified note' do - put api("/projects/#{project.id}/snippets/#{snippet.id}/"\ - "notes/#{snippet_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("/projects/#{project.id}/snippets/#{snippet.id}/"\ - "notes/12345", user), body: "Hello!" - - expect(response).to have_gitlab_http_status(404) - end - end - - context 'when noteable is a Merge Request' do - it 'returns modified note' do - put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/"\ - "notes/#{merge_request_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("/projects/#{project.id}/merge_requests/#{merge_request.iid}/"\ - "notes/12345", user), body: "Hello!" - - expect(response).to have_gitlab_http_status(404) - end - end - end - - describe 'DELETE /projects/:id/noteable/:noteable_id/notes/:note_id' do - context 'when noteable is an Issue' do - it 'deletes a note' do - delete api("/projects/#{project.id}/issues/#{issue.iid}/"\ - "notes/#{issue_note.id}", user) - - expect(response).to have_gitlab_http_status(204) - # Check if note is really deleted - delete api("/projects/#{project.id}/issues/#{issue.iid}/"\ - "notes/#{issue_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("/projects/#{project.id}/issues/#{issue.iid}/notes/12345", user) - - expect(response).to have_gitlab_http_status(404) - end - - it_behaves_like '412 response' do - let(:request) { api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{issue_note.id}", user) } - end - end - - context 'when noteable is a Snippet' do - it 'deletes a note' do - delete api("/projects/#{project.id}/snippets/#{snippet.id}/"\ - "notes/#{snippet_note.id}", user) - - expect(response).to have_gitlab_http_status(204) - # Check if note is really deleted - delete api("/projects/#{project.id}/snippets/#{snippet.id}/"\ - "notes/#{snippet_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("/projects/#{project.id}/snippets/#{snippet.id}/"\ - "notes/12345", user) - - expect(response).to have_gitlab_http_status(404) - end - - it_behaves_like '412 response' do - let(:request) { api("/projects/#{project.id}/snippets/#{snippet.id}/notes/#{snippet_note.id}", user) } - end - end - - context 'when noteable is a Merge Request' do - it 'deletes a note' do - delete api("/projects/#{project.id}/merge_requests/"\ - "#{merge_request.iid}/notes/#{merge_request_note.id}", user) - - expect(response).to have_gitlab_http_status(204) - # Check if note is really deleted - delete api("/projects/#{project.id}/merge_requests/"\ - "#{merge_request.iid}/notes/#{merge_request_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("/projects/#{project.id}/merge_requests/"\ - "#{merge_request.iid}/notes/12345", user) - - expect(response).to have_gitlab_http_status(404) - end - - it_behaves_like '412 response' do - let(:request) { api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes/#{merge_request_note.id}", user) } - end - end - end end diff --git a/spec/support/shared_examples/requests/api/discussions.rb b/spec/support/shared_examples/requests/api/discussions.rb new file mode 100644 index 00000000000..b6aeb30d69c --- /dev/null +++ b/spec/support/shared_examples/requests/api/discussions.rb @@ -0,0 +1,169 @@ +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 diff --git a/spec/support/shared_examples/requests/api/notes.rb b/spec/support/shared_examples/requests/api/notes.rb new file mode 100644 index 00000000000..79b2196660c --- /dev/null +++ b/spec/support/shared_examples/requests/api/notes.rb @@ -0,0 +1,206 @@ +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 |