feat: filter carry over behavior (#1128)

#527 #1081 #1118 - Refactors the filter history state to query parameter strings - Implements remaining carry over behavior for browse back / forward buttons - Adds E2E tests (some incomplete until #1093 is merged) ## Demo https://dev-filter-carry.cryoet.dev.si.czi.technology/
chanzuckerberg · Sep 16, 2024 · afd3497 · afd3497
1 parent e76f270
commit afd3497
Show file tree

Hide file tree

Showing 24 changed files with 339 additions and 130 deletions.
diff --git a/frontend/packages/data-portal/app/components/Breadcrumbs.tsx b/frontend/packages/data-portal/app/components/Breadcrumbs.tsx
@@ -1,8 +1,8 @@
-import { isString } from 'lodash-es'
 import { useMemo } from 'react'
 
 import { SmallChevronRightIcon } from 'app/components/icons'
 import { Link } from 'app/components/Link'
+import { TestIds } from 'app/constants/testIds'
 import { useI18n } from 'app/hooks/useI18n'
 import {
   useBrowseDatasetFilterHistory,
@@ -11,14 +11,6 @@ import {
 } from 'app/state/filterHistory'
 import { cns } from 'app/utils/cns'
 
-function encodeParams(params: [string, string | null][]): string {
-  const searchParams = new URLSearchParams(
-    params.filter((kv) => isString(kv[1])) as string[][],
-  )
-
-  return searchParams.toString()
-}
-
 function Breadcrumb({
   text,
   link,
@@ -46,8 +38,8 @@ export function Breadcrumbs({
 }) {
   const { t } = useI18n()
 
-  const { browseDatasetHistory } = useBrowseDatasetFilterHistory()
-  const { singleDatasetHistory } = useSingleDatasetFilterHistory()
+  const { previousBrowseDatasetParams } = useBrowseDatasetFilterHistory()
+  const { previousSingleDatasetParams } = useSingleDatasetFilterHistory()
   const { previousDepositionId, previousSingleDepositionParams } =
     useDepositionHistory()
 
@@ -56,24 +48,21 @@ export function Breadcrumbs({
       variant === 'deposition'
         ? '/browse-data/depositions'
         : '/browse-data/datasets'
-    const history = variant === 'deposition' ? undefined : browseDatasetHistory
-    const encodedParams = encodeParams(Array.from(history?.entries() ?? []))
+    const params =
+      variant === 'deposition' ? undefined : previousBrowseDatasetParams
 
-    return `${url}?${encodedParams}`
-  }, [browseDatasetHistory, variant])
+    return `${url}?${params}`
+  }, [previousBrowseDatasetParams, variant])
 
   const singleDatasetLink = useMemo(() => {
     if (variant === 'dataset') {
       return undefined
     }
 
     const url = `/datasets/${data.id}`
-    const encodedParams = encodeParams(
-      Array.from(singleDatasetHistory?.entries() ?? []),
-    )
 
-    return `${url}?${encodedParams}`
-  }, [singleDatasetHistory, variant, data])
+    return `${url}?${previousSingleDatasetParams}`
+  }, [variant, data.id, previousSingleDatasetParams])
 
   const returnToDepositionLink =
     previousDepositionId === null || variant === 'deposition'
@@ -85,7 +74,10 @@ export function Breadcrumbs({
   )
 
   return (
-    <div className="flex flex-col flex-auto gap-1">
+    <div
+      className="flex flex-col flex-auto gap-1"
+      data-testid={TestIds.Breadcrumbs}
+    >
       {returnToDepositionLink && (
         <Link
           className="uppercase font-semibold text-sds-caps-xxxs leading-sds-caps-xxxs text-sds-primary-400"

diff --git a/frontend/packages/data-portal/app/components/BrowseData/BrowseDataSearch.tsx b/frontend/packages/data-portal/app/components/BrowseData/BrowseDataSearch.tsx
@@ -3,6 +3,7 @@ import { useDebouncedEffect } from '@react-hookz/web'
 import { useSearchParams } from '@remix-run/react'
 import { useRef, useState } from 'react'
 
+import { QueryParams } from 'app/constants/query'
 import { i18n } from 'app/i18n'
 
 /**
@@ -14,7 +15,7 @@ const SEARCH_QUERY_DEBOUNCE_TIME_MS = 500
 
 export function BrowseDataSearch() {
   const [searchParams, setSearchParams] = useSearchParams()
-  const [query, setQuery] = useState(searchParams.get('search') ?? '')
+  const [query, setQuery] = useState(searchParams.get(QueryParams.Search) ?? '')
 
   // If the user hasn't typed in a key for 500ms, then update the search params.
   const initialLoadRef = useRef(true)

diff --git a/frontend/packages/data-portal/app/components/BrowseData/DatasetTable.tsx b/frontend/packages/data-portal/app/components/BrowseData/DatasetTable.tsx
@@ -5,7 +5,7 @@ import Skeleton from '@mui/material/Skeleton'
 import { useNavigate, useSearchParams } from '@remix-run/react'
 import { ColumnDef, createColumnHelper } from '@tanstack/react-table'
 import { range } from 'lodash-es'
-import { useCallback, useEffect, useMemo, useState } from 'react'
+import { useCallback, useMemo, useState } from 'react'
 
 import { AnnotatedObjectsList } from 'app/components/AnnotatedObjectsList'
 import { AuthorList } from 'app/components/AuthorList'
@@ -16,14 +16,11 @@ import { CellHeader, PageTable, TableCell } from 'app/components/Table'
 import { EMPIAR_ID, EMPIAR_URL } from 'app/constants/external-dbs'
 import { DATASET_FILTERS } from 'app/constants/filterQueryParams'
 import { ANNOTATED_OBJECTS_MAX, MAX_PER_PAGE } from 'app/constants/pagination'
+import { QueryParams } from 'app/constants/query'
 import { DatasetTableWidths } from 'app/constants/table'
 import { Dataset, useDatasets } from 'app/hooks/useDatasets'
 import { useI18n } from 'app/hooks/useI18n'
 import { useIsLoading } from 'app/hooks/useIsLoading'
-import {
-  BrowseDatasetHistory,
-  useBrowseDatasetFilterHistory,
-} from 'app/state/filterHistory'
 import { LogLevel } from 'app/types/logging'
 import { cnsNoMerge } from 'app/utils/cns'
 import { sendLogs } from 'app/utils/logging'
@@ -46,8 +43,7 @@ export function DatasetTable() {
   const { datasets } = useDatasets()
 
   const [searchParams, setSearchParams] = useSearchParams()
-  const { setBrowseDatasetHistory } = useBrowseDatasetFilterHistory()
-  const datasetSort = (searchParams.get('sort') ?? undefined) as
+  const datasetSort = (searchParams.get(QueryParams.Sort) ?? undefined) as
     | CellHeaderDirection
     | undefined
 
@@ -57,18 +53,6 @@ export function DatasetTable() {
   const [isClickingOnEmpiarId, setIsClickingOnEmpiarId] = useState(false)
   const navigate = useNavigate()
 
-  useEffect(
-    () =>
-      setBrowseDatasetHistory(
-        new Map(
-          Array.from(searchParams).filter(([k]) =>
-            (DATASET_FILTERS as unknown as string[]).includes(k),
-          ),
-        ) as BrowseDatasetHistory,
-      ),
-    [searchParams, setBrowseDatasetHistory],
-  )
-
   const getDatasetUrl = useCallback(
     (id: number) => {
       const url = createUrl(`/datasets/${id}`)

diff --git a/frontend/packages/data-portal/app/components/BrowseData/DepositionTable.tsx b/frontend/packages/data-portal/app/components/BrowseData/DepositionTable.tsx
@@ -15,6 +15,7 @@ import { CellHeader, PageTable, TableCell } from 'app/components/Table'
 import { IdPrefix } from 'app/constants/idPrefixes'
 import { shapeTypeToI18nKey } from 'app/constants/objectShapeTypes'
 import { ANNOTATED_OBJECTS_MAX, MAX_PER_PAGE } from 'app/constants/pagination'
+import { QueryParams } from 'app/constants/query'
 import { DepositionTableWidths } from 'app/constants/table'
 import { Deposition, useDepositions } from 'app/hooks/useDepositions'
 import { useI18n } from 'app/hooks/useI18n'
@@ -43,7 +44,7 @@ export function DepositionTable() {
   const { depositions } = useDepositions()
 
   const [searchParams, setSearchParams] = useSearchParams()
-  const depositionSort = (searchParams.get('sort') ?? undefined) as
+  const depositionSort = (searchParams.get(QueryParams.Sort) ?? undefined) as
     | CellHeaderDirection
     | undefined
 

diff --git a/frontend/packages/data-portal/app/components/Dataset/RunsTable.tsx b/frontend/packages/data-portal/app/components/Dataset/RunsTable.tsx
@@ -4,7 +4,7 @@ import Skeleton from '@mui/material/Skeleton'
 import { useNavigate, useSearchParams } from '@remix-run/react'
 import { ColumnDef, createColumnHelper } from '@tanstack/react-table'
 import { range } from 'lodash-es'
-import { useCallback, useEffect, useMemo, useState } from 'react'
+import { useCallback, useMemo, useState } from 'react'
 
 import { GetDatasetByIdQuery } from 'app/__generated__/graphql'
 import { AnnotatedObjectsList } from 'app/components/AnnotatedObjectsList'
@@ -22,10 +22,6 @@ import { TiltSeriesScore } from 'app/constants/tiltSeries'
 import { useDatasetById } from 'app/hooks/useDatasetById'
 import { useI18n } from 'app/hooks/useI18n'
 import { useIsLoading } from 'app/hooks/useIsLoading'
-import {
-  SingleDatasetHistory,
-  useSingleDatasetFilterHistory,
-} from 'app/state/filterHistory'
 import { cnsNoMerge } from 'app/utils/cns'
 import { inQualityScoreRange } from 'app/utils/tiltSeries'
 import { carryOverFilterParams, createUrl } from 'app/utils/url'
@@ -44,25 +40,12 @@ export function RunsTable() {
   const { dataset, deposition } = useDatasetById()
   const runs = dataset.runs as unknown as Run[]
   const { t } = useI18n()
-  const { setSingleDatasetHistory } = useSingleDatasetFilterHistory()
   const [searchParams] = useSearchParams()
 
   const [isHoveringOverInteractable, setIsHoveringOverInteractable] =
     useState(false)
   const navigate = useNavigate()
 
-  useEffect(
-    () =>
-      setSingleDatasetHistory(
-        new Map(
-          Array.from(searchParams).filter(([k]) =>
-            (RUN_FILTERS as unknown as string[]).includes(k),
-          ),
-        ) as SingleDatasetHistory,
-      ),
-    [searchParams, setSingleDatasetHistory],
-  )
-
   const getRunUrl = useCallback(
     (id: number) => {
       const url = createUrl(`/runs/${id}`)

diff --git a/frontend/packages/data-portal/app/components/Deposition/DatasetsTable.tsx b/frontend/packages/data-portal/app/components/Deposition/DatasetsTable.tsx
@@ -42,7 +42,7 @@ export function DatasetsTable() {
   const { deposition, datasets } = useDepositionById()
 
   const [searchParams, setSearchParams] = useSearchParams()
-  const datasetSort = (searchParams.get('sort') ?? undefined) as
+  const datasetSort = (searchParams.get(QueryParams.Sort) ?? undefined) as
     | CellHeaderDirection
     | undefined
 

diff --git a/frontend/packages/data-portal/app/components/DepositionFilterBanner.tsx b/frontend/packages/data-portal/app/components/DepositionFilterBanner.tsx
@@ -21,7 +21,7 @@ export function DepositionFilterBanner({
   deposition: Deposition
   labelI18n: I18nKeys
 }) {
-  const { singleDatasetHistory, setSingleDatasetHistory } =
+  const { previousSingleDatasetParams, setPreviousSingleDatasetParams } =
     useSingleDatasetFilterHistory()
   const { previousSingleDepositionParams } = useDepositionHistory()
   const [, setDepositionId] = useQueryParam<string>(QueryParams.DepositionId)
@@ -45,9 +45,10 @@ export function DepositionFilterBanner({
           onClick={() => {
             setDepositionId(null)
 
-            const nextHistory = new Map(singleDatasetHistory)
-            nextHistory.delete(QueryParams.DepositionId)
-            setSingleDatasetHistory(nextHistory)
+            const nextParams = new URLSearchParams(previousSingleDatasetParams)
+            nextParams.delete(QueryParams.DepositionId)
+            nextParams.sort()
+            setPreviousSingleDatasetParams(nextParams.toString())
           }}
           sdsStyle="minimal"
           sdsType="secondary"

diff --git a/frontend/packages/data-portal/app/constants/query.ts b/frontend/packages/data-portal/app/constants/query.ts
@@ -20,14 +20,16 @@ export enum QueryParams {
   MetadataDrawer = 'metadata',
   MethodType = 'method-type',
   NumberOfRuns = 'runs',
-  ObjectName = 'object',
   ObjectId = 'object-id',
+  ObjectName = 'object',
   ObjectShapeType = 'object_shape',
   Organism = 'organism',
   Page = 'page',
   QualityScore = 'quality_score',
   ReconstructionMethod = 'reconstruction_method',
   ReconstructionSoftware = 'reconstruction_software',
+  Search = 'search',
+  Sort = 'sort',
   Tab = 'tab',
   TableTab = 'table-tab',
   TiltRangeMax = 'tilt_max',

diff --git a/frontend/packages/data-portal/app/constants/testIds.ts b/frontend/packages/data-portal/app/constants/testIds.ts
@@ -1,6 +1,7 @@
 export enum TestIds {
   AnnotationId = 'annotation-id',
   AnnotationTableDivider = 'annotation-table-divider',
+  Breadcrumbs = 'breadcrumbs',
   EnvelopeIcon = 'envelope-icon',
   MetadataDrawer = 'metadata-drawer',
   MetadataDrawerCloseButton = 'metadata-drawer-close-button',

diff --git a/frontend/packages/data-portal/app/hooks/useFilter.ts b/frontend/packages/data-portal/app/hooks/useFilter.ts
@@ -126,46 +126,57 @@ export function useFilter() {
       ...getFilterState(searchParams),
 
       reset() {
-        setSearchParams((prev) => {
-          Object.values(QueryParams).forEach((param) => prev.delete(param))
-          prev.delete('page')
-
-          return prev
-        })
+        setSearchParams(
+          (prev) => {
+            Object.values(QueryParams).forEach((param) => prev.delete(param))
+            prev.delete(QueryParams.Page)
+
+            return prev
+          },
+          { replace: true },
+        )
       },
 
       updateValue(param: QueryParams, value?: FilterValue) {
         logPlausibleEvent(param, value)
 
-        setSearchParams((prev) => {
-          prev.delete(param)
-          prev.delete('page')
+        setSearchParams(
+          (prev) => {
+            prev.delete(param)
+            prev.delete(QueryParams.Page)
 
-          if (value) {
-            normalizeFilterValue(value).forEach((v) => prev.append(param, v))
-          }
+            if (value) {
+              normalizeFilterValue(value).forEach((v) => prev.append(param, v))
+            }
 
-          return prev
-        })
+            return prev
+          },
+          { replace: true },
+        )
       },
 
       updateValues(params: Partial<Record<QueryParams, FilterValue>>) {
         const entries = Object.entries(params) as [QueryParams, FilterValue][]
         entries.forEach(([param, value]) => logPlausibleEvent(param, value))
 
-        setSearchParams((prev) => {
-          prev.delete('page')
+        setSearchParams(
+          (prev) => {
+            prev.delete(QueryParams.Page)
 
-          entries.forEach(([param, value]) => {
-            prev.delete(param)
+            entries.forEach(([param, value]) => {
+              prev.delete(param)
 
-            if (value) {
-              normalizeFilterValue(value).forEach((v) => prev.append(param, v))
-            }
-          })
+              if (value) {
+                normalizeFilterValue(value).forEach((v) =>
+                  prev.append(param, v),
+                )
+              }
+            })
 
-          return prev
-        })
+            return prev
+          },
+          { replace: true },
+        )
       },
     }),
 

diff --git a/frontend/packages/data-portal/app/routes/browse-data.datasets.tsx b/frontend/packages/data-portal/app/routes/browse-data.datasets.tsx
@@ -7,19 +7,25 @@ import { DatasetTable } from 'app/components/BrowseData'
 import { DatasetFilter } from 'app/components/DatasetFilter'
 import { NoResults } from 'app/components/NoResults'
 import { TablePageLayout } from 'app/components/TablePageLayout'
+import { DATASET_FILTERS } from 'app/constants/filterQueryParams'
+import { QueryParams } from 'app/constants/query'
 import { getBrowseDatasets } from 'app/graphql/getBrowseDatasets.server'
 import { getDatasetsFilterData } from 'app/graphql/getDatasetsFilterData.server'
 import { useDatasets } from 'app/hooks/useDatasets'
 import { useFilter } from 'app/hooks/useFilter'
 import { useI18n } from 'app/hooks/useI18n'
+import {
+  useBrowseDatasetFilterHistory,
+  useSyncParamsWithState,
+} from 'app/state/filterHistory'
 
 export async function loader({ request }: LoaderFunctionArgs) {
   const url = new URL(request.url)
-  const page = +(url.searchParams.get('page') ?? '1')
-  const sort = (url.searchParams.get('sort') ?? undefined) as
+  const page = +(url.searchParams.get(QueryParams.Page) ?? '1')
+  const sort = (url.searchParams.get(QueryParams.Sort) ?? undefined) as
     | CellHeaderDirection
     | undefined
-  const query = url.searchParams.get('search') ?? ''
+  const query = url.searchParams.get(QueryParams.Search) ?? ''
 
   let orderBy: Order_By | null = null
 
@@ -49,6 +55,13 @@ export default function BrowseDatasetsPage() {
   const { reset } = useFilter()
   const { t } = useI18n()
 
+  const { setPreviousBrowseDatasetParams } = useBrowseDatasetFilterHistory()
+
+  useSyncParamsWithState({
+    filters: DATASET_FILTERS,
+    setParams: setPreviousBrowseDatasetParams,
+  })
+
   return (
     <TablePageLayout
       tabs={[