fix(ui): relationship filter renders stale values when changing fields (#11080)

Fixes #9873. The relationship filter in the "where" builder renders
stale values when switching between fields or adding additional "and"
conditions. This was because the `RelationshipFilter` component was not
responding to changes in the `relationTo` prop and failing to reset
internal state when these events took place.

While it sounds like a simple fix, it was actually quite extensive. The
`RelationshipFilter` component was previously relying on a `useEffect`
that had a callback in its dependencies. This was causing the effect to
run uncontrollably using old references. To avoid this, we use the new
`useEffectEvent` approach which allows the underlying effect to run much
more precisely. Same with the `Condition` component that wraps it. We
now run callbacks directly within event handlers as much as possible,
and rely on `useEffectEvent` _only_ for debounced value changes.

This component was also unnecessarily complex...and still is to some
degree. Previously, it was maintaining two separate refs, one to track
the relationships that have yet to fully load, and another to track the
next pages of each relationship that need to load on the next run. These
have been combined into a single ref that tracks both simultaneously, as
this data is interrelated.

This change also does some much needed housekeeping to the
`WhereBuilder` by improving types, defaulting the operator field, etc.

Related: #11023 and #11032

Unrelated: finds a few more instances where the new `addListFilter`
helper from #11026 could be used. Also removes a few duplicative tests.

Author: jacobsfletch

packages/ui/src/elements/ListControls/index.tsx

@@ -213,7 +213,6 @@ export const ListControls: React.FC<ListControlsProps> = (props) => {
           collectionPluralLabel={collectionConfig?.labels?.plural}
           collectionSlug={collectionConfig.slug}
           fields={collectionConfig?.fields}
-          key={String(hasWhereParam.current && !query?.where)}
           renderedFilters={renderedFilters}
         />
       </AnimateHeight>

packages/ui/src/elements/ViewDescription/index.scss

@@ -5,7 +5,6 @@ import { getTranslation } from '@payloadcms/translations'
 import React from 'react'
 
 import { useTranslation } from '../../providers/Translation/index.js'
-import './index.scss'
 
 export type ViewDescriptionComponent = React.ComponentType<any>
 
@@ -24,7 +23,7 @@ export const ViewDescription: React.FC<ViewDescriptionProps> = (props) => {
   const { description } = props
 
   if (description) {
-    return <div className="view-description">{getTranslation(description, i18n)}</div>
+    return <div className="custom-view-description">{getTranslation(description, i18n)}</div>
   }
 
   return null

packages/ui/src/elements/ViewDescription/index.tsx

@@ -2,7 +2,7 @@ import type { Operator, Option, SelectFieldClient, TextFieldClient } from 'paylo
 
 import React from 'react'
 
-import type { FieldCondition } from '../../types.js'
+import type { ReducedField } from '../../types.js'
 
 import { DateFilter } from '../Date/index.js'
 import { NumberFilter } from '../Number/index.js'
@@ -13,7 +13,7 @@ import { Text } from '../Text/index.js'
 type Props = {
   booleanSelect: boolean
   disabled: boolean
-  internalField: FieldCondition
+  internalField: ReducedField
   onChange: React.Dispatch<React.SetStateAction<string>>
   operator: Operator
   options: Option[]
@@ -34,6 +34,7 @@ export const DefaultFilter: React.FC<Props> = ({
       <Select
         disabled={disabled}
         field={internalField.field as SelectFieldClient}
+        isClearable={!booleanSelect}
         onChange={onChange}
         operator={operator}
         options={options}

packages/ui/src/elements/WhereBuilder/Condition/DefaultFilter/index.tsx

@@ -8,11 +8,12 @@ import type { Option } from '../../../ReactSelect/types.js'
 import type { Props, ValueWithRelation } from './types.js'
 
 import { useDebounce } from '../../../../hooks/useDebounce.js'
+import { useEffectEvent } from '../../../../hooks/useEffectEvent.js'
 import { useConfig } from '../../../../providers/Config/index.js'
 import { useTranslation } from '../../../../providers/Translation/index.js'
 import { ReactSelect } from '../../../ReactSelect/index.js'
-import './index.scss'
 import optionsReducer from './optionsReducer.js'
+import './index.scss'
 
 const baseClass = 'condition-value-relationship'
 
@@ -37,22 +38,32 @@ export const RelationshipFilter: React.FC<Props> = (props) => {
   const hasMultipleRelations = Array.isArray(relationTo)
   const [options, dispatchOptions] = useReducer(optionsReducer, [])
   const [search, setSearch] = useState('')
+  const debouncedSearch = useDebounce(search, 300)
   const [errorLoading, setErrorLoading] = useState('')
   const [hasLoadedFirstOptions, setHasLoadedFirstOptions] = useState(false)
-  const debouncedSearch = useDebounce(search, 300)
   const { i18n, t } = useTranslation()
-  const relationSlugs = hasMultipleRelations ? relationTo : [relationTo]
 
-  const initialRelationMap = () => {
-    const map: Map<string, number> = new Map()
-    relationSlugs.forEach((relation) => {
-      map.set(relation, 1)
-    })
-    return map
-  }
+  const relationSlugs = hasMultipleRelations ? relationTo : [relationTo]
 
-  const nextPageByRelationshipRef = React.useRef<Map<string, number>>(initialRelationMap())
-  const partiallyLoadedRelationshipSlugs = React.useRef<string[]>(relationSlugs)
+  const loadedRelationships = React.useRef<
+    Map<
+      string,
+      {
+        hasLoadedAll: boolean
+        nextPage: number
+      }
+    >
+  >(
+    new Map(
+      relationSlugs.map((relation) => [
+        relation,
+        {
+          hasLoadedAll: false,
+          nextPage: 1,
+        },
+      ]),
+    ),
+  )
 
   const addOptions = useCallback(
     (data, relation) => {
@@ -62,28 +73,31 @@ export const RelationshipFilter: React.FC<Props> = (props) => {
     [hasMultipleRelations, i18n, getEntityConfig],
   )
 
-  const loadRelationOptions = React.useCallback(
+  const loadOptions = useEffectEvent(
     async ({
       abortController,
       relationSlug,
     }: {
       abortController: AbortController
       relationSlug: string
     }) => {
-      if (relationSlug && partiallyLoadedRelationshipSlugs.current.includes(relationSlug)) {
+      const loadedRelationship = loadedRelationships.current.get(relationSlug)
+
+      if (relationSlug && !loadedRelationship.hasLoadedAll) {
         const collection = getEntityConfig({
           collectionSlug: relationSlug,
         })
+
         const fieldToSearch = collection?.admin?.useAsTitle || 'id'
-        const pageIndex = nextPageByRelationshipRef.current.get(relationSlug)
 
         const where: Where = {
           and: [],
         }
+
         const query = {
           depth: 0,
           limit: maxResultsPerRequest,
-          page: pageIndex,
+          page: loadedRelationship.nextPage,
           select: {
             [fieldToSearch]: true,
           },
@@ -116,38 +130,43 @@ export const RelationshipFilter: React.FC<Props> = (props) => {
               addOptions(data, relationSlug)
 
               if (data.nextPage) {
-                nextPageByRelationshipRef.current.set(relationSlug, data.nextPage)
+                loadedRelationships.current.set(relationSlug, {
+                  hasLoadedAll: false,
+                  nextPage: data.nextPage,
+                })
               } else {
-                partiallyLoadedRelationshipSlugs.current =
-                  partiallyLoadedRelationshipSlugs.current.filter(
-                    (partiallyLoadedRelation) => partiallyLoadedRelation !== relationSlug,
-                  )
+                loadedRelationships.current.set(relationSlug, {
+                  hasLoadedAll: true,
+                  nextPage: null,
+                })
               }
             }
           } else {
             setErrorLoading(t('error:unspecific'))
           }
         } catch (e) {
           if (!abortController.signal.aborted) {
-            console.error(e)
+            console.error(e) // eslint-disable-line no-console
           }
         }
       }
 
       setHasLoadedFirstOptions(true)
     },
-    [addOptions, api, debouncedSearch, getEntityConfig, i18n.language, serverURL, t],
   )
 
-  const loadMoreOptions = React.useCallback(() => {
-    if (partiallyLoadedRelationshipSlugs.current.length > 0) {
+  const handleScrollToBottom = React.useCallback(() => {
+    const relationshipToLoad = loadedRelationships.current.entries().next().value
+
+    if (relationshipToLoad[0] && !relationshipToLoad[1].hasLoadedAll) {
       const abortController = new AbortController()
-      void loadRelationOptions({
+
+      void loadOptions({
         abortController,
-        relationSlug: partiallyLoadedRelationshipSlugs.current[0],
+        relationSlug: relationshipToLoad[0],
       })
     }
-  }, [loadRelationOptions])
+  }, [])
 
   const findOptionsByValue = useCallback((): Option | Option[] => {
     if (value) {
@@ -206,15 +225,28 @@ export const RelationshipFilter: React.FC<Props> = (props) => {
     return undefined
   }, [hasMany, hasMultipleRelations, value, options])
 
-  const handleInputChange = (input: string) => {
-    if (input !== search) {
-      dispatchOptions({ type: 'CLEAR', i18n, required: false })
-      const relationSlug = partiallyLoadedRelationshipSlugs.current[0]
-      partiallyLoadedRelationshipSlugs.current = relationSlugs
-      nextPageByRelationshipRef.current.set(relationSlug, 1)
-      setSearch(input)
-    }
-  }
+  const handleInputChange = useCallback(
+    (input: string) => {
+      if (input !== search) {
+        dispatchOptions({ type: 'CLEAR', i18n, required: false })
+
+        const relationSlugs = Array.isArray(relationTo) ? relationTo : [relationTo]
+
+        loadedRelationships.current = new Map(
+          relationSlugs.map((relation) => [
+            relation,
+            {
+              hasLoadedAll: false,
+              nextPage: 1,
+            },
+          ]),
+        )
+
+        setSearch(input)
+      }
+    },
+    [i18n, relationTo, search],
+  )
 
   const addOptionByID = useCallback(
     async (id, relation) => {
@@ -239,19 +271,37 @@ export const RelationshipFilter: React.FC<Props> = (props) => {
   )
 
   /**
-   * 1. Trigger initial relationship options fetch
-   * 2. When search changes, loadRelationOptions will
-   *    fire off again
+   * When `relationTo` changes externally, reset the options and reload them from scratch
+   * The `loadOptions` dependency is a useEffectEvent which has no dependencies of its own
+   * This means we can safely depend on it without it triggering this effect to run
+   * This is useful because this effect should _only_ run when `relationTo` changes
    */
   useEffect(() => {
     const relations = Array.isArray(relationTo) ? relationTo : [relationTo]
+
+    loadedRelationships.current = new Map(
+      relations.map((relation) => [
+        relation,
+        {
+          hasLoadedAll: false,
+          nextPage: 1,
+        },
+      ]),
+    )
+
+    dispatchOptions({ type: 'CLEAR', i18n, required: false })
+    setHasLoadedFirstOptions(false)
+
     const abortControllers: AbortController[] = []
+
     relations.forEach((relation) => {
       const abortController = new AbortController()
-      void loadRelationOptions({
+
+      void loadOptions({
         abortController,
         relationSlug: relation,
       })
+
       abortControllers.push(abortController)
     })
 
@@ -266,11 +316,10 @@ export const RelationshipFilter: React.FC<Props> = (props) => {
         }
       })
     }
-  }, [i18n, loadRelationOptions, relationTo])
+  }, [i18n, relationTo, debouncedSearch])
 
   /**
-   * Load any options that were not returned
-   * in the first 10 of each relation fetch
+   * Load any other options that might exist in the value that were not loaded already
    */
   useEffect(() => {
     if (value && hasLoadedFirstOptions) {
@@ -327,6 +376,7 @@ export const RelationshipFilter: React.FC<Props> = (props) => {
               onChange(null)
               return
             }
+
             if (hasMany && Array.isArray(selected)) {
               onChange(
                 selected
@@ -352,7 +402,7 @@ export const RelationshipFilter: React.FC<Props> = (props) => {
             }
           }}
           onInputChange={handleInputChange}
-          onMenuScrollToBottom={loadMoreOptions}
+          onMenuScrollToBottom={handleScrollToBottom}
           options={options}
           placeholder={t('general:selectValue')}
           value={valueToRender}

packages/ui/src/elements/WhereBuilder/Condition/Relationship/index.tsx

@@ -11,6 +11,7 @@ import { formatOptions } from './formatOptions.js'
 
 export const Select: React.FC<Props> = ({
   disabled,
+  isClearable,
   onChange,
   operator,
   options: optionsFromProps,
@@ -41,6 +42,7 @@ export const Select: React.FC<Props> = ({
   const onSelect = React.useCallback(
     (selectedOption) => {
       let newValue
+
       if (!selectedOption) {
         newValue = null
       } else if (isMulti) {
@@ -71,6 +73,7 @@ export const Select: React.FC<Props> = ({
   return (
     <ReactSelect
       disabled={disabled}
+      isClearable={isClearable}
       isMulti={isMulti}
       onChange={onSelect}
       options={options.map((option) => ({ ...option, label: getTranslation(option.label, i18n) }))}

packages/ui/src/elements/WhereBuilder/Condition/Select/index.tsx

@@ -4,6 +4,7 @@ import type { DefaultFilterProps } from '../types.js'
 
 export type Props = {
   readonly field: SelectFieldClient
+  readonly isClearable?: boolean
   readonly onChange: (val: string) => void
   readonly options: Option[]
   readonly value: string