Skip to content

Commit

Permalink
fix(KONFLUX-5886): allow unselecting default context
Browse files Browse the repository at this point in the history
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 <bramos@redhat.com>
  • Loading branch information
CryptoRodeo committed Jan 13, 2025
1 parent 1b04567 commit f64d63d
Show file tree
Hide file tree
Showing 18 changed files with 334 additions and 117 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 All @@ -18,6 +19,29 @@ type IntegrationTestViewProps = {
integrationTest?: IntegrationTestScenarioKind;
};

interface FormContext {
name: string;
description: string;
selected?: boolean;
}

export 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;
});
};

const IntegrationTestView: React.FunctionComponent<
React.PropsWithChildren<IntegrationTestViewProps>
> = ({ applicationName, integrationTest }) => {
Expand Down Expand Up @@ -52,27 +76,14 @@ const IntegrationTestView: React.FunctionComponent<
return formParams;
};

interface FormContext {
name: string;
description: string;
}

const getFormContextValues = (contexts: Context[] | null | undefined): FormContext[] => {
if (!contexts?.length) return [];

return contexts.map((context) => {
return context.name ? { name: context.name, description: context.description } : context;
});
};

const initialValues = {
integrationTest: {
name: integrationTest?.metadata.name ?? '',
url: url?.value ?? '',
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 @@ -6,8 +6,11 @@ import { createK8sWatchResourceMock, renderWithQueryClient } from '../../../../u
import { mockApplication } from '../../../ApplicationDetails/__data__/mock-data';
import { MockComponents } from '../../../Commits/CommitDetails/visualization/__data__/MockCommitWorkflowData';
import { WorkspaceContext } from '../../../Workspace/workspace-context';
import { MockIntegrationTestsWithGit } from '../../IntegrationTestsListView/__data__/mock-integration-tests';
import IntegrationTestView from '../IntegrationTestView';
import {
MockIntegrationTests,
MockIntegrationTestsWithGit,
} from '../../IntegrationTestsListView/__data__/mock-integration-tests';
import IntegrationTestView, { getFormContextValues } from '../IntegrationTestView';
import { createIntegrationTest } from '../utils/create-utils';

jest.mock('../../../../utils/analytics');
Expand Down Expand Up @@ -187,3 +190,31 @@ describe('IntegrationTestView', () => {
wrapper.getByLabelText(/Integration test name/).setAttribute('value', 'new value');
});
});

describe('getFormContextValues', () => {
it('should return default context when creating an integration test', () => {
const result = getFormContextValues(null);
expect(result).toEqual([
{
name: 'application',
description: 'execute the integration test in all cases - this would be the default state',
selected: true,
},
]);
});

it('should return the integration test contexts', () => {
const integrationTest = MockIntegrationTests[2];
const result = getFormContextValues(integrationTest);
expect(result).toEqual([
{
description: 'Application testing 3',
name: 'application',
},
{
description: 'Group testing 3',
name: 'group',
},
]);
});
});
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
Loading

0 comments on commit f64d63d

Please sign in to comment.