Merge pull request #15613 from CvX/query-params-from-service-fix

[BUGFIX beta] Don't throw an error, when not all query params are passed to routerService.transitionTo
emberjs · Oct 10, 2017 · f4bbfe2 · f4bbfe2
2 parents f7fa758 + 1c8c6ec
commit f4bbfe2
Show file tree

Hide file tree

Showing 4 changed files with 85 additions and 4 deletions.
diff --git a/packages/ember-routing/lib/system/route.js b/packages/ember-routing/lib/system/route.js
@@ -848,7 +848,10 @@ let Route = EmberObject.extend(ActionHandler, Evented, {
         } else {
           if (presentKey) {
             svalue = params[presentKey];
-            value = route.deserializeQueryParam(svalue, qp.urlKey, qp.type);
+
+            if (svalue !== undefined) {
+              value = route.deserializeQueryParam(svalue, qp.urlKey, qp.type);
+            }
           } else {
             // No QP provided; use default value.
             svalue = qp.serializedDefaultValue;

diff --git a/packages/ember-routing/lib/system/router.js b/packages/ember-routing/lib/system/router.js
@@ -701,7 +701,9 @@ const EmberRouter = EmberObject.extend(Evented, {
     @param {String} type
   */
   _serializeQueryParam(value, type) {
-    if (type === 'array') {
+    if (value === null || value === undefined) {
+      return value;
+    } else if (type === 'array') {
       return JSON.stringify(value);
     }
 
@@ -737,7 +739,9 @@ const EmberRouter = EmberObject.extend(Evented, {
     @param {String} defaultType
   */
   _deserializeQueryParam(value, defaultType) {
-    if (defaultType === 'boolean') {
+    if (value === null || value === undefined) {
+      return value;
+    } else if (defaultType === 'boolean') {
       return value === 'true';
     } else if (defaultType === 'number') {
       return (Number(value)).valueOf();
@@ -972,7 +976,7 @@ const EmberRouter = EmberObject.extend(Evented, {
               return true;
             }
 
-            if (_fromRouterService) {
+            if (_fromRouterService && presentProp !== false) {
               return false;
             }
 

diff --git a/packages/ember/tests/routing/query_params_test.js b/packages/ember/tests/routing/query_params_test.js
@@ -1153,6 +1153,60 @@ moduleFor('Query Params - main', class extends QueryParamTestCase {
     });
   }
 
+  ['@test Setting bound query param property to null or undefined does not serialize to url'](assert) {
+    assert.expect(9);
+
+    this.router.map(function() {
+      this.route('home');
+    });
+
+    this.setSingleQPController('home', 'foo', [1, 2]);
+
+    return this.visitAndAssert('/home').then(() => {
+      var controller = this.getController('home');
+
+      assert.deepEqual(controller.get('foo'), [1,2]);
+      this.assertCurrentPath('/home');
+
+      this.setAndFlush(controller, 'foo', emberA([1,3]));
+      this.assertCurrentPath('/home?foo=%5B1%2C3%5D');
+
+      return this.transitionTo('/home').then(() => {
+        assert.deepEqual(controller.get('foo'), [1,2]);
+        this.assertCurrentPath('/home');
+
+        this.setAndFlush(controller, 'foo', null);
+        this.assertCurrentPath('/home', 'Setting property to null');
+
+        this.setAndFlush(controller, 'foo', emberA([1,3]));
+        this.assertCurrentPath('/home?foo=%5B1%2C3%5D');
+
+        this.setAndFlush(controller, 'foo', undefined);
+        this.assertCurrentPath('/home', 'Setting property to undefined');
+      });
+    });
+  }
+
+  ['@test {{link-to}} with null or undefined QPs does not get serialized into url'](assert) {
+    assert.expect(3);
+
+    this.addTemplate('home', '{{link-to \'Home\' \'home\' (query-params foo=nullValue) id=\'null-link\'}}{{link-to \'Home\' \'home\' (query-params foo=undefinedValue) id=\'undefined-link\'}}');
+
+    this.router.map(function() {
+      this.route('home');
+    });
+
+    this.setSingleQPController('home', 'foo', [], {
+      nullValue: null,
+      undefinedValue: undefined
+    });
+
+    return this.visitAndAssert('/home').then(() => {
+      assert.equal(this.$('#null-link').attr('href'), '/home');
+      assert.equal(this.$('#undefined-link').attr('href'), '/home');
+    });
+  }
+
   ['@test A child of a resource route still defaults to parent route\'s model even if the child route has a query param'](assert) {
     assert.expect(2);
 

diff --git a/packages/ember/tests/routing/router_service_test/transitionTo_test.js b/packages/ember/tests/routing/router_service_test/transitionTo_test.js
@@ -256,6 +256,26 @@ if (EMBER_ROUTING_ROUTER_SERVICE) {
         });
     }
 
+    ['@test RouterService#transitionTo with unspecified query params'](assert) {
+      assert.expect(1);
+
+      this.add('controller:parent.child', Controller.extend({
+        queryParams: ['sort', 'page', 'category', 'extra'],
+        sort: 'ASC',
+        page: null,
+        category: undefined
+      }));
+
+      let queryParams = this.buildQueryParams({ sort: 'ASC' });
+
+      return this.visit('/').then(() => {
+        return this.routerService.transitionTo('parent.child', queryParams);
+      })
+        .then(() => {
+          assert.equal(this.routerService.get('currentURL'), '/child?sort=ASC');
+        });
+    }
+
     ['@test RouterService#transitionTo with aliased query params uses the original provided key'](assert) {
       assert.expect(1);