From 41e4af4e656094ce6af519508cc8ec5ff3e9bcb6 Mon Sep 17 00:00:00 2001
From: Scott Wilson <scottraywilson@gmail.com>
Date: Fri, 11 Oct 2024 12:23:47 -0700
Subject: [PATCH] improvement: adjust policy UI for flow/clarity

---
 .../GeneralPermissionConditions.tsx           |  8 ++-
 .../components/GeneralPermissionPolicies.tsx  | 68 +++++++++++--------
 .../components/SecretPermissionConditions.tsx |  8 ++-
 3 files changed, 50 insertions(+), 34 deletions(-)

diff --git a/frontend/src/views/Project/RolePage/components/RolePermissionsSection/components/GeneralPermissionConditions.tsx b/frontend/src/views/Project/RolePage/components/RolePermissionsSection/components/GeneralPermissionConditions.tsx
index 989b7c19b..3d147c708 100644
--- a/frontend/src/views/Project/RolePage/components/RolePermissionsSection/components/GeneralPermissionConditions.tsx
+++ b/frontend/src/views/Project/RolePage/components/RolePermissionsSection/components/GeneralPermissionConditions.tsx
@@ -43,7 +43,11 @@ export const GeneralPermissionConditions = ({ position = 0, isDisabled, type }:
   });
 
   return (
-    <div className="mt-6 border-t  border-t-gray-800 bg-mineshaft-800  pt-2">
+    <div className="mt-6 border-t border-t-mineshaft-600 bg-mineshaft-800 pt-2">
+      <p className="mt-2 text-gray-300">Conditions</p>
+      <p className="mb-2 text-sm text-mineshaft-400">
+        When this policy should apply (always if no conditions are added).
+      </p>
       <div className="mt-2 flex flex-col space-y-2">
         {items.fields.map((el, index) => {
           const condition = watch(`permissions.secrets.${position}.conditions.${index}`) as {
@@ -163,7 +167,7 @@ export const GeneralPermissionConditions = ({ position = 0, isDisabled, type }:
             })
           }
         >
-          New Condition
+          Add Condition
         </Button>
       </div>
     </div>
diff --git a/frontend/src/views/Project/RolePage/components/RolePermissionsSection/components/GeneralPermissionPolicies.tsx b/frontend/src/views/Project/RolePage/components/RolePermissionsSection/components/GeneralPermissionPolicies.tsx
index 5a7a0fdef..58c403579 100644
--- a/frontend/src/views/Project/RolePage/components/RolePermissionsSection/components/GeneralPermissionPolicies.tsx
+++ b/frontend/src/views/Project/RolePage/components/RolePermissionsSection/components/GeneralPermissionPolicies.tsx
@@ -73,6 +73,44 @@ export const GeneralPermissionPolicies = <T extends keyof NonNullable<TFormSchem
         <div key={`select-${subject}-type`} className="flex flex-col space-y-4 bg-bunker-800 p-6">
           {items.fields.map((el, rootIndex) => (
             <div key={el.id} className="bg-mineshaft-800 p-5 first:rounded-t-md last:rounded-b-md">
+              {isConditionalSubjects(subject) && (
+                <div className="mt-4 mb-6 flex w-full items-center text-gray-300">
+                  <div className="w-1/4">Permission</div>
+                  <div className="mr-4 w-1/4">
+                    <Controller
+                      defaultValue={false as any}
+                      name={`permissions.${subject}.${rootIndex}.inverted`}
+                      render={({ field }) => (
+                        <Select
+                          value={String(field.value)}
+                          onValueChange={(val) => field.onChange(val === "true")}
+                          containerClassName="w-full"
+                          className="w-full"
+                        >
+                          <SelectItem value="false">Allow</SelectItem>
+                          <SelectItem value="true">Forbid</SelectItem>
+                        </Select>
+                      )}
+                    />
+                  </div>
+                  <div>
+                    <Tooltip
+                      asChild
+                      content={
+                        <>
+                          <p>
+                            Whether to allow or forbid the selected actions when the following
+                            conditions (if any) are met.
+                          </p>
+                          <p className="mt-2">Forbid rules must come after allow rules.</p>
+                        </>
+                      }
+                    >
+                      <FontAwesomeIcon icon={faInfoCircle} size="sm" className="text-gray-400" />
+                    </Tooltip>
+                  </div>
+                </div>
+              )}
               <div className="flex text-gray-300">
                 <div className="w-1/4">Actions</div>
                 <div className="flex flex-grow flex-wrap justify-start gap-8">
@@ -101,36 +139,6 @@ export const GeneralPermissionPolicies = <T extends keyof NonNullable<TFormSchem
                   })}
                 </div>
               </div>
-              {isConditionalSubjects(subject) && (
-                <div className="mt-4 flex w-full items-center text-gray-300">
-                  <div className="w-1/4">Effect</div>
-                  <div className="mr-4 w-1/4">
-                    <Controller
-                      defaultValue={false as any}
-                      name={`permissions.${subject}.${rootIndex}.inverted`}
-                      render={({ field }) => (
-                        <Select
-                          value={String(field.value)}
-                          onValueChange={(val) => field.onChange(val === "true")}
-                          containerClassName="w-full"
-                          className="w-full"
-                        >
-                          <SelectItem value="false">Allow</SelectItem>
-                          <SelectItem value="true">Disallow</SelectItem>
-                        </Select>
-                      )}
-                    />
-                  </div>
-                  <div>
-                    <Tooltip
-                      asChild
-                      content="Whether to allow or forbid. Forbid rules must be added after allow rules."
-                    >
-                      <FontAwesomeIcon icon={faInfoCircle} size="sm" className="text-gray-400" />
-                    </Tooltip>
-                  </div>
-                </div>
-              )}
               {children &&
                 cloneElement(children, {
                   position: rootIndex
diff --git a/frontend/src/views/Project/RolePage/components/RolePermissionsSection/components/SecretPermissionConditions.tsx b/frontend/src/views/Project/RolePage/components/RolePermissionsSection/components/SecretPermissionConditions.tsx
index e0e5aa471..b858545de 100644
--- a/frontend/src/views/Project/RolePage/components/RolePermissionsSection/components/SecretPermissionConditions.tsx
+++ b/frontend/src/views/Project/RolePage/components/RolePermissionsSection/components/SecretPermissionConditions.tsx
@@ -36,7 +36,11 @@ export const SecretPermissionConditions = ({ position = 0, isDisabled }: Props)
   });
 
   return (
-    <div className="mt-6 border-t  border-t-gray-800 bg-mineshaft-800  pt-2">
+    <div className="mt-6 border-t border-t-mineshaft-600 bg-mineshaft-800 pt-2">
+      <p className="mt-2 text-gray-300">Conditions</p>
+      <p className="mb-2 text-sm text-mineshaft-400">
+        When this policy should apply (always if no conditions are added).
+      </p>
       <div className="mt-2 flex flex-col space-y-2">
         {items.fields.map((el, index) => {
           const condition = watch(`permissions.secrets.${position}.conditions.${index}`) as {
@@ -157,7 +161,7 @@ export const SecretPermissionConditions = ({ position = 0, isDisabled }: Props)
             })
           }
         >
-          New Condition
+          Add Condition
         </Button>
       </div>
     </div>