From 1244ef5d80aaf403e866ee4f356ec8be676dd29d Mon Sep 17 00:00:00 2001 From: Janne Savolainen Date: Fri, 22 Jul 2022 15:48:55 +0300 Subject: [PATCH] Start resolving kube object status texts on each re-render to mimic reactivity Note: This is done due the current implementation exposed in Extension API expects it to work so. However, this is bad. Signed-off-by: Janne Savolainen --- .../show-status-for-a-kube-object.test.tsx | 271 +++++++++++------- .../kube-object-status-icon.tsx | 13 +- ...ject-status-texts-for-object.injectable.ts | 8 +- 3 files changed, 182 insertions(+), 110 deletions(-) diff --git a/src/behaviours/cluster/kube-object-status-icon/show-status-for-a-kube-object.test.tsx b/src/behaviours/cluster/kube-object-status-icon/show-status-for-a-kube-object.test.tsx index 7daf161b3d..b8af696848 100644 --- a/src/behaviours/cluster/kube-object-status-icon/show-status-for-a-kube-object.test.tsx +++ b/src/behaviours/cluster/kube-object-status-icon/show-status-for-a-kube-object.test.tsx @@ -8,16 +8,18 @@ import React from "react"; import { useFakeTime } from "../../../common/test-utils/use-fake-time"; import type { DiContainer } from "@ogre-tools/injectable"; import { getInjectable } from "@ogre-tools/injectable"; -import type { IObservableValue } from "mobx"; -import { observable, computed, runInAction } from "mobx"; -import { KubeObjectStatusIcon } from "../../../renderer/components/kube-object-status-icon"; -import { kubeObjectStatusTextInjectionToken } from "../../../renderer/components/kube-object-status-icon/kube-object-status-text-injection-token"; +import type { IAtom } from "mobx"; +import { createAtom, computed } from "mobx"; import { frontEndRouteInjectionToken } from "../../../common/front-end-routing/front-end-route-injection-token"; import { routeSpecificComponentInjectionToken } from "../../../renderer/routes/route-specific-component-injection-token"; import type { ApplicationBuilder } from "../../../renderer/components/test-utils/get-application-builder"; import { getApplicationBuilder } from "../../../renderer/components/test-utils/get-application-builder"; import { navigateToRouteInjectionToken } from "../../../common/front-end-routing/navigate-to-route-injection-token"; import type { RenderResult } from "@testing-library/react"; +import { act } from "@testing-library/react"; +import { observer } from "mobx-react"; +import { kubeObjectStatusTextInjectionToken } from "../../../renderer/components/kube-object-status-icon/kube-object-status-text-injection-token"; +import { KubeObjectStatusIcon } from "../../../renderer/components/kube-object-status-icon"; // TODO: Make tooltips free of side effects by making it deterministic jest.mock("../../../renderer/components/tooltip/withTooltip", () => ({ @@ -29,8 +31,13 @@ jest.mock("../../../renderer/components/tooltip/withTooltip", () => ({ return ( <> - -
{tooltip.children || tooltip}
+ +
+ {tooltip.children || tooltip} +
); } @@ -41,9 +48,9 @@ jest.mock("../../../renderer/components/tooltip/withTooltip", () => ({ describe("show status for a kube object", () => { let builder: ApplicationBuilder; - let infoStatusIsShown: IObservableValue; - let warningStatusIsShown: IObservableValue; - let criticalStatusIsShown: IObservableValue; + let infoStatusIsShown: boolean; + let warningStatusIsShown: boolean; + let criticalStatusIsShown: boolean; beforeEach(() => { useFakeTime("2015-10-21T07:28:00Z"); @@ -52,33 +59,66 @@ describe("show status for a kube object", () => { const rendererDi = builder.dis.rendererDi; - infoStatusIsShown = observable.box(false); - warningStatusIsShown = observable.box(false); - criticalStatusIsShown = observable.box(false); + infoStatusIsShown = false; - const infoStatusInjectable = getStatusTextInjectable( - KubeObjectStatusLevel.INFO, - "info", - "some-kind", - ["some-api-version"], - infoStatusIsShown, - ); + const infoStatusInjectable = getInjectable({ + id: "some-info-status", + injectionToken: kubeObjectStatusTextInjectionToken, - const warningStatusInjectable = getStatusTextInjectable( - KubeObjectStatusLevel.WARNING, - "warning", - "some-kind", - ["some-api-version"], - warningStatusIsShown, - ); + instantiate: () => ({ + apiVersions: ["some-api-version"], + kind: "some-kind", + enabled: computed(() => true), - const criticalStatusInjectable = getStatusTextInjectable( - KubeObjectStatusLevel.CRITICAL, - "critical", - "some-kind", - ["some-api-version"], - criticalStatusIsShown, - ); + resolve: (resource) => infoStatusIsShown ? ({ + level: KubeObjectStatusLevel.INFO, + text: `Some info status for ${resource.getName()}`, + timestamp: "2015-10-19T07:28:00Z", + }) : null, + }), + }); + + warningStatusIsShown = false; + + const warningStatusInjectable = getInjectable({ + id: "some-warning-status", + injectionToken: kubeObjectStatusTextInjectionToken, + + instantiate: () => ({ + apiVersions: ["some-api-version"], + kind: "some-kind", + enabled: computed(() => true), + + resolve: (resource) => warningStatusIsShown ? ({ + level: KubeObjectStatusLevel.WARNING, + text: `Some warning status for ${resource.getName()}`, + timestamp: "2015-10-19T07:28:00Z", + }) : null, + }), + }); + + criticalStatusIsShown = false; + + const criticalStatusInjectable = getInjectable({ + id: "some-critical-status", + injectionToken: kubeObjectStatusTextInjectionToken, + + instantiate: () => ({ + apiVersions: ["some-api-version"], + kind: "some-kind", + enabled: computed(() => true), + + resolve: (resource) => { + return criticalStatusIsShown + ? { + level: KubeObjectStatusLevel.CRITICAL, + text: `Some critical status for ${resource.getName()}`, + timestamp: "2015-10-19T07:28:00Z", + } + : null; + }, + }), + }); rendererDi.register( testRouteInjectable, @@ -86,6 +126,7 @@ describe("show status for a kube object", () => { infoStatusInjectable, warningStatusInjectable, criticalStatusInjectable, + someAtomInjectable, ); builder.setEnvironmentToClusterFrame(); @@ -94,12 +135,17 @@ describe("show status for a kube object", () => { describe("given application starts and in test page", () => { let rendererDi: DiContainer; let rendered: RenderResult; + let rerenderParent: () => void; beforeEach(async () => { rendered = await builder.render(); rendererDi = builder.dis.rendererDi; + const someAtom = rendererDi.inject(someAtomInjectable); + + rerenderParent = rerenderParentFor(someAtom); + const navigateToRoute = rendererDi.inject(navigateToRouteInjectionToken); const testRoute = rendererDi.inject(testRouteInjectable); @@ -121,6 +167,8 @@ describe("show status for a kube object", () => { describe("when status for irrelevant kube object kind emerges", () => { beforeEach(() => { rendererDi.register(statusForIrrelevantKubeObjectKindInjectable); + + rerenderParent(); }); it("renders", () => { @@ -139,6 +187,8 @@ describe("show status for a kube object", () => { describe("when status for irrelevant kube object api version emerges", () => { beforeEach(() => { rendererDi.register(statusForIrrelevantKubeObjectApiVersionInjectable); + + rerenderParent(); }); it("renders", () => { @@ -156,9 +206,9 @@ describe("show status for a kube object", () => { describe("when info status emerges", () => { beforeEach(() => { - runInAction(() => { - infoStatusIsShown.set(true); - }); + infoStatusIsShown = true; + + rerenderParent(); }); it("renders", () => { @@ -178,13 +228,17 @@ describe("show status for a kube object", () => { "tooltip-content-for-kube-object-status-icon-for-some-uid", ); - expect(tooltipContent).toHaveTextContent("Some info status for some-name"); + expect(tooltipContent).toHaveTextContent( + "Some info status for some-name", + ); }); }); describe("when warning status emerges", () => { beforeEach(() => { - warningStatusIsShown.set(true); + warningStatusIsShown = true; + + rerenderParent(); }); it("renders", () => { @@ -196,13 +250,17 @@ describe("show status for a kube object", () => { "tooltip-content-for-kube-object-status-icon-for-some-uid", ); - expect(tooltipContent).toHaveTextContent("Some warning status for some-name"); + expect(tooltipContent).toHaveTextContent( + "Some warning status for some-name", + ); }); }); describe("when critical status emerges", () => { beforeEach(() => { - criticalStatusIsShown.set(true); + criticalStatusIsShown = true; + + rerenderParent(); }); it("renders", () => { @@ -214,7 +272,9 @@ describe("show status for a kube object", () => { "tooltip-content-for-kube-object-status-icon-for-some-uid", ); - expect(tooltipContent).toHaveTextContent("Some critical status for some-name"); + expect(tooltipContent).toHaveTextContent( + "Some critical status for some-name", + ); }); }); }); @@ -232,76 +292,85 @@ const testRouteInjectable = getInjectable({ injectionToken: frontEndRouteInjectionToken, }); +const rerenderParentFor = (atom: IAtom) => () => { + act(() => { + atom.reportChanged(); + }); +}; + +const TestComponent = observer(({ someAtom }: { someAtom: IAtom }) => { + void someAtom.reportObserved(); + + return ( + + ); +}); + const testRouteComponentInjectable = getInjectable({ id: "test-route-component", - instantiate: (di) => ({ - route: di.inject(testRouteInjectable), + instantiate: (di) => { + const someAtom = di.inject(someAtomInjectable); - Component: () => ( - - ), - }), + return { + route: di.inject(testRouteInjectable), + Component: () => , + }; + }, injectionToken: routeSpecificComponentInjectionToken, }); -const getKubeObjectStub = (kind: string, apiVersion: string) => KubeObject.create({ - apiVersion, - kind, - metadata: { - uid: "some-uid", - name: "some-name", - resourceVersion: "some-resource-version", - namespace: "some-namespace", - selfLink: "/foo", - }, +const someAtomInjectable = getInjectable({ + id: "some-atom", + instantiate: () => createAtom("some-atom"), }); -const getStatusTextInjectable = ( - level: KubeObjectStatusLevel, - title: string, - kind: string, - apiVersions: string[], - statusIsShown?: IObservableValue, -) => - getInjectable({ - id: title, - - instantiate: () => ({ - apiVersions, - kind, - - resolve: (kubeObject: KubeObject) => { - if (statusIsShown && !statusIsShown.get()) { - return null; - } - - return { - level, - text: `Some ${title} status for ${kubeObject.getName()}`, - timestamp: "2015-10-19T07:28:00Z", - }; - }, - - enabled: computed(() => true), - }), - - injectionToken: kubeObjectStatusTextInjectionToken, +const getKubeObjectStub = (kind: string, apiVersion: string) => + KubeObject.create({ + apiVersion, + kind, + metadata: { + uid: "some-uid", + name: "some-name", + resourceVersion: "some-resource-version", + namespace: "some-namespace", + selfLink: "/foo", + }, }); -const statusForIrrelevantKubeObjectKindInjectable = getStatusTextInjectable( - KubeObjectStatusLevel.INFO, - "irrelevant", - "some-other-kind", - ["some-api-version"], -); +const statusForIrrelevantKubeObjectKindInjectable = getInjectable({ + id: "status-for-irrelevant-kube-object-kind", + injectionToken: kubeObjectStatusTextInjectionToken, -const statusForIrrelevantKubeObjectApiVersionInjectable = getStatusTextInjectable( - KubeObjectStatusLevel.INFO, - "irrelevant", - "some-kind", - ["some-other-api-version"], -); + instantiate: () => ({ + apiVersions: ["some-api-version"], + kind: "some-other-kind", + enabled: computed(() => true), + + resolve: () => ({ + level: KubeObjectStatusLevel.INFO, + text: "irrelevant", + timestamp: "2015-10-19T07:28:00Z", + }), + }), +}); + +const statusForIrrelevantKubeObjectApiVersionInjectable = getInjectable({ + id: "status-for-irrelevant-kube-object-api-version", + injectionToken: kubeObjectStatusTextInjectionToken, + + instantiate: () => ({ + apiVersions: ["some-other-api-version"], + kind: "some-kind", + enabled: computed(() => true), + + resolve: () => ({ + level: KubeObjectStatusLevel.INFO, + text: "irrelevant", + timestamp: "2015-10-19T07:28:00Z", + }), + }), +}); diff --git a/src/renderer/components/kube-object-status-icon/kube-object-status-icon.tsx b/src/renderer/components/kube-object-status-icon/kube-object-status-icon.tsx index 828995eef1..1024fbd0f7 100644 --- a/src/renderer/components/kube-object-status-icon/kube-object-status-icon.tsx +++ b/src/renderer/components/kube-object-status-icon/kube-object-status-icon.tsx @@ -15,6 +15,7 @@ import type { KubeObjectStatus } from "../../../common/k8s-api/kube-object-statu import { KubeObjectStatusLevel } from "../../../common/k8s-api/kube-object-status"; import type { IComputedValue } from "mobx"; import { observer } from "mobx-react"; +import type { KubeObjectStatusText } from "./kube-object-status-text-injection-token"; function statusClassName(level: KubeObjectStatusLevel): string { switch (level) { @@ -78,7 +79,7 @@ export interface KubeObjectStatusIconProps { } interface Dependencies { - statuses: IComputedValue; + statuses: IComputedValue; } @observer @@ -107,7 +108,11 @@ class NonInjectedKubeObjectStatusIcon extends React.Component statusText.resolve(this.props.object)) + .filter(isNotEmpty); if (statuses.length === 0) { return null; @@ -134,6 +139,10 @@ class NonInjectedKubeObjectStatusIcon extends React.Component(item: T | null | undefined): item is T { + return !!item; +} + export const KubeObjectStatusIcon = withInjectables( NonInjectedKubeObjectStatusIcon, diff --git a/src/renderer/components/kube-object-status-icon/kube-object-status-texts-for-object.injectable.ts b/src/renderer/components/kube-object-status-icon/kube-object-status-texts-for-object.injectable.ts index 949d9d143a..12ce4c2b9c 100644 --- a/src/renderer/components/kube-object-status-icon/kube-object-status-texts-for-object.injectable.ts +++ b/src/renderer/components/kube-object-status-icon/kube-object-status-texts-for-object.injectable.ts @@ -8,10 +8,6 @@ import type { KubeObject } from "../../../common/k8s-api/kube-object"; import { conforms, eq, includes } from "lodash/fp"; import { computed } from "mobx"; -function isNotEmpty(item: T | null | undefined): item is T { - return !!item; -} - const kubeObjectStatusTextsForObjectInjectable = getInjectable({ id: "kube-object-status-texts-for-object", @@ -21,9 +17,7 @@ const kubeObjectStatusTextsForObjectInjectable = getInjectable({ return computed(() => allStatusTexts .get() - .filter(toKubeObjectRelated(kubeObject)) - .map(item => item.resolve(kubeObject)) - .filter(isNotEmpty), + .filter(toKubeObjectRelated(kubeObject)), ); },