Bug 1635673 - When ordering a service via the API the service dialog is not executed
Summary: When ordering a service via the API the service dialog is not executed
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Red Hat CloudForms Management Engine
Classification: Red Hat
Component: API
Version: 5.9.4
Hardware: x86_64
OS: Linux
high
high
Target Milestone: GA
: 5.10.0
Assignee: Tina Fitzgerald
QA Contact: Parthvi Vala
URL:
Whiteboard:
: 1626595 (view as bug list)
Depends On:
Blocks: 1639413
TreeView+ depends on / blocked
 
Reported: 2018-10-03 13:17 UTC by Patrick Rutledge
Modified: 2019-02-12 16:53 UTC (History)
9 users (show)

Fixed In Version: 5.10.0.20
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
: 1639413 1648819 (view as bug list)
Environment:
Last Closed: 2019-02-12 16:53:08 UTC
Category: ---
Cloudforms Team: CFME Core
Target Upstream Version:


Attachments (Terms of Use)

Description Patrick Rutledge 2018-10-03 13:17:15 UTC
Description of problem:
When ordering a service from the service catalog via the API, none of the defaults defined in the catalog item's service dialog are applied and no dynamic dialog elements are executed.  This used to work in previous versions of CFME.

Version-Release number of selected component (if applicable):
cfme-5.9.4.7-1.el7cf.x86_64

How reproducible:
Create a service dialog with at least one static element that has a default and one dynamic element that executes a method to return a default.

Steps to Reproduce:
1. Create a service dialog
2. Add at least one static element that has a default 
2. Add one dynamic element that executes a method to return a default
3. Order the service via the api

Actual results:
1. When monitoring the automation.log you see that the dynamic element does not execute the method.
2. Nothing is set in $evm.root['dialog_<var>'] from the dynamic element
3. Nothing is set in $evm.root['dialog_<var>'] from the static element default

Expected results:
1. Dynamic elements should run the specified method when the service is ordered and the default set by the method should be passed to $evm.root['dialog_<var>']
2. Static elements should pass the default set in the service dialog to $evm.root['dialog_<var>']

Additional info:

Comment 3 drew uhlmann 2018-10-04 15:46:59 UTC
"When ordering a service from the service catalog via the API, none of the defaults defined in the catalog item's service dialog are applied and no dynamic dialog elements are executed." 

I believe that this is two separate issues and I'd like to address one of them first: the expected behavior of the API is that it will not run Automate. I don't believe the expectation that a dynamic dialog field would be executed from an API call is valid. 

The part of this ticket that I'm looking into is the fact that the default set through the UI won't show up in the API request.

