From d462b47b59e1ef00d2cfcca0423c7617a0044eaa Mon Sep 17 00:00:00 2001 From: Keith Garner Date: Tue, 4 Apr 2017 18:03:46 -0500 Subject: [PATCH] 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 Reviewed-by: Derek Bender QA-Review: KC Naegle Product-Review: Christi Wruck --- app/coffeescripts/gradezilla/Gradebook.coffee | 25 ++-- .../jsx/gradezilla/GradebookSpec.js | 119 ++++++++++++++++++ 2 files changed, 134 insertions(+), 10 deletions(-) diff --git a/app/coffeescripts/gradezilla/Gradebook.coffee b/app/coffeescripts/gradezilla/Gradebook.coffee index c2b9c5b1b88..0f5a04d94ff 100644 --- a/app/coffeescripts/gradezilla/Gradebook.coffee +++ b/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) -> diff --git a/spec/javascripts/jsx/gradezilla/GradebookSpec.js b/spec/javascripts/jsx/gradezilla/GradebookSpec.js index 1aaaef9fd51..4a1910307a0 100644 --- a/spec/javascripts/jsx/gradezilla/GradebookSpec.js +++ 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;