From f82c9dbe44d5d003dbeb084f07615ba26c2294b6 Mon Sep 17 00:00:00 2001
From: Thong Kuah <tkuah@gitlab.com>
Date: Tue, 11 Dec 2018 18:13:29 +1300
Subject: [PATCH] Use finder to decide to show note to user
Given the note is about how to interpret ancestor clusters, use the
finder which actually knows if there are any ancestor clusters to find
out if the note should be shown, rather than passing the same info via a
view to a helper
Added note about Kaminari.paginate_array
Link to followup issue too
---
app/controllers/clusters/clusters_controller.rb | 14 +++++++++++++-
app/finders/cluster_ancestors_finder.rb | 11 ++++++++++-
app/helpers/clusters_helper.rb | 4 ----
app/views/clusters/clusters/index.html.haml | 2 +-
4 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/app/controllers/clusters/clusters_controller.rb b/app/controllers/clusters/clusters_controller.rb
index 7a4ed840e3ae1..b9717b9764012 100644
--- a/app/controllers/clusters/clusters_controller.rb
+++ b/app/controllers/clusters/clusters_controller.rb
@@ -18,8 +18,20 @@ class Clusters::ClustersController < Clusters::BaseController
STATUS_POLLING_INTERVAL = 10_000
def index
- clusters = ClusterAncestorsFinder.new(clusterable.subject, current_user).execute
+ finder = ClusterAncestorsFinder.new(clusterable.subject, current_user)
+ clusters = finder.execute
+
+ # Note: We are paginating through an array here but this should OK as:
+ #
+ # In CE, we can have a maximum group nesting depth of 21, so including
+ # project cluster, we can have max 22 clusters for a group hierachy.
+ # In EE (Premium) we can have any number, as multiple clusters are
+ # supported, but the number of clusters are fairly low currently.
+ #
+ # See https://gitlab.com/gitlab-org/gitlab-ce/issues/55260 also.
@clusters = Kaminari.paginate_array(clusters).page(params[:page]).per(20)
+
+ @has_ancestor_clusters = finder.has_ancestor_clusters?
end
def new
diff --git a/app/finders/cluster_ancestors_finder.rb b/app/finders/cluster_ancestors_finder.rb
index 586fceb258a06..2f9709ee0578b 100644
--- a/app/finders/cluster_ancestors_finder.rb
+++ b/app/finders/cluster_ancestors_finder.rb
@@ -1,6 +1,8 @@
# frozen_string_literal: true
class ClusterAncestorsFinder
+ include Gitlab::Utils::StrongMemoize
+
def initialize(clusterable, current_user)
@clusterable = clusterable
@current_user = current_user
@@ -12,6 +14,10 @@ def execute
clusterable.clusters + ancestor_clusters
end
+ def has_ancestor_clusters?
+ ancestor_clusters.any?
+ end
+
private
attr_reader :clusterable, :current_user
@@ -20,7 +26,10 @@ def can_read_clusters?
Ability.allowed?(current_user, :read_cluster, clusterable)
end
+ # This unfortunately returns an Array, not a Relation!
def ancestor_clusters
- Clusters::Cluster.ancestor_clusters_for_clusterable(clusterable)
+ strong_memoize(:ancestor_clusters) do
+ Clusters::Cluster.ancestor_clusters_for_clusterable(clusterable)
+ end
end
end
diff --git a/app/helpers/clusters_helper.rb b/app/helpers/clusters_helper.rb
index fac606ee8db0a..916dcb1a3083c 100644
--- a/app/helpers/clusters_helper.rb
+++ b/app/helpers/clusters_helper.rb
@@ -14,8 +14,4 @@ def render_gcp_signup_offer
render 'clusters/clusters/gcp_signup_offer_banner'
end
end
-
- def render_cluster_help_content?(clusters, clusterable)
- clusters.length > clusterable.clusters.length
- end
end
diff --git a/app/views/clusters/clusters/index.html.haml b/app/views/clusters/clusters/index.html.haml
index 932395eb50e3b..58d0a30436392 100644
--- a/app/views/clusters/clusters/index.html.haml
+++ b/app/views/clusters/clusters/index.html.haml
@@ -12,7 +12,7 @@
= s_("ClusterIntegration|Kubernetes clusters can be used to deploy applications and to provide Review Apps for this project")
= render 'clusters/clusters/buttons'
- - if render_cluster_help_content?(@clusters, clusterable)
+ - if @has_ancestor_clusters
.bs-callout.bs-callout-info
= s_("ClusterIntegration|Clusters are utilized by selecting the nearest ancestor with a matching environment scope. For example, project clusters will override group clusters.")
%strong
--
GitLab