From bb094f60c1c1d0437392741d65e2214939eae78f Mon Sep 17 00:00:00 2001 From: Scott Wilson Date: Fri, 29 Nov 2024 10:44:05 -0800 Subject: [PATCH] improvement: update secret approval policy form to use filterable selects w/ UI revisions --- .../v2/FilterableSelect/FilterableSelect.tsx | 17 +- frontend/src/helpers/members.ts | 12 + .../ApprovalPolicyList/ApprovalPolicyList.tsx | 8 +- .../components/AccessPolicyModal.tsx | 284 +++++++++--------- .../components/ApprovalPolicyRow.tsx | 236 ++++----------- 5 files changed, 238 insertions(+), 319 deletions(-) create mode 100644 frontend/src/helpers/members.ts diff --git a/frontend/src/components/v2/FilterableSelect/FilterableSelect.tsx b/frontend/src/components/v2/FilterableSelect/FilterableSelect.tsx index bca2bf516..f17083248 100644 --- a/frontend/src/components/v2/FilterableSelect/FilterableSelect.tsx +++ b/frontend/src/components/v2/FilterableSelect/FilterableSelect.tsx @@ -34,17 +34,22 @@ export const FilterableSelect = ({ tabSelectsValue={tabSelectsValue} components={{ DropdownIndicator, ClearIndicator, MultiValueRemove, Option }} classNames={{ - container: () => "w-full text-sm font-inter", - control: ({ isFocused }) => + container: ({ isDisabled }) => + twMerge("w-full text-sm font-inter", isDisabled && "!pointer-events-auto opacity-50"), + control: ({ isFocused, isDisabled }) => twMerge( - isFocused ? "border-primary-400/50" : "border-mineshaft-600 hover:border-gray-400", - "border w-full p-0.5 rounded-md text-mineshaft-200 font-inter bg-mineshaft-900 hover:cursor-pointer" + isFocused ? "border-primary-400/50" : "border-mineshaft-600 ", + `border w-full p-0.5 rounded-md text-mineshaft-200 font-inter bg-mineshaft-900 ${ + isDisabled ? "!cursor-not-allowed" : "hover:border-gray-400 hover:cursor-pointer" + } ` ), placeholder: () => `${isMulti ? "py-[0.22rem]" : "leading-7"} text-mineshaft-400 text-sm pl-1`, - input: () => "pl-1 py-0.5", + input: () => "pl-1", valueContainer: () => - `p-1 max-h-[14rem] ${isMulti ? "!overflow-y-auto thin-scrollbar" : ""} gap-1`, + `px-1 max-h-[8.2rem] ${ + isMulti ? "!overflow-y-auto thin-scrollbar py-1" : "py-[0.1rem]" + } gap-1`, singleValue: () => "leading-7 ml-1", multiValue: () => "bg-mineshaft-600 text-sm rounded items-center py-0.5 px-2 gap-1.5", multiValueLabel: () => "leading-6 text-sm", diff --git a/frontend/src/helpers/members.ts b/frontend/src/helpers/members.ts new file mode 100644 index 000000000..871ef3ec6 --- /dev/null +++ b/frontend/src/helpers/members.ts @@ -0,0 +1,12 @@ +import { TWorkspaceUser } from "@app/hooks/api/users/types"; + +export const getMemberLabel = (member: TWorkspaceUser) => { + const { + inviteEmail, + user: { firstName, lastName, username, email } + } = member; + + return firstName || lastName + ? `${firstName ?? ""} ${lastName ?? ""}`.trim() + : username || email || inviteEmail; +}; diff --git a/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/ApprovalPolicyList.tsx b/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/ApprovalPolicyList.tsx index a3e06459e..de8daec9e 100644 --- a/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/ApprovalPolicyList.tsx +++ b/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/ApprovalPolicyList.tsx @@ -188,8 +188,8 @@ export const ApprovalPolicyList = ({ workspaceId }: IProps) => { Name Environment Secret Path - Eligible Approvers - Eligible Group Approvers + Eligible Approvers + Eligible Group Approvers Approval Required @@ -256,9 +256,9 @@ export const ApprovalPolicyList = ({ workspaceId }: IProps) => { {!!currentWorkspace && filteredPolicies?.map((policy) => (
- ( - - - - )} - /> - ( - - - - )} - /> - ( - - option.slug} - getOptionLabel={(option) => option.name} - /> - - )} - /> - ( - - - - )} - /> - ( - - field.onChange(parseInt(el.target.value, 10))} - /> - - )} - /> - ( - - {field.value === EnforcementLevel.Hard - ? `Hard enforcement requires at least ${approversRequired} approver(s) to approve the request.` - : `At least ${approversRequired} approver(s) must approve the request; however, the requester can bypass approval requirements in emergencies.`} -
- } - > - onChange(val as PolicyType)} + className="w-full border border-mineshaft-500" + > + {Object.values(PolicyType).map((policyType) => { + return ( + + {policyDetails[policyType].name} + + ); + })} + + + )} + /> + ( + - {Object.values(EnforcementLevel).map((level) => { - return ( - - {level} - - ); - })} - - - )} - /> + field.onChange(parseInt(el.target.value, 10))} + /> + + )} + /> + ( + + + + )} + /> + ( + +

+ Determines the level of enforcement for required approvers of a request: +

+

+ Hard enforcement requires at least{" "} + {approversRequired} approver(s) to + approve the request.` +

+

+ Soft enforcement At least{" "} + {approversRequired} approver(s) must + approve the request; however, the requester can bypass approval + requirements in emergencies. +

+ + } + > + + + )} + /> + + ( + + option.slug} + getOptionLabel={(option) => option.name} + /> + + )} + /> + ( + + + + )} + /> +

Approvers

@@ -399,14 +414,7 @@ export const AccessPolicyForm = ({ if (!member) return option.id; - const { - inviteEmail, - user: { firstName, lastName, username, email } - } = member; - - return firstName || lastName - ? `${firstName ?? ""} ${lastName ?? ""}`.trim() - : username || email || inviteEmail; + return getMemberLabel(member); }} value={value} onChange={onChange} diff --git a/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/components/ApprovalPolicyRow.tsx b/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/components/ApprovalPolicyRow.tsx index 4b13029df..5b9a882cd 100644 --- a/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/components/ApprovalPolicyRow.tsx +++ b/frontend/src/views/SecretApprovalPage/components/ApprovalPolicyList/components/ApprovalPolicyRow.tsx @@ -1,5 +1,5 @@ -import { useState } from "react"; -import { faCheckCircle, faEllipsis } from "@fortawesome/free-solid-svg-icons"; +import { useMemo } from "react"; +import { faEllipsis } from "@fortawesome/free-solid-svg-icons"; import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; import { twMerge } from "tailwind-merge"; @@ -8,19 +8,19 @@ import { DropdownMenu, DropdownMenuContent, DropdownMenuItem, - DropdownMenuLabel, DropdownMenuTrigger, - Input, Td, + Tooltip, Tr } from "@app/components/v2"; import { Badge } from "@app/components/v2/Badge"; -import { ProjectPermissionActions, ProjectPermissionSub, useProjectPermission } from "@app/context"; +import { ProjectPermissionActions, ProjectPermissionSub } from "@app/context"; +import { getMemberLabel } from "@app/helpers/members"; import { policyDetails } from "@app/helpers/policies"; -import { useUpdateAccessApprovalPolicy, useUpdateSecretApprovalPolicy } from "@app/hooks/api"; -import { Approver, ApproverType } from "@app/hooks/api/accessApproval/types"; +import { Approver } from "@app/hooks/api/accessApproval/types"; import { TGroupMembership } from "@app/hooks/api/groups/types"; import { EnforcementLevel, PolicyType } from "@app/hooks/api/policies/enums"; +import { ApproverType } from "@app/hooks/api/secretApproval/types"; import { WorkspaceEnv } from "@app/hooks/api/types"; import { TWorkspaceUser } from "@app/hooks/api/users/types"; @@ -35,14 +35,14 @@ interface IPolicy { updatedAt: Date; policyType: PolicyType; enforcementLevel: EnforcementLevel; -}; +} type Props = { policy: IPolicy; members?: TWorkspaceUser[]; groups?: TGroupMembership[]; - projectSlug: string; - workspaceId: string; + // projectSlug: string; + // workspaceId: string; onEdit: () => void; onDelete: () => void; }; @@ -51,175 +51,69 @@ export const ApprovalPolicyRow = ({ policy, members = [], groups = [], - projectSlug, - workspaceId, + // projectSlug, + // workspaceId, onEdit, onDelete }: Props) => { - const [selectedApprovers, setSelectedApprovers] = useState(policy.approvers?.filter((approver) => approver.type === ApproverType.User) || []); - const [selectedGroupApprovers, setSelectedGroupApprovers] = useState(policy.approvers?.filter((approver) => approver.type === ApproverType.Group) || []); - const { mutate: updateAccessApprovalPolicy, isLoading: isAccessApprovalPolicyLoading } = useUpdateAccessApprovalPolicy(); - const { mutate: updateSecretApprovalPolicy, isLoading: isSecretApprovalPolicyLoading } = useUpdateSecretApprovalPolicy(); - const isLoading = isAccessApprovalPolicyLoading || isSecretApprovalPolicyLoading; + // TODO(scott): add back to enable editing from modal? edit modal for policy is fine for now + // const [selectedApprovers, setSelectedApprovers] = useState( + // policy.approvers?.filter((approver) => approver.type === ApproverType.User) || [] + // ); + // const [selectedGroupApprovers, setSelectedGroupApprovers] = useState( + // policy.approvers?.filter((approver) => approver.type === ApproverType.Group) || [] + // ); + // const { mutate: updateAccessApprovalPolicy, isLoading: isAccessApprovalPolicyLoading } = + // useUpdateAccessApprovalPolicy(); + // const { mutate: updateSecretApprovalPolicy, isLoading: isSecretApprovalPolicyLoading } = + // useUpdateSecretApprovalPolicy(); + // const isLoading = isAccessApprovalPolicyLoading || isSecretApprovalPolicyLoading; + // + // const { permission } = useProjectPermission(); - const { permission } = useProjectPermission(); + const labels = useMemo(() => { + const usersInPolicy = policy.approvers + ?.filter((approver) => approver.type === ApproverType.User) + .map((approver) => approver.id); + + const groupsInPolicy = policy.approvers + ?.filter((approver) => approver.type === ApproverType.Group) + .map((approver) => approver.id); + + const memberLabels = usersInPolicy?.length + ? members + .filter((member) => usersInPolicy?.includes(member.user.id)) + .map((member) => getMemberLabel(member)) + .join(", ") + : null; + + const groupLabels = groupsInPolicy?.length + ? groups + .filter(({ group }) => groupsInPolicy?.includes(group.id)) + .map(({ group }) => group.name) + .join(", ") + : null; + + return { + members: memberLabels, + groups: groupLabels + }; + }, [policy, members, groups]); return ( {policy.name} {policy.environment.slug} {policy.secretPath || "*"} - - { - if (!isOpen) { - if (policy.policyType === PolicyType.AccessPolicy) { - updateAccessApprovalPolicy( - { - projectSlug, - id: policy.id, - approvers: selectedApprovers.concat(selectedGroupApprovers), - }, - { - onError: () => { - setSelectedApprovers(policy?.approvers?.filter((approver) => approver.type === ApproverType.User) || []); - } - } - ); - } else { - updateSecretApprovalPolicy( - { - workspaceId, - id: policy.id, - approvers: selectedApprovers.concat(selectedGroupApprovers), - }, - { - onError: () => { - setSelectedApprovers(policy?.approvers?.filter((approver) => approver.type === ApproverType.User) || []); - } - } - ); - } - } else { - setSelectedApprovers(policy?.approvers?.filter((approver) => approver.type === ApproverType.User) || []); - } - }} - > - - - - - - Select members that are allowed to approve changes - - {members?.map(({ user }) => { - const userId = user.id; - const isChecked = selectedApprovers?.filter((el: { id: string, type: ApproverType }) => el.id === userId && el.type === ApproverType.User).length > 0; - return ( - { - evt.preventDefault(); - setSelectedApprovers((state) => - isChecked ? state.filter((el) => el.id !== userId || el.type !== ApproverType.User) : [...state, { id: userId, type: ApproverType.User }] - ); - }} - key={`create-policy-members-${userId}`} - iconPos="right" - icon={isChecked && } - > - {user.username} - - ); - })} - - + + +

{labels.members ?? "-"}

+ - - { - if (!isOpen) { - if (policy.policyType === PolicyType.AccessPolicy) { - updateAccessApprovalPolicy( - { - projectSlug, - id: policy.id, - approvers: selectedApprovers.concat(selectedGroupApprovers), - }, - { - onError: () => { - setSelectedGroupApprovers(policy?.approvers?.filter((approver) => approver.type === ApproverType.Group) || []); - } - }, - ); - } else { - updateSecretApprovalPolicy( - { - workspaceId, - id: policy.id, - approvers: selectedApprovers.concat(selectedGroupApprovers), - }, - { - onError: () => { - setSelectedGroupApprovers(policy?.approvers?.filter((approver) => approver.type === ApproverType.Group) || []); - } - } - ); - } - } else { - setSelectedGroupApprovers(policy?.approvers?.filter((approver) => approver.type === ApproverType.Group) || []); - } - }} - > - - - - - - Select groups that are allowed to approve requests - - {groups && groups.map(({ group }) => { - const { id } = group; - const isChecked = selectedGroupApprovers?.filter((el: { id: string, type: ApproverType }) => el.id === id && el.type === ApproverType.Group).length > 0; - return ( - { - evt.preventDefault(); - setSelectedGroupApprovers( - isChecked - ? selectedGroupApprovers?.filter((el) => el.id !== id || el.type !== ApproverType.Group) - : [...(selectedGroupApprovers || []), { id, type: ApproverType.Group }] - ); - }} - key={`create-policy-groups-${id}`} - iconPos="right" - icon={isChecked && } - > - {group.name} - - ); - })} - - + + +

{labels.groups ?? "-"}

+ {policy.approvals} @@ -229,12 +123,12 @@ export const ApprovalPolicyRow = ({ - -
+ +
- +