From bea4303840308e10e073134cab294181c744c770 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 1 Dec 2017 12:56:31 +0100 Subject: [PATCH 1/5] Fix clusters active/inactive toggling --- app/assets/javascripts/clusters/clusters_index.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/clusters/clusters_index.js b/app/assets/javascripts/clusters/clusters_index.js index 3fd188a8770..82ac8d93035 100644 --- a/app/assets/javascripts/clusters/clusters_index.js +++ b/app/assets/javascripts/clusters/clusters_index.js @@ -28,7 +28,7 @@ const toggleValue = (button) => { * * When the user clicks the toggle button for each cluster, it: * - toggles the button - * - shows a loding and disabled state + * - shows a loading and disabled state * - Makes a put request to the given endpoint * Once we receive the response, either: * 1) Show updated status in case of successfull response @@ -38,12 +38,13 @@ export default function setClusterTableToggles() { document.querySelectorAll('.js-toggle-cluster-list') .forEach(button => button.addEventListener('click', (e) => { const toggleButton = e.currentTarget; - const value = toggleButton.classList.contains('checked').toString(); const endpoint = toggleButton.getAttribute('data-endpoint'); toggleValue(toggleButton); toggleLoadingButton(toggleButton); + const value = toggleButton.classList.contains('is-checked'); + ClustersService.updateCluster(endpoint, { cluster: { enabled: value } }) .then(() => { toggleLoadingButton(toggleButton); From d7ebb823bf788bbfc05c368ab9c134a7fc84650d Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sun, 3 Dec 2017 23:01:18 +0100 Subject: [PATCH 2/5] Use actual namespace --- app/views/projects/clusters/_cluster.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/projects/clusters/_cluster.html.haml b/app/views/projects/clusters/_cluster.html.haml index ba36828d407..d292d68386c 100644 --- a/app/views/projects/clusters/_cluster.html.haml +++ b/app/views/projects/clusters/_cluster.html.haml @@ -8,7 +8,7 @@ .table-mobile-content= cluster.environment_scope .table-section.section-30 .table-mobile-header{ role: "rowheader" }= s_("ClusterIntegration|Project namespace") - .table-mobile-content= cluster.platform_kubernetes&.namespace + .table-mobile-content= cluster.platform_kubernetes&.actual_namespace .table-section.section-10 .table-mobile-header{ role: "rowheader" } .table-mobile-content From b6d899667c395bf67c8def537e96b1a81e71a7eb Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Mon, 4 Dec 2017 12:56:04 +0000 Subject: [PATCH 3/5] Fix navbar CSS --- app/assets/javascripts/clusters/clusters_index.js | 2 +- app/assets/stylesheets/pages/clusters.scss | 4 ++++ app/views/projects/clusters/_tabs.html.haml | 4 ++-- spec/features/projects/clusters_spec.rb | 13 +++++++------ 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/app/assets/javascripts/clusters/clusters_index.js b/app/assets/javascripts/clusters/clusters_index.js index 82ac8d93035..6844d1dbd83 100644 --- a/app/assets/javascripts/clusters/clusters_index.js +++ b/app/assets/javascripts/clusters/clusters_index.js @@ -28,7 +28,7 @@ const toggleValue = (button) => { * * When the user clicks the toggle button for each cluster, it: * - toggles the button - * - shows a loading and disabled state + * - shows a loading and disables button * - Makes a put request to the given endpoint * Once we receive the response, either: * 1) Show updated status in case of successfull response diff --git a/app/assets/stylesheets/pages/clusters.scss b/app/assets/stylesheets/pages/clusters.scss index 1d9a5150065..e577df535c2 100644 --- a/app/assets/stylesheets/pages/clusters.scss +++ b/app/assets/stylesheets/pages/clusters.scss @@ -17,4 +17,8 @@ .empty-state .svg-content img { width: 145px; } + + .top-area .nav-controls > .btn.btn-add-cluster { + margin-right: 0; + } } diff --git a/app/views/projects/clusters/_tabs.html.haml b/app/views/projects/clusters/_tabs.html.haml index 955a9940727..920ed40ea69 100644 --- a/app/views/projects/clusters/_tabs.html.haml +++ b/app/views/projects/clusters/_tabs.html.haml @@ -14,5 +14,5 @@ = link_to project_clusters_path(@project), class: "js-all-tab" do = s_("ClusterIntegration|All") %span.badge= @all_count - .pull-right.nav-bar-right - = link_to s_("ClusterIntegration|Add cluster"), new_project_cluster_path(@project), class: "btn btn-success disabled has-tooltip js-add-cluster", title: s_("ClusterIntegration|Multiple clusters are available in GitLab Entreprise Edition Premium and Ultimate") + .nav-controls + = link_to s_("ClusterIntegration|Add cluster"), new_project_cluster_path(@project), class: "btn btn-success btn-add-cluster disabled has-tooltip js-add-cluster", title: s_("ClusterIntegration|Multiple clusters are available in GitLab Entreprise Edition Premium and Ultimate") diff --git a/spec/features/projects/clusters_spec.rb b/spec/features/projects/clusters_spec.rb index b85bd0f7123..5a96bd58d5b 100644 --- a/spec/features/projects/clusters_spec.rb +++ b/spec/features/projects/clusters_spec.rb @@ -122,12 +122,13 @@ feature 'Clusters', :js do context 'with sucessfull request' do it 'user sees updated cluster' do - expect do - page.find('.js-toggle-cluster-list').click - wait_for_requests - end.to change { cluster.enabled } + expect(page).to have_selector('.js-toggle-cluster-list.is-checked') + # Cluster is enabled + page.find('.js-toggle-cluster-list').click - expect(page).not_to have_selector('.is-checked') + wait_for_requests + # Cluster must be disabled + expect(page).not_to have_selector('.js-toggle-cluster-list.is-checked') end end @@ -137,7 +138,7 @@ feature 'Clusters', :js do page.find('.js-toggle-cluster-list').click Clusters::Cluster.last.provider.make_errored!('Something wrong!') - + # Cluster must still be disabled expect(page).not_to have_selector('.js-toggle-cluster-list.is-checked') end end From a42b43ce7a9146ac1035328f2bc1cfc5f06e9371 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Mon, 4 Dec 2017 16:51:06 +0100 Subject: [PATCH 4/5] Remove cluster from Project model --- app/models/project.rb | 1 - spec/models/project_spec.rb | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index eaf4f555d3b..0cb07e14cd8 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -189,7 +189,6 @@ class Project < ActiveRecord::Base has_one :statistics, class_name: 'ProjectStatistics' has_one :cluster_project, class_name: 'Clusters::Project' - has_one :cluster, through: :cluster_project, class_name: 'Clusters::Cluster' has_many :clusters, through: :cluster_project, class_name: 'Clusters::Cluster' # Container repositories need to remove data from the container registry, diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 521b7bd70ba..57f0034c1d4 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -78,7 +78,7 @@ describe Project do it { is_expected.to have_many(:uploads).dependent(:destroy) } it { is_expected.to have_many(:pipeline_schedules) } it { is_expected.to have_many(:members_and_requesters) } - it { is_expected.to have_one(:cluster) } + it { is_expected.to have_many(:clusters) } it { is_expected.to have_many(:custom_attributes).class_name('ProjectCustomAttribute') } context 'after initialized' do From f1357a1e15bead6b58c049357c765dd991c04d9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Mon, 4 Dec 2017 17:02:18 +0100 Subject: [PATCH 5/5] Remove references of project.cluster from specs --- app/controllers/projects/clusters_controller.rb | 2 +- spec/controllers/projects/clusters_controller_spec.rb | 4 ++-- ...migrate_gcp_clusters_to_new_clusters_architectures_spec.rb | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/controllers/projects/clusters_controller.rb b/app/controllers/projects/clusters_controller.rb index 2843045fdc4..789a6254e2d 100644 --- a/app/controllers/projects/clusters_controller.rb +++ b/app/controllers/projects/clusters_controller.rb @@ -75,7 +75,7 @@ class Projects::ClustersController < Projects::ApplicationController end format.html do flash[:notice] = "Cluster was successfully updated." - redirect_to project_cluster_path(project, project.cluster) + redirect_to project_cluster_path(project, cluster) end end else diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb index 6ff52aff35a..1e546a66886 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -255,7 +255,7 @@ describe Projects::ClustersController do it 'creates a new cluster' do expect(ClusterProvisionWorker).to receive(:perform_async) expect { go }.to change { Clusters::Cluster.count } - expect(response).to redirect_to(project_cluster_path(project, project.cluster)) + expect(response).to redirect_to(project_cluster_path(project, project.clusters.last)) end end end @@ -422,7 +422,7 @@ describe Projects::ClustersController do go cluster.reload - expect(response).to redirect_to(project_cluster_path(project, project.cluster)) + expect(response).to redirect_to(project_cluster_path(project, cluster)) expect(flash[:notice]).to eq('Cluster was successfully updated.') expect(cluster.enabled).to be_falsey end diff --git a/spec/migrations/migrate_gcp_clusters_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_gcp_clusters_to_new_clusters_architectures_spec.rb index 9f41534441b..05f281fffff 100644 --- a/spec/migrations/migrate_gcp_clusters_to_new_clusters_architectures_spec.rb +++ b/spec/migrations/migrate_gcp_clusters_to_new_clusters_architectures_spec.rb @@ -57,7 +57,7 @@ describe MigrateGcpClustersToNewClustersArchitectures, :migration do expect(cluster.platform_type).to eq('kubernetes') expect(cluster.project).to eq(project) - expect(project.cluster).to eq(cluster) + expect(project.clusters).to include(cluster) expect(cluster.provider_gcp.cluster).to eq(cluster) expect(cluster.provider_gcp.status).to eq(status) @@ -134,7 +134,7 @@ describe MigrateGcpClustersToNewClustersArchitectures, :migration do expect(cluster.platform_type).to eq('kubernetes') expect(cluster.project).to eq(project) - expect(project.cluster).to eq(cluster) + expect(project.clusters).to include(cluster) expect(cluster.provider_gcp.cluster).to eq(cluster) expect(cluster.provider_gcp.status).to eq(status)