From dd9329caea1cd2ff769339613f385fa8127e9f6c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carl-Erik=20Bergstr=C3=B6m?= <cbergstrom@spotify.com>
Date: Wed, 30 Aug 2023 20:13:03 +0200
Subject: [PATCH] fix: change default entity toggle state for user entities
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Carl-Erik Bergström <cbergstrom@spotify.com>
---
 .changeset/long-flowers-sort.md               |   5 +
 .../OwnershipCard/OwnershipCard.test.tsx      | 110 ++++++++++++++----
 .../Cards/OwnershipCard/OwnershipCard.tsx     |  22 ++--
 3 files changed, 110 insertions(+), 27 deletions(-)
 create mode 100644 .changeset/long-flowers-sort.md

diff --git a/.changeset/long-flowers-sort.md b/.changeset/long-flowers-sort.md
new file mode 100644
index 0000000000..2f25260030
--- /dev/null
+++ b/.changeset/long-flowers-sort.md
@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-org': patch
+---
+
+Entity relations toggle should by default be aggregated for User entities
diff --git a/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.test.tsx b/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.test.tsx
index faba9b5627..72a912e16e 100644
--- a/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.test.tsx
+++ b/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.test.tsx
@@ -152,6 +152,27 @@ describe('OwnershipCard', () => {
     ],
   };
 
+  const userEntity: UserEntity = {
+    apiVersion: 'backstage.io/v1alpha1',
+    kind: 'User',
+    metadata: {
+      name: 'the-user',
+    },
+    spec: {
+      memberOf: ['my-team'],
+    },
+    relations: [
+      {
+        type: 'memberOf',
+        targetRef: 'group:default/my-team',
+      },
+      {
+        type: 'memberOf',
+        targetRef: 'group:custom/some-team',
+      },
+    ],
+  };
+
   it('displays entity counts', async () => {
     const catalogApi: jest.Mocked<CatalogApi> = {
       getEntities: jest.fn(),
@@ -271,26 +292,6 @@ describe('OwnershipCard', () => {
   });
 
   it('links to the catalog with the user and groups filters from an user profile', async () => {
-    const userEntity: UserEntity = {
-      apiVersion: 'backstage.io/v1alpha1',
-      kind: 'User',
-      metadata: {
-        name: 'the-user',
-      },
-      spec: {
-        memberOf: ['my-team'],
-      },
-      relations: [
-        {
-          type: 'memberOf',
-          targetRef: 'group:default/my-team',
-        },
-        {
-          type: 'memberOf',
-          targetRef: 'group:custom/some-team',
-        },
-      ],
-    };
     const catalogApi: jest.Mocked<CatalogApi> = {
       getEntities: jest.fn(),
     } as any;
@@ -386,5 +387,74 @@ describe('OwnershipCard', () => {
 
       expect(getByTitle('Aggregated Relations')).toBeInTheDocument();
     });
+
+    it('defaults to aggregated for User entity kind', async () => {
+      const catalogApi: jest.Mocked<CatalogApi> = {
+        getEntities: jest.fn(),
+      } as any;
+
+      catalogApi.getEntities.mockImplementation(getEntitiesMock);
+
+      const { getByLabelText } = await renderInTestApp(
+        <TestApiProvider apis={[[catalogApiRef, catalogApi]]}>
+          <EntityProvider entity={userEntity}>
+            <OwnershipCard />
+          </EntityProvider>
+        </TestApiProvider>,
+        {
+          mountedRoutes: {
+            '/create': catalogIndexRouteRef,
+          },
+        },
+      );
+
+      expect(getByLabelText('Ownership Type Switch')).toBeChecked();
+    });
+
+    it('defaults to direct for all entity kinds except User', async () => {
+      const catalogApi: jest.Mocked<CatalogApi> = {
+        getEntities: jest.fn(),
+      } as any;
+
+      catalogApi.getEntities.mockImplementation(getEntitiesMock);
+
+      const { getByLabelText } = await renderInTestApp(
+        <TestApiProvider apis={[[catalogApiRef, catalogApi]]}>
+          <EntityProvider entity={groupEntity}>
+            <OwnershipCard />
+          </EntityProvider>
+        </TestApiProvider>,
+        {
+          mountedRoutes: {
+            '/create': catalogIndexRouteRef,
+          },
+        },
+      );
+
+      expect(getByLabelText('Ownership Type Switch')).not.toBeChecked();
+    });
+
+    it('defaults to provided relationsType', async () => {
+      const catalogApi: jest.Mocked<CatalogApi> = {
+        getEntities: jest.fn(),
+      } as any;
+
+      catalogApi.getEntities.mockImplementation(getEntitiesMock);
+
+      const { getByLabelText } = await renderInTestApp(
+        <TestApiProvider apis={[[catalogApiRef, catalogApi]]}>
+          <EntityProvider entity={userEntity}>
+            <OwnershipCard relationsType="direct" />
+          </EntityProvider>
+        </TestApiProvider>,
+        {
+          mountedRoutes: {
+            '/create': catalogIndexRouteRef,
+          },
+        },
+      );
+
+      expect(getByLabelText('Ownership Type Switch')).not.toBeChecked();
+    });
   });
 });
diff --git a/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.tsx b/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.tsx
index 2fb7826e52..791ce5ca64 100644
--- a/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.tsx
+++ b/plugins/org/src/components/Cards/OwnershipCard/OwnershipCard.tsx
@@ -25,7 +25,7 @@ import {
   Switch,
   Tooltip,
 } from '@material-ui/core';
-import React, { useState } from 'react';
+import React, { useEffect, useState } from 'react';
 import { ComponentsGrid } from './ComponentsGrid';
 import { EntityRelationAggregation } from './types';
 
@@ -71,10 +71,18 @@ export const OwnershipCard = (props: {
     hideRelationsToggle === undefined ? false : hideRelationsToggle;
   const classes = useStyles();
   const { entity } = useEntity();
+
+  const defaultRelationsType = entity.kind === 'User' ? 'aggregated' : 'direct';
   const [getRelationsType, setRelationsType] = useState(
-    relationsType || 'direct',
+    relationsType ?? defaultRelationsType,
   );
 
+  useEffect(() => {
+    if (!relationsType) {
+      setRelationsType(defaultRelationsType);
+    }
+  }, [setRelationsType, defaultRelationsType, relationsType]);
+
   return (
     <InfoCard title="Ownership" variant={variant}>
       {!relationsToggle && (
@@ -95,11 +103,11 @@ export const OwnershipCard = (props: {
                 <Switch
                   color="primary"
                   checked={getRelationsType !== 'direct'}
-                  onChange={() =>
-                    getRelationsType === 'direct'
-                      ? setRelationsType('aggregated')
-                      : setRelationsType('direct')
-                  }
+                  onChange={() => {
+                    const updatedRelationsType =
+                      getRelationsType === 'direct' ? 'aggregated' : 'direct';
+                    setRelationsType(updatedRelationsType);
+                  }}
                   name="pin"
                   inputProps={{ 'aria-label': 'Ownership Type Switch' }}
                 />