From 911f01ace35f3372269fcb3fa959cf6d86c2f782 Mon Sep 17 00:00:00 2001 From: Tim Hansen Date: Fri, 4 Feb 2022 12:47:18 -0700 Subject: [PATCH] Set initial value from queryParameters Add tests for external queryParameter updates Signed-off-by: Tim Hansen --- .../EntityLifecyclePicker.test.tsx | 30 +++++++++++++ .../EntityLifecyclePicker.tsx | 18 +++++--- .../EntityOwnerPicker.test.tsx | 30 +++++++++++++ .../EntityOwnerPicker/EntityOwnerPicker.tsx | 16 ++++--- .../EntityTagPicker/EntityTagPicker.test.tsx | 30 +++++++++++++ .../EntityTagPicker/EntityTagPicker.tsx | 18 +++++--- .../EntityTypePicker.test.tsx | 34 ++++++++++++++ .../UserListPicker/UserListPicker.test.tsx | 36 +++++++++++++++ .../UserListPicker/UserListPicker.tsx | 16 +++++-- .../src/hooks/useEntityTypeFilter.tsx | 16 ++++--- .../catalog-react/src/testUtils/providers.tsx | 44 ++++++++++++------- 11 files changed, 245 insertions(+), 43 deletions(-) diff --git a/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.test.tsx b/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.test.tsx index f7d915ef5b..47e85292e2 100644 --- a/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.test.tsx +++ b/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.test.tsx @@ -161,4 +161,34 @@ describe('', () => { lifecycles: undefined, }); }); + + it('responds to external queryParameters changes', () => { + const updateFilters = jest.fn(); + const rendered = render( + + + , + ); + expect(updateFilters).toHaveBeenLastCalledWith({ + lifecycles: new EntityLifecycleFilter(['experimental']), + }); + rendered.rerender( + + + , + ); + expect(updateFilters).toHaveBeenLastCalledWith({ + lifecycles: new EntityLifecycleFilter(['production']), + }); + }); }); diff --git a/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.tsx b/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.tsx index e4e42eb7ba..be0351ff97 100644 --- a/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.tsx +++ b/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.tsx @@ -51,18 +51,24 @@ export const EntityLifecyclePicker = () => { const { updateFilters, backendEntities, filters, queryParameters } = useEntityListProvider(); + const queryParamLifecycles = useMemo( + () => [queryParameters.lifecycles].flat().filter(Boolean) as string[], + [queryParameters], + ); + const [selectedLifecycles, setSelectedLifecycles] = useState( - filters.lifecycles?.values ?? [], + queryParamLifecycles.length + ? queryParamLifecycles + : filters.lifecycles?.values ?? [], ); // Set selected lifecycles on query parameter updates; this happens at initial page load and from // external updates to the page location. useEffect(() => { - const queryParamLifecycles = [queryParameters.lifecycles] - .flat() - .filter(Boolean) as string[]; - setSelectedLifecycles(queryParamLifecycles); - }, [queryParameters]); + if (queryParamLifecycles.length) { + setSelectedLifecycles(queryParamLifecycles); + } + }, [queryParamLifecycles]); useEffect(() => { updateFilters({ diff --git a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx index 277a691a84..14abc358db 100644 --- a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx +++ b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx @@ -191,4 +191,34 @@ describe('', () => { owner: undefined, }); }); + + it('responds to external queryParameters changes', () => { + const updateFilters = jest.fn(); + const rendered = render( + + + , + ); + expect(updateFilters).toHaveBeenLastCalledWith({ + owners: new EntityOwnerFilter(['team-a']), + }); + rendered.rerender( + + + , + ); + expect(updateFilters).toHaveBeenLastCalledWith({ + owners: new EntityOwnerFilter(['team-b']), + }); + }); }); diff --git a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx index 95e09ed434..d2a740d850 100644 --- a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx +++ b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx @@ -53,18 +53,22 @@ export const EntityOwnerPicker = () => { const { updateFilters, backendEntities, filters, queryParameters } = useEntityListProvider(); + const queryParamOwners = useMemo( + () => [queryParameters.owners].flat().filter(Boolean) as string[], + [queryParameters], + ); + const [selectedOwners, setSelectedOwners] = useState( - filters.owners?.values ?? [], + queryParamOwners.length ? queryParamOwners : filters.owners?.values ?? [], ); // Set selected owners on query parameter updates; this happens at initial page load and from // external updates to the page location. useEffect(() => { - const queryParamOwners = [queryParameters.owners] - .flat() - .filter(Boolean) as string[]; - setSelectedOwners(queryParamOwners); - }, [queryParameters]); + if (queryParamOwners.length) { + setSelectedOwners(queryParamOwners); + } + }, [queryParamOwners]); useEffect(() => { updateFilters({ diff --git a/plugins/catalog-react/src/components/EntityTagPicker/EntityTagPicker.test.tsx b/plugins/catalog-react/src/components/EntityTagPicker/EntityTagPicker.test.tsx index 770c187b4e..c6b985bc8b 100644 --- a/plugins/catalog-react/src/components/EntityTagPicker/EntityTagPicker.test.tsx +++ b/plugins/catalog-react/src/components/EntityTagPicker/EntityTagPicker.test.tsx @@ -149,4 +149,34 @@ describe('', () => { tags: undefined, }); }); + + it('responds to external queryParameters changes', () => { + const updateFilters = jest.fn(); + const rendered = render( + + + , + ); + expect(updateFilters).toHaveBeenLastCalledWith({ + tags: new EntityTagFilter(['tag1']), + }); + rendered.rerender( + + + , + ); + expect(updateFilters).toHaveBeenLastCalledWith({ + tags: new EntityTagFilter(['tag2']), + }); + }); }); diff --git a/plugins/catalog-react/src/components/EntityTagPicker/EntityTagPicker.tsx b/plugins/catalog-react/src/components/EntityTagPicker/EntityTagPicker.tsx index 9753ae14f4..4cd17b3070 100644 --- a/plugins/catalog-react/src/components/EntityTagPicker/EntityTagPicker.tsx +++ b/plugins/catalog-react/src/components/EntityTagPicker/EntityTagPicker.tsx @@ -51,16 +51,22 @@ export const EntityTagPicker = () => { const { updateFilters, backendEntities, filters, queryParameters } = useEntityListProvider(); - const [selectedTags, setSelectedTags] = useState(filters.tags?.values ?? []); + const queryParamTags = useMemo( + () => [queryParameters.tags].flat().filter(Boolean) as string[], + [queryParameters], + ); + + const [selectedTags, setSelectedTags] = useState( + queryParamTags.length ? queryParamTags : filters.tags?.values ?? [], + ); // Set selected tags on query parameter updates; this happens at initial page load and from // external updates to the page location. useEffect(() => { - const queryParamTags = [queryParameters.tags] - .flat() - .filter(Boolean) as string[]; - setSelectedTags(queryParamTags); - }, [queryParameters]); + if (queryParamTags.length) { + setSelectedTags(queryParamTags); + } + }, [queryParamTags]); useEffect(() => { updateFilters({ diff --git a/plugins/catalog-react/src/components/EntityTypePicker/EntityTypePicker.test.tsx b/plugins/catalog-react/src/components/EntityTypePicker/EntityTypePicker.test.tsx index f6e9d296df..9ed27bbef8 100644 --- a/plugins/catalog-react/src/components/EntityTypePicker/EntityTypePicker.test.tsx +++ b/plugins/catalog-react/src/components/EntityTypePicker/EntityTypePicker.test.tsx @@ -151,4 +151,38 @@ describe('', () => { type: new EntityTypeFilter(['tool']), }); }); + + it('responds to external queryParameters changes', async () => { + const updateFilters = jest.fn(); + const rendered = await renderWithEffects( + + + + + , + ); + expect(updateFilters).toHaveBeenLastCalledWith({ + type: new EntityTypeFilter(['service']), + }); + rendered.rerender( + + + + + , + ); + expect(updateFilters).toHaveBeenLastCalledWith({ + type: new EntityTypeFilter(['tool']), + }); + }); }); diff --git a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx index b776f26926..d4cfedecbf 100644 --- a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx +++ b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx @@ -253,6 +253,42 @@ describe('', () => { }); }); + it('responds to external queryParameters changes', () => { + const updateFilters = jest.fn(); + const rendered = render( + + + + + , + ); + expect(updateFilters).toHaveBeenLastCalledWith({ + user: new UserListFilter('all', mockIsOwnedEntity, mockIsStarredEntity), + }); + rendered.rerender( + + + + + , + ); + expect(updateFilters).toHaveBeenLastCalledWith({ + user: new UserListFilter('owned', mockIsOwnedEntity, mockIsStarredEntity), + }); + }); + describe.each` type | filterFn ${'owned'} | ${mockIsOwnedEntity} diff --git a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx index 4820a4f0f9..cbf6089f2e 100644 --- a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx +++ b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx @@ -161,7 +161,14 @@ export const UserListPicker = ({ [isOwnedEntity, isStarredEntity], ); - const [selectedUserFilter, setSelectedUserFilter] = useState(initialFilter); + const queryParamUserFilter = useMemo( + () => [queryParameters.user].flat()[0], + [queryParameters], + ); + + const [selectedUserFilter, setSelectedUserFilter] = useState( + queryParamUserFilter ?? initialFilter, + ); // To show proper counts for each section, apply all other frontend filters _except_ the user // filter that's controlled by this picker. @@ -191,9 +198,10 @@ export const UserListPicker = ({ // Set selected user filter on query parameter updates; this happens at initial page load and from // external updates to the page location. useEffect(() => { - const queryParamUserFilter = [queryParameters.user].flat()[0]; - setSelectedUserFilter(queryParamUserFilter as UserListFilterKind); - }, [queryParameters]); + if (queryParamUserFilter) { + setSelectedUserFilter(queryParamUserFilter as UserListFilterKind); + } + }, [queryParamUserFilter]); useEffect(() => { if ( diff --git a/plugins/catalog-react/src/hooks/useEntityTypeFilter.tsx b/plugins/catalog-react/src/hooks/useEntityTypeFilter.tsx index cbc9d4ed45..c4531d9117 100644 --- a/plugins/catalog-react/src/hooks/useEntityTypeFilter.tsx +++ b/plugins/catalog-react/src/hooks/useEntityTypeFilter.tsx @@ -42,18 +42,22 @@ export function useEntityTypeFilter(): EntityTypeReturn { updateFilters, } = useEntityListProvider(); + const queryParamTypes = useMemo( + () => [queryParameters.type].flat().filter(Boolean) as string[], + [queryParameters], + ); + const [selectedTypes, setSelectedTypes] = useState( - typeFilter?.getTypes() ?? [], + queryParamTypes.length ? queryParamTypes : typeFilter?.getTypes() ?? [], ); // Set selected types on query parameter updates; this happens at initial page load and from // external updates to the page location. useEffect(() => { - const queryParamTypes = [queryParameters.type] - .flat() - .filter(Boolean) as string[]; - setSelectedTypes(queryParamTypes); - }, [queryParameters]); + if (queryParamTypes.length) { + setSelectedTypes(queryParamTypes); + } + }, [queryParamTypes]); const [availableTypes, setAvailableTypes] = useState([]); const kind = useMemo(() => kindFilter?.value, [kindFilter]); diff --git a/plugins/catalog-react/src/testUtils/providers.tsx b/plugins/catalog-react/src/testUtils/providers.tsx index 2172f6c5a9..5b2eae04a5 100644 --- a/plugins/catalog-react/src/testUtils/providers.tsx +++ b/plugins/catalog-react/src/testUtils/providers.tsx @@ -14,7 +14,12 @@ * limitations under the License. */ -import React, { PropsWithChildren, useCallback, useState } from 'react'; +import React, { + PropsWithChildren, + useCallback, + useMemo, + useState, +} from 'react'; import { DefaultEntityFilters, EntityListContext, @@ -32,6 +37,7 @@ export const MockEntityListContextProvider = ({ const [filters, setFilters] = useState( value?.filters ?? {}, ); + const updateFilters = useCallback( ( update: @@ -49,23 +55,31 @@ export const MockEntityListContextProvider = ({ [], ); - const defaultContext: EntityListContextProps = { - entities: [], - backendEntities: [], - updateFilters, - filters, - loading: false, - queryParameters: {}, - }; + // Memoize the default values since pickers have useEffect triggers on these; naively defaulting + // below with `?? ` breaks referential equality on subsequent updates. + const defaultValues = useMemo( + () => ({ + entities: [], + backendEntities: [], + queryParameters: {}, + }), + [], + ); - // Extract value.filters to avoid overwriting it; some tests exercise filter updates. The value - // provided is used as the initial seed in useState above. - const { filters: _, ...otherContextFields } = value ?? {}; + const resolvedValue: EntityListContextProps = useMemo( + () => ({ + entities: value?.entities ?? defaultValues.entities, + backendEntities: value?.backendEntities ?? defaultValues.backendEntities, + updateFilters: value?.updateFilters ?? updateFilters, + filters, + loading: value?.loading ?? false, + queryParameters: value?.queryParameters ?? defaultValues.queryParameters, + }), + [value, defaultValues, filters, updateFilters], + ); return ( - + {children} );