✨ New Login Page #1504
✨ New Login Page #1504
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
WalkthroughThis pull request introduces a comprehensive overhaul of the authentication and user experience flow in the web application. The changes focus on restructuring the login, registration, and verification processes, enhancing internationalization, and improving the overall user interface. Key modifications include creating modular authentication components, updating localization files, implementing more robust error handling, and refining the authentication middleware. Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
lukevella
force-pushed
the
guest-user-flow
branch
from
1b9d7d9 to
dbfbfae
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 32
🔭 Outside diff range comments (1)
apps/web/src/i18n/server.ts (1)
Line range hint
6-13: Add locale validation and error handling.The translation function needs:
- Validation for locale parameter
- Error handling for invalid locales
- Type safety improvements
+import { availableLocales } from "./settings"; + +function isValidLocale(locale: string): locale is typeof availableLocales[number] { + return availableLocales.includes(locale as any); +} + export async function getTranslation(localeOverride?: string) { const localeFromPath = getLocaleFromPath(); const locale = localeOverride || localeFromPath; + + if (!isValidLocale(locale)) { + console.warn(`Invalid locale: ${locale}, falling back to default`); + locale = availableLocales[0]; + } + const i18nextInstance = await initI18next(locale, defaultNS); return { t: i18nextInstance.getFixedT(locale, defaultNS), i18n: i18nextInstance, }; }
🧹 Nitpick comments (33)
apps/web/src/app/[locale]/(auth)/layout.tsx (1)
47-49: Simplify theclassNameusage with thecnfunction.The
cnfunction is used to conditionally join class names. Since there's only one class string being passed, the use ofcnis unnecessary here. You can pass the class string directly toclassName.Apply this diff to simplify:
- className={cn( - "[mask-image:radial-gradient(400px_circle_at_top,white,transparent)]", - )} + className="[mask-image:radial-gradient(400px_circle_at_top,white,transparent)]"apps/web/src/app/[locale]/(auth)/login/page.tsx (2)
25-37: Enhance caching strategy for OAuth providers.While caching OAuth providers using
unstable_cacheimproves performance, consider the implications of not revalidating the cache. If providers change or configurations are updated, the cache may become stale.Update the caching options to include a reasonable revalidation period:
const getCachedOAuthProviders = unstable_cache( getOAuthProviders, ["oauth-providers"], { - revalidate: false, + revalidate: 60, // Revalidate every 60 seconds }, );
48-58: Simplify provider filtering logic.In the
LoginPagecomponent, the filtering ofoAuthProvidersforsocialProvidersandoidcProvidercan be optimized by usingArray.partition(if available) or by reducing the number of iterations over the array.Consider refactoring as follows:
const oAuthProviders = await getCachedOAuthProviders(); -const socialProviders = oAuthProviders.filter( - (provider) => provider.id !== "oidc", -); - -const oidcProvider = oAuthProviders.find( - (provider) => provider.id === "oidc", -); + +const socialProviders = []; +let oidcProvider = null; +for (const provider of oAuthProviders) { + if (provider.id === "oidc") { + oidcProvider = provider; + } else { + socialProviders.push(provider); + } +}packages/utils/src/sleep.ts (1)
1-3: Consider usingAbortControllerfor better control over the sleep function.While the
sleepfunction works as intended, introducing anAbortControllercan provide the ability to cancel the sleep if needed, enhancing its usefulness in various scenarios.Here's an example implementation:
export function sleep(ms: number) { - return new Promise((resolve) => setTimeout(resolve, ms)); + const controller = new AbortController(); + const { signal } = controller; + return { + promise: new Promise<void>((resolve, reject) => { + const timeout = setTimeout(() => resolve(), ms); + signal.addEventListener('abort', () => { + clearTimeout(timeout); + reject(new Error('Sleep aborted')); + }); + }), + controller, + }; }This allows you to abort the sleep when needed by calling
controller.abort().apps/web/src/app/[locale]/(auth)/register/components/schema.ts (1)
3-6: Enhance form validation rules.Consider adding more specific validation rules and custom error messages for better user experience.
export const registerNameFormSchema = z.object({ - name: z.string().min(1), - email: z.string().email(), + name: z.string() + .min(2, "Name must be at least 2 characters") + .max(50, "Name must be less than 50 characters") + .regex(/^[a-zA-Z\s-']+$/, "Name can only contain letters, spaces, hyphens and apostrophes"), + email: z.string() + .email("Please enter a valid email address") + .max(100, "Email must be less than 100 characters"), });apps/web/src/app/[locale]/(auth)/login/components/or-divider.tsx (1)
1-9: Enhance accessibility for the divider component.While the implementation is clean, consider these accessibility improvements:
-export function OrDivider({ text }: { text: string }) { +export function OrDivider({ text }: { text: string }) { return ( - <div className="flex items-center gap-x-2.5"> + <div + className="flex items-center gap-x-2.5" + role="separator" + aria-orientation="horizontal" + > <hr className="grow border-gray-100" /> - <div className="text-muted-foreground lowercase">{text}</div> + <div className="text-muted-foreground" style={{ textTransform: 'lowercase' }}> + {text} + </div> <hr className="grow border-gray-100" /> </div> ); }This improves screen reader support by:
- Adding proper ARIA roles
- Moving text transformation to CSS to avoid affecting screen readers
apps/web/src/features/quick-create/components/relative-date.tsx (1)
1-14: LGTM! Consider memoization for list usage.The component is well-structured and follows React best practices. If this component is used within lists or frequently re-rendered containers, consider memoizing it with
React.memoto optimize performance.-export function RelativeDate({ date }: { date: Date }) { +export const RelativeDate = React.memo(function RelativeDate({ date }: { date: Date }) { return ( <Trans i18nKey="createdTime" defaults="Created {relativeTime}" values={{ relativeTime: dayjs(date).fromNow() }} /> ); -} +});apps/web/src/app/[locale]/(auth)/login/components/auth-errors.tsx (2)
8-18: Consider handling additional OAuth error cases.While the current implementation handles the "OAuthAccountNotLinked" case well, consider extending error handling for other common OAuth scenarios (e.g., "AccessDenied", "EmailVerificationRequired").
12-15: Move default error message to translation file.Consider moving the hardcoded default value to the translation file for better maintainability and consistency.
- defaultValue: - "A user with this email already exists. Please log in using the original method.", + defaultValue: t("errors.accountNotLinked"),apps/web/src/features/quick-create/quick-create-button.tsx (1)
12-18: Add aria-label for better accessibility.Consider adding an aria-label to improve accessibility for screen readers.
- <Button className="rounded-full" asChild> + <Button className="rounded-full" aria-label={t("quickCreate")} asChild>apps/web/src/components/input-otp.tsx (1)
4-26: Improve accessibility and user experience.Add ARIA attributes and keyboard handling for better accessibility.
const InputOTP = React.forwardRef< HTMLInputElement, React.ComponentProps<typeof Input> & { onValidCode?: (code: string) => void } >(({ onValidCode, onChange, ...rest }, ref) => { return ( <Input ref={ref} {...rest} + aria-label="One-time password input" + aria-description="Enter the 6-digit code sent to your device" onChange={(e) => { onChange?.(e); if (e.target.value.length === 6) { onValidCode?.(e.target.value); } }} + onKeyDown={(e) => { + if (e.key === 'Enter' && e.currentTarget.value.length === 6) { + onValidCode?.(e.currentTarget.value); + } + }} maxLength={6} data-1p-ignore inputMode="numeric" autoComplete="one-time-code" pattern="\d{6}" /> ); });apps/web/src/components/poll/language-selector.tsx (1)
21-24: Consider adding aria-label for accessibility.The addition of the globe icon improves visual clarity. Consider adding an aria-label to the button to ensure screen readers can properly announce its purpose.
- <Button variant="ghost"> + <Button variant="ghost" aria-label="Select language">apps/web/src/app/[locale]/(auth)/components/auth-page.tsx (2)
1-3: Consider using semantic HTML section element.For better document structure and accessibility, consider using
sectioninstead ofdivfor the container.- return <div className="space-y-8 lg:space-y-10">{children}</div>; + return <section className="space-y-8 lg:space-y-10">{children}</section>;
13-19: Add aria-description for accessibility.Since this component is used for descriptive text, consider adding an appropriate ARIA role.
- return <p className="text-muted-foreground">{children}</p>; + return <p className="text-muted-foreground" role="doc-subtitle">{children}</p>;apps/web/src/app/[locale]/(auth)/components/full-page-card.tsx (2)
7-9: Consider responsive padding and max-width constraints.The current implementation might stretch too wide on larger screens. Consider adding max-width and adjusting padding for better readability.
- <div className="flex h-screen flex-col gap-6 bg-gray-50 p-4"> - <div className="rounded-xl border bg-white p-4">{children}</div> + <div className="flex h-screen flex-col items-center gap-6 bg-gray-50 p-4"> + <div className="w-full max-w-md rounded-xl border bg-white p-4 sm:p-6">{children}</div>
21-23: Add appropriate heading styles.The title component should maintain consistent heading styles with the rest of the application.
- return <h1>{children}</h1>; + return <h1 className="text-2xl font-semibold tracking-tight">{children}</h1>;packages/ui/src/dot-pattern.tsx (1)
31-37: Consider enhancing SVG accessibility.While
aria-hidden="true"is correctly used for decorative SVG, consider addingrole="presentation"as a defensive measure for older screen readers.<svg aria-hidden="true" + role="presentation" className={cn( "pointer-events-none absolute inset-0 h-full w-full fill-gray-300", className, )} {...props}apps/web/src/app/[locale]/quick-create/page.tsx (1)
11-14: Consider adding a redirect for self-hosted users.Instead of showing a 404 page, consider redirecting users to a more appropriate page or showing a helpful message explaining why the feature is unavailable.
if (isSelfHosted) { - // self hosted users should not see this page - notFound(); + redirect("/polls"); // or another appropriate page }apps/web/src/app/[locale]/(auth)/login/sso-providers.tsx (1)
23-30: Consider adding cache invalidation strategy.The current implementation uses
revalidate: false, which means the cache will never be invalidated. Consider adding a reasonable revalidation period.const getCachedOAuthProviders = unstable_cache( getOAuthProviders, ["oauth-providers"], { - revalidate: false, + revalidate: 60 * 5, // Revalidate every 5 minutes }, );apps/web/src/app/[locale]/(auth)/login/components/sso-provider.tsx (3)
13-16: Optimize image loading for SSO provider logos.Add
priorityprop to the Image components to optimize loading of above-the-fold images, and consider adding explicit width/height for better CLS (Cumulative Layout Shift) performance.- <Image src="/static/google.svg" width={16} alt="Google" height={16} /> + <Image + src="/static/google.svg" + width={16} + height={16} + alt="Google" + priority + />Also applies to: 19-26
49-60: Enhance button accessibility.While the button has an aria-label, it could benefit from additional accessibility improvements.
<Button size="lg" + type="button" + role="button" aria-label={t("continueWithProvider", { provider: name, ns: "app", defaultValue: "Continue with {{provider}}", })} + aria-busy={false} + aria-disabled={false} key={providerId} onClick={() => { signIn(providerId); }} >
11-38: Consider using a more maintainable approach for provider icons.The current implementation with multiple if statements could be refactored to use a mapping object for better maintainability.
+const providerIcons = { + google: () => ( + <Image src="/static/google.svg" width={16} height={16} alt="Google" priority /> + ), + "azure-ad": () => ( + <Image src="/static/microsoft.svg" width={16} height={16} alt="Microsoft" priority /> + ), + oidc: () => ( + <Icon> + <UserIcon /> + </Icon> + ), +} as const; + function SSOImage({ provider }: { provider: string }) { - if (provider === "google") { - return ( - <Image src="/static/google.svg" width={16} alt="Google" height={16} /> - ); - } - - if (provider === "azure-ad") { - return ( - <Image - src="/static/microsoft.svg" - width={16} - alt="Microsoft" - height={16} - /> - ); - } - - if (provider === "oidc") { - return ( - <Icon> - <UserIcon /> - </Icon> - ); - } - - return null; + const IconComponent = providerIcons[provider as keyof typeof providerIcons]; + return IconComponent ? <IconComponent /> : null; }apps/web/src/app/[locale]/(auth)/register/page.tsx (2)
16-60: Consider adding loading and error states for better UX.While the registration form implementation looks good, it would benefit from explicit loading and error states to handle various scenarios:
- Network issues during translation loading
- Form submission states
- Registration failures
export default async function Register({ params, }: { params: { locale: string }; }) { const { t } = await getTranslation(params.locale); + // Add error boundary wrapper + return ( + <ErrorBoundary fallback={<AuthErrorState />}> <AuthPageContainer> {/* ... existing content ... */} </AuthPageContainer> + </ErrorBoundary> ); }
Line range hint
61-69: Enhance metadata for SEO optimization.While the basic metadata is good, consider adding more SEO-friendly metadata:
export async function generateMetadata({ params, }: { params: { locale: string }; }) { const { t } = await getTranslation(params.locale); return { title: t("register"), + description: t("registerDescription"), + robots: "noindex, nofollow", // Prevent indexing of auth pages }; }apps/web/src/app/[locale]/(auth)/auth/login/components/login-page.tsx (1)
42-61: Consider adding loading states and error boundaries.The UI shows skeleton states for name/email but doesn't handle failed data fetches or other error scenarios.
Consider wrapping the content in an error boundary and adding appropriate error states:
+ import { ErrorBoundary } from "@/components/error-boundary"; + <div className="flex h-full w-full flex-col items-center justify-center"> + <ErrorBoundary fallback={<div>Failed to load user information</div>}> <div className="w-48 space-y-8 text-center"> // ... existing content </div> + </ErrorBoundary> </div>apps/web/src/app/[locale]/(auth)/register/verify/components/otp-form.tsx (1)
95-97: Add confirmation step before auto-submission.The current implementation automatically submits the form when a valid code is entered, which could lead to accidental submissions before the user has verified their input.
Consider adding a brief delay or confirmation step:
onValidCode={() => { + // Add a brief delay to allow user to verify input + setTimeout(() => { handleSubmit(); + }, 500); }}apps/web/src/components/user-provider.tsx (1)
110-113: Add error handling to logout process.The logout process should handle potential failures gracefully to ensure users aren't left in an inconsistent state.
Add error handling:
logout: async () => { + try { await signOut(); posthog?.capture("logout"); posthog?.reset(); + } catch (error) { + console.error("Logout failed:", error); + // Optionally show a user-friendly error message + throw new Error("Failed to sign out. Please try again."); + } },apps/web/src/app/[locale]/(auth)/login/components/login-email-form.tsx (1)
81-93: Improve input field security and accessibility.The input field has potential security and accessibility issues:
data-1p-ignoreprevents password managers, which might hurt user experience- Missing ARIA attributes for better accessibility
Apply these improvements:
<Input {...field} size="lg" type="text" - data-1p-ignore + aria-label={t("email")} + aria-required="true" disabled={formState.isSubmitting || formState.isSubmitSuccessful} autoFocus={true} placeholder={t("emailPlaceholder")} - autoComplete="false" + autoComplete="email" error={!!formState.errors.identifier} />apps/web/src/app/[locale]/(auth)/register/components/register-name-form.tsx (2)
74-81: Consider adding aria-label for better accessibility.The name input field could benefit from an aria-label to improve screen reader support.
<Input {...field} size="lg" data-1p-ignore placeholder={t("namePlaceholder")} disabled={form.formState.isSubmitting} autoFocus={true} + aria-label={t("name")} />
97-102: Add data-1p-ignore attribute for consistency.The email input field is missing the
data-1p-ignoreattribute that's present on the name input field.<Input size="lg" placeholder={t("emailPlaceholder")} disabled={form.formState.isSubmitting} + data-1p-ignore {...field} />apps/web/src/features/quick-create/quick-create-widget.tsx (2)
119-119: Fix duplicate word in translation key.The text contains a duplicate word "notifications".
- defaults="Get email notifications notifications" + defaults="Get email notifications"
51-54: Add aria-label to improve list item accessibility.The list items containing polls could benefit from descriptive aria-labels.
<Link href={`/poll/${poll.id}`} className="flex items-center gap-3 rounded-xl border bg-white p-3 hover:bg-gray-50 active:bg-gray-100" + aria-label={`View poll: ${poll.title}`} >apps/web/src/trpc/routers/user.ts (1)
129-139: Improve error message for better debugging.While the added error handling is good, consider making the error message more descriptive to aid in debugging.
- message: "User not found", + message: "Failed to find user in database during email change request",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (69)
apps/web/public/locales/en/app.json(1 hunks)apps/web/src/app/[locale]/(admin)/app-card.tsx(1 hunks)apps/web/src/app/[locale]/(auth)/auth/login/components/login-page.tsx(1 hunks)apps/web/src/app/[locale]/(auth)/auth/login/page.tsx(1 hunks)apps/web/src/app/[locale]/(auth)/components/auth-page.tsx(1 hunks)apps/web/src/app/[locale]/(auth)/components/full-page-card.tsx(1 hunks)apps/web/src/app/[locale]/(auth)/components/version-badge.tsx(1 hunks)apps/web/src/app/[locale]/(auth)/layout.tsx(1 hunks)apps/web/src/app/[locale]/(auth)/login/actions.ts(1 hunks)apps/web/src/app/[locale]/(auth)/login/components/auth-errors.tsx(1 hunks)apps/web/src/app/[locale]/(auth)/login/components/login-email-form.tsx(1 hunks)apps/web/src/app/[locale]/(auth)/login/components/login-with-oidc.tsx(1 hunks)apps/web/src/app/[locale]/(auth)/login/components/or-divider.tsx(1 hunks)apps/web/src/app/[locale]/(auth)/login/components/sso-provider.tsx(1 hunks)apps/web/src/app/[locale]/(auth)/login/loading.tsx(1 hunks)apps/web/src/app/[locale]/(auth)/login/login-form.tsx(0 hunks)apps/web/src/app/[locale]/(auth)/login/page.tsx(1 hunks)apps/web/src/app/[locale]/(auth)/login/sso-providers.tsx(1 hunks)apps/web/src/app/[locale]/(auth)/login/verify/components/otp-form.tsx(1 hunks)apps/web/src/app/[locale]/(auth)/login/verify/page.tsx(1 hunks)apps/web/src/app/[locale]/(auth)/register/actions.ts(1 hunks)apps/web/src/app/[locale]/(auth)/register/components/register-name-form.tsx(1 hunks)apps/web/src/app/[locale]/(auth)/register/components/schema.ts(1 hunks)apps/web/src/app/[locale]/(auth)/register/loading.tsx(1 hunks)apps/web/src/app/[locale]/(auth)/register/page.tsx(1 hunks)apps/web/src/app/[locale]/(auth)/register/register-page.tsx(0 hunks)apps/web/src/app/[locale]/(auth)/register/verify/components/otp-form.tsx(1 hunks)apps/web/src/app/[locale]/(auth)/register/verify/page.tsx(1 hunks)apps/web/src/app/[locale]/quick-create/page.tsx(1 hunks)apps/web/src/app/api/trpc/[trpc]/route.ts(0 hunks)apps/web/src/app/components/user-language-switcher.tsx(1 hunks)apps/web/src/app/providers.tsx(1 hunks)apps/web/src/auth.ts(2 hunks)apps/web/src/components/auth/auth-forms.tsx(0 hunks)apps/web/src/components/auth/auth-layout.tsx(0 hunks)apps/web/src/components/input-otp.tsx(1 hunks)apps/web/src/components/logo.tsx(2 hunks)apps/web/src/components/poll/language-selector.tsx(2 hunks)apps/web/src/components/user-provider.tsx(2 hunks)apps/web/src/components/user.tsx(1 hunks)apps/web/src/components/version-display.tsx(1 hunks)apps/web/src/features/quick-create/components/relative-date.tsx(1 hunks)apps/web/src/features/quick-create/constants.ts(1 hunks)apps/web/src/features/quick-create/index.ts(1 hunks)apps/web/src/features/quick-create/lib/get-guest-polls.ts(1 hunks)apps/web/src/features/quick-create/quick-create-button.tsx(1 hunks)apps/web/src/features/quick-create/quick-create-widget.tsx(1 hunks)apps/web/src/i18n/server.ts(1 hunks)apps/web/src/middleware.ts(2 hunks)apps/web/src/style.css(2 hunks)apps/web/src/trpc/client/config.ts(1 hunks)apps/web/src/trpc/context.ts(0 hunks)apps/web/src/trpc/routers/auth.ts(1 hunks)apps/web/src/trpc/routers/dashboard.ts(1 hunks)apps/web/src/trpc/routers/polls.ts(3 hunks)apps/web/src/trpc/routers/scheduled-events.ts(1 hunks)apps/web/src/trpc/routers/user.ts(4 hunks)apps/web/src/trpc/trpc.ts(1 hunks)apps/web/src/utils/locale/get-locale-from-path.ts(1 hunks)apps/web/tests/authentication.spec.ts(7 hunks)package.json(1 hunks)packages/languages/index.ts(1 hunks)packages/ui/src/button.tsx(1 hunks)packages/ui/src/dot-pattern.tsx(1 hunks)packages/ui/src/form.tsx(1 hunks)packages/ui/src/input.tsx(1 hunks)packages/ui/src/label.tsx(1 hunks)packages/utils/src/sleep.ts(1 hunks)scripts/inject-version.js(1 hunks)
💤 Files with no reviewable changes (6)
- apps/web/src/components/auth/auth-layout.tsx
- apps/web/src/app/api/trpc/[trpc]/route.ts
- apps/web/src/trpc/context.ts
- apps/web/src/app/[locale]/(auth)/register/register-page.tsx
- apps/web/src/app/[locale]/(auth)/login/login-form.tsx
- apps/web/src/components/auth/auth-forms.tsx
✅ Files skipped from review due to trivial changes (4)
- apps/web/src/features/quick-create/index.ts
- apps/web/src/app/[locale]/(auth)/login/loading.tsx
- apps/web/src/features/quick-create/constants.ts
- apps/web/src/app/[locale]/(auth)/auth/login/page.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Integration tests
🔇 Additional comments (33)
apps/web/src/components/version-display.tsx (1)
1-3: Verify the environment variable configuration.Ensure that
NEXT_PUBLIC_APP_VERSIONis properly configured in the build pipeline and development environment.Let's verify the environment variable configuration:
✅ Verification successful
Environment variable configuration is properly implemented ✅
The
NEXT_PUBLIC_APP_VERSIONis correctly configured with:
- Build-time injection via
inject-version.jscombining package version and git hash- Proper Docker environment setup
- TypeScript declarations for type safety
- Consistent usage across components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for environment variable configuration in various config files echo "Checking for NEXT_PUBLIC_APP_VERSION configuration..." # Check common Next.js environment files for file in $(fd -t f "^\.env(\..+)?$"); do echo "Checking $file:" rg "NEXT_PUBLIC_APP_VERSION" "$file" || echo "Not found in $file" done # Check for usage in other files echo -e "\nChecking for other usages:" rg "NEXT_PUBLIC_APP_VERSION"Length of output: 1134
Script:
#!/bin/bash # Check the version injection script echo "Content of scripts/inject-version.js:" cat scripts/inject-version.js || echo "Script not found" # Check if there's any package.json version field that might be used echo -e "\nChecking package.json version fields:" rg '"version"' package.jsonLength of output: 547
apps/web/src/app/[locale]/(auth)/layout.tsx (1)
58-63: Verify the structure of themetadataexport.Ensure that the
metadataobject conforms to Next.js guidelines. Thetitlefield should be either a string or an object with specific properties. Double-check thattemplateanddefaultare valid properties and used correctly.Refer to the Next.js documentation on Metadata to confirm the correct usage.
packages/languages/index.ts (1)
5-6: LGTM! Good addition for i18n support.The explicit declaration of
defaultLocaleenhances the internationalization configuration.apps/web/src/app/[locale]/(auth)/register/loading.tsx (1)
1-9: Clean and well-structured loading component!The implementation follows React and Next.js best practices, with proper styling and flexible height adjustment.
apps/web/src/components/logo.tsx (2)
26-31: LGTM! Good improvement to image dimension handling.The change to use style-based dimensions with
width: 0, height: 0is a recommended pattern for Next.js Image component when using dynamic dimensions. This approach provides better control over aspect ratio and responsiveness.
5-5: Verify the impact of increased logo width.The width for the 'sm' size has been increased from 120px to 140px. This change might affect existing layouts using the small logo variant.
✅ Verification successful
Width change appears safe to proceed with
The codebase analysis shows no direct usage of the Logo component with
size="sm"prop. Existing usages either rely on the default "md" size or handle layout through className props, making the width increase from 120px to 140px a safe change.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find all files that use the Logo component with "sm" size rg -l "Logo.*size.*sm" --type tsxLength of output: 63
Script:
#!/bin/bash # Find Logo component imports and usages rg -l "Logo" --type ts --type tsx --type jsx || rg -l "Logo" # If files are found, show context for Logo usage rg "Logo" -A 2 -B 2 --type ts --type tsx --type jsx || rg "Logo" -A 2 -B 2Length of output: 9892
apps/web/src/trpc/routers/dashboard.ts (1)
6-6: Security improvement: Restricted dashboard info to authenticated users only.The change from
possiblyPublicProceduretoprivateProcedureimproves security by ensuring only authenticated users can access dashboard information.✅ Verification successful
Security improvement verified: Dashboard info endpoint correctly restricted
The change from
possiblyPublicProceduretoprivateProcedureis verified as a security improvement:
privateProcedureenforces strict authentication for all users- Dashboard information is now protected regardless of deployment type
- The change aligns with security best practices for sensitive data
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any remaining public procedures in other routers rg "possiblyPublicProcedure" --type tsLength of output: 1177
Script:
#!/bin/bash # Check the implementation of possiblyPublicProcedure ast-grep --pattern 'export const possiblyPublicProcedure = $_'Length of output: 8845
Script:
#!/bin/bash # Check the implementation of privateProcedure ast-grep --pattern 'export const privateProcedure = $_'Length of output: 8762
packages/ui/src/label.tsx (1)
11-11: Verify the impact of label color change across the application.The label text color has been changed from muted to standard foreground, which could affect the visual hierarchy across all forms.
Run the following script to identify all components using this label:
✅ Verification successful
Label color change to foreground is appropriate for its usage context
The Label component is primarily used in settings, forms, and administrative interfaces where clear readability is important. The change from muted to standard foreground color aligns well with these use cases, improving text visibility in critical user interfaces.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find all components using the Label component rg -l "from.*['\"]@rallly/ui/.*label['\"]" --type tsxLength of output: 87
Script:
#!/bin/bash # Search for Label imports (both absolute and relative) rg "import.*Label.*from.*['\"](@rallly/ui|\.\.?/.*label)['\"]" # Search for Label component usage patterns ast-grep --pattern '<Label $$$>$$$</Label>'Length of output: 3174
apps/web/src/components/user.tsx (1)
28-28: Verify size consistency across components.The size increase from
size-10tosize-12for the "lg" variant should be consistent with other large-sized components in the application.✅ Verification successful
Size-12 usage is consistent with existing patterns
The size increase from
size-8tosize-12for the "lg" variant aligns with existing patterns in the codebase, where larger components like date cards and scheduled events consistently usesize-12. Different components maintain their own appropriate size progressions based on their specific needs and visual hierarchy.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other components using size-10 or size-12 classes echo "Searching for size-10 usage:" rg 'size-10' --type tsx echo -e "\nSearching for size-12 usage:" rg 'size-12' --type tsxLength of output: 243
Script:
#!/bin/bash # Search for size classes in tsx/jsx files with context echo "Searching for size-10 usage:" rg 'size-10' -g '*.tsx' -g '*.jsx' -B 1 -A 1 echo -e "\nSearching for size-12 usage:" rg 'size-12' -g '*.tsx' -g '*.jsx' -B 1 -A 1 # Also search for other size classes to understand the pattern echo -e "\nSearching for other common size patterns:" rg 'size-[0-9]+' -g '*.tsx' -g '*.jsx' | sort | uniqLength of output: 17744
packages/ui/src/dot-pattern.tsx (1)
5-15: LGTM! Well-structured props interface.The
DotPatternPropsinterface is well-defined with optional parameters and includes a catch-all for additional props.apps/web/src/trpc/client/config.ts (1)
9-27: 🛠️ Refactor suggestionReplace direct window.location with Next.js router.
Using
window.locationfor navigation can cause a full page reload. Consider using Next.js router for a smoother experience.+import { useRouter } from "next/navigation"; const errorHandlingLink: TRPCLink<AppRouter> = () => { + const router = useRouter(); return ({ next, op }) => { return observable((observer) => { const unsubscribe = next(op).subscribe({ next: (result) => observer.next(result), error: (error) => { if ( error instanceof TRPCClientError && error.data?.code === "UNAUTHORIZED" ) { - window.location.href = "/login"; + router.push("/login"); } observer.error(error); }, }); return unsubscribe; }); }; };Likely invalid or redundant comment.
apps/web/src/trpc/routers/scheduled-events.ts (1)
15-15: Verify the impact of restricting scheduled events access.The change from
possiblyPublicProceduretoprivateProceduremeans that scheduled events can only be accessed by authenticated users. While this improves security, it's a breaking change that could affect existing integrations or public event sharing functionality.✅ Verification successful
Security change aligns with admin-only usage
The change to
privateProcedureis appropriate as the scheduled events list endpoint is only used within admin-protected routes. No public-facing components or integrations are affected by this change.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any public-facing components or routes that might be affected rg -l 'scheduledEvents.*list'Length of output: 149
Script:
#!/bin/bash # Check the content of the files to understand the context echo "=== Content of past-events.tsx ===" cat "apps/web/src/app/[locale]/(admin)/events/past-events.tsx" echo -e "\n=== Content of upcoming-events.tsx ===" cat "apps/web/src/app/[locale]/(admin)/events/upcoming-events.tsx" # Search for alternative usage patterns echo -e "\n=== Searching for other potential usages ===" rg -l "trpc.*scheduled.*events"Length of output: 2787
packages/ui/src/input.tsx (2)
16-16: LGTM! Focus state styling improved.The updated focus state styling with
focus:ring-ring focus:ring-2provides better visual feedback and aligns with modern UI practices.
24-24: LGTM! Text size adjustment improves readability.Changing the large input variant from
text-lgtotext-basemaintains better visual hierarchy and consistency.apps/web/src/app/providers.tsx (1)
Line range hint
18-25: LGTM! Stability improvement in tRPC configuration.The change from
unstable_overridestooverridesindicates that this API is now stable. The mutation behavior remains unchanged.apps/web/src/app/[locale]/(auth)/register/page.tsx (1)
1-15: LGTM! Well-structured imports and component organization.The imports are well-organized, separating external dependencies from internal components. The auth page components are properly modularized.
apps/web/src/app/[locale]/(auth)/login/verify/page.tsx (1)
44-51: Add rate limiting for OTP attempts.The OTP form implementation should include rate limiting to prevent brute force attacks.
✅ Verification successful
Rate limiting is already implemented
The application has proper rate limiting implemented through tRPC middleware using a sliding window algorithm that limits requests to 5 per minute per IP address. This protection applies to all authentication-related endpoints, including OTP verification.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if rate limiting is implemented in the API routes rg -A 5 "export.*async.*function.*verify.*otp"Length of output: 48
Script:
#!/bin/bash # Find OTPForm component implementation ast-grep --pattern 'export $_ function OTPForm' # Find API routes containing 'otp' fd -e ts -e tsx 'otp' apps/web/src/app/api # Search for rate limiting related code rg -i "ratelimit|rate.?limit" --type ts --type tsxLength of output: 173
Script:
#!/bin/bash # Find OTPForm component with broader pattern ast-grep --pattern 'OTPForm' # Find auth/verification related files fd -e ts -e tsx '(auth|verify|otp)' apps/web/src # Search for rate limiting with fixed flags rg -i "ratelimit|rate.?limit" -t tsLength of output: 2503
Script:
#!/bin/bash # Check auth router for OTP verification rg -A 10 "verify.*otp" apps/web/src/trpc/routers/auth.tsLength of output: 58
Script:
#!/bin/bash # Check auth router with broader patterns rg -A 10 "otp|verif" apps/web/src/trpc/routers/auth.ts # Also check the OTP form implementation cat apps/web/src/app/[locale]/\(auth\)/login/verify/components/otp-form.tsxLength of output: 2903
apps/web/src/app/[locale]/(admin)/app-card.tsx (1)
43-43: LGTM! Consistent border radius.The change from
rounded-lgtorounded-mdaligns with the medium border radius used in other size variants of the component.packages/ui/src/button.tsx (2)
13-13: Great accessibility improvement with focus-visible!The addition of
focus-visiblering improves keyboard navigation accessibility.
31-33: Verify button size changes across the application.The height changes for default and lg sizes might affect existing layouts.
Let's check the impact:
✅ Verification successful
Height changes are safe to proceed with
The button height modifications are safe as they maintain the default size and use the large size (h-12) only in appropriate contexts like authentication forms and hero sections where taller buttons are beneficial for user interaction.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find all button size="lg" usages rg -l 'size="lg"' | xargs rg -A 2 -B 2 'size="lg"'Length of output: 12457
Script:
#!/bin/bash # Find Button component usage with default size (implicit, no size prop) rg -l '<Button' | xargs rg -A 2 -B 2 '<Button(?![^>]*size=)'Length of output: 294
Script:
#!/bin/bash # Find all Button component usage rg -l '<Button' | xargs rg -B 2 -A 2 '<Button\s|<Button>'Length of output: 28950
apps/web/src/trpc/routers/auth.ts (1)
32-32: LGTM! Validation change is equivalent.The change from
nonempty()tomin(1)maintains the same validation requirements.apps/web/tests/authentication.spec.ts (1)
140-146: Add test for rate limiting.Consider adding a test case to verify rate limiting behavior for multiple failed login attempts.
packages/ui/src/form.tsx (1)
85-93: Consider preserving error state styling for FormLabel.The removal of error state styling from FormLabel might reduce the visual feedback for users. Consider keeping the error state styling or documenting why it was removed.
apps/web/src/trpc/routers/user.ts (2)
28-28: LGTM! Security improvement by restricting billing access.The change from
possiblyPublicProceduretoprivateProcedureensures that billing information is only accessible to authenticated users.
156-156: LGTM! Using database values for email operations.Using
currentUser.emailfrom the database instead ofctx.user.emailis more reliable as it ensures we're working with the latest data.Also applies to: 170-170
apps/web/src/auth.ts (2)
162-162: LGTM! Improved provider identification.Adding
name: "Microsoft"enhances the clarity of the authentication provider in the UI.
189-189: LGTM! Added verification request page.The addition of
verifyRequest: "/login/verify"aligns with the PR's objective of overhauling the authentication flow.apps/web/src/trpc/routers/polls.ts (1)
45-45: LGTM! Enhanced security by restricting poll access.The change from
possiblyPublicProceduretoprivateProcedurefor bothgetCountByStatusandinfiniteListensures that poll data is only accessible to authenticated users, which aligns with the PR's security improvements.Also applies to: 65-65
apps/web/src/trpc/trpc.ts (1)
67-74: LGTM! Improved authentication check.The new implementation using
isGuest !== falseis more secure as it strictly ensures only authenticated users can access private routes. This change properly handles all edge cases whereisGuestmight be undefined or null.apps/web/src/style.css (3)
18-19: LGTM! Proper full-height layout implementation.The height rule applied to both body and #__next ensures consistent full-height layout across the application.
27-29: LGTM! Enhanced text readability.Adding
leading-normalto paragraphs improves readability by setting appropriate line height.
42-42: LGTM! Modern link styling.The updated text-link class provides better accessibility with focus states and maintains modern styling conventions.
apps/web/public/locales/en/app.json (1)
284-309: LGTM! Comprehensive authentication localization.The new authentication-related strings provide clear and consistent messaging throughout the login and registration flow.
apps/web/src/app/[locale]/(auth)/register/components/register-name-form.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/web/src/app/[locale]/(auth)/login/actions.ts (1)
6-11:⚠️ Potential issueAdd input validation and rate limiting for security.
The server action needs essential security measures:
- Email format validation
- Rate limiting to prevent enumeration attacks
- Optimized database query
+import { z } from "zod"; +import { rateLimit } from "@/utils/rate-limit"; + +const emailSchema = z.string().email(); + export async function setVerificationEmail(email: string) { + const rateLimitResult = await rateLimit.check(req.ip); + if (!rateLimitResult.success) { + throw new Error("Too many attempts. Please try again later."); + } + + const parsedEmail = emailSchema.safeParse(email); + if (!parsedEmail.success) { + throw new Error("Invalid email format"); + } + const count = await prisma.user.count({ where: { - email, + email: parsedEmail.data, }, + take: 1, });
🧹 Nitpick comments (7)
apps/web/src/app/[locale]/(auth)/layout.tsx (2)
10-14: Consider adding error boundary for async component.Since this is an async component, consider implementing an error boundary to gracefully handle any potential errors during rendering.
19-21: Enhance accessibility for the Logo component.Consider adding an aria-label to the Logo component for better screen reader support.
- <Logo className="mx-auto" /> + <Logo className="mx-auto" aria-label="Rallly Logo" />apps/web/tests/create-delete-poll.spec.ts (2)
35-35: Enhance test coverage for the delete poll flow.The URL redirect to the login page with a callback is correct and aligns with the new authentication requirements. However, consider adding additional test coverage:
- Verify that the poll is actually deleted after completing the authentication flow
- Verify that accessing the deleted poll's URL returns an appropriate error
- Verify that the user is correctly redirected back to /polls after login
Would you like me to help implement these additional test cases?
Line range hint
7-36: Improve test reliability and isolation.While the tests work, there are a few improvements that could make them more reliable:
- Add explicit waitFor conditions before dialog interactions to handle animations/transitions
- Consider splitting the create and delete tests to run independently for better isolation
- Add error handling for cases where the dialog or buttons might not appear
Example improvement for dialog interaction:
await manageButton.click(); - await page.click("text=Delete"); + const deleteButton = page.getByText("Delete"); + await deleteButton.waitFor({ state: "visible" }); + await deleteButton.click(); const deletePollDialog = page.getByRole("dialog"); + await deletePollDialog.waitFor({ state: "visible" });apps/web/tests/authentication.spec.ts (1)
117-131: Consider adding rate limiting tests.While the error handling for invalid codes is now covered, consider adding tests for rate limiting or multiple failed attempts to ensure the security measures are working as expected.
Would you like me to provide an example test case for rate limiting verification?
apps/web/src/app/[locale]/(auth)/login/actions.ts (2)
13-18: Enhance cookie security configuration.While the essential security attributes are set, consider these improvements:
- Add
path: "/"attribute to restrict cookie scope- Document the maxAge duration in the code
cookies().set("verification-email", email, { httpOnly: true, secure: process.env.NODE_ENV === "production", sameSite: "lax", - maxAge: 15 * 60, + maxAge: 15 * 60, // 15 minutes + path: "/", });
20-20: Consider providing more detailed error information.Instead of just returning a boolean, consider returning a more detailed status that can help the UI show appropriate messages.
- return count > 0; + return { + exists: count > 0, + status: count > 0 ? "found" : "not_found", + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/public/locales/en/app.json(1 hunks)apps/web/src/app/[locale]/(auth)/layout.tsx(1 hunks)apps/web/src/app/[locale]/(auth)/login/actions.ts(1 hunks)apps/web/src/app/[locale]/(auth)/register/actions.ts(1 hunks)apps/web/src/features/quick-create/quick-create-widget.tsx(1 hunks)apps/web/tests/authentication.spec.ts(6 hunks)apps/web/tests/create-delete-poll.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/src/features/quick-create/quick-create-widget.tsx
- apps/web/src/app/[locale]/(auth)/register/actions.ts
- apps/web/public/locales/en/app.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Integration tests
🔇 Additional comments (8)
apps/web/src/app/[locale]/(auth)/layout.tsx (3)
1-9: LGTM! Well-organized imports.The imports are properly structured and all dependencies are relevant to the component's functionality.
25-44: LGTM! Well-implemented responsive design.The conditional rendering and responsive layout implementation is well thought out:
- Mobile-first approach with proper breakpoints
- Clean separation of mobile/desktop layouts
- Efficient use of flexbox for layout control
50-55: LGTM! Well-structured metadata configuration.The metadata configuration follows Next.js best practices with proper title templating.
apps/web/tests/authentication.spec.ts (4)
43-43: LGTM! Consistent button label updates.The change from "Login with Email" to "Continue with Email" across all test cases improves clarity and better represents the multi-step authentication flow.
Also applies to: 98-98, 124-124, 140-140, 156-156
54-54: LGTM! More personalized registration text.The update to "Create Your Account" adds a personal touch to the registration flow and is consistently applied.
Also applies to: 77-77
117-131: Thank you for implementing the suggested test case!The new test case properly validates the error handling for invalid verification codes.
69-69: LGTM! Simplified login flow verification.The removal of redundant button clicks and direct verification of the "Test User" text improves test reliability while maintaining proper authentication validation.
Also applies to: 145-145
apps/web/src/app/[locale]/(auth)/login/actions.ts (1)
1-4: LGTM! Proper server-side setup.The server action is correctly configured with the necessary imports and "use server" directive.
lukevella
force-pushed
the
guest-user-flow
branch
from
da21890 to
d84ca86
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/web/src/app/[locale]/(auth)/register/verify/components/otp-form.tsx (2)
31-31: Consider using a more specific type for the token prop.The
tokenprop could benefit from a more specific type definition to ensure type safety and improve code documentation.-export function OTPForm({ token }: { token: string }) { +type OTPFormProps = { + token: string; // Registration verification token +}; + +export function OTPForm({ token }: OTPFormProps) {
109-117: Enhance user feedback during form submission.Consider adding a loading spinner or text to indicate the submission state.
<Button className="w-full" variant="primary" type="submit" size="lg" disabled={form.formState.isSubmitting} > + {form.formState.isSubmitting ? ( + <span className="flex items-center gap-2"> + <Spinner size="sm" /> + <Trans i18nKey="creating" defaults="Creating..." /> + </span> + ) : ( <Trans i18nKey="createAccount" defaults="Create Account" /> + )} </Button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/src/app/[locale]/(auth)/register/verify/components/otp-form.tsx(1 hunks)apps/web/tests/authentication.spec.ts(6 hunks)apps/web/tests/create-delete-poll.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/tests/create-delete-poll.spec.ts
- apps/web/tests/authentication.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Unit tests
- GitHub Check: Integration tests
- GitHub Check: Type check
- GitHub Check: Linting
🔇 Additional comments (3)
apps/web/src/app/[locale]/(auth)/register/verify/components/otp-form.tsx (3)
25-27: LGTM! Schema validation is well-defined.The OTP validation schema correctly enforces a 6-character requirement, which is a standard length for OTP codes.
49-73: Enhance error handling in form submission.The form submission lacks proper error handling for network failures and doesn't provide adequate user feedback.
Consider memoizing the submit handler.
The submit handler could be memoized using
useCallbackto prevent unnecessary re-renders.+import { useCallback } from "react"; + - const handleSubmit = form.handleSubmit(async (data) => { + const handleSubmit = useCallback(form.handleSubmit(async (data) => { // ... existing code ... - }); + }), [authenticateRegistration, token, timeZone, locale, queryClient, posthog, searchParams]);
95-97: Review the onValidCode handler implementation.The handler calls
handleSubmit()without parameters, which might cause issues since the form's handleSubmit expects a data parameter.Consider using the form's submit method instead:
-onValidCode={() => { - handleSubmit(); -}} +onValidCode={() => { + form.handleSubmit((data) => handleSubmit(data))(); +}}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/web/src/app/[locale]/(auth)/register/verify/components/otp-form.tsx (1)
49-73: 🛠️ Refactor suggestionEnhance error handling in form submission.
The form submission lacks proper error handling for network failures and doesn't provide adequate user feedback.
🧹 Nitpick comments (3)
apps/web/src/app/[locale]/(auth)/register/verify/components/otp-form.tsx (2)
25-27: Enhance OTP validation schema.The schema should explicitly validate the character type (numeric/alphanumeric) and pattern of the OTP.
const otpFormSchema = z.object({ - otp: z.string().length(6), + otp: z.string().length(6).regex(/^\d+$/, "OTP must contain only numbers"), });
89-102: Enhance input accessibility.Add ARIA labels and improve screen reader support for the OTP input.
<FormControl> <InputOTP size="lg" placeholder={t("verificationCodePlaceholder", { ns: "app", })} disabled={isLoading} autoFocus={true} + aria-label={t("verificationCodeAriaLabel", "Enter verification code")} + aria-describedby="otp-description" onValidCode={() => { handleSubmit(); }} {...field} /> </FormControl>apps/web/src/app/[locale]/(auth)/layout.tsx (1)
19-21: Consider adding aria-label to the Logo component.For better accessibility, consider adding an aria-label to the Logo component to provide context for screen readers.
- <Logo className="mx-auto" /> + <Logo className="mx-auto" aria-label="Rallly Logo" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/src/app/[locale]/(auth)/layout.tsx(1 hunks)apps/web/src/app/[locale]/(auth)/register/verify/components/otp-form.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Type check
- GitHub Check: Integration tests
- GitHub Check: Linting
🔇 Additional comments (3)
apps/web/src/app/[locale]/(auth)/register/verify/components/otp-form.tsx (1)
31-48: Well-structured form initialization!The form setup is clean and comprehensive, properly integrating form validation, internationalization, and API utilities.
apps/web/src/app/[locale]/(auth)/layout.tsx (2)
16-45: LGTM! Well-structured responsive layout.The layout implementation demonstrates good practices:
- Proper use of flexbox for alignment
- Mobile-first responsive design
- Clean conditional rendering
- Consistent spacing and padding
50-55: LGTM! Well-configured metadata.The metadata configuration follows Next.js best practices and provides good SEO support with a consistent title template.
| signIn("registration-token", { | ||
| token, | ||
| callbackUrl: searchParams?.get("callbackUrl") ?? "/", | ||
| }); |
There was a problem hiding this comment.
Validate and sanitize the callback URL.
The callback URL from search params should be validated to prevent open redirect vulnerabilities.
+ const isValidUrl = (url: string) => {
+ try {
+ const validatedUrl = new URL(url, window.location.origin);
+ return validatedUrl.origin === window.location.origin;
+ } catch {
+ return false;
+ }
+ };
signIn("registration-token", {
token,
- callbackUrl: searchParams?.get("callbackUrl") ?? "/",
+ callbackUrl: (() => {
+ const url = searchParams?.get("callbackUrl");
+ return url && isValidUrl(url) ? url : "/";
+ })(),
});📝 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.
| signIn("registration-token", { | |
| token, | |
| callbackUrl: searchParams?.get("callbackUrl") ?? "/", | |
| }); | |
| const isValidUrl = (url: string) => { | |
| try { | |
| const validatedUrl = new URL(url, window.location.origin); | |
| return validatedUrl.origin === window.location.origin; | |
| } catch { | |
| return false; | |
| } | |
| }; | |
| signIn("registration-token", { | |
| token, | |
| callbackUrl: (() => { | |
| const url = searchParams?.get("callbackUrl"); | |
| return url && isValidUrl(url) ? url : "/"; | |
| })(), | |
| }); |
| export default async function Layout({ | ||
| children, | ||
| }: { | ||
| children: React.ReactNode; | ||
| }) { |
There was a problem hiding this comment.
💡 Codebase verification
Remove unnecessary async keyword
The layout component doesn't perform any asynchronous operations. It's a purely presentational component that renders static JSX with UI components and styling. The async keyword can be safely removed.
🔗 Analysis chain
Verify the necessity of the async keyword.
The component is marked as async but doesn't appear to perform any asynchronous operations. If there are no server-side data fetches or async operations planned, consider removing the async keyword.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any async operations or data fetches in related auth files
rg -l "fetch|await" "apps/web/src/app/\[locale\]/\(auth\)/"
Length of output: 139
Script:
#!/bin/bash
# First, find auth-related files
fd -t f . apps/web/src/app | grep -i "auth"
# Then search for async/await usage in the layout file specifically
fd "layout.tsx" apps/web/src/app | grep -i "auth" | xargs rg "async|await|fetch" -A 3
Length of output: 1535
Script:
#!/bin/bash
# Check imports and content of the auth layout
rg -A 10 "^import|'use server'" "apps/web/src/app/[locale]/(auth)/layout.tsx" || true
# Check for server actions in auth-related action files
rg -A 5 "'use server'" "apps/web/src/app/[locale]/(auth)/login/actions.ts" "apps/web/src/app/[locale]/(auth)/register/actions.ts" || true
Length of output: 1012
Summary by CodeRabbit
Release Notes
New Features
User Interface
Security
Performance
Localization