UISAUTHCOM-43: Fix capabilities from unselected application sometimes shown as assigned for a role after editing.

CHANGELOG.md

@@ -12,6 +12,7 @@
 * [UISAUTHCOM-40](https://folio-org.atlassian.net/browse/UISAUTHCOM-40) Add ability to select entire columns of capabilities in each capability/capabilitySet grid/accordion.
 * [UISAUTHCOM-41](https://folio-org.atlassian.net/browse/UISAUTHCOM-41) Fix disabled the issue with capabilities included in capability set are not deselected when deselecting a set.
 * [UISAUTHCOM-42](https://folio-org.atlassian.net/browse/UISAUTHCOM-42) Disable hyperlinks for user names in role details page if user doesn't have access to Users module.
+* [UISAUTHCOM-43](https://folio-org.atlassian.net/browse/UISAUTHCOM-43) Fix capabilities from unselected application sometimes shown as assigned for a role after editing.
 
 ## [1.0.0](https://github.com/folio-org/stripes-authorization-components/tree/v1.0.0) (2024-11-01)
 

lib/Role/RoleCreate/RoleCreate.js

@@ -1,7 +1,7 @@
 import PropTypes from 'prop-types';
 import { useState } from 'react';
 import { useHistory } from 'react-router';
-import { isEmpty } from 'lodash';
+import isEmpty from 'lodash/isEmpty';
 
 import { useQueryClient } from 'react-query';
 import { MUTATION_ACTION_TYPE } from '../../constants';
@@ -35,7 +35,9 @@ export const RoleCreate = ({
     isLoading: isAppCapabilitiesLoading,
     queryKeys: applicationCapabilitiesQueryKeys,
     actionCapabilities
-  } = useApplicationCapabilities(checkedAppIdsMap, { tenantId });
+  } = useApplicationCapabilities({ checkedAppIdsMap,
+    options:{ tenantId },
+    setDisabledCapabilities });
 
   const {
     capabilitySets,

lib/Role/RoleEdit/RoleEdit.js

@@ -1,4 +1,5 @@
-import { isEmpty, isEqual } from 'lodash';
+import isEmpty from 'lodash/isEmpty';
+import isEqual from 'lodash/isEqual';
 import PropTypes from 'prop-types';
 import { useEffect, useState } from 'react';
 import { useQueryClient } from 'react-query';
@@ -60,7 +61,9 @@ export const RoleEdit = ({ path, tenantId }) => {
     isLoading: isAppCapabilitiesLoading,
     queryKeys: applicationCapabilitiesQueryKeys,
     actionCapabilities
-  } = useApplicationCapabilities(checkedAppIdsMap, { tenantId });
+  } = useApplicationCapabilities({ checkedAppIdsMap,
+    options: { tenantId },
+    setDisabledCapabilities });
 
   const {
     capabilitySets,
@@ -72,7 +75,8 @@ export const RoleEdit = ({ path, tenantId }) => {
     isLoading: isAppCapabilitySetsLoading,
     queryKeys: applicationCapabilitySetsQueryKeys,
     actionCapabilitySets
-  } = useApplicationCapabilitySets(checkedAppIdsMap, { tenantId });
+  } = useApplicationCapabilitySets({ checkedAppIdsMap,
+    options: { tenantId } });
 
   const unselectAllCapabilitiesAndSets = () => {
     setSelectedCapabilitiesMap({});
@@ -180,7 +184,7 @@ export const RoleEdit = ({ path, tenantId }) => {
       onClose();
       showCallout({ messageId: 'stripes-authorization-components.role.edit.success' });
     } catch (error) {
-      handleError(MUTATION_ACTION_TYPE.update, error);
+      await handleError(MUTATION_ACTION_TYPE.update, error);
     }
   };
 

lib/hooks/useApplicationCapabilities/useApplicationCapabilities.js

@@ -5,18 +5,36 @@ import {
   useState,
 } from 'react';
 
+import isEmpty from 'lodash/isEmpty';
 import {
   extractSelectedIdsFromObject,
   getCapabilitiesGroupedByTypeAndResource,
-  getOnlyIntersectedWithApplicationsCapabilities,
 } from '../../utils';
 import { useChunkedApplicationCapabilities } from '../useChunkedApplicationCapabilities';
 
+function removeUncheckedCaps(checkedAppCaps) {
+  return (prevCaps) => {
+    const copy = { ...prevCaps };
+    let hasChanged = false;
+
+    for (const cap in copy) {
+      if (!(cap in checkedAppCaps)) {
+        delete copy[cap];
+        hasChanged = true;
+      }
+    }
+
+    return hasChanged ? copy : prevCaps;
+  };
+}
+
 /**
  * Custom hook that retrieves application capabilities based on the selected application IDs.
  *
  * @param {Object} checkedAppIdsMap - An object containing the selected application IDs. Using
  *   this object, the hook will retrieve the capabilities for the selected applications.
+ * @param {Object} options - query options.
+ * @param {Function} setDisabledCapabilities - disabled capabilities setter function.
  * @return {Object} An object containing the following properties:
  *   - capabilities: An object containing the capabilities grouped by type and resource.
  *   - selectedCapabilitiesMap: An object containing the selected capabilities mapped by their IDs.
@@ -27,7 +45,11 @@ import { useChunkedApplicationCapabilities } from '../useChunkedApplicationCapab
  *   - action capabilities: An object with capabilities based on capability type and action
  */
 
-export const useApplicationCapabilities = (checkedAppIdsMap, options = {}) => {
+export const useApplicationCapabilities = ({
+  checkedAppIdsMap,
+  options = {},
+  setDisabledCapabilities,
+}) => {
   const { tenantId } = options;
 
   const selectedAppIds = extractSelectedIdsFromObject(checkedAppIdsMap);
@@ -48,15 +70,6 @@ export const useApplicationCapabilities = (checkedAppIdsMap, options = {}) => {
       .filter(Boolean);
   }, [capabilities, roleCapabilitiesListIds]);
 
-  useEffect(() => {
-    if (!isCapabilitiesLoading) {
-      const updatedSelectedCapabilitiesMap = getOnlyIntersectedWithApplicationsCapabilities(capabilities, roleCapabilitiesListIds);
-      setSelectedCapabilitiesMap(updatedSelectedCapabilitiesMap);
-    }
-    // isCapabilitiesLoading only information we need to know if capabilities fetched
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [isCapabilitiesLoading]);
-
   const memoizedCapabilities = useMemo(() => getCapabilitiesGroupedByTypeAndResource(capabilities), [capabilities]);
 
   const actionCapabilities = useMemo(() => {
@@ -70,6 +83,41 @@ export const useApplicationCapabilities = (checkedAppIdsMap, options = {}) => {
     }, initialStructure);
   }, [capabilities]);
 
+  const checkedAppCaps = useMemo(
+    () => capabilities.reduce((acc, capSet) => {
+      acc[capSet.id] = true;
+      return acc;
+    }, {}),
+    [capabilities],
+  );
+
+  /*
+    What happens in useEffect?
+    Our useApplicationCapabilities hook receives a list of the selected applications as argument,
+    and we retrieve the capabilities ONLY for the selected applications.
+    The user will then be able to select from the provided capabilities.
+    useEffect handles the situation where the user changes the selected applications, causing new capabilities to be retrieved.
+    However, previously selected capabilities may no longer be present in the new list of selected application capabilities.
+    It filters them by checking whether the selected capability set is included in the new list of capabilities.
+    It iterates over selectedCapabilitiesMap, disabledCapabilities, and if any of them are not in the new list of capabilitySets,
+    they are removed.
+    Example,
+      const selectedCapabilitiesMap = { id1: true, id2: true },
+            disabledCapabilities = { id3: true }
+            checkedAppCaps = { id1: true };
+     1. Since checkedAppCaps does not include id2, it is removed from selectedCapabilitiesMap.
+     2. Since checkedAppCaps does not include id3, it is removed from disabledCapabilities
+   */
+
+  useEffect(() => {
+    if (!isCapabilitiesLoading && !isEmpty(checkedAppCaps)) {
+      setSelectedCapabilitiesMap(removeUncheckedCaps(checkedAppCaps));
+      setDisabledCapabilities(removeUncheckedCaps(checkedAppCaps));
+    }
+    // setDisabledCapabilities comes from outside, so we don't need to add it to the dependencies list
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [checkedAppCaps, isCapabilitiesLoading]);
+
   return {
     capabilities: memoizedCapabilities,
     selectedCapabilitiesMap,
@@ -78,6 +126,6 @@ export const useApplicationCapabilities = (checkedAppIdsMap, options = {}) => {
     setSelectedCapabilitiesMap,
     isLoading: isCapabilitiesLoading,
     queryKeys,
-    actionCapabilities
+    actionCapabilities,
   };
 };

lib/hooks/useApplicationCapabilities/useApplicationCapabilities.test.js

@@ -27,7 +27,10 @@ describe('useApplicationCapabilities', () => {
 
   it('should test if returning fields and methods are defined', () => {
     useChunkedApplicationCapabilities.mockReset().mockReturnValue({ items: [], isLoading: false, queryKeys: [['key1', 'key2']] });
-    const { result } = renderHook(useApplicationCapabilities, { initialProps: { cap1: true } });
+    const { result } = renderHook(useApplicationCapabilities, { initialProps: {
+      checkedAppIdsMap: { cap1: true },
+      setDisabledCapabilities: jest.fn()
+    } });
 
     expect(result.current.capabilities).toStrictEqual({ data: [], settings: [], procedural: [] });
     expect(result.current.roleCapabilitiesListIds).toStrictEqual([]);
@@ -44,7 +47,10 @@ describe('useApplicationCapabilities', () => {
     ];
     useChunkedApplicationCapabilities.mockClear().mockReturnValue({ items, isLoading: false });
 
-    const { result } = renderHook(useApplicationCapabilities, { initialProps: { cap1: true } });
+    const { result } = renderHook(useApplicationCapabilities, { initialProps: {
+      checkedAppIdsMap:{ cap1: true },
+      setDisabledCapabilities: jest.fn()
+    } });
 
     expect(result.current.capabilities).toStrictEqual({
       data:  [
@@ -72,11 +78,42 @@ describe('useApplicationCapabilities', () => {
 
   it('should set empty capabilities in the case of empty appIds', async () => {
     useChunkedApplicationCapabilities.mockClear().mockReturnValue({ items: [], isLoading: false, queryKeys: [] });
-    const { result } = renderHook(useApplicationCapabilities, { initialProps: {} });
+    const { result } = renderHook(useApplicationCapabilities, { initialProps: { checkedAppIdsMap: {}, setDisabledCapabilities: jest.fn() } });
 
     await waitFor(async () => {
       expect(result.current.capabilities).toStrictEqual({ data: [], settings: [], procedural: [] });
       expect(result.current.selectedCapabilitiesMap).toStrictEqual({});
     });
   });
+
+  it('should remove unchecked capabilities from selectedCapabilitiesMap', async () => {
+    useChunkedApplicationCapabilities.mockClear().mockReturnValue({
+      items: [
+        { id: 1, applicationId: 'app1', type: 'data', action:'edit', resource: 'r1' },
+      ],
+      isLoading: false
+    });
+
+    const { result, rerender } = renderHook(useApplicationCapabilities, { initialProps: {
+      checkedAppIdsMap: { app1: true },
+      setDisabledCapabilities: jest.fn()
+    } });
+
+    await waitFor(() => {
+      result.current.setSelectedCapabilitiesMap({ 333: true, 222: true });
+    });
+
+    rerender({ checkedAppIdsMap:{ app2: true }, setDisabledCapabilities: jest.fn() });
+    expect(result.current.selectedCapabilitiesMap).toEqual({ 222:true, 333: true });
+
+    useChunkedApplicationCapabilities.mockClear().mockReturnValue({
+      items: [
+        { id: 222, applicationId: 'app1', type: 'data', action:'edit', resource: 'r1' },
+      ],
+      isLoading: false
+    });
+
+    rerender({ checkedAppIdsMap:{ app2:true }, setDisabledCapabilities: jest.fn() });
+    expect(result.current.selectedCapabilitiesMap).toEqual({ 222:true });
+  });
 });

lib/hooks/useApplicationCapabilitySets/useApplicationCapabilitySets.js

@@ -1,15 +1,14 @@
 import keyBy from 'lodash/keyBy';
 import { useEffect, useMemo, useState } from 'react';
 
+import isEmpty from 'lodash/isEmpty';
 import {
-  extractSelectedIdsFromObject,
   getCapabilitiesGroupedByTypeAndResource,
-  getOnlyIntersectedWithApplicationsCapabilities
 } from '../../utils';
 import { useChunkedApplicationCapabilitySets } from '../useChunkedApplicationCapabilitySets';
 
 /**
- * Customhook that fetches and manages capability sets for a given list of checked application IDs.
+ * Custom hook that fetches and manages capability sets for a given list of checked application IDs.
  *
  * @param {Object} checkedAppIdsMap - An object mapping application IDs to boolean values indicating whether they are checked.
  *   Using this object, the hook will fetch the capability sets for the selected applications.
@@ -23,18 +22,18 @@ import { useChunkedApplicationCapabilitySets } from '../useChunkedApplicationCap
  *   - isLoading: A boolean indicating whether capability sets are currently being fetched.
  */
 
-export const useApplicationCapabilitySets = (checkedAppIdsMap, options = {}) => {
+export const useApplicationCapabilitySets = ({ checkedAppIdsMap, options = {} }) => {
   const { tenantId } = options;
 
-  const selectedAppIds = extractSelectedIdsFromObject(checkedAppIdsMap);
+  const selectedAppIds = Object.keys(checkedAppIdsMap);
   const {
     items: capabilitySets,
     isLoading: isCapabilitySetsLoading,
     queryKeys = [],
   } = useChunkedApplicationCapabilitySets(selectedAppIds, { tenantId });
 
   const [selectedCapabilitySetsMap, setSelectedCapabilitySetsMap] = useState({});
-  const roleCapabilitySetsListIds = extractSelectedIdsFromObject(selectedCapabilitySetsMap);
+  const roleCapabilitySetsListIds = Object.keys(selectedCapabilitySetsMap);
 
   const roleCapabilitySetsListNames = useMemo(() => {
     const capabilitySetsMap = keyBy(capabilitySets, 'id');
@@ -44,15 +43,6 @@ export const useApplicationCapabilitySets = (checkedAppIdsMap, options = {}) =>
       .filter(Boolean);
   }, [capabilitySets, roleCapabilitySetsListIds]);
 
-  useEffect(() => {
-    if (!isCapabilitySetsLoading) {
-      const updatedSelectedCapabilitySetsMap = getOnlyIntersectedWithApplicationsCapabilities(capabilitySets, roleCapabilitySetsListIds);
-      setSelectedCapabilitySetsMap(updatedSelectedCapabilitySetsMap);
-    }
-    // isCapabilitySetsLoading only information we need to know if capability sets fetched
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [isCapabilitySetsLoading]);
-
   const memoizedCapabilitySets = useMemo(() => getCapabilitiesGroupedByTypeAndResource(capabilitySets), [capabilitySets]);
 
   const actionCapabilitySets = useMemo(() => {
@@ -66,15 +56,56 @@ export const useApplicationCapabilitySets = (checkedAppIdsMap, options = {}) =>
     }, initialStructure);
   }, [capabilitySets]);
 
+  const checkedAppCapSets = useMemo(
+    () => capabilitySets.reduce((acc, capSet) => {
+      acc[capSet.id] = true;
+      return acc;
+    }, {}),
+    [capabilitySets],
+  );
+
+  /*
+    What happens in useEffect?
+    Our useApplicationCapabilitySets hook receives a list of the selected applications as argument,
+    and we retrieve the capability sets ONLY for the selected applications.
+    The user will then be able to select from the provided capabilitySets.
+    useEffect handles the situation where the user changes the selected applications, causing new capabilitySets to be retrieved.
+    However, previously selected capability sets may no longer be present in the new list of selected application capability sets.
+    It filters them by checking whether the selected capability set is included in the new list of capabilitySets.
+    It iterates over selectedCapabilitySets, and if any of them are not in the new list of capabilitySets, they are removed.
+    Example,
+      const selectedCapabilitiesMap = { id1: true, id2: true },
+            checkedAppCapSets = { id1: true };
+     Since checkedAppCapSets does not include id2, it is removed from selectedCapabilitySetsMap.
+   */
+
+  useEffect(() => {
+    if (!isCapabilitySetsLoading && !isEmpty(checkedAppCapSets)) {
+      setSelectedCapabilitySetsMap((prevCapSets) => {
+        const copy = { ...prevCapSets };
+        let hasChanged = false;
+
+        for (const capSet in copy) {
+          if (!(capSet in checkedAppCapSets)) {
+            delete copy[capSet];
+            hasChanged = true;
+          }
+        }
+
+        return hasChanged ? copy : prevCapSets;
+      });
+    }
+  }, [checkedAppCapSets, isCapabilitySetsLoading]);
+
   return {
-    capabilitySets:memoizedCapabilitySets,
+    capabilitySets: memoizedCapabilitySets,
     capabilitySetsList: capabilitySets,
     selectedCapabilitySetsMap,
     setSelectedCapabilitySetsMap,
     roleCapabilitySetsListIds,
     roleCapabilitySetsListNames,
     isLoading: isCapabilitySetsLoading,
     queryKeys,
-    actionCapabilitySets
+    actionCapabilitySets,
   };
 };