Merge pull request #1843 from Ninfa-Jeon/latest-revision-bug-fix

fix: Compute latest revision from revisions array
canonical · Feb 5, 2025 · 2410cdc · 2410cdc
2 parents 3b25638 + 3704cb0
commit 2410cdc
Show file tree

Hide file tree

Showing 15 changed files with 222 additions and 49 deletions.
diff --git a/src/components/secrets/RevisionField/RevisionField.test.tsx b/src/components/secrets/RevisionField/RevisionField.test.tsx
@@ -89,4 +89,17 @@ describe("RevisionField", () => {
     ).toBeInTheDocument();
     expect(screen.getByRole("option", { name: "1" })).toBeInTheDocument();
   });
+
+  it("does not list revisions if they are empty", async () => {
+    renderComponent(
+      <Formik<{ revision: string }>
+        initialValues={{ revision: "" }}
+        onSubmit={vi.fn()}
+      >
+        <RevisionField secretURI="secret:anothersecret" modelUUID="abc123" />
+      </Formik>,
+      { state, path, url },
+    );
+    expect(screen.queryAllByRole("option")).toStrictEqual([]);
+  });
 });
diff --git a/src/components/secrets/RevisionField/RevisionField.tsx b/src/components/secrets/RevisionField/RevisionField.tsx
@@ -5,7 +5,7 @@ import type {
 } from "@canonical/react-components";
 import { FormikField, Select } from "@canonical/react-components";
 
-import { getSecretByURI } from "store/juju/selectors";
+import { getSecretByURI, getSecretLatestRevision } from "store/juju/selectors";
 import { useAppSelector } from "store/store";
 
 import { Label } from "./types";
