Skip to content

Commit 7d016fb

Browse files
rsun19rsun19
committed
updated status labels, mocks, and unit tests
Signed-off-by: rsun19 <robertssun1234@gmail.com>
1 parent 140221a commit 7d016fb

File tree

16 files changed

+327
-45
lines changed

16 files changed

+327
-45
lines changed

‎clients/ui/bff/internal/api/model_registry_settings_handler.go‎

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,33 @@ func (app *App) GetAllModelRegistriesSettingsHandler(w http.ResponseWriter, r *h
3636
Key: "ssl-secret-key",
3737
}
3838

39+
lastTransitionTime, _ := time.Parse(time.RFC3339, "2024-03-22T09:30:02Z")
40+
deletionTime := lastTransitionTime
3941
registries := []models.ModelRegistryKind{
40-
createSampleModelRegistry("model-registry", namespace, &sslRootCertificateConfigMap, nil),
41-
createSampleModelRegistry("model-registry-dora", namespace, nil, &sslRootCertificateSecret),
42-
createSampleModelRegistry("model-registry-bella", namespace, nil, nil),
42+
createSampleModelRegistry("model-registry", namespace, &sslRootCertificateConfigMap, nil, nil, nil),
43+
createSampleModelRegistry("model-registry-dora", namespace, nil, &sslRootCertificateSecret, nil, nil),
44+
createSampleModelRegistry("model-registry-bella", namespace, nil, nil, nil, nil),
45+
createSampleModelRegistry("model-registry-ready", namespace, nil, nil, nil,
46+
[]models.Condition{
47+
{LastTransitionTime: lastTransitionTime, Message: "Deployment for custom resource model-registry-ready is available", Reason: "Available", Status: "True", Type: "Available"},
48+
}),
49+
createSampleModelRegistry("model-registry-starting", namespace, nil, nil, nil,
50+
[]models.Condition{
51+
{LastTransitionTime: lastTransitionTime, Message: "Deployment for custom resource model-registry-starting was successfully created", Reason: "CreatedDeployment", Status: "True", Type: "Progressing"},
52+
}),
53+
createSampleModelRegistry("model-registry-stopping", namespace, nil, nil, &deletionTime,
54+
[]models.Condition{
55+
{LastTransitionTime: lastTransitionTime, Message: "Deployment for custom resource model-registry-stopping is available", Reason: "Available", Status: "True", Type: "Available"},
56+
}),
57+
createSampleModelRegistry("model-registry-degrading", namespace, nil, nil, nil,
58+
[]models.Condition{
59+
{LastTransitionTime: lastTransitionTime, Message: "Service is degrading", Reason: "Degraded", Status: "True", Type: "Degraded"},
60+
}),
61+
createSampleModelRegistry("model-registry-unavailable", namespace, nil, nil, nil,
62+
[]models.Condition{
63+
{LastTransitionTime: lastTransitionTime, Message: "Service is unavailable", Reason: "Unavailable", Status: "False", Type: "Available"},
64+
{LastTransitionTime: lastTransitionTime, Message: "Istio resources are unavailable", Reason: "IstioUnavailable", Status: "False", Type: "IstioAvailable"},
65+
}),
4366
}
4467

