Merge branch '296986-remove-geo-beta' into 'master'

Geo Nodes Beta - Remove "Beta"

See merge request gitlab-org/gitlab!61728

ee/app/assets/javascripts/pages/admin/geo/nodes/index.js

 import initGeoNodes from 'ee/geo_nodes';
+import { initGeoNodesBeta } from 'ee/geo_nodes_beta';
+
 import PersistentUserCallout from '~/persistent_user_callout';
 
-initGeoNodes();
-const callout = document.querySelector('.user-callout');
-PersistentUserCallout.factory(callout);
+if (gon.features?.geoNodesBeta) {
+  initGeoNodesBeta();
+} else {
+  initGeoNodes();
+  const callout = document.querySelector('.user-callout');
+  PersistentUserCallout.factory(callout);
+}

ee/app/assets/javascripts/pages/admin/geo/nodes_beta/index.js

-import { initGeoNodesBeta } from 'ee/geo_nodes_beta';
-
-initGeoNodesBeta();

ee/app/controllers/admin/geo/nodes_beta_controller.rb

-# frozen_string_literal: true
-
-class Admin::Geo::NodesBetaController < Admin::Geo::ApplicationController
-  before_action :check_license!
-
-  def index
-    redirect_to admin_geo_nodes_path if Feature.disabled?(:geo_nodes_beta)
-  end
-end

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

@@ -3,11 +3,16 @@
 class Admin::Geo::NodesController < Admin::Geo::ApplicationController
   before_action :check_license!, except: :index
   before_action :load_node, only: [:edit, :update]
+  before_action only: [:index] do
+    push_frontend_feature_flag(:geo_nodes_beta)
+  end
 
   # rubocop: disable CodeReuse/ActiveRecord
   def index
-    @nodes = GeoNode.all.order(:id)
-    @node = GeoNode.new
+    if Feature.disabled?(:geo_nodes_beta)
+      @nodes = GeoNode.all.order(:id)
+      @node = GeoNode.new
+    end
   end
   # rubocop: enable CodeReuse/ActiveRecord
 

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

-- page_title _('Geo Nodes')
+- page_title _('Geo sites')
 
 = render_enable_hashed_storage_warning
 = render_migrate_hashed_storage_warning
 = render partial: 'admin/geo/shared/license_alert'
 
-.d-flex.align-items-center.border-bottom.border-default.mb-4
-  %h3{ :class => "page-title" }
-    = _("Geo Nodes")
-  - if @nodes.any?
-    = link_to s_("GeoNodes|New node"), new_admin_geo_node_path, class: 'gl-button btn btn-confirm ml-auto qa-new-node-link'
+- if Feature.enabled?(:geo_nodes_beta)
+  #js-geo-nodes-beta{ data: node_vue_list_properties }
+- else
+  .d-flex.align-items-center.border-bottom.border-default.mb-4
+    %h3{ :class => "page-title" }
+      = _("Geo Nodes")
+    - if @nodes.any?
+      = link_to s_("GeoNodes|New node"), new_admin_geo_node_path, class: 'gl-button btn btn-confirm ml-auto qa-new-node-link'
 
-%p.page-subtitle.light
-  = s_('GeoNodes|With %{geo} you can install a special read-only and replicated instance anywhere. Before you add nodes, follow the %{instructions} in the exact order they appear.').html_safe % { geo: link_to('GitLab Geo', help_page_path('administration/geo/index.md'), target: '_blank'), instructions: link_to('setup instructions', help_page_path('administration/geo/setup/index.md'), target: '_blank') }
+  %p.page-subtitle.light
+    = s_('GeoNodes|With %{geo} you can install a special read-only and replicated instance anywhere. Before you add nodes, follow the %{instructions} in the exact order they appear.').html_safe % { geo: link_to('GitLab Geo', help_page_path('administration/geo/index.md'), target: '_blank'), instructions: link_to('setup instructions', help_page_path('administration/geo/setup/index.md'), target: '_blank') }
 
-- if @nodes.any?
-  #js-geo-nodes{ data: node_vue_list_properties }
-- else
-  = render 'shared/empty_states/geo'
+  - if @nodes.any?
+    #js-geo-nodes{ data: node_vue_list_properties }
+  - else
+    = render 'shared/empty_states/geo'

ee/app/views/admin/geo/nodes_beta/index.html.haml

-- page_title _('Geo sites')
-
-#js-geo-nodes-beta{ data: node_vue_list_properties }

ee/app/views/layouts/nav/ee/admin/_geo_sidebar.html.haml

-= nav_link(controller: %w(admin/geo/nodes admin/geo/nodes_beta admin/geo/projects admin/geo/uploads admin/geo/settings admin/geo/designs admin/geo/replicables)) do
+= nav_link(controller: %w(admin/geo/nodes admin/geo/projects admin/geo/uploads admin/geo/settings admin/geo/designs admin/geo/replicables)) do
   = link_to admin_geo_nodes_path, class: "qa-link-geo-menu" do
     .nav-icon-container
       = sprite_icon('location-dot')
     %span.nav-item-name
       #{ _('Geo') }
   %ul.sidebar-sub-level-items
-    = nav_link(controller: '%w(admin/geo/nodes admin/geo/nodes_beta)', html_options: { class: "fly-out-top-item" } ) do
+    = nav_link(controller: '%w(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') }

ee/config/routes/admin.rb

@@ -52,8 +52,6 @@ namespace :admin do
 
     resources :nodes, only: [:index, :create, :new, :edit, :update]
 
-    resources :nodes_beta, only: [:index]
-
     scope '/replication' do
       get '/', to: redirect(path: 'admin/geo/replication/projects')
 

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

@@ -65,6 +65,37 @@ RSpec.describe Admin::Geo::NodesController do
         expect(response).not_to redirect_to(:forbidden)
       end
     end
+
+    context 'without :geo_nodes_beta feature flag' do
+      before do
+        stub_feature_flags(geo_nodes_beta: false)
+        go
+      end
+
+      it 'sets @nodes and @node variables' do
+        expect(subject.instance_variable_get(:@nodes)).to eq(GeoNode.all.order(:id))
+        expect(subject.instance_variable_get(:@node)).to be_an_instance_of(GeoNode)
+      end
+    end
+
+    context 'with :geo_nodes_beta feature flag' do
+      before do
+        stub_feature_flags(geo_nodes_beta: true)
+        go
+      end
+
+      it 'does not set @nodes and @node variables' do
+        expect(subject.instance_variable_get(:@nodes)).to be_nil
+        expect(subject.instance_variable_get(:@node)).to be_nil
+      end
+    end
+
+    it 'pushes :geo_nodes_beta feature flag to the frontend' do
+      allow(subject).to receive(:push_frontend_feature_flag).and_call_original
+      expect(subject).to receive(:push_frontend_feature_flag).with(:geo_nodes_beta)
+
+      go
+    end
   end
 
   describe '#create' do

ee/spec/features/admin/geo/admin_geo_nodes_spec.rb

@@ -33,13 +33,33 @@ RSpec.describe 'admin Geo Nodes', :js, :geo do
   end
 
   describe 'index' do
-    it 'show all public Geo Nodes and create new node link' do
-      visit admin_geo_nodes_path
-      wait_for_requests
+    context 'without :geo_nodes_beta FF' do
+      before do
+        stub_feature_flags(geo_nodes_beta: false)
+        visit admin_geo_nodes_path
+        wait_for_requests
+      end
 
-      expect(page).to have_link('New node', href: new_admin_geo_node_path)
-      page.within(find('.card', match: :first)) do
-        expect(page).to have_content(geo_node.url)
+      it 'shows all public Geo Nodes and create new node link' do
+        expect(page).to have_link('New node', href: new_admin_geo_node_path)
+        page.within(find('.card', match: :first)) do
+          expect(page).to have_content(geo_node.url)
+        end
+      end
+    end
+
+    context 'with :geo_nodes_beta FF' do
+      before do
+        stub_feature_flags(geo_nodes_beta: true)
+        visit admin_geo_nodes_path
+        wait_for_requests
+      end
+
+      it 'shows all public Geo Nodes and create new node link' do
+        expect(page).to have_link('Add site', href: new_admin_geo_node_path)
+        page.within(find('.geo-node-core-details-grid-columns', match: :first)) do
+          expect(page).to have_content(geo_node.url)
+        end
       end
     end
 
@@ -128,7 +148,7 @@ RSpec.describe 'admin Geo Nodes', :js, :geo do
       wait_for_requests
       expect(current_path).to eq admin_geo_nodes_path
 
-      page.within(find('.card', match: :first)) do
+      page.within(find('.geo-node-core-details-grid-columns', match: :first)) do
         expect(page).to have_content(geo_node.url)
       end
     end
@@ -153,15 +173,8 @@ RSpec.describe 'admin Geo Nodes', :js, :geo do
       wait_for_requests
       expect(current_path).to eq admin_geo_nodes_path
 
-      page.within(find('.card', match: :first)) do
+      page.within(find('.geo-node-core-details-grid-columns', match: :first)) do
         expect(page).to have_content('http://newsite.com')
-        expect(page).to have_content('Primary')
-
-        click_button 'Other information'
-
-        page.within(find('.other-section')) do
-          expect(page).to have_content('http://internal-url.com')
-        end
       end
     end
 
@@ -177,16 +190,15 @@ RSpec.describe 'admin Geo Nodes', :js, :geo do
     end
 
     it 'removes an existing Geo Node' do
-      page.within(find('[data-testid="nodeActions"]', match: :first)) do
-        page.click_button('Remove')
-      end
-      page.within('.modal') do
-        page.click_button('Remove')
+      page.click_button('Remove')
+
+      page.within('.gl-modal') do
+        page.click_button('Remove node')
       end
 
       expect(current_path).to eq admin_geo_nodes_path
       wait_for_requests
-      expect(page).not_to have_css('.card')
+      expect(page).not_to have_css('.geo-node-core-details-grid-columns')
     end
   end
 
@@ -198,13 +210,11 @@ RSpec.describe 'admin Geo Nodes', :js, :geo do
     end
 
     it 'hides the New Node button' do
-      expect(page).not_to have_link('New node', href: new_admin_geo_node_path)
+      expect(page).not_to have_link('Add site', href: new_admin_geo_node_path)
     end
 
     it 'shows Discover GitLab Geo' do
-      page.within(find('h4')) do
-        expect(page).to have_content('Discover GitLab Geo')
-      end
+      expect(page).to have_content('Discover GitLab Geo')
     end
   end
 

ee/spec/features/geo_node_spec.rb

@@ -47,6 +47,8 @@ RSpec.describe 'GEO Nodes', :geo do
 
     describe 'Geo Nodes admin screen' do
       it "has a 'Replication details' button on listed secondary geo nodes pointing to correct URL", :js do
+        # TODO: Remove this spec when geo_nodes_beta is removed as this UI element is removed in new UI.
+        stub_feature_flags(geo_nodes_beta: false)
         visit admin_geo_nodes_path
 
         expect(page).to have_content(geo_primary.url)

ee/spec/requests/admin/geo/nodes_beta_controller_spec.rb → ee/spec/requests/admin/geo/nodes_controller_spec.rb

@@ -2,10 +2,11 @@
 
 require 'spec_helper'
 
-RSpec.describe Admin::Geo::NodesBetaController, :geo do
+RSpec.describe Admin::Geo::NodesController, :geo do
   include AdminModeHelper
 
   let_it_be(:admin) { create(:admin) }
+  let_it_be(:geo_node) { create(:geo_node) }
 
   before do
     enable_admin_mode!(admin)
@@ -24,10 +25,10 @@ RSpec.describe Admin::Geo::NodesBetaController, :geo do
         end
 
         it 'renders the Geo Nodes Beta View', :aggregate_failures do
-          get admin_geo_nodes_beta_path
+          get admin_geo_nodes_path
 
           expect(response).to render_template(:index)
-          expect(response.body).to include('Geo sites')
+          expect(response.body).to include('js-geo-nodes-beta')
         end
       end
 
@@ -36,10 +37,11 @@ RSpec.describe Admin::Geo::NodesBetaController, :geo do
           stub_feature_flags(geo_nodes_beta: false)
         end
 
-        it 'redirects to Geo Nodes View' do
-          get admin_geo_nodes_beta_path
+        it 'renders the Geo Nodes Legacy View', :aggregate_failures do
+          get admin_geo_nodes_path
 
-          expect(response).to redirect_to(admin_geo_nodes_path)
+          expect(response).to render_template(:index)
+          expect(response.body).to include('js-geo-nodes')
         end
       end
     end
@@ -47,12 +49,16 @@ RSpec.describe Admin::Geo::NodesBetaController, :geo do
     context 'without a valid license' do
       before do
         stub_licensed_features(geo: false)
+        get admin_geo_nodes_path
       end
 
-      it 'returns a 403' do
-        get admin_geo_nodes_beta_path
+      it 'does show license alert' do
+        expect(response).to render_template(partial: '_license_alert')
+        expect(response.body).to include('Geo is only available for users who have at least a Premium license.')
+      end
 
-        expect(response).to have_gitlab_http_status(:forbidden)
+      it 'does not redirects to the 403 page' do
+        expect(response).not_to have_gitlab_http_status(:forbidden)
       end
     end
   end