From a6a9c2404dc970a8dd1734ce10d1bb80a2306c25 Mon Sep 17 00:00:00 2001 From: = Date: Fri, 20 Jun 2025 00:12:49 +0530 Subject: [PATCH] feat: patched up approval sequence ui bugs --- .../src/hooks/api/accessApproval/types.ts | 2 +- .../components/ReviewAccessModal.tsx | 2 +- .../ApprovalPolicyList/ApprovalPolicyList.tsx | 31 +- .../components/AccessPolicyModal.tsx | 879 +++++++++--------- .../components/ApprovalPolicyRow.tsx | 4 +- 5 files changed, 457 insertions(+), 461 deletions(-) diff --git a/frontend/src/hooks/api/accessApproval/types.ts b/frontend/src/hooks/api/accessApproval/types.ts index 40725a16c..b0080ce8d 100644 --- a/frontend/src/hooks/api/accessApproval/types.ts +++ b/frontend/src/hooks/api/accessApproval/types.ts @@ -35,7 +35,7 @@ export type Approver = { id: string; type: ApproverType; sequence?: number; - approvals?: number; + approvalsRequired?: number; }; export type Bypasser = { diff --git a/frontend/src/pages/secret-manager/SecretApprovalsPage/components/AccessApprovalRequest/components/ReviewAccessModal.tsx b/frontend/src/pages/secret-manager/SecretApprovalsPage/components/AccessApprovalRequest/components/ReviewAccessModal.tsx index e6df8d814..4cea61cbe 100644 --- a/frontend/src/pages/secret-manager/SecretApprovalsPage/components/AccessApprovalRequest/components/ReviewAccessModal.tsx +++ b/frontend/src/pages/secret-manager/SecretApprovalsPage/components/AccessApprovalRequest/components/ReviewAccessModal.tsx @@ -185,7 +185,7 @@ export const ReviewAccessRequestModal = ({ return acc; } - const approvals = curr.approvals || policy.approvals; + const approvals = curr.approvalsRequired || policy.approvals; const sequence = curr.sequence || 1; acc.push( diff --git a/frontend/src/pages/secret-manager/SecretApprovalsPage/components/ApprovalPolicyList/ApprovalPolicyList.tsx b/frontend/src/pages/secret-manager/SecretApprovalsPage/components/ApprovalPolicyList/ApprovalPolicyList.tsx index 3d1ba70ef..5dc89fe20 100644 --- a/frontend/src/pages/secret-manager/SecretApprovalsPage/components/ApprovalPolicyList/ApprovalPolicyList.tsx +++ b/frontend/src/pages/secret-manager/SecretApprovalsPage/components/ApprovalPolicyList/ApprovalPolicyList.tsx @@ -19,6 +19,8 @@ import { DropdownMenuLabel, DropdownMenuTrigger, EmptyState, + Modal, + ModalContent, Table, TableContainer, TableSkeleton, @@ -264,14 +266,29 @@ export const ApprovalPolicyList = ({ workspaceId }: IProps) => { - handlePopUpToggle("policyForm", isOpen)} - members={members} - editValues={popUp.policyForm.data as TAccessApprovalPolicy} - /> + onOpenChange={(isOpen) => handlePopUpToggle("policyForm", isOpen)} + > + + handlePopUpToggle("policyForm", isOpen)} + members={members} + editValues={popUp.policyForm.data as TAccessApprovalPolicy} + /> + + { const [draggedItem, setDraggedItem] = useState(null); const [dragOverItem, setDragOverItem] = useState(null); - const modalContainer = useRef(null); const { control, handleSubmit, @@ -153,34 +150,34 @@ export const AccessPolicyForm = ({ .map(({ id, type }) => ({ id, type: type as BypasserType.Group })) || [], approvals: editValues?.approvals, allowedSelfApprovals: editValues?.allowedSelfApprovals, - sequenceApprovers: editValues.approvers - ?.sort((a, b) => (a?.sequence || 0) - (b?.sequence || 0)) - .reduce( - (acc, curr) => { - if (acc.length && acc[acc.length - 1].sequence === curr.sequence) { - acc[acc.length - 1][curr.type]?.push(curr); - return acc; - } - const approvals = curr.approvals || editValues.approvals; - acc.push( - curr.type === ApproverType.User - ? { - user: [curr], - group: [], - sequence: 1, - approvals - } - : { group: [curr], user: [], sequence: 1, approvals } - ); + sequenceApprovers: editValues.approvers?.reduce( + (acc, curr) => { + if (acc.length && acc[acc.length - 1].sequence === curr.sequence) { + acc[acc.length - 1][curr.type]?.push(curr); return acc; - }, - [] as { user: Approver[]; group: Approver[]; sequence?: number; approvals: number }[] - ) + } + const approvals = curr.approvalsRequired || editValues.approvals; + acc.push( + curr.type === ApproverType.User + ? { + user: [curr], + group: [], + sequence: 1, + approvals + } + : { group: [curr], user: [], sequence: 1, approvals } + ); + return acc; + }, + [] as { user: Approver[]; group: Approver[]; sequence?: number; approvals: number }[] + ) } as TFormSchema) : undefined, - defaultValues: { - sequenceApprovers: [{ approvals: 1 }] - } + defaultValues: !editValues + ? { + sequenceApprovers: [{ approvals: 1 }] + } + : undefined }); const sequenceApproversFieldArray = useFieldArray({ control, @@ -204,7 +201,6 @@ export const AccessPolicyForm = ({ const { mutateAsync: createSecretApprovalPolicy } = useCreateSecretApprovalPolicy(); const { mutateAsync: updateSecretApprovalPolicy } = useUpdateSecretApprovalPolicy(); - const policyName = policyDetails[watch("policyType")]?.name || "Policy"; const enforcementLevel = watch("enforcementLevel"); const formUserBypassers = watch("userBypassers"); @@ -391,446 +387,429 @@ export const AccessPolicyForm = ({ setDraggedItem(null); setDragOverItem(null); }; - return ( - - -
-
-
- ( - - - - )} - /> - {!isAccessPolicyType && ( - ( - - field.onChange(parseInt(el.target.value, 10))} - /> - - )} - /> - )} -
-
- ( - - - - )} - /> - ( - - - - )} - /> -
+
+ +
+ ( + + + + )} + /> + {!isAccessPolicyType && ( ( + name="approvals" + defaultValue={1} + render={({ field, fieldState: { error } }) => ( + field.onChange(parseInt(el.target.value, 10))} + /> + + )} + /> + )} +
+
+ ( + + + + )} + /> + ( + + + + )} + /> +
+ ( + + option.slug} + getOptionLabel={(option) => option.name} + /> + + )} + /> +
+

