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

add assignment group score sorting in gradebook_user_ids 

Add the ability to server side sort gradebook users by assignment
group scores. Also unlocks the UI in the new gradebook to allow the
sorting.

closes GRADE-7

test plan:
 - Have a course with multiple students
 - Have an assignment in the default "assignments" assignment group
 - Have an assignment in a second assignment group
 - In the gradebook, grade the students
 - Sort the gradebook by the default assignment group, score low to
   high
 - Reload the gradebook
 - Ensure that the default assignment group sort is persisted across
   the reload
 - Sort the gradebook by the second assignment group, score high to
   low
 - Reload the gradebook
 - Ensure that the second assignment group sort is persisted across
   the reload

Change-Id: Ia3647d43d8460124e413d83d5c6013dbebd8c33e
Reviewed-on: https://gerrit.instructure.com/136981
Reviewed-by: Spencer Olson <solson@instructure.com>
Tested-by: Jenkins
Reviewed-by: Jeremy Neander <jneander@instructure.com>
QA-Review: Anju Reddy <areddy@instructure.com>
Product-Review: Keith T. Garner <kgarner@instructure.com>
This commit is contained in:
Keith Garner 2018-01-04 17:18:35 -06:00 committed by Keith T. Garner
parent 13c2d0b7d2
commit 36e29e4b42
4 changed files with 127 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
app/coffeescripts/gradezilla/Gradebook.coffee
View File

