v2.29.2 #1973
v2.29.2 #1973
Conversation
| The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements a comprehensive session management and user password reset system. The changes refactor session validation from individual components into a centralized Redux-based system and add functionality for administrators to reset user passwords.
Key Changes:
- Centralized session management: Moved session validation from component-level checks to Redux state management
- Password reset functionality: Added a new admin feature to reset user passwords with validation and auto-generation capabilities
- Improved error handling: Enhanced Parse error handling to properly detect and respond to session expiration (error code 209)
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/OpenSignServer/cloud/parsefunction/resetPassword.js | New server-side Parse Cloud function for password reset with admin authorization |
| apps/OpenSignServer/cloud/main.js | Registers the new resetPassword cloud function |
| apps/OpenSign/src/redux/store.js | Adds userReducer to Redux store |
| apps/OpenSign/src/redux/reducers/userReducer.js | New Redux reducer for managing session state |
| apps/OpenSign/src/primitives/Validate.jsx | Refactored to use centralized session modal component |
| apps/OpenSign/src/primitives/SessionExpiredModal.jsx | New reusable session expired modal component |
| apps/OpenSign/src/primitives/PasswordResetModal.jsx | New modal for password reset with validation and auto-generation |
| apps/OpenSign/src/pages/UserList.jsx | Integrated password reset functionality for admin users |
| apps/OpenSign/src/pages/Form.jsx | Added session expiration detection on Parse errors |
| apps/OpenSign/src/layout/HomeLayout.jsx | Updated to use Redux-based session management |
| apps/OpenSign/src/components/Header.jsx | Updated logout flow to reset session state |
| apps/OpenSign/src/App.jsx | Removed redundant session validation wrapper |
| Multiple translation files | Added translations for password reset functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| await navigator.clipboard.writeText(password); | ||
| showAlert("success", t("copied"), 1200); | ||
| setCopied(true); | ||
| setTimeout(() => setCopied(false, 1200)); |
There was a problem hiding this comment.
The setTimeout callback has incorrect parameters. It should be setTimeout(() => setCopied(false), 1200) - the timeout duration should be the second parameter to setTimeout, not passed to setCopied.
| setTimeout(() => setCopied(false, 1200)); | |
| setTimeout(() => setCopied(false), 1200); |
Copilot uses AI. Check for mistakes.
| dispatch(sessionStatus(true)); | ||
| setIsLoader(false); | ||
| } else { | ||
| dispatch(sessionStatus(true)); |
There was a problem hiding this comment.
When user validation fails, the session status is incorrectly set to true. This should be dispatch(sessionStatus(false)) to indicate an invalid session.
| dispatch(sessionStatus(true)); | |
| dispatch(sessionStatus(false)); |
Copilot uses AI. Check for mistakes.
| throw new Parse.Error(Parse.Error.INVALID_SESSION_TOKEN, 'User is not authenticated.'); | ||
| } | ||
| | ||
| if (request.user.id === userId) { |
There was a problem hiding this comment.
The comparison between request.user.id and userId may fail due to type mismatch. request.user.id is typically a string while userId might be passed as an object. Consider using request.user.id === userId after ensuring both are strings, or use request.user.get('objectId') === userId.
| if (request.user.id === userId) { | |
| // Ensure userId is a string for comparison | |
| const userIdStr = typeof userId === 'object' && userId !== null | |
| ? userId.objectId || userId.id || '' | |
| : String(userId); | |
| if (request.user.id === userIdStr) { |
Copilot uses AI. Check for mistakes.
| useEffect(() => { | ||
| const onDocClick = (e) => { | ||
| if (!e.target.closest('[data-dropdown-root="1"]')) setIsOption({}); | ||
| }; | ||
| document.addEventListener("click", onDocClick); | ||
| return () => document.removeEventListener("click", onDocClick); | ||
| }, []); |
There was a problem hiding this comment.
The click event listener is added on every component mount but the cleanup function may not properly remove the exact same function reference. Consider using useCallback to memoize the event handler function.
| useEffect(() => { | |
| const onDocClick = (e) => { | |
| if (!e.target.closest('[data-dropdown-root="1"]')) setIsOption({}); | |
| }; | |
| document.addEventListener("click", onDocClick); | |
| return () => document.removeEventListener("click", onDocClick); | |
| }, []); | |
| const onDocClick = React.useCallback((e) => { | |
| if (!e.target.closest('[data-dropdown-root="1"]')) setIsOption({}); | |
| }, []); | |
| useEffect(() => { | |
| document.addEventListener("click", onDocClick); | |
| return () => document.removeEventListener("click", onDocClick); | |
| }, [onDocClick]); |
Copilot uses AI. Check for mistakes.
| useEffect(() => { | ||
| const onDocClick = (e) => { | ||
| if (!e.target.closest('[data-dropdown-root="1"]')) setIsOption({}); | ||
| }; | ||
| document.addEventListener("click", onDocClick); | ||
| return () => document.removeEventListener("click", onDocClick); | ||
| }, []); |
There was a problem hiding this comment.
The click event listener is added on every component mount but the cleanup function may not properly remove the exact same function reference. Consider using useCallback to memoize the event handler function.
Copilot uses AI. Check for mistakes.
| return str[Math.floor(Math.random() * str.length)]; | ||
| } | ||
| function shuffle(arr) { | ||
| return arr.sort(() => Math.random() - 0.5); |
There was a problem hiding this comment.
Using Math.random() - 0.5 for array shuffling is not truly random and can introduce bias. Consider using the Fisher-Yates shuffle algorithm for proper randomization.
| return arr.sort(() => Math.random() - 0.5); | |
| // Fisher-Yates shuffle | |
| for (let i = arr.length - 1; i > 0; i--) { | |
| const j = Math.floor(Math.random() * (i + 1)); | |
| [arr[i], arr[j]] = [arr[j], arr[i]]; | |
| } | |
| return arr; |
Copilot uses AI. Check for mistakes.
2 participants
No description provided.