diff --git a/app/models/environment.rb b/app/models/environment.rb index 9eff0fdab0306897f4e4321927748e98ea44bde7..baed106e8c884e313ddc587f9572d107da840f5a 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -3,6 +3,8 @@ class Environment < ActiveRecord::Base has_many :deployments + before_validation :nullify_external_url + validates :name, presence: true, uniqueness: { scope: :project_id }, @@ -12,9 +14,15 @@ class Environment < ActiveRecord::Base validates :external_url, uniqueness: { scope: :project_id }, - length: { maximum: 255 } + length: { maximum: 255 }, + allow_nil: true, + addressable_url: true def last_deployment deployments.last end + + def nullify_external_url + self.external_url = nil if self.external_url.blank? + end end diff --git a/db/migrate/20160725083350_add_external_url_to_enviroments.rb b/db/migrate/20160725083350_add_external_url_to_enviroments.rb index e887341159bdf4770691f6c64a2d2ae070575f25..21a8abd310b21fe66cf2367a867e2739ce16f89b 100644 --- a/db/migrate/20160725083350_add_external_url_to_enviroments.rb +++ b/db/migrate/20160725083350_add_external_url_to_enviroments.rb @@ -1,6 +1,3 @@ -# 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 include Gitlab::Database::MigrationHelpers diff --git a/db/schema.rb b/db/schema.rb index 4365af98962ca224119dded81aece08b1a228b36..5b35a528e71b7c1ff0fa6028d94fe0391c88c211 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # 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 enable_extension "plpgsql" diff --git a/doc/api/enviroments.md b/doc/api/enviroments.md index c4b844fe77ef5eccfdc9366ef7bf7d5fbb9416a2..16bf2627fefd5b02052efe3f82dd70f93f85ea22 100644 --- a/doc/api/enviroments.md +++ b/doc/api/enviroments.md @@ -32,8 +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 -and 409 if the environment already exists. +It returns 200 if the environment was successfully created, 400 for wrong parameters. ``` POST /projects/:id/environment @@ -43,7 +42,7 @@ POST /projects/:id/environment | ------------- | ------- | -------- | ---------------------------- | | `id` | integer | yes | The ID of the project | | `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 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: ## 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. diff --git a/lib/api/environments.rb b/lib/api/environments.rb index 66a047f72fc60e239e139f2df838199af968f1bb..532baec42c7a561b1f82dba43e7f58c74c77591f 100644 --- a/lib/api/environments.rb +++ b/lib/api/environments.rb @@ -30,9 +30,6 @@ module API required_attributes! [:name] 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) @@ -52,7 +49,7 @@ module API # Example Request: # DELETE /projects/:id/environments/:environment_id delete ':id/environments/:environment_id' do - authorize! :admin_environment, user_project + authorize! :update_environment, user_project environment = user_project.environments.find(params[:environment_id]) diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index b91a99d6b2ef3a796172e9680232d22224d3ab13..768105cae9580ed9d93add1448ebfaf968a76dbc 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -11,12 +11,10 @@ describe Projects::EnvironmentsController do sign_in(user) end - render_views - describe 'GET show' do context 'with valid id' 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 end @@ -24,16 +22,18 @@ describe Projects::EnvironmentsController do context 'with invalid id' 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 describe 'GET edit' 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 end @@ -41,10 +41,18 @@ describe Projects::EnvironmentsController do describe 'PATCH #update' do it 'responds with a 302' do - patch :update, namespace_id: project.namespace, project_id: - project, id: environment.id, environment: { external_url: 'https://git.gitlab.com' } + patch_params = environment_params.merge(environment: { external_url: 'https://git.gitlab.com' }) + patch :update, patch_params expect(response).to have_http_status(302) end end + + def environment_params + { + namespace_id: project.namespace, + project_id: project, + id: environment.id + } + end end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 6c11cfc4c9be376f044670dd5564f6a9686ce09f..ef2148be1bdd630c691c1b89e401f7ccfedc9188 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -21,4 +21,12 @@ describe Environment, models: true do is_expected.to validate_uniqueness_of(:external_url).scoped_to(:project_id) 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 diff --git a/spec/requests/api/environments_spec.rb b/spec/requests/api/environments_spec.rb index 822139dbf3ba614bf9c58c7905b6da64a624530b..b731c58a20660ce08ba66be294c2654bdb969b06 100644 --- a/spec/requests/api/environments_spec.rb +++ b/spec/requests/api/environments_spec.rb @@ -5,7 +5,7 @@ describe API::API, api: true do let(:user) { 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) } before do @@ -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 labels' do + it 'should return project environments' do get api("/projects/#{project.id}/environments", user) expect(response).to have_http_status(200) @@ -34,7 +34,7 @@ describe API::API, api: true do end end - describe 'POST /projects/:id/labels' do + describe 'POST /projects/:id/environments' do context 'as a member' do it 'creates a environment with valid params' do post api("/projects/#{project.id}/environments", user), name: "mepmep" @@ -50,11 +50,10 @@ describe API::API, api: true do expect(response).to have_http_status(400) 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 - expect(response).to have_http_status(409) - expect(json_response['message']).to eq('Environment already exists') + expect(response).to have_http_status(400) end end @@ -87,11 +86,11 @@ describe API::API, api: true do 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: 'mepmep.whatever.ninja' + 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('mepmep.whatever.ninja') + expect(json_response['external_url']).to eq('https://mepmep.whatever.ninja') end it 'should return 404 if the environment does not exist' do