Skip to content

G
gitlab-ce
Commit 4850d381 authored by Mark Florian's avatar Mark Florian
Browse files
Options

Merge branch... 

Merge branch '196843-fix-giant-loader-in-container-registry-expiration-policy-settings' into 'master'

Contain registry expiration policy adjust loader and use toast

See merge request gitlab-org/gitlab!23242
parents 96ddac4f e13d67a5
Hide whitespace changes
Inline Side-by-side
Showing with 226 additions and 174 deletions
app/assets/javascripts/registry/settings/components/registry_settings_app.vue
View file @ 4850d381
<script>
import { mapState, mapActions } from 'vuex';
import { GlLoadingIcon } from '@gitlab/ui';
import { mapActions } from 'vuex';
import { FETCH_SETTINGS_ERROR_MESSAGE } from '../constants';
import SettingsForm from './settings_form.vue';
export default {
components: {
GlLoadingIcon,
SettingsForm,
},
computed: {
...mapState({
isLoading: 'isLoading',
}),
},
mounted() {
this.fetchSettings();
this.fetchSettings().catch(() =>
this.$toast.show(FETCH_SETTINGS_ERROR_MESSAGE, { type: 'error' }),
);
},
methods: {
...mapActions(['fetchSettings']),
......@@ -37,7 +34,6 @@ export default {
}}
</li>
</ul>
<gl-loading-icon v-if="isLoading" ref="loading-icon" size="xl" />
<settings-form v-else ref="settings-form" />
<settings-form ref="settings-form" />
</div>
</template>
app/assets/javascripts/registry/settings/components/settings_form.vue
View file @ 4850d381
<script>
import { mapActions, mapState } from 'vuex';
import { GlFormGroup, GlToggle, GlFormSelect, GlFormTextarea, GlButton, GlCard } from '@gitlab/ui';
import {
GlFormGroup,
GlToggle,
GlFormSelect,
GlFormTextarea,
GlButton,
GlCard,
GlLoadingIcon,
} from '@gitlab/ui';
import { s__, __, sprintf } from '~/locale';
import { NAME_REGEX_LENGTH } from '../constants';
import {
NAME_REGEX_LENGTH,
UPDATE_SETTINGS_ERROR_MESSAGE,
UPDATE_SETTINGS_SUCCESS_MESSAGE,
} from '../constants';
import { mapComputed } from '~/vuex_shared/bindings';
export default {
......@@ -13,13 +25,14 @@ export default {
GlFormTextarea,
GlButton,
GlCard,
GlLoadingIcon,
},
labelsConfig: {
cols: 3,
align: 'right',
},
computed: {
...mapState(['formOptions']),
...mapState(['formOptions', 'isLoading']),
...mapComputed(
[
'enabled',
......@@ -64,15 +77,26 @@ export default {
formIsInvalid() {
return this.nameRegexState === false;
},
isFormElementDisabled() {
return !this.enabled || this.isLoading;
},
isSubmitButtonDisabled() {
return this.formIsInvalid || this.isLoading;
},
},
methods: {
...mapActions(['resetSettings', 'saveSettings']),
submit() {
this.saveSettings()
.then(() => this.$toast.show(UPDATE_SETTINGS_SUCCESS_MESSAGE, { type: 'success' }))
.catch(() => this.$toast.show(UPDATE_SETTINGS_ERROR_MESSAGE, { type: 'error' }));
},
},
};
</script>
<template>
<form ref="form-element" @submit.prevent="saveSettings" @reset.prevent="resetSettings">
<form ref="form-element" @submit.prevent="submit" @reset.prevent="resetSettings">
<gl-card>
<template #header>
{{ s__('ContainerRegistry|Tag expiration policy') }}
......@@ -86,7 +110,7 @@ export default {
:label="s__('ContainerRegistry|Expiration policy:')"
>
<div class="d-flex align-items-start">
<gl-toggle id="expiration-policy-toggle" v-model="enabled" />
<gl-toggle id="expiration-policy-toggle" v-model="enabled" :disabled="isLoading" />
<span class="mb-2 ml-1 lh-2" v-html="toggleDescriptionText"></span>
</div>
</gl-form-group>
......@@ -98,7 +122,11 @@ export default {
label-for="expiration-policy-interval"
:label="s__('ContainerRegistry|Expiration interval:')"
>
<gl-form-select id="expiration-policy-interval" v-model="older_than" :disabled="!enabled">
<gl-form-select
id="expiration-policy-interval"
v-model="older_than"
:disabled="isFormElementDisabled"
>
<option v-for="option in formOptions.olderThan" :key="option.key" :value="option.key">
{{ option.label }}
</option>
......@@ -112,7 +140,11 @@ export default {
label-for="expiration-policy-schedule"
:label="s__('ContainerRegistry|Expiration schedule:')"
>
<gl-form-select id="expiration-policy-schedule" v-model="cadence" :disabled="!enabled">
<gl-form-select
id="expiration-policy-schedule"
v-model="cadence"
:disabled="isFormElementDisabled"
>
<option v-for="option in formOptions.cadence" :key="option.key" :value="option.key">
{{ option.label }}
</option>
......@@ -126,7 +158,11 @@ export default {
label-for="expiration-policy-latest"
:label="s__('ContainerRegistry|Number of tags to retain:')"
>
<gl-form-select id="expiration-policy-latest" v-model="keep_n" :disabled="!enabled">
<gl-form-select
id="expiration-policy-latest"
v-model="keep_n"
:disabled="isFormElementDisabled"
>
<option v-for="option in formOptions.keepN" :key="option.key" :value="option.key">
{{ option.label }}
</option>
......@@ -149,7 +185,7 @@ export default {
v-model="name_regex"
:placeholder="nameRegexPlaceholder"
:state="nameRegexState"
:disabled="!enabled"
:disabled="isFormElementDisabled"
trim
/>
<template #description>
......@@ -159,17 +195,18 @@ export default {
</template>
<template #footer>
<div class="d-flex justify-content-end">
<gl-button ref="cancel-button" type="reset" class="mr-2 d-block">{{
__('Cancel')
}}</gl-button>
<gl-button ref="cancel-button" type="reset" class="mr-2 d-block" :disabled="isLoading">
{{ __('Cancel') }}
</gl-button>
<gl-button
ref="save-button"
type="submit"
:disabled="formIsInvalid"
:disabled="isSubmitButtonDisabled"
variant="success"
class="d-block"
class="d-flex justify-content-center align-items-center js-no-auto-disable"
>
{{ __('Save expiration policy') }}
<gl-loading-icon v-if="isLoading" class="ml-2" />
</gl-button>
</div>
</template>
......
app/assets/javascripts/registry/settings/registry_settings_bundle.js
View file @ 4850d381
import Vue from 'vue';
import { GlToast } from '@gitlab/ui';
import Translate from '~/vue_shared/translate';
import store from './store/';
import RegistrySettingsApp from './components/registry_settings_app.vue';
Vue.use(GlToast);
Vue.use(Translate);
export default () => {
......
app/assets/javascripts/registry/settings/store/actions.js
View file @ 4850d381
import Api from '~/api';
import createFlash from '~/flash';
import {
FETCH_SETTINGS_ERROR_MESSAGE,
UPDATE_SETTINGS_ERROR_MESSAGE,
UPDATE_SETTINGS_SUCCESS_MESSAGE,
} from '../constants';
import * as types from './mutation_types';
export const setInitialState = ({ commit }, data) => commit(types.SET_INITIAL_STATE, data);
export const updateSettings = ({ commit }, data) => commit(types.UPDATE_SETTINGS, data);
export const toggleLoading = ({ commit }) => commit(types.TOGGLE_LOADING);
export const receiveSettingsSuccess = ({ commit }, data = {}) => commit(types.SET_SETTINGS, data);
export const receiveSettingsError = () => createFlash(FETCH_SETTINGS_ERROR_MESSAGE);
export const updateSettingsError = () => createFlash(UPDATE_SETTINGS_ERROR_MESSAGE);
export const resetSettings = ({ commit }) => commit(types.RESET_SETTINGS);
export const fetchSettings = ({ dispatch, state }) => {
......@@ -21,7 +13,6 @@ export const fetchSettings = ({ dispatch, state }) => {
.then(({ data: { container_expiration_policy } }) =>
dispatch('receiveSettingsSuccess', container_expiration_policy),
)
.catch(() => dispatch('receiveSettingsError'))
.finally(() => dispatch('toggleLoading'));
};
......@@ -30,11 +21,9 @@ export const saveSettings = ({ dispatch, state }) => {
return Api.updateProject(state.projectId, {
container_expiration_policy_attributes: state.settings,
})
.then(({ data: { container_expiration_policy } }) => {
dispatch('receiveSettingsSuccess', container_expiration_policy);
createFlash(UPDATE_SETTINGS_SUCCESS_MESSAGE, 'success');
})
.catch(() => dispatch('updateSettingsError'))
.then(({ data: { container_expiration_policy } }) =>
dispatch('receiveSettingsSuccess', container_expiration_policy),
)
.finally(() => dispatch('toggleLoading'));
};
......
spec/features/projects/settings/registry_settings_spec.rb
View file @ 4850d381
......@@ -17,10 +17,10 @@ describe 'Project > Settings > CI/CD > Container registry tag expiration policy'
expect(settings_block).to have_text 'Container Registry tag expiration policy'
end
it 'Save expiration policy submit the form', :js do
it 'Save expiration policy submit the form' do
within '#js-registry-policies' do
within '.card-body' do
click_button(class: 'gl-toggle')
find('#expiration-policy-toggle button:not(.is-disabled)').click
select('7 days until tags are automatically removed', from: 'expiration-policy-interval')
select('Every day', from: 'expiration-policy-schedule')
select('50 tags per image name', from: 'expiration-policy-latest')
......@@ -30,8 +30,8 @@ describe 'Project > Settings > CI/CD > Container registry tag expiration policy'
expect(submit_button).not_to be_disabled
submit_button.click
end
flash_text = find('.flash-text')
expect(flash_text).to have_content('Expiration policy successfully saved.')
toast = find('.gl-toast')
expect(toast).to have_content('Expiration policy successfully saved.')
end
end
end
spec/frontend/registry/settings/components/__snapshots__/settings_form_spec.js.snap
View file @ 4850d381
......@@ -161,17 +161,20 @@ exports[`Settings Form renders 1`] = `
class="mr-2 d-block"
type="reset"
>
Cancel
</glbutton-stub>
<glbutton-stub
class="d-block"
class="d-flex justify-content-center align-items-center js-no-auto-disable"
type="submit"
variant="success"
>
Save expiration policy
<!---->
</glbutton-stub>
</div>
</div>
......
spec/frontend/registry/settings/components/registry_settings_app_spec.js
View file @ 4850d381
import Vuex from 'vuex';
import { shallowMount, createLocalVue } from '@vue/test-utils';
import { shallowMount } from '@vue/test-utils';
import component from '~/registry/settings/components/registry_settings_app.vue';
import { createStore } from '~/registry/settings/store/';
const localVue = createLocalVue();
localVue.use(Vuex);
import { FETCH_SETTINGS_ERROR_MESSAGE } from '~/registry/settings/constants';
describe('Registry Settings App', () => {
let wrapper;
let store;
let fetchSpy;
const findSettingsComponent = () => wrapper.find({ ref: 'settings-form' });
const findLoadingComponent = () => wrapper.find({ ref: 'loading-icon' });
const mountComponent = (options = {}) => {
fetchSpy = jest.fn();
const mountComponent = ({ dispatchMock } = {}) => {
store = createStore();
const dispatchSpy = jest.spyOn(store, 'dispatch');
if (dispatchMock) {
dispatchSpy[dispatchMock]();
}
wrapper = shallowMount(component, {
store,
methods: {
fetchSettings: fetchSpy,
mocks: {
$toast: {
show: jest.fn(),
},
},
...options,
store,
});
};
beforeEach(() => {
store = createStore();
mountComponent();
});
afterEach(() => {
wrapper.destroy();
});
it('renders', () => {
mountComponent({ dispatchMock: 'mockResolvedValue' });
expect(wrapper.element).toMatchSnapshot();
});
it('call the store function to load the data on mount', () => {
expect(fetchSpy).toHaveBeenCalled();
mountComponent({ dispatchMock: 'mockResolvedValue' });
expect(store.dispatch).toHaveBeenCalledWith('fetchSettings');
});
it('renders a loader if isLoading is true', () => {
store.dispatch('toggleLoading');
return wrapper.vm.$nextTick().then(() => {
expect(findLoadingComponent().exists()).toBe(true);
expect(findSettingsComponent().exists()).toBe(false);
});
it('show a toast if fetchSettings fails', () => {
mountComponent({ dispatchMock: 'mockRejectedValue' });
return wrapper.vm.$nextTick().then(() =>
expect(wrapper.vm.$toast.show).toHaveBeenCalledWith(FETCH_SETTINGS_ERROR_MESSAGE, {
type: 'error',
}),
);
});
it('renders the setting form', () => {
mountComponent({ dispatchMock: 'mockResolvedValue' });
expect(findSettingsComponent().exists()).toBe(true);
});
});
spec/frontend/registry/settings/components/settings_form_spec.js
View file @ 4850d381
import Vuex from 'vuex';
import { mount, createLocalVue } from '@vue/test-utils';
import { mount } from '@vue/test-utils';
import stubChildren from 'helpers/stub_children';
import component from '~/registry/settings/components/settings_form.vue';
import { createStore } from '~/registry/settings/store/';
import { NAME_REGEX_LENGTH } from '~/registry/settings/constants';
import {
NAME_REGEX_LENGTH,
UPDATE_SETTINGS_ERROR_MESSAGE,
UPDATE_SETTINGS_SUCCESS_MESSAGE,
} from '~/registry/settings/constants';
import { stringifiedFormOptions } from '../mock_data';
const localVue = createLocalVue();
localVue.use(Vuex);
describe('Settings Form', () => {
let wrapper;
let store;
let saveSpy;
let resetSpy;
let dispatchSpy;
const FORM_ELEMENTS_ID_PREFIX = '#expiration-policy';
const GlLoadingIcon = { name: 'gl-loading-icon-stub', template: '<svg></svg>' };
const findFormGroup = name => wrapper.find(`#expiration-policy-${name}-group`);
const findFormElements = (name, father = wrapper) => father.find(`#expiration-policy-${name}`);
const findFormGroup = name => wrapper.find(`${FORM_ELEMENTS_ID_PREFIX}-${name}-group`);
const findFormElements = (name, parent = wrapper) =>
parent.find(`${FORM_ELEMENTS_ID_PREFIX}-${name}`);
const findCancelButton = () => wrapper.find({ ref: 'cancel-button' });
const findSaveButton = () => wrapper.find({ ref: 'save-button' });
const findForm = () => wrapper.find({ ref: 'form-element' });
const findLoadingIcon = (parent = wrapper) => parent.find(GlLoadingIcon);
const mountComponent = (options = {}) => {
saveSpy = jest.fn();
resetSpy = jest.fn();
wrapper = mount(component, {
stubs: {
...stubChildren(component),
GlCard: false,
GlLoadingIcon,
},
store,
methods: {
saveSettings: saveSpy,
resetSettings: resetSpy,
mocks: {
$toast: {
show: jest.fn(),
},
},
store,
...options,
});
};
......@@ -41,6 +46,7 @@ describe('Settings Form', () => {
beforeEach(() => {
store = createStore();
store.dispatch('setInitialState', stringifiedFormOptions);
dispatchSpy = jest.spyOn(store, 'dispatch');
mountComponent();
});
......@@ -59,48 +65,53 @@ describe('Settings Form', () => {
${'schedule'} | ${'cadence'} | ${'foo'} | ${'disabled'}
${'latest'} | ${'keep_n'} | ${'foo'} | ${'disabled'}
${'name-matching'} | ${'name_regex'} | ${'foo'} | ${'disabled'}
`('$elementName form element', ({ elementName, modelName, value, disabledByToggle }) => {
let formGroup;
beforeEach(() => {
formGroup = findFormGroup(elementName);
});
it(`${elementName} form group exist in the dom`, () => {
expect(formGroup.exists()).toBe(true);
});
`(
`${FORM_ELEMENTS_ID_PREFIX}-$elementName form element`,
({ elementName, modelName, value, disabledByToggle }) => {
let formGroup;
beforeEach(() => {
formGroup = findFormGroup(elementName);
});
it(`${elementName} form group exist in the dom`, () => {
expect(formGroup.exists()).toBe(true);
});
it(`${elementName} form group has a label-for property`, () => {
expect(formGroup.attributes('label-for')).toBe(`expiration-policy-${elementName}`);
});
it(`${elementName} form group has a label-for property`, () => {
expect(formGroup.attributes('label-for')).toBe(`expiration-policy-${elementName}`);
});
it(`${elementName} form group has a label-cols property`, () => {
expect(formGroup.attributes('label-cols')).toBe(`${wrapper.vm.$options.labelsConfig.cols}`);
});
it(`${elementName} form group has a label-cols property`, () => {
expect(formGroup.attributes('label-cols')).toBe(`${wrapper.vm.$options.labelsConfig.cols}`);
});
it(`${elementName} form group has a label-align property`, () => {
expect(formGroup.attributes('label-align')).toBe(`${wrapper.vm.$options.labelsConfig.align}`);
});
it(`${elementName} form group has a label-align property`, () => {
expect(formGroup.attributes('label-align')).toBe(
`${wrapper.vm.$options.labelsConfig.align}`,
);
});
it(`${elementName} form group contains an input element`, () => {
expect(findFormElements(elementName, formGroup).exists()).toBe(true);
});
it(`${elementName} form group contains an input element`, () => {
expect(findFormElements(elementName, formGroup).exists()).toBe(true);
});
it(`${elementName} form element change updated ${modelName} with ${value}`, () => {
const element = findFormElements(elementName, formGroup);
const modelUpdateEvent = element.vm.$options.model
? element.vm.$options.model.event
: 'input';
element.vm.$emit(modelUpdateEvent, value);
return wrapper.vm.$nextTick().then(() => {
expect(wrapper.vm[modelName]).toBe(value);
it(`${elementName} form element change updated ${modelName} with ${value}`, () => {
const element = findFormElements(elementName, formGroup);
const modelUpdateEvent = element.vm.$options.model
? element.vm.$options.model.event
: 'input';
element.vm.$emit(modelUpdateEvent, value);
return wrapper.vm.$nextTick().then(() => {
expect(wrapper.vm[modelName]).toBe(value);
});
});
});
it(`${elementName} is ${disabledByToggle} by enabled set to false`, () => {
store.dispatch('updateSettings', { enabled: false });
const expectation = disabledByToggle === 'disabled' ? 'true' : undefined;
expect(findFormElements(elementName, formGroup).attributes('disabled')).toBe(expectation);
});
});
it(`${elementName} is ${disabledByToggle} by enabled set to false`, () => {
store.dispatch('updateSettings', { enabled: false });
const expectation = disabledByToggle === 'disabled' ? 'true' : undefined;
expect(findFormElements(elementName, formGroup).attributes('disabled')).toBe(expectation);
});
},
);
describe('form actions', () => {
let form;
......@@ -112,17 +123,79 @@ describe('Settings Form', () => {
});
it('form reset event call the appropriate function', () => {
dispatchSpy.mockReturnValue();
form.trigger('reset');
expect(resetSpy).toHaveBeenCalled();
// expect.any(Object) is necessary because the event payload is passed to the function
expect(dispatchSpy).toHaveBeenCalledWith('resetSettings', expect.any(Object));
});
it('save has type submit', () => {
expect(findSaveButton().attributes('type')).toBe('submit');
});
it('form submit event call the appropriate function', () => {
form.trigger('submit');
expect(saveSpy).toHaveBeenCalled();
describe('when isLoading is true', () => {
beforeEach(() => {
store.dispatch('toggleLoading');
});
afterEach(() => {
store.dispatch('toggleLoading');
});
it.each`
elementName
${'toggle'}
${'interval'}
${'schedule'}
${'latest'}
${'name-matching'}
`(`${FORM_ELEMENTS_ID_PREFIX}-$elementName is disabled`, ({ elementName }) => {
expect(findFormElements(elementName).attributes('disabled')).toBe('true');
});
it('submit button is disabled and shows a spinner', () => {
const button = findSaveButton();
expect(button.attributes('disabled')).toBeTruthy();
expect(findLoadingIcon(button)).toExist();
});
it('cancel button is disabled', () => {
expect(findCancelButton().attributes('disabled')).toBeTruthy();
});
});
describe('form submit event ', () => {
it('calls the appropriate function', () => {
dispatchSpy.mockResolvedValue();
form.trigger('submit');
expect(dispatchSpy).toHaveBeenCalled();
});
it('dispatches the saveSettings action', () => {
dispatchSpy.mockResolvedValue();
form.trigger('submit');
expect(dispatchSpy).toHaveBeenCalledWith('saveSettings');
});
it('show a success toast when submit succeed', () => {
dispatchSpy.mockResolvedValue();
form.trigger('submit');
return wrapper.vm.$nextTick().then(() => {
expect(wrapper.vm.$toast.show).toHaveBeenCalledWith(UPDATE_SETTINGS_SUCCESS_MESSAGE, {
type: 'success',
});
});
});
it('show an error toast when submit fails', () => {
dispatchSpy.mockRejectedValue();
form.trigger('submit');
return wrapper.vm.$nextTick().then(() => {
expect(wrapper.vm.$toast.show).toHaveBeenCalledWith(UPDATE_SETTINGS_ERROR_MESSAGE, {
type: 'error',
});
});
});
});
});
......@@ -160,7 +233,7 @@ describe('Settings Form', () => {
it('toggleDescriptionText text reflects enabled property', () => {
const toggleHelpText = findFormGroup('toggle').find('span');
expect(toggleHelpText.html()).toContain('disabled');
wrapper.vm.enabled = true;
wrapper.setData({ enabled: true });
return wrapper.vm.$nextTick().then(() => {
expect(toggleHelpText.html()).toContain('enabled');
});
......
spec/frontend/registry/settings/store/actions_spec.js
View file @ 4850d381
import Api from '~/api';
import createFlash from '~/flash';
import testAction from 'helpers/vuex_action_helper';
import * as actions from '~/registry/settings/store/actions';
import * as types from '~/registry/settings/store/mutation_types';
import {
UPDATE_SETTINGS_ERROR_MESSAGE,
FETCH_SETTINGS_ERROR_MESSAGE,
UPDATE_SETTINGS_SUCCESS_MESSAGE,
} from '~/registry/settings/constants';
jest.mock('~/flash');
describe('Actions Registry Store', () => {
describe.each`
......@@ -25,19 +17,6 @@ describe('Actions Registry Store', () => {
});
});
describe.each`
actionName | message
${'receiveSettingsError'} | ${FETCH_SETTINGS_ERROR_MESSAGE}
${'updateSettingsError'} | ${UPDATE_SETTINGS_ERROR_MESSAGE}
`('%s action', ({ actionName, message }) => {
it(`should call createFlash with ${message}`, done => {
testAction(actions[actionName], null, null, [], [], () => {
expect(createFlash).toHaveBeenCalledWith(message);
done();
});
});
});
describe('fetchSettings', () => {
const state = {
projectId: 'bar',
......@@ -64,18 +43,6 @@ describe('Actions Registry Store', () => {
done,
);
});
it('should call receiveSettingsError on error', done => {
Api.project = jest.fn().mockRejectedValue();
testAction(
actions.fetchSettings,
null,
state,
[],
[{ type: 'toggleLoading' }, { type: 'receiveSettingsError' }, { type: 'toggleLoading' }],
done,
);
});
});
describe('saveSettings', () => {
......@@ -102,21 +69,6 @@ describe('Actions Registry Store', () => {
{ type: 'receiveSettingsSuccess', payload: payload.data.container_expiration_policy },
{ type: 'toggleLoading' },
],
() => {
expect(createFlash).toHaveBeenCalledWith(UPDATE_SETTINGS_SUCCESS_MESSAGE, 'success');
done();
},
);
});
it('should call receiveSettingsError on error', done => {
Api.updateProject = jest.fn().mockRejectedValue();
testAction(
actions.saveSettings,
null,
state,
[],
[{ type: 'toggleLoading' }, { type: 'updateSettingsError' }, { type: 'toggleLoading' }],
done,
);
});
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7