Address feedback for MTV-1686 fixes

Author: jpuzz0

packages/common/src/components/LoadingSpinner/LoadingSpinner.tsx

@@ -0,0 +1,14 @@
+import React, { PropsWithChildren } from 'react';
+
+import { Spinner, SpinnerProps } from '@patternfly/react-core';
+
+type LoadingSpinnerProps = PropsWithChildren &
+  SpinnerProps & {
+    isLoading: boolean;
+  };
+
+export const LoadingSpinner: React.FC<LoadingSpinnerProps> = ({
+  isLoading,
+  children,
+  ...spinnerProps
+}) => (isLoading ? <Spinner {...spinnerProps} /> : children);

packages/common/src/components/LoadingSpinner/index.ts

@@ -0,0 +1 @@
+export { LoadingSpinner } from './LoadingSpinner';

packages/common/src/components/index.ts

@@ -6,6 +6,7 @@ export * from './FormGroupWithHelpText';
 export * from './HelpIconPopover';
 export * from './Icons';
 export * from './LoadingDots';
+export * from './LoadingSpinner';
 export * from './Page';
 export * from './QueryClientHoc';
 export * from './TableView';

packages/forklift-console-plugin/src/modules/Plans/actions/PlanActionsDropdownItems.tsx

@@ -1,4 +1,4 @@
-import React from 'react';
+import React, { useMemo } from 'react';
 import { DropdownItemLink } from 'src/components/actions/DropdownItemLink';
 import { useModal } from 'src/modules/Providers/modals';
 import { getResourceUrl } from 'src/modules/Providers/utils/helpers';
@@ -53,7 +53,7 @@ export const PlanActionsDropdownItems = ({ data }: PlanActionsDropdownItemsProps
     showModal(<PlanDeleteModal resource={plan} model={PlanModel} />);
   };
 
