From 300f8cdaee54fc40fc94c493e0c1a40cdd50fcfe Mon Sep 17 00:00:00 2001 From: MT Lewis Date: Fri, 14 Jan 2022 13:35:24 +0000 Subject: [PATCH] catalog-react: fix filter resetting bug in UserListPicker Signed-off-by: MT Lewis --- .changeset/itchy-bulldogs-dance.md | 5 ++ .../UserListPicker/UserListPicker.test.tsx | 83 ++++++++++++++++++- .../UserListPicker/UserListPicker.tsx | 44 ++++++---- 3 files changed, 113 insertions(+), 19 deletions(-) create mode 100644 .changeset/itchy-bulldogs-dance.md diff --git a/.changeset/itchy-bulldogs-dance.md b/.changeset/itchy-bulldogs-dance.md new file mode 100644 index 0000000000..a5123c0c27 --- /dev/null +++ b/.changeset/itchy-bulldogs-dance.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-react': patch +--- + +Fix bug with filter resetting based on user ownership diff --git a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx index f6a4c59374..a032d8fc46 100644 --- a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx +++ b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.test.tsx @@ -69,8 +69,9 @@ const apis = TestApiRegistry.from( [storageApiRef, MockStorageApi.create()], ); -const mockIsOwnedEntity = (entity: Entity) => - entity.metadata.name === 'component-1'; +const mockIsOwnedEntity = jest.fn( + (entity: Entity) => entity.metadata.name === 'component-1', +); const mockIsStarredEntity = (entity: Entity) => entity.metadata.name === 'component-3'; @@ -250,4 +251,82 @@ describe('', () => { ), }); }); + + describe('filter resetting', () => { + let updateFilters: jest.Mock; + + const picker = ({ loading }: { loading: boolean }) => ( + + + + + + ); + + beforeEach(() => { + updateFilters = jest.fn(); + }); + + describe('when the user does not own any entities', () => { + beforeEach(() => { + mockIsOwnedEntity.mockReturnValue(false); + }); + + it('does not reset the filter while entities are loading', () => { + render(picker({ loading: true })); + + expect(updateFilters).not.toHaveBeenCalledWith({ + user: new UserListFilter( + 'all', + mockIsOwnedEntity, + mockIsStarredEntity, + ), + }); + }); + + it('resets the filter to "all" when entities are loaded', () => { + render(picker({ loading: false })); + + expect(updateFilters).toHaveBeenLastCalledWith({ + user: new UserListFilter( + 'all', + mockIsOwnedEntity, + mockIsStarredEntity, + ), + }); + }); + }); + + describe('when the user owns entities', () => { + beforeEach(() => { + mockIsOwnedEntity.mockReturnValue(true); + }); + + it('does not reset the filter while entities are loading', () => { + render(picker({ loading: true })); + + expect(updateFilters).not.toHaveBeenCalledWith({ + user: new UserListFilter( + 'all', + mockIsOwnedEntity, + mockIsStarredEntity, + ), + }); + }); + + it('does not reset the filter when entities are loaded', () => { + render(picker({ loading: false })); + + expect(updateFilters).toHaveBeenLastCalledWith({ + user: new UserListFilter( + 'owned', + mockIsOwnedEntity, + mockIsStarredEntity, + ), + }); + }); + }); + }); }); diff --git a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx index 8746752b08..dcad74f760 100644 --- a/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx +++ b/plugins/catalog-react/src/components/UserListPicker/UserListPicker.tsx @@ -130,7 +130,7 @@ export const UserListPicker = ({ const classes = useStyles(); const configApi = useApi(configApiRef); const orgName = configApi.getOptionalString('organization.name') ?? 'Company'; - const { filters, updateFilters, backendEntities, queryParameters } = + const { filters, updateFilters, backendEntities, queryParameters, loading } = useEntityListProvider(); // Remove group items that aren't in availableFilters and exclude @@ -161,19 +161,36 @@ export const UserListPicker = ({ [isOwnedEntity, isStarredEntity], ); + const [selectedUserFilter, setSelectedUserFilter] = useState( + [queryParameters.user].flat()[0] ?? initialFilter, + ); + // To show proper counts for each section, apply all other frontend filters _except_ the user // filter that's controlled by this picker. - const [entitiesWithoutUserFilter, setEntitiesWithoutUserFilter] = - useState(backendEntities); - const totalOwnedUserEntities = entitiesWithoutUserFilter.filter(entity => - ownedFilter.filterEntity(entity), - ).length; - const [selectedUserFilter, setSelectedUserFilter] = useState( - totalOwnedUserEntities > 0 - ? [queryParameters.user].flat()[0] ?? initialFilter - : 'all', + const entitiesWithoutUserFilter = useMemo( + () => + backendEntities.filter( + reduceEntityFilters( + compact(Object.values({ ...filters, user: undefined })), + ), + ), + [filters, backendEntities], ); + const totalOwnedUserEntities = useMemo( + () => + entitiesWithoutUserFilter.filter(entity => + ownedFilter.filterEntity(entity), + ).length, + [entitiesWithoutUserFilter, ownedFilter], + ); + + useEffect(() => { + if (!loading && totalOwnedUserEntities === 0) { + setSelectedUserFilter('all'); + } + }, [loading, totalOwnedUserEntities, setSelectedUserFilter]); + useEffect(() => { updateFilters({ user: selectedUserFilter @@ -186,13 +203,6 @@ export const UserListPicker = ({ }); }, [selectedUserFilter, isOwnedEntity, isStarredEntity, updateFilters]); - useEffect(() => { - const filterFn = reduceEntityFilters( - compact(Object.values({ ...filters, user: undefined })), - ); - setEntitiesWithoutUserFilter(backendEntities.filter(filterFn)); - }, [filters, backendEntities]); - function getFilterCount(id: UserListFilterKind) { switch (id) { case 'owned':