From db054bc1668e2c9a1d745d00dbe3ba4dd74f6929 Mon Sep 17 00:00:00 2001 From: syasonik Date: Wed, 16 Jan 2019 14:05:19 -0600 Subject: [PATCH] Support flat response for envs index route To support environment folders in the UI on the Environments List page, the environments index route previously returned one environment per folder, excluding those other than the latest deploy. However, the environtments dropdown on the metrics dashboard requires that any environment be selectable. To accommodate both use cases, support an optional 'nested' parameter in the index route to return either a flat, complete response or a nested response based on the use case in question. The new default response structure is the flat response. --- .../environments/mixins/environments_mixin.js | 2 +- .../services/environments_service.js | 4 +- .../environments/stores/environments_store.js | 3 +- .../monitoring/components/dashboard.vue | 8 +- .../monitoring/stores/monitoring_store.js | 4 +- .../projects/environments_controller.rb | 21 ++--- changelogs/unreleased/fix-49388.yml | 5 ++ .../projects/environments_controller_spec.rb | 43 ++++++++-- spec/javascripts/monitoring/mock_data.js | 80 ++++++++----------- 9 files changed, 97 insertions(+), 73 deletions(-) create mode 100644 changelogs/unreleased/fix-49388.yml diff --git a/app/assets/javascripts/environments/mixins/environments_mixin.js b/app/assets/javascripts/environments/mixins/environments_mixin.js index 96dc1f07cb9..e81a1525df0 100644 --- a/app/assets/javascripts/environments/mixins/environments_mixin.js +++ b/app/assets/javascripts/environments/mixins/environments_mixin.js @@ -143,7 +143,7 @@ export default { */ created() { this.service = new EnvironmentsService(this.endpoint); - this.requestData = { page: this.page, scope: this.scope }; + this.requestData = { page: this.page, scope: this.scope, nested: true }; this.poll = new Poll({ resource: this.service, diff --git a/app/assets/javascripts/environments/services/environments_service.js b/app/assets/javascripts/environments/services/environments_service.js index 4e07ccba91a..cb4ff6856db 100644 --- a/app/assets/javascripts/environments/services/environments_service.js +++ b/app/assets/javascripts/environments/services/environments_service.js @@ -7,8 +7,8 @@ export default class EnvironmentsService { } fetchEnvironments(options = {}) { - const { scope, page } = options; - return axios.get(this.environmentsEndpoint, { params: { scope, page } }); + const { scope, page, nested } = options; + return axios.get(this.environmentsEndpoint, { params: { scope, page, nested } }); } // eslint-disable-next-line class-methods-use-this diff --git a/app/assets/javascripts/environments/stores/environments_store.js b/app/assets/javascripts/environments/stores/environments_store.js index 5808a2d4afa..ac9a31c202c 100644 --- a/app/assets/javascripts/environments/stores/environments_store.js +++ b/app/assets/javascripts/environments/stores/environments_store.js @@ -20,7 +20,8 @@ export default class EnvironmentsStore { * * Stores the received environments. * - * In the main environments endpoint, each environment has the following schema + * In the main environments endpoint (with { nested: true } in params), each folder + * has the following schema: * { name: String, size: Number, latest: Object } * In the endpoint to retrieve environments from each folder, the environment does * not have the `latest` key and the data is all in the root level. diff --git a/app/assets/javascripts/monitoring/components/dashboard.vue b/app/assets/javascripts/monitoring/components/dashboard.vue index cea5c1a56ca..973fc8e10c9 100644 --- a/app/assets/javascripts/monitoring/components/dashboard.vue +++ b/app/assets/javascripts/monitoring/components/dashboard.vue @@ -196,13 +196,13 @@ export default { class="dropdown-menu dropdown-menu-selectable dropdown-menu-drop-up" > diff --git a/app/assets/javascripts/monitoring/stores/monitoring_store.js b/app/assets/javascripts/monitoring/stores/monitoring_store.js index 8692c873a41..96ecc5ab8a8 100644 --- a/app/assets/javascripts/monitoring/stores/monitoring_store.js +++ b/app/assets/javascripts/monitoring/stores/monitoring_store.js @@ -66,9 +66,7 @@ export default class MonitoringStore { } storeEnvironmentsData(environmentsData = []) { - this.environmentsData = environmentsData.filter( - environment => !!environment.latest.last_deployment, - ); + this.environmentsData = environmentsData.filter(environment => !!environment.last_deployment); } getMetricsCount() { diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index a63eea0ca0e..1a1b024d766 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -15,6 +15,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController push_frontend_feature_flag(:area_chart, project) end + # Returns all environments or all folders based on the :nested param def index @environments = project.environments .with_state(params[:scope] || :available) @@ -25,11 +26,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController Gitlab::PollingInterval.set_header(response, interval: 3_000) render json: { - environments: EnvironmentSerializer - .new(project: @project, current_user: @current_user) - .with_pagination(request, response) - .within_folders - .represent(@environments), + environments: serialize_environments(request, response, params[:nested]), available_count: project.environments.available.count, stopped_count: project.environments.stopped.count } @@ -37,6 +34,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController end end + # Returns all environments for a given folder # rubocop: disable CodeReuse/ActiveRecord def folder folder_environments = project.environments.where(environment_type: params[:id]) @@ -48,10 +46,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController format.html format.json do render json: { - environments: EnvironmentSerializer - .new(project: @project, current_user: @current_user) - .with_pagination(request, response) - .represent(@environments), + environments: serialize_environments(request, response), available_count: folder_environments.available.count, stopped_count: folder_environments.stopped.count } @@ -186,6 +181,14 @@ class Projects::EnvironmentsController < Projects::ApplicationController @environment ||= project.environments.find(params[:id]) end + def serialize_environments(request, response, nested = false) + serializer = EnvironmentSerializer + .new(project: @project, current_user: @current_user) + .with_pagination(request, response) + serializer = serializer.within_folders if nested + serializer.represent(@environments) + end + def authorize_stop_environment! access_denied! unless can?(current_user, :stop_environment, environment) end diff --git a/changelogs/unreleased/fix-49388.yml b/changelogs/unreleased/fix-49388.yml new file mode 100644 index 00000000000..f8b5e3e1943 --- /dev/null +++ b/changelogs/unreleased/fix-49388.yml @@ -0,0 +1,5 @@ +--- +title: Update metrics environment dropdown to show complete option set +merge_request: 24441 +author: +type: fixed diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index 94fb85f217c..a4d494a820f 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -47,9 +47,43 @@ describe Projects::EnvironmentsController do let(:environments) { json_response['environments'] } + context 'with default parameters' do + before do + get :index, params: environment_params(format: :json) + end + + it 'responds with a flat payload describing available environments' do + expect(environments.count).to eq 3 + expect(environments.first['name']).to eq 'production' + expect(environments.second['name']).to eq 'staging/review-1' + expect(environments.third['name']).to eq 'staging/review-2' + expect(json_response['available_count']).to eq 3 + expect(json_response['stopped_count']).to eq 1 + end + + it 'sets the polling interval header' do + expect(response).to have_gitlab_http_status(:ok) + expect(response.headers['Poll-Interval']).to eq("3000") + end + end + + context 'when a folder-based nested structure is requested' do + before do + get :index, params: environment_params(format: :json, nested: true) + end + + it 'responds with a payload containing the latest environment for each folder' do + expect(environments.count).to eq 2 + expect(environments.first['name']).to eq 'production' + expect(environments.second['name']).to eq 'staging' + expect(environments.second['size']).to eq 2 + expect(environments.second['latest']['name']).to eq 'staging/review-2' + end + end + context 'when requesting available environments scope' do before do - get :index, params: environment_params(format: :json, scope: :available) + get :index, params: environment_params(format: :json, nested: true, scope: :available) end it 'responds with a payload describing available environments' do @@ -64,16 +98,11 @@ describe Projects::EnvironmentsController do expect(json_response['available_count']).to eq 3 expect(json_response['stopped_count']).to eq 1 end - - it 'sets the polling interval header' do - expect(response).to have_gitlab_http_status(:ok) - expect(response.headers['Poll-Interval']).to eq("3000") - end end context 'when requesting stopped environments scope' do before do - get :index, params: environment_params(format: :json, scope: :stopped) + get :index, params: environment_params(format: :json, nested: true, scope: :stopped) end it 'responds with a payload describing stopped environments' do diff --git a/spec/javascripts/monitoring/mock_data.js b/spec/javascripts/monitoring/mock_data.js index 18ad9843d22..b4e2cd75d47 100644 --- a/spec/javascripts/monitoring/mock_data.js +++ b/spec/javascripts/monitoring/mock_data.js @@ -6597,58 +6597,46 @@ export function convertDatesMultipleSeries(multipleSeries) { export const environmentData = [ { + id: 34, name: 'production', - size: 1, - latest: { - id: 34, - name: 'production', - state: 'available', - external_url: 'http://root-autodevops-deploy.my-fake-domain.com', - environment_type: null, - stop_action: false, - metrics_path: '/root/hello-prometheus/environments/34/metrics', - environment_path: '/root/hello-prometheus/environments/34', - stop_path: '/root/hello-prometheus/environments/34/stop', - terminal_path: '/root/hello-prometheus/environments/34/terminal', - folder_path: '/root/hello-prometheus/environments/folders/production', - created_at: '2018-06-29T16:53:38.301Z', - updated_at: '2018-06-29T16:57:09.825Z', - last_deployment: { - id: 127, - }, + state: 'available', + external_url: 'http://root-autodevops-deploy.my-fake-domain.com', + environment_type: null, + stop_action: false, + metrics_path: '/root/hello-prometheus/environments/34/metrics', + environment_path: '/root/hello-prometheus/environments/34', + stop_path: '/root/hello-prometheus/environments/34/stop', + terminal_path: '/root/hello-prometheus/environments/34/terminal', + folder_path: '/root/hello-prometheus/environments/folders/production', + created_at: '2018-06-29T16:53:38.301Z', + updated_at: '2018-06-29T16:57:09.825Z', + last_deployment: { + id: 127, }, }, { - name: 'review', - size: 1, - latest: { - id: 35, - name: 'review/noop-branch', - state: 'available', - external_url: 'http://root-autodevops-deploy-review-noop-branc-die93w.my-fake-domain.com', - environment_type: 'review', - stop_action: true, - metrics_path: '/root/hello-prometheus/environments/35/metrics', - environment_path: '/root/hello-prometheus/environments/35', - stop_path: '/root/hello-prometheus/environments/35/stop', - terminal_path: '/root/hello-prometheus/environments/35/terminal', - folder_path: '/root/hello-prometheus/environments/folders/review', - created_at: '2018-07-03T18:39:41.702Z', - updated_at: '2018-07-03T18:44:54.010Z', - last_deployment: { - id: 128, - }, + id: 35, + name: 'review/noop-branch', + state: 'available', + external_url: 'http://root-autodevops-deploy-review-noop-branc-die93w.my-fake-domain.com', + environment_type: 'review', + stop_action: true, + metrics_path: '/root/hello-prometheus/environments/35/metrics', + environment_path: '/root/hello-prometheus/environments/35', + stop_path: '/root/hello-prometheus/environments/35/stop', + terminal_path: '/root/hello-prometheus/environments/35/terminal', + folder_path: '/root/hello-prometheus/environments/folders/review', + created_at: '2018-07-03T18:39:41.702Z', + updated_at: '2018-07-03T18:44:54.010Z', + last_deployment: { + id: 128, }, }, { - name: 'no-deployment', - size: 1, - latest: { - id: 36, - name: 'no-deployment/noop-branch', - state: 'available', - created_at: '2018-07-04T18:39:41.702Z', - updated_at: '2018-07-04T18:44:54.010Z', - }, + id: 36, + name: 'no-deployment/noop-branch', + state: 'available', + created_at: '2018-07-04T18:39:41.702Z', + updated_at: '2018-07-04T18:44:54.010Z', }, ];