Change 'trigger_id' to 'token' as resource ID in triggers API

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