From 9fb1df8f9781ea5f1c5db71ef4baeabc9b369528 Mon Sep 17 00:00:00 2001 From: Mike Greiling Date: Fri, 2 Aug 2019 20:08:53 +0000 Subject: [PATCH] Hide cluster details until cluster is created Only display the details of the cluster page when the cluster exists. If it is in "creating" state, show a message and a spinner --- .../javascripts/clusters/clusters_bundle.js | 62 +++++++++++++------ .../projects/gke_cluster_namespace/index.js | 6 +- app/views/clusters/clusters/_banner.html.haml | 5 +- app/views/clusters/clusters/show.html.haml | 41 ++++++------ ...automatically-after-cluster-is-created.yml | 5 ++ locale/gitlab.pot | 2 +- .../projects/clusters/applications_spec.rb | 5 +- .../frontend/clusters/clusters_bundle_spec.js | 59 ++++++++++++++---- 8 files changed, 126 insertions(+), 59 deletions(-) create mode 100644 changelogs/unreleased/50130-cluster-cluster-details-update-automatically-after-cluster-is-created.yml diff --git a/app/assets/javascripts/clusters/clusters_bundle.js b/app/assets/javascripts/clusters/clusters_bundle.js index aacfa0d87e6..5f5c8044b49 100644 --- a/app/assets/javascripts/clusters/clusters_bundle.js +++ b/app/assets/javascripts/clusters/clusters_bundle.js @@ -48,6 +48,9 @@ export default class Clusters { } = document.querySelector('.js-edit-cluster-form').dataset; this.clusterId = clusterId; + this.clusterNewlyCreatedKey = `cluster_${this.clusterId}_newly_created`; + this.clusterBannerDismissedKey = `cluster_${this.clusterId}_banner_dismissed`; + this.store = new ClustersStore(); this.store.setHelpPaths(helpPath, ingressHelpPath, ingressDnsHelpPath); this.store.setManagePrometheusPath(managePrometheusPath); @@ -81,18 +84,19 @@ export default class Clusters { this.showTokenButton = document.querySelector('.js-show-cluster-token'); this.tokenField = document.querySelector('.js-cluster-token'); this.ingressDomainHelpText = document.querySelector('.js-ingress-domain-help-text'); - this.ingressDomainSnippet = this.ingressDomainHelpText.querySelector( - '.js-ingress-domain-snippet', - ); + this.ingressDomainSnippet = + this.ingressDomainHelpText && + this.ingressDomainHelpText.querySelector('.js-ingress-domain-snippet'); Clusters.initDismissableCallout(); initSettingsPanels(); - setupToggleButtons(document.querySelector('.js-cluster-enable-toggle-area')); + const toggleButtonsContainer = document.querySelector('.js-cluster-enable-toggle-area'); + if (toggleButtonsContainer) { + setupToggleButtons(toggleButtonsContainer); + } this.initApplications(clusterType); - if (this.store.state.status !== 'created') { - this.updateContainer(null, this.store.state.status, this.store.state.statusReason); - } + this.updateContainer(null, this.store.state.status, this.store.state.statusReason); this.addListeners(); if (statusPath) { @@ -247,35 +251,56 @@ export default class Clusters { setBannerDismissedState(status, isDismissed) { if (AccessorUtilities.isLocalStorageAccessSafe()) { - window.localStorage.setItem( - `cluster_${this.clusterId}_banner_dismissed`, - `${status}_${isDismissed}`, - ); + window.localStorage.setItem(this.clusterBannerDismissedKey, `${status}_${isDismissed}`); } } isBannerDismissed(status) { let bannerState; if (AccessorUtilities.isLocalStorageAccessSafe()) { - bannerState = window.localStorage.getItem(`cluster_${this.clusterId}_banner_dismissed`); + bannerState = window.localStorage.getItem(this.clusterBannerDismissedKey); } return bannerState === `${status}_true`; } - updateContainer(prevStatus, status, error) { - this.hideAll(); + setClusterNewlyCreated(state) { + if (AccessorUtilities.isLocalStorageAccessSafe()) { + window.localStorage.setItem(this.clusterNewlyCreatedKey, Boolean(state)); + } + } - if (this.isBannerDismissed(status)) { + isClusterNewlyCreated() { + // once this is true, it will always be true for a given page load + if (!this.isNewlyCreated) { + let newlyCreated; + if (AccessorUtilities.isLocalStorageAccessSafe()) { + newlyCreated = window.localStorage.getItem(this.clusterNewlyCreatedKey); + } + + this.isNewlyCreated = newlyCreated === 'true'; + } + return this.isNewlyCreated; + } + + updateContainer(prevStatus, status, error) { + if (status !== 'created' && this.isBannerDismissed(status)) { return; } this.setBannerDismissedState(status, false); - // We poll all the time but only want the `created` banner to show when newly created - if (this.store.state.status !== 'created' || prevStatus !== this.store.state.status) { + if (prevStatus !== status) { + this.hideAll(); + switch (status) { case 'created': - this.successContainer.classList.remove('hidden'); + if (this.isClusterNewlyCreated()) { + this.setClusterNewlyCreated(false); + this.successContainer.classList.remove('hidden'); + } else if (prevStatus) { + this.setClusterNewlyCreated(true); + window.location.reload(); + } break; case 'errored': this.errorContainer.classList.remove('hidden'); @@ -292,7 +317,6 @@ export default class Clusters { this.creatingContainer.classList.remove('hidden'); break; default: - this.hideAll(); } } } diff --git a/app/assets/javascripts/projects/gke_cluster_namespace/index.js b/app/assets/javascripts/projects/gke_cluster_namespace/index.js index 288740203ad..0ec4d8807b0 100644 --- a/app/assets/javascripts/projects/gke_cluster_namespace/index.js +++ b/app/assets/javascripts/projects/gke_cluster_namespace/index.js @@ -28,8 +28,10 @@ const setState = glManagedCheckbox => { const initGkeNamespace = () => { const glManagedCheckbox = document.querySelector('.js-gl-managed'); - setState(glManagedCheckbox); // this is needed in order to set the initial state - glManagedCheckbox.addEventListener('change', () => setState(glManagedCheckbox)); + if (glManagedCheckbox) { + setState(glManagedCheckbox); // this is needed in order to set the initial state + glManagedCheckbox.addEventListener('change', () => setState(glManagedCheckbox)); + } }; export default initGkeNamespace; diff --git a/app/views/clusters/clusters/_banner.html.haml b/app/views/clusters/clusters/_banner.html.haml index a5de67be96b..4b4278075a6 100644 --- a/app/views/clusters/clusters/_banner.html.haml +++ b/app/views/clusters/clusters/_banner.html.haml @@ -3,7 +3,8 @@ %p.js-error-reason .hidden.js-cluster-creating.bs-callout.bs-callout-info{ role: 'alert' } - = s_('ClusterIntegration|Kubernetes cluster is being created on Google Kubernetes Engine...') + %span.spinner.spinner-dark.spinner-sm{ 'aria-label': 'Loading' } + %span.prepend-left-4= s_('ClusterIntegration|Kubernetes cluster is being created on Google Kubernetes Engine...') .hidden.row.js-cluster-api-unreachable.bs-callout.bs-callout-warning{ role: 'alert' } .col-11 @@ -18,4 +19,4 @@ %button.js-close-banner.close.cluster-application-banner-close.h-100.m-0= "×" .hidden.js-cluster-success.bs-callout.bs-callout-success{ role: 'alert' } - = s_("ClusterIntegration|Kubernetes cluster was successfully created on Google Kubernetes Engine. Refresh the page to see Kubernetes cluster's details") + = s_("ClusterIntegration|Kubernetes cluster was successfully created on Google Kubernetes Engine.") diff --git a/app/views/clusters/clusters/show.html.haml b/app/views/clusters/clusters/show.html.haml index 4dfbb310142..913d4caa0bc 100644 --- a/app/views/clusters/clusters/show.html.haml +++ b/app/views/clusters/clusters/show.html.haml @@ -33,26 +33,29 @@ %section#cluster-integration %h4= @cluster.name = render 'banner' - = render 'form' - = render_if_exists 'projects/clusters/prometheus_graphs' + - unless @cluster.status_name.in? %i/scheduled creating/ + = render 'form' - .cluster-applications-table#js-cluster-applications + - unless @cluster.status_name.in? %i/scheduled creating/ + = render_if_exists 'projects/clusters/prometheus_graphs' - %section.settings#js-cluster-details{ class: ('expanded' if expanded) } - .settings-header - %h4= s_('ClusterIntegration|Kubernetes cluster details') - %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? _('Collapse') : _('Expand') - %p= s_('ClusterIntegration|See and edit the details for your Kubernetes cluster') - .settings-content - = render 'clusters/platforms/kubernetes/form', cluster: @cluster, platform: @cluster.platform_kubernetes, update_cluster_url_path: clusterable.cluster_path(@cluster) + .cluster-applications-table#js-cluster-applications - %section.settings.no-animate#js-cluster-advanced-settings{ class: ('expanded' if expanded) } - .settings-header - %h4= _('Advanced settings') - %button.btn.js-settings-toggle{ type: 'button' } - = expanded ? _('Collapse') : _('Expand') - %p= s_("ClusterIntegration|Advanced options on this Kubernetes cluster's integration") - .settings-content#advanced-settings-section - = render 'advanced_settings' + %section.settings#js-cluster-details{ class: ('expanded' if expanded) } + .settings-header + %h4= s_('ClusterIntegration|Kubernetes cluster details') + %button.btn.js-settings-toggle{ type: 'button' } + = expanded ? _('Collapse') : _('Expand') + %p= s_('ClusterIntegration|See and edit the details for your Kubernetes cluster') + .settings-content + = render 'clusters/platforms/kubernetes/form', cluster: @cluster, platform: @cluster.platform_kubernetes, update_cluster_url_path: clusterable.cluster_path(@cluster) + + %section.settings.no-animate#js-cluster-advanced-settings{ class: ('expanded' if expanded) } + .settings-header + %h4= _('Advanced settings') + %button.btn.js-settings-toggle{ type: 'button' } + = expanded ? _('Collapse') : _('Expand') + %p= s_("ClusterIntegration|Advanced options on this Kubernetes cluster's integration") + .settings-content#advanced-settings-section + = render 'advanced_settings' diff --git a/changelogs/unreleased/50130-cluster-cluster-details-update-automatically-after-cluster-is-created.yml b/changelogs/unreleased/50130-cluster-cluster-details-update-automatically-after-cluster-is-created.yml new file mode 100644 index 00000000000..dc718572cfb --- /dev/null +++ b/changelogs/unreleased/50130-cluster-cluster-details-update-automatically-after-cluster-is-created.yml @@ -0,0 +1,5 @@ +--- +title: Update cluster page automatically when cluster is created +merge_request: 27189 +author: +type: changed diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 45a73004954..117625e717f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2647,7 +2647,7 @@ msgstr "" msgid "ClusterIntegration|Kubernetes cluster name" msgstr "" -msgid "ClusterIntegration|Kubernetes cluster was successfully created on Google Kubernetes Engine. Refresh the page to see Kubernetes cluster's details" +msgid "ClusterIntegration|Kubernetes cluster was successfully created on Google Kubernetes Engine." msgstr "" msgid "ClusterIntegration|Kubernetes clusters allow you to use review apps, deploy your applications, run your pipelines, and much more in an easy way." diff --git a/spec/features/projects/clusters/applications_spec.rb b/spec/features/projects/clusters/applications_spec.rb index 217bf1f5506..8cfd23d16df 100644 --- a/spec/features/projects/clusters/applications_spec.rb +++ b/spec/features/projects/clusters/applications_spec.rb @@ -22,9 +22,8 @@ describe 'Clusters Applications', :js do let(:cluster) { create(:cluster, :providing_by_gcp, projects: [project]) } it 'user is unable to install applications' do - page.within('.js-cluster-application-row-helm') do - expect(page).to have_css('.js-cluster-application-install-button[disabled]', exact_text: 'Install') - end + expect(page).not_to have_css('.js-cluster-application-row-helm') + expect(page).not_to have_css('.js-cluster-application-install-button') end end diff --git a/spec/frontend/clusters/clusters_bundle_spec.js b/spec/frontend/clusters/clusters_bundle_spec.js index 6de06a9e2d5..80816faa5fc 100644 --- a/spec/frontend/clusters/clusters_bundle_spec.js +++ b/spec/frontend/clusters/clusters_bundle_spec.js @@ -147,47 +147,80 @@ describe('Clusters', () => { }); describe('updateContainer', () => { + const { location } = window; + + beforeEach(() => { + delete window.location; + window.location = { + reload: jest.fn(), + hash: location.hash, + }; + }); + + afterEach(() => { + window.location = location; + }); + describe('when creating cluster', () => { it('should show the creating container', () => { cluster.updateContainer(null, 'creating'); expect(cluster.creatingContainer.classList.contains('hidden')).toBeFalsy(); - expect(cluster.successContainer.classList.contains('hidden')).toBeTruthy(); - expect(cluster.errorContainer.classList.contains('hidden')).toBeTruthy(); + expect(window.location.reload).not.toHaveBeenCalled(); }); it('should continue to show `creating` banner with subsequent updates of the same status', () => { + cluster.updateContainer(null, 'creating'); cluster.updateContainer('creating', 'creating'); expect(cluster.creatingContainer.classList.contains('hidden')).toBeFalsy(); - expect(cluster.successContainer.classList.contains('hidden')).toBeTruthy(); - expect(cluster.errorContainer.classList.contains('hidden')).toBeTruthy(); + expect(window.location.reload).not.toHaveBeenCalled(); }); }); describe('when cluster is created', () => { - it('should show the success container and fresh the page', () => { - cluster.updateContainer(null, 'created'); + it('should hide the "creating" banner and refresh the page', () => { + jest.spyOn(cluster, 'setClusterNewlyCreated'); + cluster.updateContainer(null, 'creating'); + cluster.updateContainer('creating', 'created'); expect(cluster.creatingContainer.classList.contains('hidden')).toBeTruthy(); - - expect(cluster.successContainer.classList.contains('hidden')).toBeFalsy(); - + expect(cluster.successContainer.classList.contains('hidden')).toBeTruthy(); expect(cluster.errorContainer.classList.contains('hidden')).toBeTruthy(); + expect(window.location.reload).toHaveBeenCalled(); + expect(cluster.setClusterNewlyCreated).toHaveBeenCalledWith(true); }); - it('should not show a banner when status is already `created`', () => { + it('when the page is refreshed, it should show the "success" banner', () => { + jest.spyOn(cluster, 'setClusterNewlyCreated'); + jest.spyOn(cluster, 'isClusterNewlyCreated').mockReturnValue(true); + + cluster.updateContainer(null, 'created'); cluster.updateContainer('created', 'created'); expect(cluster.creatingContainer.classList.contains('hidden')).toBeTruthy(); - - expect(cluster.successContainer.classList.contains('hidden')).toBeTruthy(); - + expect(cluster.successContainer.classList.contains('hidden')).toBeFalsy(); expect(cluster.errorContainer.classList.contains('hidden')).toBeTruthy(); + expect(window.location.reload).not.toHaveBeenCalled(); + expect(cluster.setClusterNewlyCreated).toHaveBeenCalledWith(false); + }); + + it('should not show a banner when status is already `created`', () => { + jest.spyOn(cluster, 'setClusterNewlyCreated'); + jest.spyOn(cluster, 'isClusterNewlyCreated').mockReturnValue(false); + + cluster.updateContainer(null, 'created'); + cluster.updateContainer('created', 'created'); + + expect(cluster.creatingContainer.classList.contains('hidden')).toBeTruthy(); + expect(cluster.successContainer.classList.contains('hidden')).toBeTruthy(); + expect(cluster.errorContainer.classList.contains('hidden')).toBeTruthy(); + expect(window.location.reload).not.toHaveBeenCalled(); + expect(cluster.setClusterNewlyCreated).not.toHaveBeenCalled(); }); });