[Feature:Forum] Unique anonymous names#12459
[Feature:Forum] Unique anonymous names#12459GarvitKhandelwal31 wants to merge 15 commits intoSubmitty:mainfrom
Conversation
…uffix - Add chatroom_anonymous_names table for persistent anonymous IDs - Generate truly random names instead of hashing - Format: Anonymous [Adjective] [Noun] #XXXX - Persist across leave/rejoin, logout, cookie clear - Add Shuffle Names button for instructors
|
Hi @williamschen23, could you please approve the workflows for this PR when you have a moment so the CI checks can initiate? Thank you! |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #12459 +/- ##
============================================
+ Coverage 21.66% 23.17% +1.51%
- Complexity 9638 9658 +20
============================================
Files 268 247 -21
Lines 36223 33625 -2598
Branches 486 0 -486
============================================
- Hits 7847 7792 -55
+ Misses 27894 25833 -2061
+ Partials 482 0 -482
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
Hi @GarvitKhandelwal31, it looks like you're missing a closing curly-bracket on line 200 of chatroom.js, which is causing most of the CI tests to fail. If you fix this I will re-approve the CI workflows to run. |
|
@Eli-J-Schwartz I have added the bracket in the latest commit. Can you please re-run workflows now? |
…ith persistent storage - Merge changes from upstream/main for read-only chat feature (PR Submitty#12428) - Resolve Chatroom.php conflict: combine anonymous name functions with read-only methods - Fix JavaScript lint errors: remove line continuation after else clause, add parentheses to arrow function - Fix migration idempotency: check for table/column existence before creating - All migrations complete successfully - Ready for PR review
|
@Eli-J-Schwartz Can you please re-run workflows. It should pass now. |
|
I've fixed all the failing workflows: Database Migrations:
Schema Dump Mismatch:
PHP (PHPCS):
JavaScript Linting:
Cypress Test Failure:
Can you please re-run? |
There was a problem hiding this comment.
Pull request overview
Implements improved anonymous naming for Live Chat by switching from deterministic/predictable name generation to persistent, randomly generated anonymous display names stored in the course database, plus an instructor action to reshuffle names.
Changes:
- Adds a
chatroom_anonymous_namestable (and Doctrine entity) to persist per-(chatroom,user) anonymous display names. - Updates chatroom anonymous name generation to read/write the persisted name and include a
#XXXXsuffix. - Adds an instructor-only “Shuffle Names” UI action and socket event handling to regenerate names for a chatroom.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| site/public/templates/chat/ChatroomRow.twig | Adds “Shuffle Names” button for active anonymous-enabled chatrooms. |
| site/app/templates/chat/ChatroomRow.twig | Same “Shuffle Names” button addition for server-side twig template. |
| site/public/js/chatroom.js | Adds AJAX call for reshuffling and client handling of the anon_names_regenerated socket event. |
| site/app/views/ChatroomView.php | Uses the new calcAnonName(..., EntityManager) signature so names can persist via DB. |
| site/app/entities/chat/ChatroomAnonymousName.php | New Doctrine entity for persisted anonymous display names. |
| site/app/entities/chat/Chatroom.php | Reworks anonymous name generation to persist and adds bulk regeneration helper. |
| site/app/controllers/ChatroomController.php | Adds instructor-only endpoint to regenerate names and broadcasts socket message. |
| migration/migrator/migrations/course/20250222150000_chatroom_anonymous_names.py | Creates chatroom_anonymous_names table and supporting index. |
| migration/migrator/migrations/course/20250222150100_chatroom_anonymous_names_display_name.py | Migration path for legacy schema to new display_name column. |
| migration/migrator/data/course_tables.sql | Adds chatroom_anonymous_names table + PK + index to baseline schema. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
migration/migrator/migrations/course/20250222150000_chatroom_anonymous_names.py
Outdated
Show resolved
Hide resolved
migration/migrator/migrations/course/20250222150100_chatroom_anonymous_names_display_name.py
Outdated
Show resolved
Hide resolved
… varchar length, and double route
JManion32
left a comment
There was a problem hiding this comment.
This must've been a lot of work, great job! Please include screenshots of user interface changes, specifically the dashboard where the Shuffle Names button is now located:
A few things regarding the functionality here:
- If the chat allows users to join anonymously, the
Shuffle Namesbutton should be visible, rather than only being visible when the session has started. - The dashboard for chatrooms is starting to become cluttered. How can we improve the user interface without obscuring features? A solid solution to this should probably be a separate PR, but for now, I think we should start by keeping the
Shuffle Namesbutton on the same row as the rest of the buttons on the table - The
Shuffle Namesbutton should be white and blue, like all other utility buttons on Submitty. - When a session has started and I click the
Shuffle Namesbutton, the functionality works as intended. Any subsequent attempts to click the button does not shuffle the names.
Some other thoughts:
- Please add documentation to your description for the different "join messages" that can occur for the user when the instructor clicks
Shuffle Names. I noticed it in the code, but if I hadn't, it would've gone untested. - Can we add a basic Cypress test to support this change? I can't imagine it would be too much work especially since
live_chat.spec.jsis already pretty thorough. - There should be a follow-up to this that allows instructors to more easily view the real names of anonymous chatters.
Overall, great work here! I will likely conduct a more thorough review later, but I think this is a good starting point. Feel free to ping me here if you have any questions.
|
@JManion32 Addressed requested changes and updated this PR for re-review. Shuffle Names is now visible whenever anonymous mode is enabled. |
|
@williamjallen For my PR, the scope currently includes: persistent anonymous names with DB-backed storage/migration updates |
JManion32
left a comment
There was a problem hiding this comment.
@GarvitKhandelwal31 I am unable to test your changes due to this error:
I even did a vagrant destroy and vagrant up to verify it was your branch causing it. I assume you will experience this same error on the site, so please resolve before I review any further. Thank you!
|
Hi @JManion32, thanks for reporting this. I reproduced the issue and confirmed it was an environment sync problem (missing course migration in s26 sample + runtime site not fully refreshed), not the anonymous naming logic itself. Run these in vagrant ssh cd /usr/local/submitty/GIT_CHECKOUT/Submitty/migration cd /usr/local/submitty/GIT_CHECKOUT/Submitty After that, the chat page loads correctly and the anonymous-name behavior works as expected (including suffix and shuffle flow). |
Why is this Change Important & Necessary?
Fixes #11999
Live Chat anonymous names were previously deterministic/predictable and could collide.
This PR switches to random anonymous naming with persistence so users keep a stable name during a chat session lifecycle, while still allowing instructors to reshuffle names when needed.
What is the New Behavior?
Join/Shuffle Message Cases (Exact Text)
On chatroom join (any user):
You have successfully joined as <displayName>.When instructor clicks Shuffle Names (success toast on click action):
Anonymous names have been shuffled!When clients receive shuffle socket event (broadcast/system toast):
Anonymous names have been shuffled by the instructor.For anonymous users after shuffle (personal name update toast):
Your anonymous name is now <newName>.Steps to Test
Automated Testing & Documentation
Screenshots
The below image shows hashing is working correctly.
Now if a user leaves and joins again he gets the same name showing persistent naming.
When pressed shuffle anonymous names are shuffled correctly.
Other Information