Skip to content

Adding a rubric system to Autolab #2291

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.

Sign up for GitHub

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

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Adding a rubric system to Autolab #2291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
184e4c2
add basic structure for creating rubrics for problems
emiilyxie Apr 26, 2025
214fecc
clean up unused javascript
emiilyxie May 4, 2025
481d917
merged
ajain-1 May 4, 2025
a0ee1be
added feature to assign rubric items
ajain-1 May 5, 2025
14622d4
added feature to associate rubric items with annotations
ajain-1 May 5, 2025
0324d68
added feature to associate rubric items with annotations
ajain-1 May 5, 2025
b5e2c8d
minor filename fix
ajain-1 May 5, 2025
add6c76
minor schema fixes
ajain-1 May 5, 2025
a84d74e
style fixes
ajain-1 May 5, 2025
d63036a
fix linting
ajain-1 May 5, 2025
6139fbb
Merge remote-tracking branch 'upstream/master' into rubrics
ajain-1 May 5, 2025
6d2ac2c
Update config
ajain-1 May 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ gem 'rake', '>=10.3.2'
gem 'populator', '>=1.0.0'

# To communicate with MySQL database
gem 'mysql2', '~>0.5'
gem 'mysql2', '=0.5.4'
Copy link
Contributor

@coderabbitai coderabbitai bot May 5, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Verify rationale for pinning mysql2 to exact version 0.5.4

Changing from a pessimistic version constraint (~> 0.5) to an exact version constraint (=0.5.4) for the mysql2 gem prevents automatic updates to newer patch versions, which might include security fixes and bug fixes. While this ensures consistency across environments, it could introduce maintenance overhead.

Please also consider adding a comment in the Gemfile explaining why this exact version is required, as this can help future maintainers understand the rationale behind this constraint.

🏁 Script executed:

#!/bin/bash
# Check if there are security advisories for mysql2 gem versions above 0.5.4
curl -s "https://rubygems.org/api/v1/versions/mysql2.json" | jq -r '.[] | select(.number > "0.5.4" and .number < "0.6.0") | .number' | sort -V

# Check if this version is the latest in the 0.5.x series
curl -s "https://rubygems.org/api/v1/gems/mysql2.json" | jq -r '.version'

Length of output: 232

Re-evaluate mysql2 version constraint

The mysql2 gem has newer patch releases (0.5.5, 0.5.6) and the latest 0.5.x version is 0.5.6. Pinning to = 0.5.4 blocks these updates (which may include bug and security fixes). Consider one of the following:

  • Allow patch upgrades within 0.5.x by using a pessimistic constraint: 
    gem 'mysql2', '~> 0.5.4'
  • If you must lock to an exact patch for reproducibility, bump to 0.5.6 and add an inline comment explaining the rationale: 
    # Pin to 0.5.6 because of [REASON]
gem 'mysql2', '= 0.5.6'

This ensures you get critical fixes while maintaining consistency.

🤖 Prompt for AI Agents (early access) 
In the Gemfile at line 52, the mysql2 gem is pinned to an exact version 0.5.4, which blocks patch updates that may include important fixes. To fix this, either change the version constraint to a pessimistic one like '~> 0.5.4' to allow patch upgrades within the 0.5.x series, or if you need to lock to an exact version, update it to the latest patch 0.5.6 and add a comment explaining the reason for this pinning to help future maintainers.


# Development server
gem 'thin'
Expand Down
5 changes: 3 additions & 2 deletions Gemfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ GEM
momentjs-rails (2.15.1)
railties (>= 3.1)
multi_xml (0.6.0)
mysql2 (0.5.5)
mysql2 (0.5.4)
net-http (0.4.0)
uri
net-imap (0.4.9)
Expand Down Expand Up @@ -470,6 +470,7 @@ PLATFORMS
arm64-darwin-21
arm64-darwin-22
arm64-darwin-23
arm64-darwin-24
x86_64-darwin-19
x86_64-darwin-20
x86_64-darwin-21
Expand Down Expand Up @@ -509,7 +510,7 @@ DEPENDENCIES
mini_racer (~> 0.6.3)
moment_timezone-rails
momentjs-rails (>= 2.9.0)
mysql2 (~> 0.5)
mysql2 (= 0.5.4)
net-http
net-ldap
newrelic_rpm
Expand Down
143 changes: 105 additions & 38 deletions app/assets/javascripts/annotations.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ $(window).on('resize', function () {
function getSharedCommentsForProblem(problem_id) {
return localCache['shared_comments'][problem_id]?.map(
(annotation) => {
return {label: annotation.comment ?? annotation, value: annotation}
return { label: annotation.comment ?? annotation, value: annotation }
}
)
}

const selectAnnotation = box => (e, ui) => {
const {label, value} = ui.item;
const { label, value } = ui.item;

const score = value.value ?? 0;
box.find('#comment-score').val(score);
Expand All @@ -44,7 +44,7 @@ const selectAnnotation = box => (e, ui) => {
return false;
}

function focusAnnotation( event, ui ) {
function focusAnnotation(event, ui) {
$(this).val(ui.item.label);
return false;
}
Expand Down Expand Up @@ -191,7 +191,7 @@ function plusFix(n) {
function fillAnnotationBox() {
retrieveSharedComments();
$('#loadScreen').css('display', 'flex');
$.get(document.URL, function(data) {
$.get(document.URL, function (data) {
const $page = $('<div />').html(data);
$('.problemGrades').html($page.find('.problemGrades'));
$('#annotationPane').html($page.find(' #annotationPane'));
Expand Down Expand Up @@ -382,7 +382,7 @@ function attachEvents() {
$('.code-table').scrollTo($annotationLine.children().last(), { duration: "fast" });
refreshAnnotations();
} else {
M.toast({html: 'Only one annotation can be created per line at a time!'});
M.toast({ html: 'Only one annotation can be created per line at a time!' });
}
});
}
Expand Down Expand Up @@ -461,7 +461,7 @@ function attachAnnotationPaneEvents() {
});

// Chevron events (collapse / show problem)
$('.collapsible-header-controls .collapse-icon, .collapsible-header-controls .expand-icon').on('click', function(e) {
$('.collapsible-header-controls .collapse-icon, .collapsible-header-controls .expand-icon').on('click', function (e) {
e.preventDefault();
if ($('#loadScreen').css('display') === 'flex') return;
$(e.target).closest(".collapsible-header-wrap").find(".collapsible-header").click();
Expand Down Expand Up @@ -545,23 +545,31 @@ function newAnnotationFormCode() {
problemGraderId[score.problem_id] = score.grader_id;
});

// Clear any existing options from the problem dropdown
var $problemSelect = box.find('select.problem-id');
$problemSelect.empty();

// Add a default select option
$problemSelect.append($('<option value="">-- Select Problem --</option>'));

// Add problems to the dropdown
var processStarred = false;
_.each(problems, function (problem) {
if (problemGraderId[problem.id] !== 0) { // Because grader == 0 is autograder
if(problem.starred && !processStarred){
box.find('select').append(
$('<option disabled></option>').text('Starred Problems')
if (problem.starred && !processStarred) {
box.find('select.problem-id').append(
$('<option disabled></option>').text('Starred Problems')
);
processStarred=true;
processStarred = true;
}
if(!problem.starred && processStarred){
box.find('select').append(
$('<option disabled></option>').text('-------------------')
if (!problem.starred && processStarred) {
box.find('select.problem-id').append(
$('<option disabled></option>').text('-------------------')
);
processStarred=false;
processStarred = false;
}
box.find("select").append(
$("<option />").val(problem.id).text(problem.name)
box.find("select.problem-id").append(
$("<option />").val(problem.id).text(problem.name)
);
}
});
Comment on lines +548 to 575
Copy link
Contributor

@coderabbitai coderabbitai bot May 5, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Missing re-initialisation of Materialize <select> after dynamic option rebuild

$problemSelect.empty() + option injection rebuilds the <select>, but Materialize caches the original markup; consequently the dropdown shown to users will still reference the old option set until the page is refreshed.

Add a one-liner after the options are injected:

   $problemSelect.append($('<option value="">-- Select Problem --</option>'));
   // … append problem options …
+
+  // Re-initialise the Materialize component so the UI reflects the new list
+  if (M.FormSelect) {
+    M.FormSelect.init($problemSelect[0]);
+  }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Clear any existing options from the problem dropdown
var $problemSelect = box.find('select.problem-id');
$problemSelect.empty();
// Add a default select option
$problemSelect.append($('<option value="">-- Select Problem --</option>'));
// Add problems to the dropdown
var processStarred = false;
_.each(problems, function (problem) {
if (problemGraderId[problem.id] !== 0) { // Because grader == 0 is autograder
if(problem.starred && !processStarred){
box.find('select').append(
$('<option disabled></option>').text('Starred Problems')
if (problem.starred && !processStarred) {
box.find('select.problem-id').append(
$('<option disabled></option>').text('Starred Problems')
);
processStarred=true;
processStarred = true;
}
if(!problem.starred && processStarred){
box.find('select').append(
$('<option disabled></option>').text('-------------------')
if (!problem.starred && processStarred) {
box.find('select.problem-id').append(
$('<option disabled></option>').text('-------------------')
);
processStarred=false;
processStarred = false;
}
box.find("select").append(
$("<option />").val(problem.id).text(problem.name)
box.find("select.problem-id").append(
$("<option />").val(problem.id).text(problem.name)
);
}
});
// Clear any existing options from the problem dropdown
var $problemSelect = box.find('select.problem-id');
$problemSelect.empty();
// Add a default select option
$problemSelect.append($('<option value="">-- Select Problem --</option>'));
// Add problems to the dropdown
var processStarred = false;
_.each(problems, function (problem) {
if (problemGraderId[problem.id] !== 0) { // Because grader == 0 is autograder
if (problem.starred && !processStarred) {
box.find('select.problem-id').append(
$('<option disabled></option>').text('Starred Problems')
);
processStarred = true;
}
if (!problem.starred && processStarred) {
box.find('select.problem-id').append(
$('<option disabled></option>').text('-------------------')
);
processStarred = false;
}
box.find("select.problem-id").append(
$("<option />").val(problem.id).text(problem.name)
);
}
});
// Re-initialise the Materialize component so the UI reflects the new list
if (M.FormSelect) {
M.FormSelect.init($problemSelect[0]);
}
🤖 Prompt for AI Agents (early access) 
In app/assets/javascripts/annotations.js around lines 548 to 575, after dynamically rebuilding the options in the problem dropdown using $problemSelect.empty() and appending new options, the Materialize select component is not re-initialized, causing the UI to show stale options. Fix this by calling the Materialize select initialization method on $problemSelect immediately after all options are appended to refresh the dropdown UI.

Expand All @@ -571,37 +579,88 @@ function newAnnotationFormCode() {
e.preventDefault();
$(this).parents(".annotation-form").parent().remove();
refreshAnnotations();
})
});

// Setup autocomplete for the comment field
box.find('#comment-textarea').autocomplete({
appendTo: box.find('#comment-textarea').parent(),
source: getSharedCommentsForProblem(box.find("select").val()) || [],
minLength: 0,
delay: 0,
source: getSharedCommentsForProblem(box.find("select.problem-id").val()) || [],
select: selectAnnotation(box),
focus: focusAnnotation
}).focus(function () {
M.textareaAutoResize($(this));
$(this).autocomplete('search', $(this).val())
});

box.tooltip();
// Handle problem selection change - update rubric items dropdown
box.find("select.problem-id").on('change', function () {
var problem_id = $(this).val();
var $rubricSelect = box.find('select.rubric-item-id');
var $rubricContainer = $rubricSelect.closest('.row');

box.find("select").on('change', function () {
const problem_id = $(this).val();
// Clear existing options
$rubricSelect.empty();

// Update autocomplete to display shared comments for selected problem
box.find("#comment-textarea").autocomplete({
source: getSharedCommentsForProblem(problem_id) || []
// If no problem selected, hide the rubric dropdown
if (!problem_id) {
$rubricContainer.hide();
return;
}

// Check if selected problem has any rubric items
var rubricItems = rubricItemsByProblem[problem_id] || [];
if (rubricItems.length === 0) {
// No rubric items for this problem, hide the dropdown
$rubricContainer.hide();
return;
}

// Problem has rubric items, show the dropdown
$rubricContainer.show();
$rubricSelect.prop('disabled', false);

// Add "No Rubric" option
$rubricSelect.append($('<option value="">-- No Rubric Item --</option>'));

// Add rubric items for the selected problem
rubricItems.forEach(function (item) {
$rubricSelect.append(
$('<option />')
.val(item.id)
.text(item.description + " (" + plusFix(item.points) + " points)")
);
});

// Update autocomplete for the selected problem
box.find('#comment-textarea').autocomplete({
source: getSharedCommentsForProblem(problem_id) || [],
});
});

// Initial state: hide rubric items dropdown until a problem with rubric items is selected
box.find('select.rubric-item-id').closest('.row').hide();

// Setup event for rubric item selection
box.find('.rubric-item-id').on('change', function () {
const selectedRubricId = $(this).val();
if (selectedRubricId) {
const problem_id = box.find('select.problem-id').val();
const rubricItems = rubricItemsByProblem[problem_id] || [];
const rubricItem = rubricItems.find(function (item) { return item.id == selectedRubricId; });
}
});

box.tooltip();

box.find('.annotation-form').submit(function (e) {
e.preventDefault();
var comment = $(this).find(".comment").val();
var shared_comment = $(this).find("#shared-comment").is(":checked");
var score = $(this).find(".score").val();
var problem_id = $(this).find(".problem-id").val();
var rubric_item_id = $(this).find(".rubric-item-id").val() || null;
var line = $(this).parent().parent().data("lineId");

if (comment === undefined || comment === "") {
Expand All @@ -622,8 +681,7 @@ function newAnnotationFormCode() {
return;
}


submitNewAnnotation(comment, shared_comment, false, score, problem_id, line, $(this));
submitNewAnnotation(comment, shared_comment, false, score, problem_id, line, $(this), rubric_item_id);
});

return box;
Expand Down Expand Up @@ -723,20 +781,20 @@ function initializeBoxForm(box, annotation) {
var processStarred = false;
_.each(problems, function (problem) {
if (problemGraderId[problem.id] !== 0) { // Because grader == 0 is autograder
if(problem.starred && !processStarred){
if (problem.starred && !processStarred) {
box.find('select').append(
$('<option disabled></option>').text('Starred Problems')
$('<option disabled></option>').text('Starred Problems')
);
processStarred=true;
processStarred = true;
}
if(!problem.starred && processStarred){
if (!problem.starred && processStarred) {
box.find('select').append(
$('<option disabled></option>').text('-------------------')
$('<option disabled></option>').text('-------------------')
);
processStarred=false;
processStarred = false;
}
box.find("select").append(
$("<option />").val(problem.id).text(problem.name)
$("<option />").val(problem.id).text(problem.name)
);
}
});
Expand Down Expand Up @@ -815,7 +873,7 @@ function newAnnotationBox(annotation) {
e.preventDefault();
box.find('.annotation-box').hide();
box.find('.annotation-form').show().css('width', '100%');

M.textareaAutoResize(box.find('#comment-textarea'));

box.find('#comment-textarea').autocomplete({
Expand All @@ -830,7 +888,7 @@ function newAnnotationBox(annotation) {
$(this).autocomplete('search', $(this).val())
});
box.tooltip();

refreshAnnotations();
});

Expand All @@ -839,7 +897,7 @@ function newAnnotationBox(annotation) {

// Update autocomplete to display shared comments for selected problem
box.find("#comment-textarea").autocomplete({
source: getSharedCommentsForProblem(problem_id) || [],
source: getSharedCommentsForProblem(problem_id) || [],
});
});

Expand Down Expand Up @@ -1318,9 +1376,18 @@ var submitNewPDFAnnotation = function (comment, value, problem_id, pageInd, xRat
}

/* sets up and calls $.ajax to submit an annotation */
var submitNewAnnotation = function (comment, shared_comment, global_comment, value, problem_id, lineInd, form) {
var submitNewAnnotation = function (comment, shared_comment, global_comment, value, problem_id, lineInd, form, rubric_item_id) {
var newAnnotation = createAnnotation();
Object.assign(newAnnotation, { line: parseInt(lineInd), comment, value, problem_id, filename: fileNameStr, shared_comment, global_comment });
Object.assign(newAnnotation, {
line: parseInt(lineInd),
comment,
value,
problem_id,
filename: fileNameStr,
shared_comment,
global_comment,
rubric_item_id // Add this parameter
});

if (comment === undefined || comment === "") {
$(form).find('.error').text("Could not save annotation. Please refresh the page and try again.").show();
Expand Down
Loading
Loading