From 9b5c33527bd3b91dfc5fc23bbe63f0d02ddfa4dd Mon Sep 17 00:00:00 2001
From: Zach Waterfield <zlwaterfield@gmail.com>
Date: Fri, 27 Dec 2024 14:23:25 -0500
Subject: [PATCH] improve access control object ui

---
 .../access_control/AccessControlObject.tsx    | 185 +++++++++++-------
 1 file changed, 117 insertions(+), 68 deletions(-)

diff --git a/frontend/src/layout/navigation-3000/sidepanel/panels/access_control/AccessControlObject.tsx b/frontend/src/layout/navigation-3000/sidepanel/panels/access_control/AccessControlObject.tsx
index 93e14755e12d5..d0660023786fc 100644
--- a/frontend/src/layout/navigation-3000/sidepanel/panels/access_control/AccessControlObject.tsx
+++ b/frontend/src/layout/navigation-3000/sidepanel/panels/access_control/AccessControlObject.tsx
@@ -1,9 +1,10 @@
-import { IconX } from '@posthog/icons'
+import { IconTrash } from '@posthog/icons'
 import {
     LemonBanner,
     LemonButton,
     LemonDialog,
     LemonInputSelect,
+    LemonModal,
     LemonSelect,
     LemonSelectProps,
     LemonTable,
@@ -37,7 +38,7 @@ export function AccessControlObject(props: AccessControlLogicProps): JSX.Element
 
     return (
         <BindLogic logic={accessControlLogic} props={props}>
-            <div className="space-y-4">
+            <div className="space-y-6">
                 {canEditAccessControls === false ? (
                     <LemonBanner type="info">
                         <b>You don't have permission to edit access controls for {suffix}.</b>
@@ -48,12 +49,10 @@ export function AccessControlObject(props: AccessControlLogicProps): JSX.Element
                 <h3>Default access to {suffix}</h3>
                 <AccessControlObjectDefaults />
 
-                <h3>Members</h3>
                 <PayGateMini feature={AvailableFeature.PROJECT_BASED_PERMISSIONING}>
                     <AccessControlObjectUsers />
                 </PayGateMini>
 
-                <h3>Roles</h3>
                 <PayGateMini feature={AvailableFeature.ROLE_BASED_ACCESS}>
                     <AccessControlObjectRoles />
                 </PayGateMini>
@@ -88,11 +87,19 @@ function AccessControlObjectDefaults(): JSX.Element | null {
 
 function AccessControlObjectUsers(): JSX.Element | null {
     const { user } = useValues(userLogic)
-    const { membersById, addableMembers, accessControlMembers, accessControlsLoading, availableLevels } =
-        useValues(accessControlLogic)
+    const {
+        membersById,
+        addableMembers,
+        accessControlMembers,
+        accessControlsLoading,
+        availableLevels,
+        canEditAccessControls,
+    } = useValues(accessControlLogic)
     const { updateAccessControlMembers } = useAsyncActions(accessControlLogic)
     const { guardAvailableFeature } = useValues(upgradeModalLogic)
 
+    const [modelOpen, setModelOpen] = useState(false)
+
     if (!user) {
         return null
     }
@@ -104,33 +111,26 @@ function AccessControlObjectUsers(): JSX.Element | null {
     // TODO: WHAT A MESS - Fix this to do the index mapping beforehand...
     const columns: LemonTableColumns<AccessControlTypeMember> = [
         {
-            key: 'user_profile_picture',
-            render: function ProfilePictureRender(_, ac) {
-                return <ProfilePicture user={member(ac)?.user} />
-            },
-            width: 32,
-        },
-        {
-            title: 'Name',
-            key: 'user_first_name',
+            key: 'user',
+            title: 'User',
             render: (_, ac) => (
-                <b>
-                    {member(ac)?.user.uuid == user.uuid
-                        ? `${member(ac)?.user.first_name} (you)`
-                        : member(ac)?.user.first_name}
-                </b>
+                <div className="flex items-center gap-2">
+                    <ProfilePicture user={member(ac)?.user} />
+                    <div>
+                        <p className="font-medium mb-0">
+                            {member(ac)?.user.uuid == user.uuid
+                                ? `${member(ac)?.user.first_name} (you)`
+                                : member(ac)?.user.first_name}
+                        </p>
+                        <p className="text-muted-alt mb-0">{member(ac)?.user.email}</p>
+                    </div>
+                </div>
             ),
             sorter: (a, b) => member(a)?.user.first_name.localeCompare(member(b)?.user.first_name),
         },
         {
-            title: 'Email',
-            key: 'user_email',
-            render: (_, ac) => member(ac)?.user.email,
-            sorter: (a, b) => member(a)?.user.email.localeCompare(member(b)?.user.email),
-        },
-        {
-            title: 'Level',
             key: 'level',
+            title: 'Level',
             width: 0,
             render: function LevelRender(_, { access_level, organization_member }) {
                 return (
@@ -164,12 +164,30 @@ function AccessControlObjectUsers(): JSX.Element | null {
     ]
 
     return (
-        <div className="space-y-2">
-            <AddItemsControls
+        <>
+            <div className="space-y-2">
+                <div className="flex gap-2 items-center justify-between">
+                    <h3 className="mb-0">Members</h3>
+                    <LemonButton
+                        type="primary"
+                        onClick={() => setModelOpen(true)}
+                        disabledReason={!canEditAccessControls ? 'You cannot edit this' : undefined}
+                    >
+                        Add
+                    </LemonButton>
+                </div>
+
+                <LemonTable columns={columns} dataSource={accessControlMembers} loading={accessControlsLoading} />
+            </div>
+
+            <AddItemsControlsModal
+                modelOpen={modelOpen}
+                setModelOpen={setModelOpen}
                 placeholder="Search for team members to add…"
                 onAdd={async (newValues, level) => {
                     if (guardAvailableFeature(AvailableFeature.PROJECT_BASED_PERMISSIONING)) {
                         await updateAccessControlMembers(newValues.map((member) => ({ member, level })))
+                        setModelOpen(false)
                     }
                 }}
                 options={addableMembers.map((member) => ({
@@ -178,18 +196,24 @@ function AccessControlObjectUsers(): JSX.Element | null {
                     labelComponent: <UserSelectItem user={member.user} />,
                 }))}
             />
-
-            <LemonTable columns={columns} dataSource={accessControlMembers} loading={accessControlsLoading} />
-        </div>
+        </>
     )
 }
 
 function AccessControlObjectRoles(): JSX.Element | null {
-    const { accessControlRoles, accessControlsLoading, addableRoles, rolesById, availableLevels } =
-        useValues(accessControlLogic)
+    const {
+        accessControlRoles,
+        accessControlsLoading,
+        addableRoles,
+        rolesById,
+        availableLevels,
+        canEditAccessControls,
+    } = useValues(accessControlLogic)
     const { updateAccessControlRoles } = useAsyncActions(accessControlLogic)
     const { guardAvailableFeature } = useValues(upgradeModalLogic)
 
+    const [modelOpen, setModelOpen] = useState(false)
+
     const columns: LemonTableColumns<AccessControlTypeRole> = [
         {
             title: 'Role',
@@ -253,12 +277,30 @@ function AccessControlObjectRoles(): JSX.Element | null {
     ]
 
     return (
-        <div className="space-y-2">
-            <AddItemsControls
+        <>
+            <div className="space-y-2">
+                <div className="flex gap-2 items-center justify-between">
+                    <h3 className="mb-0">Roles</h3>
+                    <LemonButton
+                        type="primary"
+                        onClick={() => setModelOpen(true)}
+                        disabledReason={!canEditAccessControls ? 'You cannot edit this' : undefined}
+                    >
+                        Add
+                    </LemonButton>
+                </div>
+
+                <LemonTable columns={columns} dataSource={accessControlRoles} loading={accessControlsLoading} />
+            </div>
+
+            <AddItemsControlsModal
+                modelOpen={modelOpen}
+                setModelOpen={setModelOpen}
                 placeholder="Search for roles to add…"
                 onAdd={async (newValues, level) => {
                     if (guardAvailableFeature(AvailableFeature.PROJECT_BASED_PERMISSIONING)) {
                         await updateAccessControlRoles(newValues.map((role) => ({ role, level })))
+                        setModelOpen(false)
                     }
                 }}
                 options={addableRoles.map((role) => ({
@@ -266,9 +308,7 @@ function AccessControlObjectRoles(): JSX.Element | null {
                     label: role.name,
                 }))}
             />
-
-            <LemonTable columns={columns} dataSource={accessControlRoles} loading={accessControlsLoading} />
-        </div>
+        </>
     )
 }
 
@@ -306,8 +346,7 @@ function RemoveAccessButton({
 
     return (
         <LemonButton
-            icon={<IconX />}
-            status="danger"
+            icon={<IconTrash />}
             size="small"
             disabledReason={!canEditAccessControls ? 'You cannot edit this' : undefined}
             onClick={() =>
@@ -325,7 +364,9 @@ function RemoveAccessButton({
     )
 }
 
-function AddItemsControls(props: {
+function AddItemsControlsModal(props: {
+    modelOpen: boolean
+    setModelOpen: (open: boolean) => void
     placeholder: string
     onAdd: (newValues: string[], level: AccessControlType['access_level']) => Promise<void>
     options: {
@@ -352,32 +393,40 @@ function AddItemsControls(props: {
             : undefined
 
     return (
-        <div className="flex gap-2 items-center">
-            <div className="min-w-[16rem]">
-                <LemonInputSelect
-                    placeholder={props.placeholder}
-                    value={items}
-                    onChange={(newValues: string[]) => setItems(newValues)}
-                    mode="multiple"
-                    options={props.options}
-                    disabled={!canEditAccessControls}
-                />
+        <LemonModal
+            isOpen={props.modelOpen || false}
+            onClose={() => props.setModelOpen(false)}
+            title="Add access"
+            maxWidth="30rem"
+            description="Allow other users or roles to access this resource"
+        >
+            <div className="flex gap-2 items-center">
+                <div className="min-w-[16rem]">
+                    <LemonInputSelect
+                        placeholder={props.placeholder}
+                        value={items}
+                        onChange={(newValues: string[]) => setItems(newValues)}
+                        mode="multiple"
+                        options={props.options}
+                        disabled={!canEditAccessControls}
+                    />
+                </div>
+                <SimplLevelComponent levels={availableLevels} level={level} onChange={setLevel} />
+
+                <LemonButton
+                    type="primary"
+                    onClick={onSubmit}
+                    disabledReason={
+                        !canEditAccessControls
+                            ? 'You cannot edit this'
+                            : !onSubmit
+                            ? 'Please choose what you want to add and at what level'
+                            : undefined
+                    }
+                >
+                    Add
+                </LemonButton>
             </div>
-            <SimplLevelComponent levels={availableLevels} level={level} onChange={setLevel} />
-
-            <LemonButton
-                type="primary"
-                onClick={onSubmit}
-                disabledReason={
-                    !canEditAccessControls
-                        ? 'You cannot edit this'
-                        : !onSubmit
-                        ? 'Please choose what you want to add and at what level'
-                        : undefined
-                }
-            >
-                Add
-            </LemonButton>
-        </div>
+        </LemonModal>
     )
 }