fix: hide signup buttons when disallowSignUp is configured #1498
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements a fix to hide signup buttons when signup is disabled via configuration.
- Updates the login route with server-side hydration using a public setting query.
- Introduces a new hook (useDisallowSignUp) to determine whether signup should be hidden.
- Conditionally renders signup links in both LoginPage and SignForm components based on the disallowSignUp flag.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| apps/nextjs-app/src/pages/auth/login.tsx | Adds hydration support and fetches public settings server-side to provide the disallowSignUp flag. |
| apps/nextjs-app/src/features/auth/useDisallowSignUp.ts | Implements a hook to retrieve the public setting that controls signup visibility. |
| apps/nextjs-app/src/features/auth/pages/LoginPage.tsx | Uses the disallowSignUp flag to conditionally render the signup button. |
| apps/nextjs-app/src/features/auth/components/SignForm.tsx | Uses the disallowSignUp flag to hide the signup link when in signin mode. |
| queryFn: () => getPublicSetting().then(({ data }) => data), | ||
| }); | ||
| const { disallowSignUp } = setting ?? {}; | ||
| return disallowSignUp; |
There was a problem hiding this comment.
Consider returning a default boolean value (e.g., false) when 'setting' is undefined to ensure a consistent evaluation of the disallowSignUp flag.
| return disallowSignUp; | |
| return disallowSignUp ?? false; |
Copilot uses AI. Check for mistakes.
| {type === 'signin' ? t('auth:button.signup') : t('auth:button.signin')} | ||
| </Link> | ||
| </div> | ||
| {!disallowSignUp && type === 'signin' && ( |
There was a problem hiding this comment.
[nitpick] The conditional rendering for the signup link appears in both LoginPage and SignForm. Consider centralizing this logic to avoid duplication and improve maintainability.
Copilot uses AI. Check for mistakes.
| await Promise.all([ | ||
| queryClient.fetchQuery({ | ||
| queryKey: ReactQueryKeys.getPublicSetting(), | ||
| queryFn: () => ssrApi.getPublicSetting(), | ||
| }), | ||
| ]); |
There was a problem hiding this comment.
[nitpick] Consider adding error handling around the API call to fetch public settings to gracefully manage potential service failures on the server side.
| await Promise.all([ | |
| queryClient.fetchQuery({ | |
| queryKey: ReactQueryKeys.getPublicSetting(), | |
| queryFn: () => ssrApi.getPublicSetting(), | |
| }), | |
| ]); | |
| try { | |
| await Promise.all([ | |
| queryClient.fetchQuery({ | |
| queryKey: ReactQueryKeys.getPublicSetting(), | |
| queryFn: () => ssrApi.getPublicSetting(), | |
| }), | |
| ]); | |
| } catch (error) { | |
| console.error("Failed to fetch public settings:", error); | |
| // Optionally, handle fallback logic here if needed | |
| } |
Copilot uses AI. Check for mistakes.
🧹 Preview Environment Cleanup
|
No description provided.