Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add dynamic dropdown list support for orchestration service dialog #12693

Conversation

gberginc
Copy link
Contributor

This patchs adds a new orchestration template parameter constraint
referencing the required method from the automate. Contrary to stack
parameters of the orchestration template service dialog where dynamic
dropdowns are supported via resource_actions. However, for additional
template parameters, the values for the presented dropdowns are created
using a static list of values (static at the time of dialog creation).

With this patch, it is possible to construct a parameter whose allowed
values are retrieved when the dialog is opened, i.e. when the user wants
to order the orchestration service.

The patch is required in order to allow the VMware vCloud provider to
create dropdowns for the available networks that can be set for each VM
that is part of the vApp template (orchestration template).

@miq-bot add_label orchestration, ui, enhancement

@gberginc
Copy link
Contributor Author

@bzwei This is the change that I need in order to continue #12273. Although I thought I resolved the blocking issue in #12570, it turned out that the latter actually filled the values at the time of dialog creation. This means that if the underlying networks have changed in VMware vCloud, the list of networks available in the dropdown would not be updated.

That's why I introduced another constraint type that only sets the automate method which will then dynamically fill the dropdown, when shown.

I understand this kind of change might be difficult to accept, but I hope we can do it somehow. Let me know if improvements are needed.

/cc @agrare This is the one I mentioned yesterday in the call.

dropdown = parameter.constraints.detect { |c| c.kind_of? OrchestrationTemplate::OrchestrationParameterAllowed }
checkbox = parameter.constraints.detect { |c| c.kind_of? OrchestrationTemplate::OrchestrationParameterBoolean } unless dropdown
end
if dropdown
if dynamic_dropdown
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we may write something like

if parameter.contrains
  dynamic_dropdown = parameter.constaints.detect ...
  return create_parameter_dynamic_dropdown_list(...) if dynamic_dropdown

  dropdown = ...
  return create_parameter_dropdown_list(...) if dropdown

  checkbox = ...
  return create_parameter_checkbox(...) if checkbox
end

create_parameter_text_field(...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion @bzwei!

@gberginc gberginc force-pushed the orchestration/add_automate_action_support_for_dropdowns branch from b1674ab to bb28408 Compare November 17, 2016 11:40
@chessbyte chessbyte changed the title Add dynaimc dropdown list support for orchestration service dialog Add dynamic dropdown list support for orchestration service dialog Nov 17, 2016
@miq-bot
Copy link
Member

miq-bot commented Nov 17, 2016

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

@gberginc gberginc force-pushed the orchestration/add_automate_action_support_for_dropdowns branch from bb28408 to 4e32ab1 Compare November 18, 2016 14:47
@gberginc
Copy link
Contributor Author

@miq-bot add_label euwe/no

@gberginc
Copy link
Contributor Author

@bzwei this is now rebased to include the changes from #12570.

@@ -0,0 +1,5 @@
class OrchestrationTemplate
class OrchestrationParameterFqname < OrchestrationParameterConstraint
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OrchestrationParameterAllowedDynamic is a better name. There are other dynamic types of controls that we may add in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think the attr_accessor fqname should also be renamed? I had a bit of a problem with choosing the proper name. fqname is used in the resource_action of the dialog field.

Patch is updated with the suggested name change.

This patchs adds a new orchestration template parameter constraint
referencing the required method from the automate. Contrary to stack
parameters of the orchestration template service dialog where dynamic
dropdowns are supported via `resource_actions`. However, for additional
template parameters, the values for the presented dropdowns are created
using a static list of values (static at the time of dialog creation).

With this patch, it is possible to construct a parameter whose allowed
values are retrieved when the dialog is opened, i.e. when the user wants
to order the orchestration service.

The patch is required in order to allow the VMware vCloud provider to
create dropdowns for the available networks that can be set for each VM
that is part of the vApp template (orchestration template).

Signed-off-by: Gregor Berginc <[email protected]>
@gberginc gberginc force-pushed the orchestration/add_automate_action_support_for_dropdowns branch from 4e32ab1 to 788d12f Compare November 18, 2016 17:54
@miq-bot
Copy link
Member

miq-bot commented Nov 18, 2016

Checked commit xlab-si@788d12f with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
3 files checked, 5 offenses detected

app/services/orchestration_template_dialog_service.rb

spec/services/orchestration_template_dialog_service_spec.rb

@@ -0,0 +1,5 @@
class OrchestrationTemplate
class OrchestrationParameterAllowedDynamic < OrchestrationParameterConstraint
attr_accessor :fqname
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fqname is fine to me.

@agrare
Copy link
Member

agrare commented Nov 18, 2016

@bzwei ready to merge?

@bzwei
Copy link
Contributor

bzwei commented Nov 18, 2016

👍

@agrare agrare merged commit 7d18e84 into ManageIQ:master Nov 18, 2016
@agrare agrare added this to the Sprint 50 Ending Dec 5, 2016 milestone Nov 18, 2016
@agrare
Copy link
Member

agrare commented Nov 18, 2016

Great job @gberginc

@tadeboro tadeboro deleted the orchestration/add_automate_action_support_for_dropdowns branch March 30, 2018 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants