[WIP] Migrate add cloud provider form to data driven forms #5542

Hyperkid123 · 2019-05-07T11:54:14Z

Changes

Implemented in React (data-driven-forms)
Uses API to collect and submit data
Uses fields name spacing to avoid field names collisions and rewriting when changing provider type
- if you set provider specific field it will be store under that type
- openstack.userid for instance

Dependencies

TO DO

Hyperkid123 · 2019-05-07T11:55:12Z

@miq-bot assign @martinpovolny
@miq-bot add_label reac
cc @skateman @himdel @rvsia @h-kataria

@martinpovolny can you add it to the React project?

miq-bot · 2019-05-07T12:01:33Z

@Hyperkid123 Cannot apply the following label because they are not recognized: reac

himdel · 2019-05-07T12:03:30Z

app/javascript/components/cloud-provider/cloud-ec2-provider.schema.js

+    azure_tenant_id: get(azureValues, 'uid_ems'),
+    subscription: get(azureValues, 'subscription'),
+    zone: 'default',
+    default_url: get(azureValues, 'endpoints[0].url'),


why not just azureValues.endpoints[0].url?

All those get calls are suprising, given the right side is a constant.

They might not be inside the form state. Not all of the nested fields are required and if you don;t change it it will be undefined. I agree there is a lot of gets but that whole validation call will change and will be via API. Its just a placeholder for now.

himdel · 2019-05-07T12:06:27Z

app/javascript/forms/mappers/formsFieldsMapper.jsx

@@ -12,6 +24,9 @@ const fieldsMapper = {
  'validate-credentials': AsyncCredentials,
  'credentials-password-edit': EditSecretField,
  'secret-switch-field': SecretSwitchField,
+  wizard: MiqWizard,
+  'provider-initial-step': InitialWizardStep,
+  summary,


summary is not a good generic name for a div with a title and some specific json.

Since this would affect any new form, maybe this should be JsonSummary or something?

ah this is left over it will be deleted. First iteration was in wizard but we scrapped that.

himdel · 2019-05-07T12:06:31Z

app/javascript/forms/mappers/formsFieldsMapper.jsx

+
+const summary = ({ formOptions }) => (
+  <div>
+    <h1>Summary</h1>


himdel · 2019-05-07T12:06:49Z

app/javascript/packs/component-definitions-common.js

@@ -44,3 +45,4 @@ ManageIQ.component.addReact('CatalogForm', CatalogForm);
 ManageIQ.component.addReact('Breadcrumbs', Breadcrumbs);
 ManageIQ.component.addReact('TaggingWrapperConnected', TaggingWrapperConnected);
 ManageIQ.component.addReact('RemoveCatalogItemModal', RemoveCatalogItemModal);
+ManageIQ.component.addReact('CloudProviderForm', CloudProviderForm);


sort please :)

himdel · 2019-05-07T12:07:13Z

package.json

@@ -64,7 +64,7 @@
    "numeral": "~2.0.6",
    "patternfly": "~3.59.1",
    "patternfly-bootstrap-treeview": "~2.1.7",
-    "patternfly-react": "2.10.1",
+    "patternfly-react": "2.30.6",


exact version, no ~ ?

himdel · 2019-05-07T12:18:19Z

app/javascript/components/cloud-provider/validators.js

@@ -0,0 +1,29 @@
+
+const isHostname = function(value) {
+  return /^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9-]*[a-zA-Z0-9])\.)*([A-Za-z0-9][A-Za-z0-9-]*[A-Za-z0-9])$/.test(value);


This doesn't look right.

You probably want something like \p{L} instead of a-zA-Z. Otherwise, this won't match any national domains...

> isHostname('♡.com') false

Yet, http://♡.com exists and is a valid domain.

EDIT: the full list of valid unicode chars is probably here https://unicode.org/faq/idn.html

EDIT2: that said, this is fine if we require people to punycode-convert the domains first

himdel · 2019-05-07T12:18:48Z

app/javascript/components/cloud-provider/validators.js

