feat: allow deleting ses settings #289
Conversation
Deploying usesend with
|
| Latest commit: |
a5b2f0f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://baf22911.usesend.pages.dev |
| Branch Preview URL: | https://codex-add-option-to-delete-s.usesend.pages.dev |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis pull request implements SES configuration deletion functionality across the application stack. A new React component displays a confirmation dialog in the admin UI for deleting SES settings. The backend introduces a TRPC mutation that orchestrates the deletion workflow, which includes removing AWS configuration sets, SNS topics, cleaning up email queue services for the region, and deleting the database record. Supporting AWS and service layer functions were added to handle SES configuration set deletion and email queue region cleanup. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/web/src/app/(dashboard)/admin/ses-configurations.tsx (1)
16-16: Actions cell wiring looks correct; consider fixing table colSpan while you’re here
- Importing and rendering
DeleteSesConfigurationalongsideEditSesConfigurationin a flex row is consistent and keeps the Actions cell clear.- One small adjacent nit: the table defines 8 columns, but the loading/empty rows still use
colSpan={6}(Lines 38–52). That can leave the spinner/empty state misaligned; consider bumpingcolSpanto 8 in those rows in a follow‑up.Also applies to: 75-79
apps/web/src/server/service/email-queue-service.ts (1)
111-130: Queue teardown for a region looks correct; be aware of stranded jobs semanticsThe
removeRegionimplementation cleanly closes transactional/marketing workers and queues for the region and removes them from the maps; the method is effectively idempotent and safe to call as part of SES setting deletion.One behavioral detail to keep in mind: any jobs still in Redis for those queues will remain but won’t be processed once workers are closed. That’s likely the right trade‑off for a deleted SES region, but worth being aware of if you ever add “soft delete” or region re‑activation flows.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web/src/app/(dashboard)/admin/delete-ses-configuration.tsx(1 hunks)apps/web/src/app/(dashboard)/admin/ses-configurations.tsx(2 hunks)apps/web/src/server/api/routers/admin.ts(1 hunks)apps/web/src/server/aws/ses.ts(2 hunks)apps/web/src/server/service/email-queue-service.ts(1 hunks)apps/web/src/server/service/ses-settings-service.ts(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/server/service/email-queue-service.tsapps/web/src/server/service/ses-settings-service.tsapps/web/src/server/api/routers/admin.tsapps/web/src/server/aws/ses.tsapps/web/src/app/(dashboard)/admin/ses-configurations.tsxapps/web/src/app/(dashboard)/admin/delete-ses-configuration.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/server/service/email-queue-service.tsapps/web/src/server/service/ses-settings-service.tsapps/web/src/server/api/routers/admin.tsapps/web/src/server/aws/ses.tsapps/web/src/app/(dashboard)/admin/ses-configurations.tsxapps/web/src/app/(dashboard)/admin/delete-ses-configuration.tsx
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier 3 (run pnpm format)
Files:
apps/web/src/server/service/email-queue-service.tsapps/web/src/server/service/ses-settings-service.tsapps/web/src/server/api/routers/admin.tsapps/web/src/server/aws/ses.tsapps/web/src/app/(dashboard)/admin/ses-configurations.tsxapps/web/src/app/(dashboard)/admin/delete-ses-configuration.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/server/service/email-queue-service.tsapps/web/src/server/service/ses-settings-service.tsapps/web/src/server/api/routers/admin.tsapps/web/src/server/aws/ses.tsapps/web/src/app/(dashboard)/admin/ses-configurations.tsxapps/web/src/app/(dashboard)/admin/delete-ses-configuration.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Name React component files in PascalCase (e.g., AppSideBar.tsx)
Files:
apps/web/src/app/(dashboard)/admin/ses-configurations.tsxapps/web/src/app/(dashboard)/admin/delete-ses-configuration.tsx
🧬 Code graph analysis (6)
apps/web/src/server/service/email-queue-service.ts (1)
apps/web/src/server/logger/log.ts (1)
logger(31-63)
apps/web/src/server/service/ses-settings-service.ts (2)
apps/web/src/server/db.ts (1)
db(20-20)apps/web/src/server/service/email-queue-service.ts (1)
EmailQueueService(39-331)
apps/web/src/server/api/routers/admin.ts (2)
apps/web/src/server/api/trpc.ts (1)
adminProcedure(267-272)apps/web/src/server/service/ses-settings-service.ts (1)
SesSettingsService(22-260)
apps/web/src/server/aws/ses.ts (1)
apps/web/src/server/logger/log.ts (1)
logger(31-63)
apps/web/src/app/(dashboard)/admin/ses-configurations.tsx (2)
apps/web/src/app/(dashboard)/admin/edit-ses-configuration.tsx (1)
EditSesConfiguration(38-68)apps/web/src/app/(dashboard)/admin/delete-ses-configuration.tsx (1)
DeleteSesConfiguration(21-93)
apps/web/src/app/(dashboard)/admin/delete-ses-configuration.tsx (3)
packages/ui/src/toaster.tsx (1)
toast(31-31)packages/ui/src/dialog.tsx (7)
Dialog(113-113)DialogTrigger(117-117)DialogContent(118-118)DialogHeader(119-119)DialogTitle(121-121)DialogDescription(122-122)DialogFooter(120-120)packages/ui/src/button.tsx (1)
Button(80-80)
⏰ 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 (3)
apps/web/src/server/api/routers/admin.ts (1)
125-135: Admin deleteSesSettings mutation is wired correctlyInput validation, auth via
adminProcedure, and delegation toSesSettingsService.deleteSesSettingall look consistent with the existing SES settings procedures. Returningtrueis fine for now; you can always extend the payload later if needed.apps/web/src/server/aws/ses.ts (1)
1-15: SES configuration set deletion helper is implemented safelyImporting
DeleteConfigurationSetCommandand wrapping it indeleteConfigurationSetwith explicit handling forNotFoundExceptionis a good fit for the cleanup path; other errors correctly bubble up so the caller can decide whether to abort the overall delete. The usage pattern matches AWS SESv2 expectations: constructing the command with an input object including ConfigurationSetName and sending it via an SESv2Client instance.apps/web/src/server/service/ses-settings-service.ts (1)
205-240: SES setting delete flow is coherent and retry‑friendlyThe deletion sequence (load setting → delete config sets → delete SNS topic → remove region queues → delete DB record → invalidate cache) is well ordered and safe to retry. Failures in AWS calls abort before the DB row is removed, and both configuration‑set deletion and SNS topic deletion are idempotent operations that tolerate missing resources.
| import { useState } from "react"; | ||
| import { SesSetting } from "@prisma/client"; | ||
| import { Trash } from "lucide-react"; |
There was a problem hiding this comment.
🧩 Analysis chain
Avoid Prisma value imports on the client; add an accessible label for the delete button
- In a
"use client"component, importingSesSettingas a value (import { SesSetting } from "@prisma/client";) can cause Prisma (a server‑side dependency) to be pulled into the browser bundle depending on TS/compiler settings. Since it’s only used for typing, switch to a type‑only import to keep the client bundle clean:
-import { SesSetting } from "@prisma/client";
+import type { SesSetting } from "@prisma/client";- The Trash icon trigger button is visually clear but has no accessible label, so screen readers won’t know what it does. Consider adding an
aria-labelor sr‑only text:
- <Button
- variant="ghost"
- size="sm"
- className="text-destructive hover:text-destructive"
- >
- <Trash className="h-4 w-4" />
- </Button>
+ <Button
+ variant="ghost"
+ size="sm"
+ aria-label="Delete SES configuration"
+ className="text-destructive hover:text-destructive"
+ >
+ <Trash className="h-4 w-4" />
+ </Button>The mutation wiring, toasts, and pending state handling otherwise look solid.
Also applies to: 21-49, 52-92
🌐 Web query:
Check how Next.js and TypeScript handle non-`import type` Prisma imports in client components (e.g., whether `@prisma/client` is tree-shaken or bundled by default when used only in type positions).
💡 Result:
Short answer: If you import @prisma/client (non‑type) from inside a Next.js client component, it becomes part of the client module graph and will be included in the client bundle (so it is not automatically erased/tree‑shaken away). Use type‑only imports (import type ...) or the Prisma browser/lightweight exports to avoid bundling server code. [1][2][3]
Evidence / pointers:
- Next.js: once a module is marked "use client", everything imported into it is treated as client code and enters the client bundle boundary. [1]
- Prisma docs: the standard generated client (client.ts / @prisma/client) contains transitive server/node dependencies and is not suitable for browser contexts; Prisma provides a browser/lightweight output and recommends explicit imports for client vs server usage. [2]
- Community/Prisma issue: historically enums and other parts lived in the same bundle which prevented safe tree‑shaking from @prisma/client; this is a known concern and drives the need for separate/browser exports or type‑only imports. [3]
If you want, I can show a small example of the safe patterns (import type / importing Prisma browser enums) and what to avoid.
Avoid Prisma value imports on the client; add an accessible label for the delete button
The concerns in the original review are confirmed:
- Non-
import typeimports of@prisma/clientin "use client" components are bundled into the client bundle and are not tree-shaken away. Use type-only imports to keep server code out of the browser:
-import { SesSetting } from "@prisma/client";
+import type { SesSetting } from "@prisma/client";- The Trash icon trigger button lacks an accessible label. Add
aria-labelso screen readers announce its purpose:
- <Button
- variant="ghost"
- size="sm"
- className="text-destructive hover:text-destructive"
- >
- <Trash className="h-4 w-4" />
- </Button>
+ <Button
+ variant="ghost"
+ size="sm"
+ aria-label="Delete SES configuration"
+ className="text-destructive hover:text-destructive"
+ >
+ <Trash className="h-4 w-4" />
+ </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.
| import { useState } from "react"; | |
| import { SesSetting } from "@prisma/client"; | |
| import { Trash } from "lucide-react"; | |
| import { useState } from "react"; | |
| import type { SesSetting } from "@prisma/client"; | |
| import { Trash } from "lucide-react"; |
🤖 Prompt for AI Agents
In apps/web/src/app/(dashboard)/admin/delete-ses-configuration.tsx around lines
3 to 5, the file currently imports SesSetting from @prisma/client which pulls
runtime code into the client bundle and the Trash icon button lacks an
accessible label; change the Prisma import to a type-only import (e.g., "import
type { SesSetting } from '@prisma/client'") so no runtime code is included, and
add an accessible label to the delete trigger (e.g., add an aria-label or
aria-labelledby attribute to the button describing its action such as "Delete
SES configuration") ensuring screen readers can announce its purpose.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Summary
Testing
Codex Task
Summary by cubic
Adds a delete action for SES configurations in the admin dashboard. Deleting a configuration cleans up SES/SNS resources, stops regional queues, and removes the database record.
Written for commit a5b2f0f. Summary will update automatically on new commits.
Summary by CodeRabbit
Release Notes