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

[Part of BZ#1789479] UI changes for UCI: use VMs instead of Hosts for RHV conversion host configuration #1085

Merged
merged 3 commits into from Jan 14, 2020

Conversation

@mturley
Copy link
Contributor

mturley commented Jan 9, 2020

BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1789479
JIRA: https://issues.redhat.com/browse/MIGENG-316

When configuring a RHV conversion host, we now choose from VMs instead of Hosts. I've updated the messaging on that step of the wizard to refer to "Select VMs" / "VMs to configure as conversion hosts" etc, since it will now always be VMs regardless of provider type.

The CONVERSION_HOST_TYPES constant, which is used for filtering conversion hosts by provider type via their resource_type, now includes both the Vm and Host types for RHV. This is to support legacy conversion hosts that were configured before UCI support was added.

I also added an id prop to the TypeAheadSelect in this step of the wizard because I was seeing warnings that it will be required in a future version of react-bootstrap-typeahead.

@mturley mturley requested a review from mzazrivec Jan 9, 2020
@mturley mturley added this to In progress in v2v UI via automation Jan 9, 2020
@mzazrivec

This comment has been minimized.

Copy link
Contributor

mzazrivec commented Jan 10, 2020

There's one scenario that fails for me with an error:

  1. launch the conv. host wizard
  2. select RHV
  3. in next step, select a host
  4. go back to initial step and select openstack this time
  5. hit next
  6. see the following error:
react-dom.development.js:11365 Uncaught TypeError: Cannot read property 'vms' of undefined
    at ConversionHostWizardHostsStep (ConversionHostWizardHostsStep.js:24)
    at renderWithHooks (react-dom.development.js:15108)
    at mountIndeterminateComponent (react-dom.development.js:17342)
    at beginWork$1 (react-dom.development.js:18486)
    at HTMLUnknownElement.callCallback (react-dom.development.js:347)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:397)
    at invokeGuardedCallback (react-dom.development.js:454)
    at beginWork$$1 (react-dom.development.js:23217)
    at performUnitOfWork (react-dom.development.js:22208)
    at workLoopSync (react-dom.development.js:22185)
@mzazrivec

This comment has been minimized.

Copy link
Contributor

mzazrivec commented Jan 10, 2020

Although exactly the same problem shows on master too, so it seems this PR is not to blame.

@mzazrivec

This comment has been minimized.

Copy link
Contributor

mzazrivec commented Jan 10, 2020

The following error occurs on my end when I select OpenStack initially and reach the authentication tab (happens on master too):

miq_debug.self-1351f771c2cd2fab0d83069fd3f62aadbb7effc11a01ed4942f52308793453be.js?body=1:30 The above error occurred in the <Modal> component:
    in Modal (created by Modal)
    in Modal (created by Wizard)
    in Wizard (created by ConversionHostWizard)
    in ConversionHostWizard (created by ConnectFunction)
    in ConnectFunction (created by ConversionHostsSettings)
    in Spinner (created by ConversionHostsSettings)
    in ConversionHostsSettings (created by ConnectFunction)
    in ConnectFunction (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 ForwardRef)
    in ForwardRef (created by Settings)
    in div (created by Settings)
    in Settings (created by ConnectFunction)
    in ConnectFunction (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 Context.Consumer)
    in ConnectedRouterWithContext (created by ConnectFunction)
    in ConnectFunction (created by App)
    in App (created by ConnectFunction)
    in ConnectFunction
    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.

