DA - grade calculation works on backend
fixes CNVS-13715
test plan:
- create a student who is in one group and not another
- create an assignment only visible to each group
- grade the student for both assignments
- turn DA on
- as the student and teacher, go to the student grades page
- both of the assignments should be visible
- final grade should be correct (factoring in both)
* final grade = ungraded assignments count too
- delete one of the grades and return
- only one of the assignments should be visible
- final grade should be correct (using just one assignment)
- turn DA off
- as the student and teacher, go to the student grades page
- both assignments are visible
- final grade should factor in both assignments
- with DA on and off, ensure that the following work:
- drop rules
- never drop rules
- assignment stats (mean median etc)
- what if scores
Change-Id: I727aff943b14c91089ccffa6d3b63ba026abbeec
Reviewed-on: https://gerrit.instructure.com/36762
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Cameron Sutter <csutter@instructure.com>
QA-Review: Caleb Guanzon <cguanzon@instructure.com>
Product-Review: Mike Nomitch <mnomitch@instructure.com>
This commit is contained in:
Michael Nomitch
Mike Nomitch
parent
00bf9b4ea9
commit
b4840c75cc
|
|
@ -63,7 +63,7 @@ class GradebooksController < ApplicationController
|
|||
'score' => s.grants_right?(@current_user, :read_grade)? s.score : nil
|
||||
}
|
||||
}
|
||||
ags_json = light_weight_ags_json(@presenter.groups)
|
||||
ags_json = light_weight_ags_json(@presenter.groups, {student: @presenter.student})
|
||||
js_env submissions: submissions_json,
|
||||
assignment_groups: ags_json,
|
||||
group_weighting_scheme: @context.group_weighting_scheme,
|
||||
|
|
@ -78,9 +78,9 @@ class GradebooksController < ApplicationController
|
|||
end
|
||||
end
|
||||
|
||||
def light_weight_ags_json(assignment_groups)
|
||||
def light_weight_ags_json(assignment_groups, opts={})
|
||||
assignment_groups.map do |ag|
|
||||
assignments = ag.visible_assignments(@current_user).map do |a|
|
||||
assignments = ag.visible_assignments(opts[:student] || @current_user).map do |a|
|
||||
{
|
||||
:id => a.id,
|
||||
:submission_types => a.submission_types_array,
|
||||
|
|
|
|||
|
|
@ -771,6 +771,10 @@ class Submission < ActiveRecord::Base
|
|||
scope :for_user, lambda { |user| where(:user_id => user) }
|
||||
scope :needing_screenshot, -> { where("submissions.submission_type='online_url' AND submissions.attachment_id IS NULL AND submissions.process_attempts<3").order(:updated_at) }
|
||||
|
||||
def assignment_visible_to_student?(user_id)
|
||||
assignment.students_with_visibility.pluck(:id).include?(user_id.to_i)
|
||||
end
|
||||
|
||||
def needs_regrading?
|
||||
graded? && !grade_matches_current_submission?
|
||||
end
|
||||
|
|
|
|||
|
|
@ -138,6 +138,7 @@ class GradeSummaryPresenter
|
|||
:content_participations)
|
||||
.where("assignments.workflow_state != 'deleted'")
|
||||
.find_all_by_user_id(student)
|
||||
.select{ |submission| submission.assignment_visible_to_student?(student_id)}
|
||||
|
||||
assignments_index = assignments.index_by(&:id)
|
||||
|
||||
|
|
@ -167,19 +168,27 @@ class GradeSummaryPresenter
|
|||
end
|
||||
|
||||
def assignment_stats
|
||||
@stats ||= @context.assignments.active
|
||||
@stats ||= begin
|
||||
chain = @context.assignments.active
|
||||
.except(:order)
|
||||
.joins(:submissions)
|
||||
.where("submissions.user_id in (?)", real_and_active_student_ids)
|
||||
.group("assignments.id")
|
||||
if @context.feature_enabled?(:differentiated_assignments)
|
||||
# if DA is on, further restrict the assignments to those visible to each student
|
||||
chain = chain.joins(:assignment_student_visibilities).where("""
|
||||
submissions.assignment_id = assignments.id
|
||||
AND submissions.assignment_id = assignment_student_visibilities.assignment_id
|
||||
AND submissions.user_id = assignment_student_visibilities.user_id
|
||||
""")
|
||||
end
|
||||
chain.group("assignments.id")
|
||||
.select("assignments.id, max(score) max, min(score) min, avg(score) avg")
|
||||
.index_by(&:id)
|
||||
end
|
||||
end
|
||||
|
||||
def real_and_active_student_ids
|
||||
@context.all_real_student_enrollments
|
||||
.where("workflow_state not in (?)", ['rejected','inactive'])
|
||||
.pluck(:user_id).uniq
|
||||
@context.all_real_student_enrollments.where("workflow_state not in (?)", ['rejected','inactive']).pluck(:user_id).uniq
|
||||
end
|
||||
|
||||
def assignment_presenters
|
||||
|
|
|
|||
|
|
@ -53,6 +53,7 @@ class GradeCalculator
|
|||
submissions_by_user = @submissions.group_by(&:user_id)
|
||||
@user_ids.map do |user_id|
|
||||
user_submissions = submissions_by_user[user_id] || []
|
||||
user_submissions.select!{|submission| submission.assignment_visible_to_student?(user_id)}
|
||||
current, current_groups = calculate_current_score(user_id, user_submissions)
|
||||
final, final_groups = calculate_final_score(user_id, user_submissions)
|
||||
[[current, current_groups], [final, final_groups]]
|
||||
|
|
@ -89,7 +90,7 @@ class GradeCalculator
|
|||
end
|
||||
|
||||
def calculate_score(submissions, user_id, grade_updates, ignore_ungraded)
|
||||
group_sums = create_group_sums(submissions, ignore_ungraded)
|
||||
group_sums = create_group_sums(submissions, ignore_ungraded, user_id)
|
||||
info = calculate_total_from_group_scores(group_sums)
|
||||
grade_updates[user_id] = info[:grade]
|
||||
[info, group_sums.index_by { |s| s[:id] }]
|
||||
|
|
@ -105,8 +106,8 @@ class GradeCalculator
|
|||
# :weight => 50},
|
||||
# ...]
|
||||
# each group
|
||||
def create_group_sums(submissions, ignore_ungraded=true)
|
||||
assignments_by_group_id = @assignments.group_by(&:assignment_group_id)
|
||||
def create_group_sums(submissions, ignore_ungraded=true, user_id)
|
||||
assignments_by_group_id = assignments_for_user(user_id).group_by(&:assignment_group_id)
|
||||
submissions_by_assignment_id = Hash[
|
||||
submissions.map { |s| [s.assignment_id, s] }
|
||||
]
|
||||
|
|
@ -146,6 +147,11 @@ class GradeCalculator
|
|||
end
|
||||
end
|
||||
|
||||
def assignments_for_user(user_id)
|
||||
valid_ids = User.find(user_id).assignments_visibile_in_course(@course).pluck(:id)
|
||||
@assignments.select{|assig| valid_ids.include? assig.id}
|
||||
end
|
||||
|
||||
# see comments for dropAssignments in grade_calculator.coffee
|
||||
def drop_assignments(submissions, rules)
|
||||
drop_lowest = rules[:drop_lowest] || 0
|
||||
|
|
|
|||
|
|
@ -598,5 +598,88 @@ describe GradeCalculator do
|
|||
check_grades(grade, grade)
|
||||
end
|
||||
end
|
||||
|
||||
context "differentiated assignments grade calculation" do
|
||||
def set_up_course_for_differentiated_assignments(enabler_method)
|
||||
@course.send(enabler_method, :differentiated_assignments)
|
||||
set_grades [[5, 20], [15, 20], [10,20], [nil, 20], [20, 20], [10,20], [nil, 20]]
|
||||
@assignments.each do |a|
|
||||
a.only_visible_to_overrides = true
|
||||
a.save!
|
||||
end
|
||||
@overridden_lowest = @assignments[0]
|
||||
@overridden_highest = @assignments[1]
|
||||
@overridden_middle = @assignments[2]
|
||||
@non_overridden_lowest = @assignments[3]
|
||||
@non_overridden_highest = @assignments[4]
|
||||
@non_overridden_middle = @assignments[5]
|
||||
@not_graded = @assignments.last
|
||||
|
||||
@user.enrollments.each(&:destroy)
|
||||
@section = @course.course_sections.create!(name: "test section")
|
||||
student_in_section(@section, user: @user)
|
||||
|
||||
create_section_override_for_assignment(@overridden_lowest, course_section: @section)
|
||||
create_section_override_for_assignment(@overridden_highest, course_section: @section)
|
||||
create_section_override_for_assignment(@overridden_middle, course_section: @section)
|
||||
end
|
||||
|
||||
def get_user_points_and_course_total(user,course)
|
||||
calc = GradeCalculator.new [user.id], course.id
|
||||
final_grade_info = calc.compute_scores.first.last.first
|
||||
[final_grade_info[:total], final_grade_info[:possible]]
|
||||
end
|
||||
|
||||
context "DA enabled" do
|
||||
before do
|
||||
set_up_course_for_differentiated_assignments(:enable_feature!)
|
||||
end
|
||||
it "should calculate scores based on visible assignments only" do
|
||||
# 5 + 15 + 10 + 20 + 10
|
||||
get_user_points_and_course_total(@user,@course). should == [60,100]
|
||||
end
|
||||
it "should drop the lowest visible when that rule is in place" do
|
||||
@group.update_attribute(:rules, 'drop_lowest:1')
|
||||
# 5 + 15 + 10 + 20 + 10 - 5
|
||||
get_user_points_and_course_total(@user,@course). should == [55,80]
|
||||
end
|
||||
it "should drop the highest visible when that rule is in place" do
|
||||
@group.update_attribute(:rules, 'drop_highest:1')
|
||||
# 5 + 15 + 10 + 20 + 10 - 20
|
||||
get_user_points_and_course_total(@user,@course). should == [40,80]
|
||||
end
|
||||
it "shouldnt count an invisible assingment with never drop on" do
|
||||
@group.update_attribute(:rules, "drop_lowest:2\nnever_drop:#{@overridden_lowest.id}")
|
||||
# 5 + 15 + 10 + 20 + 10 - 10 - 10
|
||||
get_user_points_and_course_total(@user,@course). should == [40,60]
|
||||
end
|
||||
end
|
||||
|
||||
context "DA disabled" do
|
||||
before do
|
||||
set_up_course_for_differentiated_assignments(:disable_feature!)
|
||||
end
|
||||
it "should calculate scores based on all active assignments" do
|
||||
# 5 + 15 + 10 + 0 + 20 + 10
|
||||
get_user_points_and_course_total(@user,@course). should == [60,140]
|
||||
end
|
||||
it "should drop the lowest visible when that rule is in place" do
|
||||
@group.update_attribute(:rules, 'drop_lowest:1')
|
||||
# 5 + 15 + 10 + 0 + 20 + 10 - 0
|
||||
get_user_points_and_course_total(@user,@course). should == [60,120]
|
||||
end
|
||||
it "should drop the highest visible when that rule is in place" do
|
||||
@group.update_attribute(:rules, 'drop_highest:1')
|
||||
# 5 + 15 + 10 + 0 + 20 + 10 - 20
|
||||
get_user_points_and_course_total(@user,@course). should == [40,120]
|
||||
end
|
||||
it "shouldnt count an invisible assingment with never drop on" do
|
||||
@group.update_attribute(:rules, "drop_lowest:2\nnever_drop:#{@non_overridden_lowest.id}")
|
||||
# 5 + 15 + 10 + 0 + 20 + 10 - 5 - 0
|
||||
get_user_points_and_course_total(@user,@course). should == [55,100]
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -1033,6 +1033,46 @@ describe Submission do
|
|||
submission1.assign_assessor(submission2)
|
||||
end
|
||||
end
|
||||
|
||||
describe "assignment_visible_to_student?" do
|
||||
before(:each) do
|
||||
student_in_course(active_all: true)
|
||||
@assignment.only_visible_to_overrides = true
|
||||
@assignment.save!
|
||||
@submission = @assignment.submit_homework(@student, body: 'Lorem ipsum dolor')
|
||||
end
|
||||
|
||||
it "submission should be visible with normal course" do
|
||||
@submission.assignment_visible_to_student?(@student.id).should be_true
|
||||
end
|
||||
|
||||
it "submission should not be visible with DA and no override or grade" do
|
||||
@course.enable_feature!(:differentiated_assignments)
|
||||
@submission.assignment_visible_to_student?(@student.id).should be_false
|
||||
end
|
||||
|
||||
it "submission should be visible with DA and an override" do
|
||||
@course.enable_feature!(:differentiated_assignments)
|
||||
@student.enrollments.each(&:destroy!)
|
||||
@section = @course.course_sections.create!(name: "test section")
|
||||
student_in_section(@section, user: @student)
|
||||
create_section_override_for_assignment(@submission.assignment, course_section: @section)
|
||||
@submission.reload
|
||||
@submission.assignment_visible_to_student?(@student.id).should be_true
|
||||
end
|
||||
|
||||
it "submission should be visible with DA and a grade" do
|
||||
@course.enable_feature!(:differentiated_assignments)
|
||||
@student.enrollments.each(&:destroy!)
|
||||
@section = @course.course_sections.create!(name: "test section")
|
||||
student_in_section(@section, user: @student)
|
||||
@assignment.grade_student(@user, {grade: 10})
|
||||
@submission.reload
|
||||
@submission.assignment_visible_to_student?(@student.id).should be_true
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
end
|
||||
|
||||
def submission_spec_model(opts={})
|
||||
|
|
|
|||
|
|
@ -59,8 +59,12 @@ describe GradeSummaryPresenter do
|
|||
end
|
||||
|
||||
describe '#assignment_stats' do
|
||||
it 'works' do
|
||||
context "course with DA disabled" do
|
||||
before(:each) do
|
||||
teacher_in_course
|
||||
@course.disable_feature!(:differentiated_assignments)
|
||||
end
|
||||
it 'works' do
|
||||
s1, s2, s3 = n_students_in_course(3)
|
||||
a = @course.assignments.create! points_possible: 10
|
||||
a.grade_student s1, grade: 0
|
||||
|
|
@ -75,8 +79,6 @@ describe GradeSummaryPresenter do
|
|||
end
|
||||
|
||||
it 'filters out test students and inactive enrollments' do
|
||||
@course = Course.create!
|
||||
teacher_in_course({:course => @course})
|
||||
s1, s2, s3, removed_student = n_students_in_course(4, {:course => @course})
|
||||
|
||||
fake_student = course_with_user('StudentViewEnrollment', {:course => @course}).user
|
||||
|
|
@ -101,8 +103,104 @@ describe GradeSummaryPresenter do
|
|||
assignment_stats.min.to_f.should == 0
|
||||
assignment_stats.avg.to_f.should == 5
|
||||
end
|
||||
|
||||
end
|
||||
|
||||
context "course with DA enabled" do
|
||||
before(:each) do
|
||||
teacher_in_course
|
||||
@course.enable_feature!(:differentiated_assignments)
|
||||
end
|
||||
|
||||
it 'filters out test students and inactive enrollments' do
|
||||
s1, s2, s3, removed_student = n_students_in_course(4, {:course => @course})
|
||||
|
||||
fake_student = course_with_user('StudentViewEnrollment', {:course => @course}).user
|
||||
fake_student.preferences[:fake_student] = true
|
||||
|
||||
a = @course.assignments.create! points_possible: 10, only_visible_to_overrides: false
|
||||
a.grade_student s1, grade: 0
|
||||
a.grade_student s2, grade: 5
|
||||
a.grade_student s3, grade: 10
|
||||
a.grade_student removed_student, grade: 20
|
||||
a.grade_student fake_student, grade: 100
|
||||
|
||||
removed_student.enrollments.each do |enrollment|
|
||||
enrollment.workflow_state = 'inactive'
|
||||
enrollment.save!
|
||||
end
|
||||
|
||||
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
|
||||
|
||||
it 'filters out assignments that arent visible to students' do
|
||||
s1, s2, s3 = n_students_in_course(3)
|
||||
a = @course.assignments.create! points_possible: 10, only_visible_to_overrides: true
|
||||
a.grade_student s1, grade: nil
|
||||
a.grade_student s2, grade: nil
|
||||
a.grade_student s3, grade: nil
|
||||
p = GradeSummaryPresenter.new(@course, @teacher, nil)
|
||||
stats = p.assignment_stats
|
||||
assignment_stats = stats[a.id]
|
||||
assignment_stats.should be_nil
|
||||
end
|
||||
|
||||
it 'gets the right submissions when only_visible_to_overrides is true' do
|
||||
s1, s2 = n_students_in_course(2)
|
||||
s3 = User.create!
|
||||
@section = @course.course_sections.create!(name: "test section")
|
||||
student_in_section(@section, user: s3)
|
||||
a = @course.assignments.create! points_possible: 10, only_visible_to_overrides: true
|
||||
create_section_override_for_assignment(a, course_section: @section)
|
||||
|
||||
a.grade_student s1, grade: nil
|
||||
a.grade_student s2, grade: 8
|
||||
a.grade_student s3, grade: 10
|
||||
|
||||
|
||||
p = GradeSummaryPresenter.new(@course, @teacher, nil)
|
||||
stats = p.assignment_stats
|
||||
assignment_stats = stats[a.id]
|
||||
# ensure s1 is filtered and s2 and s3 are not
|
||||
assignment_stats.max.to_f.should == 10
|
||||
assignment_stats.min.to_f.should == 8
|
||||
assignment_stats.avg.to_f.should == 9
|
||||
end
|
||||
|
||||
it 'does not filter out submissions that would be invisibile when only_visible_to_overrides is true (but is really false or nil)' do
|
||||
s1, s2 = n_students_in_course(2)
|
||||
s3 = User.create!
|
||||
@section = @course.course_sections.create!(name: "test section")
|
||||
student_in_section(@section, user: s3)
|
||||
a1 = @course.assignments.create! points_possible: 10, only_visible_to_overrides: false
|
||||
a2 = @course.assignments.create! points_possible: 10, only_visible_to_overrides: nil
|
||||
create_section_override_for_assignment(a1, course_section: @section)
|
||||
create_section_override_for_assignment(a2, course_section: @section)
|
||||
|
||||
[a1,a2].each do |a|
|
||||
a.grade_student s1, grade: 0
|
||||
a.grade_student s2, grade: 5
|
||||
a.grade_student s3, grade: 10
|
||||
end
|
||||
|
||||
p = GradeSummaryPresenter.new(@course, @teacher, nil)
|
||||
stats = p.assignment_stats
|
||||
[a1,a2].each do |a|
|
||||
assignment_1_stats = stats[a.id]
|
||||
assignment_1_stats.max.to_f.should == 10
|
||||
assignment_1_stats.min.to_f.should == 0
|
||||
assignment_1_stats.avg.to_f.should == 5
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
describe '#submission count' do
|
||||
it 'filters out test students and inactive enrollments' do
|
||||
@course = Course.create!
|
||||
|
|
@ -141,7 +239,7 @@ describe GradeSummaryPresenter do
|
|||
|
||||
a2.destroy
|
||||
|
||||
p = GradeSummaryPresenter.new(@course, @teacher, @student)
|
||||
p = GradeSummaryPresenter.new(@course, @teacher, @student.id)
|
||||
p.submissions.map(&:assignment_id).should == [a1.id]
|
||||
end
|
||||
|
||||
|
|
@ -152,7 +250,7 @@ describe GradeSummaryPresenter do
|
|||
assign.grade_student @student, grade: 10
|
||||
assign.update_attribute(:submission_types, "not_graded")
|
||||
|
||||
p = GradeSummaryPresenter.new(@course, @teacher, @student)
|
||||
p = GradeSummaryPresenter.new(@course, @teacher, @student.id)
|
||||
p.submissions.map(&:assignment_id).should == [assign.id]
|
||||
end
|
||||
end
|
||||
|
|
@ -166,7 +264,7 @@ describe GradeSummaryPresenter do
|
|||
unpublished_assign = @course.assignments.create!
|
||||
unpublished_assign.update_attribute(:workflow_state, "unpublished")
|
||||
|
||||
p = GradeSummaryPresenter.new(@course, @teacher, @student)
|
||||
p = GradeSummaryPresenter.new(@course, @teacher, @student.id)
|
||||
p.assignments.should == [published_assignment]
|
||||
end
|
||||
end
|
||||
|
|
|
|||
Loading…
Reference in New Issue