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

Redesign the Provider Forms #3097

Closed
martinpovolny opened this issue Dec 18, 2017 · 7 comments
Closed

Redesign the Provider Forms #3097

martinpovolny opened this issue Dec 18, 2017 · 7 comments

Comments

@martinpovolny
Copy link
Member

The provider forms are an area of frequent complaints of contributors.

The code has too many if/else/case/when statements, is hard to maintain, hard to change.

We need:

  • some declarative way of specifying types of credentials needed for each provider type,
  • components to implement the particular field types and
  • generic code to wrap it all up.

A provider team should not need to touch the UI code if all the field types are already implemented. Also when the UI technology changes Vue/Angular/React/whatever.js the provider teams need not be involved.

The provider teams would be only owners of the definition and if a new field type is needed they (someone) would need to add support for that new field type. But the rest would be part of the UI and should not be needed to change. (Well we might need some provider specific function to map the data from the HTML form to a correct message for the API call for the provider registration etc.)

I think this: https://github.com/ManageIQ/manageiq-ui-classic/blob/master/app/assets/javascripts/controllers/ansible_credentials/ansible_credentials_form_controller.js and https://github.com/ManageIQ/manageiq-ui-classic/blob/master/app/assets/javascripts/components/ansible-credential-options.js
is a nice start in that direction.

Ping @AparnaKarve, @himdel, @cben

See also:

@himdel
Copy link
Contributor

himdel commented Dec 18, 2017

Yes 👍

We may want to start with Credentials because those are separate from the rest anyway, but this goes equally well for all the rest of the fields.

Definitely need a declarative method ruby-side which will just return a list of allowed authentication types for that provider, with info on which are optional and which are required. (Can there be a "required FOO or BAR"?)

The JS code should never know or use the provider type itself, except to reach this JSON endpoint and retrieve the data.

(After that, the same approach can likely be used for other common fields.)

@himdel
Copy link
Contributor

himdel commented Dec 18, 2017

(Also, AnsibleCredentialsFormComponent was written pretty much for this purpose, so we should use that, getting the field list is already handled outside of that component, so it should be trivially reusable.)

@miq-bot
Copy link
Member

miq-bot commented Jun 25, 2018

This issue has been automatically marked as stale because it has not been updated for at least 6 months.

If you can still reproduce this issue on the current release or on master, please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions!

@JPrause
Copy link
Member

JPrause commented Jan 28, 2019

@martinpovolny is this still a valid issue? If yes, lease remove the stale label. If not can you close.
If there's no update by next week, I'll be closing this issue.

@himdel
Copy link
Contributor

himdel commented Jan 28, 2019

Definitely is :).

@miq-bot remove_label stale

@mfeifer
Copy link
Contributor

mfeifer commented Dec 13, 2019

related to ManageIQ/manageiq#18818

@gtanzillo
Copy link
Member

gtanzillo commented Feb 22, 2021

This should be complete with the implementation of DDF form for providers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants