-
Notifications
You must be signed in to change notification settings - Fork 223
[dev] [Marfuen] mariano/make-portal-great-again #1945
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
PR SummaryModernizes authentication and the employee portal, with API endpoints to support it.
Written by Cursor Bugbot for commit 4cee4f1. This will update automatically on new commits. Configure here. |
Graphite Automations"Auto-assign PRs to Author" took an action on this PR • (12/18/25)1 reviewer was added to this PR based on Mariano Fuentes's automation. |
…te in UI components
| return true; | ||
| } catch (error) { | ||
| throw new UnauthorizedException('Invalid or expired JWT token'); | ||
| } |
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.
JWT guard swallows specific auth error messages
The catch block at the end of handleJwtAuth catches all errors including the specific UnauthorizedException instances thrown within the try block (e.g., "BETTER_AUTH_URL not configured", "missing user information", "User does not have access to organization"). These informative error messages are discarded and replaced with a generic "Invalid or expired JWT token" message, making it very difficult to diagnose authentication failures in production or during development.
| const created = await db.employeeTrainingVideoCompletion.create({ | ||
| data: { memberId: member.id, videoId, completedAt: new Date() }, | ||
| }); | ||
| return { success: true, data: created }; |
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.
Race condition causes unhandled error in video completion
The completeTrainingVideo method has a race condition between the findFirst check and the create call. The database schema has a @@unique([memberId, videoId]) constraint on EmployeeTrainingVideoCompletion. If two concurrent requests attempt to complete the same video (e.g., user double-clicks, or same page open in multiple tabs), both requests may find no existing record, and both will try to create one. The second create will fail with an unhandled unique constraint violation, resulting in a 500 error to the user. Using upsert or catching the unique constraint error would prevent this.
| onNext, | ||
| allVideosCompleted, | ||
| isMarkingComplete, | ||
| onWatchAgain, |
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.
isRewatching state persists incorrectly across different videos
The isRewatching state is initialized once and never resets when the video prop changes. When a user clicks "Watch Again" on a completed video, isRewatching becomes true and stays true even when navigating to a different video in the carousel. This causes the "Video Completed" overlay to not appear for subsequent completed videos, since the condition isCompleted && !isRewatching will be false. The state needs to reset when the video changes, such as with a useEffect that resets isRewatching to false when video.youtubeId changes.
| await db.policy.update({ | ||
| where: { id: policyId }, | ||
| data: { signedBy: { push: member.id } }, | ||
| }); |
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.
Race condition allows duplicate entries in policy signatures
The acknowledgePolicy and acknowledgePolicies methods have a TOCTOU (time-of-check-time-of-use) race condition. The check at line 54 (policy.signedBy.includes(member.id)) and the subsequent push update at lines 58-60 are not atomic. If two concurrent requests occur for the same member and policy, both may pass the check before either completes the update, resulting in duplicate member.id entries in the signedBy array. While this doesn't break functionality (the includes check still works), it causes data inconsistency. The same pattern exists in acknowledgePolicies at lines 89-98.
This is an automated pull request to merge mariano/make-portal-great-again into dev.
It was created by the [Auto Pull Request] action.