Fixes #34177 - Domain update doesn't reset DNS implicitly

lib/hammer_cli_foreman/command_extensions.rb

@@ -6,3 +6,4 @@
 require 'hammer_cli_foreman/command_extensions/status'
 require 'hammer_cli_foreman/command_extensions/user'
 require 'hammer_cli_foreman/command_extensions/subnet'
+require 'hammer_cli_foreman/command_extensions/domain'

lib/hammer_cli_foreman/command_extensions/domain.rb

@@ -0,0 +1,20 @@
+module HammerCLIForeman
+  module CommandExtensions
+    class Domain < HammerCLI::CommandExtensions
+      option_sources do |sources, command|
+        sources.find_by_name('IdResolution').insert_relative(
+          :after,
+          'IdParams',
+          HammerCLIForeman::OptionSources::ReferencedResourceIdParams.new(command)
+        )
+        sources
+      end
+
+      option_family associate: 'dns' do
+        child '--dns', 'DNS_NAME', _('Name of DNS proxy to use within this domain'),
+              attribute_name: :option_dns_name,
+              referenced_resource: :smart_proxy
+      end
+    end
+  end
+end

lib/hammer_cli_foreman/domain.rb

@@ -1,28 +1,5 @@
 module HammerCLIForeman
-
-  module DomainUpdateCreateCommons
-
-    def self.included(base)
-      base.option "--dns-id", "DNS_ID",  _("ID of DNS proxy to use within this domain")
-      base.option "--dns", "DNS_NAME", _("Name of DNS proxy to use within this domain")
-    end
-
-    def request_params
-      params = super
-      params['domain']["dns_id"] = option_dns_id || dns_id(option_dns)
-      params
-    end
-
-    private
-
-    def dns_id(name)
-      resolver.smart_proxy_id('option_name' => name) if name
-    end
-
-  end
-
   class Domain < HammerCLIForeman::Command
-
     resource :domains
 
     class ListCommand < HammerCLIForeman::ListCommand
@@ -52,24 +29,24 @@ class InfoCommand < HammerCLIForeman::InfoCommand
 
 
     class CreateCommand < HammerCLIForeman::CreateCommand
-      include DomainUpdateCreateCommons
-
       success_message _("Domain [%{name}] created.")
       failure_message _("Could not create the domain")
 
       option "--description", "DESC", _("Full name describing the domain"), :attribute_name => :option_fullname
-      build_options :without => [:domain_parameters_attributes, :fullname, :dns_id]
+      build_options :without => [:domain_parameters_attributes, :fullname]
+
+      extend_with(HammerCLIForeman::CommandExtensions::Domain.new)
     end
 
 
     class UpdateCommand < HammerCLIForeman::UpdateCommand
-      include DomainUpdateCreateCommons
-
       success_message _("Domain [%{name}] updated.")
       failure_message _("Could not update the domain")
 
       option "--description", "DESC", _("Full name describing the domain"), :attribute_name => :option_fullname
       build_options :without => [:domain_parameters_attributes, :fullname]
+
+      extend_with(HammerCLIForeman::CommandExtensions::Domain.new)
     end
 
 

test/functional/domain/create_test.rb

