Commit 1a1fdf8e authored by Thong Kuah's avatar Thong Kuah

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
parent 28dabc67
import initDismissableCallout from '~/dismissable_callout';
import initGkeDropdowns from '~/projects/gke_cluster_dropdowns';
document.addEventListener('DOMContentLoaded', () => {
initDismissableCallout('.gcp-signup-offer');
initGkeDropdowns();
});
import initDismissableCallout from '~/dismissable_callout';
import initGkeDropdowns from '~/projects/gke_cluster_dropdowns';
document.addEventListener('DOMContentLoaded', () => {
initDismissableCallout('.gcp-signup-offer');
initGkeDropdowns();
});
import initDismissableCallout from '~/dismissable_callout';
import initGkeDropdowns from '~/projects/gke_cluster_dropdowns';
document.addEventListener('DOMContentLoaded', () => {
initDismissableCallout('.gcp-signup-offer');
initGkeDropdowns();
});
import initDismissableCallout from '~/dismissable_callout';
import initGkeDropdowns from '~/projects/gke_cluster_dropdowns';
import Project from './project'; import Project from './project';
import ShortcutsNavigation from '../../behaviors/shortcuts/shortcuts_navigation'; import ShortcutsNavigation from '../../behaviors/shortcuts/shortcuts_navigation';
document.addEventListener('DOMContentLoaded', () => { 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 Project(); // eslint-disable-line no-new
new ShortcutsNavigation(); // eslint-disable-line no-new new ShortcutsNavigation(); // eslint-disable-line no-new
}); });
...@@ -2,31 +2,25 @@ ...@@ -2,31 +2,25 @@
class Clusters::BaseController < ApplicationController class Clusters::BaseController < ApplicationController
include RoutableActions include RoutableActions
include ProjectUnauthorized
skip_before_action :authenticate_user! 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! before_action :authorize_read_cluster!
layout :determine_layout
helper_method :clusterable helper_method :clusterable
private private
# We can extend to `#group_type?` in the future def cluster
def require_project_id @cluster ||= clusterable.clusters.find(params[:id])
not_found unless project_type? .present(current_user: current_user)
end end
def project def authorize_update_cluster!
@project ||= find_routable!(Project, File.join(params[:namespace_id], params[:project_id]), not_found_or_authorized_proc: project_unauthorized_proc) access_denied! unless can?(current_user, :update_cluster, cluster)
end end
def repository def authorize_admin_cluster!
@repository ||= project.repository access_denied! unless can?(current_user, :admin_cluster, cluster)
end end
def authorize_read_cluster! def authorize_read_cluster!
...@@ -37,17 +31,7 @@ class Clusters::BaseController < ApplicationController ...@@ -37,17 +31,7 @@ class Clusters::BaseController < ApplicationController
access_denied! unless can?(current_user, :create_cluster, clusterable) access_denied! unless can?(current_user, :create_cluster, clusterable)
end end
def determine_layout
if project_type?
'project'
end
end
def clusterable def clusterable
@clusterable ||= ClusterablePresenter.fabricate(project, current_user: current_user) raise NotImplementedError
end
def project_type?
params[:project_id].present?
end end
end end
# frozen_string_literal: true # 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 :cluster, except: [:index, :new, :create_gcp, :create_user]
before_action :generate_gcp_authorize_url, only: [:new] before_action :generate_gcp_authorize_url, only: [:new]
before_action :validate_gcp_token, only: [:new] before_action :validate_gcp_token, only: [:new]
...@@ -9,7 +11,7 @@ class ClustersController < Clusters::BaseController ...@@ -9,7 +11,7 @@ class ClustersController < Clusters::BaseController
before_action :authorize_create_cluster!, only: [:new] before_action :authorize_create_cluster!, only: [:new]
before_action :authorize_update_cluster!, only: [:update] before_action :authorize_update_cluster!, only: [:update]
before_action :authorize_admin_cluster!, only: [:destroy] 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 helper_method :token_in_session
...@@ -23,7 +25,8 @@ class ClustersController < Clusters::BaseController ...@@ -23,7 +25,8 @@ class ClustersController < Clusters::BaseController
def new def new
end end
def status # Overridding ActionController::Metal#status is NOT a good idea
def cluster_status
respond_to do |format| respond_to do |format|
format.json do format.json do
Gitlab::PollingInterval.set_header(response, interval: STATUS_POLLING_INTERVAL) Gitlab::PollingInterval.set_header(response, interval: STATUS_POLLING_INTERVAL)
...@@ -107,11 +110,6 @@ class ClustersController < Clusters::BaseController ...@@ -107,11 +110,6 @@ class ClustersController < Clusters::BaseController
private private
def cluster
@cluster ||= clusterable.clusters.find(params[:id])
.present(current_user: current_user)
end
def update_params def update_params
if cluster.managed? if cluster.managed?
params.require(:cluster).permit( params.require(:cluster).permit(
...@@ -214,14 +212,6 @@ class ClustersController < Clusters::BaseController ...@@ -214,14 +212,6 @@ class ClustersController < Clusters::BaseController
end end
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 def update_applications_status
@cluster.applications.each(&:schedule_status_update) @cluster.applications.each(&:schedule_status_update)
end end
......
# 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
# 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
...@@ -11,7 +11,7 @@ module ClustersHelper ...@@ -11,7 +11,7 @@ module ClustersHelper
return unless show_gcp_signup_offer? return unless show_gcp_signup_offer?
content_tag :section, class: 'no-animate expanded' do content_tag :section, class: 'no-animate expanded' do
render 'clusters/gcp_signup_offer_banner' render 'clusters/clusters/gcp_signup_offer_banner'
end end
end end
......
...@@ -9,6 +9,26 @@ class ProjectClusterablePresenter < ClusterablePresenter ...@@ -9,6 +9,26 @@ class ProjectClusterablePresenter < ClusterablePresenter
new_project_cluster_path(clusterable) new_project_cluster_path(clusterable)
end 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 def clusterable_params
{ project_id: clusterable.to_param, namespace_id: clusterable.namespace.to_param } { project_id: clusterable.to_param, namespace_id: clusterable.namespace.to_param }
end end
......
...@@ -12,4 +12,4 @@ ...@@ -12,4 +12,4 @@
= s_('ClusterIntegration|Remove Kubernetes cluster integration') = s_('ClusterIntegration|Remove Kubernetes cluster integration')
%p %p
= s_("ClusterIntegration|Remove this Kubernetes cluster's configuration from this project. This will not delete your actual Kubernetes cluster.") = 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.")})
...@@ -16,7 +16,7 @@ ...@@ -16,7 +16,7 @@
class: "#{'is-checked' if cluster.enabled?} #{'is-disabled' if !cluster.can_toggle_cluster?}", class: "#{'is-checked' if cluster.enabled?} #{'is-disabled' if !cluster.can_toggle_cluster?}",
"aria-label": s_("ClusterIntegration|Toggle Kubernetes Cluster"), "aria-label": s_("ClusterIntegration|Toggle Kubernetes Cluster"),
disabled: !cluster.can_toggle_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? } %input.js-project-feature-toggle-input{ type: "hidden", value: cluster.enabled? }
= icon("spinner spin", class: "loading-icon") = icon("spinner spin", class: "loading-icon")
%span.toggle-icon %span.toggle-icon
......
= 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) = form_errors(@cluster)
= hidden_clusterable_fields = hidden_clusterable_fields
.form-group .form-group
......
...@@ -12,7 +12,7 @@ ...@@ -12,7 +12,7 @@
%p= link_to('Select a different Google account', @authorize_url) %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) = form_errors(@gcp_cluster)
= hidden_clusterable_fields = hidden_clusterable_fields
.form-group .form-group
......
...@@ -6,7 +6,7 @@ ...@@ -6,7 +6,7 @@
%span.input-group-append %span.input-group-append
= clipboard_button(text: @cluster.name, title: s_('ClusterIntegration|Copy Kubernetes cluster name'), class: 'input-group-text btn-default') = 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) = form_errors(@cluster)
= hidden_clusterable_fields = hidden_clusterable_fields
......
...@@ -19,9 +19,9 @@ ...@@ -19,9 +19,9 @@
.tab-content.gitlab-tab-content .tab-content.gitlab-tab-content
.tab-pane{ id: 'create-gcp-cluster-pane', class: active_when(active_tab == 'gcp'), role: 'tabpanel' } .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 - if @valid_gcp_token
= render 'clusters/gcp/form' = render 'clusters/clusters/gcp/form'
- elsif @authorize_url - elsif @authorize_url
.signin-with-google .signin-with-google
= link_to(image_tag('auth_buttons/signin_with_google.png', width: '191px'), @authorize_url) = link_to(image_tag('auth_buttons/signin_with_google.png', width: '191px'), @authorize_url)
...@@ -32,5 +32,5 @@ ...@@ -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 } = 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' } .tab-pane{ id: 'add-user-cluster-pane', class: active_when(active_tab == 'user'), role: 'tabpanel' }
= render 'clusters/user/header' = render 'clusters/clusters/user/header'
= render 'clusters/user/form' = render 'clusters/clusters/user/form'
...@@ -6,13 +6,13 @@ ...@@ -6,13 +6,13 @@
- expanded = Rails.env.test? - 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, .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_helm_path: clusterable.install_applications_cluster_path(@cluster, :helm),
install_ingress_path: install_applications_cluster_path(@cluster, :ingress, clusterable.clusterable_params), install_ingress_path: clusterable.install_applications_cluster_path(@cluster, :ingress),
install_prometheus_path: install_applications_cluster_path(@cluster, :prometheus, clusterable.clusterable_params), install_prometheus_path: clusterable.install_applications_cluster_path(@cluster, :prometheus),
install_runner_path: install_applications_cluster_path(@cluster, :runner, clusterable.clusterable_params), install_runner_path: clusterable.install_applications_cluster_path(@cluster, :runner),
install_jupyter_path: install_applications_cluster_path(@cluster, :jupyter, clusterable.clusterable_params), install_jupyter_path: clusterable.install_applications_cluster_path(@cluster, :jupyter),
toggle_status: @cluster.enabled? ? 'true': 'false', toggle_status: @cluster.enabled? ? 'true': 'false',
cluster_status: @cluster.status_name, cluster_status: @cluster.status_name,
cluster_status_reason: @cluster.status_reason, cluster_status_reason: @cluster.status_reason,
...@@ -39,9 +39,9 @@ ...@@ -39,9 +39,9 @@
%p= s_('ClusterIntegration|See and edit the details for your Kubernetes cluster') %p= s_('ClusterIntegration|See and edit the details for your Kubernetes cluster')
.settings-content .settings-content
- if @cluster.managed? - if @cluster.managed?
= render 'clusters/gcp/show' = render 'clusters/clusters/gcp/show'
- else - else
= render 'clusters/user/show' = render 'clusters/clusters/user/show'
%section.settings.no-animate#js-cluster-advanced-settings{ class: ('expanded' if expanded) } %section.settings.no-animate#js-cluster-advanced-settings{ class: ('expanded' if expanded) }
.settings-header .settings-header
......
= 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) = form_errors(@user_cluster)
= hidden_clusterable_fields = hidden_clusterable_fields
.form-group .form-group
......
= 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) = form_errors(@cluster)
= hidden_clusterable_fields = hidden_clusterable_fields
.form-group .form-group
......
...@@ -74,7 +74,18 @@ Rails.application.routes.draw do ...@@ -74,7 +74,18 @@ Rails.application.routes.draw do
resources :issues, module: :boards, only: [:index, :update] resources :issues, module: :boards, only: [:index, :update]
end 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 collection do
post :create_user post :create_user
post :create_gcp post :create_gcp
...@@ -85,22 +96,9 @@ Rails.application.routes.draw do ...@@ -85,22 +96,9 @@ Rails.application.routes.draw do
post '/:application', to: 'clusters/applications#create', as: :install_applications post '/:application', to: 'clusters/applications#create', as: :install_applications
end end
get :status, format: :json get :cluster_status, format: :json
end end
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 end
draw :api draw :api
......
...@@ -4,7 +4,7 @@ module QA ...@@ -4,7 +4,7 @@ module QA
module Operations module Operations
module Kubernetes module Kubernetes
class Add < Page::Base 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 element :add_existing_cluster_button, "Add existing cluster" # rubocop:disable QA/ElementWithPattern
end end
......
...@@ -4,7 +4,7 @@ module QA ...@@ -4,7 +4,7 @@ module QA
module Operations module Operations
module Kubernetes module Kubernetes
class AddExisting < Page::Base 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 :cluster_name, 'text_field :name' # rubocop:disable QA/ElementWithPattern
element :api_url, 'text_field :api_url' # 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 element :ca_certificate, 'text_area :ca_cert' # rubocop:disable QA/ElementWithPattern
......
...@@ -4,7 +4,7 @@ module QA ...@@ -4,7 +4,7 @@ module QA
module Operations module Operations
module Kubernetes module Kubernetes
class Index < Page::Base 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 element :add_kubernetes_cluster_button, "link_to s_('ClusterIntegration|Add Kubernetes cluster')" # rubocop:disable QA/ElementWithPattern
end end
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
require 'spec_helper' require 'spec_helper'
describe Clusters::ApplicationsController do describe Projects::Clusters::ApplicationsController do
include AccessMatchersForController include AccessMatchersForController
def current_application def current_application
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
require 'spec_helper' require 'spec_helper'
describe ClustersController do describe Projects::ClustersController do
include AccessMatchersForController include AccessMatchersForController
include GoogleApi::CloudPlatformHelpers include GoogleApi::CloudPlatformHelpers
...@@ -320,11 +320,11 @@ describe ClustersController do ...@@ -320,11 +320,11 @@ describe ClustersController do
end end
end end
describe 'GET status' do describe 'GET cluster_status' do
let(:cluster) { create(:cluster, :providing_by_gcp, projects: [project]) } let(:cluster) { create(:cluster, :providing_by_gcp, projects: [project]) }
def go def go
get :status, get :cluster_status,
namespace_id: project.namespace.to_param, namespace_id: project.namespace.to_param,
project_id: project.to_param, project_id: project.to_param,
id: cluster, id: cluster,
......
...@@ -9,16 +9,16 @@ describe 'Gcp Cluster', :js do ...@@ -9,16 +9,16 @@ describe 'Gcp Cluster', :js do
before do before do
project.add_maintainer(user) project.add_maintainer(user)
gitlab_sign_in(user) gitlab_sign_in(user)
allow(ClustersController).to receive(:STATUS_POLLING_INTERVAL) { 100 } allow(Projects::ClustersController).to receive(:STATUS_POLLING_INTERVAL) { 100 }
end end
context 'when user has signed with Google' do context 'when user has signed with Google' do
let(:project_id) { 'test-project-1234' } let(:project_id) { 'test-project-1234' }
before do before do
allow_any_instance_of(ClustersController) allow_any_instance_of(Projects::ClustersController)
.to receive(:token_in_session).and_return('token') .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) .to receive(:expires_at_in_session).and_return(1.hour.since.to_i.to_s)
end end
......
...@@ -9,7 +9,7 @@ describe 'User Cluster', :js do ...@@ -9,7 +9,7 @@ describe 'User Cluster', :js do
before do before do
project.add_maintainer(user) project.add_maintainer(user)
gitlab_sign_in(user) gitlab_sign_in(user)
allow(ClustersController).to receive(:STATUS_POLLING_INTERVAL) { 100 } allow(Projects::ClustersController).to receive(:STATUS_POLLING_INTERVAL) { 100 }
end end
context 'when user does not have a cluster and visits cluster index page' do context 'when user does not have a cluster and visits cluster index page' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment