Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use base_model name instead of demodulize #5182

Merged
merged 1 commit into from Feb 1, 2019
Merged

Use base_model name instead of demodulize #5182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
63 changes: 9 additions & 54 deletions app/services/dialog_local_service.rb
View file
Open in desktop
@@ -1,40 +1,4 @@
class DialogLocalService
NEW_DIALOG_USERS = %w(
AvailabilityZone
CloudNetwork
CloudObjectStoreContainer
CloudSubnet
CloudTenant
CloudVolume
ContainerGroup
ContainerImage
ContainerNode
ContainerProject
ContainerTemplate
ContainerVolume
EmsCluster
GenericObject
Host
InfraManager
LoadBalancer
MiqGroup
NetworkRouter
OrchestrationStack
SecurityGroup
Service
ServiceAnsiblePlaybook
ServiceAnsibleTower
ServiceContainerTemplate
ServiceGeneric
ServiceOrchestration
Storage
Switch
Template
Tenant
User
Vm
).freeze

def determine_dialog_locals_for_svc_catalog_provision(resource_action, target, finish_submit_endpoint)
api_submit_endpoint = "/api/service_catalogs/#{target.service_template_catalog_id}/service_templates/#{target.id}"

Expand All @@ -53,13 +17,9 @@ def determine_dialog_locals_for_svc_catalog_provision(resource_action, target, f
end

def determine_dialog_locals_for_custom_button(obj, button_name, resource_action, display_options = {})
dialog_locals = {:force_old_dialog_use => true}

return dialog_locals unless NEW_DIALOG_USERS.include?(obj.class.name.demodulize)

submit_endpoint, cancel_endpoint = determine_api_endpoints(obj, display_options)

dialog_locals.merge!(
{
:resource_action_id => resource_action.id,
:target_id => obj.id,
:target_type => determine_target_type(obj),
Expand All @@ -70,29 +30,30 @@ def determine_dialog_locals_for_custom_button(obj, button_name, resource_action,
:finish_submit_endpoint => cancel_endpoint,
:cancel_endpoint => cancel_endpoint,
:open_url => false
)

dialog_locals
}
end

private

def determine_api_endpoints(obj, display_options = {})
base_name = obj.class.name.demodulize
base_name = obj.class.base_model.name
case base_name
when /EmsCluster/
api_collection_name = "clusters"
cancel_endpoint = "/ems_cluster"
when /MiqTemplate/
api_collection_name = "templates"
cancel_endpoint = display_options[:cancel_endpoint] || "/vm_or_template/explorer"
when /GenericObject/
api_collection_name = "generic_objects"
cancel_endpoint = if !display_options.empty? && display_options[:display_id]
"/service/show/#{display_options[:display_id]}?display=generic_objects"
else
"/service/explorer"
end
when /InfraManager/
when /ExtManagementSystem/
d-m-u marked this conversation as resolved.
Show resolved Hide resolved
api_collection_name = "providers"
cancel_endpoint = "/ems_infra"
cancel_endpoint = obj.class.name.demodulize == "CloudManager" ? "/ems_cloud" : "/ems_infra"
when /MiqGroup/
api_collection_name = "groups"
cancel_endpoint = "/ops/explorer"
Expand All @@ -105,12 +66,6 @@ def determine_api_endpoints(obj, display_options = {})
when /Switch/
api_collection_name = "switches"
cancel_endpoint = "/infra_networking/explorer"

# ^ is necessary otherwise we match on ContainerTemplates
when /^Template/
api_collection_name = "templates"
cancel_endpoint = display_options[:cancel_endpoint] || "/vm_or_template/explorer"

# ^ is necessary otherwise we match CloudTenant
when /^Tenant/
api_collection_name = "tenants"
Expand Down Expand Up @@ -140,7 +95,7 @@ def determine_target_type(obj)
when /^Service/
"service"
else
obj.class.name.demodulize.underscore
obj.class.base_model.name.underscore.downcase
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a much different return value in some cases. Were you able to fully verify this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ones that are different from the old file with this change are:
cloud_network for what would have been private and
host for what would've returned host_esx

end
end
end
34 changes: 22 additions & 12 deletions spec/services/dialog_local_service_spec.rb
View file
Open in desktop
Expand Up @@ -44,8 +44,7 @@
let(:resource_action) { instance_double("ResourceAction", :id => 321, :dialog_id => 654) }
let(:display_options) { {} }

shared_examples_for "DialogLocalService#determine_dialog_locals_for_custom_button return value" do
|target_type, collection_name, finish_endpoint|
shared_examples_for "DialogLocalService#determine_dialog_locals_for_custom_button return value" do |target_type, collection_name, finish_endpoint|
it "returns a hash with the proper parameters" do
expect(service.determine_dialog_locals_for_custom_button(obj, button_name, resource_action, display_options))
.to eq(
Expand Down Expand Up @@ -77,6 +76,13 @@
"cloud_network", "cloud_networks", "/cloud_network"
end

context "when the object is a private CloudNetwork" do
let(:obj) { double(:class => ManageIQ::Providers::Openstack::NetworkManager::CloudNetwork::Private, :id => 123) }

include_examples "DialogLocalService#determine_dialog_locals_for_custom_button return value",
"cloud_network", "cloud_networks", "/cloud_network"
end

context "when the object is a CloudObjectStoreContainer" do
let(:obj) { double(:class => ManageIQ::Providers::Amazon::StorageManager::S3::CloudObjectStoreContainer, :id => 123) }

Expand Down Expand Up @@ -168,13 +174,27 @@
"host", "hosts", "/host"
end

context "when the object is a HostEsx" do
let(:obj) { double(:class => ManageIQ::Providers::Vmware::InfraManager::HostEsx, :id => 123) }

include_examples "DialogLocalService#determine_dialog_locals_for_custom_button return value",
"host", "hosts", "/host"
end

context "when the object is an InfraManager" do
let(:obj) { double(:class => ManageIQ::Providers::Vmware::InfraManager, :id => 123) }

include_examples "DialogLocalService#determine_dialog_locals_for_custom_button return value",
"ext_management_system", "providers", "/ems_infra"
end

context "when the object is a CloudManager" do
let(:obj) { double(:class => ManageIQ::Providers::Vmware::CloudManager, :id => 123) }

include_examples "DialogLocalService#determine_dialog_locals_for_custom_button return value",
"ext_management_system", "providers", "/ems_cloud"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is "/ems_cloud" right? Do you know how that is used?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, is that a UI route?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah but it'd be real spiffing if someone on the UI team could confirm that that's what is supposed to happen here.

end

context "when the object is a LoadBalancer" do
let(:obj) { double(:class => ManageIQ::Providers::Amazon::NetworkManager::LoadBalancer, :id => 123) }

Expand Down Expand Up @@ -292,15 +312,5 @@
"vm", "vms", "/vm_infra/explorer"
end
end

context "when the object does not support new dialogs" do
let(:obj) { double(:id => 123) }

it "returns a hash with 'force_old_dialog_use' set to true" do
expect(service.determine_dialog_locals_for_custom_button(obj, button_name, resource_action)).to eq(
:force_old_dialog_use => true
)
end
end
end
end