Address review comments: Fixed async pagination race risk, refactored PaginationControls to be generic, Moved error checks before the initial skeleton fallback

Author: ekraffmiller

src/sections/dataset/dataset-versions/DatasetVersions.tsx

@@ -89,7 +89,7 @@ export function DatasetVersions({
 
       void fetchSummaries(paginationInfoToFetch).then((totalCount) => {
         if (typeof totalCount === 'number') {
-          setPaginationInfo(paginationInfoToDisplay.withTotal(totalCount))
+          setPaginationInfo((currentPaginationInfo) => currentPaginationInfo.withTotal(totalCount))
         }
       })
     }
@@ -99,14 +99,14 @@ export function DatasetVersions({
     setSelectedVersions([])
   }, [paginationInfo.page, paginationInfo.pageSize])
 
-  if (!datasetVersionSummaries) {
-    return <DatasetVersionsLoadingSkeleton />
-  }
-
   if (error) {
     return <Alert variant="danger">Error loading dataset versions</Alert>
   }
 
+  if (!datasetVersionSummaries) {
+    return <DatasetVersionsLoadingSkeleton />
+  }
+
   return (
     <>
       {selectedVersions.length === 2 && (
@@ -217,15 +217,7 @@ export function DatasetVersions({
       </div>
       <PaginationControls
         initialPaginationInfo={paginationInfo}
-        onPaginationInfoChange={(newPaginationInfo) =>
-          setPaginationInfo(
-            new DatasetVersionPaginationInfo(
-              newPaginationInfo.page,
-              newPaginationInfo.pageSize,
-              newPaginationInfo.totalItems
-            )
-          )
-        }
+        onPaginationInfoChange={setPaginationInfo}
       />
     </>
   )

src/sections/dataset/dataset-versions/useGetDatasetVersionsSummaries.ts

@@ -1,4 +1,4 @@
-import { useCallback, useEffect, useState } from 'react'
+import { useCallback, useEffect, useRef, useState } from 'react'
 import { DatasetVersionSummaryInfo } from '@/dataset/domain/models/DatasetVersionSummaryInfo'
 import { DatasetRepository } from '@/dataset/domain/repositories/DatasetRepository'
 import { getDatasetVersionsSummaries } from '@/dataset/domain/useCases/getDatasetVersionsSummaries'
@@ -25,9 +25,12 @@ export const useGetDatasetVersionsSummaries = ({
   const [summaries, setSummaries] = useState<DatasetVersionSummaryInfo[]>()
   const [isLoading, setIsLoading] = useState<boolean>(autoFetch)
   const [error, setError] = useState<string | null>(null)
+  const latestRequestId = useRef(0)
 
   const fetchSummaries = useCallback(
     async (paginationInfo?: DatasetVersionPaginationInfo) => {
+      const requestId = latestRequestId.current + 1
+      latestRequestId.current = requestId
       setIsLoading(true)
       setError(null)
 
@@ -37,17 +40,25 @@ export const useGetDatasetVersionsSummaries = ({
           persistentId,
           paginationInfo
         )
+        if (requestId !== latestRequestId.current) {
+          return undefined
+        }
         setSummaries(versionSummaries.summaries)
         return versionSummaries.totalCount
       } catch (err) {
+        if (requestId !== latestRequestId.current) {
+          return undefined
+        }
         const errorMessage =
           err instanceof Error && err.message
             ? err.message
             : 'Something went wrong getting the information from the dataset versions summaries. Try again later.'
         setError(errorMessage)
         return undefined
       } finally {
-        setIsLoading(false)
+        if (requestId === latestRequestId.current) {
+          setIsLoading(false)
+        }
       }
     },
     [datasetRepository, persistentId]

src/sections/file/file-version/FileVersions.tsx

@@ -53,20 +53,20 @@ export function FileVersions({
 
       void fetchSummaries(paginationInfoToFetch).then((totalCount) => {
         if (typeof totalCount === 'number') {
-          setPaginationInfo(paginationInfoToFetch.withTotal(totalCount))
+          setPaginationInfo((currentPaginationInfo) => currentPaginationInfo.withTotal(totalCount))
         }
       })
     }
   }, [isInView, fetchSummaries, paginationInfo.page, paginationInfo.pageSize])
 
-  if (!fileVersionSummaries) {
-    return <FileVersionsLoadingSkeleton />
-  }
-
   if (error) {
     return <Alert variant="danger">{t('fileVersion.error')}</Alert>
   }
 
+  if (!fileVersionSummaries) {
+    return <FileVersionsLoadingSkeleton />
+  }
+
   const datasetVersionDisplayMap: Record<string, string> = {
     [DatasetNonNumericVersion.DRAFT]: DatasetNonNumericVersionSearchParam.DRAFT
   }
@@ -144,15 +144,7 @@ export function FileVersions({
       </div>
       <PaginationControls
         initialPaginationInfo={paginationInfo}
-        onPaginationInfoChange={(newPaginationInfo) =>
-          setPaginationInfo(
-            new FileVersionPaginationInfo(
-              newPaginationInfo.page,
-              newPaginationInfo.pageSize,
-              newPaginationInfo.totalItems
-            )
-          )
-        }
+        onPaginationInfoChange={setPaginationInfo}
       />
     </>
   )

src/sections/file/file-version/useGetFileVersionsSummaries.ts

@@ -1,4 +1,4 @@
-import { useCallback, useEffect, useState } from 'react'
+import { useCallback, useEffect, useRef, useState } from 'react'
 import { FileVersionSummaryInfo } from '@/files/domain/models/FileVersionSummaryInfo'
 import { FileRepository } from '@/files/domain/repositories/FileRepository'
 import { getFileVersionSummaries } from '@/files/domain/useCases/getFileVersionSummaries'
@@ -25,9 +25,12 @@ export const useGetFileVersionsSummaries = ({
   const [summaries, setSummaries] = useState<FileVersionSummaryInfo[]>()
   const [isLoading, setIsLoading] = useState<boolean>(autoFetch)
   const [error, setError] = useState<string | null>(null)
+  const latestRequestId = useRef(0)
 
   const fetchSummaries = useCallback(
     async (paginationInfo?: FileVersionPaginationInfo) => {
+      const requestId = latestRequestId.current + 1
+      latestRequestId.current = requestId
       setIsLoading(true)
       setError(null)
       try {
@@ -36,17 +39,25 @@ export const useGetFileVersionsSummaries = ({
           fileId,
           paginationInfo
         )
+        if (requestId !== latestRequestId.current) {
+          return undefined
+        }
         setSummaries(versionSummaries.summaries)
         return versionSummaries.totalCount
       } catch (err) {
+        if (requestId !== latestRequestId.current) {
+          return undefined
+        }
         const errorMessage =
           err instanceof Error && err.message
             ? err.message
             : 'Something went wrong getting the information from the file versions summaries. Try again later.'
         setError(errorMessage)
         return undefined
       } finally {
-        setIsLoading(false)
+        if (requestId === latestRequestId.current) {
+          setIsLoading(false)
+        }
       }
     },
     [fileRepository, fileId]

src/sections/shared/pagination/PaginationControls.tsx

@@ -4,32 +4,19 @@ import { PageSizeSelector } from './PageSizeSelector'
 import styles from './Pagination.module.scss'
 import { PaginationInfo } from '../../../shared/pagination/domain/models/PaginationInfo'
 import { useEffect, useState } from 'react'
-import { FilePaginationInfo } from '../../../files/domain/models/FilePaginationInfo'
-import { DatasetPaginationInfo } from '../../../dataset/domain/models/DatasetPaginationInfo'
-import { NotificationsPaginationInfo } from '@/notifications/domain/models/NotificationsPaginationInfo'
-import { DatasetVersionPaginationInfo } from '@/dataset/domain/models/DatasetVersionPaginationInfo'
-import { FileVersionPaginationInfo } from '@/files/domain/models/FileVersionPaginationInfo'
 
-type SupportedPaginationInfo =
-  | DatasetPaginationInfo
-  | FilePaginationInfo
-  | NotificationsPaginationInfo
-  | DatasetVersionPaginationInfo
-  | FileVersionPaginationInfo
-
-interface PaginationProps {
-  onPaginationInfoChange: (paginationInfo: PaginationInfo<SupportedPaginationInfo>) => void
-  initialPaginationInfo: PaginationInfo<SupportedPaginationInfo>
+interface PaginationProps<T extends PaginationInfo<T>> {
+  onPaginationInfoChange: (paginationInfo: T) => void
+  initialPaginationInfo: T
   showPageSizeSelector?: boolean
 }
 const MINIMUM_NUMBER_OF_PAGES_TO_DISPLAY_PAGINATION = 2
-export function PaginationControls({
+export function PaginationControls<T extends PaginationInfo<T>>({
   onPaginationInfoChange,
   initialPaginationInfo,
   showPageSizeSelector = true
-}: PaginationProps) {
-  const [paginationInfo, setPaginationInfo] =
-    useState<SupportedPaginationInfo>(initialPaginationInfo)
+}: PaginationProps<T>) {
+  const [paginationInfo, setPaginationInfo] = useState<T>(initialPaginationInfo)
   const goToPage = (newPage: number) => {
     setPaginationInfo(paginationInfo.goToPage(newPage))
   }