Merge branch 'sh-show-geo-node-creation-errors' into 'master'

Properly report errors when GeoNode fails to create

Closes #3948

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

app/services/geo/node_create_service.rb

@@ -8,7 +8,7 @@ module Geo
     end
 
     def execute
-      GeoNode.create(params).persisted?
+      GeoNode.create(params)
     end
   end
 end

ee/app/controllers/admin/geo_nodes_controller.rb

@@ -14,12 +14,12 @@ class Admin::GeoNodesController < Admin::ApplicationController
   end
 
   def create
-    if Geo::NodeCreateService.new(geo_node_params).execute
+    @node = Geo::NodeCreateService.new(geo_node_params).execute
+
+    if @node.persisted?
       redirect_to admin_geo_nodes_path, notice: 'Node was successfully created.'
     else
       @nodes = GeoNode.all
-      @node = GeoNode.new(geo_node_params)
-      flash.now[:alert] = 'Failed to create new node'
 
       render :index
     end

spec/features/admin/admin_geo_nodes_spec.rb

@@ -26,7 +26,6 @@ RSpec.describe 'admin Geo Nodes', type: :feature do
     it 'creates a new Geo Node' do
       check 'This is a primary node'
       fill_in 'geo_node_url', with: 'https://test.gitlab.com'
-      fill_in 'geo_node_geo_node_key_attributes_key', with: new_ssh_key
       click_button 'Add Node'
 
       expect(current_path).to eq admin_geo_nodes_path
@@ -35,6 +34,20 @@ RSpec.describe 'admin Geo Nodes', type: :feature do
         expect(page).to have_content(geo_node.url)
       end
     end
+
+    it 'returns an error message when a duplicate primary is added' do
+      check 'This is a primary node'
+      fill_in 'geo_node_url', with: 'https://test.example.com'
+      click_button 'Add Node'
+
+      check 'This is a primary node'
+      fill_in 'geo_node_url', with: 'https://secondary.example.com'
+      click_button 'Add Node'
+
+      expect(current_path).to eq admin_geo_nodes_path
+
+      expect(page).to have_content('Primary node already exists')
+    end
   end
 
   describe 'update an existing Geo Node' do

spec/services/geo/node_create_service_spec.rb

@@ -17,13 +17,13 @@ describe Geo::NodeCreateService do
     it 'returns true when creation succeeds' do
       service = described_class.new(url: 'http://example.com')
 
-      expect(service.execute).to eq true
+      expect(service.execute.persisted?).to eq true
     end
 
     it 'returns false when creation fails' do
       service = described_class.new(url: 'ftp://example.com')
 
-      expect(service.execute).to eq false
+      expect(service.execute.persisted?).to eq false
     end
 
     it 'parses the namespace_ids when node have namespace restrictions' do