Merge branch '238993-update-geo-banner' into 'master'

Geo Secondary - Update Alert

Closes #238993

See merge request gitlab-org/gitlab!40297

ee/app/helpers/ee/application_helper.rb

@@ -13,17 +13,21 @@ module EE
     def read_only_message
       return super unless ::Gitlab::Geo.secondary?
 
-      if @limited_actions_message
-        html_escape(s_('Geo|You are on a secondary, %{b_open}read-only%{b_close} Geo node. You may be able to make a limited amount of changes or perform a limited amount of actions on this page.')) %
-          { b_open: '<b>'.html_safe, b_close: '</b>'.html_safe }
-      else
-        message = html_escape(s_('Geo|You are on a secondary, %{b_open}read-only%{b_close} Geo node. If you want to make changes, you must visit this page on the %{node_link_open}primary node%{node_link_close}.')) %
-          { node_link_open: "<a href=\"#{::Gitlab::Geo.primary_node&.url || '#'}\">".html_safe, node_link_close: "</a>".html_safe, b_open: '<b>'.html_safe, b_close: '</b>'.html_safe }
+      message = @limited_actions_message ? s_('Geo|You may be able to make a limited amount of changes or perform a limited amount of actions on this page.') : s_('Geo|If you want to make changes, you must visit the primary site.')
 
-        return "#{message} #{lag_message}".html_safe if lag_message
+      message = "#{message} #{lag_message}".html_safe if lag_message
 
-        message
+      html = tag.div do
+        tag.p(class: 'gl-mb-3') do
+          concat(sprite_icon('information-o', css_class: 'gl-icon gl-mr-3'))
+          concat(s_('Geo|You are on a secondary, %{b_open}read-only%{b_close} Geo node.').html_safe % { b_open: '<b>'.html_safe, b_close: '</b>'.html_safe })
+          concat(" #{message}")
+        end
       end
+
+      html.concat(tag.a(s_('Geo|Go to the primary site'), class: 'btn', href: ::Gitlab::Geo.primary_node.url, target: '_blank')) if ::Gitlab::Geo.primary_node.present?
+
+      html
     end
 
     def lag_message

ee/changelogs/unreleased/238993-update-geo-banner.yml

+---
+title: Add link to primary node in secondary node alert
+merge_request: 40297
+author:
+type: changed

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

@@ -6,6 +6,7 @@ RSpec.describe Admin::Geo::ProjectsController, :geo do
   include EE::GeoHelpers
 
   let_it_be(:admin) { create(:admin) }
+  let_it_be(:geo_primary) { create(:geo_node, :primary) }
   let(:synced_registry) { create(:geo_project_registry, :synced) }
 
   before do
@@ -35,6 +36,7 @@ RSpec.describe Admin::Geo::ProjectsController, :geo do
 
       it 'displays a different read-only message based on skip_readonly_message' do
         expect(subject.body).to match('You may be able to make a limited amount of changes or perform a limited amount of actions on this page')
+        expect(subject.body).to include(geo_primary.url)
       end
 
       context 'without sync_status specified' do

ee/spec/controllers/admin/projects_controller_spec.rb

@@ -5,6 +5,7 @@ require 'spec_helper'
 RSpec.describe Admin::ProjectsController, :geo do
   include EE::GeoHelpers
 
+  let_it_be(:geo_primary) { create(:geo_node, :primary) }
   let!(:project_registry) { create(:geo_project_registry) }
   let(:project) { project_registry.project }
 
@@ -34,6 +35,7 @@ RSpec.describe Admin::ProjectsController, :geo do
 
       it 'displays a different read-only message based on skip_readonly_message' do
         expect(subject.body).to match('You may be able to make a limited amount of changes or perform a limited amount of actions on this page')
+        expect(subject.body).to include(geo_primary.url)
       end
     end
 

ee/spec/features/geo_node_spec.rb

@@ -22,12 +22,14 @@ RSpec.describe 'GEO Nodes', :geo do
     describe "showing Flash Info Message" do
       it 'on dashboard' do
         visit root_dashboard_path
-        expect(page).to have_content 'You are on a secondary, read-only Geo node. If you want to make changes, you must visit this page on the primary node.'
+        expect(page).to have_content 'You are on a secondary, read-only Geo node. If you want to make changes, you must visit the primary site.'
+        expect(page).to have_content 'Go to the primary site'
       end
 
       it 'on project overview' do
         visit project_path(project)
