feat: improve domain verification UX with enhanced feedback#202
feat: improve domain verification UX with enhanced feedback#202enemyrr wants to merge 3 commits intousesend:mainfrom
Conversation
- Add proper loading states with spinner for verify button - Show detailed verification status messages during process - Add toast notifications for success, error, and start events - Improve button text based on verification state - Add visual progress indicators and status section - Enhanced error handling in startVerification mutation - Track verification completion and show appropriate feedback
WalkthroughStricter delete confirmation (requires exact domain name), richer domain verification UX with toasts/spinners/status section and polling, and server-side hardening of startVerification to validate domain existence, set isVerifying/status, and return a structured { success, message } response. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant UI as Domains Page (UI)
participant API as startVerification (trpc)
participant DB as Database
U->>UI: Click "Verify domain"
UI->>UI: Show "Starting verification..." toast
UI->>API: startVerification(domainId)
API->>DB: findUnique(domainId)
alt domain found
API->>DB: update(isVerifying=true, status=PENDING)
API-->>UI: { success: true, message }
UI->>UI: start polling (refetchInterval)
UI-->>U: Show "Checking DNS records..." / spinner
else domain not found
API-->>UI: TRPCError NOT_FOUND
UI-->>U: Show error toast "Domain not found"
end
rect rgba(230,245,255,0.6)
loop Polling while isVerifying
UI->>DB: fetch domain status
DB-->>UI: status
alt status -> SUCCESS
UI-->>U: Show success toast "Verified"
UI->>UI: Button shows "Verified ✓ - Check again"
else status -> FAILED
UI-->>U: Show error toast "Verification failed"
UI->>UI: Show retry state and guidance
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
✨ Finishing Touches
🧪 Generate unit tests
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/app/(dashboard)/domains/[domainId]/page.tsx (2)
23-37: Do not use React’s experimental use() in a client component; use useParams instead.Passing Promise props and calling use() on the client is unstable and will break on React <19. Next.js recommends useParams in client components.
Apply this diff:
-import React, { use, useEffect, useRef } from "react"; +import React, { useEffect, useRef } from "react"; +import { useParams } from "next/navigation"; @@ -export default function DomainItemPage({ - params, -}: { - params: Promise<{ domainId: string }>; -}) { - const { domainId } = use(params); +export default function DomainItemPage() { + const { domainId } = useParams<{ domainId: string }>();
202-224: Fix MX row status binding
The MX row is wired tospfDetails, which in your service maps to the AWS MailFromDomainStatus—not an MX-specific status. Since there is nomxStatus(or similar) in the API, this will render the wrong badge for MX.• File:
apps/web/src/app/(dashboard)/domains/[domainId]/page.tsx
– Lines ~219–223: the<DnsVerificationStatus>under the “MX” row usesdomainQuery.data?.spfDetails.
• Service mapping: inapps/web/src/server/service/domain-service.ts,spfDetailsis assigned fromMailFromAttributes.MailFromDomainStatus, not MX.Suggested fixes:
- Remove the status badge from the MX row if no separate MX status exists.
- —OR— Rename
spfDetailsin your service and schema tomailFromDomainStatus(or similar) and adjust the UI label to “Mail-From Domain Status” instead of “SPF” or “MX.”- <TableCell className=""> - <DnsVerificationStatus status={domainQuery.data?.spfDetails ?? "NOT_STARTED"} /> - </TableCell> + <!-- Remove or replace this block since `spfDetails` isn’t an MX-specific status -->
🧹 Nitpick comments (9)
apps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsx (6)
120-123: Harden the disable gate against whitespace/case mismatches.Users can accidentally include leading/trailing spaces or different casing. Normalize both sides to avoid unnecessary friction while still requiring an exact domain match.
Apply this diff:
- disabled={ - deleteDomainMutation.isPending || - domainForm.watch("domain") !== domain.name - } + disabled={ + deleteDomainMutation.isPending || + (domainForm.watch("domain") ?? "") + .trim() + .toLowerCase() !== domain.name.toLowerCase() + }
50-57: Normalize comparison in submit handler as well.Mirror the same normalization in the server-side check to keep behavior consistent between UI gating and form validation.
- if (values.domain !== domain.name) { + const typed = (values.domain ?? "").trim().toLowerCase(); + if (typed !== domain.name.toLowerCase()) { domainForm.setError("domain", { message: "Domain name does not match", }); return; }
58-70: Surface API errors to the user and keep cache consistent.Add an onError toast and optionally invalidate onSettled to ensure lists refresh even when the dialog remains open.
- deleteDomainMutation.mutate( + deleteDomainMutation.mutate( { id: domain.id, }, { onSuccess: () => { utils.domain.domains.invalidate(); setOpen(false); toast.success(`Domain ${domain.name} deleted`); router.replace("/domains"); }, + onError: (err) => { + toast.error(err.message || "Failed to delete domain"); + }, + onSettled: () => { + // Keep cache fresh regardless of outcome + utils.domain.domains.invalidate(); + }, } );
39-39: Remove unused local state.domainName and setDomainName are never used; drop them to reduce noise.
- const [domainName, setDomainName] = useState("");
75-77: Simplify onOpenChange.React will ignore no-op state sets; the conditional is unnecessary.
- onOpenChange={(_open) => (_open !== open ? setOpen(_open) : null)} + onOpenChange={setOpen}
117-126: Optional: show a spinner while deleting for consistency with Verify button UX.Your Button in the verification flow uses isLoading/showSpinner. Mirror that here for consistent feedback.
- <Button + <Button type="submit" variant="destructive" disabled={ deleteDomainMutation.isPending || domainForm.watch("domain") !== domain.name } - > - {deleteDomainMutation.isPending ? "Deleting..." : "Delete"} + isLoading={deleteDomainMutation.isPending} + showSpinner + > + {deleteDomainMutation.isPending ? "Deleting..." : "Delete"}apps/web/src/app/(dashboard)/domains/[domainId]/page.tsx (3)
70-86: Guard double-submissions and surface server message.Avoid starting verification while already verifying/pending, and display the message returned by the mutation for consistency with the backend.
- const handleVerify = () => { - toast.info("Starting domain verification..."); - verifyQuery.mutate( + const handleVerify = () => { + if (verifyQuery.isPending || domainQuery.data?.isVerifying) return; + toast.info("Starting domain verification..."); + verifyQuery.mutate( { id: Number(domainId) }, { - onSuccess: () => { - toast.success("Verification started successfully"); + onSuccess: (res) => { + toast.success(res?.message ?? "Verification started successfully"); }, onError: (error) => { toast.error(`Verification failed: ${error.message}`); }, onSettled: () => { domainQuery.refetch(); }, } ); };
125-141: Disable Verify while verifying to prevent redundant requests.The button shows a spinner but remains clickable when isVerifying is true. Disable it in that state as well.
- <Button + <Button variant="outline" onClick={handleVerify} isLoading={verifyQuery.isPending || domainQuery.data?.isVerifying} showSpinner={true} - disabled={verifyQuery.isPending} + disabled={verifyQuery.isPending || domainQuery.data?.isVerifying} >
382-401: Tighten prop typing for DnsVerificationStatus.This component expects domain verification states; constrain the prop to the known set to catch typos at compile time.
-const DnsVerificationStatus: React.FC<{ status: string }> = ({ status }) => { +type DomainStatusValue = "NOT_STARTED" | "PENDING" | "TEMPORARY_FAILURE" | "FAILED" | "SUCCESS"; +const DnsVerificationStatus: React.FC<{ status: DomainStatusValue }> = ({ status }) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsx(2 hunks)apps/web/src/app/(dashboard)/domains/[domainId]/page.tsx(4 hunks)apps/web/src/server/api/routers/domain.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
{apps,packages}/**/*.{js,jsx,ts,tsx,css,scss,md,mdx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier with the Tailwind plugin for code formatting
Files:
apps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsxapps/web/src/server/api/routers/domain.tsapps/web/src/app/(dashboard)/domains/[domainId]/page.tsx
{apps,packages}/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps,packages}/**/*.{js,jsx,ts,tsx}: Group imports by source (internal/external) and alphabetize them
Use camelCase for variables and functions, PascalCase for components and classes
Use try/catch with specific error types for error handling
Files:
apps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsxapps/web/src/server/api/routers/domain.tsapps/web/src/app/(dashboard)/domains/[domainId]/page.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps,packages}/**/*.{ts,tsx}: Use strong typing in TypeScript, avoidany, and use Zod for validation
Follow Vercel style guides with strict TypeScript
Files:
apps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsxapps/web/src/server/api/routers/domain.tsapps/web/src/app/(dashboard)/domains/[domainId]/page.tsx
{apps,packages}/**/*.{jsx,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps,packages}/**/*.{jsx,tsx}: Use functional React components with hooks and group related hooks together
In React components, structure code with props at the top, hooks next, helper functions, then JSX
Files:
apps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsxapps/web/src/app/(dashboard)/domains/[domainId]/page.tsx
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use tRPC for internal API endpoints
Files:
apps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsxapps/web/src/server/api/routers/domain.tsapps/web/src/app/(dashboard)/domains/[domainId]/page.tsx
**/*.{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)/domains/[domainId]/delete-domain.tsxapps/web/src/server/api/routers/domain.tsapps/web/src/app/(dashboard)/domains/[domainId]/page.tsx
🔇 Additional comments (5)
apps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsx (1)
104-104: Nice UX improvement: placeholder matches the actual domain.Showing the real domain as the placeholder reduces user error and clarifies the confirmation step. Looks good.
apps/web/src/server/api/routers/domain.ts (1)
36-45: I've requested inspection of thedomainProcedureand theteamProcedureimplementation to confirm whether team-based scoping and authorization are enforced. I’ll update my review comment once we have clarity.apps/web/src/app/(dashboard)/domains/[domainId]/page.tsx (3)
51-69: LGTM: clear, minimal transition toasts based on prior status.The previousStatusRef pattern avoids initial-render toasts and keeps transitions crisp. Nicely done.
149-185: LGTM: verification status panel adds great, actionable feedback.The progressive copy and iconography make state transitions obvious without being noisy.
56-64: No action needed: this is a server componentThe
page.tsxunderapps/web/src/app/(dashboard)/domains/[domainId]/page.tsxdoes not include a"use client"directive, so it’s a Next.js Server Component by default. Imports from@prisma/clienthere run entirely on the server and do not bloat the client bundle or affect edge runtimes. You can safely leave theDomainStatusenum imports as-is.Likely an incorrect or invalid review comment.
- Replace generic error throws with TRPCError for better error management - Use DomainStatus enum for setting domain status - Improve error messages for domain not found and user email validation
|
@enemyrr is attempting to deploy a commit to the kmkoushik's projects Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
apps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsx (3)
120-123: Trim input before comparison to prevent false negatives; consider useWatch.Users often add accidental spaces. Trim before equality check (keep case-sensitive per “exact” requirement). Optional: prefer useWatch over calling watch inside JSX.
Apply within these lines:
- disabled={ - deleteDomainMutation.isPending || - domainForm.watch("domain") !== domain.name - } + disabled={ + deleteDomainMutation.isPending || + (domainForm.watch("domain")?.trim() ?? "") !== domain.name + }And update submit validation accordingly (outside this hunk):
// in onDomainDelete const typed = values.domain.trim(); if (typed !== domain.name) { domainForm.setError("domain", { message: "Domain name does not match" }); return; }
39-39: Remove unused state.domainName/setDomainName are unused; delete to reduce noise.
- const [domainName, setDomainName] = useState("");
1-1: Rename file to PascalCase
Renameapps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsxtoDeleteDomain.tsx; no import references found that require updating.apps/web/src/server/api/routers/domain.ts (2)
39-56: Add idempotency guard to avoid redundant writes and duplicate UX toasts.Early-return when already verifying or verified.
startVerification: domainProcedure.mutation(async ({ ctx, input }) => { try { const domain = await ctx.db.domain.findUnique({ where: { id: input.id }, }); if (!domain) { throw new TRPCError({ code: "NOT_FOUND", message: "Domain not found" }); } + if (domain.isVerifying || domain.status === DomainStatus.VERIFIED) { + return { + success: true, + message: domain.status === DomainStatus.VERIFIED + ? "Domain already verified" + : "Domain verification already in progress", + }; + } await ctx.db.domain.update({ where: { id: input.id }, data: { isVerifying: true, status: DomainStatus.PENDING, }, });
68-114: Optional: standardize db access.This router mixes ctx.db and db. Pick one (prefer ctx.db) for consistency and easier mocking.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsx(2 hunks)apps/web/src/app/(dashboard)/domains/[domainId]/page.tsx(4 hunks)apps/web/src/server/api/routers/domain.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/app/(dashboard)/domains/[domainId]/page.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/server/api/routers/domain.tsapps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use 2-space indentation in TypeScript code (enforced by Prettier)
Use semicolons in TypeScript code (enforced by Prettier)
Do not use dynamic imports
Files:
apps/web/src/server/api/routers/domain.tsapps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsx
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (AGENTS.md)
Format code with Prettier 3 via pnpm format for TypeScript and Markdown files
Files:
apps/web/src/server/api/routers/domain.tsapps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsx
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
In apps/web, use the
/ alias for src imports (e.g., import { x } from "/utils/x")
Files:
apps/web/src/server/api/routers/domain.tsapps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Name React component files in PascalCase (e.g., AppSideBar.tsx)
Files:
apps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsx
🔇 Additional comments (4)
apps/web/src/app/(dashboard)/domains/[domainId]/delete-domain.tsx (1)
104-104: LGTM: clearer confirmation hint.Using the exact domain as the placeholder improves clarity for destructive confirmation.
apps/web/src/server/api/routers/domain.ts (3)
2-3: LGTM: typed errors and enum import.Brings stronger guarantees and consistent error semantics.
117-122: LGTM: precise error codes for not-found and bad-request.Improves client handling and telemetry.
56-66: LGTM: structured payload + robust TRPCError propagation. We couldn’t locate any client-sidestartVerificationcalls—please manually verify that the UI invokes this RPC and handles the returned{ success, message }payload instead of expecting void.
Summary
Changes Made
Test Plan
Summary by CodeRabbit
New Features
Improvements