Skip to content
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

Add mononym support #1580

Merged
merged 11 commits into from
Aug 30, 2022
Merged

Add mononym support #1580

merged 11 commits into from
Aug 30, 2022

Conversation

damianhxy
Copy link
Member

@damianhxy damianhxy commented Aug 28, 2022

Description

Initial attempt to add mononym support, by updating user validation to only require at least one of first_name and last_name.

Pages with style changes for mononyms

  • Assessment Gradesheet
  • Course view student CUD
  • Assessment Manage submissions
  • User Account
  • Assessment Scoreboard
  • Course Metrics Watchlist
  • Assessment Gradesheet grader information
  • (course_user_data index and user pages -- they seemed unused as show is used instead)

A good litmus test is to run grep -nr "\.first_name.*\.last_name" . and look at the results

Other changes

  • Use flash.now for roster update success flash as otherwise the flash persists for one additional page load
  • Update user.rb#full_name method to handle mononyms
  • Update user.rb#display_name method to correctly use .present? to check for presence, since empty strings are truthy. Note: this method seems pretty redundant, and is mainly used in the navbar and Account page; we could probably delete this method and use full_name directly instead

Motivation and Context

To support students who do not have first names or last names.

Fixes #1536

How Has This Been Tested?

Import roster containing mononyms

Supports Mononyms
Screen Shot 2022-08-28 at 15 11 26

Correctly validates against completely blank names
Screen Shot 2022-08-28 at 15 11 12

Check miscellaneous styling

Gradesheet
Screen Shot 2022-08-28 at 15 58 29

View student CUD
Screen Shot 2022-08-28 at 15 54 34

Manage submissions
Screen Shot 2022-08-28 at 15 52 41

Account
Screen Shot 2022-08-28 at 15 55 11

Scoreboard
Screen Shot 2022-08-28 at 16 46 27

Watchlist
Screen Shot 2022-08-28 at 16 47 20

Grader information
Screen Shot 2022-08-28 at 16 55 45

Also, check that various functionality such as sudo, extensions, create submissions still work

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have run rubocop for style check. If you haven't, run overcommit --install && overcommit --sign to use pre-commit hook for linting
  • My change requires a change to the documentation, which is located at Autolab Docs
  • I have updated the documentation accordingly, included in this PR

Copy link
Contributor

@20wildmanj 20wildmanj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the most part, everything seems to work!

Checked:

  • import course roster
  • watchlist
  • course user data
  • grading as a CA with mononym and verifying grader info is correct
  • gradesheet and submissions
  • extensions
  • scoreboard
  • submitting assessments
  • creating user through "Add User to Course"
  • creating annotations

@20wildmanj
Copy link
Contributor

I couldn't test annotations fully however because it seems like as a student, I am having difficulties getting annotations to appear, because I get an authentication error. When I look at a submission, the annotations don't appear and I get this:

Screen Shot 2022-08-29 at 11 05 54

This doesn't seem limited to just this branch however, seems like it also occurs on master. I don't know how to check this on autolab-dev/nightly because I don't know a student user I can log into, and I get a 500 error if I try to create a user.

@damianhxy
Copy link
Member Author

I couldn't test annotations fully however because it seems like as a student, I am having difficulties getting annotations to appear, because I get an authentication error. When I look at a submission, the annotations don't appear and I get this

As discussed offline, it seems that the grading deadline must pass before annotations will be available to students, even if grades are already released. Whether this is intended behavior is up for debate.

The error seen in the console is unrelated. Rather, we are polling the shared comments endpoint even for students, which naturally leads to an authentication error. The same can be observed for the metrics watchlist endpoint on the view course page.

Ideally, we should avoid polling these endpoints for students, but it's not a big deal.

@damianhxy damianhxy removed the request for review from fanpu August 29, 2022 17:37
@damianhxy damianhxy merged commit 80a004a into master Aug 30, 2022
@damianhxy damianhxy deleted the mononym-support branch August 30, 2022 00:46
@20wildmanj 20wildmanj mentioned this pull request Aug 20, 2023
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Course roster import shouldn't reject students with no first name (or no last name).
2 participants