From e00b17bc75d538decbff0ba58d3cb9fbb714f89f Mon Sep 17 00:00:00 2001
From: Greg McCullough <gmccullo@redhat.com>
Date: Sun, 17 Dec 2017 13:47:22 -0500
Subject: [PATCH] Move apply_dialog_settings logic into DialogProperties class

https://bugzilla.redhat.com/show_bug.cgi?id=1498600
---
 app/models/service.rb                         | 20 ++--------
 app/models/service/dialog_properties.rb       | 40 +++++++++++++++++++
 spec/models/service/dialog_properties_spec.rb | 35 ++++++++++++++++
 3 files changed, 78 insertions(+), 17 deletions(-)
 create mode 100644 app/models/service/dialog_properties.rb
 create mode 100644 spec/models/service/dialog_properties_spec.rb

diff --git a/app/models/service.rb b/app/models/service.rb
index 8c72c7d59b16..a5a1364c8509 100644
--- a/app/models/service.rb
+++ b/app/models/service.rb
@@ -53,7 +53,7 @@ class Service < ApplicationRecord
   virtual_has_one    :reconfigure_dialog
 
   before_validation :set_tenant_from_group
-  before_create     :apply_dialog_settings
+  before_create :update_attributes_from_dialog
 
   delegate :provision_dialog, :to => :miq_request, :allow_nil => true
   delegate :user, :to => :miq_request, :allow_nil => true
@@ -463,21 +463,7 @@ def remove_from_service(parent_service)
   def configuration_script
   end
 
-  private
-
-  def apply_dialog_settings
-    dialog_options = options[:dialog] || {}
-
-    %w(dialog_service_name dialog_service_description).each do |field_name|
-      send(field_name, dialog_options[field_name]) if dialog_options.key?(field_name)
-    end
-  end
-
-  def dialog_service_name(value)
-    self.name = value if value.present?
-  end
-
-  def dialog_service_description(value)
-    self.description = value if value.present?
+  private def update_attributes_from_dialog
+    Service::DialogProperties.parse(options[:dialog]).each { |key, value| self[key] = value }
   end
 end
diff --git a/app/models/service/dialog_properties.rb b/app/models/service/dialog_properties.rb
new file mode 100644
index 000000000000..239c000de4d3
--- /dev/null
+++ b/app/models/service/dialog_properties.rb
@@ -0,0 +1,40 @@
+class Service
+  class DialogProperties
+    def initialize(options)
+      @attributes = {}
+      @options = options || {}
+    end
+
+    def self.parse(options)
+      new(options).parse
+    end
+
+    def parse
+      @attributes.tap { parse_options }
+    end
+
+    private
+
+    def parse_options
+      %w(name description).each do |field_name|
+        dialog_field_name = "dialog_service_#{field_name}"
+        dialog_field_value = @options[dialog_field_name]
+        parse_value(field_name, dialog_field_name, dialog_field_value) if @options.key?(dialog_field_name)
+      end
+    end
+
+    def parse_value(field_name, dialog_field_name, dialog_field_value)
+      send(field_name, dialog_field_value) if @options.key?(dialog_field_name)
+    rescue StandardError => err
+      $log.error("Error setting key [#{dialog_field_name}] with value [#{dialog_field_value.inspect}] error: [#{err}]")
+    end
+
+    def name(value)
+      @attributes[:name] = value if value.present?
+    end
+
+    def description(value)
+      @attributes[:description] = value if value.present?
+    end
+  end
+end
diff --git a/spec/models/service/dialog_properties_spec.rb b/spec/models/service/dialog_properties_spec.rb
new file mode 100644
index 000000000000..7225d84e20c0
--- /dev/null
+++ b/spec/models/service/dialog_properties_spec.rb
@@ -0,0 +1,35 @@
+describe Service::DialogProperties do
+  it 'with a nil parameter' do
+    options = nil
+    expect(described_class.parse(options)).to eq({})
+  end
+
+  it 'with an empty hash' do
+    options = {}
+    expect(described_class.parse(options)).to eq({})
+  end
+
+  context 'name' do
+    it 'with an empty name' do
+      options = {'dialog_service_name' => ' '}
+      expect(described_class.parse(options)).to eq({})
+    end
+
+    it 'with option name' do
+      options = {'dialog_service_name' => 'name from dialog'}
+      expect(described_class.parse(options)).to eq(:name => 'name from dialog')
+    end
+  end
+
+  context 'description' do
+    it 'with an empty description' do
+      options = {'dialog_service_description' => ' '}
+      expect(described_class.parse(options)).to eq({})
+    end
+
+    it 'with option description' do
+      options = {'dialog_service_description' => 'test description from dialog'}
+      expect(described_class.parse(options)).to eq(:description => 'test description from dialog')
+    end
+  end
+end