From 1b72256fa14e65256d78347f81b289d43c44e991 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Fri, 29 Jul 2016 12:14:36 +0200 Subject: [PATCH] Use Grape DSL for environment endpoints Also a couple of minor edits for this branch are included --- config/routes.rb | 2 +- doc/api/enviroments.md | 64 ++++++++--------- lib/api/api.rb | 4 ++ lib/api/environments.rb | 99 ++++++++++++-------------- spec/models/environment_spec.rb | 1 + spec/requests/api/environments_spec.rb | 57 +++++++++------ 6 files changed, 119 insertions(+), 108 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index ced204be7f7..371eb4bee7f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -741,7 +741,7 @@ Rails.application.routes.draw do end end - resources :environments, constraints: { id: /\d+/ } + resources :environments resources :builds, only: [:index, :show], constraints: { id: /\d+/ } do collection do diff --git a/doc/api/enviroments.md b/doc/api/enviroments.md index 16bf2627fef..1e12ded448c 100644 --- a/doc/api/enviroments.md +++ b/doc/api/enviroments.md @@ -32,7 +32,7 @@ Example response: Creates a new environment with the given name and external_url. -It returns 200 if the environment was successfully created, 400 for wrong parameters. +It returns 201 if the environment was successfully created, 400 for wrong parameters. ``` POST /projects/:id/environment @@ -58,6 +58,37 @@ Example response: } ``` +## Edit an existing environment + +Updates an existing environment's name and/or external_url. + +It returns 200 if the environment was successfully updated. In case of an error, a status code 400 is returned. + +``` +PUT /projects/:id/environments/:environments_id +``` + +| Attribute | Type | Required | Description | +| --------------- | ------- | --------------------------------- | ------------------------------- | +| `id` | integer | yes | The ID of the project | +| `environment_id` | integer | yes | The ID of the environment | The ID of the environment | +| `name` | string | no | The new name of the environment | +| `external_url` | string | no | The new external_url | + +```bash +curl -X PUT --data "name=staging&external_url=https://staging.example.gitlab.com" -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/environment/1" +``` + +Example response: + +```json +{ + "id": 1, + "name": "staging", + "external_url": "https://staging.example.gitlab.com" +} +``` + ## Delete an environment It returns 200 if the environment was successfully deleted, and 404 if the environment does not exist. @@ -84,34 +115,3 @@ Example response: "external_url": "https://deploy.example.gitlab.com" } ``` - -## Edit an existing environment - -Updates an existing environment's name and/or external_url. - -It returns 200 if the label was successfully updated, In case of an error, an additional error message is returned. - -``` -PUT /projects/:id/environments/:environments_id -``` - -| Attribute | Type | Required | Description | -| --------------- | ------- | --------------------------------- | ------------------------------- | -| `id` | integer | yes | The ID of the project | -| `environment_id` | integer | yes | The ID of the environment | The ID of the environment | -| `name` | string | no | The new name of the environment | -| `external_url` | string | no | The new external_url | - -```bash -curl -X PUT --data "name=staging&external_url=https://staging.example.gitlab.com" -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/environment/1" -``` - -Example response: - -```json -{ - "id": 1, - "name": "staging", - "external_url": "https://staging.example.gitlab.com" -} -``` diff --git a/lib/api/api.rb b/lib/api/api.rb index 9c960d74495..bd16806892b 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -7,6 +7,10 @@ module API rack_response({ 'message' => '404 Not found' }.to_json, 404) end + rescue_from Grape::Exceptions::ValidationErrors do |e| + error!({ messages: e.full_messages }, 400) + end + rescue_from :all do |exception| # lifted from https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb#L60 # why is this not wrapped in something reusable? diff --git a/lib/api/environments.rb b/lib/api/environments.rb index 532baec42c7..a50f007d697 100644 --- a/lib/api/environments.rb +++ b/lib/api/environments.rb @@ -3,51 +3,70 @@ module API class Environments < Grape::API before { authenticate! } + params do + requires :id, type: String, desc: 'The project ID' + end resource :projects do - # Get all labels of the project - # - # Parameters: - # id (required) - The ID of a project - # Example Request: - # GET /projects/:id/environments + desc 'Get all environments of the project' do + detail 'This feature was introduced in GitLab 8.11.' + success Entities::Environment + end get ':id/environments' do authorize! :read_environment, user_project present paginate(user_project.environments), with: Entities::Environment end - # Creates a new environment - # - # Parameters: - # id (required) - The ID of a project - # name (required) - The name of the environment to be created - # external_url (optional) - URL on which this deployment is viewable - # - # Example Request: - # POST /projects/:id/labels + desc 'Creates a new environment' do + detail 'This feature was introduced in GitLab 8.11.' + success Entities::Environment + end + params do + requires :name, type: String, desc: 'The name of the environment to be created' + optional :external_url, type: String, desc: 'URL on which this deployment is viewable' + end post ':id/environments' do authorize! :create_environment, user_project - required_attributes! [:name] - attrs = attributes_for_keys [:name, :external_url] + create_params = declared(params, include_parent_namespaces: false).to_h + environment = user_project.environments.create(create_params) - environment = user_project.environments.create(attrs) - - if environment.valid? + if environment.persisted? present environment, with: Entities::Environment else render_validation_error!(environment) end end - # Deletes an existing environment - # - # Parameters: - # id (required) - The ID of a project - # environment_id (required) - The name of the environment to be deleted - # - # Example Request: - # DELETE /projects/:id/environments/:environment_id + desc 'Updates an existing environment' do + detail 'This feature was introduced in GitLab 8.11.' + success Entities::Environment + end + params do + requires :environment_id, type: Integer, desc: 'The environment ID' + optional :name, type: String, desc: 'The new environment name' + optional :external_url, type: String, desc: 'The new URL on which this deployment is viewable' + end + put ':id/environments/:environment_id' do + authorize! :update_environment, user_project + + environment = user_project.environments.find(params[:environment_id]) + + update_params = declared(params, include_missing: false).extract!(:name, :external_url).to_h + if environment.update(update_params) + present environment, with: Entities::Environment + else + render_validation_error!(environment) + end + end + + desc 'Deletes an existing environment' do + detail 'This feature was introduced in GitLab 8.11.' + success 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 @@ -55,30 +74,6 @@ module API present environment.destroy, with: Entities::Environment end - - # Updates an existing environment - # - # Parameters: - # id (required) - The ID of a project - # environment_id (required) - The ID of the environment - # name (optional) - The name of the label to be deleted - # external_url (optional) - The new name of the label - # - # Example Request: - # PUT /projects/:id/environments/:environment_id - put ':id/environments/:environment_id' do - authorize! :update_environment, user_project - - environment = user_project.environments.find(params[:environment_id]) - - attrs = attributes_for_keys [:name, :external_url] - - if environment.update(attrs) - present environment, with: Entities::Environment - else - render_validation_error!(environment) - end - end end end end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index ef2148be1bd..8a84ac0a7c7 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -27,6 +27,7 @@ describe Environment, models: true do env = build(:environment, external_url: "") expect(env.save).to be true + expect(env.external_url).to be_nil end end end diff --git a/spec/requests/api/environments_spec.rb b/spec/requests/api/environments_spec.rb index b731c58a206..d315e456dda 100644 --- a/spec/requests/api/environments_spec.rb +++ b/spec/requests/api/environments_spec.rb @@ -14,7 +14,7 @@ describe API::API, api: true do describe 'GET /projects/:id/environments' do context 'as member of the project' do - it 'should return project environments' do + it 'returns project environments' do get api("/projects/#{project.id}/environments", user) expect(response).to have_http_status(200) @@ -26,7 +26,7 @@ describe API::API, api: true do end context 'as non member' do - it 'should return a 404 status code' do + it 'returns a 404 status code' do get api("/projects/#{project.id}/environments", non_member) expect(response).to have_http_status(404) @@ -50,7 +50,7 @@ describe API::API, api: true do expect(response).to have_http_status(400) end - it 'should return 400 if environment already exists' do + it 'returns a 400 if environment already exists' do post api("/projects/#{project.id}/environments", user), name: environment.name expect(response).to have_http_status(400) @@ -61,20 +61,48 @@ describe API::API, api: true do it 'rejects the request' do post api("/projects/#{project.id}/environments", non_member) - expect(response).to have_http_status(404) + expect(response).to have_http_status(400) end end end + describe 'PUT /projects/:id/environments/:environment_id' do + it 'returns a 200 if name and external_url are changed' do + url = 'https://mepmep.whatever.ninja' + put api("/projects/#{project.id}/environments/#{environment.id}", user), + name: 'Mepmep', external_url: url + + expect(response).to have_http_status(200) + expect(json_response['name']).to eq('Mepmep') + expect(json_response['external_url']).to eq(url) + end + + it "won't update the external_url if only the name is passed" do + url = environment.external_url + put api("/projects/#{project.id}/environments/#{environment.id}", user), + name: 'Mepmep' + + expect(response).to have_http_status(200) + expect(json_response['name']).to eq('Mepmep') + expect(json_response['external_url']).to eq(url) + end + + it 'returns a 404 if the environment does not exist' do + put api("/projects/#{project.id}/environments/12345", user) + + expect(response).to have_http_status(404) + end + end + describe 'DELETE /projects/:id/environments/:environment_id' do context 'as a master' do - it 'should return 200 for an existing environment' 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) end - it 'should return 404 for non existing id' do + it 'returns a 404 for non existing id' do delete api("/projects/#{project.id}/environments/12345", user) expect(response).to have_http_status(404) @@ -82,21 +110,4 @@ describe API::API, api: true do end end end - - describe 'PUT /projects/:id/environments/:environment_id' do - it 'should return 200 if name and external_url are changed' do - put api("/projects/#{project.id}/environments/#{environment.id}", user), - name: 'Mepmep', external_url: 'https://mepmep.whatever.ninja' - - expect(response).to have_http_status(200) - expect(json_response['name']).to eq('Mepmep') - expect(json_response['external_url']).to eq('https://mepmep.whatever.ninja') - end - - it 'should return 404 if the environment does not exist' do - put api("/projects/#{project.id}/environments/12345", user) - - expect(response).to have_http_status(404) - end - end end