refactor: only fetch relations if they change and clean up the previo…

…us one

src/juju/api-hooks/permissions.test.tsx

@@ -1,6 +1,7 @@
 import { renderHook, waitFor } from "@testing-library/react";
 import configureStore from "redux-mock-store";
 
+import type { RelationshipTuple } from "juju/jimm/JIMMV4";
 import { JIMMRelation, JIMMTarget } from "juju/jimm/JIMMV4";
 import { actions as jujuActions } from "store/juju";
 import type { RootState } from "store/store";
@@ -109,6 +110,39 @@ describe("useCheckPermissions", () => {
     });
   });
 
+  it("does not try and fetch a relation if the tuple object has a new reference", async () => {
+    const tuple = {
+      object: "user-eggman@external",
+      relation: JIMMRelation.MEMBER,
+      target_object: "group-admins",
+    };
+    const store = mockStore(state);
+    const { rerender } = renderHook(() => useCheckPermissions(tuple), {
+      wrapper: (props) => (
+        <ComponentProviders {...props} path="/" store={store} />
+      ),
+    });
+    await waitFor(() => {
+      expect(
+        store
+          .getActions()
+          .filter(
+            (dispatch) => dispatch.type === jujuActions.checkRelation.type,
+          ),
+      ).toHaveLength(1);
+    });
+    rerender({ ...tuple });
+    await waitFor(() => {
+      expect(
+        store
+          .getActions()
+          .filter(
+            (dispatch) => dispatch.type === jujuActions.checkRelation.type,
+          ),
+      ).toHaveLength(1);
+    });
+  });
+
   it("does not fetch a relation that is already loading", async () => {
     const tuple = {
       object: "user-eggman@external",
@@ -163,6 +197,61 @@ describe("useCheckPermissions", () => {
     });
   });
 
+  it("cleans up a previous relation if the tuple changes", async () => {
+    const tuple = {
+      object: "user-eggman@external",
+      relation: JIMMRelation.MEMBER,
+      target_object: "group-admins",
+    };
+    state.juju.rebacRelations = [
+      rebacRelationFactory.build({
+        tuple: tuple,
+        loaded: true,
+      }),
+    ];
+    const store = mockStore(state);
+    const { rerender } = renderHook<
+      {
+        permitted: boolean;
+        loading: boolean;
+        loaded: boolean;
+      },
+      {
+        tupleObject: RelationshipTuple;
+        cleanup: boolean;
+      }
+    >(
+      ({ tupleObject, cleanup } = { tupleObject: tuple, cleanup: true }) =>
+        useCheckPermissions(tupleObject, cleanup),
+      {
+        wrapper: (props) => (
+          <ComponentProviders {...props} path="/" store={store} />
+        ),
+      },
+    );
+    await waitFor(() => {
+      expect(
+        store
+          .getActions()
+          .find((dispatch) => dispatch.type === jujuActions.checkRelation.type),
+      ).toBeUndefined();
+    });
+    rerender({ tupleObject: { ...tuple, object: "newobject" }, cleanup: true });
+    const action = jujuActions.removeCheckRelation({
+      tuple,
+    });
+    await waitFor(() => {
+      expect(
+        store
+          .getActions()
+          .find(
+            (dispatch) =>
+              dispatch.type === jujuActions.removeCheckRelation.type,
+          ),
+      ).toMatchObject(action);
+    });
+  });
+
   it("can clean up a relation", async () => {
     const tuple = {
       object: "user-eggman@external",

src/juju/api-hooks/permissions.ts

@@ -1,4 +1,6 @@
-import { useEffect } from "react";
+import { usePrevious } from "@canonical/react-components";
+import fastDeepEqual from "fast-deep-equal/es6";
+import { useEffect, useRef } from "react";
 import { useDispatch } from "react-redux";
 
 import type { RelationshipTuple } from "juju/jimm/JIMMV4";
@@ -31,6 +33,8 @@ export const useCheckPermissions = (
     getReBACPermissionLoading(state, tuple),
   );
   const rebacEnabled = useAppSelector(isReBACEnabled);
+  const previousTuple = usePrevious(tuple, false);
+  const tupleChanged = !fastDeepEqual(tuple, previousTuple);
 
   useEffect(() => {
     if (
@@ -39,6 +43,9 @@ export const useCheckPermissions = (
       !loaded &&
       wsControllerURL &&
       tuple &&
+      // Ignore changes if the object has a new reference, but the values are
+      // the same.
+      tupleChanged &&
       // Only check the relation if the controller supports rebac.
       rebacEnabled
     ) {
@@ -49,19 +56,44 @@ export const useCheckPermissions = (
         }),
       );
     }
-  }, [dispatch, loaded, loading, rebacEnabled, tuple, wsControllerURL]);
+  }, [
+    dispatch,
+    loaded,
+    loading,
+    rebacEnabled,
+    tuple,
+    wsControllerURL,
+    tupleChanged,
+  ]);
 
-  useEffect(
-    () => () => {
-      if (tuple && cleanup) {
+  useEffect(() => {
+    if (cleanup && tupleChanged && previousTuple) {
+      dispatch(
+        jujuActions.removeCheckRelation({
+          tuple: previousTuple,
+        }),
+      );
+    }
+  }, [cleanup, dispatch, previousTuple, tupleChanged]);
+
+  const cleanupTuple = useRef<(() => void) | null>(null);
+
+  useEffect(() => {
+    if (cleanup && tupleChanged && tuple) {
+      cleanupTuple.current = () =>
         dispatch(
           jujuActions.removeCheckRelation({
             tuple,
           }),
         );
-      }
+    }
+  }, [cleanup, dispatch, tuple, tupleChanged]);
+
+  useEffect(
+    () => () => {
+      cleanupTuple.current?.();
     },
-    [cleanup, dispatch, tuple],
+    [],
   );
 
   return {

src/pages/EntityDetails/EntityDetails.tsx

@@ -11,6 +11,7 @@ import NotFound from "components/NotFound";
 import type { EntityDetailsRoute } from "components/Routes";
 import WebCLI from "components/WebCLI";
 import { useEntityDetailsParams } from "components/hooks";
+import useCanConfigureModel from "hooks/useCanConfigureModel";
 import { useQueryParams } from "hooks/useQueryParams";
 import useWindowTitle from "hooks/useWindowTitle";
 import BaseLayout from "layout/BaseLayout/BaseLayout";
@@ -55,6 +56,12 @@ const EntityDetails = ({ modelWatcherError }: Props) => {
   const isK8s = useAppSelector((state) => isKubernetesModel(state, modelUUID));
   const { isNestedEntityPage } = useEntityDetailsParams();
 
+  // Cleanup is set for this hook, but not for the instances of
+  // useCanConfigureModel in other model components as this component wraps all
+  // model routes so the model permissions are removed once the user navigates
+  // away from the model.
+  useCanConfigureModel(true);
+
   const isJuju = useSelector(getIsJuju);
 
   const [query] = useQueryParams({

src/pages/EntityDetails/Model/Model.tsx

@@ -87,11 +87,7 @@ const Model = () => {
   const canListSecrets = useAppSelector((state) =>
     getCanListSecrets(state, modelUUID),
   );
-  // Cleanup is set for this hook, but not for the instances of
-  // useCanConfigureModel in other model components as this component wraps all
-  // model routes so the model permissions are removed once the user navigates
-  // away from the model.
-  const canConfigureModel = useCanConfigureModel(true);
+  const canConfigureModel = useCanConfigureModel();
 
   const machinesTableRows = useMemo(() => {
     return modelName && userName