From 8d5e7406c34cc2a58c3a2f5dedbd32c1ba6cd0c8 Mon Sep 17 00:00:00 2001 From: Vladyslav Matsiiako Date: Sun, 25 May 2025 15:53:30 -0700 Subject: [PATCH 1/2] improve change requests design --- .../SecretApprovalsPage.tsx | 3 +- .../SecretApprovalRequestAction.tsx | 112 ++++--- .../SecretApprovalRequestChangeItem.tsx | 299 ++++++++---------- .../SecretApprovalRequestChanges.tsx | 67 ++-- 4 files changed, 225 insertions(+), 256 deletions(-) diff --git a/frontend/src/pages/secret-manager/SecretApprovalsPage/SecretApprovalsPage.tsx b/frontend/src/pages/secret-manager/SecretApprovalsPage/SecretApprovalsPage.tsx index 5483dc597..a70a6a901 100644 --- a/frontend/src/pages/secret-manager/SecretApprovalsPage/SecretApprovalsPage.tsx +++ b/frontend/src/pages/secret-manager/SecretApprovalsPage/SecretApprovalsPage.tsx @@ -44,8 +44,7 @@ export const SecretApprovalsPage = () => {
-
- +
+
+
+ +
- {isMergable ? "Good to merge" : "Review required"} - - At least {approvals} approving review required +

{isMergable ? "Good to merge" : "Merging is blocked"}

+ {!isMergable && + At least {approvals} approving review{`${approvals > 1 ? "s" : ""}`} required by eligible reviewers. {Boolean(statusChangeByEmail) && `. Reopened by ${statusChangeByEmail}`} - - {isSoftEnforcement && !isMergable && canBypassApprovalPermission && ( -
- setByPassApproval(checked === true)} - isChecked={byPassApproval} - id="byPassApproval" - checkIndicatorBg="text-white" - className={twMerge( - "mr-2", - byPassApproval ? "border-red bg-red hover:bg-red-600" : "" - )} - > - - Merge without waiting for approval (bypass secret change policy) - - - {byPassApproval && ( - - setBypassReason(e.target.value)} - placeholder="Enter reason for bypass (min 10 chars)" - leftIcon={} - /> - - )} -
- )} + }
-
+
+ {isSoftEnforcement && !isMergable && canBypassApprovalPermission && ( +
+ setByPassApproval(checked === true)} + isChecked={byPassApproval} + id="byPassApproval" + checkIndicatorBg="text-white" + className={twMerge( + "mr-2", + byPassApproval ? "border-red bg-red hover:bg-red-600" : "" + )} + > + + Merge without waiting for approval (bypass secret change policy) + + + {byPassApproval && ( + + setBypassReason(e.target.value)} + placeholder="Enter reason for bypass (min 10 chars)" + leftIcon={} + /> + + )} +
+ )} +
+
{canApprove || isSoftEnforcement ? ( - <> +
@@ -186,7 +194,7 @@ export const SecretApprovalRequestAction = ({ > Merge - +
) : (
Only approvers can merge
)} @@ -197,13 +205,13 @@ export const SecretApprovalRequestAction = ({ if (hasMerged && status === "close") return ( -
-
+
+
- Secret approval merged + Change request merged - Merged by {statusChangeByEmail} + Merged by {statusChangeByEmail}.
diff --git a/frontend/src/pages/secret-manager/SecretApprovalsPage/components/SecretApprovalRequest/components/SecretApprovalRequestChangeItem.tsx b/frontend/src/pages/secret-manager/SecretApprovalsPage/components/SecretApprovalRequest/components/SecretApprovalRequestChangeItem.tsx index 212f929f7..35f6474d5 100644 --- a/frontend/src/pages/secret-manager/SecretApprovalsPage/components/SecretApprovalRequest/components/SecretApprovalRequestChangeItem.tsx +++ b/frontend/src/pages/secret-manager/SecretApprovalsPage/components/SecretApprovalRequest/components/SecretApprovalRequestChangeItem.tsx @@ -1,19 +1,12 @@ -import { faExclamationTriangle, faInfo, faKey } from "@fortawesome/free-solid-svg-icons"; +import { faCircleXmark, faExclamationTriangle, faEye, faEyeSlash, faInfo, faKey } from "@fortawesome/free-solid-svg-icons"; import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; import { - SecretInput, - Table, - TableContainer, Tag, - TBody, - Td, - Th, - THead, - Tooltip, - Tr + Tooltip } from "@app/components/v2"; import { CommitType, SecretV3Raw, TSecretApprovalSecChange, WsTag } from "@app/hooks/api/types"; +import { useState } from "react"; export type Props = { op: CommitType; @@ -29,19 +22,19 @@ export type Props = { const generateItemTitle = (op: CommitType) => { let text = { label: "", color: "" }; - if (op === CommitType.CREATE) text = { label: "create", color: "#16a34a" }; - else if (op === CommitType.UPDATE) text = { label: "change", color: "#ea580c" }; - else text = { label: "deletion", color: "#b91c1c" }; + if (op === CommitType.CREATE) text = { label: "create", color: "#60DD00" }; + else if (op === CommitType.UPDATE) text = { label: "change", color: "#F8EB30" }; + else text = { label: "deletion", color: "#F83030" }; return ( - +
Request for secret {text.label} - +
); }; const generateConflictText = (op: CommitType) => { - if (op === CommitType.CREATE) return
Secret already exist
; + if (op === CommitType.CREATE) return
Secret already exists
; if (op === CommitType.UPDATE) return
Secret not found
; return null; }; @@ -59,10 +52,12 @@ export const SecretApprovalRequestChangeItem = ({ const itemConflict = hasMerged && conflicts.find((el) => el.op === op && el.secretId === newVersion?.id); const hasConflict = Boolean(itemConflict); + const [isOldSecretValueVisible, setIsOldSecretValueVisible] = useState(false); + const [isNewSecretValueVisible, setIsNewSecretValueVisible] = useState(false); return ( -
-
+
+
{generateItemTitle(op)}
{!hasMerged && isStale && (
@@ -79,35 +74,43 @@ export const SecretApprovalRequestChangeItem = ({
)}
- - - - - {op === CommitType.UPDATE && - - - - - - - {op === CommitType.UPDATE ? ( - - - - - - - - - - - - - - - - - - - ) : ( - - - - - - - - - - )} -
} - SecretValueCommentTagsMetadata
OLD{secretVersion?.secretKey} - {newVersion?.isRotatedSecret ? ( - - Rotated Secret value will not be affected - - ) : ( - - )} - {secretVersion?.secretComment} - {secretVersion?.tags?.map(({ slug, id: tagId, color }) => ( +
+
+ {op === CommitType.UPDATE || op === CommitType.DELETE ? ( +
+
+ Legacy Secret +
+ + Deprecated +
+
+
+
Key
+
{secretVersion?.secretKey}
+
+
+
Value
+
{newVersion?.isRotatedSecret ? ( + + Rotated Secret value will not be affected + + ) : ( +
setIsOldSecretValueVisible(!isOldSecretValueVisible)} className="pl-2 border border-mineshaft-500 bg-mineshaft-900 rounded-md flex flex-row justify-between items-center"> +
{isOldSecretValueVisible ? (secretVersion?.secretValue || "EMPTY") : (secretVersion?.secretValue ? secretVersion?.secretValue?.split('').map((_, index) => "•") : "EMPTY")}
+ {secretVersion?.secretValue &&
} +
+ )} +
+
+
+
Comment
+
{secretVersion?.secretComment || -}
+
+
+
Tags
+
+ {secretVersion?.tags?.length ?? 0 ? secretVersion?.tags?.map(({ slug, id: tagId, color }) => (
{slug}
 - ))} -
+ )) : -} + + +
+
Metadata
+
{secretVersion?.secretMetadata?.length ? (
{secretVersion.secretMetadata?.map((el) => ( @@ -146,23 +152,45 @@ export const SecretApprovalRequestChangeItem = ({ ) : (

-

)} -
NEW{newVersion?.secretKey} - {newVersion?.isRotatedSecret ? ( - - Rotated Secret value will not be affected - - ) : ( - - )} - {newVersion?.secretComment} - {newVersion?.tags?.map(({ slug, id: tagId, color }) => ( + + + ) + :
Secret not existent in the previous version.
} + {op === CommitType.UPDATE || op === CommitType.CREATE ? ( +
+
+ New Secret +
+ + Current +
+
+
+
Key
+
{newVersion?.secretKey}
+
+
+
Value
+
{newVersion?.isRotatedSecret ? ( + + Rotated Secret value will not be affected + + ) : ( +
setIsNewSecretValueVisible(!isNewSecretValueVisible)} className="pl-2 border border-mineshaft-500 bg-mineshaft-900 rounded-md flex flex-row justify-between items-center"> +
{isNewSecretValueVisible ? (newVersion?.secretValue || "EMPTY") : (newVersion?.secretValue ? newVersion?.secretValue?.split('').map((_, index) => "•") : "EMPTY")}
+ {newVersion?.secretValue &&
} +
+ )} +
+
+
+
Comment
+
{newVersion?.secretComment || -}
+
+
+
Tags
+
+ {newVersion?.tags?.length ?? 0 ? newVersion?.tags?.map(({ slug, id: tagId, color }) => (
{slug}
 - ))} -
- {newVersion?.secretMetadata?.length ? ( -
- {newVersion.secretMetadata?.map((el) => ( -
- - -
{el.key}
- - -
- {el.value} -
- -
- ))} -
- ) : ( -

-

- )} -
- {op === CommitType.CREATE ? newVersion?.secretKey : secretVersion?.secretKey} - - - - {op === CommitType.CREATE - ? newVersion?.secretComment - : secretVersion?.secretComment} - - {(op === CommitType.CREATE ? newVersion?.tags : secretVersion?.tags)?.map( - ({ slug, id: tagId, color }) => ( - -
-
{slug}
- - ) - )} -
- {newVersion?.secretMetadata?.length ? ( -
- {newVersion.secretMetadata?.map((el) => ( -
- - -
{el.key}
- - -
- {el.value} -
- -
- ))} -
- ) : ( -

-

- )} -
- + )) : -} +
+
+
+
Metadata
+ {newVersion?.secretMetadata?.length ? ( +
+ {newVersion.secretMetadata?.map((el) => ( +
+ + +
{el.key}
+ + +
+ {el.value} +
+ +
+ ))} +
+ ) : ( +

-

+ )} +
+
) + :
Secret not existent in the new version.
} +
+
); }; diff --git a/frontend/src/pages/secret-manager/SecretApprovalsPage/components/SecretApprovalRequest/components/SecretApprovalRequestChanges.tsx b/frontend/src/pages/secret-manager/SecretApprovalsPage/components/SecretApprovalRequest/components/SecretApprovalRequestChanges.tsx index 4eebc65f4..9e611d3dc 100644 --- a/frontend/src/pages/secret-manager/SecretApprovalsPage/components/SecretApprovalRequest/components/SecretApprovalRequestChanges.tsx +++ b/frontend/src/pages/secret-manager/SecretApprovalsPage/components/SecretApprovalRequest/components/SecretApprovalRequestChanges.tsx @@ -40,6 +40,7 @@ import { formatReservedPaths } from "@app/lib/fn/string"; import { SecretApprovalRequestAction } from "./SecretApprovalRequestAction"; import { SecretApprovalRequestChangeItem } from "./SecretApprovalRequestChangeItem"; +import { format } from "date-fns"; export const generateCommitText = (commits: { op: CommitType }[] = []) => { const score: Record = {}; @@ -51,7 +52,7 @@ export const generateCommitText = (commits: { op: CommitType }[] = []) => { text.push( {score[CommitType.CREATE]} secret{score[CommitType.CREATE] !== 1 && "s"} - created + created ); if (score[CommitType.UPDATE]) @@ -59,7 +60,7 @@ export const generateCommitText = (commits: { op: CommitType }[] = []) => { {Boolean(text.length) && ","} {score[CommitType.UPDATE]} secret{score[CommitType.UPDATE] !== 1 && "s"} - + {" "} updated @@ -70,7 +71,7 @@ export const generateCommitText = (commits: { op: CommitType }[] = []) => { {Boolean(text.length) && "and"} {score[CommitType.DELETE]} secret{score[CommitType.UPDATE] !== 1 && "s"} - deleted + deleted ); @@ -221,29 +222,29 @@ export const SecretApprovalRequestChanges = ({
-
+
{generateCommitText(secretApprovalRequestDetails.commits)} {secretApprovalRequestDetails.isReplicated && ( (replication) )}
-
- {secretApprovalRequestDetails?.committerUser?.firstName || ""} - {secretApprovalRequestDetails?.committerUser?.lastName || ""} ( - {secretApprovalRequestDetails?.committerUser?.email}) wants to change{" "} - {secretApprovalRequestDetails.commits.length} secret values in - +
+

+ {secretApprovalRequestDetails?.committerUser?.firstName || ""} + {secretApprovalRequestDetails?.committerUser?.lastName || ""} ( + {secretApprovalRequestDetails?.committerUser?.email}) wants to change{" "} + {secretApprovalRequestDetails.commits.length} secret values in +

+

{secretApprovalRequestDetails.environment} - -

-
+

+
+

-

- -
- {formatReservedPaths(secretApprovalRequestDetails.secretPath)} -
- +

+

+ {formatReservedPaths(secretApprovalRequestDetails.secretPath)} +

@@ -256,7 +257,7 @@ export const SecretApprovalRequestChanges = ({ > @@ -205,7 +210,7 @@ export const SecretApprovalRequestAction = ({ if (hasMerged && status === "close") return ( -
+
diff --git a/frontend/src/pages/secret-manager/SecretApprovalsPage/components/SecretApprovalRequest/components/SecretApprovalRequestChangeItem.tsx b/frontend/src/pages/secret-manager/SecretApprovalsPage/components/SecretApprovalRequest/components/SecretApprovalRequestChangeItem.tsx index 35f6474d5..b7a6a8149 100644 --- a/frontend/src/pages/secret-manager/SecretApprovalsPage/components/SecretApprovalRequest/components/SecretApprovalRequestChangeItem.tsx +++ b/frontend/src/pages/secret-manager/SecretApprovalsPage/components/SecretApprovalRequest/components/SecretApprovalRequestChangeItem.tsx @@ -1,12 +1,19 @@ -import { faCircleXmark, faExclamationTriangle, faEye, faEyeSlash, faInfo, faKey } from "@fortawesome/free-solid-svg-icons"; +/* eslint-disable jsx-a11y/no-static-element-interactions */ +/* eslint-disable jsx-a11y/click-events-have-key-events */ +/* eslint-disable no-nested-ternary */ +import { useState } from "react"; +import { + faCircleXmark, + faExclamationTriangle, + faEye, + faEyeSlash, + faInfo, + faKey +} from "@fortawesome/free-solid-svg-icons"; import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; -import { - Tag, - Tooltip -} from "@app/components/v2"; +import { Tag, Tooltip } from "@app/components/v2"; import { CommitType, SecretV3Raw, TSecretApprovalSecChange, WsTag } from "@app/hooks/api/types"; -import { useState } from "react"; export type Props = { op: CommitType; @@ -27,7 +34,7 @@ const generateItemTitle = (op: CommitType) => { else text = { label: "deletion", color: "#F83030" }; return ( -
+
Request for secret {text.label}
); @@ -56,7 +63,7 @@ export const SecretApprovalRequestChangeItem = ({ const [isNewSecretValueVisible, setIsNewSecretValueVisible] = useState(false); return ( -
+
{generateItemTitle(op)}
{!hasMerged && isStale && ( @@ -75,57 +82,84 @@ export const SecretApprovalRequestChangeItem = ({ )}
-
+
{op === CommitType.UPDATE || op === CommitType.DELETE ? ( -
-
+
+
Legacy Secret -
- +
+ Deprecated
-
-
Key
+
+
Key
{secretVersion?.secretKey}
-
-
Value
-
{newVersion?.isRotatedSecret ? ( - - Rotated Secret value will not be affected - - ) : ( -
setIsOldSecretValueVisible(!isOldSecretValueVisible)} className="pl-2 border border-mineshaft-500 bg-mineshaft-900 rounded-md flex flex-row justify-between items-center"> -
{isOldSecretValueVisible ? (secretVersion?.secretValue || "EMPTY") : (secretVersion?.secretValue ? secretVersion?.secretValue?.split('').map((_, index) => "•") : "EMPTY")}
- {secretVersion?.secretValue &&
} -
- )} -
-
-
-
Comment
-
{secretVersion?.secretComment || -}
-
-
-
Tags
-
- {secretVersion?.tags?.length ?? 0 ? secretVersion?.tags?.map(({ slug, id: tagId, color }) => ( - +
Value
+
+ {newVersion?.isRotatedSecret ? ( + + Rotated Secret value will not be affected + + ) : ( +
setIsOldSecretValueVisible(!isOldSecretValueVisible)} + className="flex flex-row items-center justify-between rounded-md border border-mineshaft-500 bg-mineshaft-900 pl-2" >
-
{slug}
- - )) : -} + className={`flex font-mono ${isOldSecretValueVisible || !secretVersion?.secretValue ? "text-md py-[0.55rem]" : "text-lg"}`} + > + {isOldSecretValueVisible + ? secretVersion?.secretValue || "EMPTY" + : secretVersion?.secretValue + ? secretVersion?.secretValue?.split("").map(() => "•") + : "EMPTY"}{" "} +
+ {secretVersion?.secretValue && ( +
+ +
+ )} +
+ )}
-
-
Metadata
+
+
Comment
+
+ {secretVersion?.secretComment || ( + - + )}{" "} +
+
+
+
Tags
+
+ {(secretVersion?.tags?.length ?? 0) ? ( + secretVersion?.tags?.map(({ slug, id: tagId, color }) => ( + +
+
{slug}
+ + )) + ) : ( + - + )} +
+
+
+
Metadata
{secretVersion?.secretMetadata?.length ? (
@@ -153,59 +187,91 @@ export const SecretApprovalRequestChangeItem = ({

-

)}
-
-
) - :
Secret not existent in the previous version.
} - {op === CommitType.UPDATE || op === CommitType.CREATE ? ( -
-
+
+
+ ) : ( +
+ {" "} + Secret not existent in the previous version. +
+ )} + {op === CommitType.UPDATE || op === CommitType.CREATE ? ( +
+
New Secret -
- +
+ Current
-
-
Key
+
+
Key
{newVersion?.secretKey}
-
-
Value
-
{newVersion?.isRotatedSecret ? ( - - Rotated Secret value will not be affected - - ) : ( -
setIsNewSecretValueVisible(!isNewSecretValueVisible)} className="pl-2 border border-mineshaft-500 bg-mineshaft-900 rounded-md flex flex-row justify-between items-center"> -
{isNewSecretValueVisible ? (newVersion?.secretValue || "EMPTY") : (newVersion?.secretValue ? newVersion?.secretValue?.split('').map((_, index) => "•") : "EMPTY")}
- {newVersion?.secretValue &&
} -
- )} -
-
-
-
Comment
-
{newVersion?.secretComment || -}
-
-
-
Tags
-
- {newVersion?.tags?.length ?? 0 ? newVersion?.tags?.map(({ slug, id: tagId, color }) => ( - +
Value
+
+ {newVersion?.isRotatedSecret ? ( + + Rotated Secret value will not be affected + + ) : ( +
setIsNewSecretValueVisible(!isNewSecretValueVisible)} + className="flex flex-row items-center justify-between rounded-md border border-mineshaft-500 bg-mineshaft-900 pl-2" >
-
{slug}
- - )) : -} + className={`flex font-mono ${isNewSecretValueVisible || !newVersion?.secretValue ? "text-md py-[0.55rem]" : "text-lg"}`} + > + {isNewSecretValueVisible + ? newVersion?.secretValue || "EMPTY" + : newVersion?.secretValue + ? newVersion?.secretValue?.split("").map(() => "•") + : "EMPTY"}{" "} +
+ {newVersion?.secretValue && ( +
+ +
+ )} +
+ )}
-
-
Metadata
+
+
Comment
+
+ {newVersion?.secretComment || ( + - + )}{" "} +
+
+
+
Tags
+
+ {(newVersion?.tags?.length ?? 0) ? ( + newVersion?.tags?.map(({ slug, id: tagId, color }) => ( + +
+
{slug}
+ + )) + ) : ( + - + )} +
+
+
+
Metadata
{newVersion?.secretMetadata?.length ? (
{newVersion.secretMetadata?.map((el) => ( @@ -232,9 +298,14 @@ export const SecretApprovalRequestChangeItem = ({

-

)}
-
) - :
Secret not existent in the new version.
} -
+
+ ) : ( +
+ {" "} + Secret not existent in the new version. +
+ )} +
); diff --git a/frontend/src/pages/secret-manager/SecretApprovalsPage/components/SecretApprovalRequest/components/SecretApprovalRequestChanges.tsx b/frontend/src/pages/secret-manager/SecretApprovalsPage/components/SecretApprovalRequest/components/SecretApprovalRequestChanges.tsx index 9e611d3dc..17046450e 100644 --- a/frontend/src/pages/secret-manager/SecretApprovalsPage/components/SecretApprovalRequest/components/SecretApprovalRequestChanges.tsx +++ b/frontend/src/pages/secret-manager/SecretApprovalsPage/components/SecretApprovalRequest/components/SecretApprovalRequestChanges.tsx @@ -13,6 +13,7 @@ import { import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; import { zodResolver } from "@hookform/resolvers/zod"; import { RadioGroup, RadioGroupIndicator, RadioGroupItem } from "@radix-ui/react-radio-group"; +import { format } from "date-fns"; import { twMerge } from "tailwind-merge"; import z from "zod"; @@ -40,7 +41,6 @@ import { formatReservedPaths } from "@app/lib/fn/string"; import { SecretApprovalRequestAction } from "./SecretApprovalRequestAction"; import { SecretApprovalRequestChangeItem } from "./SecretApprovalRequestChangeItem"; -import { format } from "date-fns"; export const generateCommitText = (commits: { op: CommitType }[] = []) => { const score: Record = {}; @@ -235,14 +235,17 @@ export const SecretApprovalRequestChanges = ({ {secretApprovalRequestDetails?.committerUser?.email}) wants to change{" "} {secretApprovalRequestDetails.commits.length} secret values in

-

+

{secretApprovalRequestDetails.environment}

-

+

-

+

{formatReservedPaths(secretApprovalRequestDetails.secretPath)}

@@ -256,10 +259,7 @@ export const SecretApprovalRequestChanges = ({ onOpenChange={(isOpen) => handlePopUpToggle("reviewChanges", isOpen)} > - @@ -397,7 +397,8 @@ export const SecretApprovalRequestChanges = ({ > {reviewer?.status === ApprovalStatus.APPROVED ? "approved" : "rejected"} {" "} - the request on {format(new Date(secretApprovalRequestDetails.createdAt), "PPpp zzz")}. + the request on{" "} + {format(new Date(secretApprovalRequestDetails.createdAt), "PPpp zzz")}.
{reviewer?.comment && ( @@ -410,7 +411,7 @@ export const SecretApprovalRequestChanges = ({ ); })}
-
+
-
+
Reviewers
{secretApprovalRequestDetails?.policy?.approvers @@ -436,7 +437,7 @@ export const SecretApprovalRequestChanges = ({ const reviewer = reviewedUsers?.[requiredApprover.userId]; return (