Merge pull request #33996 from backstage/freben/owner-column-cleanup

Simplify OwnerEntityColumn to use EntityRefLink directly

.changeset/owner-column-cleanup.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-scaffolder': patch
+---
+
+Simplified the `OwnerEntityColumn` in the task list to rely on `EntityRefLink` and the entity presentation API instead of manually fetching entities from the catalog.

plugins/scaffolder/src/components/ListTasksPage/ListTaskPage.test.tsx

@@ -14,7 +14,6 @@
  * limitations under the License.
  */
 
-import { Entity } from '@backstage/catalog-model';
 import {
   renderInTestApp,
   TestApiProvider,
@@ -46,20 +45,6 @@ describe('<ListTasksPage />', () => {
   const mockPermissionApi = { authorize: jest.fn() };
 
   it('should render the page', async () => {
-    const entity: Entity = {
-      apiVersion: 'v1',
-      kind: 'service',
-      metadata: {
-        name: 'test',
-      },
-      spec: {
-        profile: {
-          displayName: 'BackUser',
-        },
-      },
-    };
-    catalogApi.getEntityByRef.mockResolvedValue(entity);
-
     scaffolderApiMock.listTasks.mockResolvedValue({ tasks: [], totalTasks: 0 });
 
     const { getByText } = await renderInTestApp(
@@ -87,19 +72,6 @@ describe('<ListTasksPage />', () => {
   });
 
   it('should render the task I am owner', async () => {
-    const entity: Entity = {
-      apiVersion: 'v1',
-      kind: 'User',
-      metadata: {
-        name: 'foo',
-      },
-      spec: {
-        profile: {
-          displayName: 'BackUser',
-        },
-      },
-    };
-    catalogApi.getEntityByRef.mockResolvedValue(entity);
     scaffolderApiMock.listTasks.mockResolvedValue({
       tasks: [
         {
@@ -151,34 +123,10 @@ describe('<ListTasksPage />', () => {
     expect(getByText('All tasks that have been started')).toBeInTheDocument();
     expect(getByText('Tasks')).toBeInTheDocument();
     expect(await findByText('One Template')).toBeInTheDocument();
-    expect(await findByText('BackUser')).toBeInTheDocument();
+    expect(await findByText('foo')).toBeInTheDocument();
   });
 
   it('should render all tasks', async () => {
-    const entity: Entity = {
-      apiVersion: 'v1',
-      kind: 'User',
-      metadata: {
-        name: 'foo',
-      },
-      spec: {
-        profile: {
-          displayName: 'BackUser',
-        },
-      },
-    };
-    catalogApi.getEntityByRef
-      .mockResolvedValue(entity)
-      .mockResolvedValue(entity)
-      .mockResolvedValue({
-        ...entity,
-        spec: {
-          profile: {
-            displayName: 'OtherUser',
-          },
-        },
-      });
-
     scaffolderApiMock.listTasks
       .mockResolvedValue({
         tasks: [
@@ -252,6 +200,6 @@ describe('<ListTasksPage />', () => {
       offset: 0,
     });
     expect(await findByText('One Template')).toBeInTheDocument();
-    expect(await findByText('OtherUser')).toBeInTheDocument();
+    expect(await findByText('boo')).toBeInTheDocument();
   });
 });

plugins/scaffolder/src/components/ListTasksPage/columns/OwnerEntityColumn.test.tsx

@@ -14,48 +14,15 @@
  * limitations under the License.
  */
 
-import { Entity } from '@backstage/catalog-model';
-import {
-  renderInTestApp,
-  TestApiProvider,
-  mockApis,
-} from '@backstage/test-utils';
-import { catalogApiRef, entityRouteRef } from '@backstage/plugin-catalog-react';
-import { catalogApiMock } from '@backstage/plugin-catalog-react/testUtils';
+import { renderInTestApp, TestApiProvider } from '@backstage/test-utils';
+import { entityRouteRef } from '@backstage/plugin-catalog-react';
 import { OwnerEntityColumn } from './OwnerEntityColumn';
-import { identityApiRef } from '@backstage/core-plugin-api';
 
 describe('<OwnerEntityColumn />', () => {
-  const catalogApi = catalogApiMock.mock();
-  const identityApi = mockApis.identity();
-
-  it('should render the column with the user', async () => {
-    const props = {
-      entityRef: 'user:default/foo',
-    };
-
-    const entity: Entity = {
-      apiVersion: 'v1',
-      kind: 'User',
-      metadata: {
-        name: 'test',
-      },
-      spec: {
-        profile: {
-          displayName: 'BackUser',
-        },
-      },
-    };
-    catalogApi.getEntityByRef.mockResolvedValue(entity);
-
-    const { getByText, getByRole } = await renderInTestApp(
-      <TestApiProvider
-        apis={[
-          [catalogApiRef, catalogApi],
-          [identityApiRef, identityApi],
-        ]}
-      >
-        <OwnerEntityColumn {...props} />
+  it('should render a link to the user entity', async () => {
+    const { getByRole } = await renderInTestApp(
+      <TestApiProvider apis={[]}>
+        <OwnerEntityColumn entityRef="user:default/foo" />
       </TestApiProvider>,
       {
         mountedRoutes: {
@@ -68,7 +35,20 @@ describe('<OwnerEntityColumn />', () => {
       'href',
       '/catalog/default/user/foo',
     );
-    const text = getByText('BackUser');
-    expect(text).toBeDefined();
+  });
+
+  it('should render Unknown when entityRef is missing', async () => {
+    const { getByText } = await renderInTestApp(
+      <TestApiProvider apis={[]}>
+        <OwnerEntityColumn />
+      </TestApiProvider>,
+      {
+        mountedRoutes: {
+          '/catalog/:namespace/:kind/:name': entityRouteRef,
+        },
+      },
+    );
+
+    expect(getByText('Unknown')).toBeInTheDocument();
   });
 });

plugins/scaffolder/src/components/ListTasksPage/columns/OwnerEntityColumn.tsx

@@ -13,37 +13,13 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { useApi } from '@backstage/core-plugin-api';
-
-import useAsync from 'react-use/esm/useAsync';
-
-import { catalogApiRef, EntityRefLink } from '@backstage/plugin-catalog-react';
-import { parseEntityRef, UserEntity } from '@backstage/catalog-model';
+import { EntityRefLink } from '@backstage/plugin-catalog-react';
 import Typography from '@material-ui/core/Typography';
 
 export const OwnerEntityColumn = ({ entityRef }: { entityRef?: string }) => {
-  const catalogApi = useApi(catalogApiRef);
-
-  const { value, loading, error } = useAsync(
-    () => catalogApi.getEntityByRef(entityRef || ''),
-    [catalogApi, entityRef],
-  );
-
   if (!entityRef) {
-    return <Typography paragraph>Unknown</Typography>;
+    return <Typography>Unknown</Typography>;
   }
 
-  if (loading || error) {
-    return null;
-  }
-
-  return (
-    <EntityRefLink
-      entityRef={parseEntityRef(entityRef)}
-      title={
-        (value as UserEntity)?.spec?.profile?.displayName ??
-        value?.metadata.name
-      }
-    />
-  );
+  return <EntityRefLink entityRef={entityRef} />;
 };