persist custom column order including totals

This updates the custom column order persistance to include all
non-frozen columns including the total grade and assignment group
totals columns.

fixes CNVS-35814

test plan:
 - Create a class with at least one student and 3 assignments.
 - Load the gradezilla gradebook and drag and drop the columns,
   including assignments, assignment groups, and total grade.
 - Reload the page, note that the custom order was preserved

Change-Id: If7b901758428203291e487259b2a24897dc0e2dd
Reviewed-on: https://gerrit.instructure.com/107573
Tested-by: Jenkins
Reviewed-by: Jeremy Neander <jneander@instructure.com>
Reviewed-by: Derek Bender <djbender@instructure.com>
QA-Review: KC Naegle <knaegle@instructure.com>
Product-Review: Christi Wruck

app/coffeescripts/gradezilla/Gradebook.coffee

@@ -493,8 +493,8 @@ define [
         sortType: 'custom'
         customOrder: []
       columns = @grid.getColumns()
-      assignment_columns = _.filter(columns, (c) -> c.type is 'assignment')
-      newSortOrder.customOrder = _.map(assignment_columns, (a) -> a.object.id)
+      scrollable_columns = columns.slice(@getFrozenColumnCount())
+      newSortOrder.customOrder = _.pluck(scrollable_columns, 'id')
       @setStoredSortOrder(newSortOrder)
 
     setArrangementTogglersVisibility: (newSortOrder) =>
@@ -514,12 +514,11 @@ define [
       @updateColumnHeaders()
 
     makeColumnSortFn: (sortOrder) =>
-      fn = switch sortOrder.sortType
-        when 'assignment_group', 'alpha' then @compareAssignmentPositions
-        when 'due_date' then @compareAssignmentDueDates
+      switch sortOrder.sortType
+        when 'assignment_group', 'alpha' then @wrapColumnSortFn(@compareAssignmentPositions)
+        when 'due_date' then @wrapColumnSortFn(@compareAssignmentDueDates)
         when 'custom' then @makeCompareAssignmentCustomOrderFn(sortOrder)
         else throw "unhandled column sort condition"
-      @wrapColumnSortFn(fn)
 
     compareAssignmentPositions: (a, b) ->
       diffOfAssignmentGroupPosition = a.object.assignment_group.position - b.object.assignment_group.position
@@ -541,17 +540,23 @@ define [
         sortMap[String(assignmentId)] = indexCounter
         indexCounter += 1
       return (a, b) =>
-        aIndex = sortMap[String(a.object.id)]
-        bIndex = sortMap[String(b.object.id)]
+        # The second lookup for each index is to maintain backwards
+        # compatibility with old gradebook sorting on load which only
+        # considered assignment ids.
+        aIndex = sortMap[a.id]
+        aIndex ?= sortMap[String(a.object.id)] if a.object?
+        bIndex = sortMap[b.id]
+        bIndex ?= sortMap[String(b.object.id)] if b.object?
         if aIndex? and bIndex?
           return aIndex - bIndex
-        # if there's a new assignment and its order has not been stored, it should come at the end
+        # if there's a new assignment or assignment group and its
+        # order has not been stored, it should come at the end
         else if aIndex? and not bIndex?
           return -1
         else if bIndex?
           return 1
         else
-          return @compareAssignmentPositions(a, b)
+          return @wrapColumnSortFn(@compareAssignmentPositions)(a, b)
 
     wrapColumnSortFn: (wrappedFn) ->
       (a, b) ->

spec/javascripts/jsx/gradezilla/GradebookSpec.js

@@ -508,6 +508,125 @@ test('returns false if "All Grading Periods" is selected and the grading period
   notOk(this.hideAggregateColumns.call(self));
 });
 
+QUnit.module('Gradebook#wrapColumnSortFn');
+
+test('returns -1 if second argument is of type total_grade', function () {
+  const sortFn = createGradebook().wrapColumnSortFn(this.stub());
+  equal(sortFn({}, { type: 'total_grade' }), -1);
+});
+
+test('returns 1 if first argument is of type total_grade', function () {
+  const sortFn = createGradebook().wrapColumnSortFn(this.stub());
+  equal(sortFn({ type: 'total_grade' }, {}), 1);
+});
+
+test('returns -1 if second argument is an assignment_group and the first is not', function () {
+  const sortFn = createGradebook().wrapColumnSortFn(this.stub());
+  equal(sortFn({}, { type: 'assignment_group' }), -1);
+});
+
+test('returns 1 if first arg is an assignment_group and second arg is not', function () {
+  const sortFn = createGradebook().wrapColumnSortFn(this.stub());
+  equal(sortFn({type: 'assignment_group'}, {}), 1);
+});
+
+test('returns difference in object.positions if both args are assignement_groups', function () {
+  const sortFn = createGradebook().wrapColumnSortFn(this.stub());
+  const a = { type: 'assignment_group', object: { position: 10 }};
+  const b = { type: 'assignment_group', object: { position: 5 }};
+
+  equal(sortFn(a, b), 5);
+});
+
+test('calls wrapped function when either column is not total_grade nor assignment_group', function () {
+  const wrappedFn = this.stub();
+  const sortFn = createGradebook().wrapColumnSortFn(wrappedFn);
+  sortFn({}, {});
+  ok(wrappedFn.called);
+});
+
+QUnit.module('Gradebook#makeCompareAssignmentCustomOrderFn');
+
+test('returns position difference if both are defined in the index', function () {
+  const sortOrder = { customOrder: ['foo', 'bar'] };
+  const gradeBook = createGradebook();
+  this.stub(gradeBook, 'compareAssignmentPositions');
+  const sortFn = gradeBook.makeCompareAssignmentCustomOrderFn(sortOrder);
+
+  const a = { id: 'foo' };
+  const b = { id: 'bar' };
+  equal(sortFn(a, b), -1);
+});
+
+test('returns -1 if the first arg is in the order and the second one is not', function () {
+  const sortOrder = { customOrder: ['foo', 'bar'] };
+  const gradeBook = createGradebook();
+  this.stub(gradeBook, 'compareAssignmentPositions');
+  const sortFn = gradeBook.makeCompareAssignmentCustomOrderFn(sortOrder);
+
+  const a = { id: 'foo' };
+  const b = { id: 'NO' };
+  equal(sortFn(a, b), -1);
+});
+
+test('returns 1 if the second arg is in the order and the first one is not', function () {
+  const sortOrder = { customOrder: ['foo', 'bar'] };
+  const gradeBook = createGradebook();
+  this.stub(gradeBook, 'compareAssignmentPositions');
+  const sortFn = gradeBook.makeCompareAssignmentCustomOrderFn(sortOrder);
+
+  const a = { id: 'NO' };
+  const b = { id: 'bar' };
+  equal(sortFn(a, b), 1);
+});
+
+test('calls wrapped compareAssignmentPositions otherwise', function () {
+  const sortOrder = { customOrder: ['foo', 'bar'] };
+  const gradeBook = createGradebook();
+  this.stub(gradeBook, 'compareAssignmentPositions');
+  const sortFn = gradeBook.makeCompareAssignmentCustomOrderFn(sortOrder);
+
+  const a = { id: 'taco' };
+  const b = { id: 'cat' };
+  sortFn(a, b);
+  ok(gradeBook.compareAssignmentPositions.called);
+});
+
+test('falls back to object id for the indexes if field is not in the map', function () {
+  const sortOrder = { customOrder: ['5', '11'] };
+  const gradeBook = createGradebook();
+  this.stub(gradeBook, 'compareAssignmentPositions');
+  const sortFn = gradeBook.makeCompareAssignmentCustomOrderFn(sortOrder);
+
+  const a = { id: 'NO', object: { id: 5 }};
+  const b = { id: 'NOPE', object: { id: 11 }};
+  equal(sortFn(a, b), -1);
+});
+
+QUnit.module('Gradebook#storeCustomColumnOrder');
+
+test('stores the custom column order (ignoring frozen columns)', function () {
+  const columns = [
+    { id: 'student' },
+    { id: 'assignment_232' },
+    { id: 'total_grade' },
+    { id: 'assignment_group_12' }
+  ];
+  const gradeBook = createGradebook();
+  this.stub(gradeBook, 'setStoredSortOrder');
+  gradeBook.grid = { getColumns: this.stub().returns(columns) };
+  gradeBook.parentColumns = [{ id: 'student' }];
+  gradeBook.customColumns = [];
+
+  const expectedSortOrder = {
+    sortType: 'custom',
+    customOrder: ['assignment_232', 'total_grade', 'assignment_group_12']
+  };
+
+  gradeBook.storeCustomColumnOrder();
+  ok(gradeBook.setStoredSortOrder.calledWith(expectedSortOrder));
+});
+
 QUnit.module('Gradebook#getVisibleGradeGridColumns', {
   setup () {
     this.getVisibleGradeGridColumns = Gradebook.prototype.getVisibleGradeGridColumns;