-
Notifications
You must be signed in to change notification settings - Fork 220
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 scoreboard sorting failing on custom scoreboardOrderSubmissions, add more error checking when creating scoreboard entries / displaying #2092
Conversation
WalkthroughThis set of changes introduces improved error handling and sorting logic in the Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
When updating the assessment rb file, if you go straight to the scoreboard (i.e. don't load the assessment index page), there is an error about an undefined method. However, after loading the assessment index page at least once, the error message is now as expected Suspect it is because |
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.
Functionality generally looks good, but left some nits
I noticed this while testing, not sure why going to index would resolve this though because doesn't seem that index doesn't do any calls to reload the config file? Not sure if it's worth looking into a fix for a hook that very few classes use (and the ones that do should understand how to reload the config file) |
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.
Ran through testing workflow, LGTM
Description
scoreboardOrderSubmissions
, in the case that an exception is raised, and tells the instructor via an error message#show
so that if a bad sort / comparison occurs, the error can be caught#show
, revert to an empty array#show
view code, check that the current entry length is greater than the index in order to show a score, else show an error icon (which handles the empty array case)Motivation and Context
Closes #2086
How Has This Been Tested?
Import the provided assessment:
randomlab5_20240217.tar.zip
autograder.tar
as the autograder)randomlab5-bad-scoreboard-order.rb
(in Edit Assessment you can upload)autograde-bad.tar
as the new autograderscoreboards_controller.rb
:( you may need to also reload the config file)
view the scoreboard as an instructor, see the following error
As a student, see something like this (all the bad scoreboard entries are at the bottom)
Types of changes