feat: added modal for email verfication #2354
Conversation
ConsoleProject ID: Sites (2)
Note You can use Avatars API to generate QR code for any text or URLs. |
WalkthroughAdds a new Svelte component src/lib/components/account/sendVerificationEmailModal.svelte with a public Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 0
🧹 Nitpick comments (5)
src/lib/components/alerts/emailVerificationBanner.svelte (2)
11-12: Avoid hard-coded '/console' in route checkUse base from $app/paths to be robust to deployments behind a path prefix.
@@ -const notOnOnboarding = $derived(!page.url?.pathname?.includes('/console/onboarding')); +import { base } from '$app/paths'; +const notOnOnboarding = $derived(!page.url?.pathname?.includes(`${base}/onboarding`));
17-31: UX OK; consider Cloud-only visibility gateIf email verification is enforced only on Cloud, gate the banner with isCloud to avoid noise on self-hosted.
+ import { isCloud } from '$lib/system'; - const shouldShowEmailBanner = $derived(hasUser && needsEmailVerification && notOnOnboarding); + const shouldShowEmailBanner = $derived(isCloud && hasUser && needsEmailVerification && notOnOnboarding);src/lib/components/account/sendVerificationEmailModal.svelte (3)
41-44: Add Cancel button and a11y busy stateLet users back out and expose loading state to AT.
@@ - <svelte:fragment slot="footer"> - <Button submit disabled={creating}>Send email</Button> - </svelte:fragment> + <svelte:fragment slot="footer"> + <Button secondary on:click={() => (show = false)} disabled={creating}>Cancel</Button> + <Button submit disabled={creating} aria-busy={creating}>Send email</Button> + </svelte:fragment>
22-24: Harden error messagingSDK errors aren’t always guaranteed to have .message. Normalize before display to avoid undefined.
- } catch (error) { - addNotification({ message: error.message, type: 'error' }); + } catch (error) { + const message = typeof error === 'object' && error && 'message' in error + ? String((error as any).message) + : 'Failed to send verification email'; + addNotification({ message, type: 'error' });
13-21: Use a canonical verification return URL (not the current page path)Change src/lib/components/account/sendVerificationEmailModal.svelte to stop sending page.url.pathname as the return URL and instead point the verification link to the route that finalizes verification (src/routes/(console)/account/updateMfa.svelte — it calls updateVerification with userId/secret). This ensures the emailed link always lands on a handler that processes the userId/secret (e.g., use
${page.url.origin}${base}/account/updateMfaor similar).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/lib/components/account/sendVerificationEmailModal.svelte(1 hunks)src/lib/components/alerts/emailVerificationBanner.svelte(1 hunks)src/routes/(console)/+layout.svelte(1 hunks)
🔇 Additional comments (2)
src/routes/(console)/+layout.svelte (1)
349-349: Verify placement: banner rendered outside ShellRendering after may bypass header layout constraints. If HeaderAlert is expected to live within Shell’s layout region, consider moving it inside Shell (e.g., right after ) or confirm CSS still positions it correctly across routes.
src/lib/components/alerts/emailVerificationBanner.svelte (1)
11-15: Svelte 5 present — confirm runes are enabled before using $state/$derivedpackage.json shows "svelte": "^5.25.3" but svelte.config.* contains no
runes: true. If this repo uses SvelteKit (SvelteKit automatically enables Svelte 5 runes) the new$state/$derivedare fine; otherwise enable runes in svelte.config or revert to classic reactivity. Fallback (classic reactivity) diff:@@ -const hasUser = $derived(!!$user); -const needsEmailVerification = $derived(hasUser && !$user.emailVerification); -const notOnOnboarding = $derived(!page.url?.pathname?.includes('/console/onboarding')); -const shouldShowEmailBanner = $derived(hasUser && needsEmailVerification && notOnOnboarding); - -let showSendVerification = $state(false); +$: hasUser = !!$user; +$: needsEmailVerification = hasUser && !$user.emailVerification; +$: notOnOnboarding = !page.url?.pathname?.includes('/console/onboarding'); +$: shouldShowEmailBanner = hasUser && needsEmailVerification && notOnOnboarding; + +let showSendVerification = false;
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/routes/(console)/account/updateEmail.svelte (1)
77-83: Consider increasing gap for better visual spacingThe current gap of
0.5remmight be too tight. Consider using0.75remor1remfor better visual balance between the label and badge.<style> .email-badge { display: flex; align-items: center; - gap: 0.5rem; + gap: 0.75rem; } </style>src/lib/components/account/sendVerificationEmailModal.svelte (2)
34-57: Clean up URL parameters after processingThe verification parameters remain in the URL after processing. Consider cleaning them up to prevent reprocessing and maintain a clean URL state.
async function updateEmailVerification() { const searchParams = page.url.searchParams; const userId = searchParams.get('userId'); const secret = searchParams.get('secret'); if (userId && secret) { + // Clean up URL immediately to prevent reprocessing + const cleanUrl = new URL(page.url); + cleanUrl.searchParams.delete('userId'); + cleanUrl.searchParams.delete('secret'); + history.replaceState(null, '', cleanUrl.toString()); + try { await sdk.forConsole.account.updateVerification({ userId, secret }); addNotification({ message: 'Email verified successfully', type: 'success' }); await Promise.all([ invalidate(Dependencies.ACCOUNT), invalidate(Dependencies.FACTORS) ]); } catch (error) { addNotification({ message: error.message, type: 'error' }); } } }
68-71: Improve text readability with semantic HTMLUsing inline styles for emphasis is not ideal. Consider using semantic HTML elements for better accessibility and maintainability.
- <Typography.Text variant="m-400" gap="m"> - To continue using Appwrite Cloud, please verify your email address. An email will be - sent to <Typography.Text variant="m-600" style="display:inline" - >{$user?.email}</Typography.Text> - </Typography.Text> + <Typography.Text variant="m-400" gap="m"> + To continue using Appwrite Cloud, please verify your email address. An email will be + sent to <strong>{$user?.email}</strong> + </Typography.Text>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/lib/components/account/sendVerificationEmailModal.svelte(1 hunks)src/routes/(console)/account/updateEmail.svelte(3 hunks)src/routes/(console)/account/updateMfa.svelte(3 hunks)
🧰 Additional context used
🪛 ESLint
src/routes/(console)/account/updateMfa.svelte
[error] 87-87: Empty block statement.
(no-empty)
⏰ 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: e2e
- GitHub Check: build
🔇 Additional comments (4)
src/routes/(console)/account/updateEmail.svelte (1)
47-49: Good UX improvement with the verification badgeThe addition of the verification badge provides clear visual feedback about the email verification status, which is a nice user experience enhancement.
src/routes/(console)/account/updateMfa.svelte (2)
61-61: Good fix for history state managementMoving
history.replaceStateinside the conditional block prevents unnecessary URL modifications when verification parameters aren't present.
90-90: Consistent cache invalidation approachGood alignment with the cache invalidation pattern used elsewhere in the codebase, ensuring both account and factors data are refreshed after MFA changes.
src/lib/components/account/sendVerificationEmailModal.svelte (1)
59-61: No change required — API call is already guarded.updateEmailVerification reads page.url.searchParams and only calls sdk.forConsole.account.updateVerification when both userId and secret are present; the modal is only mounted when opened, so onMount won’t trigger redundant API requests.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
2 participants
What does this PR do?
Test Plan
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)
Summary by CodeRabbit
New Features
Changes