diff options
20 files changed, 326 insertions, 359 deletions
diff --git a/app/assets/javascripts/environments/components/environment.js.es6 b/app/assets/javascripts/environments/components/environment.js.es6 index 0b8e1a0f898..b769161e058 100644 --- a/app/assets/javascripts/environments/components/environment.js.es6 +++ b/app/assets/javascripts/environments/components/environment.js.es6 @@ -6,15 +6,36 @@ /* globals Vue, EnvironmentsService */ /* eslint-disable no-param-reassign */ -$(() => { +(() => { // eslint-disable-line window.gl = window.gl || {}; - const filterState = state => environment => environment.state === state && environment; + /** + * Given the visibility prop provided by the url query parameter and which + * changes according to the active tab we need to filter which environments + * should be visible. + * + * The environments array is a recursive tree structure and we need to filter + * both root level environments and children environments. + * + * In order to acomplish that, both `filterState` and `filterEnvironmnetsByState` + * functions work together. + * The first one works as the filter that verifies if the given environment matches + * the given state. + * The second guarantees both root level and children elements are filtered as well. + */ - // recursiveMap :: (Function, Array) -> Array - const recursiveMap = (fn, arr) => arr.map((item) => { + const filterState = state => environment => environment.state === state && environment; + /** + * Given the filter function and the array of environments will return only + * the environments that match the state provided to the filter function. + * + * @param {Function} fn + * @param {Array} array + * @return {Array} + */ + const filterEnvironmnetsByState = (fn, arr) => arr.map((item) => { if (item.children) { - const filteredChildren = recursiveMap(fn, item.children).filter(Boolean); + const filteredChildren = filterEnvironmnetsByState(fn, item.children).filter(Boolean); if (filteredChildren.length) { item.children = filteredChildren; return item; @@ -37,26 +58,27 @@ $(() => { }, data() { - const environmentsListApp = document.querySelector('#environments-list-view'); + const environmentsData = document.querySelector('#environments-list-view').dataset; return { state: this.store.state, - endpoint: environmentsListApp.dataset.environmentsDataEndpoint, - canCreateDeployment: environmentsListApp.dataset.canCreateDeployment, - canReadEnvironment: environmentsListApp.dataset.canReadEnvironment, - canCreateEnvironment: environmentsListApp.dataset.canCreateEnvironment, - projectEnvironmentsPath: environmentsListApp.dataset.projectEnvironmentsPath, - projectStoppedEnvironmentsPath: environmentsListApp.dataset.projectStoppedEnvironmentsPath, - newEnvironmentPath: environmentsListApp.dataset.newEnvironmentPath, - helpPagePath: environmentsListApp.dataset.helpPagePath, visibility: 'available', isLoading: false, + cssContainerClass: environmentsData.cssClass, + endpoint: environmentsData.environmentsDataEndpoint, + canCreateDeployment: environmentsData.canCreateDeployment, + canReadEnvironment: environmentsData.canReadEnvironment, + canCreateEnvironment: environmentsData.canCreateEnvironment, + projectEnvironmentsPath: environmentsData.projectEnvironmentsPath, + projectStoppedEnvironmentsPath: environmentsData.projectStoppedEnvironmentsPath, + newEnvironmentPath: environmentsData.newEnvironmentPath, + helpPagePath: environmentsData.helpPagePath, }; }, computed: { filteredEnvironments() { - return recursiveMap(filterState(this.visibility), this.state.environments); + return filterEnvironmnetsByState(filterState(this.visibility), this.state.environments); }, scope() { @@ -81,7 +103,7 @@ $(() => { * Toggles loading property. */ created() { - window.gl.environmentsService = new EnvironmentsService(this.endpoint); + gl.environmentsService = new EnvironmentsService(this.endpoint); const scope = this.$options.getQueryParameter('scope'); if (scope) { @@ -90,7 +112,7 @@ $(() => { this.isLoading = true; - return window.gl.environmentsService.all() + return gl.environmentsService.all() .then(resp => resp.json()) .then((json) => { this.store.storeEnvironments(json); @@ -119,10 +141,7 @@ $(() => { * @returns {Boolean} */ convertPermissionToBoolean(string) { - if (string === 'true') { - return true; - } - return false; + return string === 'true'; }, methods: { @@ -132,10 +151,10 @@ $(() => { }, template: ` - <div class="container-fluid container-limited"> + <div :class="cssContainerClass"> <div class="top-area"> <ul v-if="!isLoading" class="nav-links"> - <li v-bind:class="{ 'active': scope === undefined}"> + <li v-bind:class="{ 'active': scope === undefined }"> <a :href="projectEnvironmentsPath"> Available <span @@ -143,7 +162,7 @@ $(() => { v-html="state.availableCounter"></span> </a> </li> - <li v-bind:class="{ 'active' : scope === 'stopped'}"> + <li v-bind:class="{ 'active' : scope === 'stopped' }"> <a :href="projectStoppedEnvironmentsPath"> Stopped <span @@ -172,19 +191,18 @@ $(() => { </h2> <p class="blank-state-text"> Environments are places where code gets deployed, such as staging or production. - <br /> - <a :href="helpPagePath"> Read more about environments </a> - <a - v-if="canCreateEnvironmentParsed" - :href="newEnvironmentPath" - class="btn btn-create"> - New Environment - </a> </p> + + <a + v-if="canCreateEnvironmentParsed" + :href="newEnvironmentPath" + class="btn btn-create"> + New Environment + </a> </div> <div @@ -227,4 +245,4 @@ $(() => { </div> `, }); -}); +})(); diff --git a/app/assets/javascripts/environments/components/environment_actions.js.es6 b/app/assets/javascripts/environments/components/environment_actions.js.es6 index 1941392ac66..edd39c02a46 100644 --- a/app/assets/javascripts/environments/components/environment_actions.js.es6 +++ b/app/assets/javascripts/environments/components/environment_actions.js.es6 @@ -44,18 +44,19 @@ <div class="dropdown"> <a class="dropdown-new btn btn-default" data-toggle="dropdown"> <span class="dropdown-play-icon-container"> - <!-- svg goes here --> </span> <i class="fa fa-caret-down"></i> </a> <ul class="dropdown-menu dropdown-menu-align-right"> <li v-for="action in actions"> - <a :href="action.play_url" data-method="post" data-rel="nofollow" class="js-manual-action-link"> - <span class="action-play-icon-container"> - <!-- svg goes here --> - </span> - <span v-html="action.name"></span> + <a :href="action.play_path" + data-method="post" + rel="nofollow" + class="js-manual-action-link"> + <span class="action-play-icon-container"> + </span> + <span v-html="action.name"></span> </a> </li> </ul> diff --git a/app/assets/javascripts/environments/components/environment_item.js.es6 b/app/assets/javascripts/environments/components/environment_item.js.es6 index 20720d69df0..9241b5472a7 100644 --- a/app/assets/javascripts/environments/components/environment_item.js.es6 +++ b/app/assets/javascripts/environments/components/environment_item.js.es6 @@ -76,8 +76,7 @@ * @returns {Boolean|Undefined} */ isFolder() { - return this.model.children && - this.model.children.length > 0; + return this.model.children && this.model.children.length > 0; }, /** @@ -97,8 +96,7 @@ * @returns {Number|Undefined} The number of environments for the current folder. */ childrenCounter() { - return this.model.children && - this.model.children.length; + return this.model.children && this.model.children.length; }, /** @@ -109,7 +107,8 @@ * @returns {Boolean} */ hasLastDeploymentKey() { - if (this.model.last_deployment && this.model.last_deployment !== {}) { + if (this.model.last_deployment && + !this.$options.isObjectEmpty(this.model.last_deployment)) { return true; } return false; @@ -168,7 +167,7 @@ return this.model.last_deployment.manual_actions.map((action) => { const parsedAction = { name: gl.text.humanize(action.name), - play_url: action.play_url, + play_path: action.play_path, }; return parsedAction; }); @@ -209,8 +208,7 @@ * @returns {Object|Undefined} */ commitRef() { - if (this.model.last_deployment && - this.model.last_deployment.ref) { + if (this.model.last_deployment && this.model.last_deployment.ref) { return this.model.last_deployment.ref; } return undefined; @@ -224,8 +222,8 @@ commitUrl() { if (this.model.last_deployment && this.model.last_deployment.commit && - this.model.last_deployment.commit.commit_url) { - return this.model.last_deployment.commit.commit_url; + this.model.last_deployment.commit.commit_path) { + return this.model.last_deployment.commit.commit_path; } return undefined; }, @@ -274,15 +272,15 @@ }, /** - * Verifies if the `retry_url` key is present and returns its value. + * Verifies if the `retry_path` key is present and returns its value. * * @returns {String|Undefined} */ retryUrl() { if (this.model.last_deployment && this.model.last_deployment.deployable && - this.model.last_deployment.deployable.retry_url) { - return this.model.last_deployment.deployable.retry_url; + this.model.last_deployment.deployable.retry_path) { + return this.model.last_deployment.deployable.retry_path; } return undefined; }, @@ -328,11 +326,8 @@ * @returns {Boolean} */ deploymentHasUser() { - if (this.model.last_deployment && - this.model.last_deployment.user) { - return true; - } - return false; + return !this.$options.isObjectEmpty(this.model.last_deployment) && + !this.$options.isObjectEmpty(this.model.last_deployment.user); }, /** @@ -342,11 +337,53 @@ * @returns {Object} */ deploymentUser() { - if (this.model.last_deployment && this.model.last_deployment.user) { + if (!this.$options.isObjectEmpty(this.model.last_deployment) && + !this.$options.isObjectEmpty(this.model.last_deployment.user)) { return this.model.last_deployment.user; } return {}; }, + + /** + * Verifies if the build name column should be rendered by verifing + * if all the information needed is present + * and if the environment is not a folder. + * + * @returns {Boolean} + */ + shouldRenderBuildName() { + return !this.isFolder && + !this.$options.isObjectEmpty(this.model.last_deployment) && + !this.$options.isObjectEmpty(this.model.last_deployment.deployable); + }, + + /** + * Verifies if deplyment internal ID should be rendered by verifing + * if all the information needed is present + * and if the environment is not a folder. + * + * @returns {Boolean} + */ + shouldRenderDeploymentID() { + return !this.isFolder && + !this.$options.isObjectEmpty(this.model.last_deployment) && + this.model.last_deployment.iid !== undefined; + }, + }, + + /** + * Helper to verify if certain given object are empty. + * Should be replaced by lodash _.isEmpty - https://lodash.com/docs/4.17.2#isEmpty + * @param {Object} object + * @returns {Bollean} + */ + isObjectEmpty(object) { + for (const key in object) { // eslint-disable-line + if (hasOwnProperty.call(object, key)) { + return false; + } + } + return true; }, template: ` @@ -355,7 +392,7 @@ <a v-if="!isFolder" class="environment-name" - :href="model.environment_url" + :href="model.environment_path" v-html="model.name"> </a> <span v-else v-on:click="toggleRow(model)" class="folder-name"> @@ -372,7 +409,7 @@ <td class="deployment-column"> <span - v-if="!isFolder && model.last_deployment && model.last_deployment.iid" + v-if="shouldRenderDeploymentID" v-html="deploymentInternalId"> </span> @@ -388,9 +425,9 @@ </td> <td> - <a v-if="!isFolder && model.last_deployment && model.last_deployment.deployable" + <a v-if="shouldRenderBuildName" class="build-link" - :href="model.last_deployment.deployable.build_url" + :href="model.last_deployment.deployable.build_path" v-html="buildName"> </a> </td> @@ -421,25 +458,29 @@ <td class="hidden-xs"> <div v-if="!isFolder"> - <div v-if="hasManualActions && canCreateDeployment" class="inline js-manual-actions-container"> + <div v-if="hasManualActions && canCreateDeployment" + class="inline js-manual-actions-container"> <actions-component :actions="manualActions"> </actions-component> </div> - <div v-if="model.external_url && canReadEnvironment" class="inline js-external-url-container"> + <div v-if="model.external_url && canReadEnvironment" + class="inline js-external-url-container"> <external-url-component :external_url="model.external_url"> </external_url-component> </div> - <div v-if="isStoppable && canCreateDeployment" class="inline js-stop-component-container"> + <div v-if="isStoppable && canCreateDeployment" + class="inline js-stop-component-container"> <stop-component - :stop_url="model.environment_url"> + :stop_url="model.environment_path"> </stop-component> </div> - <div v-if="canRetry && canCreateDeployment" class="inline js-rollback-component-container"> + <div v-if="canRetry && canCreateDeployment" + class="inline js-rollback-component-container"> <rollback-component :is_last_deployment="isLastDeployment" :retry_url="retryUrl"> diff --git a/app/assets/javascripts/environments/components/environment_stop.js.es6 b/app/assets/javascripts/environments/components/environment_stop.js.es6 index 779d16937ec..2fc56b89429 100644 --- a/app/assets/javascripts/environments/components/environment_stop.js.es6 +++ b/app/assets/javascripts/environments/components/environment_stop.js.es6 @@ -30,7 +30,7 @@ class="btn stop-env-link" :href="stopUrl" data-method="post" - data-rel="nofollow"> + rel="nofollow"> <i class="fa fa-stop stop-env-icon"></i> </a> `, diff --git a/app/assets/javascripts/environments/stores/environments_store.js.es6 b/app/assets/javascripts/environments/stores/environments_store.js.es6 index 928786f0741..0204a903ab5 100644 --- a/app/assets/javascripts/environments/stores/environments_store.js.es6 +++ b/app/assets/javascripts/environments/stores/environments_store.js.es6 @@ -89,17 +89,17 @@ toggleFolder(envType) { const environments = this.state.environments; - const environmnetsCopy = environments.map((env) => { - if (env['vue-isChildren'] === true && env.name === envType) { + const environmentsCopy = environments.map((env) => { + if (env['vue-isChildren'] && env.name === envType) { env.isOpen = !env.isOpen; } return env; }); - this.state.environments = environmnetsCopy; + this.state.environments = environmentsCopy; - return environmnetsCopy; + return environmentsCopy; }, /** @@ -125,15 +125,7 @@ const nameA = a.name.toUpperCase(); const nameB = b.name.toUpperCase(); - if (nameA < nameB) { - return -1; - } - - if (nameA > nameB) { - return 1; - } - - return 0; + return nameA < nameB ? -1 : nameA > nameB ? 1 : 0; // eslint-disable-line }, }; })(); diff --git a/app/assets/javascripts/vue_common_component/commit.js.es6 b/app/assets/javascripts/vue_common_component/commit.js.es6 index d449c550450..fd628fad4d7 100644 --- a/app/assets/javascripts/vue_common_component/commit.js.es6 +++ b/app/assets/javascripts/vue_common_component/commit.js.es6 @@ -143,7 +143,6 @@ </a> <div class="icon-container commit-icon commit-icon-container"> - <!-- svg goes here --> </div> <a class="commit-id monospace" @@ -153,7 +152,6 @@ <p class="commit-title"> <span v-if="title"> - <!-- commit author info--> <a v-if="hasAuthor" class="avatar-image-container" :href="author.web_url"> diff --git a/app/assets/stylesheets/pages/environments.scss b/app/assets/stylesheets/pages/environments.scss index dc45fbba61f..e9ff43a8adb 100644 --- a/app/assets/stylesheets/pages/environments.scss +++ b/app/assets/stylesheets/pages/environments.scss @@ -16,6 +16,8 @@ } .environments { + table-layout: fixed; + .deployment-column { .avatar { float: none; diff --git a/app/serializers/build_entity.rb b/app/serializers/build_entity.rb index 3d9ac66de0e..cf1c418a88e 100644 --- a/app/serializers/build_entity.rb +++ b/app/serializers/build_entity.rb @@ -4,21 +4,21 @@ class BuildEntity < Grape::Entity expose :id expose :name - expose :build_url do |build| - url_to(:namespace_project_build, build) + expose :build_path do |build| + path_to(:namespace_project_build, build) end - expose :retry_url do |build| - url_to(:retry_namespace_project_build, build) + expose :retry_path do |build| + path_to(:retry_namespace_project_build, build) end - expose :play_url, if: ->(build, _) { build.manual? } do |build| - url_to(:play_namespace_project_build, build) + expose :play_path, if: ->(build, _) { build.manual? } do |build| + path_to(:play_namespace_project_build, build) end private - def url_to(route, build) - send("#{route}_url", build.project.namespace, build.project, build) + def path_to(route, build) + send("#{route}_path", build.project.namespace, build.project, build) end end diff --git a/app/serializers/commit_entity.rb b/app/serializers/commit_entity.rb index f7eba6fc1e3..bc92a1c8545 100644 --- a/app/serializers/commit_entity.rb +++ b/app/serializers/commit_entity.rb @@ -3,8 +3,8 @@ class CommitEntity < API::Entities::RepoCommit expose :author, using: UserEntity - expose :commit_url do |commit| - namespace_project_tree_url( + expose :commit_path do |commit| + namespace_project_tree_path( request.project.namespace, request.project, id: commit.id) diff --git a/app/serializers/deployment_entity.rb b/app/serializers/deployment_entity.rb index ad6fc8d665b..d610fbe0c8a 100644 --- a/app/serializers/deployment_entity.rb +++ b/app/serializers/deployment_entity.rb @@ -10,8 +10,8 @@ class DeploymentEntity < Grape::Entity deployment.ref end - expose :ref_url do |deployment| - namespace_project_tree_url( + expose :ref_path do |deployment| + namespace_project_tree_path( deployment.project.namespace, deployment.project, id: deployment.ref) diff --git a/app/serializers/environment_entity.rb b/app/serializers/environment_entity.rb index ee4392cc46d..93534ef1b15 100644 --- a/app/serializers/environment_entity.rb +++ b/app/serializers/environment_entity.rb @@ -9,8 +9,8 @@ class EnvironmentEntity < Grape::Entity expose :last_deployment, using: DeploymentEntity expose :stoppable? - expose :environment_url do |environment| - namespace_project_environment_url( + expose :environment_path do |environment| + namespace_project_environment_path( environment.project.namespace, environment.project, environment) diff --git a/app/views/projects/environments/index.html.haml b/app/views/projects/environments/index.html.haml index b641d2cec34..a9235d6af35 100644 --- a/app/views/projects/environments/index.html.haml +++ b/app/views/projects/environments/index.html.haml @@ -16,4 +16,5 @@ "project-environments-path" => project_environments_path(@project), "project-stopped-environments-path" => project_environments_path(@project, scope: :stopped), "new-environment-path" => new_namespace_project_environment_path(@project.namespace, @project), - "help-page-path" => help_page_path("ci/environments")}, class: container_class } + "help-page-path" => help_page_path("ci/environments"), + "css-class" => container_class}} diff --git a/spec/features/environment_spec.rb b/spec/features/environment_spec.rb index 3bf2fd54088..0c1939fd885 100644 --- a/spec/features/environment_spec.rb +++ b/spec/features/environment_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -feature 'Environments', feature: true do +feature 'Environment', :feature do given(:project) { create(:empty_project) } given(:user) { create(:user) } given(:role) { :developer } @@ -10,13 +10,13 @@ feature 'Environments', feature: true do project.team << [user, role] end - describe 'when showing the environment' do - given(:environment) { create(:environment, project: project) } + feature 'environment details page' do + given!(:environment) { create(:environment, project: project) } given!(:deployment) { } given!(:manual) { } before do - visit namespace_project_environment_path(project.namespace, project, environment) + visit_environment(environment) end context 'without deployments' do @@ -26,20 +26,27 @@ feature 'Environments', feature: true do end context 'with deployments' do - given(:deployment) { create(:deployment, environment: environment) } + context 'when there is no related deployable' do + given(:deployment) do + create(:deployment, environment: environment, deployable: nil) + end - scenario 'does show deployment SHA' do - expect(page).to have_link(deployment.short_sha) - end + scenario 'does show deployment SHA' do + expect(page).to have_link(deployment.short_sha) + end - scenario 'does not show a re-deploy button for deployment without build' do - expect(page).not_to have_link('Re-deploy') + scenario 'does not show a re-deploy button for deployment without build' do + expect(page).not_to have_link('Re-deploy') + end end - context 'with build' do + context 'with related deployable present' do given(:pipeline) { create(:ci_pipeline, project: project) } given(:build) { create(:ci_build, pipeline: pipeline) } - given(:deployment) { create(:deployment, environment: environment, deployable: build) } + + given(:deployment) do + create(:deployment, environment: environment, deployable: build) + end scenario 'does show build name' do expect(page).to have_link("#{build.name} (##{build.id})") @@ -57,7 +64,6 @@ feature 'Environments', feature: true do given(:manual) { create(:ci_build, :manual, pipeline: pipeline, name: 'deploy to production') } scenario 'does show a play button' do - expect(page).to have_link(manual.name.humanize) end @@ -104,4 +110,52 @@ feature 'Environments', feature: true do end end end + + feature 'auto-close environment when branch is deleted' do + given(:project) { create(:project) } + + given!(:environment) do + create(:environment, :with_review_app, project: project, + ref: 'feature') + end + + scenario 'user visits environment page' do + visit_environment(environment) + + expect(page).to have_link('Stop') + end + + scenario 'user deletes the branch with running environment' do + visit namespace_project_branches_path(project.namespace, project) + + remove_branch_with_hooks(project, user, 'feature') do + page.within('.js-branch-feature') { find('a.btn-remove').click } + end + + visit_environment(environment) + + expect(page).to have_no_link('Stop') + end + + ## + # This is a workaround for problem described in #24543 + # + def remove_branch_with_hooks(project, user, branch) + params = { + oldrev: project.commit(branch).id, + newrev: Gitlab::Git::BLANK_SHA, + ref: "refs/heads/#{branch}" + } + + yield + + GitPushService.new(project, user, params).execute + end + end + + def visit_environment(environment) + visit namespace_project_environment_path(environment.project.namespace, + environment.project, + environment) + end end diff --git a/spec/features/environments_spec.rb b/spec/features/environments_spec.rb index 57c3800a85b..c7fe622c477 100644 --- a/spec/features/environments_spec.rb +++ b/spec/features/environments_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -feature 'Environments', feature: true, js: true do +feature 'Environments page', :feature, :js do given(:project) { create(:empty_project) } given(:user) { create(:user) } given(:role) { :developer } @@ -10,233 +10,138 @@ feature 'Environments', feature: true, js: true do login_as(user) end - describe 'when showing environments' do - given!(:environment) { } - given!(:deployment) { } - given!(:manual) { } + given!(:environment) { } + given!(:deployment) { } + given!(:manual) { } - before do - visit_environments(project) - end - - context 'shows two tabs' do - scenario 'shows "Available" and "Stopped" tab with links' do - expect(page).to have_link('Available') - expect(page).to have_link('Stopped') - end - end - - context 'without environments' do - scenario 'does show no environments' do - expect(page).to have_content('You don\'t have any environments right now.') - end + before do + visit_environments(project) + end - scenario 'does show 0 as counter for environments in both tabs' do - expect(page.find('.js-available-environments-count').text).to eq('0') - expect(page.find('.js-stopped-environments-count').text).to eq('0') - end + describe 'page tabs' do + scenario 'shows "Available" and "Stopped" tab with links' do + expect(page).to have_link('Available') + expect(page).to have_link('Stopped') end + end - context 'with environments' do - given(:environment) { create(:environment, project: project) } - - scenario 'does show environment name' do - expect(page).to have_link(environment.name) - end - - scenario 'does show number of available and stopped environments' do - expect(page.find('.js-available-environments-count').text).to eq('1') - expect(page.find('.js-stopped-environments-count').text).to eq('0') - end - - context 'without deployments' do - scenario 'does show no deployments' do - expect(page).to have_content('No deployments yet') - end - end - - context 'with deployments' do - given(:project) { create(:project) } - - given(:deployment) do - create(:deployment, environment: environment, - sha: project.commit.id) - end - - scenario 'does show deployment SHA' do - expect(page).to have_link(deployment.short_sha) - end - - scenario 'does show deployment internal id' do - expect(page).to have_content(deployment.iid) - end - - context 'with build and manual actions' do - given(:pipeline) { create(:ci_pipeline, project: project) } - given(:build) { create(:ci_build, pipeline: pipeline) } - given(:deployment) { create(:deployment, environment: environment, deployable: build) } - given(:manual) { create(:ci_build, :manual, pipeline: pipeline, name: 'deploy to production') } - - scenario 'does show a play button' do - find('.dropdown-play-icon-container').click - expect(page).to have_content(manual.name.humanize) - end - - scenario 'does allow to play manual action' do - expect(manual).to be_skipped - - find('.dropdown-play-icon-container').click - play_action = find('span', text: manual.name.humanize) - - expect(page).to have_content(manual.name.humanize) - expect { play_action.click }.not_to change { Ci::Pipeline.count } - - # TODO, fix me! - expect(manual.reload).to be_pending - end - - scenario 'does show build name and id' do - expect(page).to have_link("#{build.name} ##{build.id}") - end - - scenario 'does not show stop button' do - expect(page).not_to have_selector('.stop-env-link') - end - - scenario 'does not show external link button' do - expect(page).not_to have_css('external-url') - end - - context 'with external_url' do - given(:environment) { create(:environment, project: project, external_url: 'https://git.gitlab.com') } - given(:build) { create(:ci_build, pipeline: pipeline) } - given(:deployment) { create(:deployment, environment: environment, deployable: build) } - - scenario 'does show an external link button' do - expect(page).to have_link(nil, href: environment.external_url) - end - end - - context 'with stop action' do - given(:manual) { create(:ci_build, :manual, pipeline: pipeline, name: 'close_app') } - given(:deployment) { create(:deployment, environment: environment, deployable: build, on_stop: 'close_app') } - - scenario 'does show stop button' do - expect(page).to have_selector('.stop-env-link') - end - - scenario 'starts build when stop button clicked' do - find('.stop-env-link').click - - expect(page).to have_content('close_app') - end - - context 'for reporter' do - let(:role) { :reporter } - - scenario 'does not show stop button' do - expect(page).not_to have_selector('.stop-env-link') - end - end - end - end - end + context 'without environments' do + scenario 'does show no environments' do + expect(page).to have_content('You don\'t have any environments right now.') end - scenario 'does have a New environment button' do - expect(page).to have_link('New environment') + scenario 'does show 0 as counter for environments in both tabs' do + expect(page.find('.js-available-environments-count').text).to eq('0') + expect(page.find('.js-stopped-environments-count').text).to eq('0') end end describe 'when showing the environment' do given(:environment) { create(:environment, project: project) } - given!(:deployment) { } - given!(:manual) { } - before do - visit_environment(environment) + scenario 'does show environment name' do + expect(page).to have_link(environment.name) + end + + scenario 'does show number of available and stopped environments' do + expect(page.find('.js-available-environments-count').text).to eq('1') + expect(page.find('.js-stopped-environments-count').text).to eq('0') end context 'without deployments' do scenario 'does show no deployments' do - expect(page).to have_content('You don\'t have any deployments right now.') + expect(page).to have_content('No deployments yet') end end context 'with deployments' do + given(:project) { create(:project) } + given(:deployment) do - create(:deployment, environment: environment, deployable: nil) + create(:deployment, environment: environment, + sha: project.commit.id) end scenario 'does show deployment SHA' do expect(page).to have_link(deployment.short_sha) end - scenario 'does not show a re-deploy button for deployment without build' do - expect(page).not_to have_link('Re-deploy') + scenario 'does show deployment internal id' do + expect(page).to have_content(deployment.iid) end - context 'with build' do + context 'with build and manual actions' do given(:pipeline) { create(:ci_pipeline, project: project) } given(:build) { create(:ci_build, pipeline: pipeline) } - given(:deployment) { create(:deployment, environment: environment, deployable: build) } - scenario 'does show build name' do - expect(page).to have_link("#{build.name} (##{build.id})") + given(:manual) do + create(:ci_build, :manual, pipeline: pipeline, name: 'deploy to production') end - scenario 'does show re-deploy button' do - expect(page).to have_link('Re-deploy') + given(:deployment) do + create(:deployment, environment: environment, + deployable: build, + sha: project.commit.id) end - scenario 'does not show stop button' do - expect(page).not_to have_link('Stop') + scenario 'does show a play button' do + find('.dropdown-play-icon-container').click + expect(page).to have_content(manual.name.humanize) end - context 'with manual action' do - given(:manual) { create(:ci_build, :manual, pipeline: pipeline, name: 'deploy to production') } + scenario 'does allow to play manual action', js: true do + expect(manual).to be_skipped - scenario 'does show a play button' do - expect(page).to have_link(manual.name.humanize) - end + find('.dropdown-play-icon-container').click + expect(page).to have_content(manual.name.humanize) - scenario 'does allow to play manual action' do - expect(manual).to be_skipped - expect{ click_link(manual.name.humanize) }.not_to change { Ci::Pipeline.count } - expect(page).to have_content(manual.name) - expect(manual.reload).to be_pending - end + expect { click_link(manual.name.humanize) } + .not_to change { Ci::Pipeline.count } - context 'with external_url' do - given(:environment) { create(:environment, project: project, external_url: 'https://git.gitlab.com') } - given(:build) { create(:ci_build, pipeline: pipeline) } - given(:deployment) { create(:deployment, environment: environment, deployable: build) } + expect(manual.reload).to be_pending + end - scenario 'does show an external link button' do - expect(page).to have_link(nil, href: environment.external_url) - end + scenario 'does show build name and id' do + expect(page).to have_link("#{build.name} ##{build.id}") + end + + scenario 'does not show stop button' do + expect(page).not_to have_selector('.stop-env-link') + end + + scenario 'does not show external link button' do + expect(page).not_to have_css('external-url') + end + + context 'with external_url' do + given(:environment) { create(:environment, project: project, external_url: 'https://git.gitlab.com') } + given(:build) { create(:ci_build, pipeline: pipeline) } + given(:deployment) { create(:deployment, environment: environment, deployable: build) } + + scenario 'does show an external link button' do + expect(page).to have_link(nil, href: environment.external_url) end + end - context 'with stop action' do - given(:manual) { create(:ci_build, :manual, pipeline: pipeline, name: 'close_app') } - given(:deployment) { create(:deployment, environment: environment, deployable: build, on_stop: 'close_app') } + context 'with stop action' do + given(:manual) { create(:ci_build, :manual, pipeline: pipeline, name: 'close_app') } + given(:deployment) { create(:deployment, environment: environment, deployable: build, on_stop: 'close_app') } - scenario 'does show stop button' do - expect(page).to have_link('Stop') - end + scenario 'does show stop button' do + expect(page).to have_selector('.stop-env-link') + end - scenario 'does allow to stop environment' do - click_link('Stop') + scenario 'starts build when stop button clicked' do + find('.stop-env-link').click - expect(page).to have_content('close_app') - end + expect(page).to have_content('close_app') + end - context 'for reporter' do - let(:role) { :reporter } + context 'for reporter' do + let(:role) { :reporter } - scenario 'does not show stop button' do - expect(page).not_to have_link('Stop') - end + scenario 'does not show stop button' do + expect(page).not_to have_selector('.stop-env-link') end end end @@ -244,6 +149,10 @@ feature 'Environments', feature: true, js: true do end end + scenario 'does have a New environment button' do + expect(page).to have_link('New environment') + end + describe 'when creating a new environment' do before do visit_environments(project) @@ -286,55 +195,7 @@ feature 'Environments', feature: true, js: true do end end - feature 'auto-close environment when branch deleted' do - given(:project) { create(:project) } - - given!(:environment) do - create(:environment, :with_review_app, project: project, - ref: 'feature') - end - - scenario 'user visits environment page' do - visit_environment(environment) - - expect(page).to have_link('Stop') - end - - scenario 'user deletes the branch with running environment' do - visit namespace_project_branches_path(project.namespace, project) - - remove_branch_with_hooks(project, user, 'feature') do - page.within('.js-branch-feature') { find('a.btn-remove').click } - end - - visit_environment(environment) - - expect(page).to have_no_link('Stop') - end - - ## - # This is a workaround for problem described in #24543 - # - def remove_branch_with_hooks(project, user, branch) - params = { - oldrev: project.commit(branch).id, - newrev: Gitlab::Git::BLANK_SHA, - ref: "refs/heads/#{branch}" - } - - yield - - GitPushService.new(project, user, params).execute - end - end - def visit_environments(project) visit namespace_project_environments_path(project.namespace, project) end - - def visit_environment(environment) - visit namespace_project_environment_path(environment.project.namespace, - environment.project, - environment) - end end diff --git a/spec/javascripts/environments/mock_data.js.es6 b/spec/javascripts/environments/mock_data.js.es6 index 1142ace5846..9e16bc3e6a5 100644 --- a/spec/javascripts/environments/mock_data.js.es6 +++ b/spec/javascripts/environments/mock_data.js.es6 @@ -40,18 +40,18 @@ const environmentsList = [ avatar_url: 'http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80\u0026d=identicon', web_url: 'http://localhost:3000/root', }, - commit_url: 'http://localhost:3000/root/ci-folders/tree/500aabcb17c97bdcf2d0c410b70cb8556f0362dd', + commit_path: '/root/ci-folders/tree/500aabcb17c97bdcf2d0c410b70cb8556f0362dd', }, deployable: { id: 1278, name: 'build', - build_url: 'http://localhost:3000/root/ci-folders/builds/1278', - retry_url: 'http://localhost:3000/root/ci-folders/builds/1278/retry', + build_path: '/root/ci-folders/builds/1278', + retry_path: '/root/ci-folders/builds/1278/retry', }, manual_actions: [], }, 'stoppable?': true, - environment_url: 'http://localhost:3000/root/ci-folders/environments/31', + environment_path: '/root/ci-folders/environments/31', created_at: '2016-11-07T11:11:16.525Z', updated_at: '2016-11-07T11:11:16.525Z', }, @@ -95,18 +95,18 @@ const environmentsList = [ avatar_url: 'http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80\u0026d=identicon', web_url: 'http://localhost:3000/root', }, - commit_url: 'http://localhost:3000/root/ci-folders/tree/500aabcb17c97bdcf2d0c410b70cb8556f0362dd', + commit_path: '/root/ci-folders/tree/500aabcb17c97bdcf2d0c410b70cb8556f0362dd', }, deployable: { id: 1278, name: 'build', - build_url: 'http://localhost:3000/root/ci-folders/builds/1278', - retry_url: 'http://localhost:3000/root/ci-folders/builds/1278/retry', + build_path: '/root/ci-folders/builds/1278', + retry_path: '/root/ci-folders/builds/1278/retry', }, manual_actions: [], }, 'stoppable?': false, - environment_url: 'http://localhost:3000/root/ci-folders/environments/31', + environment_path: '/root/ci-folders/environments/31', created_at: '2016-11-07T11:11:16.525Z', updated_at: '2016-11-07T11:11:16.525Z', }, @@ -117,7 +117,7 @@ const environmentsList = [ environment_type: 'review', last_deployment: null, 'stoppable?': true, - environment_url: 'http://localhost:3000/root/ci-folders/environments/31', + environment_path: '/root/ci-folders/environments/31', created_at: '2016-11-07T11:11:16.525Z', updated_at: '2016-11-07T11:11:16.525Z', }, @@ -128,9 +128,8 @@ const environmentsList = [ environment_type: 'review', last_deployment: null, 'stoppable?': true, - environment_url: 'http://localhost:3000/root/ci-folders/environments/31', + environment_path: '/root/ci-folders/environments/31', created_at: '2016-11-07T11:11:16.525Z', updated_at: '2016-11-07T11:11:16.525Z', }, ]; - diff --git a/spec/serializers/build_entity_spec.rb b/spec/serializers/build_entity_spec.rb index 2734f5bedca..6dcfaec259e 100644 --- a/spec/serializers/build_entity_spec.rb +++ b/spec/serializers/build_entity_spec.rb @@ -10,9 +10,9 @@ describe BuildEntity do context 'when build is a regular job' do let(:build) { create(:ci_build) } - it 'contains url to build page and retry action' do - expect(subject).to include(:build_url, :retry_url) - expect(subject).not_to include(:play_url) + it 'contains paths to build page and retry action' do + expect(subject).to include(:build_path, :retry_path) + expect(subject).not_to include(:play_path) end it 'does not contain sensitive information' do @@ -24,8 +24,8 @@ describe BuildEntity do context 'when build is a manual action' do let(:build) { create(:ci_build, :manual) } - it 'contains url to play action' do - expect(subject).to include(:play_url) + it 'contains path to play action' do + expect(subject).to include(:play_path) end end end diff --git a/spec/serializers/commit_entity_spec.rb b/spec/serializers/commit_entity_spec.rb index 628e35c9a28..a44a23ef619 100644 --- a/spec/serializers/commit_entity_spec.rb +++ b/spec/serializers/commit_entity_spec.rb @@ -31,8 +31,8 @@ describe CommitEntity do end end - it 'contains commit URL' do - expect(subject).to include(:commit_url) + it 'contains path to commit' do + expect(subject).to include(:commit_path) end it 'needs to receive project in the request' do diff --git a/spec/serializers/deployment_entity_spec.rb b/spec/serializers/deployment_entity_spec.rb index 51b6de91571..ea87771e2a2 100644 --- a/spec/serializers/deployment_entity_spec.rb +++ b/spec/serializers/deployment_entity_spec.rb @@ -15,6 +15,6 @@ describe DeploymentEntity do it 'exposes nested information about branch' do expect(subject[:ref][:name]).to eq 'master' - expect(subject[:ref][:ref_url]).not_to be_empty + expect(subject[:ref][:ref_path]).not_to be_empty end end diff --git a/spec/serializers/environment_entity_spec.rb b/spec/serializers/environment_entity_spec.rb index 4ca8c299147..57728ce3181 100644 --- a/spec/serializers/environment_entity_spec.rb +++ b/spec/serializers/environment_entity_spec.rb @@ -13,6 +13,6 @@ describe EnvironmentEntity do end it 'exposes core elements of environment' do - expect(subject).to include(:id, :name, :state, :environment_url) + expect(subject).to include(:id, :name, :state, :environment_path) end end diff --git a/spec/serializers/environment_serializer_spec.rb b/spec/serializers/environment_serializer_spec.rb index 37bc086826c..8f95c9250b0 100644 --- a/spec/serializers/environment_serializer_spec.rb +++ b/spec/serializers/environment_serializer_spec.rb @@ -33,7 +33,7 @@ describe EnvironmentSerializer do it 'contains important elements of environment' do expect(json) - .to include(:name, :external_url, :environment_url, :last_deployment) + .to include(:name, :external_url, :environment_path, :last_deployment) end it 'contains relevant information about last deployment' do |