Skip to content

Commit e48950c

Browse files
fix the code for the actual problem
Signed-off-by: Philip Colares Carneiro <philip.colares@gmail.com>
1 parent ba18129 commit e48950c

3 files changed

Lines changed: 83 additions & 25 deletions

File tree

frontend/src/pages/modelRegistrySettings/CreateModal.tsx

Lines changed: 35 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,9 @@ import {
1111
Stack,
1212
Spinner,
1313
TextInput,
14-
Modal,
15-
ModalBody,
16-
ModalHeader,
17-
ModalFooter,
1814
} from '@patternfly/react-core';
1915
import SimpleSelect, { SimpleSelectOption } from '#~/components/SimpleSelect';
20-
import DashboardModalFooter from '#~/concepts/dashboard/DashboardModalFooter';
16+
import ContentModal from '#~/components/modals/ContentModal';
2117
import { ModelRegistryKind } from '#~/k8sTypes';
2218
import { ModelRegistryModel } from '#~/api';
2319
import {
@@ -54,6 +50,7 @@ import {
5450
DEFAULT_DATABASE_NAME,
5551
DEFAULT_MYSQL_PORT,
5652
DEFAULT_POSTGRES_PORT,
53+
MAX_MODEL_REGISTRY_NAME_LENGTH,
5754
ResourceType,
5855
SecureDBRType,
5956
} from './const';
@@ -390,9 +387,8 @@ const CreateModal: React.FC<CreateModalProps> = ({ onClose, refresh, modelRegist
390387
const hasContent = (value: string): boolean => !!value.trim().length;
391388

392389
const canSubmit = () => {
393-
const isValidName = isValidK8sName(
394-
nameDesc.k8sName.value || translateDisplayNameForK8s(nameDesc.name),
395-
);
390+
const k8sName = nameDesc.k8sName.value || translateDisplayNameForK8s(nameDesc.name);
391+
const isValidName = isValidK8sName(k8sName) && k8sName.length <= MAX_MODEL_REGISTRY_NAME_LENGTH;
396392

397393
if (databaseSource === DatabaseSource.DEFAULT) {
398394
// For default database, only name is required
@@ -419,11 +415,36 @@ const CreateModal: React.FC<CreateModalProps> = ({ onClose, refresh, modelRegist
419415
];
420416

421417
return (
422-
<Modal isOpen onClose={onCancelClose} variant="medium">
423-
<ModalHeader title={`${mr ? 'Edit' : 'Create'} model registry`} />
424-
<ModalBody>
418+
<ContentModal
419+
onClose={onCancelClose}
420+
variant="medium"
421+
title={`${mr ? 'Edit' : 'Create'} model registry`}
422+
error={error}
423+
alertTitle={`Error ${mr ? 'updating' : 'creating'} model registry`}
424+
buttonActions={[
425+
{
426+
label: mr ? 'Update' : 'Create',
427+
onClick: onSubmit,
428+
variant: 'primary',
429+
isLoading: isSubmitting,
430+
isDisabled: !canSubmit(),
431+
dataTestId: 'modal-submit-button',
432+
},
433+
{
434+
label: 'Cancel',
435+
onClick: onCancelClose,
436+
variant: 'link',
437+
dataTestId: 'modal-cancel-button',
438+
},
439+
]}
440+
contents={
425441
<Form>
426-
<K8sNameDescriptionField dataTestId="mr" data={nameDesc} onDataChange={setNameDesc} />
442+
<K8sNameDescriptionField
443+
dataTestId="mr"
444+
data={nameDesc}
445+
onDataChange={setNameDesc}
446+
maxLength={MAX_MODEL_REGISTRY_NAME_LENGTH}
447+
/>
427448
<FormSection title="Database" description="Choose where to store model data.">
428449
<FormGroup role="radiogroup" fieldId="mr-database-source">
429450
<Stack hasGutter>
@@ -632,19 +653,8 @@ const CreateModal: React.FC<CreateModalProps> = ({ onClose, refresh, modelRegist
632653
)}
633654
</FormSection>
634655
</Form>
635-
</ModalBody>
636-
<ModalFooter>
637-
<DashboardModalFooter
638-
onCancel={onCancelClose}
639-
onSubmit={onSubmit}
640-
submitLabel={mr ? 'Update' : 'Create'}
641-
isSubmitLoading={isSubmitting}
642-
isSubmitDisabled={!canSubmit()}
643-
error={error}
644-
alertTitle={`Error ${mr ? 'updating' : 'creating'} model registry`}
645-
/>
646-
</ModalFooter>
647-
</Modal>
656+
}
657+
/>
648658
);
649659
};
650660

frontend/src/pages/modelRegistrySettings/const.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
export const MAX_MODEL_REGISTRY_NAME_LENGTH = 40;
2+
13
export const ODH_TRUSTED_BUNDLE = 'odh-trusted-ca-bundle';
24
export const CA_BUNDLE_CRT = 'ca-bundle.crt';
35
export const ODH_CA_BUNDLE_CRT = 'odh-ca-bundle.crt';

packages/cypress/cypress/tests/mocked/modelRegistrySettings/modelRegistrySettings.cy.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,52 @@ describe('CreateModal', () => {
336336
modelRegistrySettings.shouldHaveAllErrors();
337337
});
338338

339+
it('should enforce maxLength of 40 on the name input field', () => {
340+
modelRegistrySettings.visit(true);
341+
modelRegistrySettings.findCreateButton().click();
342+
modelRegistrySettings
343+
.findFormField(FormFieldSelector.NAME)
344+
.should('have.attr', 'maxLength', '40');
345+
});
346+
347+
it('should disable submit when resource name exceeds 40 characters', () => {
348+
modelRegistrySettings.visit(true);
349+
modelRegistrySettings.findCreateButton().click();
350+
modelRegistrySettings.findDatabaseSourceDefaultRadio().should('be.checked');
351+
352+
modelRegistrySettings.findFormField(FormFieldSelector.NAME).type('a'.repeat(40));
353+
modelRegistrySettings.findSubmitButton().should('be.enabled');
354+
355+
modelRegistrySettings.k8sNameDescription.findResourceEditLink().click();
356+
modelRegistrySettings.k8sNameDescription.findResourceNameInput().clear().type('a'.repeat(41));
357+
modelRegistrySettings.findSubmitButton().should('be.disabled');
358+
});
359+
360+
it('should show character count warning when name approaches 40 limit', () => {
361+
modelRegistrySettings.visit(true);
362+
modelRegistrySettings.findCreateButton().click();
363+
364+
const nearLimitName = 'a'.repeat(31);
365+
modelRegistrySettings.findFormField(FormFieldSelector.NAME).type(nearLimitName);
366+
cy.contains('Cannot exceed 40 characters').should('be.visible');
367+
cy.contains('remaining').should('be.visible');
368+
});
369+
370+
it('should allow creation with name at exactly 40 characters using default database', () => {
371+
modelRegistrySettings.visit(true);
372+
modelRegistrySettings.findCreateButton().click();
373+
modelRegistrySettings.findDatabaseSourceDefaultRadio().should('be.checked');
374+
375+
const exactName = 'a'.repeat(40);
376+
modelRegistrySettings.findFormField(FormFieldSelector.NAME).type(exactName);
377+
modelRegistrySettings.findSubmitButton().should('be.enabled');
378+
modelRegistrySettings.findSubmitButton().click();
379+
380+
cy.wait('@createModelRegistry').then((interception) => {
381+
expect(interception.request.body.modelRegistry.metadata.name).to.have.length.at.most(40);
382+
});
383+
});
384+
339385
it('should enable submit button if fields are valid', () => {
340386
modelRegistrySettings.visit(true);
341387
modelRegistrySettings.findCreateButton().click();

0 commit comments

Comments
 (0)