Skip to content

Commit

Permalink
Merge pull request #13273 from raytiley/moar-qp-fixes
Browse files Browse the repository at this point in the history 
[BUGFIX] - Merge query params from previous transition into active transition
  • Loading branch information
@rwjblue
rwjblue authored Jun 27, 2016
2 parents 503834b + f693865 commit 7e62fb0
Show file tree
Hide file tree
Showing 2 changed files with 220 additions and 6 deletions.
59 changes: 53 additions & 6 deletions packages/ember-routing/lib/system/router.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -717,11 +717,8 @@ 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);

assign(queryParams, _queryParams);
this._prepareQueryParams(targetRouteName, models, queryParams);
Expand All @@ -734,6 +731,27 @@ const EmberRouter = EmberObject.extend(Evented, {
return transition;
},

_processActiveTransitionQueryParams(targetRouteName, models, queryParams, _queryParams) {
// merge in any queryParams from the active transition which could include
// queryParams from the url on initial load.
if (!this.router.activeTransition) { return; }

var unchangedQPs = {};
var qpUpdates = this._qpUpdates || {};
for (var key in this.router.activeTransition.queryParams) {
if (!qpUpdates[key]) {
unchangedQPs[key] = this.router.activeTransition.queryParams[key];
}
}

// 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);
},

_prepareQueryParams(targetRouteName, models, queryParams) {
this._hydrateUnsuppliedQueryParams(targetRouteName, models, queryParams);
this._serializeQueryParams(targetRouteName, queryParams);
Expand Down Expand Up @@ -778,6 +796,32 @@ const EmberRouter = EmberObject.extend(Evented, {
};
},

_fullyScopeQueryParams(leafRouteName, contexts, queryParams) {
var state = calculatePostTransitionState(this, leafRouteName, contexts);
var handlerInfos = state.handlerInfos;
stashParamNames(this, handlerInfos);

for (var i = 0, len = handlerInfos.length; i < len; ++i) {
var route = handlerInfos[i].handler;
var qpMeta = get(route, '_qp');

for (var j = 0, qpLen = qpMeta.qps.length; j < qpLen; ++j) {
var qp = qpMeta.qps[j];

var presentProp = qp.prop in queryParams && qp.prop ||
qp.scopedPropertyName in queryParams && qp.scopedPropertyName ||
qp.urlKey in queryParams && qp.urlKey;

if (presentProp) {
if (presentProp !== qp.scopedPropertyName) {
queryParams[qp.scopedPropertyName] = queryParams[presentProp];
delete queryParams[presentProp];
}
}
}
}
},

_hydrateUnsuppliedQueryParams(leafRouteName, contexts, queryParams) {
let state = calculatePostTransitionState(this, leafRouteName, contexts);
let handlerInfos = state.handlerInfos;
Expand All @@ -792,7 +836,8 @@ const EmberRouter = EmberObject.extend(Evented, {
let qp = qpMeta.qps[j];

let presentProp = qp.prop in queryParams && qp.prop ||
qp.scopedPropertyName in queryParams && qp.scopedPropertyName;
qp.scopedPropertyName in queryParams && qp.scopedPropertyName ||
qp.urlKey in queryParams && qp.urlKey;

if (presentProp) {
if (presentProp !== qp.scopedPropertyName) {
Expand Down Expand Up @@ -1048,6 +1093,8 @@ function calculatePostTransitionState(emberRouter, leafRouteName, contexts) {

function updatePaths(router) {
let infos = router.router.currentHandlerInfos;
if (infos.length === 0) { return; }

let path = EmberRouter._routePath(infos);
let currentRouteName = infos[infos.length - 1].name;

Expand Down
167 changes: 167 additions & 0 deletions packages/ember/tests/routing/query_params_test.js
View file
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -2490,6 +2491,45 @@ if (isEnabled('ember-routing-route-configured-query-params')) {
bootApplication();
});

QUnit.test('queryParams are updated when a controller property is set and the route is refreshed. Issue #13263 ', function() {
setTemplates({
application: compile(
'<button id="test-button" {{action \'increment\'}}>Increment</button>' +
'<span id="test-value">{{foo}}</span>' +
'{{outlet}}'
)
});
App.ApplicationController = Controller.extend({
queryParams: ['foo'],
foo: 1,
actions: {
increment: function() {
this.incrementProperty('foo');
this.send('refreshRoute');
}
}
});

App.ApplicationRoute = Route.extend({
actions: {
refreshRoute: function() {
this.refresh();
}
}
});

startingURL = '/';
bootApplication();
equal(jQuery('#test-value').text().trim(), '1');
equal(router.get('location.path'), '/', 'url is correct');
run(jQuery('#test-button'), 'click');
equal(jQuery('#test-value').text().trim(), '2');
equal(router.get('location.path'), '/?foo=2', 'url is correct');
run(jQuery('#test-button'), 'click');
equal(jQuery('#test-value').text().trim(), '3');
equal(router.get('location.path'), '/?foo=3', 'url is correct');
});

QUnit.test('Use Ember.get to retrieve query params \'refreshModel\' configuration', function() {
expect(6);
App.ApplicationController = Controller.extend({
Expand Down Expand Up @@ -3184,6 +3224,133 @@ if (isEnabled('ember-routing-route-configured-query-params')) {
let controller = container.lookup('controller:example');
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() {
Expand Down

0 comments on commit 7e62fb0

Please sign in to comment.