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
Expand Up @@ -54,16 +54,19 @@ export const DeploymentNavBar: React.FC<DeploymentNavBarProps> = ({
const variablesUrl = `/${workspaceSlug}/systems/${systemSlug}/deployments/${deploymentSlug}/variables`;
const releaseChannelsUrl = `/${workspaceSlug}/systems/${systemSlug}/deployments/${deploymentSlug}/release-channels`;
const overviewUrl = `/${workspaceSlug}/systems/${systemSlug}/deployments/${deploymentSlug}`;
const hooksUrl = `/${workspaceSlug}/systems/${systemSlug}/deployments/${deploymentSlug}/hooks`;

const isReleasesActive = pathname.includes("/releases");
const isVariablesActive = pathname.includes("/variables");
const isJobsActive = pathname.includes("/jobs");
const isReleaseChannelsActive = pathname.includes("/release-channels");
const isHooksActive = pathname.includes("/hooks");
const isSettingsActive =
!isReleasesActive &&
!isVariablesActive &&
!isJobsActive &&
!isReleaseChannelsActive;
!isReleaseChannelsActive &&
!isHooksActive;

return (
<div className="flex items-center justify-between border-b p-2">
Expand Down Expand Up @@ -116,6 +119,16 @@ export const DeploymentNavBar: React.FC<DeploymentNavBarProps> = ({
</NavigationMenuLink>
</Link>
</NavigationMenuItem>
<NavigationMenuItem>
<Link href={hooksUrl} legacyBehavior passHref>
<NavigationMenuLink
className="group inline-flex h-9 w-max items-center justify-center rounded-md px-4 py-2 text-sm font-medium transition-colors hover:bg-accent/50 hover:text-accent-foreground focus:outline-none disabled:pointer-events-none disabled:opacity-50 data-[active]:bg-accent/50 data-[state=open]:bg-accent/50"
active={isHooksActive}
>
Hooks
</NavigationMenuLink>
</Link>
</NavigationMenuItem>
<NavigationMenuItem>
<Link href={overviewUrl} legacyBehavior passHref>
<NavigationMenuLink
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { Button } from "@ctrlplane/ui/button";

import { CreateReleaseDialog } from "~/app/[workspaceSlug]/_components/CreateRelease";
import { api } from "~/trpc/react";
import { CreateHookDialog } from "./hooks/CreateHookDialog";
import { CreateReleaseChannelDialog } from "./release-channels/CreateReleaseChannelDialog";
import { CreateVariableDialog } from "./releases/CreateVariableDialog";

Expand All @@ -17,11 +18,15 @@ export const NavigationMenuAction: React.FC<{
const pathname = usePathname();
const isVariablesActive = pathname.includes("variables");
const isReleaseChannelsActive = pathname.includes("release-channels");
const isHooksActive = pathname.includes("hooks");

const releaseChannelsQ =
api.deployment.releaseChannel.list.byDeploymentId.useQuery(deploymentId);
const releaseChannels = releaseChannelsQ.data ?? [];

const runbooksQ = api.runbook.bySystemId.useQuery(systemId);
const runbooks = runbooksQ.data ?? [];

return (
<div>
{isVariablesActive && (
Expand All @@ -43,7 +48,15 @@ export const NavigationMenuAction: React.FC<{
</CreateReleaseChannelDialog>
)}

{!isVariablesActive && !isReleaseChannelsActive && (
{isHooksActive && (
<CreateHookDialog deploymentId={deploymentId} runbooks={runbooks}>
<Button size="sm" variant="secondary">
New Hook
</Button>
</CreateHookDialog>
)}

{!isVariablesActive && !isReleaseChannelsActive && !isHooksActive && (
<CreateReleaseDialog deploymentId={deploymentId} systemId={systemId}>
<Button size="sm" variant="secondary">
New Release
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,232 @@
"use client";

import type * as SCHEMA from "@ctrlplane/db/schema";
import { useState } from "react";
import { useRouter } from "next/navigation";
import { IconPlus, IconSelector, IconX } from "@tabler/icons-react";
import { z } from "zod";

import { Badge } from "@ctrlplane/ui/badge";
import { Button } from "@ctrlplane/ui/button";
import {
Command,
CommandInput,
CommandItem,
CommandList,
} from "@ctrlplane/ui/command";
import {
Dialog,
DialogContent,
DialogDescription,
DialogFooter,
DialogHeader,
DialogTitle,
DialogTrigger,
} from "@ctrlplane/ui/dialog";
import {
Form,
FormControl,
FormField,
FormItem,
FormLabel,
FormMessage,
useFieldArray,
useForm,
} from "@ctrlplane/ui/form";
import { Input } from "@ctrlplane/ui/input";
import { Label } from "@ctrlplane/ui/label";
import { Popover, PopoverContent, PopoverTrigger } from "@ctrlplane/ui/popover";
import { hookActions, hookActionsList } from "@ctrlplane/validators/events";

import { api } from "~/trpc/react";

type CreateHookDialogProps = {
deploymentId: string;
runbooks: SCHEMA.Runbook[];
children: React.ReactNode;
};

const schema = z.object({
name: z.string().min(1),
action: hookActions,
runbookIds: z.array(z.object({ id: z.string().uuid() })),
});

export const CreateHookDialog: React.FC<CreateHookDialogProps> = ({
deploymentId,
runbooks,
children,
}) => {
const [open, setOpen] = useState(false);
const [actionsOpen, setActionsOpen] = useState(false);
const [runbooksOpen, setRunbooksOpen] = useState(false);
const createHook = api.deployment.hook.create.useMutation();
const utils = api.useUtils();
const router = useRouter();

Comment on lines +55 to +66
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

Consider improving state management and error handling.

  1. The multiple dialog states could be consolidated into a single state object for better maintainability.
  2. Add error handling for failed mutations to provide better user feedback.
-  const [open, setOpen] = useState(false);
-  const [actionsOpen, setActionsOpen] = useState(false);
-  const [runbooksOpen, setRunbooksOpen] = useState(false);
+  const [dialogState, setDialogState] = useState({
+    main: false,
+    actions: false,
+    runbooks: false
+  });
+  const [error, setError] = useState<string | null>(null);
   const createHook = api.deployment.hook.create.useMutation({
+    onError: (err) => {
+      setError(err.message);
+    }
   });
📝 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
export const CreateHookDialog: React.FC<CreateHookDialogProps> = ({
deploymentId,
runbooks,
children,
}) => {
const [open, setOpen] = useState(false);
const [actionsOpen, setActionsOpen] = useState(false);
const [runbooksOpen, setRunbooksOpen] = useState(false);
const createHook = api.deployment.hook.create.useMutation();
const utils = api.useUtils();
const router = useRouter();
export const CreateHookDialog: React.FC<CreateHookDialogProps> = ({
deploymentId,
runbooks,
children,
}) => {
const [dialogState, setDialogState] = useState({
main: false,
actions: false,
runbooks: false
});
const [error, setError] = useState<string | null>(null);
const createHook = api.deployment.hook.create.useMutation({
onError: (err) => {
setError(err.message);
}
});
const utils = api.useUtils();
const router = useRouter();

const defaultValues = { name: "", action: "", runbookIds: [] };
const form = useForm({ schema, defaultValues });
const onSubmit = form.handleSubmit((data) =>
createHook
.mutateAsync({
...data,
scopeType: "deployment",
scopeId: deploymentId,
runbookIds: data.runbookIds.map((r) => r.id),
})
.then(() => utils.deployment.hook.list.invalidate(deploymentId))
.then(() => router.refresh())
.then(() => setOpen(false)),
);

const { fields, append, remove } = useFieldArray({
control: form.control,
name: "runbookIds",
});

const selectedRunbookIds = form.watch("runbookIds").map((r) => r.id);
const unselectedRunbooks = runbooks.filter(
(r) => !selectedRunbookIds.includes(r.id),
);

return (
<Dialog open={open} onOpenChange={setOpen}>
<DialogTrigger asChild>{children}</DialogTrigger>
<DialogContent>
<DialogHeader>
<DialogTitle>Create Hook</DialogTitle>
<DialogDescription>
Trigger actions for events in this deployment.
</DialogDescription>
</DialogHeader>
<Form {...form}>
<form onSubmit={onSubmit} className="space-y-4">
<FormField
control={form.control}
name="name"
render={({ field }) => (
<FormItem>
<FormLabel>Name</FormLabel>
<FormControl>
<Input {...field} />
</FormControl>
<FormMessage />
</FormItem>
)}
/>

<FormField
control={form.control}
name="action"
render={({ field: { onChange, value } }) => (
<FormItem className="flex flex-col gap-2">
<FormLabel>Action</FormLabel>
<FormControl>
<Popover open={actionsOpen} onOpenChange={setActionsOpen}>
<PopoverTrigger asChild>
<Button
variant="outline"
role="combobox"
aria-expanded={actionsOpen}
className="items-center justify-start gap-2 px-2"
>
<IconSelector className="h-4 w-4" />
{value === "" ? "Select action..." : value}
</Button>
</PopoverTrigger>
<PopoverContent align="start" className="w-[462px] p-0">
<Command>
<CommandInput placeholder="Search action..." />
<CommandList>
{hookActionsList.map((action) => (
<CommandItem
key={action}
value={action}
onSelect={() => {
onChange(action);
setActionsOpen(false);
}}
>
{action}
</CommandItem>
))}
</CommandList>
</Command>
</PopoverContent>
</Popover>
</FormControl>
</FormItem>
)}
/>

<div className="flex flex-col gap-2">
<Label>Runbooks</Label>
<div className="flex flex-wrap gap-2">
{fields.map((field, index) => (
<FormField
key={field.id}
control={form.control}
name={`runbookIds.${index}.id`}
render={({ field }) => {
const runbook = runbooks.find(
(r) => r.id === field.value,
);
return (
<Badge
variant="outline"
className="flex items-center gap-2"
>
{runbook?.name ?? ""}
<IconX
className="h-4 w-4 cursor-pointer"
onClick={() => remove(index)}
/>
</Badge>
);
}}
/>
))}
</div>
</div>

<Popover open={runbooksOpen} onOpenChange={setRunbooksOpen}>
<PopoverTrigger asChild>
<Button variant="outline" className="flex items-center gap-2">
<IconPlus className="h-4 w-4" />
Add Runbook
</Button>
</PopoverTrigger>
<PopoverContent align="start" className="p-0">
<Command>
<CommandInput placeholder="Search runbook..." />
<CommandList>
{unselectedRunbooks.map((runbook) => (
<CommandItem
key={runbook.id}
onSelect={() => {
append({ id: runbook.id });
setRunbooksOpen(false);
}}
>
{runbook.name}
</CommandItem>
))}
</CommandList>
</Command>
</PopoverContent>
</Popover>

<DialogFooter>
<Button
type="submit"
disabled={createHook.isPending || !form.formState.isDirty}
>
Create
</Button>
</DialogFooter>
</form>
</Form>
</DialogContent>
</Dialog>
);
};
Comment on lines +92 to +232
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

Enhance accessibility and responsive design.

Consider the following improvements:

  1. Add aria-labels to interactive elements
  2. Improve keyboard navigation
  3. Enhance mobile responsiveness
   <Dialog open={open} onOpenChange={setOpen}>
-    <DialogTrigger asChild>{children}</DialogTrigger>
+    <DialogTrigger asChild aria-label="Open create hook dialog">{children}</DialogTrigger>
     <DialogContent>
       <DialogHeader>
         <DialogTitle>Create Hook</DialogTitle>
         <DialogDescription>
           Trigger actions for events in this deployment.
         </DialogDescription>
       </DialogHeader>
-      <Form {...form}>
+      <Form {...form} className="max-w-full">
         <form onSubmit={onSubmit} className="space-y-4">
           // ... existing form fields ...
           <div className="flex flex-col gap-2">
-            <div className="flex flex-wrap gap-2">
+            <div className="flex flex-wrap gap-2 max-h-[200px] overflow-y-auto">
               // ... existing badge list ...
             </div>
           </div>
           // ... rest of the form ...
         </form>
       </Form>
     </DialogContent>
   </Dialog>

Committable suggestion skipped: line range outside the PR's diff.

Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
"use client";

import { useState } from "react";
import { useRouter } from "next/navigation";

import {
AlertDialog,
AlertDialogAction,
AlertDialogCancel,
AlertDialogContent,
AlertDialogDescription,
AlertDialogFooter,
AlertDialogHeader,
AlertDialogTitle,
AlertDialogTrigger,
} from "@ctrlplane/ui/alert-dialog";
import { buttonVariants } from "@ctrlplane/ui/button";

import { api } from "~/trpc/react";

type DeleteHookDialogProps = {
hookId: string;
onClose: () => void;
children: React.ReactNode;
};

export const DeleteHookDialog: React.FC<DeleteHookDialogProps> = ({
hookId,
onClose,
children,
}) => {
const [open, setOpen] = useState(false);
const deleteHook = api.deployment.hook.delete.useMutation();
const utils = api.useUtils();
const router = useRouter();
Comment on lines +27 to +35
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

Consider adding error handling for the delete mutation.

While the basic setup is good, adding error handling would improve reliability and user experience.

Consider this enhancement:

 export const DeleteHookDialog: React.FC<DeleteHookDialogProps> = ({
   hookId,
   onClose,
   children,
 }) => {
   const [open, setOpen] = useState(false);
-  const deleteHook = api.deployment.hook.delete.useMutation();
+  const deleteHook = api.deployment.hook.delete.useMutation({
+    onError: (error) => {
+      console.error('Failed to delete hook:', error);
+      // Consider adding a toast notification here
+    }
+  });
   const utils = api.useUtils();
   const router = useRouter();

Committable suggestion skipped: line range outside the PR's diff.


const onDelete = () =>
deleteHook
.mutateAsync(hookId)
.then(() => utils.deployment.hook.list.invalidate(hookId))
.then(() => router.refresh())
.then(() => setOpen(false))
.then(() => onClose());
Comment on lines +37 to +43
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

Improve delete handler implementation.

The current implementation has several areas for improvement:

  1. The promise chain could be simplified using async/await
  2. Error handling should be added
  3. The cache invalidation might be using hookId incorrectly as it's typically used with query parameters

Consider this enhancement:

-  const onDelete = () =>
-    deleteHook
-      .mutateAsync(hookId)
-      .then(() => utils.deployment.hook.list.invalidate(hookId))
-      .then(() => router.refresh())
-      .then(() => setOpen(false))
-      .then(() => onClose());
+  const onDelete = async () => {
+    try {
+      await deleteHook.mutateAsync(hookId);
+      await utils.deployment.hook.list.invalidate();  // Remove hookId parameter
+      router.refresh();
+      setOpen(false);
+      onClose();
+    } catch (error) {
+      console.error('Failed to delete hook:', error);
+      // Consider adding a toast notification here
+    }
+  };
📝 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 onDelete = () =>
deleteHook
.mutateAsync(hookId)
.then(() => utils.deployment.hook.list.invalidate(hookId))
.then(() => router.refresh())
.then(() => setOpen(false))
.then(() => onClose());
const onDelete = async () => {
try {
await deleteHook.mutateAsync(hookId);
await utils.deployment.hook.list.invalidate(); // Remove hookId parameter
router.refresh();
setOpen(false);
onClose();
} catch (error) {
console.error('Failed to delete hook:', error);
// Consider adding a toast notification here
}
};


return (
<AlertDialog
open={open}
onOpenChange={(o) => {
setOpen(o);
if (!o) onClose();
}}
>
<AlertDialogTrigger asChild>{children}</AlertDialogTrigger>
<AlertDialogContent>
<AlertDialogHeader>
<AlertDialogTitle>
Are you sure you want to delete this hook?
</AlertDialogTitle>
<AlertDialogDescription>
This action cannot be undone.
</AlertDialogDescription>
</AlertDialogHeader>
<AlertDialogFooter>
<AlertDialogCancel>Cancel</AlertDialogCancel>
<AlertDialogAction
onClick={onDelete}
disabled={deleteHook.isPending}
className={buttonVariants({ variant: "destructive" })}
>
Delete
</AlertDialogAction>
</AlertDialogFooter>
</AlertDialogContent>
</AlertDialog>
);
};
Loading
Loading