From 1163b235391668d53ae0cea80bc22d40b365e0a7 Mon Sep 17 00:00:00 2001
From: Thong Kuah <tkuah@gitlab.com>
Date: Tue, 30 Oct 2018 23:33:43 +1300
Subject: Move view and path concerns to presenters

- Move show path for cluster to ClusterPresenter

- Create ClusterablePresenter to encapsulate logic. Consolidates
scattered methods from BaseController and ClustersHelper into an object.
---
 app/controllers/clusters/base_controller.rb        | 24 +----------
 app/controllers/clusters_controller.rb             | 16 ++++---
 app/helpers/clusters_helper.rb                     | 19 +-------
 app/presenters/clusterable_presenter.rb            | 30 +++++++++++++
 app/presenters/clusters/cluster_presenter.rb       |  8 ++++
 app/presenters/project_clusterable_presenter.rb    | 15 +++++++
 app/views/clusters/_advanced_settings.html.haml    |  2 +-
 app/views/clusters/_cluster.html.haml              |  4 +-
 app/views/clusters/_empty_state.html.haml          |  4 +-
 app/views/clusters/show.html.haml                  | 14 +++---
 spec/presenters/clusterable_presenter_spec.rb      | 17 ++++++++
 spec/presenters/clusters/cluster_presenter_spec.rb | 14 +++++-
 .../project_clusterable_presenter_spec.rb          | 50 ++++++++++++++++++++++
 13 files changed, 157 insertions(+), 60 deletions(-)
 create mode 100644 app/presenters/clusterable_presenter.rb
 create mode 100644 app/presenters/project_clusterable_presenter.rb
 create mode 100644 spec/presenters/clusterable_presenter_spec.rb
 create mode 100644 spec/presenters/project_clusterable_presenter_spec.rb

diff --git a/app/controllers/clusters/base_controller.rb b/app/controllers/clusters/base_controller.rb
index 2e9997dfc08..8908b26b914 100644
--- a/app/controllers/clusters/base_controller.rb
+++ b/app/controllers/clusters/base_controller.rb
@@ -11,7 +11,7 @@ class Clusters::BaseController < ApplicationController
 
   layout :determine_layout
 
-  helper_method :clusters_page_path, :cluster_page_path, :new_cluster_page_path
+  helper_method :clusterable
 
   private
 
@@ -43,27 +43,7 @@ class Clusters::BaseController < ApplicationController
   end
 
   def clusterable
-    if project_type?
-      project
-    end
-  end
-
-  def cluster_page_path(cluster)
-    if project_type?
-      project_cluster_path(project, cluster)
-    end
-  end
-
-  def clusters_page_path
-    if project_type?
-      project_clusters_path(project)
-    end
-  end
-
-  def new_cluster_page_path
-    if project_type?
-      new_project_cluster_path(project)
-    end
+    @clusterable ||= ClusterablePresenter.fabricate(project, current_user: current_user)
   end
 
   def project_type?
diff --git a/app/controllers/clusters_controller.rb b/app/controllers/clusters_controller.rb
index 5290ab1e624..7aad70870ba 100644
--- a/app/controllers/clusters_controller.rb
+++ b/app/controllers/clusters_controller.rb
@@ -50,7 +50,7 @@ class ClustersController < Clusters::BaseController
         end
         format.html do
           flash[:notice] = _('Kubernetes cluster was successfully updated.')
-          redirect_to cluster_page_path(cluster)
+          redirect_to cluster.show_path
         end
       end
     else
@@ -64,7 +64,7 @@ class ClustersController < Clusters::BaseController
   def destroy
     if cluster.destroy
       flash[:notice] = _('Kubernetes cluster integration was successfully removed.')
-      redirect_to clusters_page_path, status: :found
+      redirect_to clusterable.index_path, status: :found
     else
       flash[:notice] = _('Kubernetes cluster integration was not removed.')
       render :show
@@ -75,9 +75,10 @@ class ClustersController < Clusters::BaseController
     @gcp_cluster = ::Clusters::CreateService
       .new(current_user, create_gcp_cluster_params)
       .execute(access_token: token_in_session)
+      .present(current_user: current_user)
 
     if @gcp_cluster.persisted?
-      redirect_to cluster_page_path(@gcp_cluster)
+      redirect_to @gcp_cluster.show_path
     else
       generate_gcp_authorize_url
       validate_gcp_token
@@ -91,9 +92,10 @@ class ClustersController < Clusters::BaseController
     @user_cluster = ::Clusters::CreateService
       .new(current_user, create_user_cluster_params)
       .execute(access_token: token_in_session)
+      .present(current_user: current_user)
 
     if @user_cluster.persisted?
-      redirect_to cluster_page_path(@user_cluster)
+      redirect_to @user_cluster.show_path
     else
       generate_gcp_authorize_url
       validate_gcp_token
@@ -148,7 +150,7 @@ class ClustersController < Clusters::BaseController
       ]).merge(
         provider_type: :gcp,
         platform_type: :kubernetes,
-        clusterable: clusterable
+        clusterable: clusterable.subject
       )
   end
 
@@ -166,12 +168,12 @@ class ClustersController < Clusters::BaseController
       ]).merge(
         provider_type: :user,
         platform_type: :kubernetes,
-        clusterable: clusterable
+        clusterable: clusterable.subject
       )
   end
 
   def generate_gcp_authorize_url
-    state = generate_session_key_redirect(new_cluster_page_path.to_s)
+    state = generate_session_key_redirect(clusterable.new_path.to_s)
 
     @authorize_url = GoogleApi::CloudPlatform::Client.new(
       nil, callback_google_api_auth_url,
diff --git a/app/helpers/clusters_helper.rb b/app/helpers/clusters_helper.rb
index 4b4945adc4b..360885fe179 100644
--- a/app/helpers/clusters_helper.rb
+++ b/app/helpers/clusters_helper.rb
@@ -6,14 +6,6 @@ module ClustersHelper
     false
   end
 
-  def clusterable
-    @project
-  end
-
-  def can_create_cluster?
-    can?(current_user, :create_cluster, clusterable)
-  end
-
   def render_gcp_signup_offer
     return if Gitlab::CurrentSettings.current_application_settings.hide_third_party_offers?
     return unless show_gcp_signup_offer?
@@ -24,17 +16,8 @@ module ClustersHelper
   end
 
   def hidden_clusterable_fields
-    clusterable_params.map do |key, value|
+    clusterable.clusterable_params.map do |key, value|
       hidden_field_tag(key, value)
     end.reduce(&:safe_concat)
   end
-
-  def clusterable_params
-    case clusterable
-    when Project
-      { project_id: clusterable.to_param, namespace_id: clusterable.namespace.to_param }
-    else
-      {}
-    end
-  end
 end
diff --git a/app/presenters/clusterable_presenter.rb b/app/presenters/clusterable_presenter.rb
new file mode 100644
index 00000000000..c857d57b003
--- /dev/null
+++ b/app/presenters/clusterable_presenter.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+class ClusterablePresenter < Gitlab::View::Presenter::Delegated
+  presents :clusterable
+
+  def self.fabricate(clusterable, **attributes)
+    presenter_class = "#{clusterable.class.name}ClusterablePresenter".constantize
+    attributes_with_presenter_class = attributes.merge(presenter_class: presenter_class)
+
+    Gitlab::View::Presenter::Factory
+      .new(clusterable, attributes_with_presenter_class)
+      .fabricate!
+  end
+
+  def can_create_cluster?
+    can?(current_user, :create_cluster, clusterable)
+  end
+
+  def index_path
+    raise NotImplementedError
+  end
+
+  def new_path
+    raise NotImplementedError
+  end
+
+  def clusterable_params
+    raise NotImplementedError
+  end
+end
diff --git a/app/presenters/clusters/cluster_presenter.rb b/app/presenters/clusters/cluster_presenter.rb
index dfdd8e82f97..78d632eb77c 100644
--- a/app/presenters/clusters/cluster_presenter.rb
+++ b/app/presenters/clusters/cluster_presenter.rb
@@ -11,5 +11,13 @@ module Clusters
     def can_toggle_cluster?
       can?(current_user, :update_cluster, cluster) && created?
     end
+
+    def show_path
+      if cluster.project_type?
+        project_cluster_path(project, cluster)
+      else
+        raise NotImplementedError
+      end
+    end
   end
 end
diff --git a/app/presenters/project_clusterable_presenter.rb b/app/presenters/project_clusterable_presenter.rb
new file mode 100644
index 00000000000..f986b5584a3
--- /dev/null
+++ b/app/presenters/project_clusterable_presenter.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class ProjectClusterablePresenter < ClusterablePresenter
+  def index_path
+    project_clusters_path(clusterable)
+  end
+
+  def new_path
+    new_project_cluster_path(clusterable)
+  end
+
+  def clusterable_params
+    { project_id: clusterable.to_param, namespace_id: clusterable.namespace.to_param }
+  end
+end
diff --git a/app/views/clusters/_advanced_settings.html.haml b/app/views/clusters/_advanced_settings.html.haml
index c91202d9359..e25076248d2 100644
--- a/app/views/clusters/_advanced_settings.html.haml
+++ b/app/views/clusters/_advanced_settings.html.haml
@@ -12,4 +12,4 @@
       = s_('ClusterIntegration|Remove Kubernetes cluster integration')
     %p
       = s_("ClusterIntegration|Remove this Kubernetes cluster's configuration from this project. This will not delete your actual Kubernetes cluster.")
-    = link_to(s_('ClusterIntegration|Remove integration'), cluster_path(@cluster, clusterable_params), method: :delete, class: 'btn btn-danger', data: { confirm: s_("ClusterIntegration|Are you sure you want to remove this Kubernetes cluster's integration? This will not delete your actual Kubernetes cluster.")})
+    = link_to(s_('ClusterIntegration|Remove integration'), cluster_path(@cluster, clusterable.clusterable_params), method: :delete, class: 'btn btn-danger', data: { confirm: s_("ClusterIntegration|Are you sure you want to remove this Kubernetes cluster's integration? This will not delete your actual Kubernetes cluster.")})
diff --git a/app/views/clusters/_cluster.html.haml b/app/views/clusters/_cluster.html.haml
index a3dcbc25399..709d6711f33 100644
--- a/app/views/clusters/_cluster.html.haml
+++ b/app/views/clusters/_cluster.html.haml
@@ -2,7 +2,7 @@
   .table-section.section-30
     .table-mobile-header{ role: "rowheader" }= s_("ClusterIntegration|Kubernetes cluster")
     .table-mobile-content
-      = link_to cluster.name, cluster_page_path(cluster)
+      = link_to cluster.name, cluster.show_path
   .table-section.section-30
     .table-mobile-header{ role: "rowheader" }= s_("ClusterIntegration|Environment scope")
     .table-mobile-content= cluster.environment_scope
@@ -16,7 +16,7 @@
         class: "#{'is-checked' if cluster.enabled?} #{'is-disabled' if !cluster.can_toggle_cluster?}",
         "aria-label": s_("ClusterIntegration|Toggle Kubernetes Cluster"),
         disabled: !cluster.can_toggle_cluster?,
-        data: { endpoint: cluster_path(cluster, clusterable_params.merge(format: :json)) } }
+        data: { endpoint: cluster_path(cluster, clusterable.clusterable_params.merge(format: :json)) } }
         %input.js-project-feature-toggle-input{ type: "hidden", value: cluster.enabled? }
         = icon("spinner spin", class: "loading-icon")
         %span.toggle-icon
diff --git a/app/views/clusters/_empty_state.html.haml b/app/views/clusters/_empty_state.html.haml
index 200c19e45b1..800e76d92ef 100644
--- a/app/views/clusters/_empty_state.html.haml
+++ b/app/views/clusters/_empty_state.html.haml
@@ -7,6 +7,6 @@
       - link_to_help_page = link_to(_('Learn more about Kubernetes'), help_page_path('user/project/clusters/index'), target: '_blank', rel: 'noopener noreferrer')
       %p= s_('ClusterIntegration|Kubernetes clusters allow you to use review apps, deploy your applications, run your pipelines, and much more in an easy way. %{link_to_help_page}').html_safe % { link_to_help_page: link_to_help_page}
 
-      - if can_create_cluster?
+      - if clusterable.can_create_cluster?
         .text-center
-          = link_to s_('ClusterIntegration|Add Kubernetes cluster'), new_cluster_page_path, class: 'btn btn-success'
+          = link_to s_('ClusterIntegration|Add Kubernetes cluster'), clusterable.new_path, class: 'btn btn-success'
diff --git a/app/views/clusters/show.html.haml b/app/views/clusters/show.html.haml
index dd48ffb8ef7..c0169fbf5a2 100644
--- a/app/views/clusters/show.html.haml
+++ b/app/views/clusters/show.html.haml
@@ -1,18 +1,18 @@
 - @content_class = "limit-container-width" unless fluid_layout
-- add_to_breadcrumbs "Kubernetes Clusters", clusters_page_path
+- add_to_breadcrumbs "Kubernetes Clusters", clusterable.index_path
 - breadcrumb_title @cluster.name
 - page_title _("Kubernetes Cluster")
 - manage_prometheus_path = edit_project_service_path(@cluster.project, 'prometheus') if @project
 
 - expanded = Rails.env.test?
 
