Merge branch 'admin-geo-namespace-support' into 'master'

Geo: Refactor Admin controllers to use Geo specific namespace

See merge request gitlab-org/gitlab-ee!7395

app/views/layouts/nav/sidebar/_admin.html.haml

@@ -7,14 +7,14 @@
         .sidebar-context-title
           = _('Admin Area')
     %ul.sidebar-top-level-items
-      = nav_link(controller: %w(dashboard admin projects users groups jobs runners gitaly_servers), html_options: {class: 'home'}) do
+      = nav_link(controller: %w(dashboard admin admin/projects users groups jobs runners gitaly_servers), html_options: {class: 'home'}) do
         = link_to admin_root_path, class: 'shortcuts-tree' do
           .nav-icon-container
             = sprite_icon('overview')
           %span.nav-item-name
             = _('Overview')
         %ul.sidebar-sub-level-items
-          = nav_link(controller: %w(dashboard admin projects users groups jobs runners gitaly_servers), html_options: { class: "fly-out-top-item" } ) do
+          = nav_link(controller: %w(dashboard admin admin/projects users groups jobs runners gitaly_servers), html_options: { class: "fly-out-top-item" } ) do
             = link_to admin_root_path do
               %strong.fly-out-top-item-name
                 = _('Overview')
@@ -23,7 +23,7 @@
             = link_to admin_root_path, title: _('Overview') do
               %span
                 = _('Dashboard')
-          = nav_link(controller: [:admin, :projects]) do
+          = nav_link(controller: [:admin, 'admin/projects']) do
             = link_to admin_projects_path, title: _('Projects') do
               %span
                 = _('Projects')
@@ -170,7 +170,7 @@
               %strong.fly-out-top-item-name
                 #{ _('Push Rules') }
 
-      = nav_link(controller: [:geo_nodes, :geo_projects]) do
+      = nav_link(controller: %w(admin/geo/nodes admin/geo/projects)) do
         = link_to admin_geo_nodes_path do
           .nav-icon-container
             = sprite_icon('location-dot')
@@ -178,16 +178,16 @@
             #{ _('Geo Nodes') }
         - if Gitlab::Geo.secondary?
           %ul.sidebar-sub-level-items
-            = nav_link(controller: :geo_nodes, html_options: { class: "fly-out-top-item" } ) do
+            = nav_link(controller: 'admin/geo/nodes', html_options: { class: "fly-out-top-item" } ) do
               = link_to admin_geo_nodes_path do
                 %strong.fly-out-top-item-name
                   #{ _('Geo Nodes') }
             %li.divider.fly-out-top-item
-            = nav_link(path: 'geo_nodes#index') do
+            = nav_link(path: 'admin/geo/nodes#index') do
               = link_to admin_geo_nodes_path, title: 'Nodes' do
                 %span
                   #{ _('Nodes') }
-            = nav_link(path: 'geo_projects#index') do
+            = nav_link(path: 'admin/geo/projects#index') do
               = link_to admin_geo_projects_path, title: 'Projects' do
                 %span
                   #{ _('Projects') }

config/routes/admin.rb

@@ -132,13 +132,15 @@ namespace :admin do
     get :download, on: :member
   end
 
-  resources :geo_nodes, only: [:index, :create, :new, :edit, :update]
+  namespace :geo do
+    resources :nodes, only: [:index, :create, :new, :edit, :update]
 
-  resources :geo_projects, only: [:index] do
-    member do
-      post :recheck
-      post :resync
-      post :force_redownload
+    resources :projects, only: :index do
+      member do
+        post :recheck
+        post :resync
+        post :force_redownload
+      end
     end
   end
 

ee/app/controllers/admin/geo_nodes_controller.rb → ee/app/controllers/admin/geo/nodes_controller.rb

-class Admin::GeoNodesController < Admin::ApplicationController
+# frozen_string_literal: true
+
+class Admin::Geo::NodesController < Admin::ApplicationController
   before_action :check_license, except: :index
   before_action :load_node, only: [:edit, :update]
 

ee/app/controllers/admin/geo_projects_controller.rb → ee/app/controllers/admin/geo/projects_controller.rb

 # frozen_string_literal: true
 
-class Admin::GeoProjectsController < Admin::ApplicationController
+class Admin::Geo::ProjectsController < Admin::ApplicationController
   before_action :check_license
   before_action :load_registry, except: [:index]
 

ee/app/views/admin/geo_nodes/index.html.haml → ee/app/views/admin/geo/nodes/index.html.haml

-- page_title 'Geo nodes'
+- page_title 'Geo Nodes'
 - @content_class = "geo-admin-container"
 
 %h2.page-title.clearfix

ee/app/views/admin/geo_projects/index.html.haml → ee/app/views/admin/geo/projects/index.html.haml

-- page_title 'Projects'
+- page_title 'Geo Projects'
 - @no_container = true
 - @content_class = "geo-admin-container geo-admin-projects"
 - params[:sync_status] ||= []

ee/lib/ee/gitlab/middleware/read_only/controller.rb

@@ -6,11 +6,11 @@ module EE
           extend ::Gitlab::Utils::Override
 
           WHITELISTED_GEO_ROUTES = {
-            'admin/geo_nodes' => %w{update}
+            'admin/geo/nodes' => %w{update}
           }.freeze
 
           WHITELISTED_GEO_ROUTES_TRACKING_DB = {
-            'admin/geo_projects' => %w{resync recheck force_redownload}
+            'admin/geo/projects' => %w{resync recheck force_redownload}
           }.freeze
 
           private
@@ -22,7 +22,7 @@ module EE
 
           def geo_node_update_route
             # Calling route_hash may be expensive. Only do it if we think there's a possible match
-            return false unless request.path =~ %r{/admin/geo_}
+            return false unless request.path.start_with?('/admin/geo/')
 
             controller = route_hash[:controller]
             action = route_hash[:action]
@@ -36,8 +36,8 @@ module EE
 
           def geo_proxy_git_push_ssh_route
             routes = ::Gitlab::Middleware::ReadOnly::API_VERSIONS.map do |version|
-              ["/api/v#{version}/geo/proxy_git_push_ssh/info_refs",
-               "/api/v#{version}/geo/proxy_git_push_ssh/push"]
+              %W(/api/v#{version}/geo/proxy_git_push_ssh/info_refs
+                 /api/v#{version}/geo/proxy_git_push_ssh/push)
             end
             routes.flatten.include?(request.path)
           end

ee/spec/controllers/admin/geo_nodes_controller_spec.rb → ee/spec/controllers/admin/geo/nodes_controller_spec.rb

+# frozen_string_literal: true
 require 'spec_helper'
 
-describe Admin::GeoNodesController, :postgresql do
+describe Admin::Geo::NodesController, :postgresql do
   shared_examples 'unlicensed geo action' do
     it 'redirects to the license page' do
       expect(response).to redirect_to(admin_license_path)

ee/spec/controllers/admin/geo_projects_controller_spec.rb → ee/spec/controllers/admin/geo/projects_controller_spec.rb

 # frozen_string_literal: true
 require 'spec_helper'
 
-describe Admin::GeoProjectsController, :geo do
+describe Admin::Geo::ProjectsController, :geo do
   set(:admin) { create(:admin) }
   let(:synced_registry) { create(:geo_project_registry, :synced) }
 

ee/spec/javascripts/geo_nodes/mock_data.js

@@ -3,7 +3,7 @@ export const PRIMARY_VERSION = {
   revision: 'b93c51849b',
 };
 
-export const NODE_DETAILS_PATH = '/admin/geo_nodes';
+export const NODE_DETAILS_PATH = '/admin/geo/nodes';
 
 export const mockNodes = [
   {
@@ -19,7 +19,7 @@ export const mockNodes = [
       self: 'http://127.0.0.1:3001/api/v4/geo_nodes/1',
       repair: 'http://127.0.0.1:3001/api/v4/geo_nodes/1/repair',
       status: 'http://127.0.0.1:3001/api/v4/geo_nodes/1/status',
-      web_edit: 'http://127.0.0.1:3001/admin/geo_nodes/1/edit',
+      web_edit: 'http://127.0.0.1:3001/admin/geo/nodes/1/edit',
     },
   },
   {
@@ -35,7 +35,7 @@ export const mockNodes = [
       self: 'http://127.0.0.1:3001/api/v4/geo_nodes/2',
       repair: 'http://127.0.0.1:3001/api/v4/geo_nodes/2/repair',
       status: 'http://127.0.0.1:3001/api/v4/geo_nodes/2/status',
-      web_edit: 'http://127.0.0.1:3001/admin/geo_nodes/2/edit',
+      web_edit: 'http://127.0.0.1:3001/admin/geo/nodes/2/edit',
     },
   },
 ];
@@ -50,7 +50,7 @@ export const mockNode = {
   basePath: 'http://127.0.0.1:3001/api/v4/geo_nodes/1',
   repairPath: 'http://127.0.0.1:3001/api/v4/geo_nodes/1/repair',
   statusPath: 'http://127.0.0.1:3001/api/v4/geo_nodes/1/status',
-  editPath: 'http://127.0.0.1:3001/admin/geo_nodes/1/edit',
+  editPath: 'http://127.0.0.1:3001/admin/geo/nodes/1/edit',
 };
 
 export const rawMockNodeDetails = {

ee/spec/lib/ee/gitlab/middleware/read_only_spec.rb

@@ -44,7 +44,7 @@ describe Gitlab::Middleware::ReadOnly do
     context 'whitelisted requests' do
       it 'expects a PATCH request to geo_nodes update URL to be allowed' do
         expect(Rails.application.routes).to receive(:recognize_path).and_call_original
-        response = request.patch('/admin/geo_nodes/1')
+        response = request.patch('/admin/geo/nodes/1')
 
         expect(response).not_to be_redirect
         expect(subject).not_to disallow_request

ee/spec/requests/api/geo_nodes_spec.rb

@@ -34,7 +34,7 @@ describe API::GeoNodes, :geo, :prometheus, api: true do
 
       expect(response).to have_gitlab_http_status(200)
       expect(response).to match_response_schema('public_api/v4/geo_node', dir: 'ee')
-      expect(json_response['web_edit_url']).to end_with("/admin/geo_nodes/#{primary.id}/edit")
+      expect(json_response['web_edit_url']).to end_with("/admin/geo/nodes/#{primary.id}/edit")
 
       links = json_response['_links']
       expect(links['self']).to end_with("/api/v4/geo_nodes/#{primary.id}")

ee/spec/routing/admin_routing_spec.rb

@@ -2,23 +2,47 @@
 require 'spec_helper'
 
 describe 'EE-specific admin routing' do
-  describe Admin::GeoProjectsController, 'routing' do
+  describe Admin::Geo::ProjectsController, 'routing' do
     let(:project_registry) { create(:geo_project_registry) }
 
-    it 'routes ../ to #index' do
-      expect(get('/admin/geo_projects')).to route_to('admin/geo_projects#index')
+    it 'routes / to #index' do
+      expect(get('/admin/geo/projects')).to route_to('admin/geo/projects#index')
     end
 
-    it 'routes ../:id/recheck to #recheck' do
-      expect(post("admin/geo_projects/#{project_registry.id}/recheck")).to route_to('admin/geo_projects#recheck', id: project_registry.id.to_s)
+    it 'routes /:id/recheck to #recheck' do
+      expect(post("admin/geo/projects/#{project_registry.id}/recheck")).to route_to('admin/geo/projects#recheck', id: project_registry.to_param)
     end
 
-    it 'routes ../id:/resync to #resync' do
-      expect(post("admin/geo_projects/#{project_registry.id}/resync")).to route_to('admin/geo_projects#resync', id: project_registry.id.to_s)
+    it 'routes /id:/resync to #resync' do
+      expect(post("admin/geo/projects/#{project_registry.id}/resync")).to route_to('admin/geo/projects#resync', id: project_registry.to_param)
     end
 
-    it 'routes ../id:/force_redownload to #force_redownload' do
-      expect(post("admin/geo_projects/#{project_registry.id}/force_redownload")).to route_to('admin/geo_projects#force_redownload', id: project_registry.id.to_s)
+    it 'routes /id:/force_redownload to #force_redownload' do
+      expect(post("admin/geo/projects/#{project_registry.id}/force_redownload")).to route_to('admin/geo/projects#force_redownload', id: project_registry.to_param)
+    end
+  end
+
+  describe Admin::Geo::NodesController, 'routing' do
+    let(:geo_node) { create(:geo_node) }
+
+    it 'routes / to #index' do
+      expect(get('/admin/geo/nodes')).to route_to('admin/geo/nodes#index')
+    end
+
+    it 'routes /new to #new' do
+      expect(get('/admin/geo/nodes/new')).to route_to('admin/geo/nodes#new')
+    end
+
+    it 'routes /edit to #edit' do
+      expect(get("/admin/geo/nodes/#{geo_node.id}/edit")).to route_to('admin/geo/nodes#edit', id: geo_node.to_param)
+    end
+
+    it 'routes post / to #create' do
+      expect(post('/admin/geo/nodes/')).to route_to('admin/geo/nodes#create')
+    end
+
+    it 'routes patch /:id to #update' do
+      expect(patch("/admin/geo/nodes/#{geo_node.id}")).to route_to('admin/geo/nodes#update', id: geo_node.to_param)
     end
   end
 end

qa/qa/ee/page/admin/geo/nodes/new.rb

@@ -5,12 +5,12 @@ module QA
         module Geo
           module Nodes
             class New < QA::Page::Base
-              view 'ee/app/views/admin/geo_nodes/_form.html.haml' do
+              view 'ee/app/views/admin/geo/nodes/_form.html.haml' do
                 element :node_url_field, 'text_field :url'
                 element :node_url_placeholder, "label :url, 'URL'"
               end
 
-              view 'ee/app/views/admin/geo_nodes/new.html.haml' do
+              view 'ee/app/views/admin/geo/nodes/new.html.haml' do
                 element :add_node_button, "submit 'Add Node'"
               end
 

qa/qa/ee/page/admin/geo/nodes/show.rb

@@ -5,7 +5,7 @@ module QA
         module Geo
           module Nodes
             class Show < QA::Page::Base
-              view 'ee/app/views/admin/geo_nodes/index.html.haml' do
+              view 'ee/app/views/admin/geo/nodes/index.html.haml' do
                 element :new_node_link, /link_to .*New node/
               end