multi-select student groups

closes EVAL-3827
flag=multiselect_gradebook_filters

Test Plan:
1. Enable the "Multi-select Gradebook Filters" SiteAdmin feature flag.
2. In a course with students in multiple groups, go to the Gradebook.
3. Verify you can select multiple groups when filtering by group, and
   verify it filters students as expected.
4. Verify when all student group filters are removed that all students
   are shown.
5. Apply multiple group filters and then refresh the page. Verify after
   refresh that the filters are still active.
6. Disable the "Multi-select Gradebook Filters" SiteAdmin feature flag.
   Verify you can no longer select multiple groups when filtering by group,
   and verify it filters students as expected.

Change-Id: Icd18e699be5a17eed3ee22590b04414a2081b79e
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/340295
Reviewed-by: Cameron Ray <cameron.ray@instructure.com>
Reviewed-by: Kai Bjorkman <kbjorkman@instructure.com>
QA-Review: Kai Bjorkman <kbjorkman@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Product-Review: Ravi Koll <ravi.koll@instructure.com>
This commit is contained in:
Spencer Olson 2024-02-12 13:13:18 -06:00 committed by Rohan Chugh
parent 670cda40fe
commit 309fdca284
14 changed files with 273 additions and 101 deletions

View File

@ -48,7 +48,8 @@ class GradebookSettingsController < ApplicationController
],
filter_rows_by: [
:section_id,
:student_group_id
:student_group_id,
{ student_group_ids: [] }
],
selected_view_options_filters: []
},

View File

@ -299,6 +299,29 @@
],
"note": ""
},
{
"warning_type": "Mass Assignment",
"warning_code": 70,
"fingerprint": "32b64d41a0918e3360b4d3396a3031784769aed85f82338ab2c4f409b28cc9f6",
"check_name": "MassAssignment",
"message": "Specify exact keys allowed for mass assignment instead of using `permit!` which allows any keys",
"file": "app/controllers/gradebook_settings_controller.rb",
"line": 72,
"link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/",
"code": "params.require(:gradebook_settings).permit({ :filter_columns_by => ([:context_module_id, :grading_period_id, :assignment_group_id, :submissions, :start_date, :end_date]), :filter_rows_by => ([:section_id, :student_group_id, { :student_group_ids => ([]) }]), :selected_view_options_filters => ([]) }, :enter_grades_as, :hide_assignment_group_totals, :hide_total, :show_concluded_enrollments, :show_inactive_enrollments, :show_unpublished_assignments, :show_separate_first_last_names, :student_column_display_as, :student_column_secondary_info, :sort_rows_by_column_id, :sort_rows_by_setting_key, :sort_rows_by_direction, :view_ungraded_as_zero, :colors => ([:late, :missing, :resubmitted, :dropped, :excused, :extended])).permit!",
"render_path": null,
"location": {
"type": "method",
"class": "GradebookSettingsController",
"method": "gradebook_settings_params"
},
"user_input": null,
"confidence": "Medium",
"cwe_id": [
915
],
"note": ""
},
{
"warning_type": "SQL Injection",
"warning_code": 0,
@ -1650,29 +1673,6 @@
185
],
"note": ""
},
{
"warning_type": "Mass Assignment",
"warning_code": 70,
"fingerprint": "fe51dc8ef7eaef41744c62633219ae140f5a2df7c95adc6f7ef126e84860b034",
"check_name": "MassAssignment",
"message": "Specify exact keys allowed for mass assignment instead of using `permit!` which allows any keys",
"file": "app/controllers/gradebook_settings_controller.rb",
"line": 71,
"link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/",
"code": "params.require(:gradebook_settings).permit({ :filter_columns_by => ([:context_module_id, :grading_period_id, :assignment_group_id, :submissions, :start_date, :end_date]), :filter_rows_by => ([:section_id, :student_group_id]), :selected_view_options_filters => ([]) }, :enter_grades_as, :hide_assignment_group_totals, :hide_total, :show_concluded_enrollments, :show_inactive_enrollments, :show_unpublished_assignments, :show_separate_first_last_names, :student_column_display_as, :student_column_secondary_info, :sort_rows_by_column_id, :sort_rows_by_setting_key, :sort_rows_by_direction, :view_ungraded_as_zero, :colors => ([:late, :missing, :resubmitted, :dropped, :excused, :extended])).permit!",
"render_path": null,
"location": {
"type": "method",
"class": "GradebookSettingsController",
"method": "gradebook_settings_params"
},
"user_input": null,
"confidence": "Medium",
"cwe_id": [
915
],
"note": ""
}
],
"updated": "2024-02-15 19:40:42 +0000",

View File

