fix(org): improve relationship entity toggle

Signed-off-by: Carl-Erik Bergström <cbergstrom@spotify.com>

.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/package.json

@@ -59,6 +59,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",
     "@types/react": "^16.13.1 || ^17.0.0",

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

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

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

@@ -299,7 +299,7 @@ describe('OwnershipCard', () => {
     const { getByText } = await renderInTestApp(
       <TestApiProvider apis={[[catalogApiRef, catalogApi]]}>
         <EntityProvider entity={userEntity}>
-          <OwnershipCard />
+          <OwnershipCard relationsType="aggregated" />
         </EntityProvider>
       </TestApiProvider>,
       {

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 { AnyRelationsType, DefaultRelationType } from './useGetEntities';
 
 const useStyles = makeStyles(theme => ({
   list: {
@@ -56,7 +57,7 @@ export const OwnershipCard = (props: {
   variant?: InfoCardVariants;
   entityFilterKind?: string[];
   hideRelationsToggle?: boolean;
-  relationsType?: string;
+  relationsType?: AnyRelationsType;
   entityLimit?: number;
 }) => {
   const {
@@ -70,9 +71,8 @@ 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',
+    relationsType || DefaultRelationType.Direct,
   );
 
   return (
@@ -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/useGetEntities.test.ts

@@ -0,0 +1,65 @@
+/*
+ * 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 { Entity } from '@backstage/catalog-model';
+import { useGetEntities } from './useGetEntities';
+import { CatalogApi } from '@backstage/catalog-client';
+import { renderHook } from '@testing-library/react-hooks';
+
+const catalogApiMock: Pick<CatalogApi, 'getEntities'> = {
+  getEntities: jest.fn(async () => {
+    return Promise.resolve({ items: [] });
+  }),
+};
+
+jest.mock('@backstage/core-plugin-api', () => ({
+  useApi: jest.fn(() => catalogApiMock),
+}));
+jest.mock('@backstage/plugin-catalog-react', () => ({
+  catalogApiRef: {},
+  getEntityRelations: jest.fn(() => []),
+}));
+
+describe('useGetEntities', () => {
+  describe('given aggregated relationsType', () => {
+    it('when entity is group should aggregate child ownership', async () => {
+      const givenSquad = 'team.squad1';
+      const whenEntity = {
+        kind: 'Group',
+        metadata: {
+          name: givenSquad,
+        },
+      } as Partial<Entity> as Entity;
+
+      const hook = renderHook(
+        ({ entity }) => useGetEntities(entity, 'aggregated'),
+        {
+          initialProps: { entity: whenEntity },
+        },
+      );
+
+      await hook.waitForNextUpdate();
+      expect(catalogApiMock.getEntities).toHaveBeenCalledWith(
+        expect.objectContaining({
+          filter: expect.arrayContaining([
+            expect.objectContaining({
+              'relations.ownedBy': [`group:default/${givenSquad}`],
+            }),
+          ]),
+        }),
+      );
+    });
+  });
+});

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

@@ -57,22 +57,18 @@ 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 ownerGroupsName = ownerGroups.map(ownerGroup =>
-      stringifyEntityRef({
-        kind: ownerGroup.kind,
-        namespace: ownerGroup.namespace,
-        name: ownerGroup.name,
-      }),
-    );
-    owners = [...owners, ...ownerGroupsName];
-  }
-  return owners;
+const getMemberOfEntityRefs = (owner: Entity): string[] => {
+  const ownerGroups = getEntityRelations(owner, RELATION_MEMBER_OF, {
+    kind: 'Group',
+  });
+  const ownerGroupsNames = ownerGroups.map(ownerGroup =>
+    stringifyEntityRef({
+      kind: ownerGroup.kind,
+      namespace: ownerGroup.namespace,
+      name: ownerGroup.name,
+    }),
+  );
+  return ownerGroupsNames;
 };
 
 const getAggregatedOwnersEntityRef = async (
@@ -82,7 +78,6 @@ const getAggregatedOwnersEntityRef = async (
   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) {
@@ -125,10 +120,45 @@ const getAggregatedOwnersEntityRef = async (
   return Array.from(processedEntities);
 };
 
+const getOwners = async (
+  entity: Entity,
+  relationsType: AnyRelationsType,
+  catalogApi: CatalogApi,
+): Promise<string[]> => {
+  const isGroup = entity.kind === 'Group';
+  const isAggregated = relationsType === 'aggregated';
+  const isUserEntity = entity.kind === 'User';
+
+  const owners = [stringifyEntityRef(entity)];
+
+  if (isAggregated && isGroup) {
+    const childEntityRefs = await getAggregatedOwnersEntityRef(
+      entity,
+      catalogApi,
+    );
+    owners.push.apply(owners, childEntityRefs);
+  }
+
+  if (isAggregated && isUserEntity) {
+    const parentEntityRefs = getMemberOfEntityRefs(entity);
+    owners.push.apply(owners, parentEntityRefs);
+  }
+
+  return owners;
+};
+
+export const DefaultRelationType = {
+  Direct: 'direct',
+  Aggregated: 'aggregated',
+} as const;
+
+export type AnyRelationsType =
+  | (typeof DefaultRelationType)[keyof typeof DefaultRelationType]
+  | string;
+
 export function useGetEntities(
   entity: Entity,
-  relationsType: string,
-  isGroup: boolean,
+  relationsType: AnyRelationsType,
   entityFilterKind?: string[],
   entityLimit = 6,
 ): {
@@ -151,10 +181,8 @@ export function useGetEntities(
     error,
     value: componentsWithCounters,
   } = useAsync(async () => {
-    const owners =
-      relationsType === 'aggregated' && isGroup
-        ? await getAggregatedOwnersEntityRef(entity, catalogApi)
-        : getOwnersEntityRef(entity);
+    const owners = await getOwners(entity, relationsType, catalogApi);
+
     const ownedEntitiesList = await catalogApi.getEntities({
       filter: [
         {