Merge pull request #1214 from spotify/feat/star-components

Ability to star items in the catalog table

plugins/catalog/package.json

@@ -30,6 +30,7 @@
     "@material-ui/core": "^4.9.1",
     "@material-ui/icons": "^4.9.1",
     "@material-ui/lab": "4.0.0-alpha.45",
+    "node-cache": "^5.1.1",
     "react": "^16.13.1",
     "react-dom": "^16.13.1",
     "react-router": "^5.2.0",

plugins/catalog/src/api/CatalogClient.ts

@@ -21,8 +21,13 @@ import {
   Location,
   LOCATION_ANNOTATION,
 } from '@backstage/catalog-model';
+import Cache from 'node-cache';
 
 export class CatalogClient implements CatalogApi {
+  // TODO(blam): This cache is just temporary until we have GraphQL.
+  // And client side caching using things like React Apollo or Relay.
+  // There's a lot of loading states that cause flickering around the app which aren't needed.
+  private cache: Cache;
   private apiOrigin: string;
   private basePath: string;
   constructor({
@@ -34,6 +39,7 @@ export class CatalogClient implements CatalogApi {
   }) {
     this.apiOrigin = apiOrigin;
     this.basePath = basePath;
+    this.cache = new Cache({ stdTTL: 10 });
   }
   async getLocationById(id: String): Promise<Location | undefined> {
     const response = await fetch(
@@ -78,6 +84,11 @@ export class CatalogClient implements CatalogApi {
   async getEntities(
     filter?: Record<string, string>,
   ): Promise<DescriptorEnvelope[]> {
+    const cachedValue = this.cache.get<DescriptorEnvelope[]>(
+      `get:${JSON.stringify(filter)}`,
+    );
+    if (cachedValue) return cachedValue;
+
     let url = `${this.apiOrigin}${this.basePath}/entities`;
     if (filter) {
       url += '?';
@@ -95,8 +106,13 @@ export class CatalogClient implements CatalogApi {
         `Request failed with ${response.status} ${response.statusText}, ${payload}`,
       );
     }
+    const value = await response.json();
 
-    return await response.json();
+    if (value?.length) {
+      this.cache.set(`get:${JSON.stringify(filter)}`, value);
+    }
+
+    return value;
   }
 
   async getEntity({

plugins/catalog/src/components/CatalogFilter/StarredCount.test.tsx

@@ -24,6 +24,8 @@ describe('Starred Count', () => {
   it('should render the count returned from the hook', async () => {
     jest.spyOn(Hooks, 'useStarredEntities').mockReturnValue({
       starredEntities: new Set(['id1', 'id2', 'id3', 'id4']),
+      isStarredEntity: () => false,
+      toggleStarredEntity: () => undefined,
     });
 
     const { findByText } = render(wrapInTestApp(<StarredCount />));

plugins/catalog/src/components/CatalogPage/CatalogPage.tsx

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { LocationSpec } from '@backstage/catalog-model';
+import { LocationSpec, Entity } from '@backstage/catalog-model';
 import {
   Content,
   ContentHeader,
@@ -27,16 +27,20 @@ import {
   SupportButton,
   useApi,
 } from '@backstage/core';
+
 import { rootRoute as scaffolderRootRoute } from '@backstage/plugin-scaffolder';
-import { Button, Link, makeStyles, Typography } from '@material-ui/core';
-import Edit from '@material-ui/icons/Edit';
+import { Button, makeStyles, Typography, Link } from '@material-ui/core';
 import GitHub from '@material-ui/icons/GitHub';
+import StarOutline from '@material-ui/icons/StarBorder';
+import Star from '@material-ui/icons/Star';
+
+import Edit from '@material-ui/icons/Edit';
+
 import React, { FC, useCallback, useState } from 'react';
 import { Link as RouterLink } from 'react-router-dom';
 import { useAsync } from 'react-use';
 import { catalogApiRef } from '../..';
-import { Component } from '../../data/component';
-import { dataResolvers, defaultFilter, filterGroups } from '../../data/filters';
+import { defaultFilter, filterGroups, dataResolvers } from '../../data/filters';
 import { entityToComponent, findLocationForEntityMeta } from '../../data/utils';
 import { useStarredEntities } from '../../hooks/useStarredEntites';
 import {
@@ -60,13 +64,18 @@ const useStyles = makeStyles(theme => ({
 
 export const CatalogPage: FC<{}> = () => {
   const catalogApi = useApi(catalogApiRef);
-  const { starredEntities } = useStarredEntities();
+  const {
+    starredEntities,
+    toggleStarredEntity,
+    isStarredEntity,
+  } = useStarredEntities();
   const [selectedFilter, setSelectedFilter] = useState<CatalogFilterItem>(
     defaultFilter,
   );
+
   const { value, error, loading } = useAsync(
-    () => dataResolvers[selectedFilter.id]({ catalogApi, starredEntities }),
-    [selectedFilter.id],
+    () => dataResolvers[selectedFilter.id]({ catalogApi, isStarredEntity }),
+    [selectedFilter.id, starredEntities.size],
   );
 
   const onFilterSelected = useCallback(
@@ -77,7 +86,7 @@ export const CatalogPage: FC<{}> = () => {
   const styles = useStyles();
 
   const actions = [
-    (rowData: Component) => {
+    (rowData: Entity) => {
       const location = findLocationForEntityMeta(rowData.metadata);
       return {
         icon: GitHub,
@@ -89,7 +98,7 @@ export const CatalogPage: FC<{}> = () => {
         hidden: location ? location?.type !== 'github' : true,
       };
     },
-    (rowData: Component) => {
+    (rowData: Entity) => {
       const createEditLink = (location: LocationSpec): string => {
         switch (location.type) {
           case 'github':
@@ -112,6 +121,14 @@ export const CatalogPage: FC<{}> = () => {
         hidden: location ? location?.type !== 'github' : true,
       };
     },
+    (rowData: Entity) => {
+      const isStarred = isStarredEntity(rowData);
+      return {
+        icon: isStarred ? Star : StarOutline,
+        tooltip: isStarred ? 'Remove from favorites' : 'Add to favorites',
+        onClick: () => toggleStarredEntity(rowData),
+      };
+    },
   ];
 
   // TODO: replace me with the proper tabs implemntation

plugins/catalog/src/data/filters.ts

@@ -58,10 +58,10 @@ export const filterGroups: CatalogFilterGroup[] = [
 
 type ResolverFunction = ({
   catalogApi,
-  starredEntities,
+  isStarredEntity,
 }: {
   catalogApi: CatalogApi;
-  starredEntities: Set<string>;
+  isStarredEntity: (entity: Entity) => boolean;
 }) => Promise<Entity[]>;
 
 export const dataResolvers: Record<FilterGroupItem, ResolverFunction> = {
@@ -69,12 +69,10 @@ export const dataResolvers: Record<FilterGroupItem, ResolverFunction> = {
   [FilterGroupItem.ALL]: async ({ catalogApi }) => {
     return catalogApi.getEntities();
   },
-  [FilterGroupItem.STARRED]: async ({ catalogApi, starredEntities }) => {
+  [FilterGroupItem.STARRED]: async ({ catalogApi, isStarredEntity }) => {
     const allEntities = await catalogApi.getEntities();
 
-    return allEntities.filter(entity =>
-      starredEntities.has(entity.metadata.name),
-    );
+    return allEntities.filter(entity => isStarredEntity(entity));
   },
 };
 

plugins/catalog/src/hooks/useStarredEntites.ts

@@ -13,17 +13,24 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { useState, useEffect } from 'react';
+import { useState, useEffect, useCallback } from 'react';
 import { useApi, storageApiRef } from '@backstage/core';
 import { useObservable } from 'react-use';
+import { Entity } from '@backstage/catalog-model';
+
+const buildEntityKey = (component: Entity) =>
+  `entity:${component.kind}:${component.metadata.namespace ?? 'default'}:${
+    component.metadata.name
+  }`;
 
 export const useStarredEntities = () => {
   const storageApi = useApi(storageApiRef);
   const settingsStore = storageApi.forBucket('settings');
-  const rawStarredItems = settingsStore.get<string[]>('starredEntities') ?? [];
+  const rawStarredEntityKeys =
+    settingsStore.get<string[]>('starredEntities') ?? [];
 
   const [starredEntities, setStarredEntities] = useState(
-    new Set(rawStarredItems),
+    new Set(rawStarredEntityKeys),
   );
 
   const observedItems = useObservable(
@@ -37,7 +44,31 @@ export const useStarredEntities = () => {
     }
   }, [observedItems?.newValue]);
 
+  const toggleStarredEntity = useCallback(
+    (entity: Entity) => {
+      const entityKey = buildEntityKey(entity);
+      if (starredEntities.has(entityKey)) {
+        starredEntities.delete(entityKey);
+      } else {
+        starredEntities.add(entityKey);
+      }
+
+      settingsStore.set('starredEntities', Array.from(starredEntities));
+    },
+    [starredEntities, settingsStore],
+  );
+
+  const isStarredEntity = useCallback(
+    (entity: Entity) => {
+      const entityKey = buildEntityKey(entity);
+      return starredEntities.has(entityKey);
+    },
+    [starredEntities],
+  );
+
   return {
     starredEntities,
+    toggleStarredEntity,
+    isStarredEntity,
   };
 };

plugins/catalog/src/hooks/useStarredEntities.test.tsx

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 import React from 'react';
-import { renderHook } from '@testing-library/react-hooks';
+import { renderHook, act } from '@testing-library/react-hooks';
 import { useStarredEntities } from './useStarredEntites';
 import {
   ApiProvider,
@@ -24,10 +24,28 @@ import {
   StorageApi,
 } from '@backstage/core';
 import { MockErrorApi } from '@backstage/test-utils';
+import { Entity } from '@backstage/catalog-model';
 
 describe('useStarredEntities', () => {
   let mockStorage: StorageApi | undefined;
 
+  const mockEntity: Entity = {
+    apiVersion: '1',
+    kind: 'Component',
+    metadata: {
+      name: 'mock',
+    },
+  };
+
+  const secondMockEntity: Entity = {
+    apiVersion: '1',
+    kind: 'Component',
+    metadata: {
+      namespace: 'test',
+      name: 'mock2',
+    },
+  };
+
   const wrapper: React.FC<{}> = ({ children }) => {
     return (
       <ApiProvider apis={ApiRegistry.with(storageApiRef, mockStorage)}>
@@ -58,23 +76,45 @@ describe('useStarredEntities', () => {
     }
   });
   it('should listen to changes when the storage is set elsewhere', async () => {
-    const store = mockStorage?.forBucket('settings');
-
     const { result, waitForNextUpdate } = renderHook(
       () => useStarredEntities(),
       { wrapper },
     );
 
     expect(result.current.starredEntities.size).toBe(0);
-    expect(result.current.starredEntities.has('something')).toBeFalsy();
+    expect(result.current.isStarredEntity(mockEntity)).toBeFalsy();
 
     // Make this happen after awaiting for the next update so we can
     // catch when the hook re-renders with the latest data
-    setTimeout(() => store?.set('starredEntities', ['something']), 1);
+    setTimeout(() => result.current.toggleStarredEntity(mockEntity), 1);
 
     await waitForNextUpdate();
 
     expect(result.current.starredEntities.size).toBe(1);
-    expect(result.current.starredEntities.has('something')).toBeTruthy();
+    expect(result.current.isStarredEntity(mockEntity)).toBeTruthy();
+  });
+
+  it('should write new entries to the local store when adding a togglging entity', async () => {
+    const { result } = renderHook(() => useStarredEntities(), { wrapper });
+
+    act(() => {
+      result.current.toggleStarredEntity(mockEntity);
+    });
+
+    expect(result.current.isStarredEntity(mockEntity)).toBeTruthy();
+    expect(result.current.isStarredEntity(secondMockEntity)).toBeFalsy();
+  });
+
+  it('should remove an existing entity when toggling entries', async () => {
+    const { result } = renderHook(() => useStarredEntities(), { wrapper });
+
+    act(() => {
+      result.current.toggleStarredEntity(mockEntity);
+      result.current.toggleStarredEntity(secondMockEntity);
+      result.current.toggleStarredEntity(mockEntity);
+    });
+
+    expect(result.current.isStarredEntity(mockEntity)).toBeFalsy();
+    expect(result.current.isStarredEntity(secondMockEntity)).toBeTruthy();
   });
 });