Merge pull request #8167 from epixa/8059-ascsortterms

Deprecate ascending sort for terms aggregations

src/ui/public/agg_types/buckets/terms.js

@@ -6,12 +6,15 @@ import VisSchemasProvider from 'ui/vis/schemas';
 import AggTypesBucketsCreateFilterTermsProvider from 'ui/agg_types/buckets/create_filter/terms';
 import orderAggTemplate from 'ui/agg_types/controls/order_agg.html';
 import orderAndSizeTemplate from 'ui/agg_types/controls/order_and_size.html';
+import routeBasedNotifierProvider from 'ui/route_based_notifier';
+
 export default function TermsAggDefinition(Private) {
   let BucketAggType = Private(AggTypesBucketsBucketAggTypeProvider);
   let bucketCountBetween = Private(AggTypesBucketsBucketCountBetweenProvider);
   let AggConfig = Private(VisAggConfigProvider);
   let Schemas = Private(VisSchemasProvider);
   let createFilter = Private(AggTypesBucketsCreateFilterTermsProvider);
+  const routeBasedNotifier = Private(routeBasedNotifierProvider);
 
   let orderAggSchema = (new Schemas([
     {
@@ -149,6 +152,9 @@ export default function TermsAggDefinition(Private) {
           }
 
           if (orderAgg.type.name === 'count') {
+            if (dir === 'asc') {
+              routeBasedNotifier.warning('Sorting in Ascending order by Count in Terms aggregations is deprecated');
+            }
             order._count = dir;
             return;
           }

src/ui/public/route_based_notifier/__tests__/index.js

@@ -0,0 +1,76 @@
+import { filter, find, remove } from 'lodash';
+import expect from 'expect.js';
+import ngMock from 'ng_mock';
+import Notifier from '../../notify/notifier';
+import routeBasedNotifierProvider from '../index';
+
+describe('ui/route_based_notifier', function () {
+  let $rootScope;
+  let routeBasedNotifier;
+
+  beforeEach(ngMock.module('kibana'));
+  beforeEach(ngMock.inject(($injector) => {
+    remove(Notifier.prototype._notifs); // hack to reset the global notification array
+    const Private = $injector.get('Private');
+    routeBasedNotifier = Private(routeBasedNotifierProvider);
+    $rootScope = $injector.get('$rootScope');
+  }));
+
+  describe('#warning()', () => {
+    it('adds a warning notification', () => {
+      routeBasedNotifier.warning('wat');
+      const notification = find(Notifier.prototype._notifs, {
+        type: 'warning',
+        content: 'wat'
+      });
+      expect(notification).not.to.be(undefined);
+    });
+
+    it('can be used more than once for different notifications', () => {
+      routeBasedNotifier.warning('wat');
+      routeBasedNotifier.warning('nowai');
+
+      const notification1 = find(Notifier.prototype._notifs, {
+        type: 'warning',
+        content: 'wat'
+      });
+      const notification2 = find(Notifier.prototype._notifs, {
+        type: 'warning',
+        content: 'nowai'
+      });
+
+      expect(notification1).not.to.be(undefined);
+      expect(notification2).not.to.be(undefined);
+    });
+
+    it('only adds a notification if it was not previously added in the current route', () => {
+      routeBasedNotifier.warning('wat');
+      routeBasedNotifier.warning('wat');
+
+      const notification = find(Notifier.prototype._notifs, {
+        type: 'warning',
+        content: 'wat'
+      });
+
+      expect(notification.count).to.equal(1);
+    });
+
+    it('can add a previously added notification so long as the route changes', () => {
+      routeBasedNotifier.warning('wat');
+      const notification1 = find(Notifier.prototype._notifs, {
+        type: 'warning',
+        content: 'wat'
+      });
+      expect(notification1.count).to.equal(1);
+
+      $rootScope.$broadcast('$routeChangeSuccess');
+
+      routeBasedNotifier.warning('wat');
+      const notification2 = find(Notifier.prototype._notifs, {
+        type: 'warning',
+        content: 'wat'
+      });
+      expect(notification2.count).to.equal(2);
+    });
+  });
+});

src/ui/public/route_based_notifier/index.js

@@ -0,0 +1,45 @@
+import { includes, mapValues } from 'lodash';
+import Notifier from 'ui/notify/notifier';
+
+/*
+ * Caches notification attempts so each one is only actually sent to the
+ * notifier service once per route.
+ */
+export default function routeBasedNotifierProvider($rootScope) {
+  const notifier = new Notifier();
+
+  let notifications = {
+    warnings: []
+  };
+
+  // empty the tracked notifications whenever the route changes so we can start
+  // fresh for the next route cycle
+  $rootScope.$on('$routeChangeSuccess', () => {
+    notifications = mapValues(notifications, () => []);
+  });
+
+  // Executes the given notify function if the message has not been seen in
+  // this route cycle
+  function executeIfNew(messages, message, notifyFn) {
+    if (includes(messages, message)) {
+      return;
+    }
+
+    messages.push(message);
+    notifyFn.call(notifier, message);
+  }
+
+  return {
+    /**
+     * Notify a given warning once in this route cycle
+     * @param {string} message
+     */
+    warning(message) {
+      executeIfNew(
+        notifications.warnings,
+        message,
+        notifier.warning
+      );
+    }
+  };
+};