@ -28,6 +28,7 @@ class GradebookUserIds
@sort_by = settings[:sort_rows_by_setting_key] || "name"
@selected_grading_period_id = settings.dig(:filter_columns_by, :grading_period_id)
@selected_section_id = settings.dig(:filter_rows_by, :section_id)
@selected_student_group_ids = settings.dig(:filter_rows_by, :student_group_ids)
@selected_student_group_id = settings.dig(:filter_rows_by, :student_group_id)
@direction = settings[:sort_rows_by_direction] || "ascending"
end
@ -210,14 +211,22 @@ class GradebookUserIds
.joins("LEFT JOIN #{Enrollment.quoted_table_name} ON enrollments.user_id=users.id")
.merge(student_enrollments_scope)
if student_group_id.present?
students.joins(group_memberships: :group)
.where(group_memberships: { group: student_group_id, workflow_state: :accepted })
multiselect_filters_enabled = Account.site_admin.feature_enabled?(:multiselect_gradebook_filters)
if multiselect_filters_enabled && student_group_ids.present?
students_in_groups(students, student_group_ids)
elsif !multiselect_filters_enabled && student_group_id.present?
students_in_groups(students, student_group_id)
else
students
end
end
def students_in_groups(students, group_id_or_group_ids)
students.joins(group_memberships: :group)
.where(group_memberships: { group: group_id_or_group_ids, workflow_state: :accepted })
.merge(Group.active)
end
def sort_by_scores(type = :total_grade, id = nil)
score_scope = case type
when :assignment_group
@ -262,6 +271,18 @@ class GradebookUserIds
def student_group_id
return nil if @selected_student_group_id.nil? || ["0", "null"].include?(@selected_student_group_id)
Group.active.where(id: @selected_student_group_id).exists? ? @selected_student_group_id : nil
active_groups_exist?(@selected_student_group_id) ? @selected_student_group_id : nil
end
def student_group_ids
@student_group_ids ||= if @selected_student_group_ids.blank? || !active_groups_exist?(@selected_student_group_ids)
[]
else
@selected_student_group_ids
end
end
def active_groups_exist?(id_or_ids)
Group.active.where(id: id_or_ids).exists?
end
end

View File