Uncaught Invariant Violation: Maximum update depth exceeded. This can happen when a component repeatedly calls setState inside componentWillUpdate or componentDidUpdate. React limits the number of nested updates to prevent infinite loops.
    at http://localhost:3000/packs/vendor-fa9bda05266affa03459.js:416322:28
    at checkForNestedUpdates (http://localhost:3000/packs/vendor-fa9bda05266affa03459.js:416325:7)
    at scheduleUpdateOnFiber (http://localhost:3000/packs/vendor-fa9bda05266affa03459.js:414596:3)
    at Object.enqueueSetState (http://localhost:3000/packs/vendor-fa9bda05266affa03459.js:406307:5)
    at Modal.push../node_modules/react/cjs/react.development.js.Component.setState (http://localhost:3000/packs/vendor-fa9bda05266affa03459.js:433724:16)
    at Modal.UNSAFE_componentWillReceiveProps (http://localhost:3000/packs/vendor-fa9bda05266affa03459.js:389721:12)
    at callComponentWillReceiveProps (http://localhost:3000/packs/vendor-fa9bda05266affa03459.js:406598:14)
    at updateClassInstance (http://localhost:3000/packs/vendor-fa9bda05266affa03459.js:406805:7)
    at updateClassComponent (http://localhost:3000/packs/vendor-fa9bda05266affa03459.js:410199:20)
    at beginWork$1 (http://localhost:3000/packs/vendor-fa9bda05266affa03459.js:411712:16)
(anonymous) @ react-dom.development.js:23115
checkForNestedUpdates @ react-dom.development.js:23118
scheduleUpdateOnFiber @ react-dom.development.js:21389
enqueueSetState @ react-dom.development.js:13100
push../node_modules/react/cjs/react.development.js.Component.setState @ react.development.js:325
UNSAFE_componentWillReceiveProps @ Modal.js:216
callComponentWillReceiveProps @ react-dom.development.js:13391
updateClassInstance @ react-dom.development.js:13598
updateClassComponent @ react-dom.development.js:16992
beginWork$1 @ react-dom.development.js:18505
beginWork$$1 @ react-dom.development.js:23193
performUnitOfWork @ react-dom.development.js:22208
workLoopSync @ react-dom.development.js:22185
renderRoot @ react-dom.development.js:21878
runRootCallback @ react-dom.development.js:21554
(anonymous) @ react-dom.development.js:11353
unstable_runWithPriority @ scheduler.development.js:643
runWithPriority$2 @ react-dom.development.js:11305
flushSyncCallbackQueueImpl @ react-dom.development.js:11349
flushSyncCallbackQueue @ react-dom.development.js:11338
discreteUpdates$1 @ react-dom.development.js:21677
discreteUpdates @ react-dom.development.js:2359
dispatchDiscreteEvent @ react-dom.development.js:5979
@mzazrivec

This comment has been minimized.

Copy link
Contributor

mzazrivec commented Jan 10, 2020

The following error shows on auth tab when I initially select RHV (happens on master too):

Warning: The prop `disableClick` of `Dropzone` is deprecated. Use onClick={evt => evt.preventDefault()} instead. This prop will be removed in the next major version. 
    in TextFileInput (created by FormField)
    in FormField (created by ConnectedField)
    in ConnectedField (created by ConnectFunction)
    in ConnectFunction (created by Connect(ConnectedField))
    in Connect(ConnectedField) (created by Field)
    in Field (created by Context.Consumer)
    in Context.Consumer (created by Hoc)
    in Hoc (created by Field)
    in Field (created by TextFileField)
    in TextFileField (created by ConversionHostWizardAuthenticationStep)
    in ConversionHostWizardAuthenticationStep (created by Form(ConversionHostWizardAuthenticationStep))
    in Form(ConversionHostWizardAuthenticationStep) (created by ConnectFunction)
    in ConnectFunction (created by Connect(Form(ConversionHostWizardAuthenticationStep)))
    in Connect(Form(ConversionHostWizardAuthenticationStep)) (created by ReduxForm)
    in ReduxForm (created by Context.Consumer)
    in Context.Consumer (created by Hoc)
    in Hoc (created by ReduxForm)
    in ReduxForm (created by ConnectFunction)
    in ConnectFunction (created by ModalWizardBody)
    in ModalWizardBody (created by ConversionHostWizardBody)
    in ConversionHostWizardBody (created by ConversionHostWizard)
    in ConversionHostWizard (created by ConnectFunction)
    in ConnectFunction (created by ConversionHostsSettings)
    in ConversionHostsSettings (created by ConnectFunction)
    in ConnectFunction (created by Settings)
    in Settings (created by ConnectFunction)
    in ConnectFunction (created by I18nProviderWrapper(Connect(Settings)))
    in I18nProviderWrapper(Connect(Settings)) (created by Route)
    in Route (created by Routes)
    in Routes (created by App)
    in App (created by ConnectFunction)
    in ConnectFunction
@mzazrivec

This comment has been minimized.

Copy link
Contributor

mzazrivec commented Jan 10, 2020

@mturley I checked your code in this PR and it looks good. I found some errors in my console which do not seem to be related to your code (since those happen on master too) but they make it virtually impossible for me to test the conversion host wizard properly.

Could you please look at the errors I posted above and confirm that these are unrelated to your work before I can safely merge this PR?

Thanks.

@mturley mturley force-pushed the mturley:bz1789479-uci-support branch from 2803e12 to 9ce5396 Jan 13, 2020
@@ -46,11 +46,11 @@ const mockConversionHosts = [
];

const mockTasksByResource = {
[CONVERSION_HOST_TYPES[RHV]]: {
[CONVERSION_HOST_TYPES[RHV][0]]: {

This comment has been minimized.

Copy link
@codeclimate

codeclimate bot Jan 13, 2020

Similar blocks of code found in 2 locations. Consider refactoring.

'1': { enable: [{ mock: 'task', state: FINISHED }] },
'2': { enable: [{ mock: 'task', state: 'Active' }] }
},
[CONVERSION_HOST_TYPES[OPENSTACK]]: {
[CONVERSION_HOST_TYPES[OPENSTACK][0]]: {

This comment has been minimized.

Copy link
@codeclimate

codeclimate bot Jan 13, 2020

Similar blocks of code found in 2 locations. Consider refactoring.

@miq-bot

This comment has been minimized.

Copy link
Member

miq-bot commented Jan 13, 2020

Checked commits mturley/manageiq-v2v@a8de593~...9ce5396 with ruby 2.5.5, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 🏆

@mturley

This comment has been minimized.

Copy link
Contributor Author

mturley commented Jan 13, 2020

Pushed a rebase here, waiting on my environment to update before I finish verifying the errors on master.

@mturley mturley changed the title [BZ#1789479] UI changes to support UCI [Part of BZ#1789479] UI changes for UCI: use VMs instead of Hosts for RHV conversion host configuration Jan 13, 2020
@mturley

This comment has been minimized.

Copy link
Contributor Author

mturley commented Jan 13, 2020

@mzazrivec I'm actually not seeing that first error (Cannot read property 'vms' of undefined) on either this branch or on master, maybe I'm not reproducing it correctly. I do see the other two errors you pointed out on both branches, though. The Maximum update depth exceeded one is pretty nasty... I wonder if these were introduced by dependency changes when we removed yarn.lock. We should definitely open another issue and get these fixed.

@mturley

This comment has been minimized.

Copy link
Contributor Author

mturley commented Jan 13, 2020

Opened #1087 to cover these errors. I'm curious why I can't reproduce your first error, but it appears that both our environments behave consistently on master as they do on this branch, so we can probably merge this PR.

@mzazrivec mzazrivec merged commit 26bf001 into ManageIQ:master Jan 14, 2020
2 of 3 checks passed
2 of 3 checks passed
codeclimate 4 issues to fix
Details
Hakiri No security warnings were found.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
v2v UI automation moved this from In progress to Done Jan 14, 2020
@mzazrivec

This comment has been minimized.

Copy link
Contributor

mzazrivec commented Jan 14, 2020

I opened #1089 so that the errors from conversion host wizard do not get lost.

@mturley mturley deleted the mturley:bz1789479-uci-support branch Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
v2v UI
  
Done
3 participants
You can’t perform that action at this time.