improvement: address feedback

This commit is contained in:
Scott Wilson
2025-07-01 16:13:14 -07:00
parent aff97374a9
commit 19ff045d2e
16 changed files with 201 additions and 70 deletions
@@ -60,7 +60,8 @@ export const registerAccessApprovalRequestRouter = async (server: FastifyZodProv
method: "GET", method: "GET",
schema: { schema: {
querystring: z.object({ querystring: z.object({
projectSlug: z.string().trim() projectSlug: z.string().trim(),
policyId: z.string().trim().optional()
}), }),
response: { response: {
200: z.object({ 200: z.object({
@@ -73,6 +74,7 @@ export const registerAccessApprovalRequestRouter = async (server: FastifyZodProv
handler: async (req) => { handler: async (req) => {
const { count } = await server.services.accessApprovalRequest.getCount({ const { count } = await server.services.accessApprovalRequest.getCount({
projectSlug: req.query.projectSlug, projectSlug: req.query.projectSlug,
policyId: req.query.policyId,
actor: req.permission.type, actor: req.permission.type,
actorId: req.permission.id, actorId: req.permission.id,
actorOrgId: req.permission.orgId, actorOrgId: req.permission.orgId,
@@ -94,7 +94,8 @@ export const registerSecretApprovalRequestRouter = async (server: FastifyZodProv
}, },
schema: { schema: {
querystring: z.object({ querystring: z.object({
workspaceId: z.string().trim() workspaceId: z.string().trim(),
policyId: z.string().trim().optional()
}), }),
response: { response: {
200: z.object({ 200: z.object({
@@ -112,7 +113,8 @@ export const registerSecretApprovalRequestRouter = async (server: FastifyZodProv
actorId: req.permission.id, actorId: req.permission.id,
actorAuthMethod: req.permission.authMethod, actorAuthMethod: req.permission.authMethod,
actorOrgId: req.permission.orgId, actorOrgId: req.permission.orgId,
projectId: req.query.workspaceId projectId: req.query.workspaceId,
policyId: req.query.policyId
}); });
return { approvals }; return { approvals };
} }
@@ -220,7 +220,7 @@ export interface TAccessApprovalRequestDALFactory extends Omit<TOrmify<TableName
bypassers: string[]; bypassers: string[];
}[] }[]
>; >;
getCount: ({ projectId }: { projectId: string }) => Promise<{ getCount: ({ projectId }: { projectId: string; policyId?: string }) => Promise<{
pendingCount: number; pendingCount: number;
finalizedCount: number; finalizedCount: number;
}>; }>;
@@ -702,7 +702,7 @@ export const accessApprovalRequestDALFactory = (db: TDbClient): TAccessApprovalR
} }
}; };
const getCount: TAccessApprovalRequestDALFactory["getCount"] = async ({ projectId }) => { const getCount: TAccessApprovalRequestDALFactory["getCount"] = async ({ projectId, policyId }) => {
try { try {
const accessRequests = await db const accessRequests = await db
.replicaNode()(TableName.AccessApprovalRequest) .replicaNode()(TableName.AccessApprovalRequest)
@@ -723,8 +723,10 @@ export const accessApprovalRequestDALFactory = (db: TDbClient): TAccessApprovalR
`${TableName.AccessApprovalRequest}.id`, `${TableName.AccessApprovalRequest}.id`,
`${TableName.AccessApprovalRequestReviewer}.requestId` `${TableName.AccessApprovalRequestReviewer}.requestId`
) )
.where(`${TableName.Environment}.projectId`, projectId) .where(`${TableName.Environment}.projectId`, projectId)
.where((qb) => {
if (policyId) void qb.where(`${TableName.AccessApprovalPolicy}.id`, policyId);
})
.select(selectAllTableCols(TableName.AccessApprovalRequest)) .select(selectAllTableCols(TableName.AccessApprovalRequest))
.select(db.ref("status").withSchema(TableName.AccessApprovalRequestReviewer).as("reviewerStatus")) .select(db.ref("status").withSchema(TableName.AccessApprovalRequestReviewer).as("reviewerStatus"))
.select(db.ref("reviewerUserId").withSchema(TableName.AccessApprovalRequestReviewer).as("reviewerUserId")) .select(db.ref("reviewerUserId").withSchema(TableName.AccessApprovalRequestReviewer).as("reviewerUserId"))
@@ -560,6 +560,7 @@ export const accessApprovalRequestServiceFactory = ({
const getCount: TAccessApprovalRequestServiceFactory["getCount"] = async ({ const getCount: TAccessApprovalRequestServiceFactory["getCount"] = async ({
projectSlug, projectSlug,
policyId,
actor, actor,
actorAuthMethod, actorAuthMethod,
actorId, actorId,
@@ -580,7 +581,7 @@ export const accessApprovalRequestServiceFactory = ({
throw new ForbiddenRequestError({ message: "You are not a member of this project" }); throw new ForbiddenRequestError({ message: "You are not a member of this project" });
} }
const count = await accessApprovalRequestDAL.getCount({ projectId: project.id }); const count = await accessApprovalRequestDAL.getCount({ projectId: project.id, policyId });
return { count }; return { count };
}; };
@@ -12,6 +12,7 @@ export type TVerifyPermission = {
export type TGetAccessRequestCountDTO = { export type TGetAccessRequestCountDTO = {
projectSlug: string; projectSlug: string;
policyId?: string;
} & Omit<TProjectPermission, "projectId">; } & Omit<TProjectPermission, "projectId">;
export type TReviewAccessRequestDTO = { export type TReviewAccessRequestDTO = {
@@ -290,7 +290,7 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => {
} }
}; };
const findProjectRequestCount = async (projectId: string, userId: string, tx?: Knex) => { const findProjectRequestCount = async (projectId: string, userId: string, policyId?: string, tx?: Knex) => {
try { try {
const docs = await (tx || db) const docs = await (tx || db)
.with( .with(
@@ -309,6 +309,9 @@ export const secretApprovalRequestDALFactory = (db: TDbClient) => {
`${TableName.SecretApprovalPolicy}.id` `${TableName.SecretApprovalPolicy}.id`
) )
.where({ projectId }) .where({ projectId })
.where((qb) => {
if (policyId) void qb.where(`${TableName.SecretApprovalPolicy}.id`, policyId);
})
.andWhere( .andWhere(
(bd) => (bd) =>
void bd void bd
@@ -168,7 +168,14 @@ export const secretApprovalRequestServiceFactory = ({
microsoftTeamsService, microsoftTeamsService,
folderCommitService folderCommitService
}: TSecretApprovalRequestServiceFactoryDep) => { }: TSecretApprovalRequestServiceFactoryDep) => {
const requestCount = async ({ projectId, actor, actorId, actorOrgId, actorAuthMethod }: TApprovalRequestCountDTO) => { const requestCount = async ({
projectId,
policyId,
actor,
actorId,
actorOrgId,
actorAuthMethod
}: TApprovalRequestCountDTO) => {
if (actor === ActorType.SERVICE) throw new BadRequestError({ message: "Cannot use service token" }); if (actor === ActorType.SERVICE) throw new BadRequestError({ message: "Cannot use service token" });
await permissionService.getProjectPermission({ await permissionService.getProjectPermission({
@@ -180,7 +187,7 @@ export const secretApprovalRequestServiceFactory = ({
actionProjectType: ActionProjectType.SecretManager actionProjectType: ActionProjectType.SecretManager
}); });
const count = await secretApprovalRequestDAL.findProjectRequestCount(projectId, actorId); const count = await secretApprovalRequestDAL.findProjectRequestCount(projectId, actorId, policyId);
return count; return count;
}; };
@@ -84,7 +84,7 @@ export type TReviewRequestDTO = {
comment?: string; comment?: string;
} & Omit<TProjectPermission, "projectId">; } & Omit<TProjectPermission, "projectId">;
export type TApprovalRequestCountDTO = TProjectPermission; export type TApprovalRequestCountDTO = TProjectPermission & { policyId?: string };
export type TListApprovalsDTO = { export type TListApprovalsDTO = {
projectId: string; projectId: string;
@@ -20,6 +20,7 @@ type Props = {
children?: ReactNode; children?: ReactNode;
deletionMessage?: ReactNode; deletionMessage?: ReactNode;
buttonColorSchema?: "danger" | "primary" | "secondary" | "gray" | null; buttonColorSchema?: "danger" | "primary" | "secondary" | "gray" | null;
isDisabled?: boolean;
}; };
export const DeleteActionModal = ({ export const DeleteActionModal = ({
@@ -34,6 +35,7 @@ export const DeleteActionModal = ({
formContent, formContent,
deletionMessage, deletionMessage,
buttonColorSchema = "danger", buttonColorSchema = "danger",
isDisabled,
children children
}: Props): JSX.Element => { }: Props): JSX.Element => {
const [inputData, setInputData] = useState(""); const [inputData, setInputData] = useState("");
@@ -70,7 +72,7 @@ export const DeleteActionModal = ({
<Button <Button
className="mr-4" className="mr-4"
colorSchema={buttonColorSchema} colorSchema={buttonColorSchema}
isDisabled={!(deleteKey === inputData) || isLoading} isDisabled={!(deleteKey === inputData) || isLoading || isDisabled}
onClick={onDelete} onClick={onDelete}
isLoading={isLoading} isLoading={isLoading}
> >
@@ -25,8 +25,8 @@ export const accessApprovalKeys = {
requestedBy?: string, requestedBy?: string,
bypassReason?: string bypassReason?: string
) => [{ projectSlug, envSlug, requestedBy, bypassReason }, "access-approvals-requests"] as const, ) => [{ projectSlug, envSlug, requestedBy, bypassReason }, "access-approvals-requests"] as const,
getAccessApprovalRequestCount: (projectSlug: string) => getAccessApprovalRequestCount: (projectSlug: string, policyId?: string) =>
[{ projectSlug }, "access-approval-request-count"] as const [{ projectSlug }, "access-approval-request-count", ...(policyId ? [policyId] : [])] as const
}; };
export const fetchPolicyApprovalCount = async ({ export const fetchPolicyApprovalCount = async ({
@@ -87,21 +87,22 @@ const fetchApprovalRequests = async ({
})); }));
}; };
const fetchAccessRequestsCount = async (projectSlug: string) => { const fetchAccessRequestsCount = async (projectSlug: string, policyId?: string) => {
const { data } = await apiRequest.get<TAccessRequestCount>( const { data } = await apiRequest.get<TAccessRequestCount>(
"/api/v1/access-approvals/requests/count", "/api/v1/access-approvals/requests/count",
{ params: { projectSlug } } { params: { projectSlug, policyId } }
); );
return data; return data;
}; };
export const useGetAccessRequestsCount = ({ export const useGetAccessRequestsCount = ({
projectSlug, projectSlug,
policyId,
options = {} options = {}
}: TGetAccessApprovalRequestsDTO & TReactQueryOptions) => }: TGetAccessApprovalRequestsDTO & TReactQueryOptions) =>
useQuery({ useQuery({
queryKey: accessApprovalKeys.getAccessApprovalRequestCount(projectSlug), queryKey: accessApprovalKeys.getAccessApprovalRequestCount(projectSlug, policyId),
queryFn: () => fetchAccessRequestsCount(projectSlug), queryFn: () => fetchAccessRequestsCount(projectSlug, policyId),
...options, ...options,
enabled: Boolean(projectSlug) && (options?.enabled ?? true) enabled: Boolean(projectSlug) && (options?.enabled ?? true)
}); });
@@ -147,6 +147,7 @@ export type TCreateAccessRequestDTO = {
export type TGetAccessApprovalRequestsDTO = { export type TGetAccessApprovalRequestsDTO = {
projectSlug: string; projectSlug: string;
policyId?: string;
envSlug?: string; envSlug?: string;
authorUserId?: string; authorUserId?: string;
}; };
@@ -34,9 +34,10 @@ export const secretApprovalRequestKeys = {
] as const, ] as const,
detail: ({ id }: Omit<TGetSecretApprovalRequestDetails, "decryptKey">) => detail: ({ id }: Omit<TGetSecretApprovalRequestDetails, "decryptKey">) =>
[{ id }, "secret-approval-request-detail"] as const, [{ id }, "secret-approval-request-detail"] as const,
count: ({ workspaceId }: TGetSecretApprovalRequestCount) => [ count: ({ workspaceId, policyId }: TGetSecretApprovalRequestCount) => [
{ workspaceId }, { workspaceId },
"secret-approval-request-count" "secret-approval-request-count",
...(policyId ? [policyId] : [])
] ]
}; };
@@ -204,10 +205,13 @@ export const useGetSecretApprovalRequestDetails = ({
enabled: Boolean(id) && (options?.enabled ?? true) enabled: Boolean(id) && (options?.enabled ?? true)
}); });
const fetchSecretApprovalRequestCount = async ({ workspaceId }: TGetSecretApprovalRequestCount) => { const fetchSecretApprovalRequestCount = async ({
workspaceId,
policyId
}: TGetSecretApprovalRequestCount) => {
const { data } = await apiRequest.get<{ approvals: TSecretApprovalRequestCount }>( const { data } = await apiRequest.get<{ approvals: TSecretApprovalRequestCount }>(
"/api/v1/secret-approval-requests/count", "/api/v1/secret-approval-requests/count",
{ params: { workspaceId } } { params: { workspaceId, policyId } }
); );
return data.approvals; return data.approvals;
@@ -215,6 +219,7 @@ const fetchSecretApprovalRequestCount = async ({ workspaceId }: TGetSecretApprov
export const useGetSecretApprovalRequestCount = ({ export const useGetSecretApprovalRequestCount = ({
workspaceId, workspaceId,
policyId,
options = {} options = {}
}: TGetSecretApprovalRequestCount & { }: TGetSecretApprovalRequestCount & {
options?: Omit< options?: Omit<
@@ -228,8 +233,8 @@ export const useGetSecretApprovalRequestCount = ({
>; >;
}) => }) =>
useQuery({ useQuery({
queryKey: secretApprovalRequestKeys.count({ workspaceId }), queryKey: secretApprovalRequestKeys.count({ workspaceId, policyId }),
refetchInterval: 15000, refetchInterval: 15000,
queryFn: () => fetchSecretApprovalRequestCount({ workspaceId }), queryFn: () => fetchSecretApprovalRequestCount({ workspaceId, policyId }),
enabled: Boolean(workspaceId) && (options?.enabled ?? true) enabled: Boolean(workspaceId) && (options?.enabled ?? true)
}); });
@@ -118,6 +118,7 @@ export type TGetSecretApprovalRequestList = {
export type TGetSecretApprovalRequestCount = { export type TGetSecretApprovalRequestCount = {
workspaceId: string; workspaceId: string;
policyId?: string;
}; };
export type TGetSecretApprovalRequestDetails = { export type TGetSecretApprovalRequestDetails = {
@@ -164,7 +164,10 @@ export const MinimizedOrgSidebar = () => {
const handleCopyToken = async () => { const handleCopyToken = async () => {
try { try {
await window.navigator.clipboard.writeText(getAuthToken()); await window.navigator.clipboard.writeText(getAuthToken());
createNotification({ type: "success", text: "Copied current login session token to clipboard" }); createNotification({
type: "success",
text: "Copied current login session token to clipboard"
});
} catch (error) { } catch (error) {
console.log(error); console.log(error);
createNotification({ type: "error", text: "Failed to copy user token to clipboard" }); createNotification({ type: "error", text: "Failed to copy user token to clipboard" });
@@ -16,11 +16,9 @@ import { AnimatePresence, motion } from "framer-motion";
import { twMerge } from "tailwind-merge"; import { twMerge } from "tailwind-merge";
import { UpgradePlanModal } from "@app/components/license/UpgradePlanModal"; import { UpgradePlanModal } from "@app/components/license/UpgradePlanModal";
import { createNotification } from "@app/components/notifications";
import { ProjectPermissionCan } from "@app/components/permissions"; import { ProjectPermissionCan } from "@app/components/permissions";
import { import {
Button, Button,
DeleteActionModal,
DropdownMenu, DropdownMenu,
DropdownMenuContent, DropdownMenuContent,
DropdownMenuItem, DropdownMenuItem,
@@ -54,8 +52,6 @@ import {
} from "@app/helpers/userTablePreferences"; } from "@app/helpers/userTablePreferences";
import { usePagination, usePopUp, useResetPageHelper } from "@app/hooks"; import { usePagination, usePopUp, useResetPageHelper } from "@app/hooks";
import { import {
useDeleteAccessApprovalPolicy,
useDeleteSecretApprovalPolicy,
useGetSecretApprovalPolicies, useGetSecretApprovalPolicies,
useGetWorkspaceUsers, useGetWorkspaceUsers,
useListWorkspaceGroups useListWorkspaceGroups
@@ -67,6 +63,7 @@ import { TAccessApprovalPolicy, Workspace } from "@app/hooks/api/types";
import { AccessPolicyForm } from "./components/AccessPolicyModal"; import { AccessPolicyForm } from "./components/AccessPolicyModal";
import { ApprovalPolicyRow } from "./components/ApprovalPolicyRow"; import { ApprovalPolicyRow } from "./components/ApprovalPolicyRow";
import { RemoveApprovalPolicyModal } from "./components/RemoveApprovalPolicyModal";
interface IProps { interface IProps {
workspaceId: string; workspaceId: string;
@@ -122,7 +119,7 @@ const useApprovalPolicies = (permission: TProjectPermission, currentWorkspace?:
}; };
export const ApprovalPolicyList = ({ workspaceId }: IProps) => { export const ApprovalPolicyList = ({ workspaceId }: IProps) => {
const { handlePopUpToggle, handlePopUpOpen, handlePopUpClose, popUp } = usePopUp([ const { handlePopUpToggle, handlePopUpOpen, popUp } = usePopUp([
"policyForm", "policyForm",
"deletePolicy", "deletePolicy",
"upgradePlan" "upgradePlan"
@@ -213,39 +210,6 @@ export const ApprovalPolicyList = ({ workspaceId }: IProps) => {
setPage setPage
}); });
const { mutateAsync: deleteSecretApprovalPolicy } = useDeleteSecretApprovalPolicy();
const { mutateAsync: deleteAccessApprovalPolicy } = useDeleteAccessApprovalPolicy();
const handleDeletePolicy = async () => {
const { id, policyType } = popUp.deletePolicy.data as TAccessApprovalPolicy;
if (!currentWorkspace?.slug) return;
try {
if (policyType === PolicyType.ChangePolicy) {
await deleteSecretApprovalPolicy({
workspaceId,
id
});
} else {
await deleteAccessApprovalPolicy({
projectSlug: currentWorkspace?.slug,
id
});
}
createNotification({
type: "success",
text: "Successfully deleted policy"
});
handlePopUpClose("deletePolicy");
} catch (err) {
console.log(err);
createNotification({
type: "error",
text: "Failed to delete policy"
});
}
};
const isTableFiltered = filters.type !== null || Boolean(filters.environmentIds.length); const isTableFiltered = filters.type !== null || Boolean(filters.environmentIds.length);
const handleSort = (column: PolicyOrderBy) => { const handleSort = (column: PolicyOrderBy) => {
@@ -528,13 +492,14 @@ export const ApprovalPolicyList = ({ workspaceId }: IProps) => {
members={members} members={members}
editValues={popUp.policyForm.data as TAccessApprovalPolicy} editValues={popUp.policyForm.data as TAccessApprovalPolicy}
/> />
<DeleteActionModal {popUp.deletePolicy.data && (
isOpen={popUp.deletePolicy.isOpen} <RemoveApprovalPolicyModal
deleteKey="remove" isOpen={popUp.deletePolicy.isOpen}
title="Do you want to remove this policy?" onOpenChange={(isOpen) => handlePopUpToggle("deletePolicy", isOpen)}
onChange={(isOpen) => handlePopUpToggle("deletePolicy", isOpen)} policyType={popUp.deletePolicy.data.policyType}
onDeleteApproved={handleDeletePolicy} policyId={popUp.deletePolicy.data.id}
/> />
)}
<UpgradePlanModal <UpgradePlanModal
isOpen={popUp.upgradePlan.isOpen} isOpen={popUp.upgradePlan.isOpen}
onOpenChange={(isOpen) => handlePopUpToggle("upgradePlan", isOpen)} onOpenChange={(isOpen) => handlePopUpToggle("upgradePlan", isOpen)}
@@ -0,0 +1,135 @@
import { faCheck, faWarning } from "@fortawesome/free-solid-svg-icons";
import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
import { twMerge } from "tailwind-merge";
import { createNotification } from "@app/components/notifications";
import { DeleteActionModal, Spinner } from "@app/components/v2";
import { useWorkspace } from "@app/context";
import {
useDeleteAccessApprovalPolicy,
useDeleteSecretApprovalPolicy,
useGetAccessRequestsCount,
useGetSecretApprovalRequestCount
} from "@app/hooks/api";
import { PolicyType } from "@app/hooks/api/policies/enums";
type Props = {
policyId: string;
policyType: PolicyType;
isOpen: boolean;
onOpenChange: (isOpen: boolean) => void;
};
export const RemoveApprovalPolicyModal = ({
policyId,
policyType,
isOpen,
onOpenChange
}: Props) => {
const { mutateAsync: deleteSecretApprovalPolicy } = useDeleteSecretApprovalPolicy();
const { mutateAsync: deleteAccessApprovalPolicy } = useDeleteAccessApprovalPolicy();
const { currentWorkspace } = useWorkspace();
const handleDeletePolicy = async () => {
try {
if (policyType === PolicyType.ChangePolicy) {
await deleteSecretApprovalPolicy({
workspaceId: currentWorkspace.id,
id: policyId
});
} else {
await deleteAccessApprovalPolicy({
projectSlug: currentWorkspace.slug,
id: policyId
});
}
createNotification({
type: "success",
text: "Successfully deleted policy"
});
onOpenChange(false);
} catch {
createNotification({
type: "error",
text: "Failed to delete policy"
});
}
};
const deleteSecretApprovalData = useGetSecretApprovalRequestCount({
policyId,
workspaceId: currentWorkspace.id,
options: {
enabled: Boolean(policyId) && policyType === PolicyType.ChangePolicy
}
});
const deleteAccessApprovalData = useGetAccessRequestsCount({
projectSlug: currentWorkspace.slug,
policyId,
options: {
enabled: Boolean(policyId) && policyType === PolicyType.AccessPolicy
}
});
let openCount: number | undefined;
let isPending: boolean;
if (policyType === PolicyType.ChangePolicy) {
openCount = deleteSecretApprovalData.data?.open;
isPending = deleteSecretApprovalData.isPending;
} else {
openCount = deleteAccessApprovalData.data?.pendingCount;
isPending = deleteAccessApprovalData.isPending;
}
return (
<DeleteActionModal
isOpen={isOpen}
deleteKey="remove"
title="Do you want to remove this policy?"
onChange={onOpenChange}
onDeleteApproved={handleDeletePolicy}
isDisabled={isPending}
>
{isPending ? (
<div className="mt-4 flex w-full items-center gap-2 p-2">
<Spinner size="xs" className="text-mineshaft-600" />
<span className="text-sm text-mineshaft-400">Checking for open requests...</span>
</div>
) : (
<div
className={twMerge(
"mt-4 flex w-full items-start gap-2 rounded border p-2 text-sm",
(openCount ?? 0) > 0
? "border-yellow/20 bg-yellow/10 text-yellow"
: "border-green/20 bg-green/10 text-green"
)}
>
{(openCount ?? 0) > 0 ? (
<>
<FontAwesomeIcon className="mt-1" icon={faWarning} />
<div className="flex flex-col">
<span>
This policy has {openCount} open request
{(openCount ?? 0) > 1 ? "s" : ""}
</span>
<p className="text-xs text-mineshaft-200">
Removing this policy will close all open requests.
</p>
</div>
</>
) : (
<>
<FontAwesomeIcon className="mt-1" icon={faCheck} />
<div className="flex flex-col">
<span>This policy has no open requests</span>
<p className="text-xs text-mineshaft-200">This policy is safe to remove.</p>
</div>
</>
)}
</div>
)}
</DeleteActionModal>
);
};