Skip to content

Manage submissions speed #2303

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 4 commits into
base: master
Choose a base branch
from
Open

Manage submissions speed #2303

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f0a5d37
Bump github/codeql-action from 3 to 4 (#2298)
dwang3851 Oct 30, 2025
806cc0f
Using pagination for manage submissions page
dwang3851 Oct 30, 2025
8ace901
Small nits
dwang3851 Oct 30, 2025
a6f0c1e
Modifying sorting query
dwang3851 Oct 30, 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
6 changes: 3 additions & 3 deletions .github/workflows/codeql-analysis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ jobs:

# Initializes the CodeQL tools for scanning.
- name: Initialize CodeQL
uses: github/codeql-action/init@v3
uses: github/codeql-action/init@v4
with:
languages: ${{ matrix.language }}
# If you wish to specify custom queries, you can do so here or in a config file.
Expand All @@ -58,7 +58,7 @@ jobs:
# Autobuild attempts to build any compiled languages (C/C++, C#, or Java).
# If this step fails, then you should remove it and run the build manually (see below)
- name: Autobuild
uses: github/codeql-action/autobuild@v3
uses: github/codeql-action/autobuild@v4

# ℹ️ Command-line programs to run using the OS shell.
# 📚 See https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsrun
Expand All @@ -71,4 +71,4 @@ jobs:
# ./location_of_script_within_repo/buildscript.sh

- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@v3
uses: github/codeql-action/analyze@v4
118 changes: 83 additions & 35 deletions app/assets/javascripts/manage_submissions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ const buttonIDs = ['#regrade-selected', '#delete-selected', '#download-selected'
let tweaks = [];
let currentPage = 0;
$(document).ready(function() {
var submission_info = {}
var submission_info = {};
var selectedStudentCids = [];
var selectedSubmissions = [];
Copy link
Contributor

@KesterTan KesterTan Nov 11, 2025

Choose a reason for hiding this comment

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

should we make these sets, would this be faster when we check for inclusion?

var submissions_to_cud = {}; // Build dynamically from server data
const EditTweakButton = (totalSum) => {
if (totalSum == null) {
return `
Expand Down Expand Up @@ -70,7 +73,7 @@ $(document).ready(function() {
$(document).ready(function () {
Copy link
Contributor

@KesterTan KesterTan Nov 11, 2025

Choose a reason for hiding this comment

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

Can we remove these double document.ready functions

$('.modal').modal();

$('.score-details').on('click', function () {
$(document).on('click', '.score-details', function () {
// Get the email
const course_user_datum_id = $(this).data('cuid');
const email = $(this).data('email');
Expand Down Expand Up @@ -229,48 +232,94 @@ $(document).ready(function() {
});
});

var selectedStudentCids = [];
var selectedSubmissions = [];

// Initialize DataTable with server-side processing
var table = $('#submissions').DataTable({
'dom': 'f<"selected-buttons">rtip', // show buttons, search, table
'paging': true,
'createdRow': completeRow,
'sPaginationType': 'full_numbers',
'pageLength': 100,
'info': true,
'deferRender': true,
dom: 'f<"selected-buttons">rtip',
processing: true,
serverSide: true,
ajax: {
url: window.location.pathname + '.json',
type: 'GET',
dataSrc: function (json) {
json.data.forEach(row => {
submissions_to_cud[row[7]] = row[8];
});
return json.data;
},
error: function (xhr, error, code) {
console.error('DataTables error:', error, code);
}
},
columns: [
{
data: null, orderable: false, className: 'submissions-td submissions-checkbox',
render: function (data, type, row, meta) {
return `<div><label class="submissions-cbox-label"><input class="cbox" type="checkbox" id="cbox-${row[7]}"><span></span></label></div>`;
}
},
{
data: 1, className: 'submissions-td',
render: function (data, type, row, meta) {
var excusedLabel = data.excused ? '<a class="submissions-excused-label" title="Click to unexcuse this student">EXCUSED</a>' : '';
return `<div class="submissions-name">${data.name || ''}${excusedLabel}</div>${data.email}`;
}
},
{data: 2, className: 'submissions-td'},
{
data: 3, className: 'submissions-td',
render: function (data, type, row, meta) {
var score = data.score != null ? data.score : '-';
return `<div class="submissions-score-align"><div class="score-num">${score}</div><div class="score-icon"><a class="modal-trigger score-details" data-email="${data.email}" data-cuid="${data.cud_id}"><i class="material-icons submissions-score-icon">zoom_in</i></a></div></div>`;
}
},
{data: 4, className: 'submissions-td', render: d => `<span class="moment-date-time">${d}</span>`},
{
data: 5,
orderable: false,
className: 'submissions-td',
render: d => d.has_file ? `<div class="submissions-center-icons"><a href="submissions/${d.submission_id}/view" title="View the file for this submission" class="btn small"><i class='material-icons'>zoom_in</i></a><p>View File</p></div>` : 'None'
},
{
data: 6, orderable: false, className: 'submissions-td exclude-click', render: function (data, row) {
var regradeBtn = data.is_autograded ? `<div class="submissions-center-icons"><a href="regradeBatch?submission_ids[]=${data.submission_id}" data-method="post" title="Regrade this submission" class="btn small"><i class='material-icons'>autorenew</i></a><p>Regrade</p></div>` : '';
return `${regradeBtn}<div class="submissions-center-icons"><a href="submissions/${data.submission_id}/destroyConfirm" title="Destroy this submission forever" class="btn small"><i class='material-icons'>delete_outline</i></a><p>Delete</p></div>`;
}
},
{data: 7, visible: false},
{data: 8, visible: false}
],
pageLength: 100,
lengthMenu: [[25, 50, 100, 200], [25, 50, 100, 200]],
order: [[4, 'desc']],
createdRow: function (row, data) {
var submissionId = data[7];
$(row).attr('id', 'row-' + submissionId).attr('data-submission-id', submissionId).addClass('submission-row');
},
drawCallback: function () {
$('#submissions tbody .cbox').each(function () {
var submissionId = parseInt($(this).attr('id').replace('cbox-', ''), 10);
$(this).prop('checked', selectedSubmissions.includes(submissionId));
if (selectedSubmissions.includes(submissionId)) $(this).closest('tr').addClass('selected');
});
updateSelectAllCheckbox();
}
});

// Check if the table is empty
if (table.data().count() === 0) {
$('#submissions').closest('.dataTables_wrapper').hide(); // Hide the table and its controls
$('#no-data-message').show(); // Optionally show a custom message
} else {
$('#no-data-message').hide(); // Hide custom message when there is data
}

function completeRow(row, data, index) {
var submission = additional_data[index];
$(row).attr('data-submission-id', submission['submission-id']);
}

$('thead').on('click', function(e) {
if (currentPage < 0) {
currentPage = 0
}
if (currentPage > table.page.info().pages) {
currentPage = table.page.info().pages - 1
}
table.page(currentPage).draw(false);
})

// Listen for select-all checkbox click
$('#cbox-select-all').on('click', async function(e) {
var selectAll = $(this).is(':checked');
await toggleAllRows(selectAll);
});

function updateSelectAllCheckbox() {
var allChecked = true, anyChecked = false;
$('#submissions tbody .cbox').each(function () {
if ($(this).prop('checked')) anyChecked = true; else allChecked = false;
});
$('#cbox-select-all').prop('checked', allChecked && anyChecked);
}

// Function to toggle all checkboxes
function toggleAllRows(selectAll) {
$('#submissions tbody .cbox').each(function() {
Expand Down Expand Up @@ -396,7 +445,6 @@ $(document).ready(function() {
return;
}
$(document).off("click", id).on("click", id, function (event) {
console.log(`${id} button clicked`);
event.preventDefault();
if (selectedSubmissions.length === 0) {
alert("No submissions selected.");
Expand Down
130 changes: 112 additions & 18 deletions app/controllers/submissions_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,29 +17,123 @@ class SubmissionsController < ApplicationController

action_auth_level :index, :instructor
def index
# cache ids instead of entire entries
submission_ids = Rails.cache.fetch(["submission_ids", @assessment.id], expires_in: 1.day) do
@assessment.submissions.order("created_at DESC").pluck(:id)
end
@submissions = Submission.where(id: submission_ids).includes({ course_user_datum: :user })
@autograded = @assessment.has_autograder?
@problems = @assessment.problems.to_a
@excused_cids = AssessmentUserDatum
.where(assessment_id: @assessment.id, grade_type: AssessmentUserDatum::EXCUSED)
.pluck(:course_user_datum_id)

@submissions_to_cud =
Rails.cache.fetch(["submissions_to_cud", @assessment.id], expires_in: 1.day) do
submissions_to_cud = {}
@submissions.each do |submission|
submissions_to_cud[submission.id] = submission.course_user_datum_id
respond_to do |format|
format.html do
@submissions = @assessment.submissions
.includes(course_user_datum: :user)
.order(created_at: :desc)
.limit(100)

@submissions_to_cud = @submissions.pluck(:id, :course_user_datum_id).to_h
end

format.json do
draw = params[:draw].to_i
start = params[:start].to_i
length = params[:length].to_i
search_value = params.dig(:search, :value)
order_column_index = params.dig(:order, '0', :column).to_i
order_direction = params.dig(:order, '0', :dir) || 'desc'

columns_map = {
1 => 'users.email',
2 => 'submissions.version',
3 => 'calculated_score',
4 => 'submissions.created_at',
}

order_column = columns_map[order_column_index] || 'submissions.created_at'

base_query = @assessment.submissions
.joins(course_user_datum: :user)

if search_value.present?
base_query = base_query.where(
"users.email LIKE :search",
search: "%#{search_value}%"
)
end
submissions_to_cud.to_json

total_records = @assessment.submissions.count
filtered_records = base_query.count

# If sorting by score, we have to load the data into ruby b/c we don't stored
# final scores in our database
if order_column == 'calculated_score'
all_submissions = base_query.includes(course_user_datum: :user).to_a
submissions_with_scores = all_submissions.map do |submission|
cud = submission.course_user_datum
[submission, submission.final_score(cud)]
end
sorted_submissions = submissions_with_scores.sort_by { |_, score| score }
sorted_submissions.reverse! if order_direction == 'desc'
paginated_submissions = sorted_submissions[start, length] || []

data = paginated_submissions.map do |submission, score|
format_submission_for_datatable(submission, score)
end
else
submissions = base_query
.includes(course_user_datum: :user)
.order("#{order_column} #{order_direction}")
.limit(length)
.offset(start)

data = submissions.map do |submission|
cud = submission.course_user_datum
score = submission.final_score(cud)
format_submission_for_datatable(submission, score)
end
end
Comment on lines +68 to +93
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 30, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Consider performance implications of in-memory score sorting.

When sorting by calculated_score, all filtered submissions are loaded into memory, scored, sorted in Ruby, then paginated. For assessments with many submissions matching the search filter, this could consume significant memory and time.

While this is a reasonable tradeoff given that final_score is computed dynamically rather than stored in the database, consider monitoring performance for large datasets or implementing score materialization if this becomes a bottleneck.

🤖 Prompt for AI Agents 
In app/controllers/submissions_controller.rb around lines 68-93, the controller
loads all filtered submissions into memory to compute and sort by
calculated_score which can be very expensive; change this to avoid full
in-memory sorting by either (A) materializing/calculating and persisting the
score on submissions (or a denormalized column on course_user_datum) and
updating it when relevant so the DB can order and paginate, or (B) if
materialization is not feasible now, fetch only the paginated IDs first (apply
the same filters and search in SQL with limit/offset), load those submissions,
compute final_score for that page and format — additionally add monitoring
(metrics/timing) around this path to detect when it becomes a bottleneck and
consider adding a background job to backfill scores if you implement
materialization.


render json: {
draw:,
recordsTotal: total_records,
recordsFiltered: filtered_records,
data:,
}
end
end
end

@excused_cids = []
excused_students = AssessmentUserDatum.where(
assessment_id: @assessment.id,
grade_type: AssessmentUserDatum::EXCUSED
)
@excused_cids = excused_students.pluck(:course_user_datum_id)
@problems = @assessment.problems.to_a
action_auth_level :format_submission_for_datatable, :instructor
def format_submission_for_datatable(submission, score = nil)
cud = submission.course_user_datum
user = cud.user
excused = @excused_cids.include?(cud.id)
score ||= submission.final_score(cud)
[
'',
{
name: [user.first_name, user.last_name].reject(&:blank?).join(' '),
email: user.email,
excused:,
cud_id: cud.id
},
submission.version,
{
score: computed_score { score },
email: user.email,
cud_id: cud.id
},
submission.created_at.in_time_zone.to_s,
{
has_file: submission.filename.present?,
submission_id: submission.id
},
{
submission_id: submission.id,
is_autograded: @autograded
},
submission.id,
cud.id
]
end

action_auth_level :score_details, :instructor
Expand Down
Loading