From eccf3864d376518f7477056f986f966420eb97c1 Mon Sep 17 00:00:00 2001 From: scalvert Date: Thu, 2 Mar 2017 16:13:20 -0800 Subject: [PATCH 1/4] Router Service Phase 3 - urlFor --- packages/ember-routing/lib/services/router.js | 24 +++++-- packages/ember-routing/lib/system/router.js | 4 +- .../router_service_test/replaceWith_test.js | 18 +++++ .../router_service_test/transitionTo_test.js | 32 +++++++++ .../router_service_test/urlFor_test.js | 67 +++++++++++++++++++ .../lib/test-cases/router.js | 2 +- 6 files changed, 140 insertions(+), 7 deletions(-) create mode 100644 packages/ember/tests/routing/router_service_test/urlFor_test.js diff --git a/packages/ember-routing/lib/services/router.js b/packages/ember-routing/lib/services/router.js index 4dddb018893..2c72d1de3ce 100644 --- a/packages/ember-routing/lib/services/router.js +++ b/packages/ember-routing/lib/services/router.js @@ -32,7 +32,7 @@ const RouterService = Service.extend({ @method transitionTo @category ember-routing-router-service - @param {String} name the name of the route or a URL + @param {String} routeNameOrUrl the name of the route or a URL @param {...Object} models the model(s) or identifier(s) to be used while transitioning to the route. @param {Object} [options] optional hash with a queryParams property @@ -41,7 +41,7 @@ const RouterService = Service.extend({ attempted transition @public */ - transitionTo() { + transitionTo(/* routeNameOrUrl, ...models, options */) { return this.router.transitionTo(...arguments); }, @@ -53,7 +53,7 @@ const RouterService = Service.extend({ @method replaceWith @category ember-routing-router-service - @param {String} name the name of the route or a URL + @param {String} routeNameOrUrl the name of the route or a URL @param {...Object} models the model(s) or identifier(s) to be used while transitioning to the route. @param {Object} [options] optional hash with a queryParams property @@ -62,8 +62,24 @@ const RouterService = Service.extend({ attempted transition @public */ - replaceWith() { + replaceWith(/* routeNameOrUrl, ...models, options */) { return this.router.replaceWith(...arguments); + }, + + /** + Generate a URL based on the supplied route name. + + @method urlFor + @param {String} routeName the name of the route + @param {...Object} models the model(s) or identifier(s) to be used while + transitioning to the route. + @param {Object} [options] optional hash with a queryParams property + containing a mapping of query parameters + @return {String} the string representing the generated URL + @public + */ + urlFor(/* routeName, ...models, options */) { + return this.router.generate(...arguments); } }); diff --git a/packages/ember-routing/lib/system/router.js b/packages/ember-routing/lib/system/router.js index 4769fc1ce92..51772593191 100644 --- a/packages/ember-routing/lib/system/router.js +++ b/packages/ember-routing/lib/system/router.js @@ -411,8 +411,8 @@ const EmberRouter = EmberObject.extend(Evented, { return this.transitionTo(...arguments).method('replace'); }, - generate() { - let url = this._routerMicrolib.generate(...arguments); + generate(...args) { + let url = this._routerMicrolib.generate(...args); return this.location.formatURL(url); }, diff --git a/packages/ember/tests/routing/router_service_test/replaceWith_test.js b/packages/ember/tests/routing/router_service_test/replaceWith_test.js index 4f068793f32..179294d7fd9 100644 --- a/packages/ember/tests/routing/router_service_test/replaceWith_test.js +++ b/packages/ember/tests/routing/router_service_test/replaceWith_test.js @@ -67,6 +67,24 @@ if (isFeatureEnabled('ember-routing-router-service')) { }); } + ['@test RouterService#replaceWith with basic route using URLs replaces location'](assert) { + assert.expect(1); + + return this.visit('/') + .then(() => { + return this.routerService.transitionTo('/child'); + }) + .then(() => { + return this.routerService.transitionTo('/sister'); + }) + .then(() => { + return this.routerService.replaceWith('/brother'); + }) + .then(() => { + assert.deepEqual(this.state, ['/', '/child', '/brother']); + }); + } + ['@test RouterService#replaceWith transitioning back to previously visited route replaces location'](assert) { assert.expect(1); diff --git a/packages/ember/tests/routing/router_service_test/transitionTo_test.js b/packages/ember/tests/routing/router_service_test/transitionTo_test.js index 9b510f94f7d..d1a2f8f30aa 100644 --- a/packages/ember/tests/routing/router_service_test/transitionTo_test.js +++ b/packages/ember/tests/routing/router_service_test/transitionTo_test.js @@ -127,6 +127,38 @@ if (isFeatureEnabled('ember-routing-router-service')) { }); } + ['@test RouterService#transitionTo with basic route using URL'](assert) { + assert.expect(1); + + let componentInstance; + + this.registerTemplate('parent.index', '{{foo-bar}}'); + + this.registerComponent('foo-bar', { + ComponentClass: Component.extend({ + routerService: inject.service('router'), + init() { + this._super(); + componentInstance = this; + }, + actions: { + transitionToSister() { + get(this, 'routerService').transitionTo('/sister'); + } + } + }), + template: `foo-bar` + }); + + return this.visit('/').then(() => { + run(function() { + componentInstance.send('transitionToSister'); + }); + + assert.equal(this.routerService.get('currentRouteName'), 'parent.sister'); + }); + } + ['@test RouterService#transitionTo with dynamic segment'](assert) { assert.expect(3); diff --git a/packages/ember/tests/routing/router_service_test/urlFor_test.js b/packages/ember/tests/routing/router_service_test/urlFor_test.js new file mode 100644 index 00000000000..0226d81fa55 --- /dev/null +++ b/packages/ember/tests/routing/router_service_test/urlFor_test.js @@ -0,0 +1,67 @@ +import { inject } from 'ember-runtime'; +import { Component } from 'ember-glimmer'; +import { Route, NoneLocation } from 'ember-routing'; +import { + get, + set +} from 'ember-metal'; +import { + RouterTestCase, + moduleFor +} from 'internal-test-helpers'; + +import { isFeatureEnabled } from 'ember-metal'; + +if (isFeatureEnabled('ember-routing-router-service')) { + moduleFor('Router Service - urlFor', class extends RouterTestCase { + constructor() { + super(); + } + + ['@test RouterService#urlFor returns URL for simple route'](assert) { + assert.expect(1); + + return this.visit('/').then(() => { + let expectedURL = this.routerService.urlFor('parent.child'); + + assert.equal('/child', expectedURL); + }); + } + + ['@test RouterService#urlFor returns URL for simple route with dynamic segments'](assert) { + assert.expect(1); + + let dynamicModel = { id: 1, contents: 'much dynamicism' }; + + return this.visit('/').then(() => { + let expectedURL = this.routerService.urlFor('dynamic', dynamicModel); + + assert.equal('/dynamic/1', expectedURL); + }); + } + + ['@test RouterService#urlFor returns URL for simple route with query params'](assert) { + assert.expect(1); + + let queryParams = { queryParams: { foo: 'bar' } }; + + return this.visit('/').then(() => { + let expectedURL = this.routerService.urlFor('parent.child', queryParams); + + assert.equal('/child?foo=bar', expectedURL); + }); + } + + ['@test RouterService#urlFor returns URL for simple route with dynamic segments and query params'](assert) { + assert.expect(1); + + let queryParams = { queryParams: { foo: 'bar' } }; + + return this.visit('/').then(() => { + let expectedURL = this.routerService.urlFor('dynamic', { id: 1 }, queryParams); + + assert.equal('/dynamic/1?foo=bar', expectedURL); + }); + } + }); +} diff --git a/packages/internal-test-helpers/lib/test-cases/router.js b/packages/internal-test-helpers/lib/test-cases/router.js index 985d19ba8da..ab0636e2205 100644 --- a/packages/internal-test-helpers/lib/test-cases/router.js +++ b/packages/internal-test-helpers/lib/test-cases/router.js @@ -11,7 +11,7 @@ export default class RouterTestCase extends ApplicationTestCase { this.route('sister'); this.route('brother'); }); - this.route('dynamic', { path: '/dynamic/:post_id' }); + this.route('dynamic', { path: '/dynamic/:dynamic_id' }); }); } From 156f5d0537bf04d5c868c952be8d5e21d6a96f89 Mon Sep 17 00:00:00 2001 From: scalvert Date: Sun, 5 Mar 2017 11:37:15 -0800 Subject: [PATCH 2/4] Adding controllers to ensure they throw when calling urlFor and controllers are looked up --- packages/ember-routing/lib/system/router.js | 4 ++-- .../router_service_test/urlFor_test.js | 19 ++++++++++++++++++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/packages/ember-routing/lib/system/router.js b/packages/ember-routing/lib/system/router.js index 51772593191..4769fc1ce92 100644 --- a/packages/ember-routing/lib/system/router.js +++ b/packages/ember-routing/lib/system/router.js @@ -411,8 +411,8 @@ const EmberRouter = EmberObject.extend(Evented, { return this.transitionTo(...arguments).method('replace'); }, - generate(...args) { - let url = this._routerMicrolib.generate(...args); + generate() { + let url = this._routerMicrolib.generate(...arguments); return this.location.formatURL(url); }, diff --git a/packages/ember/tests/routing/router_service_test/urlFor_test.js b/packages/ember/tests/routing/router_service_test/urlFor_test.js index 0226d81fa55..95cd2050427 100644 --- a/packages/ember/tests/routing/router_service_test/urlFor_test.js +++ b/packages/ember/tests/routing/router_service_test/urlFor_test.js @@ -1,4 +1,8 @@ -import { inject } from 'ember-runtime'; +import { + Controller, + inject, + String +} from 'ember-runtime'; import { Component } from 'ember-glimmer'; import { Route, NoneLocation } from 'ember-routing'; import { @@ -12,10 +16,23 @@ import { import { isFeatureEnabled } from 'ember-metal'; +function defineController(app, name) { + let controllerName = `${String.capitalize(name)}Controller`; + + Object.defineProperty(app, controllerName, { + get() { + throw new Error(`Generating a URL should not require instantiation of a ${controllerName}.`); + } + }); +} + if (isFeatureEnabled('ember-routing-router-service')) { moduleFor('Router Service - urlFor', class extends RouterTestCase { constructor() { super(); + + ['dynamic', 'child'] + .forEach((name) => { defineController(this.application, name); }); } ['@test RouterService#urlFor returns URL for simple route'](assert) { From 72a5592561e2f0b197f2590eabac2d6f104f9220 Mon Sep 17 00:00:00 2001 From: scalvert Date: Mon, 6 Mar 2017 09:39:30 -0800 Subject: [PATCH 3/4] Adding more tests for permutations of urlFor calls --- .../router_service_test/urlFor_test.js | 86 ++++++++++++++++++- 1 file changed, 82 insertions(+), 4 deletions(-) diff --git a/packages/ember/tests/routing/router_service_test/urlFor_test.js b/packages/ember/tests/routing/router_service_test/urlFor_test.js index 95cd2050427..3dadbd7849a 100644 --- a/packages/ember/tests/routing/router_service_test/urlFor_test.js +++ b/packages/ember/tests/routing/router_service_test/urlFor_test.js @@ -26,6 +26,12 @@ function defineController(app, name) { }); } +function buildQueryParams(queryParams) { + return { + queryParams + }; +} + if (isFeatureEnabled('ember-routing-router-service')) { moduleFor('Router Service - urlFor', class extends RouterTestCase { constructor() { @@ -57,10 +63,10 @@ if (isFeatureEnabled('ember-routing-router-service')) { }); } - ['@test RouterService#urlFor returns URL for simple route with query params'](assert) { + ['@test RouterService#urlFor returns URL for simple route with basic query params'](assert) { assert.expect(1); - let queryParams = { queryParams: { foo: 'bar' } }; + let queryParams = buildQueryParams({ foo: 'bar' }); return this.visit('/').then(() => { let expectedURL = this.routerService.urlFor('parent.child', queryParams); @@ -69,10 +75,46 @@ if (isFeatureEnabled('ember-routing-router-service')) { }); } - ['@test RouterService#urlFor returns URL for simple route with dynamic segments and query params'](assert) { + ['@test RouterService#urlFor returns URL for simple route with array as query params'](assert) { + assert.expect(1); + + let queryParams = buildQueryParams({ selectedItems: ['a', 'b', 'c'] }); + + return this.visit('/').then(() => { + let expectedURL = this.routerService.urlFor('parent.child', queryParams); + + assert.equal('/child?selectedItems[]=a&selectedItems[]=b&selectedItems[]=c', expectedURL); + }); + } + + ['@test RouterService#urlFor returns URL for simple route with null query params'](assert) { + assert.expect(1); + + let queryParams = buildQueryParams({ foo: null }); + + return this.visit('/').then(() => { + let expectedURL = this.routerService.urlFor('parent.child', queryParams); + + assert.equal('/child', expectedURL); + }); + } + + ['@test RouterService#urlFor returns URL for simple route with undefined query params'](assert) { + assert.expect(1); + + let queryParams = buildQueryParams({ foo: undefined }); + + return this.visit('/').then(() => { + let expectedURL = this.routerService.urlFor('parent.child', queryParams); + + assert.equal('/child', expectedURL); + }); + } + + ['@test RouterService#urlFor returns URL for simple route with dynamic segments and basic query params'](assert) { assert.expect(1); - let queryParams = { queryParams: { foo: 'bar' } }; + let queryParams = buildQueryParams({ foo: 'bar' }); return this.visit('/').then(() => { let expectedURL = this.routerService.urlFor('dynamic', { id: 1 }, queryParams); @@ -80,5 +122,41 @@ if (isFeatureEnabled('ember-routing-router-service')) { assert.equal('/dynamic/1?foo=bar', expectedURL); }); } + + ['@test RouterService#urlFor returns URL for simple route with dynamic segments and array as query params'](assert) { + assert.expect(1); + + let queryParams = buildQueryParams({ selectedItems: ['a', 'b', 'c'] }); + + return this.visit('/').then(() => { + let expectedURL = this.routerService.urlFor('dynamic', { id: 1 }, queryParams); + + assert.equal('/dynamic/1?selectedItems[]=a&selectedItems[]=b&selectedItems[]=c', expectedURL); + }); + } + + ['@test RouterService#urlFor returns URL for simple route with dynamic segments and null query params'](assert) { + assert.expect(1); + + let queryParams = buildQueryParams({ foo: null }); + + return this.visit('/').then(() => { + let expectedURL = this.routerService.urlFor('dynamic', { id: 1 }, queryParams); + + assert.equal('/dynamic/1', expectedURL); + }); + } + + ['@test RouterService#urlFor returns URL for simple route with dynamic segments and undefined query params'](assert) { + assert.expect(1); + + let queryParams = buildQueryParams({ foo: undefined }); + + return this.visit('/').then(() => { + let expectedURL = this.routerService.urlFor('dynamic', { id: 1 }, queryParams); + + assert.equal('/dynamic/1', expectedURL); + }); + } }); } From 5d01fd3b347531170dfa5da27f207e52f288de15 Mon Sep 17 00:00:00 2001 From: scalvert Date: Tue, 7 Mar 2017 19:48:55 -0800 Subject: [PATCH 4/4] Adding more tests for ensuring we route to generated URL --- .../router_service_test/urlFor_test.js | 97 +++++++++++++++++-- 1 file changed, 89 insertions(+), 8 deletions(-) diff --git a/packages/ember/tests/routing/router_service_test/urlFor_test.js b/packages/ember/tests/routing/router_service_test/urlFor_test.js index 3dadbd7849a..54b83541357 100644 --- a/packages/ember/tests/routing/router_service_test/urlFor_test.js +++ b/packages/ember/tests/routing/router_service_test/urlFor_test.js @@ -16,7 +16,7 @@ import { import { isFeatureEnabled } from 'ember-metal'; -function defineController(app, name) { +function setupController(app, name) { let controllerName = `${String.capitalize(name)}Controller`; Object.defineProperty(app, controllerName, { @@ -34,13 +34,6 @@ function buildQueryParams(queryParams) { if (isFeatureEnabled('ember-routing-router-service')) { moduleFor('Router Service - urlFor', class extends RouterTestCase { - constructor() { - super(); - - ['dynamic', 'child'] - .forEach((name) => { defineController(this.application, name); }); - } - ['@test RouterService#urlFor returns URL for simple route'](assert) { assert.expect(1); @@ -54,6 +47,8 @@ if (isFeatureEnabled('ember-routing-router-service')) { ['@test RouterService#urlFor returns URL for simple route with dynamic segments'](assert) { assert.expect(1); + setupController(this.application, 'dynamic'); + let dynamicModel = { id: 1, contents: 'much dynamicism' }; return this.visit('/').then(() => { @@ -158,5 +153,91 @@ if (isFeatureEnabled('ember-routing-router-service')) { assert.equal('/dynamic/1', expectedURL); }); } + + ['@test RouterService#urlFor correctly transitions to route via generated path'](assert) { + assert.expect(1); + + let expectedURL; + + return this.visit('/') + .then(() => { + expectedURL = this.routerService.urlFor('parent.child'); + + return this.routerService.transitionTo(expectedURL); + }) + .then(() => { + assert.equal(expectedURL, this.routerService.get('currentURL')); + }); + } + + ['@test RouterService#urlFor correctly transitions to route via generated path with dynamic segments'](assert) { + assert.expect(1); + + let expectedURL; + let dynamicModel = { id: 1 }; + + this.registerRoute('dynamic', Route.extend({ + model() { + return dynamicModel; + } + })); + + return this.visit('/') + .then(() => { + expectedURL = this.routerService.urlFor('dynamic', dynamicModel); + + return this.routerService.transitionTo(expectedURL); + }) + .then(() => { + assert.equal(expectedURL, this.routerService.get('currentURL')); + }); + } + + ['@test RouterService#urlFor correctly transitions to route via generated path with query params'](assert) { + assert.expect(1); + + let expectedURL; + let actualURL; + let queryParams = buildQueryParams({ foo: 'bar' }); + + return this.visit('/') + .then(() => { + expectedURL = this.routerService.urlFor('parent.child', queryParams); + + return this.routerService.transitionTo(expectedURL); + }) + .then(() => { + actualURL = `${this.routerService.get('currentURL')}?foo=bar`; + + assert.equal(expectedURL, actualURL); + }); + } + + ['@test RouterService#urlFor correctly transitions to route via generated path with dynamic segments and query params'](assert) { + assert.expect(1); + + let expectedURL; + let actualURL; + let queryParams = buildQueryParams({ foo: 'bar' }); + let dynamicModel = { id: 1 }; + + this.registerRoute('dynamic', Route.extend({ + model() { + return dynamicModel; + } + })); + + return this.visit('/') + .then(() => { + expectedURL = this.routerService.urlFor('dynamic', dynamicModel, queryParams); + + return this.routerService.transitionTo(expectedURL); + }) + .then(() => { + actualURL = `${this.routerService.get('currentURL')}?foo=bar`; + + assert.equal(expectedURL, actualURL); + }); + } }); }