-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Reset password - Stuck on password resetting modal after validating 'change password' #15139
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR fixes a UX issue where users were getting stuck on the password reset modal after successfully changing their password. The changes enhance the redirection logic in PasswordReset.tsx
to properly handle multi-workspace environments.
The update adds imports for workspace utilities and user loading functionality, then implements robust redirection logic that checks if multi-workspace is enabled and if the user has exactly one available workspace. In such cases, it redirects to the specific workspace domain; otherwise, it falls back to local navigation. The implementation includes proper error handling with a try-catch block to ensure graceful fallback if user loading fails.
This change integrates with the existing workspace management system and multi-workspace functionality to provide a smoother post-password-reset experience, ensuring users land on the appropriate destination rather than remaining stuck in the modal.
Important Files Changed
Changed Files
Filename | Score | Overview |
---|---|---|
packages/twenty-front/src/pages/auth/PasswordReset.tsx | 4/5 | Added robust redirection logic for multi-workspace scenarios after successful password reset with proper error handling |
Confidence score: 4/5
- This PR is safe to merge with minimal risk as it adds fallback logic and doesn't break existing functionality
- Score reflects well-structured error handling and integration with existing workspace systems, though the async user loading introduces minor complexity
- Pay close attention to the redirection logic to ensure it handles all edge cases properly in multi-workspace environments
Sequence Diagram
sequenceDiagram
participant User
participant PasswordResetPage as "PasswordReset Component"
participant ValidationQuery as "useValidatePasswordResetTokenQuery"
participant UpdateMutation as "useUpdatePasswordViaResetTokenMutation"
participant AuthService as "useAuth"
participant CaptchaService as "useReadCaptchaToken"
participant UserLoader as "useLoadCurrentUser"
participant Navigation as "useNavigateApp/useRedirectToWorkspaceDomain"
participant SnackBar as "useSnackBar"
User->>PasswordResetPage: "Access reset password URL with token"
PasswordResetPage->>ValidationQuery: "Validate password reset token"
alt Token is invalid
ValidationQuery-->>PasswordResetPage: "Error response"
PasswordResetPage->>SnackBar: "Show error notification"
PasswordResetPage->>Navigation: "Navigate to AppPath.Index"
else Token is valid
ValidationQuery-->>PasswordResetPage: "Return email address"
PasswordResetPage->>PasswordResetPage: "Set isTokenValid=true, display form"
end
User->>PasswordResetPage: "Enter new password and submit"
PasswordResetPage->>UpdateMutation: "Update password via reset token"
alt Password update fails
UpdateMutation-->>PasswordResetPage: "Update failed"
PasswordResetPage->>SnackBar: "Show error message"
else Password update succeeds
UpdateMutation-->>PasswordResetPage: "Password updated successfully"
alt User is already logged in
PasswordResetPage->>SnackBar: "Show success message"
PasswordResetPage->>Navigation: "Navigate to AppPath.Index"
else User needs to sign in
PasswordResetPage->>CaptchaService: "Read captcha token"
CaptchaService-->>PasswordResetPage: "Return captcha token"
PasswordResetPage->>AuthService: "Sign in with new credentials"
AuthService-->>PasswordResetPage: "Sign in successful"
PasswordResetPage->>UserLoader: "Load current user data"
alt Multi-workspace enabled and user has exactly one workspace
UserLoader-->>PasswordResetPage: "Return user with single workspace"
PasswordResetPage->>Navigation: "Redirect to workspace domain"
else Default case or user loading fails
PasswordResetPage->>Navigation: "Navigate to AppPath.Index locally"
end
end
end
1 file reviewed, 1 comment
} catch (e) { | ||
// Fallback to local navigation if loading current user fails | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Empty catch block should at least log the error for debugging purposes
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/twenty-front/src/pages/auth/PasswordReset.tsx
Line: 182:184
Comment:
**style:** Empty catch block should at least log the error for debugging purposes
How can I resolve this? If you propose a fix, please make it concise.
Welcome!
Hello there, congrats on your first PR! We're excited to have you contributing to this project. |
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:19981 This environment will automatically shut down when the PR is closed or after 5 hours. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution, I left some comments.
// After signing in, redirect appropriately. | ||
// If multi-workspace is enabled and the user has exactly one workspace, | ||
// redirect to that workspace domain home. Otherwise, navigate locally. | ||
try { | ||
const { user } = await loadCurrentUser(); | ||
if (isMultiWorkspaceEnabled && countAvailableWorkspaces(user.availableWorkspaces) === 1) { | ||
const targetWorkspace = getFirstAvailableWorkspaces(user.availableWorkspaces); | ||
await redirectToWorkspaceDomain( | ||
getWorkspaceUrl(targetWorkspace.workspaceUrls), | ||
AppPath.Index, | ||
); | ||
return; | ||
} | ||
} catch (e) { | ||
// Fallback to local navigation if loading current user fails | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
Currently, resetting password is only available from workspace domain sign in. So user is already located on his workspace we want to redirect him on.
I think you can simply use theuseRedirect
hook here. -
Please, run the
npx nx lint twenty-server --fix
command before pushing (you can see that ci has failed) -
We try as much as possible to avoid comments in the codebase.
I updated pages/auth/PasswordReset.tsx to handle redirection more robustly after a successful password change and implicit sign-in: