Merge branch '11729-better-license-alert' into 'master'

Refactor Geo Node License Flash

Closes #11729

See merge request gitlab-org/gitlab!22933

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

+import initVueAlerts from '~/vue_alerts';
+
+document.addEventListener('DOMContentLoaded', initVueAlerts);

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

@@ -7,8 +7,7 @@ class Admin::Geo::ApplicationController < Admin::ApplicationController
 
   def check_license!
     unless Gitlab::Geo.license_allows?
-      flash[:alert] = _('You need a different license to use Geo replication.')
-      redirect_to admin_license_path
+      render_403
     end
   end
 end

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

@@ -10,10 +10,6 @@ class Admin::Geo::NodesController < Admin::Geo::ApplicationController
     @nodes = GeoNode.all.order(:id)
     @node = GeoNode.new
 
-    unless Gitlab::Geo.license_allows?
-      flash.now[:alert] = _('You need a different license to use Geo replication.')
-    end
-
     unless Gitlab::Database.postgresql_minimum_supported_version?
       flash.now[:warning] = _('Please upgrade PostgreSQL to version 9.6 or greater. The status of the replication cannot be determined reliably with the current version.')
     end

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

@@ -2,7 +2,7 @@
 
 class Admin::Geo::SettingsController < Admin::ApplicationSettingsController
   helper ::EE::GeoHelper
-  before_action :check_license!
+  before_action :check_license!, except: :show
 
   def show
   end
@@ -11,8 +11,7 @@ class Admin::Geo::SettingsController < Admin::ApplicationSettingsController
 
   def check_license!
     unless Gitlab::Geo.license_allows?
-      flash[:alert] = _('You need a different license to use Geo replication.')
-      redirect_to admin_license_path
+      render_403
     end
   end
 end

ee/app/helpers/license_helper.rb

@@ -88,6 +88,10 @@ module LicenseHelper
     @current_license = License.current
   end
 
+  def current_license_title
+    @current_license_title ||= License.current ? License.current.plan.titleize : 'Core'
+  end
+
   def new_trial_url
     return_to_url = CGI.escape(Gitlab.config.gitlab.url)
     uri = URI.parse(::EE::SUBSCRIPTIONS_URL)

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

@@ -2,6 +2,7 @@
 
 = 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
   %h2.page-title

ee/app/views/admin/geo/settings/show.html.haml

 - page_title "Geo Settings"
 - @content_class = "limit-container-width geo-admin-settings" unless fluid_layout
 
+= render partial: 'admin/geo/shared/license_alert'
+
 = render_if_exists 'admin/geo/settings/form'

ee/app/views/admin/geo/shared/_license_alert.haml

+- return if Gitlab::Geo.license_allows?
+.js-vue-alert{ 'v-cloak': true, data: { variant: 'tip',
+  primary_button_text: _('Manage your license'),
+  primary_button_link: admin_license_path,
+  dismissible: 'true' } }
+  = _('This GitLab instance is licensed at the %{insufficient_license} tier. Geo is only available for users who have at least a Premium license.') % { insufficient_license: current_license_title }

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

@@ -3,12 +3,8 @@ require 'spec_helper'
 
 describe Admin::Geo::NodesController do
   shared_examples 'unlicensed geo action' do
-    it 'redirects to the license page' do
-      expect(response).to redirect_to(admin_license_path)
-    end
-
-    it 'displays a flash message' do
-      expect(controller).to set_flash[:alert].to('You need a different license to use Geo replication.')
+    it 'redirects to the 403 page' do
+      expect(response).to have_gitlab_http_status(:forbidden)
     end
   end
 
@@ -42,24 +38,31 @@ describe Admin::Geo::NodesController do
       get :index
     end
 
-    context 'with add-on license available' do
+    context 'with valid license' do
       before do
         allow(Gitlab::Geo).to receive(:license_allows?).and_return(true)
+        go
       end
 
-      it_behaves_like 'no flash message', :alert
+      it 'does not show license alert' do
+        expect(response).to render_template(partial: '_license_alert')
+        expect(response.body).not_to include('Geo is only available for users who have at least a Premium license.')
+      end
     end
 
-    context 'without add-on license available' do
+    context 'without valid license' do
       before do
         allow(Gitlab::Geo).to receive(:license_allows?).and_return(false)
+        go
       end
 
-      it_behaves_like 'with flash message', :alert, 'You need a different license to use Geo replication'
+      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
 
-      it 'does not redirects to the license page' do
-        go
-        expect(response).not_to redirect_to(admin_license_path)
+      it 'does not redirects to the 403 page' do
+        expect(response).not_to redirect_to(:forbidden)
       end
     end
 

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

