Skip to content

Commit

Permalink
fix: handle query errors in ChooseSession
Browse files Browse the repository at this point in the history 
The error and loading messages are just placeholders for now. The
crucial change here is to let the student know when we can't display
up-to-date session information for some reason.

Note that we show data if it's available, without first checking for
`isLoading` or `isError`. This means we may show stale data, but we prefer
this over showing a loading message or an error for short server
outages. For some justification of this approach, see:

https://tkdodo.eu/blog/status-checks-in-react-query

Note that React Query will not show stale data indefinitely, and will
eventually show an error message if the data is stale for too long.

Signed-off-by: Drew Hess <[email protected]>
  • Loading branch information
@dhess
dhess committed Jul 10, 2023
1 parent a1709ab commit 500c49f
Showing 1 changed file with 85 additions and 61 deletions.
146 changes: 85 additions & 61 deletions src/components/ChooseSession/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,31 +27,6 @@ const ChooseSession = (): JSX.Element => {
queryClient.invalidateQueries(getGetSessionListQueryKey()),
},
});
const { data } = useGetSessionList({
page,
pageSize,
nameLike: sessionNameFilter,
});

const sessions: Session[] = data ? data.items : [];
const meta: PaginatedMeta = data
? data.meta
: { totalItems: 0, pageSize: 1, thisPage: 1, firstPage: 1, lastPage: 1 };
const startIndex: number = (meta.thisPage - 1) * meta.pageSize + 1;

// If we're on the last page of results, and the student deletes the last
// session on that page; or if we somehow request a page beyond the last page
// of results; then the API will return an empty list of sessions, and the
// last page will be less than the current page. When this happens, it means
// we've gone beyond the last page of results, and therefore we want to fetch
// the new last page.
//
// Note that when there are no sessions at all, then the current page and the
// last page will both be 1, and therefore we can be sure that we won't do
// this ad infinitum.
if (sessions.length == 0 && meta.thisPage > meta.lastPage) {
setPage(meta.lastPage);
}

const navigate = useNavigate();
const newSession = useCreateSession({
Expand All @@ -63,45 +38,94 @@ const ChooseSession = (): JSX.Element => {
},
});

const onClickNextPage: MouseEventHandler<unknown> | undefined =
meta.thisPage < meta.lastPage ? () => setPage(page + 1) : undefined;
const onClickPreviousPage: MouseEventHandler<unknown> | undefined =
meta.thisPage > 1 ? () => setPage(page - 1) : undefined;
const { isError, data, error } = useGetSessionList(
{
page,
pageSize,
nameLike: sessionNameFilter,
},
{
query: {
onSuccess: (data) => {
// If we're on the last page of results, and the student deletes the
// last session on that page; or if we somehow request a page beyond
// the last page of results; then the API will return an empty list of
// sessions, and the last page will be less than the current page. When
// this happens, it means we've gone beyond the last page of results,
// and therefore we want to fetch the new last page.
//
// Note that when there are no sessions at all, then the current page
// and the last page will both be 1, and therefore we can be sure that
// we won't do this ad infinitum.
if (
data.items.length == 0 &&
data.meta.thisPage > data.meta.lastPage
) {
setPage(data.meta.lastPage);
}
},
},
}
);

return (
<SessionsPage
account={{ ...exampleAccount, id: cookies.id }}
sessions={sessions}
startIndex={startIndex}
numItems={meta.pageSize}
totalItems={meta.totalItems}
onClickNewProgram={(name: string, importPrelude: boolean) =>
newSession.mutate({ data: { name, importPrelude } })
}
onClickNextPage={onClickNextPage}
onClickPreviousPage={onClickPreviousPage}
onClickDelete={(sessionId) => deleteSession.mutate({ sessionId })}
onSubmitSearch={(nameFilter: string) => {
// Unlike `onChangeSearch`, this callback is always triggered
// by an explicit action, and never by, e.g., a page refresh,
// so we always want to reset the page when this callback is
// invoked.
setSessionNameFilter(nameFilter);
setPage(1);
}}
onChangeSearch={(nameFilter: string) => {
// For technical reasons, this callback may be triggered even
// if the value of the search term didn't actually change
// (e.g., because the page is redrawn), and in these cases, we
// don't want to update the page, so we filter these spurious
// "changes" out.
if (nameFilter != sessionNameFilter) {
// Note that we show data if it's available, without first checking for
// `isLoading` or `isError`. This means we may show stale data, but we prefer
// this over showing a loading message or an error for short server outages. See:
//
// https://tkdodo.eu/blog/status-checks-in-react-query
//
// Note that React Query will not show stale data indefinitely, and will
// eventually show an error message if the data is stale for too long.

if (data) {
const sessions: Session[] = data.items;
const meta: PaginatedMeta = data.meta;
const startIndex: number = (meta.thisPage - 1) * meta.pageSize + 1;

const onClickNextPage: MouseEventHandler<unknown> | undefined =
meta.thisPage < meta.lastPage ? () => setPage(page + 1) : undefined;
const onClickPreviousPage: MouseEventHandler<unknown> | undefined =
meta.thisPage > 1 ? () => setPage(page - 1) : undefined;

return (
<SessionsPage
account={{ ...exampleAccount, id: cookies.id }}
sessions={sessions}
startIndex={startIndex}
numItems={meta.pageSize}
totalItems={meta.totalItems}
onClickNewProgram={(name: string, importPrelude: boolean) =>
newSession.mutate({ data: { name, importPrelude } })
}
onClickNextPage={onClickNextPage}
onClickPreviousPage={onClickPreviousPage}
onClickDelete={(sessionId) => deleteSession.mutate({ sessionId })}
onSubmitSearch={(nameFilter: string) => {
// Unlike `onChangeSearch`, this callback is always triggered
// by an explicit action, and never by, e.g., a page refresh,
// so we always want to reset the page when this callback is
// invoked.
setSessionNameFilter(nameFilter);
setPage(1);
}
}}
/>
);
}}
onChangeSearch={(nameFilter: string) => {
// For technical reasons, this callback may be triggered even
// if the value of the search term didn't actually change
// (e.g., because the page is redrawn), and in these cases, we
// don't want to update the page, so we filter these spurious
// "changes" out.
if (nameFilter != sessionNameFilter) {
setSessionNameFilter(nameFilter);
setPage(1);
}
}}
/>
);
} else if (isError) {
return <div>Error: {error.message}</div>;
} else {
return <div>Loading...</div>;
}
};

export default ChooseSession;

0 comments on commit 500c49f

Please sign in to comment.