Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Init release channels ui #181

Merged
merged 3 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
@@ -0,0 +1,31 @@
import type * as SCHEMA from "@ctrlplane/db/schema";

import { Badge } from "@ctrlplane/ui/badge";

type ReleaseBadgeListProps = {
releases: {
items: SCHEMA.Release[];
total: number;
};
};

export const ReleaseBadgeList: React.FC<ReleaseBadgeListProps> = ({
releases,
}) => (
<div className="flex gap-1">
{releases.items.map((release) => (
<Badge key={release.id} variant="outline">
<span className="truncate text-xs text-muted-foreground">
{release.name}
</span>
</Badge>
))}
{releases.total > releases.items.length && (
<Badge variant="outline">
<span className="text-xs text-muted-foreground">
+{releases.total - releases.items.length}
</span>
</Badge>
)}
</div>
);
adityachoudhari26 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import type * as SCHEMA from "@ctrlplane/db/schema";
import type React from "react";
import { useRouter } from "next/navigation";
import { z } from "zod";

import { Button } from "@ctrlplane/ui/button";
import {
Form,
FormControl,
FormField,
FormItem,
FormLabel,
useForm,
} from "@ctrlplane/ui/form";
import { Input } from "@ctrlplane/ui/input";
import { Textarea } from "@ctrlplane/ui/textarea";

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

type OverviewProps = {
releaseChannel: SCHEMA.ReleaseChannel;
};

const schema = z.object({
name: z.string().min(1),
description: z.string().optional(),
});

