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

fix speedgrader anonymous moderated bug 

flag=none
closes EVAL-2131
closes EVAL-2332

Test Plan 1:
* Prerequisites: Published course with active teachers and students, and
  the anonymous grading feature option enabled.

1. Create an assignment. Enable moderated and anonymous grading.
2. Submit some content as a student.
3. Enter some grades as the graders.
4. De-activate the student that submitted.
5. Go to Gradebook and in the student header menu select to view
   'Inactive' students.
6. Go to SpeedGrader and verify the deactivated student and their
   submission shows up. Take note of the student's anonymous_id in
   the URL query params.
7. Go to the Moderate page for the assignment as the moderator, and
   verify the anonymous name for the student (i.e. "Student 1") matches
   their anonymous name in SpeedGrader. You can verify this by hovering
   over the student link on the Moderate page and seeing what their
   anonymous_id is, and comparing it to the anonymous_id you noted in
   step 6.

Test Plan 2: Legacy Support for Concluded Students
1. Switch to the 'master' branch.
2. Conclude a student that has only a single active enrollment (this
   student must not have other enrollments, not even soft-deleted ones).
3. Create an assignment that is due for everyone.
4. In a rails console, verify no submission was created for the
   concluded student:

   user = User.find(<concluded student id>)
   Assignment.find(<assignment id>).submissions.where(user: user).count
   => should be 0

5. Switch to the feature branch for this commit
6. As a teacher, go to Gradebook and in the student header choose to
   view 'Concluded' students.
7. Go to SpeedGrader for the assignment you made in step 3.
8. Verify the concluded student that does not have a submission does
   not show up in SpeedGrader.

Test Plan 3:
1. Conclude a student in a course. It's important that the student is
   concluded before any of the following steps. It's also important
   that the student only has a single enrollment in the course
   (including soft-deleted enrollments). You can make sure the student
   only has 1 enrollment in the course with:

   # should return 1
   Enrollment.where(course_id: <course id>, user_id: <student id>).count

2. Create an assignment.
3. In the gradebook, select the option to show concluded enrollments.
4. Launch the SpeedGrader for the assignment.
5. Enable the Hide student names in the SpeedGrader settings.
6. Refresh the page and verify SpeedGrader loads as expected. And verify
   you can navigate to the concluded student in SpeedGrader.

Change-Id: I265ee97953d1a9ebd098911ed4388b0e1da49af7
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/282581
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Dustin Cowles <dustin.cowles@instructure.com>
Product-Review: Jody Sailor
Reviewed-by: Dustin Cowles <dustin.cowles@instructure.com>
Reviewed-by: Kai Bjorkman <kbjorkman@instructure.com>
This commit is contained in:
Spencer Olson 2022-01-11 15:56:41 -06:00
parent 93c6b2a79e
commit 8b70802bee
6 changed files with 91 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
app/models/speed_grader/assignment.rb
View File

@ -73,10 +73,7 @@ module SpeedGrader
res[:context][:rep_for_student] = {}
# If we're working with anonymous IDs, skip students who don't have a
# valid submission object, which means no inactive or concluded students
# even if the user has elected to show them in gradebook
includes = assignment.anonymize_students? ? [] : gradebook_includes(user: current_user, course: course)
includes = gradebook_includes(user: current_user, course: course)
students = assignment.representatives(user: current_user, includes: includes, group_id: group_id_filter, section_id: section_id_filter) do |rep, others|
others.each { |s| res[:context][:rep_for_student][s.id] = rep.id }
end

15
lib/due_date_cacher.rb
View File

@ -177,14 +177,13 @@ class DueDateCacher
assignments_by_id = Assignment.find(@assignment_ids).index_by(&:id)
effective_due_dates.to_hash.each do |assignment_id, student_due_dates|
students_without_priors = student_due_dates.keys - enrollment_counts.prior_student_ids
existing_anonymous_ids = existing_anonymous_ids_by_assignment_id[assignment_id]
create_moderation_selections_for_assignment(assignments_by_id[assignment_id], student_due_dates.keys, @user_ids)
quiz_lti = quiz_lti_assignments.include?(assignment_id)
students_without_priors.each do |student_id|
student_due_dates.each_key do |student_id|
submission_info = student_due_dates[student_id]
due_date = submission_info[:due_at] ? "'#{submission_info[:due_at].iso8601}'::timestamptz" : "NULL"
grading_period_id = submission_info[:grading_period_id] || "NULL"
@ -284,7 +283,7 @@ class DueDateCacher
# The various workflow states below try to mimic similarly named scopes off of course
scope = Enrollment.select(
:user_id,
"count(nullif(workflow_state not in ('rejected', 'deleted', 'completed'), false)) as accepted_count",
"count(nullif(workflow_state not in ('rejected', 'deleted'), false)) as accepted_count",
"count(nullif(workflow_state in ('completed'), false)) as prior_count",
"count(nullif(workflow_state in ('rejected', 'deleted'), false)) as deleted_count"
)
@ -294,14 +293,12 @@ class DueDateCacher
scope = scope.where(user_id: @user_ids) if @user_ids.present?
scope.find_each do |record|
if record.accepted_count == 0 && record.deleted_count > 0
counts.deleted_student_ids << record.user_id
elsif record.accepted_count == 0 && record.prior_count > 0
counts.prior_student_ids << record.user_id
elsif record.accepted_count > 0
counts.prior_student_ids << record.user_id if record.prior_count > 0
if record.accepted_count > 0
counts.accepted_student_ids << record.user_id
else
raise "Unknown enrollment state: #{record.accepted_count}, #{record.prior_count}, #{record.deleted_count}"
counts.deleted_student_ids << record.user_id
end
end
end

