Skip to content

Commit

Permalink
Merge pull request #58 from CryptoRodeo/konflux-5886
Browse files Browse the repository at this point in the history
fix(KONFLUX-5886): allow unselecting default context
  • Loading branch information
CryptoRodeo authored Jan 6, 2025
2 parents 69a23b0 + fcd4962 commit 6fa97ba
Show file tree
Hide file tree
Showing 16 changed files with 241 additions and 94 deletions.
10 changes: 5 additions & 5 deletions src/components/IntegrationTests/ContextSelectList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
Expand All @@ -22,7 +22,7 @@ type ContextSelectListProps = {
inputValue: string;
onInputValueChange: (value: string) => void;
onRemoveAll: () => void;
editing: boolean;
error?: string;
};

export const ContextSelectList: React.FC<ContextSelectListProps> = ({
Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand All @@ -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}
Expand Down Expand Up @@ -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}
Expand Down
20 changes: 6 additions & 14 deletions src/components/IntegrationTests/ContextsField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down Expand Up @@ -101,7 +93,7 @@ const ContextsField: React.FC<IntegrationTestContextProps> = ({ heading, fieldNa
inputValue={inputValue}
onInputValueChange={setInputValue}
onRemoveAll={() => handleRemoveAll(arrayHelpers)}
editing={editing}
error={error}
/>
)}
/>
Expand Down
8 changes: 5 additions & 3 deletions src/components/IntegrationTests/EditContextsModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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;
Expand All @@ -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,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ describe('validation-utils', () => {
url: 'test-url',
path: 'test-path',
revision: 'revision',
contexts: [
{
name: 'test',
description: 'test description',
},
],
},
}),
).not.toThrow();
Expand All @@ -21,6 +27,12 @@ describe('validation-utils', () => {
url: 'test-url',
path: 'test-path',
revision: 'revision',
contexts: [
{
name: 'test',
description: 'test description',
},
],
},
}),
).rejects.toThrow('Required');
Expand All @@ -34,6 +46,12 @@ describe('validation-utils', () => {
url: 'test-url',
path: 'test-path',
revision: 'revision',
contexts: [
{
name: 'test',
description: 'test description',
},
],
},
}),
).rejects.toThrow(
Expand All @@ -49,6 +67,12 @@ describe('validation-utils', () => {
url: 'test-url',
path: 'test-path',
revision: 'revision',
contexts: [
{
name: 'test',
description: 'test description',
},
],
},
}),
).rejects.toThrow(
Expand Down Expand Up @@ -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');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand Down Expand Up @@ -157,7 +143,7 @@ export const createIntegrationTest = (
],
},
params: formatParams(params),
contexts: formatContexts(contexts, true),
contexts: formatContexts(contexts),
},
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}),
});
Loading

0 comments on commit 6fa97ba

Please sign in to comment.