Commit b9da1851 authored by Andrew Fontaine's avatar Andrew Fontaine Committed by Peter Hegman

Fall back to avaiable scope if scope is invalid

On environments, only 2 scopes are valid: available and stopped. If an
invalid scope is passed via the query params, the controller attempted
to use it and would return a 500 error.

This behaviour is still useful for the JSON API, and so we only fetch
the environments if the format is JSON.

However, as the environments are fetched via API after the HTML is
returned, there's no need to query the environments when the format is
HTML, meaning no need to use the scope. This stops invalid query
params (like ?scope=foo) from returning a 500 error. To stop the
resultant API calls from failing, we can check to see if the requested
scope is valid. If it is not, default to the available scope.

Changelog: fixed
parent 5960d9cc
......@@ -3,6 +3,7 @@ import { GlButton, GlCollapse, GlIcon, GlBadge, GlLink } from '@gitlab/ui';
import { __, s__ } from '~/locale';
import pollIntervalQuery from '../graphql/queries/poll_interval.query.graphql';
import folderQuery from '../graphql/queries/folder.query.graphql';
import { ENVIRONMENT_COUNT_BY_SCOPE } from '../constants';
import EnvironmentItem from './new_environment_item.vue';
export default {
......@@ -56,7 +57,8 @@ export default {
return this.visible ? this.$options.i18n.collapse : this.$options.i18n.expand;
},
count() {
return this.folder?.[`${this.scope}Count`] ?? 0;
const count = ENVIRONMENT_COUNT_BY_SCOPE[this.scope];
return this.folder?.[count] ?? 0;
},
folderClass() {
return { 'gl-font-weight-bold': this.visible };
......
......@@ -9,6 +9,7 @@ import environmentToDeleteQuery from '../graphql/queries/environment_to_delete.q
import environmentToRollbackQuery from '../graphql/queries/environment_to_rollback.query.graphql';
import environmentToStopQuery from '../graphql/queries/environment_to_stop.query.graphql';
import environmentToChangeCanaryQuery from '../graphql/queries/environment_to_change_canary.query.graphql';
import { ENVIRONMENTS_SCOPE } from '../constants';
import EnvironmentFolder from './new_environment_folder.vue';
import EnableReviewAppModal from './enable_review_app_modal.vue';
import StopEnvironmentModal from './stop_environment_modal.vue';
......@@ -82,12 +83,14 @@ export default {
},
modalId: 'enable-review-app-info',
data() {
const { page = '1', scope = 'available' } = queryToObject(window.location.search);
const { page = '1', scope } = queryToObject(window.location.search);
return {
interval: undefined,
isReviewAppModalVisible: false,
page: parseInt(page, 10),
scope,
scope: Object.values(ENVIRONMENTS_SCOPE).includes(scope)
? scope
: ENVIRONMENTS_SCOPE.AVAILABLE,
environmentToDelete: {},
environmentToRollback: {},
environmentToStop: {},
......@@ -188,6 +191,7 @@ export default {
});
},
},
ENVIRONMENTS_SCOPE,
};
</script>
<template>
......@@ -209,7 +213,10 @@ export default {
query-param-name="scope"
@primary="showReviewAppModal"
>
<gl-tab query-param-value="available" @click="setScope('available')">
<gl-tab
:query-param-value="$options.ENVIRONMENTS_SCOPE.AVAILABLE"
@click="setScope($options.ENVIRONMENTS_SCOPE.AVAILABLE)"
>
<template #title>
<span>{{ $options.i18n.available }}</span>
<gl-badge size="sm" class="gl-tab-counter-badge">
......@@ -217,7 +224,10 @@ export default {
</gl-badge>
</template>
</gl-tab>
<gl-tab query-param-value="stopped" @click="setScope('stopped')">
<gl-tab
:query-param-value="$options.ENVIRONMENTS_SCOPE.STOPPED"
@click="setScope($options.ENVIRONMENTS_SCOPE.STOPPED)"
>
<template #title>
<span>{{ $options.i18n.stopped }}</span>
<gl-badge size="sm" class="gl-tab-counter-badge">
......
......@@ -38,3 +38,13 @@ export const CANARY_STATUS = {
};
export const CANARY_UPDATE_MODAL = 'confirm-canary-change';
export const ENVIRONMENTS_SCOPE = {
AVAILABLE: 'available',
STOPPED: 'stopped',
};
export const ENVIRONMENT_COUNT_BY_SCOPE = {
[ENVIRONMENTS_SCOPE.AVAILABLE]: 'availableCount',
[ENVIRONMENTS_SCOPE.STOPPED]: 'stoppedCount',
};
......@@ -29,13 +29,14 @@ class Projects::EnvironmentsController < Projects::ApplicationController
feature_category :continuous_delivery
def index
@environments = project.environments
.with_state(params[:scope] || :available)
@project = ProjectPresenter.new(project, current_user: current_user)
respond_to do |format|
format.html
format.json do
@environments = project.environments
.with_state(params[:scope] || :available)
Gitlab::PollingInterval.set_header(response, interval: 3_000)
environments_count_by_state = project.environments.count_by_state
......@@ -52,14 +53,15 @@ class Projects::EnvironmentsController < Projects::ApplicationController
# Returns all environments for a given folder
# rubocop: disable CodeReuse/ActiveRecord
def folder
folder_environments = project.environments.where(environment_type: params[:id])
@environments = folder_environments.with_state(params[:scope] || :available)
.order(:name)
@folder = params[:id]
respond_to do |format|
format.html
format.json do
folder_environments = project.environments.where(environment_type: params[:id])
@environments = folder_environments.with_state(params[:scope] || :available)
.order(:name)
render json: {
environments: serialize_environments(request, response),
available_count: folder_environments.available.count,
......
......@@ -70,8 +70,9 @@ describe('~/environments/components/new_environments_app.vue', () => {
previousPage: 1,
__typename: 'LocalPageInfo',
},
location = '?scope=available&page=2',
}) => {
setWindowLocation('?scope=available&page=2');
setWindowLocation(location);
environmentAppMock.mockReturnValue(environmentsApp);
environmentFolderMock.mockReturnValue(folder);
paginationMock.mockReturnValue(pageInfo);
......@@ -98,6 +99,21 @@ describe('~/environments/components/new_environments_app.vue', () => {
wrapper.destroy();
});
it('should request available environments if the scope is invalid', async () => {
await createWrapperWithMocked({
environmentsApp: resolvedEnvironmentsApp,
folder: resolvedFolder,
location: '?scope=bad&page=2',
});
expect(environmentAppMock).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({ scope: 'available', page: 2 }),
expect.anything(),
expect.anything(),
);
});
it('should show all the folders that are fetched', async () => {
await createWrapperWithMocked({
environmentsApp: resolvedEnvironmentsApp,
......
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