-        expect(page).to have_content 'You are on a secondary, read-only Geo node. If you want to make changes, you must visit this page on the primary node.'
+        expect(page).to have_content 'You are on a secondary, read-only Geo node. If you want to make changes, you must visit the primary site.'
+        expect(page).to have_content 'Go to the primary site'
       end
     end
   end

ee/spec/features/read_only_spec.rb

@@ -18,6 +18,7 @@ RSpec.describe 'Geo read-only message', :geo do
 
     visit root_dashboard_path
 
-    expect(page).to have_content('You are on a secondary, read-only Geo node. If you want to make changes, you must visit this page on the primary node.')
+    expect(page).to have_content('You are on a secondary, read-only Geo node. If you want to make changes, you must visit the primary site.')
+    expect(page).to have_content('Go to the primary site')
   end
 end

ee/spec/helpers/application_helper_spec.rb

@@ -19,75 +19,71 @@ RSpec.describe ApplicationHelper do
     end
 
     context 'when in a Geo Secondary' do
+      let_it_be(:geo_primary) { create(:geo_node, :primary) }
+
       before do
         stub_current_geo_node(create(:geo_node))
       end
 
-      context 'when there is no Geo Primary node configured' do
-        it 'returns a read-only Geo message without a link to a primary node' do
-          expect(helper.read_only_message).to match(/If you want to make changes, you must visit this page on the .*primary node/)
-          expect(helper.read_only_message).not_to include('http://')
-        end
+      it 'includes button to visit primary node' do
+        expect(helper.read_only_message).to match(/Go to the primary site/)
+        expect(helper.read_only_message).to include(geo_primary.url)
+      end
+
+      it 'returns a read-only Geo message with a link to primary node' do
+        expect(helper.read_only_message).to match(/If you want to make changes, you must visit the primary site./)
+        expect(helper.read_only_message).to include(geo_primary.url)
+      end
+
+      it 'returns a limited actions message when @limited_actions_message is true' do
+        assign(:limited_actions_message, true)
+
+        expect(helper.read_only_message).to match(/You may be able to make a limited amount of changes or perform a limited amount of actions on this page/)
+        expect(helper.read_only_message).to include(geo_primary.url)
+      end
+
+      it 'includes a warning about database lag' do
+        allow_any_instance_of(::Gitlab::Geo::HealthCheck).to receive(:db_replication_lag_seconds).and_return(120)
+
+        expect(helper.read_only_message).to match(/If you want to make changes, you must visit the primary site./)
+        expect(helper.read_only_message).to match(/The database is currently 2 minutes behind the primary node/)
+        expect(helper.read_only_message).to include(geo_primary.url)
       end
 
-      context 'when there is a Geo Primary node configured' do
-        let!(:geo_primary) { create(:geo_node, :primary) }
+      context 'event lag' do
+        it 'includes a lag warning about a node lag' do
+          event_log = create(:geo_event_log, created_at: 4.minutes.ago)
+          create(:geo_event_log, created_at: 3.minutes.ago)
+          create(:geo_event_log_state, event_id: event_log.id)
 
-        it 'returns a read-only Geo message with a link to primary node' do
-          expect(helper.read_only_message).to match(/If you want to make changes, you must visit this page on the .*primary node/)
+          expect(helper.read_only_message).to match(/If you want to make changes, you must visit the primary site./)
+          expect(helper.read_only_message).to match(/The node is currently 3 minutes behind the primary/)
           expect(helper.read_only_message).to include(geo_primary.url)
         end
 
-        it 'returns a limited actions message when @limited_actions_message is true' do
-          assign(:limited_actions_message, true)
+        it 'does not include a lag warning because the last event is too fresh' do
+          event_log = create(:geo_event_log, created_at: 3.minutes.ago)
+          create(:geo_event_log)
+          create(:geo_event_log_state, event_id: event_log.id)
 
-          expect(helper.read_only_message).to match(/You may be able to make a limited amount of changes or perform a limited amount of actions on this page/)
-          expect(helper.read_only_message).not_to include('http://')
+          expect(helper.read_only_message).to match(/If you want to make changes, you must visit the primary site./)
+          expect(helper.read_only_message).not_to match(/The node is currently 3 minutes behind the primary/)
+          expect(helper.read_only_message).to include(geo_primary.url)
         end
 
