Commit d7a3f875 authored by Fatih Acet's avatar Fatih Acet

Merge branch '56398-fix-broken-cluster-installation-loading-state' into 'master'

Fix cluster installation processing spinner (reopened)

Closes #56398

See merge request gitlab-org/gitlab-ce!24814
parents 9a0dd375 5b211055
...@@ -6,7 +6,7 @@ import Flash from '../flash'; ...@@ -6,7 +6,7 @@ import Flash from '../flash';
import Poll from '../lib/utils/poll'; import Poll from '../lib/utils/poll';
import initSettingsPanels from '../settings_panels'; import initSettingsPanels from '../settings_panels';
import eventHub from './event_hub'; import eventHub from './event_hub';
import { APPLICATION_STATUS, REQUEST_LOADING, REQUEST_SUCCESS, REQUEST_FAILURE } from './constants'; import { APPLICATION_STATUS, REQUEST_SUBMITTED, REQUEST_FAILURE } from './constants';
import ClustersService from './services/clusters_service'; import ClustersService from './services/clusters_service';
import ClustersStore from './stores/clusters_store'; import ClustersStore from './stores/clusters_store';
import Applications from './components/applications.vue'; import Applications from './components/applications.vue';
...@@ -231,15 +231,11 @@ export default class Clusters { ...@@ -231,15 +231,11 @@ export default class Clusters {
installApplication(data) { installApplication(data) {
const appId = data.id; const appId = data.id;
this.store.updateAppProperty(appId, 'requestStatus', REQUEST_LOADING); this.store.updateAppProperty(appId, 'requestStatus', REQUEST_SUBMITTED);
this.store.updateAppProperty(appId, 'requestReason', null); this.store.updateAppProperty(appId, 'requestReason', null);
this.store.updateAppProperty(appId, 'statusReason', null);
this.service this.service.installApplication(appId, data.params).catch(() => {
.installApplication(appId, data.params)
.then(() => {
this.store.updateAppProperty(appId, 'requestStatus', REQUEST_SUCCESS);
})
.catch(() => {
this.store.updateAppProperty(appId, 'requestStatus', REQUEST_FAILURE); this.store.updateAppProperty(appId, 'requestStatus', REQUEST_FAILURE);
this.store.updateAppProperty( this.store.updateAppProperty(
appId, appId,
......
...@@ -4,12 +4,7 @@ import { s__, sprintf } from '../../locale'; ...@@ -4,12 +4,7 @@ import { s__, sprintf } from '../../locale';
import eventHub from '../event_hub'; import eventHub from '../event_hub';
import identicon from '../../vue_shared/components/identicon.vue'; import identicon from '../../vue_shared/components/identicon.vue';
import loadingButton from '../../vue_shared/components/loading_button.vue'; import loadingButton from '../../vue_shared/components/loading_button.vue';
import { import { APPLICATION_STATUS, REQUEST_SUBMITTED, REQUEST_FAILURE } from '../constants';
APPLICATION_STATUS,
REQUEST_LOADING,
REQUEST_SUCCESS,
REQUEST_FAILURE,
} from '../constants';
export default { export default {
components: { components: {
...@@ -72,6 +67,13 @@ export default { ...@@ -72,6 +67,13 @@ export default {
isKnownStatus() { isKnownStatus() {
return Object.values(APPLICATION_STATUS).includes(this.status); return Object.values(APPLICATION_STATUS).includes(this.status);
}, },
isInstalling() {
return (
this.status === APPLICATION_STATUS.SCHEDULED ||
this.status === APPLICATION_STATUS.INSTALLING ||
(this.requestStatus === REQUEST_SUBMITTED && !this.statusReason && !this.isInstalled)
);
},
isInstalled() { isInstalled() {
return ( return (
this.status === APPLICATION_STATUS.INSTALLED || this.status === APPLICATION_STATUS.INSTALLED ||
...@@ -79,6 +81,18 @@ export default { ...@@ -79,6 +81,18 @@ export default {
this.status === APPLICATION_STATUS.UPDATING this.status === APPLICATION_STATUS.UPDATING
); );
}, },
canInstall() {
if (this.isInstalling) {
return false;
}
return (
this.status === APPLICATION_STATUS.NOT_INSTALLABLE ||
this.status === APPLICATION_STATUS.INSTALLABLE ||
this.status === APPLICATION_STATUS.ERROR ||
this.isUnknownStatus
);
},
hasLogo() { hasLogo() {
return !!this.logoUrl; return !!this.logoUrl;
}, },
...@@ -90,12 +104,7 @@ export default { ...@@ -90,12 +104,7 @@ export default {
return `js-cluster-application-row-${this.id}`; return `js-cluster-application-row-${this.id}`;
}, },
installButtonLoading() { installButtonLoading() {
return ( return !this.status || this.status === APPLICATION_STATUS.SCHEDULED || this.isInstalling;
!this.status ||
this.status === APPLICATION_STATUS.SCHEDULED ||
this.status === APPLICATION_STATUS.INSTALLING ||
this.requestStatus === REQUEST_LOADING
);
}, },
installButtonDisabled() { installButtonDisabled() {
// Avoid the potential for the real-time data to say APPLICATION_STATUS.INSTALLABLE but // Avoid the potential for the real-time data to say APPLICATION_STATUS.INSTALLABLE but
...@@ -104,30 +113,17 @@ export default { ...@@ -104,30 +113,17 @@ export default {
return ( return (
((this.status !== APPLICATION_STATUS.INSTALLABLE && ((this.status !== APPLICATION_STATUS.INSTALLABLE &&
this.status !== APPLICATION_STATUS.ERROR) || this.status !== APPLICATION_STATUS.ERROR) ||
this.requestStatus === REQUEST_LOADING || this.isInstalling) &&
this.requestStatus === REQUEST_SUCCESS) &&
this.isKnownStatus this.isKnownStatus
); );
}, },
installButtonLabel() { installButtonLabel() {
let label; let label;
if ( if (this.canInstall) {
this.status === APPLICATION_STATUS.NOT_INSTALLABLE ||
this.status === APPLICATION_STATUS.INSTALLABLE ||
this.status === APPLICATION_STATUS.ERROR ||
this.isUnknownStatus
) {
label = s__('ClusterIntegration|Install'); label = s__('ClusterIntegration|Install');
} else if ( } else if (this.isInstalling) {
this.status === APPLICATION_STATUS.SCHEDULED ||
this.status === APPLICATION_STATUS.INSTALLING
) {
label = s__('ClusterIntegration|Installing'); label = s__('ClusterIntegration|Installing');
} else if ( } else if (this.isInstalled) {
this.status === APPLICATION_STATUS.INSTALLED ||
this.status === APPLICATION_STATUS.UPDATED ||
this.status === APPLICATION_STATUS.UPDATING
) {
label = s__('ClusterIntegration|Installed'); label = s__('ClusterIntegration|Installed');
} }
...@@ -140,7 +136,10 @@ export default { ...@@ -140,7 +136,10 @@ export default {
return s__('ClusterIntegration|Manage'); return s__('ClusterIntegration|Manage');
}, },
hasError() { hasError() {
return this.status === APPLICATION_STATUS.ERROR || this.requestStatus === REQUEST_FAILURE; return (
!this.isInstalling &&
(this.status === APPLICATION_STATUS.ERROR || this.requestStatus === REQUEST_FAILURE)
);
}, },
generalErrorDescription() { generalErrorDescription() {
return sprintf(s__('ClusterIntegration|Something went wrong while installing %{title}'), { return sprintf(s__('ClusterIntegration|Something went wrong while installing %{title}'), {
......
...@@ -18,8 +18,7 @@ export const APPLICATION_STATUS = { ...@@ -18,8 +18,7 @@ export const APPLICATION_STATUS = {
}; };
// These are only used client-side // These are only used client-side
export const REQUEST_LOADING = 'request-loading'; export const REQUEST_SUBMITTED = 'request-submitted';
export const REQUEST_SUCCESS = 'request-success';
export const REQUEST_FAILURE = 'request-failure'; export const REQUEST_FAILURE = 'request-failure';
export const INGRESS = 'ingress'; export const INGRESS = 'ingress';
export const JUPYTER = 'jupyter'; export const JUPYTER = 'jupyter';
......
---
title: Fix cluster installation processing spinner
merge_request: 24814
author:
type: fixed
...@@ -48,9 +48,9 @@ describe 'Clusters Applications', :js do ...@@ -48,9 +48,9 @@ describe 'Clusters Applications', :js do
it 'they see status transition' do it 'they see status transition' do
page.within('.js-cluster-application-row-helm') do page.within('.js-cluster-application-row-helm') do
# FE sends request and gets the response, then the buttons is "Install" # FE sends request and gets the response, then the buttons is "Installing"
expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to eq('true') expect(page.find(:css, '.js-cluster-application-install-button')['disabled']).to eq('true')
expect(page).to have_css('.js-cluster-application-install-button', exact_text: 'Install') expect(page).to have_css('.js-cluster-application-install-button', exact_text: 'Installing')
wait_until_helm_created! wait_until_helm_created!
...@@ -118,7 +118,7 @@ describe 'Clusters Applications', :js do ...@@ -118,7 +118,7 @@ describe 'Clusters Applications', :js do
page.within('.js-cluster-application-row-cert_manager') do page.within('.js-cluster-application-row-cert_manager') do
expect(email_form_value).to eq(cluster.user.email) expect(email_form_value).to eq(cluster.user.email)
expect(page).to have_css('.js-cluster-application-install-button', exact_text: 'Install') expect(page).to have_css('.js-cluster-application-install-button', exact_text: 'Installing')
page.find('.js-email').set("new_email@example.org") page.find('.js-email').set("new_email@example.org")
Clusters::Cluster.last.application_cert_manager.make_installing! Clusters::Cluster.last.application_cert_manager.make_installing!
...@@ -153,9 +153,9 @@ describe 'Clusters Applications', :js do ...@@ -153,9 +153,9 @@ describe 'Clusters Applications', :js do
it 'they see status transition' do it 'they see status transition' do
page.within('.js-cluster-application-row-ingress') do page.within('.js-cluster-application-row-ingress') do
# FE sends request and gets the response, then the buttons is "Install" # FE sends request and gets the response, then the buttons is "Installing"
expect(page).to have_css('.js-cluster-application-install-button[disabled]') expect(page).to have_css('.js-cluster-application-install-button[disabled]')
expect(page).to have_css('.js-cluster-application-install-button', exact_text: 'Install') expect(page).to have_css('.js-cluster-application-install-button', exact_text: 'Installing')
Clusters::Cluster.last.application_ingress.make_installing! Clusters::Cluster.last.application_ingress.make_installing!
......
import Clusters from '~/clusters/clusters_bundle'; import Clusters from '~/clusters/clusters_bundle';
import { import { REQUEST_SUBMITTED, REQUEST_FAILURE, APPLICATION_STATUS } from '~/clusters/constants';
REQUEST_LOADING,
REQUEST_SUCCESS,
REQUEST_FAILURE,
APPLICATION_STATUS,
} from '~/clusters/constants';
import getSetTimeoutPromise from 'spec/helpers/set_timeout_promise_helper'; import getSetTimeoutPromise from 'spec/helpers/set_timeout_promise_helper';
describe('Clusters', () => { describe('Clusters', () => {
...@@ -196,67 +191,43 @@ describe('Clusters', () => { ...@@ -196,67 +191,43 @@ describe('Clusters', () => {
}); });
describe('installApplication', () => { describe('installApplication', () => {
it('tries to install helm', done => { it('tries to install helm', () => {
spyOn(cluster.service, 'installApplication').and.returnValue(Promise.resolve()); spyOn(cluster.service, 'installApplication').and.returnValue(Promise.resolve());
expect(cluster.store.state.applications.helm.requestStatus).toEqual(null); expect(cluster.store.state.applications.helm.requestStatus).toEqual(null);
cluster.installApplication({ id: 'helm' }); cluster.installApplication({ id: 'helm' });
expect(cluster.store.state.applications.helm.requestStatus).toEqual(REQUEST_LOADING); expect(cluster.store.state.applications.helm.requestStatus).toEqual(REQUEST_SUBMITTED);
expect(cluster.store.state.applications.helm.requestReason).toEqual(null); expect(cluster.store.state.applications.helm.requestReason).toEqual(null);
expect(cluster.service.installApplication).toHaveBeenCalledWith('helm', undefined); expect(cluster.service.installApplication).toHaveBeenCalledWith('helm', undefined);
getSetTimeoutPromise()
.then(() => {
expect(cluster.store.state.applications.helm.requestStatus).toEqual(REQUEST_SUCCESS);
expect(cluster.store.state.applications.helm.requestReason).toEqual(null);
})
.then(done)
.catch(done.fail);
}); });
it('tries to install ingress', done => { it('tries to install ingress', () => {
spyOn(cluster.service, 'installApplication').and.returnValue(Promise.resolve()); spyOn(cluster.service, 'installApplication').and.returnValue(Promise.resolve());
expect(cluster.store.state.applications.ingress.requestStatus).toEqual(null); expect(cluster.store.state.applications.ingress.requestStatus).toEqual(null);
cluster.installApplication({ id: 'ingress' }); cluster.installApplication({ id: 'ingress' });
expect(cluster.store.state.applications.ingress.requestStatus).toEqual(REQUEST_LOADING); expect(cluster.store.state.applications.ingress.requestStatus).toEqual(REQUEST_SUBMITTED);
expect(cluster.store.state.applications.ingress.requestReason).toEqual(null); expect(cluster.store.state.applications.ingress.requestReason).toEqual(null);
expect(cluster.service.installApplication).toHaveBeenCalledWith('ingress', undefined); expect(cluster.service.installApplication).toHaveBeenCalledWith('ingress', undefined);
getSetTimeoutPromise()
.then(() => {
expect(cluster.store.state.applications.ingress.requestStatus).toEqual(REQUEST_SUCCESS);
expect(cluster.store.state.applications.ingress.requestReason).toEqual(null);
})
.then(done)
.catch(done.fail);
}); });
it('tries to install runner', done => { it('tries to install runner', () => {
spyOn(cluster.service, 'installApplication').and.returnValue(Promise.resolve()); spyOn(cluster.service, 'installApplication').and.returnValue(Promise.resolve());
expect(cluster.store.state.applications.runner.requestStatus).toEqual(null); expect(cluster.store.state.applications.runner.requestStatus).toEqual(null);
cluster.installApplication({ id: 'runner' }); cluster.installApplication({ id: 'runner' });
expect(cluster.store.state.applications.runner.requestStatus).toEqual(REQUEST_LOADING); expect(cluster.store.state.applications.runner.requestStatus).toEqual(REQUEST_SUBMITTED);
expect(cluster.store.state.applications.runner.requestReason).toEqual(null); expect(cluster.store.state.applications.runner.requestReason).toEqual(null);
expect(cluster.service.installApplication).toHaveBeenCalledWith('runner', undefined); expect(cluster.service.installApplication).toHaveBeenCalledWith('runner', undefined);
getSetTimeoutPromise()
.then(() => {
expect(cluster.store.state.applications.runner.requestStatus).toEqual(REQUEST_SUCCESS);
expect(cluster.store.state.applications.runner.requestReason).toEqual(null);
})
.then(done)
.catch(done.fail);
}); });
it('tries to install jupyter', done => { it('tries to install jupyter', () => {
spyOn(cluster.service, 'installApplication').and.returnValue(Promise.resolve()); spyOn(cluster.service, 'installApplication').and.returnValue(Promise.resolve());
expect(cluster.store.state.applications.jupyter.requestStatus).toEqual(null); expect(cluster.store.state.applications.jupyter.requestStatus).toEqual(null);
...@@ -265,19 +236,11 @@ describe('Clusters', () => { ...@@ -265,19 +236,11 @@ describe('Clusters', () => {
params: { hostname: cluster.store.state.applications.jupyter.hostname }, params: { hostname: cluster.store.state.applications.jupyter.hostname },
}); });
expect(cluster.store.state.applications.jupyter.requestStatus).toEqual(REQUEST_LOADING); expect(cluster.store.state.applications.jupyter.requestStatus).toEqual(REQUEST_SUBMITTED);
expect(cluster.store.state.applications.jupyter.requestReason).toEqual(null); expect(cluster.store.state.applications.jupyter.requestReason).toEqual(null);
expect(cluster.service.installApplication).toHaveBeenCalledWith('jupyter', { expect(cluster.service.installApplication).toHaveBeenCalledWith('jupyter', {
hostname: cluster.store.state.applications.jupyter.hostname, hostname: cluster.store.state.applications.jupyter.hostname,
}); });
getSetTimeoutPromise()
.then(() => {
expect(cluster.store.state.applications.jupyter.requestStatus).toEqual(REQUEST_SUCCESS);
expect(cluster.store.state.applications.jupyter.requestReason).toEqual(null);
})
.then(done)
.catch(done.fail);
}); });
it('sets error request status when the request fails', done => { it('sets error request status when the request fails', done => {
...@@ -289,7 +252,7 @@ describe('Clusters', () => { ...@@ -289,7 +252,7 @@ describe('Clusters', () => {
cluster.installApplication({ id: 'helm' }); cluster.installApplication({ id: 'helm' });
expect(cluster.store.state.applications.helm.requestStatus).toEqual(REQUEST_LOADING); expect(cluster.store.state.applications.helm.requestStatus).toEqual(REQUEST_SUBMITTED);
expect(cluster.store.state.applications.helm.requestReason).toEqual(null); expect(cluster.store.state.applications.helm.requestReason).toEqual(null);
expect(cluster.service.installApplication).toHaveBeenCalled(); expect(cluster.service.installApplication).toHaveBeenCalled();
......
import Vue from 'vue'; import Vue from 'vue';
import eventHub from '~/clusters/event_hub'; import eventHub from '~/clusters/event_hub';
import { import { APPLICATION_STATUS, REQUEST_SUBMITTED, REQUEST_FAILURE } from '~/clusters/constants';
APPLICATION_STATUS,
REQUEST_LOADING,
REQUEST_SUCCESS,
REQUEST_FAILURE,
} from '~/clusters/constants';
import applicationRow from '~/clusters/components/application_row.vue'; import applicationRow from '~/clusters/components/application_row.vue';
import mountComponent from 'spec/helpers/vue_mount_component_helper'; import mountComponent from 'spec/helpers/vue_mount_component_helper';
import { DEFAULT_APPLICATION_STATE } from '../services/mock_data'; import { DEFAULT_APPLICATION_STATE } from '../services/mock_data';
...@@ -57,6 +52,12 @@ describe('Application Row', () => { ...@@ -57,6 +52,12 @@ describe('Application Row', () => {
expect(vm.installButtonLabel).toBeUndefined(); expect(vm.installButtonLabel).toBeUndefined();
}); });
it('has install button', () => {
const installationBtn = vm.$el.querySelector('.js-cluster-application-install-button');
expect(installationBtn).not.toBe(null);
});
it('has disabled "Install" when APPLICATION_STATUS.NOT_INSTALLABLE', () => { it('has disabled "Install" when APPLICATION_STATUS.NOT_INSTALLABLE', () => {
vm = mountComponent(ApplicationRow, { vm = mountComponent(ApplicationRow, {
...DEFAULT_APPLICATION_STATE, ...DEFAULT_APPLICATION_STATE,
...@@ -101,6 +102,18 @@ describe('Application Row', () => { ...@@ -101,6 +102,18 @@ describe('Application Row', () => {
expect(vm.installButtonDisabled).toEqual(true); expect(vm.installButtonDisabled).toEqual(true);
}); });
it('has loading "Installing" when REQUEST_SUBMITTED', () => {
vm = mountComponent(ApplicationRow, {
...DEFAULT_APPLICATION_STATE,
status: APPLICATION_STATUS.INSTALLABLE,
requestStatus: REQUEST_SUBMITTED,
});
expect(vm.installButtonLabel).toEqual('Installing');
expect(vm.installButtonLoading).toEqual(true);
expect(vm.installButtonDisabled).toEqual(true);
});
it('has disabled "Installed" when APPLICATION_STATUS.INSTALLED', () => { it('has disabled "Installed" when APPLICATION_STATUS.INSTALLED', () => {
vm = mountComponent(ApplicationRow, { vm = mountComponent(ApplicationRow, {
...DEFAULT_APPLICATION_STATE, ...DEFAULT_APPLICATION_STATE,
...@@ -134,30 +147,6 @@ describe('Application Row', () => { ...@@ -134,30 +147,6 @@ describe('Application Row', () => {
expect(vm.installButtonDisabled).toEqual(false); expect(vm.installButtonDisabled).toEqual(false);
}); });
it('has loading "Install" when REQUEST_LOADING', () => {
vm = mountComponent(ApplicationRow, {
...DEFAULT_APPLICATION_STATE,
status: APPLICATION_STATUS.INSTALLABLE,
requestStatus: REQUEST_LOADING,
});
expect(vm.installButtonLabel).toEqual('Install');
expect(vm.installButtonLoading).toEqual(true);
expect(vm.installButtonDisabled).toEqual(true);
});
it('has disabled "Install" when REQUEST_SUCCESS', () => {
vm = mountComponent(ApplicationRow, {
...DEFAULT_APPLICATION_STATE,
status: APPLICATION_STATUS.INSTALLABLE,
requestStatus: REQUEST_SUCCESS,
});
expect(vm.installButtonLabel).toEqual('Install');
expect(vm.installButtonLoading).toEqual(false);
expect(vm.installButtonDisabled).toEqual(true);
});
it('has enabled "Install" when REQUEST_FAILURE (so you can try installing again)', () => { it('has enabled "Install" when REQUEST_FAILURE (so you can try installing again)', () => {
vm = mountComponent(ApplicationRow, { vm = mountComponent(ApplicationRow, {
...DEFAULT_APPLICATION_STATE, ...DEFAULT_APPLICATION_STATE,
......
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