Refactor table sort properties (#11073)

* Refactor sorting capabilities

* clean up and comments

* Move to ui_framework/services

- add tests
- address code review feedback
- Include new folders in build output.

* Rename to sortableProperties

Require initial sorted property name.

* fix a refactor miss

src/core_plugins/kibana/public/dashboard/listing/dashboard_listing.html

@@ -121,11 +121,11 @@
           >
         </th>
 
-        <th class="kuiTableHeaderCell" ng-click="listingController.toggleSort()">
+        <th class="kuiTableHeaderCell" ng-click="listingController.sortOn('title')">
           Name
           <span
             class="fa"
-            ng-class="listingController.isAscending ? 'fa-caret-up' : 'fa-caret-down'">
+            ng-class="listingController.isAscending('title') ? 'fa-caret-up' : 'fa-caret-down'">
           </span>
         </th>
       </tr>

src/core_plugins/kibana/public/dashboard/listing/dashboard_listing.js

@@ -1,8 +1,8 @@
 import SavedObjectRegistryProvider from 'ui/saved_objects/saved_object_registry';
 import 'ui/pager_control';
 import 'ui/pager';
-import _ from 'lodash';
 import { DashboardConstants, createDashboardEditUrl } from '../dashboard_constants';
+import { SortableProperties } from 'ui_framework/services';
 
 export function DashboardListingController($injector, $scope) {
   const $filter = $injector.get('$filter');
@@ -21,21 +21,17 @@ export function DashboardListingController($injector, $scope) {
   const notify = new Notifier({ location: 'Dashboard' });
 
   let selectedItems = [];
-
-  /**
-   * Sorts hits either ascending or descending
-   * @param  {Array} hits Array of saved finder object hits
-   * @return {Array} Array sorted either ascending or descending
-   */
-  const sortItems = () => {
-    this.items =
-      this.isAscending
-      ? _.sortBy(this.items, 'title')
-      : _.sortBy(this.items, 'title').reverse();
-  };
+  const sortableProperties = new SortableProperties([
+    {
+      name: 'title',
+      getValue: item => item.title,
+      isAscending: true,
+    }
+  ],
+  'title');
 
   const calculateItemsOnPage = () => {
-    sortItems();
+    this.items = sortableProperties.sortItems(this.items);
     this.pager.setTotalItems(this.items.length);
     this.pageOfItems = limitTo(this.items, this.pager.pageSize, this.pager.startIndex);
   };
@@ -70,16 +66,11 @@ export function DashboardListingController($injector, $scope) {
     deselectAll();
     fetchItems();
   });
+  this.isAscending = (name) => sortableProperties.isAscendingByName(name);
+  this.getSortedProperty = () => sortableProperties.getSortedProperty();
 
-  /**
-   * Boolean that keeps track of whether hits are sorted ascending (true)
-   * or descending (false) by title
-   * @type {Boolean}
-   */
-  this.isAscending = true;
-
-  this.toggleSort = function toggleSort() {
-    this.isAscending = !this.isAscending;
+  this.sortOn = function sortOn(propertyName) {
+    sortableProperties.sortOn(propertyName);
     deselectAll();
     calculateItemsOnPage();
   };

src/core_plugins/kibana/public/visualize/listing/visualize_listing.html

@@ -127,7 +127,7 @@
             <span
               class="kuiIcon"
               ng-show="listingController.getSortProperty().name === 'title'"
-              ng-class="listingController.isAscending() ? 'fa-caret-up' : 'fa-caret-down'"
+              ng-class="listingController.isAscending('title') ? 'fa-caret-up' : 'fa-caret-down'"
             ></span>
           </th>
 
@@ -139,7 +139,7 @@
             <span
               class="kuiIcon"
               ng-show="listingController.getSortProperty().name === 'type'"
-              ng-class="listingController.isAscending() ? 'fa-caret-up' : 'fa-caret-down'"
+              ng-class="listingController.isAscending('type') ? 'fa-caret-up' : 'fa-caret-down'"
             ></span>
           </th>
         </tr>

src/core_plugins/kibana/public/visualize/listing/visualize_listing.js

@@ -1,7 +1,8 @@
 import SavedObjectRegistryProvider from 'ui/saved_objects/saved_object_registry';
 import 'ui/pager_control';
 import 'ui/pager';
-import _ from 'lodash';
+
+import { SortableProperties } from 'ui_framework/services';
 
 export function VisualizeListingController($injector, $scope) {
   const $filter = $injector.get('$filter');
@@ -20,23 +21,22 @@ export function VisualizeListingController($injector, $scope) {
   const notify = new Notifier({ location: 'Visualize' });
 
   let selectedItems = [];
-
-  /**
-   * Sorts hits either ascending or descending
-   * @param  {Array} hits Array of saved finder object hits
-   * @return {Array} Array sorted either ascending or descending
-   */
-  const sortItems = () => {
-    const sortProperty = this.getSortProperty();
-
-    this.items =
-      sortProperty.isAscending
-      ? _.sortBy(this.items, sortProperty.getValue)
-      : _.sortBy(this.items, sortProperty.getValue).reverse();
-  };
+  const sortableProperties = new SortableProperties([
+    {
+      name: 'title',
+      getValue: item => item.title,
+      isAscending: true,
+    },
+    {
+      name: 'type',
+      getValue: item => item.type.title,
+      isAscending: true,
+    }
+  ],
+  'title');
 
   const calculateItemsOnPage = () => {
-    sortItems();
+    this.items = sortableProperties.sortItems(this.items);
     this.pager.setTotalItems(this.items.length);
     this.pageOfItems = limitTo(this.items, this.pager.pageSize, this.pager.startIndex);
   };
@@ -60,6 +60,15 @@ export function VisualizeListingController($injector, $scope) {
     selectedItems = this.pageOfItems.slice(0);
   };
 
+  this.isAscending = (name) => sortableProperties.isAscendingByName(name);
+  this.getSortedProperty = () => sortableProperties.getSortedProperty();
+
+  this.sortOn = function sortOn(propertyName) {
+    sortableProperties.sortOn(propertyName);
+    deselectAll();
+    calculateItemsOnPage();
+  };
+
   this.isFetchingItems = false;
   this.items = [];
   this.pageOfItems = [];
@@ -72,48 +81,6 @@ export function VisualizeListingController($injector, $scope) {
     fetchItems();
   });
 
-  /**
-   * Remember sort direction per property.
-   */
-  this.sortProperties = [{
-    name: 'title',
-    getValue: item => item.title,
-    isSelected: true,
-    isAscending: true,
-  }, {
-    name: 'type',
-    getValue: item => item.type.title,
-    isSelected: false,
-    isAscending: true,
-  }];
-
-  this.getSortProperty = function getSortProperty() {
-    return this.sortProperties.find(property => property.isSelected);
-  };
-
-  this.getSortPropertyByName = function getSortPropertyByName(name) {
-    return this.sortProperties.find(property => property.name === name);
-  };
-
-  this.isAscending = function isAscending() {
-    const sortProperty = this.getSortProperty();
-    return sortProperty.isAscending;
-  };
-
-  this.sortOn = function sortOn(propertyName) {
-    const sortProperty = this.getSortProperty();
-
-    if (sortProperty.name === propertyName) {
-      sortProperty.isAscending = !sortProperty.isAscending;
-    } else {
-      sortProperty.isSelected = false;
-      this.getSortPropertyByName(propertyName).isSelected = true;
-    }
-
-    deselectAll();
-    calculateItemsOnPage();
-  };
-
   this.toggleAll = function toggleAll() {
     if (this.areAllItemsChecked()) {
       deselectAll();

src/jest/config.js

@@ -3,6 +3,9 @@ import { resolve } from 'path';
 export const config = {
   roots: ['<rootDir>/ui_framework/'],
   collectCoverageFrom: [
+    'ui_framework/services/**/*.js',
+    '!ui_framework/services/index.js',
+    '!ui_framework/services/**/*/index.js',
     'ui_framework/components/**/*.js',
     '!ui_framework/components/index.js',
     '!ui_framework/components/**/*/index.js',

tasks/config/copy.js

@@ -14,6 +14,8 @@ module.exports = function () {
         '!src/ui_framework/doc_site/**',
         '!src/es_archiver/**',
         'bin/**',
+        'ui_framework/components/**',
+        'ui_framework/services/**',
         'ui_framework/dist/**',
         'webpackShims/**',
         'config/kibana.yml',

ui_framework/services/index.js

@@ -0,0 +1 @@
+export { SortableProperties } from './sort';

ui_framework/services/sort/index.js

@@ -0,0 +1 @@
+export { SortableProperties } from './sortable_properties';

ui_framework/services/sort/sortable_properties.js

@@ -0,0 +1,99 @@
+import _ from 'lodash';
+
+/**
+ * @typedef {Object} SortableProperty
+ * @property {string} sortableProperty.name - Name of the property.
+ * @property {function} sortableProperty.getValue - A function that takes in an object and returns a value to sort
+ * by.
+ * @property {boolean} sortableProperty.isAscending - The direction of the last sort by this property. Used to preserve
+ * past sort orders.
+ */
+
+/**
+ * Stores sort information for a set of SortableProperties, including which property is currently being sorted on, as
+ * well as the last sort order for each property.
+ */
+export class SortableProperties {
+  /**
+   * @param {Array<SortableProperty>} sortableProperties - a set of sortable properties.
+   * @param {string} initialSortablePropertyName - Which sort property should be sorted on by default.
+   */
+  constructor(sortableProperties, initialSortablePropertyName) {
+    this.sortableProperties = sortableProperties;
+    /**
+     * The current property that is being sorted on.
+     * @type {SortableProperty}
+     */
+    this.currentSortedProperty = this.getSortablePropertyByName(initialSortablePropertyName);
+    if (!this.currentSortedProperty) {
+      throw new Error(`No property with the name ${initialSortablePropertyName}`);
+    }
+  }
+
+  /**
+   * @returns {SortableProperty} The current property that is being sorted on. Undefined if no sort order is applied.
+   */
+  getSortedProperty() {
+    return this.currentSortedProperty;
+  }
+
+  /**
+   * Sorts the items passed in and returns a newly sorted array.
+   * @param items {Array.<Object>}
+   * @returns {Array.<Object>} sorted array of items, based off the sort properties.
+   */
+  sortItems(items) {
+    return this.isCurrentSortAscending()
+      ? _.sortBy(items, this.getSortedProperty().getValue)
+      : _.sortBy(items, this.getSortedProperty().getValue).reverse();
+  }
+
+  /**
+   * Returns the SortProperty with the given name, if found.
+   * @param {String} propertyName
+   * @returns {SortableProperty|undefined}
+   */
+  getSortablePropertyByName(propertyName) {
+    return this.sortableProperties.find(property => property.name === propertyName);
+  }
+
+  /**
+   * Updates the sort property, potentially flipping the sort order based on whether the same
+   * property was already being sorted.
+   * @param propertyName {String}
+   */
+  sortOn(propertyName) {
+    const newSortedProperty = this.getSortablePropertyByName(propertyName);
+    const sortedProperty = this.getSortedProperty();
+    if (sortedProperty.name === newSortedProperty.name) {
+      this.flipCurrentSortOrder();
+    } else {
+      this.currentSortedProperty = newSortedProperty;
+    }
+  }
+
+  /**
+   * @returns {boolean} True if the current sortable property is sorted in ascending order.
+   */
+  isCurrentSortAscending() {
+    const sortedProperty = this.getSortedProperty();
+    return sortedProperty ? this.isAscendingByName(sortedProperty.name) : false;
+  }
+
+  /**
+   * @param {string} propertyName
+   * @returns {boolean} True if the given sort property is sorted in ascending order.
+   */
+  isAscendingByName(propertyName) {
+    const sortedProperty = this.getSortablePropertyByName(propertyName);
+    return sortedProperty ? sortedProperty.isAscending : false;
+  }
+
+  /**
+   * Flips the current sorted property sort order.
+   */
+  flipCurrentSortOrder() {
+    this.currentSortedProperty.isAscending = !this.currentSortedProperty.isAscending;
+  }
+}
+

ui_framework/services/sort/sortable_properties.test.js

@@ -0,0 +1,122 @@
+import _ from 'lodash';
+import {
+  SortableProperties,
+} from './sortable_properties';
+
+describe('SortProperties', () => {
+  const name = {
+    name: 'name',
+    getValue: bird => bird.name,
+    isAscending: true,
+  };
+
+  const size = {
+    name: 'size',
+    getValue: bird => bird.size,
+    isAscending: false,
+  };
+
+  const color = {
+    name: 'color',
+    getValue: bird => bird.color,
+    isAscending: true,
+  };
+
+  const birds = [
+    {
+      name: 'cardinal',
+      color: 'red',
+      size: 7,
+    },
+    {
+      name: 'bluejay',
+      color: 'blue',
+      size: 8,
+    },
+    {
+      name: 'chickadee',
+      color: 'black and white',
+      size: 3,
+    }
+  ];
+
+  describe('initialSortProperty', () => {
+    test('is set', () => {
+      const sortableProperties = new SortableProperties([name, size, color], 'size');
+      expect(sortableProperties.getSortedProperty().name).toBe('size');
+    });
+
+    test('throws an error with an invalid property name', () => {
+      expect(() => new SortableProperties([name, size, color], 'does not exist')).toThrow();
+    });
+
+    test('throws an error property name is not defined', () => {
+      expect(() => new SortableProperties([name, size, color])).toThrow();
+    });
+  });
+
+  describe('isAscendingByName', () => {
+    test('returns initial ascending values', () => {
+      const initialColorAscending = color.isAscending;
+      const initialNameAscending = name.isAscending;
+      const initialSizeAscending = size.isAscending;
+
+      const sortableProperties = new SortableProperties([name, size, color], 'color');
+
+      expect(sortableProperties.isAscendingByName('color')).toBe(initialColorAscending);
+      expect(sortableProperties.isAscendingByName('name')).toBe(initialNameAscending);
+      expect(sortableProperties.isAscendingByName('size')).toBe(initialSizeAscending);
+    });
+  });
+
+  describe('sortOn', () => {
+    test('a new property name retains the original sort order', () => {
+      const initialColorAscending = color.isAscending;
+      const sortableProperties = new SortableProperties([name, size, color], 'color');
+      sortableProperties.sortOn('name');
+      expect(sortableProperties.isAscendingByName('color')).toBe(initialColorAscending);
+    });
+
+    test('the same property name twice flips sort order', () => {
+      const initialColorAscending = color.isAscending;
+      const sortableProperties = new SortableProperties([name, size, color], 'color');
+      sortableProperties.sortOn('color');
+      expect(sortableProperties.isAscendingByName('color')).toBe(!initialColorAscending);
+    });
+  });
+
+  describe('sortItems', () => {
+    test('sorts by initialSortProperty', () => {
+      const sortableProperties = new SortableProperties([name, size, color], 'name');
+      const sortedItems = sortableProperties.sortItems(birds);
+      expect(sortedItems.length).toBe(birds.length);
+      expect(sortedItems[0].name).toBe('bluejay');
+      expect(sortedItems[1].name).toBe('cardinal');
+      expect(sortedItems[2].name).toBe('chickadee');
+    });
+
+    test('first sorts by initial isAscending value', () => {
+      const sortableProperties = new SortableProperties([name, size, color], 'size');
+      const sortedItems = sortableProperties.sortItems(birds);
+      expect(sortedItems[0].size).toBe(8);
+      expect(sortedItems[1].size).toBe(7);
+      expect(sortedItems[2].size).toBe(3);
+    });
+
+    test('sorts by descending', () => {
+      const sortableProperties = new SortableProperties([name, size, color], 'name');
+      sortableProperties.sortOn('size');
+      sortableProperties.sortOn('size');
+      const sortedItems = sortableProperties.sortItems(birds);
+      expect(sortedItems[0].size).toBe(3);
+      expect(sortedItems[1].size).toBe(7);
+      expect(sortedItems[2].size).toBe(8);
+    });
+
+    test('empty items array is a noop', () => {
+      const sortableProperties = new SortableProperties([name, size, color], 'color');
+      const sortedItems = sortableProperties.sortItems([]);
+      expect(sortedItems.length).toBe(0);
+    });
+  });
+});