-        it 'includes a warning about database lag' do
-          allow_any_instance_of(::Gitlab::Geo::HealthCheck).to receive(:db_replication_lag_seconds).and_return(120)
+        it 'does not include a lag warning because the last event is processed' do
+          event_log = create(:geo_event_log, created_at: 3.minutes.ago)
+          create(:geo_event_log_state, event_id: event_log.id)
 
-          expect(helper.read_only_message).to match(/If you want to make changes, you must visit this page on the .*primary node/)
-          expect(helper.read_only_message).to match(/The database is currently 2 minutes behind the primary node/)
+          expect(helper.read_only_message).to match(/If you want to make changes, you must visit the primary site./)
+          expect(helper.read_only_message).not_to match(/The node is currently 3 minutes behind the primary/)
           expect(helper.read_only_message).to include(geo_primary.url)
         end
 
-        context 'event lag' do
-          it 'includes a lag warning about a node lag' do
-            event_log = create(:geo_event_log, created_at: 4.minutes.ago)
-            create(:geo_event_log, created_at: 3.minutes.ago)
-            create(:geo_event_log_state, event_id: event_log.id)
-
-            expect(helper.read_only_message).to match(/If you want to make changes, you must visit this page on the .*primary node/)
-            expect(helper.read_only_message).to match(/The node is currently 3 minutes behind the primary/)
-            expect(helper.read_only_message).to include(geo_primary.url)
-          end
-
-          it 'does not include a lag warning because the last event is too fresh' do
-            event_log = create(:geo_event_log, created_at: 3.minutes.ago)
-            create(:geo_event_log)
-            create(:geo_event_log_state, event_id: event_log.id)
-
-            expect(helper.read_only_message).to match(/If you want to make changes, you must visit this page on the .*primary node/)
-            expect(helper.read_only_message).not_to match(/The node is currently 3 minutes behind the primary/)
-            expect(helper.read_only_message).to include(geo_primary.url)
-          end
-
-          it 'does not include a lag warning because the last event is processed' do
-            event_log = create(:geo_event_log, created_at: 3.minutes.ago)
-            create(:geo_event_log_state, event_id: event_log.id)
-
-            expect(helper.read_only_message).to match(/If you want to make changes, you must visit this page on the .*primary node/)
-            expect(helper.read_only_message).not_to match(/The node is currently 3 minutes behind the primary/)
-            expect(helper.read_only_message).to include(geo_primary.url)
-          end
-
-          it 'does not include a lag warning because there are no events yet' do
-            expect(helper.read_only_message).to match(/If you want to make changes, you must visit this page on the .*primary node/)
-            expect(helper.read_only_message).not_to match(/minutes behind the primary/)
-            expect(helper.read_only_message).to include(geo_primary.url)
-          end
+        it 'does not include a lag warning because there are no events yet' do
+          expect(helper.read_only_message).to match(/If you want to make changes, you must visit the primary site./)
+          expect(helper.read_only_message).not_to match(/minutes behind the primary/)
+          expect(helper.read_only_message).to include(geo_primary.url)
         end
       end
     end

locale/gitlab.pot

@@ -11305,6 +11305,12 @@ msgstr ""
 msgid "Geo|Geo Status"
 msgstr ""
 
+msgid "Geo|Go to the primary site"
+msgstr ""
+
+msgid "Geo|If you want to make changes, you must visit the primary site."
+msgstr ""
+
 msgid "Geo|In progress"
 msgstr ""
 
@@ -11437,10 +11443,10 @@ msgstr ""
 msgid "Geo|Waiting for scheduler"
 msgstr ""
 
-msgid "Geo|You are on a secondary, %{b_open}read-only%{b_close} Geo node. If you want to make changes, you must visit this page on the %{node_link_open}primary node%{node_link_close}."
+msgid "Geo|You are on a secondary, %{b_open}read-only%{b_close} Geo node."
 msgstr ""
 
-msgid "Geo|You are on a secondary, %{b_open}read-only%{b_close} Geo node. You may be able to make a limited amount of changes or perform a limited amount of actions on this page."
+msgid "Geo|You may be able to make a limited amount of changes or perform a limited amount of actions on this page."
 msgstr ""
 
 msgid "Geo|misconfigured"