From 61565da01b4e1a72ad28f931af02f66ef774f4b5 Mon Sep 17 00:00:00 2001 From: Charles Thao Date: Fri, 19 Sep 2025 11:40:20 -0400 Subject: [PATCH] feat: Render Loading if Namespaces are fetching Signed-off-by: Charles Thao --- .../cypress/tests/mocked/workspace.mock.ts | 2 +- .../workspaces/WorkspaceDetailsActivity.cy.ts | 48 ++++++++++--------- .../workspaces/filterWorkspacesTest.cy.ts | 21 ++++---- .../app/context/NamespaceContextProvider.tsx | 4 +- .../hooks/__tests__/useWorkspaces.spec.tsx | 11 +++++ .../frontend/src/app/hooks/useWorkspaces.ts | 8 +++- .../src/app/pages/Workspaces/Workspaces.tsx | 4 +- 7 files changed, 60 insertions(+), 38 deletions(-) diff --git a/workspaces/frontend/src/__tests__/cypress/cypress/tests/mocked/workspace.mock.ts b/workspaces/frontend/src/__tests__/cypress/cypress/tests/mocked/workspace.mock.ts index a3e2196c..a5e33aec 100644 --- a/workspaces/frontend/src/__tests__/cypress/cypress/tests/mocked/workspace.mock.ts +++ b/workspaces/frontend/src/__tests__/cypress/cypress/tests/mocked/workspace.mock.ts @@ -23,7 +23,7 @@ const generateMockWorkspace = ( deferUpdates: paused, paused, pausedTime, - pendingRestart: Math.random() < 0.5, //to generate randomly True/False value + pendingRestart: true, // Make it deterministic for testing state, stateMessage: state === WorkspacesWorkspaceState.WorkspaceStateRunning diff --git a/workspaces/frontend/src/__tests__/cypress/cypress/tests/mocked/workspaces/WorkspaceDetailsActivity.cy.ts b/workspaces/frontend/src/__tests__/cypress/cypress/tests/mocked/workspaces/WorkspaceDetailsActivity.cy.ts index 528ca287..f1a22546 100644 --- a/workspaces/frontend/src/__tests__/cypress/cypress/tests/mocked/workspaces/WorkspaceDetailsActivity.cy.ts +++ b/workspaces/frontend/src/__tests__/cypress/cypress/tests/mocked/workspaces/WorkspaceDetailsActivity.cy.ts @@ -1,12 +1,25 @@ import { mockBFFResponse } from '~/__mocks__/utils'; +import { mockNamespaces } from '~/__mocks__/mockNamespaces'; import { mockWorkspaces } from '~/__tests__/cypress/cypress/tests/mocked/workspace.mock'; +import { navBar } from '~/__tests__/cypress/cypress/pages/navBar'; describe('WorkspaceDetailsActivity Component', () => { beforeEach(() => { - cy.intercept('GET', 'api/v1/workspaces', { + cy.intercept('GET', '/api/v1/namespaces', { + body: mockBFFResponse(mockNamespaces), + }).as('getNamespaces'); + cy.intercept('GET', '/api/v1/workspaces', { body: mockBFFResponse(mockWorkspaces), }).as('getWorkspaces'); + cy.intercept('GET', '/api/v1/workspaces/default', { + body: mockBFFResponse(mockWorkspaces), + }).as('getDefaultWorkspaces'); cy.visit('/'); + cy.wait('@getNamespaces'); + // Select a namespace to enable workspace loading + navBar.selectNamespace('default'); + // Wait for workspaces to load after namespace selection + cy.wait('@getDefaultWorkspaces'); }); // This tests depends on the mocked workspaces data at home page, needs revisit once workspace data fetched from BE @@ -17,26 +30,17 @@ describe('WorkspaceDetailsActivity Component', () => { .find('button') .should('be.visible') .click(); - // Extract first workspace from mock data - cy.wait('@getWorkspaces').then((interception) => { - if (!interception.response || !interception.response.body) { - throw new Error('Intercepted response is undefined or empty'); - } - const workspace = interception.response.body.data[0]; - cy.findByTestId('action-viewDetails').click(); - cy.findByTestId('activityTab').click(); - cy.findByTestId('lastActivity') - .invoke('text') - .then((text) => { - console.log('Rendered lastActivity:', text); - }); - cy.findByTestId('pauseTime').should('have.text', 'Jan 1, 2025, 12:00:00 AM'); - cy.findByTestId('lastActivity').should('have.text', 'Jan 2, 2025, 12:00:00 AM'); - cy.findByTestId('lastUpdate').should('have.text', 'Jan 3, 2025, 12:00:00 AM'); - cy.findByTestId('pendingRestart').should( - 'have.text', - workspace.pendingRestart ? 'Yes' : 'No', - ); - }); + cy.findByTestId('action-viewDetails').click(); + cy.findByTestId('activityTab').click(); + cy.findByTestId('lastActivity') + .invoke('text') + .then((text) => { + console.log('Rendered lastActivity:', text); + }); + cy.findByTestId('pauseTime').should('have.text', 'Jan 1, 2025, 12:00:00 AM'); + cy.findByTestId('lastActivity').should('have.text', 'Jan 2, 2025, 12:00:00 AM'); + cy.findByTestId('lastUpdate').should('have.text', 'Jan 3, 2025, 12:00:00 AM'); + // Use mock data directly since we can't access intercepted response here + cy.findByTestId('pendingRestart').should('have.text', 'Yes'); }); }); diff --git a/workspaces/frontend/src/__tests__/cypress/cypress/tests/mocked/workspaces/filterWorkspacesTest.cy.ts b/workspaces/frontend/src/__tests__/cypress/cypress/tests/mocked/workspaces/filterWorkspacesTest.cy.ts index bab66536..ded6c227 100644 --- a/workspaces/frontend/src/__tests__/cypress/cypress/tests/mocked/workspaces/filterWorkspacesTest.cy.ts +++ b/workspaces/frontend/src/__tests__/cypress/cypress/tests/mocked/workspaces/filterWorkspacesTest.cy.ts @@ -2,6 +2,7 @@ import { mockNamespaces } from '~/__mocks__/mockNamespaces'; import { mockWorkspaces } from '~/__mocks__/mockWorkspaces'; import { mockBFFResponse } from '~/__mocks__/utils'; import { home } from '~/__tests__/cypress/cypress/pages/home'; +import { navBar } from '~/__tests__/cypress/cypress/pages/navBar'; import { mockWorkspaceKinds } from '~/shared/mock/mockNotebookServiceData'; const useFilter = (filterKey: string, filterName: string, searchValue: string) => { @@ -16,25 +17,28 @@ describe('Application', () => { beforeEach(() => { cy.intercept('GET', '/api/v1/namespaces', { body: mockBFFResponse(mockNamespaces), - }); + }).as('getNamespaces'); cy.intercept('GET', '/api/v1/workspaces', { body: mockBFFResponse(mockWorkspaces), }).as('getWorkspaces'); + cy.intercept('GET', '/api/v1/workspaces/default', { + body: mockBFFResponse(mockWorkspaces), + }).as('getDefaultWorkspaces'); cy.intercept('GET', '/api/v1/workspaces/custom-namespace', { body: mockBFFResponse(mockWorkspaces), }); cy.intercept('GET', '/api/v1/workspacekinds', { body: mockBFFResponse(mockWorkspaceKinds), }); - cy.intercept('GET', '/api/namespaces/test-namespace/workspaces').as('getWorkspaces'); + home.visit(); + cy.wait('@getNamespaces'); + // Select a namespace to enable workspace loading + navBar.selectNamespace('default'); + // Wait for workspaces to load after namespace selection + cy.wait('@getDefaultWorkspaces'); }); it('filter rows with single filter', () => { - home.visit(); - - // Wait for the API call before trying to interact with the UI - cy.wait('@getWorkspaces'); - useFilter('name', 'Name', 'My'); cy.get("[id$='workspaces-table-content']").find('tr').should('have.length', 2); cy.get("[id$='workspaces-table-row-1']").contains('My First Jupyter Notebook'); @@ -42,7 +46,6 @@ describe('Application', () => { }); it('filter rows with multiple filters', () => { - home.visit(); // First filter by name useFilter('name', 'Name', 'My'); cy.get("[id$='workspaces-table-content']").find('tr').should('have.length', 2); @@ -57,7 +60,6 @@ describe('Application', () => { }); it('filter rows with multiple filters and remove one', () => { - home.visit(); // Add name filter useFilter('name', 'Name', 'My'); cy.get("[id$='workspaces-table-content']").find('tr').should('have.length', 2); @@ -79,7 +81,6 @@ describe('Application', () => { }); it('filter rows with multiple filters and remove all', () => { - home.visit(); // Add name filter useFilter('name', 'Name', 'My'); cy.get("[id$='workspaces-table-content']").find('tr').should('have.length', 2); diff --git a/workspaces/frontend/src/app/context/NamespaceContextProvider.tsx b/workspaces/frontend/src/app/context/NamespaceContextProvider.tsx index 3b86104a..d489533a 100644 --- a/workspaces/frontend/src/app/context/NamespaceContextProvider.tsx +++ b/workspaces/frontend/src/app/context/NamespaceContextProvider.tsx @@ -5,6 +5,7 @@ const storageKey = 'kubeflow.notebooks.namespace.lastUsed'; interface NamespaceContextType { selectedNamespace: string; + namespacesLoaded: boolean; } const NamespaceContext = React.createContext(undefined); @@ -91,8 +92,9 @@ export const NamespaceContextProvider: React.FC = const namespacesContextValues = useMemo( () => ({ selectedNamespace, + namespacesLoaded, }), - [selectedNamespace], + [selectedNamespace, namespacesLoaded], ); return ( diff --git a/workspaces/frontend/src/app/hooks/__tests__/useWorkspaces.spec.tsx b/workspaces/frontend/src/app/hooks/__tests__/useWorkspaces.spec.tsx index cd125d43..ca00d264 100644 --- a/workspaces/frontend/src/app/hooks/__tests__/useWorkspaces.spec.tsx +++ b/workspaces/frontend/src/app/hooks/__tests__/useWorkspaces.spec.tsx @@ -17,6 +17,17 @@ jest.mock('~/app/hooks/useNotebookAPI', () => ({ useNotebookAPI: jest.fn(), })); +// Mock the namespace context for this test file only +const mockNamespaceContext = { + selectedNamespace: 'test-namespace', + namespacesLoaded: true, +}; + +jest.mock('~/app/context/NamespaceContextProvider', () => ({ + useNamespaceContext: () => mockNamespaceContext, + NamespaceContextProvider: ({ children }: { children: React.ReactNode }) => children, +})); + const mockUseNotebookAPI = useNotebookAPI as jest.MockedFunction; describe('useWorkspaces', () => { diff --git a/workspaces/frontend/src/app/hooks/useWorkspaces.ts b/workspaces/frontend/src/app/hooks/useWorkspaces.ts index 0228a6a6..4e31804f 100644 --- a/workspaces/frontend/src/app/hooks/useWorkspaces.ts +++ b/workspaces/frontend/src/app/hooks/useWorkspaces.ts @@ -2,11 +2,13 @@ import { FetchState, FetchStateCallbackPromise, useFetchState } from 'mod-arch-c import { useCallback } from 'react'; import { useNotebookAPI } from '~/app/hooks/useNotebookAPI'; import { ApiWorkspaceListEnvelope } from '~/generated/data-contracts'; +import { useNamespaceContext } from '~/app/context/NamespaceContextProvider'; export const useWorkspacesByNamespace = ( namespace: string, ): FetchState => { const { api, apiAvailable } = useNotebookAPI(); + const { namespacesLoaded, selectedNamespace } = useNamespaceContext(); const call = useCallback< FetchStateCallbackPromise @@ -14,10 +16,12 @@ export const useWorkspacesByNamespace = ( if (!apiAvailable) { return Promise.reject(new Error('API not yet available')); } - + if (!namespacesLoaded || selectedNamespace === '') { + return Promise.reject(new Error('Namespaces not yet available')); + } const envelope = await api.workspaces.listWorkspacesByNamespace(namespace); return envelope.data; - }, [api, apiAvailable, namespace]); + }, [api.workspaces, apiAvailable, namespace, namespacesLoaded, selectedNamespace]); return useFetchState(call, []); }; diff --git a/workspaces/frontend/src/app/pages/Workspaces/Workspaces.tsx b/workspaces/frontend/src/app/pages/Workspaces/Workspaces.tsx index 5e16debb..f1b75278 100644 --- a/workspaces/frontend/src/app/pages/Workspaces/Workspaces.tsx +++ b/workspaces/frontend/src/app/pages/Workspaces/Workspaces.tsx @@ -13,7 +13,7 @@ import { WorkspacesWorkspaceState } from '~/generated/data-contracts'; import { POLL_INTERVAL } from '~/shared/utilities/const'; export const Workspaces: React.FunctionComponent = () => { - const { selectedNamespace } = useNamespaceContext(); + const { namespacesLoaded, selectedNamespace } = useNamespaceContext(); const [workspaces, workspacesLoaded, workspacesLoadError, refreshWorkspaces] = useWorkspacesByNamespace(selectedNamespace); @@ -46,7 +46,7 @@ export const Workspaces: React.FunctionComponent = () => { return ; } - if (!workspacesLoaded) { + if (!workspacesLoaded || !namespacesLoaded || selectedNamespace === '') { return ; }