Managed email domain deletion and Cloudflare DNS import UX#1442
Open
mantrakp04 wants to merge 1 commit into
Open
Managed email domain deletion and Cloudflare DNS import UX#1442mantrakp04 wants to merge 1 commit into
mantrakp04 wants to merge 1 commit into
Conversation
Let projects remove unused managed domains with upstream cleanup, and streamline Cloudflare NS record import during onboarding. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThis PR implements end-to-end deletion for managed email domains. The changes add database helpers, backend service logic with DNSimple zone cleanup, API routes and admin interface methods, dashboard UI with Cloudflare DNS detection, and comprehensive E2E tests. ChangesManaged Email Domain Deletion
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds managed email domain deletion support end-to-end (backend endpoint + SDK + dashboard UX), plus Cloudflare-specific DNS setup affordances and E2E coverage to validate deletion behavior and authorization.
Changes:
- Added an internal admin-only delete endpoint for managed email domains, including provider-side cleanup and “in-use” rejection.
- Exposed the delete operation through the admin SDK/template layer and surfaced it in the dashboard UI with confirmation.
- Improved DNS setup UX with Cloudflare detection, zone file download, and import instructions; added E2E tests for delete flows.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/template/src/lib/stack-app/apps/interfaces/admin-app.ts | Adds deleteManagedEmailDomain to the admin app interface. |
| packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts | Implements the new admin SDK method and maps parameters to the interface request payload. |
| packages/stack-shared/src/interface/admin-interface.ts | Adds a StackAdminInterface method to call the internal delete endpoint. |
| apps/e2e/tests/backend/endpoints/api/v1/internal/managed-email-onboarding.test.ts | Adds E2E tests for auth, success, in-use rejection, post-switch deletion, and 404 deletion cases. |
| apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-settings/domain-settings.tsx | Adds dashboard UI for deleting unused managed domains and Cloudflare DNS import UX. |
| apps/backend/src/lib/managed-email-onboarding.tsx | Implements managed domain deletion logic with Resend/DNSimple cleanup and in-use guard. |
| apps/backend/src/lib/managed-email-domains.tsx | Adds DB helpers to delete a managed domain row and count remaining references to a subdomain. |
| apps/backend/src/app/api/latest/internal/emails/managed-onboarding/delete/route.tsx | Introduces the internal admin delete endpoint route. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+14
to
+16
| body: yupObject({ | ||
| resend_domain_id: yupString().defined(), | ||
| }).defined(), |
Comment on lines
+492
to
+501
| async deleteManagedEmailDomain(data: { | ||
| resend_domain_id: string, | ||
| }): Promise<{ status: "deleted" }> { | ||
| const response = await this.sendAdminRequest("/internal/emails/managed-onboarding/delete", { | ||
| method: "POST", | ||
| headers: { | ||
| "content-type": "application/json", | ||
| }, | ||
| body: JSON.stringify(data), | ||
| }, null); |
| checkManagedEmailStatus(options: { domainId: string, subdomain: string, senderLocalPart: string }): Promise<ManagedEmailProviderStatus>, | ||
| listManagedEmailDomains(): Promise<ManagedEmailProviderListItem[]>, | ||
| applyManagedEmailProvider(options: { domainId: string }): Promise<{ status: "applied" }>, | ||
| deleteManagedEmailDomain(options: { resendDomainId: string }): Promise<{ status: "deleted" }>, |
Comment on lines
+694
to
+696
| async deleteManagedEmailDomain(options: { resendDomainId: string }): Promise<{ status: "deleted" }> { | ||
| return await this._interface.deleteManagedEmailDomain({ | ||
| resend_domain_id: options.resendDomainId, |
| size="sm" | ||
| variant="outline" | ||
| className="gap-1.5" | ||
| onClick={() => { window.open(`https://dash.cloudflare.com/?to=/:account/${encodeURIComponent(cloudflareApex)}/dns/records`, "_blank", "noopener,noreferrer"); }} |
Comment on lines
+113
to
+119
| // eslint-disable-next-line @next/next/no-img-element | ||
| <img | ||
| src="https://www.cloudflare.com/favicon.ico" | ||
| alt="" | ||
| aria-hidden | ||
| className={className} | ||
| /> |
Contributor
Greptile SummaryThis PR adds a managed email domain delete flow end-to-end: a new admin-only
Confidence Score: 3/5Safe to merge after fixing error handling in the delete confirmation dialog; the backend logic and SDK wiring are solid. The delete confirmation dialog's async okButton.onClick has no error handling and neither does ActionDialog's button wrapper, so any API failure during deletion is silently swallowed — the dialog closes, no toast appears, and the domain remains in the list with the user none the wiser. The backend, SDK, and e2e layers are well-implemented. apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-settings/domain-settings.tsx — the ActionDialog delete handler needs error handling before this ships. Important Files Changed
Sequence DiagramsequenceDiagram
participant Dashboard
participant AdminApp as Admin SDK
participant Backend as DELETE /managed-onboarding/delete
participant DB as Database
participant Resend
participant DNSimple
Dashboard->>AdminApp: "deleteManagedEmailDomain({ resendDomainId })"
AdminApp->>Backend: POST (admin auth)
Backend->>DB: getManagedEmailDomainByResendDomainId()
DB-->>Backend: domain row
Backend->>Backend: isManagedEmailDomainInUseForTenancy()?
alt domain is in use
Backend-->>Dashboard: 409 Conflict
else not in use
Backend->>Resend: "DELETE /domains/{id}"
Resend-->>Backend: 200 / 404
Backend->>DB: countManagedEmailDomainsBySubdomainExcludingId()
DB-->>Backend: remaining count
alt "remaining == 0"
Backend->>DNSimple: "DELETE /zones/{zoneName}"
DNSimple-->>Backend: deleted / not_found
end
Backend->>DB: deleteManagedEmailDomainById()
DB-->>Backend: deleted row
Backend-->>Dashboard: "200 { status: deleted }"
Dashboard->>Dashboard: refreshDomains() + toast
end
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-settings/domain-settings.tsx:1182-1190
**Silent error swallowing on domain delete**
`ActionDialog`'s button wraps `okButton.onClick` with `await okButton.onClick?.()` but has no `try/catch` (see `action-dialog.tsx` line 131). If `deleteManagedEmailDomain` throws — for example due to a network failure or an unexpected server error — the error is swallowed and the user receives zero feedback: the dialog closes, the domain still appears in the list, and no alert or toast is shown. Per the repo rule, async button handlers should use `runAsynchronouslyWithAlert` so errors are automatically surfaced to users.
Reviews (1): Last reviewed commit: "Add managed email domain deletion and Cl..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+1182
to
+1190
| okButton={{ | ||
| label: "Remove", | ||
| onClick: async () => { | ||
| if (!confirmDelete) return; | ||
| await stackAdminApp.deleteManagedEmailDomain({ resendDomainId: confirmDelete.domainId }); | ||
| toast({ title: "Domain removed", description: `${confirmDelete.senderLocalPart}@${confirmDelete.subdomain} was removed.`, variant: "success" }); | ||
| await refreshDomains(); | ||
| }, | ||
| }} |
Contributor
There was a problem hiding this comment.
ActionDialog's button wraps okButton.onClick with await okButton.onClick?.() but has no try/catch (see action-dialog.tsx line 131). If deleteManagedEmailDomain throws — for example due to a network failure or an unexpected server error — the error is swallowed and the user receives zero feedback: the dialog closes, the domain still appears in the list, and no alert or toast is shown. Per the repo rule, async button handlers should use runAsynchronouslyWithAlert so errors are automatically surfaced to users.
Rule Used: Use runAsynchronouslyWithAlert from `@stackframe... (source)
Learned From
stack-auth/stack-auth#943
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-settings/domain-settings.tsx
Line: 1182-1190
Comment:
**Silent error swallowing on domain delete**
`ActionDialog`'s button wraps `okButton.onClick` with `await okButton.onClick?.()` but has no `try/catch` (see `action-dialog.tsx` line 131). If `deleteManagedEmailDomain` throws — for example due to a network failure or an unexpected server error — the error is swallowed and the user receives zero feedback: the dialog closes, the domain still appears in the list, and no alert or toast is shown. Per the repo rule, async button handlers should use `runAsynchronouslyWithAlert` so errors are automatically surfaced to users.
**Rule Used:** Use `runAsynchronouslyWithAlert` from `@stackframe... ([source](https://app.greptile.com/review/custom-context?memory=5e671275-7493-402a-93a8-969537ec4d63))
**Learned From**
[stack-auth/stack-auth#943](https://github.com/stack-auth/stack-auth/pull/943)
How can I resolve this? If you propose a fix, please make it concise.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/e2e/tests/backend/endpoints/api/v1/internal/managed-email-onboarding.test.ts (1)
205-206: ⚡ Quick winReplace fixed sleeps with condition-based polling to reduce E2E flakiness.
Line 205 and Line 243 use
wait(1500)as a timing assumption for verification readiness. This can intermittently fail under slower CI timing; poll/checkuntil"verified"with a bounded timeout instead.♻️ Proposed fix
import { wait } from "`@stackframe/stack-shared/dist/utils/promises`"; import { describe } from "vitest"; import { it } from "../../../../../helpers"; import { Project, niceBackendFetch } from "../../../../backend-helpers"; +async function waitForManagedDomainVerified(domainId: string, subdomain: string, senderLocalPart: string) { + const timeoutAt = performance.now() + 10_000; + while (true) { + const checkResponse = await niceBackendFetch("/api/v1/internal/emails/managed-onboarding/check", { + method: "POST", + accessType: "admin", + body: { + domain_id: domainId, + subdomain, + sender_local_part: senderLocalPart, + }, + }); + if (checkResponse.body.status === "verified") return; + if (performance.now() > timeoutAt) { + throw new Error(`Timed out waiting for managed domain ${domainId} to become verified`); + } + await wait(250); + } +} + describe("managed email onboarding internal endpoints", () => { ... - await wait(1500); + await waitForManagedDomainVerified(setupResponse.body.domain_id, "mail.example.com", "noreply"); ... - await wait(1500); + await waitForManagedDomainVerified(setupResponse.body.domain_id, "mail.example.com", "noreply");Also applies to: 243-244
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/e2e/tests/backend/endpoints/api/v1/internal/managed-email-onboarding.test.ts` around lines 205 - 206, Replace the fixed wait(1500) sleeps used before verification with a condition-based polling loop that repeatedly calls the /check endpoint until it returns a response indicating "verified" or a bounded timeout elapses; update the test that uses wait(1500) (and the other occurrence) to poll at short intervals (e.g., 200–500ms) for a maximum duration (e.g., 10s), assert success when the /check response contains "verified", and fail the test if the timeout is reached—use the existing wait helper only for the poll interval and keep the polling logic next to the verification steps so the test no longer depends on a fixed sleep.apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-settings/domain-settings.tsx (1)
326-341: ⚡ Quick winKey the Cloudflare lookup off the subdomain, not the whole setup object.
Line 341 depends on
setupState, so everysetSetupState({ ...setupState, status })during verification re-runs the same DoH lookup even though the queried domain did not change. That adds an extra third-party request to each verification click for no benefit.♻️ Proposed fix
+ const setupSubdomain = setupState?.subdomain; + useEffect(() => { - if (!props.open || stage !== 2 || !setupState) { + if (!props.open || stage !== 2 || setupSubdomain == null) { setCloudflareApex(null); return; } let cancelled = false; runAsynchronously(async () => { - const zone = await findZoneApex(setupState.subdomain); + const zone = await findZoneApex(setupSubdomain); if (cancelled || !zone) return; const usesCloudflare = zone.nameServers.some((n) => /(^|\.)cloudflare\.com$/i.test(n)); if (usesCloudflare) setCloudflareApex(zone.apex); }); return () => { cancelled = true; }; - }, [props.open, stage, setupState]); + }, [props.open, stage, setupSubdomain]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/email-settings/domain-settings.tsx around lines 326 - 341, The effect that runs findZoneApex is being re-triggered on every change to the entire setupState object; change its dependency to the specific subdomain so the DoH lookup only runs when the queried domain changes: in the useEffect that calls findZoneApex (and updates setCloudflareApex), replace the dependency on setupState with the subdomain value (e.g., setupState?.subdomain) and guard the early-return checks against that subdomain variable instead of the whole setupState to avoid unnecessary lookups when setSetupState({...setupState, status}) is called.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/backend/src/lib/managed-email-domains.tsx`:
- Around line 244-249: The COUNT query in globalPrismaClient.$queryRaw against
"ManagedEmailDomain" counts archived/inactive rows, causing the deletion flow to
think a zone is still referenced; update the WHERE clause in that query (the one
using options.subdomain and options.excludeId) to exclude archived/inactive
records (for example add AND "archivedAt" IS NULL or AND "isArchived" = false
depending on the actual schema column used for soft-deletes) so only live
domains are counted; make sure to use the same archived/active column and
semantics used elsewhere in the codebase.
In `@apps/backend/src/lib/managed-email-onboarding.tsx`:
- Around line 720-729: The current "last reference" check uses
countManagedEmailDomainsBySubdomainExcludingId followed separately by
deleteDnsimpleZoneByName, which is racy; serialize cleanup for a given subdomain
by acquiring a per-subdomain critical section (e.g., a DB advisory lock or mutex
keyed by domain.subdomain) and perform the count-and-delete inside the same
transaction/locked section so the decision is atomic. Update the logic around
countManagedEmailDomainsBySubdomainExcludingId and deleteDnsimpleZoneByName (or
the caller createOrReuseDnsimpleZone flow) to: acquire lock for
domain.subdomain, re-check count (excluding the row being deleted) inside the
lock/transaction, call deleteDnsimpleZoneByName only if count === 0, then
release the lock; ensure lock acquisition/release errors are handled and do not
leave the zone orphaned.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/email-settings/domain-settings.tsx:
- Around line 1085-1093: The icon-only delete button (the button with
onClick={() => setConfirmDelete(d)} and the <Trash /> icon) lacks an accessible
name; add an explicit aria-label (for example aria-label={`Remove domain
${d.domain}` or a generic "Remove domain") on that button to provide a clear
label for screen readers while keeping the existing title; ensure the aria-label
text conveys the destructive action and identifies the domain when possible.
---
Nitpick comments:
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/email-settings/domain-settings.tsx:
- Around line 326-341: The effect that runs findZoneApex is being re-triggered
on every change to the entire setupState object; change its dependency to the
specific subdomain so the DoH lookup only runs when the queried domain changes:
in the useEffect that calls findZoneApex (and updates setCloudflareApex),
replace the dependency on setupState with the subdomain value (e.g.,
setupState?.subdomain) and guard the early-return checks against that subdomain
variable instead of the whole setupState to avoid unnecessary lookups when
setSetupState({...setupState, status}) is called.
In
`@apps/e2e/tests/backend/endpoints/api/v1/internal/managed-email-onboarding.test.ts`:
- Around line 205-206: Replace the fixed wait(1500) sleeps used before
verification with a condition-based polling loop that repeatedly calls the
/check endpoint until it returns a response indicating "verified" or a bounded
timeout elapses; update the test that uses wait(1500) (and the other occurrence)
to poll at short intervals (e.g., 200–500ms) for a maximum duration (e.g., 10s),
assert success when the /check response contains "verified", and fail the test
if the timeout is reached—use the existing wait helper only for the poll
interval and keep the polling logic next to the verification steps so the test
no longer depends on a fixed sleep.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 535d9443-4e74-49ef-8313-e3dde8def862
⛔ Files ignored due to path filters (1)
apps/dashboard/public/assets/cloudflare-import-dns.pngis excluded by!**/*.png
📒 Files selected for processing (8)
apps/backend/src/app/api/latest/internal/emails/managed-onboarding/delete/route.tsxapps/backend/src/lib/managed-email-domains.tsxapps/backend/src/lib/managed-email-onboarding.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-settings/domain-settings.tsxapps/e2e/tests/backend/endpoints/api/v1/internal/managed-email-onboarding.test.tspackages/stack-shared/src/interface/admin-interface.tspackages/template/src/lib/stack-app/apps/implementations/admin-app-impl.tspackages/template/src/lib/stack-app/apps/interfaces/admin-app.ts
Comment on lines
+244
to
+249
| const rows = await globalPrismaClient.$queryRaw<{ count: bigint }[]>(Prisma.sql` | ||
| SELECT COUNT(*)::bigint AS count | ||
| FROM "ManagedEmailDomain" | ||
| WHERE "subdomain" = ${options.subdomain} | ||
| AND "id" <> ${options.excludeId} | ||
| `); |
Contributor
There was a problem hiding this comment.
Exclude inactive rows from the sibling count.
This query currently counts every row with the same subdomain. If an archived/inactive ManagedEmailDomain remains, the deletion flow will think the zone is still referenced and skip DNSimple cleanup even after the last live domain is removed.
Suggested fix
const rows = await globalPrismaClient.$queryRaw<{ count: bigint }[]>(Prisma.sql`
SELECT COUNT(*)::bigint AS count
FROM "ManagedEmailDomain"
WHERE "subdomain" = ${options.subdomain}
AND "id" <> ${options.excludeId}
+ AND "isActive" = true
`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const rows = await globalPrismaClient.$queryRaw<{ count: bigint }[]>(Prisma.sql` | |
| SELECT COUNT(*)::bigint AS count | |
| FROM "ManagedEmailDomain" | |
| WHERE "subdomain" = ${options.subdomain} | |
| AND "id" <> ${options.excludeId} | |
| `); | |
| const rows = await globalPrismaClient.$queryRaw<{ count: bigint }[]>(Prisma.sql` | |
| SELECT COUNT(*)::bigint AS count | |
| FROM "ManagedEmailDomain" | |
| WHERE "subdomain" = ${options.subdomain} | |
| AND "id" <> ${options.excludeId} | |
| AND "isActive" = true | |
| `); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/backend/src/lib/managed-email-domains.tsx` around lines 244 - 249, The
COUNT query in globalPrismaClient.$queryRaw against "ManagedEmailDomain" counts
archived/inactive rows, causing the deletion flow to think a zone is still
referenced; update the WHERE clause in that query (the one using
options.subdomain and options.excludeId) to exclude archived/inactive records
(for example add AND "archivedAt" IS NULL or AND "isArchived" = false depending
on the actual schema column used for soft-deletes) so only live domains are
counted; make sure to use the same archived/active column and semantics used
elsewhere in the codebase.
Comment on lines
+720
to
+729
| // createOrReuseDnsimpleZone lets multiple ManagedEmailDomain rows share a zone (when | ||
| // two tenancies pick the same subdomain). Only delete the zone if this row is the | ||
| // last one referencing it. | ||
| const remaining = await countManagedEmailDomainsBySubdomainExcludingId({ | ||
| subdomain: domain.subdomain, | ||
| excludeId: domain.id, | ||
| }); | ||
| if (remaining === 0) { | ||
| await deleteDnsimpleZoneByName(domain.subdomain); | ||
| } |
Contributor
There was a problem hiding this comment.
Serialize shared-zone cleanup for a subdomain.
This remaining check happens before the current row is removed and without any lock. If two sibling domains for the same subdomain are deleted concurrently, each request can observe one remaining row and both skip DNSimple zone deletion, leaking the shared zone permanently. This needs a per-subdomain critical section (for example, an advisory lock plus delete/count in one DB transaction) so the “last reference” decision is made atomically.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/backend/src/lib/managed-email-onboarding.tsx` around lines 720 - 729,
The current "last reference" check uses
countManagedEmailDomainsBySubdomainExcludingId followed separately by
deleteDnsimpleZoneByName, which is racy; serialize cleanup for a given subdomain
by acquiring a per-subdomain critical section (e.g., a DB advisory lock or mutex
keyed by domain.subdomain) and perform the count-and-delete inside the same
transaction/locked section so the decision is atomic. Update the logic around
countManagedEmailDomainsBySubdomainExcludingId and deleteDnsimpleZoneByName (or
the caller createOrReuseDnsimpleZone flow) to: acquire lock for
domain.subdomain, re-check count (excluding the row being deleted) inside the
lock/transaction, call deleteDnsimpleZoneByName only if count === 0, then
release the lock; ensure lock acquisition/release errors are handled and do not
leave the zone orphaned.
Comment on lines
+1085
to
+1093
| {!isInUse && ( | ||
| <button | ||
| type="button" | ||
| title="Remove domain" | ||
| onClick={() => setConfirmDelete(d)} | ||
| className="shrink-0 p-1.5 rounded-md hover:bg-destructive/10 text-muted-foreground hover:text-destructive transition-colors" | ||
| > | ||
| <Trash className="h-3.5 w-3.5" /> | ||
| </button> |
Contributor
There was a problem hiding this comment.
Give the delete icon button an explicit accessible name.
Line 1088 only provides title. For an icon-only destructive control, screen readers need an aria-label; otherwise the action is hard to discover and operate.
♿ Proposed fix
{!isInUse && (
<button
type="button"
+ aria-label={`Remove ${d.senderLocalPart}@${d.subdomain}`}
title="Remove domain"
onClick={() => setConfirmDelete(d)}
className="shrink-0 p-1.5 rounded-md hover:bg-destructive/10 text-muted-foreground hover:text-destructive transition-colors"
>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/email-settings/domain-settings.tsx
around lines 1085 - 1093, The icon-only delete button (the button with
onClick={() => setConfirmDelete(d)} and the <Trash /> icon) lacks an accessible
name; add an explicit aria-label (for example aria-label={`Remove domain
${d.domain}` or a generic "Remove domain") on that button to provide a clear
label for screen readers while keeping the existing title; ensure the aria-label
text conveys the destructive action and identifies the domain when possible.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test plan
pnpm test run managed-email-onboardingMade with Cursor
Summary by CodeRabbit