From 4e65bd9a58ecae8390d43f7f28692ece070487f4 Mon Sep 17 00:00:00 2001 From: Sarah Medeiros Date: Tue, 6 Dec 2022 17:56:24 -0500 Subject: [PATCH 1/6] Fix owner and lifecycle picker Signed-off-by: Sarah Medeiros --- .../EntityLifecyclePicker.tsx | 22 +++++++------- .../EntityOwnerPicker/EntityOwnerPicker.tsx | 30 +++++++++---------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.tsx b/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.tsx index f6e6b4eeaa..693c8ab85c 100644 --- a/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.tsx +++ b/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.tsx @@ -83,17 +83,17 @@ export const EntityLifecyclePicker = () => { }); }, [selectedLifecycles, updateFilters]); - const availableLifecycles = useMemo( - () => - [ - ...new Set( - backendEntities - .map((e: Entity) => e.spec?.lifecycle) - .filter(Boolean) as string[], - ), - ].sort(), - [backendEntities], - ); + const availableLifecycles = useMemo(() => { + const lifecycles = [ + ...new Set( + backendEntities + .map((e: Entity) => e.spec?.lifecycle) + .filter(Boolean) as string[], + ), + ].sort(); + if (lifecycles.length === 0) setSelectedLifecycles([]); + return lifecycles; + }, [backendEntities]); if (!availableLifecycles.length) return null; diff --git a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx index 0a4a79857e..d389ead06a 100644 --- a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx +++ b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx @@ -83,21 +83,21 @@ export const EntityOwnerPicker = () => { }); }, [selectedOwners, updateFilters]); - const availableOwners = useMemo( - () => - [ - ...new Set( - backendEntities - .flatMap((e: Entity) => - getEntityRelations(e, RELATION_OWNED_BY).map(o => - humanizeEntityRef(o, { defaultKind: 'group' }), - ), - ) - .filter(Boolean) as string[], - ), - ].sort(), - [backendEntities], - ); + const availableOwners = useMemo(() => { + const owners = [ + ...new Set( + backendEntities + .flatMap((e: Entity) => + getEntityRelations(e, RELATION_OWNED_BY).map(o => + humanizeEntityRef(o, { defaultKind: 'group' }), + ), + ) + .filter(Boolean) as string[], + ), + ].sort(); + if (owners.length === 0) setSelectedOwners([]); + return owners; + }, [backendEntities]); if (!availableOwners.length) return null; From c773242555dab1008e68644f82e795e25230c9bd Mon Sep 17 00:00:00 2001 From: Sarah Medeiros Date: Wed, 7 Dec 2022 14:08:46 -0500 Subject: [PATCH 2/6] Remove check from useMemo in order to keep function pure Signed-off-by: Sarah Medeiros --- .../EntityLifecyclePicker.tsx | 38 +++++++-------- .../EntityOwnerPicker/EntityOwnerPicker.tsx | 46 +++++++++---------- 2 files changed, 42 insertions(+), 42 deletions(-) diff --git a/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.tsx b/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.tsx index 693c8ab85c..dda24c3639 100644 --- a/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.tsx +++ b/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.tsx @@ -67,14 +67,6 @@ export const EntityLifecyclePicker = () => { : 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(() => { - if (queryParamLifecycles.length) { - setSelectedLifecycles(queryParamLifecycles); - } - }, [queryParamLifecycles]); - useEffect(() => { updateFilters({ lifecycles: selectedLifecycles.length @@ -83,17 +75,25 @@ export const EntityLifecyclePicker = () => { }); }, [selectedLifecycles, updateFilters]); - const availableLifecycles = useMemo(() => { - const lifecycles = [ - ...new Set( - backendEntities - .map((e: Entity) => e.spec?.lifecycle) - .filter(Boolean) as string[], - ), - ].sort(); - if (lifecycles.length === 0) setSelectedLifecycles([]); - return lifecycles; - }, [backendEntities]); + const availableLifecycles = useMemo( + () => + [ + ...new Set( + backendEntities + .map((e: Entity) => e.spec?.lifecycle) + .filter(Boolean) as string[], + ), + ].sort(), + [backendEntities], + ); + + // Set selected lifecycles on query parameter updates; this happens at initial page load and from + // external updates to the page location. + useEffect(() => { + if (queryParamLifecycles.length && availableLifecycles.length) { + setSelectedLifecycles(queryParamLifecycles); + } + }, [queryParamLifecycles, availableLifecycles]); if (!availableLifecycles.length) return null; diff --git a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx index d389ead06a..3804324619 100644 --- a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx +++ b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx @@ -67,14 +67,6 @@ export const EntityOwnerPicker = () => { 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(() => { - if (queryParamOwners.length) { - setSelectedOwners(queryParamOwners); - } - }, [queryParamOwners]); - useEffect(() => { updateFilters({ owners: selectedOwners.length @@ -83,21 +75,29 @@ export const EntityOwnerPicker = () => { }); }, [selectedOwners, updateFilters]); - const availableOwners = useMemo(() => { - const owners = [ - ...new Set( - backendEntities - .flatMap((e: Entity) => - getEntityRelations(e, RELATION_OWNED_BY).map(o => - humanizeEntityRef(o, { defaultKind: 'group' }), - ), - ) - .filter(Boolean) as string[], - ), - ].sort(); - if (owners.length === 0) setSelectedOwners([]); - return owners; - }, [backendEntities]); + const availableOwners = useMemo( + () => + [ + ...new Set( + backendEntities + .flatMap((e: Entity) => + getEntityRelations(e, RELATION_OWNED_BY).map(o => + humanizeEntityRef(o, { defaultKind: 'group' }), + ), + ) + .filter(Boolean) as string[], + ), + ].sort(), + [backendEntities], + ); + + // Set selected owners on query parameter updates; this happens at initial page load and from + // external updates to the page location. + useEffect(() => { + if (queryParamOwners.length && availableOwners.length) { + setSelectedOwners(queryParamOwners); + } + }, [queryParamOwners, availableOwners]); if (!availableOwners.length) return null; From 50aa75ed8042133a5607f84b0b249c6e1d08572c Mon Sep 17 00:00:00 2001 From: Sarah Medeiros Date: Wed, 7 Dec 2022 14:25:01 -0500 Subject: [PATCH 3/6] Add available filters check back Signed-off-by: Sarah Medeiros --- .../EntityLifecyclePicker/EntityLifecyclePicker.tsx | 4 ++++ .../src/components/EntityOwnerPicker/EntityOwnerPicker.tsx | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.tsx b/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.tsx index dda24c3639..c19da47feb 100644 --- a/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.tsx +++ b/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.tsx @@ -95,6 +95,10 @@ export const EntityLifecyclePicker = () => { } }, [queryParamLifecycles, availableLifecycles]); + useEffect(() => { + if (!availableLifecycles.length) setSelectedLifecycles([]); + }, [availableLifecycles]); + if (!availableLifecycles.length) return null; return ( diff --git a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx index 3804324619..894b82c92d 100644 --- a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx +++ b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx @@ -99,6 +99,10 @@ export const EntityOwnerPicker = () => { } }, [queryParamOwners, availableOwners]); + useEffect(() => { + if (!availableOwners.length) setSelectedOwners([]); + }, [availableOwners]); + if (!availableOwners.length) return null; return ( From 3e3edaea2701199b985ee79032ba54844c76efe0 Mon Sep 17 00:00:00 2001 From: Sarah Medeiros Date: Thu, 8 Dec 2022 10:15:28 -0500 Subject: [PATCH 4/6] Simplify fix Signed-off-by: Sarah Medeiros --- .../EntityLifecyclePicker.tsx | 29 +++++++++---------- .../EntityOwnerPicker/EntityOwnerPicker.tsx | 29 +++++++++---------- 2 files changed, 26 insertions(+), 32 deletions(-) diff --git a/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.tsx b/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.tsx index c19da47feb..b941f94531 100644 --- a/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.tsx +++ b/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.tsx @@ -67,13 +67,13 @@ export const EntityLifecyclePicker = () => { : 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(() => { - updateFilters({ - lifecycles: selectedLifecycles.length - ? new EntityLifecycleFilter(selectedLifecycles) - : undefined, - }); - }, [selectedLifecycles, updateFilters]); + if (queryParamLifecycles.length) { + setSelectedLifecycles(queryParamLifecycles); + } + }, [queryParamLifecycles]); const availableLifecycles = useMemo( () => @@ -87,17 +87,14 @@ export const EntityLifecyclePicker = () => { [backendEntities], ); - // Set selected lifecycles on query parameter updates; this happens at initial page load and from - // external updates to the page location. useEffect(() => { - if (queryParamLifecycles.length && availableLifecycles.length) { - setSelectedLifecycles(queryParamLifecycles); - } - }, [queryParamLifecycles, availableLifecycles]); - - useEffect(() => { - if (!availableLifecycles.length) setSelectedLifecycles([]); - }, [availableLifecycles]); + updateFilters({ + lifecycles: + selectedLifecycles.length && availableLifecycles.length + ? new EntityLifecycleFilter(selectedLifecycles) + : undefined, + }); + }, [selectedLifecycles, updateFilters, availableLifecycles]); if (!availableLifecycles.length) return null; diff --git a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx index 894b82c92d..b067bda0f5 100644 --- a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx +++ b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.tsx @@ -67,13 +67,13 @@ export const EntityOwnerPicker = () => { 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(() => { - updateFilters({ - owners: selectedOwners.length - ? new EntityOwnerFilter(selectedOwners) - : undefined, - }); - }, [selectedOwners, updateFilters]); + if (queryParamOwners.length) { + setSelectedOwners(queryParamOwners); + } + }, [queryParamOwners]); const availableOwners = useMemo( () => @@ -91,17 +91,14 @@ export const EntityOwnerPicker = () => { [backendEntities], ); - // Set selected owners on query parameter updates; this happens at initial page load and from - // external updates to the page location. useEffect(() => { - if (queryParamOwners.length && availableOwners.length) { - setSelectedOwners(queryParamOwners); - } - }, [queryParamOwners, availableOwners]); - - useEffect(() => { - if (!availableOwners.length) setSelectedOwners([]); - }, [availableOwners]); + updateFilters({ + owners: + selectedOwners.length && availableOwners.length + ? new EntityOwnerFilter(selectedOwners) + : undefined, + }); + }, [selectedOwners, updateFilters, availableOwners]); if (!availableOwners.length) return null; From 5be74dcb5945fc33f373c355e088209562d57843 Mon Sep 17 00:00:00 2001 From: Sarah Medeiros Date: Fri, 9 Dec 2022 14:26:33 -0500 Subject: [PATCH 5/6] fix failing tests Signed-off-by: Sarah Medeiros --- .../EntityLifecyclePicker/EntityLifecyclePicker.test.tsx | 3 +++ .../components/EntityOwnerPicker/EntityOwnerPicker.test.tsx | 2 ++ 2 files changed, 5 insertions(+) diff --git a/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.test.tsx b/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.test.tsx index f925625b23..d1fab823be 100644 --- a/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.test.tsx +++ b/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.test.tsx @@ -169,6 +169,7 @@ describe('', () => { value={{ updateFilters, queryParameters: { lifecycles: ['experimental'] }, + backendEntities: sampleEntities, }} > @@ -182,6 +183,8 @@ describe('', () => { value={{ updateFilters, queryParameters: { lifecycles: ['production'] }, + backendEntities: sampleEntities, + q, }} > diff --git a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx index c816810b10..6eebaa2409 100644 --- a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx +++ b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx @@ -183,6 +183,7 @@ describe('', () => { value={{ updateFilters, queryParameters: { owners: ['team-a'] }, + backendEntities: sampleEntities, }} > @@ -196,6 +197,7 @@ describe('', () => { value={{ updateFilters, queryParameters: { owners: ['team-b'] }, + backendEntities: sampleEntities, }} > From 7a7073f75c70f43a4a9334bfb98c130caffe495e Mon Sep 17 00:00:00 2001 From: Sarah Medeiros Date: Fri, 9 Dec 2022 14:57:00 -0500 Subject: [PATCH 6/6] Add test for new functionality Signed-off-by: Sarah Medeiros --- .../EntityLifecyclePicker.test.tsx | 18 +++++++++++++++++- .../EntityOwnerPicker.test.tsx | 17 +++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.test.tsx b/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.test.tsx index d1fab823be..ef77f7615f 100644 --- a/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.test.tsx +++ b/plugins/catalog-react/src/components/EntityLifecyclePicker/EntityLifecyclePicker.test.tsx @@ -184,7 +184,6 @@ describe('', () => { updateFilters, queryParameters: { lifecycles: ['production'] }, backendEntities: sampleEntities, - q, }} > @@ -194,4 +193,21 @@ describe('', () => { lifecycles: new EntityLifecycleFilter(['production']), }); }); + it('removes lifecycles from filters if there are no available lifecycles', () => { + const updateFilters = jest.fn(); + render( + + + , + ); + expect(updateFilters).toHaveBeenLastCalledWith({ + lifecycles: undefined, + }); + }); }); diff --git a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx index 6eebaa2409..b6e928b519 100644 --- a/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx +++ b/plugins/catalog-react/src/components/EntityOwnerPicker/EntityOwnerPicker.test.tsx @@ -207,4 +207,21 @@ describe('', () => { owners: new EntityOwnerFilter(['team-b']), }); }); + it('removes owners from filters if there are none available', () => { + const updateFilters = jest.fn(); + render( + + + , + ); + expect(updateFilters).toHaveBeenLastCalledWith({ + owners: undefined, + }); + }); });