-
Notifications
You must be signed in to change notification settings - Fork 1
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: Set release channels for policy and env #183
Changes from 3 commits
669029a
55e54a0
3b9603a
47c0813
b69967c
32a7d52
757cb92
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,145 @@ | ||||||||||||||||||
"use client"; | ||||||||||||||||||
|
||||||||||||||||||
import React, { useState } from "react"; | ||||||||||||||||||
import { useParams, useRouter, useSearchParams } from "next/navigation"; | ||||||||||||||||||
import { | ||||||||||||||||||
IconDotsVertical, | ||||||||||||||||||
IconFilter, | ||||||||||||||||||
IconInfoCircle, | ||||||||||||||||||
IconLoader2, | ||||||||||||||||||
IconPlant, | ||||||||||||||||||
IconTarget, | ||||||||||||||||||
} from "@tabler/icons-react"; | ||||||||||||||||||
|
||||||||||||||||||
import { Button } from "@ctrlplane/ui/button"; | ||||||||||||||||||
import { Drawer, DrawerContent, DrawerTitle } from "@ctrlplane/ui/drawer"; | ||||||||||||||||||
|
||||||||||||||||||
import { api } from "~/trpc/react"; | ||||||||||||||||||
import { TabButton } from "../TabButton"; | ||||||||||||||||||
import { EnvironmentDropdownMenu } from "./EnvironmentDropdownMenu"; | ||||||||||||||||||
import { EditFilterForm } from "./Filter"; | ||||||||||||||||||
import { Overview } from "./Overview"; | ||||||||||||||||||
import { ReleaseChannels } from "./ReleaseChannels"; | ||||||||||||||||||
|
||||||||||||||||||
const param = "environment_id"; | ||||||||||||||||||
export const useEnvironmentDrawer = () => { | ||||||||||||||||||
const router = useRouter(); | ||||||||||||||||||
const params = useSearchParams(); | ||||||||||||||||||
const environmentId = params.get(param); | ||||||||||||||||||
|
||||||||||||||||||
const setEnvironmentId = (id: string | null) => { | ||||||||||||||||||
const url = new URL(window.location.href); | ||||||||||||||||||
if (id === null) { | ||||||||||||||||||
url.searchParams.delete(param); | ||||||||||||||||||
router.replace(`${url.pathname}?${url.searchParams.toString()}`); | ||||||||||||||||||
return; | ||||||||||||||||||
} | ||||||||||||||||||
url.searchParams.set(param, id); | ||||||||||||||||||
router.replace(`${url.pathname}?${url.searchParams.toString()}`); | ||||||||||||||||||
}; | ||||||||||||||||||
|
||||||||||||||||||
const removeEnvironmentId = () => setEnvironmentId(null); | ||||||||||||||||||
|
||||||||||||||||||
return { environmentId, setEnvironmentId, removeEnvironmentId }; | ||||||||||||||||||
}; | ||||||||||||||||||
|
||||||||||||||||||
export const EnvironmentDrawer: React.FC = () => { | ||||||||||||||||||
const { environmentId, removeEnvironmentId } = useEnvironmentDrawer(); | ||||||||||||||||||
const isOpen = Boolean(environmentId); | ||||||||||||||||||
const setIsOpen = removeEnvironmentId; | ||||||||||||||||||
const environmentQ = api.environment.byId.useQuery(environmentId ?? "", { | ||||||||||||||||||
enabled: isOpen, | ||||||||||||||||||
}); | ||||||||||||||||||
const environment = environmentQ.data; | ||||||||||||||||||
|
||||||||||||||||||
const { workspaceSlug } = useParams<{ workspaceSlug: string }>(); | ||||||||||||||||||
const workspaceQ = api.workspace.bySlug.useQuery(workspaceSlug, { | ||||||||||||||||||
enabled: isOpen, | ||||||||||||||||||
}); | ||||||||||||||||||
const workspace = workspaceQ.data; | ||||||||||||||||||
|
||||||||||||||||||
const deploymentsQ = api.deployment.bySystemId.useQuery( | ||||||||||||||||||
environment?.systemId ?? "", | ||||||||||||||||||
{ enabled: isOpen && environment != null }, | ||||||||||||||||||
); | ||||||||||||||||||
const deployments = deploymentsQ.data; | ||||||||||||||||||
|
||||||||||||||||||
const [activeTab, setActiveTab] = useState("overview"); | ||||||||||||||||||
|
||||||||||||||||||
const loading = | ||||||||||||||||||
environmentQ.isLoading || workspaceQ.isLoading || deploymentsQ.isLoading; | ||||||||||||||||||
Comment on lines
+69
to
+70
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adjust With Update the -const loading =
- environmentQ.isLoading || workspaceQ.isLoading || deploymentsQ.isLoading;
+const loading =
+ environmentQ.isLoading ||
+ (workspaceQ.isLoading && workspaceQ.options.enabled) ||
+ (deploymentsQ.isLoading && deploymentsQ.options.enabled); 📝 Committable suggestion
Suggested change
|
||||||||||||||||||
|
||||||||||||||||||
return ( | ||||||||||||||||||
<Drawer open={isOpen} onOpenChange={setIsOpen}> | ||||||||||||||||||
<DrawerContent | ||||||||||||||||||
showBar={false} | ||||||||||||||||||
className="left-auto right-0 top-0 mt-0 h-screen w-2/3 max-w-6xl overflow-auto rounded-none focus-visible:outline-none" | ||||||||||||||||||
> | ||||||||||||||||||
Comment on lines
+74
to
+77
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider making drawer width more responsive. The current fixed width of 2/3 viewport with a max-width might be too wide on certain screen sizes. Consider using responsive breakpoints for better adaptability. -className="left-auto right-0 top-0 mt-0 h-screen w-2/3 max-w-6xl overflow-auto rounded-none focus-visible:outline-none"
+className="left-auto right-0 top-0 mt-0 h-screen w-full overflow-auto rounded-none focus-visible:outline-none md:w-3/4 lg:w-2/3 max-w-6xl focus-visible:outline-none" 📝 Committable suggestion
Suggested change
|
||||||||||||||||||
<DrawerTitle className="flex items-center gap-2 border-b p-6"> | ||||||||||||||||||
<div className="flex-shrink-0 rounded bg-green-500/20 p-1 text-green-400"> | ||||||||||||||||||
<IconPlant className="h-4 w-4" /> | ||||||||||||||||||
</div> | ||||||||||||||||||
{environment?.name} | ||||||||||||||||||
{environment != null && ( | ||||||||||||||||||
<EnvironmentDropdownMenu environment={environment}> | ||||||||||||||||||
<Button variant="ghost" size="icon" className="h-6 w-6"> | ||||||||||||||||||
<IconDotsVertical className="h-4 w-4" /> | ||||||||||||||||||
</Button> | ||||||||||||||||||
</EnvironmentDropdownMenu> | ||||||||||||||||||
)} | ||||||||||||||||||
</DrawerTitle> | ||||||||||||||||||
|
||||||||||||||||||
{loading && ( | ||||||||||||||||||
<div className="flex h-full items-center justify-center"> | ||||||||||||||||||
<IconLoader2 className="h-8 w-8 animate-spin" /> | ||||||||||||||||||
</div> | ||||||||||||||||||
)} | ||||||||||||||||||
|
||||||||||||||||||
{!loading && ( | ||||||||||||||||||
<div className="flex w-full gap-6 p-6"> | ||||||||||||||||||
<div className="space-y-1"> | ||||||||||||||||||
<TabButton | ||||||||||||||||||
active={activeTab === "overview"} | ||||||||||||||||||
onClick={() => setActiveTab("overview")} | ||||||||||||||||||
icon={<IconInfoCircle className="h-4 w-4" />} | ||||||||||||||||||
label="Overview" | ||||||||||||||||||
/> | ||||||||||||||||||
<TabButton | ||||||||||||||||||
active={activeTab === "targets"} | ||||||||||||||||||
onClick={() => setActiveTab("targets")} | ||||||||||||||||||
icon={<IconTarget className="h-4 w-4" />} | ||||||||||||||||||
label="Targets" | ||||||||||||||||||
/> | ||||||||||||||||||
<TabButton | ||||||||||||||||||
active={activeTab === "release-channels"} | ||||||||||||||||||
onClick={() => setActiveTab("release-channels")} | ||||||||||||||||||
icon={<IconFilter className="h-4 w-4" />} | ||||||||||||||||||
label="Release Channels" | ||||||||||||||||||
/> | ||||||||||||||||||
</div> | ||||||||||||||||||
|
||||||||||||||||||
{environment != null && ( | ||||||||||||||||||
<div className="w-full overflow-auto"> | ||||||||||||||||||
{activeTab === "overview" && ( | ||||||||||||||||||
<Overview environment={environment} /> | ||||||||||||||||||
)} | ||||||||||||||||||
{activeTab === "targets" && workspace != null && ( | ||||||||||||||||||
<EditFilterForm | ||||||||||||||||||
environment={environment} | ||||||||||||||||||
workspaceId={workspace.id} | ||||||||||||||||||
/> | ||||||||||||||||||
)} | ||||||||||||||||||
{activeTab === "release-channels" && deployments != null && ( | ||||||||||||||||||
<ReleaseChannels | ||||||||||||||||||
environment={environment} | ||||||||||||||||||
deployments={deployments} | ||||||||||||||||||
/> | ||||||||||||||||||
)} | ||||||||||||||||||
</div> | ||||||||||||||||||
)} | ||||||||||||||||||
</div> | ||||||||||||||||||
)} | ||||||||||||||||||
</DrawerContent> | ||||||||||||||||||
</Drawer> | ||||||||||||||||||
); | ||||||||||||||||||
}; |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,104 @@ | ||||||||||||||||||||||||||||||||||||
import type * as SCHEMA from "@ctrlplane/db/schema"; | ||||||||||||||||||||||||||||||||||||
import { useState } from "react"; | ||||||||||||||||||||||||||||||||||||
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 { useEnvironmentDrawer } from "./EnvironmentDrawer"; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
const DeleteEnvironmentDialog: React.FC<{ | ||||||||||||||||||||||||||||||||||||
environment: SCHEMA.Environment; | ||||||||||||||||||||||||||||||||||||
children: React.ReactNode; | ||||||||||||||||||||||||||||||||||||
onClose: () => void; | ||||||||||||||||||||||||||||||||||||
}> = ({ environment, children, onClose }) => { | ||||||||||||||||||||||||||||||||||||
Comment on lines
+27
to
+31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add type safety for required environment fields. The component assumes Consider adding a type guard or making the required fields explicit: - environment: SCHEMA.Environment;
+ environment: SCHEMA.Environment & {
+ systemId: string; // Make required fields explicit
+ }; 📝 Committable suggestion
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the type already has an explicit systemId There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @adityachoudhari26, thank you for the clarification. You're correct that (🐰💭) ✏️ Learnings added
|
||||||||||||||||||||||||||||||||||||
const [open, setOpen] = useState(false); | ||||||||||||||||||||||||||||||||||||
const deleteEnvironment = api.environment.delete.useMutation(); | ||||||||||||||||||||||||||||||||||||
const utils = api.useUtils(); | ||||||||||||||||||||||||||||||||||||
const { removeEnvironmentId } = useEnvironmentDrawer(); | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
const onDelete = () => | ||||||||||||||||||||||||||||||||||||
deleteEnvironment | ||||||||||||||||||||||||||||||||||||
.mutateAsync(environment.id) | ||||||||||||||||||||||||||||||||||||
.then(() => utils.environment.bySystemId.invalidate(environment.systemId)) | ||||||||||||||||||||||||||||||||||||
.then(() => removeEnvironmentId()) | ||||||||||||||||||||||||||||||||||||
.then(() => setOpen(false)); | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
Comment on lines
+37
to
+43
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add error handling and loading state for deletion. The deletion operation lacks error handling and loading state management, which could lead to a poor user experience if the operation fails. Consider implementing this improved version: - const onDelete = () =>
- deleteEnvironment
- .mutateAsync(environment.id)
- .then(() => utils.environment.bySystemId.invalidate(environment.systemId))
- .then(() => removeEnvironmentId())
- .then(() => setOpen(false));
+ const onDelete = async () => {
+ try {
+ await deleteEnvironment.mutateAsync(environment.id);
+ await utils.environment.bySystemId.invalidate(environment.systemId);
+ removeEnvironmentId();
+ setOpen(false);
+ } catch (error) {
+ console.error('Failed to delete environment:', error);
+ // Add error notification here
+ }
+ }; 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||||||||||
<AlertDialog | ||||||||||||||||||||||||||||||||||||
open={open} | ||||||||||||||||||||||||||||||||||||
onOpenChange={(o) => { | ||||||||||||||||||||||||||||||||||||
setOpen(o); | ||||||||||||||||||||||||||||||||||||
if (!o) onClose(); | ||||||||||||||||||||||||||||||||||||
}} | ||||||||||||||||||||||||||||||||||||
> | ||||||||||||||||||||||||||||||||||||
<AlertDialogTrigger asChild>{children}</AlertDialogTrigger> | ||||||||||||||||||||||||||||||||||||
<AlertDialogContent> | ||||||||||||||||||||||||||||||||||||
<AlertDialogHeader> | ||||||||||||||||||||||||||||||||||||
<AlertDialogTitle>Delete Environment</AlertDialogTitle> | ||||||||||||||||||||||||||||||||||||
</AlertDialogHeader> | ||||||||||||||||||||||||||||||||||||
<AlertDialogDescription> | ||||||||||||||||||||||||||||||||||||
Are you sure you want to delete this environment? You will have to | ||||||||||||||||||||||||||||||||||||
recreate it from scratch. | ||||||||||||||||||||||||||||||||||||
</AlertDialogDescription> | ||||||||||||||||||||||||||||||||||||
<AlertDialogFooter> | ||||||||||||||||||||||||||||||||||||
<AlertDialogCancel>Cancel</AlertDialogCancel> | ||||||||||||||||||||||||||||||||||||
<AlertDialogAction | ||||||||||||||||||||||||||||||||||||
onClick={onDelete} | ||||||||||||||||||||||||||||||||||||
className={buttonVariants({ variant: "destructive" })} | ||||||||||||||||||||||||||||||||||||
> | ||||||||||||||||||||||||||||||||||||
Delete | ||||||||||||||||||||||||||||||||||||
</AlertDialogAction> | ||||||||||||||||||||||||||||||||||||
</AlertDialogFooter> | ||||||||||||||||||||||||||||||||||||
</AlertDialogContent> | ||||||||||||||||||||||||||||||||||||
</AlertDialog> | ||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
type EnvironmentDropdownMenuProps = { | ||||||||||||||||||||||||||||||||||||
environment: SCHEMA.Environment; | ||||||||||||||||||||||||||||||||||||
children: React.ReactNode; | ||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
export const EnvironmentDropdownMenu: React.FC< | ||||||||||||||||||||||||||||||||||||
EnvironmentDropdownMenuProps | ||||||||||||||||||||||||||||||||||||
> = ({ environment, children }) => { | ||||||||||||||||||||||||||||||||||||
const [open, setOpen] = useState(false); | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||||||||||
<DropdownMenu open={open} onOpenChange={setOpen}> | ||||||||||||||||||||||||||||||||||||
<DropdownMenuTrigger asChild>{children}</DropdownMenuTrigger> | ||||||||||||||||||||||||||||||||||||
<DropdownMenuContent> | ||||||||||||||||||||||||||||||||||||
<DeleteEnvironmentDialog | ||||||||||||||||||||||||||||||||||||
environment={environment} | ||||||||||||||||||||||||||||||||||||
onClose={() => setOpen(false)} | ||||||||||||||||||||||||||||||||||||
> | ||||||||||||||||||||||||||||||||||||
<DropdownMenuItem | ||||||||||||||||||||||||||||||||||||
onSelect={(e) => e.preventDefault()} | ||||||||||||||||||||||||||||||||||||
className="flex cursor-pointer items-center gap-2" | ||||||||||||||||||||||||||||||||||||
> | ||||||||||||||||||||||||||||||||||||
<IconTrash className="h-4 w-4 text-red-500" /> | ||||||||||||||||||||||||||||||||||||
<span>Delete</span> | ||||||||||||||||||||||||||||||||||||
</DropdownMenuItem> | ||||||||||||||||||||||||||||||||||||
</DeleteEnvironmentDialog> | ||||||||||||||||||||||||||||||||||||
</DropdownMenuContent> | ||||||||||||||||||||||||||||||||||||
</DropdownMenu> | ||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix 'onOpenChange' handler to properly handle the open state
The
setIsOpen
function is assigned toremoveEnvironmentId
, which doesn't accept any arguments. However, theonOpenChange
handler expects a function that accepts a boolean parameter indicating the open state. This mismatch can lead to unexpected behavior when opening or closing the drawer.To fix this, modify
setIsOpen
to handle theopen
parameter correctly:📝 Committable suggestion