[BUGFIX] normalize query params when merging in from active transition

packages/ember-routing/lib/system/router.js

@@ -668,11 +668,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);
@@ -685,6 +682,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);
@@ -729,6 +747,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;
@@ -743,7 +787,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) {
@@ -999,6 +1044,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;
 

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';
@@ -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({
@@ -3185,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);