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

[#901] Conversion Host Enablement - List View - Add Retry button with modal for re-entering SSH keys #916

Merged
merged 10 commits into from
Mar 26, 2019

Conversation

mturley
Copy link
Contributor

@mturley mturley commented Mar 21, 2019

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

Closes #901.

Note: this depends on ManageIQ/manageiq#18541 to be tested, but we can merge it ahead since the conversion hosts page is still hidden. The retry button will only appear on tasks that were created with that back end change in place.

While we wait for that back end PR to be ready (I'm not sure it even creates the request_params yet), to test this PR you can fake some context data by making a change like this: mturley@0acaab2

We can also test against this database which has one failed task at the end of the list (but will require a hack like the above, since the task has no context data): https://drive.google.com/open?id=1l26rbhp36v6lCfpwKGASxEJknKnI32oe

Screens

This PR adds a Retry button to the list view rows on the conversion hosts settings page, which will appear on any failed enablement task row which has context_data.request_params in the task object:

Screenshot 2019-03-21 17 07 59

When clicked, a modal will appear explaining that the user needs to re-enter any SSH private keys they provided the first time through the wizard, as they were not saved. The SSH key fields shown match what was shown in the wizard, depending on their original selection of a transformation method. If SSH was originally selected in the wizard, both the conversion host SSH private key and the vmware hypervisors SSH private key fields will appear:

Screenshot 2019-03-20 17 04 39

If VDDK was originally selected in the wizard, only the conversion host SSH private key field will appear:

Screenshot 2019-03-21 10 47 46

The fields are required, and when they are filled the "Retry" button at the bottom will become enabled:

Screenshot 2019-03-21 10 47 54

When the user clicks Retry, the original properties from the wizard are recovered from the task's context data, combined with these re-entered SSH key(s), and sent to the API with the same action the wizard uses to submit.

Implementation Details

  • In order for the redux-form state to be reset when the modal closes, it needs to be fully unmounted when it's not visible. We didn't need this for the Remove/Delete modal, because it has no state, only a button, so leaving that modal always mounted and passing a single conversionHostDeleteModalVisible boolean to the show prop was sufficient. For this modal however, just like the wizards, if we had used the single boolean as a condition of mounting and always passed show={true}, we would lose the animation that happens when show turns to false. So just like the wizards, we needed distinct conversionHostRetryModalMounted and conversionHostRetryModalVisible booleans, toggled on by the show action together, and toggled off by the actions we pass to onHide and onExited separately. onExited gets called when the hide animation completes.
  • Because the modal uses the same two SSH key fields from the Authentication step of the wizard, I abstracted those out into a new SSHKeyField component which wraps a TextFileInput in a redux-form Field. I also moved the logic for specifying the info popover message of the conversion host SSH private key into a helper function to be reused.
  • The presence or absence of the vmware_vddk_package_url in a task's context_data.request_params determines whether the modal should use a VDDK transformation method or SSH transformation method.
  • There is quite a lot of "prop drilling" from ConversionHostsSettings -> ConversionHostsList -> ConversionHostsListItem. We could clean this up in the future by connecting more of the inner components to redux directly (and all of the components used by ConversionHostsList could be in their own directory) but I decided against a big refactor as part of this PR. I might come back and do that as part of unit testing.

@mturley mturley added enhancement bz Issues filed by QE or having a BZ hammer/yes z-stream v1.2 labels Mar 21, 2019
@miq-bot
Copy link
Member

miq-bot commented Mar 22, 2019

Checked commits mturley/manageiq-v2v@d29af77~...dead78b 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. 🍰

@mturley mturley marked this pull request as ready for review March 22, 2019 18:27
@mturley mturley closed this Mar 22, 2019
@mturley mturley reopened this Mar 22, 2019
@mturley mturley requested a review from mzazrivec March 22, 2019 18:28
@mturley
Copy link
Contributor Author

mturley commented Mar 22, 2019

Weird, https://travis-ci.org/ManageIQ/manageiq-v2v/builds/510050291 passed but didn't update the status on the PR. Closing and reopening one more time to try it again... @mzazrivec I think I'm just going to stop using draft PRs until we have time to figure out why they break Travis.

@mturley mturley closed this Mar 22, 2019
@mturley mturley reopened this Mar 22, 2019
@mzazrivec mzazrivec merged commit 4d7e37c into ManageIQ:master Mar 26, 2019
@mzazrivec mzazrivec self-assigned this Mar 26, 2019
@mturley mturley deleted the 901-conv-host-retry branch March 26, 2019 17:37
simaishi pushed a commit that referenced this pull request Apr 5, 2019
[#901] Conversion Host Enablement - List View - Add Retry button with modal for re-entering SSH keys

(cherry picked from commit 4d7e37c)

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 e5b7742b2506071b8b902e3c8320fd6c596c5d6c
Author: Milan Zázrivec <mzazrivec@redhat.com>
Date:   Tue Mar 26 17:53:41 2019 +0100

    Merge pull request #916 from mturley/901-conv-host-retry
    
    [#901] Conversion Host Enablement - List View - Add Retry button with modal for re-entering SSH keys
    
    (cherry picked from commit 4d7e37cea2aaf83cc5ce8a230c1f556e9b33134f)
    
    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.

Conversion Host Enablement - List View - Retry button on a failed enablement task - Need API support
4 participants