From adef2d0d98061d3c0c9c672b2a50c69b005d38cf Mon Sep 17 00:00:00 2001 From: Igor Terzic Date: Wed, 20 Apr 2016 18:35:35 -0700 Subject: [PATCH] Add renetrant test for query params changes --- packages/ember-routing/lib/system/router.js | 13 +- .../ember/tests/routing/query_params_test.js | 139 +++++++++++++++++- 2 files changed, 138 insertions(+), 14 deletions(-) diff --git a/packages/ember-routing/lib/system/router.js b/packages/ember-routing/lib/system/router.js index 5335b1efb17..8ca15cdcf1a 100644 --- a/packages/ember-routing/lib/system/router.js +++ b/packages/ember-routing/lib/system/router.js @@ -668,11 +668,6 @@ const EmberRouter = EmberObject.extend(Evented, { assert(`The route ${targetRouteName} was not found`, targetRouteName && this.router.hasRoute(targetRouteName)); let queryParams = {}; - // merge in any queryParams from the active transition which could include - // queryparams from the url on initial load. - if (this.router.activeTransition) { - assign(queryParams, this.router.activeTransition.queryParams); - } this._processActiveTransitionQueryParams(targetRouteName, models, queryParams, _queryParams); @@ -689,7 +684,7 @@ const EmberRouter = EmberObject.extend(Evented, { _processActiveTransitionQueryParams(targetRouteName, models, queryParams, _queryParams) { // merge in any queryParams from the active transition which could include - // queryparams from the url on initial load. + // queryParams from the url on initial load. if (!this.router.activeTransition) { return; } var unchangedQPs = {}; @@ -700,9 +695,9 @@ const EmberRouter = EmberObject.extend(Evented, { } } - // We need to fully scope query params so that we can create one object - // that represetns both pased in query params and ones that arent' changed - // from the actice transition + // We need to fully scope queryParams so that we can create one object + // that represents both pased in queryParams and ones that aren't changed + // from the active transition. this._fullyScopeQueryParams(targetRouteName, models, _queryParams); this._fullyScopeQueryParams(targetRouteName, models, unchangedQPs); assign(queryParams, unchangedQPs); diff --git a/packages/ember/tests/routing/query_params_test.js b/packages/ember/tests/routing/query_params_test.js index 0d1e83722cc..30a29890cd4 100644 --- a/packages/ember/tests/routing/query_params_test.js +++ b/packages/ember/tests/routing/query_params_test.js @@ -1,4 +1,5 @@ import Controller from 'ember-runtime/controllers/controller'; +import RSVP from 'ember-runtime/ext/rsvp'; import Route from 'ember-routing/system/route'; import run from 'ember-metal/run_loop'; import get from 'ember-metal/property_get'; @@ -2491,11 +2492,13 @@ if (isEnabled('ember-routing-route-configured-query-params')) { }); QUnit.test('queryParams are updated when a controller property is set and the route is refreshed. Issue #13263 ', function() { - Ember.TEMPLATES.application = compile( - '' + - '{{foo}}' + - '{{outlet}}' - ); + setTemplates({ + application: compile( + '' + + '{{foo}}' + + '{{outlet}}' + ) + }); App.ApplicationController = Controller.extend({ queryParams: ['foo'], foo: 1, @@ -3222,6 +3225,132 @@ if (isEnabled('ember-routing-route-configured-query-params')) { equal(get(controller, 'foo'), undefined); }); } +QUnit.test('when refreshModel is true and loading action returns false, model hook will rerun when QPs change even if previous did not finish', function() { + expect(6); + + var appModelCount = 0; + var promiseResolve; + + App.ApplicationRoute = Route.extend({ + queryParams: { + 'appomg': { + defaultValue: 'applol' + } + }, + model(params) { + appModelCount++; + } + }); + + App.IndexController = Controller.extend({ + queryParams: ['omg'] + // uncommon to not support default value, but should assume undefined. + }); + + var indexModelCount = 0; + App.IndexRoute = Route.extend({ + queryParams: { + omg: { + refreshModel: true + } + }, + actions: { + loading: function() { + return false; + } + }, + model(params) { + indexModelCount++; + if (indexModelCount === 2) { + deepEqual(params, { omg: 'lex' }); + return new RSVP.Promise(function(resolve) { + promiseResolve = resolve; + return; + }); + } else if (indexModelCount === 3) { + deepEqual(params, { omg: 'hello' }, 'Model hook reruns even if the previous one didnt finish'); + } + } + }); + + bootApplication(); + + equal(indexModelCount, 1); + + var indexController = container.lookup('controller:index'); + setAndFlush(indexController, 'omg', 'lex'); + equal(indexModelCount, 2); + + setAndFlush(indexController, 'omg', 'hello'); + equal(indexModelCount, 3); + run(function() { + promiseResolve(); + }); + equal(get(indexController, 'omg'), 'hello', 'At the end last value prevails'); +}); + +QUnit.test('when refreshModel is true and loading action does not return false, model hook will not rerun when QPs change even if previous did not finish', function() { + expect(7); + + var appModelCount = 0; + var promiseResolve; + + App.ApplicationRoute = Route.extend({ + queryParams: { + 'appomg': { + defaultValue: 'applol' + } + }, + model(params) { + appModelCount++; + } + }); + + App.IndexController = Controller.extend({ + queryParams: ['omg'] + // uncommon to not support default value, but should assume undefined. + }); + + var indexModelCount = 0; + App.IndexRoute = Route.extend({ + queryParams: { + omg: { + refreshModel: true + } + }, + model(params) { + indexModelCount++; + + if (indexModelCount === 2) { + deepEqual(params, { omg: 'lex' }); + return new RSVP.Promise(function(resolve) { + promiseResolve = resolve; + return; + }); + } else if (indexModelCount === 3) { + ok(false, 'shouldnt get here'); + } + } + }); + + bootApplication(); + + equal(appModelCount, 1); + equal(indexModelCount, 1); + + var indexController = container.lookup('controller:index'); + setAndFlush(indexController, 'omg', 'lex'); + + equal(appModelCount, 1); + equal(indexModelCount, 2); + + setAndFlush(indexController, 'omg', 'hello'); + equal(get(indexController, 'omg'), 'hello', ' value was set'); + equal(indexModelCount, 2); + run(function() { + promiseResolve(); + }); +}); QUnit.test('warn user that routes query params configuration must be an Object, not an Array', function() { expect(1);