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

Replace cloud provider forms with an API-driven DDF approach #6698

Merged
merged 2 commits into from
May 28, 2020

Conversation

skateman
Copy link
Member

@skateman skateman commented Feb 21, 2020

Depends on: ManageIQ/manageiq-api#741 ManageIQ/manageiq-api#830
Replaces: #6402
Parent issue: ManageIQ/manageiq#18818

The following providers are already supported:

Known issues:

@miq-bot add_label react, enhancement, compute/cloud

Copy link
Contributor

@Hyperkid123 Hyperkid123 left a comment

Choose a reason for hiding this comment

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

So far so good. I am excited about the tests 😉

@skateman
Copy link
Member Author

I'm sure I'm not able to write tests, so I will rely on the experts 😁 @brumik can you help me in a followup pr?

@brumik
Copy link

brumik commented Feb 25, 2020

I think so :)

@skateman
Copy link
Member Author

skateman commented May 27, 2020

When adding a new one..

search

Search... is wrong as that select is not searchable.
But it's not in providers-amazon, it doesn't say Search, it seems to be a ddf default - maybe we should override the default to Choose? (globally in miq ddf configuration I mean)

This is not related to this PR, also happens on master:
Screenshot from 2020-05-27 09-20-25

It just needs a placeholder property in each provider. Gonna go through them all, not related to this PR.

EDIT: maybe it's better if we set it globally, but still not related to this PR

@Hyperkid123
Copy link
Contributor

@skateman you can use https://data-driven-forms.org/renderer/global-component-props if you want to override global props.

@skateman
Copy link
Member Author

@skateman you can use https://data-driven-forms.org/renderer/global-component-props if you want to override global props.

That's great, but does this work on DDFv1?

@Hyperkid123
Copy link
Contributor

@skateman oh right sorry. No its v2 feature. I keep forgetting.

@miq-bot
Copy link
Member

miq-bot commented May 27, 2020

This pull request is not mergeable. Please rebase and repush.

@skateman skateman changed the title [WIP] Replace cloud provider forms with an API-driven DDF approach Replace cloud provider forms with an API-driven DDF approach May 27, 2020
@skateman skateman requested a review from h-kataria May 27, 2020 15:10
@miq-bot miq-bot removed the wip label May 27, 2020
@h-kataria
Copy link
Contributor

h-kataria commented May 27, 2020

@skateman

  • Add new Provider form should have "Add/Cancel" buttons instead of "Save/Reset/Cancel", to be consistent with other forms in the product
  • Flash message is missing when after changes in the Edit provider form are Reset
  • Just wondering if form buttons should be disabled while Validation is in process on Edit provider form.

@skateman
Copy link
Member Author

@skateman

  • Add new Provider form should have "Add/Cancel" buttons instead of "Save/Reset/Cancel", to be consistent with other forms in the product

Fixed

  • Flash message is missing when after changes in the Edit provider form are Reset

I'm not entirely sure why is this necessary, the reset doesn't do a server roundtrip anymore so there's no way it would fail due to a server error.

  • Just wondering if form buttons should be disabled while Validation is in process on Edit provider form.

Maybe it's possible to do it, but I'd like to take it into a separate PR and especially after we are on DDFv2.

@h-kataria
Copy link
Contributor

@skateman

  • Add new Provider form should have "Add/Cancel" buttons instead of "Save/Reset/Cancel", to be consistent with other forms in the product

Fixed

  • Flash message is missing when after changes in the Edit provider form are Reset

I'm not entirely sure why is this necessary, the reset doesn't do a server roundtrip anymore so there's no way it would fail due to a server error.

  • Just wondering if form buttons should be disabled while Validation is in process on Edit provider form.

Maybe it's possible to do it, but I'd like to take it into a separate PR and especially after we are on DDFv2.

I feel the Save should not be allowed while credentials are being validated, that will give user an opportunity to change/fix them in case validation failed. Let's do the disabling of Save button in a separate PR

I am not sure about removing the Reset flash message, we do have that in other DDF forms tho, is there a plan to remove it from all new forms?

@skateman
Copy link
Member Author

Okay, I'll put it there...but will create an issue about getting rid of this.

@miq-bot
Copy link
Member

miq-bot commented May 28, 2020

Checked commits skateman/manageiq-ui-classic@48e04b3~...4e9d756 with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.28.0, and yamllint
3 files checked, 3 offenses detected

app/views/ems_cloud/edit.html.haml

  • ⚠️ - Line 1 - Layout/TrailingBlankLines: Final newline missing.
  • ⚠️ - Line 1 - Line is too long. [190/160]

app/views/ems_cloud/new.html.haml

  • ⚠️ - Line 3 - Layout/TrailingBlankLines: Final newline missing.

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.

8 participants