From 5c7d1442557eebe6baf64bb1dc1ec69bc49d8761 Mon Sep 17 00:00:00 2001 From: Keenan Brock Date: Tue, 17 May 2022 16:00:34 -0400 Subject: [PATCH] move AvailabilityMixin over to SupportsFeatureMixin - convert is_available? to supports? - convert supports_#{feature} to supports?(:feature) --- ...cloud_object_store_container_controller.rb | 2 +- .../mixins/actions/host_actions/misc.rb | 2 +- app/helpers/application_helper.rb | 8 +---- .../generic_feature_button_with_disable.rb | 9 +----- .../button/host_feature_button.rb | 4 ++- .../host_feature_button_with_disable.rb | 4 +-- spec/controllers/storage_controller_spec.rb | 2 +- .../buttons/generic_feature_button_spec.rb | 2 +- .../buttons/host_feature_button_spec.rb | 2 +- ...ed_examples_for_generic_feature_buttons.rb | 32 +++++-------------- 10 files changed, 20 insertions(+), 47 deletions(-) diff --git a/app/controllers/cloud_object_store_container_controller.rb b/app/controllers/cloud_object_store_container_controller.rb index 4e9a6f3d507..1c0d7b9e2d2 100644 --- a/app/controllers/cloud_object_store_container_controller.rb +++ b/app/controllers/cloud_object_store_container_controller.rb @@ -74,7 +74,7 @@ def record_class end def retrieve_provider_regions - managers = ManageIQ::Providers::CloudManager.permitted_subclasses.select(&:supports_regions?) + managers = ManageIQ::Providers::CloudManager.permitted_subclasses.select { |subclass| subclass.supports?(:regions) } managers.each_with_object({}) do |manager, provider_regions| regions = manager.module_parent::Regions.all.sort_by { |r| r[:description] } provider_regions[manager.name] = regions.map { |region| [region[:description], region[:name]] } diff --git a/app/controllers/mixins/actions/host_actions/misc.rb b/app/controllers/mixins/actions/host_actions/misc.rb index d7dbe697970..84f6ddfdb64 100644 --- a/app/controllers/mixins/actions/host_actions/misc.rb +++ b/app/controllers/mixins/actions/host_actions/misc.rb @@ -133,7 +133,7 @@ def process_hosts_provide(hosts, display_name) def process_hosts_generic(hosts, task, display_name) each_host(hosts, display_name) do |host| - if host.respond_to?(task) && host.is_available?(task) + if host.supports?(task.to_sym) host.send(task.to_sym) add_flash(_("\"%{record}\": %{task} successfully initiated") % {:record => host.name, :task => display_name}) else diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 4e76257511d..4acc00ecdbd 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -201,13 +201,7 @@ def restful_routed?(record_or_model) # - false in case the record (or one of many records) does not # support the feature def records_support_feature?(records, feature) - Array.wrap(records).all? do |record| - if record.respond_to?("supports_#{feature}?") - record.supports?(feature) - else # TODO: remove with deleting AvailabilityMixin module - record.is_available?(feature) - end - end + Array.wrap(records).all? { |record| record.supports?(feature.to_sym) } end # Create a url for a record that links to the proper controller diff --git a/app/helpers/application_helper/button/generic_feature_button_with_disable.rb b/app/helpers/application_helper/button/generic_feature_button_with_disable.rb index 2cbc68cae95..22f66671761 100644 --- a/app/helpers/application_helper/button/generic_feature_button_with_disable.rb +++ b/app/helpers/application_helper/button/generic_feature_button_with_disable.rb @@ -2,14 +2,7 @@ class ApplicationHelper::Button::GenericFeatureButtonWithDisable < ApplicationHe needs :@record def disabled? - @error_message = - # Feature supported via SupportsFeatureMixin - if @record.respond_to?("supports_#{@feature}?") - @record.unsupported_reason(@feature) unless @record.supports?(@feature) - else # Feature supported via AvailabilityMixin - @record.is_available_now_error_message(@feature) unless @record.is_available?(@feature) - end - + @error_message = @record.unsupported_reason(@feature) unless @record.supports?(@feature) @error_message.present? end diff --git a/app/helpers/application_helper/button/host_feature_button.rb b/app/helpers/application_helper/button/host_feature_button.rb index 7e6d96eb5de..6a3a1361896 100644 --- a/app/helpers/application_helper/button/host_feature_button.rb +++ b/app/helpers/application_helper/button/host_feature_button.rb @@ -1,13 +1,15 @@ class ApplicationHelper::Button::HostFeatureButton < ApplicationHelper::Button::GenericFeatureButton def visible? + # TODO: @feature.nil? || @record.nil? || @record.supports?(@feature) + # TODO: ensure start stop are supported in openstack unless @feature.nil? || @record.nil? if @record.kind_of?(ManageIQ::Providers::Openstack::InfraManager) return true if %w[start stop].include?(@feature.to_s) return false end - return @record.is_available?(@feature) if @record.kind_of?(ManageIQ::Providers::Openstack::InfraManager::Host) + return @record.supports?(@feature) if @record.kind_of?(ManageIQ::Providers::Openstack::InfraManager::Host) end true end diff --git a/app/helpers/application_helper/button/host_feature_button_with_disable.rb b/app/helpers/application_helper/button/host_feature_button_with_disable.rb index e92ad309892..88478ba3747 100644 --- a/app/helpers/application_helper/button/host_feature_button_with_disable.rb +++ b/app/helpers/application_helper/button/host_feature_button_with_disable.rb @@ -1,9 +1,9 @@ class ApplicationHelper::Button::HostFeatureButtonWithDisable < ApplicationHelper::Button::GenericFeatureButtonWithDisable - def visible? + # TODO: @feature.nil? || @record.nil? || !@record.kind_of?(Host) || @record.supports?(@feature.to_sym) unless @feature.nil? || @record.nil? return false if @record.kind_of?(ManageIQ::Providers::Openstack::InfraManager) - return @record.is_available?(@feature) if @record.kind_of?(ManageIQ::Providers::Openstack::InfraManager::Host) + return @record.supports?(@feature.to_sym) if @record.kind_of?(ManageIQ::Providers::Openstack::InfraManager::Host) end true end diff --git a/spec/controllers/storage_controller_spec.rb b/spec/controllers/storage_controller_spec.rb index efb8178d558..9a7626284af 100644 --- a/spec/controllers/storage_controller_spec.rb +++ b/spec/controllers/storage_controller_spec.rb @@ -90,7 +90,7 @@ host = FactoryBot.create(:host) command = button.split('_', 2)[1] - allow_any_instance_of(Host).to receive(:is_available?).with(command).and_return(true) + allow_any_instance_of(Host).to receive(:supports?).with(command.to_sym).and_return(true) controller.params = {:pressed => button, :miq_grid_checks => host.id.to_s} controller.instance_variable_set(:@lastaction, "show_list") diff --git a/spec/helpers/application_helper/buttons/generic_feature_button_spec.rb b/spec/helpers/application_helper/buttons/generic_feature_button_spec.rb index 35723e31bdb..3db3d1fb7cc 100644 --- a/spec/helpers/application_helper/buttons/generic_feature_button_spec.rb +++ b/spec/helpers/application_helper/buttons/generic_feature_button_spec.rb @@ -1,6 +1,6 @@ describe ApplicationHelper::Button::GenericFeatureButton do let(:view_context) { setup_view_context_with_sandbox({}) } - let(:record) { double } + let(:record) { Vm.new } let(:feature) { :some_feature } let(:props) { {:options => {:feature => feature}} } let(:button) { described_class.new(view_context, {}, {'record' => record}, props) } diff --git a/spec/helpers/application_helper/buttons/host_feature_button_spec.rb b/spec/helpers/application_helper/buttons/host_feature_button_spec.rb index 7149f1675f6..fa600622f56 100644 --- a/spec/helpers/application_helper/buttons/host_feature_button_spec.rb +++ b/spec/helpers/application_helper/buttons/host_feature_button_spec.rb @@ -24,7 +24,7 @@ end context 'when record.kind_of?(ManageIQ::Providers::Openstack::InfraManager::Host)' do let(:record) { FactoryBot.create(:host_openstack_infra) } - before { allow(record).to receive(:is_available?).and_return(available) } + before { allow(record).to receive(:supports?).and_return(available) } context 'and feature is available' do let(:feature) { :stop } let(:available) { true } diff --git a/spec/support/examples_group/shared_examples_for_generic_feature_buttons.rb b/spec/support/examples_group/shared_examples_for_generic_feature_buttons.rb index 63373ba54a7..88769801ac6 100644 --- a/spec/support/examples_group/shared_examples_for_generic_feature_buttons.rb +++ b/spec/support/examples_group/shared_examples_for_generic_feature_buttons.rb @@ -35,35 +35,19 @@ shared_examples_for 'GenericFeatureButtonWithDisabled#calculate_properties' do describe '#calculate_properties' do - let(:available) { true } before do - allow(record).to receive(:is_available?).with(feature).and_return(available) - allow(record).to receive(:is_available_now_error_message).and_return('unavailable') - allow(record).to receive("supports_#{feature}?").and_return(support) if defined? support - allow(record).to receive(:unsupported_reason).with(feature).and_return("Feature not available/supported") if defined? support && !support + allow(record).to receive("supports?").with(feature.to_sym).and_return(support) + allow(record).to receive(:unsupported_reason).with(feature).and_return("Feature not available/supported") if !support button.calculate_properties end - context 'when feature exists' do - let(:feature) { :existent_feature } - context 'and feature is supported' do - let(:support) { true } - it_behaves_like 'an enabled button' - end - context 'and feature is not supported' do - let(:support) { false } - it_behaves_like 'a disabled button', 'Feature not available/supported' - end + context 'when feature is supported' do + let(:support) { true } + it_behaves_like 'an enabled button' end - context 'when feature is unknown' do - let(:feature) { :non_existent_feature } - context 'and feature is not available' do - let(:available) { false } - it_behaves_like 'a disabled button', 'unavailable' - end - context 'but feature is available' do - it_behaves_like 'an enabled button' - end + context 'when feature is not supported' do + let(:support) { false } + it_behaves_like 'a disabled button', 'Feature not available/supported' end end end