Skip to content

Commit

Permalink
Update 'Resource name' fields to meet UX guidelines: Image streams
Browse files Browse the repository at this point in the history
  • Loading branch information
jeff-phillips-18 committed Nov 12, 2024
1 parent 429d339 commit 7a571df
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 61 deletions.
7 changes: 4 additions & 3 deletions backend/src/routes/api/images/imageUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,9 +264,10 @@ export const postImage = async (
'opendatahub.io/dashboard': 'true',
};
const imageStreams = (await getImageStreams(fastify, labels)) as ImageStream[];
const validName = imageStreams.filter((is) => is.metadata.name === body.name);
const name = body.name || `custom-${translateDisplayNameForK8s(body.display_name)}`;
const existingImage = imageStreams.find((is) => is.metadata.name === name);

if (validName.length > 0) {
if (existingImage) {
fastify.log.error('Duplicate name unable to add notebook image');
return {
success: false,
Expand All @@ -287,7 +288,7 @@ export const postImage = async (
body.recommendedAcceleratorIdentifiers ?? [],
),
},
name: `custom-${translateDisplayNameForK8s(body.display_name)}`,
name,
namespace: namespace,
labels: labels,
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { appChrome } from '~/__tests__/cypress/cypress/pages/appChrome';
import { Modal } from '~/__tests__/cypress/cypress/pages/components/Modal';
import { TableRow } from '~/__tests__/cypress/cypress/pages/components/table';
import { K8sNameDescriptionField } from '~/__tests__/cypress/cypress/pages/components/subComponents/K8sNameDescriptionField';
import { DeleteModal } from './components/DeleteModal';
import { TableToolbar } from './components/TableToolbar';

Expand Down Expand Up @@ -86,6 +87,8 @@ class NotebookImageDeleteModal extends DeleteModal {
}

class ImportUpdateNotebookImageModal extends Modal {
k8sNameDescription = new K8sNameDescriptionField('byon-image');

constructor(private edit = false) {
super(`${edit ? 'Update' : 'Import'} notebook image`);
}
Expand All @@ -99,11 +102,11 @@ class ImportUpdateNotebookImageModal extends Modal {
}

findNameInput() {
return this.find().findByTestId('byon-image-name-input');
return this.k8sNameDescription.findDisplayNameInput();
}

findDescriptionInput() {
return this.find().findByTestId('byon-image-description-input');
return this.k8sNameDescription.findDescriptionInput();
}

// Software tab
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,24 @@ describe('Notebook image settings', () => {

importNotebookImageModal.findSaveResourceButton('Software').click();
importNotebookImageModal.findSubmitButton().should('be.enabled');

// test resource name validation
importNotebookImageModal.k8sNameDescription.findResourceEditLink().click();
importNotebookImageModal.k8sNameDescription
.findResourceNameInput()
.should('have.attr', 'aria-invalid', 'false');
// Invalid character k8s names fail
importNotebookImageModal.k8sNameDescription
.findResourceNameInput()
.clear()
.type('InVaLiD vAlUe!');
importNotebookImageModal.k8sNameDescription
.findResourceNameInput()
.should('have.attr', 'aria-invalid', 'true');
importNotebookImageModal.findSubmitButton().should('be.disabled');
importNotebookImageModal.k8sNameDescription.findResourceNameInput().clear().type('image');
importNotebookImageModal.findSubmitButton().should('be.enabled');

let notebookImageTabRow = importNotebookImageModal.getSoftwareRow('software');
notebookImageTabRow.shouldHaveVersionColumn('version');

Expand Down Expand Up @@ -198,6 +216,7 @@ describe('Notebook image settings', () => {
expect(interception.request.body).to.eql({
/* eslint-disable-next-line camelcase */
display_name: 'image',
name: 'image',
url: 'image:latest',
description: '',
recommendedAcceleratorIdentifiers: [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import React from 'react';
import {
Form,
FormGroup,
TextInput,
Modal,
ModalVariant,
Tabs,
Expand All @@ -18,6 +17,10 @@ import DashboardModalFooter from '~/concepts/dashboard/DashboardModalFooter';
import { filterBlankPackages } from '~/pages/BYONImages/utils';
import { AcceleratorIdentifierMultiselect } from '~/pages/BYONImages/BYONImageModal/AcceleratorIdentifierMultiselect';
import DashboardPopupIconButton from '~/concepts/dashboard/DashboardPopupIconButton';
import K8sNameDescriptionField, {
useK8sNameDescriptionFieldData,
} from '~/concepts/k8s/K8sNameDescriptionField/K8sNameDescriptionField';
import { isK8sNameDescriptionDataValid } from '~/concepts/k8s/K8sNameDescriptionField/utils';
import ImageLocationField from './ImageLocationField';
import DisplayedContentTabContent from './DisplayedContentTabContent';

Expand All @@ -37,8 +40,6 @@ const ManageBYONImageModal: React.FC<ManageBYONImageModalProps> = ({ existingIma
);
const [isProgress, setIsProgress] = React.useState(false);
const [repository, setRepository] = React.useState('');
const [displayName, setDisplayName] = React.useState('');
const [description, setDescription] = React.useState('');
const [recommendedAcceleratorIdentifiers, setRecommendedAcceleratorIdentifiers] = React.useState<
string[]
>([]);
Expand All @@ -52,11 +53,19 @@ const ManageBYONImageModal: React.FC<ManageBYONImageModalProps> = ({ existingIma

const isEditing = editIndex !== undefined;

const { data: byonNameDesc, onDataChange: setByonNameDesc } = useK8sNameDescriptionFieldData({
initialData: existingImage
? {
name: existingImage.display_name,
k8sName: existingImage.name,
description: existingImage.description,
}
: undefined,
});

React.useEffect(() => {
if (existingImage) {
setRepository(existingImage.url);
setDisplayName(existingImage.display_name);
setDescription(existingImage.description);
setPackages(existingImage.packages);
setSoftware(existingImage.software);
setTempPackages(existingImage.packages);
Expand All @@ -65,46 +74,34 @@ const ManageBYONImageModal: React.FC<ManageBYONImageModalProps> = ({ existingIma
}
}, [existingImage]);

const onBeforeClose = (submitted: boolean) => {
onClose(submitted);
setIsProgress(false);
setActiveTabKey(DisplayedContentTab.SOFTWARE);
setRepository('');
setDisplayName('');
setDescription('');
setRecommendedAcceleratorIdentifiers([]);
setSoftware([]);
setPackages([]);
setTempSoftware([]);
setTempPackages([]);
setError(undefined);
};

const handleResponse = (response: ResponseStatus) => {
setIsProgress(false);
if (response.success === false) {
setError(new Error(response.error));
} else {
onBeforeClose(true);
onClose(true);
}
};

const submit = () => {
setIsProgress(true);
if (existingImage) {
updateBYONImage({
name: existingImage.name,
// eslint-disable-next-line camelcase
display_name: displayName,
description,
display_name: byonNameDesc.name,
description: byonNameDesc.description,
recommendedAcceleratorIdentifiers,
packages: filterBlankPackages(packages),
software: filterBlankPackages(software),
}).then(handleResponse);
} else {
importBYONImage({
// eslint-disable-next-line camelcase
display_name: displayName,
display_name: byonNameDesc.name,
name: byonNameDesc.k8sName.value,
url: repository,
description,
description: byonNameDesc.description,
recommendedAcceleratorIdentifiers,
provider: userName,
packages: filterBlankPackages(packages),
Expand All @@ -119,16 +116,21 @@ const ManageBYONImageModal: React.FC<ManageBYONImageModalProps> = ({ existingIma
variant={ModalVariant.medium}
title={`${existingImage ? 'Update' : 'Import'} notebook image`}
isOpen
onClose={() => onBeforeClose(false)}
onClose={() => onClose(false)}
showClose={!isEditing}
footer={
<DashboardModalFooter
error={error}
alertTitle={`Error ${existingImage ? 'updating' : 'importing'} notebook image`}
submitLabel={existingImage ? 'Update' : 'Import'}
isSubmitDisabled={isProgress || displayName === '' || repository === '' || isEditing}
isSubmitDisabled={
isProgress ||
!isK8sNameDescriptionDataValid(byonNameDesc) ||
repository === '' ||
isEditing
}
onSubmit={submit}
onCancel={() => onBeforeClose(false)}
onCancel={() => onClose(false)}
isCancelDisabled={isEditing}
/>
}
Expand All @@ -145,34 +147,11 @@ const ManageBYONImageModal: React.FC<ManageBYONImageModalProps> = ({ existingIma
location={repository}
setLocation={setRepository}
/>

<FormGroup label="Name" isRequired fieldId="byon-image-name-input">
<TextInput
id="byon-image-name-input"
isRequired
type="text"
data-testid="byon-image-name-input"
name="byon-image-name-input"
value={displayName}
onChange={(e, value) => {
setDisplayName(value);
}}
/>
</FormGroup>
<FormGroup label="Description" fieldId="byon-image-description-input">
<TextInput
id="byon-image-description-input"
isRequired
type="text"
data-testid="byon-image-description-input"
name="byon-image-description-input"
aria-describedby="byon-image-description-input"
value={description}
onChange={(e, value) => {
setDescription(value);
}}
/>
</FormGroup>
<K8sNameDescriptionField
data={byonNameDesc}
onDataChange={setByonNameDesc}
dataTestId="byon-image"
/>
<FormGroup
label="Accelerator identifier"
labelIcon={
Expand Down

0 comments on commit 7a571df

Please sign in to comment.