@ -1728,14 +1728,6 @@ define [
sort_rows_by_direction: sortRowsBy.direction
colors: colors
# TODO: include the "sort rows by" setting for Assignment Groups
# when fully supported by the Gradebook `user_ids` endpoint.
sortingByIncompleteSortFeature = data.gradebook_settings.sort_rows_by_column_id.match(/^assignment_group_/)
if sortingByIncompleteSortFeature
delete data.gradebook_settings.sort_rows_by_column_id
delete data.gradebook_settings.sort_rows_by_setting_key
delete data.gradebook_settings.sort_rows_by_direction
$.ajaxJSON(@options.settings_update_url, 'PUT', data, successFn, errorFn)
## Grid Sorting Methods

70
lib/gradebook_user_ids.rb
View File

@ -31,9 +31,12 @@ class GradebookUserIds
def user_ids
if @column == "student"
sort_by_student_name
elsif @column&.include?("assignment")
elsif @column =~ /assignment_\d+$/
assignment_id = @column[/\d+$/]
send("sort_by_assignment_#{@sort_by}", assignment_id)
elsif @column =~ /^assignment_group_\d+$/
assignment_id = @column[/\d+$/]
sort_by_assignment_group(assignment_id)
elsif @column == "total_grade"
sort_by_total_grade
else
@ -54,7 +57,8 @@ class GradebookUserIds
def sort_by_assignment_grade(assignment_id)
students.
joins("LEFT JOIN #{Submission.quoted_table_name} ON submissions.user_id=users.id AND
submissions.workflow_state<>'deleted' AND submissions.assignment_id=#{assignment_id}").
submissions.workflow_state<>'deleted' AND
submissions.assignment_id=#{Submission.sanitize(assignment_id)}").
order("#{Enrollment.table_name}.type = 'StudentViewEnrollment'").
order("#{Submission.table_name}.score #{sort_direction} NULLS LAST").
order("#{Submission.table_name}.id IS NULL").
@ -72,31 +76,11 @@ class GradebookUserIds
end
def sort_by_total_grade
score_scope = grading_period_id ? "scores.grading_period_id=#{grading_period_id}" : "scores.course_score IS TRUE"
grading_period_id ? sort_by_scores(:grading_period, grading_period_id) : sort_by_scores(:total_grade)
end
# In this query we need to jump through enrollments to go see if
# there are scores for the user. Because of some AR internal
# stuff, if we did students.joins("LEFT JOIN scores ON
# enrollments.id=scores.enrollment_id AND ...") as you'd expect,
# it plops the new join before the enrollments join and the
# database gets angry because it doesn't know what what this
# "enrollments" we're querying against is. Because of this, we
# have to WET up the method and hand do the enrollment join
# here. Without doing the score conditions in the join, we lose
# data, so it has to be this way... For example, we might lose
# concluded enrollments who don't have a Score.
#
# That is a super long way of saying, make sure this stays in sync
# with what happens in the students method below in reguards to
# enrollments and enrollments scoping.
User.joins("LEFT JOIN #{Enrollment.quoted_table_name} on users.id=enrollments.user_id
LEFT JOIN #{Score.quoted_table_name} ON scores.enrollment_id=enrollments.id AND
scores.workflow_state='active' AND #{score_scope}").
merge(student_enrollments_scope).
order("#{Enrollment.table_name}.type = 'StudentViewEnrollment'").
order("#{Score.table_name}.unposted_current_score #{sort_direction} NULLS LAST").
order_by_sortable_name(direction: @direction.to_sym).
pluck(:id).uniq
def sort_by_assignment_group(assignment_group_id)
sort_by_scores(:assignment_group, assignment_group_id)
end
def all_user_ids
@ -159,6 +143,40 @@ class GradebookUserIds
User.left_joins(:enrollments).merge(student_enrollments_scope)
end
def sort_by_scores(type = :total_grade, id = nil)
score_scope = if type == :assignment_group
"scores.assignment_group_id=#{Score.sanitize(id)}"
elsif type == :grading_period
"scores.grading_period_id=#{Score.sanitize(id)}"
else
"scores.course_score IS TRUE"
end
# In this query we need to jump through enrollments to go see if
# there are scores for the user. Because of some AR internal
# stuff, if we did students.joins("LEFT JOIN scores ON
# enrollments.id=scores.enrollment_id AND ...") as you'd expect,
# it plops the new join before the enrollments join and the
# database gets angry because it doesn't know what what this
# "enrollments" we're querying against is. Because of this, we
# have to WET up the method and hand do the enrollment join
# here. Without doing the score conditions in the join, we lose
# data, so it has to be this way... For example, we might lose
# concluded enrollments who don't have a Score.
#
# That is a super long way of saying, make sure this stays in sync
# with what happens in the students method above in regards to
# enrollments and enrollments scoping.
User.joins("LEFT JOIN #{Enrollment.quoted_table_name} on users.id=enrollments.user_id
LEFT JOIN #{Score.quoted_table_name} ON scores.enrollment_id=enrollments.id AND
scores.workflow_state='active' AND #{score_scope}").
merge(student_enrollments_scope).
order("#{Enrollment.table_name}.type = 'StudentViewEnrollment'").
order("#{Score.table_name}.unposted_current_score #{sort_direction} NULLS LAST").
order_by_sortable_name(direction: @direction.to_sym).
pluck(:id).uniq
end
def sort_direction
@direction == "ascending" ? :asc : :desc
end

12
spec/javascripts/jsx/gradezilla/GradebookSpec.js
View File

@ -5533,18 +5533,6 @@ test('calls errorFn when response is not successful', function () {
strictEqual(errorFn.callCount, 1);
});
test('excludes "sort rows by" settings when sorting by an assignment group', function () {
// This is temporary until the Gradebook `user_ids` supports this sorting.
const ajaxJSONStub = this.stub($, 'ajaxJSON');
const gradebook = createGradebook();
gradebook.setSortRowsBySetting('assignment_group_2301', 'grade', 'ascending');
gradebook.saveSettings();
const requestData = ajaxJSONStub.firstCall.args[2];
notOk('sort_rows_by_column_id' in requestData.gradebook_settings, 'request excludes "sort_rows_by_column_id"');
notOk('sort_rows_by_setting_key' in requestData.gradebook_settings, 'request excludes "sort_rows_by_setting_key"');
notOk('sort_rows_by_direction' in requestData.gradebook_settings, 'request excludes "sort_rows_by_direction"');
});
QUnit.module('Gradebook#updateColumnsAndRenderViewOptionsMenu', function (hooks) {
let gradebook;

110
spec/lib/gradebook_user_ids_spec.rb
View File

@ -603,13 +603,13 @@ describe GradebookUserIds do
end
end
describe "total grade sorting" do
describe "score sorting" do
before(:once) do
@now = Time.zone.now
@assignment1 = @course.assignments.create!(points_possible: 10, due_at: 1.month.from_now(@now))
second_assignment_group = @course.assignment_groups.create(name: 'second group')
@second_assignment_group = @course.assignment_groups.create(name: 'second group')
@assignment2 = @course.assignments.create!(
points_possible: 100, due_at: 3.months.from_now(@now), assignment_group: second_assignment_group
points_possible: 100, due_at: 3.months.from_now(@now), assignment_group: @second_assignment_group
)
@assignment1.grade_student(@student1, grade: 1, grader: @teacher)
@ -631,21 +631,49 @@ describe GradebookUserIds do
skip 'requires pg_collkey installed SD-2747' unless has_pg_collkey
end
it "sorts by total grade ascending" do
expected_user_ids = [@student1.id, @student2.id, @student4.id, @student3.id, @fake_student.id]
expect(gradebook_user_ids.user_ids).to eq(expected_user_ids)
context "with total grade" do
it "sorts by total grade ascending" do
expected_user_ids = [@student1.id, @student2.id, @student4.id, @student3.id, @fake_student.id]
expect(gradebook_user_ids.user_ids).to eq(expected_user_ids)
end
it "sorts by total grade descending" do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_direction] = "descending"
expected_user_ids = [@student3.id, @student4.id, @student2.id, @student1.id, @fake_student.id]
expect(gradebook_user_ids.user_ids).to eq(expected_user_ids)
end
it "includes concluded students ids if the course is concluded" do
@course.complete!
all_students = [@student1.id, @student2.id, @student4.id, @student3.id,
@concluded_student.id, @fake_student.id]
expect(gradebook_user_ids.user_ids).to eq(all_students)
end
end
it "sorts by total grade descending" do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_direction] = "descending"
expected_user_ids = [@student3.id, @student4.id, @student2.id, @student1.id, @fake_student.id]
expect(gradebook_user_ids.user_ids).to eq(expected_user_ids)
end
context "with assignment group" do
before(:once) do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_column_id] =
"assignment_group_#{@second_assignment_group.id}"
end
it "includes concluded students ids if the course is concluded" do
@course.complete!
all_students = [@student1.id, @student2.id, @student4.id, @student3.id, @concluded_student.id, @fake_student.id]
expect(gradebook_user_ids.user_ids).to eq(all_students)
it "sorts by assignment group grade ascending" do
expected_user_ids = [@student2.id, @student1.id, @student4.id, @student3.id, @fake_student.id]
expect(gradebook_user_ids.user_ids).to eq(expected_user_ids)
end
it "sorts by assignment group grade descending" do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_direction] = "descending"
expected_user_ids = [@student3.id, @student4.id, @student1.id, @student2.id, @fake_student.id]
expect(gradebook_user_ids.user_ids).to eq(expected_user_ids)
end
it "includes concluded students ids if the course is concluded" do
@course.complete!
all_students = [@student2.id, @student1.id, @student4.id, @student3.id,
@concluded_student.id, @fake_student.id]
expect(gradebook_user_ids.user_ids).to eq(all_students)
end
end
end
@ -654,21 +682,49 @@ describe GradebookUserIds do
skip 'requires no pg_collkey installed SD-2747' if has_pg_collkey
end
it "sorts by total grade ascending" do
expected_user_ids = [@student1.id, @student2.id, @student3.id, @student4.id, @fake_student.id]
expect(gradebook_user_ids.user_ids).to eq(expected_user_ids)
context "with total grade" do
it "sorts by total grade ascending" do
expected_user_ids = [@student1.id, @student2.id, @student3.id, @student4.id, @fake_student.id]
expect(gradebook_user_ids.user_ids).to eq(expected_user_ids)
end
it "sorts by total grade descending" do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_direction] = "descending"
expected_user_ids = [@student4.id, @student3.id, @student2.id, @student1.id, @fake_student.id]
expect(gradebook_user_ids.user_ids).to eq(expected_user_ids)
end
it "includes concluded students ids if the course is concluded" do
@course.complete!
all_students = [@student1.id, @student2.id, @student3.id, @student4.id,
@concluded_student.id, @fake_student.id]
expect(gradebook_user_ids.user_ids).to eq(all_students)
end
end
it "sorts by total grade descending" do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_direction] = "descending"
expected_user_ids = [@student4.id, @student3.id, @student2.id, @student1.id, @fake_student.id]
expect(gradebook_user_ids.user_ids).to eq(expected_user_ids)
end
context "with assignment groups" do
before(:once) do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_column_id] =
"assignment_group_#{@second_assignment_group.id}"
end
it "includes concluded students ids if the course is concluded" do
@course.complete!
all_students = [@student1.id, @student2.id, @student3.id, @student4.id, @concluded_student.id, @fake_student.id]
expect(gradebook_user_ids.user_ids).to eq(all_students)
it "sorts by assignment group grade ascending" do
expected_user_ids = [@student2.id, @student1.id, @student4.id, @student3.id, @fake_student.id]
expect(gradebook_user_ids.user_ids).to eq(expected_user_ids)
end
it "sorts by assignment group grade descending" do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_direction] = "descending"
expected_user_ids = [@student3.id, @student4.id, @student1.id, @student2.id, @fake_student.id]
expect(gradebook_user_ids.user_ids).to eq(expected_user_ids)
end
it "includes concluded students ids if the course is concluded" do
@course.complete!
all_students = [@student2.id, @student1.id, @student4.id, @student3.id,
@concluded_student.id, @fake_student.id]
expect(gradebook_user_ids.user_ids).to eq(all_students)
end
end
end