Address review comments with GeoNode edit support

app/controllers/admin/geo_nodes_controller.rb

 class Admin::GeoNodesController < Admin::ApplicationController
   before_action :check_license, except: [:index, :destroy]
-  before_action :load_node, only: [:destroy, :edit, :update, :repair, :toggle, :status]
+  before_action :load_node, only: [:edit, :update, :destroy, :repair, :toggle, :status]
 
   def index
     @nodes = GeoNode.all.order(:id)
@@ -24,8 +24,7 @@ class Admin::GeoNodesController < Admin::ApplicationController
 
   def update
     if @node.update_attributes(geo_node_params.except(:geo_node_key_attributes))
-      flash[:notice] = 'GeoNode was successfully updated.'
-      redirect_to admin_geo_nodes_path
+      redirect_to admin_geo_nodes_path, notice: 'Geo Node was successfully updated.'
     else
       render 'edit'
     end

app/views/admin/geo_nodes/_form.html.haml

-- disable_key_edit = defined?(edit) && edit
+- disable_key_edit = local_assigns.fetch(:disable_key_edit, false)
 
-- if geo_node.errors.any?
-  .alert.alert-danger
-    - geo_node.errors.full_messages.each do |msg|
-      %p= msg
+= form_errors(geo_node)
 .form-group
   .col-sm-offset-2.col-sm-10
     .checkbox

app/views/admin/geo_nodes/edit.html.haml

@@ -3,7 +3,7 @@
   Edit Geo Node
 
 = form_for [:admin, @node], html: { class: 'form-horizontal' } do |f|
-  = render partial: 'form', locals: { form: f, geo_node: @node, edit: true }
+  = render partial: 'form', locals: { form: f, geo_node: @node, disable_key_edit: true }
   .form-actions
     = f.submit 'Save changes', class: 'btn btn-create'
     = link_to 'Cancel', admin_geo_nodes_path, class: 'btn btn-cancel'

app/views/admin/geo_nodes/index.html.haml

@@ -10,7 +10,7 @@
 
 - if Gitlab::Geo.license_allows?
   = form_for [:admin, @node], as: :geo_node, url: admin_geo_nodes_path, html: { class: 'form-horizontal' } do |f|
-    = render partial: 'form', locals: { form: f, geo_node: @node, add_key: true }
+    = render partial: 'form', locals: { form: f, geo_node: @node }
     .form-actions
       = f.submit 'Add Node', class: 'btn btn-create'
 

spec/controllers/admin/geo_nodes_controller_spec.rb

@@ -20,7 +20,10 @@ describe Admin::GeoNodesController, :postgresql do
 
   describe '#index' do
     render_views
-    subject { get :index }
+
+    def go
+      get :index
+    end
 
     context 'with add-on license available' do
       before do
@@ -28,7 +31,7 @@ describe Admin::GeoNodesController, :postgresql do
       end
 
       it 'renders creation form' do
-        expect(subject).to render_template(partial: 'admin/geo_nodes/_form')
+        expect(go).to render_template(partial: 'admin/geo_nodes/_form')
       end
     end
 
@@ -38,16 +41,16 @@ describe Admin::GeoNodesController, :postgresql do
       end
 
       it 'does not render the creation form' do
-        expect(subject).not_to render_template(partial: 'admin/geo_nodes/_form')
+        expect(go).not_to render_template(partial: 'admin/geo_nodes/_form')
       end
 
       it 'displays a flash message' do
-        subject
+        go
         expect(controller).to set_flash.now[:alert].to('You need a different license to enable Geo replication')
       end
 
       it 'does not redirects to the license page' do
-        subject
+        go
         expect(response).not_to redirect_to(admin_license_path)
       end
     end
@@ -55,7 +58,8 @@ describe Admin::GeoNodesController, :postgresql do
 
   describe '#destroy' do
     let!(:geo_node) { create(:geo_node) }
-    subject do
+
+    def go
       delete(:destroy, id: geo_node)
     end
 
@@ -65,7 +69,7 @@ describe Admin::GeoNodesController, :postgresql do
       end
 
       it 'deletes the node' do
-        expect { subject }.to change { GeoNode.count }.by(-1)
+        expect { go }.to change { GeoNode.count }.by(-1)
       end
     end
 
@@ -75,19 +79,22 @@ describe Admin::GeoNodesController, :postgresql do
       end
 
       it 'deletes the node' do
-        expect { subject }.to change { GeoNode.count }.by(-1)
+        expect { go }.to change { GeoNode.count }.by(-1)
       end
     end
   end
 
   describe '#create' do
     let(:geo_node_attributes) { { url: 'http://example.com', geo_node_key_attributes: { key: SSHKeygen.generate } } }
-    subject { post :create, geo_node: geo_node_attributes }
+
+    def go
+      post :create, geo_node: geo_node_attributes
+    end
 
     context 'without add-on license' do
       before do
         allow(Gitlab::Geo).to receive(:license_allows?) { false }
-        subject
+        go
       end
 
       it_behaves_like 'unlicensed geo action'
@@ -99,21 +106,24 @@ describe Admin::GeoNodesController, :postgresql do
       end
 
       it 'creates the node' do
-        expect { subject }.to change { GeoNode.count }.by(1)
+        expect { go }.to change { GeoNode.count }.by(1)
       end
     end
   end
 
   describe '#update' do
-    let(:geo_node_attributes) { { url: 'http://example.com', geo_node_key_attributes: { key: SSHKeygen.generate } } }
+    let(:geo_node_attributes) { { url: 'http://example.com', geo_node_key_attributes: attributes_for(:key) } }
     let(:geo_node) { create(:geo_node) }
+    let!(:original_fingerprint) { geo_node.geo_node_key.fingerprint }
 
-    subject { post :update, id: geo_node, geo_node: geo_node_attributes }
+    def go
+      post :update, id: geo_node, geo_node: geo_node_attributes
+    end
 
     context 'without add-on license' do
       before do
         allow(Gitlab::Geo).to receive(:license_allows?) { false }
-        subject
+        go
       end
 
       it_behaves_like 'unlicensed geo action'
@@ -122,11 +132,10 @@ describe Admin::GeoNodesController, :postgresql do
     context 'with add-on license' do
       before do
         allow(Gitlab::Geo).to receive(:license_allows?).and_return(true)
-        subject
+        go
       end
 
       it 'updates the node without changing the key' do
-        original_fingerprint = geo_node.geo_node_key.fingerprint
         geo_node.reload
 
         expect(geo_node.url.chomp('/')).to eq(geo_node_attributes[:url])
@@ -137,11 +146,13 @@ describe Admin::GeoNodesController, :postgresql do
 
   describe '#repair' do
     let(:geo_node) { create(:geo_node) }
-    subject { post :repair, id: geo_node }
+    def go
+      post :repair, id: geo_node
+    end
 
     before do
       allow(Gitlab::Geo).to receive(:license_allows?) { false }
-      subject
+      go
     end
 
     it_behaves_like 'unlicensed geo action'