grade calculator: ignore unposted anonymous assignments

closes EVAL-1272
flag=grade_calc_ignore_unposted_anonymous

Test Plan:
1. Enable the 'Ignore Unposted Anonymous Assignments in Grade
   Calculation' release flag. Create an anonymous assignment and
   grade some students.
2. Before posting the assignment, enter a rails console and check the
   unposted_* scores for the students. Verify:
   - The unposted_* scores do not include the submission score from the
     anonymous unposted assignment.
3. Disable the 'Ignore Unposted Anonymous Assignments in Grade
   Calculation' release flag.
4. Enter a rails console again and check the unposted_* scores for the
   students. Verify:
   - The unposted_* scores include the submission score from the
     anonymous unposted assignment.
5. Post the assignment to students. Then enter a rails console again and
   verify the unposted_* scores for the students include the submission
   score from the anonymous (now posted) assignment.
6. Re-enable the 'Ignore Unposted Anonymous Assignments in Grade
   Calculation' release flag. Verify the unposted_* scores for the
   students include the submission score from the anonymous (now posted)
   assignment.

Change-Id: Ibb42b9f3164c08e3bc2a21fc02736760f58af51a
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/251786
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Product-Review: Syed Hussain <shussain@instructure.com>
Reviewed-by: Adrian Packel <apackel@instructure.com>
Reviewed-by: Gary Mei <gmei@instructure.com>
QA-Review: Kai Bjorkman <kbjorkman@instructure.com>
This commit is contained in:
Spencer Olson 2020-11-03 13:43:24 -06:00
parent 63f3256897
commit b0df8f5fac
6 changed files with 103 additions and 3 deletions

View File

