Skip to content

Commit

Permalink
Fix no model show up when search a model (#238)
Browse files Browse the repository at this point in the history
* fix: reset to page 1 after name or state search

Signed-off-by: Lin Wang <wonglam@amazon.com>

* refactor: remove unnecessary act call

Signed-off-by: Lin Wang <wonglam@amazon.com>

---------

Signed-off-by: Lin Wang <wonglam@amazon.com>
(cherry picked from commit 24aa9df)
  • Loading branch information
wanglam authored and github-actions[bot] committed Aug 2, 2023
1 parent 5fc14a6 commit b31080b
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 27 deletions.
91 changes: 64 additions & 27 deletions public/components/monitoring/tests/use_monitoring.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const mockEmptyRecords = () =>

describe('useMonitoring', () => {
beforeEach(() => {
jest.spyOn(Model.prototype, 'search').mockResolvedValueOnce({
jest.spyOn(Model.prototype, 'search').mockResolvedValue({
data: [
{
id: 'model-1-id',
Expand All @@ -44,9 +44,7 @@ describe('useMonitoring', () => {

await waitFor(() => result.current.pageStatus === 'normal');

act(() => {
result.current.searchByNameOrId('foo');
});
result.current.searchByNameOrId('foo');
await waitFor(() =>
expect(Model.prototype.search).toHaveBeenCalledWith(
expect.objectContaining({
Expand All @@ -56,9 +54,7 @@ describe('useMonitoring', () => {
)
);

act(() => {
result.current.searchByStatus(['responding']);
});
result.current.searchByStatus(['responding']);
await waitFor(() =>
expect(Model.prototype.search).toHaveBeenCalledWith(
expect.objectContaining({
Expand All @@ -68,13 +64,10 @@ describe('useMonitoring', () => {
)
);

act(() => {
result.current.resetSearch();
});
result.current.resetSearch();
await waitFor(() => result.current.pageStatus === 'normal');
act(() => {
result.current.searchByStatus(['partial-responding']);
});

result.current.searchByStatus(['partial-responding']);
await waitFor(() =>
expect(Model.prototype.search).toHaveBeenCalledWith(
expect.objectContaining({
Expand All @@ -97,11 +90,9 @@ describe('useMonitoring', () => {
)
);

act(() => {
result.current.handleTableChange({
sort: { field: 'name', direction: 'desc' },
pagination: { currentPage: 2, pageSize: 10 },
});
result.current.handleTableChange({
sort: { field: 'name', direction: 'desc' },
pagination: { currentPage: 2, pageSize: 10 },
});
await waitFor(() =>
expect(Model.prototype.search).toHaveBeenCalledWith(
Expand All @@ -119,9 +110,7 @@ describe('useMonitoring', () => {

await waitFor(() => expect(Model.prototype.search).toHaveBeenCalledTimes(1));

act(() => {
result.current.reload();
});
result.current.reload();
await waitFor(() => expect(Model.prototype.search).toHaveBeenCalledTimes(2));
});

Expand Down Expand Up @@ -172,6 +161,58 @@ describe('useMonitoring', () => {

searchMock.mockRestore();
});

it('should call searchByNameOrId with from 0 after page changed', async () => {
const { result, waitFor } = renderHook(() => useMonitoring());

result.current.handleTableChange({
pagination: {
currentPage: 2,
pageSize: 15,
},
});

await waitFor(() => {
expect(result.current.pagination?.currentPage).toBe(2);
});

result.current.searchByNameOrId('foo');

await waitFor(() => {
expect(Model.prototype.search).toHaveBeenCalledTimes(3);
expect(Model.prototype.search).toHaveBeenLastCalledWith(
expect.objectContaining({
from: 0,
})
);
});
});

it('should call searchByStatus with from 0 after page changed', async () => {
const { result, waitFor } = renderHook(() => useMonitoring());

result.current.handleTableChange({
pagination: {
currentPage: 2,
pageSize: 15,
},
});

await waitFor(() => {
expect(result.current.pagination?.currentPage).toBe(2);
});

result.current.searchByStatus(['responding']);

await waitFor(() => {
expect(Model.prototype.search).toHaveBeenCalledTimes(3);
expect(Model.prototype.search).toHaveBeenLastCalledWith(
expect.objectContaining({
from: 0,
})
);
});
});
});

describe('useMonitoring.pageStatus', () => {
Expand Down Expand Up @@ -217,9 +258,7 @@ describe('useMonitoring.pageStatus', () => {

// Mock search function to return empty result
mockEmptyRecords();
act(() => {
result.current.searchByNameOrId('foo');
});
result.current.searchByNameOrId('foo');
await waitFor(() => expect(result.current.pageStatus).toBe('reset-filter'));
});

Expand All @@ -231,9 +270,7 @@ describe('useMonitoring.pageStatus', () => {

// assume result is empty
mockEmptyRecords();
act(() => {
result.current.searchByStatus(['responding']);
});
result.current.searchByStatus(['responding']);
await waitFor(() => expect(result.current.pageStatus).toBe('reset-filter'));
});

Expand Down
2 changes: 2 additions & 0 deletions public/components/monitoring/use_monitoring.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,13 +122,15 @@ export const useMonitoring = () => {
setParams((previousValue) => ({
...previousValue,
nameOrId,
currentPage: 1,
}));
}, []);

const searchByStatus = useCallback((status: ModelDeployStatus[]) => {
setParams((previousValue) => ({
...previousValue,
status,
currentPage: 1,
}));
}, []);

Expand Down

0 comments on commit b31080b

Please sign in to comment.