From ecc2d539a522504007833a5a30193d9af979273f Mon Sep 17 00:00:00 2001 From: Owen Smith Date: Thu, 8 Jan 2015 16:40:19 -0500 Subject: [PATCH] fix(ngRoute): dont duplicate optional params into query When calling updateParams with properties which were optional, but previously undefined, they would be duplicated into the query params as well as into the path. --- src/ngRoute/route.js | 9 ++------- test/ngRoute/routeSpec.js | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/src/ngRoute/route.js b/src/ngRoute/route.js index 56ec491a207e..57722af4e30d 100644 --- a/src/ngRoute/route.js +++ b/src/ngRoute/route.js @@ -481,15 +481,10 @@ function $RouteProvider() { */ updateParams: function(newParams) { if (this.current && this.current.$$route) { - var searchParams = {}, self=this; - - angular.forEach(Object.keys(newParams), function(key) { - if (!self.current.pathParams[key]) searchParams[key] = newParams[key]; - }); - newParams = angular.extend({}, this.current.params, newParams); $location.path(interpolate(this.current.$$route.originalPath, newParams)); - $location.search(angular.extend({}, $location.search(), searchParams)); + // interpolate modifies newParams, only query params are left + $location.search(newParams); } else { throw $routeMinErr('norout', 'Tried updating route when with no current route'); diff --git a/test/ngRoute/routeSpec.js b/test/ngRoute/routeSpec.js index 1839fd0e17cf..368dadf3c9ae 100644 --- a/test/ngRoute/routeSpec.js +++ b/test/ngRoute/routeSpec.js @@ -1341,6 +1341,30 @@ describe('$route', function() { }); }); + it('should not update query params when an optional property was previously not in path', function() { + var routeChangeSpy = jasmine.createSpy('route change'); + + module(function($routeProvider) { + $routeProvider.when('/bar/:barId/:fooId/:spamId/:eggId?', {controller: angular.noop}); + }); + + inject(function($route, $routeParams, $location, $rootScope) { + $rootScope.$on('$routeChangeSuccess', routeChangeSpy); + + $location.path('/bar/1/2/3'); + $location.search({initial: 'true'}); + $rootScope.$digest(); + routeChangeSpy.reset(); + + $route.updateParams({barId: '5', fooId: '6', eggId: '4'}); + $rootScope.$digest(); + + expect($routeParams).toEqual({barId: '5', fooId: '6', spamId: '3', eggId: '4', initial: 'true'}); + expect(routeChangeSpy).toHaveBeenCalledOnce(); + expect($location.path()).toEqual('/bar/5/6/3/4'); + expect($location.search()).toEqual({initial: 'true'}); + }); + }); it('should complain if called without an existing route', inject(function($route) { expect($route.updateParams).toThrowMinErr('ngRoute', 'norout');