diff --git a/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.test.ts b/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.test.ts index 189aa4db30..587dc689e3 100644 --- a/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.test.ts +++ b/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.test.ts @@ -92,13 +92,7 @@ describe('useGetEntities', () => { describe('when given entity is a group', () => { beforeEach(() => { getEntityRelationsMock - .mockReturnValueOnce([ - { - kind: 'Group', - namespace: 'default', - name: givenLeafGroup, - } as CompoundEntityRef, - ]) + .mockReturnValueOnce([createGroupRefFromName(givenLeafGroup)]) .mockReturnValue([]); }); @@ -120,71 +114,84 @@ describe('useGetEntities', () => { }); }); - it('should retrieve entities owned by any children', async () => { + describe('when relations are deep (children of children)', () => { const givenIntermediateGroup = 'intermediate-group'; const givenIntermediateGroupEntity = createGroupEntityFromName( givenIntermediateGroup, ); - getEntitiesByRefsMock.mockRestore(); - getEntitiesByRefsMock.mockImplementation( - async ({ entityRefs: [ref] }) => { - if (ref.includes(givenParentGroup)) { - return { items: [givenParentGroupEntity] }; - } + beforeEach(() => { + getEntitiesByRefsMock.mockRestore(); + getEntitiesByRefsMock.mockImplementation( + async ({ entityRefs: [ref] }) => { + if (ref.includes(givenParentGroup)) { + return { items: [givenParentGroupEntity] }; + } - if (ref.includes(givenIntermediateGroup)) { - return { items: [givenIntermediateGroupEntity] }; - } + if (ref.includes(givenIntermediateGroup)) { + return { items: [givenIntermediateGroupEntity] }; + } - return { items: [givenLeafGroupEntity] }; - }, - ); - - getEntityRelationsMock.mockRestore(); - getEntityRelationsMock.mockImplementation(entity => { - if (entity?.metadata.name === givenParentGroup) { - return [ - { - kind: 'Group', - namespace: 'default', - name: givenIntermediateGroup, - }, - ]; - } - - if (entity?.metadata.name === givenIntermediateGroup) { - return [ - { - kind: 'Group', - namespace: 'default', - name: givenLeafGroup, - }, - ]; - } - - return []; + return { items: [givenLeafGroupEntity] }; + }, + ); }); - await whenHookIsCalledWith(givenParentGroupEntity); - expect(catalogApiMock.getEntities).toHaveBeenCalledWith( - ownersFilter( - `group:default/${givenParentGroup}`, - `group:default/${givenIntermediateGroup}`, - `group:default/${givenLeafGroup}`, - ), - ); + it('should retrieve entities owned by any children', async () => { + getEntityRelationsMock.mockRestore(); + getEntityRelationsMock.mockImplementation(entity => { + if (entity?.metadata.name === givenParentGroup) { + return [createGroupRefFromName(givenIntermediateGroup)]; + } + + if (entity?.metadata.name === givenIntermediateGroup) { + return [createGroupRefFromName(givenLeafGroup)]; + } + + return []; + }); + + await whenHookIsCalledWith(givenParentGroupEntity); + expect(catalogApiMock.getEntities).toHaveBeenCalledWith( + ownersFilter( + `group:default/${givenParentGroup}`, + `group:default/${givenIntermediateGroup}`, + `group:default/${givenLeafGroup}`, + ), + ); + }); + + it('should retrieve entities owned by any children when circular relation', async () => { + getEntityRelationsMock.mockRestore(); + getEntityRelationsMock.mockImplementation(entity => { + if (entity?.metadata.name === givenParentGroup) { + return [createGroupRefFromName(givenIntermediateGroup)]; + } + + if (entity?.metadata.name === givenIntermediateGroup) { + return [createGroupRefFromName(givenLeafGroup)]; + } + + // returns parent by default so givenLeafGroup will have the givenParentGroup as child + return [createGroupRefFromName(givenParentGroup)]; + }); + + await whenHookIsCalledWith(givenParentGroupEntity); + expect(catalogApiMock.getEntities).toHaveBeenCalledWith( + ownersFilter( + `group:default/${givenParentGroup}`, + `group:default/${givenIntermediateGroup}`, + `group:default/${givenLeafGroup}`, + ), + ); + }); }); }); describe('when given entity is a user', () => { it('should aggregate parent ownership and direct', async () => { getEntityRelationsMock.mockReturnValue([ - { - kind: 'Group', - namespace: 'default', - name: givenLeafGroup, - } as CompoundEntityRef, + createGroupRefFromName(givenLeafGroup), ]); await whenHookIsCalledWith(givenUserEntity); @@ -226,7 +233,7 @@ describe('useGetEntities', () => { }); }); -function createGroupEntityFromName(name: String): Entity { +function createGroupEntityFromName(name: string): Entity { return { kind: 'Group', metadata: { @@ -234,3 +241,11 @@ function createGroupEntityFromName(name: String): Entity { }, } as Partial as Entity; } + +function createGroupRefFromName(name: string): CompoundEntityRef { + return { + kind: 'Group', + namespace: 'default', + name: name, + }; +} diff --git a/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.ts b/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.ts index cc2bb78a23..b6fa9cbfd1 100644 --- a/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.ts +++ b/plugins/org/src/components/Cards/OwnershipCard/useGetEntities.ts @@ -81,6 +81,7 @@ const isEntity = (entity: Entity | undefined): entity is Entity => const getChildOwnershipEntityRefs = async ( entity: Entity, catalogApi: CatalogApi, + alreadyRetrievedParentRefs: string[] = [], ): Promise => { const childGroups = getEntityRelations(entity, RELATION_PARENT_OF, { kind: 'Group', @@ -88,6 +89,7 @@ const getChildOwnershipEntityRefs = async ( const hasChildGroups = childGroups.length > 0; + const entityRef = stringifyEntityRef(entity); if (hasChildGroups) { const entityRefs = childGroups.map(r => stringifyEntityRef(r)); const childGroupResponse = await catalogApi.getEntitiesByRefs({ @@ -96,20 +98,29 @@ const getChildOwnershipEntityRefs = async ( }); const childGroupEntities = childGroupResponse.items.filter(isEntity); + const unknownChildren = childGroupEntities.filter( + childGroupEntity => + !alreadyRetrievedParentRefs.includes( + stringifyEntityRef(childGroupEntity), + ), + ); const childrenRefs = ( await Promise.all( - childGroupEntities.map(childGroupEntity => + unknownChildren.map(childGroupEntity => limiter(() => - getChildOwnershipEntityRefs(childGroupEntity, catalogApi), + getChildOwnershipEntityRefs(childGroupEntity, catalogApi, [ + ...alreadyRetrievedParentRefs, + entityRef, + ]), ), ), ) ).flatMap(aggregated => aggregated); - return uniq([...childrenRefs, stringifyEntityRef(entity)]); + return uniq([...childrenRefs, entityRef]); } - return [stringifyEntityRef(entity)]; + return [entityRef]; }; const getOwners = async (