Commit 76e9b684 authored by Z.J. van de Weg's avatar Z.J. van de Weg

Incorporate feedback

parent 84cd2120
...@@ -3,6 +3,8 @@ class Environment < ActiveRecord::Base ...@@ -3,6 +3,8 @@ class Environment < ActiveRecord::Base
has_many :deployments has_many :deployments
before_validation :nullify_external_url
validates :name, validates :name,
presence: true, presence: true,
uniqueness: { scope: :project_id }, uniqueness: { scope: :project_id },
...@@ -12,9 +14,15 @@ class Environment < ActiveRecord::Base ...@@ -12,9 +14,15 @@ class Environment < ActiveRecord::Base
validates :external_url, validates :external_url,
uniqueness: { scope: :project_id }, uniqueness: { scope: :project_id },
length: { maximum: 255 } length: { maximum: 255 },
allow_nil: true,
addressable_url: true
def last_deployment def last_deployment
deployments.last deployments.last
end end
def nullify_external_url
self.external_url = nil if self.external_url.blank?
end
end end
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddExternalUrlToEnviroments < ActiveRecord::Migration class AddExternalUrlToEnviroments < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers include Gitlab::Database::MigrationHelpers
......
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20160722221922) do ActiveRecord::Schema.define(version: 20160726093600) do
# These are extensions that must be enabled in order to support this database # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
......
...@@ -32,8 +32,7 @@ Example response: ...@@ -32,8 +32,7 @@ Example response:
Creates a new environment with the given name and external_url. 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 200 if the environment was successfully created, 400 for wrong parameters.
and 409 if the environment already exists.
``` ```
POST /projects/:id/environment POST /projects/:id/environment
...@@ -43,7 +42,7 @@ POST /projects/:id/environment ...@@ -43,7 +42,7 @@ POST /projects/:id/environment
| ------------- | ------- | -------- | ---------------------------- | | ------------- | ------- | -------- | ---------------------------- |
| `id` | integer | yes | The ID of the project | | `id` | integer | yes | The ID of the project |
| `name` | string | yes | The name of the environment | | `name` | string | yes | The name of the environment |
| `external_url` | string | yes | Place to link to for this environment | | `external_url` | string | no | Place to link to for this environment |
```bash ```bash
curl --data "name=deploy&external_url=https://deploy.example.gitlab.com" -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/environments" curl --data "name=deploy&external_url=https://deploy.example.gitlab.com" -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/environments"
...@@ -88,7 +87,7 @@ Example response: ...@@ -88,7 +87,7 @@ Example response:
## Edit an existing environment ## Edit an existing environment
Updates an existing environments name and/or external_url. 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. It returns 200 if the label was successfully updated, In case of an error, an additional error message is returned.
......
...@@ -30,9 +30,6 @@ module API ...@@ -30,9 +30,6 @@ module API
required_attributes! [:name] required_attributes! [:name]
attrs = attributes_for_keys [:name, :external_url] attrs = attributes_for_keys [:name, :external_url]
environment = user_project.environments.find_by(name: attrs[:name])
conflict!('Environment already exists') if environment
environment = user_project.environments.create(attrs) environment = user_project.environments.create(attrs)
...@@ -52,7 +49,7 @@ module API ...@@ -52,7 +49,7 @@ module API
# Example Request: # Example Request:
# DELETE /projects/:id/environments/:environment_id # DELETE /projects/:id/environments/:environment_id
delete ':id/environments/:environment_id' do delete ':id/environments/:environment_id' do
authorize! :admin_environment, user_project authorize! :update_environment, user_project
environment = user_project.environments.find(params[:environment_id]) environment = user_project.environments.find(params[:environment_id])
......
...@@ -11,12 +11,10 @@ describe Projects::EnvironmentsController do ...@@ -11,12 +11,10 @@ describe Projects::EnvironmentsController do
sign_in(user) sign_in(user)
end end
render_views
describe 'GET show' do describe 'GET show' do
context 'with valid id' do context 'with valid id' do
it 'responds with a status code 200' do it 'responds with a status code 200' do
get :show, namespace_id: project.namespace, project_id: project, id: environment.id get :show, environment_params
expect(response).to be_ok expect(response).to be_ok
end end
...@@ -24,16 +22,18 @@ describe Projects::EnvironmentsController do ...@@ -24,16 +22,18 @@ describe Projects::EnvironmentsController do
context 'with invalid id' do context 'with invalid id' do
it 'responds with a status code 404' do it 'responds with a status code 404' do
get :show, namespace_id: project.namespace, project_id: project, id: 12345 params = environment_params
params[:id] = 12345
get :show, params
expect(response).to be_not_found expect(response).to have_http_status(404)
end end
end end
end end
describe 'GET edit' do describe 'GET edit' do
it 'responds with a status code 200' do it 'responds with a status code 200' do
get :edit, namespace_id: project.namespace, project_id: project, id: environment.id get :edit, environment_params
expect(response).to be_ok expect(response).to be_ok
end end
...@@ -41,10 +41,18 @@ describe Projects::EnvironmentsController do ...@@ -41,10 +41,18 @@ describe Projects::EnvironmentsController do
describe 'PATCH #update' do describe 'PATCH #update' do
it 'responds with a 302' do it 'responds with a 302' do
patch :update, namespace_id: project.namespace, project_id: patch_params = environment_params.merge(environment: { external_url: 'https://git.gitlab.com' })
project, id: environment.id, environment: { external_url: 'https://git.gitlab.com' } patch :update, patch_params
expect(response).to have_http_status(302) expect(response).to have_http_status(302)
end end
end end
def environment_params
{
namespace_id: project.namespace,
project_id: project,
id: environment.id
}
end
end end
...@@ -21,4 +21,12 @@ describe Environment, models: true do ...@@ -21,4 +21,12 @@ describe Environment, models: true do
is_expected.to validate_uniqueness_of(:external_url).scoped_to(:project_id) is_expected.to validate_uniqueness_of(:external_url).scoped_to(:project_id)
end end
describe '#nullify_external_url' do
it 'replaces a blank url with nil' do
env = build(:environment, external_url: "")
expect(env.save).to be true
end
end
end end
...@@ -5,7 +5,7 @@ describe API::API, api: true do ...@@ -5,7 +5,7 @@ describe API::API, api: true do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:non_member) { create(:user) } let(:non_member) { create(:user) }
let(:project) { create(:project, :private, creator_id: user.id, namespace: user.namespace) } let(:project) { create(:project, :private, namespace: user.namespace) }
let!(:environment) { create(:environment, project: project) } let!(:environment) { create(:environment, project: project) }
before do before do
...@@ -14,7 +14,7 @@ describe API::API, api: true do ...@@ -14,7 +14,7 @@ describe API::API, api: true do
describe 'GET /projects/:id/environments' do describe 'GET /projects/:id/environments' do
context 'as member of the project' do context 'as member of the project' do
it 'should return project labels' do it 'should return project environments' do
get api("/projects/#{project.id}/environments", user) get api("/projects/#{project.id}/environments", user)
expect(response).to have_http_status(200) expect(response).to have_http_status(200)
...@@ -34,7 +34,7 @@ describe API::API, api: true do ...@@ -34,7 +34,7 @@ describe API::API, api: true do
end end
end end
describe 'POST /projects/:id/labels' do describe 'POST /projects/:id/environments' do
context 'as a member' do context 'as a member' do
it 'creates a environment with valid params' do it 'creates a environment with valid params' do
post api("/projects/#{project.id}/environments", user), name: "mepmep" post api("/projects/#{project.id}/environments", user), name: "mepmep"
...@@ -50,11 +50,10 @@ describe API::API, api: true do ...@@ -50,11 +50,10 @@ describe API::API, api: true do
expect(response).to have_http_status(400) expect(response).to have_http_status(400)
end end
it 'should return 409 if environment already exists' do it 'should return 400 if environment already exists' do
post api("/projects/#{project.id}/environments", user), name: environment.name post api("/projects/#{project.id}/environments", user), name: environment.name
expect(response).to have_http_status(409) expect(response).to have_http_status(400)
expect(json_response['message']).to eq('Environment already exists')
end end
end end
...@@ -87,11 +86,11 @@ describe API::API, api: true do ...@@ -87,11 +86,11 @@ describe API::API, api: true do
describe 'PUT /projects/:id/environments/:environment_id' do describe 'PUT /projects/:id/environments/:environment_id' do
it 'should return 200 if name and external_url are changed' do it 'should return 200 if name and external_url are changed' do
put api("/projects/#{project.id}/environments/#{environment.id}", user), put api("/projects/#{project.id}/environments/#{environment.id}", user),
name: 'Mepmep', external_url: 'mepmep.whatever.ninja' name: 'Mepmep', external_url: 'https://mepmep.whatever.ninja'
expect(response).to have_http_status(200) expect(response).to have_http_status(200)
expect(json_response['name']).to eq('Mepmep') expect(json_response['name']).to eq('Mepmep')
expect(json_response['external_url']).to eq('mepmep.whatever.ninja') expect(json_response['external_url']).to eq('https://mepmep.whatever.ninja')
end end
it 'should return 404 if the environment does not exist' do it 'should return 404 if the environment does not exist' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment