Skip to content

fix: hide signup buttons when disallowSignUp is configured#1498

Merged
boris-w merged 2 commits into
developfrom
fix/setting-disabled-signup
May 6, 2025
Merged

fix: hide signup buttons when disallowSignUp is configured#1498
boris-w merged 2 commits into
developfrom
fix/setting-disabled-signup

Conversation

@boris-w
Copy link
Copy Markdown
Contributor

@boris-w boris-w commented Apr 30, 2025

No description provided.

@boris-w boris-w requested a review from tea-artist April 30, 2025 10:54
@boris-w boris-w linked an issue Apr 30, 2025 that may be closed by this pull request
@boris-w boris-w requested a review from Copilot April 30, 2025 10:54
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
Copy link

Copilot AI Apr 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider returning a default boolean value (e.g., false) when 'setting' is undefined to ensure a consistent evaluation of the disallowSignUp flag.

Suggested change
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' && (
Copy link

Copilot AI Apr 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.
Comment on lines +39 to +44
await Promise.all([
queryClient.fetchQuery({
queryKey: ReactQueryKeys.getPublicSetting(),
queryFn: () => ssrApi.getPublicSetting(),
}),
]);
Copy link

Copilot AI Apr 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider adding error handling around the API call to fetch public settings to gracefully manage potential service failures on the server side.

Suggested change
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.
@boris-w boris-w merged commit 5d23a94 into develop May 6, 2025
10 checks passed
@boris-w boris-w deleted the fix/setting-disabled-signup branch May 6, 2025 03:20
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2025

🧹 Preview Environment Cleanup

tea-artist added a commit that referenced this pull request Mar 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sign Up is still there

3 participants