diff --git a/Changelog.md b/Changelog.md index e096873ab6..95d741b08d 100644 --- a/Changelog.md +++ b/Changelog.md @@ -11,6 +11,7 @@ - Update autotest settings form UI (#7777) - Store start and end date for courses (#7783) - Update batch test runs table UI (#7790) +- Implement assignment caching (#7787) ### 🐛 Bug fixes - Fixed the editing form of marking schemes to include newly added assessments (#7788) diff --git a/app/models/assignment.rb b/app/models/assignment.rb index 62fab2d0d8..9b29a71dff 100644 --- a/app/models/assignment.rb +++ b/app/models/assignment.rb @@ -940,6 +940,65 @@ def get_num_marked(ta_id = nil, bulk: false) end end + # Batch load TA stats for multiple assignments to avoid N+1 queries. + # Returns { assignment_id => { num_assigned: Integer, num_marked: Integer } } + def self.batch_ta_stats(assignments, ta_id) + return {} if ta_id.nil? || assignments.empty? + + assignment_ids = assignments.map(&:id) + + # Query 1: Count assigned groupings per assignment + assigned_counts = TaMembership + .joins(:grouping) + .where(role_id: ta_id, groupings: { assessment_id: assignment_ids }) + .group('groupings.assessment_id') + .count + + # Identify criteria-based assignments for this TA + criteria_assignment_ids = CriterionTaAssociation + .joins(:criterion) + .where(ta_id: ta_id, criteria: { assessment_id: assignment_ids }) + .distinct + .pluck('criteria.assessment_id') + + criteria_based_ids = assignments + .select { |a| a.assign_graders_to_criteria && criteria_assignment_ids.include?(a.id) } + .map(&:id) + + regular_ids = assignment_ids - criteria_based_ids + + # Query 2: Count marked results for non-criteria-based assignments + marked_counts = if regular_ids.any? + Result + .joins(submission: { grouping: :ta_memberships }) + .where( + submissions: { submission_version_used: true }, + memberships: { role_id: ta_id }, + groupings: { assessment_id: regular_ids }, + marking_state: Result::MARKING_STATES[:complete] + ) + .group('groupings.assessment_id') + .count + else + {} + end + + # Build result hash + result = {} + assignments.each do |a| + num_marked = if criteria_based_ids.include?(a.id) + a.get_num_marked(ta_id) # Fall back for criteria-based + else + marked_counts[a.id] || 0 + end + result[a.id] = { + num_assigned: assigned_counts[a.id] || 0, + num_marked: num_marked + } + end + result + end + def get_num_annotations(ta_id = nil) if ta_id.nil? num_annotations_all diff --git a/app/views/assignments/_list_manage.html.erb b/app/views/assignments/_list_manage.html.erb index b3d49ad16a..73ce568e84 100644 --- a/app/views/assignments/_list_manage.html.erb +++ b/app/views/assignments/_list_manage.html.erb @@ -2,6 +2,7 @@ .includes(:submission_rule, :assessment_section_properties, :pr_assignment) .order(:due_date, :short_identifier) %> <% action = @current_role.instructor? ? 'edit' : 'summary' %> +<% ta_stats = @current_role.ta? ? Assignment.batch_ta_stats(assignments, @current_role.id) : {} %> <% if assignments.empty? %>

<%= t('assignments.none') %>

@@ -39,12 +40,17 @@ <% end %> <% if assignment.submission_rule.can_collect_all_now? %> - <% ta_id = @current_role.instructor? ? nil : @current_role.id %>
- <%= t('submissions.how_many_marked', - num_marked: assignment.get_num_marked(ta_id), - num_assigned: assignment.get_num_assigned(ta_id)) %> + <% if ta_stats[assignment.id] %> + <%= t('submissions.how_many_marked', + num_marked: ta_stats[assignment.id][:num_marked], + num_assigned: ta_stats[assignment.id][:num_assigned]) %> + <% else %> + <%= t('submissions.how_many_marked', + num_marked: assignment.get_num_marked, + num_assigned: assignment.get_num_assigned) %> + <% end %>
<%= link_to t('assignments.grades'), diff --git a/spec/models/assignment_spec.rb b/spec/models/assignment_spec.rb index 86c3ecbf48..7c56315ea1 100644 --- a/spec/models/assignment_spec.rb +++ b/spec/models/assignment_spec.rb @@ -2804,6 +2804,88 @@ def grouping_count(groupings) end end + describe '#get_num_assigned' do + let(:ta) { create(:ta) } + let(:assignment) { create(:assignment) } + let!(:grouping1) { create(:grouping_with_inviter, assignment: assignment) } + let!(:grouping2) { create(:grouping_with_inviter, assignment: assignment) } + + before do + create(:grouping_with_inviter, assignment: assignment) # grouping3 - not assigned to TA + create(:ta_membership, role: ta, grouping: grouping1) + create(:ta_membership, role: ta, grouping: grouping2) + end + + context 'when user is instructor' do + it 'returns total number of groupings' do + expect(assignment.get_num_assigned).to eq(3) + end + end + + context 'when user is TA' do + it 'returns number of groupings assigned to them' do + expect(assignment.get_num_assigned(ta.id)).to eq(2) + end + end + end + + describe '.batch_ta_stats' do + let(:ta) { create(:ta) } + let(:assignment1) { create(:assignment) } + let(:assignment2) { create(:assignment) } + + let(:grouping1) { create(:grouping_with_inviter_and_submission, assignment: assignment1, is_collected: true) } + let(:grouping2) { create(:grouping_with_inviter_and_submission, assignment: assignment1, is_collected: true) } + let(:grouping3) { create(:grouping_with_inviter_and_submission, assignment: assignment2, is_collected: true) } + + before do + create(:complete_result, submission: grouping1.submissions.first) + create(:incomplete_result, submission: grouping2.submissions.first) + create(:complete_result, submission: grouping3.submissions.first) + create(:ta_membership, role: ta, grouping: grouping1) + create(:ta_membership, role: ta, grouping: grouping2) + create(:ta_membership, role: ta, grouping: grouping3) + end + + it 'returns empty hash when ta_id is nil' do + expect(Assignment.batch_ta_stats([assignment1, assignment2], nil)).to eq({}) + end + + it 'returns empty hash when assignments is empty' do + expect(Assignment.batch_ta_stats([], ta.id)).to eq({}) + end + + it 'returns correct stats for multiple assignments' do + stats = Assignment.batch_ta_stats([assignment1, assignment2], ta.id) + + expect(stats[assignment1.id][:num_assigned]).to eq(2) + expect(stats[assignment1.id][:num_marked]).to eq(1) + expect(stats[assignment2.id][:num_assigned]).to eq(1) + expect(stats[assignment2.id][:num_marked]).to eq(1) + end + + context 'when assignment has criteria-based grading' do + let(:assignment3) do + create(:assignment_with_criteria_and_results_with_remark, + assignment_properties_attributes: { assign_graders_to_criteria: true }) + end + let(:grouping4) { create(:grouping_with_inviter_and_submission, assignment: assignment3) } + + before do + create(:ta_membership, role: ta, grouping: assignment3.groupings.first) + create(:ta_membership, role: ta, grouping: grouping4) + create(:criterion_ta_association, ta: ta, criterion: assignment3.criteria.first) + end + + it 'falls back to get_num_marked for criteria-based assignments' do + stats = Assignment.batch_ta_stats([assignment3], ta.id) + + expect(stats[assignment3.id][:num_assigned]).to eq(2) + expect(stats[assignment3.id][:num_marked]).to eq(assignment3.get_num_marked(ta.id)) + end + end + end + describe '#completed_result_marks' do let(:assignment) { create(:assignment) } let!(:criteria) { create_list(:rubric_criterion, 2, assignment: assignment, max_mark: 4) }