@@ -0,0 +1,91 @@
+require_relative '../test_helper'
+
+describe 'Domain' do
+  describe 'CreateCommand' do
+    let(:cmd) { %w[domain create] }
+    let(:minial_params) { %w[--name=dom1.com] }
+
+    def domain_params(additional_params = {})
+      params = {
+        domain: {
+          name: 'dom1.com'
+        }
+      }
+      params[:domain].merge!(additional_params)
+      params
+    end
+
+    it 'should print error on missing --name' do
+      expected_result = missing_args_error_result(cmd, '--name')
+
+      api_expects_no_call
+
+      result = run_cmd(cmd)
+      assert_cmd(expected_result, result)
+    end
+
+    it 'allows minimal options' do
+      api_expects(:domains, :create).with_params(domain_params)
+
+      run_cmd(cmd + minial_params)
+    end
+
+    it 'allows description' do
+      params = %w[--description=shortdesc]
+      api_expects(:domains, :create).with_params(domain_params(fullname: 'shortdesc'))
+
+      run_cmd(cmd + minial_params + params)
+    end
+
+    it 'allows dns id' do
+      params = %w[--dns-id=1]
+      api_expects(:domains, :create).with_params(domain_params(dns_id: 1))
+
+      run_cmd(cmd + minial_params + params)
+    end
+
+    it 'allows dns name' do
+      params = %w[--dns=sp1]
+      api_expects_search(:smart_proxies, { name: 'sp1' }).returns(
+        index_response([{ 'id' => 1 }])
+      )
+      api_expects(:domains, :create).with_params(domain_params(dns_id: 1))
+
+      run_cmd(cmd + minial_params + params)
+    end
+
+    it 'allows location ids' do
+      params = %w[--location-ids=1,4]
+      api_expects(:domains, :create).with_params(domain_params(location_ids: %w[1 4]))
+
+      run_cmd(cmd + minial_params + params)
+    end
+
+    it 'allows location names' do
+      params = %w[--locations=loc1,loc2]
+      api_expects(:locations, :index) do |p|
+        p[:search] == 'name = "loc1" or name = "loc2"'
+      end.returns(index_response([{ 'id' => 1 }, { 'id' => 2 }]))
+      api_expects(:domains, :create).with_params(domain_params(location_ids: [1, 2]))
+
+      run_cmd(cmd + minial_params + params)
+    end
+
+    it 'allows organization ids' do
+      params = %w[--organization-ids=1,4]
+      api_expects(:domains, :create).with_params(domain_params(organization_ids: %w[1 4]))
+
+      run_cmd(cmd + minial_params + params)
+    end
+
+    it 'allows organization names' do
+      params = %w[--organizations=org1,org2]
+      api_expects(:organizations, :index) do |p|
+        p[:search] == 'name = "org1" or name = "org2"'
+      end.returns(index_response([{ 'id' => 1 }, { 'id' => 2 }]))
+      api_expects(:domains, :create).with_params(domain_params(organization_ids: [1, 2]))
+
+      run_cmd(cmd + minial_params + params)
+    end
+  end
+end

test/functional/domain/update_test.rb

@@ -0,0 +1,90 @@
+require_relative '../test_helper'
+
+describe 'Domain' do
+  describe 'UpdateCommand' do
+    let(:cmd) { %w[domain update] }
+    let(:minial_params) { %w[--id=1] }
+
+    def domain_params(additional_params = {})
+      params = {
+        id: '1',
+        domain: {}
+      }
+      params[:domain].merge!(additional_params)
+      params
+    end
+
+    it 'should print error on missing --id' do
+      expected_result = missing_args_error_result(cmd, '--id')
+
+      api_expects_no_call
+
+      result = run_cmd(cmd)
+      assert_cmd(expected_result, result)
+    end
+
+    it 'allows minimal options' do
+      api_expects(:domains, :update).with_params(domain_params)
+
+      run_cmd(cmd + minial_params)
+    end
+
+    it 'allows description' do
+      params = %w[--description=shortdesc]
+      api_expects(:domains, :update).with_params(domain_params(fullname: 'shortdesc'))
+
+      run_cmd(cmd + minial_params + params)
+    end
+
+    it 'allows dns id' do
+      params = %w[--dns-id=1]
+      api_expects(:domains, :update).with_params(domain_params(dns_id: 1))
+
+      run_cmd(cmd + minial_params + params)
+    end
+
+    it 'allows dns name' do
+      params = %w[--dns=sp1]
+      api_expects_search(:smart_proxies, { name: 'sp1' }).returns(
+        index_response([{ 'id' => 1 }])
+      )
+      api_expects(:domains, :update).with_params(domain_params(dns_id: 1))
+
+      run_cmd(cmd + minial_params + params)
+    end
+
+    it 'allows location ids' do
+      params = %w[--location-ids=1,4]
+      api_expects(:domains, :update).with_params(domain_params(location_ids: %w[1 4]))
+
+      run_cmd(cmd + minial_params + params)
+    end
+
+    it 'allows location names' do
+      params = %w[--locations=loc1,loc2]
+      api_expects(:locations, :index) do |p|
+        p[:search] == 'name = "loc1" or name = "loc2"'
+      end.returns(index_response([{ 'id' => 1 }, { 'id' => 2 }]))
+      api_expects(:domains, :update).with_params(domain_params(location_ids: [1, 2]))
+
+      run_cmd(cmd + minial_params + params)
+    end
+
+    it 'allows organization ids' do
+      params = %w[--organization-ids=1,4]
+      api_expects(:domains, :update).with_params(domain_params(organization_ids: %w[1 4]))
+
+      run_cmd(cmd + minial_params + params)
+    end
+
+    it 'allows organization names' do
+      params = %w[--organizations=org1,org2]
+      api_expects(:organizations, :index) do |p|
+        p[:search] == 'name = "org1" or name = "org2"'
+      end.returns(index_response([{ 'id' => 1 }, { 'id' => 2 }]))
+      api_expects(:domains, :update).with_params(domain_params(organization_ids: [1, 2]))
+
+      run_cmd(cmd + minial_params + params)
+    end
+  end
+end