4568
modelRegistryRes := ModelRegistrySettingsListEnvelope{
@@ -62,7 +85,7 @@ func (app *App) GetModelRegistrySettingsHandler(w http.ResponseWriter, r *http.R
6285
}
6386

6487
modelId := ps.ByName(ModelRegistryId)
65-
registry := createSampleModelRegistry(modelId, namespace, nil, nil)
88+
registry := createSampleModelRegistry(modelId, namespace, nil, nil, nil, nil)
6689

6790
modelRegistryWithCreds := models.ModelRegistryAndCredentials{
6891
ModelRegistry: registry,
@@ -102,7 +125,7 @@ func (app *App) CreateModelRegistrySettingsHandler(w http.ResponseWriter, r *htt
102125

103126
ctxLogger.Info("Creating model registry", "name", modelRegistryName)
104127

105-
registry := createSampleModelRegistry(modelRegistryName, namespace, nil, nil)
128+
registry := createSampleModelRegistry(modelRegistryName, namespace, nil, nil, nil, nil)
106129

107130
modelRegistryRes := ModelRegistrySettingsEnvelope{
108131
Data: registry,
@@ -125,7 +148,7 @@ func (app *App) UpdateModelRegistrySettingsHandler(w http.ResponseWriter, r *htt
125148
}
126149

127150
modelId := ps.ByName(ModelRegistryId)
128-
registry := createSampleModelRegistry(modelId, namespace, nil, nil)
151+
registry := createSampleModelRegistry(modelId, namespace, nil, nil, nil, nil)
129152

130153
modelRegistryRes := ModelRegistrySettingsEnvelope{
131154
Data: registry,
@@ -148,7 +171,7 @@ func (app *App) DeleteModelRegistrySettingsHandler(w http.ResponseWriter, r *htt
148171

149172
// This is a temporary fix to handle frontend error (as it is expecting ModelRegistryKind response) until we have a real implementation
150173
modelId := ps.ByName(ModelRegistryId)
151-
registry := createSampleModelRegistry(modelId, namespace, nil, nil)
174+
registry := createSampleModelRegistry(modelId, namespace, nil, nil, nil, nil)
152175

153176
modelRegistryRes := ModelRegistrySettingsEnvelope{
154177
Data: registry,
@@ -161,10 +184,9 @@ func (app *App) DeleteModelRegistrySettingsHandler(w http.ResponseWriter, r *htt
161184
}
162185

163186
// This function is a temporary function to create a sample model registry kind until we have a real implementation
164-
func createSampleModelRegistry(name string, namespace string, SSLRootCertificateConfigMap *models.Entry, SSLRootCertificateSecret *models.Entry) models.ModelRegistryKind {
187+
func createSampleModelRegistry(name string, namespace string, SSLRootCertificateConfigMap *models.Entry, SSLRootCertificateSecret *models.Entry, deletionTimestamp *time.Time, conditions []models.Condition) models.ModelRegistryKind {
165188

166189
creationTime, _ := time.Parse(time.RFC3339, "2024-03-14T08:01:42Z")
167-
lastTransitionTime, _ := time.Parse(time.RFC3339, "2024-03-22T09:30:02Z")
168190

169191
return models.ModelRegistryKind{
170192
APIVersion: "modelregistry.io/v1alpha1",
@@ -173,6 +195,7 @@ func createSampleModelRegistry(name string, namespace string, SSLRootCertificate
173195
Name: name,
174196
Namespace: namespace,
175197
CreationTimestamp: creationTime,
198+
DeletionTimestamp: deletionTimestamp,
176199
Annotations: map[string]string{},
177200
},
178201
Spec: models.ModelRegistrySpec{
@@ -205,15 +228,7 @@ func createSampleModelRegistry(name string, namespace string, SSLRootCertificate
205228
},
206229
},
207230
Status: models.Status{
208-
Conditions: []models.Condition{
209-
{
210-
LastTransitionTime: lastTransitionTime,
211-
Message: "Deployment for custom resource " + name + " was successfully created",
212-
Reason: "CreatedDeployment",
213-
Status: "True",
214-
Type: "Progressing",
215-
},
216-
},
231+
Conditions: conditions,
217232
},
218233
}
219234
}

‎clients/ui/bff/internal/models/model_registry_kind.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ type Metadata struct {
2121
Name string `json:"name"`
2222
Namespace string `json:"namespace"`
2323
CreationTimestamp time.Time `json:"creationTimestamp"`
24+
DeletionTimestamp *time.Time `json:"deletionTimestamp,omitempty"`
2425
Annotations map[string]string `json:"annotations,omitempty"`
2526
}
2627

‎clients/ui/frontend/src/__tests__/cypress/cypress/tests/mocked/modelRegistry/modelTransferJobs.cy.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ describe('Model transfer jobs', () => {
250250
failedRow.find().findByTestId('job-status').should('contain.text', 'Failed');
251251

252252
const cancelledRow = modelTransferJobsPage.getRow('job-cancelled');
253-
cancelledRow.find().findByTestId('job-status').should('contain.text', 'Cancelled');
253+
cancelledRow.find().findByTestId('job-status').should('contain.text', 'Canceled');
254254

255255
completedRow.find().findByTestId('job-status').click();
256256
cy.findByTestId('transfer-job-status-modal').should('be.visible');

‎clients/ui/frontend/src/app/pages/modelCatalogSettings/components/CatalogSourceStatus.tsx‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ const CatalogSourceStatus: React.FC<CatalogSourceStatusProps> = ({ catalogSource
3838
const startingOrUnknownLabel = (
3939
<Label
4040
color="grey"
41+
variant="outline"
4142
icon={<InProgressIcon />}
4243
data-testid={`source-status-${catalogSourcesLoadError ? 'unknown' : 'starting'}-${catalogSourceConfig.id}`}
4344
>
@@ -53,7 +54,11 @@ const CatalogSourceStatus: React.FC<CatalogSourceStatusProps> = ({ catalogSource
5354
switch (matchingSource.status) {
5455
case CatalogSourceStatusEnum.AVAILABLE:
5556
return (
56-
<Label status="success" data-testid={`source-status-connected-${catalogSourceConfig.id}`}>
57+
<Label
58+
status="success"
59+
variant="outline"
60+
data-testid={`source-status-connected-${catalogSourceConfig.id}`}
61+
>
5762
Connected
5863
</Label>
5964
);
@@ -65,7 +70,11 @@ const CatalogSourceStatus: React.FC<CatalogSourceStatusProps> = ({ catalogSource
6570
<>
6671
<Stack hasGutter>
6772
<StackItem>
68-
<Label status="danger" data-testid={`source-status-failed-${catalogSourceConfig.id}`}>
73+
<Label
74+
status="danger"
75+
variant="outline"
76+
data-testid={`source-status-failed-${catalogSourceConfig.id}`}
77+
>
6978
Failed
7079
</Label>
7180
</StackItem>

‎clients/ui/frontend/src/app/pages/modelCatalogSettings/components/CatalogSourceStatusErrorModal.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ const CatalogSourceStatusErrorModal: React.FC<CatalogSourceStatusErrorModalProps
2626
<Flex spaceItems={{ default: 'spaceItemsSm' }} alignItems={{ default: 'alignItemsCenter' }}>
2727
<FlexItem>Source status</FlexItem>
2828
<FlexItem>
29-
<Label color="red" icon={<ExclamationCircleIcon />}>
29+
<Label color="red" variant="outline" icon={<ExclamationCircleIcon />}>
3030
Failed
3131
</Label>
3232
</FlexItem>

‎clients/ui/frontend/src/app/pages/modelCatalogSettings/components/__tests__/CatalogSourceStatus.spec.tsx‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
import React from 'react';
2+
import { render, screen } from '@testing-library/react';
3+
import '@testing-library/jest-dom';
4+
import {
5+
ModelCatalogSettingsContext,
6+
ModelCatalogSettingsContextType,
7+
} from '~/app/context/modelCatalogSettings/ModelCatalogSettingsContext';
8+
import { CatalogSourceConfig, CatalogSourceType } from '~/app/modelCatalogTypes';
9+
import CatalogSourceStatus from '~/app/pages/modelCatalogSettings/components/CatalogSourceStatus';
10+
11+
const mockConfig: CatalogSourceConfig = {
12+
id: 'test-source',
13+
name: 'Test Source',
14+
type: CatalogSourceType.YAML,
15+
enabled: true,
16+
};
17+
18+
const defaultPagination = { size: 0, pageSize: 10, nextPageToken: '' };
19+
20+
const renderWithContext = (
21+
config: CatalogSourceConfig,
22+
contextOverrides: Partial<ModelCatalogSettingsContextType>,
23+
) => {
24+
const defaultContext: ModelCatalogSettingsContextType = {
25+
apiState: {
26+
apiAvailable: false,
27+
api: null as unknown as ModelCatalogSettingsContextType['apiState']['api'],
28+
},
29+
refreshAPIState: jest.fn(),
30+
catalogSourceConfigs: null,
31+
catalogSourceConfigsLoaded: false,
32+
catalogSourceConfigsLoadError: undefined,
33+
refreshCatalogSourceConfigs: jest.fn(),
34+
catalogSources: null,
35+
catalogSourcesLoaded: true,
36+
catalogSourcesLoadError: undefined,
37+
refreshCatalogSources: jest.fn(),
38+
...contextOverrides,
39+
};
40+
41+
return render(
42+
<ModelCatalogSettingsContext.Provider value={defaultContext}>
43+
<CatalogSourceStatus catalogSourceConfig={config} />
44+
</ModelCatalogSettingsContext.Provider>,
45+
);
46+
};
47+
48+
describe('CatalogSourceStatus', () => {
49+
it('renders "Connected" label with outline variant', () => {
50+
renderWithContext(mockConfig, {
51+
catalogSources: {
52+
...defaultPagination,
53+
items: [{ id: 'test-source', name: 'Test', labels: [], status: 'available' }],
54+
},
55+
catalogSourcesLoaded: true,
56+
});
57+
58+
const label = screen.getByTestId('source-status-connected-test-source');
59+
expect(screen.getByText('Connected')).toBeVisible();
60+
expect(label.className).toMatch(/outline/);
61+
expect(label.className).not.toMatch(/filled/);
62+
});
63+
64+
it('renders "Failed" label with outline variant', () => {
65+
renderWithContext(mockConfig, {
66+
catalogSources: {
67+
...defaultPagination,
68+
items: [
69+
{
70+
id: 'test-source',
71+
name: 'Test',
72+
labels: [],
73+
status: 'error',
74+
error: 'Connection refused',
75+
},
76+
],
77+
},
78+
catalogSourcesLoaded: true,
79+
});
80+
81+
const label = screen.getByTestId('source-status-failed-test-source');
82+
expect(screen.getByText('Failed')).toBeVisible();
83+
expect(label.className).toMatch(/outline/);
84+
expect(label.className).not.toMatch(/filled/);
85+
});
86+
87+
it('renders "Starting" label with outline variant when source has no status', () => {
88+
renderWithContext(mockConfig, {
89+
catalogSources: {
90+
...defaultPagination,
91+
items: [{ id: 'test-source', name: 'Test', labels: [] }],
92+
},
93+
catalogSourcesLoaded: true,
94+
});
95+
96+
const label = screen.getByTestId('source-status-starting-test-source');
97+
expect(screen.getByText('Starting')).toBeVisible();
98+
expect(label.className).toMatch(/outline/);
99+
});
100+
101+
it('renders "Unknown" label with outline variant when there is a load error', () => {
102+
renderWithContext(mockConfig, {
103+
catalogSources: null,
104+
catalogSourcesLoaded: true,
105+
catalogSourcesLoadError: new Error('API error'),
106+
});
107+
108+
const label = screen.getByTestId('source-status-unknown-test-source');
109+
expect(screen.getByText('Unknown')).toBeVisible();
110+
expect(label.className).toMatch(/outline/);
111+
});
112+
113+
it('renders "-" for default sources', () => {
114+
renderWithContext({ ...mockConfig, isDefault: true }, { catalogSourcesLoaded: true });
115+
expect(screen.getByText('-')).toBeVisible();
116+
});
117+
118+
it('renders "-" for disabled sources', () => {
119+
renderWithContext({ ...mockConfig, enabled: false }, { catalogSourcesLoaded: true });
120+
expect(screen.getByText('-')).toBeVisible();
121+
});
122+
});

‎clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelTransferJobs/ModelTransferJobStatusModal.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ const ModelTransferJobStatusModal: React.FC<ModelTransferJobStatusModalProps> =
8888
<Flex spaceItems={{ default: 'spaceItemsSm' }} alignItems={{ default: 'alignItemsCenter' }}>
8989
<FlexItem>{getModalTitle(job.uploadIntent)}</FlexItem>
9090
<FlexItem>
91-
<Label color={statusInfo.color} icon={statusInfo.icon}>
91+
<Label color={statusInfo.color} icon={statusInfo.icon} variant="outline">
9292
{statusInfo.label}
9393
</Label>
9494
</FlexItem>

‎clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelTransferJobs/ModelTransferJobTableRow.tsx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,11 @@ export const getStatusLabel = (
3535
case ModelTransferJobStatus.RUNNING:
3636
return { label: 'Running', color: 'blue', icon: <InProgressIcon /> };
3737
case ModelTransferJobStatus.PENDING:
38-
return { label: 'Pending', color: 'grey', icon: <PendingIcon /> };
38+
return { label: 'Pending', color: 'purple', icon: <PendingIcon /> };
3939
case ModelTransferJobStatus.FAILED:
4040
return { label: 'Failed', color: 'red', icon: <ExclamationCircleIcon /> };
4141
case ModelTransferJobStatus.CANCELLED:
42-
return { label: 'Cancelled', color: 'grey', icon: <BanIcon /> };
42+
return { label: 'Canceled', color: 'grey', icon: <BanIcon /> };
4343
default:
4444
return { label: status, color: 'grey', icon: null };
4545
}
@@ -136,6 +136,7 @@ const ModelTransferJobTableRow: React.FC<ModelTransferJobTableRowProps> = ({
136136
color={statusInfo.color}
137137
icon={statusInfo.icon}
138138
data-testid="job-status"
139+
variant="filled"
139140
isClickable
140141
onClick={() => setIsStatusModalOpen(true)}
141142
>

‎clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelTransferJobs/__tests__/ModelTransferJobStatusModal.spec.tsx‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,25 @@ describe('ModelTransferJobStatusModal', () => {
5454
});
5555
});
5656

57+
it('renders the status label in the modal title with outline variant', () => {
58+
const job = mockModelTransferJob({
59+
status: ModelTransferJobStatus.COMPLETED,
60+
namespace: 'kubeflow',
61+
});
62+
63+
mockUseFetchState.mockReturnValue([[], true, undefined, jest.fn()]);
64+
65+
render(<ModelTransferJobStatusModal job={job} isOpen onClose={jest.fn()} />);
66+
67+
const label = screen.getByText('Complete');
68+
expect(label).toBeVisible();
69+
70+
const labelWrapper = label.closest('span.pf-v6-c-label');
71+
expect(labelWrapper).not.toBeNull();
72+
expect(labelWrapper!.className).toMatch(/outline/);
73+
expect(labelWrapper!.className).not.toMatch(/filled/);
74+
});
75+
5776
it('shows unknown failure reason and danger alert when events fail to load', () => {
5877
// Arrange job without an explicit errorMessage so the fallback text is used.
5978
const job = mockModelTransferJob({

‎clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelTransferJobs/__tests__/getStatusLabel.spec.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import { ModelTransferJobStatus } from '~/app/types';
2+
import { getStatusLabel } from '~/app/pages/modelRegistry/screens/ModelTransferJobs/ModelTransferJobTableRow';
3+
4+
describe('getStatusLabel', () => {
5+
it('returns green "Complete" for COMPLETED status', () => {
6+
const result = getStatusLabel(ModelTransferJobStatus.COMPLETED);
7+
expect(result.label).toBe('Complete');
8+
expect(result.color).toBe('green');
9+
});
10+
11+
it('returns blue "Running" for RUNNING status', () => {
12+
const result = getStatusLabel(ModelTransferJobStatus.RUNNING);
13+
expect(result.label).toBe('Running');
14+
expect(result.color).toBe('blue');
15+
});
16+
17+
it('returns purple "Pending" for PENDING status', () => {
18+
const result = getStatusLabel(ModelTransferJobStatus.PENDING);
19+
expect(result.label).toBe('Pending');
20+
expect(result.color).toBe('purple');
21+
});
22+
23+
it('returns red "Failed" for FAILED status', () => {
24+
const result = getStatusLabel(ModelTransferJobStatus.FAILED);
25+
expect(result.label).toBe('Failed');
26+
expect(result.color).toBe('red');
27+
});
28+
29+
it('returns grey "Canceled" for CANCELLED status', () => {
30+
const result = getStatusLabel(ModelTransferJobStatus.CANCELLED);
31+
expect(result.label).toBe('Canceled');
32+
expect(result.color).toBe('grey');
33+
});
34+
});

0 commit comments

Comments
 (0)