7
spec/javascripts/jsx/speed_graderSpec.js
View File

@ -2362,6 +2362,13 @@ QUnit.module('SpeedGrader', rootHooks => {
]
})
test('filters out students that do not have submissions', () => {
window.jsonData.submissions.shift()
SpeedGrader.EG.jsonReady()
const ids = window.jsonData.studentsWithSubmissions.map(student => student.id)
deepEqual(ids, ['1102', '1103', '1104'])
})
test('preserves student order (from server) when sorting alphabetically', () => {
SpeedGrader.EG.jsonReady()
const ids = window.jsonData.studentsWithSubmissions.map(student => student.id)

34
spec/lib/due_date_cacher_spec.rb
View File

@ -612,17 +612,6 @@ describe DueDateCacher do
cacher.recompute
expect(submission.reload.cached_due_date).to be_nil
end
it "does not update submissions for students with concluded enrollments" do
student2 = user_factory
@course.enroll_student(student2, enrollment_state: "active")
submission2 = submission_model(assignment: @assignment, user: student2)
submission2.update(cached_due_date: nil)
student2.enrollments.find_by(course: @course).conclude
DueDateCacher.new(@course, [@assignment]).recompute
expect(submission2.reload.cached_due_date).to be nil
end
end
context "one applicable override" do
@ -667,17 +656,6 @@ describe DueDateCacher do
cacher.recompute
expect(submission.reload.cached_due_date).to eq @assignment.due_at.change(usec: 0)
end
it "does not update submissions for students with concluded enrollments" do
student2 = user_factory
@course.enroll_student(student2, enrollment_state: "active")
submission2 = submission_model(assignment: @assignment, user: student2)
submission2.update(cached_due_date: nil)
student2.enrollments.find_by(course: @course).conclude
DueDateCacher.new(@course, [@assignment]).recompute
expect(submission2.reload.cached_due_date).to be nil
end
end
context "adhoc override" do
@ -706,12 +684,6 @@ describe DueDateCacher do
cacher.recompute
expect(@submission1.reload.cached_due_date).to eq @assignment.due_at.change(usec: 0)
end
it "does not update submissions for students with concluded enrollments" do
@student2.enrollments.find_by(course: @course).conclude
DueDateCacher.new(@course, [@assignment]).recompute
expect(@submission2.reload.cached_due_date).to be nil
end
end
context "section override" do
@ -794,12 +766,6 @@ describe DueDateCacher do
@group.add_user(@student1, "deleted")
expect(@submission1.reload.cached_due_date).to eq @assignment.due_at.change(usec: 0)
end
it "does not update submissions for students with concluded enrollments" do
@student2.enrollments.find_by(course: @course).conclude
DueDateCacher.new(@course, [@assignment]).recompute
expect(@submission2.reload.cached_due_date).to be nil
end
end
context "multiple overrides" do

67
spec/models/speed_grader/assignment_spec.rb
View File

@ -1507,31 +1507,68 @@ describe SpeedGrader::Assignment do
course_with_teacher
@active_student = @course.enroll_student(User.create!, enrollment_state: "active").user
@course.enroll_student(User.create!, enrollment_state: "inactive")
@course.enroll_student(User.create!, enrollment_state: "completed")
@course.enroll_student(User.create!, enrollment_state: "completed")
@inactive_student = User.create!
@course.enroll_student(@inactive_student, enrollment_state: "inactive")
@concluded_student = User.create!
@course.enroll_student(@concluded_student, enrollment_state: "completed")
@concluded_student2 = User.create!
@course.enroll_student(@concluded_student2, enrollment_state: "completed")
end
let(:assignment) { @course.assignments.create!(title: "anonymous", anonymous_grading: true) }
let(:speed_grader_json) { SpeedGrader::Assignment.new(assignment, @teacher).json }
let(:students) { speed_grader_json[:context][:students] }
let(:returned_ids) { students.map { |student| student["anonymous_id"] } }
it "returns only active students if assignment is muted" do
active_student_submission = assignment.submission_for_student(@active_student)
context "unposted assignments" do
it "returns only active students if the teacher is not viewing inactive or concluded" do
active_student_submission = assignment.submission_for_student(@active_student)
expect(returned_ids).to match_array [active_student_submission.anonymous_id]
end
returned_ids = students.map { |student| student["anonymous_id"] }
expect(returned_ids).to match_array [active_student_submission.anonymous_id]
it "returns active and inactive if the teacher is viewing inactive" do
@teacher.preferences[:gradebook_settings] = {
@course.global_id => { "show_inactive_enrollments" => "true" }
}
expected_ids = assignment.submissions.where(user: [@active_student, @inactive_student]).pluck(:anonymous_id)
expect(returned_ids).to match_array expected_ids
end
it "returns active and concluded if the teacher is viewing concluded" do
@teacher.preferences[:gradebook_settings] = {
@course.global_id => { "show_concluded_enrollments" => "true" }
}
expected_ids = assignment.submissions.where(
user: [@active_student, @concluded_student, @concluded_student2]
).pluck(:anonymous_id)
expect(returned_ids).to match_array expected_ids
end
it "returns all students if teacher is viewing inactive and concluded" do
@teacher.preferences[:gradebook_settings] = {
@course.global_id => {
"show_concluded_enrollments" => "true",
"show_inactive_enrollments" => "true"
}
}
expected_ids = assignment.submissions.where(
user: [@active_student, @inactive_student, @concluded_student, @concluded_student2]
).pluck(:anonymous_id)
expect(returned_ids).to match_array expected_ids
end
end
it "returns students in accord with user gradebook preferences if assignment is not muted" do
@teacher.preferences[:gradebook_settings] = {}
@teacher.preferences[:gradebook_settings][@course.global_id] = {
"show_concluded_enrollments" => "true",
"show_inactive_enrollments" => "true"
}
assignment.unmute!
context "posted assignments" do
it "returns students in accord with user gradebook preferences if assignment is not muted" do
@teacher.preferences[:gradebook_settings] = {}
@teacher.preferences[:gradebook_settings][@course.global_id] = {
"show_concluded_enrollments" => "true",
"show_inactive_enrollments" => "true"
}
assignment.unmute!
expect(students.length).to eq 4
expect(students.length).to eq 4
end
end
end

37
ui/features/speed_grader/jquery/speed_grader.tsx
View File

@ -354,20 +354,33 @@ function mergeStudentsAndSubmission() {
window.jsonData.submissionsMap[submission[anonymizableUserId]] = submission
})
jsonData.studentsWithSubmissions = jsonData.studentsWithSubmissions.reduce(
(students, student, index) => {
const submission = jsonData.submissionsMap[student[anonymizableId]]
// Hide students that don't have a submission object. This is legacy support
// for when we used to not create submission objects for assigned concluded students.
// For all new assignments, every assigned student (regardless of concluded/inactive
// status) should have a submission object.
if (submission) {
student.enrollments = jsonData.studentEnrollmentMap[student[anonymizableId]]
student.section_ids = Object.keys(jsonData.studentSectionIdsMap[student[anonymizableId]])
student.submission = submission
student.submission_state = SpeedgraderHelpers.submissionState(student, ENV.grading_role)
student.index = index
students.push(student)
}
return students
},
[]
)
// need to presort by anonymous_id for anonymous assignments so that the index property can be consistent
if (isAnonymous)
window.jsonData.studentsWithSubmissions.sort((a, b) =>
a.anonymous_name_position > b.anonymous_name_position ? 1 : -1
)
window.jsonData.studentsWithSubmissions.forEach((student, index) => {
student.enrollments = window.jsonData.studentEnrollmentMap[student[anonymizableId]]
student.section_ids = Object.keys(window.jsonData.studentSectionIdsMap[student[anonymizableId]])
student.submission = window.jsonData.submissionsMap[student[anonymizableId]]
student.submission_state = SpeedgraderHelpers.submissionState(student, ENV.grading_role)
student.index = index
})
// handle showing students only in a certain section.
if (!window.jsonData.GROUP_GRADING_MODE) {
sectionToShow = ENV.selected_section_id
@ -2602,18 +2615,18 @@ EG = {
$grded_so_far.text(
I18n.t('portion_graded', '%{x}/%{y}', {
x: I18n.n(gradedStudents.length),
y: I18n.n(window.jsonData.context.students.length)
y: I18n.n(window.jsonData.studentsWithSubmissions.length)
})
)
},
totalStudentCount() {
if (sectionToShow) {
return _.filter(window.jsonData.context.students, student =>
return _.filter(jsonData.studentsWithSubmissions, student =>
_.includes(student.section_ids, sectionToShow)
).length
} else {
return window.jsonData.context.students.length
return jsonData.studentsWithSubmissions.length
}
},
@ -3661,7 +3674,7 @@ EG = {
snapshot =>
snapshot &&
$.map(
window.jsonData.context.students,
jsonData.studentsWithSubmissions,
student => snapshot === student && student.name
)[0]
)