ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

faster grades summary page 

fixes CNVS-8940

Test plan:
  * load the grades page as a student
  * everything should work
  * repeat as a teacher visiting the student grades page

Change-Id: I2f989c0b0dba9a17c904ba516154adb18ceaaad5
Reviewed-on: https://gerrit.instructure.com/25421
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Amber Taniuchi <amber@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
This commit is contained in:
Cameron Matheson 2013-10-22 16:12:36 -06:00
parent 11981501c2
commit 26e50b0f11
10 changed files with 126 additions and 104 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
app/coffeescripts/grade_calculator.coffee
View File

@ -16,6 +16,10 @@ define [
#
# each group needs fields: id, rules, group_weight, assignments
# rules is { drop_lowest: n, drop_highest: n, never_drop: [id...] }
# assignments is [
# { id, points_possible, submission_types},
# ...
# ]
#
# if weighting_scheme is "percent", group weights are used, otherwise no
# weighting is applied

31
app/controllers/gradebooks_controller.rb
View File

@ -52,15 +52,17 @@ class GradebooksController < ApplicationController
@presenter.assignments
@presenter.groups_assignments = groups_as_assignments(@presenter.groups, :out_of_final => true, :exclude_total => @context.hide_final_grades?)
@presenter.submissions
@presenter.submissions_by_assignment
@presenter.submission_counts
@presenter.assignment_stats
end
submissions_json = @presenter.submissions.map { |s|
submission_json(s, s.assignment, @current_user, session)
{ assignment_id: s.assignment_id, score: s.score }
}
js_env :submissions => submissions_json,
:assignment_groups => assignment_groups_json,
:group_weighting_scheme => @context.group_weighting_scheme
ags_json = light_weight_ags_json(@presenter.groups)
js_env submissions: submissions_json,
assignment_groups: ags_json,
group_weighting_scheme: @context.group_weighting_scheme
format.html { render :action => 'grade_summary' }
else
format.html { render :action => 'grade_summary_list' }
@ -69,6 +71,24 @@ class GradebooksController < ApplicationController
end
end
def light_weight_ags_json(assignment_groups)
assignment_groups.map do |ag|
assignments = ag.active_assignments.map do |a|
{
:id => a.id,
:submission_types => a.submission_types_array,
:points_possible => a.points_possible,
}
end
{
:id => ag.id,
:rules => ag.rules_hash,
:group_weight => ag.group_weight,
:assignments => assignments,
}
end
end
def grading_rubrics
@rubric_contexts = @context.rubric_contexts(@current_user)
if params[:context_code]
@ -155,6 +175,7 @@ class GradebooksController < ApplicationController
@groups_order = {}
@groups.each_with_index{|group, idx| @groups_order[group.id] = idx }
@just_assignments = @context.assignments.active.gradeable.order(:due_at, Assignment.best_unicode_collation_key('title')).select{|a| @groups_order[a.assignment_group_id] }
newest = Time.parse("Jan 1 2010")
@just_assignments = @just_assignments.sort_by{|a| [a.due_at || newest, @groups_order[a.assignment_group_id] || 0, a.position || 0] }
@assignments = @just_assignments.dup + groups_as_assignments(@groups)

15
app/models/assignment.rb
View File

@ -793,21 +793,6 @@ class Assignment < ActiveRecord::Base
end
end
def grade_distribution(submissions = nil)
submissions ||= self.submissions
tally = 0
cnt = 0
scores = submissions.map{|s| s.score}.compact
scores.each do |score|
tally += score
cnt += 1
end
high = scores.max
low = scores.min
mean = tally.to_f / cnt.to_f
[high, low, mean]
end
# Everyone, students, TAs, teachers
def participants
context.participants

14
app/models/submission.rb
View File

@ -1100,11 +1100,21 @@ class Submission < ActiveRecord::Base
def read_state(current_user)
return "read" unless current_user #default for logged out user
uid = current_user.is_a?(User) ? current_user.id : current_user
state = content_participations.find_by_user_id(uid).try(:workflow_state)
cp = if content_participations.loaded?
content_participations.detect { |cp| cp.user_id == uid }
else
content_participations.find_by_user_id(uid)
end
state = cp.try(:workflow_state)
return state if state.present?
return "read" if (assignment.deleted? || assignment.muted? || !self.user_id)
return "unread" if (self.grade || self.score)
return "unread" if self.submission_comments.where("author_id<>?", user_id).exists?
has_comments = if visible_submission_comments.loaded?
visible_submission_comments.detect { |c| c.author_id != user_id }
else
visible_submission_comments.where("author_id<>?", user_id).first
end
return "unread" if has_comments
return "read"
end

14
app/presenters/grade_summary_assignment_presenter.rb
View File

@ -9,7 +9,8 @@ class GradeSummaryAssignmentPresenter
end
def hide_distribution_graphs?
submissions.length < 5 || assignment.context.hide_distribution_graphs?
submission_count = @summary.submission_counts[assignment.id] || 0
submission_count < 5 || assignment.context.hide_distribution_graphs?
end
def is_unread?
@ -108,7 +109,12 @@ class GradeSummaryAssignmentPresenter
end
def grade_distribution
@grade_distribution ||= assignment.grade_distribution(submissions)
@grade_distribution ||= begin
stats = @summary.assignment_stats[assignment.id]
[stats.max.to_f.round(1),
stats.min.to_f.round(1),
stats.avg.to_f.round(1)]
end
end
def graph
@ -123,10 +129,6 @@ class GradeSummaryAssignmentPresenter
@file ||= submission.attachments.detect{|a| submission.turnitin_data && submission.turnitin_data[a.asset_string] }
end
def submissions
@submissions ||= @summary.submissions_by_assignment[assignment.id] || []
end
def comments
submission.visible_submission_comments
end

59
app/presenters/grade_summary_presenter.rb
View File

@ -83,44 +83,56 @@ class GradeSummaryPresenter
end
def groups
@groups ||= @context.assignment_groups.active.all
@groups ||= @context.assignment_groups.
active.includes(:active_assignments => :assignment_overrides).all
end
def assignments
@assignments ||= begin
scope = @context.assignments.active.gradeable
array = scope.includes(:assignment_overrides).collect{|a| a.overridden_for(student)}.sort
# pre-cache the assignment group for each assignment object
group_index = groups.index_by(&:id)
array.each{ |a| a.assignment_group = group_index[a.assignment_group_id] }
array
groups.flat_map(&:active_assignments).select { |a|
a.submission_types != 'not_graded'
}.map { |a|
# prevent extra loads
a.context = @context
a.assignment_group = group_index[a.assignment_group_id]
a.overridden_for(student)
}.sort
end
end
def submissions
@submissions ||= @context.submissions.
except(:includes).
includes(:submission_comments, :rubric_assessments, :assignment).
includes(:visible_submission_comments, {:rubric_assessments => :rubric}, :content_participations).
find_all_by_user_id(student)
end
def submissions_by_assignment
@submissions_by_assignment ||=
if allow_loading_all_submissions?
# Yes, fetch *all* submissions for this course; otherwise the view will end up doing a query for each
# assignment in order to calculate grade distributions
@context.submissions.
select([:assignment_id, :score, :grade, :quiz_submission_id]).
except(:includes).
group_by(&:assignment_id)
else
{}
end
def submission_counts
@submission_counts ||= @context.assignments.active
.joins(:submissions)
.group("assignments.id")
.count("submissions.id")
end
def assignment_stats
@stats ||= @context.active_assignments
.joins(:submissions)
.group("assignments.id")
.select("assignments.id, max(score), min(score), avg(score)")
.index_by(&:id)
end
def assignment_presenters
submission_index = submissions.index_by(&:assignment_id)
assignments.map{|a| GradeSummaryAssignmentPresenter.new(self, @current_user, a, submission_index[a.id]) }
assignments.map{ |a|
if s = submission_index[a.id]
s.assignment = a # prevent extra assignment loads
end
GradeSummaryAssignmentPresenter.new(self, @current_user, a, s)
}
end
def has_muted_assignments?
@ -168,11 +180,4 @@ class GradeSummaryPresenter
@groups_assignments = value
assignments.concat(value)
end
protected
def allow_loading_all_submissions?
threshold = Setting.get('grade_distributions_submission_count_threshold', '0').to_i
@context.allows_gradebook_uploads? && (threshold == 0 || @context.submissions.count < threshold)
end
end

6
spec/controllers/assignments_controller_spec.rb
View File

@ -19,10 +19,6 @@
require File.expand_path(File.dirname(__FILE__) + '/../sharding_spec_helper')
describe AssignmentsController do
# it "should use AssignmentsController" do
# controller.should be_an_instance_of(AssignmentsController)
# end
def course_assignment(course = nil)
course ||= @course
@group = course.assignment_groups.create(:name => "some group")
@ -39,7 +35,7 @@ describe AssignmentsController do
get 'index'
assert_status(404)
end
it "should return unauthorized without a valid session" do
course_with_student(:active_all => true)
get 'index', :course_id => @course.id

53
spec/controllers/gradebooks_controller_spec.rb
View File

@ -178,31 +178,6 @@ describe GradebooksController do
assigns[:courses_with_grades].should be_nil
end
it "should assign submissions_by_assignment" do
course_with_teacher_logged_in(:active_all => true)
student_in_course(:active_all => true)
assignment1 = @course.assignments.create(:title => "Assignment 1")
submission1 = assignment1.submit_homework(@student)
assignment2 = @course.assignments.create(:title => "Assignment 2")
submission2 = assignment2.submit_homework(@student)
get 'grade_summary', :course_id => @course.id, :id => @student.id
assigns[:presenter].submissions_by_assignment.values.map(&:count).should == [1,1]
end
it "should assign an empty submissions_by_assignment for MOOCs" do
course_with_teacher_logged_in(:active_all => true)
@course.settings = { :large_roster => true }
student_in_course(:active_all => true)
assignment1 = @course.assignments.create(:title => "Assignment 1")
submission1 = assignment1.submit_homework(@student)
assignment2 = @course.assignments.create(:title => "Assignment 2")
submission2 = assignment2.submit_homework(@student)
get 'grade_summary', :course_id => @course.id, :id => @student.id
assigns[:presenter].submissions_by_assignment.should == {}
end
it "should assign values for grade calculator to ENV" do
course_with_teacher_logged_in(:active_all => true)
student_in_course(:active_all => true)
@ -219,7 +194,7 @@ describe GradebooksController do
assignment1.save!
get 'grade_summary', :course_id => @course.id, :id => @student.id
assigns[:js_env][:assignment_groups].first["assignments"].first["discussion_topic"].should be_nil
assigns[:js_env][:assignment_groups].first[:assignments].first["discussion_topic"].should be_nil
end
it "should sort assignments by due date (null last), then title" do
@ -284,7 +259,7 @@ describe GradebooksController do
session[:become_user_id] = @fake_student.id
get 'grade_summary', :course_id => @course.id, :id => @fake_student.id
assigns[:presenter].assignments.find{|a| a.class == Assignment}.due_at.should == @due_at
assigns[:presenter].assignments.find{|a| a.class == Assignment}.due_at.to_i.should == @due_at.to_i
end
it "should reflect group overrides when student is a member" do
@ -545,4 +520,28 @@ describe GradebooksController do
@teacher.reload.preferences[:enable_speedgrader_grade_by_question].should_not be_true
end
end
describe '#light_weight_ags_json' do
it 'should return the necessary JSON for GradeCalculator' do
course_with_student
ag = @course.assignment_groups.create! group_weight: 100
a = ag.assignments.create! :submission_types => 'online_upload',
:points_possible => 10,
:context => @course
@controller.light_weight_ags_json([ag]).should == [
{
id: ag.id,
rules: {},
group_weight: 100,
assignments: [
{
id: a.id,
points_possible: 10,
submission_types: ['online_upload'],
}
],
},
]
end
end
end

2
spec/models/submission_comment_spec.rb
View File

@ -664,7 +664,7 @@ This text has a http://www.google.com link in it...
@submission.unread?(@student).should be_true
end
it "should be unread after submission is commented on by self" do
it "should be read after submission is commented on by self" do
expect {
@comment = SubmissionComment.create!(@valid_attributes.merge({:author => @student}))
}.to change(ContentParticipation, :count).by(0)

32
spec/presenters/grade_summary_presenter_spec.rb
View File

@ -18,22 +18,6 @@ describe GradeSummaryPresenter do
it 'includes courses where the user is enrolled' do
presenter.selectable_courses.should include(course)
end
describe "submissions_by_assignment" do
before do
Setting.set('grade_distributions_submission_count_threshold', '2')
assignment.submissions.create!(:user => @user)
end
it "loads submissions in a small course" do
presenter.submissions_by_assignment[assignment.id].size.should == 1
end
it "doesn't load in a large course" do
assignment.submissions.create!(:user => student_in_course(:course => course).user)
presenter.submissions_by_assignment.size.should == 0
end
end
end
describe 'across shards' do
@ -73,5 +57,21 @@ describe GradeSummaryPresenter do
end
end
describe '#assignment_stats' do
it 'works' do
teacher_in_course
s1, s2, s3 = n_students_in_course(3)
a = @course.assignments.create! points_possible: 10
a.grade_student s1, grade: 0
a.grade_student s2, grade: 5
a.grade_student s3, grade: 10
p = GradeSummaryPresenter.new(@course, @teacher, nil)
stats = p.assignment_stats
assignment_stats = stats[a.id]
assignment_stats.max.to_f.should == 10
assignment_stats.min.to_f.should == 0
assignment_stats.avg.to_f.should == 5
end
end
end
end