export const Overview: React.FC<OverviewProps> = ({ releaseChannel }) => {
const defaultValues = {
name: releaseChannel.name,
description: releaseChannel.description ?? undefined,
};
const form = useForm({ schema, defaultValues });
const router = useRouter();
const utils = api.useUtils();

const updateReleaseChannel =
api.deployment.releaseChannel.update.useMutation();
const onSubmit = form.handleSubmit((data) =>
updateReleaseChannel
.mutateAsync({ id: releaseChannel.id, data })
.then(() => form.reset(data))
.then(() =>
utils.deployment.releaseChannel.byId.invalidate(releaseChannel.id),
)
.then(() => router.refresh()),
);
adityachoudhari26 marked this conversation as resolved.
Show resolved Hide resolved

return (
<Form {...form}>
<form onSubmit={onSubmit} className="space-y-6 p-6">
<FormField
control={form.control}
name="name"
render={({ field }) => (
<FormItem>
<FormLabel>Name</FormLabel>
<FormControl>
<Input {...field} />
</FormControl>
</FormItem>
)}
/>

<FormField
control={form.control}
name="description"
render={({ field }) => (
<FormItem>
<FormLabel>Description</FormLabel>
<FormControl>
<Textarea {...field} />
</FormControl>
</FormItem>
)}
/>

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 error message display in the form

The form should display validation and submission errors to users.

 <form onSubmit={onSubmit} className="space-y-6 p-6">
+  {error && (
+    <div className="text-red-500 text-sm mb-4">
+      {error}
+    </div>
+  )}
   <FormField
     // ... existing form fields
   />

Committable suggestion was skipped due to low confidence.

<Button type="submit">Save</Button>
</form>
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 29, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance submit button with loading state

The submit button should indicate when the form is being submitted.

-<Button type="submit">Save</Button>
+<Button 
+  type="submit" 
+  disabled={updateReleaseChannel.isLoading || !form.formState.isDirty}
+>
+  {updateReleaseChannel.isLoading ? 'Saving...' : 'Save'}
+</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
<Button type="submit">Save</Button>
</form>
<Button
type="submit"
disabled={updateReleaseChannel.isLoading || !form.formState.isDirty}
>
{updateReleaseChannel.isLoading ? 'Saving...' : 'Save'}
</Button>
</form>

Copy link
Member

Choose a reason for hiding this comment

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

I like this

Copy link
Contributor

Choose a reason for hiding this comment

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

@jsbroks, glad you like this! (^ᵕ^)/

</Form>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
"use client";

import type React from "react";
import { IconDotsVertical, IconLoader2 } from "@tabler/icons-react";

import { Button } from "@ctrlplane/ui/button";
import { Drawer, DrawerContent, DrawerTitle } from "@ctrlplane/ui/drawer";
import { Separator } from "@ctrlplane/ui/separator";

import { api } from "~/trpc/react";
import { Overview } from "./Overview";
import { ReleaseChannelDropdown } from "./ReleaseChannelDropdown";
import { ReleaseFilter } from "./ReleaseFilter";
import { useReleaseChannelDrawer } from "./useReleaseChannelDrawer";

export const ReleaseChannelDrawer: React.FC = () => {
const { releaseChannelId, removeReleaseChannelId } =
useReleaseChannelDrawer();
const isOpen = releaseChannelId != null && releaseChannelId != "";
const setIsOpen = removeReleaseChannelId;

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

Simplify the isOpen condition for better type safety.

The current condition could be simplified to be more type-safe and readable.

-  const isOpen = releaseChannelId != null && releaseChannelId != "";
+  const isOpen = Boolean(releaseChannelId);
📝 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 ReleaseChannelDrawer: React.FC = () => {
const { releaseChannelId, removeReleaseChannelId } =
useReleaseChannelDrawer();
const isOpen = releaseChannelId != null && releaseChannelId != "";
const setIsOpen = removeReleaseChannelId;
export const ReleaseChannelDrawer: React.FC = () => {
const { releaseChannelId, removeReleaseChannelId } =
useReleaseChannelDrawer();
const isOpen = Boolean(releaseChannelId);
const setIsOpen = removeReleaseChannelId;

const releaseChannelQ = api.deployment.releaseChannel.byId.useQuery(
releaseChannelId ?? "",
{ enabled: isOpen },
);
adityachoudhari26 marked this conversation as resolved.
Show resolved Hide resolved
const releaseChannel = releaseChannelQ.data;

const filter = releaseChannel?.releaseFilter ?? undefined;
const deploymentId = releaseChannel?.deploymentId ?? "";
const releasesQ = api.release.list.useQuery(
{ deploymentId, filter },
{ enabled: isOpen && releaseChannel != null },
);
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Prevent potential race condition in releases query.

The query might run with an empty deploymentId. Consider adding it to the enabled condition.

   const releasesQ = api.release.list.useQuery(
     { deploymentId, filter },
-    { enabled: isOpen && releaseChannel != null },
+    { enabled: isOpen && releaseChannel != null && deploymentId !== "" },
   );
📝 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 filter = releaseChannel?.releaseFilter ?? undefined;
const deploymentId = releaseChannel?.deploymentId ?? "";
const releasesQ = api.release.list.useQuery(
{ deploymentId, filter },
{ enabled: isOpen && releaseChannel != null },
);
const filter = releaseChannel?.releaseFilter ?? undefined;
const deploymentId = releaseChannel?.deploymentId ?? "";
const releasesQ = api.release.list.useQuery(
{ deploymentId, filter },
{ enabled: isOpen && releaseChannel != null && deploymentId !== "" },
);


const loading = releaseChannelQ.isLoading || releasesQ.isLoading;

return (
<Drawer open={isOpen} onOpenChange={setIsOpen}>
<DrawerContent
showBar={false}
className="scrollbar-thin scrollbar-thumb-neutral-800 scrollbar-track-neutral-900 left-auto right-0 top-0 mt-0 h-screen w-1/3 overflow-auto rounded-none focus-visible:outline-none"
>
adityachoudhari26 marked this conversation as resolved.
Show resolved Hide resolved
{loading && (
<div className="flex h-full w-full items-center justify-center">
<IconLoader2 className="h-8 w-8 animate-spin" />
</div>
)}
{!loading && releaseChannel != null && (
<>
<DrawerTitle className="flex items-center gap-2 border-b p-6">
{releaseChannel.name}
<ReleaseChannelDropdown releaseChannelId={releaseChannel.id}>
<Button variant="ghost" size="icon" className="h-6 w-6">
<IconDotsVertical className="h-4 w-4" />
</Button>
</ReleaseChannelDropdown>
</DrawerTitle>

<div className="flex flex-col">
<Overview releaseChannel={releaseChannel} />
<Separator />
<ReleaseFilter releaseChannel={releaseChannel} />
</div>
</>
)}
</DrawerContent>
</Drawer>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
import React, { useState } from "react";
import { useRouter } from "next/navigation";
import { IconTrash } from "@tabler/icons-react";

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

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

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

const DeleteReleaseChannelDialog: React.FC<DeleteReleaseChannelDialogProps> = ({
releaseChannelId,
onClose,
children,
}) => {
const [open, setOpen] = useState(false);
const { removeReleaseChannelId } = useReleaseChannelDrawer();
const router = useRouter();
const deleteReleaseChannel =
api.deployment.releaseChannel.delete.useMutation();
const onDelete = () =>
deleteReleaseChannel
.mutateAsync(releaseChannelId)
.then(() => removeReleaseChannelId())
.then(() => router.refresh())
.then(() => setOpen(false));
Comment on lines +42 to +48
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error handling and loading state for deletion.

The deletion mutation chain lacks error handling and user feedback. Consider these improvements:

Apply this diff to enhance the error handling and user experience:

 const deleteReleaseChannel =
   api.deployment.releaseChannel.delete.useMutation();
+const [isDeleting, setIsDeleting] = useState(false);
 const onDelete = () => {
+  setIsDeleting(true);
   deleteReleaseChannel
     .mutateAsync(releaseChannelId)
     .then(() => removeReleaseChannelId())
     .then(() => router.refresh())
-    .then(() => setOpen(false));
+    .then(() => setOpen(false))
+    .catch((error) => {
+      console.error('Failed to delete release channel:', error);
+      // Show error toast/notification to user
+    })
+    .finally(() => setIsDeleting(false));
 };

Also update the Delete button to show loading state:

 <AlertDialogAction
   onClick={onDelete}
+  disabled={isDeleting}
   className={buttonVariants({ variant: "destructive" })}
 >
-  Delete
+  {isDeleting ? "Deleting..." : "Delete"}
 </AlertDialogAction>

Committable suggestion was skipped due to low confidence.

Comment on lines +43 to +48
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Enhance error handling and user feedback for deletion.

The deletion operation needs better error handling and user feedback:

  1. Add error handling with user notification
  2. Update the button text to reflect the loading state
  3. Consider adding a toast notification for successful deletion

Apply this diff to enhance the user experience:

 const onDelete = () =>
   deleteReleaseChannel
     .mutateAsync(releaseChannelId)
     .then(() => removeReleaseChannelId())
     .then(() => router.refresh())
-    .then(() => setOpen(false));
+    .then(() => {
+      setOpen(false);
+      toast.success('Release channel deleted successfully');
+    })
+    .catch((error) => {
+      toast.error('Failed to delete release channel');
+      console.error('Delete error:', error);
+    });
 <AlertDialogAction
   onClick={onDelete}
   disabled={deleteReleaseChannel.isPending}
   className={buttonVariants({ variant: "destructive" })}
 >
-  Delete
+  {deleteReleaseChannel.isPending ? "Deleting..." : "Delete"}
 </AlertDialogAction>
📝 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 = () =>
deleteReleaseChannel
.mutateAsync(releaseChannelId)
.then(() => removeReleaseChannelId())
.then(() => router.refresh())
.then(() => setOpen(false));
const onDelete = () =>
deleteReleaseChannel
.mutateAsync(releaseChannelId)
.then(() => removeReleaseChannelId())
.then(() => router.refresh())
.then(() => {
setOpen(false);
toast.success('Release channel deleted successfully');
})
.catch((error) => {
toast.error('Failed to delete release channel');
console.error('Delete error:', error);
});
<AlertDialogAction
onClick={onDelete}
disabled={deleteReleaseChannel.isPending}
className={buttonVariants({ variant: "destructive" })}
>
{deleteReleaseChannel.isPending ? "Deleting..." : "Delete"}
</AlertDialogAction>


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 release channel?
</AlertDialogTitle>
<AlertDialogDescription>
This action cannot be undone.
</AlertDialogDescription>
</AlertDialogHeader>
<AlertDialogFooter>
<AlertDialogCancel>Cancel</AlertDialogCancel>
<div className="flex-grow" />
<AlertDialogAction
onClick={onDelete}
className={buttonVariants({ variant: "destructive" })}
>
Delete
</AlertDialogAction>
</AlertDialogFooter>
</AlertDialogContent>
</AlertDialog>
);
};

type ReleaseChannelDropdownProps = {
releaseChannelId: string;
children: React.ReactNode;
};

export const ReleaseChannelDropdown: React.FC<ReleaseChannelDropdownProps> = ({
releaseChannelId,
children,
}) => {
const [open, setOpen] = useState(false);
return (
<DropdownMenu open={open} onOpenChange={setOpen}>
<DropdownMenuTrigger asChild>{children}</DropdownMenuTrigger>
<DropdownMenuContent>
<DeleteReleaseChannelDialog
releaseChannelId={releaseChannelId}
onClose={() => setOpen(false)}
>
<DropdownMenuItem
className="flex cursor-pointer items-center gap-2"
onSelect={(e) => e.preventDefault()}
>
<IconTrash className="h-4 w-4 text-red-500" />
Delete
</DropdownMenuItem>
</DeleteReleaseChannelDialog>
</DropdownMenuContent>
adityachoudhari26 marked this conversation as resolved.
Show resolved Hide resolved
</DropdownMenu>
);
};
Loading
Loading