From 93d5f73089c55f1c4f39696cfd70b136401bfe2d Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Wed, 4 Jan 2023 15:38:30 -0500 Subject: [PATCH] Fixing edit-namespace-from-new-tab test Signed-off-by: Sebastian Malton --- .../edit-namespace-from-new-tab.test.tsx.snap | 137 +------------- .../edit-namespace-from-new-tab.test.tsx | 101 ++++------- .../edit-resource-model.injectable.tsx | 171 +++++++++--------- .../components/dock/edit-resource/view.tsx | 54 +++--- .../test-utils/get-application-builder.tsx | 8 +- 5 files changed, 148 insertions(+), 323 deletions(-) diff --git a/src/features/cluster/namespaces/__snapshots__/edit-namespace-from-new-tab.test.tsx.snap b/src/features/cluster/namespaces/__snapshots__/edit-namespace-from-new-tab.test.tsx.snap index 7f60a06fbe..6675611fca 100644 --- a/src/features/cluster/namespaces/__snapshots__/edit-namespace-from-new-tab.test.tsx.snap +++ b/src/features/cluster/namespaces/__snapshots__/edit-namespace-from-new-tab.test.tsx.snap @@ -721,48 +721,7 @@ exports[`cluster/namespaces - edit namespace from new tab when navigating to nam
-
-
- - - info_outline - - -
-
- Loading resource failed: some-error -
-
- - - close - - -
-
-
+ />
-
-
- - - info_outline - - -
-
-

- Failed to save resource: - - some-error -

-
-
- - - close - - -
-
-
+ />
-
-
- - - info_outline - - -
-
-

- Failed to save resource: - - Some error -

-
-
- - - close - - -
-
-
+ />
{ let builder: ApplicationBuilder; - let callForNamespaceMock: AsyncFnMock; - let callForPatchNamespaceMock: AsyncFnMock; + let callForResourceMock: AsyncFnMock; + let callForPatchResourceMock: AsyncFnMock; let showSuccessNotificationMock: jest.Mock; let showErrorNotificationMock: jest.Mock; let storagesAreReady: () => Promise; @@ -38,12 +38,6 @@ describe("cluster/namespaces - edit namespace from new tab", () => { builder.setEnvironmentToClusterFrame(); - callForNamespaceMock = asyncFn(); - callForPatchNamespaceMock = asyncFn(); - - showSuccessNotificationMock = jest.fn(); - showErrorNotificationMock = jest.fn(); - builder.beforeWindowStart((windowDi) => { windowDi.override( directoryForLensLocalStorageInjectable, @@ -54,15 +48,11 @@ describe("cluster/namespaces - edit namespace from new tab", () => { storagesAreReady = controlWhenStoragesAreReady(windowDi); - windowDi.override( - showSuccessNotificationInjectable, - () => showSuccessNotificationMock, - ); + showSuccessNotificationMock = jest.fn(); + windowDi.override(showSuccessNotificationInjectable, () => showSuccessNotificationMock); - windowDi.override( - showErrorNotificationInjectable, - () => showErrorNotificationMock, - ); + showErrorNotificationMock = jest.fn(); + windowDi.override(showErrorNotificationInjectable, () => showErrorNotificationMock); windowDi.override(getRandomIdForEditResourceTabInjectable, () => jest @@ -71,31 +61,11 @@ describe("cluster/namespaces - edit namespace from new tab", () => { .mockReturnValueOnce("some-second-tab-id"), ); - windowDi.override(callForResourceInjectable, () => async (selfLink: string) => { - if ( - [ - "/apis/some-api-version/namespaces/some-uid", - "/apis/some-api-version/namespaces/some-other-uid", - ].includes(selfLink) - ) { - return await callForNamespaceMock(selfLink); - } + callForResourceMock = asyncFn(); + windowDi.override(callForResourceInjectable, () => callForResourceMock); - return undefined; - }); - - windowDi.override(callForPatchResourceInjectable, () => async (namespace, ...args) => { - if ( - [ - "/apis/some-api-version/namespaces/some-uid", - "/apis/some-api-version/namespaces/some-other-uid", - ].includes(namespace.selfLink) - ) { - return await callForPatchNamespaceMock(namespace, ...args); - } - - return undefined; - }); + callForPatchResourceMock = asyncFn(); + windowDi.override(callForPatchResourceInjectable, () => callForPatchResourceMock); }); builder.allowKubeResource({ @@ -115,14 +85,11 @@ describe("cluster/namespaces - edit namespace from new tab", () => { windowDi = builder.applicationWindow.only.di; - const navigateToNamespaces = windowDi.inject( - navigateToNamespacesInjectable, - ); + const navigateToNamespaces = windowDi.inject(navigateToNamespacesInjectable); + const dockStore = windowDi.inject(dockStoreInjectable); navigateToNamespaces(); - const dockStore = windowDi.inject(dockStoreInjectable); - // TODO: Make TerminalWindow unit testable to allow realistic behaviour dockStore.closeTab("terminal"); }); @@ -193,7 +160,7 @@ describe("cluster/namespaces - edit namespace from new tab", () => { }); it("calls for namespace", () => { - expect(callForNamespaceMock).toHaveBeenCalledWith( + expect(callForResourceMock).toHaveBeenCalledWith( "/apis/some-api-version/namespaces/some-uid", ); }); @@ -216,7 +183,7 @@ describe("cluster/namespaces - edit namespace from new tab", () => { }, }); - await callForNamespaceMock.resolve({ + await callForResourceMock.resolve({ callWasSuccessful: true, response: someNamespace, }); @@ -263,7 +230,7 @@ metadata: }); it("calls for save with empty values", () => { - expect(callForPatchNamespaceMock).toHaveBeenCalledWith( + expect(callForPatchResourceMock).toHaveBeenCalledWith( someNamespace, [], ); @@ -293,7 +260,7 @@ metadata: describe("when saving resolves with success", () => { beforeEach(async () => { - await callForPatchNamespaceMock.resolve({ + await callForPatchResourceMock.resolve({ callWasSuccessful: true, response: { name: "some-name", kind: "Namespace" }, }); @@ -342,7 +309,7 @@ metadata: describe("when saving resolves with failure", () => { beforeEach(async () => { - await callForPatchNamespaceMock.resolve({ + await callForPatchResourceMock.resolve({ callWasSuccessful: false, error: "some-error", }); @@ -411,7 +378,7 @@ metadata: describe("when saving resolves with success", () => { beforeEach(async () => { - await callForPatchNamespaceMock.resolve({ + await callForPatchResourceMock.resolve({ callWasSuccessful: true, response: { name: "some-name", kind: "Namespace" }, }); @@ -430,7 +397,7 @@ metadata: describe("when saving resolves with failure", () => { beforeEach(async () => { - await callForPatchNamespaceMock.resolve({ + await callForPatchResourceMock.resolve({ callWasSuccessful: false, error: "Some error", }); @@ -558,7 +525,7 @@ metadata: }); it("calls for save with changed configuration", () => { - expect(callForPatchNamespaceMock).toHaveBeenCalledWith( + expect(callForPatchResourceMock).toHaveBeenCalledWith( someNamespace, [ { @@ -580,7 +547,7 @@ metadata: }); it("given save resolves and another change in configuration, when saving, calls for save with changed configuration", async () => { - await callForPatchNamespaceMock.resolve({ + await callForPatchResourceMock.resolve({ callWasSuccessful: true, response: { @@ -610,7 +577,7 @@ metadata: }); - callForPatchNamespaceMock.mockClear(); + callForPatchResourceMock.mockClear(); const saveButton = rendered.getByTestId( "save-edit-resource-from-tab-for-some-first-tab-id", @@ -618,7 +585,7 @@ metadata: fireEvent.click(saveButton); - expect(callForPatchNamespaceMock).toHaveBeenCalledWith( + expect(callForPatchResourceMock).toHaveBeenCalledWith( someNamespace, [ { @@ -717,7 +684,7 @@ metadata: describe("given clicking the context menu for second namespace, when clicking to edit namespace", () => { beforeEach(() => { - callForNamespaceMock.mockClear(); + callForResourceMock.mockClear(); // TODO: Make implementation match the description const namespaceStub = new Namespace(someOtherNamespaceDataStub); @@ -750,7 +717,7 @@ metadata: }); it("calls for second namespace", () => { - expect(callForNamespaceMock).toHaveBeenCalledWith( + expect(callForResourceMock).toHaveBeenCalledWith( "/apis/some-api-version/namespaces/some-other-uid", ); }); @@ -772,7 +739,7 @@ metadata: }, }); - await callForNamespaceMock.resolve({ + await callForResourceMock.resolve({ callWasSuccessful: true, response: someOtherNamespace, }); @@ -798,7 +765,7 @@ metadata: }); it("when selecting to save, calls for save of second namespace", () => { - callForPatchNamespaceMock.mockClear(); + callForPatchResourceMock.mockClear(); const saveButton = rendered.getByTestId( "save-edit-resource-from-tab-for-some-second-tab-id", @@ -806,7 +773,7 @@ metadata: fireEvent.click(saveButton); - expect(callForPatchNamespaceMock).toHaveBeenCalledWith( + expect(callForPatchResourceMock).toHaveBeenCalledWith( someOtherNamespace, [], ); @@ -814,7 +781,7 @@ metadata: describe("when clicking dock tab for the first namespace", () => { beforeEach(() => { - callForNamespaceMock.mockClear(); + callForResourceMock.mockClear(); const tab = rendered.getByTestId("dock-tab-for-some-first-tab-id"); @@ -844,7 +811,7 @@ metadata: }); it("does not call for namespace", () => { - expect(callForNamespaceMock).not.toHaveBeenCalled(); + expect(callForResourceMock).not.toHaveBeenCalledWith("/apis/some-api-version/namespaces/some-uid"); }); it("has configuration in the editor", () => { @@ -865,7 +832,7 @@ metadata: }); it("when selecting to save, calls for save of first namespace", () => { - callForPatchNamespaceMock.mockClear(); + callForPatchResourceMock.mockClear(); const saveButton = rendered.getByTestId( "save-edit-resource-from-tab-for-some-first-tab-id", @@ -873,7 +840,7 @@ metadata: fireEvent.click(saveButton); - expect(callForPatchNamespaceMock).toHaveBeenCalledWith( + expect(callForPatchResourceMock).toHaveBeenCalledWith( someNamespace, [], ); @@ -885,7 +852,7 @@ metadata: describe("when call for namespace resolves without namespace", () => { beforeEach(async () => { - await callForNamespaceMock.resolve({ + await callForResourceMock.resolve({ callWasSuccessful: true, response: undefined, }); @@ -914,7 +881,7 @@ metadata: describe("when call for namespace resolves with failure", () => { beforeEach(async () => { - await callForNamespaceMock.resolve({ + await callForResourceMock.resolve({ callWasSuccessful: false, error: "some-error", }); diff --git a/src/renderer/components/dock/edit-resource/edit-resource-model/edit-resource-model.injectable.tsx b/src/renderer/components/dock/edit-resource/edit-resource-model/edit-resource-model.injectable.tsx index f6e3d09d63..f7efb026d8 100644 --- a/src/renderer/components/dock/edit-resource/edit-resource-model/edit-resource-model.injectable.tsx +++ b/src/renderer/components/dock/edit-resource/edit-resource-model/edit-resource-model.injectable.tsx @@ -7,8 +7,8 @@ import type { CallForResource } from "./call-for-resource/call-for-resource.inje import callForResourceInjectable from "./call-for-resource/call-for-resource.injectable"; import { waitUntilDefined } from "../../../../../common/utils"; import editResourceTabStoreInjectable from "../store.injectable"; -import type { EditingResource, EditResourceTabStore } from "../store"; -import { action, computed, observable, runInAction } from "mobx"; +import type { EditResourceTabStore } from "../store"; +import { action, computed, makeObservable, observable, runInAction } from "mobx"; import type { KubeObject } from "../../../../../common/k8s-api/kube-object"; import yaml from "js-yaml"; import assert from "assert"; @@ -24,18 +24,13 @@ const editResourceModelInjectable = getInjectable({ id: "edit-resource-model", instantiate: async (di, tabId: string) => { - const store = di.inject(editResourceTabStoreInjectable); - const model = new EditResourceModel({ callForResource: di.inject(callForResourceInjectable), callForPatchResource: di.inject(callForPatchResourceInjectable), showSuccessNotification: di.inject(showSuccessNotificationInjectable), showErrorNotification: di.inject(showErrorNotificationInjectable), - store, + store: di.inject(editResourceTabStoreInjectable), tabId, - - waitForEditingResource: () => - waitUntilDefined(() => store.getData(tabId)), }); await model.load(); @@ -53,15 +48,15 @@ export default editResourceModelInjectable; interface Dependencies { callForResource: CallForResource; callForPatchResource: CallForPatchResource; - waitForEditingResource: () => Promise; showSuccessNotification: ShowNotification; showErrorNotification: ShowNotification; - store: EditResourceTabStore; - tabId: string; + readonly store: EditResourceTabStore; + readonly tabId: string; } export class EditResourceModel { - constructor(private dependencies: Dependencies) { + constructor(private readonly dependencies: Dependencies) { + makeObservable(this); } readonly configuration = { @@ -81,103 +76,103 @@ export class EditResourceModel { }, }; - @observable private _resource: KubeObject | undefined; + @observable private _resource: KubeObject | undefined; - @computed get shouldShowErrorAboutNoResource() { - return !this._resource; - } + @computed get shouldShowErrorAboutNoResource() { + return !this._resource; + } - @computed get resource() { - assert(this._resource, "Resource does not have data"); + @computed get resource() { + assert(this._resource, "Resource does not have data"); - return this._resource; - } + return this._resource; + } - @computed get editingResource() { - const resource = this.dependencies.store.getData(this.dependencies.tabId); + @computed get editingResource() { + const resource = this.dependencies.store.getData(this.dependencies.tabId); - assert(resource, "Resource is not present in the store"); + assert(resource, "Resource is not present in the store"); - return resource; - } + return resource; + } - @computed private get selfLink() { - return this.editingResource.resource; - } + @computed private get selfLink() { + return this.editingResource.resource; + } - load = async () => { - await this.dependencies.waitForEditingResource(); + load = async () => { + await waitUntilDefined(() => this.dependencies.store.getData(this.dependencies.tabId)); - const result = await this.dependencies.callForResource(this.selfLink); + const result = await this.dependencies.callForResource(this.selfLink); - if (!result.callWasSuccessful) { - this.dependencies.showErrorNotification( - `Loading resource failed: ${result.error}`, - ); + if (!result.callWasSuccessful) { + this.dependencies.showErrorNotification( + `Loading resource failed: ${result.error}`, + ); - return; - } + return; + } - const resource = result.response; + runInAction(() => { + this._resource = result.response; - runInAction(() => { - this._resource = resource; - }); + if (this._resource) { + this.editingResource.firstDraft = yaml.dump( + this._resource.toPlainObject(), + ); + } + }); + }; - if (!resource) { - return; - } + get namespace() { + return this.resource.metadata.namespace || "default"; + } - runInAction(() => { - this.editingResource.firstDraft = yaml.dump(resource.toPlainObject()); - }); - }; + get name() { + return this.resource.metadata.name; + } - get namespace() { - return this.resource.metadata.namespace || "default"; - } + get kind() { + return this.resource.kind; + } - get name() { - return this.resource.metadata.name; - } + save = async () => { + const currentValue = this.configuration.value.get(); + const currentVersion = yaml.load(currentValue); + const firstVersion = yaml.load( + this.editingResource.firstDraft ?? currentValue, + ); + const patches = createPatch(firstVersion, currentVersion); - get kind() { - return this.resource.kind; - } + const result = await this.dependencies.callForPatchResource( + this.resource, + patches, + ); - save = async () => { - const currentValue = this.configuration.value.get(); - const currentVersion = yaml.load(currentValue); - const firstVersion = yaml.load(this.editingResource.firstDraft ?? currentValue); - const patches = createPatch(firstVersion, currentVersion); + if (!result.callWasSuccessful) { + this.dependencies.showErrorNotification(( +

+ Failed to save resource: + {" "} + {result.error} +

+ )); - const result = await this.dependencies.callForPatchResource(this.resource, patches); + return; + } - if (!result.callWasSuccessful) { - this.dependencies.showErrorNotification( -

- Failed to save resource: - {" "} - {result.error} -

, - ); + const { kind, name } = result.response; - return; - } + this.dependencies.showSuccessNotification(( +

+ {`${kind} `} + {name} + {" updated."} +

+ )); - const { kind, name } = result.response; - - this.dependencies.showSuccessNotification( -

- {kind} - {" "} - {name} - {" updated."} -

, - ); - - runInAction(() => { - this.editingResource.firstDraft = currentValue; - }); - }; + runInAction(() => { + this.editingResource.firstDraft = currentValue; + }); + }; } diff --git a/src/renderer/components/dock/edit-resource/view.tsx b/src/renderer/components/dock/edit-resource/view.tsx index d448a86ce8..0465f80c93 100644 --- a/src/renderer/components/dock/edit-resource/view.tsx +++ b/src/renderer/components/dock/edit-resource/view.tsx @@ -22,17 +22,19 @@ interface Dependencies { model: EditResourceModel; } -const NonInjectedEditResource = observer( - ({ model, tabId }: EditResourceProps & Dependencies) => { - return ( -
- {model.shouldShowErrorAboutNoResource && ( +const NonInjectedEditResource = observer(({ + model, + tabId, +}: EditResourceProps & Dependencies) => ( +
+ { + model.shouldShowErrorAboutNoResource + ? ( Resource not found - )} - - {!model.shouldShowErrorAboutNoResource && ( + ) + : ( <> Namespace:
- )} - /> + )} /> + onError={model.configuration.error.onChange} /> - )} -
- ); - }, + ) + } +
+), ); -export const EditResource = withInjectables( - NonInjectedEditResource, - { - getPlaceholder: () => ( - - ), - - getProps: async (di, props) => ({ - model: await di.inject(editResourceModelInjectable, props.tabId), - ...props, - }), - }, -); +export const EditResource = withInjectables(NonInjectedEditResource, { + getPlaceholder: () => ( + + ), + getProps: async (di, props) => ({ + ...props, + model: await di.inject(editResourceModelInjectable, props.tabId), + }), +}); diff --git a/src/renderer/components/test-utils/get-application-builder.tsx b/src/renderer/components/test-utils/get-application-builder.tsx index c27d677425..41f26803d4 100644 --- a/src/renderer/components/test-utils/get-application-builder.tsx +++ b/src/renderer/components/test-utils/get-application-builder.tsx @@ -512,9 +512,10 @@ export const getApplicationBuilder = () => { ); windowDi.override(hostedClusterIdInjectable, () => clusterStub.id); + windowDi.override(hostedClusterInjectable, () => clusterStub); // TODO: Figure out a way to remove this stub. - const namespaceStoreStub = { + windowDi.override(namespaceStoreInjectable, () => ({ isLoaded: true, get contextNamespaces() { return Array.from(selectedNamespaces); @@ -531,10 +532,7 @@ export const getApplicationBuilder = () => { pickOnlySelected: () => [], isSelectedAll: () => false, getTotalCount: () => namespaceItems.length, - } as Partial as NamespaceStore; - - windowDi.override(namespaceStoreInjectable, () => namespaceStoreStub); - windowDi.override(hostedClusterInjectable, () => clusterStub); + } as Partial as NamespaceStore)); }); return builder;