Uh oh!
There was an error while loading. Please reload this page.
add delete resource modal - #280
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughA new reusable DeleteResource component was added and used to replace in-component Dialog/form deletion flows across campaign, contact (two places), contact book, API key, domain, and template deletion UIs. Each deletion now uses a schema-driven "confirmation" field validated against the resource name. The packages/ui Input component’s placeholder styling was adjusted and Input is now explicitly exported. Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web/src/app/(dashboard)/templates/delete-template.tsx (1)
26-36: Await mutation; rely onisPending+ close on success.Use
mutateAsyncso the dialog can close only after success and errors can be caught.- deleteTemplateMutation.mutate(- { templateId: template.id },- {- onSuccess: () => {- utils.template.getTemplates.invalidate();- toast.success(`Template deleted`);- },- },- );+ try {+ await deleteTemplateMutation.mutateAsync({ templateId: template.id });+ await utils.template.getTemplates.invalidate();+ toast.success("Template deleted");+ } catch (e) {+ // DeleteResource will also show field error; keep toast if desired:+ // toast.error(e instanceof Error ? e.message : "Template not deleted");+ throw e; // let DeleteResource map to field error+ }apps/web/src/app/(dashboard)/contacts/delete-contact-book.tsx (1)
31-41: UsemutateAsync+ await; close after success.Consistent with the template flow; lets DeleteResource close on success.
- deleteContactBookMutation.mutate(- { contactBookId: contactBook.id },- {- onSuccess: () => {- utils.contacts.getContactBooks.invalidate();- toast.success(`Contact book deleted`);- },- },- );+ try {+ await deleteContactBookMutation.mutateAsync({ contactBookId: contactBook.id });+ await utils.contacts.getContactBooks.invalidate();+ toast.success("Contact book deleted");+ } catch (e) {+ // Optional toast; then rethrow to surface on the field+ throw e;+ }apps/web/src/app/(dashboard)/contacts/[contactBookId]/delete-contact.tsx (1)
26-39: PrefermutateAsync+ await; unify success/error flow.This ensures DeleteResource closes post-success and surfaces errors to the field.
- deleteContactMutation.mutate(- {- contactId: contact.id,- contactBookId: contact.contactBookId,- },- {- onSuccess: () => {- utils.contacts.contacts.invalidate();- toast.success(`Contact deleted`);- },- onError: (e) => {- toast.error(`Contact not deleted: ${e.message}`);- },- },- );+ try {+ await deleteContactMutation.mutateAsync({+ contactId: contact.id,+ contactBookId: contact.contactBookId,+ });+ await utils.contacts.contacts.invalidate();+ toast.success("Contact deleted");+ } catch (e) {+ toast.error(`Contact not deleted: ${e instanceof Error ? e.message : "Unknown error"}`);+ throw e;+ }
♻️ Duplicate comments (2)
apps/web/src/app/(dashboard)/campaigns/delete-campaign.tsx (1)
22-24: Same error handling issue as in delete-api-key.tsx.This has the identical problem flagged in
delete-api-key.tsxlines 21-24: thrown errors won't be displayed in the form UI. Apply the same Zod refinement solution to move validation into the schema.Use a dynamic schema factory:
-const campaignSchema = z.object({- confirmation: z.string().min(1, "Please type the campaign name to confirm"),-});+const createCampaignSchema = (campaignName: string) =>+ z.object({+ confirmation: z+ .string()+ .min(1, "Please type the campaign name to confirm")+ .refine((val) => val === campaignName, {+ message: "Campaign name does not match",+ }),+ });Update onCampaignDelete and DeleteResource accordingly (remove throw logic, use dynamic schema).
apps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsx (1)
21-23: Same error handling issue as other delete components.Identical to the problem in
delete-api-key.tsx(lines 21-24) anddelete-campaign.tsx(lines 22-24): thrown errors won't display in the form. Apply the Zod refinement pattern.-const domainSchema = z.object({- confirmation: z.string().min(1, "Please type the domain name to confirm"),-});+const createDomainSchema = (domainName: string) =>+ z.object({+ confirmation: z+ .string()+ .min(1, "Please type the domain name to confirm")+ .refine((val) => val === domainName, {+ message: "Domain name does not match",+ }),+ });Then update the component to remove the throw logic and use
schema={createDomainSchema(domain.name)}.
🧹 Nitpick comments (9)
apps/web/src/components/DeleteResource.tsx (3)
151-153: Add an accessible label for the confirmation input.There’s no visible label. Add
aria-label(or a visually hidden label) for screen readers.-<Input placeholder={`${resourceName}`} {...field} />+<Input+ aria-label="Type the resource name to confirm"+ placeholder={`${resourceName}`}+ {...field} />
126-141: Improve copy button accessibility.Add an aria-label/title so icon-only control is announced.
-<Button+<Button type="button" variant="ghost" size="sm" className="h-4 w-4 p-0 hover:bg-transparent" + aria-label={copied ? "Copied" : "Copy resource name"}+ title={copied ? "Copied" : "Copy resource name"} onClick={(e) => { e.stopPropagation(); copyToClipboard(); }} >
68-71: Optional: strengthen typing for custom schemas.If you plan to extend beyond
{ confirmation: string }, make the schema generic soonConfirmis correctly typed.-const defaultSchema = z.object({ confirmation: z.string() });+const defaultSchema = z.object({ confirmation: z.string() });++type AnyZod = z.ZodTypeAny;++export interface DeleteResourceProps<S extends AnyZod = typeof defaultSchema> {+ // ...- onConfirm: (values: z.infer<typeof defaultSchema>) => void | Promise<void>;+ onConfirm: (values: z.infer<S>) => void | Promise<void>; open?: boolean; onOpenChange?: (open: boolean) => void; - schema?: typeof defaultSchema;+ schema?: S; // ... } -export const DeleteResource: React.FC<DeleteResourceProps> = ({+export const DeleteResource = <S extends AnyZod = typeof defaultSchema>({ // ... - schema = defaultSchema,+ schema = defaultSchema as S, // ... -}: DeleteResourceProps) => {+}: DeleteResourceProps<S>) => {- const form = useForm<z.infer<typeof schema>>({+ const form = useForm<z.infer<S>>({ resolver: zodResolver(schema), });apps/web/src/app/(dashboard)/templates/delete-template.tsx (2)
21-24: Don’t throw; validate or surface error to the form.Replace the throw with schema-level validation (preferred) or use
mutateAsync+ try/catch so DeleteResource can map errors.Option A (schema enforces exact match):
-const templateSchema = z.object({- confirmation: z.string().min(1, "Please type the template name to confirm"),-});+const templateSchema = z.object({+ confirmation: z.literal(template.name ?? "", {+ errorMap: () => ({ message: "Template name does not match" }),+ }),+});Option B (keep check in handler, but no throw):
- if (values.confirmation !== template.name) {- throw new Error("Template name does not match");- }+ if (values.confirmation !== template.name) {+ return Promise.reject(new Error("Template name does not match"));+ }
41-52: Ensurenameis present; improve trigger a11y.
- Passing
resourceName={template.name || ""}yields a blank prompt ifnameis missing. Prefer requiring it in props:template: Pick<Template, "id" | "name">.- Add
aria-labelto the icon button.-export const DeleteTemplate: React.FC<{ template: Partial<Template> & { id: string } }> = ({ template }) => {+export const DeleteTemplate: React.FC<{ template: Pick<Template, "id" | "name"> }> = ({ template }) => { @@ - trigger={- <Button variant="ghost" size="sm" className="p-0 hover:bg-transparent">+ trigger={+ <Button+ variant="ghost"+ size="sm"+ className="p-0 hover:bg-transparent"+ aria-label="Delete template"+ title="Delete template"+ > <Trash2 className="h-[18px] w-[18px] text-red/80" /> </Button> }apps/web/src/app/(dashboard)/contacts/delete-contact-book.tsx (2)
27-29: Replace throw with validation or rejection.Align with DeleteResource’s error mapping; avoid unhandled throw.
Option A:
-const contactBookSchema = z.object({- confirmation: z- .string()- .min(1, "Please type the contact book name to confirm"),-});+const contactBookSchema = z.object({+ confirmation: z.literal(contactBook.name ?? "", {+ errorMap: () => ({ message: "Contact book name does not match" }),+ }),+});Option B:
- if (values.confirmation !== contactBook.name) {- throw new Error("Contact book name does not match");- }+ if (values.confirmation !== contactBook.name) {+ return Promise.reject(new Error("Contact book name does not match"));+ }
45-57: Requirenameand add trigger a11y.Prevent empty confirmation prompts and improve accessibility.
-export const DeleteContactBook: React.FC<{ contactBook: Partial<ContactBook> & { id: string } }>+export const DeleteContactBook: React.FC<{ contactBook: Pick<ContactBook, "id" | "name"> }> @@ - trigger={- <Button variant="ghost" size="sm" className="p-0 hover:bg-transparent ">+ trigger={+ <Button+ variant="ghost"+ size="sm"+ className="p-0 hover:bg-transparent"+ aria-label="Delete contact book"+ title="Delete contact book"+ > <Trash2 className="h-[18px] w-[18px] text-red/80 hover:text-red/70" /> </Button> }apps/web/src/app/(dashboard)/contacts/[contactBookId]/delete-contact.tsx (2)
21-24: Avoid throwing; validate or reject promise.Mirror the other files: enforce equality via schema or return a rejected Promise for DeleteResource to display.
- if (values.confirmation !== contact.email) {- throw new Error("Email does not match");- }+ if (values.confirmation !== contact.email) {+ return Promise.reject(new Error("Email does not match"));+ }Or:
-const contactSchema = z.object({- confirmation: z.string().email("Please enter a valid email address"),-});+const contactSchema = z.object({+ confirmation: z.literal(contact.email ?? "", {+ errorMap: () => ({ message: "Email does not match" }),+ }),+});
44-56: Add trigger a11y and require
- Provide aria-label/title to the icon-only button.
- Ensure
-export const DeleteContact: React.FC<{- contact: Partial<Contact> & { id: string; contactBookId: string };-}>+export const DeleteContact: React.FC<{+ contact: Pick<Contact, "id" | "contactBookId" | "email">;+}> @@ - trigger={- <Button variant="ghost" size="sm">+ trigger={+ <Button variant="ghost" size="sm" aria-label="Delete contact" title="Delete contact"> <Trash2 className="h-4 w-4 text-red/80" /> </Button> }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/web/src/app/(dashboard)/campaigns/delete-campaign.tsx(2 hunks)apps/web/src/app/(dashboard)/contacts/[contactBookId]/delete-contact.tsx(2 hunks)apps/web/src/app/(dashboard)/contacts/delete-contact-book.tsx(2 hunks)apps/web/src/app/(dashboard)/dev-settings/api-keys/delete-api-key.tsx(2 hunks)apps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsx(2 hunks)apps/web/src/app/(dashboard)/templates/delete-template.tsx(2 hunks)apps/web/src/components/DeleteResource.tsx(1 hunks)packages/ui/src/input.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Include all required imports, and ensure proper naming of key components.
Files:
apps/web/src/app/(dashboard)/contacts/delete-contact-book.tsxapps/web/src/app/(dashboard)/templates/delete-template.tsxapps/web/src/components/DeleteResource.tsxapps/web/src/app/(dashboard)/contacts/[contactBookId]/delete-contact.tsxapps/web/src/app/(dashboard)/campaigns/delete-campaign.tsxapps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsxapps/web/src/app/(dashboard)/dev-settings/api-keys/delete-api-key.tsxpackages/ui/src/input.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: TypeScript-first: use .ts/.tsx for source code (avoid JavaScript source files)
Use 2-space indentation and semicolons (Prettier 3 enforces these)
Adhere to @usesend/eslint-config; fix all ESLint warnings (CI fails on warnings)
Do not use dynamic imports; always place imports at the top of the module
Files:
apps/web/src/app/(dashboard)/contacts/delete-contact-book.tsxapps/web/src/app/(dashboard)/templates/delete-template.tsxapps/web/src/components/DeleteResource.tsxapps/web/src/app/(dashboard)/contacts/[contactBookId]/delete-contact.tsxapps/web/src/app/(dashboard)/campaigns/delete-campaign.tsxapps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsxapps/web/src/app/(dashboard)/dev-settings/api-keys/delete-api-key.tsxpackages/ui/src/input.tsx
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier 3 (run pnpm format)
Files:
apps/web/src/app/(dashboard)/contacts/delete-contact-book.tsxapps/web/src/app/(dashboard)/templates/delete-template.tsxapps/web/src/components/DeleteResource.tsxapps/web/src/app/(dashboard)/contacts/[contactBookId]/delete-contact.tsxapps/web/src/app/(dashboard)/campaigns/delete-campaign.tsxapps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsxapps/web/src/app/(dashboard)/dev-settings/api-keys/delete-api-key.tsxpackages/ui/src/input.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Name React component files in PascalCase (e.g., AppSideBar.tsx)
Files:
apps/web/src/app/(dashboard)/contacts/delete-contact-book.tsxapps/web/src/app/(dashboard)/templates/delete-template.tsxapps/web/src/components/DeleteResource.tsxapps/web/src/app/(dashboard)/contacts/[contactBookId]/delete-contact.tsxapps/web/src/app/(dashboard)/campaigns/delete-campaign.tsxapps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsxapps/web/src/app/(dashboard)/dev-settings/api-keys/delete-api-key.tsxpackages/ui/src/input.tsx
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/**/*.{ts,tsx}: In apps/web, use the/ alias for src imports (e.g., import { x } from "/utils/x")
Prefer using tRPC in apps/web unless explicitly asked otherwise
Files:
apps/web/src/app/(dashboard)/contacts/delete-contact-book.tsxapps/web/src/app/(dashboard)/templates/delete-template.tsxapps/web/src/components/DeleteResource.tsxapps/web/src/app/(dashboard)/contacts/[contactBookId]/delete-contact.tsxapps/web/src/app/(dashboard)/campaigns/delete-campaign.tsxapps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsxapps/web/src/app/(dashboard)/dev-settings/api-keys/delete-api-key.tsx
🧬 Code graph analysis (7)
apps/web/src/app/(dashboard)/contacts/delete-contact-book.tsx (1)
apps/web/src/components/DeleteResource.tsx (1)
DeleteResource(52-185)
apps/web/src/app/(dashboard)/templates/delete-template.tsx (1)
apps/web/src/components/DeleteResource.tsx (1)
DeleteResource(52-185)
apps/web/src/components/DeleteResource.tsx (4)
packages/ui/src/dialog.tsx (6)
Dialog(113-113)DialogTrigger(117-117)DialogContent(118-118)DialogHeader(119-119)DialogTitle(121-121)DialogDescription(122-122)packages/ui/src/form.tsx (6)
Form(170-170)FormField(176-176)FormItem(171-171)FormControl(173-173)FormMessage(175-175)FormDescription(174-174)packages/ui/src/button.tsx (1)
Button(80-80)packages/ui/src/input.tsx (1)
Input(25-25)
apps/web/src/app/(dashboard)/contacts/[contactBookId]/delete-contact.tsx (1)
apps/web/src/components/DeleteResource.tsx (1)
DeleteResource(52-185)
apps/web/src/app/(dashboard)/campaigns/delete-campaign.tsx (1)
apps/web/src/components/DeleteResource.tsx (1)
DeleteResource(52-185)
apps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsx (1)
apps/web/src/components/DeleteResource.tsx (1)
DeleteResource(52-185)
apps/web/src/app/(dashboard)/dev-settings/api-keys/delete-api-key.tsx (1)
apps/web/src/components/DeleteResource.tsx (1)
DeleteResource(52-185)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Cloudflare Pages
🔇 Additional comments (6)
packages/ui/src/input.tsx (1)
14-16: LGTM: placeholder tone + exportStyling tweak and explicit export look good. No behavior changes; forwardRef closure is valid and Prettier-friendly.
Also applies to: 21-21
apps/web/src/components/DeleteResource.tsx (1)
101-105: No issues found: all callers use uncontrolled mode.Verification confirms that none of the 6 DeleteResource callers pass the
openprop—with or withoutonOpenChange. All callers use the component's uncontrolled mode exclusively, relying on internal state management. The specific risk outlined in the review (passingopenwithoutonOpenChange) is not realized in the current codebase.apps/web/src/app/(dashboard)/contacts/[contactBookId]/delete-contact.tsx (1)
32-34: Remove review comment - naming is correct.The
utils.contacts.contacts.invalidate()call is the correct tRPC pattern. Thecontactsrouter has acontactsquery procedure (defined at line 63 of the contacts router), so the usage follows the standard pattern:utils.<routerName>.<procedureName>.invalidate(). This is no different from other procedures likeutils.contacts.getContactBooks.invalidate()orutils.contacts.getContactBookDetails.invalidate().Likely an incorrect or invalid review comment.
apps/web/src/app/(dashboard)/dev-settings/api-keys/delete-api-key.tsx (1)
40-52: LGTM with concerns noted above.The DeleteResource integration is clean and follows the expected props pattern. The fallback to empty string for
resourceNameis appropriate given the partial type.Note: This approval is contingent on fixing the error handling issue flagged in lines 21-24.
apps/web/src/app/(dashboard)/campaigns/delete-campaign.tsx (1)
40-52: DeleteResource integration looks good.The component usage follows the correct pattern with appropriate props and fallback for
resourceName.Approval is contingent on fixing the error handling issue in lines 22-24.
apps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsx (1)
40-52: DeleteResource integration is correct.Props are properly configured. Since
Domain.nameis a required field, the lack of a fallback is acceptable.Approval depends on fixing the error handling in lines 21-23.
Uh oh!
There was an error while loading. Please reload this page.
| import { | ||
| Form, | ||
| FormControl, | ||
| FormDescription, | ||
| FormField, | ||
| FormItem, | ||
| FormLabel, | ||
| FormMessage, | ||
| } from "@usesend/ui/src/form"; |
There was a problem hiding this comment.
Fix import hygiene (unused + duplicate).
- Remove unused
FormLabelandtoast. - Merge duplicate
reactimports into one:import React, { useState, ReactNode } from "react";
This prevents ESLint warnings which fail CI.
Apply:
-import {- Form,- FormControl,- FormDescription,- FormField,- FormItem,- FormLabel,- FormMessage,-} from "@usesend/ui/src/form";+import {+ Form,+ FormControl,+ FormDescription,+ FormField,+ FormItem,+ FormMessage,+} from "@usesend/ui/src/form";--import React, { useState } from "react";-import { toast } from "@usesend/ui/src/toaster";+import React, { useState, ReactNode } from "react";
@@
-import { ReactNode } from "react";Also applies to: 24-31
🤖 Prompt for AI Agents
In apps/web/src/components/DeleteResource.tsx around lines 14-22 (and similarly
for lines 24-31), remove the unused FormLabel and any unused toast import, and
consolidate duplicate React imports into a single statement: import React with
the necessary named exports (useState, ReactNode) in one line; ensure no
duplicate react import remains and only used identifiers are imported to satisfy
ESLint.
| const handleSubmit = async (values: z.infer<typeof schema>) => { | ||
| await onConfirm(values); | ||
| }; | ||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Handle submit errors and close on success.
Catching thrown errors from onConfirm and mapping them to the field avoids unhandled rejections and shows feedback. Also close/reset after success.
Apply:
-const handleSubmit = async (values: z.infer<typeof schema>) => {- await onConfirm(values);-};+const handleSubmit = async (values: z.infer<typeof schema>) => {+ try {+ await onConfirm(values);+ // success → close and reset+ setOpen(false);+ form.reset();+ } catch (e) {+ const msg =+ e instanceof Error ? e.message : "Something went wrong. Please try again.";+ form.setError("confirmation" as any, { type: "validate", message: msg });+ }+};Also applies to: 100-105
🤖 Prompt for AI Agents
In apps/web/src/components/DeleteResource.tsx around lines 88-91 (and also apply
same change at lines 100-105), the handleSubmit currently awaits onConfirm
without error handling or cleanup; wrap the onConfirm call in a try/catch, on
success call the dialog/cleanup function (e.g., close and reset form state), and
in the catch map the thrown error to the form field(s) using the form API
(setError/setFieldError or equivalent) so the user sees validation/submit
feedback and to avoid unhandled rejections. Ensure you rethrow only if necessary
and always perform any needed UI cleanup only after successful completion.
There was a problem hiding this comment.
3 issues found across 8 files
Prompt for AI agents (all 3 issues)
Understand the root cause of the following 3 issues and fix them.
<file name="apps/web/src/app/(dashboard)/campaigns/delete-campaign.tsx">
<violation number="1" location="apps/web/src/app/(dashboard)/campaigns/delete-campaign.tsx:22">
Throwing an Error when the confirmation text mismatches leaves the DeleteResource form without any field-level feedback, because DeleteResource’s submit handler doesn’t catch or surface that error. Users who mistype the campaign name will see nothing happen, regressing the previous inline error message.</violation>
</file>
<file name="apps/web/src/app/(dashboard)/templates/delete-template.tsx">
<violation number="1" location="apps/web/src/app/(dashboard)/templates/delete-template.tsx:23">
Throwing here bubbles an unhandled error through DeleteResource, so the dialog never surfaces a validation message when the confirmation text is wrong. Return early with user-facing feedback instead of throwing.</violation>
</file>
<file name="apps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsx">
<violation number="1" location="apps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsx:22">
Throwing an Error when the confirmation text does not match bubbles out of DeleteResource’s submit handler, producing an unhandled runtime error instead of validation feedback. Please surface the mismatch as form validation instead of throwing.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| message: "Name does not match", | ||
| }); | ||
| return; | ||
| if (values.confirmation !== campaign.name) { |
There was a problem hiding this comment.
Throwing an Error when the confirmation text mismatches leaves the DeleteResource form without any field-level feedback, because DeleteResource’s submit handler doesn’t catch or surface that error. Users who mistype the campaign name will see nothing happen, regressing the previous inline error message.
Prompt for AI agents
Address the following comment on apps/web/src/app/(dashboard)/campaigns/delete-campaign.tsx at line 22:
<comment>Throwing an Error when the confirmation text mismatches leaves the DeleteResource form without any field-level feedback, because DeleteResource’s submit handler doesn’t catch or surface that error. Users who mistype the campaign name will see nothing happen, regressing the previous inline error message.</comment>
<file context>
@@ -1,55 +1,26 @@
- message: "Name does not match",
- });
- return;
+ if (values.confirmation !== campaign.name) {
+ throw new Error("Campaign name does not match");
}
</file context>
✅ Addressed in 86ffada
| }); | ||
| return; | ||
| if (values.confirmation !== template.name) { | ||
| throw new Error("Template name does not match"); |
There was a problem hiding this comment.
Throwing here bubbles an unhandled error through DeleteResource, so the dialog never surfaces a validation message when the confirmation text is wrong. Return early with user-facing feedback instead of throwing.
Prompt for AI agents
Address the following comment on apps/web/src/app/(dashboard)/templates/delete-template.tsx at line 23:
<comment>Throwing here bubbles an unhandled error through DeleteResource, so the dialog never surfaces a validation message when the confirmation text is wrong. Return early with user-facing feedback instead of throwing.</comment>
<file context>
@@ -1,55 +1,26 @@
- });
- return;
+ if (values.confirmation !== template.name) {
+ throw new Error("Template name does not match");
}
</file context>
| thrownewError("Template name does not match"); | |
| returntoast.error("Template name does not match"); |
✅ Addressed in 86ffada
| }); | ||
| return; | ||
| if (values.confirmation !== domain.name) { | ||
| throw new Error("Domain name does not match"); |
There was a problem hiding this comment.
Throwing an Error when the confirmation text does not match bubbles out of DeleteResource’s submit handler, producing an unhandled runtime error instead of validation feedback. Please surface the mismatch as form validation instead of throwing.
Prompt for AI agents
Address the following comment on apps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsx at line 22:
<comment>Throwing an Error when the confirmation text does not match bubbles out of DeleteResource’s submit handler, producing an unhandled runtime error instead of validation feedback. Please surface the mismatch as form validation instead of throwing.</comment>
<file context>
@@ -1,58 +1,25 @@
- });
- return;
+ if (values.confirmation !== domain.name) {
+ throw new Error("Domain name does not match");
}
</file context>
✅ Addressed in 86ffada
Deploying usesend with |
| Latest commit: | 86ffada |
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8756cc80.usesend.pages.dev |
| Branch Preview URL: | https://km-2025-10-19-delete-modal.usesend.pages.dev |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
apps/web/src/components/DeleteResource.tsx (2)
14-22: Clean up unused imports to satisfy ESLint.
FormLabelandtoastare imported but unused. ESLint warnings fail CI.import { Form, FormControl, FormDescription, FormField, FormItem, - FormLabel, FormMessage, } from "@usesend/ui/src/form"; @@ -import { toast } from "@usesend/ui/src/toaster";Also applies to: 26-26
102-105: Handle submit errors and close/reset on success.Without try/catch, thrown errors from
onConfirmare unhandled; the dialog also stays open after success. Catch, map to the field, and close/reset on success.- const handleSubmit = async (values: z.infer<Schema>) => {- await onConfirm(values);- };+ const handleSubmit = async (values: z.infer<Schema>) => {+ try {+ await onConfirm(values);+ setOpen(false);+ form.reset();+ } catch (e) {+ const msg =+ e instanceof Error+ ? e.message+ : "Something went wrong. Please try again.";+ // Map to the only field for visible feedback+ form.setError("confirmation" as any, { message: msg });+ }+ };
🧹 Nitpick comments (6)
apps/web/src/app/(dashboard)/templates/delete-template.tsx (1)
28-39: Add error feedback on mutation failure (optional).Surface
onErrorwith a toast for consistency with other delete flows.deleteTemplateMutation.mutate( @@ - {- onSuccess: () => {+ {+ onSuccess: () => { utils.template.getTemplates.invalidate(); toast.success(`Template deleted`); - },+ },+ onError: (e) => toast.error(`Template not deleted: ${e.message}`), },apps/web/src/app/(dashboard)/campaigns/delete-campaign.tsx (1)
28-39: Show error toast on failure (optional).Align with other flows by adding
onErrorto surface failures.deleteCampaignMutation.mutate( @@ - {- onSuccess: () => {+ {+ onSuccess: () => { utils.campaign.getCampaigns.invalidate(); toast.success(`Campaign deleted`); - },+ },+ onError: (e) => toast.error(`Campaign not deleted: ${e.message}`), },apps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsx (2)
25-25: Remove or underscore unused parameter.
valuesis unused and may trigger ESLintno-unused-vars.-async function onDomainDelete(values: z.infer<typeof domainSchema>) {+async function onDomainDelete(_values: z.infer<typeof domainSchema>) {
42-45: Consistent title/label casing (optional).Other delete flows use title‑case (“Delete Domain”). Consider aligning for consistency.
- title="Delete domain"+ title="Delete Domain" @@ - confirmLabel="Delete domain"+ confirmLabel="Delete Domain"Also applies to: 52-53
apps/web/src/components/DeleteResource.tsx (2)
45-61: Unify prop node types (optional).Use the imported
ReactNodefortriggerandchildrento keep types consistent.- trigger?: React.ReactNode;- children?: React.ReactNode;+ trigger?: ReactNode;+ children?: ReactNode;
139-156: Add accessible label to copy button (optional).Improve a11y with an aria-label.
- <Button+ <Button type="button" variant="ghost" size="sm" className="h-4 w-4 p-0 hover:bg-transparent" + aria-label="Copy resource name" onClick={(e) => { e.stopPropagation(); copyToClipboard(); }} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/src/app/(dashboard)/campaigns/delete-campaign.tsx(1 hunks)apps/web/src/app/(dashboard)/contacts/[contactBookId]/delete-contact.tsx(2 hunks)apps/web/src/app/(dashboard)/contacts/delete-contact-book.tsx(1 hunks)apps/web/src/app/(dashboard)/dev-settings/api-keys/delete-api-key.tsx(1 hunks)apps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsx(2 hunks)apps/web/src/app/(dashboard)/templates/delete-template.tsx(1 hunks)apps/web/src/components/DeleteResource.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/app/(dashboard)/dev-settings/api-keys/delete-api-key.tsx
- apps/web/src/app/(dashboard)/contacts/delete-contact-book.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Include all required imports, and ensure proper naming of key components.
Files:
apps/web/src/app/(dashboard)/contacts/[contactBookId]/delete-contact.tsxapps/web/src/components/DeleteResource.tsxapps/web/src/app/(dashboard)/campaigns/delete-campaign.tsxapps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsxapps/web/src/app/(dashboard)/templates/delete-template.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: TypeScript-first: use .ts/.tsx for source code (avoid JavaScript source files)
Use 2-space indentation and semicolons (Prettier 3 enforces these)
Adhere to @usesend/eslint-config; fix all ESLint warnings (CI fails on warnings)
Do not use dynamic imports; always place imports at the top of the module
Files:
apps/web/src/app/(dashboard)/contacts/[contactBookId]/delete-contact.tsxapps/web/src/components/DeleteResource.tsxapps/web/src/app/(dashboard)/campaigns/delete-campaign.tsxapps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsxapps/web/src/app/(dashboard)/templates/delete-template.tsx
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier 3 (run pnpm format)
Files:
apps/web/src/app/(dashboard)/contacts/[contactBookId]/delete-contact.tsxapps/web/src/components/DeleteResource.tsxapps/web/src/app/(dashboard)/campaigns/delete-campaign.tsxapps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsxapps/web/src/app/(dashboard)/templates/delete-template.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Name React component files in PascalCase (e.g., AppSideBar.tsx)
Files:
apps/web/src/app/(dashboard)/contacts/[contactBookId]/delete-contact.tsxapps/web/src/components/DeleteResource.tsxapps/web/src/app/(dashboard)/campaigns/delete-campaign.tsxapps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsxapps/web/src/app/(dashboard)/templates/delete-template.tsx
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/**/*.{ts,tsx}: In apps/web, use the/ alias for src imports (e.g., import { x } from "/utils/x")
Prefer using tRPC in apps/web unless explicitly asked otherwise
Files:
apps/web/src/app/(dashboard)/contacts/[contactBookId]/delete-contact.tsxapps/web/src/components/DeleteResource.tsxapps/web/src/app/(dashboard)/campaigns/delete-campaign.tsxapps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsxapps/web/src/app/(dashboard)/templates/delete-template.tsx
🧬 Code graph analysis (5)
apps/web/src/app/(dashboard)/contacts/[contactBookId]/delete-contact.tsx (1)
apps/web/src/components/DeleteResource.tsx (1)
DeleteResource(63-199)
apps/web/src/components/DeleteResource.tsx (4)
packages/ui/src/dialog.tsx (6)
Dialog(113-113)DialogTrigger(117-117)DialogContent(118-118)DialogHeader(119-119)DialogTitle(121-121)DialogDescription(122-122)packages/ui/src/form.tsx (6)
Form(170-170)FormField(176-176)FormItem(171-171)FormControl(173-173)FormMessage(175-175)FormDescription(174-174)packages/ui/src/button.tsx (1)
Button(80-80)packages/ui/src/input.tsx (1)
Input(25-25)
apps/web/src/app/(dashboard)/campaigns/delete-campaign.tsx (1)
apps/web/src/components/DeleteResource.tsx (1)
DeleteResource(63-199)
apps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsx (1)
apps/web/src/components/DeleteResource.tsx (1)
DeleteResource(63-199)
apps/web/src/app/(dashboard)/templates/delete-template.tsx (1)
apps/web/src/components/DeleteResource.tsx (1)
DeleteResource(63-199)
| const campaignSchema = z | ||
| .object({ | ||
| confirmation: z | ||
| .string() | ||
| .min(1, "Please type the campaign name to confirm"), | ||
| }) | ||
| .refine((data) => data.confirmation === campaign.name, { | ||
| message: "Campaign name does not match", | ||
| path: ["confirmation"], | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Require campaign.name (avoid impossible confirmation).
campaign is Partial<Campaign>, but the schema and resourceName depend on name. If name is undefined, confirmation will always fail. Tighten the prop type (or add a guard/fallback).
-export const DeleteCampaign: React.FC<{- campaign: Partial<Campaign> & { id: string };-}> = ({ campaign }) => {+export const DeleteCampaign: React.FC<{+ campaign: Pick<Campaign, "id" | "name">;+}> = ({ campaign }) => {Also applies to: 45-46
| const contactSchema = z | ||
| .object({ | ||
| confirmation: z.string().email("Please enter a valid email address"), | ||
| }) | ||
| .refine((data) => data.confirmation === contact.email, { | ||
| message: "Email does not match", | ||
| path: ["confirmation"], | ||
| }); |
There was a problem hiding this comment.
Require contact.email (avoid impossible confirmation).
contact is Partial<Contact>, but the schema and resourceName require email. If undefined, users cannot confirm. Tighten the prop type (or guard).
-export const DeleteContact: React.FC<{- contact: Partial<Contact> & { id: string; contactBookId: string };-}> = ({ contact }) => {+export const DeleteContact: React.FC<{+ contact: Pick<Contact, "id" | "contactBookId" | "email">;+}> = ({ contact }) => {Also applies to: 47-48
| const templateSchema = z | ||
| .object({ | ||
| confirmation: z | ||
| .string() | ||
| .min(1, "Please type the template name to confirm"), | ||
| }) | ||
| .refine((data) => data.confirmation === template.name, { | ||
| message: "Template name does not match", | ||
| path: ["confirmation"], | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Require template.name (avoid impossible confirmation).
template is Partial<Template> but the schema and resourceName depend on template.name. If name is undefined, users can never confirm. Tighten the prop type (or guard).
Apply:
-export const DeleteTemplate: React.FC<{- template: Partial<Template> & { id: string };-}> = ({ template }) => {+export const DeleteTemplate: React.FC<{+ template: Pick<Template, "id" | "name">;+}> = ({ template }) => {Also applies to: 45-46
Summary by cubic
Introduced a reusable DeleteResource modal for destructive actions and replaced per-page delete dialogs across campaigns, contacts, contact books, API keys, domains, and templates. This standardizes confirmation UX and cuts a lot of duplicate code.
New Features
Refactors
Summary by CodeRabbit
New Features
Style