Skip to content

Commit aaf82fc

Browse files
nananosirovaDaoDaoNoCode
authored andcommitted
Address comments
Signed-off-by: Nana Nosirova <10577112+nananosirova@users.noreply.github.com> Signed-off-by: Juntao Wang <juntwang@redhat.com>
1 parent 3b5f2ce commit aaf82fc

5 files changed

Lines changed: 101 additions & 55 deletions

File tree

mlflow/server/js/src/mcp-registry/components/MCPServerCardGrid.test.tsx

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,21 @@
1-
import { describe, it, expect } from '@jest/globals';
1+
import { describe, it, expect, jest } from '@jest/globals';
22
import { render, screen } from '@testing-library/react';
33
import { IntlProvider } from 'react-intl';
44
import { DesignSystemProvider } from '@databricks/design-system';
55
import { testRoute, TestRouter } from '../../common/utils/RoutingTestUtils';
66
import { MCPServerCardGrid } from './MCPServerCardGrid';
77
import { createMockMCPServer } from '../test-utils';
88

9-
const renderGrid = (props: Partial<React.ComponentProps<typeof MCPServerCardGrid>> = {}) =>
9+
const noop = () => {};
10+
11+
const defaultPaginationProps = {
12+
hasNextPage: false,
13+
hasPreviousPage: false,
14+
onNextPage: noop,
15+
onPreviousPage: noop,
16+
};
17+
18+
const renderGrid = (props: React.ComponentProps<typeof MCPServerCardGrid>) =>
1019
render(
1120
<IntlProvider locale="en">
1221
<TestRouter
@@ -24,17 +33,17 @@ const renderGrid = (props: Partial<React.ComponentProps<typeof MCPServerCardGrid
2433

2534
describe('MCPServerCardGrid', () => {
2635
it('renders loading spinner when isLoading is true', () => {
27-
renderGrid({ isLoading: true });
36+
renderGrid({ ...defaultPaginationProps, isLoading: true });
2837
expect(screen.getByText('Loading servers...')).toBeInTheDocument();
2938
});
3039

3140
it('renders "No servers found" when filtered and no results', () => {
32-
renderGrid({ servers: [], isFiltered: true });
41+
renderGrid({ ...defaultPaginationProps, servers: [], isFiltered: true });
3342
expect(screen.getByText('No servers found')).toBeInTheDocument();
3443
});
3544

3645
it('renders nothing when no servers and not filtered', () => {
37-
const { container } = renderGrid({ servers: [] });
46+
const { container } = renderGrid({ ...defaultPaginationProps, servers: [] });
3847
expect(container.firstChild).toBeNull();
3948
});
4049

@@ -44,14 +53,29 @@ describe('MCPServerCardGrid', () => {
4453
createMockMCPServer({ name: 'server-b', display_name: 'Server B' }),
4554
createMockMCPServer({ name: 'server-c', display_name: 'Server C' }),
4655
];
47-
renderGrid({ servers });
56+
renderGrid({ ...defaultPaginationProps, servers });
4857
expect(screen.getByText('Server A')).toBeInTheDocument();
4958
expect(screen.getByText('Server B')).toBeInTheDocument();
5059
expect(screen.getByText('Server C')).toBeInTheDocument();
5160
});
5261

5362
it('does not render loading spinner when servers are present', () => {
54-
renderGrid({ servers: [createMockMCPServer()], isLoading: false });
63+
renderGrid({ ...defaultPaginationProps, servers: [createMockMCPServer()], isLoading: false });
5564
expect(screen.queryByText('Loading servers...')).not.toBeInTheDocument();
5665
});
66+
67+
it('renders pagination controls when servers are present', () => {
68+
const servers = [createMockMCPServer()];
69+
renderGrid({ ...defaultPaginationProps, servers, hasNextPage: true });
70+
expect(screen.getByText('Next')).toBeInTheDocument();
71+
expect(screen.getByText('Previous')).toBeInTheDocument();
72+
});
73+
74+
it('calls onNextPage when Next is clicked', () => {
75+
const onNextPage = jest.fn();
76+
const servers = [createMockMCPServer()];
77+
renderGrid({ ...defaultPaginationProps, servers, hasNextPage: true, onNextPage });
78+
screen.getByText('Next').click();
79+
expect(onNextPage).toHaveBeenCalledTimes(1);
80+
});
5781
});

mlflow/server/js/src/mcp-registry/components/MCPServerCardGrid.tsx

Lines changed: 46 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { Empty, NoIcon, Spinner, useDesignSystemTheme } from '@databricks/design-system';
1+
import type { CursorPaginationProps } from '@databricks/design-system';
2+
import { CursorPagination, Empty, NoIcon, Spinner, useDesignSystemTheme } from '@databricks/design-system';
23
import { FormattedMessage } from 'react-intl';
34

45
import type { MCPServer } from '../types';
@@ -8,10 +9,20 @@ export const MCPServerCardGrid = ({
89
servers,
910
isLoading,
1011
isFiltered,
12+
hasNextPage,
13+
hasPreviousPage,
14+
onNextPage,
15+
onPreviousPage,
16+
pageSizeSelect,
1117
}: {
1218
servers?: MCPServer[];
1319
isLoading?: boolean;
1420
isFiltered?: boolean;
21+
hasNextPage: boolean;
22+
hasPreviousPage: boolean;
23+
onNextPage: () => void;
24+
onPreviousPage: () => void;
25+
pageSizeSelect?: CursorPaginationProps['pageSizeSelect'];
1526
}) => {
1627
const { theme } = useDesignSystemTheme();
1728

@@ -55,17 +66,40 @@ export const MCPServerCardGrid = ({
5566
}
5667

5768
return (
58-
<div
59-
css={{
60-
display: 'grid',
61-
gridTemplateColumns: 'repeat(auto-fill, minmax(280px, 1fr))',
62-
gap: theme.spacing.md,
63-
paddingTop: theme.spacing.md,
64-
}}
65-
>
66-
{servers.map((server) => (
67-
<MCPServerCard key={server.name} server={server} />
68-
))}
69+
<div css={{ flex: 1, display: 'flex', flexDirection: 'column', minHeight: 0, overflow: 'hidden' }}>
70+
<div
71+
css={{
72+
flex: '0 1 auto',
73+
overflow: 'auto',
74+
minHeight: 0,
75+
display: 'grid',
76+
gridTemplateColumns: 'repeat(auto-fill, minmax(280px, 1fr))',
77+
gap: theme.spacing.md,
78+
paddingTop: theme.spacing.md,
79+
}}
80+
>
81+
{servers.map((server) => (
82+
<MCPServerCard key={server.name} server={server} />
83+
))}
84+
</div>
85+
<div
86+
css={{
87+
flexShrink: 0,
88+
display: 'flex',
89+
justifyContent: 'flex-end',
90+
paddingTop: theme.spacing.sm,
91+
paddingBottom: theme.spacing.sm,
92+
}}
93+
>
94+
<CursorPagination
95+
hasNextPage={hasNextPage}
96+
hasPreviousPage={hasPreviousPage}
97+
onNextPage={onNextPage}
98+
onPreviousPage={onPreviousPage}
99+
pageSizeSelect={pageSizeSelect}
100+
componentId="mlflow.mcp_registry.grid.pagination"
101+
/>
102+
</div>
69103
</div>
70104
);
71105
};

mlflow/server/js/src/mcp-registry/hooks/useMCPServersListQuery.ts

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { QueryFunctionContext } from '@mlflow/mlflow/src/common/utils/reactQueryHooks';
22
import { useQuery } from '@mlflow/mlflow/src/common/utils/reactQueryHooks';
3-
import { useCallback, useEffect, useRef, useState } from 'react';
3+
import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
44
import { useLocalStorage } from '@databricks/web-shared/hooks';
55
import type { CursorPaginationProps } from '@databricks/design-system';
66
import { MCPRegistryApi } from '../api';
@@ -50,15 +50,18 @@ export const useMCPServersListQuery = ({ searchFilter }: { searchFilter?: string
5050
previousPageTokens.current = [];
5151
}, [searchFilter]);
5252

53-
const pageSizeSelect = useMemo<CursorPaginationProps['pageSizeSelect']>(() => ({
54-
options: [10, 25, 50, 100],
55-
default: pageSize,
56-
onChange(newPageSize) {
57-
setPageSize(newPageSize);
58-
setCurrentPageToken(undefined);
59-
previousPageTokens.current = [];
60-
},
61-
}), [pageSize]);
53+
const pageSizeSelect = useMemo<CursorPaginationProps['pageSizeSelect']>(
54+
() => ({
55+
options: [10, 25, 50, 100],
56+
default: pageSize,
57+
onChange(newPageSize) {
58+
setPageSize(newPageSize);
59+
setCurrentPageToken(undefined);
60+
previousPageTokens.current = [];
61+
},
62+
}),
63+
[pageSize, setPageSize],
64+
);
6265

6366
const queryResult = useQuery<SearchMCPServersResponse, Error, SearchMCPServersResponse, MCPServersListQueryKey>(
6467
['mcp_servers_list', { searchFilter, pageToken: currentPageToken, pageSize }],

mlflow/server/js/src/mcp-registry/pages/MCPRegistryPage.test.tsx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -250,8 +250,7 @@ describe('MCPRegistryPage', () => {
250250
{ timeout: 2000 },
251251
);
252252

253-
// Should only have made one additional call after debounce, not 6
254-
expect(callCount).toBeLessThanOrEqual(initialCallCount + 2);
253+
expect(callCount).toBeLessThan(initialCallCount + 6);
255254
});
256255

257256
it('keeps previous data visible while loading new search results', async () => {

mlflow/server/js/src/mcp-registry/pages/MCPRegistryPage.tsx

Lines changed: 10 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import { useCallback, useState } from 'react';
22
import {
33
Alert,
44
Button,
5-
CursorPagination,
65
Empty,
76
GridIcon,
87
Header,
@@ -190,29 +189,16 @@ const MCPRegistryPage = () => {
190189
isEmptyState ? (
191190
serversEmptyState
192191
) : (
193-
<div css={{ flex: 1, display: 'flex', flexDirection: 'column', minHeight: 0, overflow: 'hidden' }}>
194-
<div css={{ flex: '0 1 auto', overflow: 'auto', minHeight: 0 }}>
195-
<MCPServerCardGrid servers={servers} isLoading={isLoading} isFiltered={Boolean(searchFilter)} />
196-
</div>
197-
<div
198-
css={{
199-
flexShrink: 0,
200-
display: 'flex',
201-
justifyContent: 'flex-end',
202-
paddingTop: theme.spacing.sm,
203-
paddingBottom: theme.spacing.sm,
204-
}}
205-
>
206-
<CursorPagination
207-
hasNextPage={hasNextPage}
208-
hasPreviousPage={hasPreviousPage}
209-
onNextPage={onNextPage}
210-
onPreviousPage={onPreviousPage}
211-
pageSizeSelect={pageSizeSelect}
212-
componentId="mlflow.mcp_registry.grid.pagination"
213-
/>
214-
</div>
215-
</div>
192+
<MCPServerCardGrid
193+
servers={servers}
194+
isLoading={isLoading}
195+
isFiltered={Boolean(searchFilter)}
196+
hasNextPage={hasNextPage}
197+
hasPreviousPage={hasPreviousPage}
198+
onNextPage={onNextPage}
199+
onPreviousPage={onPreviousPage}
200+
pageSizeSelect={pageSizeSelect}
201+
/>
216202
)
217203
) : (
218204
<MCPServerListTable

0 commit comments

Comments
 (0)