Fix broken column-sort toggles, add unit test coverage for column-sort toggles, #4268 (#4281)

awx/ui/client/src/shared/column-sort/column-sort.controller.js

@@ -1,9 +1,7 @@
 export default ['$scope', '$state', 'QuerySet', 'GetBasePath',
     function($scope, $state, qs, GetBasePath) {
 
-        let queryset, path,
+        let queryset, path, order_by;
-            order_by = $state.params[`${$scope.columnIterator}_search`].order_by,
-            activeField = isDescending(order_by) ? order_by.substring(1, order_by.length) : order_by;
 
         function isDescending(str) {
                 if (str){
@@ -15,15 +13,16 @@ export default ['$scope', '$state', 'QuerySet', 'GetBasePath',
                 }
         }
         function invertOrderBy(str) {
-            return order_by.charAt(0) === '-' ? `${str.substring(1, str.length)}` : `-${str}`;
+            return str.charAt(0) === '-' ? `${str.substring(1, str.length)}` : `-${str}`;
         }
         $scope.orderByIcon = function() {
+            order_by = $state.params[`${$scope.columnIterator}_search`].order_by;
             // column sort is inactive
-            if (activeField !== $scope.columnField) {
+            if (order_by !== $scope.columnField && order_by !== invertOrderBy($scope.columnField)) {
                 return 'fa-sort';
             }
             // column sort is active (governed by order_by) and descending
-            else if (isDescending(order_by)) {
+            else if (isDescending($state.params[`${$scope.columnIterator}_search`].order_by)) {
                 return 'fa-sort-down';
             }
             // column sort is active governed by order_by) and asscending
@@ -33,17 +32,19 @@ export default ['$scope', '$state', 'QuerySet', 'GetBasePath',
         };
 
         $scope.toggleColumnOrderBy = function() {
-            // toggle active sort order
+            let order_by = $state.params[`${$scope.columnIterator}_search`].order_by;
-            if (activeField === $scope.columnField) {
+
+            if (order_by === $scope.columnField || order_by === invertOrderBy($scope.columnField)) {
                 order_by = invertOrderBy(order_by);
             }
             // set new active sort order
             else {
                 order_by = $scope.columnField;
             }
+
             queryset = _.merge($state.params[`${$scope.columnIterator}_search`], { order_by: order_by });
             path = GetBasePath($scope.basePath) || $scope.basePath;
-            $state.go('.', { [$scope.iterator + '_search']: queryset });
+            $state.go('.', { [$scope.columnIterator + '_search']: queryset });
             qs.search(path, queryset).then((res) =>{
                 $scope.dataset = res.data;
                 $scope.collection = res.data.results;

awx/ui/client/src/shared/column-sort/column-sort.partial.html

@@ -1,4 +1,4 @@
 <th id="{{columnIterator}}-{{columnField}}-header" class="List-tableHeader list-header {{columnCustomClass}}" ng-click="toggleColumnOrderBy()" ng-class="{'list-header-noSort' : columnNoSort === 'true'}">
     {{columnLabel}}
-    <i ng-hide="columnNoSort === 'true'" class="fa" ng-class="orderByIcon()">
+    <i ng-hide="columnNoSort === 'true'" class="fa columnSortIcon" ng-class="orderByIcon()">
 </th>

awx/ui/client/src/shared/smart-search/smart-search.controller.js

@@ -24,7 +24,7 @@ export default ['$stateParams', '$scope', '$state', 'QuerySet', 'GetBasePath', '
         function stripDefaultParams(params) {
             let stripped =_.pick(params, (value, key) => {
                 // setting the default value of a term to null in a state definition is a very explicit way to ensure it will NEVER generate a search tag, even with a non-default value
-                return defaults[key] !== value && key !== 'page' && key !== 'page_size' && defaults[key] !== null;
+                return defaults[key] !== value && key !== 'order_by' && key !== 'page' && key !== 'page_size' && defaults[key] !== null;
             });
             return _(stripped).map(qs.decodeParam).flatten().value();
         }

awx/ui/tests/spec/column-sort/column-sort.directive-test.js

@@ -0,0 +1,101 @@
+'use strict';
+
+describe('Directive: column-sort', () =>{
+
+    let $scope, template, $compile, QuerySet, GetBasePath;
+
+    beforeEach(angular.mock.module('templateUrl'));
+    beforeEach(function(){
+
+        this.mock = {
+            dataset: [
+                {name: 'zero', idx: 0},
+                {name: 'one', idx: 1},
+                {name: 'two', idx: 2}
+            ]
+        };
+
+        this.name_field = angular.element(`<column-sort
+            collection="mock"
+            dataset="mock.dataset"
+            base-path="mock"
+            column-iterator="mock"
+            column-field="name"
+            column-label="Name">
+            </column-sort>`);
+
+        this.idx_field = angular.element(`<column-sort
+            collection="mock"
+            dataset="mock.dataset"
+            base-path="mock"
+            column-iterator="mock"
+            column-field="idx"
+            column-label="Index">
+            </column-sort>`);
+
+        this.$state = {
+            params: {},
+            go: jasmine.createSpy('go')
+        };
+
+
+        angular.mock.module('ColumnSortModule', ($provide) =>{
+
+
+        QuerySet = jasmine.createSpyObj('qs', ['search']);
+        QuerySet.search.and.callFake(() => { return { then: function(){} } });
+        GetBasePath = jasmine.createSpy('GetBasePath');
+        $provide.value('QuerySet', QuerySet);
+        $provide.value('GetBasePath', GetBasePath);
+        $provide.value('$state', this.$state);
+
+        });
+    });
+
+    beforeEach(angular.mock.inject(($templateCache, _$rootScope_, _$compile_) => {
+        template = window.__html__['client/src/shared/column-sort/column-sort.partial.html'];
+        $templateCache.put('/static/partials/shared/column-sort/column-sort.partial.html', template);
+                
+        $compile = _$compile_;
+        $scope = _$rootScope_.$new();
+    }));
+
+    it('should be ordered by name', function(){
+
+        this.$state.params = {
+            mock_search: {order_by: 'name'}
+        };
+
+        $compile(this.name_field)($scope);
+        $compile(this.idx_field)($scope)
+
+        $scope.$digest();
+        expect( $(this.name_field).find('.columnSortIcon').hasClass('fa-sort-up') ).toEqual(true);
+        expect( $(this.idx_field).find('.columnSortIcon').hasClass('fa-sort') ).toEqual(true);                
+    });
+
+    it('should toggle to ascending name order, then ascending idx, then descending idx', function(){
+
+        this.$state.params = {
+            mock_search: {order_by: 'idx'}
+        };
+
+        $compile(this.name_field)($scope);
+        $compile(this.idx_field)($scope)
+
+        $scope.$digest();
+
+        $(this.name_field).click();
+        expect( $(this.name_field).find('.columnSortIcon').hasClass('fa-sort-up') ).toEqual(true);
+        expect( $(this.idx_field).find('.columnSortIcon').hasClass('fa-sort') ).toEqual(true);  
+
+        $(this.idx_field).click(); 
+        expect( $(this.name_field).find('.columnSortIcon').hasClass('fa-sort') ).toEqual(true);
+        expect( $(this.idx_field).find('.columnSortIcon').hasClass('fa-sort-up') ).toEqual(true);
+
+        $(this.idx_field).click();
+        expect( $(this.name_field).find('.columnSortIcon').hasClass('fa-sort') ).toEqual(true);
+        expect( $(this.idx_field).find('.columnSortIcon').hasClass('fa-sort-down') ).toEqual(true)
+    });
+
+});