diff --git a/config/feature_flags/apogee_release_flags.yml b/config/feature_flags/apogee_release_flags.yml index 099f2c53f49..983ce0354b4 100644 --- a/config/feature_flags/apogee_release_flags.yml +++ b/config/feature_flags/apogee_release_flags.yml @@ -44,3 +44,8 @@ analytics2_api_access: applies_to: RootAccount display_name: New Analytics API Access description: Enables new API endpoints for the course and user data provided by New Analytics. +gradebook_export_sort_order_bugfix: + state: hidden + applies_to: SiteAdmin + display_name: Gradebook Export Sort Order Bugfix (Tally-219) + description: Changes the sort order of the gradebook export to more closely match the canvas documentation diff --git a/lib/gradebook_exporter.rb b/lib/gradebook_exporter.rb index feff6947728..7e3fc4d5d69 100644 --- a/lib/gradebook_exporter.rb +++ b/lib/gradebook_exporter.rb @@ -93,9 +93,17 @@ class GradebookExporter assignments = select_in_grading_period calc.assignments - assignments = assignments.sort_by do |a| - [a.assignment_group_id, a.position || 0, a.due_at || CanvasSort::Last, a.title] + if Account.site_admin.feature_enabled?(:gradebook_export_sort_order_bugfix) + ActiveRecord::Associations::Preloader.new.preload(assignments, :assignment_group) + assignments.sort_by! do |a| + [a.assignment_group.position, a.position || 0, a.due_at || CanvasSort::Last, a.title] + end + else + assignments.sort_by! do |a| + [a.assignment_group_id, a.position || 0, a.due_at || CanvasSort::Last, a.title] + end end + groups = calc.groups read_only = I18n.t('csv.read_only_field', '(read only)') diff --git a/spec/lib/gradebook_exporter_spec.rb b/spec/lib/gradebook_exporter_spec.rb index 847299392b6..8f72cad7d96 100644 --- a/spec/lib/gradebook_exporter_spec.rb +++ b/spec/lib/gradebook_exporter_spec.rb @@ -36,6 +36,37 @@ describe GradebookExporter do GradebookExporter.new(@course, @teacher, opts) end + describe "assignment group order" do + before(:once) do + Account.site_admin.enable_feature!(:gradebook_export_sort_order_bugfix) + + student_in_course(course: @course, active_all: true) + + # The assignment groups are created out of order on purpose. The old code would order by assignment_group.id, so + # by creating the assignment groups out of order, we should get ids that are out of order. The new code orders + # using assignment_group.position which is guaranteed to be there in the model. + @first_group = @course.assignment_groups.create!(name: "first group", position: 1) + @last_group = @course.assignment_groups.create!(name: "last group", position: 3) + @second_group = @course.assignment_groups.create!(name: "second group", position: 2) + + @assignments = [] + @assignments[0] = @course.assignments.create!(name: "First group assignment", assignment_group: @first_group) + @assignments[2] = @course.assignments.create!(name: "last group assignment", assignment_group: @last_group) + @assignments[1] = @course.assignments.create!(name: "second group assignment", assignment_group: @second_group) + end + + it "returns assignments ordered first by assignment group position" do + csv = GradebookExporter.new(@course, @teacher).to_csv + rows = CSV.parse(csv, headers: true) + + # Our assignments should be columns 4, 5, and 6 + assignment_headers = rows.headers[4,3] + expected_headers = @assignments.map { |a| "#{a.name} (#{a.id})" } + + expect(assignment_headers).to eq(expected_headers) + end + end + describe "custom columns" do before(:once) do first_column = @course.custom_gradebook_columns.create! title: "Custom Column 1"