Handle NS record flow and auto-verification of sub-domains #2784
Conversation
WalkthroughThis PR introduces a new Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/(console)/project-[region]-[project]/settings/domains/add-domain/verify-[domain]/+page.svelte (1)
162-163: MissingruleStatusprop on NameserverTable.The
NameserverTablehere only receivesdomainandverified, but the functions verify page (and other usages in this PR) also passruleStatus={data.proxyRule.status}. This inconsistency will cause different badge rendering behavior.Suggested fix
- <NameserverTable domain={data.proxyRule.domain} {verified} /> + <NameserverTable + domain={data.proxyRule.domain} + {verified} + ruleStatus={data.proxyRule.status} />
🤖 Fix all issues with AI agents
In
@src/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/add-domain/verify-[domain]/+page.svelte:
- Around line 80-83: The invalidate calls after awaiting goto(routeBase) can
fail because the component may unmount; move the invalidation to before
navigation by calling invalidate(Dependencies.DOMAINS) and
invalidate(Dependencies.FUNCTION_DOMAINS) prior to awaiting goto(routeBase) so
the invalidations run in the current component context (keep the same Dependency
constants and routeBase variable).
In
@src/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/retryDomainModal.svelte:
- Around line 63-65: The catch block currently uses the parameter name "error",
which shadows the component's "error" state variable (declared earlier) and
prevents updating it; rename the catch parameter to something like "err" or
"caughtError", update any references inside that catch block to the new name,
and if the intent was to record the failure set the component state "error"
(e.g., error = String(err) or error = err.message) instead of ignoring it so the
component state is not shadowed.
In
@src/routes/(console)/project-[region]-[project]/settings/domains/add-domain/+page.svelte:
- Around line 41-50: In the catch block handling domain add failures (the
try/catch around the add-domain flow), make the error handling consistent by
guarding access to error.message with optional chaining and a safe fallback;
replace direct use of error.message with error?.message || String(error) (or
similar) and ensure the alreadyAdded check remains error?.type ===
'domain_already_exists' so you won’t throw if error is null/undefined.
In
@src/routes/(console)/project-[region]-[project]/settings/domains/add-domain/verify-[domain]/+page.svelte:
- Around line 80-82: The code calls await goto(routeBase) before await
invalidate(Dependencies.DOMAINS), which can unmount the page before cache is
refreshed; move the await invalidate(Dependencies.DOMAINS) call to before await
goto(routeBase) so Dependencies.DOMAINS is invalidated while the current
component is still mounted (update the sequence in the function that performs
navigation so invalidate(...) runs prior to goto(routeBase)).
In
@src/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/retryDomainModal.svelte:
- Around line 63-65: The catch block is shadowing the component state variable
named error; rename the catch parameter (e.g., from error to err or ex) in
retryDomainModal.svelte so you don't mask the component's error state, and if
you need to inspect or log the thrown exception use the new name (err) while
leaving the component-level error variable unchanged.
🧹 Nitpick comments (4)
src/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/add-domain/+page.svelte (1)
69-78: Consider defensive access forerror.message.If
erroris truthy but lacks amessageproperty, the notification would displayundefined. Consider using optional chaining or a fallback.💡 Suggested improvement
if (!alreadyAdded) { addNotification({ type: 'error', - message: error.message + message: error?.message ?? 'Failed to create apex domain' }); return; }src/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/add-domain/+page.svelte (2)
59-68: Consistent optional chaining on error object.Line 61 uses optional chaining (
error?.type) but line 65 accesseserror.messagedirectly. While unlikely to be undefined here, consider usingerror?.messagefor consistency.addNotification({ type: 'error', - message: error.message + message: error?.message ?? 'Failed to create apex domain' });
100-112: Defensive check for undefined rule.If
ruleis undefined (unlikely but possible if behaviour has an unexpected value),rule?.status !== 'created'would evaluate totrue, incorrectly treating the domain as verified.Suggested defensive improvement
- const verified = rule?.status !== 'created'; + const verified = rule && rule.status !== 'created'; if (verified) {src/routes/(console)/project-[region]-[project]/settings/domains/retryDomainModal.svelte (1)
63-65: Variable shadowing:errorparameter shadows component state.The
catch (error)on line 63 shadows theerrorstate variable declared on line 45. While the error is intentionally ignored here, this shadowing could cause confusion or unintended behavior if the code is modified later.Suggested fix
- } catch (error) { + } catch { // Ignore error }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/lib/components/domains/nameserverTable.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/add-domain/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/add-domain/verify-[domain]/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/table.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/add-domain/+page.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/add-domain/verify-[domain]/+page.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/table.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/add-domain/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/add-domain/verify-[domain]/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/table.svelte
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/routes/(console)/project-[region]-[project]/settings/domains/table.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/add-domain/verify-[domain]/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/table.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/table.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/add-domain/verify-[domain]/+page.sveltesrc/lib/components/domains/nameserverTable.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/add-domain/+page.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/add-domain/verify-[domain]/+page.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/add-domain/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/add-domain/+page.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/retryDomainModal.svelte
src/routes/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Files:
src/routes/(console)/project-[region]-[project]/settings/domains/table.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/add-domain/verify-[domain]/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/table.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/table.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/add-domain/verify-[domain]/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/add-domain/+page.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/add-domain/verify-[domain]/+page.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/add-domain/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/add-domain/+page.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/retryDomainModal.svelte
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/routes/(console)/project-[region]-[project]/settings/domains/table.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/add-domain/verify-[domain]/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/table.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/table.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/add-domain/verify-[domain]/+page.sveltesrc/lib/components/domains/nameserverTable.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/add-domain/+page.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/add-domain/verify-[domain]/+page.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/add-domain/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/add-domain/+page.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/retryDomainModal.svelte
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/routes/(console)/project-[region]-[project]/settings/domains/table.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/add-domain/verify-[domain]/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/table.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/table.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/add-domain/verify-[domain]/+page.sveltesrc/lib/components/domains/nameserverTable.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/add-domain/+page.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/add-domain/verify-[domain]/+page.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/add-domain/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/add-domain/+page.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/retryDomainModal.svelte
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(console)/project-[region]-[project]/settings/domains/table.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/add-domain/verify-[domain]/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/table.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/table.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/add-domain/verify-[domain]/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/add-domain/+page.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/add-domain/verify-[domain]/+page.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/add-domain/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/add-domain/+page.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/retryDomainModal.svelte
src/lib/components/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use PascalCase for component file names and place them in src/lib/components/[feature]/ directory structure
Files:
src/lib/components/domains/nameserverTable.svelte
🧠 Learnings (5)
📓 Common learnings
Learnt from: vermakhushboo
Repo: appwrite/console PR: 2364
File: src/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/add-domain/verify-[domain]/+page.svelte:52-89
Timestamp: 2025-09-24T10:27:36.797Z
Learning: In sites domain verification flows, domain creation should be handled in the background using promise chains (.then/.catch) with silent error handling, not awaited synchronously. This prevents domain creation failures from interrupting the main verification flow.
📚 Learning: 2025-09-24T10:27:36.797Z
Learnt from: vermakhushboo
Repo: appwrite/console PR: 2364
File: src/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/add-domain/verify-[domain]/+page.svelte:52-89
Timestamp: 2025-09-24T10:27:36.797Z
Learning: In sites domain verification flows, domain creation should be handled in the background using promise chains (.then/.catch) with silent error handling, not awaited synchronously. This prevents domain creation failures from interrupting the main verification flow.
Applied to files:
src/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/add-domain/verify-[domain]/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/add-domain/verify-[domain]/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/add-domain/+page.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/add-domain/verify-[domain]/+page.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/add-domain/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/add-domain/+page.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/retryDomainModal.svelte
📚 Learning: 2025-12-05T09:24:15.846Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2670
File: src/routes/(console)/organization-[organization]/+layout.ts:134-150
Timestamp: 2025-12-05T09:24:15.846Z
Learning: In the Appwrite console codebase, `prefs.organization` is maintained to always reference an organization with Platform.Appwrite, so when using preferences.organization as a fallback in redirect logic, no additional platform validation is required.
Applied to files:
src/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/add-domain/verify-[domain]/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/add-domain/verify-[domain]/+page.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/add-domain/verify-[domain]/+page.svelte
📚 Learning: 2025-10-13T05:13:54.542Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2413
File: src/routes/(console)/project-[region]-[project]/databases/table.svelte:33-39
Timestamp: 2025-10-13T05:13:54.542Z
Learning: In Svelte 5, `import { page } from '$app/state'` provides a reactive state proxy that can be accessed directly (e.g., `page.params`), unlike the older `import { page } from '$app/stores'` which returns a readable store requiring the `$page` syntax for auto-subscription in components.
Applied to files:
src/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/add-domain/verify-[domain]/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/add-domain/verify-[domain]/+page.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/add-domain/verify-[domain]/+page.svelte
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Applies to src/routes/**/*.svelte : Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Applied to files:
src/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/add-domain/verify-[domain]/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/add-domain/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/add-domain/+page.svelte
⏰ 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: build
- GitHub Check: e2e
🔇 Additional comments (16)
src/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/add-domain/+page.svelte (2)
61-61: LGTM!Using
constis appropriate sincedomainis only used for the existence check and is never reassigned.
110-122: LGTM!The refactored flow correctly:
- Invalidates domain data before status evaluation to ensure fresh state.
- Uses rule status to determine verification need, avoiding unnecessary verification pages for already-verified domains.
- Provides appropriate user feedback based on verification state.
src/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/add-domain/verify-[domain]/+page.svelte (1)
163-167: LGTM!The
NameserverTableusage correctly passes the newruleStatusprop alongsideverifiedanddomain, aligning with the updated component API.src/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/retryDomainModal.svelte (1)
135-149: LGTM!The
NameserverTableandRecordTablecomponents correctly receive theruleStatusprop and navigation callbacks, enabling proper state-driven rendering.src/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/table.svelte (1)
224-226: LGTM!The
domainsListprop is correctly passed toRetryDomainModal, enabling apex domain lookups for nameserver updates in cloud environments.src/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/retryDomainModal.svelte (1)
52-88: LGTM!The retry flow correctly implements the two-step verification (nameserver update followed by rule verification), with appropriate success/error handling and cache invalidation.
src/lib/components/domains/nameserverTable.svelte (2)
26-40: Consider handlingruleStatus === 'verified'explicitly.The badge logic checks
verified === trueafter exhausting otherruleStatusvalues. IfruleStatusis'verified'butverifiedisfalse(e.g., verification attempt failed on an already-verified domain), no badge renders. Consider whether showing the "Verified" badge based onruleStatus === 'verified'is more appropriate.{:else if ruleStatus === 'unverified'} <Badge variant="secondary" type="error" size="xs" content="Certificate generation failed" /> - {:else if verified === true} + {:else if verified === true || ruleStatus === 'verified'} <Badge variant="secondary" type="success" size="xs" content="Verified" /> {/if}
6-14: LGTM!The props API is well-structured with appropriate TypeScript typing. The
ruleStatusunion type clearly documents the expected state machine values.src/routes/(console)/project-[region]-[project]/settings/domains/retryDomainModal.svelte (2)
52-88: LGTM on the two-phase verification flow.The structure correctly separates the optional nameserver update (with silent error handling) from the main rule verification. This aligns with the established pattern where domain-related operations should not interrupt the main verification flow. The
verifiedstate management (undefined→true/false) provides clear tri-state feedback to child components.
136-139: TheruleStatusprop is properly declared in the NameserverTable component with correct TypeScript typing ('created' | 'verifying' | 'unverified' | 'verified'). No changes required.src/routes/(console)/project-[region]-[project]/settings/domains/table.svelte (1)
229-231: LGTM on wiringdomainsListprop to RetryDomainModal.The
organizationDomainsdata is correctly passed to the modal, enabling the apex domain resolution flow for nameserver updates in cloud environments.src/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/table.svelte (1)
224-226: LGTM - consistent with settings domain table.The
domainsListprop is correctly wired to the modal, maintaining consistency with the parallel implementation in the settings domains table.src/routes/(console)/project-[region]-[project]/settings/domains/add-domain/+page.svelte (1)
54-70: LGTM on the streamlined domain creation and verification flow.The logic correctly:
- Invalidates domain data before evaluating rule status
- Uses
rule?.status !== 'created'to determine verification success- Routes to verification page only when needed
This approach ensures the UI reflects the latest state before making navigation decisions.
src/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/add-domain/verify-[domain]/+page.svelte (3)
56-91: LGTM on the verification flow implementation.The two-phase approach correctly:
- Attempts nameserver update silently (consistent with the learning that domain operations should not interrupt main flow)
- Handles rule verification with proper success/error notifications
- Manages the
verifiedstate for child component feedbackThis aligns with the pattern established in
retryDomainModal.svelte.
163-166: Consistent prop passing to NameserverTable.The
ruleStatusprop is correctly passed fromdata.proxyRule.status, matching the pattern used in other domain verification components.
58-59:data.domainsListis properly provided by the page loader.The
+page.tsloader (lines 15-27) fetches and returnsdomainsListwith a guaranteed structure: either from the SDK or a fallback object withdomains: []. No additional null checks are needed.
...ion]-[project]/functions/function-[function]/domains/add-domain/verify-[domain]/+page.svelte
Show resolved
Hide resolved
...le)/project-[region]-[project]/functions/function-[function]/domains/retryDomainModal.svelte
Outdated
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/settings/domains/add-domain/+page.svelte
Show resolved
Hide resolved
...console)/project-[region]-[project]/settings/domains/add-domain/verify-[domain]/+page.svelte
Show resolved
Hide resolved
...outes/(console)/project-[region]-[project]/sites/site-[site]/domains/retryDomainModal.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/retryDomainModal.svelte (1)
42-50: Add a safeguard to ensure selectedTab always corresponds to a visible tab.In non-cloud deployments where all regional variables are missing or set to default values,
getDefaultTab()returns'nameserver'but the NS tab remains hidden sinceshowNSTabis false. This creates a UI inconsistency whereselectedTabpoints to an unavailable tab, and the NameserverTable content renders without a visible tab button.Either initialize
selectedTabto the first visible tab, or verify this configuration scenario cannot occur in practice.
🧹 Nitpick comments (4)
src/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/retryDomainModal.svelte (1)
81-86: Add type guard for error handling.Accessing
e.messageon anunknowntype may cause TypeScript issues in strict mode. Consider narrowing the type.Suggested improvement
} catch (e) { verified = false; - error = - e.message ?? - 'Domain verification failed. Please check your domain settings or try again later'; + error = e instanceof Error + ? e.message + : 'Domain verification failed. Please check your domain settings or try again later'; trackError(e, Submit.DomainUpdateVerification); }src/routes/(console)/project-[region]-[project]/settings/domains/retryDomainModal.svelte (3)
18-26: Minor inconsistency: missing default value forshowprop.The
showprop uses$bindable()without a default value, while the equivalent component in the sites directory uses$bindable(false). Consider adding a default for consistency.Suggested fix
let { - show = $bindable(), + show = $bindable(false), selectedProxyRule, domainsList
81-86: Add type guard for error handling.Same as the sites version—accessing
e.messageon anunknowntype may cause TypeScript issues. Consider narrowing the type.Suggested improvement
} catch (e) { verified = false; - error = - e.message ?? - 'Domain verification failed. Please check your domain settings or try again later'; + error = e instanceof Error + ? e.message + : 'Domain verification failed. Please check your domain settings or try again later'; trackError(e, Submit.DomainUpdateVerification); }
52-88: Consider extracting shared retry logic.This component shares ~95% of its code with
sites/site-[site]/domains/retryDomainModal.svelte. The only differences are the dependency constant (Dependencies.DOMAINSvsSITES_DOMAINS), theserviceprop value, and minor configuration. Consider extracting the sharedretryDomainlogic into a reusable helper to reduce duplication and ease future maintenance.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/retryDomainModal.svelte
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/retryDomainModal.svelte
src/routes/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Files:
src/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/retryDomainModal.svelte
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/retryDomainModal.svelte
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/retryDomainModal.svelte
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/retryDomainModal.svelte
🧠 Learnings (2)
📚 Learning: 2025-10-21T06:07:53.455Z
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2477
File: src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte:27-38
Timestamp: 2025-10-21T06:07:53.455Z
Learning: In the Appwrite Console codebase, the `copy` helper utility (src/lib/helpers/copy.ts) already handles errors internally with try-catch blocks and returns a boolean success indicator. Additional error handling with try-catch is not needed when calling this utility.
Applied to files:
src/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/retryDomainModal.svelte
📚 Learning: 2025-09-24T10:27:36.797Z
Learnt from: vermakhushboo
Repo: appwrite/console PR: 2364
File: src/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/add-domain/verify-[domain]/+page.svelte:52-89
Timestamp: 2025-09-24T10:27:36.797Z
Learning: In sites domain verification flows, domain creation should be handled in the background using promise chains (.then/.catch) with silent error handling, not awaited synchronously. This prevents domain creation failures from interrupting the main verification flow.
Applied to files:
src/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/settings/domains/retryDomainModal.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/retryDomainModal.svelte
⏰ 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: build
- GitHub Check: e2e
🔇 Additional comments (9)
src/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/retryDomainModal.svelte (3)
18-26: Props structure is well-defined.The addition of the optional
domainsListprop and proper typing forselectedProxyRuleis clean. The optional prop is correctly handled with?in the type definition.
55-65: Apex domain pre-verification logic is sound.The silent error handling for the nameserver update aligns with the established pattern where domain-related operations should not interrupt the main verification flow. Based on learnings, this is the correct approach.
135-150: Table components receive appropriate props.The
ruleStatusprop is correctly propagated to bothNameserverTableandRecordTable, enabling proper UI signaling for domain verification status.src/routes/(console)/project-[region]-[project]/settings/domains/retryDomainModal.svelte (1)
135-150: Correct props for settings context.The
service="general"andDependencies.DOMAINScorrectly differentiate this component from the sites version. TheruleStatusprop is properly propagated.src/routes/(console)/project-[region]-[project]/functions/function-[function]/domains/retryDomainModal.svelte (5)
1-16: LGTM!Imports are well-organized and use the
$libalias as per coding guidelines.
18-26: LGTM!Props are well-typed with appropriate optionality for
domainsList.
90-94: LGTM!Good practice to clear the error state when the modal is closed, ensuring a clean slate on reopen.
135-150: LGTM!The conditional rendering and prop passing to
NameserverTableandRecordTablecomponents is well-structured. TheruleStatusprop is consistently passed to both components for status-aware rendering.
52-88: API signature confirmed - updateNameservers acceptsdomainIdobject parameter.The verification logic follows the established pattern across domain verification flows (sites, settings, organization). Silent error handling for the nameserver update prevents interrupting the main rule verification flow. Note that the learning suggests using promise chains (
.then/.catch) instead ofawaitfor domain operations in background flows; however, this pattern is not currently implemented in the similar sites domain verification code either, suggesting it may be a future improvement target for consistency.
Closes SER-720
Closes SER-90
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.