Skip to content

Commit 9e13d06

Browse files
ashley-o0oashley-o0o
authored
Create Review Page for Wizard (opendatahub-io#5267)
* Create Review Page for Wizard * Update structure and conncetions section * Tidy up * Fix runtime arg textbox
1 parent 4a371ac commit 9e13d06

File tree

5 files changed

+402
-31
lines changed

5 files changed

+402
-31
lines changed

‎frontend/src/__tests__/cypress/cypress/tests/mocked/modelServing/modelServingDeploy.cy.ts‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -405,9 +405,9 @@ describe('Model Serving Deploy Wizard', () => {
405405
modelServingWizard.findServiceNameAlert().should('not.exist');
406406
modelServingWizard.findRemoveServiceAccountByIndex(1).click();
407407
modelServingWizard.findServiceAccountByIndex(0).clear();
408-
modelServingWizard.findSubmitButton().should('be.disabled'); //TODO: Change back to findNextButton() when submit page is added
408+
modelServingWizard.findNextButton().should('be.disabled');
409409
modelServingWizard.findServiceAccountByIndex(0).clear().type('new name');
410-
// modelServingWizard.findNextButton().should('be.enabled').click(); //TODO: Uncomment when submit page is added
410+
modelServingWizard.findNextButton().should('be.enabled').click();
411411

412412
// Step 4: Summary
413413
modelServingWizard.findSubmitButton().should('be.enabled').click();
@@ -682,7 +682,7 @@ describe('Model Serving Deploy Wizard', () => {
682682
modelServingWizard.findEnvVariableName('0').clear().type('valid_name');
683683
modelServingWizard.findEnvVariableValue('0').type('test-value');
684684

685-
// modelServingWizard.findNextButton().should('be.enabled').click(); //TODO: Uncomment when submit page is added
685+
modelServingWizard.findNextButton().should('be.enabled').click();
686686

687687
// Step 4: Summary
688688

@@ -814,7 +814,7 @@ describe('Model Serving Deploy Wizard', () => {
814814
modelServingWizard.findServingRuntimeTemplateSearchSelector().click();
815815
modelServingWizard.findGlobalScopedTemplateOption('OpenVINO').should('exist').click();
816816
modelServingWizard.findNextButton().should('be.enabled').click();
817-
// modelServingWizard.findNextButton().should('be.enabled').click(); //TODO: Uncomment when submit page is added
817+
modelServingWizard.findNextButton().should('be.enabled').click();
818818

819819
// test submitting form, an error should appear
820820
modelServingWizard.findSubmitButton().should('be.enabled').click();
@@ -985,7 +985,7 @@ describe('Model Serving Deploy Wizard', () => {
985985
modelServingWizard.findNextButton().should('be.enabled').click();
986986

987987
// Verify submit is enabled before testing env vars
988-
modelServingWizard.findSubmitButton().should('be.enabled'); //TODO: Change back to findNextButton() when submit page is added
988+
modelServingWizard.findNextButton().should('be.enabled');
989989

990990
// Add environment variable with invalid name
991991
modelServingWizard.findEnvVariablesCheckbox().click();
@@ -995,23 +995,23 @@ describe('Model Serving Deploy Wizard', () => {
995995
'Environment variable name must start with a letter or underscore and contain only letters, numbers, and underscores',
996996
).should('be.visible');
997997
// Verify submit is disabled with invalid env var
998-
modelServingWizard.findSubmitButton().should('be.disabled'); //TODO: Change back to findNextButton() when submit page is added
998+
modelServingWizard.findNextButton().should('be.disabled');
999999

10001000
// Test invalid env var name with special characters
10011001
modelServingWizard.findEnvVariableName('0').clear().type('invalid@name');
10021002
cy.findByText(
10031003
'Environment variable name must start with a letter or underscore and contain only letters, numbers, and underscores',
10041004
).should('be.visible');
10051005
// Verify submit is disabled with invalid env var
1006-
modelServingWizard.findSubmitButton().should('be.disabled'); //TODO: Change back to findNextButton() when submit page is added
1006+
modelServingWizard.findNextButton().should('be.disabled');
10071007

10081008
// Test valid env var name
10091009
modelServingWizard.findEnvVariableName('0').clear().type('VALID_NAME');
10101010
cy.findByText(
10111011
'Environment variable name must start with a letter or underscore and contain only letters, numbers, and underscores',
10121012
).should('not.exist');
10131013
// Verify submit is enabled with valid env var
1014-
modelServingWizard.findSubmitButton().should('be.enabled'); //TODO: Change back to findNextButton() when submit page is added
1014+
modelServingWizard.findNextButton().should('be.enabled');
10151015
});
10161016

10171017
it('Deploy OCI Model and check paste functionality', () => {
@@ -1072,7 +1072,7 @@ describe('Model Serving Deploy Wizard', () => {
10721072
modelServingWizard.findGlobalScopedTemplateOption('OpenVINO').should('exist').click();
10731073
modelServingWizard.findNextButton().should('be.enabled').click();
10741074
//Step 3: Advanced Options
1075-
// modelServingWizard.findNextButton().should('be.enabled').click(); //TODO: Uncomment when submit page is added
1075+
modelServingWizard.findNextButton().should('be.enabled').click();
10761076
//Step 4: Summary
10771077
modelServingWizard.findSubmitButton().should('be.enabled').click();
10781078

@@ -1213,7 +1213,7 @@ describe('Model Serving Deploy Wizard', () => {
12131213
modelServingWizard.findGlobalScopedTemplateOption('OpenVINO').should('exist').click();
12141214
modelServingWizard.findNextButton().should('be.enabled').click();
12151215
//Step 3: Advanced Options
1216-
// modelServingWizard.findNextButton().should('be.enabled').click(); //TODO: Uncomment when submit page is added
1216+
modelServingWizard.findNextButton().should('be.enabled').click();
12171217
//Step 4: Summary
12181218
modelServingWizard.findSubmitButton().should('be.enabled').click();
12191219

@@ -1362,7 +1362,9 @@ describe('Model Serving Deploy Wizard', () => {
13621362
modelServingWizardEdit.findExternalRouteCheckbox().should('be.checked');
13631363
modelServingWizardEdit.findTokenAuthenticationCheckbox().click();
13641364
modelServingWizardEdit.findServiceAccountByIndex(0).should('have.value', 'default-name');
1365-
modelServingWizardEdit.findUpdateDeploymentButton().should('be.enabled').click(); //TODO: Change back to findNextButton() when submit page is added
1365+
modelServingWizardEdit.findNextButton().should('be.enabled').click();
1366+
1367+
modelServingWizardEdit.findUpdateDeploymentButton().should('be.enabled').click();
13661368
});
13671369

13681370
it('Verify cpu and memory request and limits values when editing KServe model', () => {
@@ -1520,7 +1522,7 @@ describe('Model Serving Deploy Wizard', () => {
15201522

15211523
// Step 3: Advanced Options
15221524
modelServingWizard.findAdvancedOptionsStep().should('be.enabled');
1523-
// modelServingWizard.findNextButton().should('be.enabled').click(); //TODO: Uncomment when submit page is added
1525+
modelServingWizard.findNextButton().should('be.enabled').click();
15241526

15251527
// Step 4: Summary
15261528
modelServingWizard.findSubmitButton().should('be.enabled').click();

‎frontend/src/__tests__/cypress/cypress/tests/mocked/modelServing/modelServingLlmd.cy.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ describe('Model Serving LLMD', () => {
313313
modelServingWizard.findNextButton().should('be.enabled').click();
314314

315315
// Step 3: Advanced Options
316-
modelServingWizard.findSubmitButton().should('be.enabled'); //TODO: Change back to findNextButton() when submit page is added
316+
modelServingWizard.findNextButton().should('be.enabled');
317317
modelServingWizard.findExternalRouteCheckbox().should('not.exist');
318318
modelServingWizard.findTokenAuthenticationCheckbox().should('be.enabled');
319319
modelServingWizard.findTokenAuthenticationCheckbox().click();
@@ -324,7 +324,7 @@ describe('Model Serving LLMD', () => {
324324
modelServingWizard.findAddVariableButton().click();
325325
modelServingWizard.findEnvVariableName('0').clear().type('MY_ENV');
326326
modelServingWizard.findEnvVariableValue('0').type('MY_VALUE');
327-
// modelServingWizard.findNextButton().should('be.enabled').click(); //TODO: Uncomment when submit page is added
327+
modelServingWizard.findNextButton().should('be.enabled').click();
328328

329329
// Step 4: Summary
330330
modelServingWizard.findSubmitButton().should('be.enabled').click();
@@ -473,15 +473,15 @@ describe('Model Serving LLMD', () => {
473473
modelServingWizardEdit.findNextButton().should('be.enabled').click();
474474

475475
// Step 3: Advanced Options
476-
//modelServingWizardEdit.findNextButton().should('be.enabled'); //TODO: Uncomment when summary page is added back
476+
modelServingWizardEdit.findNextButton().should('be.enabled');
477477
modelServingWizardEdit.findTokenAuthenticationCheckbox().should('be.checked');
478478
modelServingWizardEdit.findTokenAuthenticationCheckbox().click();
479479
modelServingWizardEdit.findTokenAuthenticationCheckbox().should('not.be.checked');
480480
modelServingWizardEdit.findRuntimeArgsCheckbox().click();
481481
modelServingWizardEdit.findRuntimeArgsTextBox().type('--arg=value1');
482482
modelServingWizardEdit.findEnvVariableName('0').clear().type('MY_ENV');
483483
modelServingWizardEdit.findEnvVariableValue('0').clear().type('MY_VALUE');
484-
//modelServingWizardEdit.findNextButton().should('be.enabled').click(); //TODO: Uncomment when summary page is added back
484+
modelServingWizardEdit.findNextButton().should('be.enabled').click();
485485

486486
// Step 4: Summary
487487
modelServingWizardEdit.findSubmitButton().should('be.enabled').click();
@@ -541,7 +541,7 @@ describe('Model Serving LLMD', () => {
541541
modelServingWizard.findSaveAsMaaSCheckbox().click();
542542
modelServingWizard.findSaveAsMaaSCheckbox().should('be.checked');
543543
modelServingWizard.findUseCaseInput().should('be.visible').type('Test MaaS use case');
544-
// modelServingWizard.findNextButton().click(); //TODO: Uncomment when summary page is added
544+
modelServingWizard.findNextButton().click();
545545

546546
// Submit and verify MaaS-specific annotations and gateway refs
547547
modelServingWizard.findSubmitButton().click();

‎packages/model-serving/src/components/deploymentWizard/ModelDeploymentWizard.tsx‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { useModelDeploymentWizardValidation } from './useDeploymentWizardValidat
1212
import { ModelSourceStepContent } from './steps/ModelSourceStep';
1313
import { AdvancedSettingsStepContent } from './steps/AdvancedOptionsStep';
1414
import { ModelDeploymentStepContent } from './steps/ModelDeploymentStep';
15+
import { ReviewStepContent } from './steps/ReviewStep';
1516
import { useDeployMethod } from './useDeployMethod';
1617
import type { InitialWizardFormData } from './types';
1718
import { WizardFooterWithDisablingNext } from '../generic/WizardFooterWithDisablingNext';
@@ -147,19 +148,11 @@ const ModelDeploymentWizard: React.FC<ModelDeploymentWizardProps> = ({
147148
clearError={() => setSubmitError(null)}
148149
isLoading={isLoading}
149150
submitButtonText={primaryButtonText}
150-
isAdvancedSettingsStepValid={validation.isAdvancedSettingsStepValid} //TODO: Remove this line once summary page is added
151151
overwriteSupported={deployMethod?.properties.supportsOverwrite}
152152
onSave={onSave}
153153
/>
154154
),
155-
[
156-
submitError,
157-
isLoading,
158-
primaryButtonText,
159-
deployMethod?.properties.supportsOverwrite,
160-
onSave,
161-
validation.isAdvancedSettingsStepValid,
162-
], //TODO: Remove validation.isAdvancedSettingsStepValid once summary page is added
155+
[submitError, isLoading, primaryButtonText, deployMethod?.properties.supportsOverwrite, onSave],
163156
);
164157

165158
return (
@@ -197,8 +190,7 @@ const ModelDeploymentWizard: React.FC<ModelDeploymentWizardProps> = ({
197190
<Spinner />
198191
)}
199192
</WizardStep>
200-
{/* TODO: Uncomment when summary page is added */}
201-
{/* <WizardStep
193+
<WizardStep
202194
name="Review"
203195
id="summary-step"
204196
isDisabled={
@@ -207,8 +199,12 @@ const ModelDeploymentWizard: React.FC<ModelDeploymentWizardProps> = ({
207199
!validation.isAdvancedSettingsStepValid
208200
}
209201
>
210-
{wizardState.loaded.summaryLoaded ? 'Review step content' : <Spinner />}
211-
</WizardStep> */}
202+
{wizardState.loaded.summaryLoaded ? (
203+
<ReviewStepContent wizardState={wizardState} projectName={project.metadata.name} />
204+
) : (
205+
<Spinner />
206+
)}
207+
</WizardStep>
212208
</Wizard>
213209
</ApplicationsPage>
214210
);

‎packages/model-serving/src/components/deploymentWizard/fields/RuntimeArgsField.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ export const RuntimeArgsField: React.FC<RuntimeArgsFieldProps> = ({
6464
};
6565

6666
const handleTextAreaChange = (_e: React.FormEvent<HTMLTextAreaElement>, textValue: string) => {
67-
const newData = { ...data, args: textValue.split('\n').filter((arg) => arg.trim() !== '') };
67+
const newData = { ...data, args: textValue.split('\n') };
6868
onChange?.(newData);
6969
};
7070

0 commit comments

Comments
 (0)