From 7a571df1df4f511d84948d92badfa92202e2caa1 Mon Sep 17 00:00:00 2001 From: Jeffrey Phillips Date: Thu, 7 Nov 2024 07:52:47 -0500 Subject: [PATCH] Update 'Resource name' fields to meet UX guidelines: Image streams --- backend/src/routes/api/images/imageUtils.ts | 7 +- .../cypress/pages/notebookImageSettings.ts | 7 +- .../notebookImageSettings.cy.ts | 19 ++++ .../BYONImageModal/ManageBYONImageModal.tsx | 91 +++++++------------ 4 files changed, 63 insertions(+), 61 deletions(-) diff --git a/backend/src/routes/api/images/imageUtils.ts b/backend/src/routes/api/images/imageUtils.ts index c826584643..0e49a19732 100644 --- a/backend/src/routes/api/images/imageUtils.ts +++ b/backend/src/routes/api/images/imageUtils.ts @@ -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, @@ -287,7 +288,7 @@ export const postImage = async ( body.recommendedAcceleratorIdentifiers ?? [], ), }, - name: `custom-${translateDisplayNameForK8s(body.display_name)}`, + name, namespace: namespace, labels: labels, }, diff --git a/frontend/src/__tests__/cypress/cypress/pages/notebookImageSettings.ts b/frontend/src/__tests__/cypress/cypress/pages/notebookImageSettings.ts index 31e470a6cd..3cef8c1982 100644 --- a/frontend/src/__tests__/cypress/cypress/pages/notebookImageSettings.ts +++ b/frontend/src/__tests__/cypress/cypress/pages/notebookImageSettings.ts @@ -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'; @@ -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`); } @@ -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 diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/notebookImageSettings/notebookImageSettings.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/notebookImageSettings/notebookImageSettings.cy.ts index 84b3e11d23..155bd47218 100644 --- a/frontend/src/__tests__/cypress/cypress/tests/mocked/notebookImageSettings/notebookImageSettings.cy.ts +++ b/frontend/src/__tests__/cypress/cypress/tests/mocked/notebookImageSettings/notebookImageSettings.cy.ts @@ -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'); @@ -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: [], diff --git a/frontend/src/pages/BYONImages/BYONImageModal/ManageBYONImageModal.tsx b/frontend/src/pages/BYONImages/BYONImageModal/ManageBYONImageModal.tsx index c9723b4d6c..dfe9787b82 100644 --- a/frontend/src/pages/BYONImages/BYONImageModal/ManageBYONImageModal.tsx +++ b/frontend/src/pages/BYONImages/BYONImageModal/ManageBYONImageModal.tsx @@ -2,7 +2,6 @@ import React from 'react'; import { Form, FormGroup, - TextInput, Modal, ModalVariant, Tabs, @@ -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'; @@ -37,8 +40,6 @@ const ManageBYONImageModal: React.FC = ({ 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[] >([]); @@ -52,11 +53,19 @@ const ManageBYONImageModal: React.FC = ({ 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); @@ -65,36 +74,23 @@ const ManageBYONImageModal: React.FC = ({ 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), @@ -102,9 +98,10 @@ const ManageBYONImageModal: React.FC = ({ existingIma } 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), @@ -119,16 +116,21 @@ const ManageBYONImageModal: React.FC = ({ existingIma variant={ModalVariant.medium} title={`${existingImage ? 'Update' : 'Import'} notebook image`} isOpen - onClose={() => onBeforeClose(false)} + onClose={() => onClose(false)} showClose={!isEditing} footer={ onBeforeClose(false)} + onCancel={() => onClose(false)} isCancelDisabled={isEditing} /> } @@ -145,34 +147,11 @@ const ManageBYONImageModal: React.FC = ({ existingIma location={repository} setLocation={setRepository} /> - - - { - setDisplayName(value); - }} - /> - - - { - setDescription(value); - }} - /> - +