Bug 1635673

"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.

(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?

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.

(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.

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?

(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.

Could you please tell me how you created the dialog? Is the UI involved at all? And if so, which UI?

(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.

(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.

https://github.com/ManageIQ/manageiq/pull/18061
https://github.com/ManageIQ/manageiq-api/pull/485

The patch worked for me.  Thank you.

New commit detected on ManageIQ/manageiq/hammer:

https://github.com/ManageIQ/manageiq/commit/4b30495e2085c2e8d721d22bd5d87ed592a6662d
commit 4b30495e2085c2e8d721d22bd5d87ed592a6662d
Author:     Greg McCullough <gmccullo>
AuthorDate: Mon Oct 15 09:39:38 2018 -0400
Commit:     Greg McCullough <gmccullo>
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(-)

New commit detected on ManageIQ/manageiq-api/hammer:

https://github.com/ManageIQ/manageiq-api/commit/d394c47a8c23092acfdd3812ce8cbde59c86f485
commit d394c47a8c23092acfdd3812ce8cbde59c86f485
Author:     Brandon Dunne <brandondunne>
AuthorDate: Mon Oct 15 12:40:20 2018 -0400
Commit:     Brandon Dunne <brandondunne>
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(-)

*** Bug 1626595 has been marked as a duplicate of this bug. ***

@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.

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

Hi Parthvi,

Can you supply a reproducer environment for this issue?

Thanks,
Tina

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

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.