Merge pull request #1124 from mturley/bz1708367-remove-failed-conv-host

[#900][BZ#1708367] Allow removal of a failed conversion host configuration task

(cherry picked from commit 00ad970)

https://bugzilla.redhat.com/show_bug.cgi?id=1708367

app/javascript/react/screens/App/Settings/SettingsActions.js

@@ -108,9 +108,9 @@ const _postConversionHostsActionCreator = (url, postBodies) => dispatch =>
 export const postConversionHostsAction = (url, postBodies) =>
   _postConversionHostsActionCreator(new URI(url).toString(), postBodies);
 
-export const setHostToDeleteAction = host => ({
+export const setHostToDeleteAction = hostOrTask => ({
   type: SET_V2V_CONVERSION_HOST_TO_DELETE,
-  payload: host
+  payload: hostOrTask
 });
 
 export const showConversionHostDeleteModalAction = () => ({
@@ -121,11 +121,11 @@ export const hideConversionHostDeleteModalAction = () => ({
   type: HIDE_V2V_CONVERSION_HOST_DELETE_MODAL
 });
 
-export const _deleteConversionHostActionCreator = (url, host) => dispatch =>
+export const _deleteConversionHostActionCreator = url => dispatch =>
   dispatch({
     type: DELETE_V2V_CONVERSION_HOST,
     payload: new Promise((resolve, reject) => {
-      API.post(`${url}/${host.id}`, {
+      API.post(url, {
         action: 'delete'
       })
         .then(response => {
@@ -135,8 +135,8 @@ export const _deleteConversionHostActionCreator = (url, host) => dispatch =>
     })
   });
 
-export const deleteConversionHostAction = (url, host) =>
-  _deleteConversionHostActionCreator(new URI(url).toString(), host);
+export const deleteConversionHostAction = hostOrTask =>
+  _deleteConversionHostActionCreator(new URI(hostOrTask.href).toString());
 
 export const setConversionHostTaskToRetryAction = task => ({
   type: SET_V2V_CONVERSION_HOST_TASK_TO_RETRY,

app/javascript/react/screens/App/Settings/__tests__/SettingsActions.test.js

@@ -229,27 +229,27 @@ describe('settings actions', () => {
 
   it('should delete conversion host and return PENDING and FULFILLED action', () => {
     const url = '/api/conversion_hosts';
-    const host = { mock: 'host', id: '12345' };
+    const host = { mock: 'host', id: '12345', href: `${url}/12345` };
     mockRequest({
       method: 'POST',
       url: `${url}/${host.id}`,
       status: 200,
       response: { mock: 'response' }
     });
-    return store.dispatch(actions.deleteConversionHostAction(url, host)).then(() => {
+    return store.dispatch(actions.deleteConversionHostAction(host)).then(() => {
       expect(store.getActions()).toMatchSnapshot();
     });
   });
 
   it('should delete conversion host and return PENDING and REJECTED action', () => {
     const url = '/api/conversion_hosts';
-    const host = { mock: 'host', id: '12345' };
+    const host = { mock: 'host', id: '12345', href: `${url}/12345` };
     mockRequest({
       method: 'POST',
       url: `${url}/${host.id}`,
       status: 500
     });
-    return store.dispatch(actions.deleteConversionHostAction(url, host)).catch(() => {
+    return store.dispatch(actions.deleteConversionHostAction(host)).catch(() => {
       expect(store.getActions()).toMatchSnapshot();
     });
   });

app/javascript/react/screens/App/Settings/helpers.js

@@ -94,7 +94,7 @@ export const getCombinedConversionHostListItems = (conversionHosts, tasksWithMet
 export const inferTransportMethod = conversionHostsListItem => {
   const vddk = __('VDDK');
   const ssh = __('SSH');
-  if (conversionHostsListItem.meta.isTask) {
+  if (conversionHostsListItem.meta.isTask && conversionHostsListItem.context_data) {
     const { request_params } = conversionHostsListItem.context_data;
     return request_params.vmware_vddk_package_url ? vddk : ssh;
   }

app/javascript/react/screens/App/Settings/screens/ConversionHostsSettings/ConversionHostsSettings.js

@@ -75,7 +75,6 @@ class ConversionHostsSettings extends React.Component {
       conversionHostWizardMounted,
       hideConversionHostDeleteModalAction,
       deleteConversionHostAction,
-      deleteConversionHostActionUrl,
       fetchConversionHostsAction,
       fetchConversionHostsUrl,
       conversionHostRetryModalMounted,
@@ -123,7 +122,6 @@ class ConversionHostsSettings extends React.Component {
               <ConversionHostsList
                 combinedListItems={combinedListItems}
                 deleteConversionHostAction={deleteConversionHostAction}
-                deleteConversionHostActionUrl={deleteConversionHostActionUrl}
                 fetchConversionHostsAction={fetchConversionHostsAction}
                 fetchConversionHostsUrl={fetchConversionHostsUrl}
                 setHostToDeleteAction={setHostToDeleteAction}
@@ -150,7 +148,6 @@ class ConversionHostsSettings extends React.Component {
 
 ConversionHostsSettings.propTypes = {
   deleteConversionHostAction: PropTypes.func,
-  deleteConversionHostActionUrl: PropTypes.string,
   fetchProvidersUrl: PropTypes.string,
   fetchProvidersAction: PropTypes.func,
   isFetchingProviders: PropTypes.bool,
@@ -177,7 +174,6 @@ ConversionHostsSettings.propTypes = {
 };
 
 ConversionHostsSettings.defaultProps = {
-  deleteConversionHostActionUrl: '/api/conversion_hosts',
   fetchProvidersUrl: FETCH_V2V_PROVIDERS_URL,
   fetchConversionHostsUrl: FETCH_CONVERSION_HOSTS_URL,
   fetchConversionHostTasksUrl:

app/javascript/react/screens/App/Settings/screens/ConversionHostsSettings/__tests__/__snapshots__/index.test.js.snap

@@ -8,7 +8,6 @@ Object {
   "conversionHostToDelete": null,
   "conversionHostWizardMounted": false,
   "deleteConversionHostAction": [Function],
-  "deleteConversionHostActionUrl": "/api/conversion_hosts",
   "fetchConversionHostTasksAction": [Function],
   "fetchConversionHostTasksUrl": "/api/tasks?expand=resources&attributes=id,name,state,status,message,started_on,updated_on,pct_complete,context_data&filter[]=name=\\"%25Configuring a conversion_host%25\\"&sort_by=updated_on&sort_order=descending",
   "fetchConversionHostsAction": [Function],

app/javascript/react/screens/App/Settings/screens/ConversionHostsSettings/components/ConversionHostRemoveButton.js

@@ -2,12 +2,17 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import { Button } from 'patternfly-react';
 
-const ConversionHostRemoveButton = ({ host, setHostToDeleteAction, showConversionHostDeleteModalAction, ...props }) => (
+const ConversionHostRemoveButton = ({
+  hostOrTask,
+  setHostToDeleteAction,
+  showConversionHostDeleteModalAction,
+  ...props
+}) => (
   <Button
-    id={`remove_${host.id}`}
+    id={`remove_${hostOrTask.id}`}
     onClick={e => {
       e.stopPropagation();
-      setHostToDeleteAction(host);
+      setHostToDeleteAction(hostOrTask);
       showConversionHostDeleteModalAction();
     }}
     {...props}
@@ -17,7 +22,7 @@ const ConversionHostRemoveButton = ({ host, setHostToDeleteAction, showConversio
 );
 
 ConversionHostRemoveButton.propTypes = {
-  host: PropTypes.object,
+  hostOrTask: PropTypes.object,
   setHostToDeleteAction: PropTypes.func,
   showConversionHostDeleteModalAction: PropTypes.func
 };

app/javascript/react/screens/App/Settings/screens/ConversionHostsSettings/components/ConversionHostsList.js

@@ -10,7 +10,6 @@ const ConversionHostsList = ({
   combinedListItems,
   conversionHostToDelete,
   deleteConversionHostAction,
-  deleteConversionHostActionUrl,
   hideConversionHostDeleteModalAction,
   setHostToDeleteAction,
   conversionHostDeleteModalVisible,
@@ -73,7 +72,6 @@ const ConversionHostsList = ({
     <DeleteConversionHostConfirmationModal
       conversionHostToDelete={conversionHostToDelete}
       deleteConversionHostAction={deleteConversionHostAction}
-      deleteConversionHostActionUrl={deleteConversionHostActionUrl}
       hideConversionHostDeleteModalAction={hideConversionHostDeleteModalAction}
       conversionHostDeleteModalVisible={conversionHostDeleteModalVisible}
       isDeletingConversionHost={isDeletingConversionHost}
@@ -88,7 +86,6 @@ ConversionHostsList.propTypes = {
   combinedListItems: PropTypes.arrayOf(PropTypes.object),
   conversionHostToDelete: PropTypes.object,
   deleteConversionHostAction: PropTypes.func,
-  deleteConversionHostActionUrl: PropTypes.string,
   hideConversionHostDeleteModalAction: PropTypes.func,
   setHostToDeleteAction: PropTypes.func,
   conversionHostDeleteModalVisible: PropTypes.bool,

app/javascript/react/screens/App/Settings/screens/ConversionHostsSettings/components/ConversionHostsListItem.js

@@ -8,8 +8,6 @@ import StopPropagationOnClick from '../../../../common/StopPropagationOnClick';
 import { FINISHED, ERROR, ENABLE, DISABLE } from '../ConversionHostsSettingsConstants';
 import { getConversionHostTaskLogFile, inferTransportMethod } from '../../../helpers';
 
-const removeFailedTaskSupported = false; // TODO remove me when the Remove button works
-
 const ConversionHostsListItem = ({
   listItem,
   isTask,
@@ -72,11 +70,13 @@ const ConversionHostsListItem = ({
 
   let actionButtons;
   if (isTask) {
-    const removeButton = <Button disabled={mostRecentTask.state !== FINISHED}>{__('Remove') /* TODO */}</Button>;
+    const isFinished = mostRecentTask.state === FINISHED;
+    const isFailed = mostRecentTask.status === ERROR;
     const taskHasRequestParams = mostRecentTask.context_data && mostRecentTask.context_data.request_params;
     actionButtons = (
       <React.Fragment>
-        {mostRecentTask.status === ERROR &&
+        {isFinished &&
+          isFailed &&
           taskHasRequestParams && (
             <ConversionHostRetryButton
               task={mostRecentTask}
@@ -85,14 +85,18 @@ const ConversionHostsListItem = ({
               disabled={isPostingConversionHosts}
             />
           )}
-        {(mostRecentTask.state !== FINISHED || removeFailedTaskSupported) &&
-          removeButton /* currently only renders when it will be disabled. once removeFailedTaskSupported is true / removed, this button should always render. */}
+        <ConversionHostRemoveButton
+          hostOrTask={mostRecentTask}
+          setHostToDeleteAction={setHostToDeleteAction}
+          showConversionHostDeleteModalAction={showConversionHostDeleteModalAction}
+          disabled={!(isFinished && isFailed)}
+        />
       </React.Fragment>
     );
   } else {
     actionButtons = (
       <ConversionHostRemoveButton
-        host={listItem}
+        hostOrTask={listItem}
         setHostToDeleteAction={setHostToDeleteAction}
         showConversionHostDeleteModalAction={showConversionHostDeleteModalAction}
         disabled={mostRecentTask && mostRecentTask.meta.operation === DISABLE && mostRecentTask.state !== FINISHED}

app/javascript/react/screens/App/Settings/screens/ConversionHostsSettings/components/DeleteConversionHostConfirmationModal.js

@@ -5,7 +5,6 @@ import { Button, Modal, Icon } from 'patternfly-react';
 const DeleteConversionHostConfirmationModal = ({
   conversionHostToDelete,
   deleteConversionHostAction,
-  deleteConversionHostActionUrl,
   hideConversionHostDeleteModalAction,
   conversionHostDeleteModalVisible,
   isDeletingConversionHost
@@ -20,7 +19,9 @@ const DeleteConversionHostConfirmationModal = ({
         <Icon type="pf" name="delete" />
       </div>
       <div className="warning-modal-body--list">
-        <h4>{__('Are you sure you want to remove the following conversion host?')}</h4>
+        <h4>
+          {__('Are you sure you want to remove the following conversion host?') /* TODO different text for task? */}
+        </h4>
         <div>
           <ul>
             <h4>
@@ -38,7 +39,7 @@ const DeleteConversionHostConfirmationModal = ({
         bsStyle="primary"
         disabled={isDeletingConversionHost}
         onClick={() => {
-          deleteConversionHostAction(deleteConversionHostActionUrl, conversionHostToDelete);
+          deleteConversionHostAction(conversionHostToDelete);
         }}
       >
         {__('Remove')}
@@ -50,7 +51,6 @@ const DeleteConversionHostConfirmationModal = ({
 DeleteConversionHostConfirmationModal.propTypes = {
   conversionHostToDelete: PropTypes.object,
   deleteConversionHostAction: PropTypes.func,
-  deleteConversionHostActionUrl: PropTypes.string,
   hideConversionHostDeleteModalAction: PropTypes.func,
   conversionHostDeleteModalVisible: PropTypes.bool,
   isDeletingConversionHost: PropTypes.bool

app/javascript/react/screens/App/Settings/screens/ConversionHostsSettings/components/__tests__/ConversionHostRemoveButton.test.js

@@ -4,7 +4,7 @@ import ConversionHostRemoveButton from '../ConversionHostRemoveButton';
 
 describe('conversion host remove button', () => {
   const getBaseProps = () => ({
-    host: { id: '12345', name: 'mock-host' },
+    hostOrTask: { id: '12345', name: 'mock-host' },
     setHostToDeleteAction: jest.fn(),
     showConversionHostDeleteModalAction: jest.fn()
   });
@@ -16,7 +16,9 @@ describe('conversion host remove button', () => {
 
   it('renders with a unique id for each host', () => {
     const c1 = shallow(<ConversionHostRemoveButton {...getBaseProps()} />);
-    const c2 = shallow(<ConversionHostRemoveButton {...getBaseProps()} host={{ id: '67890', name: 'mock-host' }} />);
+    const c2 = shallow(
+      <ConversionHostRemoveButton {...getBaseProps()} hostOrTask={{ id: '67890', name: 'mock-host' }} />
+    );
     expect(c1.find('Button').props().id).not.toEqual(c2.find('Button').props().id);
   });
 
@@ -32,7 +34,7 @@ describe('conversion host remove button', () => {
     component.find('Button').simulate('click', { stopPropagation });
     expect(stopPropagation).toHaveBeenCalledTimes(1);
     expect(props.setHostToDeleteAction).toHaveBeenCalledTimes(1);
-    expect(props.setHostToDeleteAction).toHaveBeenCalledWith(props.host);
+    expect(props.setHostToDeleteAction).toHaveBeenCalledWith(props.hostOrTask);
     expect(props.showConversionHostDeleteModalAction).toHaveBeenCalledTimes(1);
   });
 });

app/javascript/react/screens/App/Settings/screens/ConversionHostsSettings/components/__tests__/ConversionHostsList.test.js

@@ -9,7 +9,6 @@ describe('conversion hosts list', () => {
     combinedListItems: [{ id: '1', mock: 'task', meta: { isTask: true } }, { id: '2', mock: 'host', meta: {} }],
     conversionHostToDelete: null,
     deleteConversionHostAction: jest.fn(),
-    deleteConversionHostActionUrl: '/mock/delete/url',
     hideConversionHostDeleteModalAction: jest.fn(),
     setHostToDeleteAction: jest.fn(),
     conversionHostDeleteModalVisible: false,

app/javascript/react/screens/App/Settings/screens/ConversionHostsSettings/components/__tests__/DeleteConversionHostConfirmationModal.test.js

@@ -6,7 +6,6 @@ describe('delete conversion host confirmation modal', () => {
   const getBaseProps = () => ({
     conversionHostToDelete: { id: '1', name: 'Mock Conversion Host' },
     deleteConversionHostAction: jest.fn(),
-    deleteConversionHostActionUrl: '/mock/delete/url',
     hideConversionHostDeleteModalAction: jest.fn(),
     conversionHostDeleteModalVisible: true,
     isDeletingConversionHost: false
@@ -34,7 +33,7 @@ describe('delete conversion host confirmation modal', () => {
     const component = shallow(<DeleteConversionHostConfirmationModal {...props} />);
     component.find('Button[bsStyle="primary"]').simulate('click');
     expect(props.deleteConversionHostAction).toHaveBeenCalledTimes(1);
-    expect(props.deleteConversionHostAction).toHaveBeenCalledWith('/mock/delete/url', {
+    expect(props.deleteConversionHostAction).toHaveBeenCalledWith({
       id: '1',
       name: 'Mock Conversion Host'
     });

app/javascript/react/screens/App/Settings/screens/ConversionHostsSettings/components/__tests__/__snapshots__/ConversionHostsList.test.js.snap

@@ -222,7 +222,6 @@ exports[`conversion hosts list renders the outer components correctly 1`] = `
     conversionHostDeleteModalVisible={false}
     conversionHostToDelete={null}
     deleteConversionHostAction={[MockFunction]}
-    deleteConversionHostActionUrl="/mock/delete/url"
     hideConversionHostDeleteModalAction={[MockFunction]}
     isDeletingConversionHost={false}
   />