@ -24,7 +24,7 @@ describe GradebookUserIds do
@course = Course.create!
@teacher = teacher_in_course(course: @course, active_all: true).user
@teacher.preferences[:gradebook_settings] = {}
@teacher.preferences[:gradebook_settings][@course.id] = {
@teacher.preferences[:gradebook_settings][@course.global_id] = {
show_inactive_enrollments: "false",
show_concluded_enrollments: "false",
sort_rows_by_column_id: "student",
@ -168,7 +168,7 @@ describe GradebookUserIds do
@student2.enrollments.find_by(course_section: viewable_section).conclude
@teacher.preferences[:gradebook_settings] = {
@course.id => {
@course.global_id => {
show_inactive_enrollments: "true",
show_concluded_enrollments: "true"
}
@ -180,18 +180,20 @@ describe GradebookUserIds do
describe "filtering by student group" do
let_once(:category) do
category = @course.group_categories.create!(name: "whatever")
category.create_groups(2)
category.create_groups(3)
category.groups.first.add_user(@student1)
category.groups.second.add_user(@student2)
category.groups.third.add_user(@student3)
category
end
let_once(:group) { category.groups.first }
let_once(:group2) { category.groups.second }
context "when a group is specified" do
before(:once) do
@teacher.preferences[:gradebook_settings] = {
@course.id => {
@course.global_id => {
filter_rows_by: {
student_group_id: group.id
}
@ -200,7 +202,7 @@ describe GradebookUserIds do
end
it "does not blow up when sorting by first name" do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_column_id] = "student_firstname"
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_column_id] = "student_firstname"
expect { gradebook_user_ids.user_ids }.not_to raise_error
end
@ -209,7 +211,7 @@ describe GradebookUserIds do
end
it "only returns students in the selected group when sorting by total_grade" do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_column_id] = "total_grade"
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_column_id] = "total_grade"
expect(gradebook_user_ids.user_ids).to contain_exactly(@student1.id)
end
@ -219,13 +221,40 @@ describe GradebookUserIds do
end
end
context "when multiple groups are specified" do
before(:once) do
@teacher.preferences[:gradebook_settings] = {
@course.global_id => {
filter_rows_by: {
student_group_ids: [group.id, group2.id]
}
}
}
end
it "ignores the student_group_ids setting when multiselect_gradebook_filters is disabled" do
expect(gradebook_user_ids.user_ids).to match_array(@course.student_ids)
end
it "only returns students in the selected groups when multiselect_gradebook_filters is enabled" do
Account.site_admin.enable_feature!(:multiselect_gradebook_filters)
expect(gradebook_user_ids.user_ids).to contain_exactly(@student1.id, @student2.id)
end
it "ignores students in groups that have been deleted" do
group.destroy!
Account.site_admin.enable_feature!(:multiselect_gradebook_filters)
expect(gradebook_user_ids.user_ids).to contain_exactly(@student2.id)
end
end
context "when no group is specified" do
it "returns students in all groups" do
expect(gradebook_user_ids.user_ids).to match_array(@course.students.pluck(:id))
end
it "returns students in all groups when sorting by total_grade" do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_column_id] = "total_grade"
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_column_id] = "total_grade"
expect(gradebook_user_ids.user_ids).to match_array(@course.students.pluck(:id))
end
end
@ -238,7 +267,7 @@ describe GradebookUserIds do
end
it "sorts by sortable name ascending if the user's sort preferences are not supported" do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_column_id] = "some_new_column"
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_column_id] = "some_new_column"
expected_user_ids = [@student1.id, @student4.id, @student3.id, @student2.id, @fake_student.id]
expect(gradebook_user_ids.user_ids).to eq(expected_user_ids)
end
@ -263,12 +292,12 @@ describe GradebookUserIds do
end
it "includes inactive student ids if the user preferences include show_inactive_enrollments" do
@teacher.preferences[:gradebook_settings][@course.id][:show_inactive_enrollments] = "true"
@teacher.preferences[:gradebook_settings][@course.global_id][:show_inactive_enrollments] = "true"
expect(gradebook_user_ids.user_ids).to include @inactive_student.id
end
it "includes concluded student ids if the user preferences include show_concluded_enrollments" do
@teacher.preferences[:gradebook_settings][@course.id][:show_concluded_enrollments] = "true"
@teacher.preferences[:gradebook_settings][@course.global_id][:show_concluded_enrollments] = "true"
expect(gradebook_user_ids.user_ids).to include @concluded_student.id
end
@ -279,7 +308,7 @@ describe GradebookUserIds do
describe "student sortable name sorting" do
before(:once) do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_setting_key] = "sortable_name"
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_setting_key] = "sortable_name"
end
it "sorts by student sortable name ascending" do
@ -289,13 +318,13 @@ describe GradebookUserIds do
it "sorts by student sortable name ascending if passed an invalid sort_rows_by_setting_key for the column" do
# "grade" is invalid here because the "student" column cannot be sorted by grade
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_setting_key] = "grade"
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_setting_key] = "grade"
expected_user_ids = [@student1.id, @student4.id, @student3.id, @student2.id, @fake_student.id]
expect(gradebook_user_ids.user_ids).to eq(expected_user_ids)
end
it "sorts by student sortable name descending" do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_direction] = "descending"
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_direction] = "descending"
expected_user_ids = [@student2.id, @student3.id, @student4.id, @student1.id, @fake_student.id]
expect(gradebook_user_ids.user_ids).to eq(expected_user_ids)
end
@ -310,7 +339,7 @@ describe GradebookUserIds do
describe "sorting by student first name" do
before(:once) do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_column_id] = "student_firstname"
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_column_id] = "student_firstname"
end
it "sorts by student first name ascending" do
@ -319,7 +348,7 @@ describe GradebookUserIds do
end
it "sorts by student first name descending" do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_direction] = "descending"
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_direction] = "descending"
expected_user_ids = [@student2.id, @student3.id, @student4.id, @student1.id, @fake_student.id]
expect(gradebook_user_ids.user_ids).to eq(expected_user_ids)
end
@ -327,7 +356,7 @@ describe GradebookUserIds do
describe "sorting by login ID" do
before(:once) do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_setting_key] = "login_id"
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_setting_key] = "login_id"
end
it "sorts by student login ID ascending" do
@ -336,7 +365,7 @@ describe GradebookUserIds do
end
it "sorts by student login ID descending" do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_direction] = "descending"
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_direction] = "descending"
expected_user_ids = [@student2.id, @student4.id, @student3.id, @student1.id, @fake_student.id]
expect(gradebook_user_ids.user_ids).to eq(expected_user_ids)
end
@ -351,7 +380,7 @@ describe GradebookUserIds do
describe "sorting by SIS ID" do
before(:once) do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_setting_key] = "sis_user_id"
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_setting_key] = "sis_user_id"
end
it "sorts by SIS ID ascending" do
@ -360,7 +389,7 @@ describe GradebookUserIds do
end
it "sorts by SIS ID descending" do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_direction] = "descending"
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_direction] = "descending"
expected_user_ids = [@student2.id, @student3.id, @student4.id, @student1.id, @fake_student.id]
expect(gradebook_user_ids.user_ids).to eq(expected_user_ids)
end
@ -375,7 +404,7 @@ describe GradebookUserIds do
describe "sorting by integration ID" do
before(:once) do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_setting_key] = "integration_id"
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_setting_key] = "integration_id"
end
it "sorts by integration ID ascending" do
@ -384,7 +413,7 @@ describe GradebookUserIds do
end
it "sorts by integration ID descending" do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_direction] = "descending"
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_direction] = "descending"
expected_user_ids = [@student2.id, @student3.id, @student4.id, @student1.id, @fake_student.id]
expect(gradebook_user_ids.user_ids).to eq(expected_user_ids)
end
@ -409,8 +438,8 @@ describe GradebookUserIds do
@assignment.grade_student(@student2, grade: 1, grader: @teacher)
@assignment.grade_student(@student3, grade: 9, grader: @teacher)
@assignment.grade_student(@student4, grade: 9, grader: @teacher)
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_column_id] = "assignment_#{@assignment.id}"
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_setting_key] = "missing"
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_column_id] = "assignment_#{@assignment.id}"
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_setting_key] = "missing"
end
it "returns user ids for users with missing submissions first" do
@ -432,12 +461,12 @@ describe GradebookUserIds do
end
it "includes inactive student ids if the user preferences include show_inactive_enrollments" do
@teacher.preferences[:gradebook_settings][@course.id][:show_inactive_enrollments] = "true"
@teacher.preferences[:gradebook_settings][@course.global_id][:show_inactive_enrollments] = "true"
expect(gradebook_user_ids.user_ids).to include @inactive_student.id
end
it "includes concluded student ids if the user preferences include show_concluded_enrollments" do
@teacher.preferences[:gradebook_settings][@course.id][:show_concluded_enrollments] = "true"
@teacher.preferences[:gradebook_settings][@course.global_id][:show_concluded_enrollments] = "true"
expect(gradebook_user_ids.user_ids).to include @concluded_student.id
end
@ -470,8 +499,8 @@ describe GradebookUserIds do
@assignment.grade_student(@student2, grade: 1, grader: @teacher)
@assignment.grade_student(@student3, grade: 9, grader: @teacher)
@assignment.grade_student(@student4, grade: 9, grader: @teacher)
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_column_id] = "assignment_#{@assignment.id}"
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_setting_key] = "late"
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_column_id] = "assignment_#{@assignment.id}"
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_setting_key] = "late"
end
it "returns user ids for users with late submissions first" do
@ -493,12 +522,12 @@ describe GradebookUserIds do
end
it "includes inactive student ids if the user preferences include show_inactive_enrollments" do
@teacher.preferences[:gradebook_settings][@course.id][:show_inactive_enrollments] = "true"
@teacher.preferences[:gradebook_settings][@course.global_id][:show_inactive_enrollments] = "true"
expect(gradebook_user_ids.user_ids).to include @inactive_student.id
end
it "includes concluded student ids if the user preferences include show_concluded_enrollments" do
@teacher.preferences[:gradebook_settings][@course.id][:show_concluded_enrollments] = "true"
@teacher.preferences[:gradebook_settings][@course.global_id][:show_concluded_enrollments] = "true"
expect(gradebook_user_ids.user_ids).to include @concluded_student.id
end
@ -531,8 +560,8 @@ describe GradebookUserIds do
@assignment.grade_student(@student2, grade: 1, grader: @teacher)
@assignment.grade_student(@student3, grade: 9, grader: @teacher)
@assignment.grade_student(@student4, grade: 9, grader: @teacher)
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_column_id] = "assignment_#{@assignment.id}"
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_setting_key] = "grade"
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_column_id] = "assignment_#{@assignment.id}"
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_setting_key] = "grade"
end
it "includes concluded students ids if the course is concluded" do
@ -549,7 +578,7 @@ describe GradebookUserIds do
context "ascending" do
before(:once) do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_direction] = "ascending"
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_direction] = "ascending"
end
it "excludes fake students if they are deactivated" do
@ -586,7 +615,7 @@ describe GradebookUserIds do
it "returns all students even if only a subset is assigned" do
assignment = @course.assignments.create!(points_possible: 10, only_visible_to_overrides: true)
create_adhoc_override_for_assignment(assignment, [@student1, @student3], due_at: nil)
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_column_id] =
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_column_id] =
"assignment_#{assignment.id}"
expect(gradebook_user_ids.user_ids).to eq(
@ -597,7 +626,7 @@ describe GradebookUserIds do
context "descending" do
before(:once) do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_direction] = "descending"
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_direction] = "descending"
end
it "excludes fake students if they are deactivated" do
@ -635,12 +664,12 @@ describe GradebookUserIds do
end
it "includes inactive student ids if the user preferences include show_inactive_enrollments" do
@teacher.preferences[:gradebook_settings][@course.id][:show_inactive_enrollments] = "true"
@teacher.preferences[:gradebook_settings][@course.global_id][:show_inactive_enrollments] = "true"
expect(gradebook_user_ids.user_ids).to include @inactive_student.id
end
it "includes concluded student ids if the user preferences include show_concluded_enrollments" do
@teacher.preferences[:gradebook_settings][@course.id][:show_concluded_enrollments] = "true"
@teacher.preferences[:gradebook_settings][@course.global_id][:show_concluded_enrollments] = "true"
expect(gradebook_user_ids.user_ids).to include @concluded_student.id
end
end
@ -665,8 +694,8 @@ describe GradebookUserIds do
@assignment2.grade_student(@student3, grade: 100, grader: @teacher)
@assignment2.grade_student(@student4, grade: 99, grader: @teacher)
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_column_id] = "total_grade"
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_setting_key] = "grade"
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_column_id] = "total_grade"
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_setting_key] = "grade"
end
context "with total grade" do
@ -676,7 +705,7 @@ describe GradebookUserIds do
end
it "sorts by total grade descending" do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_direction] = "descending"
@teacher.preferences[:gradebook_settings][@course.global_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
@ -695,7 +724,7 @@ describe GradebookUserIds do
context "with assignment group" do
before(:once) do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_column_id] =
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_column_id] =
"assignment_group_#{@second_assignment_group.id}"
end
@ -705,7 +734,7 @@ describe GradebookUserIds do
end
it "sorts by assignment group grade descending" do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_direction] = "descending"
@teacher.preferences[:gradebook_settings][@course.global_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
@ -728,12 +757,12 @@ describe GradebookUserIds do
end
it "includes inactive student ids if the user preferences include show_inactive_enrollments" do
@teacher.preferences[:gradebook_settings][@course.id][:show_inactive_enrollments] = "true"
@teacher.preferences[:gradebook_settings][@course.global_id][:show_inactive_enrollments] = "true"
expect(gradebook_user_ids.user_ids).to include @inactive_student.id
end
it "includes concluded student ids if the user preferences include show_concluded_enrollments" do
@teacher.preferences[:gradebook_settings][@course.id][:show_concluded_enrollments] = "true"
@teacher.preferences[:gradebook_settings][@course.global_id][:show_concluded_enrollments] = "true"
expect(gradebook_user_ids.user_ids).to include @concluded_student.id
end
@ -768,14 +797,14 @@ describe GradebookUserIds do
it "sorts by the current grading period totals if a grading period ID of 'null' is in user preferences" do
allow(@course).to receive(:grading_periods?).and_return(true)
@teacher.preferences[:gradebook_settings][@course.id][:filter_columns_by][:grading_period_id] = "null"
@teacher.preferences[:gradebook_settings][@course.global_id][:filter_columns_by][:grading_period_id] = "null"
expected_user_ids = [@student1.id, @student3.id, @student4.id, @student2.id, @fake_student.id]
expect(gradebook_user_ids.user_ids).to eq(expected_user_ids)
end
it "sorts by the selected grading period totals if a selected grading period is in user preferences" do
allow(@course).to receive(:grading_periods?).and_return(true)
@teacher.preferences[:gradebook_settings][@course.id][:filter_columns_by][:grading_period_id] =
@teacher.preferences[:gradebook_settings][@course.global_id][:filter_columns_by][:grading_period_id] =
@future_period.id.to_s
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)
@ -783,7 +812,7 @@ describe GradebookUserIds do
it "sorts by 'All Grading Periods' if a grading period ID of '0' is in user preferences" do
allow(@course).to receive(:grading_periods?).and_return(true)
@teacher.preferences[:gradebook_settings][@course.id][:filter_columns_by][:grading_period_id] = "0"
@teacher.preferences[:gradebook_settings][@course.global_id][:filter_columns_by][:grading_period_id] = "0"
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
@ -791,7 +820,7 @@ describe GradebookUserIds do
context "descending" do
before(:once) do
@teacher.preferences[:gradebook_settings][@course.id][:sort_rows_by_direction] = "descending"
@teacher.preferences[:gradebook_settings][@course.global_id][:sort_rows_by_direction] = "descending"
end
it "sorts by the current grading period totals if no selected grading period is in user preferences" do
@ -802,14 +831,14 @@ describe GradebookUserIds do
it "sorts by the current grading period totals if a grading period ID of 'null' is in user preferences" do
allow(@course).to receive(:grading_periods?).and_return(true)
@teacher.preferences[:gradebook_settings][@course.id][:filter_columns_by][:grading_period_id] = "null"
@teacher.preferences[:gradebook_settings][@course.global_id][:filter_columns_by][:grading_period_id] = "null"
expected_user_ids = [@student2.id, @student4.id, @student3.id, @student1.id, @fake_student.id]
expect(gradebook_user_ids.user_ids).to eq(expected_user_ids)
end
it "sorts by the selected grading period totals if a selected grading period is in user preferences" do
allow(@course).to receive(:grading_periods?).and_return(true)
@teacher.preferences[:gradebook_settings][@course.id][:filter_columns_by][:grading_period_id] =
@teacher.preferences[:gradebook_settings][@course.global_id][:filter_columns_by][:grading_period_id] =
@future_period.id.to_s
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)
@ -817,7 +846,7 @@ describe GradebookUserIds do
it "sorts by 'All Grading Periods' if a grading period ID of '0' is in user preferences" do
allow(@course).to receive(:grading_periods?).and_return(true)
@teacher.preferences[:gradebook_settings][@course.id][:filter_columns_by][:grading_period_id] = "0"
@teacher.preferences[:gradebook_settings][@course.global_id][:filter_columns_by][:grading_period_id] = "0"
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

