-
Notifications
You must be signed in to change notification settings - Fork 231
[BUGFIX+IMPROVEMENT] Fixed and added autocomplete for inputing students #1443
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
Conversation
I'm not necessarily suggesting that my versions of these fixes are better, but you might want to investigate cg2v@057493b and cg2v@752a44f as alternatives to 8d76c19. I approve of ad627d2 as identical to cg2v@f943353 (I could not remember how to reproduce the latter, so never submitted my own PR for it) |
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.
I will circle back on the Base64 encoding issue
@usersEncoded = {} | ||
@course.course_user_data.each do |cud| | ||
@users[cud.full_name_with_email] = cud.id | ||
@usersEncoded[Base64.encode64(cud.full_name_with_email.strip).strip] = cud.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.
It seems like we have this Base64 encoding here for a reason, perhaps for students who username is not stored in base64 format? I will have to take a further look before deciding what's the appropriate fix here.
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.
Original issue: #931
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.
Basically, autocomplete seems to strip leading spaces, etc. from names which can make ID lookup fail.
cg2v@057493b and cg2v@752a44f seems to be the correct fix, in addressing the issue brought up a while back. #1013 |
This PR coincides with the latest feature request by Prof Iliano (autocompletion for sudo) and is worth revisiting. |
There are mutiple bugs regarding the autocompletion for students in the new submission site and the assessment extension site. This PR fixes them and adds autocompletion to the sudo site as well.
Motivation and Context
There are multiple bugs due to code redundancies and base64 that breaks the encoded string after 60 characters which results in invalid javascript. I removed the base64 conversion entirely because its not necessary.
How Has This Been Tested?
Autocompletion and submitting the form on the new submission, assessment extension and sudo site.
Types of changes
Checklist:
Other issues / help required
This is not an optimal solution as we still have code redundancies. We should only write one autocompletion javascript file and import it wherever we need it.