Breaking: Make navi more compatible with Ember 3.x apps (#321)

* Breaking: Change dynamic route segments to use snake case

* Fix: Ember 3.x apps breaking when report state is set

* Fix: Tests in app

packages/app/app/router.js

@@ -17,20 +17,20 @@ Router.map(function() {
     this.route('my-data');
   });
 
-  this.route('dashboardCollections', function() {
-    this.route('collection', { path: '/:collectionId' });
+  this.route('dashboard-collections', function() {
+    this.route('collection', { path: '/:collection_id' });
   });
 
   this.route('dashboards', function() {
     this.route('new');
-    this.route('dashboard', { path: '/:dashboardId' }, function() {
+    this.route('dashboard', { path: '/:dashboard_id' }, function() {
       this.route('clone');
       this.route('view');
 
       this.route('widgets', function() {
         this.route('add');
         this.route('new');
-        this.route('widget', { path: '/:widgetId' }, function() {
+        this.route('widget', { path: '/:widget_id' }, function() {
           this.route('edit');
           this.route('invalid');
           this.route('view');
@@ -42,7 +42,7 @@ Router.map(function() {
 
   this.route('reports', function() {
     this.route('new');
-    this.route('report', { path: '/:reportId' }, function() {
+    this.route('report', { path: '/:report_id' }, function() {
       this.route('edit');
       this.route('invalid');
       this.route('view');
@@ -53,7 +53,7 @@ Router.map(function() {
 
   this.route('beta', function() {
     this.route('reports', function() {
-      this.route('report', { path: '/:reportId' }, function() {
+      this.route('report', { path: '/:report_id' }, function() {
         this.route('view');
       });
     });

packages/core/addon/mirage/serializers/base-json-serializer.js

@@ -4,7 +4,7 @@ import { camelize } from '@ember/string';
 export default JSONAPISerializer.extend({
   alwaysIncludeLinkageData: true,
 
-  keyForAttribute: attr => camelize(attr),
+  keyForAttribute: attr => attr,
 
   keyForModel: attr => camelize(attr),
 

packages/dashboards/addon/routes/dashboard-collections/collection.js

@@ -1,18 +1,18 @@
 /**
- * Copyright 2017, Yahoo Holdings Inc.
+ * Copyright 2019, Yahoo Holdings Inc.
  * Licensed under the terms of the MIT license. See accompanying LICENSE.md file for terms.
  */
 
-import Ember from 'ember';
+import Route from '@ember/routing/route';
 
-export default Ember.Route.extend({
+export default Route.extend({
   /**
    * @method model
    * @override
    *
    * Makes an ajax request to retrieve relevant dashbords in the collection
    */
-  model({ collectionId }) {
-    return this.store.find('dashboard-collection', collectionId);
+  model({ collection_id }) {
+    return this.store.find('dashboard-collection', collection_id);
   }
 });

packages/dashboards/addon/routes/dashboards/dashboard.js

@@ -1,17 +1,19 @@
 /**
- * Copyright 2017, Yahoo Holdings Inc.
+ * Copyright 2019, Yahoo Holdings Inc.
  * Licensed under the terms of the MIT license. See accompanying LICENSE.md file for terms.
  */
-
-import Ember from 'ember';
-
-const { computed, get, makeArray, run, set, setProperties } = Ember;
-
-export default Ember.Route.extend({
+import { computed, get, set, setProperties } from '@ember/object';
+import { A as arr, makeArray } from '@ember/array';
+import { isEmpty } from '@ember/utils';
+import { run } from '@ember/runloop';
+import Route from '@ember/routing/route';
+import { inject as service } from '@ember/service';
+
+export default Route.extend({
   /**
    * @property {Service} user
    */
-  user: Ember.inject.service(),
+  user: service(),
 
   /**
    * @property {DS.Model} currentDashboard - current dashboard model
@@ -23,7 +25,7 @@ export default Ember.Route.extend({
   /**
    * @property {Service} naviNotifications
    */
-  naviNotifications: Ember.inject.service(),
+  naviNotifications: service(),
 
   /**
    * Makes an ajax request to retrieve relevant widgets in the dashboard
@@ -35,8 +37,8 @@ export default Ember.Route.extend({
    * @returns {Promise} Promise that resolves to a dashboard model
    *
    */
-  model({ dashboardId }) {
-    return get(this, 'store').find('dashboard', dashboardId);
+  model({ dashboard_id }) {
+    return get(this, 'store').find('dashboard', dashboard_id);
   },
 
   /**
@@ -50,7 +52,7 @@ export default Ember.Route.extend({
    */
   _updateLayout(updatedWidgets) {
     let dashboard = get(this, 'currentDashboard'),
-      layout = Ember.A(get(dashboard, 'presentation.layout'));
+      layout = arr(get(dashboard, 'presentation.layout'));
 
     makeArray(updatedWidgets).forEach(updatedWidget => {
       let modelWidget = layout.findBy('widgetId', Number(updatedWidget.id));
@@ -138,7 +140,7 @@ export default Ember.Route.extend({
 
           // Remove layout reference
           let presentation = get(this, 'currentDashboard.presentation'),
-            newLayout = Ember.A(get(presentation, 'layout')).rejectBy('widgetId', Number(id));
+            newLayout = arr(get(presentation, 'layout')).rejectBy('widgetId', Number(id));
           set(presentation, 'layout', newLayout);
           this.send('saveDashboard');
 
@@ -182,7 +184,7 @@ export default Ember.Route.extend({
      * @param {String} title
      */
     updateTitle(title) {
-      if (!Ember.isEmpty(title)) {
+      if (!isEmpty(title)) {
         let dashboard = get(this, 'currentDashboard');
         set(dashboard, 'title', title);
         this.send('saveDashboard');

packages/dashboards/addon/routes/dashboards/dashboard/widgets/widget.js

@@ -1,5 +1,5 @@
 /**
- * Copyright 2018, Yahoo Holdings Inc.
+ * Copyright 2019, Yahoo Holdings Inc.
  * Licensed under the terms of the MIT license. See accompanying LICENSE.md file for terms.
  */
 import Ember from 'ember';
@@ -14,15 +14,15 @@ export default ReportRoute.extend({
    * @param {String} params.widgetId - persisted id or temp id of widget to fetch
    * @returns {DS.Model} model for requested widget
    */
-  model({ widgetId }) {
+  model({ widget_id }) {
     let widgets = this.modelFor('dashboards.dashboard.widgets'),
-      widgetModel = widgets.findBy('id', widgetId) || this._findTempWidget(widgetId);
+      widgetModel = widgets.findBy('id', widget_id) || this._findTempWidget(widget_id);
 
     if (widgetModel) {
       return widgetModel;
     }
 
-    throw new Error(`Widget ${widgetId} could not be found`);
+    throw new Error(`Widget ${widget_id} could not be found`);
   },
 
   /**

packages/dashboards/blueprints/navi-dashboards/index.js

@@ -1,5 +1,5 @@
 /**
- * Copyright 2017, Yahoo Holdings Inc.
+ * Copyright 2019, Yahoo Holdings Inc.
  * Licensed under the terms of the MIT license. See accompanying LICENSE.md file for terms.
  */
 
@@ -18,7 +18,7 @@ module.exports = {
       "\tthis.route('dashboardCollections', function() { " +
       EOL +
       "\
-  this.route('collection', {path:'/:collectionId'}); " +
+  this.route('collection', {path:'/:collection_id'}); " +
       EOL +
       '\
 });' +
@@ -30,7 +30,7 @@ this.route('print', function() { " +
   this.route('dashboards', function() { " +
       EOL +
       "\
-    this.route('dashboard', { path: '/:dashboardId' }, function() {" +
+    this.route('dashboard', { path: '/:dashboard_id' }, function() {" +
       EOL +
       "\
       this.route('view'); " +
@@ -51,7 +51,7 @@ this.route('dashboards', function() { " +
   this.route('new'); " +
       EOL +
       "\
-  this.route('dashboard', { path: '/:dashboardId' }, function() {" +
+  this.route('dashboard', { path: '/:dashboard_id' }, function() {" +
       EOL +
       "\
     this.route('clone'); " +
@@ -69,7 +69,7 @@ this.route('dashboards', function() { " +
       this.route('new'); " +
       EOL +
       "\
-      this.route('widget', { path: '/:widgetId'}, function() { " +
+      this.route('widget', { path: '/:widget_id'}, function() { " +
       EOL +
       "\
         this.route('edit'); " +

packages/dashboards/tests/dummy/app/router.js

@@ -12,19 +12,19 @@ Router.map(function() {
   });
 
   this.route('dashboardCollections', function() {
-    this.route('collection', { path: '/:collectionId' });
+    this.route('collection', { path: '/:collection_id' });
   });
   this.route('dashboards', function() {
     this.route('new');
 
-    this.route('dashboard', { path: '/:dashboardId' }, function() {
+    this.route('dashboard', { path: '/:dashboard_id' }, function() {
       this.route('clone');
       this.route('view');
 
       this.route('widgets', function() {
         this.route('add');
         this.route('new');
-        this.route('widget', { path: '/:widgetId' }, function() {
+        this.route('widget', { path: '/:widget_id' }, function() {
           this.route('clone-to-report');
           this.route('edit');
           this.route('view');
@@ -36,14 +36,14 @@ Router.map(function() {
 
   this.route('print', function() {
     this.route('dashboards', function() {
-      this.route('dashboard', { path: '/:dashboardId' }, function() {
+      this.route('dashboard', { path: '/:dashboard_id' }, function() {
         this.route('view');
       });
     });
   });
 
   this.route('reports', function() {
-    this.route('report', { path: '/:reportId' }, function() {
+    this.route('report', { path: '/:report_id' }, function() {
       this.route('view');
       this.route('clone');
       this.route('edit');

packages/dashboards/tests/unit/routes/dashboard-collections/dashboard-collections-test.js

@@ -35,16 +35,16 @@ test('model', function(assert) {
   assert.expect(4);
 
   return Ember.run(() => {
-    let params = { collectionId: 1 },
+    let params = { collection_id: 1 },
       route = this.subject(),
       modelPromise = route.model(params);
 
     assert.ok(modelPromise.then, 'Route returns a promise in the model hook');
 
     return modelPromise.then(model => {
-      assert.equal(model.id, params.collectionId, 'The requested collection is retrieved');
+      assert.equal(model.id, params.collection_id, 'The requested collection is retrieved');
 
-      assert.equal(model.get('title'), `Collection ${params.collectionId}`, 'The requested collection is retrieved');
+      assert.equal(model.get('title'), `Collection ${params.collection_id}`, 'The requested collection is retrieved');
 
       return model.get('dashboards').then(dashboards => {
         assert.equal(dashboards.length, 2, 'The requested collection is retrieved with the two dashboards');

packages/dashboards/tests/unit/routes/dashboards/dashboard/widgets/widget-test.js

@@ -27,16 +27,16 @@ test('model hook', function(assert) {
   });
 
   /* == Persisted id == */
-  let model = route.model({ widgetId: 1 });
+  let model = route.model({ widget_id: 1 });
   assert.equal(model.id, 1, 'Route model looks up widget based on id');
 
   /* == Temp id == */
-  model = route.model({ widgetId: '123-456' });
+  model = route.model({ widget_id: '123-456' });
   assert.equal(model.tempId, '123-456', 'Route can find widgets based on temp id');
 
   /* == Error widget not found == */
   try {
-    route.model({ widgetId: 10 });
+    route.model({ widget_id: 10 });
   } catch (error) {
     assert.equal(
       error.message,

packages/dashboards/tests/unit/routes/dashboards/dashboards-test.js

@@ -50,6 +50,7 @@ moduleFor('route:dashboards/dashboard', 'Unit | Route | dashboards/dashboard', {
     'model:bard-request/fragments/logicalTable',
     'model:bard-request/fragments/metric',
     'model:bard-request/fragments/sort',
+    'model:delivery-rule',
     'model:metadata/table',
     'model:metadata/dimension',
     'model:metadata/metric',
@@ -63,6 +64,9 @@ moduleFor('route:dashboards/dashboard', 'Unit | Route | dashboards/dashboard', {
     'validator:chart-type',
     'validator:request-metrics',
     'validator:request-dimension-order',
+    'validator:number',
+    'validator:request-time-grain',
+    'validator:request-filters',
     'service:bard-metadata',
     'serializer:bard-request/fragments/logical-table',
     'serializer:bard-request/fragments/interval',
@@ -99,13 +103,13 @@ test('model', function(assert) {
   assert.expect(4);
 
   return Ember.run(() => {
-    let params = { dashboardId: '1' },
+    let params = { dashboard_id: '1' },
       modelPromise = Route.model(params);
 
     assert.ok(modelPromise.then, 'Route returns a promise in the model hook');
 
     return modelPromise.then(dashboard => {
-      assert.equal(dashboard.id, params.dashboardId, 'The requested dashboard is retrieved');
+      assert.equal(dashboard.id, params.dashboard_id, 'The requested dashboard is retrieved');
 
       assert.equal(dashboard.get('title'), 'Tumblr Goals Dashboard', 'The requested dashboard is retrieved');
 
@@ -272,7 +276,7 @@ test('delete widget - failure', function(assert) {
   assert.expect(3);
 
   //Mock Server Endpoint
-  server.delete('/dashboards/:id/widgets/:widgetId', () => {
+  server.delete('/dashboards/:id/widgets/:widget_id', () => {
     return new Mirage.Response(500);
   });
 

packages/reports/addon/routes/report-collections/collection.js

@@ -1,5 +1,5 @@
 /**
- * Copyright 2017, Yahoo Holdings Inc.
+ * Copyright 2019, Yahoo Holdings Inc.
  * Licensed under the terms of the MIT license. See accompanying LICENSE.md file for terms.
  */
 import Ember from 'ember';
@@ -12,9 +12,9 @@ export default Ember.Route.extend({
    * @param collectionId
    * @returns {*|Promise|Promise.<TResult>}
    */
-  model({ collectionId }) {
+  model({ collection_id }) {
     return Ember.get(this, 'user')
       .findOrRegister()
-      .then(() => this.get('store').findRecord('reportCollection', collectionId));
+      .then(() => this.get('store').findRecord('reportCollection', collection_id));
   }
 });

packages/reports/addon/routes/reports/report.js

@@ -45,10 +45,10 @@ export default Route.extend({
    * @param {String} params.reportId - persisted id or temp id of report to fetch
    * @returns {DS.Model|Promise} model for requested report
    */
-  model({ reportId }) {
+  model({ report_id }) {
     return get(this, 'user')
       .findOrRegister()
-      .then(() => this._findByTempId(reportId) || this.store.findRecord('report', reportId))
+      .then(() => this._findByTempId(report_id) || this.store.findRecord('report', report_id))
       .then(this._defaultVisualization.bind(this));
   },