From 5f77c6d3b167ff623e2a33af84bc72127947814b Mon Sep 17 00:00:00 2001
From: marrobi <marrobi@microsoft.com>
Date: Thu, 12 Oct 2023 09:35:06 +0000
Subject: [PATCH 1/2] Nesting of PivotItems in SecuredByRole causes issues
 Fixes #3738

---
 ui/app/src/components/shared/CostsTag.tsx     |  6 +-
 ui/app/src/components/shared/ResourceBody.tsx | 16 ++--
 ui/app/src/components/shared/ResourceCard.tsx |  1 -
 .../src/components/shared/SecuredByRole.tsx   | 46 +++++------
 .../workspaces/WorkspaceProvider.tsx          | 80 ++++++++++---------
 5 files changed, 76 insertions(+), 73 deletions(-)

diff --git a/ui/app/src/components/shared/CostsTag.tsx b/ui/app/src/components/shared/CostsTag.tsx
index 5c50c6fb82..507ef37916 100644
--- a/ui/app/src/components/shared/CostsTag.tsx
+++ b/ui/app/src/components/shared/CostsTag.tsx
@@ -25,7 +25,7 @@ export const CostsTag: React.FunctionComponent<CostsTagProps> = (props: CostsTag
         costs = workspaceCtx.costs;
       } else if (costsCtx.costs.length > 0) {
         costs = costsCtx.costs;
-      } else {
+      } else if(!workspaceCtx.workspace.id) {
         let scopeId = (await apiCall(`${ApiEndpoint.Workspaces}/${props.resourceId}/scopeid`, HttpMethod.Get)).workspaceAuth.scopeId;
         const r = await apiCall(`${ApiEndpoint.Workspaces}/${props.resourceId}/${ApiEndpoint.Costs}`, HttpMethod.Get, scopeId, undefined, ResultType.JSON);
         costs = [{costs: r.costs, id: r.id, name: r.name }];
@@ -44,11 +44,11 @@ export const CostsTag: React.FunctionComponent<CostsTagProps> = (props: CostsTag
           maximumFractionDigits: 2
         }).format(resourceCosts.costs[0].cost);
         setFormattedCost(formattedCost);
-        setLoadingState(LoadingState.Ok);
       }
+      setLoadingState(LoadingState.Ok);
     }
     fetchCostData();
-  }, [apiCall, costsCtx.loadingState, props.resourceId, workspaceCtx.costs, costsCtx.costs]);
+  }, [apiCall, props.resourceId, workspaceCtx.costs, costsCtx.costs, workspaceCtx.workspace.id]);
 
   const costBadge = (
     <Stack.Item style={{ maxHeight: 18 }} className="tre-badge">
diff --git a/ui/app/src/components/shared/ResourceBody.tsx b/ui/app/src/components/shared/ResourceBody.tsx
index cf35e9681e..7041a1f843 100644
--- a/ui/app/src/components/shared/ResourceBody.tsx
+++ b/ui/app/src/components/shared/ResourceBody.tsx
@@ -62,19 +62,19 @@ export const ResourceBody: React.FunctionComponent<ResourceBodyProps> = (props:
       }
       {
         !props.readonly &&
-        <SecuredByRole allowedAppRoles={historyRoles} allowedWorkspaceRoles={historyRoles} workspaceId={workspaceId} element={
-          <PivotItem headerText="History">
+        <PivotItem headerText="History">
+          <SecuredByRole allowedAppRoles={historyRoles} allowedWorkspaceRoles={historyRoles} workspaceId={workspaceId} errorString={`Must have ${historyRoles.join(" or ")} role`} element={
             <ResourceHistoryList resource={props.resource} />
-          </PivotItem>
-        } />
+          } />
+        </PivotItem>
       }
       {
         !props.readonly &&
-        <SecuredByRole allowedAppRoles={operationsRoles} allowedWorkspaceRoles={operationsRoles} workspaceId={workspaceId} element={
-          <PivotItem headerText="Operations">
+        <PivotItem headerText="Operations">
+          <SecuredByRole allowedAppRoles={operationsRoles} allowedWorkspaceRoles={operationsRoles} workspaceId={workspaceId} errorString={`Must have ${operationsRoles.join(" or ")} role`} element={
             <ResourceOperationsList resource={props.resource} />
-          </PivotItem>
-        } />
+          } />
+        </PivotItem>
       }
     </Pivot>
   );
