Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

Conversion Host Configuration Wizard - Step #3 - Authentication #880

Merged
merged 16 commits into from
Feb 25, 2019

Conversation

mturley
Copy link
Contributor

@mturley mturley commented Feb 12, 2019

Part of the Conversion Hosts UI feature BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1693339

This is part of #855.

This PR adds the Authentication step of the wizard, including a new reusable component for collecting an uploaded or pasted text file.

Implementation Details

As part of adding the new TextFileInput component, I upgraded us from react-dropzone v4.2.8 to v8.1.0. This upgrade contained breaking changes to the Dropzone props interface, so it required changes to the other place we use a Dropzone in the CSVDropzoneField component (used on the VMs step of the Migration Plan wizard). As a bonus of these changes, we no longer need to use the hacky dropzoneRef pattern to trigger the file browse dialog.

I also updated the grey color of the drag-and-drop active border on the existing CSVDropzoneField to match the new color I added in TextFileInput, because the old color was not a patternfly variable.

I ran into an issue with redux-form where, by default, conditionally rendered fields will remain registered for validation after they are unmounted. This was causing a bug where, for example, if you selected the SSH transformation method, left the SSH key field blank, then switched to VDDK and filled in all the other fields, the wizard would still block you because the hidden SSH key field was left blank. I saw online that others have solved this by leaving the reduxForm option destroyOnUnmount set to true, but we need that set to false so the form values persist in the store after leaving the step. Instead I solved this by adding the forceUnregisterOnUnmount option, which unregisters unmounted fields from validation without removing their values from the store.

In our existing reusable FormField component, I added support for an info prop which, when present, renders an info icon to the right of the field label with a popover containing the string or node passed to the prop. In order to make room for this icon, I moved the "required" asterisk to the left of the label, which affects all FormFields in the UI. For consistency, I also moved the asterisk to the left in BootstrapSelect. I also added the controlWidth prop to FormField, so we can specify custom widths on both the label and the control, which was necessary to make space for the info icon in this particular field label.

I also added a children prop to FormField, so that we can use it more flexibly for the layout of field label and control columns, while rendering any arbitrary JSX as the actual field contents. The children prop here is a function (render props pattern) which consumes the redux-form input object. I did this to allow rendering the new TextFileInput as a child of FormField, without having to add a special "textfile" mode to FormField or duplicate the FormField layout pieces in TextFileInput. If children are present, the type prop will be ignored and children rendered instead.

I also fixed a bug in FormField where sometimes an empty HelpBlock would render below the field, causing things to shift a little on the screen.

Screens

The wizard step looks like this upon first load (when an OpenStack provider is selected, otherwise the OpenStack User field is hidden):

screenshot 2019-02-15 15 52 37

The info icon next to the Conversion Host SSH Key field label has popover text that depends on the provider type. It looks like this for OpenStack:

screenshot 2019-02-15 15 53 31

It looks like this for RHV:

screenshot 2019-02-15 15 54 13

