From 500c49f5719b4c3115835e289c1f0e5f485333a6 Mon Sep 17 00:00:00 2001
From: Drew Hess <src@drewhess.com>
Date: Wed, 5 Jul 2023 13:44:56 +0200
Subject: [PATCH] fix: handle query errors in `ChooseSession`

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 <src@drewhess.com>
---
 src/components/ChooseSession/index.tsx | 146 ++++++++++++++-----------
 1 file changed, 85 insertions(+), 61 deletions(-)

diff --git a/src/components/ChooseSession/index.tsx b/src/components/ChooseSession/index.tsx
index ed40189d..b07c5db3 100644
--- a/src/components/ChooseSession/index.tsx
+++ b/src/components/ChooseSession/index.tsx
@@ -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({
@@ -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;