feat(secret-approval): resolved infinite query bug and added support for closing, re-opening request, stale req ui

2026-09-22 13:39:35 +00:00 · 2023-10-03 16:36:44 +05:30
parent 63588b4e44
commit 04c7d49477
15 changed files with 500 additions and 233 deletions
--- a/backend/src/controllers/v1/secretApprovalRequestsController.ts
+++ b/backend/src/controllers/v1/secretApprovalRequestsController.ts
@@ -35,7 +35,7 @@ export const getSecretApprovalRequests = async (req: Request, res: Response) =>
    .skip(offset)
    .populate("policy")
    .lean();
-  if (!approvalRequests.length) return res.send({ requests: [] });
+  if (!approvalRequests.length) return res.send({ approvals: [] });

  const unqiueEnvs = environment ?? {
    $in: [...new Set(approvalRequests.map(({ environment }) => environment))]
@@ -64,7 +64,7 @@ export const getSecretApprovalRequestDetails = async (req: Request, res: Respons
    params: { id }
  } = await validateRequest(reqValidator.getSecretApprovalRequestDetails, req);
  const secretApprovalRequest = await SecretApprovalRequest.findById(id)
-    .populate("policy")
+    .populate<{ policy: ISecretApprovalPolicy }>("policy")
    .populate({
      path: "commits.secretVersion",
      populate: {
@@ -84,7 +84,9 @@ export const getSecretApprovalRequestDetails = async (req: Request, res: Respons
  if (
    membership.role !== "admin" &&
    secretApprovalRequest.committer !== membership.id &&
-    secretApprovalRequest.reviewers.find(({ member }) => member === membership.id)
+    !secretApprovalRequest.policy.approvers.find(
+      (approverId) => approverId.toString() === membership._id.toString()
+    )
  ) {
    throw UnauthorizedRequestError({ message: "User has no access" });
  }
@@ -94,11 +96,11 @@ export const getSecretApprovalRequestDetails = async (req: Request, res: Respons
  });
 };

-export const updateSecretApprovalRequestStatus = async (req: Request, res: Response) => {
+export const updateSecretApprovalReviewStatus = async (req: Request, res: Response) => {
  const {
    body: { status },
    params: { id }
-  } = await validateRequest(reqValidator.updateSecretApprovalRequestStatus, req);
+  } = await validateRequest(reqValidator.updateSecretApprovalReviewStatus, req);
  const secretApprovalRequest = await SecretApprovalRequest.findById(id).populate<{
    policy: ISecretApprovalPolicy;
  }>("policy");
@@ -132,7 +134,7 @@ export const updateSecretApprovalRequestStatus = async (req: Request, res: Respo

 export const mergeSecretApprovalRequest = async (req: Request, res: Response) => {
  const {
-    body: { id }
+    params: { id }
  } = await validateRequest(reqValidator.mergeSecretApprovalRequest, req);

  const secretApprovalRequest = await SecretApprovalRequest.findById(id).populate<{
@@ -149,7 +151,7 @@ export const mergeSecretApprovalRequest = async (req: Request, res: Response) =>
  if (
    membership.role !== "admin" &&
    secretApprovalRequest.committer !== membership.id &&
-    !secretApprovalRequest.policy.approvers.find((approverId) => approverId === membership.id)
+    !secretApprovalRequest.policy.approvers.find((approverId) => approverId.equals(membership.id))
  ) {
    throw UnauthorizedRequestError({ message: "User has no access" });
  }
@@ -166,6 +168,51 @@ export const mergeSecretApprovalRequest = async (req: Request, res: Response) =>

  if (!hasMinApproval) throw BadRequestError({ message: "Doesn't have minimum approvals needed" });

-  const approval = await performSecretApprovalRequestMerge(id, req.authData);
+  const approval = await performSecretApprovalRequestMerge(
+    id,
+    req.authData,
+    membership._id.toString()
+  );
  return res.send({ approval });
 };
+
+export const updateSecretApprovalRequestStatus = async (req: Request, res: Response) => {
+  const {
+    body: { status },
+    params: { id }
+  } = await validateRequest(reqValidator.updateSecretApprovalRequestStatus, req);
+
+  const secretApprovalRequest = await SecretApprovalRequest.findById(id).populate<{
+    policy: ISecretApprovalPolicy;
+  }>("policy");
+
+  if (!secretApprovalRequest)
+    throw BadRequestError({ message: "Secret approval request not found" });
+
+  const { membership } = await getUserProjectPermissions(
+    req.user._id,
+    secretApprovalRequest.workspace.toString()
+  );
+
+  if (
+    membership.role !== "admin" &&
+    secretApprovalRequest.committer !== membership.id &&
+    !secretApprovalRequest.policy.approvers.find((approverId) => approverId.equals(membership._id))
+  ) {
+    throw UnauthorizedRequestError({ message: "User has no access" });
+  }
+
+  if (secretApprovalRequest.hasMerged)
+    throw BadRequestError({ message: "Approval request has been merged" });
+  if (secretApprovalRequest.status === "close" && status === "close")
+    throw BadRequestError({ message: "Approval request is already closed" });
+  if (secretApprovalRequest.status === "open" && status === "open")
+    throw BadRequestError({ message: "Approval request is already open" });
+
+  const updatedRequest = await SecretApprovalRequest.findByIdAndUpdate(
+    id,
+    { status, statusChangeBy: membership._id },
+    { new: true }
+  );
+  return res.send({ approval: updatedRequest });
+};
--- a/backend/src/models/secretApprovalRequest.ts
+++ b/backend/src/models/secretApprovalRequest.ts
@@ -54,6 +54,7 @@ export type ISecretCommits<T = Types.ObjectId, J = Types.ObjectId> = Array<
 export interface ISecretApprovalRequest {
  _id: Types.ObjectId;
  committer: Types.ObjectId;
+  statusChangeBy: Types.ObjectId;
  reviewers: {
    member: Types.ObjectId;
    status: ApprovalStatus;
@@ -159,6 +160,7 @@ const secretApprovalRequestSchema = new Schema<ISecretApprovalRequest>(
    hasMerged: { type: Boolean, default: false },
    status: { type: String, enum: ["close", "open"], default: "open" },
    committer: { type: Schema.Types.ObjectId, ref: "Membership" },
+    statusChangeBy: { type: Schema.Types.ObjectId, ref: "Membership" },
    commits: [
      {
        secret: { type: Types.ObjectId, ref: "Secret" },
--- a/backend/src/routes/v1/secretApprovalRequest.ts
+++ b/backend/src/routes/v1/secretApprovalRequest.ts
@@ -21,7 +21,7 @@ router.get(
 );

 router.post(
-  "/merge",
+  "/:id/merge",
  requireAuth({
    acceptedAuthModes: [AuthMode.JWT]
  }),
@@ -29,7 +29,15 @@ router.post(
 );

 router.post(
-  "/:id",
+  "/:id/review",
+  requireAuth({
+    acceptedAuthModes: [AuthMode.JWT]
+  }),
+  secretApprovalRequestController.updateSecretApprovalReviewStatus
+);
+
+router.post(
+  "/:id/status",
  requireAuth({
    acceptedAuthModes: [AuthMode.JWT]
  }),
--- a/backend/src/services/SecretApprovalService.ts
+++ b/backend/src/services/SecretApprovalService.ts
@@ -317,7 +317,11 @@ export const generateSecretApprovalRequest = async ({
 };

 // validation for a merge conditions happen in another function in controller
-export const performSecretApprovalRequestMerge = async (id: string, authData: AuthData) => {
+export const performSecretApprovalRequestMerge = async (
+  id: string,
+  authData: AuthData,
+  userMembershipId: string
+) => {
  const secretApprovalRequest = await SecretApprovalRequest.findById(id)
    .populate<{ commits: ISecretCommits<ISecret> }>({
      path: "commits.secret",
@@ -636,7 +640,8 @@ export const performSecretApprovalRequestMerge = async (id: string, authData: Au
    {
      conflicts,
      hasMerged: true,
-      status: "close"
+      status: "close",
+      statusChangeBy: userMembershipId
    },
    { new: true }
  );
--- a/backend/src/validation/secretApprovalRequest.ts
+++ b/backend/src/validation/secretApprovalRequest.ts
@@ -18,7 +18,7 @@ export const getSecretApprovalRequestDetails = z.object({
  })
 });

-export const updateSecretApprovalRequestStatus = z.object({
+export const updateSecretApprovalReviewStatus = z.object({
  body: z.object({
    status: z.enum([ApprovalStatus.APPROVED, ApprovalStatus.REJECTED])
  }),
@@ -28,7 +28,16 @@ export const updateSecretApprovalRequestStatus = z.object({
 });

 export const mergeSecretApprovalRequest = z.object({
-  body: z.object({
+  params: z.object({
    id: z.string().trim()
  })
 });
+
+export const updateSecretApprovalRequestStatus = z.object({
+  params: z.object({
+    id: z.string().trim()
+  }),
+  body: z.object({
+    status: z.enum(["open", "close"])
+  })
+});
--- a/frontend/src/components/v2/Button/Button.tsx
+++ b/frontend/src/components/v2/Button/Button.tsx
@@ -11,6 +11,9 @@ type Props = {
  isLoading?: boolean;
 };

+// refactor(akhilmhdh): both color and size variants are together need to split it
+// colorSchema should handle all color class names
+// variant should handle how the button padding and other  types should be set
 const buttonVariants = cva(
  [
    "button",
@@ -106,6 +109,12 @@ const buttonVariants = cva(
        variant: "outline",
        className: "text-red hover:bg-red hover:text-black"
      },
+      {
+        colorSchema: "danger",
+        variant: "outline_bg",
+        className:
+          "bg-mineshaft-600 border border-red-500 hover:bg-red/[0.1] hover:border-red/40 text-red-500"
+      },
      {
        colorSchema: "primary",
        variant: "plain",
--- a/frontend/src/hooks/api/secretApprovalRequest/index.tsx
+++ b/frontend/src/hooks/api/secretApprovalRequest/index.tsx
@@ -1,5 +1,6 @@
 export {
  usePerformSecretApprovalRequestMerge,
-  useUpdateSecretApprovalRequestStatus
+  useUpdateSecretApprovalRequestStatus,
+  useUpdateSecretApprovalReviewStatus
 } from "./mutation";
 export { useGetSecretApprovalRequestDetails, useGetSecretApprovalRequests } from "./queries";
--- a/frontend/src/hooks/api/secretApprovalRequest/mutation.tsx
+++ b/frontend/src/hooks/api/secretApprovalRequest/mutation.tsx
@@ -3,14 +3,36 @@ import { useMutation, useQueryClient } from "@tanstack/react-query";
 import { apiRequest } from "@app/config/request";

 import { secretApprovalRequestKeys } from "./queries";
-import { TPerformSecretApprovalRequestMerge, TUpdateSecretApprovalRequestStatusDTO } from "./types";
+import {
+  TPerformSecretApprovalRequestMerge,
+  TUpdateSecretApprovalRequestStatusDTO,
+  TUpdateSecretApprovalReviewStatusDTO
+} from "./types";
+
+export const useUpdateSecretApprovalReviewStatus = () => {
+  const queryClient = useQueryClient();
+
+  return useMutation<{}, {}, TUpdateSecretApprovalReviewStatusDTO>({
+    mutationFn: async ({ id, status }) => {
+      const { data } = await apiRequest.post(`/api/v1/secret-approval-requests/${id}/review`, {
+        status
+      });
+      return data;
+    },
+    onSuccess: (_, { id }) => {
+      queryClient.invalidateQueries(secretApprovalRequestKeys.detail({ id }));
+    }
+  });
+};

 export const useUpdateSecretApprovalRequestStatus = () => {
  const queryClient = useQueryClient();

  return useMutation<{}, {}, TUpdateSecretApprovalRequestStatusDTO>({
    mutationFn: async ({ id, status }) => {
-      const { data } = await apiRequest.post(`/api/v1/secret-approval-requests/${id}`, { status });
+      const { data } = await apiRequest.post(`/api/v1/secret-approval-requests/${id}/status`, {
+        status
+      });
      return data;
    },
    onSuccess: (_, { id }) => {
@@ -24,11 +46,12 @@ export const usePerformSecretApprovalRequestMerge = () => {

  return useMutation<{}, {}, TPerformSecretApprovalRequestMerge>({
    mutationFn: async ({ id }) => {
-      const { data } = await apiRequest.post("/api/v1/secret-approval-requests/merge", { id });
+      const { data } = await apiRequest.post(`/api/v1/secret-approval-requests/${id}/merge`);
      return data;
    },
-    onSuccess: (_, { id }) => {
+    onSuccess: (_, { id, workspaceId }) => {
      queryClient.invalidateQueries(secretApprovalRequestKeys.detail({ id }));
+      queryClient.invalidateQueries(secretApprovalRequestKeys.list({ workspaceId }));
    }
  });
 };
--- a/frontend/src/hooks/api/secretApprovalRequest/types.ts
+++ b/frontend/src/hooks/api/secretApprovalRequest/types.ts
@@ -58,6 +58,7 @@ export type TSecretApprovalRequest<
  hasMerged: boolean;
  status: "open" | "close";
  policy: TSecretApprovalPolicy;
+  statusChangeBy: string;
  commits: {
    // if there is no secret means it was creation
    secret?: { version: number };
@@ -82,11 +83,17 @@ export type TGetSecretApprovalRequestDetails = {
  decryptKey: UserWsKeyPair;
 };

-export type TUpdateSecretApprovalRequestStatusDTO = {
+export type TUpdateSecretApprovalReviewStatusDTO = {
  status: ApprovalStatus;
  id: string;
 };

+export type TUpdateSecretApprovalRequestStatusDTO = {
+  status: "open" | "close";
+  id: string;
+};
+
 export type TPerformSecretApprovalRequestMerge = {
  id: string;
+  workspaceId: string;
 };
--- a/frontend/src/layouts/AppLayout/AppLayout.tsx
+++ b/frontend/src/layouts/AppLayout/AppLayout.tsx
@@ -482,11 +482,11 @@ export const AppLayout = ({ children }: LayoutProps) => {
                          <a>
                            <MenuItem
                              isSelected={
-                                router.asPath === `/project/${currentWorkspace?._id}/allowlist`
+                                router.asPath === `/project/${currentWorkspace?._id}/approval`
                              }
                              icon="system-outline-126-verified"
                            >
-                              Admin Panel
+                              Secret Approval
                            </MenuItem>
                          </a>
                        </Link>
--- a/frontend/src/views/SecretApprovalPage/components/SecretApprovalRequest/SecretApprovalRequest.tsx
+++ b/frontend/src/views/SecretApprovalPage/components/SecretApprovalRequest/SecretApprovalRequest.tsx
@@ -39,7 +39,8 @@ export const SecretApprovalRequest = () => {
    data: secretApprovalRequests,
    isFetchingNextPage: isFetchingNextApprovalRequest,
    fetchNextPage: fetchNextApprovalRequest,
-    hasNextPage: hasNextApprovalPage
+    hasNextPage: hasNextApprovalPage,
+    refetch
  } = useGetSecretApprovalRequests({
    workspaceId,
    status: statusFilter,
@@ -51,9 +52,13 @@ export const SecretApprovalRequest = () => {
    (prev, curr) => ({ ...prev, [curr._id]: curr }),
    {}
  );
-
  const isSecretApprovalScreen = Boolean(selectedApproval);

+  const handleGoBackSecretRequestDetail = () => {
+    setSelectedApproval(null);
+    refetch({ refetchPage: (_page, index) => index === 0 });
+  };
+
  return (
    <AnimatePresence exitBeforeEnter>
      {isSecretApprovalScreen ? (
@@ -68,7 +73,7 @@ export const SecretApprovalRequest = () => {
            workspaceId={workspaceId}
            members={membersGroupById}
            approvalRequestId={selectedApproval?._id || ""}
-            onGoBack={() => setSelectedApproval(null)}
+            onGoBack={handleGoBackSecretRequestDetail}
            committer={membersGroupById?.[selectedApproval?.committer || ""]}
          />
        </motion.div>
@@ -192,16 +197,18 @@ export const SecretApprovalRequest = () => {
              </Fragment>
            ))}
          </div>
-          <Button
-            className="mt-4 text-sm"
-            isFullWidth
-            variant="star"
-            isLoading={isFetchingNextApprovalRequest}
-            isDisabled={isFetchingNextApprovalRequest || !hasNextApprovalPage}
-            onClick={() => fetchNextApprovalRequest()}
-          >
-            {hasNextApprovalPage ? "Load More" : "End of history"}
-          </Button>
+          {hasNextApprovalPage && (
+            <Button
+              className="mt-4 text-sm"
+              isFullWidth
+              variant="star"
+              isLoading={isFetchingNextApprovalRequest}
+              isDisabled={isFetchingNextApprovalRequest || !hasNextApprovalPage}
+              onClick={() => fetchNextApprovalRequest()}
+            >
+              {hasNextApprovalPage ? "Load More" : "End of history"}
+            </Button>
+          )}
        </motion.div>
      )}
    </AnimatePresence>
--- a/frontend/src/views/SecretApprovalPage/components/SecretApprovalRequest/components/SecretApprovalRequestAction.tsx
+++ b/frontend/src/views/SecretApprovalPage/components/SecretApprovalRequest/components/SecretApprovalRequestAction.tsx
@@ -1,47 +1,160 @@
-import { faCheck, faClose } from "@fortawesome/free-solid-svg-icons";
+import {
+  faCheck,
+  faClose,
+  faLockOpen,
+  faSquareCheck,
+  faSquareXmark,
+  faUserLock
+} from "@fortawesome/free-solid-svg-icons";
 import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
+import { twMerge } from "tailwind-merge";

+import { useNotificationContext } from "@app/components/context/Notifications/NotificationProvider";
 import { Button } from "@app/components/v2";
+import {
+  usePerformSecretApprovalRequestMerge,
+  useUpdateSecretApprovalRequestStatus
+} from "@app/hooks/api";

 type Props = {
+  approvalRequestId: string;
  hasMerged?: boolean;
-  status: "close" | "open";
  isMergable?: boolean;
-  isMerging?: boolean;
-  onMerge: () => void;
-  onClose?: () => void;
+  status: "close" | "open";
+  approvals: number;
+  statusChangeByEmail: string;
+  workspaceId: string;
 };
+
 export const SecretApprovalRequestAction = ({
+  approvalRequestId,
  hasMerged,
  status,
  isMergable,
-  onMerge,
-  isMerging,
-  onClose
+  approvals,
+  statusChangeByEmail,
+  workspaceId
 }: Props) => {
+  const { createNotification } = useNotificationContext();
+  const { mutateAsync: performSecretApprovalMerge, isLoading: isMerging } =
+    usePerformSecretApprovalRequestMerge();
+
+  const { mutateAsync: updateSecretStatusChange, isLoading: isStatusChanging } =
+    useUpdateSecretApprovalRequestStatus();
+
+  const handleSecretApprovalRequestMerge = async () => {
+    try {
+      await performSecretApprovalMerge({
+        id: approvalRequestId,
+        workspaceId
+      });
+      createNotification({
+        type: "success",
+        text: "Successfully merged the request"
+      });
+    } catch (err) {
+      console.log(err);
+      createNotification({
+        type: "error",
+        text: "Failed to update the request status"
+      });
+    }
+  };
+
+  const handleSecretApprovalStatusChange = async (reqState: "open" | "close") => {
+    try {
+      await updateSecretStatusChange({
+        id: approvalRequestId,
+        status: reqState
+      });
+      createNotification({
+        type: "success",
+        text: "Successfully updated the request"
+      });
+    } catch (err) {
+      console.log(err);
+      createNotification({
+        type: "error",
+        text: "Failed to update the request status"
+      });
+    }
+  };
+
  if (!hasMerged && status === "open") {
    return (
-      <>
-        <Button
-          leftIcon={<FontAwesomeIcon icon={faCheck} />}
-          isDisabled={!isMergable}
-          isLoading={isMerging}
-          onClick={onMerge}
-        >
-          Merge
-        </Button>
-        <Button
-          onClick={onClose}
-          variant="outline_bg"
-          leftIcon={<FontAwesomeIcon icon={faClose} />}
-        >
-          Close request
-        </Button>
-      </>
+      <div className="flex justify-between items-center w-full">
+        <div className="flex space-x-4 items-start">
+          <FontAwesomeIcon
+            icon={isMergable ? faSquareCheck : faSquareXmark}
+            className={twMerge("text-2xl pt-1", isMergable ? "text-primary" : "text-red-600")}
+          />
+          <span className="flex flex-col">
+            {isMergable ? "Good to merge" : "Review required"}
+            <span className="inline-block text-xs text-bunker-200">
+              At least {approvals} approving review required
+              {Boolean(statusChangeByEmail) && `. Reopened by ${statusChangeByEmail}`}
+            </span>
+          </span>
+        </div>
+        <div className="flex items-center space-x-6">
+          <Button
+            leftIcon={<FontAwesomeIcon icon={faCheck} />}
+            isDisabled={!isMergable}
+            isLoading={isMerging}
+            onClick={handleSecretApprovalRequestMerge}
+          >
+            Merge
+          </Button>
+          <Button
+            onClick={() => handleSecretApprovalStatusChange("close")}
+            isLoading={isStatusChanging}
+            variant="outline_bg"
+            colorSchema="danger"
+            leftIcon={<FontAwesomeIcon icon={faClose} />}
+          >
+            Close request
+          </Button>
+        </div>
+      </div>
    );
  }

-  if (hasMerged && status === "close") return <span>This approval request has been merged</span>;
+  if (hasMerged && status === "close")
+    return (
+      <div className="flex justify-between items-center w-full">
+        <div className="flex space-x-4 items-start">
+          <FontAwesomeIcon icon={faCheck} className="text-2xl text-primary pt-1" />
+          <span className="flex flex-col">
+            Change request merged
+            <span className="inline-block text-xs text-bunker-200">
+              Merged by {statusChangeByEmail}
+            </span>
+          </span>
+        </div>
+      </div>
+    );

-  return <span>This approval request has been closed</span>;
+  return (
+    <div className="flex justify-between items-center w-full">
+      <div className="flex space-x-4 items-start">
+        <FontAwesomeIcon icon={faUserLock} className="text-2xl text-primary pt-1" />
+        <span className="flex flex-col">
+          Change request has been closed
+          <span className="inline-block text-xs text-bunker-200">
+            Closed by {statusChangeByEmail}
+          </span>
+        </span>
+      </div>
+      <div className="flex items-center space-x-6">
+        <Button
+          onClick={() => handleSecretApprovalStatusChange("open")}
+          isLoading={isStatusChanging}
+          variant="outline_bg"
+          leftIcon={<FontAwesomeIcon icon={faLockOpen} />}
+        >
+          Reopen request
+        </Button>
+      </div>
+    </div>
+  );
 };
--- a/frontend/src/views/SecretApprovalPage/components/SecretApprovalRequest/components/SecretApprovalRequestChangeItem.tsx
+++ b/frontend/src/views/SecretApprovalPage/components/SecretApprovalRequest/components/SecretApprovalRequestChangeItem.tsx
@@ -0,0 +1,154 @@
+import { faFilePen } from "@fortawesome/free-solid-svg-icons";
+import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
+
+import {
+  SecretInput,
+  Table,
+  TableContainer,
+  Tag,
+  TBody,
+  Td,
+  Th,
+  THead,
+  Tr
+} from "@app/components/v2";
+import { CommitType, DecryptedSecret, TSecretApprovalSecChange, WsTag } from "@app/hooks/api/types";
+
+export type Props = {
+  op: CommitType;
+  secretVersion?: DecryptedSecret;
+  newVersion?: Omit<TSecretApprovalSecChange, "tags"> & { tags?: WsTag[] };
+  presentSecretVersionNumber: number;
+};
+
+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" };
+
+  return (
+    <span>
+      Request for <span style={{ color: text.color }}>secret {text.label}</span>
+    </span>
+  );
+};
+
+export const SecretApprovalRequestChangeItem = ({
+  op,
+  secretVersion,
+  newVersion,
+  presentSecretVersionNumber
+}: Props) => {
+  // meaning request has changed
+  const isStale = (secretVersion?.version || 1) < presentSecretVersionNumber;
+  return (
+    <div className="bg-bunker-500 rounded-lg pt-2 pb-4 px-4">
+      <div className="py-3 px-1 flex items-center">
+        <div className="flex-grow">{generateItemTitle(op)}</div>
+        {isStale && (
+          <div className="flex items-center">
+            <FontAwesomeIcon icon={faFilePen} className="text-primary-600 text-sm" />
+            <span className="text-xs ml-2">Secret has been changed(stale)</span>
+          </div>
+        )}
+      </div>
+      <TableContainer>
+        <Table>
+          <THead>
+            <Tr>
+              {op === CommitType.UPDATE && <Th className="w-12" />}
+              <Th className="min-table-row">Secret</Th>
+              <Th>Value</Th>
+              <Th className="min-table-row">Comment</Th>
+              <Th className="min-table-row">Tags</Th>
+            </Tr>
+          </THead>
+          {op === CommitType.UPDATE ? (
+            <TBody>
+              <Tr>
+                <Td className="text-red-600">OLD</Td>
+                <Td>{secretVersion?.key}</Td>
+                <Td>
+                  <SecretInput isReadOnly value={secretVersion?.value} />
+                </Td>
+                <Td>{secretVersion?.comment}</Td>
+                <Td>
+                  {secretVersion?.tags?.map(({ name, _id: tagId, tagColor }) => (
+                    <Tag
+                      className="flex items-center space-x-2 w-min"
+                      key={`${secretVersion._id}-${tagId}`}
+                    >
+                      <div
+                        className="w-3 h-3 rounded-full"
+                        style={{ backgroundColor: tagColor || "#bec2c8" }}
+                      />
+                      <div className="text-sm">{name}</div>
+                    </Tag>
+                  ))}
+                </Td>
+              </Tr>
+              <Tr>
+                <Td className="text-green-600">NEW</Td>
+                <Td>{newVersion?.secretKey}</Td>
+                <Td>
+                  <SecretInput isReadOnly value={newVersion?.secretValue} />
+                </Td>
+                <Td>{newVersion?.secretComment}</Td>
+                <Td>
+                  {newVersion?.tags?.map(({ name, _id: tagId, tagColor }) => (
+                    <Tag
+                      className="flex items-center space-x-2 w-min"
+                      key={`${newVersion._id}-${tagId}`}
+                    >
+                      <div
+                        className="w-3 h-3 rounded-full"
+                        style={{ backgroundColor: tagColor || "#bec2c8" }}
+                      />
+                      <div className="text-sm">{name}</div>
+                    </Tag>
+                  ))}
+                </Td>
+              </Tr>
+            </TBody>
+          ) : (
+            <TBody>
+              <Tr>
+                <Td>{op === CommitType.CREATE ? newVersion?.secretKey : secretVersion?.key}</Td>
+                <Td>
+                  <SecretInput
+                    isReadOnly
+                    value={
+                      op === CommitType.CREATE ? newVersion?.secretValue : secretVersion?.value
+                    }
+                  />
+                </Td>
+                <Td>
+                  {op === CommitType.CREATE ? newVersion?.secretComment : secretVersion?.comment}
+                </Td>
+                <Td>
+                  {(op === CommitType.CREATE ? newVersion?.tags : secretVersion?.tags)?.map(
+                    ({ name, _id: tagId, tagColor }) => (
+                      <Tag
+                        className="flex items-center space-x-2 w-min"
+                        key={`${
+                          op === CommitType.CREATE ? newVersion?._id : secretVersion?._id
+                        }-${tagId}`}
+                      >
+                        <div
+                          className="w-3 h-3 rounded-full"
+                          style={{ backgroundColor: tagColor || "#bec2c8" }}
+                        />
+                        <div className="text-sm">{name}</div>
+                      </Tag>
+                    )
+                  )}
+                </Td>
+              </Tr>
+            </TBody>
+          )}
+        </Table>
+      </TableContainer>
+    </div>
+  );
+};
--- a/frontend/src/views/SecretApprovalPage/components/SecretApprovalRequest/components/SecretApprovalRequestChanges.tsx
+++ b/frontend/src/views/SecretApprovalPage/components/SecretApprovalRequest/components/SecretApprovalRequestChanges.tsx
@@ -9,33 +9,18 @@ import {
 } from "@fortawesome/free-solid-svg-icons";
 import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";

-import {
-  Button,
-  ContentLoader,
-  IconButton,
-  SecretInput,
-  Table,
-  TableContainer,
-  Tag,
-  TBody,
-  Td,
-  Th,
-  THead,
-  Tooltip,
-  Tr
-} from "@app/components/v2";
+import { useNotificationContext } from "@app/components/context/Notifications/NotificationProvider";
+import { Button, ContentLoader, EmptyState, IconButton, Tooltip } from "@app/components/v2";
 import { useUser } from "@app/context";
 import {
  useGetSecretApprovalRequestDetails,
  useGetUserWsKey,
-  usePerformSecretApprovalRequestMerge,
-  useUpdateSecretApprovalRequestStatus
+  useUpdateSecretApprovalReviewStatus
 } from "@app/hooks/api";
 import { ApprovalStatus, CommitType, TWorkspaceUser } from "@app/hooks/api/types";

-import { useNotificationContext } from "~/components/context/Notifications/NotificationProvider";
-
 import { SecretApprovalRequestAction } from "./SecretApprovalRequestAction";
+import { SecretApprovalRequestChangeItem } from "./SecretApprovalRequestChangeItem";

 export const generateCommitText = (commits: { op: CommitType }[] = []) => {
  const score: Record<string, number> = {};
@@ -112,9 +97,7 @@ export const SecretApprovalRequestChanges = ({
    mutateAsync: updateSecretApprovalRequestStatus,
    isLoading: isUpdatingRequestStatus,
    variables
-  } = useUpdateSecretApprovalRequestStatus();
-  const { mutateAsync: performSecretApprovalMerge, isLoading: isMerging } =
-    usePerformSecretApprovalRequestMerge();
+  } = useUpdateSecretApprovalReviewStatus();

  const isApproving = variables?.status === ApprovalStatus.APPROVED && isUpdatingRequestStatus;
  const isRejecting = variables?.status === ApprovalStatus.REJECTED && isUpdatingRequestStatus;
@@ -155,31 +138,18 @@ export const SecretApprovalRequestChanges = ({
    }
  };

-  const handleSecretApprovalRequestMerge = async () => {
-    try {
-      await performSecretApprovalMerge({
-        id: approvalRequestId
-      });
-      createNotification({
-        type: "success",
-        text: "Successfully merged the request"
-      });
-    } catch (err) {
-      console.log(err);
-      createNotification({
-        type: "error",
-        text: "Failed to update the request status"
-      });
-    }
-  };
-
  if (isSecretApprovalRequestLoading) {
    <div>
      <ContentLoader />
    </div>;
  }

-  if (!isSecretApprovalRequestSuccess) return <div>Failed</div>;
+  if (!isSecretApprovalRequestSuccess)
+    return (
+      <div>
+        <EmptyState title="Failed to load approvals" />
+      </div>
+    );

  const isMergable =
    secretApprovalRequestDetails?.policy?.approvals <=
@@ -212,141 +182,54 @@ export const SecretApprovalRequestChanges = ({
              </span>
            </div>
          </div>
-          <Button
-            size="xs"
-            leftIcon={hasApproved && <FontAwesomeIcon icon={faCheck} />}
-            onClick={() => handleSecretApprovalStatusUpdate(ApprovalStatus.APPROVED)}
-            isLoading={isApproving}
-            isDisabled={isApproving || hasApproved}
-          >
-            {hasApproved ? "Approved" : "Approve"}
-          </Button>
-          <Button
-            size="xs"
-            colorSchema="danger"
-            leftIcon={hasRejected && <FontAwesomeIcon icon={faCheck} />}
-            onClick={() => handleSecretApprovalStatusUpdate(ApprovalStatus.REJECTED)}
-            isLoading={isRejecting}
-            isDisabled={isRejecting || hasRejected}
-          >
-            {hasRejected ? "Rejected" : "Reject"}
-          </Button>
+          {!hasMerged && secretApprovalRequestDetails.status === "open" && (
+            <>
+              <Button
+                size="xs"
+                leftIcon={hasApproved && <FontAwesomeIcon icon={faCheck} />}
+                onClick={() => handleSecretApprovalStatusUpdate(ApprovalStatus.APPROVED)}
+                isLoading={isApproving}
+                isDisabled={isApproving || hasApproved}
+              >
+                {hasApproved ? "Approved" : "Approve"}
+              </Button>
+              <Button
+                size="xs"
+                colorSchema="danger"
+                leftIcon={hasRejected && <FontAwesomeIcon icon={faCheck} />}
+                onClick={() => handleSecretApprovalStatusUpdate(ApprovalStatus.REJECTED)}
+                isLoading={isRejecting}
+                isDisabled={isRejecting || hasRejected}
+              >
+                {hasRejected ? "Rejected" : "Reject"}
+              </Button>
+            </>
+          )}
        </div>
        <div className="flex flex-col space-y-4">
-          {secretApprovalRequestDetails.commits.map(({ op, secretVersion, newVersion }, index) => (
-            <div key={`commit-change-secret-${index + 1}`}>
-              <TableContainer>
-                <Table>
-                  <THead>
-                    <Tr>
-                      {op === CommitType.UPDATE && <Th className="w-12" />}
-                      <Th className="min-table-row">Secret</Th>
-                      <Th>Value</Th>
-                      <Th className="min-table-row">Comment</Th>
-                      <Th className="min-table-row">Tags</Th>
-                    </Tr>
-                  </THead>
-                  {op === CommitType.UPDATE ? (
-                    <TBody>
-                      <Tr>
-                        <Td className="text-red-600">OLD</Td>
-                        <Td>{secretVersion?.key}</Td>
-                        <Td>
-                          <SecretInput isReadOnly value={secretVersion?.value} />
-                        </Td>
-                        <Td>{secretVersion?.comment}</Td>
-                        <Td>
-                          {secretVersion?.tags?.map(({ name, _id: tagId, tagColor }) => (
-                            <Tag
-                              className="flex items-center space-x-2 w-min"
-                              key={`${secretVersion._id}-${tagId}`}
-                            >
-                              <div
-                                className="w-3 h-3 rounded-full"
-                                style={{ backgroundColor: tagColor || "#bec2c8" }}
-                              />
-                              <div className="text-sm">{name}</div>
-                            </Tag>
-                          ))}
-                        </Td>
-                      </Tr>
-                      <Tr>
-                        <Td className="text-green-600">NEW</Td>
-                        <Td>{newVersion?.secretKey}</Td>
-                        <Td>
-                          <SecretInput isReadOnly value={newVersion?.secretValue} />
-                        </Td>
-                        <Td>{newVersion?.secretComment}</Td>
-                        <Td>
-                          {newVersion?.tags?.map(({ name, _id: tagId, tagColor }) => (
-                            <Tag
-                              className="flex items-center space-x-2 w-min"
-                              key={`${newVersion._id}-${tagId}`}
-                            >
-                              <div
-                                className="w-3 h-3 rounded-full"
-                                style={{ backgroundColor: tagColor || "#bec2c8" }}
-                              />
-                              <div className="text-sm">{name}</div>
-                            </Tag>
-                          ))}
-                        </Td>
-                      </Tr>
-                    </TBody>
-                  ) : (
-                    <TBody>
-                      <Tr>
-                        <Td>
-                          {op === CommitType.CREATE ? newVersion?.secretKey : secretVersion?.key}
-                        </Td>
-                        <Td>
-                          <SecretInput
-                            isReadOnly
-                            value={
-                              op === CommitType.CREATE
-                                ? newVersion?.secretValue
-                                : secretVersion?.value
-                            }
-                          />
-                        </Td>
-                        <Td>
-                          {op === CommitType.CREATE
-                            ? newVersion?.secretComment
-                            : secretVersion?.comment}
-                        </Td>
-                        <Td>
-                          {(op === CommitType.CREATE ? newVersion?.tags : secretVersion?.tags)?.map(
-                            ({ name, _id: tagId, tagColor }) => (
-                              <Tag
-                                className="flex items-center space-x-2 w-min"
-                                key={`${
-                                  op === CommitType.CREATE ? newVersion?._id : secretVersion?._id
-                                }-${tagId}`}
-                              >
-                                <div
-                                  className="w-3 h-3 rounded-full"
-                                  style={{ backgroundColor: tagColor || "#bec2c8" }}
-                                />
-                                <div className="text-sm">{name}</div>
-                              </Tag>
-                            )
-                          )}
-                        </Td>
-                      </Tr>
-                    </TBody>
-                  )}
-                </Table>
-              </TableContainer>
-            </div>
-          ))}
+          {secretApprovalRequestDetails.commits.map(
+            ({ op, secretVersion, secret, newVersion }, index) => (
+              <SecretApprovalRequestChangeItem
+                op={op}
+                secretVersion={secretVersion}
+                presentSecretVersionNumber={secret?.version || 0}
+                newVersion={newVersion}
+                key={`${op}-${index + 1}-${secretVersion?._id}`}
+              />
+            )
+          )}
        </div>
-        <div className="flex items-center px-4 py-6 rounded-lg space-x-6 bg-mineshaft-800 mt-8">
+        <div className="flex items-center px-5 py-6 rounded-lg space-x-6 bg-mineshaft-800 mt-8">
          <SecretApprovalRequestAction
+            approvalRequestId={secretApprovalRequestDetails._id}
            hasMerged={hasMerged}
+            approvals={secretApprovalRequestDetails.policy.approvals || 0}
            status={secretApprovalRequestDetails.status}
-            isMerging={isMerging}
            isMergable={isMergable}
-            onMerge={handleSecretApprovalRequestMerge}
+            statusChangeByEmail={
+              members[secretApprovalRequestDetails?.statusChangeBy || ""]?.user?.email || ""
+            }
+            workspaceId={workspaceId}
          />
        </div>
      </div>
--- a/frontend/src/views/SecretMainPage/components/SecretListView/SecretListView.tsx
+++ b/frontend/src/views/SecretMainPage/components/SecretListView/SecretListView.tsx
@@ -183,7 +183,6 @@ export const SecretListView = ({
      modSecret: Omit<DecryptedSecret, "tags"> & { tags: { _id: string }[] },
      cb?: () => void
    ) => {
-      console.log(orgSecret, modSecret);
      const { key: oldKey } = orgSecret;
      const { key, value, overrideAction, idOverride, valueOverride, tags, comment } = modSecret;
      const hasKeyChanged = oldKey !== key;
@@ -219,7 +218,7 @@ export const SecretListView = ({
            newKey: hasKeyChanged ? key : undefined,
            skipMultilineEncoding: modSecret.skipMultilineEncoding
          });
-          if (isProtectedBranch) cb?.();
+          if (isProtectedBranch) cb();
        }

        queryClient.invalidateQueries(