fix(KONFLUX-5886): allow unselecting default context

The user should be able to unselect the default context during the creation of an integration test. Other changes: - Contexts are now required (at least one) Signed-off-by: Bryan Ramos <[email protected]>
konflux-ci · Dec 19, 2024 · fcd4962 · fcd4962
1 parent 69a23b0
commit fcd4962
Show file tree

Hide file tree

Showing 16 changed files with 241 additions and 94 deletions.
diff --git a/src/components/IntegrationTests/ContextSelectList.tsx b/src/components/IntegrationTests/ContextSelectList.tsx
@@ -13,7 +13,7 @@ import {
   Button,
 } from '@patternfly/react-core';
 import { TimesIcon } from '@patternfly/react-icons/dist/esm/icons/times-icon';
-import { ContextOption } from './utils';
+import { ContextOption } from './utils/creation-utils';
 
 type ContextSelectListProps = {
   allContexts: ContextOption[];
@@ -22,7 +22,7 @@ type ContextSelectListProps = {
   inputValue: string;
   onInputValueChange: (value: string) => void;
   onRemoveAll: () => void;
-  editing: boolean;
+  error?: string;
 };
 
 export const ContextSelectList: React.FC<ContextSelectListProps> = ({
@@ -32,7 +32,7 @@ export const ContextSelectList: React.FC<ContextSelectListProps> = ({
   onRemoveAll,
   inputValue,
   onInputValueChange,
-  editing,
+  error,
 }) => {
   const [isOpen, setIsOpen] = useState(false);
   const [focusedItemIndex, setFocusedItemIndex] = useState<number | null>(null);
@@ -144,6 +144,7 @@ export const ContextSelectList: React.FC<ContextSelectListProps> = ({
       isExpanded={isOpen}
       style={{ minWidth: '750px' } as React.CSSProperties}
       data-test="context-dropdown-toggle"
+      status={error ? 'danger' : 'success'}
     >
       <TextInputGroup isPlain>
         <TextInputGroupMain
@@ -155,7 +156,7 @@ export const ContextSelectList: React.FC<ContextSelectListProps> = ({
           id="multi-typeahead-select-input"
           autoComplete="off"
           innerRef={textInputRef}
-          placeholder="Select a context"
+          placeholder={error ? 'You must select at least one context' : 'Select a context'}
           {...(activeItemId && { 'aria-activedescendant': activeItemId })}
           role="combobox"
           isExpanded={isOpen}
@@ -204,7 +205,6 @@ export const ContextSelectList: React.FC<ContextSelectListProps> = ({
             isSelected={ctx.selected}
             description={ctx.description}
             ref={null}
-            isDisabled={!editing && ctx.name === 'application'}
             data-test={`context-option-${ctx.name}`}
           >
             {ctx.name}

diff --git a/src/components/IntegrationTests/ContextsField.tsx b/src/components/IntegrationTests/ContextsField.tsx
@@ -11,40 +11,32 @@ import {
   contextOptions,
   mapContextsWithSelection,
   addComponentContexts,
-} from './utils';
+} from './utils/creation-utils';
 
 interface IntegrationTestContextProps {
   heading?: React.ReactNode;
   fieldName: string;
-  editing: boolean;
 }
 
-const ContextsField: React.FC<IntegrationTestContextProps> = ({ heading, fieldName, editing }) => {
+const ContextsField: React.FC<IntegrationTestContextProps> = ({ heading, fieldName }) => {
   const { namespace, workspace } = useWorkspaceInfo();
   const { applicationName } = useParams();
   const [components, componentsLoaded] = useComponents(namespace, workspace, applicationName);
-  const [, { value: contexts }] = useField(fieldName);
+  const [, { value: contexts, error }] = useField(fieldName);
   const fieldId = getFieldId(fieldName, 'dropdown');
   const [inputValue, setInputValue] = React.useState('');
 
   // The names of the existing selected contexts.
   const selectedContextNames: string[] = (contexts ?? []).map((c: ContextOption) => c.name);
   // All the context options available to the user.
   const allContexts = React.useMemo(() => {
-    let initialSelectedContexts = mapContextsWithSelection(selectedContextNames, contextOptions);
-    // If this is a new integration test, ensure that 'application' is selected by default
-    if (!editing && !selectedContextNames.includes('application')) {
-      initialSelectedContexts = initialSelectedContexts.map((ctx) => {
-        return ctx.name === 'application' ? { ...ctx, selected: true } : ctx;
-      });
-    }
-
+    const initialSelectedContexts = mapContextsWithSelection(selectedContextNames, contextOptions);
     // If we have components and they are loaded, add to context option list.
     // Else, return the base context list.
     return componentsLoaded && components
       ? addComponentContexts(initialSelectedContexts, selectedContextNames, components)
       : initialSelectedContexts;
-  }, [componentsLoaded, components, selectedContextNames, editing]);
+  }, [componentsLoaded, components, selectedContextNames]);
 
   // This holds the contexts that are filtered using the user input value.
   const filteredContexts = React.useMemo(() => {
@@ -101,7 +93,7 @@ const ContextsField: React.FC<IntegrationTestContextProps> = ({ heading, fieldNa
               inputValue={inputValue}
               onInputValueChange={setInputValue}
               onRemoveAll={() => handleRemoveAll(arrayHelpers)}
-              editing={editing}
+              error={error}
             />
           )}
         />

diff --git a/src/components/IntegrationTests/EditContextsModal.tsx b/src/components/IntegrationTests/EditContextsModal.tsx
@@ -16,6 +16,7 @@ import { IntegrationTestScenarioKind, Context } from '../../types/coreBuildServi
 import { ComponentProps, createModalLauncher } from '../modal/createModalLauncher';
 import ContextsField from './ContextsField';
 import { UnformattedContexts, formatContexts } from './IntegrationTestForm/utils/create-utils';
+import { contextModalValidationSchema } from './utils/validation-utils';
 
 type EditContextsModalProps = ComponentProps & {
   intTest: IntegrationTestScenarioKind;
@@ -74,17 +75,18 @@ export const EditContextsModal: React.FC<React.PropsWithChildren<EditContextsMod
       onSubmit={updateIntegrationTest}
       initialValues={{ contexts: initialContexts, confirm: false }}
       onReset={onReset}
+      validationSchema={contextModalValidationSchema}
     >
       {({ handleSubmit, handleReset, isSubmitting, values }) => {
         const isChanged = values.contexts !== initialContexts;
-        const showConfirmation = isChanged && values.strategy === 'Automatic';
-        const isValid = isChanged && (showConfirmation ? values.confirm : true);
+        const isPopulated = values.contexts.length > 0;
+        const isValid = isChanged && isPopulated;
 
         return (
           <div data-test={'edit-contexts-modal'} onKeyDown={handleKeyDown}>
             <Stack hasGutter>
               <StackItem>
-                <ContextsField fieldName="contexts" editing={true} />
+                <ContextsField fieldName="contexts" />
               </StackItem>
               <StackItem>
                 {error && (

diff --git a/src/components/IntegrationTests/IntegrationTestForm/IntegrationTestSection.tsx b/src/components/IntegrationTests/IntegrationTestForm/IntegrationTestSection.tsx
@@ -69,7 +69,7 @@ const IntegrationTestSection: React.FC<React.PropsWithChildren<Props>> = ({ isIn
           data-test="git-path-repo"
           required
         />
-        <ContextsField fieldName="integrationTest.contexts" editing={edit} />
+        <ContextsField fieldName="integrationTest.contexts" />
         <FormikParamsField fieldName="integrationTest.params" />
 
         <CheckboxField

diff --git a/src/components/IntegrationTests/IntegrationTestForm/IntegrationTestView.tsx b/src/components/IntegrationTests/IntegrationTestForm/IntegrationTestView.tsx
@@ -1,9 +1,10 @@
 import React from 'react';
 import { useNavigate } from 'react-router-dom';
 import { Formik } from 'formik';
-import { IntegrationTestScenarioKind, Context } from '../../../types/coreBuildService';
+import { IntegrationTestScenarioKind } from '../../../types/coreBuildService';
 import { useTrackEvent, TrackEvents } from '../../../utils/analytics';
 import { useWorkspaceInfo } from '../../Workspace/useWorkspaceInfo';
+import { defaultSelectedContextOption } from '../utils/creation-utils';
 import IntegrationTestForm from './IntegrationTestForm';
 import { IntegrationTestFormValues, IntegrationTestLabels } from './types';
 import {
@@ -55,10 +56,20 @@ const IntegrationTestView: React.FunctionComponent<
   interface FormContext {
     name: string;
     description: string;
+    selected?: boolean;
   }
 
-  const getFormContextValues = (contexts: Context[] | null | undefined): FormContext[] => {
-    if (!contexts?.length) return [];
+  const getFormContextValues = (
+    integrateTest: IntegrationTestScenarioKind | null | undefined,
+  ): FormContext[] => {
+    const contexts = integrateTest?.spec?.contexts;
+    // NOTE: If this is a new integration test,
+    // have the 'application' context selected by default.
+    if (!integrateTest) {
+      return [defaultSelectedContextOption];
+    } else if (!contexts?.length) {
+      return [];
+    }
 
     return contexts.map((context) => {
       return context.name ? { name: context.name, description: context.description } : context;
@@ -72,7 +83,7 @@ const IntegrationTestView: React.FunctionComponent<
       revision: revision?.value ?? '',
       path: path?.value ?? '',
       params: getFormParamValues(integrationTest?.spec?.params),
-      contexts: getFormContextValues(integrationTest?.spec?.contexts),
+      contexts: getFormContextValues(integrationTest),
       optional:
         integrationTest?.metadata.labels?.[IntegrationTestLabels.OPTIONAL] === 'true' ?? false,
     },

diff --git a/...components/IntegrationTests/IntegrationTestForm/__tests__/IntegrationTestSection.spec.tsx b/...components/IntegrationTests/IntegrationTestForm/__tests__/IntegrationTestSection.spec.tsx
@@ -58,4 +58,13 @@ describe('IntegrationTestSection', () => {
 
     screen.queryByTestId('its-param-field');
   });
+
+  it('should render contexts section', () => {
+    formikRenderer(<IntegrationTestSection isInPage />, {
+      source: 'test-source',
+      secret: null,
+    });
+
+    screen.queryByTestId('its-context-field');
+  });
 });
diff --git a/src/components/IntegrationTests/IntegrationTestForm/utils/__tests__/create-utils.spec.ts b/src/components/IntegrationTests/IntegrationTestForm/utils/__tests__/create-utils.spec.ts
@@ -56,6 +56,13 @@ describe('Create Utils', () => {
         url: 'test-url',
         path: 'test-path',
         optional: false,
+        contexts: [
+          {
+            name: 'application',
+            description:
+              'execute the integration test in all cases - this would be the default state',
+          },
+        ],
       },
       'Test Application',
       'test-ns',

diff --git a/src/components/IntegrationTests/IntegrationTestForm/utils/__tests__/validation-utils.spec.ts b/src/components/IntegrationTests/IntegrationTestForm/utils/__tests__/validation-utils.spec.ts
@@ -9,6 +9,12 @@ describe('validation-utils', () => {
           url: 'test-url',
           path: 'test-path',
           revision: 'revision',
+          contexts: [
+            {
+              name: 'test',
+              description: 'test description',
+            },
+          ],
         },
       }),
     ).not.toThrow();
@@ -21,6 +27,12 @@ describe('validation-utils', () => {
           url: 'test-url',
           path: 'test-path',
           revision: 'revision',
+          contexts: [
+            {
+              name: 'test',
+              description: 'test description',
+            },
+          ],
         },
       }),
     ).rejects.toThrow('Required');
@@ -34,6 +46,12 @@ describe('validation-utils', () => {
           url: 'test-url',
           path: 'test-path',
           revision: 'revision',
+          contexts: [
+            {
+              name: 'test',
+              description: 'test description',
+            },
+          ],
         },
       }),
     ).rejects.toThrow(
@@ -49,6 +67,12 @@ describe('validation-utils', () => {
           url: 'test-url',
           path: 'test-path',
           revision: 'revision',
+          contexts: [
+            {
+              name: 'test',
+              description: 'test description',
+            },
+          ],
         },
       }),
     ).rejects.toThrow(
@@ -78,4 +102,16 @@ describe('validation-utils', () => {
       }),
     ).rejects.toThrow('Required');
   });
+
+  it('should fail when contexts is missing', async () => {
+    await expect(
+      integrationTestValidationSchema.validate({
+        integrationTest: {
+          name: 'test-name',
+          url: 'test-url',
+          path: 'test-path',
+        },
+      }),
+    ).rejects.toThrow('Required');
+  });
 });
diff --git a/src/components/IntegrationTests/IntegrationTestForm/utils/create-utils.ts b/src/components/IntegrationTests/IntegrationTestForm/utils/create-utils.ts
@@ -47,22 +47,8 @@ export const formatParams = (params): Param[] => {
 };
 
 export type UnformattedContexts = { name: string; description: string }[];
-export const formatContexts = (
-  contexts: UnformattedContexts = [],
-  setDefault: boolean = false,
-): Context[] | null => {
-  const defaultContext = {
-    name: 'application',
-    description: 'execute the integration test in all cases - this would be the default state',
-  };
-  const newContextNames = new Set(contexts.map((ctx) => ctx.name));
+export const formatContexts = (contexts: UnformattedContexts = []): Context[] | null => {
   const newContexts = contexts.map(({ name, description }) => ({ name, description }));
-  // Even though this option is preselected in the context option list,
-  // it's not appended to the Formik field array when submitting.
-  // Lets set the default here so we know it will be applied.
-  if (setDefault && !newContextNames.has('application')) {
-    newContexts.push(defaultContext);
-  }
 
   return newContexts.length ? newContexts : null;
 };
@@ -157,7 +143,7 @@ export const createIntegrationTest = (
         ],
       },
       params: formatParams(params),
-      contexts: formatContexts(contexts, true),
+      contexts: formatContexts(contexts),
     },
   };
 

diff --git a/src/components/IntegrationTests/IntegrationTestForm/utils/validation-utils.ts b/src/components/IntegrationTests/IntegrationTestForm/utils/validation-utils.ts
@@ -12,5 +12,15 @@ export const integrationTestValidationSchema = yup.object({
       .string()
       .required('Required')
       .max(2000, 'Please enter a path that is less than 2000 characters.'),
+    contexts: yup
+      .array()
+      .of(
+        yup.object().shape({
+          name: yup.string(),
+          description: yup.string(),
+        }),
+      )
+      .required('Required')
+      .min(1),
   }),
 });