Skip to content

Commit cb22f3e

Browse files
katieperry4katieperry4
committed
PR comments
1 parent f52353c commit cb22f3e

File tree

5 files changed

+60
-55
lines changed

5 files changed

+60
-55
lines changed

‎packages/maas/frontend/src/odh/modelServingExtensions/modelDeploymentWizard/MaaSEndpointCheckbox.tsx‎

Lines changed: 43 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,46 @@ export type MaaSTierValue = {
4242
selectedTierNames?: string[]; // if tiersDropdownSelection is 'specify-tiers', this is the list of selected tier names
4343
};
4444

45+
//// Dropdown options ////
46+
47+
export enum TierDropdownOption {
48+
AllTiers = 'all-tiers',
49+
NoTiers = 'no-tiers',
50+
SpecifyTiers = 'specify-tiers',
51+
}
52+
53+
const TIER_DROPDOWN_OPTIONS: Array<{ key: TierDropdownOption; label: string }> = [
54+
{ key: TierDropdownOption.AllTiers, label: 'All tiers' },
55+
{ key: TierDropdownOption.NoTiers, label: 'No tiers' },
56+
{ key: TierDropdownOption.SpecifyTiers, label: 'Specific tiers' },
57+
];
58+
59+
const getTierDropdownLabel = (key: TierDropdownOption): string => {
60+
const option = TIER_DROPDOWN_OPTIONS.find((opt) => opt.key === key);
61+
return option?.label ?? 'All tiers';
62+
};
63+
64+
const getMaaSTierLabel = (
65+
value: MaaSTierValue,
66+
externalData?: { data: MaaSEndpointsExternalData },
67+
): string => {
68+
const availableTiers = externalData?.data.tiers ?? [];
69+
const selection = value.tiersDropdownSelection ?? TierDropdownOption.AllTiers;
70+
if (selection === TierDropdownOption.NoTiers) {
71+
return getTierDropdownLabel(TierDropdownOption.NoTiers);
72+
}
73+
if (selection === TierDropdownOption.SpecifyTiers) {
74+
// Matching the display name of the selected tiers
75+
return (
76+
availableTiers
77+
.filter((tier: Tier) => value.selectedTierNames?.includes(tier.name ?? ''))
78+
.map((tier: Tier) => tier.displayName ?? tier.name ?? '')
79+
.join(', ') || 'Tiers selected'
80+
);
81+
}
82+
return getTierDropdownLabel(TierDropdownOption.AllTiers);
83+
};
84+
4585
//// Zod Validation Schema ////
4686

