diff --git a/lib/hammer_cli_foreman/filter.rb b/lib/hammer_cli_foreman/filter.rb index b3fb3270..f294efde 100644 --- a/lib/hammer_cli_foreman/filter.rb +++ b/lib/hammer_cli_foreman/filter.rb @@ -9,8 +9,6 @@ class ListCommand < HammerCLIForeman::ListCommand field :id, _("Id") field :resource_type, _("Resource type") field :search, _("Search") - field :unlimited?, _("Unlimited?"), Fields::Boolean - field :override?, _("Override?"), Fields::Boolean field :role, _("Role"), Fields::Reference field :permissions, _("Permissions"), Fields::List end @@ -27,8 +25,15 @@ def extend_data(filter) class InfoCommand < HammerCLIForeman::InfoCommand output ListCommand.output_definition do - HammerCLIForeman::References.taxonomies(self) HammerCLIForeman::References.timestamps(self) + from :role do + collection :locations, _("Locations"), numbered: false, hide_blank: true do + custom_field Fields::Reference, name_key: :title + end + collection :organizations, _("Organizations"), numbered: false, hide_blank: true do + custom_field Fields::Reference, name_key: :title + end + end end def extend_data(filter) @@ -42,81 +47,18 @@ def extend_data(filter) end - module TaxonomyCheck - - def self.included(base) - def taxonomy_options? - opt_names = ['location_ids', 'organization_ids'] - opt_names += resolver.searchables(:locations).map { |s| 'location_' + s.plural_name } - opt_names += resolver.searchables(:organizations).map { |s| 'organization_' + s.plural_name } - opt_names.any? { |opt| send(HammerCLI.option_accessor_name(opt)) } - end - - def signal_override_usage_error - signal_usage_error _('Organizations and locations can be set only for overriding filters.') - end - - base.extend_help do |base_h| - base_h.section(_('Overriding organizations and locations')) do |section_h| - override_condition = "--override=true" - org_opts = '--organization[s|-ids]' - loc_opts = '--location[s|-ids]' - - section_h.text(_("Filters inherit organizations and locations from its role by default. This behavior can be changed by setting %{condition}.%{wsp}" + - "Therefore options %{org_opts} and %{loc_opts} are applicable only when the override flag is set.") % { - :wsp => "\n", - :org_opts => org_opts, - :loc_opts => loc_opts, - :condition => override_condition - }) - end - end - end - end - - class CreateCommand < HammerCLIForeman::CreateCommand - include TaxonomyCheck - success_message _("Permission filter for [%s] created.") failure_message _("Could not create the permission filter") - def validate_options - signal_override_usage_error if !option_override && taxonomy_options? - end - build_options end class UpdateCommand < HammerCLIForeman::UpdateCommand - include TaxonomyCheck - success_message _("Permission filter for [%s] updated.") failure_message _("Could not update the permission filter") - def request_params - params = super - unless override? - # Clear taxonomies in case the filter is switching override from true to false - params['filter']['location_ids'] = [] if !@filter || !@filter['locations'].empty? - params['filter']['organization_ids'] = [] if !@filter || !@filter['organizations'].empty? - end - params - end - - def validate_options - signal_override_usage_error if !override? && taxonomy_options? - end - - def override? - if option_override.nil? - filter['override?'] - else - option_override - end - end - def filter @filter ||= HammerCLIForeman.foreman_resource!(:filters).action(:show).call({ :id => get_identifier }, request_headers, request_options) end diff --git a/test/functional/filter_test.rb b/test/functional/filter_test.rb index 7cdf5ed9..777198fd 100644 --- a/test/functional/filter_test.rb +++ b/test/functional/filter_test.rb @@ -4,10 +4,9 @@ describe 'filter' do def api_expects_filter_info(options) - override = !!options[:override] api_expects(:filters, :show, 'Get filter info').with_params( id: '1' - ).returns(@filter.merge('override?' => override)) + ).returns(@filter) end def taxonomy_usage_error(action, cmd) @@ -27,15 +26,6 @@ def assert_update_success(result) @cmd = %w[filter create] end - it 'prints error when taxonomies are used for a not-overriding filter' do - params = ['--organization-ids=1,2', '--location-ids=3,4', '--override=false'] - - api_expects_no_call - - result = run_cmd(@cmd + params) - assert_cmd(taxonomy_usage_error('create', @cmd), result) - end - it 'should create a filter' do params = ['--role-id=1', '--permission-ids=[1]'] @@ -67,8 +57,8 @@ def assert_update_success(result) api_expects(:filters, :index, 'List filters').returns(@filters) output = IndexMatcher.new([ - ['ID', 'RESOURCE TYPE', 'SEARCH', 'UNLIMITED?', 'OVERRIDE?', 'ROLE', 'PERMISSIONS'], - ['1', 'Architecture', 'none', 'yes', 'no', 'Manager', 'view_architectures'] + ['ID', 'RESOURCE TYPE', 'SEARCH', 'ROLE', 'PERMISSIONS'], + ['1', 'Architecture', 'none', 'Manager', 'view_architectures'] ]) expected_result = success_result(output) @@ -100,115 +90,4 @@ def assert_update_success(result) assert_cmd(success_result("Permission filter deleted.\n"), result) end end - - describe 'update' do - before do - @cmd = %w[filter update] - @filter = { - 'search' => nil, - 'resource_type_label' => 'User', - 'resource_type' => 'User', - 'unlimited?' => false, - 'created_at' => '2017-07-18 14:34:09 UTC', - 'updated_at' => '2017-07-18 14:34:09 UTC', - 'override?' => true, - 'id' => 404, - 'role' => { - 'name' => 'Some Role', - 'id' => 28, - 'description' => "Description\nof the new\nrole", - 'origin' => nil - }, - 'permissions' => [{ - 'name' => 'view_users', - 'id' => 164, - 'resource_type' => 'User' - }], - 'locations' => [{ - 'id' => 28, - 'name' => 'location74', - 'title' => 'location74', - 'description' => nil - }], - 'organizations' => [{ - 'id' => 27, - 'name' => 'organization74', - 'title' => 'organization74', - 'description' => nil - }] - } - end - - it 'resets taxonomies when a filter is not-overriding' do - params = ['--id=1'] - - api_expects_filter_info(override: false) - api_expects(:filters, :update, 'Update the filter').with_params( - 'filter' => { - 'organization_ids' => [], - 'location_ids' => [] - } - ).returns(@filter) - - assert_update_success(run_cmd(@cmd + params)) - end - - it 'resets taxonomies when switching a filter to not-overriding' do - params = ['--id=1', '--override=false'] - - api_expects(:filters, :update, 'Update the filter').with_params( - 'filter' => { - 'organization_ids' => [], - 'location_ids' => [] - } - ).returns(@filter) - - assert_update_success(run_cmd(@cmd + params)) - end - - it 'can add taxonomies when a filter is overriding' do - params = ['--id=1', '--organization-ids=1,2', '--location-ids=3,4'] - - api_expects_filter_info(override: true) - api_expects(:filters, :update, 'Update the filter').with_params( - 'filter' => { - 'organization_ids' => %w[1 2], - 'location_ids' => %w[3 4] - } - ).returns(@filter) - - assert_update_success(run_cmd(@cmd + params)) - end - - it 'can add taxonomies when switching a filter to overriding' do - params = ['--id=1', '--organization-ids=1,2', '--location-ids=3,4', '--override=true'] - - api_expects(:filters, :update, 'Update the filter').with_params( - 'filter' => { - 'organization_ids' => %w[1 2], - 'location_ids' => %w[3 4] - } - ).returns(@filter) - - assert_update_success(run_cmd(@cmd + params)) - end - - it 'prints error when taxonomies are used on not-overriding' do - params = ['--id=1', '--organization-ids=1,2', '--location-ids=3,4'] - - api_expects_filter_info(override: false) - - result = run_cmd(@cmd + params) - assert_cmd(taxonomy_usage_error('update', @cmd), result) - end - - it 'prints error when taxonomies are used when switching a filter to not-overriding' do - params = ['--id=1', '--organization-ids=1,2', '--location-ids=3,4', '--override=false'] - - api_expects_no_call - - result = run_cmd(@cmd + params) - assert_cmd(taxonomy_usage_error('update', @cmd), result) - end - end end diff --git a/test/unit/filter_test.rb b/test/unit/filter_test.rb index 7db73f45..fed42966 100644 --- a/test/unit/filter_test.rb +++ b/test/unit/filter_test.rb @@ -60,11 +60,6 @@ describe "CreateCommand" do let(:cmd) { HammerCLIForeman::Filter::CreateCommand.new("", ctx) } - before do - # FIXME: remove stubbing option_override once tests are switched to apidoc 1.14+ - cmd.stubs(:option_override).returns(false) - end - describe "parameters" do it_should_accept "role id and permission ids", ["--role-id=1", "--permission-ids=1,2"] it_should_accept "role name and permission ids", ["--role=role", "--permission-ids=1,2"] @@ -85,11 +80,6 @@ describe "UpdateCommand" do let(:cmd) { HammerCLIForeman::Filter::UpdateCommand.new("", ctx) } - before do - # FIXME: remove stubbing option_override once tests are switched to apidoc 1.14+ - cmd.stubs(:option_override).returns(false) - end - describe "parameters" do it_should_accept "id, role id and permission ids", ["--id=1", "--role-id=1", "--permission-ids=1,2"] it_should_accept "id, role name and permission ids", ["--id=1", "--role=role", "--permission-ids=1,2"]