Add missing routes to compare managed VMs of a Datastore

[hstastna]

Fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=1733120
https://bugzilla.redhat.com/show_bug.cgi?id=1746214
https://bugzilla.redhat.com/show_bug.cgi?id=1746449

This PR adds missing routes to enable ability to compare Managed VMs of a Datastore (selected VMs, displayed in a nested list from Relationships table of a chosen Datastore).

Note:
Missing Cancel button will be fixed via https://bugzilla.redhat.com/show_bug.cgi?id=1733295.

Before:

After:

[hstastna]

@miq-bot add_label bug, hammer/yes, ivanchuk/yes

[h-kataria]

@hstastna compare routes should be added to ems_infra controller section as well, please check maybe there are others that have these routes missing too.

[hstastna]

Yes, the same problem occurs for comparing VMs displayed from an Infra provider's details page.

[hstastna]

Just a note that I've checked other controllers and routes shouldn't be missing anywhere else. But in some controllers, comparing does not work (nothing happens in the UI, also no error). This is because of a different problem, the appropriate method for comparing is missing in those controllers. This will be fixed in a different PR.

[himdel]

So, all of these controllers seem to display vms or instances in some form:
(added a checkbox to mark the ones that already have the route)

• auth_key_pair
• availability_zone
• cloud_network
• cloud_subnet
• cloud_tenant
• cloud_volume
• ems_cloud
• ems_cluster
• ems_infra (new in this PR)
• flavor
• floating_ip
• host
• host_aggregate
• load_balancer
• network_router
• orchestration_stack
• resource_pool
• security_group
• storage (new in this PR)
• vm_cloud
• vm_infra
• vm_or_template

And possibly (via EmsCommon, not sure)

• automation_manager
• ems_block_storage
• ems_container
• ems_network
• ems_object_storage
• ems_physical_infra
• ems_storage
• provider_foreman

Still investigating if any of those unchecked controllers actually allow the user to compare things.

[hstastna]

@himdel I've tested and found that there is another issue with comparing items: some compare method is totally missing (so not just route) and there IS possibility to compare (I've checked the toolbar and tried). See where:

a) comparing hosts displayed as a nested list

• this will be reproducible after adding missing route

b) Networks > networks > some network > Instances > compare

c) Networks > subnets > some subnet > Instances > compare

d) Networks > Network Routers > some router > Instances > compare

e) Networks > Security Groups > some group > Instances > compare

As it is a slightly different issue, should I open a BZ/issue for this? Thank you.

[hstastna]

Another issue is that accordion with more comparing options is/will be missing for comparing. This is because we limit displaying it just for explorer screens. For me it does not make sense but maybe it has some reason. Maybe we did not allow comparing operation in the past if we were not in an explorer screen. I am not sure. See: https://github.com/ManageIQ/manageiq-ui-classic/blob/master/app/views/layouts/listnav/_compare_sections.html.haml#L1
What do you think about this?

[himdel]

compare method is totally missing (so not just route)

What do you mean?

There should be no compare method - the toolbar always calls it miq_template_compare or vm_compare or image_compare, ems_cluster_compare, host_compare`.

All of those end up calling comparemiq, either via an alias in app/controllers/application_controller/compare.rb or through an explicit switch in button.

So... I guess it could be that a controller implements its own button method and doesn't allow calling comparemiq?

[himdel]

Ah, tested on the first case you mentioned...

The request is POST "/cloud_network/button/14?pressed=instance_compare", parameters: {"miq_grid_checks"=>"132,133", "pressed"=>"instance_compare", "id"=>"14"}

And indeed, CloudNetwork has it's own button implementation which does not have the entry for instance_compare.

So agreed, that's a second bug that needs to be fixed too. This may be as easy as adding a else; super to the switch. But an explicit when 'instance_compare': comparemiq should be what you need there.

[hstastna]

Sorry, my bad, I meant calling comparemiq method for compare operation in some controllers is missing.

[miq-bot]

Checked commit hstastna@37d40fb with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 👍

[himdel]

Fixes comparison in 2 controllers (ems_infra and storage), so merging now :).

There may still be issues in other controllers, both with missing routes or with missing entries in the button function, leaving those for another PR :)

[hstastna]

@miq-bot add_label blocker

[simaishi]

Ivanchuk backport details:

$ git log -1
commit fcb914511a1f65f8441a7989dc542eec2804e07a
Author: Martin Hradil <mhradil@redhat.com>
Date:   Wed Aug 14 14:26:51 2019 +0200

    Merge pull request #5876 from hstastna/No_route_compare_Datastores_VMs

    Add missing routes to compare managed VMs of a Datastore

    (cherry picked from commit 5b2253c2efbc5f80fad280745ff0e8b0ef6535f7)

    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1784179
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1784180
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1784181

[simaishi]

Hammer backport details:

$ git log -1
commit 4d35778571c46d7e60feae2434f549957e98fc44
Author: Martin Hradil <mhradil@redhat.com>
Date:   Wed Aug 14 14:26:51 2019 +0200

    Merge pull request #5876 from hstastna/No_route_compare_Datastores_VMs

    Add missing routes to compare managed VMs of a Datastore

    (cherry picked from commit 5b2253c2efbc5f80fad280745ff0e8b0ef6535f7)

    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1794434
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1794436
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1794438