Resolve controller sharing concern

Use ClustersController as base while having Projects::ClustersController
to inform what `clusterable` is. Thanks @ayufan for the great suggestion
!

- View changes to work with new approach

- Fix javascript for new approach

- Fix feature specs for new approach

- Fix QA

app/assets/javascripts/pages/clusters/create_gcp/index.js

-import initDismissableCallout from '~/dismissable_callout';
-import initGkeDropdowns from '~/projects/gke_cluster_dropdowns';
-
-document.addEventListener('DOMContentLoaded', () => {
-  initDismissableCallout('.gcp-signup-offer');
-  initGkeDropdowns();
-});

app/assets/javascripts/pages/clusters/create_user/index.js

-import initDismissableCallout from '~/dismissable_callout';
-import initGkeDropdowns from '~/projects/gke_cluster_dropdowns';
-
-document.addEventListener('DOMContentLoaded', () => {
-  initDismissableCallout('.gcp-signup-offer');
-  initGkeDropdowns();
-});

app/assets/javascripts/pages/clusters/new/index.js

-import initDismissableCallout from '~/dismissable_callout';
-import initGkeDropdowns from '~/projects/gke_cluster_dropdowns';
-
-document.addEventListener('DOMContentLoaded', () => {
-  initDismissableCallout('.gcp-signup-offer');
-  initGkeDropdowns();
-});

app/assets/javascripts/pages/projects/index.js

+import initDismissableCallout from '~/dismissable_callout';
+import initGkeDropdowns from '~/projects/gke_cluster_dropdowns';
 import Project from './project';
 import ShortcutsNavigation from '../../behaviors/shortcuts/shortcuts_navigation';
 
 document.addEventListener('DOMContentLoaded', () => {
+  const { page } = document.body.dataset;
+  const newClusterViews = [
+    'projects:clusters:new',
+    'projects:clusters:create_gcp',
+    'projects:clusters:create_user',
+  ];
+
+  if (newClusterViews.indexOf(page) > -1) {
+    initDismissableCallout('.gcp-signup-offer');
+    initGkeDropdowns();
+  }
+
   new Project(); // eslint-disable-line no-new
   new ShortcutsNavigation(); // eslint-disable-line no-new
 });

app/controllers/clusters/base_controller.rb

@@ -2,31 +2,25 @@
 
 class Clusters::BaseController < ApplicationController
   include RoutableActions
-  include ProjectUnauthorized
 
   skip_before_action :authenticate_user!
-  before_action :require_project_id
-  before_action :project, if: :project_type?
-  before_action :repository, if: :project_type?
   before_action :authorize_read_cluster!
 
-  layout :determine_layout
-
   helper_method :clusterable
 
   private
 
-  # We can extend to `#group_type?` in the future
-  def require_project_id
-    not_found unless project_type?
+  def cluster
+    @cluster ||= clusterable.clusters.find(params[:id])
+                                 .present(current_user: current_user)
   end
 
-  def project
-    @project ||= find_routable!(Project, File.join(params[:namespace_id], params[:project_id]), not_found_or_authorized_proc: project_unauthorized_proc)
+  def authorize_update_cluster!
+    access_denied! unless can?(current_user, :update_cluster, cluster)
   end
 
-  def repository
-    @repository ||= project.repository
+  def authorize_admin_cluster!
+    access_denied! unless can?(current_user, :admin_cluster, cluster)
   end
 
   def authorize_read_cluster!
@@ -37,17 +31,7 @@ class Clusters::BaseController < ApplicationController
     access_denied! unless can?(current_user, :create_cluster, clusterable)
   end
 
-  def determine_layout
-    if project_type?
-      'project'
-    end
-  end
-
   def clusterable
-    @clusterable ||= ClusterablePresenter.fabricate(project, current_user: current_user)
-  end
-
-  def project_type?
-    params[:project_id].present?
+    raise NotImplementedError
   end
 end

app/controllers/clusters_controller.rb → app/controllers/clusters/clusters_controller.rb

 # frozen_string_literal: true
 
-class ClustersController < Clusters::BaseController
+class Clusters::ClustersController < Clusters::BaseController
+  include RoutableActions
+
   before_action :cluster, except: [:index, :new, :create_gcp, :create_user]
   before_action :generate_gcp_authorize_url, only: [:new]
   before_action :validate_gcp_token, only: [:new]
@@ -9,7 +11,7 @@ class ClustersController < Clusters::BaseController
   before_action :authorize_create_cluster!, only: [:new]
   before_action :authorize_update_cluster!, only: [:update]
   before_action :authorize_admin_cluster!, only: [:destroy]
-  before_action :update_applications_status, only: [:status]
+  before_action :update_applications_status, only: [:cluster_status]
 
   helper_method :token_in_session
 
@@ -23,7 +25,8 @@ class ClustersController < Clusters::BaseController
   def new
   end
 
-  def status
+  # Overridding ActionController::Metal#status is NOT a good idea
+  def cluster_status
     respond_to do |format|
       format.json do
         Gitlab::PollingInterval.set_header(response, interval: STATUS_POLLING_INTERVAL)
@@ -107,11 +110,6 @@ class ClustersController < Clusters::BaseController
 
   private
 
-  def cluster
-    @cluster ||= clusterable.clusters.find(params[:id])
-                                 .present(current_user: current_user)
-  end
-
   def update_params
     if cluster.managed?
       params.require(:cluster).permit(
@@ -214,14 +212,6 @@ class ClustersController < Clusters::BaseController
     end
   end
 
-  def authorize_update_cluster!
-    access_denied! unless can?(current_user, :update_cluster, cluster)
-  end
-
-  def authorize_admin_cluster!
-    access_denied! unless can?(current_user, :admin_cluster, cluster)
-  end
-
   def update_applications_status
     @cluster.applications.each(&:schedule_status_update)
   end

app/controllers/projects/clusters/applications_controller.rb

+# frozen_string_literal: true
+
+class Projects::Clusters::ApplicationsController < Clusters::ApplicationsController
+  include ProjectUnauthorized
+
+  prepend_before_action :project
+
+  private
+
+  def clusterable
+    @clusterable ||= ClusterablePresenter.fabricate(project, current_user: current_user)
+  end
+
+  def project
+    @project ||= find_routable!(Project, File.join(params[:namespace_id], params[:project_id]), not_found_or_authorized_proc: project_unauthorized_proc)
+  end
+end

app/controllers/projects/clusters_controller.rb

+# frozen_string_literal: true
+
+class Projects::ClustersController < Clusters::ClustersController
+  include ProjectUnauthorized
+
+  prepend_before_action :project
+  before_action :repository
+
+  layout 'project'
+
+  private
+
+  def clusterable
+    @clusterable ||= ClusterablePresenter.fabricate(project, current_user: current_user)
+  end
+
+  def project
+    @project ||= find_routable!(Project, File.join(params[:namespace_id], params[:project_id]), not_found_or_authorized_proc: project_unauthorized_proc)
+  end
+
+  def repository
+    @repository ||= project.repository
+  end
+end

app/helpers/clusters_helper.rb

@@ -11,7 +11,7 @@ module ClustersHelper
     return unless show_gcp_signup_offer?
 
     content_tag :section, class: 'no-animate expanded' do
-      render 'clusters/gcp_signup_offer_banner'
+      render 'clusters/clusters/gcp_signup_offer_banner'
     end
   end
 

app/presenters/project_clusterable_presenter.rb

@@ -9,6 +9,26 @@ class ProjectClusterablePresenter < ClusterablePresenter
     new_project_cluster_path(clusterable)
   end
 
+  def create_user_clusters_path
+    create_user_project_clusters_path(clusterable)
+  end
+
+  def create_gcp_clusters_path
+    create_gcp_project_clusters_path(clusterable)
+  end
+
+  def cluster_status_cluster_path(cluster, params = {})
+    cluster_status_project_cluster_path(clusterable, cluster, params)
+  end
+
+  def install_applications_cluster_path(cluster, application)
+    install_applications_project_cluster_path(clusterable, cluster, application)
+  end
+
+  def cluster_path(cluster, params = {})
+    project_cluster_path(clusterable, cluster, params)
+  end
+
   def clusterable_params
     { project_id: clusterable.to_param, namespace_id: clusterable.namespace.to_param }
   end

app/views/clusters/_advanced_settings.html.haml → app/views/clusters/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.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'), clusterable.cluster_path(@cluster), 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.")})

app/views/clusters/_cluster.html.haml → app/views/clusters/clusters/_cluster.html.haml

@@ -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.clusterable_params.merge(format: :json)) } }
+        data: { endpoint: clusterable.cluster_path(cluster, format: :json) } }
         %input.js-project-feature-toggle-input{ type: "hidden", value: cluster.enabled? }
         = icon("spinner spin", class: "loading-icon")
         %span.toggle-icon

app/views/clusters/_integration_form.html.haml → app/views/clusters/clusters/_integration_form.html.haml

-= form_for @cluster, url: cluster_path(@cluster), as: :cluster do |field|
+= form_for @cluster, url: clusterable.cluster_path(@cluster), as: :cluster do |field|
   = form_errors(@cluster)
   = hidden_clusterable_fields
   .form-group

app/views/clusters/gcp/_form.html.haml → app/views/clusters/clusters/gcp/_form.html.haml

@@ -12,7 +12,7 @@
 
 %p= link_to('Select a different Google account', @authorize_url)
 
-= form_for @gcp_cluster, html: { class: 'js-gke-cluster-creation prepend-top-20', data: { token: token_in_session } }, url: create_gcp_clusters_path, as: :cluster do |field|
+= form_for @gcp_cluster, html: { class: 'js-gke-cluster-creation prepend-top-20', data: { token: token_in_session } }, url: clusterable.create_gcp_clusters_path, as: :cluster do |field|
   = form_errors(@gcp_cluster)
   = hidden_clusterable_fields
   .form-group

app/views/clusters/gcp/_show.html.haml → app/views/clusters/clusters/gcp/_show.html.haml

@@ -6,7 +6,7 @@
     %span.input-group-append
       = clipboard_button(text: @cluster.name, title: s_('ClusterIntegration|Copy Kubernetes cluster name'), class: 'input-group-text btn-default')
 
-= form_for @cluster, url: cluster_path(@cluster), as: :cluster do |field|
+= form_for @cluster, url: clusterable.cluster_path(@cluster), as: :cluster do |field|
   = form_errors(@cluster)
   = hidden_clusterable_fields
 

app/views/clusters/new.html.haml → app/views/clusters/clusters/new.html.haml

@@ -19,9 +19,9 @@
 
     .tab-content.gitlab-tab-content
       .tab-pane{ id: 'create-gcp-cluster-pane', class: active_when(active_tab == 'gcp'), role: 'tabpanel' }
-        = render 'clusters/gcp/header'
+        = render 'clusters/clusters/gcp/header'
         - if @valid_gcp_token
-          = render 'clusters/gcp/form'
+          = render 'clusters/clusters/gcp/form'
         - elsif @authorize_url
           .signin-with-google
             = link_to(image_tag('auth_buttons/signin_with_google.png', width: '191px'), @authorize_url)
@@ -32,5 +32,5 @@
           = s_('Google authentication is not %{link_to_documentation}. Ask your GitLab administrator if you want to use this service.').html_safe % { link_to_documentation: link }
 
       .tab-pane{ id: 'add-user-cluster-pane', class: active_when(active_tab == 'user'), role: 'tabpanel' }
-        = render 'clusters/user/header'
-        = render 'clusters/user/form'
+        = render 'clusters/clusters/user/header'
+        = render 'clusters/clusters/user/form'

app/views/clusters/show.html.haml → app/views/clusters/clusters/show.html.haml

@@ -6,13 +6,13 @@
 
 - expanded = Rails.env.test?
 
-- status_path = status_cluster_path(@cluster.id, clusterable.clusterable_params.merge(format: :json)) if can?(current_user, :admin_cluster, @cluster)
+- status_path = clusterable.cluster_status_cluster_path(@cluster.id, 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.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),
+  install_helm_path: clusterable.install_applications_cluster_path(@cluster, :helm),
+  install_ingress_path: clusterable.install_applications_cluster_path(@cluster, :ingress),
+  install_prometheus_path: clusterable.install_applications_cluster_path(@cluster, :prometheus),
+  install_runner_path: clusterable.install_applications_cluster_path(@cluster, :runner),
+  install_jupyter_path: clusterable.install_applications_cluster_path(@cluster, :jupyter),
   toggle_status: @cluster.enabled? ? 'true': 'false',
   cluster_status: @cluster.status_name,
   cluster_status_reason: @cluster.status_reason,
@@ -39,9 +39,9 @@
       %p= s_('ClusterIntegration|See and edit the details for your Kubernetes cluster')
     .settings-content
       - if @cluster.managed?
-        = render 'clusters/gcp/show'
+        = render 'clusters/clusters/gcp/show'
       - else
-        = render 'clusters/user/show'
+        = render 'clusters/clusters/user/show'
 
   %section.settings.no-animate#js-cluster-advanced-settings{ class: ('expanded' if expanded) }
     .settings-header

app/views/clusters/user/_form.html.haml → app/views/clusters/clusters/user/_form.html.haml

-= form_for @user_cluster, url: create_user_clusters_path, as: :cluster do |field|
+= form_for @user_cluster, url: clusterable.create_user_clusters_path, as: :cluster do |field|
   = form_errors(@user_cluster)
   = hidden_clusterable_fields
   .form-group

app/views/clusters/user/_show.html.haml → app/views/clusters/clusters/user/_show.html.haml

-= form_for @cluster, url: cluster_path(@cluster), as: :cluster do |field|
+= form_for @cluster, url: clusterable.cluster_path(@cluster), as: :cluster do |field|
   = form_errors(@cluster)
   = hidden_clusterable_fields
   .form-group

config/routes.rb

@@ -74,7 +74,18 @@ Rails.application.routes.draw do
       resources :issues, module: :boards, only: [:index, :update]
     end
 
-    resources :clusters, only: [:update, :destroy] do
+    # UserCallouts
+    resources :user_callouts, only: [:create]
+
+    get 'ide' => 'ide#index'
+    get 'ide/*vueroute' => 'ide#index', format: false
+
+    draw :operations
+    draw :instance_statistics
+  end
+
+  concern :clusterable do
+    resources :clusters, only: [:index, :new, :show, :update, :destroy] do
       collection do
         post :create_user
         post :create_gcp
@@ -85,22 +96,9 @@ Rails.application.routes.draw do
           post '/:application', to: 'clusters/applications#create', as: :install_applications
         end
 
-        get :status, format: :json
+        get :cluster_status, format: :json
       end
     end
-
-    # UserCallouts
-    resources :user_callouts, only: [:create]
-
-    get 'ide' => 'ide#index'
-    get 'ide/*vueroute' => 'ide#index', format: false
-
-    draw :operations
-    draw :instance_statistics
-  end
-
-  concern :clusterable do
-    resources :clusters, only: [:index, :new, :show], controller: '/clusters'
   end
 
   draw :api

qa/qa/page/project/operations/kubernetes/add.rb

@@ -4,7 +4,7 @@ module QA
       module Operations
         module Kubernetes
           class Add < Page::Base
-            view 'app/views/clusters/new.html.haml' do
+            view 'app/views/clusters/clusters/new.html.haml' do
               element :add_existing_cluster_button, "Add existing cluster" # rubocop:disable QA/ElementWithPattern
             end
 

qa/qa/page/project/operations/kubernetes/add_existing.rb

@@ -4,7 +4,7 @@ module QA
       module Operations
         module Kubernetes
           class AddExisting < Page::Base
-            view 'app/views/clusters/user/_form.html.haml' do
+            view 'app/views/clusters/clusters/user/_form.html.haml' do
               element :cluster_name, 'text_field :name' # rubocop:disable QA/ElementWithPattern
               element :api_url, 'text_field :api_url' # rubocop:disable QA/ElementWithPattern
               element :ca_certificate, 'text_area :ca_cert' # rubocop:disable QA/ElementWithPattern

qa/qa/page/project/operations/kubernetes/index.rb

@@ -4,7 +4,7 @@ module QA
       module Operations
         module Kubernetes
           class Index < Page::Base
-            view 'app/views/clusters/_empty_state.html.haml' do
+            view 'app/views/clusters/clusters/_empty_state.html.haml' do
               element :add_kubernetes_cluster_button, "link_to s_('ClusterIntegration|Add Kubernetes cluster')" # rubocop:disable QA/ElementWithPattern
             end
 

spec/controllers/clusters/applications_controller_spec.rb → spec/controllers/projects/clusters/applications_controller_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe Clusters::ApplicationsController do
+describe Projects::Clusters::ApplicationsController do
   include AccessMatchersForController
 
   def current_application

spec/controllers/clusters_controller_spec.rb → spec/controllers/projects/clusters_controller_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe ClustersController do
+describe Projects::ClustersController do
   include AccessMatchersForController
   include GoogleApi::CloudPlatformHelpers
 
@@ -320,11 +320,11 @@ describe ClustersController do
     end
   end
 
-  describe 'GET status' do
+  describe 'GET cluster_status' do
     let(:cluster) { create(:cluster, :providing_by_gcp, projects: [project]) }
 
     def go
-      get :status,
+      get :cluster_status,
         namespace_id: project.namespace.to_param,
         project_id: project.to_param,
         id: cluster,

spec/features/projects/clusters/gcp_spec.rb

@@ -9,16 +9,16 @@ describe 'Gcp Cluster', :js do
   before do
     project.add_maintainer(user)
     gitlab_sign_in(user)
-    allow(ClustersController).to receive(:STATUS_POLLING_INTERVAL) { 100 }
+    allow(Projects::ClustersController).to receive(:STATUS_POLLING_INTERVAL) { 100 }
   end
 
   context 'when user has signed with Google' do
     let(:project_id) { 'test-project-1234' }
 
     before do
-      allow_any_instance_of(ClustersController)
+      allow_any_instance_of(Projects::ClustersController)
         .to receive(:token_in_session).and_return('token')
-      allow_any_instance_of(ClustersController)
+      allow_any_instance_of(Projects::ClustersController)
         .to receive(:expires_at_in_session).and_return(1.hour.since.to_i.to_s)
     end
 

spec/features/projects/clusters/user_spec.rb

@@ -9,7 +9,7 @@ describe 'User Cluster', :js do
   before do
     project.add_maintainer(user)
     gitlab_sign_in(user)
-    allow(ClustersController).to receive(:STATUS_POLLING_INTERVAL) { 100 }
+    allow(Projects::ClustersController).to receive(:STATUS_POLLING_INTERVAL) { 100 }
   end
 
   context 'when user does not have a cluster and visits cluster index page' do