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 old angular config provider form with the new ProviderForm #7047

Merged
merged 1 commit into from
Jul 2, 2020

Conversation

skateman
Copy link
Member

@skateman skateman commented May 14, 2020

Depends on: ManageIQ/manageiq-providers-foreman#62
Parent issue: ManageIQ/manageiq#18818

Before:
Screenshot from 2020-05-28 21-28-38

After:
Screenshot from 2020-05-28 21-25-06

This change will allow us to add any other configuration management provider on this form.

@miq-bot add_label pending core

@skateman skateman force-pushed the foreman-provider-form branch from b28dc2d to ddeeb4c Compare May 28, 2020 19:24
@miq-bot
Copy link
Member

miq-bot commented May 28, 2020

Checked commit skateman@ddeeb4c with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.28.0, and yamllint
4 files checked, 4 offenses detected

app/views/ems_configuration/edit.html.haml

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

app/views/ems_configuration/new.html.haml

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

@skateman skateman requested review from h-kataria and himdel May 28, 2020 19:29
@skateman skateman changed the title [WIP] Replace old angular config provider form with the new ProviderForm Replace old angular config provider form with the new ProviderForm May 28, 2020
@miq-bot miq-bot removed the wip label May 28, 2020
@skateman
Copy link
Member Author

Note that there's an issue with setting the name in the provider PR. But other than that, everything should be working.

@h-kataria
Copy link
Contributor

@skateman found couple of issues, am missing something missing in my environment, I have ManageIQ/manageiq-providers-foreman#62 checked out locally

  • when I try to add a dummy CAM/Foreman provider, I get API pop up error message
    Data {"error":{"kind":"bad_request","message":"Could not create the new provider - unknown attribute 'base_url' for Endpoint.","klass":"Api::BadRequestError"}}

  • When i tried to edit existing CAM/Foreman providers, URL/Username/Password fields do not get populated even tho i have values for those saved, see screenshots

Screenshot from 2020-05-28 16-33-49

Screenshot from 2020-05-28 16-32-58

@skateman
Copy link
Member Author

@h-kataria forgot to mention that ManageIQ/manageiq-providers-foreman#62 needs a rebase, but asked Adam to do that on the PR as well.

@h-kataria
Copy link
Contributor

@skateman after I rebased ManageIQ/manageiq-providers-foreman#62 i edited an existing Foreman Provider, this time all fields were populated in the form, but after i edited the zone of the provider and pressed save, that cleared my saved url/username/password data.

@skateman
Copy link
Member Author

@h-kataria yeah, it's a delegation error in the provider PR, I'll make some suggestions there.

@skateman
Copy link
Member Author

skateman commented Jul 2, 2020

@h-kataria this is ready for a final round, all deps are in

Copy link
Member

@agrare agrare left a comment

Choose a reason for hiding this comment

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

👍 works for me

@h-kataria h-kataria merged commit 47ed9f5 into ManageIQ:master Jul 2, 2020
@skateman skateman deleted the foreman-provider-form branch July 2, 2020 12:58
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.

5 participants