diff options
| author | Sean McGivern <sean@mcgivern.me.uk> | 2017-02-28 12:30:41 +0000 |
|---|---|---|
| committer | Sean McGivern <sean@mcgivern.me.uk> | 2017-02-28 12:30:41 +0000 |
| commit | a69aa3dad51d5e881b3c92f3ac09c689edc00ab3 (patch) | |
| tree | 8617787330de85e2ec62a59b190bed5813e4cf35 | |
| parent | 71fbbc9da428268411c5a3cef319f7537b63d199 (diff) | |
| parent | 2b8005ae41e7bd29010d54fe8a2c03c7e58b5c66 (diff) | |
| download | gitlab-ce-a69aa3dad51d5e881b3c92f3ac09c689edc00ab3.tar.gz | |
Merge branch 'api-empty-return' into 'master'
Return 204 on all DELETE endpoints
Closes #20429
See merge request !9397
100 files changed, 1611 insertions, 477 deletions
@@ -68,7 +68,7 @@ gem 'gollum-rugged_adapter', '~> 0.4.2', require: false gem 'github-linguist', '~> 4.7.0', require: 'linguist' # API -gem 'grape', '~> 0.18.0' +gem 'grape', '~> 0.19.0' gem 'grape-entity', '~> 0.6.0' gem 'rack-cors', '~> 0.4.0', require: 'rack/cors' diff --git a/Gemfile.lock b/Gemfile.lock index 5ff18442c4f..472cee510cb 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -304,7 +304,7 @@ GEM multi_json (~> 1.11) os (~> 0.9) signet (~> 0.7) - grape (0.18.0) + grape (0.19.1) activesupport builder hashie (>= 2.1.0) @@ -353,8 +353,8 @@ GEM json (~> 1.8) multi_xml (>= 0.5.2) httpclient (2.8.2) - i18n (0.8.0) - ice_nine (0.11.1) + i18n (0.8.1) + ice_nine (0.11.2) influxdb (0.2.3) cause json @@ -417,7 +417,7 @@ GEM minitest (5.7.0) mousetrap-rails (1.4.6) multi_json (1.12.1) - multi_xml (0.5.5) + multi_xml (0.6.0) multipart-post (2.0.0) mustermann (0.4.0) tool (~> 0.2) @@ -758,7 +758,7 @@ GEM eventmachine (~> 1.0, >= 1.0.4) rack (>= 1, < 3) thor (0.19.4) - thread_safe (0.3.5) + thread_safe (0.3.6) tilt (2.0.6) timecop (0.8.1) timfel-krb5-auth (0.8.3) @@ -886,7 +886,7 @@ DEPENDENCIES gollum-rugged_adapter (~> 0.4.2) gon (~> 6.1.0) google-api-client (~> 0.8.6) - grape (~> 0.18.0) + grape (~> 0.19.0) grape-entity (~> 0.6.0) haml_lint (~> 0.21.0) hamlit (~> 2.6.1) @@ -1011,4 +1011,4 @@ DEPENDENCIES wikicloth (= 0.8.1) BUNDLED WITH - 1.14.3 + 1.14.4 diff --git a/changelogs/unreleased/api-empty-return.yml b/changelogs/unreleased/api-empty-return.yml new file mode 100644 index 00000000000..7810e83eb0e --- /dev/null +++ b/changelogs/unreleased/api-empty-return.yml @@ -0,0 +1,4 @@ +--- +title: 'API: Return 204 for all delete endpoints' +merge_request: 9397 +author: Robert Schilling diff --git a/doc/api/README.md b/doc/api/README.md index b334ca46caf..1c3b2ad0fbc 100644 --- a/doc/api/README.md +++ b/doc/api/README.md @@ -159,6 +159,7 @@ The following table shows the possible return codes for API requests. | Return values | Description | | ------------- | ----------- | | `200 OK` | The `GET`, `PUT` or `DELETE` request was successful, the resource(s) itself is returned as JSON. | +| `204 OK` | The server has successfully fulfilled the request and that there is no additional content to send in the response payload body. | | `201 Created` | The `POST` request was successful and the resource is returned as JSON. | | `304 Not Modified` | Indicates that the resource has not been modified since the last request. | | `400 Bad Request` | A required attribute of the API request is missing, e.g., the title of an issue is not given. | diff --git a/doc/api/award_emoji.md b/doc/api/award_emoji.md index 58092bdd400..c6fd8c5fa53 100644 --- a/doc/api/award_emoji.md +++ b/doc/api/award_emoji.md @@ -178,27 +178,6 @@ Parameters: curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" http://gitlab.example.com/api/v3/projects/1/issues/80/award_emoji/344 ``` -Example Response: - -```json -{ - "id": 344, - "name": "blowfish", - "user": { - "name": "Administrator", - "username": "root", - "id": 1, - "state": "active", - "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "web_url": "http://gitlab.example.com/root" - }, - "created_at": "2016-06-17T17:47:29.266Z", - "updated_at": "2016-06-17T17:47:29.266Z", - "awardable_id": 80, - "awardable_type": "Issue" -} -``` - ## Award Emoji on Notes The endpoints documented above are available for Notes as well. Notes @@ -350,25 +329,4 @@ Parameters: curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" http://gitlab.example.com/api/v3/projects/1/issues/80/award_emoji/345 ``` -Example Response: - -```json -{ - "id": 345, - "name": "rocket", - "user": { - "name": "Administrator", - "username": "root", - "id": 1, - "state": "active", - "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "web_url": "http://gitlab.example.com/root" - }, - "created_at": "2016-06-17T19:59:55.888Z", - "updated_at": "2016-06-17T19:59:55.888Z", - "awardable_id": 1, - "awardable_type": "Note" -} -``` - [ce-4575]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/4575 diff --git a/doc/api/boards.md b/doc/api/boards.md index c83db6df80c..f80b98f960b 100644 --- a/doc/api/boards.md +++ b/doc/api/boards.md @@ -226,16 +226,3 @@ DELETE /projects/:id/boards/:board_id/lists/:list_id ```bash curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/boards/1/lists/1 ``` -Example response: - -```json -{ - "id" : 1, - "label" : { - "name" : "Testing", - "color" : "#F0AD4E", - "description" : null - }, - "position" : 1 -} -``` diff --git a/doc/api/branches.md b/doc/api/branches.md index 765ca439720..f29a8518945 100644 --- a/doc/api/branches.md +++ b/doc/api/branches.md @@ -244,14 +244,6 @@ In case of an error, an explaining message is provided. curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/5/repository/branches/newbranch" ``` -Example response: - -```json -{ - "branch_name": "newbranch" -} -``` - ## Delete merged branches Will delete all branches that are merged into the project's default branch. diff --git a/doc/api/broadcast_messages.md b/doc/api/broadcast_messages.md index a3e9c01f335..fecfb142ab1 100644 --- a/doc/api/broadcast_messages.md +++ b/doc/api/broadcast_messages.md @@ -138,17 +138,3 @@ DELETE /broadcast_messages/:id ```bash curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/broadcast_messages/1 ``` - -Example response: - -```json -{ - "message":"Update message", - "starts_at":"2016-08-26T00:41:35.060Z", - "ends_at":"2016-08-26T01:41:35.060Z", - "color":"#000", - "font":"#FFFFFF", - "id":1, - "active": true -} -``` diff --git a/doc/api/build_triggers.md b/doc/api/build_triggers.md index b6459971420..6adefe8c58c 100644 --- a/doc/api/build_triggers.md +++ b/doc/api/build_triggers.md @@ -106,13 +106,3 @@ DELETE /projects/:id/triggers/:token ``` curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/triggers/7b9148c158980bbd9bcea92c17522d" ``` - -```json -{ - "created_at": "2015-12-23T16:25:56.760Z", - "deleted_at": "2015-12-24T12:32:20.100Z", - "last_used": null, - "token": "7b9148c158980bbd9bcea92c17522d", - "updated_at": "2015-12-24T12:32:20.100Z" -} -``` diff --git a/doc/api/build_variables.md b/doc/api/build_variables.md index 917e9773913..c21d5ab2787 100644 --- a/doc/api/build_variables.md +++ b/doc/api/build_variables.md @@ -119,10 +119,3 @@ DELETE /projects/:id/variables/:key ``` curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/variables/VARIABLE_1" ``` - -```json -{ - "key": "VARIABLE_1", - "value": "VALUE_1" -} -``` diff --git a/doc/api/deploy_keys.md b/doc/api/deploy_keys.md index 39afc4b2df5..d03d94cb867 100644 --- a/doc/api/deploy_keys.md +++ b/doc/api/deploy_keys.md @@ -152,18 +152,6 @@ DELETE /projects/:id/deploy_keys/:key_id curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/5/deploy_keys/13" ``` -Example response: - -```json -{ - "id": 6, - "deploy_key_id": 14, - "project_id": 1, - "created_at" : "2015-08-29T12:50:57.259Z", - "updated_at" : "2015-08-29T12:50:57.259Z" -} -``` - ## Enable a deploy key Enables a deploy key for a project so this can be used. Returns the enabled key, with a status code 201 when successful. diff --git a/doc/api/enviroments.md b/doc/api/enviroments.md index e0ee20d9610..e510f723e26 100644 --- a/doc/api/enviroments.md +++ b/doc/api/enviroments.md @@ -108,14 +108,3 @@ DELETE /projects/:id/environments/:environment_id ```bash curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/environments/1" ``` - -Example response: - -```json -{ - "id": 1, - "name": "deploy", - "slug": "deploy", - "external_url": "https://deploy.example.gitlab.com" -} -``` diff --git a/doc/api/issues.md b/doc/api/issues.md index 5266077e098..b6798fba0ae 100644 --- a/doc/api/issues.md +++ b/doc/api/issues.md @@ -581,43 +581,6 @@ POST /projects/:id/issues/:issue_id/unsubscribe curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/issues/93/unsubscribe ``` -Example response: - -```json -{ - "id": 93, - "iid": 12, - "project_id": 5, - "title": "Incidunt et rerum ea expedita iure quibusdam.", - "description": "Et cumque architecto sed aut ipsam.", - "state": "opened", - "created_at": "2016-04-05T21:41:45.217Z", - "updated_at": "2016-04-07T13:02:37.905Z", - "labels": [], - "milestone": null, - "assignee": { - "name": "Edwardo Grady", - "username": "keyon", - "id": 21, - "state": "active", - "avatar_url": "http://www.gravatar.com/avatar/3e6f06a86cf27fa8b56f3f74f7615987?s=80&d=identicon", - "web_url": "https://gitlab.example.com/keyon" - }, - "author": { - "name": "Vivian Hermann", - "username": "orville", - "id": 11, - "state": "active", - "avatar_url": "http://www.gravatar.com/avatar/5224fd70153710e92fb8bcf79ac29d67?s=80&d=identicon", - "web_url": "https://gitlab.example.com/orville" - }, - "subscribed": false, - "due_date": null, - "web_url": "http://example.com/example/example/issues/12", - "confidential": false -} -``` - ## Create a todo Manually creates a todo for the current user on an issue. If diff --git a/doc/api/labels.md b/doc/api/labels.md index 8e0855fe9e2..85bd9647a7b 100644 --- a/doc/api/labels.md +++ b/doc/api/labels.md @@ -131,22 +131,6 @@ DELETE /projects/:id/labels curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/labels?name=bug" ``` -Example response: - -```json -{ - "id" : 1, - "name" : "bug", - "color" : "#d9534f", - "description": "Bug reported by user", - "open_issues_count": 1, - "closed_issues_count": 0, - "open_merge_requests_count": 1, - "subscribed": false, - "priority": null -} -``` - ## Edit an existing label Updates an existing label with new name or new color. At least one parameter @@ -239,19 +223,3 @@ POST /projects/:id/labels/:label_id/unsubscribe ```bash curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/labels/1/unsubscribe ``` - -Example response: - -```json -{ - "id" : 1, - "name" : "bug", - "color" : "#d9534f", - "description": "Bug reported by user", - "open_issues_count": 1, - "closed_issues_count": 0, - "open_merge_requests_count": 1, - "subscribed": false, - "priority": null -} -``` diff --git a/doc/api/notes.md b/doc/api/notes.md index dced821cc6d..7dc1fd930de 100644 --- a/doc/api/notes.md +++ b/doc/api/notes.md @@ -123,30 +123,6 @@ Parameters: curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/issues/11/notes/636 ``` -Example Response: - -```json -{ - "id": 636, - "body": "This is a good idea.", - "attachment": null, - "author": { - "id": 1, - "username": "pipin", - "email": "admin@example.com", - "name": "Pip", - "state": "active", - "created_at": "2013-09-30T13:46:01Z", - "avatar_url": "http://www.gravatar.com/avatar/5224fd70153710e92fb8bcf79ac29d67?s=80&d=identicon", - "web_url": "https://gitlab.example.com/pipin" - }, - "created_at": "2016-04-05T22:10:44.164Z", - "system": false, - "noteable_id": 11, - "noteable_type": "Issue" -} -``` - ## Snippets ### List all snippet notes @@ -245,30 +221,6 @@ Parameters: curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/snippets/52/notes/1659 ``` -Example Response: - -```json -{ - "id": 1659, - "body": "This is a good idea.", - "attachment": null, - "author": { - "id": 1, - "username": "pipin", - "email": "admin@example.com", - "name": "Pip", - "state": "active", - "created_at": "2013-09-30T13:46:01Z", - "avatar_url": "http://www.gravatar.com/avatar/5224fd70153710e92fb8bcf79ac29d67?s=80&d=identicon", - "web_url": "https://gitlab.example.com/pipin" - }, - "created_at": "2016-04-06T16:51:53.239Z", - "system": false, - "noteable_id": 52, - "noteable_type": "Snippet" -} -``` - ## Merge Requests ### List all merge request notes @@ -369,27 +321,3 @@ Parameters: ```bash curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/merge_requests/7/notes/1602 ``` - -Example Response: - -```json -{ - "id": 1602, - "body": "This is a good idea.", - "attachment": null, - "author": { - "id": 1, - "username": "pipin", - "email": "admin@example.com", - "name": "Pip", - "state": "active", - "created_at": "2013-09-30T13:46:01Z", - "avatar_url": "http://www.gravatar.com/avatar/5224fd70153710e92fb8bcf79ac29d67?s=80&d=identicon", - "web_url": "https://gitlab.example.com/pipin" - }, - "created_at": "2016-04-05T22:11:59.923Z", - "system": false, - "noteable_id": 7, - "noteable_type": "MergeRequest" -} -``` diff --git a/doc/api/runners.md b/doc/api/runners.md index 28610762dca..27d8e7640b2 100644 --- a/doc/api/runners.md +++ b/doc/api/runners.md @@ -210,18 +210,6 @@ DELETE /runners/:id curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/runners/6" ``` -Example response: - -```json -{ - "active": true, - "description": "test-1-20150125-test", - "id": 6, - "is_shared": false, - "name": null, -} -``` - ## List project's runners List all runners (specific and shared) available in the project. Shared runners @@ -308,15 +296,3 @@ DELETE /projects/:id/runners/:runner_id ``` curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/9/runners/9" ``` - -Example response: - -```json -{ - "active": true, - "description": "test-2016-02-01", - "id": 9, - "is_shared": false, - "name": null -} -``` diff --git a/doc/api/system_hooks.md b/doc/api/system_hooks.md index 3fb8b73be6d..a9edff799ac 100644 --- a/doc/api/system_hooks.md +++ b/doc/api/system_hooks.md @@ -125,22 +125,3 @@ Example request: ```bash curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/hooks/2 ``` - -Example response: - -```json -{ - "note_events" : false, - "project_id" : null, - "enable_ssl_verification" : true, - "url" : "https://gitlab.example.com/hook", - "updated_at" : "2015-11-04T20:12:15.931Z", - "issues_events" : false, - "merge_requests_events" : false, - "created_at" : "2015-11-04T20:12:15.931Z", - "service_id" : null, - "id" : 2, - "push_events" : true, - "tag_push_events" : false -} -``` diff --git a/doc/api/tags.md b/doc/api/tags.md index 7f78ffc2390..abeb4bfb40e 100644 --- a/doc/api/tags.md +++ b/doc/api/tags.md @@ -141,11 +141,6 @@ Parameters: - `id` (required) - The ID of a project - `tag_name` (required) - The name of a tag -```json -{ - "tag_name": "v4.3.0" -} -``` ## Create a new release diff --git a/lib/api/api.rb b/lib/api/api.rb index 7aa95a4a3c1..b27ac3f1d15 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -5,10 +5,13 @@ module API version %w(v3 v4), using: :path version 'v3', using: :path do + mount ::API::V3::AwardEmoji mount ::API::V3::Boards mount ::API::V3::Branches + mount ::API::V3::BroadcastMessages mount ::API::V3::Commits mount ::API::V3::DeployKeys + mount ::API::V3::Environments mount ::API::V3::Files mount ::API::V3::Groups mount ::API::V3::Issues @@ -21,12 +24,16 @@ module API mount ::API::V3::Projects mount ::API::V3::ProjectSnippets mount ::API::V3::Repositories + mount ::API::V3::Runners + mount ::API::V3::Services mount ::API::V3::Subscriptions mount ::API::V3::SystemHooks mount ::API::V3::Tags - mount ::API::V3::Todos mount ::API::V3::Templates + mount ::API::V3::Todos + mount ::API::V3::Triggers mount ::API::V3::Users + mount ::API::V3::Variables end before { allow_access_with_scope :api } diff --git a/lib/api/award_emoji.rb b/lib/api/award_emoji.rb index 301271118d4..07a1bcdbe18 100644 --- a/lib/api/award_emoji.rb +++ b/lib/api/award_emoji.rb @@ -83,7 +83,6 @@ module API unauthorized! unless award.user == current_user || current_user.admin? award.destroy - present award, with: Entities::AwardEmoji end end end diff --git a/lib/api/boards.rb b/lib/api/boards.rb index f4226e5a89d..b6843c1b6af 100644 --- a/lib/api/boards.rb +++ b/lib/api/boards.rb @@ -127,9 +127,7 @@ module API service = ::Boards::Lists::DestroyService.new(user_project, current_user) - if service.execute(list) - present list, with: Entities::List - else + unless service.execute(list) render_api_error!({ error: 'List could not be deleted!' }, 400) end end diff --git a/lib/api/branches.rb b/lib/api/branches.rb index 34f136948c2..73a7e939627 100644 --- a/lib/api/branches.rb +++ b/lib/api/branches.rb @@ -124,11 +124,7 @@ module API result = DeleteBranchService.new(user_project, current_user). execute(params[:branch]) - if result[:status] == :success - { - branch: params[:branch] - } - else + if result[:status] != :success render_api_error!(result[:message], result[:return_code]) end end diff --git a/lib/api/broadcast_messages.rb b/lib/api/broadcast_messages.rb index 1217002bf8e..395c401203c 100644 --- a/lib/api/broadcast_messages.rb +++ b/lib/api/broadcast_messages.rb @@ -91,7 +91,7 @@ module API delete ':id' do message = find_message - present message.destroy, with: Entities::BroadcastMessage + message.destroy end end end diff --git a/lib/api/environments.rb b/lib/api/environments.rb index 1a7e68f0528..dbdf29a9640 100644 --- a/lib/api/environments.rb +++ b/lib/api/environments.rb @@ -79,7 +79,7 @@ module API environment = user_project.environments.find(params[:environment_id]) - present environment.destroy, with: Entities::Environment + environment.destroy end end end diff --git a/lib/api/files.rb b/lib/api/files.rb index 500f9d3c787..9c4e43d77cc 100644 --- a/lib/api/files.rb +++ b/lib/api/files.rb @@ -118,10 +118,7 @@ module API file_params = declared_params(include_missing: false) result = ::Files::DestroyService.new(user_project, current_user, commit_params(file_params)).execute - if result[:status] == :success - status(200) - commit_response(file_params) - else + if result[:status] != :success render_api_error!(result[:message], 400) end end diff --git a/lib/api/labels.rb b/lib/api/labels.rb index d2955af3f95..59f0e7cb647 100644 --- a/lib/api/labels.rb +++ b/lib/api/labels.rb @@ -1,7 +1,7 @@ module API class Labels < Grape::API include PaginationParams - + before { authenticate! } params do @@ -56,7 +56,7 @@ module API label = user_project.labels.find_by(title: params[:name]) not_found!('Label') unless label - present label.destroy, with: Entities::Label, current_user: current_user, project: user_project + label.destroy end desc 'Update an existing label. At least one optional parameter is required.' do diff --git a/lib/api/members.rb b/lib/api/members.rb index 5f6913d1a27..baf85e6075a 100644 --- a/lib/api/members.rb +++ b/lib/api/members.rb @@ -93,24 +93,10 @@ module API end delete ":id/members/:user_id" do source = find_source(source_type, params[:id]) + # Ensure that memeber exists + source.members.find_by!(user_id: params[:user_id]) - # This is to ensure back-compatibility but find_by! should be used - # in that casse in 9.0! - member = source.members.find_by(user_id: params[:user_id]) - - # This is to ensure back-compatibility but this should be removed in - # favor of find_by! in 9.0! - not_found!("Member: user_id:#{params[:user_id]}") if source_type == 'group' && member.nil? - - # This is to ensure back-compatibility but 204 behavior should be used - # for all DELETE endpoints in 9.0! - if member.nil? - { message: "Access revoked", id: params[:user_id].to_i } - else - ::Members::DestroyService.new(source, current_user, declared_params).execute - - present member.user, with: Entities::Member, member: member - end + ::Members::DestroyService.new(source, current_user, declared_params).execute end end end diff --git a/lib/api/notes.rb b/lib/api/notes.rb index f559a7f74a0..3b3e45cbd06 100644 --- a/lib/api/notes.rb +++ b/lib/api/notes.rb @@ -132,8 +132,6 @@ module API authorize! :admin_note, note ::Notes::DestroyService.new(user_project, current_user).execute(note) - - present note, with: Entities::Note end end end diff --git a/lib/api/project_hooks.rb b/lib/api/project_hooks.rb index f7a28d7ad10..57a5f97dc7f 100644 --- a/lib/api/project_hooks.rb +++ b/lib/api/project_hooks.rb @@ -90,12 +90,9 @@ module API requires :hook_id, type: Integer, desc: 'The ID of the hook to delete' end delete ":id/hooks/:hook_id" do - begin - present user_project.hooks.destroy(params[:hook_id]), with: Entities::ProjectHook - rescue - # ProjectHook can raise Error if hook_id not found - not_found!("Error deleting hook #{params[:hook_id]}") - end + hook = user_project.hooks.find(params.delete(:hook_id)) + + hook.destroy end end end diff --git a/lib/api/projects.rb b/lib/api/projects.rb index b89bddc7e29..310a896e958 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -353,7 +353,6 @@ module API not_found!('Group Link') unless link link.destroy - no_content! end desc 'Upload a file' diff --git a/lib/api/runner.rb b/lib/api/runner.rb index 804b27d40a7..47858f1866b 100644 --- a/lib/api/runner.rb +++ b/lib/api/runner.rb @@ -38,7 +38,7 @@ module API end desc 'Deletes a registered Runner' do - http_codes [[200, 'Runner was deleted'], [403, 'Forbidden']] + http_codes [[204, 'Runner was deleted'], [403, 'Forbidden']] end params do requires :token, type: String, desc: %q(Runner's authentication token) diff --git a/lib/api/runners.rb b/lib/api/runners.rb index 252e59bfa58..2e41f16f8c6 100644 --- a/lib/api/runners.rb +++ b/lib/api/runners.rb @@ -78,9 +78,8 @@ module API delete ':id' do runner = get_runner(params[:id]) authenticate_delete_runner!(runner) - runner.destroy! - present runner, with: Entities::Runner + runner.destroy! end end @@ -136,8 +135,6 @@ module API forbidden!("Only one project associated with the runner. Please remove the runner instead") if runner.projects.count == 1 runner_project.destroy - - present runner, with: Entities::Runner end end diff --git a/lib/api/services.rb b/lib/api/services.rb index ad856115485..79a5f27dc4d 100644 --- a/lib/api/services.rb +++ b/lib/api/services.rb @@ -654,9 +654,7 @@ module API hash.merge!(key => nil) end - if service.update_attributes(attrs.merge(active: false)) - true - else + unless service.update_attributes(attrs.merge(active: false)) render_api_error!('400 Bad Request', 400) end end diff --git a/lib/api/snippets.rb b/lib/api/snippets.rb index ac03fbd2a3d..0f86fdb3075 100644 --- a/lib/api/snippets.rb +++ b/lib/api/snippets.rb @@ -118,9 +118,10 @@ module API delete ':id' do snippet = snippets_for_current_user.find_by(id: params.delete(:id)) return not_found!('Snippet') unless snippet + authorize! :destroy_personal_snippet, snippet + snippet.destroy - no_content! end desc 'Get a raw snippet' do diff --git a/lib/api/system_hooks.rb b/lib/api/system_hooks.rb index d038a3fa828..ed7b23b474a 100644 --- a/lib/api/system_hooks.rb +++ b/lib/api/system_hooks.rb @@ -66,7 +66,7 @@ module API hook = SystemHook.find_by(id: params[:id]) not_found!('System hook') unless hook - present hook.destroy, with: Entities::Hook + hook.destroy end end end diff --git a/lib/api/tags.rb b/lib/api/tags.rb index 86759ab882f..d31ef9de26b 100644 --- a/lib/api/tags.rb +++ b/lib/api/tags.rb @@ -66,11 +66,7 @@ module API result = ::Tags::DestroyService.new(user_project, current_user). execute(params[:tag_name]) - if result[:status] == :success - { - tag_name: params[:tag_name] - } - else + if result[:status] != :success render_api_error!(result[:message], result[:return_code]) end end diff --git a/lib/api/triggers.rb b/lib/api/triggers.rb index ea0ad852633..b7c9c5f2b7f 100644 --- a/lib/api/triggers.rb +++ b/lib/api/triggers.rb @@ -93,8 +93,6 @@ module API return not_found!('Trigger') unless trigger trigger.destroy - - present trigger, with: Entities::Trigger end end end diff --git a/lib/api/users.rb b/lib/api/users.rb index 94b2b6653d2..7bb4b76f830 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -236,7 +236,7 @@ module API key = user.keys.find_by(id: params[:key_id]) not_found!('Key') unless key - present key.destroy, with: Entities::SSHKey + key.destroy end desc 'Add an email address to a specified user. Available only for admins.' do @@ -422,7 +422,7 @@ module API key = current_user.keys.find_by(id: params[:key_id]) not_found!('Key') unless key - present key.destroy, with: Entities::SSHKey + key.destroy end desc "Get the currently authenticated user's email addresses" do diff --git a/lib/api/v3/award_emoji.rb b/lib/api/v3/award_emoji.rb new file mode 100644 index 00000000000..1e35283631f --- /dev/null +++ b/lib/api/v3/award_emoji.rb @@ -0,0 +1,59 @@ +module API + module V3 + class AwardEmoji < Grape::API + include PaginationParams + + before { authenticate! } + AWARDABLES = %w[issue merge_request snippet].freeze + + resource :projects do + AWARDABLES.each do |awardable_type| + awardable_string = awardable_type.pluralize + awardable_id_string = "#{awardable_type}_id" + + params do + requires :id, type: String, desc: 'The ID of a project' + requires :"#{awardable_id_string}", type: Integer, desc: "The ID of an Issue, Merge Request or Snippet" + end + + [":id/#{awardable_string}/:#{awardable_id_string}/award_emoji", + ":id/#{awardable_string}/:#{awardable_id_string}/notes/:note_id/award_emoji"].each do |endpoint| + desc 'Delete a +awardables+ award emoji' do + detail 'This feature was introduced in 8.9' + success ::API::Entities::AwardEmoji + end + params do + requires :award_id, type: Integer, desc: 'The ID of an award emoji' + end + delete "#{endpoint}/:award_id" do + award = awardable.award_emoji.find(params[:award_id]) + + unauthorized! unless award.user == current_user || current_user.admin? + + present award.destroy, with: ::API::Entities::AwardEmoji + end + end + end + end + + helpers do + def awardable + @awardable ||= + begin + if params.include?(:note_id) + note_id = params.delete(:note_id) + + awardable.notes.find(note_id) + elsif params.include?(:issue_id) + user_project.issues.find(params[:issue_id]) + elsif params.include?(:merge_request_id) + user_project.merge_requests.find(params[:merge_request_id]) + else + user_project.snippets.find(params[:snippet_id]) + end + end + end + end + end + end +end diff --git a/lib/api/v3/boards.rb b/lib/api/v3/boards.rb index 31d708bc2c8..b1c2a3c59f2 100644 --- a/lib/api/v3/boards.rb +++ b/lib/api/v3/boards.rb @@ -44,6 +44,27 @@ module API authorize!(:read_board, user_project) present board_lists, with: ::API::Entities::List end + + desc 'Delete a board list' do + detail 'This feature was introduced in 8.13' + success ::API::Entities::List + end + params do + requires :list_id, type: Integer, desc: 'The ID of a board list' + end + delete "/lists/:list_id" do + authorize!(:admin_list, user_project) + + list = board_lists.find(params[:list_id]) + + service = ::Boards::Lists::DestroyService.new(user_project, current_user) + + if service.execute(list) + present list, with: ::API::Entities::List + else + render_api_error!({ error: 'List could not be deleted!' }, 400) + end + end end end end diff --git a/lib/api/v3/branches.rb b/lib/api/v3/branches.rb index 51eb566cf7d..699e41b5537 100644 --- a/lib/api/v3/branches.rb +++ b/lib/api/v3/branches.rb @@ -19,6 +19,26 @@ module API present branches, with: ::API::Entities::RepoBranch, project: user_project end + desc 'Delete a branch' + params do + requires :branch, type: String, desc: 'The name of the branch' + end + delete ":id/repository/branches/:branch", requirements: { branch: /.+/ } do + authorize_push_project + + result = DeleteBranchService.new(user_project, current_user). + execute(params[:branch]) + + if result[:status] == :success + status(200) + { + branch_name: params[:branch] + } + else + render_api_error!(result[:message], result[:return_code]) + end + end + desc 'Delete all merged branches' delete ":id/repository/merged_branches" do DeleteMergedBranchesService.new(user_project, current_user).async_execute diff --git a/lib/api/v3/broadcast_messages.rb b/lib/api/v3/broadcast_messages.rb new file mode 100644 index 00000000000..417e4ad0b26 --- /dev/null +++ b/lib/api/v3/broadcast_messages.rb @@ -0,0 +1,31 @@ +module API + module V3 + class BroadcastMessages < Grape::API + include PaginationParams + + before { authenticate! } + before { authenticated_as_admin! } + + resource :broadcast_messages do + helpers do + def find_message + BroadcastMessage.find(params[:id]) + end + end + + desc 'Delete a broadcast message' do + detail 'This feature was introduced in GitLab 8.12.' + success ::API::Entities::BroadcastMessage + end + params do + requires :id, type: Integer, desc: 'Broadcast message ID' + end + delete ':id' do + message = find_message + + present message.destroy, with: ::API::Entities::BroadcastMessage + end + end + end + end +end diff --git a/lib/api/v3/environments.rb b/lib/api/v3/environments.rb new file mode 100644 index 00000000000..3effccfa708 --- /dev/null +++ b/lib/api/v3/environments.rb @@ -0,0 +1,29 @@ +module API + module V3 + class Environments < Grape::API + include PaginationParams + + before { authenticate! } + + params do + requires :id, type: String, desc: 'The project ID' + end + resource :projects do + desc 'Deletes an existing environment' do + detail 'This feature was introduced in GitLab 8.11.' + success ::API::Entities::Environment + end + params do + requires :environment_id, type: Integer, desc: 'The environment ID' + end + delete ':id/environments/:environment_id' do + authorize! :update_environment, user_project + + environment = user_project.environments.find(params[:environment_id]) + + present environment.destroy, with: ::API::Entities::Environment + end + end + end + end +end diff --git a/lib/api/v3/issues.rb b/lib/api/v3/issues.rb index d0af09f0e1e..5d7dfabfcd6 100644 --- a/lib/api/v3/issues.rb +++ b/lib/api/v3/issues.rb @@ -226,6 +226,8 @@ module API not_found!('Issue') unless issue authorize!(:destroy_issue, issue) + + status(200) issue.destroy end end diff --git a/lib/api/v3/labels.rb b/lib/api/v3/labels.rb index 5c3261311bf..41f45d244e3 100644 --- a/lib/api/v3/labels.rb +++ b/lib/api/v3/labels.rb @@ -13,6 +13,21 @@ module API get ':id/labels' do present available_labels, with: ::API::Entities::Label, current_user: current_user, project: user_project end + + desc 'Delete an existing label' do + success ::API::Entities::Label + end + params do + requires :name, type: String, desc: 'The name of the label to be deleted' + end + delete ':id/labels' do + authorize! :admin_label, user_project + + label = user_project.labels.find_by(title: params[:name]) + not_found!('Label') unless label + + present label.destroy, with: ::API::Entities::Label, current_user: current_user, project: user_project + end end end end diff --git a/lib/api/v3/members.rb b/lib/api/v3/members.rb index 19f276d5484..3d4972afd9d 100644 --- a/lib/api/v3/members.rb +++ b/lib/api/v3/members.rb @@ -119,6 +119,7 @@ module API # This is to ensure back-compatibility but 204 behavior should be used # for all DELETE endpoints in 9.0! if member.nil? + status(200 ) { message: "Access revoked", id: params[:user_id].to_i } else ::Members::DestroyService.new(source, current_user, declared_params).execute diff --git a/lib/api/v3/merge_requests.rb b/lib/api/v3/merge_requests.rb index 129f9d850e9..c6574a9104b 100644 --- a/lib/api/v3/merge_requests.rb +++ b/lib/api/v3/merge_requests.rb @@ -103,6 +103,8 @@ module API merge_request = find_project_merge_request(params[:merge_request_id]) authorize!(:destroy_merge_request, merge_request) + + status(200) merge_request.destroy end diff --git a/lib/api/v3/project_snippets.rb b/lib/api/v3/project_snippets.rb index e03e941d30b..809ca4f37ba 100644 --- a/lib/api/v3/project_snippets.rb +++ b/lib/api/v3/project_snippets.rb @@ -121,6 +121,8 @@ module API authorize! :admin_project_snippet, snippet snippet.destroy + + status(200) end desc 'Get a raw project snippet' diff --git a/lib/api/v3/projects.rb b/lib/api/v3/projects.rb index c3821555452..f3cd0b22e9b 100644 --- a/lib/api/v3/projects.rb +++ b/lib/api/v3/projects.rb @@ -359,6 +359,8 @@ module API desc 'Remove a project' delete ":id" do authorize! :remove_project, user_project + + status(200) ::Projects::DestroyService.new(user_project, current_user, {}).async_execute end @@ -384,6 +386,7 @@ module API authorize! :remove_fork_project, user_project if user_project.forked? + status(200) user_project.forked_project_link.destroy else not_modified! diff --git a/lib/api/v3/runners.rb b/lib/api/v3/runners.rb new file mode 100644 index 00000000000..8967141fe3d --- /dev/null +++ b/lib/api/v3/runners.rb @@ -0,0 +1,65 @@ +module API + module V3 + class Runners < Grape::API + include PaginationParams + + before { authenticate! } + + resource :runners do + desc 'Remove a runner' do + success ::API::Entities::Runner + end + params do + requires :id, type: Integer, desc: 'The ID of the runner' + end + delete ':id' do + runner = Ci::Runner.find(params[:id]) + not_found!('Runner') unless runner + + authenticate_delete_runner!(runner) + + status(200) + runner.destroy + end + end + + params do + requires :id, type: String, desc: 'The ID of a project' + end + resource :projects do + before { authorize_admin_project } + + desc "Disable project's runner" do + success ::API::Entities::Runner + end + params do + requires :runner_id, type: Integer, desc: 'The ID of the runner' + end + delete ':id/runners/:runner_id' do + runner_project = user_project.runner_projects.find_by(runner_id: params[:runner_id]) + not_found!('Runner') unless runner_project + + runner = runner_project.runner + forbidden!("Only one project associated with the runner. Please remove the runner instead") if runner.projects.count == 1 + + runner_project.destroy + + present runner, with: ::API::Entities::Runner + end + end + + helpers do + def authenticate_delete_runner!(runner) + return if current_user.is_admin? + forbidden!("Runner is shared") if runner.is_shared? + forbidden!("Runner associated with more than one project") if runner.projects.count > 1 + forbidden!("No access granted") unless user_can_access_runner?(runner) + end + + def user_can_access_runner?(runner) + current_user.ci_authorized_runners.exists?(runner.id) + end + end + end + end +end diff --git a/lib/api/v3/services.rb b/lib/api/v3/services.rb new file mode 100644 index 00000000000..af0a058f69b --- /dev/null +++ b/lib/api/v3/services.rb @@ -0,0 +1,573 @@ +module API + module V3 + class Services < Grape::API + services = { + 'asana' => [ + { + required: true, + name: :api_key, + type: String, + desc: 'User API token' + }, + { + required: false, + name: :restrict_to_branch, + type: String, + desc: 'Comma-separated list of branches which will be automatically inspected. Leave blank to include all branches' + } + ], + 'assembla' => [ + { + required: true, + name: :token, + type: String, + desc: 'The authentication token' + }, + { + required: false, + name: :subdomain, + type: String, + desc: 'Subdomain setting' + } + ], + 'bamboo' => [ + { + required: true, + name: :bamboo_url, + type: String, + desc: 'Bamboo root URL like https://bamboo.example.com' + }, + { + required: true, + name: :build_key, + type: String, + desc: 'Bamboo build plan key like' + }, + { + required: true, + name: :username, + type: String, + desc: 'A user with API access, if applicable' + }, + { + required: true, + name: :password, + type: String, + desc: 'Passord of the user' + } + ], + 'bugzilla' => [ + { + required: true, + name: :new_issue_url, + type: String, + desc: 'New issue URL' + }, + { + required: true, + name: :issues_url, + type: String, + desc: 'Issues URL' + }, + { + required: true, + name: :project_url, + type: String, + desc: 'Project URL' + }, + { + required: false, + name: :description, + type: String, + desc: 'Description' + }, + { + required: false, + name: :title, + type: String, + desc: 'Title' + } + ], + 'buildkite' => [ + { + required: true, + name: :token, + type: String, + desc: 'Buildkite project GitLab token' + }, + { + required: true, + name: :project_url, + type: String, + desc: 'The buildkite project URL' + }, + { + required: false, + name: :enable_ssl_verification, + type: Boolean, + desc: 'Enable SSL verification for communication' + } + ], + 'builds-email' => [ + { + required: true, + name: :recipients, + type: String, + desc: 'Comma-separated list of recipient email addresses' + }, + { + required: false, + name: :add_pusher, + type: Boolean, + desc: 'Add pusher to recipients list' + }, + { + required: false, + name: :notify_only_broken_builds, + type: Boolean, + desc: 'Notify only broken builds' + } + ], + 'campfire' => [ + { + required: true, + name: :token, + type: String, + desc: 'Campfire token' + }, + { + required: false, + name: :subdomain, + type: String, + desc: 'Campfire subdomain' + }, + { + required: false, + name: :room, + type: String, + desc: 'Campfire room' + } + ], + 'custom-issue-tracker' => [ + { + required: true, + name: :new_issue_url, + type: String, + desc: 'New issue URL' + }, + { + required: true, + name: :issues_url, + type: String, + desc: 'Issues URL' + }, + { + required: true, + name: :project_url, + type: String, + desc: 'Project URL' + }, + { + required: false, + name: :description, + type: String, + desc: 'Description' + }, + { + required: false, + name: :title, + type: String, + desc: 'Title' + } + ], + 'drone-ci' => [ + { + required: true, + name: :token, + type: String, + desc: 'Drone CI token' + }, + { + required: true, + name: :drone_url, + type: String, + desc: 'Drone CI URL' + }, + { + required: false, + name: :enable_ssl_verification, + type: Boolean, + desc: 'Enable SSL verification for communication' + } + ], + 'emails-on-push' => [ + { + required: true, + name: :recipients, + type: String, + desc: 'Comma-separated list of recipient email addresses' + }, + { + required: false, + name: :disable_diffs, + type: Boolean, + desc: 'Disable code diffs' + }, + { + required: false, + name: :send_from_committer_email, + type: Boolean, + desc: 'Send from committer' + } + ], + 'external-wiki' => [ + { + required: true, + name: :external_wiki_url, + type: String, + desc: 'The URL of the external Wiki' + } + ], + 'flowdock' => [ + { + required: true, + name: :token, + type: String, + desc: 'Flowdock token' + } + ], + 'gemnasium' => [ + { + required: true, + name: :api_key, + type: String, + desc: 'Your personal API key on gemnasium.com' + }, + { + required: true, + name: :token, + type: String, + desc: "The project's slug on gemnasium.com" + } + ], + 'hipchat' => [ + { + required: true, + name: :token, + type: String, + desc: 'The room token' + }, + { + required: false, + name: :room, + type: String, + desc: 'The room name or ID' + }, + { + required: false, + name: :color, + type: String, + desc: 'The room color' + }, + { + required: false, + name: :notify, + type: Boolean, + desc: 'Enable notifications' + }, + { + required: false, + name: :api_version, + type: String, + desc: 'Leave blank for default (v2)' + }, + { + required: false, + name: :server, + type: String, + desc: 'Leave blank for default. https://hipchat.example.com' + } + ], + 'irker' => [ + { + required: true, + name: :recipients, + type: String, + desc: 'Recipients/channels separated by whitespaces' + }, + { + required: false, + name: :default_irc_uri, + type: String, + desc: 'Default: irc://irc.network.net:6697' + }, + { + required: false, + name: :server_host, + type: String, + desc: 'Server host. Default localhost' + }, + { + required: false, + name: :server_port, + type: Integer, + desc: 'Server port. Default 6659' + }, + { + required: false, + name: :colorize_messages, + type: Boolean, + desc: 'Colorize messages' + } + ], + 'jira' => [ + { + required: true, + name: :url, + type: String, + desc: 'The URL to the JIRA project which is being linked to this GitLab project, e.g., https://jira.example.com' + }, + { + required: true, + name: :project_key, + type: String, + desc: 'The short identifier for your JIRA project, all uppercase, e.g., PROJ' + }, + { + required: false, + name: :username, + type: String, + desc: 'The username of the user created to be used with GitLab/JIRA' + }, + { + required: false, + name: :password, + type: String, + desc: 'The password of the user created to be used with GitLab/JIRA' + }, + { + required: false, + name: :jira_issue_transition_id, + type: Integer, + desc: 'The ID of a transition that moves issues to a closed state. You can find this number under the JIRA workflow administration (**Administration > Issues > Workflows**) by selecting **View** under **Operations** of the desired workflow of your project. The ID of each state can be found inside the parenthesis of each transition name under the **Transitions (id)** column ([see screenshot][trans]). By default, this ID is set to `2`' + } + ], + + 'kubernetes' => [ + { + required: true, + name: :namespace, + type: String, + desc: 'The Kubernetes namespace to use' + }, + { + required: true, + name: :api_url, + type: String, + desc: 'The URL to the Kubernetes cluster API, e.g., https://kubernetes.example.com' + }, + { + required: true, + name: :token, + type: String, + desc: 'The service token to authenticate against the Kubernetes cluster with' + }, + { + required: false, + name: :ca_pem, + type: String, + desc: 'A custom certificate authority bundle to verify the Kubernetes cluster with (PEM format)' + }, + ], + 'mattermost-slash-commands' => [ + { + required: true, + name: :token, + type: String, + desc: 'The Mattermost token' + } + ], + 'slack-slash-commands' => [ + { + required: true, + name: :token, + type: String, + desc: 'The Slack token' + } + ], + 'pipelines-email' => [ + { + required: true, + name: :recipients, + type: String, + desc: 'Comma-separated list of recipient email addresses' + }, + { + required: false, + name: :notify_only_broken_builds, + type: Boolean, + desc: 'Notify only broken builds' + } + ], + 'pivotaltracker' => [ + { + required: true, + name: :token, + type: String, + desc: 'The Pivotaltracker token' + }, + { + required: false, + name: :restrict_to_branch, + type: String, + desc: 'Comma-separated list of branches which will be automatically inspected. Leave blank to include all branches.' + } + ], + 'pushover' => [ + { + required: true, + name: :api_key, + type: String, + desc: 'The application key' + }, + { + required: true, + name: :user_key, + type: String, + desc: 'The user key' + }, + { + required: true, + name: :priority, + type: String, + desc: 'The priority' + }, + { + required: true, + name: :device, + type: String, + desc: 'Leave blank for all active devices' + }, + { + required: true, + name: :sound, + type: String, + desc: 'The sound of the notification' + } + ], + 'redmine' => [ + { + required: true, + name: :new_issue_url, + type: String, + desc: 'The new issue URL' + }, + { + required: true, + name: :project_url, + type: String, + desc: 'The project URL' + }, + { + required: true, + name: :issues_url, + type: String, + desc: 'The issues URL' + }, + { + required: false, + name: :description, + type: String, + desc: 'The description of the tracker' + } + ], + 'slack' => [ + { + required: true, + name: :webhook, + type: String, + desc: 'The Slack webhook. e.g. https://hooks.slack.com/services/...' + }, + { + required: false, + name: :new_issue_url, + type: String, + desc: 'The user name' + }, + { + required: false, + name: :channel, + type: String, + desc: 'The channel name' + } + ], + 'mattermost' => [ + { + required: true, + name: :webhook, + type: String, + desc: 'The Mattermost webhook. e.g. http://mattermost_host/hooks/...' + } + ], + 'teamcity' => [ + { + required: true, + name: :teamcity_url, + type: String, + desc: 'TeamCity root URL like https://teamcity.example.com' + }, + { + required: true, + name: :build_type, + type: String, + desc: 'Build configuration ID' + }, + { + required: true, + name: :username, + type: String, + desc: 'A user with permissions to trigger a manual build' + }, + { + required: true, + name: :password, + type: String, + desc: 'The password of the user' + } + ] + } + + resource :projects do + before { authenticate! } + before { authorize_admin_project } + + helpers do + def service_attributes(service) + service.fields.inject([]) do |arr, hash| + arr << hash[:name].to_sym + end + end + end + + desc "Delete a service for project" + params do + requires :service_slug, type: String, values: services.keys, desc: 'The name of the service' + end + delete ":id/services/:service_slug" do + service = user_project.find_or_initialize_service(params[:service_slug].underscore) + + attrs = service_attributes(service).inject({}) do |hash, key| + hash.merge!(key => nil) + end + + if service.update_attributes(attrs.merge(active: false)) + status(200) + true + else + render_api_error!('400 Bad Request', 400) + end + end + end + end + end +end diff --git a/lib/api/v3/system_hooks.rb b/lib/api/v3/system_hooks.rb index 391510b9ee0..5787c06fc12 100644 --- a/lib/api/v3/system_hooks.rb +++ b/lib/api/v3/system_hooks.rb @@ -13,6 +13,19 @@ module API get do present SystemHook.all, with: ::API::Entities::Hook end + + desc 'Delete a hook' do + success ::API::Entities::Hook + end + params do + requires :id, type: Integer, desc: 'The ID of the system hook' + end + delete ":id" do + hook = SystemHook.find_by(id: params[:id]) + not_found!('System hook') unless hook + + present hook.destroy, with: ::API::Entities::Hook + end end end end diff --git a/lib/api/v3/tags.rb b/lib/api/v3/tags.rb index 016e3d86932..6913720d9c5 100644 --- a/lib/api/v3/tags.rb +++ b/lib/api/v3/tags.rb @@ -14,6 +14,26 @@ module API tags = user_project.repository.tags.sort_by(&:name).reverse present tags, with: ::API::Entities::RepoTag, project: user_project end + + desc 'Delete a repository tag' + params do + requires :tag_name, type: String, desc: 'The name of the tag' + end + delete ":id/repository/tags/:tag_name", requirements: { tag_name: /.+/ } do + authorize_push_project + + result = ::Tags::DestroyService.new(user_project, current_user). + execute(params[:tag_name]) + + if result[:status] == :success + status(200) + { + tag_name: params[:tag_name] + } + else + render_api_error!(result[:message], result[:return_code]) + end + end end end end diff --git a/lib/api/v3/todos.rb b/lib/api/v3/todos.rb index 4f9b5fe72a6..e60cb25e57b 100644 --- a/lib/api/v3/todos.rb +++ b/lib/api/v3/todos.rb @@ -19,6 +19,8 @@ module API desc 'Mark all todos as done' delete do + status(200) + todos = TodosFinder.new(current_user, params).execute TodoService.new.mark_todos_as_done(todos, current_user) end diff --git a/lib/api/v3/triggers.rb b/lib/api/v3/triggers.rb new file mode 100644 index 00000000000..4051d4bca8d --- /dev/null +++ b/lib/api/v3/triggers.rb @@ -0,0 +1,30 @@ +module API + module V3 + class Triggers < Grape::API + include PaginationParams + + params do + requires :id, type: String, desc: 'The ID of a project' + end + resource :projects do + desc 'Delete a trigger' do + success ::API::Entities::Trigger + end + params do + requires :token, type: String, desc: 'The unique token of trigger' + end + delete ':id/triggers/:token' do + authenticate! + authorize! :admin_build, user_project + + trigger = user_project.triggers.find_by(token: params[:token].to_s) + return not_found!('Trigger') unless trigger + + trigger.destroy + + present trigger, with: ::API::Entities::Trigger + end + end + end + end +end diff --git a/lib/api/v3/users.rb b/lib/api/v3/users.rb index 7838cdc46a7..14f54731730 100644 --- a/lib/api/v3/users.rb +++ b/lib/api/v3/users.rb @@ -92,6 +92,25 @@ module API present paginate(events), with: ::API::V3::Entities::Event end + + desc 'Delete an existing SSH key from a specified user. Available only for admins.' do + success ::API::Entities::SSHKey + end + params do + requires :id, type: Integer, desc: 'The ID of the user' + requires :key_id, type: Integer, desc: 'The ID of the SSH key' + end + delete ':id/keys/:key_id' do + authenticated_as_admin! + + user = User.find_by(id: params[:id]) + not_found!('User') unless user + + key = user.keys.find_by(id: params[:key_id]) + not_found!('Key') unless key + + present key.destroy, with: ::API::Entities::SSHKey + end end resource :user do @@ -111,6 +130,19 @@ module API get "emails" do present current_user.emails, with: ::API::Entities::Email end + + desc 'Delete an SSH key from the currently authenticated user' do + success ::API::Entities::SSHKey + end + params do + requires :key_id, type: Integer, desc: 'The ID of the SSH key' + end + delete "keys/:key_id" do + key = current_user.keys.find_by(id: params[:key_id]) + not_found!('Key') unless key + + present key.destroy, with: ::API::Entities::SSHKey + end end end end diff --git a/lib/api/v3/variables.rb b/lib/api/v3/variables.rb new file mode 100644 index 00000000000..0f55a14fb28 --- /dev/null +++ b/lib/api/v3/variables.rb @@ -0,0 +1,29 @@ +module API + module V3 + class Variables < Grape::API + include PaginationParams + + before { authenticate! } + before { authorize! :admin_build, user_project } + + params do + requires :id, type: String, desc: 'The ID of a project' + end + + resource :projects do + desc 'Delete an existing variable from a project' do + success ::API::Entities::Variable + end + params do + requires :key, type: String, desc: 'The key of the variable' + end + delete ':id/variables/:key' do + variable = user_project.variables.find_by(key: params[:key]) + not_found!('Variable') unless variable + + present variable.destroy, with: ::API::Entities::Variable + end + end + end + end +end diff --git a/lib/api/variables.rb b/lib/api/variables.rb index f623b1dfe9f..77e5d54c225 100644 --- a/lib/api/variables.rb +++ b/lib/api/variables.rb @@ -1,5 +1,4 @@ module API - # Projects variables API class Variables < Grape::API include PaginationParams @@ -81,10 +80,9 @@ module API end delete ':id/variables/:key' do variable = user_project.variables.find_by(key: params[:key]) + not_found!('Variable') unless variable - return not_found!('Variable') unless variable - - present variable.destroy, with: Entities::Variable + variable.destroy end end end diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb index 0e17ac24d5a..b51e76d93f2 100644 --- a/lib/ci/api/builds.rb +++ b/lib/ci/api/builds.rb @@ -217,6 +217,7 @@ module Ci build = Ci::Build.find_by_id(params[:id]) authenticate_build!(build) + status(200) build.erase_artifacts! end end diff --git a/lib/ci/api/runners.rb b/lib/ci/api/runners.rb index 2a611a67eaf..c1fd959ef14 100644 --- a/lib/ci/api/runners.rb +++ b/lib/ci/api/runners.rb @@ -8,6 +8,8 @@ module Ci end delete "delete" do authenticate_runner! + + status(200) Ci::Runner.find_by_token(params[:token]).destroy end diff --git a/spec/requests/api/access_requests_spec.rb b/spec/requests/api/access_requests_spec.rb index 919c98d6437..46edbd49b28 100644 --- a/spec/requests/api/access_requests_spec.rb +++ b/spec/requests/api/access_requests_spec.rb @@ -200,7 +200,7 @@ describe API::AccessRequests, api: true do expect do delete api("/#{source_type.pluralize}/#{source.id}/access_requests/#{access_requester.id}", access_requester) - expect(response).to have_http_status(200) + expect(response).to have_http_status(204) end.to change { source.requesters.count }.by(-1) end end @@ -210,7 +210,7 @@ describe API::AccessRequests, api: true do expect do delete api("/#{source_type.pluralize}/#{source.id}/access_requests/#{access_requester.id}", master) - expect(response).to have_http_status(200) + expect(response).to have_http_status(204) end.to change { source.requesters.count }.by(-1) end diff --git a/spec/requests/api/award_emoji_spec.rb b/spec/requests/api/award_emoji_spec.rb index 6cc1ef315db..9756991162e 100644 --- a/spec/requests/api/award_emoji_spec.rb +++ b/spec/requests/api/award_emoji_spec.rb @@ -242,9 +242,9 @@ describe API::AwardEmoji, api: true do it 'deletes the award' do expect do delete api("/projects/#{project.id}/issues/#{issue.id}/award_emoji/#{award_emoji.id}", user) - end.to change { issue.award_emoji.count }.from(1).to(0) - expect(response).to have_http_status(200) + expect(response).to have_http_status(204) + end.to change { issue.award_emoji.count }.from(1).to(0) end it 'returns a 404 error when the award emoji can not be found' do @@ -258,9 +258,9 @@ describe API::AwardEmoji, api: true do it 'deletes the award' do expect do delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}/award_emoji/#{downvote.id}", user) - end.to change { merge_request.award_emoji.count }.from(1).to(0) - expect(response).to have_http_status(200) + expect(response).to have_http_status(204) + end.to change { merge_request.award_emoji.count }.from(1).to(0) end it 'returns a 404 error when note id not found' do @@ -277,9 +277,9 @@ describe API::AwardEmoji, api: true do it 'deletes the award' do expect do delete api("/projects/#{project.id}/snippets/#{snippet.id}/award_emoji/#{award.id}", user) - end.to change { snippet.award_emoji.count }.from(1).to(0) - expect(response).to have_http_status(200) + expect(response).to have_http_status(204) + end.to change { snippet.award_emoji.count }.from(1).to(0) end end end @@ -290,9 +290,9 @@ describe API::AwardEmoji, api: true do it 'deletes the award' do expect do delete api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji/#{rocket.id}", user) - end.to change { note.award_emoji.count }.from(1).to(0) - expect(response).to have_http_status(200) + expect(response).to have_http_status(204) + end.to change { note.award_emoji.count }.from(1).to(0) end end end diff --git a/spec/requests/api/boards_spec.rb b/spec/requests/api/boards_spec.rb index 71df534ebe1..87c36639cd4 100644 --- a/spec/requests/api/boards_spec.rb +++ b/spec/requests/api/boards_spec.rb @@ -195,8 +195,7 @@ describe API::Boards, api: true do it "deletes the list if an admin requests it" do delete api("#{base_url}/#{dev_list.id}", owner) - expect(response).to have_http_status(200) - expect(json_response['position']).to eq(1) + expect(response).to have_http_status(204) end end end diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb index cacdb21c692..ab5a7e4d3de 100644 --- a/spec/requests/api/branches_spec.rb +++ b/spec/requests/api/branches_spec.rb @@ -325,15 +325,14 @@ describe API::Branches, api: true do it "removes branch" do delete api("/projects/#{project.id}/repository/branches/#{branch_name}", user) - expect(response).to have_http_status(200) - expect(json_response['branch']).to eq(branch_name) + + expect(response).to have_http_status(204) end it "removes a branch with dots in the branch name" do delete api("/projects/#{project.id}/repository/branches/with.1.2.3", user) - expect(response).to have_http_status(200) - expect(json_response['branch']).to eq("with.1.2.3") + expect(response).to have_http_status(204) end it 'returns 404 if branch not exists' do diff --git a/spec/requests/api/broadcast_messages_spec.rb b/spec/requests/api/broadcast_messages_spec.rb index 921d8714173..024fa66848c 100644 --- a/spec/requests/api/broadcast_messages_spec.rb +++ b/spec/requests/api/broadcast_messages_spec.rb @@ -174,8 +174,11 @@ describe API::BroadcastMessages, api: true do end it 'deletes the broadcast message for admins' do - expect { delete api("/broadcast_messages/#{message.id}", admin) } - .to change { BroadcastMessage.count }.by(-1) + expect do + delete api("/broadcast_messages/#{message.id}", admin) + + expect(response).to have_http_status(204) + end.to change { BroadcastMessage.count }.by(-1) end end end diff --git a/spec/requests/api/deploy_keys_spec.rb b/spec/requests/api/deploy_keys_spec.rb index 7e682e91bd1..4f4b18cf0e0 100644 --- a/spec/requests/api/deploy_keys_spec.rb +++ b/spec/requests/api/deploy_keys_spec.rb @@ -116,6 +116,8 @@ describe API::DeployKeys, api: true do it 'should delete existing key' do expect do delete api("/projects/#{project.id}/deploy_keys/#{deploy_key.id}", admin) + + expect(response).to have_http_status(204) end.to change{ project.deploy_keys.count }.by(-1) end diff --git a/spec/requests/api/environments_spec.rb b/spec/requests/api/environments_spec.rb index d0958d39d44..d66eb63fd0a 100644 --- a/spec/requests/api/environments_spec.rb +++ b/spec/requests/api/environments_spec.rb @@ -122,7 +122,7 @@ describe API::Environments, api: true do it 'returns a 200 for an existing environment' do delete api("/projects/#{project.id}/environments/#{environment.id}", user) - expect(response).to have_http_status(200) + expect(response).to have_http_status(204) end it 'returns a 404 for non existing id' do diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index 29d67b5259e..31b1aca6d73 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -201,11 +201,7 @@ describe API::Files, api: true do it "deletes existing file in project repo" do delete api("/projects/#{project.id}/repository/files", user), valid_params - expect(response).to have_http_status(200) - expect(json_response['file_path']).to eq(file_path) - last_commit = project.repository.commit.raw - expect(last_commit.author_email).to eq(user.email) - expect(last_commit.author_name).to eq(user.name) + expect(response).to have_http_status(204) end it "returns a 400 bad request if no params given" do @@ -228,10 +224,7 @@ describe API::Files, api: true do delete api("/projects/#{project.id}/repository/files", user), valid_params - expect(response).to have_http_status(200) - last_commit = project.repository.commit.raw - expect(last_commit.author_email).to eq(author_email) - expect(last_commit.author_name).to eq(author_name) + expect(response).to have_http_status(204) end end end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index fb3dc1b074e..b0ba3ea912d 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -467,7 +467,7 @@ describe API::Groups, api: true do it "removes group" do delete api("/groups/#{group1.id}", user1) - expect(response).to have_http_status(200) + expect(response).to have_http_status(204) end it "does not remove a group if not an owner" do @@ -496,7 +496,7 @@ describe API::Groups, api: true do it "removes any existing group" do delete api("/groups/#{group2.id}", admin) - expect(response).to have_http_status(200) + expect(response).to have_http_status(204) end it "does not remove a non existing group" do diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 7cb75310204..ddc2e51821e 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -1175,8 +1175,8 @@ describe API::Issues, api: true do it "deletes the issue if an admin requests it" do delete api("/projects/#{project.id}/issues/#{issue.id}", owner) - expect(response).to have_http_status(200) - expect(json_response['state']).to eq 'opened' + + expect(response).to have_http_status(204) end end diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb index af271dbd4f5..a1adaba7b98 100644 --- a/spec/requests/api/labels_spec.rb +++ b/spec/requests/api/labels_spec.rb @@ -175,9 +175,10 @@ describe API::Labels, api: true do end describe 'DELETE /projects/:id/labels' do - it 'returns 200 for existing label' do + it 'returns 204 for existing label' do delete api("/projects/#{project.id}/labels", user), name: 'label1' - expect(response).to have_http_status(200) + + expect(response).to have_http_status(204) end it 'returns 404 for non existing label' do diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index 127498ed109..2d37d026a39 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -263,18 +263,18 @@ describe API::Members, api: true do expect do delete api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", developer) - expect(response).to have_http_status(200) + expect(response).to have_http_status(204) end.to change { source.members.count }.by(-1) end end context 'when authenticated as a master/owner' do context 'and member is a requester' do - it "returns #{source_type == 'project' ? 200 : 404}" do + it 'returns 404' do expect do delete api("/#{source_type.pluralize}/#{source.id}/members/#{access_requester.id}", master) - expect(response).to have_http_status(source_type == 'project' ? 200 : 404) + expect(response).to have_http_status(404) end.not_to change { source.requesters.count } end end @@ -283,15 +283,15 @@ describe API::Members, api: true do expect do delete api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", master) - expect(response).to have_http_status(200) + expect(response).to have_http_status(204) end.to change { source.members.count }.by(-1) end end - it "returns #{source_type == 'project' ? 200 : 404} if member does not exist" do + it 'returns 404 if member does not exist' do delete api("/#{source_type.pluralize}/#{source.id}/members/123", master) - expect(response).to have_http_status(source_type == 'project' ? 200 : 404) + expect(response).to have_http_status(404) end end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index b87d0cd7de9..5522154899c 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -411,7 +411,7 @@ describe API::MergeRequests, api: true do it "destroys the merge request owners can destroy" do delete api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user) - expect(response).to have_http_status(200) + expect(response).to have_http_status(204) end end end diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index 3cca4468be7..9d3c821b692 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -373,7 +373,7 @@ describe API::Notes, api: true do delete api("/projects/#{project.id}/issues/#{issue.id}/"\ "notes/#{issue_note.id}", user) - expect(response).to have_http_status(200) + expect(response).to have_http_status(204) # Check if note is really deleted delete api("/projects/#{project.id}/issues/#{issue.id}/"\ "notes/#{issue_note.id}", user) @@ -392,7 +392,7 @@ describe API::Notes, api: true do delete api("/projects/#{project.id}/snippets/#{snippet.id}/"\ "notes/#{snippet_note.id}", user) - expect(response).to have_http_status(200) + expect(response).to have_http_status(204) # Check if note is really deleted delete api("/projects/#{project.id}/snippets/#{snippet.id}/"\ "notes/#{snippet_note.id}", user) @@ -412,7 +412,7 @@ describe API::Notes, api: true do delete api("/projects/#{project.id}/merge_requests/"\ "#{merge_request.id}/notes/#{merge_request_note.id}", user) - expect(response).to have_http_status(200) + expect(response).to have_http_status(204) # Check if note is really deleted delete api("/projects/#{project.id}/merge_requests/"\ "#{merge_request.id}/notes/#{merge_request_note.id}", user) diff --git a/spec/requests/api/project_hooks_spec.rb b/spec/requests/api/project_hooks_spec.rb index 20c76bd2c05..f286568547d 100644 --- a/spec/requests/api/project_hooks_spec.rb +++ b/spec/requests/api/project_hooks_spec.rb @@ -183,13 +183,9 @@ describe API::ProjectHooks, 'ProjectHooks', api: true do it "deletes hook from project" do expect do delete api("/projects/#{project.id}/hooks/#{hook.id}", user) - end.to change {project.hooks.count}.by(-1) - expect(response).to have_http_status(200) - end - it "returns success when deleting hook" do - delete api("/projects/#{project.id}/hooks/#{hook.id}", user) - expect(response).to have_http_status(200) + expect(response).to have_http_status(204) + end.to change {project.hooks.count}.by(-1) end it "returns a 404 error when deleting non existent hook" do diff --git a/spec/requests/api/project_snippets_spec.rb b/spec/requests/api/project_snippets_spec.rb index da9df56401b..2c4602faf2c 100644 --- a/spec/requests/api/project_snippets_spec.rb +++ b/spec/requests/api/project_snippets_spec.rb @@ -189,7 +189,7 @@ describe API::ProjectSnippets, api: true do delete api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/", admin) - expect(response).to have_http_status(200) + expect(response).to have_http_status(204) end it 'returns 404 for invalid snippet id' do @@ -212,7 +212,7 @@ describe API::ProjectSnippets, api: true do end it 'returns 404 for invalid snippet id' do - delete api("/projects/#{snippet.project.id}/snippets/1234", admin) + get api("/projects/#{snippet.project.id}/snippets/1234/raw", admin) expect(response).to have_http_status(404) expect(json_response['message']).to eq('404 Snippet Not Found') diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 5de4426f3bd..d2acce90a24 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -820,8 +820,9 @@ describe API::Projects, api: true do it 'deletes existing project snippet' do expect do delete api("/projects/#{project.id}/snippets/#{snippet.id}", user) + + expect(response).to have_http_status(204) end.to change { Snippet.count }.by(-1) - expect(response).to have_http_status(200) end it 'returns 404 when deleting unknown snippet id' do @@ -905,8 +906,10 @@ describe API::Projects, api: true do project_fork_target.reload expect(project_fork_target.forked_from_project).not_to be_nil expect(project_fork_target.forked?).to be_truthy + delete api("/projects/#{project_fork_target.id}/fork", admin) - expect(response).to have_http_status(200) + + expect(response).to have_http_status(204) project_fork_target.reload expect(project_fork_target.forked_from_project).to be_nil expect(project_fork_target.forked?).not_to be_truthy diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index 73e82647ca0..e83202e4196 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -123,6 +123,7 @@ describe API::Runner do context 'when no token is provided' do it 'returns 400 error' do delete api('/runners') + expect(response).to have_http_status 400 end end @@ -130,6 +131,7 @@ describe API::Runner do context 'when invalid token is provided' do it 'returns 403 error' do delete api('/runners'), token: 'invalid' + expect(response).to have_http_status 403 end end @@ -139,7 +141,8 @@ describe API::Runner do it 'deletes Runner' do delete api('/runners'), token: runner.token - expect(response).to have_http_status 200 + + expect(response).to have_http_status 204 expect(Ci::Runner.count).to eq(0) end end diff --git a/spec/requests/api/runners_spec.rb b/spec/requests/api/runners_spec.rb index 103d6755888..8a82543a830 100644 --- a/spec/requests/api/runners_spec.rb +++ b/spec/requests/api/runners_spec.rb @@ -277,8 +277,9 @@ describe API::Runners, api: true do it 'deletes runner' do expect do delete api("/runners/#{shared_runner.id}", admin) + + expect(response).to have_http_status(204) end.to change{ Ci::Runner.shared.count }.by(-1) - expect(response).to have_http_status(200) end end @@ -286,15 +287,17 @@ describe API::Runners, api: true do it 'deletes unused runner' do expect do delete api("/runners/#{unused_specific_runner.id}", admin) + + expect(response).to have_http_status(204) end.to change{ Ci::Runner.specific.count }.by(-1) - expect(response).to have_http_status(200) end it 'deletes used runner' do expect do delete api("/runners/#{specific_runner.id}", admin) + + expect(response).to have_http_status(204) end.to change{ Ci::Runner.specific.count }.by(-1) - expect(response).to have_http_status(200) end end @@ -327,8 +330,9 @@ describe API::Runners, api: true do it 'deletes runner for one owned project' do expect do delete api("/runners/#{specific_runner.id}", user) + + expect(response).to have_http_status(204) end.to change{ Ci::Runner.specific.count }.by(-1) - expect(response).to have_http_status(200) end end end @@ -457,8 +461,9 @@ describe API::Runners, api: true do it "disables project's runner" do expect do delete api("/projects/#{project.id}/runners/#{two_projects_runner.id}", user) + + expect(response).to have_http_status(204) end.to change{ project.runners.count }.by(-1) - expect(response).to have_http_status(200) end end diff --git a/spec/requests/api/services_spec.rb b/spec/requests/api/services_spec.rb index 776dc655650..fd334934ca5 100644 --- a/spec/requests/api/services_spec.rb +++ b/spec/requests/api/services_spec.rb @@ -55,7 +55,7 @@ describe API::Services, api: true do it "deletes #{service}" do delete api("/projects/#{project.id}/services/#{dashed_service}", user) - expect(response).to have_http_status(200) + expect(response).to have_http_status(204) project.send(service_method).reload expect(project.send(service_method).activated?).to be_falsey end diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index 41def7cd1d4..5219f6eed42 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -74,7 +74,7 @@ describe API::Snippets, api: true do end it 'returns 404 for invalid snippet id' do - delete api("/snippets/1234", user) + get api("/snippets/1234/raw", user) expect(response).to have_http_status(404) expect(json_response['message']).to eq('404 Snippet Not Found') diff --git a/spec/requests/api/system_hooks_spec.rb b/spec/requests/api/system_hooks_spec.rb index b59da632c00..d1e10f12657 100644 --- a/spec/requests/api/system_hooks_spec.rb +++ b/spec/requests/api/system_hooks_spec.rb @@ -91,6 +91,8 @@ describe API::SystemHooks, api: true do it "deletes a hook" do expect do delete api("/hooks/#{hook.id}", admin) + + expect(response).to have_http_status(204) end.to change { SystemHook.count }.by(-1) end diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb index 8a4f078182f..b132d033a61 100644 --- a/spec/requests/api/tags_spec.rb +++ b/spec/requests/api/tags_spec.rb @@ -137,8 +137,8 @@ describe API::Tags, api: true do context 'delete tag' do it 'deletes an existing tag' do delete api("/projects/#{project.id}/repository/tags/#{tag_name}", user) - expect(response).to have_http_status(200) - expect(json_response['tag_name']).to eq(tag_name) + + expect(response).to have_http_status(204) end it 'raises 404 if the tag does not exist' do diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb index 92dfc2aa277..153e2791cbe 100644 --- a/spec/requests/api/triggers_spec.rb +++ b/spec/requests/api/triggers_spec.rb @@ -190,8 +190,9 @@ describe API::Triggers do it 'deletes trigger' do expect do delete api("/projects/#{project.id}/triggers/#{trigger.token}", user) + + expect(response).to have_http_status(204) end.to change{project.triggers.count}.by(-1) - expect(response).to have_http_status(200) end it 'responds with 404 Not Found if requesting non-existing trigger' do diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 603da9f49fc..e5e4c84755f 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -540,10 +540,12 @@ describe API::Users, api: true do it 'deletes existing key' do user.keys << key user.save + expect do delete api("/users/#{user.id}/keys/#{key.id}", admin) + + expect(response).to have_http_status(204) end.to change { user.keys.count }.by(-1) - expect(response).to have_http_status(200) end it 'returns 404 error if user not found' do @@ -637,10 +639,12 @@ describe API::Users, api: true do it 'deletes existing email' do user.emails << email user.save + expect do delete api("/users/#{user.id}/emails/#{email.id}", admin) + + expect(response).to have_http_status(204) end.to change { user.emails.count }.by(-1) - expect(response).to have_http_status(200) end it 'returns 404 error if user not found' do @@ -671,10 +675,10 @@ describe API::Users, api: true do it "deletes user" do delete api("/users/#{user.id}", admin) - expect(response).to have_http_status(200) + + expect(response).to have_http_status(204) expect { User.find(user.id) }.to raise_error ActiveRecord::RecordNotFound expect { Namespace.find(namespace.id) }.to raise_error ActiveRecord::RecordNotFound - expect(json_response['email']).to eq(user.email) end it "does not delete for unauthenticated user" do @@ -869,10 +873,12 @@ describe API::Users, api: true do it "deletes existed key" do user.keys << key user.save + expect do delete api("/user/keys/#{key.id}", user) + + expect(response).to have_http_status(204) end.to change{user.keys.count}.by(-1) - expect(response).to have_http_status(200) end it "returns 404 if key ID not found" do @@ -976,10 +982,12 @@ describe API::Users, api: true do it "deletes existed email" do user.emails << email user.save + expect do delete api("/user/emails/#{email.id}", user) + + expect(response).to have_http_status(204) end.to change{user.emails.count}.by(-1) - expect(response).to have_http_status(200) end it "returns 404 if email ID not found" do diff --git a/spec/requests/api/v3/award_emoji_spec.rb b/spec/requests/api/v3/award_emoji_spec.rb new file mode 100644 index 00000000000..91145c8e72c --- /dev/null +++ b/spec/requests/api/v3/award_emoji_spec.rb @@ -0,0 +1,74 @@ +require 'spec_helper' + +describe API::V3::AwardEmoji, api: true do + include ApiHelpers + + let(:user) { create(:user) } + let!(:project) { create(:empty_project) } + let(:issue) { create(:issue, project: project) } + let!(:award_emoji) { create(:award_emoji, awardable: issue, user: user) } + let!(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + let!(:downvote) { create(:award_emoji, :downvote, awardable: merge_request, user: user) } + let!(:note) { create(:note, project: project, noteable: issue) } + + before { project.team << [user, :master] } + + describe 'DELETE /projects/:id/awardable/:awardable_id/award_emoji/:award_id' do + context 'when the awardable is an Issue' do + it 'deletes the award' do + expect do + delete v3_api("/projects/#{project.id}/issues/#{issue.id}/award_emoji/#{award_emoji.id}", user) + + expect(response).to have_http_status(200) + end.to change { issue.award_emoji.count }.from(1).to(0) + end + + it 'returns a 404 error when the award emoji can not be found' do + delete v3_api("/projects/#{project.id}/issues/#{issue.id}/award_emoji/12345", user) + + expect(response).to have_http_status(404) + end + end + + context 'when the awardable is a Merge Request' do + it 'deletes the award' do + expect do + delete v3_api("/projects/#{project.id}/merge_requests/#{merge_request.id}/award_emoji/#{downvote.id}", user) + + expect(response).to have_http_status(200) + end.to change { merge_request.award_emoji.count }.from(1).to(0) + end + + it 'returns a 404 error when note id not found' do + delete v3_api("/projects/#{project.id}/merge_requests/#{merge_request.id}/notes/12345", user) + + expect(response).to have_http_status(404) + end + end + + context 'when the awardable is a Snippet' do + let(:snippet) { create(:project_snippet, :public, project: project) } + let!(:award) { create(:award_emoji, awardable: snippet, user: user) } + + it 'deletes the award' do + expect do + delete v3_api("/projects/#{project.id}/snippets/#{snippet.id}/award_emoji/#{award.id}", user) + + expect(response).to have_http_status(200) + end.to change { snippet.award_emoji.count }.from(1).to(0) + end + end + end + + describe 'DELETE /projects/:id/awardable/:awardable_id/award_emoji/:award_emoji_id' do + let!(:rocket) { create(:award_emoji, awardable: note, name: 'rocket', user: user) } + + it 'deletes the award' do + expect do + delete v3_api("/projects/#{project.id}/issues/#{issue.id}/notes/#{note.id}/award_emoji/#{rocket.id}", user) + + expect(response).to have_http_status(200) + end.to change { note.award_emoji.count }.from(1).to(0) + end + end +end diff --git a/spec/requests/api/v3/boards_spec.rb b/spec/requests/api/v3/boards_spec.rb index 8aaf3be4f87..eb95934f354 100644 --- a/spec/requests/api/v3/boards_spec.rb +++ b/spec/requests/api/v3/boards_spec.rb @@ -5,6 +5,7 @@ describe API::V3::Boards, api: true do let(:user) { create(:user) } let(:guest) { create(:user) } + let(:non_member) { create(:user) } let!(:project) { create(:empty_project, :public, creator_id: user.id, namespace: user.namespace ) } let!(:dev_label) do @@ -76,4 +77,37 @@ describe API::V3::Boards, api: true do expect(response).to have_http_status(404) end end + + describe "DELETE /projects/:id/board/lists/:list_id" do + let(:base_url) { "/projects/#{project.id}/boards/#{board.id}/lists" } + + it "rejects a non member from deleting a list" do + delete v3_api("#{base_url}/#{dev_list.id}", non_member) + + expect(response).to have_http_status(403) + end + + it "rejects a user with guest role from deleting a list" do + delete v3_api("#{base_url}/#{dev_list.id}", guest) + + expect(response).to have_http_status(403) + end + + it "returns 404 error if list id not found" do + delete v3_api("#{base_url}/44444", user) + + expect(response).to have_http_status(404) + end + + context "when the user is project owner" do + let(:owner) { create(:user) } + let(:project) { create(:empty_project, namespace: owner.namespace) } + + it "deletes the list if an admin requests it" do + delete v3_api("#{base_url}/#{dev_list.id}", owner) + + expect(response).to have_http_status(200) + end + end + end end diff --git a/spec/requests/api/v3/branches_spec.rb b/spec/requests/api/v3/branches_spec.rb index a3e1581fcc5..e4cedf98e64 100644 --- a/spec/requests/api/v3/branches_spec.rb +++ b/spec/requests/api/v3/branches_spec.rb @@ -5,8 +5,12 @@ describe API::V3::Branches, api: true do include ApiHelpers let(:user) { create(:user) } + let(:user2) { create(:user) } let!(:project) { create(:project, :repository, creator: user) } let!(:master) { create(:project_member, :master, user: user, project: project) } + let!(:guest) { create(:project_member, :guest, user: user2, project: project) } + let!(:branch_name) { 'feature' } + let!(:branch_with_dot) { CreateBranchService.new(project, user).execute("with.1.2.3", "master") } describe "GET /projects/:id/repository/branches" do it "returns an array of project branches" do @@ -21,6 +25,44 @@ describe API::V3::Branches, api: true do end end + describe "DELETE /projects/:id/repository/branches/:branch" do + before do + allow_any_instance_of(Repository).to receive(:rm_branch).and_return(true) + end + + it "removes branch" do + delete v3_api("/projects/#{project.id}/repository/branches/#{branch_name}", user) + + expect(response).to have_http_status(200) + expect(json_response['branch_name']).to eq(branch_name) + end + + it "removes a branch with dots in the branch name" do + delete v3_api("/projects/#{project.id}/repository/branches/with.1.2.3", user) + + expect(response).to have_http_status(200) + expect(json_response['branch_name']).to eq("with.1.2.3") + end + + it 'returns 404 if branch not exists' do + delete v3_api("/projects/#{project.id}/repository/branches/foobar", user) + expect(response).to have_http_status(404) + end + + it "removes protected branch" do + create(:protected_branch, project: project, name: branch_name) + delete v3_api("/projects/#{project.id}/repository/branches/#{branch_name}", user) + expect(response).to have_http_status(405) + expect(json_response['message']).to eq('Protected branch cant be removed') + end + + it "does not remove HEAD branch" do + delete v3_api("/projects/#{project.id}/repository/branches/master", user) + expect(response).to have_http_status(405) + expect(json_response['message']).to eq('Cannot remove HEAD branch') + end + end + describe "DELETE /projects/:id/repository/merged_branches" do before do allow_any_instance_of(Repository).to receive(:rm_branch).and_return(true) @@ -33,10 +75,7 @@ describe API::V3::Branches, api: true do end it 'returns a 403 error if guest' do - user_b = create :user - create(:project_member, :guest, user: user_b, project: project) - - delete v3_api("/projects/#{project.id}/repository/merged_branches", user_b) + delete v3_api("/projects/#{project.id}/repository/merged_branches", user2) expect(response).to have_http_status(403) end diff --git a/spec/requests/api/v3/broadcast_messages_spec.rb b/spec/requests/api/v3/broadcast_messages_spec.rb new file mode 100644 index 00000000000..06556401a29 --- /dev/null +++ b/spec/requests/api/v3/broadcast_messages_spec.rb @@ -0,0 +1,34 @@ +require 'spec_helper' + +describe API::V3::BroadcastMessages, api: true do + include ApiHelpers + + let(:user) { create(:user) } + let(:admin) { create(:admin) } + + describe 'DELETE /broadcast_messages/:id' do + let!(:message) { create(:broadcast_message) } + + it 'returns a 401 for anonymous users' do + delete v3_api("/broadcast_messages/#{message.id}"), + attributes_for(:broadcast_message) + + expect(response).to have_http_status(401) + end + + it 'returns a 403 for users' do + delete v3_api("/broadcast_messages/#{message.id}", user), + attributes_for(:broadcast_message) + + expect(response).to have_http_status(403) + end + + it 'deletes the broadcast message for admins' do + expect do + delete v3_api("/broadcast_messages/#{message.id}", admin) + + expect(response).to have_http_status(200) + end.to change { BroadcastMessage.count }.by(-1) + end + end +end diff --git a/spec/requests/api/v3/environments_spec.rb b/spec/requests/api/v3/environments_spec.rb new file mode 100644 index 00000000000..1ac666ab240 --- /dev/null +++ b/spec/requests/api/v3/environments_spec.rb @@ -0,0 +1,39 @@ +require 'spec_helper' + +describe API::V3::Environments, api: true do + include ApiHelpers + + let(:user) { create(:user) } + let(:non_member) { create(:user) } + let(:project) { create(:empty_project, :private, namespace: user.namespace) } + let!(:environment) { create(:environment, project: project) } + + before do + project.team << [user, :master] + end + + describe 'DELETE /projects/:id/environments/:environment_id' do + context 'as a master' do + it 'returns a 200 for an existing environment' do + delete v3_api("/projects/#{project.id}/environments/#{environment.id}", user) + + expect(response).to have_http_status(200) + end + + it 'returns a 404 for non existing id' do + delete v3_api("/projects/#{project.id}/environments/12345", user) + + expect(response).to have_http_status(404) + expect(json_response['message']).to eq('404 Not found') + end + end + + context 'a non member' do + it 'rejects the request' do + delete v3_api("/projects/#{project.id}/environments/#{environment.id}", non_member) + + expect(response).to have_http_status(404) + end + end + end +end diff --git a/spec/requests/api/v3/files_spec.rb b/spec/requests/api/v3/files_spec.rb index 52fd908af7d..93637053626 100644 --- a/spec/requests/api/v3/files_spec.rb +++ b/spec/requests/api/v3/files_spec.rb @@ -2,17 +2,6 @@ require 'spec_helper' describe API::V3::Files, api: true do include ApiHelpers - let(:user) { create(:user) } - let!(:project) { create(:project, :repository, namespace: user.namespace ) } - let(:guest) { create(:user) { |u| project.add_guest(u) } } - let(:file_path) { 'files/ruby/popen.rb' } - let(:params) do - { - file_path: file_path, - ref: 'master' - } - end - let(:author_email) { FFaker::Internet.email } # I have to remove periods from the end of the name # This happened when the user's name had a suffix (i.e. "Sr.") @@ -26,6 +15,18 @@ describe API::V3::Files, api: true do # ... # Author: Foo Sr <foo@example.com> # ... + + let(:user) { create(:user) } + let!(:project) { create(:project, :repository, namespace: user.namespace ) } + let(:guest) { create(:user) { |u| project.add_guest(u) } } + let(:file_path) { 'files/ruby/popen.rb' } + let(:params) do + { + file_path: file_path, + ref: 'master' + } + end + let(:author_email) { FFaker::Internet.email } let(:author_name) { FFaker::Name.name.chomp("\.") } before { project.team << [user, :developer] } diff --git a/spec/requests/api/v3/labels_spec.rb b/spec/requests/api/v3/labels_spec.rb index f44403374e9..dfac357d37c 100644 --- a/spec/requests/api/v3/labels_spec.rb +++ b/spec/requests/api/v3/labels_spec.rb @@ -149,4 +149,23 @@ describe API::V3::Labels, api: true do end end end + + describe 'DELETE /projects/:id/labels' do + it 'returns 200 for existing label' do + delete v3_api("/projects/#{project.id}/labels", user), name: 'label1' + + expect(response).to have_http_status(200) + end + + it 'returns 404 for non existing label' do + delete v3_api("/projects/#{project.id}/labels", user), name: 'label2' + expect(response).to have_http_status(404) + expect(json_response['message']).to eq('404 Label Not Found') + end + + it 'returns 400 for wrong parameters' do + delete v3_api("/projects/#{project.id}/labels", user) + expect(response).to have_http_status(400) + end + end end diff --git a/spec/requests/api/v3/members_spec.rb b/spec/requests/api/v3/members_spec.rb index 28c3ca03960..13814ed10c3 100644 --- a/spec/requests/api/v3/members_spec.rb +++ b/spec/requests/api/v3/members_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe API::Members, api: true do +describe API::V3::Members, api: true do include ApiHelpers let(:master) { create(:user) } diff --git a/spec/requests/api/v3/notes_spec.rb b/spec/requests/api/v3/notes_spec.rb index b51cb3055d5..b8f0260c6a2 100644 --- a/spec/requests/api/v3/notes_spec.rb +++ b/spec/requests/api/v3/notes_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe API::V3::Notes, api: true do include ApiHelpers + let(:user) { create(:user) } let!(:project) { create(:empty_project, :public, namespace: user.namespace) } let!(:issue) { create(:issue, project: project, author: user) } @@ -373,12 +374,12 @@ describe API::V3::Notes, api: true do context 'when noteable is an Issue' do it 'deletes a note' do delete v3_api("/projects/#{project.id}/issues/#{issue.id}/"\ - "notes/#{issue_note.id}", user) + "notes/#{issue_note.id}", user) expect(response).to have_http_status(200) # Check if note is really deleted delete v3_api("/projects/#{project.id}/issues/#{issue.id}/"\ - "notes/#{issue_note.id}", user) + "notes/#{issue_note.id}", user) expect(response).to have_http_status(404) end @@ -392,18 +393,18 @@ describe API::V3::Notes, api: true do context 'when noteable is a Snippet' do it 'deletes a note' do delete v3_api("/projects/#{project.id}/snippets/#{snippet.id}/"\ - "notes/#{snippet_note.id}", user) + "notes/#{snippet_note.id}", user) expect(response).to have_http_status(200) # Check if note is really deleted delete v3_api("/projects/#{project.id}/snippets/#{snippet.id}/"\ - "notes/#{snippet_note.id}", user) + "notes/#{snippet_note.id}", user) expect(response).to have_http_status(404) end it 'returns a 404 error when note id not found' do delete v3_api("/projects/#{project.id}/snippets/#{snippet.id}/"\ - "notes/12345", user) + "notes/12345", user) expect(response).to have_http_status(404) end @@ -412,18 +413,18 @@ describe API::V3::Notes, api: true do context 'when noteable is a Merge Request' do it 'deletes a note' do delete v3_api("/projects/#{project.id}/merge_requests/"\ - "#{merge_request.id}/notes/#{merge_request_note.id}", user) + "#{merge_request.id}/notes/#{merge_request_note.id}", user) expect(response).to have_http_status(200) # Check if note is really deleted delete v3_api("/projects/#{project.id}/merge_requests/"\ - "#{merge_request.id}/notes/#{merge_request_note.id}", user) + "#{merge_request.id}/notes/#{merge_request_note.id}", user) expect(response).to have_http_status(404) end it 'returns a 404 error when note id not found' do delete v3_api("/projects/#{project.id}/merge_requests/"\ - "#{merge_request.id}/notes/12345", user) + "#{merge_request.id}/notes/12345", user) expect(response).to have_http_status(404) end diff --git a/spec/requests/api/v3/runners_spec.rb b/spec/requests/api/v3/runners_spec.rb new file mode 100644 index 00000000000..ca335ce9cf0 --- /dev/null +++ b/spec/requests/api/v3/runners_spec.rb @@ -0,0 +1,154 @@ +require 'spec_helper' + +describe API::V3::Runners, api: true do + include ApiHelpers + + let(:admin) { create(:user, :admin) } + let(:user) { create(:user) } + let(:user2) { create(:user) } + + let(:project) { create(:empty_project, creator_id: user.id) } + let(:project2) { create(:empty_project, creator_id: user.id) } + + let!(:shared_runner) { create(:ci_runner, :shared) } + let!(:unused_specific_runner) { create(:ci_runner) } + + let!(:specific_runner) do + create(:ci_runner).tap do |runner| + create(:ci_runner_project, runner: runner, project: project) + end + end + + let!(:two_projects_runner) do + create(:ci_runner).tap do |runner| + create(:ci_runner_project, runner: runner, project: project) + create(:ci_runner_project, runner: runner, project: project2) + end + end + + before do + # Set project access for users + create(:project_member, :master, user: user, project: project) + create(:project_member, :reporter, user: user2, project: project) + end + + describe 'DELETE /runners/:id' do + context 'admin user' do + context 'when runner is shared' do + it 'deletes runner' do + expect do + delete v3_api("/runners/#{shared_runner.id}", admin) + + expect(response).to have_http_status(200) + end.to change{ Ci::Runner.shared.count }.by(-1) + end + end + + context 'when runner is not shared' do + it 'deletes unused runner' do + expect do + delete v3_api("/runners/#{unused_specific_runner.id}", admin) + + expect(response).to have_http_status(200) + end.to change{ Ci::Runner.specific.count }.by(-1) + end + + it 'deletes used runner' do + expect do + delete v3_api("/runners/#{specific_runner.id}", admin) + + expect(response).to have_http_status(200) + end.to change{ Ci::Runner.specific.count }.by(-1) + end + end + + it 'returns 404 if runner does not exists' do + delete v3_api('/runners/9999', admin) + + expect(response).to have_http_status(404) + end + end + + context 'authorized user' do + context 'when runner is shared' do + it 'does not delete runner' do + delete v3_api("/runners/#{shared_runner.id}", user) + expect(response).to have_http_status(403) + end + end + + context 'when runner is not shared' do + it 'does not delete runner without access to it' do + delete v3_api("/runners/#{specific_runner.id}", user2) + expect(response).to have_http_status(403) + end + + it 'does not delete runner with more than one associated project' do + delete v3_api("/runners/#{two_projects_runner.id}", user) + expect(response).to have_http_status(403) + end + + it 'deletes runner for one owned project' do + expect do + delete v3_api("/runners/#{specific_runner.id}", user) + + expect(response).to have_http_status(200) + end.to change{ Ci::Runner.specific.count }.by(-1) + end + end + end + + context 'unauthorized user' do + it 'does not delete runner' do + delete v3_api("/runners/#{specific_runner.id}") + + expect(response).to have_http_status(401) + end + end + end + + describe 'DELETE /projects/:id/runners/:runner_id' do + context 'authorized user' do + context 'when runner have more than one associated projects' do + it "disables project's runner" do + expect do + delete v3_api("/projects/#{project.id}/runners/#{two_projects_runner.id}", user) + + expect(response).to have_http_status(200) + end.to change{ project.runners.count }.by(-1) + end + end + + context 'when runner have one associated projects' do + it "does not disable project's runner" do + expect do + delete v3_api("/projects/#{project.id}/runners/#{specific_runner.id}", user) + end.to change{ project.runners.count }.by(0) + expect(response).to have_http_status(403) + end + end + + it 'returns 404 is runner is not found' do + delete v3_api("/projects/#{project.id}/runners/9999", user) + + expect(response).to have_http_status(404) + end + end + + context 'authorized user without permissions' do + it "does not disable project's runner" do + delete v3_api("/projects/#{project.id}/runners/#{specific_runner.id}", user2) + + expect(response).to have_http_status(403) + end + end + + context 'unauthorized user' do + it "does not disable project's runner" do + delete v3_api("/projects/#{project.id}/runners/#{specific_runner.id}") + + expect(response).to have_http_status(401) + end + end + end +end diff --git a/spec/requests/api/v3/services_spec.rb b/spec/requests/api/v3/services_spec.rb new file mode 100644 index 00000000000..7e8c8753d02 --- /dev/null +++ b/spec/requests/api/v3/services_spec.rb @@ -0,0 +1,22 @@ +require "spec_helper" + +describe API::V3::Services, api: true do + include ApiHelpers + + let(:user) { create(:user) } + let(:project) { create(:empty_project, creator_id: user.id, namespace: user.namespace) } + + Service.available_services_names.each do |service| + describe "DELETE /projects/:id/services/#{service.dasherize}" do + include_context service + + it "deletes #{service}" do + delete v3_api("/projects/#{project.id}/services/#{dashed_service}", user) + + expect(response).to have_http_status(200) + project.send(service_method).reload + expect(project.send(service_method).activated?).to be_falsey + end + end + end +end diff --git a/spec/requests/api/v3/system_hooks_spec.rb b/spec/requests/api/v3/system_hooks_spec.rb index da58efb6ebf..91038977c82 100644 --- a/spec/requests/api/v3/system_hooks_spec.rb +++ b/spec/requests/api/v3/system_hooks_spec.rb @@ -38,4 +38,20 @@ describe API::V3::SystemHooks, api: true do end end end + + describe "DELETE /hooks/:id" do + it "deletes a hook" do + expect do + delete v3_api("/hooks/#{hook.id}", admin) + + expect(response).to have_http_status(200) + end.to change { SystemHook.count }.by(-1) + end + + it 'returns 404 if the system hook does not exist' do + delete v3_api('/hooks/12345', admin) + + expect(response).to have_http_status(404) + end + end end diff --git a/spec/requests/api/v3/tags_spec.rb b/spec/requests/api/v3/tags_spec.rb index 6722789d928..6870cfd2668 100644 --- a/spec/requests/api/v3/tags_spec.rb +++ b/spec/requests/api/v3/tags_spec.rb @@ -64,4 +64,26 @@ describe API::V3::Tags, api: true do end end end + + describe 'DELETE /projects/:id/repository/tags/:tag_name' do + let(:tag_name) { project.repository.tag_names.sort.reverse.first } + + before do + allow_any_instance_of(Repository).to receive(:rm_tag).and_return(true) + end + + context 'delete tag' do + it 'deletes an existing tag' do + delete v3_api("/projects/#{project.id}/repository/tags/#{tag_name}", user) + + expect(response).to have_http_status(200) + expect(json_response['tag_name']).to eq(tag_name) + end + + it 'raises 404 if the tag does not exist' do + delete v3_api("/projects/#{project.id}/repository/tags/foobar", user) + expect(response).to have_http_status(404) + end + end + end end diff --git a/spec/requests/api/v3/triggers_spec.rb b/spec/requests/api/v3/triggers_spec.rb new file mode 100644 index 00000000000..721ce4a361b --- /dev/null +++ b/spec/requests/api/v3/triggers_spec.rb @@ -0,0 +1,47 @@ +require 'spec_helper' + +describe API::V3::Triggers do + include ApiHelpers + + let(:user) { create(:user) } + let(:user2) { create(:user) } + let!(:trigger_token) { 'secure_token' } + let!(:project) { create(:project, :repository, creator: user) } + let!(:master) { create(:project_member, :master, user: user, project: project) } + let!(:developer) { create(:project_member, :developer, user: user2, project: project) } + let!(:trigger) { create(:ci_trigger, project: project, token: trigger_token) } + + describe 'DELETE /projects/:id/triggers/:token' do + context 'authenticated user with valid permissions' do + it 'deletes trigger' do + expect do + delete v3_api("/projects/#{project.id}/triggers/#{trigger.token}", user) + + expect(response).to have_http_status(200) + end.to change{project.triggers.count}.by(-1) + end + + it 'responds with 404 Not Found if requesting non-existing trigger' do + delete v3_api("/projects/#{project.id}/triggers/abcdef012345", user) + + expect(response).to have_http_status(404) + end + end + + context 'authenticated user with invalid permissions' do + it 'does not delete trigger' do + delete v3_api("/projects/#{project.id}/triggers/#{trigger.token}", user2) + + expect(response).to have_http_status(403) + end + end + + context 'unauthenticated user' do + it 'does not delete trigger' do + delete v3_api("/projects/#{project.id}/triggers/#{trigger.token}") + + expect(response).to have_http_status(401) + end + end + end +end diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb index 769f04c5057..0c1413119e0 100644 --- a/spec/requests/api/variables_spec.rb +++ b/spec/requests/api/variables_spec.rb @@ -152,8 +152,9 @@ describe API::Variables, api: true do it 'deletes variable' do expect do delete api("/projects/#{project.id}/variables/#{variable.key}", user) + + expect(response).to have_http_status(204) end.to change{project.variables.count}.by(-1) - expect(response).to have_http_status(200) end it 'responds with 404 Not Found if requesting non-existing variable' do |