Add Cancel button to policy simulation page

[hstastna]

Fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=1659088
https://bugzilla.redhat.com/show_bug.cgi?id=1686617

What:
Add the missing button to the page for policy simulation of VMs/Instances which were displayed as a nested list (displayed from Relationships table of for example selected infra/cloud provider).

Steps to reproduce: (one of possible scenarios)

1. Go to Compute > Infra > Providers and click on some provider
2. Display VMs/Templates from Relationships table (or click on VMs/Templates from dashboard)
3. Choose some of the items, check their checkboxes (at least one item)
4. Choose Policy > Policy Simulation
   => no Cancel button, not able to cancel policy simulation!

Note:
Cancel button is present and not missing, if we access VMs/Instances directly, thru Compute > Infra > VMs or Compute > Clouds > Instances.

TODO:

• test this PR in more places where we can access VMs, Instances, Images - DONE
• display flash message about canceling policy sim - already fixed for non explorer screens (but it still does not appear always, probably some timing issue, but still better than before), still needs to be fixed for explorer ones (for example Compute > Infra > VMs )
  • probably this should be solved by a separate PR
• specs - DONE

Where I tested policy simulation and displaying Cancel button with this PR (and it works there):

• Compute > Infra > Providers > clicking on some provider > displaying VMs/Templates from Relationships table (or clicking on VMs/Templates from dashboard)
• Compute > Infra > Clusters / Deployment Roles > clicking on some cluster > displaying VMs/Templates from Relationships table
• Compute > Infra > VMs (works as usual)
• Compute > Clouds > Providers > clicking on some provider > displaying Instances/Images from Relationships table (or clicking on Instances/Images from dashboard)
• Compute > Clouds > Instances (works as usual)
• the same way for VMs, Instances, Images, Templates displayed from Relationships table of Availability zones, Host Aggregates, Tenants, Flavors, Stacks, Key Pairs, Clusters / Deployment Roles, Hosts / Nodes, Resource Pools, Datastores

---

Before:

After:

Displaying flash message for non explorer screens (not always):

[hstastna]

@miq-bot add_label bug, hammer/yes

[hstastna]

@skateman You can check the spec ;) Thank you! ❇️

[skateman]

@hstastna your fix does the job, however, it adds a lot of complexity to the code. However, I noticed there's a toolbar on the policy simulation page. What do you think about adding a "back" button into the toolbar instead of this complex logic?

Maybe UX won't like that much as it would be a little inconsistent with the other policy simulation screens that have a cancel button on the bottom of the screen. However, if we're here, I think the "back" button is more meaningful here than "cancel" as the policy simulation is a totally passive and noninvasive thing that doesn't do anything to the VMs, just displays some info. From this info-displaying page, you would rather go "back" than "cancel" the simulation when you're done.

Maybe we should bring in other people to discuss this, but I think this could be globally on every policy simulation screen. What do you think?

[hstastna]

@skateman I just added https://github.com/ManageIQ/manageiq-ui-classic/pull/5341/files#diff-dcf973c55609c594de2b7f1c5471d434R84 condition and the next two lines, everything else remains the same there. But I agree with you that it adds more complexity.
Another thing is that there is a bug regarding Back button and policy simulation (and not easy to fix) so I would rather avoid adding another Back button, which may make things worse, at least until the problem would be solved.
Also I don't have enough knowledge yet about how to deal with Back button in the code so from my point of view I cannot say what's better. But of course, you can initiate some discussion with UX and others if you think so. But I am not sure how much time we have for those BZs to fix.

[skateman]

Okay, I am good with this, but can't give you the seal of approval due to the introduced complexity. Someone else should also take a look.

[hstastna]

Thank you, @skateman. Anyway, some another solution came to my mind: Maybe I could create a separate method for canceling policy sim and to call it in the haml => not mixing with the existing code in policy_sim method. Then adding a condition in policy_sim wouldn't be needed, I think.

[hstastna]

@skateman And what do you think now about the solution and its complexity? Thanks in advance for any feedback :)

[miq-bot]

Checked commits hstastna/manageiq-ui-classic@086bb64~...48a8dd5 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
5 files checked, 1 offense detected

**

• 💣 💥 🔥 🚒 - Linter/Haml - Linter::Haml STDERR:

warning: parser/current is loading parser/ruby23, which recognizes
warning: 2.3.6-compliant syntax, but you are running 2.3.3.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.

[hstastna]

@h-kataria @mzazrivec @himdel Could you, please, review the changes? Thanks! :)

[himdel]

LGTM, I think my only issue is with the message itself: "Edit policy simulation was cancelled by the user"

that sounds like you're editing and then saving a policy simulation definition of some kind,
but I think what we're doing on that screen is running a simulation for the particular VM or whatever, right?

So... just "Policy simulation was cancelled by the user" maybe?

[himdel]

But actually, a second issue .. testing on master (infra/vms), I can see a Cancel button already there, in form_buttons_div.

Which matches one of the BZ comments saying that it is there sometimes.

So... if that's the case, we don't need to add another button. We should just fix replace_right_cell to always presenter.show(:form_buttons_div) in that case, or have you already tried that?

[hstastna]

@himdel Cancel button was never displayed if we wanted policy sim on some VMs displayed as a nested list (=> non explorer). And replace_right_cell is not called at all, the method is called only for explorer screens, so I needed to add Cancel button. If I tried to call replace_right_cell for non explorer, many other things did not work and I considered not to be ok to add many extra conditions to hack somehow the method to make it work also for non explorer screens (regarding policy simulation, of course). Also if I tried to set all the variables to make replace_right_cell work for non explorer, many other things did not work later. I think this is not just that something is broken but missing implementation of Cancel button for non explorer, the button was never there.
And the flash message - it was never there, no matter if explorer or non explorer, so I asked Harpreet what to write in the flash message.

[himdel]

Aah, sorry, missed that yours is a fix for the non-explorer version 👍
(agreed no point in trying to use replace_right_cell outside explorer)