Skip to content

Commit

Permalink
Refs #38077 - Don't build hidden params as options (#634)
Browse files Browse the repository at this point in the history
  • Loading branch information
ofedoren authored Jan 8, 2025
1 parent fb0970e commit fd288fe
Show file tree
Hide file tree
Showing 10 changed files with 7 additions and 34 deletions.
2 changes: 1 addition & 1 deletion lib/hammer_cli_foreman/commands.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ def exception_handler_class
def self.create_option_builder
configurator = BuilderConfigurator.new(searchables, dependency_resolver)

builder = ForemanOptionBuilder.new(searchables)
builder = ForemanOptionBuilder.new(searchables, resource_defined? ? resource.action(action) : nil)
builder.builders = super.builders
builder.builders += configurator.builders_for(resource, resource.action(action)) if resource_defined?
builder
Expand Down
6 changes: 4 additions & 2 deletions lib/hammer_cli_foreman/option_builders.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,15 +95,17 @@ def builders_for(resource, action)

class ForemanOptionBuilder < HammerCLI::OptionBuilderContainer

def initialize(searchables)
def initialize(searchables, action = nil)
@searchables = searchables
@action = action
end

def build(builder_params={})
expansion_options = builder_params[:expand] || {}

except_default = @action&.params&.select { |p| !p.show? }&.map { |p| HammerCLIForeman.param_to_resource(p.name).name } || []
allowed_resources = expansion_options[:only] || default_dependent_resources
allowed_resources -= expansion_options[:except] || []
allowed_resources -= expansion_options[:except] || except_default
allowed_resources += expansion_options[:including] || []
allowed_resources.uniq!

Expand Down
1 change: 1 addition & 0 deletions test/data/3.14/foreman_api.json

Large diffs are not rendered by default.

2 changes: 0 additions & 2 deletions test/functional/architecture_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@
)
defaults.stubs(:write_to_file).returns(true)
defaults.stubs(:providers).returns(providers)
api_expects(:users, :index, 'Find user').with_params(search: 'login=admin').returns(index_response([{ 'default_organization' => { 'id' => 2 } }]))
api_expects(:users, :index, 'Find user').with_params(search: 'login=admin').returns(index_response([{ 'default_location' => { 'id' => 1 } }]))
api_expects(:architectures, :index, 'List architectures').returns(@architectures)

result = run_cmd(@cmd, { use_defaults: true, defaults: defaults })
Expand Down
2 changes: 0 additions & 2 deletions test/functional/settings_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@
)
defaults.stubs(:write_to_file).returns(true)
defaults.stubs(:providers).returns(providers)
api_expects(:users, :index, 'Find user').with_params(search: 'login=admin').returns(index_response([{ 'default_organization' => { 'id' => 2 } }]))
api_expects(:users, :index, 'Find user').with_params(search: 'login=admin').returns(index_response([{ 'default_location' => { 'id' => 1 } }]))
api_expects(:settings, :index, 'List settings').returns(setting)

result = run_cmd(@cmd, { use_defaults: true, defaults: defaults })
Expand Down
2 changes: 0 additions & 2 deletions test/functional/usergroup_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@
)
defaults.stubs(:write_to_file).returns(true)
defaults.stubs(:providers).returns(providers)
api_expects(:users, :index, 'Find user').with_params(search: 'login=admin').returns(index_response([{ 'default_organization' => { 'id' => 2 } }]))
api_expects(:users, :index, 'Find user').with_params(search: 'login=admin').returns(index_response([{ 'default_location' => { 'id' => 1 } }]))
api_expects(:usergroups, :index, 'List user groups').returns(@user_groups)

result = run_cmd(@cmd, { use_defaults: true, defaults: defaults })
Expand Down
2 changes: 1 addition & 1 deletion test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
require 'hammer_cli'
require 'hammer_cli_foreman/testing/api_expectations'

FOREMAN_VERSION = ENV['TEST_API_VERSION'] || '3.12'
FOREMAN_VERSION = ENV['TEST_API_VERSION'] || '3.14'
unless Dir.entries('test/data').include? FOREMAN_VERSION
raise StandardError.new "Version is not correct"
end
Expand Down
10 changes: 0 additions & 10 deletions test/unit/architecture_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
describe "parameters" do
it_should_accept "no arguments"
it_should_accept_search_params
it_should_accept 'organization', ['--organization-id=1']
it_should_accept 'location', ['--location-id=1']
end