diff --git a/ui/app/src/components/shared/ResourceCard.tsx b/ui/app/src/components/shared/ResourceCard.tsx
index 0176cbf787..10b9c05136 100644
--- a/ui/app/src/components/shared/ResourceCard.tsx
+++ b/ui/app/src/components/shared/ResourceCard.tsx
@@ -149,7 +149,6 @@ export const ResourceCard: React.FunctionComponent<ResourceCardProps> = (props:
                   </Stack.Item>
                 </Stack>
               </Stack.Item>
-              {console.log("costTagsToles", costsTagsRoles)}
               <SecuredByRole allowedAppRoles={costsTagsRoles} allowedWorkspaceRoles={costsTagsRoles} workspaceId={workspaceId} element={
                 <CostsTag resourceId={props.resource.id} />
               }
diff --git a/ui/app/src/components/shared/SecuredByRole.tsx b/ui/app/src/components/shared/SecuredByRole.tsx
index e80eee5a29..049be0ea57 100644
--- a/ui/app/src/components/shared/SecuredByRole.tsx
+++ b/ui/app/src/components/shared/SecuredByRole.tsx
@@ -1,4 +1,4 @@
-import React, { useContext, useEffect, useRef, useState } from 'react';
+import React, { useContext, useEffect, useState } from 'react';
 import { WorkspaceContext } from '../../contexts/WorkspaceContext';
 import { AppRolesContext } from '../../contexts/AppRolesContext';
 import { MessageBar, MessageBarType } from '@fluentui/react';
@@ -18,44 +18,42 @@ export const SecuredByRole: React.FunctionComponent<SecuredByRoleProps> = (props
   const apiCall = useAuthApiCall();
 
   const appRoles = useContext(AppRolesContext);
-  const workspaceCtx = useRef(useContext(WorkspaceContext));
-  let [workspaceRoles, setRoles] = useState([] as Array<string>);
+  const workspaceCtx = useContext(WorkspaceContext);
+  const [workspaceRoles, setRoles] = useState([] as Array<string>);
 
   useEffect(() => {
     const getWorkspaceRoles = async () => {
-      if (!workspaceCtx.current.workspace.id && props.workspaceId !== "") {
-        let workspaceRoles = [] as Array<string>;
+      if (!workspaceCtx.workspace.id && props.workspaceId !== "") {
+        let r = [] as Array<string>;
 
         let workspaceAuth = (await apiCall(`${ApiEndpoint.Workspaces}/${props.workspaceId}/scopeid`, HttpMethod.Get)).workspaceAuth;
         if (workspaceAuth) {
           await apiCall(`${ApiEndpoint.Workspaces}/${props.workspaceId}`, HttpMethod.Get, workspaceAuth.scopeId,
-          undefined, ResultType.JSON, (roles: Array<string>) => {
-            workspaceRoles = roles;
-          }, true);
+            undefined, ResultType.JSON, (roles: Array<string>) => {
+              r = roles;
+            }, true);
         }
-        setRoles(workspaceRoles);
+        setRoles(r);
       }
     };
 
-    if (workspaceCtx.current.roles.length === 0 && props.workspaceId !== undefined){
+    if (workspaceCtx.roles.length === 0 && props.workspaceId !== undefined) {
       getWorkspaceRoles();
     }
     else {
-      setRoles(workspaceCtx.current.roles);
+      setRoles(workspaceCtx.roles);
     }
 
-  }, [apiCall, workspaceCtx.current.workspace.id , props.workspaceId, workspaceCtx.current.roles]);
-
-  if (workspaceRoles.some(x => props.allowedWorkspaceRoles?.includes(x))) return props.element;
-
-  if (appRoles.roles.some(x => props.allowedAppRoles?.includes(x))) return props.element;
-
-  return props.errorString ? (
-    <MessageBar messageBarType={MessageBarType.error} isMultiline={true}>
-      <h3>Access Denied</h3>
-      <p>{props.errorString}</p>
-    </MessageBar>
-  ) : (
-    <></>
+  }, [apiCall, workspaceCtx.workspace.id, props.workspaceId, workspaceCtx.roles]);
+
+  return (
+    (workspaceRoles.some(x => props.allowedWorkspaceRoles?.includes(x)) || appRoles.roles.some(x => props.allowedAppRoles?.includes(x)))
+      ? props.element
+      : (props.errorString && (workspaceRoles.length > 0 || appRoles.roles.length > 0)
+          ? <MessageBar messageBarType={MessageBarType.error} isMultiline={true}>
+              <h3>Access Denied</h3>
+              <p>{props.errorString}</p>
+            </MessageBar>
+          : null)
   );
 };
diff --git a/ui/app/src/components/workspaces/WorkspaceProvider.tsx b/ui/app/src/components/workspaces/WorkspaceProvider.tsx
index f62224aa65..d52e5021b2 100644
--- a/ui/app/src/components/workspaces/WorkspaceProvider.tsx
+++ b/ui/app/src/components/workspaces/WorkspaceProvider.tsx
@@ -27,6 +27,7 @@ export const WorkspaceProvider: React.FunctionComponent = () => {
   const [workspaceServices, setWorkspaceServices] = useState([] as Array<WorkspaceService>);
   const [sharedServices, setSharedServices] = useState([] as Array<SharedService>);
   const workspaceCtx = useRef(useContext(WorkspaceContext));
+  const [wsRoles, setWSRoles] = useState([] as Array<string>);
   const [loadingState, setLoadingState] = useState(LoadingState.Loading);
   const [apiError, setApiError] = useState({} as APIError);
   const { workspaceId } = useParams();
@@ -37,41 +38,6 @@ export const WorkspaceProvider: React.FunctionComponent = () => {
 
   // set workspace context from url
   useEffect(() => {
-    const getWorkspaceCosts = async () => {
-      try {
-        // TODO: amend when costs enabled in API for WorkspaceRoleName.Researcher
-        if(workspaceCtx.current.roles.includes(WorkspaceRoleName.WorkspaceOwner)){
-          let scopeId = (await apiCall(`${ApiEndpoint.Workspaces}/${workspaceId}/scopeid`, HttpMethod.Get)).workspaceAuth.scopeId;
-          const r = await apiCall(`${ApiEndpoint.Workspaces}/${workspaceId}/${ApiEndpoint.Costs}`, HttpMethod.Get, scopeId, undefined, ResultType.JSON);
-          const costs = [
-            ...r.costs,
-            ...r.workspace_services,
-            ...r.workspace_services.flatMap((ws: { user_resources: any; }) => [
-              ...ws.user_resources
-            ])
-          ];
-          workspaceCtx.current.setCosts(costs);
-        }
-      }
-      catch (e: any) {
-        if (e instanceof APIError) {
-          if (e.status === 404 /*subscription not supported*/) {
-          }
-          else if (e.status === 429 /*too many requests*/ || e.status === 503 /*service unavaiable*/) {
-            let msg = JSON.parse(e.message);
-            let retryAfter = Number(msg.error["retry-after"]);
-            setTimeout(getWorkspaceCosts, retryAfter * 1000);
-          }
-          else {
-            e.userMessage = 'Error retrieving costs';
-          }
-        }
-        else {
-          e.userMessage = 'Error retrieving costs';
-        }
-        setCostApiError(e);
-      }
-    };
 
     const getWorkspace = async () => {
       try {
@@ -95,16 +61,16 @@ export const WorkspaceProvider: React.FunctionComponent = () => {
           ws = (await apiCall(`${ApiEndpoint.Workspaces}/${workspaceId}`, HttpMethod.Get, scopeId)).workspace;
           workspaceCtx.current.setWorkspace(ws);
           workspaceCtx.current.setRoles(wsRoles);
+          setWSRoles(wsRoles);
 
           // get workspace services to pass to nav + ws services page
           const workspaceServices = await apiCall(`${ApiEndpoint.Workspaces}/${ws.id}/${ApiEndpoint.WorkspaceServices}`,
             HttpMethod.Get, ws.properties.scope_id);
           setWorkspaceServices(workspaceServices.workspaceServices);
-          setLoadingState(LoadingState.Ok);
           // get shared services to pass to nav shared services pages
           const sharedServices = await apiCall(ApiEndpoint.SharedServices, HttpMethod.Get);
           setSharedServices(sharedServices.sharedServices);
-          getWorkspaceCosts();
+          setLoadingState(LoadingState.Ok);
         } else if (appRoles.roles.includes(RoleName.TREAdmin)) {
           ws = (await apiCall(`${ApiEndpoint.Workspaces}/${workspaceId}`, HttpMethod.Get)).workspace;
           workspaceCtx.current.setWorkspace(ws);
@@ -140,6 +106,46 @@ export const WorkspaceProvider: React.FunctionComponent = () => {
     };
   }, [apiCall, workspaceId, isTREAdminUser, appRoles.roles]);
 
+  useEffect(() => {
+    const getWorkspaceCosts = async () => {
+      try {
+        // TODO: amend when costs enabled in API for WorkspaceRoleName.Researcher
+        if(wsRoles.includes(WorkspaceRoleName.WorkspaceOwner)){
+          let scopeId = (await apiCall(`${ApiEndpoint.Workspaces}/${workspaceId}/scopeid`, HttpMethod.Get)).workspaceAuth.scopeId;
+          const r = await apiCall(`${ApiEndpoint.Workspaces}/${workspaceId}/${ApiEndpoint.Costs}`, HttpMethod.Get, scopeId, undefined, ResultType.JSON);
+          const costs = [
+            ...r.costs,
+            ...r.workspace_services,
+            ...r.workspace_services.flatMap((ws: { user_resources: any; }) => [
+              ...ws.user_resources
+            ])
+          ];
+          workspaceCtx.current.setCosts(costs);
+        }
+      }
+      catch (e: any) {
+        if (e instanceof APIError) {
+          if (e.status === 404 /*subscription not supported*/) {
+          }
+          else if (e.status === 429 /*too many requests*/ || e.status === 503 /*service unavaiable*/) {
+            let msg = JSON.parse(e.message);
+            let retryAfter = Number(msg.error["retry-after"]);
+            setTimeout(getWorkspaceCosts, retryAfter * 1000);
+          }
+          else {
+            e.userMessage = 'Error retrieving costs';
+          }
+        }
+        else {
+          e.userMessage = 'Error retrieving costs';
+        }
+        setCostApiError(e);
+      }
+    };
+
+    getWorkspaceCosts();
+  },[apiCall, workspaceId, wsRoles]);
+
   const addWorkspaceService = (w: WorkspaceService) => {
     let ws = [...workspaceServices];
     ws.push(w);

From 2cbd1f894e23f2577c9ed14d0e4d5b25341e2d71 Mon Sep 17 00:00:00 2001
From: marrobi <marrobi@microsoft.com>
Date: Thu, 12 Oct 2023 09:38:48 +0000
Subject: [PATCH 2/2] Update UI version

---
 ui/app/package.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/app/package.json b/ui/app/package.json
index 6d01430cd4..fd34b55863 100644
--- a/ui/app/package.json
+++ b/ui/app/package.json
@@ -1,6 +1,6 @@
 {
   "name": "tre-ui",
-  "version": "0.5.9",
+  "version": "0.5.10",
   "private": true,
   "dependencies": {
     "@azure/msal-browser": "^2.35.0",