fix gradebook export to order by assignment group position

This patchset changes the sorting priority of assignments in the
gradebook exporter to first sort by assignment group position rather
than its ID.

fixes TALLY-219

flag=gradebook_export_sort_order_bugfix

test plan:
 - Have an a course with a student
 - On the Assignments page:
   - Create a second assignment group and name it "last assignments"
   - Create a third assignment group and name it "second assignments"
   - Create an arbitrary number of assignments in all three assignment
     groups
   - Reorder the assignments groups to have the second assignment group
     be ahead of the last assignments group
 - Go to the gradebook and export the gradebook
 - Ensure the general order of the assignments in the gradebook follow
   the UI order of the assignment groups

Change-Id: I6d7807c67ad7c4622057e0321c99c00d59677dcf
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/223418
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jeremy Neander <jneander@instructure.com>
Reviewed-by: Spencer Olson <solson@instructure.com>
QA-Review: Gary Mei <gmei@instructure.com>
Product-Review: Keith Garner <kgarner@instructure.com>

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

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)')

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"