Comment 4 Patrick Rutledge 2018-10-04 15:56:35 UTC
(In reply to drew uhlmann from comment #3)
> "When ordering a service from the service catalog via the API, none of the
> defaults defined in the catalog item's service dialog are applied and no
> dynamic dialog elements are executed." 
> 
> I believe that this is two separate issues and I'd like to address one of
> them first: the expected behavior of the API is that it will not run
> Automate. I don't believe the expectation that a dynamic dialog field would
> be executed from an API call is valid. 
> 
> The part of this ticket that I'm looking into is the fact that the default
> set through the UI won't show up in the API request.

It worked before I upgraded to CF 4.6.  It is imperative to have the dynamic elements launch when the API orders a service.  Basically all of my business logic is expecting this to work.  I need to provide input into the service before it is ordered and a lot of this comes from the dynamic elements.  This would be taking away necessary functionality from the product.  Is there justification for removing this functionality?  Was it causing some sort of problem?

Comment 5 drew uhlmann 2018-10-04 16:12:30 UTC
The order process used to call Automate. That was considered a bug, and so we removed it. I think this part is working as designed. There's a change in behavior that requires additional api call(s). 

"The correct way to do this is you have to go through the whole process:
you should request the dialog, that would then run automate and give you back the field(s) with their defaults

and then if you want to change anything and you know fields depend on refreshes, you'd have to make those requests too

and then finally once you've got all the data, that's when you call the order API endpoint" according to Erik, our resident dialog expert.

Comment 6 Patrick Rutledge 2018-10-04 17:04:40 UTC
(In reply to drew uhlmann from comment #5)
> The order process used to call Automate. That was considered a bug, and so
> we removed it. I think this part is working as designed. There's a change in
> behavior that requires additional api call(s). 
> 
> "The correct way to do this is you have to go through the whole process:
> you should request the dialog, that would then run automate and give you
> back the field(s) with their defaults
> 
> and then if you want to change anything and you know fields depend on
> refreshes, you'd have to make those requests too
> 
> and then finally once you've got all the data, that's when you call the
> order API endpoint" according to Erik, our resident dialog expert.

Ok I am willing to work with this, but it is not documented at https://access.redhat.com/documentation/en-us/red_hat_cloudforms/4.6/html/red_hat_cloudforms_rest_api/chap_examples#order-a-single-service-from-service-catalog-1 as to how to request a dialog via the API or even the need to do so.  So maybe this is a documentation problem.  If you can point me to how to request a dialog for this sort of use case as well as update the API doc I would be grateful.

Comment 7 drew uhlmann 2018-10-04 17:17:37 UTC
The endpoint to request the dialog is: whatever/api/service_dialogs/<id>, and
Erik also suggests that "ideally, you also pass in the resource action id, the target id (a service template ID), and the target type (in this case, "service_template"). In order to get those, you would have to query the service_templates endpoint". I'm making a note to look at getting the docs changed, thanks. 

To your other issue, I'm wondering how you created the service dialog with the static field that has a default -- unless the additional api calls will resolve both of the issues you listed?

Comment 8 Patrick Rutledge 2018-10-04 17:43:47 UTC
(In reply to drew uhlmann from comment #7)
> The endpoint to request the dialog is: whatever/api/service_dialogs/<id>, and
> Erik also suggests that "ideally, you also pass in the resource action id,
> the target id (a service template ID), and the target type (in this case,
> "service_template"). In order to get those, you would have to query the
> service_templates endpoint". I'm making a note to look at getting the docs
> changed, thanks. 
> 
> To your other issue, I'm wondering how you created the service dialog with
> the static field that has a default -- unless the additional api calls will
> resolve both of the issues you listed?

Ok I will look into that.  It would help to have an actual example of the JSON as it looks pretty complicated.

As for the static element, basically an example would be a pull down with 3 possible options and I set one of them as the default.  I would like that default to show up if its not overridden by the API call.  And thats not currently happening.

Comment 9 drew uhlmann 2018-10-04 17:49:22 UTC
Could you please tell me how you created the dialog? Is the UI involved at all? And if so, which UI?

Comment 10 Patrick Rutledge 2018-10-04 17:56:41 UTC
(In reply to drew uhlmann from comment #9)
> Could you please tell me how you created the dialog? Is the UI involved at
> all? And if so, which UI?

The dialog was created via the standard CloudForms UI under Automation -> Customization.  It was created in a much older version (CF 4.1) and carried over via the upgrade.

Comment 11 Patrick Rutledge 2018-10-04 19:52:29 UTC
(In reply to Patrick Rutledge from comment #8)
> (In reply to drew uhlmann from comment #7)
> > The endpoint to request the dialog is: whatever/api/service_dialogs/<id>, and
> > Erik also suggests that "ideally, you also pass in the resource action id,
> > the target id (a service template ID), and the target type (in this case,
> > "service_template"). In order to get those, you would have to query the
> > service_templates endpoint". I'm making a note to look at getting the docs
> > changed, thanks. 
> > 
> > To your other issue, I'm wondering how you created the service dialog with
> > the static field that has a default -- unless the additional api calls will
> > resolve both of the issues you listed?
> 
> Ok I will look into that.  It would help to have an actual example of the
> JSON as it looks pretty complicated.
> 
> As for the static element, basically an example would be a pull down with 3
> possible options and I set one of them as the default.  I would like that
> default to show up if its not overridden by the API call.  And thats not
> currently happening.

So calling /api/service_dialogs/<id> pulls the default only for the static elements, however it does not execute anything in automate or provide the default set as such.  Having gone through the process of finding the dialog and calling it separately, I would like to submit that running automate when a service catalog is ordered via an API call is NOT a bug and it should be re-instated.  I don't see how this functionality could have caused any harm whatsoever.  Having to jump through these extra hoops is really unnecessarily complex.

Comment 15 Patrick Rutledge 2018-10-15 12:24:18 UTC
The patch worked for me.  Thank you.

Comment 16 CFME Bot 2018-10-15 13:45:42 UTC
New commit detected on ManageIQ/manageiq/hammer:

https://github.com/ManageIQ/manageiq/commit/4b30495e2085c2e8d721d22bd5d87ed592a6662d
commit 4b30495e2085c2e8d721d22bd5d87ed592a6662d
Author:     Greg McCullough <gmccullo@redhat.com>
AuthorDate: Mon Oct 15 09:39:38 2018 -0400
Commit:     Greg McCullough <gmccullo@redhat.com>
CommitDate: Mon Oct 15 09:39:38 2018 -0400

    Merge pull request #18061 from d-m-u/bz1635673

    Add flag for init of defaults in fields

    (cherry picked from commit 2ab08ae6623053f50342dcecdbd3f5f87eadd04f)

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

 app/models/resource_action_workflow.rb | 2 +-
 app/models/service_template.rb | 8 +-
 config/settings.yml | 1 +
 spec/models/resource_action_workflow_spec.rb | 10 +
 spec/models/service_template_spec.rb | 23 +
 5 files changed, 36 insertions(+), 8 deletions(-)

Comment 18 CFME Bot 2018-10-15 16:47:40 UTC
New commit detected on ManageIQ/manageiq-api/hammer:

https://github.com/ManageIQ/manageiq-api/commit/d394c47a8c23092acfdd3812ce8cbde59c86f485
commit d394c47a8c23092acfdd3812ce8cbde59c86f485
Author:     Brandon Dunne <brandondunne@hotmail.com>
AuthorDate: Mon Oct 15 12:40:20 2018 -0400
Commit:     Brandon Dunne <brandondunne@hotmail.com>
CommitDate: Mon Oct 15 12:40:20 2018 -0400

    Merge pull request #485 from d-m-u/bz1635673

    Add flag to initialize field default values

    (cherry picked from commit 73eee92f0137aa8546c95e1e25a44e51e901cf90)

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

 app/controllers/api/mixins/service_templates.rb | 13 +-
 spec/requests/service_catalogs_spec.rb | 25 +-
 spec/requests/service_templates_spec.rb | 23 +-
 3 files changed, 50 insertions(+), 11 deletions(-)

Comment 19 Satoe Imaishi 2018-11-05 21:22:32 UTC
*** Bug 1626595 has been marked as a duplicate of this bug. ***

Comment 22 drew uhlmann 2018-11-12 13:32:33 UTC
@Parthvi, like I said in comment 13, this requires a change to the settings in the config file and I don't see a mention of that in your reproducer description. Could you please check to see that you followed my instructions? It won't work without that setting, and from what I heard from Patrick, it worked for him.

Comment 23 Parthvi Vala 2018-11-12 13:53:49 UTC
Hey Drew,

I applied the patch and it works as expected now. But here's the thing: I tested the same query successfully, without any additional patchwork on appliance 5.10.0.20, since the changes introduced to `app/controllers/api/mixins/service_templates.rb` with PR #485 were already present, but that's not the case with 5.10.0.23. Don't you think it will be better to include the changes for 5.10.0.23 as well?

Thanks,
Parthvi

Comment 24 Tina Fitzgerald 2018-11-12 16:22:57 UTC
Hi Parthvi,

Can you supply a reproducer environment for this issue?

Thanks,
Tina

Comment 25 Parthvi Vala 2018-11-13 06:03:59 UTC
Hey Tina,

As I mentioned in https://bugzilla.redhat.com/show_bug.cgi?id=1635673#c23, applying the patch gave me the expected output, but shouldn't the patch be included in 5.10.0.23 as well, since it was already present in the previous versions?

Thanks,
Parthvi

Comment 26 drew uhlmann 2018-11-14 16:14:18 UTC
Hey Parthvi, you have to change the config setting on that appliance in order for this to work as expected. Your appliance has that setting set to false. Please retest with the proper configuration. 

The code got refactored, and it's correct. If you look in that file (https://github.com/ManageIQ/manageiq-api/pull/476/files#diff-3db7e02fbc903162f12ccf71cbc6bab6R32) you'll see that the correct code just lives in a new method at the bottom of the file. 

Please retest.


Note You need to log in before you can comment on or make changes to this bug.