Skip to content

Commit

Permalink
Refs #38077 - Don't build hidden params as options
Browse files Browse the repository at this point in the history
  • Loading branch information
ofedoren committed Jan 6, 2025
1 parent fb0970e commit 5ffaec9
Show file tree
Hide file tree
Showing 10 changed files with 11 additions and 35 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
11 changes: 8 additions & 3 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 Expand Up @@ -239,14 +241,17 @@ def dependent_options(resource, resource_name_map, command)
description: _('%{types} of associated %{resource}') % { types: types, resource: associated_resource },
creator: command
)
# require 'pry-byebug'; binding.pry if associated_resource.to_s == "location"
# parent_show_from_api = ParamsNameFilter.new("#{aliased_name}_id").for_action(@resource.action(@context[:action].to_s)).first&.show?
options << family.parent(
optionamize("--#{aliased_name}-id"),
"#{aliased_name}_id".upcase,
description("#{aliased_name}_id", @context[:action]),
attribute_name: HammerCLI.option_accessor_name("#{resource_name}_id"),
referenced_resource: resource_name,
aliased_resource: aliased_name,
format: HammerCLI::Options::Normalizers::Number.new
format: HammerCLI::Options::Normalizers::Number.new,
# show: parent_show_from_api ? parent_show_from_api : true
)
end

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 5ffaec9

Please sign in to comment.