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

Filter error handler scores before rendering view statistics page #1747

Merged
merged 2 commits into from
Feb 6, 2023

Conversation

damianhxy
Copy link
Member

@damianhxy damianhxy commented Jan 30, 2023

Description

Filter out grader_ids of -1 before rendering the view statistics page.

Motivation and Context

In #1643, scores are marked with a grader_id of -1 if an error occurs (for context, see the PR).

However, this led to an issue in viewing the view statistics page if any such scores exist.

This is since in grading.rb, we map the grader_ids with :find_user, which only handles grader_id = 0 specially. As a result, an error occurs on the line @course.course_user_data.find(i) if a grader_id of -1 is present.

How Has This Been Tested?

First, make a submission that triggers the error handler. e.g. create an assessment whose autograder does not output a score json at the end (e.g. install hello.tar, modify src/driver.sh, run make). Then submit to it.

Afterwards, access the view statistics page.

BEFORE (Error)
Screenshot 2023-01-30 at 18 05 31

AFTER (Renders)
Screenshot 2023-01-30 at 18 07 33

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have run rubocop for style check. If you haven't, run overcommit --install && overcommit --sign to use pre-commit hook for linting

Copy link
Member

@lykimchee lykimchee left a comment

Choose a reason for hiding this comment

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

Tested with a different autograder (not hello.tar):

In master:
branch master before bug fix

In hide-error-handler-statistics:
statistics page after bug fix

lgtm! :>

@damianhxy damianhxy merged commit bf73afc into master Feb 6, 2023
@damianhxy damianhxy deleted the hide-error-handler-statistics branch February 6, 2023 00:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants