VM custom button dialog - use the VM instead of the parent service #1542

himdel · 2019-04-30T15:48:55Z

Triggering a VM-based custom button on a Service's VM Resource causes the right dialog to appear..

But the inital data assumes the associated resource is the service, not the VM.
Subsequent field refresh requests and submit already use the VM, but the initial load was missed in #1022.

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

Triggering a VM-based custom button on a Service's VM Resource causes the right dialog to appear.. But the inital data assumes the associated resource is the service, not the VM. Subsequent field refresh requests and submit already use the VM, but the initial load was missed in #1022. Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1687061

himdel · 2019-04-30T15:52:44Z

Have a Custom Button for VMs, associated with a service, which uses the VM's data to populate dropdown options (see the bz for exact steps).

Go to My Services
find a service with at least 1 VM associated
open the service detail (/services/4)
open the vm detail (/services/4/resource=2)
toolbar kebab custom button (goes to /services/4/custom_button_details)

does get /api/service_dialogs/110?expand=resources&attributes=content&resource_action_id=6572&target_id=4&target_type=service
dropdown says Script Error

after this, it does
get /api/service_dialogs/110?expand=resources&attributes=content&resource_action_id=6572&target_id=2&target_type=vm
dropdown is populated

h-kataria · 2019-04-30T18:16:52Z

@eclarizio do you have a setup to review/test this?

eclarizio

@h-kataria Unfortunately, I don't have a current environment to test this, but from what @himdel is saying about how it works, this does look like it will fix the issue.

@himdel Can you add a spec for this? It doesn't look like the initialization test doesn't have a vmId in the definition so it will never hit this code to verify.

himdel · 2019-05-02T08:54:07Z

Thanks @eclarizio, working on a spec now...

As for the environment, yeah, it's not exactly trivial, this should help:

So, steps to reproduce - setup:

Automation > Automate > Explorer
choose a class
tab Methods
toolbar Configuration > Add a new Method
type inline
name bz1687061
data:
  @vm = $evm.root['vm']
  dialog_hash = {}
  dialog_hash[@vm.id] = @vm.name
  $evm.object['default_value'] = dialog_hash.first[0]
  $evm.object['values'] = dialog_hash
save

tab Instances
toolbar Configuration > Add a new Instance
name bz1687061
runcheck value: bz1687061

Automation > Automate > Customization
accordion Service Dialogs
toolbar Congfiguration > Add a new Dialog
name bz1687061
add a *Dropdown*, edit
  Field Information
    Dynamic Yes
  Options
    Entry Point: .../bz1687061
  save
save

accordion Buttons
create/choose a button group under "VM and Instance"
toolbar Configuration > Add a new Button
text bz1687061
dialog bz1687061
save

Have a service which is associated with a least 1 VM/Instance:
  (in rails console)
  s = Service.first
  v = Vm.first
  s.add_resource(v)
  s.save!

Then,

In ui-service:

My Services
open the service detail (/services/4/resource=2)
toolbar kebab custom button bz1687061 (goes to /services/4/custom_button_details)

does get /api/service_dialogs/110?expand=resources&attributes=content&resource_action_id=6572&target_id=4&target_type=service
dropdown says Script Error

himdel · 2019-05-02T15:01:29Z

Tests should be ready :)

eclarizio

I think one more spec to be added, but otherwise it's looking good 👍

eclarizio · 2019-05-02T17:02:24Z

client/app/states/services/custom_button_details/custom_button_details.state.spec.js

+      id: 213
+    }
+
+    it('sends target_type=vm service dialog query on init', (done) => {


I think we're still missing the spec for when the vmId doesn't exist, right? There are still cases where the service dialogs endpoint is going to request with the target_type of 'service' and the target_id with that service's id.

I guess maybe this spec didn't exist before, but I think it needs to be added now that we have these two different calls happening.

Good catch 👍. The tests should probably be reorganized a bit to be more symmetrical, and looks like we have some SUI test inconsistencies (3 tests randomly in jest not karma), can't use object spread there, etc.

But for now, I just added a similar test for init without vmId :).

…Id and right target for service dialog query as opposed to always serviceId had to wrap the rest in another describe, to prevent beforeEach from initializing the controller before the right setup and pulled out a few of the constants up, to DRY up the tests a bit

miq-bot · 2019-05-03T13:47:05Z

Checked commits https://github.com/himdel/manageiq-ui-service/compare/41a30d5d3a34c76ecb9d9b19ec7b5351f7fc6158~...8d8c02de972ce7013cd4fe842d0daef5c25c57d2 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 🍰

eclarizio

Nice 🎉

VM custom button dialog - use the VM instead of the parent service (cherry picked from commit 61c583c) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1722817

simaishi · 2019-06-21T12:10:26Z

Hammer backport details:

$ git log -1
commit 5fe64d26c33c6e1af15b6194fd54cd2b268f4096
Author: Harpreet Kataria <[email protected]>
Date:   Fri May 3 11:24:30 2019 -0400

    Merge pull request #1542 from himdel/bz1687061-custombutton
    
    VM custom button dialog - use the VM instead of the parent service
    
    (cherry picked from commit 61c583c5a7cf63d86a6c30166f029559e060e8f1)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1722817

himdel added bug hammer/yes labels Apr 30, 2019

h-kataria requested a review from eclarizio April 30, 2019 18:17

eclarizio suggested changes May 1, 2019

View reviewed changes

eclarizio suggested changes May 2, 2019

View reviewed changes

eclarizio approved these changes May 3, 2019

View reviewed changes

h-kataria approved these changes May 3, 2019

View reviewed changes

h-kataria self-assigned this May 3, 2019

h-kataria added this to the Sprint 111 Ending May 13, 2019 milestone May 3, 2019

h-kataria merged commit 61c583c into ManageIQ:master May 3, 2019

himdel deleted the bz1687061-custombutton branch May 3, 2019 15:25

simaishi added hammer/backported and removed hammer/yes labels Jun 21, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

VM custom button dialog - use the VM instead of the parent service #1542

VM custom button dialog - use the VM instead of the parent service #1542

himdel commented Apr 30, 2019

himdel commented Apr 30, 2019 •

edited

Loading

h-kataria commented Apr 30, 2019

eclarizio left a comment

himdel commented May 2, 2019

himdel commented May 2, 2019

eclarizio left a comment

eclarizio May 2, 2019

himdel May 3, 2019

miq-bot commented May 3, 2019

eclarizio left a comment

simaishi commented Jun 21, 2019

VM custom button dialog - use the VM instead of the parent service #1542

VM custom button dialog - use the VM instead of the parent service #1542

Conversation

himdel commented Apr 30, 2019

himdel commented Apr 30, 2019 • edited Loading

h-kataria commented Apr 30, 2019

eclarizio left a comment

Choose a reason for hiding this comment

himdel commented May 2, 2019

himdel commented May 2, 2019

eclarizio left a comment

Choose a reason for hiding this comment

eclarizio May 2, 2019

Choose a reason for hiding this comment

himdel May 3, 2019

Choose a reason for hiding this comment

miq-bot commented May 3, 2019

eclarizio left a comment

Choose a reason for hiding this comment

simaishi commented Jun 21, 2019

himdel commented Apr 30, 2019 •

edited

Loading