+};
+
+export const asyncValidate = (formValues, asyncFieldsNames) => new Promise((resolve) => {
+  console.log('validating: ', formValues, asyncFieldsNames);


console.log

its just a WIP 😉

Aaah, sorry, I didn't notice :)

himdel · 2019-05-07T12:22:24Z

One thing I'm a bit worried about is the ${prefix}.foo use.

Is that so that we don't have to reload the schema after changing types?
Or do those values get sent to the server with the prefix?

(But definitely better than the original #{prefix}_foo 👍 )

Hyperkid123 · 2019-05-07T12:51:24Z

Is that so that we don't have to reload the schema after changing types

Yes

Or do those values get sent to the server with the prefix

They don't but different types share the same attributes but sometimes they have different format requirements. Scoping the will allow you to change types without resetting certain fields and saving the state if the user decides to switch back to some type. Anything that is not in general schema might cause conflict if you change the type if we don't have unique field names.

I also have a "plan" for the future that will enable us to "download" pieces of schema on the fly which would allow us also add the prefix on the fly.

himdel · 2019-05-07T13:00:29Z

OK, perfect, thanks :)

Works for me, just don't base any logic on the prefix strings.. in the future this should be a matter of settings, not types. (has keypair vs is openstack)

miq-bot · 2019-05-09T09:12:03Z

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

miq-bot · 2019-11-11T05:00:30Z

This pull request has been automatically closed because it has not been updated for at least 6 months.

Feel free to reopen this pull request if these changes are still valid.

Thank you for all your contributions!

Hyperkid123 added 2 commits May 2, 2019 16:55

initial commit.

8dff615

Updated form schemas for amazon and azure cloud providers.

430c29f

miq-bot assigned martinpovolny May 7, 2019

himdel reviewed May 7, 2019

View reviewed changes

miq-bot added the wip label May 7, 2019

himdel reviewed May 7, 2019

View reviewed changes

himdel added the react label May 7, 2019

miq-bot added the unmergeable label May 9, 2019

miq-bot closed this Nov 11, 2019

Hyperkid123 deleted the add-cloud-provider-form branch November 11, 2019 07:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Migrate add cloud provider form to data driven forms #5542

[WIP] Migrate add cloud provider form to data driven forms #5542

Hyperkid123 commented May 7, 2019 •

edited

Loading

Hyperkid123 commented May 7, 2019

miq-bot commented May 7, 2019

himdel May 7, 2019

Hyperkid123 May 7, 2019

himdel May 7, 2019

Hyperkid123 May 7, 2019

himdel May 7, 2019

Hyperkid123 May 7, 2019

himdel May 7, 2019

himdel May 7, 2019

himdel May 7, 2019 •

edited

Loading

himdel May 7, 2019

Hyperkid123 May 7, 2019

himdel May 7, 2019

himdel commented May 7, 2019 •

edited

Loading

Hyperkid123 commented May 7, 2019 •

edited

Loading

himdel commented May 7, 2019

miq-bot commented May 9, 2019

miq-bot commented Nov 11, 2019

		@@ -0,0 +1,29 @@

		const isHostname = function(value) {
		return /^(([a-zA-Z0-9]\|[a-zA-Z0-9][a-zA-Z0-9-][a-zA-Z0-9])\.)([A-Za-z0-9][A-Za-z0-9-]*[A-Za-z0-9])$/.test(value);

[WIP] Migrate add cloud provider form to data driven forms #5542

[WIP] Migrate add cloud provider form to data driven forms #5542

Conversation

Hyperkid123 commented May 7, 2019 • edited Loading

Changes

Dependencies

TO DO

Hyperkid123 commented May 7, 2019

miq-bot commented May 7, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

himdel May 7, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

himdel commented May 7, 2019 • edited Loading

Hyperkid123 commented May 7, 2019 • edited Loading

himdel commented May 7, 2019

miq-bot commented May 9, 2019

miq-bot commented Nov 11, 2019

Hyperkid123 commented May 7, 2019 •

edited

Loading

himdel May 7, 2019 •

edited

Loading

himdel commented May 7, 2019 •

edited

Loading

Hyperkid123 commented May 7, 2019 •

edited

Loading