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

Correctly display orphaned annotations on viewFeedback page #1658

Merged
merged 7 commits into from
Dec 1, 2022

Conversation

michellexliu
Copy link
Contributor

Description

Previously, if you add an annotation for a problem, delete the problem, and go view feedback, there was an error caused by calling problem.name.

If the problem is nil, the header is updated to say "Deleted Problem(s)", to match how orphaned problems are handled in #1629.
image

The issue of annotations for deleted problems was fixed by #1629, but this prevents an error on the view feedback page for pages where a problem was deleted before the patch was implemented.

Also updated check so that instructors & course assistants can view annotations even if they're not released.

How Has This Been Tested?

  • Tested with annotations for regular problems
  • Added an annotation for a problem --> deleted the problem --> clicked view feedback

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

@damianhxy
Copy link
Member

damianhxy commented Nov 29, 2022

I would suggest making the following simple change to allow orphaned annotations to be deleted (and also edited, to change the problem):

Add the lines

# Associated problem was deleted
return if score.problem_id && score.problem.nil?

inside annotation.rb#update_non_autograded_score, after score = Score.find_or_initialize_by_submission_id_and_problem_id... and before return if score.grader_id == 0

We check for score.problem_id to be future-proof, in case we implement (problem-independent) global annotations that presumably will have no problem_id at all.

Copy link
Member

@damianhxy damianhxy left a comment

Choose a reason for hiding this comment

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

Verified that course assistants and instructors can now see annotations even when the problem is not released.

Also verified that orphaned annotations show up under Deleted Problem(s).

@damianhxy damianhxy changed the title Fixed issue where error occurred on viewFeedback if there's an orphaned problem Correctly display orphaned annotations on viewFeedback page Nov 29, 2022
@damianhxy
Copy link
Member

For consistency, could you also update getProblemNameWithId in annotations.js as follows:

function getProblemNameWithId(problem_id) {
  var problem_id = parseInt(problem_id, 10);
  var problem = _.findWhere(problems, { "id": problem_id });
  if (problem == undefined) return "Deleted Problem(s)";
  return problem.name;
}

Before
Screenshot 2022-11-29 at 01 42 51

After
Screenshot 2022-11-29 at 01 42 35

@damianhxy
Copy link
Member

lgtm!

@michellexliu michellexliu merged commit fc6be53 into master Dec 1, 2022
@michellexliu michellexliu deleted the bugfix/viewFeedback-orphaned-problem branch December 1, 2022 04:47
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