Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,3 +1,16 @@
import { useState } from "react";

import * as SCHEMA from "@ctrlplane/db/schema";
import { Button } from "@ctrlplane/ui/button";
import {
Dialog,
DialogContent,
DialogDescription,
DialogFooter,
DialogHeader,
DialogTrigger,
} from "@ctrlplane/ui/dialog";
import { Textarea } from "@ctrlplane/ui/textarea";
import {
Tooltip,
TooltipContent,
Expand All @@ -8,17 +21,84 @@ import {
import { api } from "~/trpc/react";
import { Loading, Passing, Waiting } from "../StatusIcons";

const ApprovalDialog: React.FC<{
versionId: string;
versionTag: string;
environmentId: string;
onSubmit: () => void;
}> = ({ versionId, versionTag, environmentId, onSubmit }) => {
const [open, setOpen] = useState(false);
const addRecord =
api.deployment.version.checks.approval.addRecord.useMutation();

const [reason, setReason] = useState("");

const handleSubmit = (status: SCHEMA.ApprovalStatus) =>
addRecord
.mutateAsync({
deploymentVersionId: versionId,
environmentId,
status,
reason,
})
.then(() => setOpen(false))
.then(() => onSubmit());
Comment on lines +31 to +45
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add loading state and error handling to mutation.

The mutation lacks loading state feedback and error handling. If the API call fails, the user won't see any feedback.

-  const addRecord =
-    api.deployment.version.checks.approval.addRecord.useMutation();
+  const addRecord =
+    api.deployment.version.checks.approval.addRecord.useMutation({
+      onError: (error) => {
+        // Handle error (e.g., show toast notification)
+        console.error("Failed to submit approval:", error);
+      }
+    });

   const [reason, setReason] = useState("");

   const handleSubmit = (status: SCHEMA.ApprovalStatus) =>
     addRecord
       .mutateAsync({
         deploymentVersionId: versionId,
         environmentId,
         status,
         reason,
       })
       .then(() => setOpen(false))
       .then(() => onSubmit());

Consider also disabling the buttons during submission:

 <Button
   variant="outline"
+  disabled={addRecord.isLoading}
   onClick={() => handleSubmit(SCHEMA.ApprovalStatus.Rejected)}
 >
-  Reject
+  {addRecord.isLoading ? "Submitting..." : "Reject"}
 </Button>
 <Button
+  disabled={addRecord.isLoading}
   onClick={() => handleSubmit(SCHEMA.ApprovalStatus.Approved)}
 >
-  Approve
+  {addRecord.isLoading ? "Submitting..." : "Approve"}
 </Button>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const addRecord =
api.deployment.version.checks.approval.addRecord.useMutation();
const [reason, setReason] = useState("");
const handleSubmit = (status: SCHEMA.ApprovalStatus) =>
addRecord
.mutateAsync({
deploymentVersionId: versionId,
environmentId,
status,
reason,
})
.then(() => setOpen(false))
.then(() => onSubmit());
const addRecord =
api.deployment.version.checks.approval.addRecord.useMutation({
onError: (error) => {
// Handle error (e.g., show toast notification)
console.error("Failed to submit approval:", error);
}
});
const [reason, setReason] = useState("");
const handleSubmit = (status: SCHEMA.ApprovalStatus) =>
addRecord
.mutateAsync({
deploymentVersionId: versionId,
environmentId,
status,
reason,
})
.then(() => setOpen(false))
.then(() => onSubmit());
// In the JSX render block:
<Button
variant="outline"
disabled={addRecord.isLoading}
onClick={() => handleSubmit(SCHEMA.ApprovalStatus.Rejected)}
>
{addRecord.isLoading ? "Submitting..." : "Reject"}
</Button>
<Button
disabled={addRecord.isLoading}
onClick={() => handleSubmit(SCHEMA.ApprovalStatus.Approved)}
>
{addRecord.isLoading ? "Submitting..." : "Approve"}
</Button>


return (
<Dialog open={open} onOpenChange={setOpen}>
<DialogTrigger asChild>
<Button size="sm" className="h-6">
Approve
</Button>
</DialogTrigger>
<DialogContent>
<DialogHeader>Approve Release</DialogHeader>
<DialogDescription>
Are you sure you want to approve version {versionTag}?
</DialogDescription>

<Textarea
value={reason}
onChange={(e) => setReason(e.target.value)}
placeholder="Reason for approval (optional)"
/>

<DialogFooter className="flex w-full flex-row items-center justify-between sm:justify-between">
<Button variant="outline" onClick={() => setOpen(false)}>
Cancel
</Button>
<div className="flex gap-2">
<Button
variant="outline"
onClick={() => handleSubmit(SCHEMA.ApprovalStatus.Rejected)}
disabled={addRecord.isPending}
>
Reject
</Button>
<Button
onClick={() => handleSubmit(SCHEMA.ApprovalStatus.Approved)}
disabled={addRecord.isPending}
>
Approve
</Button>
</div>
</DialogFooter>
</DialogContent>
</Dialog>
);
};

export const ApprovalCheck: React.FC<{
workspaceId: string;
environmentId: string;
versionId: string;
}> = ({ workspaceId, environmentId, versionId }) => {
versionTag: string;
}> = (props) => {
const { data, isLoading } =
api.deployment.version.checks.approval.status.useQuery({
workspaceId,
environmentId,
versionId,
});
api.deployment.version.checks.approval.status.useQuery(props);
const utils = api.useUtils();
const invalidate = () =>
utils.deployment.version.checks.approval.status.invalidate(props);

const isApproved = data?.approved ?? false;
const rejectionReasonEntries = Array.from(
Expand All @@ -39,13 +119,16 @@ export const ApprovalCheck: React.FC<{
</div>
);

if (rejectionReasonEntries.length > 0) {
if (rejectionReasonEntries.length > 0)
return (
<TooltipProvider>
<Tooltip>
<TooltipTrigger>
<div className="flex items-center gap-2">
<Waiting /> Not enough approvals
<div className="flex items-center justify-between">
<div className="flex items-center gap-2">
<Waiting /> Not enough approvals
</div>
<ApprovalDialog {...props} onSubmit={invalidate} />
</div>
</TooltipTrigger>
<TooltipContent>
Expand All @@ -60,11 +143,13 @@ export const ApprovalCheck: React.FC<{
</Tooltip>
</TooltipProvider>
);
}

return (
<div className="flex items-center gap-2">
<Waiting /> Not enough approvals
<div className="flex items-center justify-between">
<div className="flex items-center gap-2">
<Waiting /> Not enough approvals
</div>
<ApprovalDialog {...props} onSubmit={invalidate} />
</div>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,13 @@ export const DenyWindowCheck: React.FC<{
versionId,
});

const isBlocked = data?.blocked ?? false;
const rejectionReasonEntries = Array.from(
data?.rejectionReasons.entries() ?? [],
);
const isBlocked = data ?? false;
const rejectionReasonEntries: Array<[string, string]> = [];

if (isLoading)
return (
<div className="flex items-center gap-2">
<Loading /> Loading approval status
<Loading /> Loading deny windows
</div>
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -373,29 +373,6 @@ export const EditConfiguration: React.FC<{
)
}
/>

<FormField
control={form.control}
name={`targets.${index}.deploymentSelector`}
render={({ field: { value, onChange } }) =>
value != null ? (
<FormItem>
<FormLabel>Deployment</FormLabel>
<div className="min-w-[1000px] text-sm">
<DeploymentConditionRender
condition={value}
onChange={onChange}
depth={0}
className="w-full"
/>
</div>
<FormMessage />
</FormItem>
) : (
<></>
)
}
/>
</div>

<Button
Expand Down
109 changes: 62 additions & 47 deletions packages/api/src/router/deployment-version-checks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import { z } from "zod";

import { and, eq } from "@ctrlplane/db";
import * as SCHEMA from "@ctrlplane/db/schema";
import { Channel, getQueue } from "@ctrlplane/events";
import {
denyWindows,
getVersionApprovalRules,
VersionReleaseManager,
} from "@ctrlplane/rule-engine";
Expand Down Expand Up @@ -75,6 +75,66 @@ const approvalRouter = createTRPCRouter({
};
},
),

addRecord: protectedProcedure
.input(
z.object({
deploymentVersionId: z.string().uuid(),
environmentId: z.string().uuid(),
status: z.nativeEnum(SCHEMA.ApprovalStatus),
reason: z.string().optional(),
}),
)
.meta({
authorizationCheck: ({ canUser, input }) =>
canUser.perform(Permission.DeploymentVersionGet).on({
type: "deploymentVersion",
id: input.deploymentVersionId,
}),
})
.mutation(async ({ ctx, input }) => {
const { deploymentVersionId, environmentId, status, reason } = input;

const record = await ctx.db
.insert(SCHEMA.policyRuleAnyApprovalRecord)
.values({
deploymentVersionId,
userId: ctx.session.user.id,
status,
reason,
approvedAt:
status === SCHEMA.ApprovalStatus.Approved ? new Date() : null,
})
.returning();

const rows = await ctx.db
.select()
.from(SCHEMA.deploymentVersion)
.innerJoin(
SCHEMA.releaseTarget,
eq(
SCHEMA.deploymentVersion.deploymentId,
SCHEMA.releaseTarget.deploymentId,
),
)
.where(
and(
eq(SCHEMA.deploymentVersion.id, deploymentVersionId),
eq(SCHEMA.releaseTarget.environmentId, environmentId),
),
);

const targets = rows.map((row) => row.release_target);
if (targets.length > 0)
await getQueue(Channel.EvaluateReleaseTarget).addBulk(
targets.map((rt) => ({
name: `${rt.resourceId}-${rt.environmentId}-${rt.deploymentId}`,
data: rt,
})),
);

return record;
}),
});

const denyWindowRouter = createTRPCRouter({
Expand All @@ -93,52 +153,7 @@ const denyWindowRouter = createTRPCRouter({
id: input.versionId,
}),
})
.query(
async ({ ctx, input: { versionId, environmentId, workspaceId } }) => {
const v = await ctx.db.query.deploymentVersion.findFirst({
where: eq(SCHEMA.deploymentVersion.id, versionId),
with: { metadata: true },
});

if (v == null)
throw new TRPCError({
code: "NOT_FOUND",
message: `Deployment version not found: ${versionId}`,
});

const metadata = Object.fromEntries(
v.metadata.map((m) => [m.key, m.value]),
);
const version = { ...v, metadata };

const { deploymentId } = version;
// since the resource does not affect the approval rules, we can just use any release target
// for the given deployment and environment
const rt = await ctx.db.query.releaseTarget.findFirst({
where: and(
eq(SCHEMA.releaseTarget.deploymentId, deploymentId),
eq(SCHEMA.releaseTarget.environmentId, environmentId),
),
});
if (rt == null) {
throw new TRPCError({
code: "NOT_FOUND",
message: `Release target not found: ${deploymentId} ${environmentId}`,
});
}

const releaseTarget = { ...rt, workspaceId };
const manager = new VersionReleaseManager(ctx.db, releaseTarget);
const { chosenCandidate, rejectionReasons } = await manager.evaluate({
versions: [version],
rules: denyWindows,
});
return {
blocked: chosenCandidate == null,
rejectionReasons,
};
},
),
.query(() => false),
});

export const deploymentVersionChecksRouter = createTRPCRouter({
Expand Down
5 changes: 5 additions & 0 deletions packages/db/src/schema/rules/approval-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ export const baseApprovalRecordFields = {
),
};

export enum ApprovalStatus {
Approved = "approved",
Rejected = "rejected",
}

// Base validation fields for approval records
export const baseApprovalRecordValidationFields = {
deploymentVersionId: z.string().uuid(),
Expand Down
6 changes: 5 additions & 1 deletion packages/rule-engine/src/manager/version-manager-rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,5 +69,9 @@ export const getVersionApprovalRules = (
export const getRules = (
policy: Policy | null,
): Array<FilterRule<Version> | PreValidationRule> => {
return [...denyWindows(policy), ...getVersionApprovalRules(policy)];
return getVersionApprovalRules(policy);
// The rrule package is being stupid and deny windows is not top priority
// right now so I am commenting this out
// https://github.com/jkbrzt/rrule/issues/478
// return [...denyWindows(policy), ...getVersionApprovalRules(policy)];
};
Loading