-
Notifications
You must be signed in to change notification settings - Fork 66
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
Fix: Speaker name in Talk is overwritten #279
Fix: Speaker name in Talk is overwritten #279
Conversation
Reviewer's Guide by SourceryThis pull request fixes an issue where the speaker name was being overwritten with the value from the JWT upon login, even if the user already had a name set. The fix ensures that the user's name is only updated if it is not already set. Logging was added to track the JWT payload and when user data is saved. Sequence diagram for user login with name updatesequenceDiagram
participant User
participant JWT
User->>JWT: Logs in
JWT-->>User: Returns payload (email, name, etc.)
User->>User: Checks if user exists with email
alt User exists
User->>User: Checks if user.name is set
alt user.name is not set and JWT name is present
User->>User: Updates user.name with JWT name
end
else User does not exist
User->>User: Creates new user
User->>User: Sets user.name with JWT name
end
User->>User: Sets user attributes (is_active, is_staff, locale, timezone, code)
User->>User: Saves user data
User->>User: Logs in user
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @hongquan - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider extracting the user update logic into a separate function for better readability.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟡 Security: 1 issue found
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
Besides the comment about logging, looks fine
Fixes #278
Check if user name has a value, before update it with the value from JWT when logging in.
How has this been tested?
Before logging out, speaker name is "Quan Outlook"

Logging out

Logging in again: The name is still as before.

Checklist