@ -1498,6 +1498,12 @@ class Assignment < ActiveRecord::Base
end
def self.preload_unposted_anonymous_submissions(assignments)
# Don't do anything if there are no assignments OR unposted anonymous submissions are already preloaded
if assignments.is_a?(Array) &&
(assignments.empty? || assignments.all? { |a| !a.unposted_anonymous_submissions.nil? })
return
end
assignment_ids_with_unposted_anonymous_submissions = Assignment.
where(id: assignments, anonymous_grading: true).
where(
@ -1512,6 +1518,8 @@ class Assignment < ActiveRecord::Base
assignments.each do |assignment|
assignment.unposted_anonymous_submissions = assignment_ids_with_unposted_anonymous_submissions.include?(assignment.id)
end
nil
end
def touch_on_unlock_if_necessary

View File

@ -71,3 +71,16 @@ gradebook_load_assignments_by_grading_period:
display_name: Load Gradebook Assignments by Grading Period
description: If set, the Gradebook will chunk out loading of assignments, loading those for the selected
grading period first.
grade_calc_ignore_unposted_anonymous:
state: hidden
applies_to: RootAccount
display_name: Ignore Unposted Anonymous Assignments in Grade Calculation
description: If set, the grade calculator will ignore unposted anonymous assignments when calculating
totals.
environments:
ci:
state: on # enable for automated testings builds and local testing
development:
state: on # enable for local development
test:
state: on # enable for the deployed 'test' environment

View File

@ -49,7 +49,20 @@ class GradeCalculator
# if we're updating a grading period score, we also need to update the
# overall course score
@update_course_score = @grading_period.present? && opts[:update_course_score]
@assignments = opts[:assignments] || @course.assignments.published.gradeable.to_a
@ignore_unposted_anonymous = opts.fetch(
:ignore_unposted_anonymous,
@course.root_account.feature_enabled?(:grade_calc_ignore_unposted_anonymous)
)
@assignments = (opts[:assignments] || @course.assignments.published.gradeable).to_a
if @ignore_unposted_anonymous
Assignment.preload_unposted_anonymous_submissions(@assignments)
# Ignore anonymous assignments with unposted submissions in the grade calculation
# so that we don't break anonymity prior to the assignment being posted
# (which is when identities are revealed)
@assignments.reject!(&:unposted_anonymous_submissions?)
end
@user_ids = Array(user_ids).map { |id| Shard.relative_id_for(id, Shard.current, @course.shard) }
@current_updates = {}
@ -335,6 +348,7 @@ class GradeCalculator
assignments: @assignments,
emit_live_event: @emit_live_event,
ignore_muted: @ignore_muted,
ignore_unposted_anonymous: @ignore_unposted_anonymous,
periods: grading_periods_for_course,
effective_due_dates: effective_due_dates,
enrollments: enrollments,

View File

@ -485,6 +485,38 @@ describe GradeCalculator do
end
end
context "with unposted anonymous assignments" do
let(:calculator) { GradeCalculator.new([@user.id], @course.id, ignore_muted: false) }
let(:computed_score_data) { calculator.compute_scores.first }
before(:each) do
@anonymized_assignment = @course.assignments.create!(anonymous_grading: true)
@anonymized_assignment.grade_student(@user, grade: "10", grader: @teacher)
end
it "does not incorporate submissions for unposted anonymous assignments" do
expect(computed_score_data[:current][:grade]).to eq 75.0
end
it "incorporates submissions for posted anonymous assignments" do
@anonymized_assignment.post_submissions
expect(computed_score_data[:current][:grade]).to eq 125.0
end
context "when including unposted anonymous assignments in grade calculations" do
let(:calculator) { GradeCalculator.new([@user.id], @course.id, ignore_muted: false, ignore_unposted_anonymous: false) }
it "incorporates submissions for unposted anonymous assignments" do
expect(computed_score_data[:current][:grade]).to eq 125.0
end
it "incorporates submissions for posted anonymous assignments" do
@anonymized_assignment.post_submissions
expect(computed_score_data[:current][:grade]).to eq 125.0
end
end
end
describe "persisting score data" do
let(:calculator) { GradeCalculator.new([@user.id], @course.id, ignore_muted: true) }
let(:enrollment) { Enrollment.find_by!(user: @user, course: @course) }

View File

@ -44,8 +44,8 @@ describe Assignment do
it { is_expected.not_to validate_presence_of(:final_grader) }
it "should create a new instance given valid attributes" do
course = @course.assignments.create!(assignment_valid_attributes)
expect(course).to be_valid
assignment = @course.assignments.create!(assignment_valid_attributes)
expect(assignment).to be_valid
end
it "should set the lti_context_id on create" do
@ -1388,6 +1388,38 @@ describe Assignment do
end
end
describe ".preload_unposted_anonymous_submissions" do
it "preloads unposted anonymous submissions for an assignment" do
assignment = @course.assignments.create!(assignment_valid_attributes.merge(anonymous_grading: true))
expect(Assignment).to receive(:where).once.and_call_original
expect { Assignment.preload_unposted_anonymous_submissions([assignment]) }.to change {
assignment.unposted_anonymous_submissions
}.from(nil).to(true)
end
it "preloads if some assignments have the attribute preloaded but others do not" do
assignment = @course.assignments.create!(assignment_valid_attributes.merge(anonymous_grading: true))
other_assignment = @course.assignments.create!(assignment_valid_attributes.merge(anonymous_grading: true))
assignment.unposted_anonymous_submissions = true
expect(Assignment).to receive(:where).once.and_call_original
Assignment.preload_unposted_anonymous_submissions([assignment, other_assignment])
end
it "does not attempt to preload if all assignments already have the attribute preloaded" do
assignment = @course.assignments.create!(assignment_valid_attributes.merge(anonymous_grading: true))
other_assignment = @course.assignments.create!(assignment_valid_attributes.merge(anonymous_grading: true))
assignment.unposted_anonymous_submissions = true
other_assignment.unposted_anonymous_submissions = true
expect(Assignment).not_to receive(:where)
Assignment.preload_unposted_anonymous_submissions([assignment, other_assignment])
end
it "does not attempt to preload if given an empty array" do
expect(Assignment).not_to receive(:where)
Assignment.preload_unposted_anonymous_submissions([])
end
end
describe "scope: importing_for_too_long" do
subject { described_class.importing_for_too_long }

View File

@ -361,6 +361,7 @@ describe Lti::LtiOutboundAdapter do
url: tool.url
)
)
allow_any_instance_of(Account).to receive(:feature_enabled?).and_call_original
end
it 'builds the expected encrypted JWT with the correct course data' do