View File

@ -106,6 +106,7 @@ import type {
} from '@canvas/grading/grading.d'
import type {
ColumnFilterKey,
FilterRowsBy,
GridColumn,
GridData,
GridDataColumnsWithObjects,
@ -214,6 +215,7 @@ import {
getDefaultSettingKeyForColumnType,
getGradeAsPercent,
getStudentGradeForColumn,
groupIdsMatch,
hiddenStudentIdsForAssignment,
htmlDecode,
isAdmin,
@ -1648,6 +1650,19 @@ class Gradebook extends React.Component<GradebookProps, GradebookState> {
}
}
updateCurrentStudentGroups = (groupIds: string[]) => {
const savedSetting = this.getFilterRowsBySetting('studentGroupIds') || []
if (!groupIdsMatch(groupIds, savedSetting)) {
this.setFilterRowsBySetting('studentGroupIds', groupIds)
const groupId = groupIds.length > 0 ? groupIds[groupIds.length - 1] : null
this.setFilterRowsBySetting('studentGroupId', groupId)
return this.saveSettings({}).then(() => {
this.updateStudentGroupFilterVisibility()
this.props.reloadStudentData()
})
}
}
assignmentGroupList = () => {
if (!this.assignmentGroups) {
return []
@ -2900,6 +2915,12 @@ class Gradebook extends React.Component<GradebookProps, GradebookState> {
view_ungraded_as_zero: viewUngradedAsZero ? 'true' : 'false',
colors,
}
if (this.options.multiselect_gradebook_filters_enabled) {
gradebook_settings.filter_rows_by.student_group_ids =
this.gridDisplaySettings.filterRowsBy.studentGroupIds
}
if (this.options.enhanced_gradebook_filters) {
return GradebookApi.saveUserSettings(this.options.context_id, gradebook_settings)
} else {
@ -3990,10 +4011,11 @@ class Gradebook extends React.Component<GradebookProps, GradebookState> {
this.updateFilterAssignmentIds()
}
getFilterRowsBySetting = (filterKey: RowFilterKey): string | null =>
this.gridDisplaySettings.filterRowsBy[filterKey]
getFilterRowsBySetting = <K extends keyof FilterRowsBy>(filterKey: K): FilterRowsBy[K] => {
return this.gridDisplaySettings.filterRowsBy[filterKey]
}
setFilterRowsBySetting = (filterKey: RowFilterKey, value: null | string) => {
setFilterRowsBySetting = <K extends keyof FilterRowsBy>(filterKey: K, value: FilterRowsBy[K]) => {
this.gridDisplaySettings.filterRowsBy[filterKey] = value
}
@ -4932,7 +4954,11 @@ class Gradebook extends React.Component<GradebookProps, GradebookState> {
// student groups
const prevStudentGroupIds = findFilterValuesOfType('student-group', prevProps.appliedFilters)
const studentGroupIds = findFilterValuesOfType('student-group', this.props.appliedFilters)
if (prevStudentGroupIds[0] !== studentGroupIds[0]) {
if (this.options.multiselect_gradebook_filters_enabled) {
if (!groupIdsMatch(prevStudentGroupIds, studentGroupIds)) {
this.updateCurrentStudentGroups(studentGroupIds)
}
} else if (prevStudentGroupIds[0] !== studentGroupIds[0]) {
if (studentGroupIds.length === 0 || !studentGroupIds[0]) {
this.updateCurrentStudentGroup(null)
} else {

View File

@ -138,6 +138,10 @@ export function getStudentGradeForColumn(student: GradebookStudent, field: strin
return student[field] || {score: null, possible: 0}
}
export function groupIdsMatch(groupIds1: string[], groupIds2: string[]): boolean {
return [...groupIds1].sort().join() === [...groupIds2].sort().join()
}
export function htmlDecode(input?: string): string | null {
return input
? new DOMParser().parseFromString(input, 'text/html').documentElement.textContent

View File

@ -119,7 +119,8 @@ export default function GradebookData(props: Props) {
props.gradebookEnv.settings.filter_columns_by || {},
props.gradebookEnv.custom_grade_statuses_enabled
? props.gradebookEnv.custom_grade_statuses
: []
: [],
props.gradebookEnv.multiselect_gradebook_filters_enabled
)
}, [
courseId,
@ -132,6 +133,7 @@ export default function GradebookData(props: Props) {
initializeAppliedFilters,
props.gradebookEnv.custom_grade_statuses_enabled,
props.gradebookEnv.custom_grade_statuses,
props.gradebookEnv.multiselect_gradebook_filters_enabled,
])
// Data loading logic goes here

View File

@ -29,6 +29,7 @@ import {
getDefaultSettingKeyForColumnType,
getGradeAsPercent,
getStudentGradeForColumn,
groupIdsMatch,
isGradedOrExcusedSubmissionUnposted,
maxAssignmentCount,
onGridKeyDown,
@ -159,6 +160,24 @@ describe('getStudentGradeForColumn', () => {
})
})
describe('groupIdsMatch', () => {
it('returns true when passed two sets of ids with the same contents', () => {
expect(groupIdsMatch(['1', '2'], ['1', '2'])).toStrictEqual(true)
})
it('returns true when passed two sets of ids with the same contents in different order', () => {
expect(groupIdsMatch(['2', '1'], ['1', '2'])).toStrictEqual(true)
})
it('returns true when passed two empty arrays', () => {
expect(groupIdsMatch([], [])).toStrictEqual(true)
})
it('returns false when passed two different sets of ids', () => {
expect(groupIdsMatch(['1'], ['1', '2'])).toStrictEqual(false)
})
})
describe('onGridKeyDown', () => {
let grid: any
let columns: any

View File

@ -242,7 +242,7 @@ function useFilterDropdownData({
value: group.id,
created_at: new Date().toISOString(),
}
toggleFilter(filter)
toggleFilterHelper(filter)
},
})),
})),

View File

@ -55,6 +55,7 @@ export type GradebookSettings = {
filter_rows_by: {
section_id: string | null
student_group_id: string | null
student_group_ids?: string[] | null
}
hide_assignment_group_totals: 'false' | 'true'
hide_total: 'false' | 'true'

View File

@ -94,7 +94,7 @@ export type GridData = {
rows: GradebookStudent[]
}
export type RowFilterKey = 'sectionId' | 'studentGroupId'
export type RowFilterKey = 'sectionId' | 'studentGroupId' | 'studentGroupIds'
export type ColumnFilterKey =
| 'assignmentGroupId'
@ -113,13 +113,19 @@ export type FilterColumnsOptions = {
endDate: null | string
}
export type FilterRowsBy = {
sectionId: string | null
studentGroupId: string | null
studentGroupIds: string[] | null
}
export type GridDisplaySettings = {
colors: StatusColors
enterGradesAs: {
[assignmentId: string]: GradeEntryMode
}
filterColumnsBy: FilterColumnsOptions
filterRowsBy: {sectionId: string | null; studentGroupId: string | null}
filterRowsBy: FilterRowsBy
hideTotal: boolean
selectedPrimaryInfo: 'last_first' | 'first_last'
selectedSecondaryInfo: string

View File

@ -65,6 +65,7 @@ export function getInitialGridDisplaySettings(
const filterRowsBy = {
sectionId: null,
studentGroupId: null,
studentGroupIds: [],
}
if (settings.filter_rows_by != null) {
Object.assign(filterRowsBy, camelizeProperties(settings.filter_rows_by))

View File

@ -260,7 +260,8 @@ describe('filtersState', () => {
.initializeAppliedFilters(
initialRowFilterSettings,
initialColumnFilterSettings,
customStatuses
customStatuses,
false
)
store.getState().initializeStagedFilters()
expect(store.getState().stagedFilters.length).toStrictEqual(0)
@ -286,7 +287,8 @@ describe('filtersState', () => {
.initializeAppliedFilters(
initialRowFilterSettings,
initialColumnFilterSettings,
customStatuses
customStatuses,
false
)
store.getState().initializeStagedFilters()
expect(store.getState().stagedFilters).not.toBeNull()
@ -319,7 +321,43 @@ describe('filtersState', () => {
.initializeAppliedFilters(
initialRowFilterSettings,
initialColumnFilterSettings,
customStatuses
customStatuses,
false
)
store.getState().initializeStagedFilters()
expect(store.getState().stagedFilters).not.toBeNull()
expect(store.getState().stagedFilters).toMatchObject([
{
id: expect.any(String),
type: 'student-group',
value: '1',
},
])
})
it('derive staged student group filter from gradebook settings (multiselect)', async () => {
const url = `/api/v1/courses/${courseId}/gradebook_filters`
fetchMock.post(url, mockResponse[0])
const initialRowFilterSettings: InitialRowFilterSettings = {
section_id: null,
student_group_id: '1',
student_group_ids: ['1'],
}
const initialColumnFilterSettings: InitialColumnFilterSettings = {
assignment_group_id: null,
context_module_id: null,
grading_period_id: null,
submissions: null,
start_date: null,
end_date: null,
}
store
.getState()
.initializeAppliedFilters(
initialRowFilterSettings,
initialColumnFilterSettings,
customStatuses,
true
)
store.getState().initializeStagedFilters()
expect(store.getState().stagedFilters).not.toBeNull()
@ -352,7 +390,8 @@ describe('filtersState', () => {
.initializeAppliedFilters(
initialRowFilterSettings,
initialColumnFilterSettings,
customStatuses
customStatuses,
false
)
store.getState().initializeStagedFilters()
expect(store.getState().stagedFilters).not.toBeNull()
@ -385,7 +424,8 @@ describe('filtersState', () => {
.initializeAppliedFilters(
initialRowFilterSettings,
initialColumnFilterSettings,
customStatuses
customStatuses,
false
)
store.getState().initializeStagedFilters()
expect(store.getState().stagedFilters).not.toBeNull()
@ -418,7 +458,8 @@ describe('filtersState', () => {
.initializeAppliedFilters(
initialRowFilterSettings,
initialColumnFilterSettings,
customStatuses
customStatuses,
false
)
store.getState().initializeStagedFilters()
expect(store.getState().stagedFilters).not.toBeNull()
@ -451,7 +492,8 @@ describe('filtersState', () => {
.initializeAppliedFilters(
initialRowFilterSettings,
initialColumnFilterSettings,
customStatuses
customStatuses,
false
)
store.getState().initializeStagedFilters()
expect(store.getState().stagedFilters.length).toStrictEqual(0)
@ -477,7 +519,8 @@ describe('filtersState', () => {
.initializeAppliedFilters(
initialRowFilterSettings,
initialColumnFilterSettings,
customStatuses
customStatuses,
false
)
store.getState().initializeStagedFilters()
expect(store.getState().stagedFilters).not.toBeNull()
@ -510,7 +553,8 @@ describe('filtersState', () => {
.initializeAppliedFilters(
initialRowFilterSettings,
initialColumnFilterSettings,
customStatuses
customStatuses,
false
)
store.getState().initializeStagedFilters()
expect(store.getState().stagedFilters).toMatchObject([
@ -542,7 +586,8 @@ describe('filtersState', () => {
.initializeAppliedFilters(
initialRowFilterSettings,
initialColumnFilterSettings,
customStatuses
customStatuses,
false
)
store.getState().initializeStagedFilters()
expect(store.getState().stagedFilters.length).toStrictEqual(0)

View File

@ -54,7 +54,8 @@ export type FiltersState = {
initializeAppliedFilters: (
initialRowFilterSettings: InitialRowFilterSettings,
initialColumnFilterSettings: InitialColumnFilterSettings,
customGradeStatuses: GradeStatus[]
customGradeStatuses: GradeStatus[],
multiselectGradebookFiltersEnabled: boolean
) => void
initializeStagedFilters: () => void
fetchFilters: () => Promise<void>
@ -80,6 +81,7 @@ export type InitialColumnFilterSettings = {
export type InitialRowFilterSettings = {
section_id: null | string
student_group_id: null | string
student_group_ids?: null | string[]
}
export default (set: SetState<GradebookStore>, get: GetState<GradebookStore>): FiltersState => ({
@ -123,7 +125,7 @@ export default (set: SetState<GradebookStore>, get: GetState<GradebookStore>): F
let appliedFilters = [...get().appliedFilters]
const excludedMultiselectFilters = ['grading-period', 'student-group']
const excludedMultiselectFilters = ['grading-period']
appliedFilters = excludedMultiselectFilters.includes(filter.type ?? '')
? appliedFilters.filter(f => f.type !== filter.type)
: appliedFilters.filter(f => !(f.type === filter.type && f.value === filter.value))
@ -136,7 +138,8 @@ export default (set: SetState<GradebookStore>, get: GetState<GradebookStore>): F
initializeAppliedFilters: (
initialRowFilterSettings: InitialRowFilterSettings,
initialColumnFilterSettings: InitialColumnFilterSettings,
customStatuses: GradeStatus[]
customStatuses: GradeStatus[],
multiselectGradebookFiltersEnabled: boolean
) => {
const appliedFilters: Filter[] = []
@ -148,8 +151,22 @@ export default (set: SetState<GradebookStore>, get: GetState<GradebookStore>): F
created_at: new Date().toISOString(),
})
}
if (typeof initialRowFilterSettings.student_group_id === 'string') {
if (
multiselectGradebookFiltersEnabled &&
typeof initialRowFilterSettings.student_group_ids === 'object'
) {
initialRowFilterSettings.student_group_ids?.forEach(value => {
appliedFilters.push({
id: uuid.v4(),
value,
type: 'student-group',
created_at: new Date().toISOString(),
})
})
} else if (
!multiselectGradebookFiltersEnabled &&
typeof initialRowFilterSettings.student_group_id === 'string'
) {
appliedFilters.push({
id: uuid.v4(),
value: initialRowFilterSettings.student_group_id,