Merge pull request #18540 from callebstrom/improve-entity-relations-toggle

Improve relationship entity toggle

.changeset/healthy-spiders-arrive.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-org': patch
+---
+
+Ensure direct relations are shown for User entities while keeping support for aggregating closest parent group ownership

plugins/org/api-report.md

@@ -30,10 +30,13 @@ export const EntityOwnershipCard: (props: {
   variant?: InfoCardVariants | undefined;
   entityFilterKind?: string[] | undefined;
   hideRelationsToggle?: boolean | undefined;
-  relationsType?: string | undefined;
+  relationsType?: EntityRelationAggregation | undefined;
   entityLimit?: number | undefined;
 }) => JSX_2.Element;
 
+// @public (undocumented)
+export type EntityRelationAggregation = 'direct' | 'aggregated';
+
 // @public (undocumented)
 export const EntityUserProfileCard: (props: {
   variant?: InfoCardVariants | undefined;
@@ -77,7 +80,7 @@ export const OwnershipCard: (props: {
   variant?: InfoCardVariants;
   entityFilterKind?: string[];
   hideRelationsToggle?: boolean;
-  relationsType?: string;
+  relationsType?: EntityRelationAggregation;
   entityLimit?: number;
 }) => React_2.JSX.Element;
 

plugins/org/package.json

@@ -60,6 +60,7 @@
     "@testing-library/dom": "^8.0.0",
     "@testing-library/jest-dom": "^5.10.1",
     "@testing-library/react": "^12.1.3",
+    "@testing-library/react-hooks": "^8.0.1",
     "@testing-library/user-event": "^14.0.0",
     "@types/node": "^16.11.26",
     "msw": "^1.0.0"

plugins/org/src/components/Cards/OwnershipCard/ComponentsGrid.tsx

@@ -34,6 +34,7 @@ import React from 'react';
 import pluralize from 'pluralize';
 import { catalogIndexRouteRef } from '../../../routes';
 import { useGetEntities } from './useGetEntities';
+import { EntityRelationAggregation } from './types';
 
 const useStyles = makeStyles((theme: BackstageTheme) =>
   createStyles({
@@ -109,13 +110,11 @@ const EntityCountTile = ({
 export const ComponentsGrid = ({
   entity,
   relationsType,
-  isGroup,
   entityFilterKind,
   entityLimit = 6,
 }: {
   entity: Entity;
-  relationsType: string;
-  isGroup: boolean;
+  relationsType: EntityRelationAggregation;
   entityFilterKind?: string[];
   entityLimit?: number;
 }) => {
@@ -123,7 +122,6 @@ export const ComponentsGrid = ({
   const { componentsWithCounters, loading, error } = useGetEntities(
     entity,
     relationsType,
-    isGroup,
     entityFilterKind,
     entityLimit,
   );

plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.test.tsx

@@ -263,10 +263,11 @@ describe('OwnershipCard', () => {
       },
     );
 
-    expect(getByText('OPENAPI').closest('a')).toHaveAttribute(
-      'href',
-      '/create/?filters%5Bkind%5D=api&filters%5Btype%5D=openapi&filters%5Bowners%5D=my-team&filters%5Buser%5D=all',
-    );
+    const href = getByText('OPENAPI').closest('a')?.href ?? '';
+    // This env does not support URLSearchParams
+    const queryParams = decodeURIComponent(href);
+
+    expect(queryParams).toContain('filters[owners]=my-team');
   });
 
   it('links to the catalog with the user and groups filters from an user profile', async () => {
@@ -299,7 +300,7 @@ describe('OwnershipCard', () => {
     const { getByText } = await renderInTestApp(
       <TestApiProvider apis={[[catalogApiRef, catalogApi]]}>
         <EntityProvider entity={userEntity}>
-          <OwnershipCard />
+          <OwnershipCard relationsType="aggregated" />
         </EntityProvider>
       </TestApiProvider>,
       {
@@ -309,9 +310,11 @@ describe('OwnershipCard', () => {
       },
     );
 
-    expect(getByText('OPENAPI').closest('a')).toHaveAttribute(
-      'href',
-      '/create/?filters%5Bkind%5D=api&filters%5Btype%5D=openapi&filters%5Bowners%5D=user%3Athe-user&filters%5Bowners%5D=my-team&filters%5Bowners%5D=custom%2Fsome-team&filters%5Buser%5D=all',
+    const href = getByText('OPENAPI').closest('a')?.href ?? '';
+    // This env does not support URLSearchParams
+    const queryParams = decodeURIComponent(href);
+    expect(queryParams).toMatch(
+      /filters\[owners\]=custom\/some\-team.*filters\[owners\]=user:the-user/,
     );
   });
 

plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.tsx

@@ -27,6 +27,7 @@ import {
 } from '@material-ui/core';
 import React, { useState } from 'react';
 import { ComponentsGrid } from './ComponentsGrid';
+import { EntityRelationAggregation } from './types';
 
 const useStyles = makeStyles(theme => ({
   list: {
@@ -56,7 +57,7 @@ export const OwnershipCard = (props: {
   variant?: InfoCardVariants;
   entityFilterKind?: string[];
   hideRelationsToggle?: boolean;
-  relationsType?: string;
+  relationsType?: EntityRelationAggregation;
   entityLimit?: number;
 }) => {
   const {
@@ -70,7 +71,6 @@ export const OwnershipCard = (props: {
     hideRelationsToggle === undefined ? false : hideRelationsToggle;
   const classes = useStyles();
   const { entity } = useEntity();
-  const isGroup = entity.kind === 'Group';
   const [getRelationsType, setRelationsType] = useState(
     relationsType || 'direct',
   );
@@ -102,7 +102,6 @@ export const OwnershipCard = (props: {
                   }
                   name="pin"
                   inputProps={{ 'aria-label': 'Ownership Type Switch' }}
-                  disabled={!isGroup}
                 />
               </Tooltip>
               Aggregated Relations
@@ -114,7 +113,6 @@ export const OwnershipCard = (props: {
         entity={entity}
         entityLimit={entityLimit}
         relationsType={getRelationsType}
-        isGroup={isGroup}
         entityFilterKind={entityFilterKind}
       />
     </InfoCard>

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

@@ -14,3 +14,4 @@
  * limitations under the License.
  */
 export * from './OwnershipCard';
+export * from './types';

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

@@ -0,0 +1,18 @@
+/*
+ * Copyright 2023 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/** @public */
+export type EntityRelationAggregation = 'direct' | 'aggregated';

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

@@ -0,0 +1,150 @@
+/*
+ * Copyright 2023 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import { CompoundEntityRef, Entity } from '@backstage/catalog-model';
+import { useGetEntities } from './useGetEntities';
+import { CatalogApi } from '@backstage/catalog-client';
+import { renderHook } from '@testing-library/react-hooks';
+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 givenUserEntity = {
+  kind: 'User',
+  metadata: {
+    name: givenUser,
+  },
+} as Partial<Entity> as Entity;
+
+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] },
+  ),
+};
+
+jest.mock('@backstage/core-plugin-api', () => ({
+  useApi: jest.fn(() => catalogApiMock),
+}));
+jest.mock('@backstage/plugin-catalog-react', () => ({
+  catalogApiRef: {},
+  getEntityRelations: jest.fn(entity => {
+    if (entity?.metadata.name === givenParentGroup) {
+      return [
+        {
+          kind: 'Group',
+          namespace: 'default',
+          name: givenLeafGroup,
+        } as CompoundEntityRef,
+      ];
+    } else if (entity?.kind === 'User') {
+      return [
+        {
+          kind: 'Group',
+          namespace: 'default',
+          name: givenLeafGroup,
+        } as CompoundEntityRef,
+      ];
+    }
+
+    return [];
+  }) as typeof getEntityRelations,
+}));
+
+describe('useGetEntities', () => {
+  const ownersFilter = (...owners: string[]) =>
+    expect.objectContaining({
+      filter: expect.arrayContaining([
+        expect.objectContaining({
+          'relations.ownedBy': owners,
+        }),
+      ]),
+    });
+
+  describe('given aggregated relationsType', () => {
+    const whenHookIsCalledWith = async (_entity: Entity) => {
+      const hook = renderHook(
+        ({ entity }) => useGetEntities(entity, 'aggregated'),
+        {
+          initialProps: { entity: _entity },
+        },
+      );
+
+      await hook.waitForNextUpdate();
+    };
+
+    it('given group entity should aggregate child ownership', async () => {
+      await whenHookIsCalledWith(givenParentGroupEntity);
+      expect(catalogApiMock.getEntities).toHaveBeenCalledWith(
+        ownersFilter(
+          `group:default/${givenParentGroup}`,
+          `group:default/${givenLeafGroup}`,
+        ),
+      );
+    });
+
+    it('given user entity should aggregate parent ownership and direct', async () => {
+      await whenHookIsCalledWith(givenUserEntity);
+      expect(catalogApiMock.getEntities).toHaveBeenCalledWith(
+        ownersFilter(
+          `group:default/${givenLeafGroup}`,
+          `user:default/${givenUser}`,
+        ),
+      );
+    });
+  });
+
+  describe('given direct relationsType', () => {
+    const whenHookIsCalledWith = async (_entity: Entity) => {
+      const hook = renderHook(
+        ({ entity }) => useGetEntities(entity, 'direct'),
+        {
+          initialProps: { entity: _entity },
+        },
+      );
+
+      await hook.waitForNextUpdate();
+    };
+
+    it('given group entity should return directly owned entities', async () => {
+      await whenHookIsCalledWith(givenLeafGroupEntity);
+      expect(catalogApiMock.getEntities).toHaveBeenCalledWith(
+        ownersFilter(`group:default/${givenLeafGroup}`),
+      );
+    });
+
+    it('given user entity should return directly owned entities', async () => {
+      await whenHookIsCalledWith(givenUserEntity);
+      expect(catalogApiMock.getEntities).toHaveBeenCalledWith(
+        ownersFilter(`user:default/${givenUser}`),
+      );
+    });
+  });
+});

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

@@ -31,6 +31,7 @@ import limiterFactory from 'p-limit';
 import { useApi } from '@backstage/core-plugin-api';
 import useAsync from 'react-use/lib/useAsync';
 import qs from 'qs';
+import { EntityRelationAggregation as EntityRelationsAggregation } from './types';
 
 const limiter = limiterFactory(10);
 
@@ -57,78 +58,109 @@ const getQueryParams = (
   return qs.stringify({ filters }, { arrayFormat: 'repeat' });
 };
 
-const getOwnersEntityRef = (owner: Entity): string[] => {
-  let owners = [stringifyEntityRef(owner)];
-  if (owner.kind === 'User') {
-    const ownerGroups = getEntityRelations(owner, RELATION_MEMBER_OF, {
-      kind: 'Group',
+const getMemberOfEntityRefs = (owner: Entity): string[] => {
+  const parentGroups = getEntityRelations(owner, RELATION_MEMBER_OF, {
+    kind: 'Group',
+  });
+
+  const ownerGroupsNames = parentGroups.map(({ kind, namespace, name }) =>
+    stringifyEntityRef({
+      kind,
+      namespace,
+      name,
+    }),
+  );
+
+  return [...ownerGroupsNames, stringifyEntityRef(owner)];
+};
+
+const isEntity = (entity: Entity | undefined): entity is Entity =>
+  entity !== undefined;
+
+const getChildOwnershipEntityRefs = async (
+  entity: Entity,
+  catalogApi: CatalogApi,
+): Promise<string[]> => {
+  const childGroups = getEntityRelations(entity, RELATION_PARENT_OF, {
+    kind: 'Group',
+  });
+
+  const hasChildGroups = childGroups.length > 0;
+
+  if (hasChildGroups) {
+    const entityRefs = childGroups.map(r => stringifyEntityRef(r));
+    const childGroupResponse = await catalogApi.getEntitiesByRefs({
+      fields: ['kind', 'metadata.namespace', 'metadata.name'],
+      entityRefs,
     });
-    const ownerGroupsName = ownerGroups.map(ownerGroup =>
-      stringifyEntityRef({
-        kind: ownerGroup.kind,
-        namespace: ownerGroup.namespace,
-        name: ownerGroup.name,
-      }),
-    );
-    owners = [...owners, ...ownerGroupsName];
+    const childGroupEntities = childGroupResponse.items.filter(isEntity);
+
+    return (
+      await Promise.all(
+        childGroupEntities.map(childGroupEntity =>
+          limiter(() =>
+            getChildOwnershipEntityRefs(childGroupEntity, catalogApi),
+          ),
+        ),
+      )
+    ).flatMap(aggregated => aggregated);
   }
+
+  return [stringifyEntityRef(entity)];
+};
+
+const getOwners = async (
+  entity: Entity,
+  relations: EntityRelationsAggregation,
+  catalogApi: CatalogApi,
+): Promise<string[]> => {
+  const isGroup = entity.kind === 'Group';
+  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 owners;
 };
 
-const getAggregatedOwnersEntityRef = async (
-  parentGroup: Entity,
+const getOwnedEntitiesByOwners = (
+  owners: string[],
+  kinds: string[],
   catalogApi: CatalogApi,
-): Promise<string[]> => {
-  const requestedEntities: Entity[] = [];
-  const outstandingEntities = new Map<string, Promise<Entity | undefined>>();
-  const processedEntities = new Set<string>();
-  requestedEntities.push(parentGroup);
-  let currentEntity = parentGroup;
-
-  while (requestedEntities.length > 0) {
-    const childRelations = getEntityRelations(
-      currentEntity,
-      RELATION_PARENT_OF,
+) =>
+  catalogApi.getEntities({
+    filter: [
       {
-        kind: 'Group',
+        kind: kinds,
+        'relations.ownedBy': owners,
       },
-    );
-
-    await Promise.all(
-      childRelations.map(childGroup =>
-        limiter(async () => {
-          const promise = catalogApi.getEntityByRef(childGroup);
-          outstandingEntities.set(childGroup.name, promise);
-          try {
-            const processedEntity = await promise;
-            if (processedEntity) {
-              requestedEntities.push(processedEntity);
-            }
-          } finally {
-            outstandingEntities.delete(childGroup.name);
-          }
-        }),
-      ),
-    );
-    requestedEntities.shift();
-    processedEntities.add(
-      stringifyEntityRef({
-        kind: currentEntity.kind,
-        namespace: currentEntity.metadata.namespace,
-        name: currentEntity.metadata.name,
-      }),
-    );
-    // always set currentEntity to the first element of array requestedEntities
-    currentEntity = requestedEntities[0];
-  }
-
-  return Array.from(processedEntities);
-};
+    ],
+    fields: [
+      'kind',
+      'metadata.name',
+      'metadata.namespace',
+      'spec.type',
+      'relations',
+    ],
+  });
 
 export function useGetEntities(
   entity: Entity,
-  relationsType: string,
-  isGroup: boolean,
+  relations: EntityRelationsAggregation,
   entityFilterKind?: string[],
   entityLimit = 6,
 ): {
@@ -151,25 +183,13 @@ export function useGetEntities(
     error,
     value: componentsWithCounters,
   } = useAsync(async () => {
-    const owners =
-      relationsType === 'aggregated' && isGroup
-        ? await getAggregatedOwnersEntityRef(entity, catalogApi)
-        : getOwnersEntityRef(entity);
-    const ownedEntitiesList = await catalogApi.getEntities({
-      filter: [
-        {
-          kind: kinds,
-          'relations.ownedBy': owners,
-        },
-      ],
-      fields: [
-        'kind',
-        'metadata.name',
-        'metadata.namespace',
-        'spec.type',
-        'relations',
-      ],
-    });
+    const owners = await getOwners(entity, relations, catalogApi);
+
+    const ownedEntitiesList = await getOwnedEntitiesByOwners(
+      owners,
+      kinds,
+      catalogApi,
+    );
 
     const counts = ownedEntitiesList.items.reduce(
       (acc: EntityTypeProps[], ownedEntity) => {
@@ -204,7 +224,7 @@ export function useGetEntities(
       kind: string;
       queryParams: string;
     }>;
-  }, [catalogApi, entity, relationsType]);
+  }, [catalogApi, entity, relations]);
 
   return {
     componentsWithCounters,

plugins/org/src/setupTests.ts

@@ -14,3 +14,7 @@
  * limitations under the License.
  */
 import '@testing-library/jest-dom';
+
+beforeEach(() => {
+  jest.clearAllMocks();
+});

yarn.lock

@@ -7918,6 +7918,7 @@ __metadata:
     "@testing-library/dom": ^8.0.0
     "@testing-library/jest-dom": ^5.10.1
     "@testing-library/react": ^12.1.3
+    "@testing-library/react-hooks": ^8.0.1
     "@testing-library/user-event": ^14.0.0
     "@types/node": ^16.11.26
     "@types/react": ^16.13.1 || ^17.0.0