From 2a624498e182c19ebedc5114a975dc65235602c8 Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Tue, 18 Jan 2022 16:49:04 -0500 Subject: [PATCH] Add support for overriding default category columns, plus tests Signed-off-by: Sebastian Malton --- src/common/catalog/catalog-entity.ts | 19 +++ src/common/utils/__tests__/bind.test.ts | 35 +++++ src/common/utils/collection-functions.ts | 56 -------- .../renderer-extensions.injectable.ts | 4 +- .../+catalog/__tests__/custom-columns.test.ts | 136 ++++++++++++++++++ .../+catalog/custom-category-columns.ts | 44 +++--- .../get-category-columns.injectable.ts | 37 +++-- .../+catalog/internal-category-columns.tsx | 25 ++-- 8 files changed, 253 insertions(+), 103 deletions(-) create mode 100644 src/common/utils/__tests__/bind.test.ts create mode 100644 src/renderer/components/+catalog/__tests__/custom-columns.test.ts diff --git a/src/common/catalog/catalog-entity.ts b/src/common/catalog/catalog-entity.ts index 70ebd1aa6b..feb13ed51b 100644 --- a/src/common/catalog/catalog-entity.ts +++ b/src/common/catalog/catalog-entity.ts @@ -8,6 +8,7 @@ import type TypedEmitter from "typed-emitter"; import { observable, makeObservable } from "mobx"; import { once } from "lodash"; import { iter, Disposer } from "../utils"; +import type { CategoryColumnRegistration } from "../../renderer/components/+catalog/custom-category-columns"; type ExtractEntityMetadataType = Entity extends CatalogEntity ? Metadata : never; type ExtractEntityStatusType = Entity extends CatalogEntity ? Status : never; @@ -46,6 +47,7 @@ export interface CatalogCategorySpec { * The grouping for for the category. This MUST be a DNS label. */ group: string; + /** * The specific versions of the constructors. * @@ -54,6 +56,10 @@ export interface CatalogCategorySpec { * `name = "v1alpha1"` then the resulting `.apiVersion` MUST be `entity.k8slens.dev/v1alpha1` */ versions: CatalogCategoryVersion[]; + + /** + * This is the concerning the category + */ names: { /** * The kind of entity that this category is for. This value MUST be a DNS @@ -62,6 +68,19 @@ export interface CatalogCategorySpec { */ kind: string; }; + + /** + * These are the columns used for displaying entities when in the catalog. + * + * If this is not provided then some default columns will be used, similar in + * scope to the columns in the "Browse" view. + * + * Even if you provide columns, a "Name" column will be provided as well with + * `priority: 0`. + * + * These columns will not be used in the "Browse" view. + */ + displayColumns?: CategoryColumnRegistration[]; } /** diff --git a/src/common/utils/__tests__/bind.test.ts b/src/common/utils/__tests__/bind.test.ts new file mode 100644 index 0000000000..33c6803e0d --- /dev/null +++ b/src/common/utils/__tests__/bind.test.ts @@ -0,0 +1,35 @@ +/** + * Copyright (c) 2021 OpenLens Authors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER + * IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +import { bind } from "../index"; + +describe("bind", () => { + it("should work correctly", () => { + function foobar(bound: number, nonBound: number): number { + expect(typeof bound).toBe("number"); + expect(typeof nonBound).toBe("number"); + + return bound + nonBound; + } + const foobarBound = bind(foobar, null, 5); + + expect(foobarBound(10)).toBe(15); + }); +}); diff --git a/src/common/utils/collection-functions.ts b/src/common/utils/collection-functions.ts index b05186c0cb..aeed04a135 100644 --- a/src/common/utils/collection-functions.ts +++ b/src/common/utils/collection-functions.ts @@ -33,59 +33,3 @@ export function getOrInsert(map: Map, key: K, value: V): V { return map.set(key, value).get(key); } - -/** - * Get the value behind `key`. If it was not pressent, first insert the value returned by `getVal` - * - * This function should be used over `getInsertWith` if it is expensive to build a `V` - * @param map The map to interact with - * @param key The key to insert into the map with - * @param builder A function that returns a new instance of `V`. - * @returns The value in the map - */ -export function getOrInsertWith(map: Map, key: K, builder: () => V): V { - if (map.has(key)) { - return map.get(key); - } - - return map.set(key, builder()).get(key); -} - -/** - * Set the value associated with `key` iff there was not a previous value - * @param map The map to interact with - * @throws if `key` already in map - * @returns `this` so that `strictSet` can be chained - */ -export function strictSet(map: Map, key: K, val: V): typeof map { - if (map.has(key)) { - throw new TypeError("Duplicate key in map"); - } - - return map.set(key, val); -} - -/** - * Get the value associated with `key` - * @param map The map to interact with - * @throws if `key` did not a value associated with it - */ -export function strictGet(map: Map, key: K): V { - if (!map.has(key)) { - throw new TypeError("key not in map"); - } - - return map.get(key); -} - -/** - * Toggles the "has" in `set` of `value` - * @param set The set to interact with - * @param value The value to either add if not present or remove if present from `set` - */ -export function toggle(set: Set, value: T): void { - if (!set.delete(value)) { - // Set.prototype.delete returns false if `value` was not in the set - set.add(value); - } -} diff --git a/src/extensions/renderer-extensions.injectable.ts b/src/extensions/renderer-extensions.injectable.ts index 24e3bd0557..3f06df5ca8 100644 --- a/src/extensions/renderer-extensions.injectable.ts +++ b/src/extensions/renderer-extensions.injectable.ts @@ -8,10 +8,8 @@ import extensionsInjectable from "./extensions.injectable"; import type { LensRendererExtension } from "./lens-renderer-extension"; const rendererExtensionsInjectable = getInjectable({ + instantiate: (di) => di.inject(extensionsInjectable) as IComputedValue, lifecycle: lifecycleEnum.singleton, - - instantiate: (di) => - di.inject(extensionsInjectable) as IComputedValue, }); export default rendererExtensionsInjectable; diff --git a/src/renderer/components/+catalog/__tests__/custom-columns.test.ts b/src/renderer/components/+catalog/__tests__/custom-columns.test.ts new file mode 100644 index 0000000000..b87b38aaf2 --- /dev/null +++ b/src/renderer/components/+catalog/__tests__/custom-columns.test.ts @@ -0,0 +1,136 @@ +/** + * Copyright (c) 2021 OpenLens Authors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER + * IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +import type { ConfigurableDependencyInjectionContainer } from "@ogre-tools/injectable"; +import { computed } from "mobx"; +import type { CatalogCategorySpec } from "../../../../common/catalog"; +import type { LensRendererExtension } from "../../../../extensions/lens-renderer-extension"; +import rendererExtensionsInjectable from "../../../../extensions/renderer-extensions.injectable"; +import { CatalogCategory } from "../../../api/catalog-entity"; +import { getDiForUnitTesting } from "../../../getDiForUnitTesting"; +import type { AdditionalCategoryColumnRegistration, CategoryColumnRegistration } from "../custom-category-columns"; +import getCategoryColumnsInjectable, { CategoryColumns, GetCategoryColumnsParams } from "../get-category-columns.injectable"; + +class TestCategory extends CatalogCategory { + apiVersion: string; + kind: string; + metadata: { name: string; icon: string; }; + spec: CatalogCategorySpec = { + group: "foo.bar.bat", + names: { + kind: "Test", + }, + versions: [], + }; + + constructor(columns?: CategoryColumnRegistration[]) { + super(); + this.spec.displayColumns = columns; + } +} + +describe("Custom Category Columns", () => { + let di: ConfigurableDependencyInjectionContainer; + + beforeEach(() => { + di = getDiForUnitTesting(); + }); + + describe("without extensions", () => { + let getCategoryColumns: (params: GetCategoryColumnsParams) => CategoryColumns; + + beforeEach(() => { + di.override(rendererExtensionsInjectable, () => computed(() => [] as LensRendererExtension[])); + getCategoryColumns = di.inject(getCategoryColumnsInjectable); + }); + + it("should contain a kind column if activeCategory is falsy", () => { + expect(getCategoryColumns({ activeCategory: null }).renderTableHeader.find(elem => elem.title === "Kind")).toBeTruthy(); + }); + + it("should not contain a kind column if activeCategory is truthy", () => { + expect(getCategoryColumns({ activeCategory: new TestCategory() }).renderTableHeader.find(elem => elem.title === "Kind")).toBeFalsy(); + }); + + it("should include the default columns if the provided category doesn't provide any", () => { + expect(getCategoryColumns({ activeCategory: new TestCategory() }).renderTableHeader.find(elem => elem.title === "Source")).toBeTruthy(); + }); + + it("should not include the default columns if the provided category provides any", () => { + expect(getCategoryColumns({ activeCategory: new TestCategory([]) }).renderTableHeader.find(elem => elem.title === "Source")).toBeFalsy(); + }); + + it("should include the displayColumns from the provided category", () => { + const columns: CategoryColumnRegistration[] = [ + { + id: "foo", + renderCell: () => null, + titleProps: { + title: "Foo", + }, + }, + ]; + + expect(getCategoryColumns({ activeCategory: new TestCategory(columns) }).renderTableHeader.find(elem => elem.title === "Foo")).toBeTruthy(); + }); + }); + + describe("with extensions", () => { + let getCategoryColumns: (params: GetCategoryColumnsParams) => CategoryColumns; + + beforeEach(() => { + di.override(rendererExtensionsInjectable, () => computed(() => [ + { + name: "test-extension", + additionalCategoryColumns: [ + { + group: "foo.bar.bat", + id: "high", + kind: "Test", + renderCell: () => "", + titleProps: { + title: "High", + }, + } as AdditionalCategoryColumnRegistration, + { + group: "foo.bar", + id: "high", + kind: "Test", + renderCell: () => "", + titleProps: { + title: "High2", + }, + } as AdditionalCategoryColumnRegistration, + ], + } as LensRendererExtension, + ])); + getCategoryColumns = di.inject(getCategoryColumnsInjectable); + }); + + it("should include columns from extensions that match", () => { + expect(getCategoryColumns({ activeCategory: new TestCategory() }).renderTableHeader.find(elem => elem.title === "High")).toBeTruthy(); + }); + + it("should not include columns from extensions that don't match", () => { + expect(getCategoryColumns({ activeCategory: new TestCategory() }).renderTableHeader.find(elem => elem.title === "High2")).toBeFalsy(); + }); + }); +}); diff --git a/src/renderer/components/+catalog/custom-category-columns.ts b/src/renderer/components/+catalog/custom-category-columns.ts index a4d4e852fd..e153cee2f7 100644 --- a/src/renderer/components/+catalog/custom-category-columns.ts +++ b/src/renderer/components/+catalog/custom-category-columns.ts @@ -28,29 +28,10 @@ import type { TableCellProps } from "../table"; */ export interface TitleCellProps { className?: string; - title?: React.ReactNode; + title: React.ReactNode; } -/** - * This is the type used to declare new catalog category columns - */ -export interface AdditionalCategoryColumnRegistration { - /** - * The catalog entity kind that is declared by the category for this registration - * - * e.g. - * - `"KubernetesCluster"` - */ - kind: string; - - /** - * The catalog entity group that is declared by the category for this registration - * - * e.g. - * - `"entity.k8slens.dev"` - */ - group: string; - +export interface CategoryColumnRegistration { /** * The sorting order value. * @@ -89,6 +70,27 @@ export interface AdditionalCategoryColumnRegistration { searchFilter?: (entity: CatalogEntity) => string | string[]; } +/** + * This is the type used to declare new catalog category columns + */ +export interface AdditionalCategoryColumnRegistration extends CategoryColumnRegistration { + /** + * The catalog entity kind that is declared by the category for this registration + * + * e.g. + * - `"KubernetesCluster"` + */ + kind: string; + + /** + * The catalog entity group that is declared by the category for this registration + * + * e.g. + * - `"entity.k8slens.dev"` + */ + group: string; +} + export interface RegisteredAdditionalCategoryColumn { id: string; priority: number; diff --git a/src/renderer/components/+catalog/get-category-columns.injectable.ts b/src/renderer/components/+catalog/get-category-columns.injectable.ts index ba722b279f..dd6f58466e 100644 --- a/src/renderer/components/+catalog/get-category-columns.injectable.ts +++ b/src/renderer/components/+catalog/get-category-columns.injectable.ts @@ -26,7 +26,7 @@ import { bind } from "../../utils"; import type { ItemListLayoutProps } from "../item-object-list"; import type { RegisteredAdditionalCategoryColumn } from "./custom-category-columns"; import categoryColumnsInjectable from "./custom-category-columns.injectable"; -import { anyCategoryColumns, browseAllColumns } from "./internal-category-columns"; +import { defaultCategoryColumns, browseAllColumns, nameCategoryColumn } from "./internal-category-columns"; interface Dependencies { extensionColumns: IComputedValue>>; @@ -38,24 +38,39 @@ export interface GetCategoryColumnsParams { export type CategoryColumns = Required, "sortingCallbacks" | "searchFilters" | "renderTableContents" | "renderTableHeader">>; -function getNonGlobalColums(activeCategory: CatalogCategory | null | undefined, extensionColumns: IComputedValue>>) { - if (activeCategory) { - return extensionColumns +function getSpecificCategoryColumns(activeCategory: CatalogCategory, extensionColumns: IComputedValue>>): RegisteredAdditionalCategoryColumn[] { + const fromExtensions = ( + extensionColumns .get() .get(activeCategory.spec.group) ?.get(activeCategory.spec.names.kind) - ?? []; - } + ?? [] + ); + const fromCategory = activeCategory.spec.displayColumns?.map(({ priority = 50, ...column }) => ({ + priority, + ...column, + })) ?? defaultCategoryColumns; - return browseAllColumns; + return [ + nameCategoryColumn, + ...fromExtensions, + ...fromCategory, + ]; +} + +function getBrowseAllColumns(): RegisteredAdditionalCategoryColumn[] { + return [ + ...browseAllColumns, + nameCategoryColumn, + ...defaultCategoryColumns, + ]; } function getCategoryColumns({ extensionColumns }: Dependencies, { activeCategory }: GetCategoryColumnsParams): CategoryColumns { const allRegistrations = orderBy( - [ - ...getNonGlobalColums(activeCategory, extensionColumns), - ...anyCategoryColumns, - ], + activeCategory + ? getSpecificCategoryColumns(activeCategory, extensionColumns) + : getBrowseAllColumns(), "priority", "asc", ); diff --git a/src/renderer/components/+catalog/internal-category-columns.tsx b/src/renderer/components/+catalog/internal-category-columns.tsx index aed4db68cf..73749476b7 100644 --- a/src/renderer/components/+catalog/internal-category-columns.tsx +++ b/src/renderer/components/+catalog/internal-category-columns.tsx @@ -79,20 +79,21 @@ export const browseAllColumns: RegisteredAdditionalCategoryColumn[] = [ }, ]; -export const anyCategoryColumns: RegisteredAdditionalCategoryColumn[] = [ - { +export const nameCategoryColumn: RegisteredAdditionalCategoryColumn = { + id: "name", + priority: 0, + renderCell: renderEntityName, + titleProps: { + title: "Name", + className: styles.entityName, id: "name", - priority: 0, - renderCell: renderEntityName, - titleProps: { - title: "Name", - className: styles.entityName, - id: "name", - sortBy: "name", - }, - searchFilter: entity => entity.getName(), - sortCallback: entity => `name=${entity.getName()}`, + sortBy: "name", }, + searchFilter: entity => entity.getName(), + sortCallback: entity => `name=${entity.getName()}`, +}; + +export const defaultCategoryColumns: RegisteredAdditionalCategoryColumn[] = [ { id: "source", priority: 10,