From 0884132c807bec032c0cea966e4de82098462993 Mon Sep 17 00:00:00 2001 From: Huw Wilkins Date: Wed, 7 Feb 2024 15:09:48 +1100 Subject: [PATCH 1/3] Add panel to remove secrets. --- src/juju/apiHooks.test.tsx | 167 +++++++++++++ src/juju/apiHooks.ts | 38 +++ .../SecretsTable/SecretsTable.test.tsx | 20 ++ .../Secrets/SecretsTable/SecretsTable.tsx | 31 ++- src/panels/Panels.test.tsx | 10 + src/panels/Panels.tsx | 3 + .../RemoveSecretPanel/Fields/Fields.test.tsx | 221 ++++++++++++++++++ .../RemoveSecretPanel/Fields/Fields.tsx | 93 ++++++++ src/panels/RemoveSecretPanel/Fields/index.ts | 1 + .../RemoveSecretPanel.test.tsx | 203 ++++++++++++++++ .../RemoveSecretPanel/RemoveSecretPanel.tsx | 150 ++++++++++++ src/panels/RemoveSecretPanel/index.ts | 1 + src/panels/RemoveSecretPanel/types.ts | 4 + 13 files changed, 940 insertions(+), 2 deletions(-) create mode 100644 src/panels/RemoveSecretPanel/Fields/Fields.test.tsx create mode 100644 src/panels/RemoveSecretPanel/Fields/Fields.tsx create mode 100644 src/panels/RemoveSecretPanel/Fields/index.ts create mode 100644 src/panels/RemoveSecretPanel/RemoveSecretPanel.test.tsx create mode 100644 src/panels/RemoveSecretPanel/RemoveSecretPanel.tsx create mode 100644 src/panels/RemoveSecretPanel/index.ts create mode 100644 src/panels/RemoveSecretPanel/types.ts diff --git a/src/juju/apiHooks.test.tsx b/src/juju/apiHooks.test.tsx index 5357e8d83..098ef0aba 100644 --- a/src/juju/apiHooks.test.tsx +++ b/src/juju/apiHooks.test.tsx @@ -24,6 +24,7 @@ import { useCreateSecrets, useModelConnectionCallback, useGetSecretContent, + useRemoveSecrets, } from "./apiHooks"; const mockStore = configureStore([]); @@ -932,3 +933,169 @@ describe("useCreateSecrets", () => { ); }); }); + +describe("useRemoveSecrets", () => { + let state: RootState; + + beforeEach(() => { + state = rootStateFactory.build({ + general: generalStateFactory.build({ + credentials: { + "wss://example.com/api": credentialFactory.build(), + }, + config: configFactory.build({ + controllerAPIEndpoint: "wss://example.com/api", + }), + }), + juju: { + models: { + abc123: modelListInfoFactory.build({ + wsControllerURL: "wss://example.com/api", + uuid: "abc123", + }), + }, + }, + }); + }); + + it("can remove secrets", async () => { + const store = mockStore(state); + changeURL("/models/eggman@external/group-test/app/etcd"); + const secrets = [ + { + uri: "secret:aabbccdd", + }, + ]; + const results = { results: [{ result: "secret:def456" }] }; + const removeSecrets = jest + .fn() + .mockImplementation(() => Promise.resolve(results)); + const loginResponse = { + conn: { + facades: { + secrets: { + removeSecrets, + }, + }, + } as unknown as Connection, + logout: jest.fn(), + }; + jest + .spyOn(jujuLib, "connectAndLogin") + .mockImplementation(() => Promise.resolve(loginResponse)); + const { result } = renderHook( + () => useRemoveSecrets("eggman@external", "test-model"), + { + wrapper: (props) => ( + + ), + }, + ); + await expect(result.current(secrets)).resolves.toBe(results); + expect(removeSecrets).toHaveBeenCalledWith({ args: secrets }); + }); + + it("handles no connection", async () => { + const store = mockStore(state); + changeURL("/models/eggman@external/group-test/app/etcd"); + const secrets = [ + { + uri: "secret:aabbccdd", + }, + ]; + const loginResponse = { + logout: jest.fn(), + }; + jest + .spyOn(jujuLib, "connectAndLogin") + .mockImplementation(() => Promise.resolve(loginResponse)); + const { result } = renderHook( + () => useRemoveSecrets("eggman@external", "test-model"), + { + wrapper: (props) => ( + + ), + }, + ); + await expect(result.current(secrets)).rejects.toStrictEqual( + new Error("Unable to connect to model"), + ); + }); + + it("handles connection errors", async () => { + const store = mockStore(state); + changeURL("/models/eggman@external/group-test/app/etcd"); + const secrets = [ + { + uri: "secret:aabbccdd", + }, + ]; + jest + .spyOn(jujuLib, "connectAndLogin") + .mockImplementation(() => Promise.reject(new Error())); + const { result } = renderHook( + () => useRemoveSecrets("eggman@external", "test-model"), + { + wrapper: (props) => ( + + ), + }, + ); + await expect(result.current(secrets)).rejects.toBe( + "Error during promise race.", + ); + }); + + it("handles errors from removing secrets", async () => { + const store = mockStore(state); + changeURL("/models/eggman@external/group-test/app/etcd"); + const secrets = [ + { + uri: "secret:aabbccdd", + }, + ]; + const removeSecrets = jest + .fn() + .mockImplementation(() => Promise.reject(new Error("Uh oh!"))); + const loginResponse = { + conn: { + facades: { + secrets: { + removeSecrets, + }, + }, + } as unknown as Connection, + logout: jest.fn(), + }; + jest + .spyOn(jujuLib, "connectAndLogin") + .mockImplementation(() => Promise.resolve(loginResponse)); + const { result } = renderHook( + () => useRemoveSecrets("eggman@external", "test-model"), + { + wrapper: (props) => ( + + ), + }, + ); + await expect(result.current(secrets)).rejects.toStrictEqual( + new Error("Uh oh!"), + ); + }); +}); diff --git a/src/juju/apiHooks.ts b/src/juju/apiHooks.ts index 468b04496..d791a9974 100644 --- a/src/juju/apiHooks.ts +++ b/src/juju/apiHooks.ts @@ -3,6 +3,8 @@ import type { ListSecretsArgs, CreateSecretArgs, StringResults, + ErrorResults, + DeleteSecretArg, } from "@canonical/jujulib/dist/api/facades/secrets/SecretsV2"; import { useEffect, useCallback } from "react"; import { useSelector } from "react-redux"; @@ -234,3 +236,39 @@ export const useCreateSecrets = (userName?: string, modelName?: string) => { [modelConnectionCallback], ); }; + +export const useRemoveSecrets = (userName?: string, modelName?: string) => { + const modelUUID = useSelector(getModelUUIDFromList(modelName, userName)); + const modelConnectionCallback = useModelConnectionCallback(modelUUID); + return useCallback( + (secrets: Partial[]) => { + return new Promise((resolve, reject) => { + modelConnectionCallback( + ( + connection?: ConnectionWithFacades | null, + error?: string | null, + ) => { + if (error) { + reject(error); + return; + } + if (!connection) { + reject(new Error("Unable to connect to model")); + return; + } + connection.facades.secrets + // Cast to `DeleteSecretArg` as the API requires either label or + // URI, but the type declares both as required. + ?.removeSecrets({ args: secrets as DeleteSecretArg[] }) + .then((response) => { + resolve(response); + return; + }) + .catch((error) => reject(error)); + }, + ); + }); + }, + [modelConnectionCallback], + ); +}; diff --git a/src/pages/EntityDetails/Model/Secrets/SecretsTable/SecretsTable.test.tsx b/src/pages/EntityDetails/Model/Secrets/SecretsTable/SecretsTable.test.tsx index ef83e1c1c..a4d3d2c2d 100644 --- a/src/pages/EntityDetails/Model/Secrets/SecretsTable/SecretsTable.test.tsx +++ b/src/pages/EntityDetails/Model/Secrets/SecretsTable/SecretsTable.test.tsx @@ -151,4 +151,24 @@ describe("SecretsTable", () => { }), ); }); + + it("can display the remove secret panel", async () => { + state.juju.secrets = secretsStateFactory.build({ + abc123: modelSecretsFactory.build({ + items: [listSecretResultFactory.build({ uri: "secret:aabbccdd" })], + loaded: true, + }), + }); + renderComponent(, { state, path, url }); + expect(window.location.search).toEqual(""); + await userEvent.click( + screen.getByRole("button", { name: Label.ACTION_MENU }), + ); + await userEvent.click( + screen.getByRole("button", { name: Label.REMOVE_BUTTON }), + ); + expect(window.location.search).toEqual( + "?panel=remove-secret&secret=secret%3Aaabbccdd", + ); + }); }); diff --git a/src/pages/EntityDetails/Model/Secrets/SecretsTable/SecretsTable.tsx b/src/pages/EntityDetails/Model/Secrets/SecretsTable/SecretsTable.tsx index 87b3e123d..d7506955d 100644 --- a/src/pages/EntityDetails/Model/Secrets/SecretsTable/SecretsTable.tsx +++ b/src/pages/EntityDetails/Model/Secrets/SecretsTable/SecretsTable.tsx @@ -3,6 +3,7 @@ import { Button, Icon, Tooltip, + ContextualMenu, } from "@canonical/react-components"; import type { ReactNode } from "react"; import { useMemo } from "react"; @@ -15,6 +16,7 @@ import RelativeDate from "components/RelativeDate"; import type { EntityDetailsRoute } from "components/Routes/Routes"; import TruncatedTooltip from "components/TruncatedTooltip"; import { copyToClipboard } from "components/utils"; +import { useQueryParams } from "hooks/useQueryParams"; import { getSecretsLoading, getSecretsLoaded, @@ -26,7 +28,9 @@ import { useAppSelector } from "store/store"; import SecretContent from "../SecretContent"; export enum Label { + ACTION_MENU = "Action menu", COPY = "Copy", + REMOVE_BUTTON = "Remove secret", } export enum TestId { @@ -43,6 +47,13 @@ const SecretsTable = () => { const secretsLoading = useAppSelector((state) => getSecretsLoading(state, modelUUID), ); + const [, setQuery] = useQueryParams<{ + panel: string | null; + secret: string | null; + }>({ + panel: null, + secret: null, + }); const tableData = useMemo(() => { if (!secrets) { @@ -97,10 +108,25 @@ const SecretsTable = () => { owner, created: , updated: , - actions: "", + actions: ( + + setQuery({ panel: "remove-secret", secret: secret.uri }), + }, + ]} + position="right" + scrollOverflow + toggleAppearance="base" + toggleClassName="has-icon u-no-margin--bottom is-small" + toggleLabel={{Label.ACTION_MENU}} + /> + ), }; }); - }, [modelUUID, secrets]); + }, [modelUUID, secrets, setQuery]); const columnData: Column[] = useMemo( () => [ @@ -136,6 +162,7 @@ const SecretsTable = () => { { Header: "Actions", accessor: "actions", + className: "u-align--right", }, ], [], diff --git a/src/panels/Panels.test.tsx b/src/panels/Panels.test.tsx index 19d2b3194..5a6c19223 100644 --- a/src/panels/Panels.test.tsx +++ b/src/panels/Panels.test.tsx @@ -8,6 +8,7 @@ import { TestId as AuditLogsFilterPanelTestId } from "./AuditLogsFilterPanel/Aud import { TestId as CharmsAndActionsPanelTestId } from "./CharmsAndActionsPanel/CharmsAndActionsPanel"; import { TestId as ConfigPanelTestId } from "./ConfigPanel/ConfigPanel"; import Panels from "./Panels"; +import { TestId as RemoveSecretPanelTestId } from "./RemoveSecretPanel/RemoveSecretPanel"; import { TestId as ShareModelTestId } from "./ShareModelPanel/ShareModel"; describe("Panels", () => { @@ -54,4 +55,13 @@ describe("Panels", () => { await screen.findByTestId(AddSecretPanelTestId.PANEL), ).toBeInTheDocument(); }); + + it("can display the remove secret panel", async () => { + renderComponent(, { + url: "/?panel=remove-secret", + }); + expect( + await screen.findByTestId(RemoveSecretPanelTestId.PANEL), + ).toBeInTheDocument(); + }); }); diff --git a/src/panels/Panels.tsx b/src/panels/Panels.tsx index fe2d61d83..f4c0f9880 100644 --- a/src/panels/Panels.tsx +++ b/src/panels/Panels.tsx @@ -8,6 +8,7 @@ import ShareModel from "panels/ShareModelPanel/ShareModel"; import AuditLogsFilterPanel from "./AuditLogsFilterPanel"; import CharmsAndActionsPanel from "./CharmsAndActionsPanel/CharmsAndActionsPanel"; import ConfigPanel from "./ConfigPanel/ConfigPanel"; +import RemoveSecretPanel from "./RemoveSecretPanel"; export default function Panels() { const [panelQs] = useQueryParams<{ panel: string | null }>({ @@ -28,6 +29,8 @@ export default function Panels() { return ; case "add-secret": return ; + case "remove-secret": + return ; default: return null; } diff --git a/src/panels/RemoveSecretPanel/Fields/Fields.test.tsx b/src/panels/RemoveSecretPanel/Fields/Fields.test.tsx new file mode 100644 index 000000000..fe3052630 --- /dev/null +++ b/src/panels/RemoveSecretPanel/Fields/Fields.test.tsx @@ -0,0 +1,221 @@ +import { screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { Formik } from "formik"; + +import type { RootState } from "store/store"; +import { rootStateFactory } from "testing/factories"; +import { + configFactory, + generalStateFactory, + credentialFactory, +} from "testing/factories/general"; +import { + modelListInfoFactory, + secretsStateFactory, + listSecretResultFactory, + modelSecretsFactory, +} from "testing/factories/juju/juju"; +import { renderComponent } from "testing/utils"; +import urls from "urls"; + +import { secretRevisionFactory } from "../../../testing/factories/juju/juju"; +import type { FormFields } from "../types"; + +import Fields, { Label } from "./Fields"; + +describe("Fields", () => { + let state: RootState; + const path = urls.model.index(null); + const url = urls.model.index({ + userName: "eggman@external", + modelName: "test-model", + }); + + beforeEach(() => { + state = rootStateFactory.build({ + general: generalStateFactory.build({ + credentials: { + "wss://example.com/api": credentialFactory.build(), + }, + config: configFactory.build({ + controllerAPIEndpoint: "wss://example.com/api", + }), + }), + juju: { + models: { + abc123: modelListInfoFactory.build({ + wsControllerURL: "wss://example.com/api", + uuid: "abc123", + }), + }, + secrets: secretsStateFactory.build({ + abc123: modelSecretsFactory.build({ + items: [ + listSecretResultFactory.build({ + uri: "secret:aabbccdd", + label: "secret1", + "latest-revision": 2, + revisions: [ + secretRevisionFactory.build({ revision: 1 }), + secretRevisionFactory.build({ revision: 2 }), + ], + }), + ], + loaded: true, + }), + }), + }, + }); + }); + + it("lists revisions", async () => { + renderComponent( + + initialValues={{ removeAll: false, revision: "" }} + onSubmit={jest.fn()} + > + + , + { state, path, url }, + ); + expect( + screen.getByRole("option", { name: "2 (latest)" }), + ).toBeInTheDocument(); + expect(screen.getByRole("option", { name: "1" })).toBeInTheDocument(); + }); + + it("can show a confirmation", async () => { + renderComponent( + + initialValues={{ removeAll: false, revision: "" }} + onSubmit={jest.fn()} + > + + , + { state, path, url }, + ); + expect( + screen.getByRole("dialog", { name: Label.CONFIRM_TITLE }), + ).toBeInTheDocument(); + }); + + it("disables the revision field if the remove all checkbox is checked", async () => { + const handleRemoveSecret = jest.fn(); + renderComponent( + + initialValues={{ removeAll: false, revision: "" }} + onSubmit={jest.fn()} + > + + , + { state, path, url }, + ); + await userEvent.click( + screen.getByRole("checkbox", { name: Label.REMOVE_ALL }), + ); + await userEvent.click( + screen.getByRole("button", { name: Label.CONFIRM_BUTTON }), + ); + expect( + screen.getByRole("combobox", { name: Label.REVISION }), + ).toBeDisabled(); + }); + + it("can cancel the confirmation", async () => { + const hideConfirm = jest.fn(); + renderComponent( + + initialValues={{ removeAll: false, revision: "" }} + onSubmit={jest.fn()} + > + + , + { state, path, url }, + ); + await userEvent.selectOptions( + screen.getByRole("combobox", { name: Label.REVISION }), + "2 (latest)", + ); + await userEvent.click( + screen.getByRole("button", { name: Label.CANCEL_BUTTON }), + ); + expect(hideConfirm).toHaveBeenCalled(); + }); + + it("can confirm a revision and call the callback", async () => { + const handleRemoveSecret = jest.fn(); + renderComponent( + + initialValues={{ removeAll: false, revision: "" }} + onSubmit={jest.fn()} + > + + , + { state, path, url }, + ); + await userEvent.selectOptions( + screen.getByRole("combobox", { name: Label.REVISION }), + "2 (latest)", + ); + await userEvent.click( + screen.getByRole("button", { name: Label.CONFIRM_BUTTON }), + ); + expect(handleRemoveSecret).toHaveBeenCalledWith({ + removeAll: false, + revision: "2", + }); + }); + + it("can confirm all revisions and call the callback", async () => { + const handleRemoveSecret = jest.fn(); + renderComponent( + + initialValues={{ removeAll: false, revision: "" }} + onSubmit={jest.fn()} + > + + , + { state, path, url }, + ); + await userEvent.click( + screen.getByRole("checkbox", { name: Label.REMOVE_ALL }), + ); + await userEvent.click( + screen.getByRole("button", { name: Label.CONFIRM_BUTTON }), + ); + expect(handleRemoveSecret).toHaveBeenCalledWith({ + removeAll: true, + revision: "", + }); + }); +}); diff --git a/src/panels/RemoveSecretPanel/Fields/Fields.tsx b/src/panels/RemoveSecretPanel/Fields/Fields.tsx new file mode 100644 index 000000000..b47ce8810 --- /dev/null +++ b/src/panels/RemoveSecretPanel/Fields/Fields.tsx @@ -0,0 +1,93 @@ +import { Select, ConfirmationModal } from "@canonical/react-components"; +import { useFormikContext } from "formik"; +import { useSelector } from "react-redux"; +import { useParams } from "react-router-dom"; +import usePortal from "react-useportal"; + +import FormikField from "components/FormikField/FormikField"; +import type { EntityDetailsRoute } from "components/Routes/Routes"; +import { getSecretByURI, getModelUUIDFromList } from "store/juju/selectors"; +import { useAppSelector } from "store/store"; + +import type { FormFields } from "../types"; + +export enum Label { + CANCEL_BUTTON = "Cancel", + CONFIRM_BUTTON = "Remove", + CONFIRM_TITLE = "Remove secret?", + REMOVE_ALL = "Remove all revisions", + REVISION = "Revision", +} + +type Props = { + hideConfirm: () => void; + handleRemoveSecret: (values: FormFields) => void; + secretURI?: string | null; + showConfirm: boolean; +}; + +const Fields = ({ + hideConfirm, + handleRemoveSecret, + secretURI, + showConfirm, +}: Props): JSX.Element => { + const { userName, modelName } = useParams(); + const modelUUID = useSelector(getModelUUIDFromList(modelName, userName)); + const { values } = useFormikContext(); + const secret = useAppSelector((state) => + getSecretByURI(state, modelUUID, secretURI), + ); + const { Portal } = usePortal(); + + return ( + <> + + ({ + label: `${revision}${revision === secret?.["latest-revision"] ? " (latest)" : ""}`, + value: revision.toString(), + })) ?? [] + } + /> + {showConfirm ? ( + + + This action can't be undone. +

+ } + onConfirm={(event: React.MouseEvent) => { + // Stop propagation of the click event in order for the Panel + // to remain open after an error occurs in executeAction(). + // Remove this manual fix once this issue gets resolved: + // https://github.com/canonical/react-components/issues/1032 + event.nativeEvent.stopImmediatePropagation(); + handleRemoveSecret(values); + }} + close={hideConfirm} + > +

Are you sure you would like to remove this secret?

+
+
+ ) : null} + + ); +}; + +export default Fields; diff --git a/src/panels/RemoveSecretPanel/Fields/index.ts b/src/panels/RemoveSecretPanel/Fields/index.ts new file mode 100644 index 000000000..9ee28253f --- /dev/null +++ b/src/panels/RemoveSecretPanel/Fields/index.ts @@ -0,0 +1 @@ +export { default } from "./Fields"; diff --git a/src/panels/RemoveSecretPanel/RemoveSecretPanel.test.tsx b/src/panels/RemoveSecretPanel/RemoveSecretPanel.test.tsx new file mode 100644 index 000000000..86ed5fe5c --- /dev/null +++ b/src/panels/RemoveSecretPanel/RemoveSecretPanel.test.tsx @@ -0,0 +1,203 @@ +import { screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; + +import * as apiHooks from "juju/apiHooks"; +import type { RootState } from "store/store"; +import { rootStateFactory } from "testing/factories"; +import { + configFactory, + generalStateFactory, + credentialFactory, +} from "testing/factories/general"; +import { + modelListInfoFactory, + secretsStateFactory, + listSecretResultFactory, + modelSecretsFactory, + secretRevisionFactory, +} from "testing/factories/juju/juju"; +import { renderComponent } from "testing/utils"; +import urls from "urls"; + +import { Label as FieldsLabel } from "./Fields/Fields"; +import RemoveSecretPanel, { Label } from "./RemoveSecretPanel"; + +jest.mock("juju/apiHooks", () => { + return { + useRemoveSecrets: jest.fn().mockReturnValue(jest.fn()), + useListSecrets: jest.fn().mockReturnValue(jest.fn()), + }; +}); + +describe("RemoveSecretPanel", () => { + let state: RootState; + const path = urls.model.index(null); + const url = `${urls.model.index({ + userName: "eggman@external", + modelName: "test-model", + })}?panel=remove-secret&secret=secret:aabbccdd`; + + beforeEach(() => { + state = rootStateFactory.build({ + general: generalStateFactory.build({ + credentials: { + "wss://example.com/api": credentialFactory.build(), + }, + config: configFactory.build({ + controllerAPIEndpoint: "wss://example.com/api", + }), + }), + juju: { + models: { + abc123: modelListInfoFactory.build({ + wsControllerURL: "wss://example.com/api", + uuid: "abc123", + }), + }, + secrets: secretsStateFactory.build({ + abc123: modelSecretsFactory.build({ + items: [ + listSecretResultFactory.build({ + uri: "secret:aabbccdd", + label: "secret1", + "latest-revision": 2, + revisions: [ + secretRevisionFactory.build({ revision: 1 }), + secretRevisionFactory.build({ revision: 2 }), + ], + }), + ], + loaded: true, + }), + }), + }, + }); + jest.spyOn(apiHooks, "useListSecrets").mockImplementation(() => jest.fn()); + }); + + it("shows a confirmation when the form is submitted", async () => { + const removeSecrets = jest + .fn() + .mockImplementation(() => Promise.resolve({ results: [] })); + jest + .spyOn(apiHooks, "useRemoveSecrets") + .mockImplementation(() => removeSecrets); + renderComponent(, { state, path, url }); + await userEvent.click(screen.getByRole("button", { name: Label.SUBMIT })); + expect( + screen.getByRole("dialog", { name: FieldsLabel.CONFIRM_TITLE }), + ).toBeInTheDocument(); + expect(removeSecrets).not.toHaveBeenCalled(); + }); + + it("can remove a secret", async () => { + const removeSecrets = jest + .fn() + .mockImplementation(() => Promise.resolve({ results: [] })); + jest + .spyOn(apiHooks, "useRemoveSecrets") + .mockImplementation(() => removeSecrets); + renderComponent(, { state, path, url }); + await userEvent.click(screen.getByRole("button", { name: Label.SUBMIT })); + await userEvent.click( + screen.getByRole("button", { name: FieldsLabel.CONFIRM_BUTTON }), + ); + expect(removeSecrets).toHaveBeenCalledWith([ + { + uri: "secret:aabbccdd", + revisions: [2], + }, + ]); + }); + + it("displays caught errors", async () => { + const removeSecrets = jest + .fn() + .mockImplementation(() => Promise.reject(new Error("Caught error"))); + jest + .spyOn(apiHooks, "useRemoveSecrets") + .mockImplementation(() => removeSecrets); + renderComponent(, { state, path, url }); + await userEvent.click(screen.getByRole("button", { name: Label.SUBMIT })); + await userEvent.click( + screen.getByRole("button", { name: FieldsLabel.CONFIRM_BUTTON }), + ); + expect( + document.querySelector(".p-notification--negative"), + ).toHaveTextContent("Caught error"); + }); + + it("displays error string results", async () => { + const removeSecrets = jest + .fn() + .mockImplementation(() => Promise.resolve("String error")); + jest + .spyOn(apiHooks, "useRemoveSecrets") + .mockImplementation(() => removeSecrets); + renderComponent(, { state, path, url }); + await userEvent.click(screen.getByRole("button", { name: Label.SUBMIT })); + await userEvent.click( + screen.getByRole("button", { name: FieldsLabel.CONFIRM_BUTTON }), + ); + expect( + document.querySelector(".p-notification--negative"), + ).toHaveTextContent("String error"); + }); + + it("displays error object results", async () => { + const removeSecrets = jest + .fn() + .mockImplementation(() => + Promise.resolve({ results: [{ error: { message: "Error result" } }] }), + ); + jest + .spyOn(apiHooks, "useRemoveSecrets") + .mockImplementation(() => removeSecrets); + renderComponent(, { state, path, url }); + await userEvent.click(screen.getByRole("button", { name: Label.SUBMIT })); + await userEvent.click( + screen.getByRole("button", { name: FieldsLabel.CONFIRM_BUTTON }), + ); + expect( + document.querySelector(".p-notification--negative"), + ).toHaveTextContent("Error result"); + }); + + it("closes the panel if successful", async () => { + const removeSecrets = jest + .fn() + .mockImplementation(() => Promise.resolve({ results: [] })); + jest + .spyOn(apiHooks, "useRemoveSecrets") + .mockImplementation(() => removeSecrets); + renderComponent(, { state, path, url }); + expect(window.location.search).toEqual( + "?panel=remove-secret&secret=secret:aabbccdd", + ); + await userEvent.click(screen.getByRole("button", { name: Label.SUBMIT })); + await userEvent.click( + screen.getByRole("button", { name: FieldsLabel.CONFIRM_BUTTON }), + ); + expect(window.location.search).toEqual(""); + }); + + it("refetches the secrets if successful", async () => { + const removeSecrets = jest + .fn() + .mockImplementation(() => Promise.resolve({ results: [] })); + jest + .spyOn(apiHooks, "useRemoveSecrets") + .mockImplementation(() => removeSecrets); + const listSecrets = jest.fn(); + jest + .spyOn(apiHooks, "useListSecrets") + .mockImplementation(() => listSecrets); + renderComponent(, { state, path, url }); + expect(listSecrets).not.toHaveBeenCalled(); + await userEvent.click(screen.getByRole("button", { name: Label.SUBMIT })); + await userEvent.click( + screen.getByRole("button", { name: FieldsLabel.CONFIRM_BUTTON }), + ); + expect(listSecrets).toHaveBeenCalled(); + }); +}); diff --git a/src/panels/RemoveSecretPanel/RemoveSecretPanel.tsx b/src/panels/RemoveSecretPanel/RemoveSecretPanel.tsx new file mode 100644 index 000000000..5dd396cbc --- /dev/null +++ b/src/panels/RemoveSecretPanel/RemoveSecretPanel.tsx @@ -0,0 +1,150 @@ +import type { ErrorResults } from "@canonical/jujulib/dist/api/facades/secrets/SecretsV2"; +import { ActionButton, Button, Spinner } from "@canonical/react-components"; +import { Form, Formik } from "formik"; +import { useId, useState, useRef, useCallback } from "react"; +import { useSelector } from "react-redux"; +import { useParams } from "react-router-dom"; + +import Panel from "components/Panel"; +import type { EntityDetailsRoute } from "components/Routes/Routes"; +import { useRemoveSecrets, useListSecrets } from "juju/apiHooks"; +import PanelInlineErrors from "panels/PanelInlineErrors"; +import { usePanelQueryParams } from "panels/hooks"; +import { getSecretByURI, getModelUUIDFromList } from "store/juju/selectors"; +import { useAppSelector } from "store/store"; + +import Fields from "./Fields"; +import { type FormFields } from "./types"; + +export enum TestId { + PANEL = "remove-secret-panel", +} + +export enum Label { + CANCEL = "Cancel", + SUBMIT = "Remove secret", +} + +const RemoveSecretPanel = () => { + const { userName, modelName } = useParams(); + const modelUUID = useSelector(getModelUUIDFromList(modelName, userName)); + const scrollArea = useRef(null); + const formId = useId(); + const [queryParams, , handleRemovePanelQueryParams] = usePanelQueryParams<{ + panel: string | null; + secret: string | null; + }>({ + panel: null, + secret: null, + }); + const secretURI = queryParams.secret; + const secret = useAppSelector((state) => + getSecretByURI(state, modelUUID, secretURI), + ); + const [inlineError, setInlineError] = useState(null); + const [saving, setSaving] = useState(false); + const [showConfirm, setShowConfirm] = useState(false); + const removeSecrets = useRemoveSecrets(userName, modelName); + const listSecrets = useListSecrets(userName, modelName); + + const handleRemoveSecret = useCallback( + (values: FormFields) => { + if (!secretURI) { + // An error is displayed instead of the form if the secretURI doesn't + // exist so it's not possible to get to this point. + return; + } + setSaving(true); + setInlineError(null); + setShowConfirm(false); + removeSecrets([ + { + revisions: values.removeAll ? undefined : [Number(values.revision)], + uri: secretURI, + }, + ]) + .then((response: ErrorResults | string) => { + let error; + if (typeof response === "string") { + error = response; + } else if (Array.isArray(response.results)) { + error = response.results[0]?.error?.message; + } + if (error) { + setSaving(false); + setInlineError(error); + return; + } + listSecrets(); + handleRemovePanelQueryParams(); + return; + }) + .catch((error) => { + setSaving(false); + if (typeof error === "string" || error instanceof Error) { + setInlineError(error instanceof Error ? error.message : error); + } + }); + }, + [handleRemovePanelQueryParams, listSecrets, removeSecrets, secretURI], + ); + + return ( + <> + + + + {Label.SUBMIT} + + + } + onRemovePanelQueryParams={handleRemovePanelQueryParams} + ref={scrollArea} + title="Add a secret" + width="narrow" + > + <> + + {secret ? ( + + initialValues={{ + removeAll: false, + revision: secret["latest-revision"].toString(), + }} + onSubmit={(values) => { + setShowConfirm(true); + }} + > +
+ setShowConfirm(false)} + secretURI={secretURI} + showConfirm={showConfirm} + /> + + + ) : ( + + )} + +
+ + ); +}; + +export default RemoveSecretPanel; diff --git a/src/panels/RemoveSecretPanel/index.ts b/src/panels/RemoveSecretPanel/index.ts new file mode 100644 index 000000000..fb8924b09 --- /dev/null +++ b/src/panels/RemoveSecretPanel/index.ts @@ -0,0 +1 @@ +export { default } from "./RemoveSecretPanel"; diff --git a/src/panels/RemoveSecretPanel/types.ts b/src/panels/RemoveSecretPanel/types.ts new file mode 100644 index 000000000..36612a243 --- /dev/null +++ b/src/panels/RemoveSecretPanel/types.ts @@ -0,0 +1,4 @@ +export type FormFields = { + removeAll: boolean; + revision: string; +}; From b737aa0e171cac87ab6556a0775e6815c8f8be22 Mon Sep 17 00:00:00 2001 From: Huw Wilkins Date: Wed, 7 Feb 2024 15:28:36 +1100 Subject: [PATCH 2/3] Disable checkbox when one revision. --- .../RemoveSecretPanel/Fields/Fields.test.tsx | 31 +++++++++++++++++++ .../RemoveSecretPanel/Fields/Fields.tsx | 1 + .../RemoveSecretPanel/RemoveSecretPanel.tsx | 2 +- 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/panels/RemoveSecretPanel/Fields/Fields.test.tsx b/src/panels/RemoveSecretPanel/Fields/Fields.test.tsx index fe3052630..2adb10308 100644 --- a/src/panels/RemoveSecretPanel/Fields/Fields.test.tsx +++ b/src/panels/RemoveSecretPanel/Fields/Fields.test.tsx @@ -136,6 +136,37 @@ describe("Fields", () => { ).toBeDisabled(); }); + it("disables the remove all checkbox if there is only one revision", async () => { + state.juju.secrets.abc123 = modelSecretsFactory.build({ + items: [ + listSecretResultFactory.build({ + uri: "secret:aabbccdd", + "latest-revision": 1, + revisions: [secretRevisionFactory.build({ revision: 1 })], + }), + ], + loaded: true, + }); + const handleRemoveSecret = jest.fn(); + renderComponent( + + initialValues={{ removeAll: false, revision: "" }} + onSubmit={jest.fn()} + > + + , + { state, path, url }, + ); + expect( + screen.getByRole("checkbox", { name: Label.REMOVE_ALL }), + ).toBeDisabled(); + }); + it("can cancel the confirmation", async () => { const hideConfirm = jest.fn(); renderComponent( diff --git a/src/panels/RemoveSecretPanel/Fields/Fields.tsx b/src/panels/RemoveSecretPanel/Fields/Fields.tsx index b47ce8810..247133968 100644 --- a/src/panels/RemoveSecretPanel/Fields/Fields.tsx +++ b/src/panels/RemoveSecretPanel/Fields/Fields.tsx @@ -47,6 +47,7 @@ const Fields = ({ label={Label.REMOVE_ALL} name="removeAll" type="checkbox" + disabled={secret?.revisions.length === 1} /> { } onRemovePanelQueryParams={handleRemovePanelQueryParams} ref={scrollArea} - title="Add a secret" + title="Remove secret" width="narrow" > <> From 02e6af7a0cdf86f322b0679e2e79e2345ed27db7 Mon Sep 17 00:00:00 2001 From: Huw Wilkins Date: Fri, 9 Feb 2024 09:46:02 +1100 Subject: [PATCH 3/3] Display a message if there is only one revision. --- .../RemoveSecretPanel/Fields/Fields.test.tsx | 18 +++++-- .../RemoveSecretPanel/Fields/Fields.tsx | 47 +++++++++++-------- 2 files changed, 41 insertions(+), 24 deletions(-) diff --git a/src/panels/RemoveSecretPanel/Fields/Fields.test.tsx b/src/panels/RemoveSecretPanel/Fields/Fields.test.tsx index 2adb10308..bbf7ab1e3 100644 --- a/src/panels/RemoveSecretPanel/Fields/Fields.test.tsx +++ b/src/panels/RemoveSecretPanel/Fields/Fields.test.tsx @@ -136,13 +136,13 @@ describe("Fields", () => { ).toBeDisabled(); }); - it("disables the remove all checkbox if there is only one revision", async () => { + it("displays a message if there is only one revision", async () => { state.juju.secrets.abc123 = modelSecretsFactory.build({ items: [ listSecretResultFactory.build({ uri: "secret:aabbccdd", - "latest-revision": 1, - revisions: [secretRevisionFactory.build({ revision: 1 })], + "latest-revision": 5, + revisions: [secretRevisionFactory.build({ revision: 5 })], }), ], loaded: true, @@ -163,8 +163,16 @@ describe("Fields", () => { { state, path, url }, ); expect( - screen.getByRole("checkbox", { name: Label.REMOVE_ALL }), - ).toBeDisabled(); + screen.queryByRole("checkbox", { name: Label.REMOVE_ALL }), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole("combobox", { name: Label.REVISION }), + ).not.toBeInTheDocument(); + expect( + screen.getByText( + /This secret has one revision \(5\) and will be completely removed./, + ), + ).toBeInTheDocument(); }); it("can cancel the confirmation", async () => { diff --git a/src/panels/RemoveSecretPanel/Fields/Fields.tsx b/src/panels/RemoveSecretPanel/Fields/Fields.tsx index 247133968..b18e397c6 100644 --- a/src/panels/RemoveSecretPanel/Fields/Fields.tsx +++ b/src/panels/RemoveSecretPanel/Fields/Fields.tsx @@ -42,25 +42,34 @@ const Fields = ({ return ( <> - - ({ - label: `${revision}${revision === secret?.["latest-revision"] ? " (latest)" : ""}`, - value: revision.toString(), - })) ?? [] - } - /> + {(secret?.revisions.length ?? 0) > 1 ? ( + <> + + ({ + label: `${revision}${revision === secret?.["latest-revision"] ? " (latest)" : ""}`, + value: revision.toString(), + })) ?? [] + } + /> + + ) : ( +

+ This secret has one revision ({secret?.["latest-revision"]}) and will + be completely removed. +

+ )} {showConfirm ? (