Merge branch '12361-fix-geo-node-name-backward-compatibility' into 'master'

Fix GeoNode#name backward compatibility

Closes #12119 and #12361

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

ee/app/models/geo_node.rb

@@ -126,6 +126,24 @@ class GeoNode < ApplicationRecord
     secondary? && clone_protocol == 'ssh'
   end
 
+  def name
+    value = read_attribute(:name)
+
+    if looks_like_url_field_missing_slash?(value)
+      add_ending_slash(value)
+    else
+      value
+    end
+  end
+
+  def name=(value)
+    if looks_like_url_field_missing_slash?(value)
+      write_with_ending_slash(:name, value)
+    else
+      write_attribute(:name, value)
+    end
+  end
+
   def url
     read_with_ending_slash(:url)
   end
@@ -317,6 +335,14 @@ class GeoNode < ApplicationRecord
     Gitlab::Geo.expire_cache!
   end
 
+  # This method is required for backward compatibility. If it
+  # returns true,  then we can be fairly confident they did not
+  # set gitlab_rails['geo_node_name']. But if it returns false,
+  # then we aren't sure, so we shouldn't touch the name value.
+  def looks_like_url_field_missing_slash?(value)
+    add_ending_slash(value) == url
+  end
+
   def read_with_ending_slash(attribute)
     value = read_attribute(attribute)
 

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

@@ -3,7 +3,7 @@
   .form-group.col-sm-6
     = form.label :name, _('Name'), class: 'font-weight-bold'
     = form.text_field :name, class: 'form-control qa-node-name-field'
-    .form-text.text-muted= _('The unique identifier for the Geo node. Must match `geo_node_name` if it is set in gitlab.rb, otherwise it must match `external_url`')
+    .form-text.text-muted= _('The unique identifier for the Geo node. Must match `geo_node_name` if it is set in gitlab.rb, otherwise it must match `external_url` with a trailing slash')
 
   .form-group.col-sm-6
     = form.label :url, s_('Geo|URL'), class: 'font-weight-bold'

ee/changelogs/unreleased/12361-fix-geo-node-name-backward-compatibility.yml

+---
+title: Fix GeoNode#name backward compatibility
+merge_request: 14564
+author:
+type: fixed

ee/spec/models/geo_node_spec.rb

@@ -379,6 +379,46 @@ describe GeoNode, :geo, type: :model do
     end
   end
 
+  describe '#name' do
+    it 'adds a trailing forward slash when name looks like url field missing slash' do
+      subject = build(:geo_node, url: 'https://foo.com', name: 'https://foo.com')
+
+      expect(subject.name).to eq('https://foo.com/')
+    end
+
+    it 'does not add a trailing forward slash when name does not looks like url field' do
+      subject = build(:geo_node, url: 'https://foo.com', name: 'https://bar.com')
+
+      expect(subject.name).to eq('https://bar.com')
+    end
+
+    it 'does not add a trailing forward slash when name is nil' do
+      subject = build(:geo_node, name: nil)
+
+      expect(subject.name).to be_nil
+    end
+
+    it 'does not add a trailing forward slash when name is an empty string' do
+      subject = build(:geo_node, name: '')
+
+      expect(subject.name).to be_empty
+    end
+  end
+
+  describe '#name=' do
+    it 'adds a trailing forward slash when name looks like url field missing slash' do
+      subject = create(:geo_node, url: 'https://foo.com', name: 'https://foo.com')
+
+      expect(subject.read_attribute(:name)).to eq('https://foo.com/')
+    end
+
+    it 'does not add a trailing forward slash when name does not looks like url field' do
+      subject = create(:geo_node, url: 'https://foo.com', name: 'https://bar.com')
+
+      expect(subject.read_attribute(:name)).to eq('https://bar.com')
+    end
+  end
+
   describe '#url' do
     it 'returns a string' do
       expect(new_node.url).to be_a String

locale/gitlab.pot

@@ -13905,7 +13905,7 @@ msgstr ""
 msgid "The time taken by each data entry gathered by that stage."
 msgstr ""
 
-msgid "The unique identifier for the Geo node. Must match `geo_node_name` if it is set in gitlab.rb, otherwise it must match `external_url`"
+msgid "The unique identifier for the Geo node. Must match `geo_node_name` if it is set in gitlab.rb, otherwise it must match `external_url` with a trailing slash"
 msgstr ""
 
 msgid "The update action will time out after %{number_of_minutes} minutes. For big repositories, use a clone/push combination."