From e0ec69d919cb44194e76034f2324ec0d4f5f1df6 Mon Sep 17 00:00:00 2001
From: Tomasz Maczukin <tomasz@maczukin.pl>
Date: Thu, 7 Jan 2016 18:48:33 +0100
Subject: [PATCH] Change 'trigger_id' to 'token' as resource ID in triggers API
---
doc/api/triggers.md | 12 +++------
lib/api/entities.rb | 2 +-
lib/api/triggers.rb | 25 ++++++------------
spec/requests/api/triggers_spec.rb | 42 ++++++++++--------------------
4 files changed, 27 insertions(+), 54 deletions(-)
diff --git a/doc/api/triggers.md b/doc/api/triggers.md
index 6adcc8fe3b8..4150d7b4c10 100644
--- a/doc/api/triggers.md
+++ b/doc/api/triggers.md
@@ -17,7 +17,6 @@ Parameters:
{
"created_at": "2015-12-23T16:24:34.716Z",
"deleted_at": null,
- "id": 1,
"last_used": "2016-01-04T15:41:21.986Z",
"token": "fbdb730c2fbdb095a0862dbd8ab88b",
"updated_at": "2015-12-23T16:24:34.716Z"
@@ -25,7 +24,6 @@ Parameters:
{
"created_at": "2015-12-23T16:25:56.760Z",
"deleted_at": null,
- "id": 2,
"last_used": null,
"token": "7b9148c158980bbd9bcea92c17522d",
"updated_at": "2015-12-23T16:25:56.760Z"
@@ -38,19 +36,18 @@ Parameters:
Get details of trigger of a project
```
-GET /projects/:id/triggers/:trigger_id
+GET /projects/:id/triggers/:token
```
Parameters:
- `id` (required) - The ID of a project
-- `trigger_id` (required) - The ID of a trigger
+- `token` (required) - The `token` of a trigger
```json
{
"created_at": "2015-12-23T16:25:56.760Z",
"deleted_at": null,
- "id": 2,
"last_used": null,
"token": "7b9148c158980bbd9bcea92c17522d",
"updated_at": "2015-12-23T16:25:56.760Z"
@@ -73,7 +70,6 @@ Parameters:
{
"created_at": "2016-01-07T09:53:58.235Z",
"deleted_at": null,
- "id": 5,
"last_used": null,
"token": "6d056f63e50fe6f8c5f8f4aa10edb7",
"updated_at": "2016-01-07T09:53:58.235Z"
@@ -85,10 +81,10 @@ Parameters:
Remove a trigger of a project
```
-DELETE /projects/:id/triggers/:trigger_id
+DELETE /projects/:id/triggers/:token
```
Parameters:
- `id` (required) - The ID of a project
-- `trigger_id` (required) - The ID of a trigger
+- `token` (required) - The `token` of a trigger
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index bc0cd76a2b8..37c483b45ec 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -367,7 +367,7 @@ module API
end
class Trigger < Grape::Entity
- expose :id, :token, :created_at, :updated_at, :deleted_at
+ expose :token, :created_at, :updated_at, :deleted_at
expose :last_used do |repo_obj, _options|
if repo_obj.respond_to?(:last_trigger_request)
request = repo_obj.last_trigger_request
diff --git a/lib/api/triggers.rb b/lib/api/triggers.rb
index 0e548b936cd..25bb8aef20b 100644
--- a/lib/api/triggers.rb
+++ b/lib/api/triggers.rb
@@ -66,23 +66,14 @@ module API
#
# Parameters:
# id (required) - The ID of a project
- # trigger_id (required) - The ID or `token` of a trigger to show; if trigger_id contains only digits it's
- # treated as ID other ways it's reated as `key`
+ # token (required) - The `token` of a trigger
# Example Request:
- # GET /projects/:id/triggers/:trigger_id
- get ':id/triggers/:trigger_id' do
+ # GET /projects/:id/triggers/:token
+ get ':id/triggers/:token' do
authenticate!
authorize_admin_project
- trigger_id = params[:trigger_id]
- triggers = user_project.triggers
- triggers =
- if trigger_id.match(/^\d+$/)
- triggers.where(id: trigger_id.to_i)
- else
- triggers.where(token: trigger_id)
- end
-
+ triggers = user_project.triggers.where(token: params[:token])
return not_found!('Trigger') if triggers.empty?
present triggers.first, with: Entities::Trigger
@@ -108,14 +99,14 @@ module API
#
# Parameters:
# id (required) - The ID of a project
- # trigger_id - The ID of trigger to delete
+ # token (required) - The `token` of a trigger
# Example Request:
- # DELETE /projects/:id/triggers/:trigger_id
- delete ':id/triggers/:trigger_id' do
+ # DELETE /projects/:id/triggers/:token
+ delete ':id/triggers/:token' do
authenticate!
authorize_admin_project
- trigger = user_project.triggers.where(id: params[:trigger_id].to_i).first
+ trigger = user_project.triggers.where(token: params[:token]).first
return not_found!('Trigger') unless trigger
trigger.destroy
diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb
index c1c2bb04b29..e8d89426ec0 100644
--- a/spec/requests/api/triggers_spec.rb
+++ b/spec/requests/api/triggers_spec.rb
@@ -93,8 +93,7 @@ describe API::API do
expect(response.status).to eq(200)
expect(json_response).to be_a(Array)
- expect(json_response[0]['token']).to eq(trigger_token)
- expect(json_response[1]['token']).to eq(trigger_token_2)
+ expect(json_response[0]).to have_key('token')
end
end
@@ -115,30 +114,17 @@ describe API::API do
end
end
- describe 'GET /projects/:id/triggers/:triggers_id' do
+ describe 'GET /projects/:id/triggers/:token' do
context 'authenticated user with valid permissions' do
- context 'ID is used as :trigger_id' do
- it 'should return trigger details' do
- get api("/projects/#{project.id}/triggers/#{trigger.id}", user)
+ it 'should return trigger details' do
+ get api("/projects/#{project.id}/triggers/#{trigger.token}", user)
- expect(response.status).to eq(200)
- expect(json_response).to be_a(Hash)
- expect(json_response['token']).to eq(trigger_token)
- end
- end
-
- context '`token` is used as :trigger_id' do
- it 'should return trigger details' do
- get api("/projects/#{project.id}/triggers/#{trigger.token}", user)
-
- expect(response.status).to eq(200)
- expect(json_response).to be_a(Hash)
- expect(json_response['id']).to eq(trigger.id)
- end
+ expect(response.status).to eq(200)
+ expect(json_response).to be_a(Hash)
end
it 'should responde with 404 Not Found if requesting non-existing trigger' do
- get api("/projects/#{project.id}/triggers/9999", user)
+ get api("/projects/#{project.id}/triggers/abcdef012345", user)
expect(response.status).to eq(404)
end
@@ -146,7 +132,7 @@ describe API::API do
context 'authenticated user with invalid permissions' do
it 'should not return triggers list' do
- get api("/projects/#{project.id}/triggers/#{trigger.id}", user2)
+ get api("/projects/#{project.id}/triggers/#{trigger.token}", user2)
expect(response.status).to eq(403)
end
@@ -154,7 +140,7 @@ describe API::API do
context 'unauthentikated user' do
it 'should not return triggers list' do
- get api("/projects/#{project.id}/triggers/#{trigger.id}")
+ get api("/projects/#{project.id}/triggers/#{trigger.token}")
expect(response.status).to eq(401)
end
@@ -190,17 +176,17 @@ describe API::API do
end
end
- describe 'DELETE /projects/:id/triggers/:trigger_id' do
+ describe 'DELETE /projects/:id/triggers/:token' do
context 'authenticated user with valid permissions' do
it 'should delete trigger' do
expect do
- delete api("/projects/#{project.id}/triggers/#{trigger.id}", user)
+ delete api("/projects/#{project.id}/triggers/#{trigger.token}", user)
end.to change{project.triggers.count}.by(-1)
expect(response.status).to eq(200)
end
it 'should responde with 404 Not Found if requesting non-existing trigger' do
- delete api("/projects/#{project.id}/triggers/9999", user)
+ delete api("/projects/#{project.id}/triggers/abcdef012345", user)
expect(response.status).to eq(404)
end
@@ -208,7 +194,7 @@ describe API::API do
context 'authenticated user with invalid permissions' do
it 'should not delete trigger' do
- delete api("/projects/#{project.id}/triggers/#{trigger.id}", user2)
+ delete api("/projects/#{project.id}/triggers/#{trigger.token}", user2)
expect(response.status).to eq(403)
end
@@ -216,7 +202,7 @@ describe API::API do
context 'unauthentikated user' do
it 'should not delete trigger' do
- delete api("/projects/#{project.id}/triggers/#{trigger.id}")
+ delete api("/projects/#{project.id}/triggers/#{trigger.token}")
expect(response.status).to eq(401)
end
--
GitLab