Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Feb 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 6 additions & 6 deletions packages/app/app/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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');
Expand All @@ -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');
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),

Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
});
30 changes: 16 additions & 14 deletions packages/dashboards/addon/routes/dashboards/dashboard.js
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand All @@ -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);
},

/**
Expand All @@ -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));
Expand Down Expand Up @@ -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');

Expand Down Expand Up @@ -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');
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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`);
},

/**
Expand Down
10 changes: 5 additions & 5 deletions packages/dashboards/blueprints/navi-dashboards/index.js
Original file line number Diff line number Diff line change
@@ -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.
*/

Expand All @@ -18,7 +18,7 @@ module.exports = {
"\tthis.route('dashboardCollections', function() { " +
EOL +
"\
this.route('collection', {path:'/:collectionId'}); " +
this.route('collection', {path:'/:collection_id'}); " +
EOL +
'\
});' +
Expand All @@ -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'); " +
Expand All @@ -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'); " +
Expand All @@ -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'); " +
Expand Down
10 changes: 5 additions & 5 deletions packages/dashboards/tests/dummy/app/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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',
Expand Down Expand Up @@ -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');

Expand Down Expand Up @@ -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);
});

Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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));
}
});
4 changes: 2 additions & 2 deletions packages/reports/addon/routes/reports/report.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
},

Expand Down