@@ -22,14 +22,17 @@ const RevisionField = ({ modelUUID, secretURI, ...props }: Props) => {
   const secret = useAppSelector((state) =>
     getSecretByURI(state, modelUUID, secretURI),
   );
+  const latestRevision = useAppSelector((state) =>
+    getSecretLatestRevision(state, modelUUID, secretURI),
+  );
   return (
     <FormikField
       label={Label.REVISION}
       name="revision"
       component={Select}
       options={
         [...(secret?.revisions ?? [])].reverse().map(({ revision }) => ({
-          label: `${revision}${revision === secret?.["latest-revision"] ? " (latest)" : ""}`,
+          label: `${revision}${revision === latestRevision ? " (latest)" : ""}`,
           value: revision.toString(),
         })) ?? []
       }

diff --git a/src/components/secrets/SecretForm/SecretForm.tsx b/src/components/secrets/SecretForm/SecretForm.tsx
@@ -28,6 +28,7 @@ import {
   getSecretsContentLoaded,
   getSecretsContentLoading,
   getModelByUUID,
+  getSecretLatestRevision,
 } from "store/juju/selectors";
 import { useAppSelector, useAppDispatch } from "store/store";
 import { toErrorString } from "utils";
@@ -75,6 +76,9 @@ const SecretForm = ({
   const secret = useAppSelector((state) =>
     getSecretByURI(state, modelUUID, secretURI),
   );
+  const latestRevision = useAppSelector((state) =>
+    getSecretLatestRevision(state, modelUUID, secretURI),
+  );
   const wsControllerURL = useAppSelector((state) =>
     getModelByUUID(state, modelUUID),
   )?.wsControllerURL;
@@ -105,10 +109,10 @@ const SecretForm = ({
   const defaultPairs = [{ key: "", value: "", isBase64: false }];
 
   useEffect(() => {
-    if (secretURI) {
-      getSecretContent(secretURI);
+    if (secretURI && latestRevision) {
+      getSecretContent(secretURI, latestRevision);
     }
-  }, [getSecretContent, secretURI]);
+  }, [getSecretContent, secretURI, latestRevision]);
 
   useEffect(
     () => () => {

diff --git a/src/pages/EntityDetails/Model/Secrets/SecretContent/SecretContent.test.tsx b/src/pages/EntityDetails/Model/Secrets/SecretContent/SecretContent.test.tsx
@@ -186,6 +186,20 @@ describe("SecretContent", () => {
     expect(screen.getByText("a value")).toBeInTheDocument();
   });
 
+  it("can display content when no secrets", async () => {
+    state.juju.secrets = secretsStateFactory.build();
+    renderComponent(<SecretContent secretURI="secret:aabbccdd" />, {
+      state,
+      path,
+      url,
+    });
+    await userEvent.click(screen.getByRole("button", { name: Label.SHOW }));
+    expect(screen.queryByRole("heading", { name: "a key" })).toBeNull();
+    expect(
+      document.querySelector(".p-code-snippet__block"),
+    ).not.toBeInTheDocument();
+  });
+
   it("can display content errors", async () => {
     state.juju.secrets.abc123.content = modelSecretsContentFactory.build({
       loaded: true,

diff --git a/src/pages/EntityDetails/Model/Secrets/SecretContent/SecretContent.tsx b/src/pages/EntityDetails/Model/Secrets/SecretContent/SecretContent.tsx
@@ -24,6 +24,7 @@ import {
   getSecretsContentLoading,
   getModelUUIDFromList,
   getModelByUUID,
+  getSecretLatestRevision,
 } from "store/juju/selectors";
 import { useAppSelector, useAppDispatch } from "store/store";
 
@@ -43,6 +44,9 @@ const SecretContent = ({ secretURI }: Props) => {
   const secret = useAppSelector((state) =>
     getSecretByURI(state, modelUUID, secretURI),
   );
+  const latestRevision = useAppSelector((state) =>
+    getSecretLatestRevision(state, modelUUID, secretURI),
+  );
   const content = useAppSelector((state) =>
     getSecretsContent(state, modelUUID),
   );
@@ -90,9 +94,7 @@ const SecretContent = ({ secretURI }: Props) => {
           >
             {secret ? (
               <Formik<{ revision: string }>
-                initialValues={{
-                  revision: secret["latest-revision"].toString(),
-                }}
+                initialValues={{ revision: (latestRevision ?? "").toString() }}
                 onSubmit={(values) => {
                   getSecretContent(secretURI, Number(values.revision));
                 }}

diff --git a/src/pages/EntityDetails/Model/Secrets/SecretsTable/SecretsTable.tsx b/src/pages/EntityDetails/Model/Secrets/SecretsTable/SecretsTable.tsx
@@ -24,7 +24,7 @@ import {
   getModelUUIDFromList,
 } from "store/juju/selectors";
 import { useAppSelector } from "store/store";
-import { secretIsAppOwned } from "utils";
+import { getLatestRevision, secretIsAppOwned } from "utils";
 
 import SecretContent from "../SecretContent";
 
@@ -55,6 +55,7 @@ const SecretsTable = () => {
       return [];
     }
     return secrets.map((secret) => {
+      const latestRevision = getLatestRevision(secret);
       const id = secret.uri.replace(/^secret:/, "");
       let owner: ReactNode = secret["owner-tag"];
       if (secret["owner-tag"]?.startsWith("model-")) {
@@ -139,7 +140,7 @@ const SecretsTable = () => {
             </div>
           </div>
         ),
-        revision: secret["latest-revision"],
+        revision: latestRevision,
         description: secret.description,
         granted: <span data-testid={TestId.GRANTED_TO}>{granted}</span>,
         owner,

diff --git a/src/panels/RemoveSecretPanel/Fields/Fields.test.tsx b/src/panels/RemoveSecretPanel/Fields/Fields.test.tsx
@@ -177,6 +177,30 @@ describe("Fields", () => {
     ).toBeInTheDocument();
   });
 
+  it("displays a message if secret does not exist", async () => {
+    state.juju.secrets.abc123 = modelSecretsFactory.build();
+    const handleRemoveSecret = vi.fn();
+    renderComponent(
+      <Formik<FormFields>
+        initialValues={{ removeAll: false, revision: "" }}
+        onSubmit={vi.fn()}
+      >
+        <Fields
+          secretURI="secret:aabbccdd"
+          hideConfirm={vi.fn()}
+          handleRemoveSecret={handleRemoveSecret}
+          showConfirm={true}
+        />
+      </Formik>,
+      { state, path, url },
+    );
+    expect(
+      screen.getByText(
+        /This secret has one revision and will be completely removed./,
+      ),
+    ).toBeInTheDocument();
+  });
+
   it("can cancel the confirmation", async () => {
     const hideConfirm = vi.fn();
     renderComponent(

diff --git a/src/panels/RemoveSecretPanel/Fields/Fields.tsx b/src/panels/RemoveSecretPanel/Fields/Fields.tsx
@@ -6,7 +6,11 @@ import usePortal from "react-useportal";
 
 import type { EntityDetailsRoute } from "components/Routes";
 import RevisionField from "components/secrets/RevisionField";
-import { getSecretByURI, getModelUUIDFromList } from "store/juju/selectors";
+import {
+  getSecretByURI,
+  getModelUUIDFromList,
+  getSecretLatestRevision,
+} from "store/juju/selectors";
 import { useAppSelector } from "store/store";
 
 import type { FormFields } from "../types";
@@ -32,6 +36,9 @@ const Fields = ({
   const secret = useAppSelector((state) =>
     getSecretByURI(state, modelUUID, secretURI),
   );
+  const latestRevision = useAppSelector((state) =>
+    getSecretLatestRevision(state, modelUUID, secretURI),
+  );
   const { Portal } = usePortal();
 
   return (
@@ -53,8 +60,9 @@ const Fields = ({
         </>
       ) : (
         <p>
-          This secret has one revision ({secret?.["latest-revision"]}) and will
-          be completely removed.
+          This secret has one revision{" "}
+          {latestRevision ? ` (${latestRevision})` : ""} and will be completely
+          removed.
         </p>
       )}
       {showConfirm ? (

diff --git a/src/panels/RemoveSecretPanel/RemoveSecretPanel.tsx b/src/panels/RemoveSecretPanel/RemoveSecretPanel.tsx
@@ -11,7 +11,11 @@ import SecretLabel from "components/secrets/SecretLabel";
 import { useRemoveSecrets, useListSecrets } from "juju/api-hooks";
 import PanelInlineErrors from "panels/PanelInlineErrors";
 import { usePanelQueryParams } from "panels/hooks";
-import { getSecretByURI, getModelUUIDFromList } from "store/juju/selectors";
+import {
+  getSecretByURI,
+  getModelUUIDFromList,
+  getSecretLatestRevision,
+} from "store/juju/selectors";
 import { useAppSelector } from "store/store";
 import { toErrorString } from "utils";
 
@@ -34,6 +38,9 @@ const RemoveSecretPanel = () => {
   const secret = useAppSelector((state) =>
     getSecretByURI(state, modelUUID, secretURI),
   );
+  const latestRevision = useAppSelector((state) =>
+    getSecretLatestRevision(state, modelUUID, secretURI),
+  );
   const [inlineError, setInlineError] = useState<string | null>(null);
   const [saving, setSaving] = useState(false);
   const [showConfirm, setShowConfirm] = useState(false);
@@ -109,7 +116,7 @@ const RemoveSecretPanel = () => {
             <Formik<FormFields>
               initialValues={{
                 removeAll: false,
-                revision: secret["latest-revision"].toString(),
+                revision: (latestRevision ?? "").toString(),
               }}
               onSubmit={() => {
                 setShowConfirm(true);

diff --git a/src/panels/SecretFormPanel/SecretFormPanel.tsx b/src/panels/SecretFormPanel/SecretFormPanel.tsx
@@ -24,39 +24,35 @@ const SecretFormPanel = ({ update }: Props) => {
   const [saving, setSaving] = useState<boolean>(false);
 
   return (
-    <>
-      <Panel
-        data-testid={TestId.PANEL}
-        drawer={
-          <>
-            <Button onClick={handleRemovePanelQueryParams}>
-              {Label.CANCEL}
-            </Button>
-            <ActionButton
-              appearance="positive"
-              disabled={saving}
-              form={formId}
-              loading={saving}
-              type="submit"
-            >
-              {update ? Label.SUBMIT_UPDATE : Label.SUBMIT_ADD}
-            </ActionButton>
-          </>
-        }
-        onRemovePanelQueryParams={handleRemovePanelQueryParams}
-        ref={scrollArea}
-        title={update ? Label.TITLE_UPDATE : Label.TITLE_ADD}
-        width="narrow"
-      >
-        <SecretForm
-          formId={formId}
-          onSuccess={() => handleRemovePanelQueryParams()}
-          secretURI={queryParams.secret}
-          setSaving={setSaving}
-          update={update}
-        />
-      </Panel>
-    </>
+    <Panel
+      data-testid={TestId.PANEL}
+      drawer={
+        <>
+          <Button onClick={handleRemovePanelQueryParams}>{Label.CANCEL}</Button>
+          <ActionButton
+            appearance="positive"
+            disabled={saving}
+            form={formId}
+            loading={saving}
+            type="submit"
+          >
+            {update ? Label.SUBMIT_UPDATE : Label.SUBMIT_ADD}
+          </ActionButton>
+        </>
+      }
+      onRemovePanelQueryParams={handleRemovePanelQueryParams}
+      ref={scrollArea}
+      title={update ? Label.TITLE_UPDATE : Label.TITLE_ADD}
+      width="narrow"
+    >
+      <SecretForm
+        formId={formId}
+        onSuccess={() => handleRemovePanelQueryParams()}
+        secretURI={queryParams.secret}
+        setSaving={setSaving}
+        update={update}
+      />
+    </Panel>
   );
 };
 

diff --git a/src/store/juju/selectors.test.ts b/src/store/juju/selectors.test.ts
@@ -33,6 +33,7 @@ import {
   modelFeaturesStateFactory,
   rebacRelationFactory,
   relationshipTupleFactory,
+  secretRevisionFactory,
 } from "testing/factories/juju/juju";
 import {
   applicationInfoFactory,
@@ -123,6 +124,7 @@ import {
   getReBACRelationsState,
   hasReBACPermission,
   isJIMMAdmin,
+  getSecretLatestRevision,
 } from "./selectors";
 
 describe("selectors", () => {
@@ -371,6 +373,47 @@ describe("selectors", () => {
     ).toStrictEqual(items[0]);
   });
 
+  it("getSecretLatestRevision", () => {
+    const items = [
+      listSecretResultFactory.build({ uri: "secret:eeffgghh" }),
+      listSecretResultFactory.build({
+        uri: "secret:aabbccdd",
+        label: "other-model",
+        "latest-revision": 3,
+        revisions: [
+          secretRevisionFactory.build({ revision: 1 }),
+          secretRevisionFactory.build({ revision: 2 }),
+        ],
+      }),
+    ];
+    expect(
+      getSecretLatestRevision(
+        rootStateFactory.build({
+          juju: jujuStateFactory.build({
+            secrets: secretsStateFactory.build({
+              abc123: modelSecretsFactory.build({ items }),
+              def456: modelSecretsFactory.build({
+                items: [
+                  listSecretResultFactory.build({
+                    uri: "secret:aabbccdd",
+                    label: "other-model",
+                    "latest-revision": 3,
+                    revisions: [
+                      secretRevisionFactory.build({ revision: 1 }),
+                      secretRevisionFactory.build({ revision: 2 }),
+                    ],
+                  }),
+                ],
+              }),
+            }),
+          }),
+        }),
+        "abc123",
+        "secret:aabbccdd",
+      ),
+    ).toStrictEqual(2);
+  });
+
   it("getSecretsErrors", () => {
     const errors = "Uh oh!";
     expect(