Made changes to save server name only if it is not blank

Also fixed the check that compares previously saved value against the value from miq_server record instead of checking `@update.config[:server][:name]` server/name key did not exist in settings.yml so the condition always returned true.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1634518

app/controllers/ops_controller/settings/common.rb

@@ -236,6 +236,18 @@ def pglogical_validate_subscription
 
   private
 
+  def update_server_name(server)
+    return if @edit[:new][:server][:name] == server.name # appliance name was modified
+    begin
+      server.name = @edit[:new][:server][:name]
+      server.save!
+    rescue => bang
+      add_flash(_("Error when saving new server name: %{message}") % {:message => bang.message}, :error)
+      javascript_flash
+      return
+    end
+  end
+
   PASSWORD_MASK = '●●●●●●●●'.freeze
 
   def prepare_subscriptions_for_saving
@@ -470,16 +482,7 @@ def settings_update_save
         if ["settings_server", "settings_authentication"].include?(@sb[:active_tab])
           server = MiqServer.find(@sb[:selected_server_id])
           server.set_config(@update)
-          if @update.config[:server][:name] != server.name # appliance name was modified
-            begin
-              server.name = @update.config[:server][:name]
-              server.save!
-            rescue => bang
-              add_flash(_("Error when saving new server name: %{message}") % {:message => bang.message}, :error)
-              javascript_flash
-              return
-            end
-          end
+          update_server_name(server)
         else
           @update.save                                              # Save other settings for current server
         end
@@ -598,6 +601,9 @@ def settings_update_cancel
 
   def settings_server_validate
     return unless @sb[:active_tab] == "settings_server" && @edit[:new][:server]
+    if @edit[:new][:server][:name].blank?
+      add_flash(_("Appliance name must be entered."), :error)
+    end
     if @edit[:new][:server][:custom_support_url].present? && @edit[:new][:server][:custom_support_url_description].blank? ||
        @edit[:new][:server][:custom_support_url].blank? && @edit[:new][:server][:custom_support_url_description].present?
       add_flash(_("Custom Support URL and Description both must be entered."), :error)
@@ -734,9 +740,9 @@ def settings_get_form_vars
       end
       @sb[:roles] = new[:server][:role].split(",")
       params.each do |var, val|
-        if var.starts_with?("server_roles_") && val.to_s == "true"
+        if var.to_s.starts_with?("server_roles_") && val.to_s == "true"
           @sb[:roles].push(var.split("server_roles_").last) unless @sb[:roles].include?(var.split("server_roles_").last)
-        elsif var.starts_with?("server_roles_") && val.downcase == "false"
+        elsif var.to_s.starts_with?("server_roles_") && val.downcase == "false"
           @sb[:roles].delete(var.split("server_roles_").last)
         end
         server_role = @sb[:roles].sort.join(",")

spec/controllers/ops_controller/settings/common_spec.rb

@@ -375,6 +375,39 @@
           expect(SettingsChange.first).to have_attributes(:key => '/api/token_ttl', :value => "1.day")
         end
       end
+
+      context "save server name in server settings" do
+        before do
+          @miq_server = FactoryGirl.create(:miq_server)
+          allow(controller).to receive(:x_node).and_return("svr-#{@miq_server.id}")
+          controller.instance_variable_set(:@sb,
+                                           :active_tab         => 'settings_server',
+                                           :selected_server_id => @miq_server.id)
+          controller.instance_variable_set(:@_params,
+                                           :id => 'server')
+          @current = VMDB::Config.new("vmdb")
+          @new = @current.config
+          @new[:server][:name] = ''
+          controller.instance_variable_set(:@edit,
+                                           :new     => @new,
+                                           :current => @current,
+                                           :key     => "settings_server_edit__#{@miq_server.id}")
+          session[:edit] = assigns(:edit)
+          expect(controller).to receive(:render)
+        end
+
+        it "does not allow to save blank appliance name" do
+          controller.send(:settings_update_save)
+          expect(assigns(:flash_array).first[:message]).to include("Appliance name must be entered.")
+        end
+
+        it "saves new server name for server record" do
+          @new[:server][:name] = 'Foo'
+          controller.send(:settings_update_save)
+          @miq_server.reload
+          expect(@miq_server.name).to eq("Foo")
+        end
+      end
     end
 
     describe '#settings_set_form_vars_server' do