-  const startActionDescription = React.useMemo(() => {
+  const startActionDescription = useMemo(() => {
     if (isPlanValidating) {
       return t('The plan is being validated');
     }

packages/forklift-console-plugin/src/modules/Plans/utils/helpers/getConditionTypes.ts

@@ -0,0 +1,17 @@
+import { V1beta1Plan } from '@kubev2v/types';
+
+import { PlanConditionStatus } from '../types/PlanCondition';
+
+/**
+ * Gets a record of plan types with truthful ('True') statuses
+ * @param plan V1beta1Plan
+ * @returns Record<string, boolean>
+ */
+export const getConditionTypes = (plan: V1beta1Plan): Record<string, boolean> =>
+  plan?.status?.conditions?.reduce((acc, condition) => {
+    if (condition.status === PlanConditionStatus.True) {
+      acc[condition.type] = true;
+    }
+
+    return acc;
+  }, {});

packages/forklift-console-plugin/src/modules/Plans/utils/helpers/getPlanPhase.ts

@@ -1,40 +1,43 @@
 import { V1beta1Plan } from '@kubev2v/types';
 
 import { PlanData, PlanPhase } from '../types';
+import { PlanConditionType } from '../types/PlanCondition';
+
+import { getConditionTypes } from './getConditionTypes';
 
 export const getPlanPhase = (data: PlanData): PlanPhase => {
   const plan = data?.obj;
 
   if (!plan) return PlanPhase.Unknown;
 
   // Check condition type
-  const conditions = getConditions(plan);
+  const conditionTypes = getConditionTypes(plan);
 
-  if (!conditions || conditions?.length < 1) {
+  if (!Object.keys(conditionTypes).length) {
     return PlanPhase.Unknown;
   }
 
   // Check for Archived
-  if (plan?.spec?.archived && !conditions.includes('Archived')) {
+  if (plan?.spec?.archived && !conditionTypes[PlanConditionType.Archived]) {
     return PlanPhase.Archiving;
   }
 
-  if (conditions.includes('Archived')) {
+  if (conditionTypes[PlanConditionType.Archived]) {
     return PlanPhase.Archived;
   }
 
   // Check for Succeeded
-  if (conditions.includes('Succeeded')) {
+  if (conditionTypes[PlanConditionType.Succeeded]) {
     return PlanPhase.Succeeded;
   }
 
   // Check for Canceled
-  if (conditions.includes('Canceled')) {
+  if (conditionTypes[PlanConditionType.Canceled]) {
     return PlanPhase.Canceled;
   }
 
   // CHeck for Running
-  if (conditions.includes('Executing')) {
+  if (conditionTypes[PlanConditionType.Executing]) {
     return PlanPhase.Running;
   }
 
@@ -50,7 +53,7 @@ export const getPlanPhase = (data: PlanData): PlanPhase => {
   // Check for vm errors
   const vmError = plan?.status?.migration?.vms?.find((vm) => vm?.error);
 
-  if (conditions.includes('Failed')) {
+  if (conditionTypes[PlanConditionType.Failed]) {
     return PlanPhase.Failed;
   }
 
@@ -67,40 +70,40 @@ export const getPlanPhase = (data: PlanData): PlanPhase => {
     return PlanPhase.Warning;
   }
 
-  if (conditions.includes('Ready')) {
+  if (conditionTypes[PlanConditionType.Ready]) {
     return PlanPhase.Ready;
   }
 
   return PlanPhase.NotReady;
 };
 
 export const canPlanStart = (plan: V1beta1Plan) => {
-  const conditions = getConditions(plan);
+  const conditionTypes = getConditionTypes(plan);
 
   return (
-    conditions?.includes('Ready') &&
-    !conditions?.includes('Executing') &&
-    !conditions?.includes('Succeeded') &&
+    conditionTypes[PlanConditionType.Ready] &&
+    !conditionTypes[PlanConditionType.Executing] &&
+    !conditionTypes[PlanConditionType.Succeeded] &&
     !plan?.spec?.archived
   );
 };
 
 export const canPlanReStart = (plan: V1beta1Plan) => {
-  const conditions = getConditions(plan);
+  const conditionTypes = getConditionTypes(plan);
 
-  return conditions?.includes('Failed') || conditions?.includes('Canceled');
+  return conditionTypes[PlanConditionType.Failed] || conditionTypes[PlanConditionType.Canceled];
 };
 
 export const isPlanExecuting = (plan: V1beta1Plan) => {
-  const conditions = getConditions(plan);
+  const conditionTypes = getConditionTypes(plan);
 
-  return conditions?.includes('Executing');
+  return conditionTypes[PlanConditionType.Executing];
 };
 
 export const isPlanSucceeded = (plan: V1beta1Plan) => {
-  const conditions = getConditions(plan);
+  const conditionTypes = getConditionTypes(plan);
 
-  return conditions?.includes('Succeeded');
+  return conditionTypes[PlanConditionType.Succeeded];
 };
 
 export const isPlanEditable = (plan: V1beta1Plan) => {
@@ -122,6 +125,3 @@ export const isPlanArchived = (plan: V1beta1Plan) => {
 
   return planStatus === PlanPhase.Archiving || planStatus === PlanPhase.Archived;
 };
-
-const getConditions = (obj: V1beta1Plan) =>
-  obj?.status?.conditions?.filter((c) => c.status === 'True').map((c) => c.type);

packages/forklift-console-plugin/src/modules/Plans/utils/helpers/getPlanSummaryStatus.ts

@@ -5,44 +5,45 @@ import {
   PlanConditionType,
 } from '../types/PlanCondition';
 
+import { getConditionTypes } from './getConditionTypes';
+
 export const getPlanSummaryStatus = (data: PlanData): PlanSummaryStatus => {
   const plan = data?.obj;
-  const conditionTypes = plan?.status?.conditions?.reduce((acc, condition) => {
-    if (condition.status === PlanConditionStatus.True) {
-      acc.push(condition.type);
-    }
-
-    return acc;
-  }, []);
+  const conditionTypes = getConditionTypes(plan);
 
-  if (!conditionTypes?.length) {
+  if (!Object.keys(conditionTypes)?.length) {
     return;
   }
 
-  const isArchiving = plan?.spec?.archived && !conditionTypes.includes(PlanConditionType.Archived);
+  const {
+    [PlanConditionType.Archived]: isArchived,
+    [PlanConditionType.Canceled]: isCanceled,
+    [PlanConditionType.Executing]: isExecuting,
+    [PlanConditionType.Running]: isRunning,
+    [PlanConditionType.Failed]: isFailed,
+    [PlanConditionType.Succeeded]: isSucceeded,
+    [PlanConditionType.Ready]: isReady,
+  } = conditionTypes;
 
   // Archived
-  if (isArchiving || conditionTypes.includes(PlanConditionType.Archived)) {
+  if ((plan?.spec?.archived && !isArchived) || isArchived) {
     return PlanSummaryStatus.Archived;
   }
 
   // Canceled
-  if (conditionTypes.includes(PlanConditionType.Canceled)) {
+  if (isCanceled) {
     return PlanSummaryStatus.Canceled;
   }
 
   // Running
-  if (
-    conditionTypes.includes(PlanConditionType.Executing) ||
-    conditionTypes.includes(PlanConditionType.Running)
-  ) {
+  if (isExecuting || isRunning) {
     return PlanSummaryStatus.Running;
   }
 
   // Incomplete
-  const vmError = plan?.status?.migration?.vms?.find((vm) => vm?.error);
+  const hasVmError = !!plan?.status?.migration?.vms?.find((vm) => vm?.error);
 
-  if (conditionTypes.includes(PlanConditionType.Failed) || vmError) {
+  if (isFailed || hasVmError) {
     return PlanSummaryStatus.Incomplete;
   }
 
@@ -63,12 +64,12 @@ export const getPlanSummaryStatus = (data: PlanData): PlanSummaryStatus => {
   }
 
   // Complete
-  if (conditionTypes.includes(PlanConditionType.Succeeded)) {
+  if (isSucceeded) {
     return PlanSummaryStatus.Complete;
   }
 
   // Ready to start
-  if (conditionTypes.includes(PlanConditionType.Ready)) {
+  if (isReady) {
     return PlanSummaryStatus.ReadyToStart;
   }
 };

packages/forklift-console-plugin/src/modules/Plans/utils/helpers/index.ts

@@ -1,4 +1,5 @@
 // @index(['./*', /style/g], f => `export * from '${f.path}';`)
+export * from './getConditionTypes';
 export * from './getMigrationPhase';
 export * from './getMigrationVmsCounts';
 export * from './getPlanPhase';

packages/forklift-console-plugin/src/modules/Plans/views/list/components/PlanStatusCell/PlanStatusCell.tsx

@@ -12,8 +12,9 @@ import { useModal } from 'src/modules/Providers/modals';
 import { getResourceUrl } from 'src/modules/Providers/utils';
 import { useForkliftTranslation } from 'src/utils/i18n';
 
+import { LoadingSpinner } from '@kubev2v/common';
 import { PlanModel, PlanModelRef } from '@kubev2v/types';
-import { Button, Flex, FlexItem, Split, SplitItem } from '@patternfly/react-core';
+import { Button, Flex, FlexItem, spinnerSize, Split, SplitItem } from '@patternfly/react-core';
 import StartIcon from '@patternfly/react-icons/dist/esm/icons/play-icon';
 
 import { CellProps } from '../CellProps';
@@ -95,7 +96,9 @@ export const PlanStatusCell: React.FC<CellProps> = ({ data }) => {
         className="plan-status-cell-label-section"
       >
         <FlexItem>
-          <PlanStatusCellLabel status={planStatus} isLoading={isPlanLoading} />
+          <LoadingSpinner isLoading={isPlanLoading} size={spinnerSize.md}>
+            <PlanStatusCellLabel status={planStatus} />
+          </LoadingSpinner>
         </FlexItem>
 
         {progressValue !== 0 && isPlanLoading && (

packages/forklift-console-plugin/src/modules/Plans/views/list/components/PlanStatusCell/PlanStatusCellLabel.tsx

@@ -2,20 +2,15 @@ import React from 'react';
 import { PlanSummaryStatus } from 'src/modules/Plans/utils';
 import { useForkliftTranslation } from 'src/utils';
 
-import { Label, Spinner } from '@patternfly/react-core';
+import { Label } from '@patternfly/react-core';
 
-interface PlanStatusCellLabelProps {
+type PlanStatusCellLabelProps = {
   status: PlanSummaryStatus;
-  isLoading: boolean;
-}
+};
 
-export const PlanStatusCellLabel: React.FC<PlanStatusCellLabelProps> = ({ status, isLoading }) => {
+export const PlanStatusCellLabel: React.FC<PlanStatusCellLabelProps> = ({ status }) => {
   const { t } = useForkliftTranslation();
 
-  if (isLoading) {
-    return <Spinner size="md" />;
-  }
-
   if (!status) {
     return t('Validating...');
   }