From 54bbe25c344606dcc28a64e72774e705bc9b04d3 Mon Sep 17 00:00:00 2001 From: Dominik Henneke Date: Mon, 27 Sep 2021 11:32:24 +0200 Subject: [PATCH 01/13] Store the namespaced bucket storage for each instance that was created with `MockStorage.create()` instead of global variable Signed-off-by: Dominik Henneke --- .changeset/stupid-elephants-invent.md | 5 ++++ .../apis/StorageApi/MockStorageApi.test.ts | 17 +++++++++++-- .../apis/StorageApi/MockStorageApi.ts | 24 ++++++++++++------- 3 files changed, 36 insertions(+), 10 deletions(-) create mode 100644 .changeset/stupid-elephants-invent.md diff --git a/.changeset/stupid-elephants-invent.md b/.changeset/stupid-elephants-invent.md new file mode 100644 index 0000000000..11b87a667d --- /dev/null +++ b/.changeset/stupid-elephants-invent.md @@ -0,0 +1,5 @@ +--- +'@backstage/test-utils': patch +--- + +Store the namespaced bucket storage for each instance that was created with `MockStorage.create()` instead of global variable. diff --git a/packages/test-utils/src/testUtils/apis/StorageApi/MockStorageApi.test.ts b/packages/test-utils/src/testUtils/apis/StorageApi/MockStorageApi.test.ts index fabfa989e2..a3028f8b57 100644 --- a/packages/test-utils/src/testUtils/apis/StorageApi/MockStorageApi.test.ts +++ b/packages/test-utils/src/testUtils/apis/StorageApi/MockStorageApi.test.ts @@ -13,8 +13,8 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { MockStorageApi } from './MockStorageApi'; import { StorageApi } from '@backstage/core-plugin-api'; +import { MockStorageApi } from './MockStorageApi'; describe('WebStorage Storage API', () => { const createMockStorage = (): StorageApi => { @@ -124,7 +124,7 @@ describe('WebStorage Storage API', () => { expect(secondStorage.get(keyName)).toBe('deerp'); }); - it('should not clash with other namesapces when creating buckets', async () => { + it('should not clash with other namespaces when creating buckets', async () => { const rootStorage = createMockStorage(); // when getting key test2 it will translate to /profile/something/deep/test2 @@ -139,4 +139,17 @@ describe('WebStorage Storage API', () => { expect(secondStorage.get('deep/test2')).toBe(undefined); }); + + it('should not reuse storage instances between different rootStorages', async () => { + const rootStorage1 = createMockStorage(); + const rootStorage2 = createMockStorage(); + + const firstStorage = rootStorage1.forBucket('something'); + const secondStorage = rootStorage2.forBucket('something'); + + await firstStorage.set('test2', true); + + expect(firstStorage.get('test2')).toBe(true); + expect(secondStorage.get('test2')).toBe(undefined); + }); }); diff --git a/packages/test-utils/src/testUtils/apis/StorageApi/MockStorageApi.ts b/packages/test-utils/src/testUtils/apis/StorageApi/MockStorageApi.ts index 9f21b4e4c8..ab5d26afc5 100644 --- a/packages/test-utils/src/testUtils/apis/StorageApi/MockStorageApi.ts +++ b/packages/test-utils/src/testUtils/apis/StorageApi/MockStorageApi.ts @@ -23,29 +23,37 @@ import ObservableImpl from 'zen-observable'; export type MockStorageBucket = { [key: string]: any }; -const bucketStorageApis = new Map(); - export class MockStorageApi implements StorageApi { private readonly namespace: string; private readonly data: MockStorageBucket; + private readonly bucketStorageApis: Map; - private constructor(namespace: string, data?: MockStorageBucket) { + private constructor( + namespace: string, + bucketStorageApis: Map, + data?: MockStorageBucket, + ) { this.namespace = namespace; + this.bucketStorageApis = bucketStorageApis; this.data = { ...data }; } static create(data?: MockStorageBucket) { - return new MockStorageApi('', data); + return new MockStorageApi('', new Map(), data); } forBucket(name: string): StorageApi { - if (!bucketStorageApis.has(name)) { - bucketStorageApis.set( + if (!this.bucketStorageApis.has(name)) { + this.bucketStorageApis.set( name, - new MockStorageApi(`${this.namespace}/${name}`, this.data), + new MockStorageApi( + `${this.namespace}/${name}`, + this.bucketStorageApis, + this.data, + ), ); } - return bucketStorageApis.get(name)!; + return this.bucketStorageApis.get(name)!; } get(key: string): T | undefined { From 82fbda923e16a028a68fac9eb888ec7113a1636d Mon Sep 17 00:00:00 2001 From: Dominik Henneke Date: Thu, 30 Sep 2021 15:09:38 +0200 Subject: [PATCH 02/13] Introduce a new `StarredEntitiesApi` that is used in the `useStarredEntities` hook Signed-off-by: Dominik Henneke --- .changeset/cool-deers-cough.md | 7 + .../components/catalog/EntityPage.test.tsx | 21 ++- .../ApiExplorerPage/ApiExplorerPage.test.tsx | 37 +++-- plugins/catalog-react/api-report.md | 40 +++++ plugins/catalog-react/package.json | 4 +- .../DefaultStarredEntitiesApi.test.ts | 145 ++++++++++++++++++ .../DefaultStarredEntitiesApi.ts | 116 ++++++++++++++ .../StarredEntitiesApi/StarredEntitiesApi.ts | 71 +++++++++ .../src/apis/StarredEntitiesApi/index.ts | 22 +++ plugins/catalog-react/src/apis/index.ts | 17 ++ .../src/hooks/useEntityListProvider.test.tsx | 30 ++-- .../src/hooks/useStarredEntities.test.tsx | 67 +++++--- .../src/hooks/useStarredEntities.ts | 50 ++---- plugins/catalog-react/src/index.ts | 1 + .../CatalogPage/CatalogPage.test.tsx | 41 +++-- .../CatalogTable/CatalogTable.test.tsx | 72 +++++---- .../EntityLayout/EntityLayout.test.tsx | 24 +-- plugins/catalog/src/plugin.ts | 10 ++ .../components/DefaultTechDocsHome.test.tsx | 19 ++- 19 files changed, 637 insertions(+), 157 deletions(-) create mode 100644 .changeset/cool-deers-cough.md create mode 100644 plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts create mode 100644 plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts create mode 100644 plugins/catalog-react/src/apis/StarredEntitiesApi/StarredEntitiesApi.ts create mode 100644 plugins/catalog-react/src/apis/StarredEntitiesApi/index.ts create mode 100644 plugins/catalog-react/src/apis/index.ts diff --git a/.changeset/cool-deers-cough.md b/.changeset/cool-deers-cough.md new file mode 100644 index 0000000000..6d3e923d7d --- /dev/null +++ b/.changeset/cool-deers-cough.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-catalog-react': minor +'@backstage/plugin-catalog': patch +--- + +Introduce a new `StarredEntitiesApi` that is used in the `useStarredEntities` hook. +The `@backstage/plugin-catalog` installs a default implementation that is backed by the `StorageApi`, but one can also override the `starredEntitiesApiRef`. diff --git a/packages/app/src/components/catalog/EntityPage.test.tsx b/packages/app/src/components/catalog/EntityPage.test.tsx index 955e477f3c..089cb9c7cf 100644 --- a/packages/app/src/components/catalog/EntityPage.test.tsx +++ b/packages/app/src/components/catalog/EntityPage.test.tsx @@ -14,13 +14,17 @@ * limitations under the License. */ -import React from 'react'; -import { EntityLayout } from '@backstage/plugin-catalog'; -import { EntityProvider } from '@backstage/plugin-catalog-react'; -import { renderInTestApp } from '@backstage/test-utils'; -import { cicdContent } from './EntityPage'; -import { githubActionsApiRef } from '@backstage/plugin-github-actions'; import { ApiProvider, ApiRegistry } from '@backstage/core-app-api'; +import { EntityLayout } from '@backstage/plugin-catalog'; +import { + DefaultStarredEntitiesApi, + EntityProvider, + starredEntitiesApiRef, +} from '@backstage/plugin-catalog-react'; +import { githubActionsApiRef } from '@backstage/plugin-github-actions'; +import { MockStorageApi, renderInTestApp } from '@backstage/test-utils'; +import React from 'react'; +import { cicdContent } from './EntityPage'; describe('EntityPage Test', () => { const entity = { @@ -48,7 +52,10 @@ describe('EntityPage Test', () => { downloadJobLogsForWorkflowRun: jest.fn(), } as jest.Mocked; - const apis = ApiRegistry.with(githubActionsApiRef, mockedApi); + const apis = ApiRegistry.with(githubActionsApiRef, mockedApi).with( + starredEntitiesApiRef, + new DefaultStarredEntitiesApi({ storageApi: MockStorageApi.create() }), + ); describe('cicdContent', () => { it('Should render GitHub Actions View', async () => { diff --git a/plugins/api-docs/src/components/ApiExplorerPage/ApiExplorerPage.test.tsx b/plugins/api-docs/src/components/ApiExplorerPage/ApiExplorerPage.test.tsx index d0b7523ead..292d03c838 100644 --- a/plugins/api-docs/src/components/ApiExplorerPage/ApiExplorerPage.test.tsx +++ b/plugins/api-docs/src/components/ApiExplorerPage/ApiExplorerPage.test.tsx @@ -15,30 +15,31 @@ */ import { Entity, RELATION_MEMBER_OF } from '@backstage/catalog-model'; -import { - CatalogApi, - catalogApiRef, - entityRouteRef, -} from '@backstage/plugin-catalog-react'; -import { MockStorageApi, wrapInTestApp } from '@backstage/test-utils'; -import { render } from '@testing-library/react'; -import React from 'react'; -import { apiDocsConfigRef } from '../../config'; -import { ApiExplorerPage } from './ApiExplorerPage'; - import { ApiProvider, ApiRegistry, ConfigReader, } from '@backstage/core-app-api'; +import { TableColumn, TableProps } from '@backstage/core-components'; import { - storageApiRef, ConfigApi, configApiRef, + storageApiRef, } from '@backstage/core-plugin-api'; -import { TableColumn, TableProps } from '@backstage/core-components'; -import DashboardIcon from '@material-ui/icons/Dashboard'; import { CatalogTableRow } from '@backstage/plugin-catalog'; +import { + CatalogApi, + catalogApiRef, + DefaultStarredEntitiesApi, + entityRouteRef, +} from '@backstage/plugin-catalog-react'; +import { starredEntitiesApiRef } from '@backstage/plugin-catalog-react/src/apis'; +import { MockStorageApi, wrapInTestApp } from '@backstage/test-utils'; +import DashboardIcon from '@material-ui/icons/Dashboard'; +import { render } from '@testing-library/react'; +import React from 'react'; +import { apiDocsConfigRef } from '../../config'; +import { ApiExplorerPage } from './ApiExplorerPage'; describe('ApiCatalogPage', () => { const catalogApi: Partial = { @@ -82,6 +83,8 @@ describe('ApiCatalogPage', () => { getApiDefinitionWidget: () => undefined, }; + const storageApi = MockStorageApi.create(); + const renderWrapped = (children: React.ReactNode) => render( wrapInTestApp( @@ -89,7 +92,11 @@ describe('ApiCatalogPage', () => { apis={ApiRegistry.from([ [catalogApiRef, catalogApi], [configApiRef, configApi], - [storageApiRef, MockStorageApi.create()], + [storageApiRef, storageApi], + [ + starredEntitiesApiRef, + new DefaultStarredEntitiesApi({ storageApi }), + ], [apiDocsConfigRef, apiDocsConfig], ])} > diff --git a/plugins/catalog-react/api-report.md b/plugins/catalog-react/api-report.md index ce7282276e..e176d56129 100644 --- a/plugins/catalog-react/api-report.md +++ b/plugins/catalog-react/api-report.md @@ -16,11 +16,13 @@ import { Entity } from '@backstage/catalog-model'; import { EntityName } from '@backstage/catalog-model'; import { IconButton } from '@material-ui/core'; import { LinkProps } from '@backstage/core-components'; +import { Observable } from '@backstage/core-plugin-api'; import { PropsWithChildren } from 'react'; import { default as React_2 } from 'react'; import { ReactNode } from 'react'; import { RouteRef } from '@backstage/core-plugin-api'; import { ScmIntegrationRegistry } from '@backstage/integration'; +import { StorageApi } from '@backstage/core-plugin-api'; import { SystemEntity } from '@backstage/catalog-model'; import { TableColumn } from '@backstage/core-components'; import { UserEntity } from '@backstage/catalog-model'; @@ -131,6 +133,23 @@ export type DefaultEntityFilters = { text?: EntityTextFilter; }; +// Warning: (ae-missing-release-tag) "DefaultStarredEntitiesApi" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) +// +// @public (undocumented) +export class DefaultStarredEntitiesApi implements StarredEntitiesApi { + constructor(opts: { storageApi: StorageApi }); + // (undocumented) + isStarred(entity: Entity): boolean; + // (undocumented) + star(entity: Entity): Promise; + // (undocumented) + starredEntities$(): Observable; + // (undocumented) + toggleStarred(entity: Entity): Promise; + // (undocumented) + unstar(entity: Entity): Promise; +} + // Warning: (ae-forgotten-export) The symbol "EntityLoadingStatus" needs to be exported by the entry point index.d.ts // // @public @deprecated (undocumented) @@ -742,6 +761,27 @@ export function reduceEntityFilters( // @public (undocumented) export const rootRoute: RouteRef; +// @public +export interface StarredEntitiesApi { + star(entity: Entity): Promise; + starredEntities$(): Observable; + toggleStarred(entity: Entity): Promise; + unstar(entity: Entity): Promise; +} + +// Warning: (ae-missing-release-tag) "StarredEntitiesApiObservable" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) +// +// @public (undocumented) +export type StarredEntitiesApiObservable = { + starredEntities: Set; + isStarred: (entity: Entity) => boolean; +}; + +// Warning: (ae-missing-release-tag) "starredEntitiesApiRef" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) +// +// @public (undocumented) +export const starredEntitiesApiRef: ApiRef; + // Warning: (ae-forgotten-export) The symbol "Props" needs to be exported by the entry point index.d.ts // Warning: (ae-missing-release-tag) "UnregisterEntityDialog" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // diff --git a/plugins/catalog-react/package.json b/plugins/catalog-react/package.json index f6b36a7191..9ddee679c3 100644 --- a/plugins/catalog-react/package.json +++ b/plugins/catalog-react/package.json @@ -45,7 +45,8 @@ "qs": "^6.9.4", "react": "^16.13.1", "react-router": "6.0.0-beta.0", - "react-use": "^17.2.4" + "react-use": "^17.2.4", + "zen-observable": "^0.8.15" }, "devDependencies": { "@backstage/cli": "^0.7.15", @@ -56,6 +57,7 @@ "@testing-library/user-event": "^13.1.8", "@types/jest": "^26.0.7", "@types/jwt-decode": "^3.1.0", + "@types/zen-observable": "^0.8.0", "cross-fetch": "^3.0.6", "react-test-renderer": "^16.13.1" }, diff --git a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts new file mode 100644 index 0000000000..6a4f90200b --- /dev/null +++ b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts @@ -0,0 +1,145 @@ +/* + * Copyright 2021 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 { StorageApi } from '@backstage/core-plugin-api'; +import { MockStorageApi } from '@backstage/test-utils'; +import { DefaultStarredEntitiesApi } from './DefaultStarredEntitiesApi'; + +describe('DefaultStarredEntitiesApi', () => { + let mockStorage: StorageApi; + let starredEntitiesApi: DefaultStarredEntitiesApi; + + const mockEntity: Entity = { + apiVersion: '1', + kind: 'Component', + metadata: { + name: 'mock', + }, + }; + + beforeEach(() => { + mockStorage = MockStorageApi.create(); + starredEntitiesApi = new DefaultStarredEntitiesApi({ + storageApi: mockStorage, + }); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('toggleStarred', () => { + it('should star unstarred entity', async () => { + expect(starredEntitiesApi.isStarred(mockEntity)).toBe(false); + + await starredEntitiesApi.toggleStarred(mockEntity); + + expect(starredEntitiesApi.isStarred(mockEntity)).toBe(true); + }); + + it('should unstar starred entity', async () => { + const bucket = mockStorage.forBucket('settings'); + await bucket.set('starredEntities', ['entity:Component:default:mock']); + + expect(starredEntitiesApi.isStarred(mockEntity)).toBe(true); + + await starredEntitiesApi.toggleStarred(mockEntity); + + expect(starredEntitiesApi.isStarred(mockEntity)).toBe(false); + }); + }); + + describe('star', () => { + it('should star unstarred entity', async () => { + expect(starredEntitiesApi.isStarred(mockEntity)).toBe(false); + + await starredEntitiesApi.star(mockEntity); + + expect(starredEntitiesApi.isStarred(mockEntity)).toBe(true); + }); + + it('should keep starred entity', async () => { + const bucket = mockStorage.forBucket('settings'); + await bucket.set('starredEntities', ['entity:Component:default:mock']); + + expect(starredEntitiesApi.isStarred(mockEntity)).toBe(true); + + await starredEntitiesApi.star(mockEntity); + + expect(starredEntitiesApi.isStarred(mockEntity)).toBe(true); + }); + }); + + describe('unstar', () => { + it('should unstar starred entity', async () => { + const bucket = mockStorage.forBucket('settings'); + await bucket.set('starredEntities', ['entity:Component:default:mock']); + + expect(starredEntitiesApi.isStarred(mockEntity)).toBe(true); + + await starredEntitiesApi.unstar(mockEntity); + + expect(starredEntitiesApi.isStarred(mockEntity)).toBe(false); + }); + + it('should keep unstarred entity', async () => { + expect(starredEntitiesApi.isStarred(mockEntity)).toBe(false); + + await starredEntitiesApi.unstar(mockEntity); + + expect(starredEntitiesApi.isStarred(mockEntity)).toBe(false); + }); + }); + + describe('starredEntities$', () => { + const handler = jest.fn(); + + beforeEach(async () => { + await new Promise(resolve => { + starredEntitiesApi.starredEntities$().subscribe({ + next: (...args) => { + handler(...args); + + if (handler.mock.calls.length >= 2) { + resolve(); + } + }, + }); + + const bucket = mockStorage.forBucket('settings'); + bucket.set('starredEntities', ['entity:Component:default:mock']).then(); + }); + }); + + it('should receive updates', async () => { + expect(handler).toBeCalledTimes(2); + expect(handler).toBeCalledWith({ + starredEntities: new Set(), + isStarred: expect.any(Function), + }); + expect(handler).toBeCalledWith({ + starredEntities: new Set(['entity:Component:default:mock']), + isStarred: expect.any(Function), + }); + }); + + it('should receive isStarred function that operates on the latest state', async () => { + expect(handler.mock.calls[0][0].isStarred(mockEntity)).toBe(true); + expect(handler.mock.calls[1][0].isStarred(mockEntity)).toBe(true); + }); + }); +}); diff --git a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts new file mode 100644 index 0000000000..fce6541dca --- /dev/null +++ b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts @@ -0,0 +1,116 @@ +/* + * Copyright 2021 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 { Observable, StorageApi } from '@backstage/core-plugin-api'; +import ObservableImpl from 'zen-observable'; +import { + StarredEntitiesApi, + StarredEntitiesApiObservable, +} from './StarredEntitiesApi'; + +const buildEntityKey = (component: Entity) => + `entity:${component.kind}:${component.metadata.namespace ?? 'default'}:${ + component.metadata.name + }`; + +export class DefaultStarredEntitiesApi implements StarredEntitiesApi { + private readonly settingsStore: StorageApi; + private starredEntities: Set; + + constructor(opts: { storageApi: StorageApi }) { + this.settingsStore = opts.storageApi.forBucket('settings'); + + this.starredEntities = new Set( + this.settingsStore.get('starredEntities') ?? [], + ); + + this.settingsStore.observe$('starredEntities').subscribe({ + next: next => { + this.starredEntities = new Set(next.newValue ?? []); + this.notifyChanges(); + }, + }); + } + + async toggleStarred(entity: Entity): Promise { + const entityKey = buildEntityKey(entity); + + if (this.starredEntities.has(entityKey)) { + await this.unstar(entity); + } else { + await this.star(entity); + } + } + + async star(entity: Entity): Promise { + const entityKey = buildEntityKey(entity); + + this.starredEntities.add(entityKey); + + await this.settingsStore.set( + 'starredEntities', + Array.from(this.starredEntities), + ); + } + + async unstar(entity: Entity): Promise { + const entityKey = buildEntityKey(entity); + + this.starredEntities.delete(entityKey); + + await this.settingsStore.set( + 'starredEntities', + Array.from(this.starredEntities), + ); + } + + starredEntities$(): Observable { + return this.observable; + } + + isStarred(entity: Entity): boolean { + const entityKey = buildEntityKey(entity); + return this.starredEntities.has(entityKey); + } + + private readonly subscribers = new Set< + ZenObservable.SubscriptionObserver + >(); + + private readonly observable = + new ObservableImpl(subscriber => { + // forward the the latest value + subscriber.next({ + starredEntities: this.starredEntities, + isStarred: e => this.isStarred(e), + }); + + this.subscribers.add(subscriber); + return () => { + this.subscribers.delete(subscriber); + }; + }); + + private notifyChanges() { + for (const subscription of this.subscribers) { + subscription.next({ + starredEntities: this.starredEntities, + isStarred: e => this.isStarred(e), + }); + } + } +} diff --git a/plugins/catalog-react/src/apis/StarredEntitiesApi/StarredEntitiesApi.ts b/plugins/catalog-react/src/apis/StarredEntitiesApi/StarredEntitiesApi.ts new file mode 100644 index 0000000000..bf29a425ed --- /dev/null +++ b/plugins/catalog-react/src/apis/StarredEntitiesApi/StarredEntitiesApi.ts @@ -0,0 +1,71 @@ +/* + * Copyright 2021 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 { ApiRef, createApiRef, Observable } from '@backstage/core-plugin-api'; + +export const starredEntitiesApiRef: ApiRef = createApiRef({ + id: 'catalog-react.starred-entities', +}); + +export type StarredEntitiesApiObservable = { + /** + * A set of starred entities + */ + starredEntities: Set; + + /** + * A function to check if an entity is starred. + * + * @param entity - the entity to check + * @returns true, if the entity is starred. + */ + isStarred: (entity: Entity) => boolean; +}; + +/** + * An API to store and retrieve starred entities + * + * @public + */ +export interface StarredEntitiesApi { + /** + * Toggle the star state of the entity + * + * @param entity - the entity to be toggled + */ + toggleStarred(entity: Entity): Promise; + + /** + * Star the entity + * + * @param entity - the entity to be starred + */ + star(entity: Entity): Promise; + + /** + * Unstar the entity + * + * @param entity - the entity to be unstarred + */ + unstar(entity: Entity): Promise; + + /** + * Observe the state of starred entities and receive a handler + * to check the star state of an entity. + */ + starredEntities$(): Observable; +} diff --git a/plugins/catalog-react/src/apis/StarredEntitiesApi/index.ts b/plugins/catalog-react/src/apis/StarredEntitiesApi/index.ts new file mode 100644 index 0000000000..c71a795b9e --- /dev/null +++ b/plugins/catalog-react/src/apis/StarredEntitiesApi/index.ts @@ -0,0 +1,22 @@ +/* + * Copyright 2021 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. + */ + +export { DefaultStarredEntitiesApi } from './DefaultStarredEntitiesApi'; +export { starredEntitiesApiRef } from './StarredEntitiesApi'; +export type { + StarredEntitiesApi, + StarredEntitiesApiObservable, +} from './StarredEntitiesApi'; diff --git a/plugins/catalog-react/src/apis/index.ts b/plugins/catalog-react/src/apis/index.ts new file mode 100644 index 0000000000..5c7e980890 --- /dev/null +++ b/plugins/catalog-react/src/apis/index.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2021 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. + */ + +export * from './StarredEntitiesApi'; diff --git a/plugins/catalog-react/src/hooks/useEntityListProvider.test.tsx b/plugins/catalog-react/src/hooks/useEntityListProvider.test.tsx index 5d711016a4..e448ee73f1 100644 --- a/plugins/catalog-react/src/hooks/useEntityListProvider.test.tsx +++ b/plugins/catalog-react/src/hooks/useEntityListProvider.test.tsx @@ -14,21 +14,8 @@ * limitations under the License. */ -import React, { PropsWithChildren } from 'react'; -import qs from 'qs'; -import { act, renderHook } from '@testing-library/react-hooks'; -import { MockStorageApi } from '@backstage/test-utils'; import { CatalogApi } from '@backstage/catalog-client'; import { Entity } from '@backstage/catalog-model'; -import { - EntityListProvider, - useEntityListProvider, -} from './useEntityListProvider'; -import { catalogApiRef } from '../api'; -import { UserListFilterKind } from '../types'; -import { EntityKindFilter, EntityTypeFilter, UserListFilter } from '../filters'; -import { EntityKindPicker, UserListPicker } from '../components'; - import { ApiProvider, ApiRegistry } from '@backstage/core-app-api'; import { ConfigApi, @@ -37,6 +24,19 @@ import { identityApiRef, storageApiRef, } from '@backstage/core-plugin-api'; +import { MockStorageApi } from '@backstage/test-utils'; +import { act, renderHook } from '@testing-library/react-hooks'; +import qs from 'qs'; +import React, { PropsWithChildren } from 'react'; +import { catalogApiRef } from '../api'; +import { DefaultStarredEntitiesApi, starredEntitiesApiRef } from '../apis'; +import { EntityKindPicker, UserListPicker } from '../components'; +import { EntityKindFilter, EntityTypeFilter, UserListFilter } from '../filters'; +import { UserListFilterKind } from '../types'; +import { + EntityListProvider, + useEntityListProvider, +} from './useEntityListProvider'; const entities: Entity[] = [ { @@ -81,6 +81,10 @@ const apis = ApiRegistry.from([ [catalogApiRef, mockCatalogApi], [identityApiRef, mockIdentityApi], [storageApiRef, MockStorageApi.create()], + [ + starredEntitiesApiRef, + new DefaultStarredEntitiesApi({ storageApi: MockStorageApi.create() }), + ], ]); const wrapper = ({ diff --git a/plugins/catalog-react/src/hooks/useStarredEntities.test.tsx b/plugins/catalog-react/src/hooks/useStarredEntities.test.tsx index 50662312da..0379e49351 100644 --- a/plugins/catalog-react/src/hooks/useStarredEntities.test.tsx +++ b/plugins/catalog-react/src/hooks/useStarredEntities.test.tsx @@ -14,16 +14,18 @@ * limitations under the License. */ -import React, { PropsWithChildren } from 'react'; -import { renderHook, act } from '@testing-library/react-hooks'; -import { useStarredEntities } from './useStarredEntities'; -import { storageApiRef, StorageApi } from '@backstage/core-plugin-api'; -import { MockErrorApi } from '@backstage/test-utils'; import { Entity } from '@backstage/catalog-model'; -import { ApiProvider, ApiRegistry, WebStorage } from '@backstage/core-app-api'; +import { ApiProvider, ApiRegistry } from '@backstage/core-app-api'; +import { StorageApi } from '@backstage/core-plugin-api'; +import { MockStorageApi } from '@backstage/test-utils'; +import { act, renderHook } from '@testing-library/react-hooks'; +import React, { PropsWithChildren } from 'react'; +import { DefaultStarredEntitiesApi, starredEntitiesApiRef } from '../apis'; +import { useStarredEntities } from './useStarredEntities'; describe('useStarredEntities', () => { - let mockStorage: StorageApi | undefined; + let mockStorage: StorageApi; + let wrapper: React.ComponentType; const mockEntity: Entity = { apiVersion: '1', @@ -42,41 +44,56 @@ describe('useStarredEntities', () => { }, }; - const wrapper = ({ children }: PropsWithChildren<{}>) => { - return ( - + beforeEach(() => { + mockStorage = MockStorageApi.create(); + wrapper = ({ children }: PropsWithChildren<{}>) => ( + {children} ); - }; - - beforeEach(() => { - mockStorage = new WebStorage('@backstage', new MockErrorApi()).forBucket( - Date.now().toString(), // TODO(blam): need something that changes every test run for now until the MockStorage is implemented - ); }); + it('should return an empty set for when there is no items in storage', async () => { - const { result } = renderHook(() => useStarredEntities(), { wrapper }); + const { result, waitForNextUpdate } = renderHook( + () => useStarredEntities(), + { wrapper }, + ); + + await waitForNextUpdate(); expect(result.current.starredEntities.size).toBe(0); }); + it('should return a set with the current items when there are items in storage', async () => { const expectedIds = ['i', 'am', 'some', 'test', 'ids']; const store = mockStorage?.forBucket('settings'); await store?.set('starredEntities', expectedIds); - const { result } = renderHook(() => useStarredEntities(), { wrapper }); + const { result, waitForNextUpdate } = renderHook( + () => useStarredEntities(), + { wrapper }, + ); + + await waitForNextUpdate(); for (const item of expectedIds) { expect(result.current.starredEntities.has(item)).toBeTruthy(); } }); + it('should listen to changes when the storage is set elsewhere', async () => { const { result, waitForNextUpdate } = renderHook( () => useStarredEntities(), { wrapper }, ); + await waitForNextUpdate(); + expect(result.current.starredEntities.size).toBe(0); expect(result.current.isStarredEntity(mockEntity)).toBeFalsy(); @@ -91,18 +108,26 @@ describe('useStarredEntities', () => { }); it('should write new entries to the local store when adding a toggling entity', async () => { - const { result } = renderHook(() => useStarredEntities(), { wrapper }); + const { result, waitForNextUpdate } = renderHook( + () => useStarredEntities(), + { wrapper }, + ); act(() => { result.current.toggleStarredEntity(mockEntity); }); + await waitForNextUpdate(); + 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 }); + const { result, waitForNextUpdate } = renderHook( + () => useStarredEntities(), + { wrapper }, + ); act(() => { result.current.toggleStarredEntity(mockEntity); @@ -110,6 +135,8 @@ describe('useStarredEntities', () => { result.current.toggleStarredEntity(mockEntity); }); + await waitForNextUpdate(); + expect(result.current.isStarredEntity(mockEntity)).toBeFalsy(); expect(result.current.isStarredEntity(secondMockEntity)).toBeTruthy(); }); diff --git a/plugins/catalog-react/src/hooks/useStarredEntities.ts b/plugins/catalog-react/src/hooks/useStarredEntities.ts index 6f7b895fc8..56a0ee6c3c 100644 --- a/plugins/catalog-react/src/hooks/useStarredEntities.ts +++ b/plugins/catalog-react/src/hooks/useStarredEntities.ts @@ -15,56 +15,24 @@ */ import { Entity } from '@backstage/catalog-model'; -import { storageApiRef, useApi } from '@backstage/core-plugin-api'; -import { useCallback, useEffect, useState } from 'react'; +import { useApi } from '@backstage/core-plugin-api'; +import { useCallback } from 'react'; import { useObservable } from 'react-use'; - -const buildEntityKey = (component: Entity) => - `entity:${component.kind}:${component.metadata.namespace ?? 'default'}:${ - component.metadata.name - }`; +import { starredEntitiesApiRef } from '../apis'; export const useStarredEntities = () => { - const storageApi = useApi(storageApiRef); - const settingsStore = storageApi.forBucket('settings'); - const rawStarredEntityKeys = - settingsStore.get('starredEntities') ?? []; + const starredEntitiesApi = useApi(starredEntitiesApiRef); - const [starredEntities, setStarredEntities] = useState( - new Set(rawStarredEntityKeys), + const { starredEntities, isStarred: isStarredEntity } = useObservable( + starredEntitiesApi.starredEntities$(), + { starredEntities: new Set(), isStarred: _ => false }, ); - const observedItems = useObservable( - settingsStore.observe$('starredEntities'), - ); - - useEffect(() => { - if (observedItems?.newValue) { - const currentValue = observedItems?.newValue ?? []; - setStarredEntities(new Set(currentValue)); - } - }, [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)); + starredEntitiesApi.toggleStarred(entity).then(); }, - [starredEntities, settingsStore], - ); - - const isStarredEntity = useCallback( - (entity: Entity) => { - const entityKey = buildEntityKey(entity); - return starredEntities.has(entityKey); - }, - [starredEntities], + [starredEntitiesApi], ); return { diff --git a/plugins/catalog-react/src/index.ts b/plugins/catalog-react/src/index.ts index 1ac67d514b..cf7a679f7a 100644 --- a/plugins/catalog-react/src/index.ts +++ b/plugins/catalog-react/src/index.ts @@ -23,6 +23,7 @@ export type { CatalogApi } from '@backstage/catalog-client'; export { CATALOG_FILTER_EXISTS } from '@backstage/catalog-client'; export { catalogApiRef } from './api'; +export * from './apis'; export * from './components'; export * from './hooks'; export * from './filters'; diff --git a/plugins/catalog/src/components/CatalogPage/CatalogPage.test.tsx b/plugins/catalog/src/components/CatalogPage/CatalogPage.test.tsx index 48ec1ef176..6b33b33b08 100644 --- a/plugins/catalog/src/components/CatalogPage/CatalogPage.test.tsx +++ b/plugins/catalog/src/components/CatalogPage/CatalogPage.test.tsx @@ -20,28 +20,32 @@ import { RELATION_MEMBER_OF, RELATION_OWNED_BY, } from '@backstage/catalog-model'; -import { TableColumn, TableProps } from '@backstage/core-components'; -import { catalogApiRef, entityRouteRef } from '@backstage/plugin-catalog-react'; -import { - MockStorageApi, - renderWithEffects, - wrapInTestApp, - mockBreakpoint, -} from '@backstage/test-utils'; -import { fireEvent, screen } from '@testing-library/react'; -import React from 'react'; -import { createComponentRouteRef } from '../../routes'; -import { EntityRow } from '../CatalogTable/types'; -import { CatalogPage } from './CatalogPage'; -import DashboardIcon from '@material-ui/icons/Dashboard'; - import { ApiProvider, ApiRegistry } from '@backstage/core-app-api'; +import { TableColumn, TableProps } from '@backstage/core-components'; import { IdentityApi, identityApiRef, ProfileInfo, storageApiRef, } from '@backstage/core-plugin-api'; +import { + catalogApiRef, + DefaultStarredEntitiesApi, + entityRouteRef, + starredEntitiesApiRef, +} from '@backstage/plugin-catalog-react'; +import { + mockBreakpoint, + MockStorageApi, + renderWithEffects, + wrapInTestApp, +} from '@backstage/test-utils'; +import DashboardIcon from '@material-ui/icons/Dashboard'; +import { fireEvent, screen } from '@testing-library/react'; +import React from 'react'; +import { createComponentRouteRef } from '../../routes'; +import { EntityRow } from '../CatalogTable'; +import { CatalogPage } from './CatalogPage'; describe('CatalogPage', () => { const origReplaceState = window.history.replaceState; @@ -120,6 +124,7 @@ describe('CatalogPage', () => { getIdToken: async () => undefined, getProfile: () => testProfile, }; + const storageApi = MockStorageApi.create(); const renderWrapped = (children: React.ReactNode) => renderWithEffects( @@ -128,7 +133,11 @@ describe('CatalogPage', () => { apis={ApiRegistry.from([ [catalogApiRef, catalogApi], [identityApiRef, identityApi], - [storageApiRef, MockStorageApi.create()], + [storageApiRef, storageApi], + [ + starredEntitiesApiRef, + new DefaultStarredEntitiesApi({ storageApi }), + ], ])} > {children} diff --git a/plugins/catalog/src/components/CatalogTable/CatalogTable.test.tsx b/plugins/catalog/src/components/CatalogTable/CatalogTable.test.tsx index 6c80e64fa9..822913a385 100644 --- a/plugins/catalog/src/components/CatalogTable/CatalogTable.test.tsx +++ b/plugins/catalog/src/components/CatalogTable/CatalogTable.test.tsx @@ -15,19 +15,22 @@ */ import { + EDIT_URL_ANNOTATION, Entity, VIEW_URL_ANNOTATION, - EDIT_URL_ANNOTATION, } from '@backstage/catalog-model'; -import { act, fireEvent } from '@testing-library/react'; -import { renderInTestApp } from '@backstage/test-utils'; -import * as React from 'react'; -import { CatalogTable } from './CatalogTable'; +import { ApiProvider, ApiRegistry } from '@backstage/core-app-api'; import { entityRouteRef, + DefaultStarredEntitiesApi, MockEntityListContextProvider, + starredEntitiesApiRef, UserListFilter, } from '@backstage/plugin-catalog-react'; +import { MockStorageApi, renderInTestApp } from '@backstage/test-utils'; +import { act, fireEvent } from '@testing-library/react'; +import * as React from 'react'; +import { CatalogTable } from './CatalogTable'; const entities: Entity[] = [ { @@ -48,6 +51,11 @@ const entities: Entity[] = [ ]; describe('CatalogTable component', () => { + const mockApis = ApiRegistry.with( + starredEntitiesApiRef, + new DefaultStarredEntitiesApi({ storageApi: MockStorageApi.create() }), + ); + beforeEach(() => { window.open = jest.fn(); }); @@ -58,9 +66,11 @@ describe('CatalogTable component', () => { it('should render error message', async () => { const rendered = await renderInTestApp( - - - , + + + + + , { mountedRoutes: { '/catalog/:namespace/:kind/:name': entityRouteRef, @@ -75,20 +85,22 @@ describe('CatalogTable component', () => { it('should display entity names when loading has finished and no error occurred', async () => { const rendered = await renderInTestApp( - false, - () => false, - ), - }, - }} - > - - , + + false, + () => false, + ), + }, + }} + > + + + , { mountedRoutes: { '/catalog/:namespace/:kind/:name': entityRouteRef, @@ -112,9 +124,11 @@ describe('CatalogTable component', () => { }; const { getByTitle } = await renderInTestApp( - - - , + + + + + , { mountedRoutes: { '/catalog/:namespace/:kind/:name': entityRouteRef, @@ -142,9 +156,11 @@ describe('CatalogTable component', () => { }; const { getByTitle } = await renderInTestApp( - - - , + + + + + , { mountedRoutes: { '/catalog/:namespace/:kind/:name': entityRouteRef, diff --git a/plugins/catalog/src/components/EntityLayout/EntityLayout.test.tsx b/plugins/catalog/src/components/EntityLayout/EntityLayout.test.tsx index 7722f6ba17..2216070984 100644 --- a/plugins/catalog/src/components/EntityLayout/EntityLayout.test.tsx +++ b/plugins/catalog/src/components/EntityLayout/EntityLayout.test.tsx @@ -13,24 +13,26 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { CatalogApi } from '@backstage/catalog-client'; import { Entity } from '@backstage/catalog-model'; +import { ApiProvider, ApiRegistry } from '@backstage/core-app-api'; +import { AlertApi, alertApiRef } from '@backstage/core-plugin-api'; import { - catalogApiRef, - EntityProvider, AsyncEntityProvider, + catalogApiRef, + DefaultStarredEntitiesApi, + EntityProvider, entityRouteRef, + starredEntitiesApiRef, } from '@backstage/plugin-catalog-react'; -import { renderInTestApp } from '@backstage/test-utils'; +import { MockStorageApi, renderInTestApp } from '@backstage/test-utils'; import { fireEvent } from '@testing-library/react'; import React from 'react'; import { act } from 'react-dom/test-utils'; import { Route, Routes } from 'react-router'; import { EntityLayout } from './EntityLayout'; -import { AlertApi, alertApiRef } from '@backstage/core-plugin-api'; -import { ApiProvider, ApiRegistry } from '@backstage/core-app-api'; - const mockEntity = { kind: 'MyKind', metadata: { @@ -38,10 +40,12 @@ const mockEntity = { }, } as Entity; -const mockApis = ApiRegistry.with(catalogApiRef, {} as CatalogApi).with( - alertApiRef, - {} as AlertApi, -); +const mockApis = ApiRegistry.with(catalogApiRef, {} as CatalogApi) + .with(alertApiRef, {} as AlertApi) + .with( + starredEntitiesApiRef, + new DefaultStarredEntitiesApi({ storageApi: MockStorageApi.create() }), + ); describe('EntityLayout', () => { it('renders simplest case', async () => { diff --git a/plugins/catalog/src/plugin.ts b/plugins/catalog/src/plugin.ts index 9b90a44aff..20f937b2e7 100644 --- a/plugins/catalog/src/plugin.ts +++ b/plugins/catalog/src/plugin.ts @@ -18,7 +18,9 @@ import { CatalogClient } from '@backstage/catalog-client'; import { catalogApiRef, catalogRouteRef, + DefaultStarredEntitiesApi, entityRouteRef, + starredEntitiesApiRef, } from '@backstage/plugin-catalog-react'; import { CatalogClientWrapper } from './CatalogClientWrapper'; import { createComponentRouteRef, viewTechDocRouteRef } from './routes'; @@ -29,6 +31,7 @@ import { createRoutableExtension, discoveryApiRef, identityApiRef, + storageApiRef, } from '@backstage/core-plugin-api'; export const catalogPlugin = createPlugin({ @@ -43,6 +46,13 @@ export const catalogPlugin = createPlugin({ identityApi, }), }), + + createApiFactory({ + api: starredEntitiesApiRef, + deps: { storageApi: storageApiRef }, + factory: ({ storageApi }) => + new DefaultStarredEntitiesApi({ storageApi }), + }), ], routes: { catalogIndex: catalogRouteRef, diff --git a/plugins/techdocs/src/home/components/DefaultTechDocsHome.test.tsx b/plugins/techdocs/src/home/components/DefaultTechDocsHome.test.tsx index 48ea67585b..0ba1f510cd 100644 --- a/plugins/techdocs/src/home/components/DefaultTechDocsHome.test.tsx +++ b/plugins/techdocs/src/home/components/DefaultTechDocsHome.test.tsx @@ -14,12 +14,6 @@ * limitations under the License. */ -import { CatalogApi, catalogApiRef } from '@backstage/plugin-catalog-react'; -import { MockStorageApi, renderInTestApp } from '@backstage/test-utils'; -import { screen } from '@testing-library/react'; -import React from 'react'; -import { DefaultTechDocsHome } from './DefaultTechDocsHome'; - import { ApiProvider, ApiRegistry, @@ -30,7 +24,17 @@ import { configApiRef, storageApiRef, } from '@backstage/core-plugin-api'; +import { + CatalogApi, + catalogApiRef, + DefaultStarredEntitiesApi, + starredEntitiesApiRef, +} from '@backstage/plugin-catalog-react'; +import { MockStorageApi, renderInTestApp } from '@backstage/test-utils'; +import { screen } from '@testing-library/react'; +import React from 'react'; import { rootDocsRouteRef } from '../../routes'; +import { DefaultTechDocsHome } from './DefaultTechDocsHome'; jest.mock('@backstage/plugin-catalog-react', () => { const actual = jest.requireActual('@backstage/plugin-catalog-react'); @@ -63,10 +67,13 @@ describe('TechDocs Home', () => { }, }); + const storageApi = MockStorageApi.create(); + const apiRegistry = ApiRegistry.from([ [catalogApiRef, mockCatalogApi], [configApiRef, configApi], [storageApiRef, MockStorageApi.create()], + [starredEntitiesApiRef, new DefaultStarredEntitiesApi({ storageApi })], ]); it('should render a TechDocs home page', async () => { From 126daa5ec34391b4e2a4389981a188b30425b53d Mon Sep 17 00:00:00 2001 From: Dominik Henneke Date: Mon, 11 Oct 2021 11:22:13 +0200 Subject: [PATCH 03/13] Resolve PR comments * rename starredEntities$ to starredEntitie$ * delete star and unstar from the public interface Signed-off-by: Dominik Henneke --- plugins/catalog-react/api-report.md | 6 ++---- .../DefaultStarredEntitiesApi.test.ts | 2 +- .../DefaultStarredEntitiesApi.ts | 2 +- .../StarredEntitiesApi/StarredEntitiesApi.ts | 16 +--------------- .../src/hooks/useStarredEntities.ts | 2 +- 5 files changed, 6 insertions(+), 22 deletions(-) diff --git a/plugins/catalog-react/api-report.md b/plugins/catalog-react/api-report.md index e176d56129..0accacce3e 100644 --- a/plugins/catalog-react/api-report.md +++ b/plugins/catalog-react/api-report.md @@ -143,7 +143,7 @@ export class DefaultStarredEntitiesApi implements StarredEntitiesApi { // (undocumented) star(entity: Entity): Promise; // (undocumented) - starredEntities$(): Observable; + starredEntitie$(): Observable; // (undocumented) toggleStarred(entity: Entity): Promise; // (undocumented) @@ -763,10 +763,8 @@ export const rootRoute: RouteRef; // @public export interface StarredEntitiesApi { - star(entity: Entity): Promise; - starredEntities$(): Observable; + starredEntitie$(): Observable; toggleStarred(entity: Entity): Promise; - unstar(entity: Entity): Promise; } // Warning: (ae-missing-release-tag) "StarredEntitiesApiObservable" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) diff --git a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts index 6a4f90200b..c182bdad39 100644 --- a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts +++ b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts @@ -110,7 +110,7 @@ describe('DefaultStarredEntitiesApi', () => { beforeEach(async () => { await new Promise(resolve => { - starredEntitiesApi.starredEntities$().subscribe({ + starredEntitiesApi.starredEntitie$().subscribe({ next: (...args) => { handler(...args); diff --git a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts index fce6541dca..6080cc4e39 100644 --- a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts +++ b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts @@ -78,7 +78,7 @@ export class DefaultStarredEntitiesApi implements StarredEntitiesApi { ); } - starredEntities$(): Observable { + starredEntitie$(): Observable { return this.observable; } diff --git a/plugins/catalog-react/src/apis/StarredEntitiesApi/StarredEntitiesApi.ts b/plugins/catalog-react/src/apis/StarredEntitiesApi/StarredEntitiesApi.ts index bf29a425ed..f64298227e 100644 --- a/plugins/catalog-react/src/apis/StarredEntitiesApi/StarredEntitiesApi.ts +++ b/plugins/catalog-react/src/apis/StarredEntitiesApi/StarredEntitiesApi.ts @@ -49,23 +49,9 @@ export interface StarredEntitiesApi { */ toggleStarred(entity: Entity): Promise; - /** - * Star the entity - * - * @param entity - the entity to be starred - */ - star(entity: Entity): Promise; - - /** - * Unstar the entity - * - * @param entity - the entity to be unstarred - */ - unstar(entity: Entity): Promise; - /** * Observe the state of starred entities and receive a handler * to check the star state of an entity. */ - starredEntities$(): Observable; + starredEntitie$(): Observable; } diff --git a/plugins/catalog-react/src/hooks/useStarredEntities.ts b/plugins/catalog-react/src/hooks/useStarredEntities.ts index 56a0ee6c3c..cb94d465bc 100644 --- a/plugins/catalog-react/src/hooks/useStarredEntities.ts +++ b/plugins/catalog-react/src/hooks/useStarredEntities.ts @@ -24,7 +24,7 @@ export const useStarredEntities = () => { const starredEntitiesApi = useApi(starredEntitiesApiRef); const { starredEntities, isStarred: isStarredEntity } = useObservable( - starredEntitiesApi.starredEntities$(), + starredEntitiesApi.starredEntitie$(), { starredEntities: new Set(), isStarred: _ => false }, ); From 8f55fb815b5e9ba8d75c96b1b9bf504c40a0ff3d Mon Sep 17 00:00:00 2001 From: Dominik Henneke Date: Tue, 12 Oct 2021 10:37:50 +0200 Subject: [PATCH 04/13] Fix test Signed-off-by: Dominik Henneke --- .../techdocs/src/home/components/DefaultTechDocsHome.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/techdocs/src/home/components/DefaultTechDocsHome.test.tsx b/plugins/techdocs/src/home/components/DefaultTechDocsHome.test.tsx index 0ba1f510cd..1d8d7d2b04 100644 --- a/plugins/techdocs/src/home/components/DefaultTechDocsHome.test.tsx +++ b/plugins/techdocs/src/home/components/DefaultTechDocsHome.test.tsx @@ -72,7 +72,7 @@ describe('TechDocs Home', () => { const apiRegistry = ApiRegistry.from([ [catalogApiRef, mockCatalogApi], [configApiRef, configApi], - [storageApiRef, MockStorageApi.create()], + [storageApiRef, storageApi], [starredEntitiesApiRef, new DefaultStarredEntitiesApi({ storageApi })], ]); From 56f0ef983e6bcaa1221beaa9cdfd1a94d51db1ee Mon Sep 17 00:00:00 2001 From: Dominik Henneke Date: Tue, 12 Oct 2021 10:16:30 +0200 Subject: [PATCH 05/13] Remove the star and unstar method from DefaultStarredEntitiesApi Signed-off-by: Dominik Henneke --- plugins/catalog-react/api-report.md | 4 -- .../DefaultStarredEntitiesApi.test.ts | 42 ------------------- .../DefaultStarredEntitiesApi.ts | 21 +--------- 3 files changed, 2 insertions(+), 65 deletions(-) diff --git a/plugins/catalog-react/api-report.md b/plugins/catalog-react/api-report.md index 0accacce3e..12ae143d8a 100644 --- a/plugins/catalog-react/api-report.md +++ b/plugins/catalog-react/api-report.md @@ -141,13 +141,9 @@ export class DefaultStarredEntitiesApi implements StarredEntitiesApi { // (undocumented) isStarred(entity: Entity): boolean; // (undocumented) - star(entity: Entity): Promise; - // (undocumented) starredEntitie$(): Observable; // (undocumented) toggleStarred(entity: Entity): Promise; - // (undocumented) - unstar(entity: Entity): Promise; } // Warning: (ae-forgotten-export) The symbol "EntityLoadingStatus" needs to be exported by the entry point index.d.ts diff --git a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts index c182bdad39..1edcdf706b 100644 --- a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts +++ b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts @@ -63,48 +63,6 @@ describe('DefaultStarredEntitiesApi', () => { }); }); - describe('star', () => { - it('should star unstarred entity', async () => { - expect(starredEntitiesApi.isStarred(mockEntity)).toBe(false); - - await starredEntitiesApi.star(mockEntity); - - expect(starredEntitiesApi.isStarred(mockEntity)).toBe(true); - }); - - it('should keep starred entity', async () => { - const bucket = mockStorage.forBucket('settings'); - await bucket.set('starredEntities', ['entity:Component:default:mock']); - - expect(starredEntitiesApi.isStarred(mockEntity)).toBe(true); - - await starredEntitiesApi.star(mockEntity); - - expect(starredEntitiesApi.isStarred(mockEntity)).toBe(true); - }); - }); - - describe('unstar', () => { - it('should unstar starred entity', async () => { - const bucket = mockStorage.forBucket('settings'); - await bucket.set('starredEntities', ['entity:Component:default:mock']); - - expect(starredEntitiesApi.isStarred(mockEntity)).toBe(true); - - await starredEntitiesApi.unstar(mockEntity); - - expect(starredEntitiesApi.isStarred(mockEntity)).toBe(false); - }); - - it('should keep unstarred entity', async () => { - expect(starredEntitiesApi.isStarred(mockEntity)).toBe(false); - - await starredEntitiesApi.unstar(mockEntity); - - expect(starredEntitiesApi.isStarred(mockEntity)).toBe(false); - }); - }); - describe('starredEntities$', () => { const handler = jest.fn(); diff --git a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts index 6080cc4e39..4c5ea63cf9 100644 --- a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts +++ b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts @@ -50,27 +50,10 @@ export class DefaultStarredEntitiesApi implements StarredEntitiesApi { const entityKey = buildEntityKey(entity); if (this.starredEntities.has(entityKey)) { - await this.unstar(entity); + this.starredEntities.delete(entityKey); } else { - await this.star(entity); + this.starredEntities.add(entityKey); } - } - - async star(entity: Entity): Promise { - const entityKey = buildEntityKey(entity); - - this.starredEntities.add(entityKey); - - await this.settingsStore.set( - 'starredEntities', - Array.from(this.starredEntities), - ); - } - - async unstar(entity: Entity): Promise { - const entityKey = buildEntityKey(entity); - - this.starredEntities.delete(entityKey); await this.settingsStore.set( 'starredEntities', From 615f668d317afbce4466f98cc811afa218e31c06 Mon Sep 17 00:00:00 2001 From: Dominik Henneke Date: Tue, 12 Oct 2021 11:02:30 +0200 Subject: [PATCH 06/13] Use entity refs as a storage format instead of the custom one Signed-off-by: Dominik Henneke --- .changeset/cool-deers-cough.md | 2 ++ plugins/catalog-react/api-report.md | 4 +--- .../DefaultStarredEntitiesApi.test.ts | 6 +++--- .../DefaultStarredEntitiesApi.ts | 16 ++++++++-------- .../StarredEntitiesApi/StarredEntitiesApi.ts | 2 +- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/.changeset/cool-deers-cough.md b/.changeset/cool-deers-cough.md index 6d3e923d7d..c2619d1fa2 100644 --- a/.changeset/cool-deers-cough.md +++ b/.changeset/cool-deers-cough.md @@ -5,3 +5,5 @@ Introduce a new `StarredEntitiesApi` that is used in the `useStarredEntities` hook. The `@backstage/plugin-catalog` installs a default implementation that is backed by the `StorageApi`, but one can also override the `starredEntitiesApiRef`. + +**BREAKING** All previously stored entities get lost with this update, because the storage format has been migrated from a custom string to an entity reference. diff --git a/plugins/catalog-react/api-report.md b/plugins/catalog-react/api-report.md index 12ae143d8a..ea4c51adda 100644 --- a/plugins/catalog-react/api-report.md +++ b/plugins/catalog-react/api-report.md @@ -133,9 +133,7 @@ export type DefaultEntityFilters = { text?: EntityTextFilter; }; -// Warning: (ae-missing-release-tag) "DefaultStarredEntitiesApi" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) -// -// @public (undocumented) +// @public export class DefaultStarredEntitiesApi implements StarredEntitiesApi { constructor(opts: { storageApi: StorageApi }); // (undocumented) diff --git a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts index 1edcdf706b..eec484863f 100644 --- a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts +++ b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts @@ -53,7 +53,7 @@ describe('DefaultStarredEntitiesApi', () => { it('should unstar starred entity', async () => { const bucket = mockStorage.forBucket('settings'); - await bucket.set('starredEntities', ['entity:Component:default:mock']); + await bucket.set('starredEntities', ['component:default/mock']); expect(starredEntitiesApi.isStarred(mockEntity)).toBe(true); @@ -79,7 +79,7 @@ describe('DefaultStarredEntitiesApi', () => { }); const bucket = mockStorage.forBucket('settings'); - bucket.set('starredEntities', ['entity:Component:default:mock']).then(); + bucket.set('starredEntities', ['component:default/mock']).then(); }); }); @@ -90,7 +90,7 @@ describe('DefaultStarredEntitiesApi', () => { isStarred: expect.any(Function), }); expect(handler).toBeCalledWith({ - starredEntities: new Set(['entity:Component:default:mock']), + starredEntities: new Set(['component:default/mock']), isStarred: expect.any(Function), }); }); diff --git a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts index 4c5ea63cf9..5783954fbc 100644 --- a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts +++ b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { Entity } from '@backstage/catalog-model'; +import { Entity, stringifyEntityRef } from '@backstage/catalog-model'; import { Observable, StorageApi } from '@backstage/core-plugin-api'; import ObservableImpl from 'zen-observable'; import { @@ -22,11 +22,11 @@ import { StarredEntitiesApiObservable, } from './StarredEntitiesApi'; -const buildEntityKey = (component: Entity) => - `entity:${component.kind}:${component.metadata.namespace ?? 'default'}:${ - component.metadata.name - }`; - +/** + * Default implementation of the StarredEntitiesApi that is backed by the StorageApi. + * + * @public + */ export class DefaultStarredEntitiesApi implements StarredEntitiesApi { private readonly settingsStore: StorageApi; private starredEntities: Set; @@ -47,7 +47,7 @@ export class DefaultStarredEntitiesApi implements StarredEntitiesApi { } async toggleStarred(entity: Entity): Promise { - const entityKey = buildEntityKey(entity); + const entityKey = stringifyEntityRef(entity); if (this.starredEntities.has(entityKey)) { this.starredEntities.delete(entityKey); @@ -66,7 +66,7 @@ export class DefaultStarredEntitiesApi implements StarredEntitiesApi { } isStarred(entity: Entity): boolean { - const entityKey = buildEntityKey(entity); + const entityKey = stringifyEntityRef(entity); return this.starredEntities.has(entityKey); } diff --git a/plugins/catalog-react/src/apis/StarredEntitiesApi/StarredEntitiesApi.ts b/plugins/catalog-react/src/apis/StarredEntitiesApi/StarredEntitiesApi.ts index f64298227e..0c8ec52445 100644 --- a/plugins/catalog-react/src/apis/StarredEntitiesApi/StarredEntitiesApi.ts +++ b/plugins/catalog-react/src/apis/StarredEntitiesApi/StarredEntitiesApi.ts @@ -23,7 +23,7 @@ export const starredEntitiesApiRef: ApiRef = createApiRef({ export type StarredEntitiesApiObservable = { /** - * A set of starred entities + * A set of entity references that are starred */ starredEntities: Set; From 78fcb898c492001e1ebbabe2b8e2f7fe9a67c2e5 Mon Sep 17 00:00:00 2001 From: Dominik Henneke Date: Tue, 12 Oct 2021 10:37:07 +0200 Subject: [PATCH 07/13] Rename the storage location of the starred entities from `/settings/starredEntities` to `/starredEntitites/entitifyRefs` Signed-off-by: Dominik Henneke --- .changeset/cool-deers-cough.md | 5 ++++- .../StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts | 8 ++++---- .../apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts | 8 ++++---- .../catalog-react/src/hooks/useStarredEntities.test.tsx | 4 ++-- 4 files changed, 14 insertions(+), 11 deletions(-) diff --git a/.changeset/cool-deers-cough.md b/.changeset/cool-deers-cough.md index c2619d1fa2..0c07c91b96 100644 --- a/.changeset/cool-deers-cough.md +++ b/.changeset/cool-deers-cough.md @@ -6,4 +6,7 @@ Introduce a new `StarredEntitiesApi` that is used in the `useStarredEntities` hook. The `@backstage/plugin-catalog` installs a default implementation that is backed by the `StorageApi`, but one can also override the `starredEntitiesApiRef`. -**BREAKING** All previously stored entities get lost with this update, because the storage format has been migrated from a custom string to an entity reference. +**BREAKING** All previously stored entities get lost with this update, because: + +1. The storage format has been migrated from a custom string to an entity reference. +2. The storage key in the local storage has been changed. diff --git a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts index eec484863f..95375aa0f6 100644 --- a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts +++ b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts @@ -52,8 +52,8 @@ describe('DefaultStarredEntitiesApi', () => { }); it('should unstar starred entity', async () => { - const bucket = mockStorage.forBucket('settings'); - await bucket.set('starredEntities', ['component:default/mock']); + const bucket = mockStorage.forBucket('starredEntities'); + await bucket.set('entityRefs', ['component:default/mock']); expect(starredEntitiesApi.isStarred(mockEntity)).toBe(true); @@ -78,8 +78,8 @@ describe('DefaultStarredEntitiesApi', () => { }, }); - const bucket = mockStorage.forBucket('settings'); - bucket.set('starredEntities', ['component:default/mock']).then(); + const bucket = mockStorage.forBucket('starredEntities'); + bucket.set('entityRefs', ['component:default/mock']).then(); }); }); diff --git a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts index 5783954fbc..fa775679ca 100644 --- a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts +++ b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts @@ -32,13 +32,13 @@ export class DefaultStarredEntitiesApi implements StarredEntitiesApi { private starredEntities: Set; constructor(opts: { storageApi: StorageApi }) { - this.settingsStore = opts.storageApi.forBucket('settings'); + this.settingsStore = opts.storageApi.forBucket('starredEntities'); this.starredEntities = new Set( - this.settingsStore.get('starredEntities') ?? [], + this.settingsStore.get('entityRefs') ?? [], ); - this.settingsStore.observe$('starredEntities').subscribe({ + this.settingsStore.observe$('entityRefs').subscribe({ next: next => { this.starredEntities = new Set(next.newValue ?? []); this.notifyChanges(); @@ -56,7 +56,7 @@ export class DefaultStarredEntitiesApi implements StarredEntitiesApi { } await this.settingsStore.set( - 'starredEntities', + 'entityRefs', Array.from(this.starredEntities), ); } diff --git a/plugins/catalog-react/src/hooks/useStarredEntities.test.tsx b/plugins/catalog-react/src/hooks/useStarredEntities.test.tsx index 0379e49351..4f41ef090f 100644 --- a/plugins/catalog-react/src/hooks/useStarredEntities.test.tsx +++ b/plugins/catalog-react/src/hooks/useStarredEntities.test.tsx @@ -71,8 +71,8 @@ describe('useStarredEntities', () => { it('should return a set with the current items when there are items in storage', async () => { const expectedIds = ['i', 'am', 'some', 'test', 'ids']; - const store = mockStorage?.forBucket('settings'); - await store?.set('starredEntities', expectedIds); + const store = mockStorage?.forBucket('starredEntities'); + await store?.set('entityRefs', expectedIds); const { result, waitForNextUpdate } = renderHook( () => useStarredEntities(), From c1d50273de5674f1e211cd148c15366eb323741d Mon Sep 17 00:00:00 2001 From: Dominik Henneke Date: Tue, 12 Oct 2021 10:54:49 +0200 Subject: [PATCH 08/13] Use entity references in the StarredEntitiesApi Signed-off-by: Dominik Henneke --- plugins/catalog-react/api-report.md | 8 +++---- .../DefaultStarredEntitiesApi.test.ts | 22 +++++++++---------- .../DefaultStarredEntitiesApi.ts | 16 +++++--------- .../StarredEntitiesApi/StarredEntitiesApi.ts | 9 ++++---- .../src/hooks/useStarredEntities.ts | 11 +++++++--- 5 files changed, 33 insertions(+), 33 deletions(-) diff --git a/plugins/catalog-react/api-report.md b/plugins/catalog-react/api-report.md index ea4c51adda..8670830584 100644 --- a/plugins/catalog-react/api-report.md +++ b/plugins/catalog-react/api-report.md @@ -137,11 +137,11 @@ export type DefaultEntityFilters = { export class DefaultStarredEntitiesApi implements StarredEntitiesApi { constructor(opts: { storageApi: StorageApi }); // (undocumented) - isStarred(entity: Entity): boolean; + isStarred(entityRef: string): boolean; // (undocumented) starredEntitie$(): Observable; // (undocumented) - toggleStarred(entity: Entity): Promise; + toggleStarred(entityRef: string): Promise; } // Warning: (ae-forgotten-export) The symbol "EntityLoadingStatus" needs to be exported by the entry point index.d.ts @@ -758,7 +758,7 @@ export const rootRoute: RouteRef; // @public export interface StarredEntitiesApi { starredEntitie$(): Observable; - toggleStarred(entity: Entity): Promise; + toggleStarred(entityRef: string): Promise; } // Warning: (ae-missing-release-tag) "StarredEntitiesApiObservable" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) @@ -766,7 +766,7 @@ export interface StarredEntitiesApi { // @public (undocumented) export type StarredEntitiesApiObservable = { starredEntities: Set; - isStarred: (entity: Entity) => boolean; + isStarred: (entityRef: string) => boolean; }; // Warning: (ae-missing-release-tag) "starredEntitiesApiRef" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) diff --git a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts index 95375aa0f6..e4f7c4ffa1 100644 --- a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts +++ b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { Entity } from '@backstage/catalog-model'; +import { stringifyEntityRef } from '@backstage/catalog-model'; import { StorageApi } from '@backstage/core-plugin-api'; import { MockStorageApi } from '@backstage/test-utils'; import { DefaultStarredEntitiesApi } from './DefaultStarredEntitiesApi'; @@ -23,13 +23,13 @@ describe('DefaultStarredEntitiesApi', () => { let mockStorage: StorageApi; let starredEntitiesApi: DefaultStarredEntitiesApi; - const mockEntity: Entity = { + const mockEntityRef = stringifyEntityRef({ apiVersion: '1', kind: 'Component', metadata: { name: 'mock', }, - }; + }); beforeEach(() => { mockStorage = MockStorageApi.create(); @@ -44,22 +44,22 @@ describe('DefaultStarredEntitiesApi', () => { describe('toggleStarred', () => { it('should star unstarred entity', async () => { - expect(starredEntitiesApi.isStarred(mockEntity)).toBe(false); + expect(starredEntitiesApi.isStarred(mockEntityRef)).toBe(false); - await starredEntitiesApi.toggleStarred(mockEntity); + await starredEntitiesApi.toggleStarred(mockEntityRef); - expect(starredEntitiesApi.isStarred(mockEntity)).toBe(true); + expect(starredEntitiesApi.isStarred(mockEntityRef)).toBe(true); }); it('should unstar starred entity', async () => { const bucket = mockStorage.forBucket('starredEntities'); await bucket.set('entityRefs', ['component:default/mock']); - expect(starredEntitiesApi.isStarred(mockEntity)).toBe(true); + expect(starredEntitiesApi.isStarred(mockEntityRef)).toBe(true); - await starredEntitiesApi.toggleStarred(mockEntity); + await starredEntitiesApi.toggleStarred(mockEntityRef); - expect(starredEntitiesApi.isStarred(mockEntity)).toBe(false); + expect(starredEntitiesApi.isStarred(mockEntityRef)).toBe(false); }); }); @@ -96,8 +96,8 @@ describe('DefaultStarredEntitiesApi', () => { }); it('should receive isStarred function that operates on the latest state', async () => { - expect(handler.mock.calls[0][0].isStarred(mockEntity)).toBe(true); - expect(handler.mock.calls[1][0].isStarred(mockEntity)).toBe(true); + expect(handler.mock.calls[0][0].isStarred(mockEntityRef)).toBe(true); + expect(handler.mock.calls[1][0].isStarred(mockEntityRef)).toBe(true); }); }); }); diff --git a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts index fa775679ca..8486d180b7 100644 --- a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts +++ b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -import { Entity, stringifyEntityRef } from '@backstage/catalog-model'; import { Observable, StorageApi } from '@backstage/core-plugin-api'; import ObservableImpl from 'zen-observable'; import { @@ -46,13 +45,11 @@ export class DefaultStarredEntitiesApi implements StarredEntitiesApi { }); } - async toggleStarred(entity: Entity): Promise { - const entityKey = stringifyEntityRef(entity); - - if (this.starredEntities.has(entityKey)) { - this.starredEntities.delete(entityKey); + async toggleStarred(entityRef: string): Promise { + if (this.starredEntities.has(entityRef)) { + this.starredEntities.delete(entityRef); } else { - this.starredEntities.add(entityKey); + this.starredEntities.add(entityRef); } await this.settingsStore.set( @@ -65,9 +62,8 @@ export class DefaultStarredEntitiesApi implements StarredEntitiesApi { return this.observable; } - isStarred(entity: Entity): boolean { - const entityKey = stringifyEntityRef(entity); - return this.starredEntities.has(entityKey); + isStarred(entityRef: string): boolean { + return this.starredEntities.has(entityRef); } private readonly subscribers = new Set< diff --git a/plugins/catalog-react/src/apis/StarredEntitiesApi/StarredEntitiesApi.ts b/plugins/catalog-react/src/apis/StarredEntitiesApi/StarredEntitiesApi.ts index 0c8ec52445..68eae0b0a1 100644 --- a/plugins/catalog-react/src/apis/StarredEntitiesApi/StarredEntitiesApi.ts +++ b/plugins/catalog-react/src/apis/StarredEntitiesApi/StarredEntitiesApi.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -import { Entity } from '@backstage/catalog-model'; import { ApiRef, createApiRef, Observable } from '@backstage/core-plugin-api'; export const starredEntitiesApiRef: ApiRef = createApiRef({ @@ -30,10 +29,10 @@ export type StarredEntitiesApiObservable = { /** * A function to check if an entity is starred. * - * @param entity - the entity to check + * @param entityRef - an entity reference to check * @returns true, if the entity is starred. */ - isStarred: (entity: Entity) => boolean; + isStarred: (entityRef: string) => boolean; }; /** @@ -45,9 +44,9 @@ export interface StarredEntitiesApi { /** * Toggle the star state of the entity * - * @param entity - the entity to be toggled + * @param entityRef - an entity reference to toggle */ - toggleStarred(entity: Entity): Promise; + toggleStarred(entityRef: string): Promise; /** * Observe the state of starred entities and receive a handler diff --git a/plugins/catalog-react/src/hooks/useStarredEntities.ts b/plugins/catalog-react/src/hooks/useStarredEntities.ts index cb94d465bc..e79e605877 100644 --- a/plugins/catalog-react/src/hooks/useStarredEntities.ts +++ b/plugins/catalog-react/src/hooks/useStarredEntities.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { Entity } from '@backstage/catalog-model'; +import { Entity, stringifyEntityRef } from '@backstage/catalog-model'; import { useApi } from '@backstage/core-plugin-api'; import { useCallback } from 'react'; import { useObservable } from 'react-use'; @@ -23,14 +23,19 @@ import { starredEntitiesApiRef } from '../apis'; export const useStarredEntities = () => { const starredEntitiesApi = useApi(starredEntitiesApiRef); - const { starredEntities, isStarred: isStarredEntity } = useObservable( + const { starredEntities, isStarred } = useObservable( starredEntitiesApi.starredEntitie$(), { starredEntities: new Set(), isStarred: _ => false }, ); + const isStarredEntity = useCallback( + (entity: Entity) => isStarred(stringifyEntityRef(entity)), + [isStarred], + ); + const toggleStarredEntity = useCallback( (entity: Entity) => { - starredEntitiesApi.toggleStarred(entity).then(); + starredEntitiesApi.toggleStarred(stringifyEntityRef(entity)).then(); }, [starredEntitiesApi], ); From bd924fece8b82161dc2e84b079dced5223aa81de Mon Sep 17 00:00:00 2001 From: Dominik Henneke Date: Tue, 12 Oct 2021 11:09:51 +0200 Subject: [PATCH 09/13] Add additional docs Signed-off-by: Dominik Henneke --- plugins/catalog-react/api-report.md | 6 +----- .../src/apis/StarredEntitiesApi/StarredEntitiesApi.ts | 8 ++++++++ 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/plugins/catalog-react/api-report.md b/plugins/catalog-react/api-report.md index 8670830584..1a37f84df3 100644 --- a/plugins/catalog-react/api-report.md +++ b/plugins/catalog-react/api-report.md @@ -761,17 +761,13 @@ export interface StarredEntitiesApi { toggleStarred(entityRef: string): Promise; } -// Warning: (ae-missing-release-tag) "StarredEntitiesApiObservable" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) -// // @public (undocumented) export type StarredEntitiesApiObservable = { starredEntities: Set; isStarred: (entityRef: string) => boolean; }; -// Warning: (ae-missing-release-tag) "starredEntitiesApiRef" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) -// -// @public (undocumented) +// @public export const starredEntitiesApiRef: ApiRef; // Warning: (ae-forgotten-export) The symbol "Props" needs to be exported by the entry point index.d.ts diff --git a/plugins/catalog-react/src/apis/StarredEntitiesApi/StarredEntitiesApi.ts b/plugins/catalog-react/src/apis/StarredEntitiesApi/StarredEntitiesApi.ts index 68eae0b0a1..93ff55aa54 100644 --- a/plugins/catalog-react/src/apis/StarredEntitiesApi/StarredEntitiesApi.ts +++ b/plugins/catalog-react/src/apis/StarredEntitiesApi/StarredEntitiesApi.ts @@ -16,10 +16,18 @@ import { ApiRef, createApiRef, Observable } from '@backstage/core-plugin-api'; +/** + * An API to store starred entities + * + * @public + */ export const starredEntitiesApiRef: ApiRef = createApiRef({ id: 'catalog-react.starred-entities', }); +/** + * @public + */ export type StarredEntitiesApiObservable = { /** * A set of entity references that are starred From 1dbaaf7801070c208bcad17ef531c85dbad753f2 Mon Sep 17 00:00:00 2001 From: Dominik Henneke Date: Wed, 13 Oct 2021 12:14:10 +0200 Subject: [PATCH 10/13] Simplify the StarredEntitiesApi interface Signed-off-by: Dominik Henneke --- plugins/catalog-react/api-report.md | 16 +++------ .../DefaultStarredEntitiesApi.test.ts | 15 ++------ .../DefaultStarredEntitiesApi.ts | 34 +++++++------------ .../StarredEntitiesApi/StarredEntitiesApi.ts | 23 ++----------- .../src/apis/StarredEntitiesApi/index.ts | 5 +-- .../src/hooks/useStarredEntities.ts | 34 +++++++++++++------ 6 files changed, 46 insertions(+), 81 deletions(-) diff --git a/plugins/catalog-react/api-report.md b/plugins/catalog-react/api-report.md index 1a37f84df3..2486cdd6e8 100644 --- a/plugins/catalog-react/api-report.md +++ b/plugins/catalog-react/api-report.md @@ -139,7 +139,7 @@ export class DefaultStarredEntitiesApi implements StarredEntitiesApi { // (undocumented) isStarred(entityRef: string): boolean; // (undocumented) - starredEntitie$(): Observable; + starredEntitie$(): Observable>; // (undocumented) toggleStarred(entityRef: string): Promise; } @@ -757,16 +757,10 @@ export const rootRoute: RouteRef; // @public export interface StarredEntitiesApi { - starredEntitie$(): Observable; + starredEntitie$(): Observable>; toggleStarred(entityRef: string): Promise; } -// @public (undocumented) -export type StarredEntitiesApiObservable = { - starredEntities: Set; - isStarred: (entityRef: string) => boolean; -}; - // @public export const starredEntitiesApiRef: ApiRef; @@ -894,10 +888,10 @@ export const UserListPicker: ({ // Warning: (ae-missing-release-tag) "useStarredEntities" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) // // @public (undocumented) -export const useStarredEntities: () => { +export function useStarredEntities(): { starredEntities: Set; - toggleStarredEntity: (entity: Entity) => void; - isStarredEntity: (entity: Entity) => boolean; + toggleStarredEntity: (entityOrRef: Entity | EntityName | string) => void; + isStarredEntity: (entityOrRef: Entity | EntityName | string) => boolean; }; // Warnings were encountered during analysis: diff --git a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts index e4f7c4ffa1..be67b03dbb 100644 --- a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts +++ b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts @@ -85,19 +85,8 @@ describe('DefaultStarredEntitiesApi', () => { it('should receive updates', async () => { expect(handler).toBeCalledTimes(2); - expect(handler).toBeCalledWith({ - starredEntities: new Set(), - isStarred: expect.any(Function), - }); - expect(handler).toBeCalledWith({ - starredEntities: new Set(['component:default/mock']), - isStarred: expect.any(Function), - }); - }); - - it('should receive isStarred function that operates on the latest state', async () => { - expect(handler.mock.calls[0][0].isStarred(mockEntityRef)).toBe(true); - expect(handler.mock.calls[1][0].isStarred(mockEntityRef)).toBe(true); + expect(handler).toBeCalledWith(new Set()); + expect(handler).toBeCalledWith(new Set(['component:default/mock'])); }); }); }); diff --git a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts index 8486d180b7..92a0eaebf2 100644 --- a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts +++ b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts @@ -16,10 +16,7 @@ import { Observable, StorageApi } from '@backstage/core-plugin-api'; import ObservableImpl from 'zen-observable'; -import { - StarredEntitiesApi, - StarredEntitiesApiObservable, -} from './StarredEntitiesApi'; +import { StarredEntitiesApi } from './StarredEntitiesApi'; /** * Default implementation of the StarredEntitiesApi that is backed by the StorageApi. @@ -58,7 +55,7 @@ export class DefaultStarredEntitiesApi implements StarredEntitiesApi { ); } - starredEntitie$(): Observable { + starredEntitie$(): Observable> { return this.observable; } @@ -67,29 +64,22 @@ export class DefaultStarredEntitiesApi implements StarredEntitiesApi { } private readonly subscribers = new Set< - ZenObservable.SubscriptionObserver + ZenObservable.SubscriptionObserver> >(); - private readonly observable = - new ObservableImpl(subscriber => { - // forward the the latest value - subscriber.next({ - starredEntities: this.starredEntities, - isStarred: e => this.isStarred(e), - }); + private readonly observable = new ObservableImpl>(subscriber => { + // forward the the latest value + subscriber.next(this.starredEntities); - this.subscribers.add(subscriber); - return () => { - this.subscribers.delete(subscriber); - }; - }); + this.subscribers.add(subscriber); + return () => { + this.subscribers.delete(subscriber); + }; + }); private notifyChanges() { for (const subscription of this.subscribers) { - subscription.next({ - starredEntities: this.starredEntities, - isStarred: e => this.isStarred(e), - }); + subscription.next(this.starredEntities); } } } diff --git a/plugins/catalog-react/src/apis/StarredEntitiesApi/StarredEntitiesApi.ts b/plugins/catalog-react/src/apis/StarredEntitiesApi/StarredEntitiesApi.ts index 93ff55aa54..4c4c68d66a 100644 --- a/plugins/catalog-react/src/apis/StarredEntitiesApi/StarredEntitiesApi.ts +++ b/plugins/catalog-react/src/apis/StarredEntitiesApi/StarredEntitiesApi.ts @@ -25,24 +25,6 @@ export const starredEntitiesApiRef: ApiRef = createApiRef({ id: 'catalog-react.starred-entities', }); -/** - * @public - */ -export type StarredEntitiesApiObservable = { - /** - * A set of entity references that are starred - */ - starredEntities: Set; - - /** - * A function to check if an entity is starred. - * - * @param entityRef - an entity reference to check - * @returns true, if the entity is starred. - */ - isStarred: (entityRef: string) => boolean; -}; - /** * An API to store and retrieve starred entities * @@ -57,8 +39,7 @@ export interface StarredEntitiesApi { toggleStarred(entityRef: string): Promise; /** - * Observe the state of starred entities and receive a handler - * to check the star state of an entity. + * Observe the set of starred entity references. */ - starredEntitie$(): Observable; + starredEntitie$(): Observable>; } diff --git a/plugins/catalog-react/src/apis/StarredEntitiesApi/index.ts b/plugins/catalog-react/src/apis/StarredEntitiesApi/index.ts index c71a795b9e..e9f9c8923a 100644 --- a/plugins/catalog-react/src/apis/StarredEntitiesApi/index.ts +++ b/plugins/catalog-react/src/apis/StarredEntitiesApi/index.ts @@ -16,7 +16,4 @@ export { DefaultStarredEntitiesApi } from './DefaultStarredEntitiesApi'; export { starredEntitiesApiRef } from './StarredEntitiesApi'; -export type { - StarredEntitiesApi, - StarredEntitiesApiObservable, -} from './StarredEntitiesApi'; +export type { StarredEntitiesApi } from './StarredEntitiesApi'; diff --git a/plugins/catalog-react/src/hooks/useStarredEntities.ts b/plugins/catalog-react/src/hooks/useStarredEntities.ts index e79e605877..8f6b09614f 100644 --- a/plugins/catalog-react/src/hooks/useStarredEntities.ts +++ b/plugins/catalog-react/src/hooks/useStarredEntities.ts @@ -14,29 +14,43 @@ * limitations under the License. */ -import { Entity, stringifyEntityRef } from '@backstage/catalog-model'; +import { + Entity, + EntityName, + stringifyEntityRef, +} from '@backstage/catalog-model'; import { useApi } from '@backstage/core-plugin-api'; import { useCallback } from 'react'; import { useObservable } from 'react-use'; import { starredEntitiesApiRef } from '../apis'; -export const useStarredEntities = () => { +function getEntityRef(entityOrRef: Entity | EntityName | string): string { + return typeof entityOrRef === 'string' + ? entityOrRef + : stringifyEntityRef(entityOrRef); +} + +export function useStarredEntities(): { + starredEntities: Set; + toggleStarredEntity: (entityOrRef: Entity | EntityName | string) => void; + isStarredEntity: (entityOrRef: Entity | EntityName | string) => boolean; +} { const starredEntitiesApi = useApi(starredEntitiesApiRef); - const { starredEntities, isStarred } = useObservable( + const starredEntities = useObservable( starredEntitiesApi.starredEntitie$(), - { starredEntities: new Set(), isStarred: _ => false }, + new Set(), ); const isStarredEntity = useCallback( - (entity: Entity) => isStarred(stringifyEntityRef(entity)), - [isStarred], + (entityOrRef: Entity | EntityName | string) => + starredEntities.has(getEntityRef(entityOrRef)), + [starredEntities], ); const toggleStarredEntity = useCallback( - (entity: Entity) => { - starredEntitiesApi.toggleStarred(stringifyEntityRef(entity)).then(); - }, + (entityOrRef: Entity | EntityName | string) => + starredEntitiesApi.toggleStarred(getEntityRef(entityOrRef)).then(), [starredEntitiesApi], ); @@ -45,4 +59,4 @@ export const useStarredEntities = () => { toggleStarredEntity, isStarredEntity, }; -}; +} From 0366c9b6671430c97efa5927286c5f9ba247bc72 Mon Sep 17 00:00:00 2001 From: Dominik Henneke Date: Wed, 13 Oct 2021 13:25:15 +0200 Subject: [PATCH 11/13] Introduce a `useStarredEntity` hook to check if a single entity is starred Signed-off-by: Dominik Henneke --- .changeset/gorgeous-laws-report.md | 7 ++ plugins/catalog-react/api-report.md | 8 ++ .../FavoriteEntity/FavoriteEntity.tsx | 21 ++-- plugins/catalog-react/src/hooks/index.ts | 1 + .../src/hooks/useStarredEntity.test.tsx | 103 ++++++++++++++++++ .../src/hooks/useStarredEntity.ts | 61 +++++++++++ .../FavouriteTemplate/FavouriteTemplate.tsx | 22 ++-- 7 files changed, 201 insertions(+), 22 deletions(-) create mode 100644 .changeset/gorgeous-laws-report.md create mode 100644 plugins/catalog-react/src/hooks/useStarredEntity.test.tsx create mode 100644 plugins/catalog-react/src/hooks/useStarredEntity.ts diff --git a/.changeset/gorgeous-laws-report.md b/.changeset/gorgeous-laws-report.md new file mode 100644 index 0000000000..47e5aa301f --- /dev/null +++ b/.changeset/gorgeous-laws-report.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-catalog-react': patch +'@backstage/plugin-scaffolder': patch +--- + +Introduce a `useStarredEntity` hook to check if a single entity is starred. +It provides a more efficient implementation compared to the `useStarredEntities` hook, because the rendering is only triggered if the selected entity is starred, not if _any_ entity is starred. diff --git a/plugins/catalog-react/api-report.md b/plugins/catalog-react/api-report.md index 2486cdd6e8..e28b8b0d32 100644 --- a/plugins/catalog-react/api-report.md +++ b/plugins/catalog-react/api-report.md @@ -894,6 +894,14 @@ export function useStarredEntities(): { isStarredEntity: (entityOrRef: Entity | EntityName | string) => boolean; }; +// Warning: (ae-missing-release-tag) "useStarredEntity" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) +// +// @public (undocumented) +export function useStarredEntity(entityOrRef: Entity | EntityName | string): { + toggleStarredEntity: () => void; + isStarredEntity: boolean; +}; + // Warnings were encountered during analysis: // // src/types.d.ts:6:49 - (tsdoc-escape-right-brace) The "}" character should be escaped using a backslash to avoid confusion with a TSDoc inline tag diff --git a/plugins/catalog-react/src/components/FavoriteEntity/FavoriteEntity.tsx b/plugins/catalog-react/src/components/FavoriteEntity/FavoriteEntity.tsx index 95e617ab7f..db60c34b66 100644 --- a/plugins/catalog-react/src/components/FavoriteEntity/FavoriteEntity.tsx +++ b/plugins/catalog-react/src/components/FavoriteEntity/FavoriteEntity.tsx @@ -14,12 +14,12 @@ * limitations under the License. */ -import React, { ComponentProps } from 'react'; -import { useStarredEntities } from '../../hooks/useStarredEntities'; -import { IconButton, Tooltip, withStyles } from '@material-ui/core'; -import StarBorder from '@material-ui/icons/StarBorder'; -import Star from '@material-ui/icons/Star'; import { Entity } from '@backstage/catalog-model'; +import { IconButton, Tooltip, withStyles } from '@material-ui/core'; +import Star from '@material-ui/icons/Star'; +import StarBorder from '@material-ui/icons/StarBorder'; +import React, { ComponentProps } from 'react'; +import { useStarredEntity } from '../../hooks/useStarredEntity'; type Props = ComponentProps & { entity: Entity }; @@ -40,16 +40,17 @@ export const favoriteEntityIcon = (isStarred: boolean) => * @param props MaterialUI IconButton props extended by required `entity` prop */ export const FavoriteEntity = (props: Props) => { - const { toggleStarredEntity, isStarredEntity } = useStarredEntities(); - const isStarred = isStarredEntity(props.entity); + const { toggleStarredEntity, isStarredEntity } = useStarredEntity( + props.entity, + ); return ( toggleStarredEntity(props.entity)} + onClick={() => toggleStarredEntity()} > - - {favoriteEntityIcon(isStarred)} + + {favoriteEntityIcon(isStarredEntity)} ); diff --git a/plugins/catalog-react/src/hooks/index.ts b/plugins/catalog-react/src/hooks/index.ts index 47bca8892a..38fc58222d 100644 --- a/plugins/catalog-react/src/hooks/index.ts +++ b/plugins/catalog-react/src/hooks/index.ts @@ -36,4 +36,5 @@ export { useEntityKinds } from './useEntityKinds'; export { useOwnUser } from './useOwnUser'; export { useRelatedEntities } from './useRelatedEntities'; export { useStarredEntities } from './useStarredEntities'; +export { useStarredEntity } from './useStarredEntity'; export { useEntityOwnership } from './useEntityOwnership'; diff --git a/plugins/catalog-react/src/hooks/useStarredEntity.test.tsx b/plugins/catalog-react/src/hooks/useStarredEntity.test.tsx new file mode 100644 index 0000000000..b8577aec0c --- /dev/null +++ b/plugins/catalog-react/src/hooks/useStarredEntity.test.tsx @@ -0,0 +1,103 @@ +/* + * Copyright 2021 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, EntityName } from '@backstage/catalog-model'; +import { ApiProvider, ApiRegistry } from '@backstage/core-app-api'; +import { renderHook } from '@testing-library/react-hooks'; +import React, { PropsWithChildren } from 'react'; +import Observable from 'zen-observable'; +import { StarredEntitiesApi, starredEntitiesApiRef } from '../apis'; +import { useStarredEntity } from './useStarredEntity'; + +describe('useStarredEntity', () => { + const mockStarredEntitiesApi: jest.Mocked = { + toggleStarred: jest.fn(), + starredEntitie$: jest.fn(), + }; + let wrapper: React.ComponentType; + + beforeEach(() => { + wrapper = ({ children }: PropsWithChildren<{}>) => ( + + {children} + + ); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe.each` + title | entityOrRef + ${'entity reference'} | ${'component:default/mock'} + ${'entity name'} | ${{ kind: 'component', namespace: 'default', name: 'mock' } as EntityName} + ${'entity'} | ${{ apiVersion: '1', kind: 'Component', metadata: { name: 'mock' } } as Entity} + `('with $title', ({ entityOrRef }) => { + describe('toggleStarredEntity', () => { + it('should toggle starred entity', () => { + mockStarredEntitiesApi.starredEntitie$.mockReturnValue(Observable.of()); + mockStarredEntitiesApi.toggleStarred.mockResolvedValue(); + + const { result } = renderHook(() => useStarredEntity(entityOrRef), { + wrapper, + }); + + result.current.toggleStarredEntity(); + + expect(mockStarredEntitiesApi.toggleStarred).toBeCalledTimes(1); + expect(mockStarredEntitiesApi.toggleStarred).toBeCalledWith( + 'component:default/mock', + ); + }); + }); + + describe('isStarredEntity', () => { + it('should return not starred entity', () => { + mockStarredEntitiesApi.starredEntitie$.mockReturnValue(Observable.of()); + mockStarredEntitiesApi.toggleStarred.mockResolvedValue(); + + const { result } = renderHook(() => useStarredEntity(entityOrRef), { + wrapper, + }); + + expect(result.current.isStarredEntity).toBe(false); + }); + + it('should return starred entity', async () => { + mockStarredEntitiesApi.starredEntitie$.mockReturnValue( + Observable.of(new Set(['component:default/mock'])), + ); + mockStarredEntitiesApi.toggleStarred.mockResolvedValue(); + + const { result, waitForNextUpdate } = renderHook( + () => useStarredEntity(entityOrRef), + { + wrapper, + }, + ); + + // the initial value will always be false because the observable triggers async + expect(result.current.isStarredEntity).toBe(false); + await waitForNextUpdate(); + + expect(result.current.isStarredEntity).toBe(true); + }); + }); + }); +}); diff --git a/plugins/catalog-react/src/hooks/useStarredEntity.ts b/plugins/catalog-react/src/hooks/useStarredEntity.ts new file mode 100644 index 0000000000..0233432196 --- /dev/null +++ b/plugins/catalog-react/src/hooks/useStarredEntity.ts @@ -0,0 +1,61 @@ +/* + * Copyright 2021 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, + EntityName, + stringifyEntityRef, +} from '@backstage/catalog-model'; +import { useApi } from '@backstage/core-plugin-api'; +import { useCallback, useEffect, useState } from 'react'; +import { starredEntitiesApiRef } from '../apis'; + +function getEntityRef(entityOrRef: Entity | EntityName | string): string { + return typeof entityOrRef === 'string' + ? entityOrRef + : stringifyEntityRef(entityOrRef); +} + +export function useStarredEntity(entityOrRef: Entity | EntityName | string): { + toggleStarredEntity: () => void; + isStarredEntity: boolean; +} { + const starredEntitiesApi = useApi(starredEntitiesApiRef); + + const [isStarredEntity, setIsStarredEntity] = useState(false); + + useEffect(() => { + const subscription = starredEntitiesApi.starredEntitie$().subscribe({ + next(starredEntities: Set) { + setIsStarredEntity(starredEntities.has(getEntityRef(entityOrRef))); + }, + }); + + return () => { + subscription.unsubscribe(); + }; + }, [entityOrRef, starredEntitiesApi]); + + const toggleStarredEntity = useCallback( + () => starredEntitiesApi.toggleStarred(getEntityRef(entityOrRef)).then(), + [entityOrRef, starredEntitiesApi], + ); + + return { + toggleStarredEntity, + isStarredEntity, + }; +} diff --git a/plugins/scaffolder/src/components/FavouriteTemplate/FavouriteTemplate.tsx b/plugins/scaffolder/src/components/FavouriteTemplate/FavouriteTemplate.tsx index d158315130..2c55ffdf88 100644 --- a/plugins/scaffolder/src/components/FavouriteTemplate/FavouriteTemplate.tsx +++ b/plugins/scaffolder/src/components/FavouriteTemplate/FavouriteTemplate.tsx @@ -14,12 +14,12 @@ * limitations under the License. */ -import React, { ComponentProps, useMemo } from 'react'; -import { useStarredEntities } from '@backstage/plugin-catalog-react'; -import { IconButton, makeStyles, Tooltip, withStyles } from '@material-ui/core'; -import StarBorder from '@material-ui/icons/StarBorder'; -import Star from '@material-ui/icons/Star'; import { Entity } from '@backstage/catalog-model'; +import { useStarredEntity } from '@backstage/plugin-catalog-react'; +import { IconButton, makeStyles, Tooltip, withStyles } from '@material-ui/core'; +import Star from '@material-ui/icons/Star'; +import StarBorder from '@material-ui/icons/StarBorder'; +import React, { ComponentProps } from 'react'; type Props = ComponentProps & { entity: Entity }; @@ -56,20 +56,18 @@ export const favouriteTemplateIcon = (isStarred: boolean) => */ export const FavouriteTemplate = (props: Props) => { const classes = useStyles(); - const { toggleStarredEntity, isStarredEntity } = useStarredEntities(); - const isStarred = useMemo( - () => isStarredEntity(props.entity), - [isStarredEntity, props.entity], + const { toggleStarredEntity, isStarredEntity } = useStarredEntity( + props.entity, ); return ( toggleStarredEntity(props.entity)} + onClick={() => toggleStarredEntity()} > - - {favouriteTemplateIcon(isStarred)} + + {favouriteTemplateIcon(isStarredEntity)} ); From 4ec21812bbd2ed199e23519f63f9a5abf6e73691 Mon Sep 17 00:00:00 2001 From: Dominik Henneke Date: Wed, 13 Oct 2021 16:07:27 +0200 Subject: [PATCH 12/13] Fix import Signed-off-by: Dominik Henneke --- .../src/components/ApiExplorerPage/ApiExplorerPage.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/api-docs/src/components/ApiExplorerPage/ApiExplorerPage.test.tsx b/plugins/api-docs/src/components/ApiExplorerPage/ApiExplorerPage.test.tsx index 292d03c838..cf40e5160c 100644 --- a/plugins/api-docs/src/components/ApiExplorerPage/ApiExplorerPage.test.tsx +++ b/plugins/api-docs/src/components/ApiExplorerPage/ApiExplorerPage.test.tsx @@ -32,8 +32,8 @@ import { catalogApiRef, DefaultStarredEntitiesApi, entityRouteRef, + starredEntitiesApiRef, } from '@backstage/plugin-catalog-react'; -import { starredEntitiesApiRef } from '@backstage/plugin-catalog-react/src/apis'; import { MockStorageApi, wrapInTestApp } from '@backstage/test-utils'; import DashboardIcon from '@material-ui/icons/Dashboard'; import { render } from '@testing-library/react'; From 5b57ba36f034765bd9f0fe14172f8b108cadd50d Mon Sep 17 00:00:00 2001 From: Dominik Henneke Date: Thu, 14 Oct 2021 11:21:45 +0200 Subject: [PATCH 13/13] Provide a migration from the old storage location and format to the new one Signed-off-by: Dominik Henneke --- .changeset/cool-deers-cough.md | 6 +- .../DefaultStarredEntitiesApi.test.ts | 11 ++ .../DefaultStarredEntitiesApi.ts | 4 + .../apis/StarredEntitiesApi/migration.test.ts | 113 ++++++++++++++++++ .../src/apis/StarredEntitiesApi/migration.ts | 62 ++++++++++ 5 files changed, 192 insertions(+), 4 deletions(-) create mode 100644 plugins/catalog-react/src/apis/StarredEntitiesApi/migration.test.ts create mode 100644 plugins/catalog-react/src/apis/StarredEntitiesApi/migration.ts diff --git a/.changeset/cool-deers-cough.md b/.changeset/cool-deers-cough.md index 0c07c91b96..4f4921db8d 100644 --- a/.changeset/cool-deers-cough.md +++ b/.changeset/cool-deers-cough.md @@ -6,7 +6,5 @@ Introduce a new `StarredEntitiesApi` that is used in the `useStarredEntities` hook. The `@backstage/plugin-catalog` installs a default implementation that is backed by the `StorageApi`, but one can also override the `starredEntitiesApiRef`. -**BREAKING** All previously stored entities get lost with this update, because: - -1. The storage format has been migrated from a custom string to an entity reference. -2. The storage key in the local storage has been changed. +This change also updates the storage format from a custom string to an entity reference and moves the location in the local storage. +A migration will convert the previously starred entities to the location on the first load of Backstage. diff --git a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts index be67b03dbb..55b992fadf 100644 --- a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts +++ b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.test.ts @@ -18,6 +18,9 @@ import { stringifyEntityRef } from '@backstage/catalog-model'; import { StorageApi } from '@backstage/core-plugin-api'; import { MockStorageApi } from '@backstage/test-utils'; import { DefaultStarredEntitiesApi } from './DefaultStarredEntitiesApi'; +import { performMigrationToTheNewBucket } from './migration'; + +jest.mock('./migration'); describe('DefaultStarredEntitiesApi', () => { let mockStorage: StorageApi; @@ -32,6 +35,8 @@ describe('DefaultStarredEntitiesApi', () => { }); beforeEach(() => { + (performMigrationToTheNewBucket as jest.Mock).mockResolvedValue(undefined); + mockStorage = MockStorageApi.create(); starredEntitiesApi = new DefaultStarredEntitiesApi({ storageApi: mockStorage, @@ -42,6 +47,12 @@ describe('DefaultStarredEntitiesApi', () => { jest.resetAllMocks(); }); + describe('constructor', () => { + it('should call migration', () => { + expect(performMigrationToTheNewBucket).toBeCalledTimes(1); + }); + }); + describe('toggleStarred', () => { it('should star unstarred entity', async () => { expect(starredEntitiesApi.isStarred(mockEntityRef)).toBe(false); diff --git a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts index 92a0eaebf2..87d99c8904 100644 --- a/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts +++ b/plugins/catalog-react/src/apis/StarredEntitiesApi/DefaultStarredEntitiesApi.ts @@ -16,6 +16,7 @@ import { Observable, StorageApi } from '@backstage/core-plugin-api'; import ObservableImpl from 'zen-observable'; +import { performMigrationToTheNewBucket } from './migration'; import { StarredEntitiesApi } from './StarredEntitiesApi'; /** @@ -28,6 +29,9 @@ export class DefaultStarredEntitiesApi implements StarredEntitiesApi { private starredEntities: Set; constructor(opts: { storageApi: StorageApi }) { + // no need to await. The updated content will be caught by the observe$ + performMigrationToTheNewBucket(opts).then(); + this.settingsStore = opts.storageApi.forBucket('starredEntities'); this.starredEntities = new Set( diff --git a/plugins/catalog-react/src/apis/StarredEntitiesApi/migration.test.ts b/plugins/catalog-react/src/apis/StarredEntitiesApi/migration.test.ts new file mode 100644 index 0000000000..1ff3be811f --- /dev/null +++ b/plugins/catalog-react/src/apis/StarredEntitiesApi/migration.test.ts @@ -0,0 +1,113 @@ +/* + * Copyright 2021 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 { StorageApi } from '@backstage/core-plugin-api'; +import { MockStorageApi } from '@backstage/test-utils'; +import { performMigrationToTheNewBucket } from './migration'; + +describe('performMigrationToTheNewBucket', () => { + let mockStorage: StorageApi; + + beforeEach(() => { + mockStorage = MockStorageApi.create(); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + it('should migrate', async () => { + const oldBucket = mockStorage.forBucket('settings'); + const newBucket = mockStorage.forBucket('starredEntities'); + + // fill NEW bucket + await newBucket.set('entityRefs', ['component:default/c']); + + // fill OLD bucket + await oldBucket.set('starredEntities', [ + 'entity:Component:default:a', + 'entity:template:custom:b', + ]); + expect(oldBucket.get('starredEntities')).not.toBeUndefined(); + + await performMigrationToTheNewBucket({ storageApi: mockStorage }); + + // read NEW bucket + expect(await newBucket.get('entityRefs')).toEqual([ + 'component:default/c', + 'component:default/a', + 'template:custom/b', + ]); + + // OLD bucket should be removed + expect(oldBucket.get('starredEntities')).toBeUndefined(); + }); + + it('should ignore invalid entries', async () => { + const oldBucket = mockStorage.forBucket('settings'); + const newBucket = mockStorage.forBucket('starredEntities'); + + // fill OLD bucket + await oldBucket.set('starredEntities', [ + 'entity:Component:default:a', + 1, + 'entity:Component:a', + 'invalid', + ]); + expect(oldBucket.get('starredEntities')).not.toBeUndefined(); + + await performMigrationToTheNewBucket({ storageApi: mockStorage }); + + // read NEW bucket + expect(await newBucket.get('entityRefs')).toEqual(['component:default/a']); + + // OLD bucket should be removed + expect(oldBucket.get('starredEntities')).toBeUndefined(); + }); + + it('should skip migration without old starred entities', async () => { + const newBucket = mockStorage.forBucket('starredEntities'); + + // fill NEW bucket + const expectedEntries = ['component:default/a']; + await newBucket.set('entityRefs', expectedEntries); + + await performMigrationToTheNewBucket({ storageApi: mockStorage }); + + // read NEW bucket + expect(newBucket.get('entityRefs')).toBe(expectedEntries); + }); + + it('should skip migration with non-array old starred entities', async () => { + const oldBucket = mockStorage.forBucket('settings'); + const newBucket = mockStorage.forBucket('starredEntities'); + + // fill OLD bucket with invalid content + await oldBucket.set('starredEntities', 'invalid'); + + // fill NEW bucket + const expectedEntries = ['component:default/a']; + await newBucket.set('entityRefs', expectedEntries); + + await performMigrationToTheNewBucket({ storageApi: mockStorage }); + + // read NEW bucket + expect(newBucket.get('entityRefs')).toBe(expectedEntries); + + // OLD bucket should be unchanged + expect(oldBucket.get('starredEntities')).toBe('invalid'); + }); +}); diff --git a/plugins/catalog-react/src/apis/StarredEntitiesApi/migration.ts b/plugins/catalog-react/src/apis/StarredEntitiesApi/migration.ts new file mode 100644 index 0000000000..7147d449ab --- /dev/null +++ b/plugins/catalog-react/src/apis/StarredEntitiesApi/migration.ts @@ -0,0 +1,62 @@ +/* + * Copyright 2021 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 { stringifyEntityRef } from '@backstage/catalog-model'; +import { StorageApi } from '@backstage/core-plugin-api'; +import { isArray, isString } from 'lodash'; + +/** + * Migrate the starred entities from the old format (entity:::) from the + * old storage location (/settings/starredEntities) to entity references in the new location + * (/starredEntities/entityRefs). + * + * This will only be executed once since the old location is cleared. + * + * @param storageApi - the StorageApi to migrate + */ +export async function performMigrationToTheNewBucket({ + storageApi, +}: { + storageApi: StorageApi; +}) { + const source = storageApi.forBucket('settings'); + const target = storageApi.forBucket('starredEntities'); + + const oldStarredEntities = source.get('starredEntities'); + + if (!isArray(oldStarredEntities)) { + // nothing to do + return; + } + + const targetEntities = new Set(target.get('entityRefs') ?? []); + + oldStarredEntities + .filter(isString) + // extract the old format 'entity:::' + .map(old => old.split(':')) + // check if the format is valid + .filter(split => split.length === 4 && split[0] === 'entity') + // convert to entity references + .map(([_, kind, namespace, name]) => + stringifyEntityRef({ kind, namespace, name }), + ) + .forEach(e => targetEntities.add(e)); + + await target.set('entityRefs', Array.from(targetEntities)); + + await source.remove('starredEntities'); +}