The SSH key upload component allows the user to either browse for a file or paste the contents of a file. It also supports drag-and-drop of a file onto the field. The file contents field can be typed in, to allow the paste use case. If the user selects a file, the filename is shown and the file contents field becomes read-only until the user clicks the Clear button. In the future, I plan to make this component available for reuse (tracked in patternfly/patternfly-react#1254).

jxlzdj2w6g

If the user selects SSH for the transformation method, another SSH key file input field is shown.

screenshot 2019-02-15 15 57 24

If the user selects VDDK for the transformation method, a VDDK library path field is shown instead. This field is currently missing its Validate button from the mockup, which will be added in a separate PR (tracked in #881).

screenshot 2019-02-15 15 57 44

All visible fields on the form are required, and when they are all filled in, the Configure button (which replaces the Next button on this step of the wizard) will become active.

screenshot 2019-02-15 14 11 31

@mturley mturley changed the title [WIP] Conversion Host Configuration Wizard - Step #3 - Authentication Conversion Host Configuration Wizard - Step #3 - Authentication Feb 15, 2019
@mturley mturley removed the wip label Feb 15, 2019
@miq-bot
Copy link
Member

miq-bot commented Feb 15, 2019

Checked commits mturley/manageiq-v2v@a05b54f~...b5560a4 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 👍

@mzazrivec
Copy link
Contributor

For some reason, when I click on the new step of the conv. host wizard, I'm getting the following javascript error:

Uncaught Invariant Violation: Expected subtree parent to be a mounted class component. This error is likely caused by a bug in React. Please file an issue.
    at invariant (http://localhost:3000/packs/vendor-57b3d6a7ca35c603b546.js:376460:15)
    at findCurrentUnmaskedContext (http://localhost:3000/packs/vendor-57b3d6a7ca35c603b546.js:531549:66)
    at getContextForSubtree (http://localhost:3000/packs/vendor-57b3d6a7ca35c603b546.js:538839:27)
    at updateContainerAtExpirationTime (http://localhost:3000/packs/vendor-57b3d6a7ca35c603b546.js:538883:21)
    at updateContainer (http://localhost:3000/packs/vendor-57b3d6a7ca35c603b546.js:538917:14)
    at ReactRoot.push../node_modules/react-bootstrap/node_modules/react-dom/cjs/react-dom.development.js.ReactRoot.legacy_renderSubtreeIntoContainer (http://localhost:3000/packs/vendor-57b3d6a7ca35c603b546.js:539225:7)
    at http://localhost:3000/packs/vendor-57b3d6a7ca35c603b546.js:539337:18
    at unbatchedUpdates (http://localhost:3000/packs/vendor-57b3d6a7ca35c603b546.js:538757:14)
    at legacyRenderSubtreeIntoContainer (http://localhost:3000/packs/vendor-57b3d6a7ca35c603b546.js:539335:9)
    at Object.unstable_renderSubtreeIntoContainer (http://localhost:3000/packs/vendor-57b3d6a7ca35c603b546.js:539398:16)
invariant @ invariant.js:42
findCurrentUnmaskedContext @ react-dom.development.js:9466
getContextForSubtree @ react-dom.development.js:16761
updateContainerAtExpirationTime @ react-dom.development.js:16805
updateContainer @ react-dom.development.js:16839
push../node_modules/react-bootstrap/node_modules/react-dom/cjs/react-dom.development.js.ReactRoot.legacy_renderSubtreeIntoContainer @ react-dom.development.js:17148
(anonymous) @ react-dom.development.js:17260
unbatchedUpdates @ react-dom.development.js:16679
legacyRenderSubtreeIntoContainer @ react-dom.development.js:17258
unstable_renderSubtreeIntoContainer @ react-dom.development.js:17321
renderOverlay @ OverlayTrigger.js:250
componentDidMount @ OverlayTrigger.js:135
commitLifeCycles @ react-dom.development.js:15961
commitAllLifeCycles @ react-dom.development.js:17262
callCallback @ react-dom.development.js:149
invokeGuardedCallbackDev @ react-dom.development.js:199
invokeGuardedCallback @ react-dom.development.js:256
commitRoot @ react-dom.development.js:17458
completeRoot @ react-dom.development.js:18912
performWorkOnRoot @ react-dom.development.js:18841
performWork @ react-dom.development.js:18749
performSyncWork @ react-dom.development.js:18723
interactiveUpdates$1 @ react-dom.development.js:18992
interactiveUpdates @ react-dom.development.js:2169
dispatchInteractiveEvent @ react-dom.development.js:4876
miq_debug.self-1351f771c2cd2fab0d83069fd3f62aadbb7effc11a01ed4942f52308793453be.js?body=1:30 The above error occurred in the <OverlayTrigger> component:
    in OverlayTrigger (created by FormField)
    in label (created by ControlLabel)
    in ControlLabel (created by Col)
    in Col (created by FormField)
    in div (created by FormGroup)
    in FormGroup (created by FormField)
    in FormField (created by ConnectedField)
    in ConnectedField (created by Connect(ConnectedField))
    in Connect(ConnectedField) (created by Field)
    in Field (created by ConversionHostWizardAuthenticationStep)
    in form (created by Form)
    in Form (created by ConversionHostWizardAuthenticationStep)
    in ConversionHostWizardAuthenticationStep (created by Form(ConversionHostWizardAuthenticationStep))
    in Form(ConversionHostWizardAuthenticationStep) (created by Connect(Form(ConversionHostWizardAuthenticationStep)))
    in Connect(Form(ConversionHostWizardAuthenticationStep)) (created by ReduxForm)
    in ReduxForm (created by Connect(ReduxForm))
    in Connect(ReduxForm) (created by ModalWizardBody)
    in div (created by WizardContents)
    in WizardContents (created by ModalWizardBody)
    in div (created by WizardMain)
    in WizardMain (created by ModalWizardBody)
    in section (created by WizardRow)
    in WizardRow (created by ModalWizardBody)
    in ModalWizardBody (created by ConversionHostWizardBody)
    in ConversionHostWizardBody (created by ConversionHostWizard)
    in div (created by ModalBody)
    in ModalBody (created by WizardBody)
    in WizardBody (created by ConversionHostWizard)
    in div (created by Wizard)
    in div (created by CustomModalDialog)
    in div (created by CustomModalDialog)
    in div (created by CustomModalDialog)
    in CustomModalDialog (created by Modal)
    in Transition (created by Fade)
    in Fade (created by DialogTransition)
    in DialogTransition (created by Modal)
    in RefHolder (created by Modal)
    in div (created by Modal)
    in Portal (created by Modal)
    in Modal (created by Modal)
    in Modal (created by Wizard)
    in Wizard (created by ConversionHostWizard)
    in ConversionHostWizard (created by Connect(ConversionHostWizard))
    in Connect(ConversionHostWizard) (created by ConversionHostsSettings)
    in Spinner (created by ConversionHostsSettings)
    in ConversionHostsSettings (created by Connect(ConversionHostsSettings))
    in Connect(ConversionHostsSettings) (created by Settings)
    in div (created by TabPane)
    in Transition (created by Fade)
    in Fade (created by TabPane)
    in TabPane (created by Tab)
    in Tab (created by Settings)
    in div (created by TabContent)
    in TabContent (created by Tabs)
    in div (created by Tabs)
    in TabContainer (created by Tabs)
    in Tabs (created by Uncontrolled(Tabs))
    in Uncontrolled(Tabs) (created by Settings)
    in div (created by Settings)
    in Settings (created by Connect(Settings))
    in Connect(Settings) (created by I18nProviderWrapper(Connect(Settings)))
    in IntlProvider (created by I18nProviderWrapper(Connect(Settings)))
    in I18nProviderWrapper(Connect(Settings)) (created by Route)
    in Route (created by Routes)
    in Routes (created by App)
    in Router (created by ConnectedRouter)
    in ConnectedRouter (created by Connect(ConnectedRouter))
    in Connect(ConnectedRouter) (created by App)
    in App (created by Connect(App))
    in Connect(App)
    in Provider

Consider adding an error boundary to your tree to customize error handling behavior.
Visit https://fb.me/react-error-boundaries to learn more about error boundaries.
window.console.(anonymous function) @ miq_debug.self-1351f771c2cd2fab0d83069fd3f62aadbb7effc11a01ed4942f52308793453be.js?body=1:30
logCapturedError @ react-dom.development.js:15749
logError @ react-dom.development.js:15783
update.callback @ react-dom.development.js:16649
callCallback @ react-dom.development.js:11164
commitUpdateEffects @ react-dom.development.js:11203
commitUpdateQueue @ react-dom.development.js:11191
commitLifeCycles @ react-dom.development.js:16010
commitAllLifeCycles @ react-dom.development.js:17262
callCallback @ react-dom.development.js:149
invokeGuardedCallbackDev @ react-dom.development.js:199
invokeGuardedCallback @ react-dom.development.js:256
commitRoot @ react-dom.development.js:17458
completeRoot @ react-dom.development.js:18912
performWorkOnRoot @ react-dom.development.js:18841
performWork @ react-dom.development.js:18749
performSyncWork @ react-dom.development.js:18723
interactiveUpdates$1 @ react-dom.development.js:18992
interactiveUpdates @ react-dom.development.js:2169
dispatchInteractiveEvent @ react-dom.development.js:4876
invariant.js:42 Uncaught Invariant Violation: Expected subtree parent to be a mounted class component. This error is likely caused by a bug in React. Please file an issue.
    at invariant (http://localhost:3000/packs/vendor-57b3d6a7ca35c603b546.js:376460:15)
    at findCurrentUnmaskedContext (http://localhost:3000/packs/vendor-57b3d6a7ca35c603b546.js:531549:66)
    at getContextForSubtree (http://localhost:3000/packs/vendor-57b3d6a7ca35c603b546.js:538839:27)
    at updateContainerAtExpirationTime (http://localhost:3000/packs/vendor-57b3d6a7ca35c603b546.js:538883:21)
    at updateContainer (http://localhost:3000/packs/vendor-57b3d6a7ca35c603b546.js:538917:14)
    at ReactRoot.push../node_modules/react-bootstrap/node_modules/react-dom/cjs/react-dom.development.js.ReactRoot.legacy_renderSubtreeIntoContainer (http://localhost:3000/packs/vendor-57b3d6a7ca35c603b546.js:539225:7)
    at http://localhost:3000/packs/vendor-57b3d6a7ca35c603b546.js:539337:18
    at unbatchedUpdates (http://localhost:3000/packs/vendor-57b3d6a7ca35c603b546.js:538757:14)
    at legacyRenderSubtreeIntoContainer (http://localhost:3000/packs/vendor-57b3d6a7ca35c603b546.js:539335:9)
    at Object.unstable_renderSubtreeIntoContainer (http://localhost:3000/packs/vendor-57b3d6a7ca35c603b546.js:539398:16)

Instead of a wizard screen, I'm getting a blank page (or well, blank v2v div).

@priley86
Copy link
Member

i have lost a lot of context here, but I don't have any concerns w/ this from a quick glance @mturley. It seems the error above is caused by React dependency conflicts though?
facebook/react#13742 (comment)

Let me know if you need any help debugging further...

@mzazrivec
Copy link
Contributor

What really puzzles me here is that things seemed to have worked on Mike's end (screenshots above), but all I'm getting here is just the error (i.e. the wizard screen disappears with the error).

@mturley
Copy link
Contributor Author

mturley commented Feb 21, 2019

@mzazrivec did you run a yarn install after you pulled my branch? Maybe it's something about the react-dropzone upgrade I made.

@mzazrivec
Copy link
Contributor

did you run a yarn install after you pulled my branch?

Yup, several times. With the updated packages I got the above error.

@mzazrivec
Copy link
Contributor

Today I thought of deleting and re-creating node_modules in manageiq-ui-classic repository and the error I showed above is gone 😕

@mzazrivec mzazrivec self-assigned this Feb 25, 2019
@mzazrivec mzazrivec merged commit c097fb8 into ManageIQ:master Feb 25, 2019
@mturley mturley deleted the conv-host-wizard-step3 branch February 25, 2019 14:57
@mturley mturley added bz Issues filed by QE or having a BZ and removed bugzilla needed labels Feb 27, 2019
@mturley mturley added v1.2 and removed v1.1 labels Mar 27, 2019
simaishi pushed a commit that referenced this pull request Apr 5, 2019
Conversion Host Configuration Wizard - Step #3 - Authentication

(cherry picked from commit c097fb8)

https://bugzilla.redhat.com/show_bug.cgi?id=1696423
@simaishi
Copy link
Contributor

simaishi commented Apr 5, 2019

Hammer backport details:

$ git log -1
commit d4f7d9fed8c4ac815305353e79573d447bf6eace
Author: Milan Zázrivec <mzazrivec@redhat.com>
Date:   Mon Feb 25 14:38:42 2019 +0100

    Merge pull request #880 from mturley/conv-host-wizard-step3
    
    Conversion Host Configuration Wizard - Step #3 - Authentication
    
    (cherry picked from commit c097fb87593a3cfa013bdb7824d569b896d21b41)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1696423

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

Successfully merging this pull request may close these issues.

5 participants