Approvers

+

+ Select members or groups that are allowed to approve requests from this policy. +

+
+ {isAccessPolicyType ? ( + <> +
+ {sequenceApproversFieldArray.fields.map((el, index) => ( +
handleDragOver(e, index)} + onDrop={handleDrop} + > +
+ Step {index + 1} +
+
Min. Approvals
+
+ ( + field.onChange(parseInt(val.target.value, 10))} + /> + )} + /> +
+ + sequenceApproversFieldArray.remove(index)} + className="text-red-500 hover:text-gray-200" + > + + + + +
handleDragStart(e, index)} + onDragEnd={handleDragEnd} + className="mr-2 cursor-move text-gray-400 hover:text-gray-200" + > + +
+
+
+
+
+ ( + + option.id} + getOptionLabel={(option) => { + const member = members?.find((m) => m.user.id === option.id); + + if (!member) return option.id; + + return getMemberLabel(member); + }} + value={value} + onChange={onChange} + /> + + )} + /> + ( + + option.id} + getOptionLabel={(option) => + groups?.find(({ group }) => group.id === option.id)?.group.name ?? + option.id + } + value={value} + onChange={onChange} + /> + + )} + /> +
+
+ ))} +
+
+ +
+ + ) : ( +
+ ( + option.id} + getOptionLabel={(option) => { + const member = members?.find((m) => m.user.id === option.id); + + if (!member) return option.id; + + return getMemberLabel(member); + }} value={value} onChange={onChange} - placeholder="Select environment..." - options={environments} - getOptionValue={(option) => option.slug} - getOptionLabel={(option) => option.name} /> )} /> -
-

Approvers

-

- Select members or groups that are allowed to approve requests from this policy. -

