Restrict anonymize_students to active enrollments

With post policies, ignore submissions belonging to concluded or
inactive enrollments when considering whether an anonymously graded
assignment should anonymize students, rather than simply checking the
assignment's muted attribute (which does not take students' enrollment
states into consideration).

fixes TALLY-151
flag=none

Test plan:
- Have a course with New Gradebook and Post Policies enabled
- Enroll at least three students (S1, S2, S3)
- Set up an anonymous assignment (A1)
- Conclude S2's enrollment in the course
- Deactivate S3's enrollment in the course
- In Gradebook:
  - Check that A1 appears as anonymous and grades are concealed
  - Post grades for A1
  - Reload the page
  - Check that grades for A1 are unconcealed after reload, and that
    "Hide grades" is now possible but posting grades isn't
- Repeat the above in SpeedGrader

Change-Id: I4ae69c088376fe6ba1acc5e8c04dc5abd1706c82
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/222155
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Spencer Olson <solson@instructure.com>
Reviewed-by: Jeremy Neander <jneander@instructure.com>
QA-Review: Robin Kuss <rkuss@instructure.com>
Product-Review: Jonathan Fenton <jfenton@instructure.com>
This commit is contained in:
Adrian Packel 2020-01-06 17:16:15 -06:00
parent fcd5a4fee8
commit 953aeb2b7d
3 changed files with 92 additions and 19 deletions

View File

@ -57,7 +57,7 @@ class Assignment < ActiveRecord::Base
anonymous_instructor_annotations
].freeze
attr_accessor :previous_id, :copying, :user_submitted, :grade_posting_in_progress
attr_accessor :previous_id, :copying, :user_submitted, :grade_posting_in_progress, :unposted_anonymous_submissions
attr_reader :assignment_changed, :posting_params_for_notifications
attr_writer :updating_user
@ -1430,6 +1430,23 @@ class Assignment < ActiveRecord::Base
])
end
def self.preload_unposted_anonymous_submissions(assignments)
assignment_ids_with_unposted_anonymous_submissions = Assignment.
where(id: assignments, anonymous_grading: true).
where(
"EXISTS (?)", Submission.active.unposted.joins(user: :enrollments).
where("submissions.user_id = users.id").
where("submissions.assignment_id = assignments.id").
where("enrollments.course_id = assignments.context_id").
where(Enrollment.active_student_conditions)
).
pluck(:id).to_set
assignments.each do |assignment|
assignment.unposted_anonymous_submissions = assignment_ids_with_unposted_anonymous_submissions.include?(assignment.id)
end
end
def touch_on_unlock_if_necessary
if self.unlock_at && Time.zone.now < self.unlock_at && (Time.zone.now + 1.hour) > self.unlock_at
Shackles.activate(:master) do
@ -3166,15 +3183,28 @@ class Assignment < ActiveRecord::Base
end
end
# If you're going to be checking this for multiple assignments, you may want
# to call .preload_unposted_anonymous_submissions on the lot of them first
def anonymize_students?
return false unless anonymous_grading?
# Only anonymize students for moderated assignments if grades have not been published.
# Only anonymize students for non-moderated assignments if the assignment is muted.
moderated_grading? ? !grades_published? : muted?
return !grades_published? if moderated_grading?
# If Post Policies isn't enabled, we can just check whether the assignment is muted.
return muted? unless course.post_policies_enabled?
# Otherwise, only anonymize students if there's at least one active student with
# an unposted submission.
unposted_anonymous_submissions?
end
alias anonymize_students anonymize_students?
def unposted_anonymous_submissions?
Assignment.preload_unposted_anonymous_submissions([self]) unless defined? @unposted_anonymous_submissions
@unposted_anonymous_submissions
end
def can_view_student_names?(user)
return false if anonymize_students?

View File

@ -43,6 +43,7 @@ module Api::V1::AssignmentGroup
# Preload assignments' post policies for Assignment#assignment_json.
if assignments.present? && assignments.first.context.post_policies_enabled?
ActiveRecord::Associations::Preloader.new.preload(assignments, :post_policy)
Assignment.preload_unposted_anonymous_submissions(assignments)
end
user_content_attachments = opts[:preloaded_user_content_attachments]

View File

@ -872,7 +872,7 @@ describe Assignment do
describe '#anonymize_students?' do
before(:once) do
@assignment = @course.assignments.build
@assignment = @course.assignments.create!
end
it 'returns false when the assignment is not graded anonymously' do
@ -881,27 +881,69 @@ describe Assignment do
context 'when the assignment is anonymously graded' do
before(:once) do
@assignment.anonymous_grading = true
@assignment.update!(anonymous_grading: true)
end
it 'returns true when the assignment is muted' do
@assignment.muted = true
expect(@assignment).to be_anonymize_students
context "when the assignment is moderated" do
before(:each) do
@assignment.moderated_grading = true
end
it 'returns true when the assignment is moderated and grades are unpublished' do
expect(@assignment).to be_anonymize_students
end
it 'returns false when the assignment is moderated and grades are published' do
@assignment.grades_published_at = Time.zone.now
expect(@assignment).not_to be_anonymize_students
end
end
it 'returns false when the assignment is unmuted' do
expect(@assignment).not_to be_anonymize_students
end
context "when the assignment is unmoderated" do
context "when Post Policies is not enabled" do
it 'returns true when the assignment is muted' do
@assignment.mute!
expect(@assignment).to be_anonymize_students
end
it 'returns true when the assignment is moderated and grades are unpublished' do
@assignment.moderated_grading = true
expect(@assignment).to be_anonymize_students
end
it 'returns false when the assignment is unmuted' do
@assignment.unmute!
expect(@assignment).not_to be_anonymize_students
end
end
it 'returns false when the assignment is moderated and grades are published' do
@assignment.moderated_grading = true
@assignment.grades_published_at = Time.zone.now
expect(@assignment).not_to be_anonymize_students
context "when Post Policies is enabled" do
let(:course) { @assignment.course }
let(:active_student) { User.create! }
let(:student2) { User.create! }
let(:student3) { User.create! }
before(:each) do
course.enable_feature!(:new_gradebook)
PostPolicy.enable_feature!
course.enroll_student(active_student, workflow_state: :active)
course.enroll_student(student2, workflow_state: :active)
course.enroll_student(student3, workflow_state: :active)
end
it "returns true when at least one active student has an unposted submission" do
expect(@assignment).to be_anonymize_students
end
it "returns false when all active submissions are posted" do
student2.enrollments.find_by(course: course).conclude
student3.enrollments.find_by(course: course).deactivate
@assignment.post_submissions
expect(@assignment).not_to be_anonymize_students
end
it "returns false when all submissions are posted" do
@assignment.post_submissions
expect(@assignment).not_to be_anonymize_students
end
end
end
end
end