paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 25 Packages Projects Releases Wiki Activity

Merge pull request #5850 from backstage/freben/entity-provider

Browse Source 
Tweak the `EntityListProvider` to do single-cycle updates.
This commit is contained in:
Fredrik Adelöw
2021-05-31 10:51:24 +02:00
committed by GitHub
parent 425ee4ef03 7af9cef074
commit 9fa6391c78
6 changed files with 101 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/beige-garlics-doubt.md
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/test-utils': patch
---
Fix a bug in `MockStorageApi` where it unhelpfully returned new empty buckets every single time
.changeset/slimy-kids-attack.md
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/plugin-catalog-react': patch
---
Tweak the `EntityListProvider` to do single-cycle updates
packages/test-utils/src/testUtils/apis/StorageApi/MockStorageApi.ts
+9 -1
View File
@@ -23,6 +23,8 @@ import ObservableImpl from 'zen-observable';
export type MockStorageBucket = { [key: string]: any };
const bucketStorageApis = new Map<string, MockStorageApi>();
export class MockStorageApi implements StorageApi {
private readonly namespace: string;
private readonly data: MockStorageBucket;
@@ -37,7 +39,13 @@ export class MockStorageApi implements StorageApi {
}
forBucket(name: string): StorageApi {
return new MockStorageApi(`${this.namespace}/${name}`, this.data);
if (!bucketStorageApis.has(name)) {
bucketStorageApis.set(
name,
new MockStorageApi(`${this.namespace}/${name}`, this.data),
);
}
return bucketStorageApis.get(name)!;
}
get<T>(key: string): T | undefined {
plugins/catalog-react/src/hooks/useEntityListProvider.test.tsx
+1
View File
@@ -157,6 +157,7 @@ describe('<EntityListProvider/>', () => {
user: new UserListFilter('owned', mockUser, () => true),
}),
);
await waitFor(() => result.current.entities.length !== 2);
expect(mockCatalogApi.getEntities).toHaveBeenCalledTimes(1);
expect(result.current.entities.length).toBe(1);
});
plugins/catalog-react/src/hooks/useEntityListProvider.tsx
+56 -51
View File
@@ -14,18 +14,17 @@
* limitations under the License.
*/
import { Entity } from '@backstage/catalog-model';
import { useApi } from '@backstage/core';
import { compact, isEqual } from 'lodash';
import React, {
createContext,
PropsWithChildren,
useCallback,
useContext,
useEffect,
useState,
} from 'react';
import { useAsyncFn, useDebounce } from 'react-use';
import { useApi } from '@backstage/core';
import { Entity } from '@backstage/catalog-model';
import { reduceCatalogFilters, reduceEntityFilters } from '../utils';
import { catalogApiRef } from '../api';
import {
EntityFilter,
@@ -34,7 +33,7 @@ import {
EntityTypeFilter,
UserListFilter,
} from '../types';
import { compact, isEqual } from 'lodash';
import { reduceCatalogFilters, reduceEntityFilters } from '../utils';
export type DefaultEntityFilters = {
kind?: EntityKindFilter;
@@ -80,51 +79,57 @@ export const EntityListContext = createContext<
EntityListContextProps<any> | undefined
>(undefined);
type OutputState<EntityFilters extends DefaultEntityFilters> = {
appliedFilters: EntityFilters;
entities: Entity[];
backendEntities: Entity[];
};
export const EntityListProvider = <EntityFilters extends DefaultEntityFilters>({
children,
}: PropsWithChildren<{}>) => {
const catalogApi = useApi(catalogApiRef);
const [requestedFilters, setRequestedFilters] = useState<EntityFilters>(
{} as EntityFilters,
);
const [outputState, setOutputState] = useState<OutputState<EntityFilters>>({
appliedFilters: {} as EntityFilters,
entities: [],
backendEntities: [],
});
const [filters, setFilters] = useState<EntityFilters>({} as EntityFilters);
const [entities, setEntities] = useState<Entity[]>([]);
const [backendEntities, setBackendEntities] = useState<Entity[]>([]);
// Store resolved catalog-backend filters and deep compare on filter updates, to avoid refetching
// when only frontend filters change
const [backendFilters, setBackendFilters] = useState<
Record<string, string | string[]>
>(reduceCatalogFilters(compact(Object.values(filters))));
useEffect(() => {
const newBackendFilters = reduceCatalogFilters(
compact(Object.values(filters)),
);
if (!isEqual(newBackendFilters, backendFilters)) {
setBackendFilters(newBackendFilters);
}
}, [backendFilters, filters]);
// The main async filter worker. Note that while it has a lot of dependencies
// in terms of its implementation, the triggering only happens (debounced)
// based on the requested filters changing.
const [{ loading, error }, refresh] = useAsyncFn(async () => {
// TODO(timbonicus): should limit fields here, but would need filter fields + table columns
const items = await catalogApi
.getEntities({
filter: backendFilters,
})
.then(response => response.items);
setBackendEntities(items);
}, [backendFilters, catalogApi]);
// Slight debounce on the catalog-backend call, to prevent eager refresh on multiple programmatic
// filter changes.
useDebounce(refresh, 10, [backendFilters]);
// Apply frontend filters
useEffect(() => {
const resolvedEntities = (backendEntities ?? []).filter(
reduceEntityFilters(compact(Object.values(filters))),
const compacted = compact(Object.values(requestedFilters));
const entityFilter = reduceEntityFilters(compacted);
const backendFilter = reduceCatalogFilters(compacted);
const previousBackendFilter = reduceCatalogFilters(
compact(Object.values(outputState.appliedFilters)),
);
setEntities(resolvedEntities);
}, [backendEntities, filters]);
if (!isEqual(previousBackendFilter, backendFilter)) {
// TODO(timbonicus): should limit fields here, but would need filter
// fields + table columns
const response = await catalogApi.getEntities({ filter: backendFilter });
setOutputState({
appliedFilters: requestedFilters,
backendEntities: response.items,
entities: response.items.filter(entityFilter),
});
} else {
setOutputState({
appliedFilters: requestedFilters,
backendEntities: outputState.backendEntities,
entities: outputState.backendEntities.filter(entityFilter),
});
}
}, [catalogApi, requestedFilters, outputState]);
// Slight debounce on the refresh, since (especially on page load) several
// filters will be calling this in rapid succession.
useDebounce(refresh, 10, [requestedFilters]);
const updateFilters = useCallback(
(
@@ -132,11 +137,11 @@ export const EntityListProvider = <EntityFilters extends DefaultEntityFilters>({
| Partial<EntityFilter>
| ((prevFilters: EntityFilters) => Partial<EntityFilters>),
) => {
if (typeof update === 'function') {
setFilters(prevFilters => ({ ...prevFilters, ...update(prevFilters) }));
} else {
setFilters(prevFilters => ({ ...prevFilters, ...update }));
}
setRequestedFilters(prevFilters => {
const newFilters =
typeof update === 'function' ? update(prevFilters) : update;
return { ...prevFilters, ...newFilters };
});
},
[],
);
@@ -144,9 +149,9 @@ export const EntityListProvider = <EntityFilters extends DefaultEntityFilters>({
return (
<EntityListContext.Provider
value={{
filters,
entities,
backendEntities,
filters: outputState.appliedFilters,
entities: outputState.entities,
backendEntities: outputState.backendEntities,
updateFilters,
loading,
error,
plugins/catalog/src/components/CatalogPage/CatalogPage.test.tsx
+25 -22
View File
@@ -34,7 +34,7 @@ import {
renderWithEffects,
wrapInTestApp,
} from '@backstage/test-utils';
import { fireEvent, waitFor } from '@testing-library/react';
import { fireEvent, screen } from '@testing-library/react';
import React from 'react';
import { createComponentRouteRef } from '../../routes';
import { CatalogPage } from './CatalogPage';
@@ -132,35 +132,38 @@ describe('CatalogPage', () => {
// related to some theme issues in mui-table
// https://github.com/mbrn/material-table/issues/1293
it('should render', async () => {
const { getByText, getByTestId } = await renderWrapped(<CatalogPage />);
expect(getByText(/Owned \(1\)/)).toBeInTheDocument();
const { findByText, getByTestId } = await renderWrapped(<CatalogPage />);
await expect(findByText(/Owned \(1\)/)).resolves.toBeInTheDocument();
fireEvent.click(getByTestId('user-picker-all'));
expect(getByText(/All \(2\)/)).toBeInTheDocument();
await expect(findByText(/All \(2\)/)).resolves.toBeInTheDocument();
});
it('should set initial filter correctly', async () => {
const { getByText } = await renderWrapped(
const { findByText } = await renderWrapped(
<CatalogPage initiallySelectedFilter="all" />,
);
expect(getByText(/All \(2\)/)).toBeInTheDocument();
await expect(findByText(/All \(2\)/)).resolves.toBeInTheDocument();
});
// this test is for fixing the bug after favoriting an entity, the matching entities defaulting
// to "owned" filter and not based on the selected filter
it('should render the correct entities filtered on the selectedfilter', async () => {
const { getByText, findAllByTitle, getByTestId } = await renderWrapped(
<CatalogPage />,
);
expect(getByText(/Owned \(1\)/)).toBeInTheDocument();
expect(getByText(/Starred/)).toBeInTheDocument();
fireEvent.click(getByTestId('user-picker-starred'));
expect(getByText(/Starred \(0\)/)).toBeInTheDocument();
fireEvent.click(getByTestId('user-picker-all'));
expect(getByText(/All \(2\)/)).toBeInTheDocument();
const starredIcons = await findAllByTitle('Add to favorites');
// this test is for fixing the bug after favoriting an entity, the matching
// entities defaulting to "owned" filter and not based on the selected filter
it('should render the correct entities filtered on the selected filter', async () => {
await renderWrapped(<CatalogPage />);
await expect(screen.findByText(/Owned \(1\)/)).resolves.toBeInTheDocument();
fireEvent.click(screen.getByTestId('user-picker-starred'));
await expect(
screen.findByText(/Starred \(0\)/),
).resolves.toBeInTheDocument();
fireEvent.click(screen.getByTestId('user-picker-all'));
await expect(screen.findByText(/All \(2\)/)).resolves.toBeInTheDocument();
const starredIcons = await screen.findAllByTitle('Add to favorites');
fireEvent.click(starredIcons[0]);
expect(getByText(/All \(2\)/)).toBeInTheDocument();
await expect(screen.findByText(/All \(2\)/)).resolves.toBeInTheDocument();
fireEvent.click(getByTestId('user-picker-starred'));
waitFor(() => expect(getByText(/Starred \(1\)/)).toBeInTheDocument());
fireEvent.click(screen.getByTestId('user-picker-starred'));
await expect(
screen.findByText(/Starred \(1\)/),
).resolves.toBeInTheDocument();
});
});