-
- {isAccessPolicyType ? ( - <> -
- {sequenceApproversFieldArray.fields.map((el, index) => ( -
handleDragOver(e, index)} - onDrop={handleDrop} - > -
- Step {index + 1} -
-
Min. Approvals
-
- ( - field.onChange(parseInt(val.target.value, 10))} - /> - )} - /> -
- - sequenceApproversFieldArray.remove(index)} - className="text-red-500 hover:text-gray-200" - > - - - - -
handleDragStart(e, index)} - onDragEnd={handleDragEnd} - className="mr-2 cursor-move text-gray-400 hover:text-gray-200" - > - -
-
-
-
-
- ( - - option.id} - getOptionLabel={(option) => { - const member = members?.find((m) => m.user.id === option.id); - - if (!member) return option.id; - - return getMemberLabel(member); - }} - value={value} - onChange={onChange} - /> - - )} - /> - ( - - option.id} - getOptionLabel={(option) => - groups?.find(({ group }) => group.id === option.id)?.group.name ?? - option.id - } - value={value} - onChange={onChange} - /> - - )} - /> -
-
- ))} -
-
-
+ )} + ( + + + Allow approvers to review their own requests + + + )} + /> + ( + + onChange(v ? EnforcementLevel.Soft : EnforcementLevel.Hard)} + > + Allow certain users to bypass policy in break-glass situations + + + )} + /> + {enforcementLevel === EnforcementLevel.Soft && ( + <> +
+ ( + - Add Step - -
- - ) : ( -
- ( - - option.id} - getOptionLabel={(option) => { - const member = members?.find((m) => m.user.id === option.id); + option.id} + getOptionLabel={(option) => { + const member = members?.find((m) => m.user.id === option.id); - if (!member) return option.id; + if (!member) return option.id; - return getMemberLabel(member); - }} - value={value} - onChange={onChange} - /> - - )} - /> - ( - - option.id} - getOptionLabel={(option) => - groups?.find(({ group }) => group.id === option.id)?.group.name ?? - option.id - } - value={value} - onChange={onChange} - /> - - )} - /> + return getMemberLabel(member); + }} + value={value} + onChange={onChange} + /> + + )} + /> + ( + + option.id} + getOptionLabel={(option) => + groups?.find(({ group }) => group.id === option.id)?.group.name ?? option.id + } + value={value} + onChange={onChange} + /> + + )} + /> +
+ + {bypasserCount <= 0 && ( +
+ Not selecting specific users or groups will allow anyone to bypass this policy.
)} - ( - - - Allow approvers to review their own requests - - - )} - /> - ( - - - onChange(v ? EnforcementLevel.Soft : EnforcementLevel.Hard) - } - > - Allow certain users to bypass policy in break-glass situations - - - )} - /> - {enforcementLevel === EnforcementLevel.Soft && ( - <> -
- ( - - option.id} - getOptionLabel={(option) => { - const member = members?.find((m) => m.user.id === option.id); - - if (!member) return option.id; - - return getMemberLabel(member); - }} - value={value} - onChange={onChange} - /> - - )} - /> - ( - - option.id} - getOptionLabel={(option) => - groups?.find(({ group }) => group.id === option.id)?.group.name ?? - option.id - } - value={value} - onChange={onChange} - /> - - )} - /> -
- - {bypasserCount <= 0 && ( -
- Not selecting specific users or groups will allow anyone to bypass this policy. -
- )} - - )} -
- - -
- + + )} +
+ +
- - + +
); }; diff --git a/frontend/src/pages/secret-manager/SecretApprovalsPage/components/ApprovalPolicyList/components/ApprovalPolicyRow.tsx b/frontend/src/pages/secret-manager/SecretApprovalsPage/components/ApprovalPolicyList/components/ApprovalPolicyRow.tsx index 9b14af5ba..7bb42de1c 100644 --- a/frontend/src/pages/secret-manager/SecretApprovalsPage/components/ApprovalPolicyList/components/ApprovalPolicyRow.tsx +++ b/frontend/src/pages/secret-manager/SecretApprovalsPage/components/ApprovalPolicyList/components/ApprovalPolicyRow.tsx @@ -59,11 +59,11 @@ export const ApprovalPolicyRow = ({ const sortedSteps = policy.approvers?.sort((a, b) => (a?.sequence || 0) - (b?.sequence || 0)); const entityInSameSequence = sortedSteps?.reduce( (acc, curr) => { - if (acc.length && acc[acc.length - 1].sequence === curr.sequence) { + if (acc.length && acc[acc.length - 1].sequence === (curr.sequence || 1)) { acc[acc.length - 1][curr.type]?.push(curr); return acc; } - const approvals = curr.approvals || policy.approvals; + const approvals = curr.approvalsRequired || policy.approvals; acc.push( curr.type === ApproverType.User ? { user: [curr], group: [], sequence: 1, approvals }