4787
/**
@@ -53,7 +93,7 @@ export type MaaSTierValue = {
5393
export const maasEndpointsFieldSchema = z
5494
.object({
5595
isChecked: z.boolean(),
56-
tiersDropdownSelection: z.enum(['all-tiers', 'no-tiers', 'specify-tiers']).optional(),
96+
tiersDropdownSelection: z.nativeEnum(TierDropdownOption).optional(),
5797
selectedTierNames: z.array(z.string()).optional(),
5898
})
5999
.refine(
@@ -97,41 +137,6 @@ const useMaaSEndpointsExternalData: MaaSEndpointsFieldType['externalDataHook'] =
97137
[tiers, hasViewTiersPermission, loaded, error],
98138
);
99139
};
100-
//// Dropdown options ////
101-
102-
type TierDropdownOption = 'all-tiers' | 'no-tiers' | 'specify-tiers';
103-
104-
const TIER_DROPDOWN_OPTIONS: Array<{ key: TierDropdownOption; label: string }> = [
105-
{ key: 'all-tiers', label: 'All tiers' },
106-
{ key: 'no-tiers', label: 'No tiers' },
107-
{ key: 'specify-tiers', label: 'Specific tiers' },
108-
];
109-
110-
const getTierDropdownLabel = (key: TierDropdownOption): string => {
111-
const option = TIER_DROPDOWN_OPTIONS.find((opt) => opt.key === key);
112-
return option?.label ?? 'All tiers';
113-
};
114-
115-
const getMaaSTierLabel = (
116-
value: MaaSTierValue,
117-
externalData?: { data: MaaSEndpointsExternalData; loaded: boolean; loadError?: Error },
118-
): string => {
119-
const availableTiers = externalData?.data.tiers ?? [];
120-
const selection = value.tiersDropdownSelection ?? 'all-tiers';
121-
if (selection === 'no-tiers') {
122-
return 'No tiers';
123-
}
124-
if (selection === 'specify-tiers') {
125-
// Matching the display name of the selected tiers
126-
return (
127-
availableTiers
128-
.filter((tier: Tier) => value.selectedTierNames?.includes(tier.name ?? ''))
129-
.map((tier: Tier) => tier.displayName ?? tier.name ?? '')
130-
.join(', ') || 'Tiers selected'
131-
);
132-
}
133-
return 'All tiers';
134-
};
135140

136141
//// Component ////
137142

@@ -175,7 +180,7 @@ const MaasEndpointField: React.FC<MaasEndpointFieldProps> = ({
175180
if (checked) {
176181
onChange({
177182
isChecked: true,
178-
tiersDropdownSelection: 'all-tiers',
183+
tiersDropdownSelection: TierDropdownOption.AllTiers,
179184
selectedTierNames: [],
180185
});
181186
} else {
@@ -266,7 +271,7 @@ const MaasEndpointField: React.FC<MaasEndpointFieldProps> = ({
266271
placeholder="Select tier access"
267272
value={value.tiersDropdownSelection ?? 'all-tiers'}
268273
toggleLabel={getTierDropdownLabel(
269-
value.tiersDropdownSelection ?? 'all-tiers',
274+
value.tiersDropdownSelection ?? TierDropdownOption.AllTiers,
270275
)}
271276
onChange={handleDropdownChange}
272277
popperProps={{ appendTo: 'inline' }}
@@ -346,8 +351,6 @@ export const MaaSEndpointFieldWizardField: MaaSEndpointsFieldType = {
346351
value: () =>
347352
getMaaSTierLabel(value, {
348353
data: externalData ?? { tiers: [], hasViewTiersPermission: false },
349-
loaded: true,
350-
loadError: undefined,
351354
}),
352355
optional: true,
353356
isVisible: () => value.isChecked,

‎packages/maas/frontend/src/odh/modelServingExtensions/modelDeploymentWizard/__tests__/maasDeploymentTransformer.test.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
extractMaaSEndpointData,
66
MAAS_TIERS_ANNOTATION,
77
} from '../maasDeploymentTransformer';
8+
import { TierDropdownOption } from '../MaaSEndpointCheckbox';
89

910
const createMockDeployment = (
1011
overrides: Partial<LLMdDeployment['model']> = {},
@@ -43,7 +44,7 @@ describe('maasDeploymentTransformer', () => {
4344
const deployment = createMockDeployment();
4445
const fieldData: MaaSTierValue = {
4546
isChecked: true,
46-
tiersDropdownSelection: 'all-tiers',
47+
tiersDropdownSelection: TierDropdownOption.AllTiers,
4748
selectedTierNames: [],
4849
};
4950

@@ -59,7 +60,7 @@ describe('maasDeploymentTransformer', () => {
5960
const deployment = createMockDeployment();
6061
const fieldData: MaaSTierValue = {
6162
isChecked: true,
62-
tiersDropdownSelection: 'no-tiers',
63+
tiersDropdownSelection: TierDropdownOption.NoTiers,
6364
};
6465

6566
const result = applyMaaSEndpointData(deployment, fieldData);
@@ -72,7 +73,7 @@ describe('maasDeploymentTransformer', () => {
7273
const deployment = createMockDeployment();
7374
const fieldData: MaaSTierValue = {
7475
isChecked: true,
75-
tiersDropdownSelection: 'specify-tiers',
76+
tiersDropdownSelection: TierDropdownOption.SpecifyTiers,
7677
selectedTierNames: ['tier-1', 'tier-2'],
7778
};
7879

@@ -93,7 +94,7 @@ describe('maasDeploymentTransformer', () => {
9394
};
9495
const fieldData: MaaSTierValue = {
9596
isChecked: true,
96-
tiersDropdownSelection: 'all-tiers',
97+
tiersDropdownSelection: TierDropdownOption.AllTiers,
9798
};
9899

99100
const result = applyMaaSEndpointData(deployment, fieldData);
@@ -118,7 +119,7 @@ describe('maasDeploymentTransformer', () => {
118119
const originalAnnotations = { ...deployment.model.metadata.annotations };
119120
const fieldData: MaaSTierValue = {
120121
isChecked: true,
121-
tiersDropdownSelection: 'all-tiers',
122+
tiersDropdownSelection: TierDropdownOption.AllTiers,
122123
};
123124

124125
applyMaaSEndpointData(deployment, fieldData);

‎packages/maas/frontend/src/odh/modelServingExtensions/modelDeploymentWizard/maasDeploymentTransformer.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { LLMdDeployment } from '@odh-dashboard/llmd-serving/types';
2-
import type { MaaSTierValue } from './MaaSEndpointCheckbox';
2+
import { MaaSTierValue, TierDropdownOption } from './MaaSEndpointCheckbox';
33

44
export const MAAS_TIERS_ANNOTATION = 'alpha.maas.opendatahub.io/tiers';
55

@@ -46,7 +46,7 @@ const convertAnnotationToTierValue = (
4646
if (annotationValue === 'null') {
4747
return {
4848
isChecked: true,
49-
tiersDropdownSelection: 'no-tiers',
49+
tiersDropdownSelection: TierDropdownOption.NoTiers,
5050
selectedTierNames: undefined,
5151
};
5252
}
@@ -57,14 +57,14 @@ const convertAnnotationToTierValue = (
5757
if (parsed.length === 0) {
5858
return {
5959
isChecked: true,
60-
tiersDropdownSelection: 'all-tiers',
60+
tiersDropdownSelection: TierDropdownOption.AllTiers,
6161
selectedTierNames: [],
6262
};
6363
}
6464
// Specific tiers
6565
return {
6666
isChecked: true,
67-
tiersDropdownSelection: 'specify-tiers',
67+
tiersDropdownSelection: TierDropdownOption.SpecifyTiers,
6868
selectedTierNames: parsed,
6969
};
7070
}

‎packages/model-serving/src/components/deploymentWizard/steps/ReviewStep.tsx‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import { UseModelDeploymentWizardState } from '../useDeploymentWizard';
1919
import { ModelLocationType, ModelTypeLabel, WizardReviewSection, WizardStepTitle } from '../types';
2020
import { deploymentStrategyRecreate } from '../fields/DeploymentStrategyField';
2121
import { ExternalDataMap } from '../ExternalDataLoader';
22+
import { isWizardStepTitle } from '../utils';
2223

2324
type ReviewStepContentProps = {
2425
wizardState: UseModelDeploymentWizardState;
@@ -350,12 +351,7 @@ const getStatusSections = (
350351
...getExtensionItems(WizardStepTitle.ADVANCED_SETTINGS, extensionStatusSections),
351352
],
352353
},
353-
...(extensionStatusSections?.filter(
354-
(section) =>
355-
section.title !== WizardStepTitle.ADVANCED_SETTINGS &&
356-
section.title !== WizardStepTitle.MODEL_DETAILS &&
357-
section.title !== WizardStepTitle.MODEL_DEPLOYMENT,
358-
) ?? []),
354+
...(extensionStatusSections?.filter((section) => !isWizardStepTitle(section.title)) ?? []),
359355
];
360356
};
361357

‎packages/model-serving/src/components/deploymentWizard/utils.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
ModelLocationData,
2121
WizardFormData,
2222
type InitialWizardFormData,
23+
WizardStepTitle,
2324
} from './types';
2425
import {
2526
handleConnectionCreation,
@@ -213,3 +214,7 @@ export const resolveConnectionType = (
213214
return undefined;
214215
}
215216
};
217+
218+
export const isWizardStepTitle = (value: string): value is WizardStepTitle => {
219+
return Object.values(WizardStepTitle).some((title) => title === value);
220+
};

0 commit comments

Comments
 (0)