-
Notifications
You must be signed in to change notification settings - Fork 11
fix: Init handling lifecycle hooks #194
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
Changes from all commits
c785bf3
926c57a
66b538d
7c68ed5
00f5ab3
b3411b4
cd7ba79
eeee7c5
d172eff
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 |
|---|---|---|
| @@ -1,10 +1,9 @@ | ||
| import _ from "lodash"; | ||
| import { isPresent } from "ts-is-present"; | ||
|
|
||
| import { eq, inArray, lte } from "@ctrlplane/db"; | ||
| import { db } from "@ctrlplane/db/client"; | ||
| import * as SCHEMA from "@ctrlplane/db/schema"; | ||
| import { logger } from "@ctrlplane/logger"; | ||
| import { handleTargetsFromEnvironmentToBeDeleted } from "@ctrlplane/job-dispatch"; | ||
|
|
||
| type QueryRow = { | ||
| environment: SCHEMA.Environment; | ||
|
|
@@ -32,22 +31,11 @@ export const run = async () => { | |
| .then(groupByEnvironment); | ||
| if (expiredEnvironments.length === 0) return; | ||
|
|
||
| const targetPromises = expiredEnvironments | ||
| .filter((env) => isPresent(env.targetFilter)) | ||
| .map(async (env) => { | ||
| const targets = await db | ||
| .select() | ||
| .from(SCHEMA.target) | ||
| .where(SCHEMA.targetMatchesMetadata(db, env.targetFilter)); | ||
|
|
||
| return { environmentId: env.id, targets }; | ||
| }); | ||
| const associatedTargets = await Promise.all(targetPromises); | ||
|
|
||
| for (const { environmentId, targets } of associatedTargets) | ||
| logger.info( | ||
| `[${targets.length}] targets are associated with expired environment [${environmentId}]`, | ||
| ); | ||
| await Promise.all( | ||
| expiredEnvironments.map((env) => | ||
| handleTargetsFromEnvironmentToBeDeleted(db, env), | ||
| ), | ||
| ); | ||
|
Comment on lines
+34
to
+38
Contributor
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. 💡 Codebase verification 🛠️ Refactor suggestion The shell script results show that Consider using Promise.allSettled with proper error handling, following established patterns - await Promise.all(
- expiredEnvironments.map((env) =>
- handleTargetsFromEnvironmentToBeDeleted(db, env),
- ),
- );
+ const results = await Promise.allSettled(
+ expiredEnvironments.map((env) =>
+ handleTargetsFromEnvironmentToBeDeleted(db, env)
+ .catch(error => {
+ console.error(`Failed to process environment ${env.id}:`, error);
+ return { environmentId: env.id, error };
+ })
+ ),
+ );
+
+ const failures = results.filter(
+ (result): result is PromiseRejectedResult => result.status === 'rejected'
+ ).length;
+
+ if (failures > 0) {
+ console.error(`Failed to process ${failures} out of ${expiredEnvironments.length} environments`);
+ }🔗 Analysis chainConsider adding error handling for concurrent operations. While using
Consider this more robust implementation: - await Promise.all(
- expiredEnvironments.map((env) =>
- handleTargetsFromEnvironmentToBeDeleted(db, env),
- ),
- );
+ const results = await Promise.allSettled(
+ expiredEnvironments.map((env) =>
+ handleTargetsFromEnvironmentToBeDeleted(db, env)
+ .catch(error => {
+ console.error(`Failed to handle targets for environment ${env.id}:`, error);
+ return { environmentId: env.id, error };
+ })
+ ),
+ );
+
+ const failures = results.filter(
+ (result): result is PromiseRejectedResult => result.status === 'rejected'
+ );
+
+ if (failures.length > 0) {
+ console.error(`Failed to process ${failures.length} environments`);
+ }Let's verify the potential impact of concurrent operations: 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check for other uses of Promise.all with database operations
rg -l "Promise\.all.*db\." --type ts
# Look for any existing error handling patterns in similar operations
ast-grep --pattern 'Promise.allSettled($$$)'
Length of output: 6178 |
||
|
|
||
| const envIds = expiredEnvironments.map((env) => env.id); | ||
| await db | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,97 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "use client"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type * as SCHEMA from "@ctrlplane/db/schema"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useState } from "react"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useRouter } from "next/navigation"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { z } from "zod"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { Button } from "@ctrlplane/ui/button"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Dialog, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| DialogContent, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| DialogFooter, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| DialogHeader, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| DialogTitle, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| DialogTrigger, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } from "@ctrlplane/ui/dialog"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Form, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| FormField, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| FormItem, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| FormLabel, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| useForm, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } from "@ctrlplane/ui/form"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Select, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| SelectContent, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| SelectItem, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| SelectTrigger, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| SelectValue, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } from "@ctrlplane/ui/select"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { api } from "~/trpc/react"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type CreateLifecycleHookDialogProps = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| deploymentId: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| runbooks: SCHEMA.Runbook[]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| children: React.ReactNode; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const schema = z.object({ runbookId: z.string() }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export const CreateLifecycleHookDialog: React.FC< | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| CreateLifecycleHookDialogProps | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| > = ({ deploymentId, runbooks, children }) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [open, setOpen] = useState(false); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const createLifecycleHook = api.deployment.lifecycleHook.create.useMutation(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const router = useRouter(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const form = useForm({ schema }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const onSubmit = form.handleSubmit((data) => | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| createLifecycleHook | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .mutateAsync({ deploymentId, ...data }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .then(() => form.reset()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .then(() => router.refresh()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .then(() => setOpen(false)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+49
to
+55
Contributor
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 form submission. The form submission lacks proper error handling and user feedback:
const onSubmit = form.handleSubmit((data) =>
createLifecycleHook
.mutateAsync({ deploymentId, ...data })
.then(() => form.reset())
.then(() => router.refresh())
- .then(() => setOpen(false)),
+ .then(() => {
+ setOpen(false);
+ // Add success toast notification
+ })
+ .catch((error) => {
+ // Add error toast notification
+ console.error('Failed to create lifecycle hook:', error);
+ }),
);
Comment on lines
+49
to
+55
Contributor
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 management. The form submission lacks error handling and loading state management. This could lead to a poor user experience if the API call fails. Consider implementing:
+ const [isSubmitting, setIsSubmitting] = useState(false);
const onSubmit = form.handleSubmit((data) => {
+ setIsSubmitting(true);
createLifecycleHook
.mutateAsync({ deploymentId, ...data })
.then(() => form.reset())
.then(() => router.refresh())
- .then(() => setOpen(false)),
+ .then(() => setOpen(false))
+ .catch((error) => {
+ console.error('Failed to create lifecycle hook:', error);
+ // Show error to user using your preferred toast/alert component
+ })
+ .finally(() => setIsSubmitting(false));
});📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <Dialog open={open} onOpenChange={setOpen}> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <DialogTrigger asChild>{children}</DialogTrigger> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <DialogContent> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <DialogHeader> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <DialogTitle>Create Lifecycle Hook</DialogTitle> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </DialogHeader> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <Form {...form}> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <form onSubmit={onSubmit} className="space-y-4"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <FormField | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| control={form.control} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name="runbookId" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| render={({ field: { value, onChange } }) => ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <FormItem> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <FormLabel>Runbook</FormLabel> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <Select value={value} onValueChange={onChange}> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <SelectTrigger> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <SelectValue placeholder="Select a runbook" /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </SelectTrigger> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <SelectContent> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {runbooks.map((runbook) => ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <SelectItem key={runbook.id} value={runbook.id}> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {runbook.name} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </SelectItem> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ))} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </SelectContent> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </Select> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </FormItem> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| )} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+65
to
+86
Contributor
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 Display form validation state to users. The form doesn't show validation errors to users, which could lead to confusion about why the form submission isn't working. Add error messages to the form: <FormItem>
<FormLabel>Runbook</FormLabel>
<Select value={value} onValueChange={onChange}>
<SelectTrigger>
<SelectValue placeholder="Select a runbook" />
</SelectTrigger>
<SelectContent>
{runbooks.map((runbook) => (
<SelectItem key={runbook.id} value={runbook.id}>
{runbook.name}
</SelectItem>
))}
</SelectContent>
</Select>
+ {form.formState.errors.runbookId && (
+ <div className="text-sm text-red-500">
+ {form.formState.errors.runbookId.message}
+ </div>
+ )}
</FormItem>📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <DialogFooter> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <Button type="submit" disabled={createLifecycleHook.isPending}> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Create | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </Button> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </DialogFooter> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </form> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </Form> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </DialogContent> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </Dialog> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| import type * as SCHEMA from "@ctrlplane/db/schema"; | ||
| import { 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"; | ||
|
|
||
| type DeleteLifecycleHookDialogProps = { | ||
| lifecycleHookId: string; | ||
| onClose: () => void; | ||
| children: React.ReactNode; | ||
| }; | ||
|
|
||
| const DeleteLifecycleHookDialog: React.FC<DeleteLifecycleHookDialogProps> = ({ | ||
| lifecycleHookId, | ||
| onClose, | ||
| children, | ||
| }) => { | ||
| const [open, setOpen] = useState(false); | ||
| const router = useRouter(); | ||
| const deleteLifecycleHook = api.deployment.lifecycleHook.delete.useMutation(); | ||
|
|
||
| const onDelete = async () => | ||
| deleteLifecycleHook | ||
| .mutateAsync(lifecycleHookId) | ||
| .then(() => router.refresh()) | ||
| .then(() => setOpen(false)); | ||
|
Comment on lines
+42
to
+46
Contributor
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 Add error handling and improve promise chain. The deletion logic has the following issues:
Consider this improvement: - const onDelete = async () =>
- deleteLifecycleHook
- .mutateAsync(lifecycleHookId)
- .then(() => router.refresh())
- .then(() => setOpen(false));
+ const onDelete = async () => {
+ try {
+ await deleteLifecycleHook.mutateAsync(lifecycleHookId);
+ router.refresh();
+ setOpen(false);
+ onClose();
+ } catch (error) {
+ console.error('Failed to delete lifecycle 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>Delete Lifecycle Hook?</AlertDialogTitle> | ||
| <AlertDialogDescription> | ||
| This action cannot be undone. | ||
| </AlertDialogDescription> | ||
| </AlertDialogHeader> | ||
| <AlertDialogFooter> | ||
| <AlertDialogCancel>Cancel</AlertDialogCancel> | ||
| <div className="flex-grow" /> | ||
| <AlertDialogAction | ||
| className={buttonVariants({ variant: "destructive" })} | ||
| onClick={onDelete} | ||
| disabled={deleteLifecycleHook.isPending} | ||
| > | ||
| Delete | ||
| </AlertDialogAction> | ||
| </AlertDialogFooter> | ||
| </AlertDialogContent> | ||
| </AlertDialog> | ||
| ); | ||
| }; | ||
|
|
||
| type LifecycleHookActionsDropdownProps = { | ||
| lifecycleHook: SCHEMA.DeploymentLifecycleHook; | ||
| children: React.ReactNode; | ||
| }; | ||
|
|
||
| export const LifecycleHookActionsDropdown: React.FC< | ||
| LifecycleHookActionsDropdownProps | ||
| > = ({ lifecycleHook, children }) => { | ||
| const [open, setOpen] = useState(false); | ||
| return ( | ||
| <DropdownMenu open={open} onOpenChange={setOpen}> | ||
| <DropdownMenuTrigger asChild>{children}</DropdownMenuTrigger> | ||
| <DropdownMenuContent> | ||
| <DeleteLifecycleHookDialog | ||
| lifecycleHookId={lifecycleHook.id} | ||
| onClose={() => setOpen(false)} | ||
| > | ||
| <DropdownMenuItem | ||
| className="flex items-center gap-2" | ||
| onSelect={(e) => e.preventDefault()} | ||
| > | ||
| Delete | ||
| <IconTrash size="icon" className="h-4 w-4 text-destructive" /> | ||
| </DropdownMenuItem> | ||
| </DeleteLifecycleHookDialog> | ||
| </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.
🛠️ Refactor suggestion
Add error handling and logging for reliability.
The concurrent processing of environments is good for performance, but there are some reliability concerns:
Consider applying these improvements:
📝 Committable suggestion