-- status_path = status_cluster_path(@cluster.id, clusterable_params.merge(format: :json)) if can?(current_user, :admin_cluster, @cluster)
+- status_path = status_cluster_path(@cluster.id, clusterable.clusterable_params.merge(format: :json)) if can?(current_user, :admin_cluster, @cluster)
 .edit-cluster-form.js-edit-cluster-form{ data: { status_path: status_path,
-  install_helm_path: install_applications_cluster_path(@cluster, :helm, clusterable_params),
-  install_ingress_path: install_applications_cluster_path(@cluster, :ingress, clusterable_params),
-  install_prometheus_path: install_applications_cluster_path(@cluster, :prometheus, clusterable_params),
-  install_runner_path: install_applications_cluster_path(@cluster, :runner, clusterable_params),
-  install_jupyter_path: install_applications_cluster_path(@cluster, :jupyter, clusterable_params),
+  install_helm_path: install_applications_cluster_path(@cluster, :helm, clusterable.clusterable_params),
+  install_ingress_path: install_applications_cluster_path(@cluster, :ingress, clusterable.clusterable_params),
+  install_prometheus_path: install_applications_cluster_path(@cluster, :prometheus, clusterable.clusterable_params),
+  install_runner_path: install_applications_cluster_path(@cluster, :runner, clusterable.clusterable_params),
+  install_jupyter_path: install_applications_cluster_path(@cluster, :jupyter, clusterable.clusterable_params),
   toggle_status: @cluster.enabled? ? 'true': 'false',
   cluster_status: @cluster.status_name,
   cluster_status_reason: @cluster.status_reason,
diff --git a/spec/presenters/clusterable_presenter_spec.rb b/spec/presenters/clusterable_presenter_spec.rb
new file mode 100644
index 00000000000..4f4ae5e07c5
--- /dev/null
+++ b/spec/presenters/clusterable_presenter_spec.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ClusterablePresenter do
+  include Gitlab::Routing.url_helpers
+
+  describe '.fabricate' do
+    let(:project) { create(:project) }
+
+    subject { described_class.fabricate(project) }
+
+    it 'creates an object from a descendant presenter' do
+      expect(subject).to be_kind_of(ProjectClusterablePresenter)
+    end
+  end
+end
diff --git a/spec/presenters/clusters/cluster_presenter_spec.rb b/spec/presenters/clusters/cluster_presenter_spec.rb
index e96dbfb73c0..7af181f37d5 100644
--- a/spec/presenters/clusters/cluster_presenter_spec.rb
+++ b/spec/presenters/clusters/cluster_presenter_spec.rb
@@ -1,7 +1,9 @@
 require 'spec_helper'
 
 describe Clusters::ClusterPresenter do
-  let(:cluster) { create(:cluster, :provided_by_gcp) }
+  include Gitlab::Routing.url_helpers
+
+  let(:cluster) { create(:cluster, :provided_by_gcp, :project) }
 
   subject(:presenter) do
     described_class.new(cluster)
@@ -71,4 +73,14 @@ describe Clusters::ClusterPresenter do
       it { is_expected.to eq(false) }
     end
   end
+
+  describe '#show_path' do
+    subject { described_class.new(cluster).show_path }
+
+    context 'project_type cluster' do
+      let(:project) { cluster.project }
+
+      it { is_expected.to eq(project_cluster_path(project, cluster)) }
+    end
+  end
 end
diff --git a/spec/presenters/project_clusterable_presenter_spec.rb b/spec/presenters/project_clusterable_presenter_spec.rb
new file mode 100644
index 00000000000..c9045f0175a
--- /dev/null
+++ b/spec/presenters/project_clusterable_presenter_spec.rb
@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ProjectClusterablePresenter do
+  include Gitlab::Routing.url_helpers
+
+  let(:presenter) { described_class.new(project) }
+  let(:project) { create(:project) }
+
+  describe '#can_create_cluster?' do
+    let(:user) { create(:user) }
+
+    subject { presenter.can_create_cluster? }
+
+    before do
+      allow(presenter).to receive(:current_user).and_return(user)
+    end
+
+    context 'when user can create' do
+      before do
+        project.add_maintainer(user)
+      end
+
+      it { is_expected.to be_truthy }
+    end
+
+    context 'when user cannot create' do
+      it { is_expected.to be_falsey }
+    end
+  end
+
+  describe '#index_path' do
+    subject { presenter.index_path }
+
+    it { is_expected.to eq(project_clusters_path(project)) }
+  end
+
+  describe '#new_path' do
+    subject { presenter.new_path }
+
+    it { is_expected.to eq(new_project_cluster_path(project)) }
+  end
+
+  describe '#clusterable_params' do
+    subject { presenter.clusterable_params }
+
+    it { is_expected.to eq({ project_id: project.to_param, namespace_id: project.namespace.to_param }) }
+  end
+end
-- 
cgit v1.2.1