Merge pull request #64 from 3scale/fix-create-service

include new available attributes on copy/update service

lib/3scale_toolbox/commands.rb

@@ -12,5 +12,13 @@ module Commands
       ThreeScaleToolbox::Commands::ImportCommand,
       ThreeScaleToolbox::Commands::UpdateCommand
     ].freeze
+
+    def self.service_valid_params
+      %w[
+        name backend_version deployment_option description
+        system_name end_user_registration_required
+        support_email tech_support_email admin_support_email
+      ]
+    end
   end
 end

lib/3scale_toolbox/commands/copy_command/copy_service.rb

@@ -54,13 +54,19 @@ def self.endpoint_from_url(url)
           url.sub /\w*@/, ''
         end
 
+
+        # Returns new hash object with not nil valid params
+        def self.filter_params(valid_params, source)
+          valid_params.each_with_object({}) do |key, target|
+            target[key] = source[key] unless source[key].nil?
+          end
+        end
+
         def self.copy_service_params(original, system_name)
-          {
-            name:        original['name'],
-            system_name: system_name,
-            backend_version: original['backend_version'],
-            end_user_registration_required: original['end_user_registration_required']
-          }.reject { |key, value| !value }
+          service_params = filter_params(Commands.service_valid_params, original)
+          service_params.tap do |hash|
+            hash['system_name'] = system_name if system_name
+          end
         end
 
         def self.copy_service(service_id, source, destination, system_name, insecure)

lib/3scale_toolbox/commands/update_command/update_service.rb

@@ -67,9 +67,7 @@ def endpoint_from_url(url)
           end
 
           def target_service_params(source)
-            # NOTE: backend_version and deployment_option are not yet returned by show_service method
-            params = %w(name backend_version deployment_option end_user_registration_required)
-            source.select { |k,v| params.include?(k) && v }
+            source.select { |k, v| Commands.service_valid_params.include?(k) && v }
           end
 
           def source_metrics

spec/shared_contexts.rb

@@ -8,6 +8,24 @@ def random_lowercase_name
   end
 end
 
+RSpec.shared_context :source_service_data do
+  include_context :random_name
+
+  let(:source_service_params) do
+    %w[
+      name backend_version deployment_option description
+      system_name end_user_registration_required
+      support_email tech_support_email admin_support_email
+    ]
+  end
+
+  let(:source_service_obj) do
+    source_service_params.each_with_object({}) do |key, target|
+      target[key] = random_lowercase_name
+    end
+  end
+end
+
 RSpec.shared_context :temp_dir do
   around(:each) do |example|
     Dir.mktmpdir('3scale_toolbox_rspec-') do |dir|

spec/unit/copy_service_spec.rb

@@ -1,6 +1,8 @@
 require '3scale_toolbox/cli'
 
 RSpec.describe ThreeScaleToolbox::Commands::CopyCommand::CopyServiceSubcommand do
+  include_context :source_service_data
+
   context '#run' do
     it 'with insecure flag' do
       expect(described_class).to receive(:copy_service).with('service_id',
@@ -31,4 +33,34 @@
       described_class.run(opts, ['service_id'])
     end
   end
+
+  context '#copy_service_params' do
+    it 'all expected params are copied' do
+      new_service_params = described_class.copy_service_params(source_service_obj, nil)
+
+      expect(new_service_params).to include(*source_service_params)
+    end
+
+    it 'extra params are not copied' do
+      extra_params = {
+        'some_weird_param' => 'value0',
+        'some_other_weird_param' => 'value1'
+      }
+      new_service_params = described_class.copy_service_params(
+        source_service_obj.merge(extra_params), nil
+      )
+      expect(new_service_params).to include(*source_service_params)
+      expect(new_service_params).not_to include(*extra_params)
+    end
+
+    it 'missing params are not copied' do
+      missing_params = %w[description backend_version]
+      missing_params.each do |key|
+        source_service_obj.delete(key)
+      end
+      new_service_params = described_class.copy_service_params(source_service_obj, nil)
+      expect(new_service_params).to include(*source_service_obj.keys)
+      expect(new_service_params).not_to include(*missing_params)
+    end
+  end
 end

spec/unit/update_service_spec.rb

@@ -36,3 +36,44 @@
     end
   end
 end
+
+RSpec.describe ThreeScaleToolbox::Commands::UpdateCommand::UpdateServiceSubcommand::ServiceUpdater do
+  include_context :source_service_data
+
+  subject do
+    described_class.new(
+      'https://[email protected]',
+      'source_service_id',
+      'https://[email protected]',
+      'destination_service_id',
+      true
+    )
+  end
+
+  context '#target_service_params' do
+    it 'all expected params are copied' do
+      target_service_obj = subject.target_service_params(source_service_obj)
+      expect(target_service_obj).to include(*source_service_params)
+    end
+    it 'extra params are not copied' do
+      extra_params = {
+        'some_weird_param' => 'value0',
+        'some_other_weird_param' => 'value1'
+      }
+      target_service_obj = subject.target_service_params(
+        source_service_obj.merge(extra_params)
+      )
+      expect(target_service_obj).to include(*source_service_params)
+      expect(target_service_obj).not_to include(*extra_params)
+    end
+    it 'missing params are not copied' do
+      missing_params = %w[description backend_version]
+      missing_params.each do |key|
+        source_service_obj.delete(key)
+      end
+      target_service_obj = subject.target_service_params(source_service_obj)
+      expect(target_service_obj).to include(*source_service_obj.keys)
+      expect(target_service_obj).not_to include(*missing_params)
+    end
+  end
+end