describe "output" do
Expand All @@ -39,8 +37,6 @@
describe "parameters" do
it_should_accept "id", ["--id=1"]
it_should_accept "name", ["--name=arch"]
it_should_accept 'organization', %w[--id=1 --organization-id=1]
it_should_accept 'location', %w[--id=1 --location-id=1]
# it_should_fail_with "no arguments" # TODO: temporarily disabled, parameters are checked in the id resolver
end

Expand All @@ -61,8 +57,6 @@

describe "parameters" do
it_should_accept "name", ["--name=arch"]
it_should_accept 'organization', %w[--name=arch --organization-id=1]
it_should_accept 'location', %w[--name=arch --location-id=1]
# it_should_fail_with "name missing", []
# TODO: temporarily disabled, parameters are checked in the api
end
Expand All @@ -77,8 +71,6 @@
describe "parameters" do
it_should_accept "name", ["--name=arch"]
it_should_accept "id", ["--id=1"]
it_should_accept 'organization', %w[--id=1 --organization-id=1]
it_should_accept 'location', %w[--id=1 --location-id=1]
# it_should_fail_with "name or id missing", [] # TODO: temporarily disabled, parameters are checked in the id resolver
end

Expand All @@ -92,8 +84,6 @@
describe "parameters" do
it_should_accept "name", ["--name=arch", "--new-name=arch2"]
it_should_accept "id", ["--id=1", "--new-name=arch2"]
it_should_accept 'organization', %w[--id=1 --new-name=arch2 --organization-id=1]
it_should_accept 'location', %w[--id=1 --new-name=arch2 --location-id=1]
# it_should_fail_with "no params", [] # TODO: temporarily disabled, parameters are checked in the id resolver
# it_should_fail_with "name or id missing", ["--new-name=arch2"] # TODO: temporarily disabled, parameters are checked in the id resolver
end
Expand Down
4 changes: 0 additions & 4 deletions test/unit/settings_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
describe "parameters" do
it_should_accept "no arguments"
it_should_accept_search_params
it_should_accept 'organization', ['--organization-id=1']
it_should_accept 'location', ['--location-id=1']
end

describe "output" do
Expand All @@ -40,8 +38,6 @@
describe "parameters" do
it_should_accept "name", ["--name=setting1", "--value=setting2"]
it_should_accept "id", ["--id=1", "--value=setting2"]
it_should_accept 'organization', %w[--id=1 --organization-id=1]
it_should_accept 'location', %w[--id=1 --location-id=1]
end

end
Expand Down
10 changes: 0 additions & 10 deletions test/unit/usergroup_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
describe "parameters" do
it_should_accept "no arguments"
it_should_accept_search_params
it_should_accept 'organization', ['--organization-id=1']
it_should_accept 'location', ['--location-id=1']
end

describe "output" do
Expand All @@ -37,8 +35,6 @@
describe "parameters" do
it_should_accept "id", ["--id=1"]
it_should_accept "name", ["--name=ug"]
it_should_accept 'organization', %w[--id=1 --organization-id=1]
it_should_accept 'location', %w[--id=1 --location-id=1]
end

describe "output" do
Expand All @@ -62,8 +58,6 @@
describe "parameters" do
it_should_accept "name", ["--name=ug"]
it_should_accept "name, role ids, user group ids and user ids", ["--name=ug", "--role-ids=1,2,3", "--user-group-ids=1,2,3", "--user-ids=1,2,3"]
it_should_accept 'organization', %w[--name=ug --organization-id=1]
it_should_accept 'location', %w[--name=ug --location-id=1]
end
end

Expand All @@ -74,8 +68,6 @@
describe "parameters" do
it_should_accept "name", ["--name=ug"]
it_should_accept "id", ["--id=1"]
it_should_accept 'organization', %w[--id=1 --organization-id=1]
it_should_accept 'location', %w[--id=1 --location-id=1]
end
end

Expand All @@ -88,8 +80,6 @@
it_should_accept "id", ["--id=1"]
it_should_accept "name and new name", ["--name=ug", "--new-name=ug2"]
it_should_accept "id, new name, role ids, user group ids and user ids", ["--id=1", "--new-name=ug", "--role-ids=1,2,3", "--user-group-ids=1,2,3", "--user-ids=1,2,3"]
it_should_accept 'organization', %w[--id=1 --organization-id=1]
it_should_accept 'location', %w[--id=1 --location-id=1]
end
end
end

0 comments on commit fd288fe

Please sign in to comment.