fix(#20232): handle intermediate groups

Signed-off-by: vdizengremel <victor.dizengremel@octo.com>

plugins/org/package.json

@@ -39,6 +39,7 @@
     "@material-ui/icons": "^4.9.1",
     "@material-ui/lab": "4.0.0-alpha.61",
     "@types/react": "^16.13.1 || ^17.0.0",
+    "lodash": "^4.17.21",
     "p-limit": "^3.1.0",
     "pluralize": "^8.0.0",
     "qs": "^6.10.1",

plugins/org/src/components/Cards/OwnershipCard/useGetEntities.test.ts

@@ -22,18 +22,8 @@ import { getEntityRelations } from '@backstage/plugin-catalog-react';
 const givenParentGroup = 'team.squad1';
 const givenLeafGroup = 'team.squad2';
 const givenUser = 'user.john';
-const givenParentGroupEntity = {
-  kind: 'Group',
-  metadata: {
-    name: givenParentGroup,
-  },
-} as Partial<Entity> as Entity;
-const givenLeafGroupEntity = {
-  kind: 'Group',
-  metadata: {
-    name: givenLeafGroup,
-  },
-} as Partial<Entity> as Entity;
+const givenParentGroupEntity = createGroupEntityFromName(givenParentGroup);
+const givenLeafGroupEntity = createGroupEntityFromName(givenLeafGroup);
 const givenUserEntity = {
   kind: 'User',
   metadata: {
@@ -41,13 +31,10 @@ const givenUserEntity = {
   },
 } as Partial<Entity> as Entity;
 
+const getEntitiesByRefsMock = jest.fn();
 const catalogApiMock: Pick<CatalogApi, 'getEntities' | 'getEntitiesByRefs'> = {
   getEntities: jest.fn(async () => Promise.resolve({ items: [] })),
-  getEntitiesByRefs: jest.fn(async ({ entityRefs: [ref] }) =>
-    ref.includes(givenParentGroup)
-      ? { items: [givenParentGroupEntity] }
-      : { items: [givenLeafGroupEntity] },
-  ),
+  getEntitiesByRefs: getEntitiesByRefsMock,
 };
 
 jest.mock('@backstage/core-plugin-api', () => ({
@@ -72,7 +59,7 @@ describe('useGetEntities', () => {
     expect.objectContaining({
       filter: expect.arrayContaining([
         expect.objectContaining({
-          'relations.ownedBy': owners,
+          'relations.ownedBy': expect.arrayContaining(owners),
         }),
       ]),
     });
@@ -89,8 +76,17 @@ describe('useGetEntities', () => {
       await hook.waitForNextUpdate();
     };
 
+    beforeEach(() => {
+      getEntitiesByRefsMock.mockImplementation(async ({ entityRefs: [ref] }) =>
+        ref.includes(givenParentGroup)
+          ? { items: [givenParentGroupEntity] }
+          : { items: [givenLeafGroupEntity] },
+      );
+    });
+
     afterEach(() => {
       getEntityRelationsMock.mockRestore();
+      getEntitiesByRefsMock.mockRestore();
     });
 
     describe('when given entity is a group', () => {
@@ -123,6 +119,62 @@ describe('useGetEntities', () => {
           fields: ['kind', 'metadata.namespace', 'metadata.name', 'relations'],
         });
       });
+
+      it('should retrieve entities owned by any children', async () => {
+        const givenIntermediateGroup = 'intermediate-group';
+        const givenIntermediateGroupEntity = createGroupEntityFromName(
+          givenIntermediateGroup,
+        );
+
+        getEntitiesByRefsMock.mockRestore();
+        getEntitiesByRefsMock.mockImplementation(
+          async ({ entityRefs: [ref] }) => {
+            if (ref.includes(givenParentGroup)) {
+              return { items: [givenParentGroupEntity] };
+            }
+
+            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 [];
+        });
+
+        await whenHookIsCalledWith(givenParentGroupEntity);
+        expect(catalogApiMock.getEntities).toHaveBeenCalledWith(
+          ownersFilter(
+            `group:default/${givenParentGroup}`,
+            `group:default/${givenIntermediateGroup}`,
+            `group:default/${givenLeafGroup}`,
+          ),
+        );
+      });
     });
 
     describe('when given entity is a user', () => {
@@ -173,3 +225,12 @@ describe('useGetEntities', () => {
     });
   });
 });
+
+function createGroupEntityFromName(name: String): Entity {
+  return {
+    kind: 'Group',
+    metadata: {
+      name: name,
+    },
+  } as Partial<Entity> as Entity;
+}

plugins/org/src/components/Cards/OwnershipCard/useGetEntities.ts

@@ -16,9 +16,9 @@
 
 import {
   Entity,
+  parseEntityRef,
   RELATION_MEMBER_OF,
   RELATION_PARENT_OF,
-  parseEntityRef,
   stringifyEntityRef,
 } from '@backstage/catalog-model';
 import {
@@ -32,6 +32,7 @@ import { useApi } from '@backstage/core-plugin-api';
 import useAsync from 'react-use/lib/useAsync';
 import qs from 'qs';
 import { EntityRelationAggregation as EntityRelationsAggregation } from './types';
+import { uniq } from 'lodash';
 
 const limiter = limiterFactory(10);
 
@@ -95,7 +96,7 @@ const getChildOwnershipEntityRefs = async (
     });
     const childGroupEntities = childGroupResponse.items.filter(isEntity);
 
-    return (
+    const childrenRefs = (
       await Promise.all(
         childGroupEntities.map(childGroupEntity =>
           limiter(() =>
@@ -104,6 +105,8 @@ const getChildOwnershipEntityRefs = async (
         ),
       )
     ).flatMap(aggregated => aggregated);
+
+    return uniq([...childrenRefs, stringifyEntityRef(entity)]);
   }
 
   return [stringifyEntityRef(entity)];
@@ -118,23 +121,15 @@ const getOwners = async (
   const isAggregated = relations === 'aggregated';
   const isUserEntity = entity.kind === 'User';
 
-  const owners: string[] = [];
-
   if (isAggregated && isGroup) {
-    const childEntityRefs = await getChildOwnershipEntityRefs(
-      entity,
-      catalogApi,
-    );
-    owners.push(stringifyEntityRef(entity));
-    owners.push.apply(owners, childEntityRefs);
-  } else if (isAggregated && isUserEntity) {
-    const parentEntityRefs = getMemberOfEntityRefs(entity);
-    owners.push.apply(owners, parentEntityRefs);
-  } else {
-    owners.push(stringifyEntityRef(entity));
+    return getChildOwnershipEntityRefs(entity, catalogApi);
   }
 
-  return owners;
+  if (isAggregated && isUserEntity) {
+    return getMemberOfEntityRefs(entity);
+  }
+
+  return [stringifyEntityRef(entity)];
 };
 
 const getOwnedEntitiesByOwners = (

yarn.lock

@@ -8139,6 +8139,7 @@ __metadata:
     "@testing-library/react-hooks": ^8.0.1
     "@testing-library/user-event": ^14.0.0
     "@types/react": ^16.13.1 || ^17.0.0
+    lodash: ^4.17.21
     msw: ^1.0.0
     p-limit: ^3.1.0
     pluralize: ^8.0.0