@@ -14,9 +14,8 @@ describe Admin::Geo::ProjectsController, :geo do
 
   shared_examples 'license required' do
     context 'without a valid license' do
-      it 'redirects to license page with a flash message' do
-        expect(subject).to redirect_to(admin_license_path)
-        expect(flash[:alert]).to include('You need a different license to use Geo replication')
+      it 'redirects to 403 page' do
+        expect(subject).to have_gitlab_http_status(:forbidden)
       end
     end
   end

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

@@ -14,9 +14,8 @@ describe Admin::Geo::SettingsController, :geo do
 
   shared_examples 'license required' do
     context 'without a valid license' do
-      it 'redirects to license page with a flash message' do
-        expect(subject).to redirect_to(admin_license_path)
-        expect(flash[:alert]).to include('You need a different license to use Geo replication')
+      it 'redirects to 403 page' do
+        expect(subject).to have_gitlab_http_status(:forbidden)
       end
     end
   end
@@ -26,20 +25,37 @@ describe Admin::Geo::SettingsController, :geo do
       sign_in(admin)
     end
 
-    subject { get :show }
+    context 'without a valid license' do
+      subject { get :show }
+
+      render_views
 
-    it_behaves_like 'license required'
+      before do
+        stub_licensed_features(geo: false)
+      end
+
+      it 'does not redirects to the 403 page' do
+        expect(subject).not_to redirect_to(:forbidden)
+      end
+
+      it 'does show license alert' do
+        expect(subject).to render_template(partial: '_license_alert')
+        expect(subject.body).to include('Geo is only available for users who have at least a Premium license.')
+      end
+    end
 
     context 'with a valid license' do
+      subject { get :show }
+
       render_views
 
       before do
         stub_licensed_features(geo: true)
       end
 
-      it 'renders the show template' do
-        expect(subject).to have_gitlab_http_status(200)
-        expect(subject).to render_template(:show)
+      it 'does not show license alert' do
+        expect(subject).to render_template(partial: '_license_alert')
+        expect(subject.body).not_to include('Geo is only available for users who have at least a Premium license.')
       end
     end
   end

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

@@ -21,9 +21,8 @@ describe Admin::Geo::UploadsController, :geo do
 
   shared_examples 'license required' do
     context 'without a valid license' do
-      it 'redirects to license page with a flash message' do
-        expect(subject).to redirect_to(admin_license_path)
-        expect(flash[:alert]).to include('You need a different license to use Geo replication')
+      it 'redirects to 403 page' do
+        expect(subject).to have_gitlab_http_status(:forbidden)
       end
     end
   end

ee/spec/features/admin/admin_settings_spec.rb

@@ -13,6 +13,11 @@ describe 'Admin updates EE-only settings' do
 
   context 'Geo settings' do
     context 'when the license has Geo feature' do
+      it 'hides JS alert' do
+        visit admin_geo_settings_path
+        expect(page).not_to have_content("Geo is only available for users who have at least a Premium license.")
+      end
+
       it 'allows users to change Geo settings' do
         visit admin_geo_settings_path
         page.within('section') do
@@ -28,12 +33,12 @@ describe 'Admin updates EE-only settings' do
     end
 
     context 'when the license does not have Geo feature' do
-      it 'shows empty page' do
+      it 'shows JS alert' do
         allow(License).to receive(:feature_available?).and_return(false)
 
         visit admin_geo_settings_path
 
-        expect(page).to have_content 'You need a different license to use Geo replication'
+        expect(page).to have_content("Geo is only available for users who have at least a Premium license.")
       end
     end
   end

locale/gitlab.pot

@@ -11547,6 +11547,9 @@ msgstr ""
 msgid "Manage two-factor authentication"
 msgstr ""
 
+msgid "Manage your license"
+msgstr ""
+
 msgid "Manifest"
 msgstr ""
 
@@ -19226,6 +19229,9 @@ msgstr ""
 msgid "This GitLab instance does not provide any shared Runners yet. Instance administrators can register shared Runners in the admin area."
 msgstr ""
 
+msgid "This GitLab instance is licensed at the %{insufficient_license} tier. Geo is only available for users who have at least a Premium license."
+msgstr ""
+
 msgid "This action can lead to data loss. To prevent accidental actions we ask you to confirm your intention."
 msgstr ""
 
@@ -21889,9 +21895,6 @@ msgstr ""
 msgid "You need a different license to enable FileLocks feature"
 msgstr ""
 
-msgid "You need a different license to use Geo replication."
-msgstr ""
-
 msgid "You need git-lfs version %{min_git_lfs_version} (or greater) to continue. Please visit https://git-lfs.github.com"
 msgstr ""