Skip to content

Commit

Permalink
Persist workflow search bar when ListWorkflows fails (#724)
Browse files Browse the repository at this point in the history
* Remove useSuspenseQuery

* Fix styling of page section

* Add client boundary to content loader
  • Loading branch information
adhityamamallan authored Nov 13, 2024
1 parent 1490f1e commit 2ab43cd
Show file tree
Hide file tree
Showing 15 changed files with 270 additions and 216 deletions.
7 changes: 4 additions & 3 deletions src/views/domain-page/config/domain-page-tabs-error.config.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import getDomainWorkflowsErrorConfig from '@/views/domain-workflows/helpers/get-domain-workflows-error-config';

import { type DomainPageTabsErrorConfig } from '../domain-page-tabs-error/domain-page-tabs-error.types';

const domainPageTabsErrorConfig: DomainPageTabsErrorConfig = {
workflows: getDomainWorkflowsErrorConfig,
workflows: () => ({
message: 'Failed to load workflows',
actions: [{ kind: 'retry', label: 'Retry' }],
}),
metadata: () => ({
message: 'Failed to load metadata',
actions: [{ kind: 'retry', label: 'Retry' }],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { withStyle } from 'baseui';

import PageSection from '@/components/page-section/page-section';

export const styled = {
PageSection: withStyle(PageSection, () => ({
display: 'flex',
flexDirection: 'column',
flex: 1,
})),
};
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
'use client';
import React from 'react';

import { notFound } from 'next/navigation';
Expand All @@ -6,6 +7,7 @@ import decodeUrlParams from '@/utils/decode-url-params';

import domainPageTabsContentConfig from '../config/domain-page-tabs-content.config';

import { styled } from './domain-page-content.styles';
import {
type DomainPageContentParams,
type Props,
Expand All @@ -22,11 +24,11 @@ export default function DomainPageContent(props: Props) {
}

return (
<section>
<styled.PageSection>
<TabContent
domain={decodedParams.domain}
cluster={decodedParams.cluster}
/>
</section>
</styled.PageSection>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import React from 'react';
import { useSuspenseQuery } from '@tanstack/react-query';

import ListTable from '@/components/list-table/list-table';
import PageSection from '@/components/page-section/page-section';
import request from '@/utils/request';

import domainPageMetadataTableConfig from '../config/domain-page-metadata-table.config';
Expand All @@ -23,13 +22,11 @@ export default function DomainPageMetadata(props: DomainPageTabContentProps) {
});

return (
<PageSection>
<styled.MetadataContainer>
<ListTable
data={domainInfo}
listTableConfig={domainPageMetadataTableConfig}
/>
</styled.MetadataContainer>
</PageSection>
<styled.MetadataContainer>
<ListTable
data={domainInfo}
listTableConfig={domainPageMetadataTableConfig}
/>
</styled.MetadataContainer>
);
}
41 changes: 19 additions & 22 deletions src/views/domain-page/domain-page-settings/domain-page-settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
} from '@tanstack/react-query';
import { toaster, ToasterContainer, PLACEMENT } from 'baseui/toast';

import PageSection from '@/components/page-section/page-section';
import updateDomain from '@/server-actions/update-domain/update-domain';
import request from '@/utils/request';
import SettingsForm from '@/views/shared/settings-form/settings-form';
Expand Down Expand Up @@ -68,27 +67,25 @@ export default function DomainPageSettings(props: DomainPageTabContentProps) {
autoHideDuration={SETTINGS_UPDATE_TOAST_DURATION_MS}
overrides={overrides.toast}
>
<PageSection>
<styled.SettingsContainer>
<SettingsForm
data={domainInfo}
zodSchema={domainPageSettingsFormSchema}
formConfig={domainPageSettingsFormConfig}
onSubmit={async (data) =>
await saveSettings.mutateAsync(data).then(() => {
queryClient.invalidateQueries({
queryKey: ['describeDomain', props],
});
toaster.positive('Successfully updated domain settings');
})
}
submitButtonText="Save settings"
onSubmitError={(e) =>
toaster.negative('Error updating domain settings: ' + e.message)
}
/>
</styled.SettingsContainer>
</PageSection>
<styled.SettingsContainer>
<SettingsForm
data={domainInfo}
zodSchema={domainPageSettingsFormSchema}
formConfig={domainPageSettingsFormConfig}
onSubmit={async (data) =>
await saveSettings.mutateAsync(data).then(() => {
queryClient.invalidateQueries({
queryKey: ['describeDomain', props],
});
toaster.positive('Successfully updated domain settings');
})
}
submitButtonText="Save settings"
onSubmitError={(e) =>
toaster.negative('Error updating domain settings: ' + e.message)
}
/>
</styled.SettingsContainer>
</ToasterContainer>
);
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
'use client';
import PageFilters from '@/components/page-filters/page-filters';
import PageSection from '@/components/page-section/page-section';
import domainPageQueryParamsConfig from '@/views/domain-page/config/domain-page-query-params.config';

import domainWorkflowsFiltersConfig from '../config/domain-workflows-filters.config';
Expand All @@ -9,15 +8,13 @@ import { styled } from './domain-workflows-filters.styles';

export default function DomainWorkflowsFilters() {
return (
<PageSection>
<styled.FiltersContainer>
<PageFilters
searchQueryParamKey="search"
searchPlaceholder="Search for Workflow ID, Run ID, or Workflow Type"
pageFiltersConfig={domainWorkflowsFiltersConfig}
pageQueryParamsConfig={domainPageQueryParamsConfig}
/>
</styled.FiltersContainer>
</PageSection>
<styled.FiltersContainer>
<PageFilters
searchQueryParamKey="search"
searchPlaceholder="Search for Workflow ID, Run ID, or Workflow Type"
pageFiltersConfig={domainWorkflowsFiltersConfig}
pageQueryParamsConfig={domainPageQueryParamsConfig}
/>
</styled.FiltersContainer>
);
}
Original file line number Diff line number Diff line change
@@ -1,14 +1,24 @@
import { Suspense } from 'react';
import { HttpResponse } from 'msw';

import { render, screen, act, fireEvent } from '@/test-utils/rtl';
import { render, screen, userEvent } from '@/test-utils/rtl';

import { type ListWorkflowsResponse } from '@/route-handlers/list-workflows/list-workflows.types';
import * as requestModule from '@/utils/request';

import type { Props as MSWMocksHandlersProps } from '../../../../test-utils/msw-mock-handlers/msw-mock-handlers.types';
import { mockDomainWorkflowsQueryParamsValues } from '../../__fixtures__/domain-workflows-query-params';
import { type Props as EndMessageProps } from '../../domain-workflows-table-end-message/domain-workflows-table-end-message.types';
import DomainWorkflowsTable from '../domain-workflows-table';

jest.mock('@/components/error-panel/error-panel', () =>
jest.fn(({ message }: { message: string }) => <div>{message}</div>)
);

jest.mock('../helpers/get-workflows-error-panel-props', () =>
jest.fn().mockImplementation(({ error }) => ({
message: error ? 'Error loading workflows' : 'No workflows found',
}))
);

jest.mock(
'../../domain-workflows-table-end-message/domain-workflows-table-end-message',
() =>
Expand All @@ -20,19 +30,19 @@ jest.mock(
);

jest.mock('query-string', () => ({
stringifyUrl: jest.fn(() => 'mock-stringified-api-url'),
stringifyUrl: jest.fn(
() => '/api/domains/mock-domain/mock-cluster/workflows'
),
}));

jest.mock('@/utils/request');

const mockSetQueryParams = jest.fn();
jest.mock('@/hooks/use-page-query-params/use-page-query-params', () =>
jest.fn(() => [mockDomainWorkflowsQueryParamsValues, mockSetQueryParams])
);

describe(DomainWorkflowsTable.name, () => {
it('renders workflows without error', async () => {
await setup({});
const { user } = setup({});

expect(await screen.findByText('Mock end message: OK')).toBeInTheDocument();
Array(10).forEach((_, index) => {
Expand All @@ -41,9 +51,7 @@ describe(DomainWorkflowsTable.name, () => {
).toBeInTheDocument();
});

act(() => {
fireEvent.click(screen.getByTestId('mock-end-message'));
});
await user.click(screen.getByTestId('mock-end-message'));

expect(await screen.findByText('Mock end message: OK')).toBeInTheDocument();
Array(10).forEach((_, index) => {
Expand All @@ -53,23 +61,22 @@ describe(DomainWorkflowsTable.name, () => {
});
});

it('does not render if the initial call fails', async () => {
let renderErrorMessage;
try {
await act(async () => {
await setup({ errorCase: 'initial-fetch-error' });
});
} catch (error) {
if (error instanceof Error) {
renderErrorMessage = error.message;
}
}

expect(renderErrorMessage).toEqual('Request failed');
it('renders error panel if the initial call fails', async () => {
setup({ errorCase: 'initial-fetch-error' });

expect(
await screen.findByText('Error loading workflows')
).toBeInTheDocument();
});

it('renders error panel if no workflows are found', async () => {
setup({ errorCase: 'no-workflows' });

expect(await screen.findByText('No workflows found')).toBeInTheDocument();
});

it('renders workflows and allows the user to try again if there is an error', async () => {
await setup({ errorCase: 'subsequent-fetch-error' });
const { user } = setup({ errorCase: 'subsequent-fetch-error' });

expect(await screen.findByText('Mock end message: OK')).toBeInTheDocument();
Array(10).forEach((_, index) => {
Expand All @@ -78,17 +85,13 @@ describe(DomainWorkflowsTable.name, () => {
).toBeInTheDocument();
});

act(() => {
fireEvent.click(screen.getByTestId('mock-end-message'));
});
await user.click(screen.getByTestId('mock-end-message'));

expect(
await screen.findByText('Mock end message: Error')
).toBeInTheDocument();

act(() => {
fireEvent.click(screen.getByTestId('mock-end-message'));
});
await user.click(screen.getByTestId('mock-end-message'));

expect(await screen.findByText('Mock end message: OK')).toBeInTheDocument();
Array(10).forEach((_, index) => {
Expand All @@ -99,41 +102,57 @@ describe(DomainWorkflowsTable.name, () => {
});
});

async function setup({
function setup({
errorCase,
}: {
errorCase?: 'initial-fetch-error' | 'subsequent-fetch-error';
errorCase?: 'initial-fetch-error' | 'subsequent-fetch-error' | 'no-workflows';
}) {
// TODO: @adhitya.mamallan - This is not type-safe, explore using a library such as nock or msw
const requestMock = jest.spyOn(requestModule, 'default') as jest.Mock;
const pages = generateWorkflowPages(2);
let currentEventIndex = 0;
const user = userEvent.setup();

render(<DomainWorkflowsTable domain="mock-domain" cluster="mock-cluster" />, {
endpointsMocks: [
{
path: '/api/domains/:domain/:cluster/workflows',
httpMethod: 'GET',
mockOnce: false,
httpResolver: async () => {
const index = currentEventIndex;
currentEventIndex++;

switch (errorCase) {
case 'no-workflows':
return HttpResponse.json({ workflows: [], nextPage: undefined });
case 'initial-fetch-error':
return HttpResponse.json(
{ message: 'Request failed' },
{ status: 500 }
);
case 'subsequent-fetch-error':
if (index === 0) {
return HttpResponse.json(pages[0]);
} else if (index === 1) {
return HttpResponse.json(
{ message: 'Request failed' },
{ status: 500 }
);
} else {
return HttpResponse.json(pages[1]);
}
default:
if (index === 0) {
return HttpResponse.json(pages[0]);
} else {
return HttpResponse.json(pages[1]);
}
}
},
},
] as MSWMocksHandlersProps['endpointsMocks'],
});

if (errorCase === 'subsequent-fetch-error') {
requestMock
.mockResolvedValueOnce({
json: () => Promise.resolve(pages[0]),
})
.mockRejectedValueOnce(new Error('Request failed'))
.mockResolvedValueOnce({
json: () => Promise.resolve(pages[1]),
});
} else if (errorCase === 'initial-fetch-error') {
requestMock.mockRejectedValueOnce(new Error('Request failed'));
} else {
requestMock
.mockResolvedValueOnce({
json: () => Promise.resolve(pages[0]),
})
.mockResolvedValueOnce({
json: () => Promise.resolve(pages[1]),
});
}

render(
<Suspense>
<DomainWorkflowsTable domain="mock-domain" cluster="mock-cluster" />
</Suspense>
);
return { user };
}

// TODO @adhitya.mamallan - Explore using fakerjs.dev for cases like this
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
export const PAGE_SIZE = 10;

export const NO_WORKFLOWS_ERROR_MESSAGE = 'Domain has no workflows';
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,10 @@ export const styled = {
TableContainer: createStyled('div', {
overflowX: 'auto',
}),
ErrorPanelContainer: createStyled('div', ({ $theme }) => ({
padding: `${$theme.sizing.scale1200} 0px`,
display: 'flex',
alignItems: 'center',
justifyContent: 'center',
})),
};
Loading

0 comments on commit 2ab43cd

Please sign in to comment.