From da6bc55b19fcbaf2602f59d316fcee4e844dc4cb Mon Sep 17 00:00:00 2001 From: Jacob Fletcher Date: Tue, 10 Dec 2024 17:37:52 -0500 Subject: [PATCH] fix(ui): ensures admin.disableListFilter is disabled despite url search params (#9874) Continuation of #9846 and partial fix for #9774. When setting `admin.disableListFilter` retroactively, it remains active within the list filter controls. Same for when the URL search query contains one of these fields, except this will actually display the _wrong_ field, falling back to the _first_ field from the config. The fix is to properly disable the condition for this field if it's an active filter, while still preventing it from ever rendering as an option within the field selector itself. --- .../elements/WhereBuilder/Condition/index.tsx | 24 ++++++-- .../WhereBuilder/reduceClientFields.tsx | 2 +- packages/ui/src/providers/ListQuery/index.tsx | 10 ++-- test/admin/e2e/2/e2e.spec.ts | 17 +++--- test/fields-relationship/e2e.spec.ts | 4 +- test/fields/collections/Email/e2e.spec.ts | 16 +++--- test/fields/collections/Number/e2e.spec.ts | 4 +- .../collections/Relationship/e2e.spec.ts | 4 +- test/fields/collections/Text/e2e.spec.ts | 57 +++++++++++++++---- test/helpers/e2e/openListColumns.ts | 26 +++++++++ test/helpers/e2e/openListFilters.ts | 26 +++++++++ test/helpers/e2e/toggleColumn.ts | 23 +------- 12 files changed, 147 insertions(+), 66 deletions(-) create mode 100644 test/helpers/e2e/openListColumns.ts create mode 100644 test/helpers/e2e/openListFilters.ts diff --git a/packages/ui/src/elements/WhereBuilder/Condition/index.tsx b/packages/ui/src/elements/WhereBuilder/Condition/index.tsx index f9865f0955..6e9e9e4521 100644 --- a/packages/ui/src/elements/WhereBuilder/Condition/index.tsx +++ b/packages/ui/src/elements/WhereBuilder/Condition/index.tsx @@ -1,5 +1,5 @@ 'use client' -import React, { useEffect, useState } from 'react' +import React, { useEffect, useMemo, useState } from 'react' import type { FieldCondition } from '../types.js' @@ -64,6 +64,11 @@ export const Condition: React.FC = (props) => { RenderedFilter, updateCondition, } = props + + const options = useMemo(() => { + return fields.filter(({ field }) => !field.admin.disableListFilter) + }, [fields]) + const [internalField, setInternalField] = useState(() => fields.find((field) => fieldName === field.value), ) @@ -113,25 +118,34 @@ export const Condition: React.FC = (props) => { valueOptions = internalField.field.options } + const disabled = + (!internalField?.value && typeof internalField?.value !== 'number') || + internalField?.field?.admin?.disableListFilter + return (
{ setInternalField(fields.find((f) => f.value === field.value)) setInternalOperatorOption(undefined) setInternalQueryValue(undefined) }} - options={fields} - value={fields.find((field) => internalField?.value === field.value) || fields[0]} + options={options} + value={ + fields.find((field) => internalField?.value === field.value) || { + value: internalField?.value, + } + } />
) => { setInternalOperatorOption(operator.value) @@ -148,7 +162,7 @@ export const Condition: React.FC = (props) => { {RenderedFilter || ( { return fields.reduce((reduced, field) => { - if (field.admin?.disableListFilter || (fieldIsHiddenOrDisabled(field) && !fieldIsID(field))) { + if (fieldIsHiddenOrDisabled(field) && !fieldIsID(field)) { return reduced } diff --git a/packages/ui/src/providers/ListQuery/index.tsx b/packages/ui/src/providers/ListQuery/index.tsx index c1a43b2d97..66184783d1 100644 --- a/packages/ui/src/providers/ListQuery/index.tsx +++ b/packages/ui/src/providers/ListQuery/index.tsx @@ -124,17 +124,19 @@ export const ListQueryProvider: React.FC = ({ } }, [ - modifySearchParams, currentQuery?.page, currentQuery?.limit, + currentQuery?.search, currentQuery?.sort, currentQuery?.where, - currentQuery?.search, preferenceKey, - router, - setPreference, + defaultLimit, + defaultSort, + modifySearchParams, onQueryChange, onQueryChangeFromProps, + setPreference, + router, ], ) diff --git a/test/admin/e2e/2/e2e.spec.ts b/test/admin/e2e/2/e2e.spec.ts index bdd2394cec..5e0930db9e 100644 --- a/test/admin/e2e/2/e2e.spec.ts +++ b/test/admin/e2e/2/e2e.spec.ts @@ -26,8 +26,10 @@ const description = 'Description' let payload: PayloadTestSDK -import { goToFirstCell, navigateToDoc } from 'helpers/e2e/navigateToDoc.js' -import { openListColumns, toggleColumn } from 'helpers/e2e/toggleColumn.js' +import { goToFirstCell } from 'helpers/e2e/navigateToDoc.js' +import { openListColumns } from 'helpers/e2e/openListColumns.js' +import { openListFilters } from 'helpers/e2e/openListFilters.js' +import { toggleColumn } from 'helpers/e2e/toggleColumn.js' import path from 'path' import { wait } from 'payload/shared' import { fileURLToPath } from 'url' @@ -273,9 +275,7 @@ describe('admin2', () => { await expect(page.locator(tableRowLocator)).toHaveCount(2) - await page.locator('.list-controls__toggle-where').click() - // wait until the filter UI is visible and fully expanded - await expect(page.locator('.list-controls__where.rah-static--height-auto')).toBeVisible() + await openListFilters(page, {}) await page.locator('.where-builder__add-first-filter').click() @@ -313,8 +313,7 @@ describe('admin2', () => { // open the column controls await page.locator('.list-controls__toggle-columns').click() - await page.locator('.list-controls__toggle-where').click() - await page.waitForSelector('.list-controls__where.rah-static--height-auto') + await openListFilters(page, {}) await page.locator('.where-builder__add-first-filter').click() const operatorField = page.locator('.condition__operator') @@ -461,7 +460,7 @@ describe('admin2', () => { await page.goto(`${postsUrl.list}?limit=10&page=2`) // add filter - await page.locator('.list-controls__toggle-where').click() + await openListFilters(page, {}) await page.locator('.where-builder__add-first-filter').click() await page.locator('.condition__field .rs__control').click() const options = page.locator('.rs__option') @@ -776,7 +775,7 @@ describe('admin2', () => { ).toHaveText('Title') // filters - await page.locator('.list-controls__toggle-where').click() + await openListFilters(page, {}) await page.locator('.where-builder__add-first-filter').click() await page.locator('.condition__field .rs__control').click() const options = page.locator('.rs__option') diff --git a/test/fields-relationship/e2e.spec.ts b/test/fields-relationship/e2e.spec.ts index c34cd42071..1c8c0e96a8 100644 --- a/test/fields-relationship/e2e.spec.ts +++ b/test/fields-relationship/e2e.spec.ts @@ -2,6 +2,7 @@ import type { Page } from '@playwright/test' import { expect, test } from '@playwright/test' import { openDocControls } from 'helpers/e2e/openDocControls.js' +import { openListFilters } from 'helpers/e2e/openListFilters.js' import path from 'path' import { wait } from 'payload/shared' import { fileURLToPath } from 'url' @@ -485,8 +486,7 @@ describe('fields - relationship', () => { await page.goto(versionedRelationshipFieldURL.list) await page.locator('.list-controls__toggle-columns').click() - await page.locator('.list-controls__toggle-where').click() - await page.waitForSelector('.list-controls__where.rah-static--height-auto') + await openListFilters(page, {}) await page.locator('.where-builder__add-first-filter').click() const conditionField = page.locator('.condition__field') diff --git a/test/fields/collections/Email/e2e.spec.ts b/test/fields/collections/Email/e2e.spec.ts index 15f54fca46..37c1782010 100644 --- a/test/fields/collections/Email/e2e.spec.ts +++ b/test/fields/collections/Email/e2e.spec.ts @@ -1,6 +1,7 @@ import type { Page } from '@playwright/test' import { expect, test } from '@playwright/test' +import { openListFilters } from 'helpers/e2e/openListFilters.js' import path from 'path' import { wait } from 'payload/shared' import { fileURLToPath } from 'url' @@ -89,7 +90,7 @@ describe('Email', () => { test('should show field in filter when admin.disableListColumn is true', async () => { await page.goto(url.list) - await page.locator('.list-controls__toggle-where').click() + await openListFilters(page, {}) await page.locator('.where-builder__add-first-filter').click() const initialField = page.locator('.condition__field') @@ -100,7 +101,7 @@ describe('Email', () => { ).toBeVisible() }) - test('should display field in list view column selector if admin.disableListColumn is false and admin.disableListFilter is true', async () => { + test('should display field in list view column selector despite admin.disableListFilter', async () => { await page.goto(url.list) await page.locator('.list-controls__toggle-columns').click() @@ -116,7 +117,7 @@ describe('Email', () => { test('should hide field in filter when admin.disableListFilter is true', async () => { await page.goto(url.list) - await page.locator('.list-controls__toggle-where').click() + await openListFilters(page, {}) await page.locator('.where-builder__add-first-filter').click() const initialField = page.locator('.condition__field') @@ -188,8 +189,7 @@ describe('Email', () => { await page.goto(url.list) // open the first filter options - await page.locator('.list-controls__toggle-where').click() - await expect(page.locator('.list-controls__where.rah-static--height-auto')).toBeVisible() + await openListFilters(page, {}) await page.locator('.where-builder__add-first-filter').click() const firstInitialField = page.locator('.condition__field') @@ -230,8 +230,7 @@ describe('Email', () => { await page.goto(url.list) // open the first filter options - await page.locator('.list-controls__toggle-where').click() - await expect(page.locator('.list-controls__where.rah-static--height-auto')).toBeVisible() + await openListFilters(page, {}) await page.locator('.where-builder__add-first-filter').click() const firstInitialField = page.locator('.condition__field') @@ -261,8 +260,7 @@ describe('Email', () => { await page.goto(url.list) // open the first filter options - await page.locator('.list-controls__toggle-where').click() - await expect(page.locator('.list-controls__where.rah-static--height-auto')).toBeVisible() + await openListFilters(page, {}) await page.locator('.where-builder__add-first-filter').click() const firstInitialField = page.locator('.condition__field') diff --git a/test/fields/collections/Number/e2e.spec.ts b/test/fields/collections/Number/e2e.spec.ts index c2a70e507b..e195836991 100644 --- a/test/fields/collections/Number/e2e.spec.ts +++ b/test/fields/collections/Number/e2e.spec.ts @@ -1,6 +1,7 @@ import type { Page } from '@playwright/test' import { expect, test } from '@playwright/test' +import { openListFilters } from 'helpers/e2e/openListFilters.js' import path from 'path' import { wait } from 'payload/shared' import { fileURLToPath } from 'url' @@ -73,8 +74,7 @@ describe('Number', () => { test('should filter Number fields in the collection view - greaterThanOrEqual', async () => { await page.goto(url.list) await expect(page.locator('table >> tbody >> tr')).toHaveCount(3) - await page.locator('.list-controls__toggle-where').click() - await expect(page.locator('.list-controls__where.rah-static--height-auto')).toBeVisible() + await openListFilters(page, {}) await page.locator('.where-builder__add-first-filter').click() const initialField = page.locator('.condition__field') const operatorField = page.locator('.condition__operator') diff --git a/test/fields/collections/Relationship/e2e.spec.ts b/test/fields/collections/Relationship/e2e.spec.ts index af3e754279..f7be4f348f 100644 --- a/test/fields/collections/Relationship/e2e.spec.ts +++ b/test/fields/collections/Relationship/e2e.spec.ts @@ -3,6 +3,7 @@ import type { Page } from '@playwright/test' import { expect, test } from '@playwright/test' import { navigateToDoc } from 'helpers/e2e/navigateToDoc.js' import { openDocControls } from 'helpers/e2e/openDocControls.js' +import { openListFilters } from 'helpers/e2e/openListFilters.js' import path from 'path' import { wait } from 'payload/shared' import { fileURLToPath } from 'url' @@ -614,8 +615,7 @@ describe('relationship', () => { await page.locator('.list-controls__toggle-columns').click() await wait(400) - await page.locator('.list-controls__toggle-where').click() - await expect(page.locator('.list-controls__where.rah-static--height-auto')).toBeVisible() + await openListFilters(page, {}) await wait(400) await page.locator('.where-builder__add-first-filter').click() diff --git a/test/fields/collections/Text/e2e.spec.ts b/test/fields/collections/Text/e2e.spec.ts index 04d21c2416..60fe79200c 100644 --- a/test/fields/collections/Text/e2e.spec.ts +++ b/test/fields/collections/Text/e2e.spec.ts @@ -2,10 +2,13 @@ import type { Page } from '@playwright/test' import type { GeneratedTypes } from 'helpers/sdk/types.js' import { expect, test } from '@playwright/test' -import { openListColumns, toggleColumn } from 'helpers/e2e/toggleColumn.js' +import { openListColumns } from 'helpers/e2e/openListColumns.js' +import { openListFilters } from 'helpers/e2e/openListFilters.js' +import { toggleColumn } from 'helpers/e2e/toggleColumn.js' import { upsertPrefs } from 'helpers/e2e/upsertPrefs.js' import path from 'path' import { wait } from 'payload/shared' +import * as qs from 'qs-esm' import { fileURLToPath } from 'url' import type { PayloadTestSDK } from '../../../helpers/sdk/index.js' @@ -180,7 +183,7 @@ describe('Text', () => { test('should show field in filter when admin.disableListColumn is true', async () => { await page.goto(url.list) - await page.locator('.list-controls__toggle-where').click() + await openListFilters(page, {}) await page.locator('.where-builder__add-first-filter').click() const initialField = page.locator('.condition__field') @@ -191,7 +194,7 @@ describe('Text', () => { ).toBeVisible() }) - test('should display field in list view column selector if admin.disableListColumn is false and admin.disableListFilter is true', async () => { + test('should display field in list view column selector despite admin.disableListFilter', async () => { await page.goto(url.list) await page.locator('.list-controls__toggle-columns').click() @@ -205,6 +208,43 @@ describe('Text', () => { ).toBeVisible() }) + test('should disable field when admin.disableListFilter is true but still exists in the query', async () => { + await page.goto( + `${url.list}${qs.stringify( + { + where: { + or: [ + { + and: [ + { + disableListFilterText: { + equals: 'Disable List Filter Text', + }, + }, + ], + }, + ], + }, + }, + { addQueryPrefix: true }, + )}`, + ) + + await openListFilters(page, {}) + + const condition = page.locator('.condition__field') + await expect(condition.locator('input.rs__input')).toBeDisabled() + await expect(page.locator('.condition__operator input.rs__input')).toBeDisabled() + await expect(page.locator('.condition__value input.condition-value-text')).toBeDisabled() + await expect(condition.locator('.rs__single-value')).toHaveText('Disable List Filter Text') + await page.locator('button.condition__actions-add').click() + const condition2 = page.locator('.condition__field').nth(1) + await condition2.click() + await expect( + condition2?.locator('.rs__menu-list:has-text("Disable List Filter Text")'), + ).toBeHidden() + }) + test('should respect admin.disableListColumn despite preferences', async () => { await upsertPrefs>({ payload, @@ -233,7 +273,7 @@ describe('Text', () => { test('should hide field in filter when admin.disableListFilter is true', async () => { await page.goto(url.list) - await page.locator('.list-controls__toggle-where').click() + await openListFilters(page, {}) await page.locator('.where-builder__add-first-filter').click() const initialField = page.locator('.condition__field') @@ -298,8 +338,7 @@ describe('Text', () => { await page.goto(url.list) // open the first filter options - await page.locator('.list-controls__toggle-where').click() - await expect(page.locator('.list-controls__where.rah-static--height-auto')).toBeVisible() + await openListFilters(page, {}) await page.locator('.where-builder__add-first-filter').click() const firstInitialField = page.locator('.condition__field') @@ -340,8 +379,7 @@ describe('Text', () => { await page.goto(url.list) // open the first filter options - await page.locator('.list-controls__toggle-where').click() - await expect(page.locator('.list-controls__where.rah-static--height-auto')).toBeVisible() + await openListFilters(page, {}) await page.locator('.where-builder__add-first-filter').click() const firstInitialField = page.locator('.condition__field') @@ -371,8 +409,7 @@ describe('Text', () => { await page.goto(url.list) // open the first filter options - await page.locator('.list-controls__toggle-where').click() - await expect(page.locator('.list-controls__where.rah-static--height-auto')).toBeVisible() + await openListFilters(page, {}) await page.locator('.where-builder__add-first-filter').click() const firstInitialField = page.locator('.condition__field') diff --git a/test/helpers/e2e/openListColumns.ts b/test/helpers/e2e/openListColumns.ts new file mode 100644 index 0000000000..a9664e58cd --- /dev/null +++ b/test/helpers/e2e/openListColumns.ts @@ -0,0 +1,26 @@ +import type { Page } from '@playwright/test' + +import { expect } from '@playwright/test' + +export const openListColumns = async ( + page: Page, + { + togglerSelector = '.list-controls__toggle-columns', + columnContainerSelector = '.list-controls__columns', + }: { + columnContainerSelector?: string + togglerSelector?: string + }, +): Promise => { + const columnContainer = page.locator(columnContainerSelector).first() + + const isAlreadyOpen = await columnContainer.isVisible() + + if (!isAlreadyOpen) { + await page.locator(togglerSelector).first().click() + } + + await expect(page.locator(`${columnContainerSelector}.rah-static--height-auto`)).toBeVisible() + + return columnContainer +} diff --git a/test/helpers/e2e/openListFilters.ts b/test/helpers/e2e/openListFilters.ts new file mode 100644 index 0000000000..3533195181 --- /dev/null +++ b/test/helpers/e2e/openListFilters.ts @@ -0,0 +1,26 @@ +import type { Page } from '@playwright/test' + +import { expect } from '@playwright/test' + +export const openListFilters = async ( + page: Page, + { + togglerSelector = '.list-controls__toggle-where', + filterContainerSelector = '.list-controls__where', + }: { + filterContainerSelector?: string + togglerSelector?: string + }, +) => { + const columnContainer = page.locator(filterContainerSelector).first() + + const isAlreadyOpen = await columnContainer.isVisible() + + if (!isAlreadyOpen) { + await page.locator(togglerSelector).first().click() + } + + await expect(page.locator(`${filterContainerSelector}.rah-static--height-auto`)).toBeVisible() + + return columnContainer +} diff --git a/test/helpers/e2e/toggleColumn.ts b/test/helpers/e2e/toggleColumn.ts index 79b1447110..74e44956a5 100644 --- a/test/helpers/e2e/toggleColumn.ts +++ b/test/helpers/e2e/toggleColumn.ts @@ -3,28 +3,7 @@ import type { Page } from '@playwright/test' import { expect } from '@playwright/test' import { exactText } from '../../helpers.js' - -export const openListColumns = async ( - page: Page, - { - togglerSelector = '.list-controls__toggle-columns', - columnContainerSelector = '.list-controls__columns', - }: { - columnContainerSelector?: string - togglerSelector?: string - }, -): Promise => { - const columnContainer = page.locator(columnContainerSelector).first() - const isAlreadyOpen = await columnContainer.isVisible() - - if (!isAlreadyOpen) { - await page.locator(togglerSelector).first().click() - } - - await expect(page.locator(`${columnContainerSelector}.rah-static--height-auto`)).toBeVisible() - - return columnContainer -} +import { openListColumns } from './openListColumns.js' export const toggleColumn = async ( page: Page,