From 9b8a7e0dfdc40d21223a446b8b490e628b54e754 Mon Sep 17 00:00:00 2001
From: Jose Buitron <jose.buitron@gmail.com>
Date: Tue, 29 Aug 2023 15:10:10 -0500
Subject: [PATCH 1/4] fix: Removed feature to allow async permissions

---
 src/group/components/GroupSharedDataUpload.js |  4 +-
 .../membership/components/GroupMembers.js     |  9 ++---
 .../components/LandscapeProfile/index.js      |  7 +---
 .../components/LandscapeSharedDataUpload.js   |  4 +-
 .../membership/components/LandscapeMembers.js |  9 +----
 src/permissions/components/Restricted.js      | 14 +------
 src/permissions/components/Restricted.test.js |  1 -
 src/permissions/index.js                      | 37 +++++--------------
 src/permissions/rules.js                      | 22 +++++------
 src/storyMap/components/StoryMapUpdate.js     |  7 +---
 10 files changed, 33 insertions(+), 81 deletions(-)

diff --git a/src/group/components/GroupSharedDataUpload.js b/src/group/components/GroupSharedDataUpload.js
index 29e78d8e8..3cb91b08a 100644
--- a/src/group/components/GroupSharedDataUpload.js
+++ b/src/group/components/GroupSharedDataUpload.js
@@ -60,13 +60,13 @@ const GroupSharedDataUpload = () => {
     navigate(`/groups/${slug}`);
   }, [navigate, slug]);
 
-  const { loading } = usePermissionRedirect(
+  usePermissionRedirect(
     'sharedData.add',
     group,
     useMemo(() => `/groups/${group?.slug}`, [group?.slug])
   );
 
-  if (fetching || loading) {
+  if (fetching) {
     return <PageLoader />;
   }
 
diff --git a/src/group/membership/components/GroupMembers.js b/src/group/membership/components/GroupMembers.js
index 3b9e80e0f..3ada4b87c 100644
--- a/src/group/membership/components/GroupMembers.js
+++ b/src/group/membership/components/GroupMembers.js
@@ -65,18 +65,15 @@ const Header = props => {
     )
   );
 
-  const { loading: loadingPermissions, allowed } = usePermission(
-    'group.manageMembers',
-    group
-  );
+  const { allowed } = usePermission('group.manageMembers', group);
 
-  const { loading } = usePermissionRedirect(
+  usePermissionRedirect(
     'group.viewMembers',
     group,
     useMemo(() => `/groups/${group?.slug}`, [group?.slug])
   );
 
-  if (fetching || loading || loadingPermissions) {
+  if (fetching) {
     return null;
   }
 
diff --git a/src/landscape/components/LandscapeProfile/index.js b/src/landscape/components/LandscapeProfile/index.js
index 4b24262c1..68a484f47 100644
--- a/src/landscape/components/LandscapeProfile/index.js
+++ b/src/landscape/components/LandscapeProfile/index.js
@@ -61,10 +61,7 @@ const LandscapeProfile = () => {
   const dispatch = useDispatch();
   const { landscape, fetching } = useSelector(state => state.landscape.profile);
   const { slug } = useParams();
-  const { loading: loadingPermissions, allowed } = usePermission(
-    'landscape.change',
-    landscape
-  );
+  const { allowed } = usePermission('landscape.change', landscape);
 
   const [isEmptySections, setIsEmptySections] = useState({
     developmentStrategy: false,
@@ -117,7 +114,7 @@ const LandscapeProfile = () => {
     dispatch(refreshLandscapeView(slug));
   }, [dispatch, slug]);
 
-  if (fetching || loadingPermissions) {
+  if (fetching) {
     return <PageLoader />;
   }
 
diff --git a/src/landscape/components/LandscapeSharedDataUpload.js b/src/landscape/components/LandscapeSharedDataUpload.js
index 243032a0a..9ac553c6b 100644
--- a/src/landscape/components/LandscapeSharedDataUpload.js
+++ b/src/landscape/components/LandscapeSharedDataUpload.js
@@ -64,13 +64,13 @@ const LandscapeSharedDataUpload = () => {
     });
   }, [navigate, slug]);
 
-  const { loading } = usePermissionRedirect(
+  usePermissionRedirect(
     'sharedData.add',
     landscape?.defaultGroup,
     useMemo(() => `/landscapes/${landscape?.slug}`, [landscape?.slug])
   );
 
-  if (fetching || loading) {
+  if (fetching) {
     return <PageLoader />;
   }
 
diff --git a/src/landscape/membership/components/LandscapeMembers.js b/src/landscape/membership/components/LandscapeMembers.js
index 3a7dd61f3..d3f25fc7e 100644
--- a/src/landscape/membership/components/LandscapeMembers.js
+++ b/src/landscape/membership/components/LandscapeMembers.js
@@ -43,10 +43,7 @@ const MemberLeaveButton = withProps(LandscapeMemberLeave, {
 
 const Header = ({ landscape, fetching }) => {
   const { t } = useTranslation();
-  const { loading: loadingPermissions, allowed } = usePermission(
-    'group.manageMembers',
-    landscape
-  );
+  const { allowed } = usePermission('group.manageMembers', landscape);
 
   useDocumentTitle(
     t('landscape.members_document_title', {
@@ -69,10 +66,6 @@ const Header = ({ landscape, fetching }) => {
     )
   );
 
-  if (loadingPermissions) {
-    return null;
-  }
-
   return (
     <>
       <PageHeader
diff --git a/src/permissions/components/Restricted.js b/src/permissions/components/Restricted.js
index 524b5230e..bdf7b387d 100644
--- a/src/permissions/components/Restricted.js
+++ b/src/permissions/components/Restricted.js
@@ -16,33 +16,21 @@
  */
 import React from 'react';
 import { usePermission } from 'permissions';
-import { useTranslation } from 'react-i18next';
-import { CircularProgress } from '@mui/material';
 
 const Restricted = props => {
-  const { t } = useTranslation();
   const {
     permission,
     resource,
     toDisallowedUsers = false,
     FallbackComponent,
-    LoadingComponent,
     children,
   } = props;
-  const { loading, allowed } = usePermission(permission, resource);
+  const { allowed } = usePermission(permission, resource);
 
   if (!resource) {
     return null;
   }
 
-  if (loading) {
-    return LoadingComponent ? (
-      <LoadingComponent />
-    ) : (
-      <CircularProgress aria-label={t('common.loader_label')} />
-    );
-  }
-
   if (toDisallowedUsers ? !allowed : allowed) {
     return <>{children}</>;
   }
diff --git a/src/permissions/components/Restricted.test.js b/src/permissions/components/Restricted.test.js
index 08c242d52..3582e3ef8 100644
--- a/src/permissions/components/Restricted.test.js
+++ b/src/permissions/components/Restricted.test.js
@@ -62,7 +62,6 @@ test('Restricted: Display custom loader', async () => {
     {
       resource: {},
       permission: 'resource.action',
-      LoadingComponent: () => <div>Loading...</div>,
       children: <div>Restricted content</div>,
     },
     rules
diff --git a/src/permissions/index.js b/src/permissions/index.js
index 51ffc01de..3d434c8f0 100644
--- a/src/permissions/index.js
+++ b/src/permissions/index.js
@@ -14,7 +14,7 @@
  * You should have received a copy of the GNU Affero General Public License
  * along with this program. If not, see https://www.gnu.org/licenses/.
  */
-import React, { useContext, useEffect, useRef, useState } from 'react';
+import React, { useContext, useEffect, useMemo } from 'react';
 import _ from 'lodash/fp';
 import { useSelector } from 'react-redux';
 import { useNavigate } from 'react-router-dom';
@@ -47,45 +47,26 @@ export const PermissionsProvider = ({ rules, children }) => {
 export const usePermissionRedirect = (permission, resource, path) => {
   const navigate = useNavigate();
 
-  const { loading, allowed } = usePermission(permission, resource);
+  const { allowed } = usePermission(permission, resource);
 
   useEffect(() => {
-    if (loading) {
-      return;
-    }
-
     if (!allowed && path) {
       navigate(path);
     }
-  }, [allowed, loading, navigate, path]);
+  }, [allowed, navigate, path]);
 
-  return { loading };
+  return { allowed };
 };
 
 export const usePermission = (permission, resource) => {
-  const isMounted = useRef(false);
-  const [loading, setLoading] = useState(true);
-  const [allowed, setAllowed] = useState();
   const { data: user } = useSelector(state => state.account.currentUser);
 
   const { isAllowedTo } = useContext(PermissionsContext);
 
-  useEffect(() => {
-    if (!resource) {
-      return;
-    }
-    isMounted.current = true;
-    setLoading(true);
-    isAllowedTo(permission, user, resource).then(allowed => {
-      if (isMounted.current) {
-        setLoading(false);
-        setAllowed(allowed);
-      }
-    });
-    return () => {
-      isMounted.current = false;
-    };
-  }, [isAllowedTo, permission, resource, user]);
+  const allowed = useMemo(
+    () => isAllowedTo(permission, user, resource),
+    [isAllowedTo, permission, resource, user]
+  );
 
-  return { loading, allowed };
+  return { allowed };
 };
diff --git a/src/permissions/rules.js b/src/permissions/rules.js
index a081939f2..97692108a 100644
--- a/src/permissions/rules.js
+++ b/src/permissions/rules.js
@@ -57,7 +57,7 @@ const isAllowedToEditSharedData = ({
 }) => {
   const isManager = hasRole({ group, role: ROLE_MANAGER });
   const isOwner = _.get('createdBy.id', dataEntry) === _.get('id', user);
-  return Promise.resolve(isManager || isOwner);
+  return isManager || isOwner;
 };
 
 const isAllowedToDeleteSharedData = ({ resource, user }) => {
@@ -71,43 +71,43 @@ const isAllowedToDeleteVisualization = ({
   const isManager = hasRole({ group, role: ROLE_MANAGER });
   const isOwner =
     _.get('createdBy.id', visualizationConfig) === _.get('id', user);
-  return Promise.resolve(isManager || isOwner);
+  return isManager || isOwner;
 };
 
 const isAllowedToDownloadSharedData = ({ resource: group }) => {
   const isMember = isApprovedMember(group);
-  return Promise.resolve(isMember);
+  return isMember;
 };
 
 const isAllowedToAddSharedData = ({ resource: group }) => {
   const isMember = isApprovedMember(group);
-  return Promise.resolve(isMember);
+  return isMember;
 };
 
 const isAllowedToChangeGroup = ({ resource: group }) => {
   const isManager = hasRole({ group, role: ROLE_MANAGER });
-  return Promise.resolve(isManager);
+  return isManager;
 };
 
 // is open group or closed + you are a member
 const isAllowedToViewGroupMembers = ({ resource: group }) => {
   const isOpenGroup = group.membershipType === MEMBERSHIP_OPEN;
   if (isOpenGroup) {
-    return Promise.resolve(true);
+    return true;
   }
 
   const isMember = isApprovedMember(group);
-  return Promise.resolve(isMember);
+  return isMember;
 };
 
 const isAllowedToManageGroupMembers = ({ resource: group }) => {
   const isManager = hasRole({ group, role: ROLE_MANAGER });
-  return Promise.resolve(isManager);
+  return isManager;
 };
 
 const isAllowedToViewGroupSharedData = ({ resource: group }) => {
   const isMember = isApprovedMember(group);
-  return Promise.resolve(isMember);
+  return isMember;
 };
 
 const isAllowedToChangeLandscape = ({ resource: landscape }) => {
@@ -115,12 +115,12 @@ const isAllowedToChangeLandscape = ({ resource: landscape }) => {
     group: landscape.defaultGroup,
     role: ROLE_MANAGER,
   });
-  return Promise.resolve(isManager);
+  return isManager;
 };
 
 const isAllowedToChangeStoryMap = ({ resource: storyMap, user }) => {
   const isOwner = _.get('createdBy.id', storyMap) === _.get('id', user);
-  return Promise.resolve(isOwner);
+  return isOwner;
 };
 
 const rules = {
diff --git a/src/storyMap/components/StoryMapUpdate.js b/src/storyMap/components/StoryMapUpdate.js
index b1a46c048..a35a542d6 100644
--- a/src/storyMap/components/StoryMapUpdate.js
+++ b/src/storyMap/components/StoryMapUpdate.js
@@ -131,10 +131,7 @@ const ContextWrapper = props => {
   const { slug, storyMapId } = useParams();
   const dispatch = useDispatch();
   const { fetching, data: storyMap } = useSelector(_.get('storyMap.form'));
-  const { loading: loadingPermissions, allowed } = usePermission(
-    'storyMap.change',
-    storyMap
-  );
+  const { allowed } = usePermission('storyMap.change', storyMap);
 
   useEffect(() => {
     dispatch(resetForm());
@@ -147,7 +144,7 @@ const ContextWrapper = props => {
     )
   );
 
-  if (fetching || loadingPermissions) {
+  if (fetching) {
     return <PageLoader />;
   }
 

From 73ad4b314081ad1c3fd597e2d97b480f3e7df5b3 Mon Sep 17 00:00:00 2001
From: Jose Buitron <jose.buitron@gmail.com>
Date: Tue, 29 Aug 2023 16:19:47 -0500
Subject: [PATCH 2/4] fix: Removed permissions resolution promises

---
 src/permissions/index.js | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/permissions/index.js b/src/permissions/index.js
index 3d434c8f0..0fa6e2fc9 100644
--- a/src/permissions/index.js
+++ b/src/permissions/index.js
@@ -14,31 +14,31 @@
  * You should have received a copy of the GNU Affero General Public License
  * along with this program. If not, see https://www.gnu.org/licenses/.
  */
-import React, { useContext, useEffect, useMemo } from 'react';
+import React, { useCallback, useContext, useEffect, useMemo } from 'react';
 import _ from 'lodash/fp';
 import { useSelector } from 'react-redux';
 import { useNavigate } from 'react-router-dom';
 
 const defaultBehaviour = {
-  isAllowedTo: () => Promise.resolve(false),
+  isAllowedTo: () => false,
 };
 
 export const PermissionsContext = React.createContext(defaultBehaviour);
 
 export const PermissionsProvider = ({ rules, children }) => {
-  const isAllowedTo = (permission, user, resource) => {
+  const isAllowedTo = useCallback((permission, user, resource) => {
     const ruleResolver = _.getOr(
       defaultBehaviour.isAllowedTo,
       permission,
       rules
     );
     return !resource
-      ? Promise.resolve(false)
+      ? false
       : ruleResolver({ user, resource });
-  };
+  }, []);
 
   return (
-    <PermissionsContext.Provider value={{ isAllowedTo }}>
+    <PermissionsContext.Provider value={isAllowedTo}>
       {children}
     </PermissionsContext.Provider>
   );
@@ -61,7 +61,7 @@ export const usePermissionRedirect = (permission, resource, path) => {
 export const usePermission = (permission, resource) => {
   const { data: user } = useSelector(state => state.account.currentUser);
 
-  const { isAllowedTo } = useContext(PermissionsContext);
+  const isAllowedTo = useContext(PermissionsContext);
 
   const allowed = useMemo(
     () => isAllowedTo(permission, user, resource),

From 115a2b700b48c7852a2b93ca1c4050911f025177 Mon Sep 17 00:00:00 2001
From: Jose Buitron <jose.buitron@gmail.com>
Date: Tue, 29 Aug 2023 16:20:18 -0500
Subject: [PATCH 3/4] fix: Removed loaders from test, removed loader tests

---
 src/permissions/components/Restricted.test.js | 42 ++-----------------
 1 file changed, 4 insertions(+), 38 deletions(-)

diff --git a/src/permissions/components/Restricted.test.js b/src/permissions/components/Restricted.test.js
index 3582e3ef8..9d999e962 100644
--- a/src/permissions/components/Restricted.test.js
+++ b/src/permissions/components/Restricted.test.js
@@ -39,40 +39,9 @@ const setup = async (props, rules) => {
   );
 };
 
-test('Restricted: Display default loader', async () => {
-  const rules = {
-    'resource.action': () => new Promise(() => {}),
-  };
-  await setup(
-    {
-      resource: {},
-      permission: 'resource.action',
-      children: <div>Restricted content</div>,
-    },
-    rules
-  );
-  expect(screen.queryByText('Restricted content')).not.toBeInTheDocument();
-  expect(screen.getByRole('progressbar')).toBeInTheDocument();
-});
-test('Restricted: Display custom loader', async () => {
-  const rules = {
-    'resource.action': () => new Promise(() => {}),
-  };
-  await setup(
-    {
-      resource: {},
-      permission: 'resource.action',
-      children: <div>Restricted content</div>,
-    },
-    rules
-  );
-  expect(screen.queryByText('Restricted content')).not.toBeInTheDocument();
-  expect(screen.queryByRole('progressbar')).not.toBeInTheDocument();
-  expect(screen.getByText('Loading...')).toBeInTheDocument();
-});
 test('Restricted: Display allowed component', async () => {
   const rules = {
-    'resource.action': () => Promise.resolve(true),
+    'resource.action': () => true,
   };
   await setup(
     {
@@ -86,7 +55,7 @@ test('Restricted: Display allowed component', async () => {
 });
 test('Restricted: Hide denied component', async () => {
   const rules = {
-    'resource.action': () => Promise.resolve(false),
+    'resource.action': () => false,
   };
   await setup(
     {
@@ -97,11 +66,10 @@ test('Restricted: Hide denied component', async () => {
     rules
   );
   expect(screen.queryByText('Restricted content')).not.toBeInTheDocument();
-  expect(screen.queryByRole('progressbar')).not.toBeInTheDocument();
 });
 test('Restricted: Display component for unallowed users', async () => {
   const rules = {
-    'resource.action': () => Promise.resolve(false),
+    'resource.action': () => false,
   };
   await setup(
     {
@@ -116,7 +84,7 @@ test('Restricted: Display component for unallowed users', async () => {
 });
 test('Restricted: Hide component for allowed users', async () => {
   const rules = {
-    'resource.action': () => Promise.resolve(true),
+    'resource.action': () => true,
   };
   await setup(
     {
@@ -128,7 +96,6 @@ test('Restricted: Hide component for allowed users', async () => {
     rules
   );
   expect(screen.queryByText('Restricted content')).not.toBeInTheDocument();
-  expect(screen.queryByRole('progressbar')).not.toBeInTheDocument();
 });
 test('Restricted: Display fallback component', async () => {
   await setup({
@@ -138,6 +105,5 @@ test('Restricted: Display fallback component', async () => {
     children: <div>Restricted content</div>,
   });
   expect(screen.queryByText('Restricted content')).not.toBeInTheDocument();
-  expect(screen.queryByRole('progressbar')).not.toBeInTheDocument();
   expect(screen.getByText('Fallback content')).toBeInTheDocument();
 });

From 16b33b79d61a45d81bf065839bd663c389033d0e Mon Sep 17 00:00:00 2001
From: Jose Buitron <jose.buitron@gmail.com>
Date: Tue, 29 Aug 2023 16:21:50 -0500
Subject: [PATCH 4/4] fix: Linter fixes

---
 src/permissions/index.js | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/src/permissions/index.js b/src/permissions/index.js
index 0fa6e2fc9..dd0cfe6a8 100644
--- a/src/permissions/index.js
+++ b/src/permissions/index.js
@@ -26,16 +26,17 @@ const defaultBehaviour = {
 export const PermissionsContext = React.createContext(defaultBehaviour);
 
 export const PermissionsProvider = ({ rules, children }) => {
-  const isAllowedTo = useCallback((permission, user, resource) => {
-    const ruleResolver = _.getOr(
-      defaultBehaviour.isAllowedTo,
-      permission,
-      rules
-    );
-    return !resource
-      ? false
-      : ruleResolver({ user, resource });
-  }, []);
+  const isAllowedTo = useCallback(
+    (permission, user, resource) => {
+      const ruleResolver = _.getOr(
+        defaultBehaviour.isAllowedTo,
+        permission,
+        rules
+      );
+      return !resource ? false : ruleResolver({ user, resource });
+    },
+    [rules]
+  );
 
   return (
     <PermissionsContext.Provider value={isAllowedTo}>