[Testing:InstructorUI] Legal name privacy#12562
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12562 +/- ##
=========================================
Coverage 21.64% 21.64%
Complexity 9654 9654
=========================================
Files 268 268
Lines 36244 36244
Branches 487 487
=========================================
Hits 7845 7845
Misses 27916 27916
Partials 483 483
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
github-project-automation
Bot
moved this from Seeking Reviewer
to Work in Progress
in Submitty Development
Khine12
changed the title
automateprojectmangement
Bot
moved this from Work in Progress
to In Review
in Submitty Development
github-project-automation
Bot
moved this from In Review
to Work in Progress
in Submitty Development
automateprojectmangement
Bot
moved this from Work in Progress
to In Review
in Submitty Development
automateprojectmangement
Bot
moved this from Work in Progress
to In Review
in Submitty Development
Thank you for your time in reviewing my PRs and for the feedback! For pages, the test currently checks: users, graders, student_photos, forum, and navigation. I think those cover the main instructor-facing pages. Happy to add more if you think of any! |
Is there a reason you're only doing instructor facing pages? |
No particular reason — good catch! I've updated the test to also check pages as TA and student. The test now has separate checks for each role: instructor (users, graders, student_photos, forum, navigation), TA (graders, forum, navigation), and student (forum, navigation). |
When you get a chance, could a maintainer approve the workflow so CI can run? No rush — just want to make sure the new tests pass. |
| it('Legal names should not appear on instructor pages', () => { | ||
| cy.login('instructor'); | ||
| const pages = ['users', 'graders', 'student_photos', 'forum', 'navigation']; | ||
| pages.forEach((page) => { | ||
| cy.visit(['sample', page]); | ||
| checkNoLegalNames(); | ||
| }); | ||
| cy.logout(); | ||
| }); | ||
|
|
||
| it('Legal names should not appear on TA pages', () => { | ||
| cy.login('ta'); | ||
| const pages = ['graders', 'forum', 'navigation']; | ||
| pages.forEach((page) => { | ||
| cy.visit(['sample', page]); | ||
| checkNoLegalNames(); | ||
| }); | ||
| cy.logout(); | ||
| }); | ||
|
|
||
| it('Legal names should not appear on student pages', () => { | ||
| cy.login('student'); | ||
| const pages = ['forum', 'navigation']; | ||
| pages.forEach((page) => { | ||
| cy.visit(['sample', page]); | ||
| checkNoLegalNames(); | ||
| }); | ||
| cy.logout(); | ||
| }); |
There was a problem hiding this comment.
Since these are such small tests, they can fall under the same umbrella it block, something like it('Should show preferred names not legal names')
There was a problem hiding this comment.
Done — combined the three it blocks into a single it('Should show preferred names not legal names') block as suggested.
github-actions
Bot
added
the
Abandoned PR - Needs New Owner
|
Hi, just checking in — I've addressed the review feedback, and this PR is still active on my end. Happy to make any additional changes if needed. Let me know! |
Rkoester47
left a comment
Rkoester47
left a comment
There was a problem hiding this comment.
The tests pass locally and you have implemented all suggested changes!
4 participants
Why is this Change Important & Necessary?
Closes #8435
Legal names should never be displayed when a preferred name exists.
This PR adds E2E tests to verify legal names never appear in the UI.
What is the New Behavior?
Added name_privacy.spec.js to cypress/e2e/Cypress-UI/ which:
Sets preferred names for student, instructor, and TA users via the profile page
Visits multiple pages and verifies that none of their legal names appear in the HTML
What steps should a reviewer take to reproduce or test the bug or new feature?
Run: npx cypress run --headless --browser electron --spec cypress/e2e/Cypress-UI/name_privacy.spec.js
All tests should pass.
Automated Testing & Documentation
1 test added covering: users page, graders page, student photos, forum, and navigation page.
Other information
No breaking changes. No migrations needed.