From 45de84c183c8d200808792b013948ad164568f56 Mon Sep 17 00:00:00 2001 From: Andrew Henry Date: Wed, 7 Dec 2016 13:33:53 -0800 Subject: [PATCH] [Time Conductor] Prevent route change when time conductor values change (#1342) * [Time Conductor] Prevent route change on setting search parameters. fixes #1341 * [Inspector] Fixed incorrect listener deregistration which was causing errors on scope destruction * Bare route is redirect to browse * [Browse] handle routing without breaking $route Manage route transitions such that route changes are properly prevented and navigation events occur while still updating the url. Resolves a number of issues where path and search updates had to be supported in a very hacky manner. https://github.com/nasa/openmct/pull/1342 * [URL] Set search without hacks Changes in previous commit allow the search parameters to be modified without accidentally triggering a page reload. https://github.com/nasa/openmct/pull/1342 * [Views] Update on location changes If the user has a bookmark or tries to change the current view of an object by specifying view=someView as a search parameter, the change would not previously take effect. This resolves that bug. https://github.com/nasa/openmct/pull/1342 * [TC] Set query params to undefined Instead of setting params to null, which would eventually result in those parameters equaling undefined, set them to undefined to skip the extra step. https://github.com/nasa/openmct/pull/1342 * [Instantiate] Instantiate objects with context Add context to instantiate objects so that they can be navigated to for editing. https://github.com/nasa/openmct/pull/1342 * [Tests] Update specs Update specs to match new expectations. * [Style] Fix style * [TC] Remove unused dependency Remove $route dependency from time conductor controller as it was not being used. Resolves review comments. https://github.com/nasa/openmct/pull/1342#pullrequestreview-11449260 --- platform/commonUI/browse/bundle.js | 5 +- .../commonUI/browse/src/BrowseController.js | 196 +++++++---------- .../browse/src/BrowseObjectController.js | 16 +- .../browse/src/InspectorPaneController.js | 4 +- .../browse/test/BrowseControllerSpec.js | 207 ++++++++---------- .../browse/test/BrowseObjectControllerSpec.js | 9 +- .../capabilities/InstantiationCapability.js | 8 +- .../InstantiationCapabilitySpec.js | 15 +- .../core/src/ui/TimeConductorController.js | 37 +++- .../src/ui/TimeConductorControllerSpec.js | 68 +----- 10 files changed, 245 insertions(+), 320 deletions(-) diff --git a/platform/commonUI/browse/bundle.js b/platform/commonUI/browse/bundle.js index b0aa7e4277..bed2fd1fbd 100644 --- a/platform/commonUI/browse/bundle.js +++ b/platform/commonUI/browse/bundle.js @@ -72,14 +72,13 @@ define([ "extensions": { "routes": [ { - "when": "/browse/:ids*", + "when": "/browse/:ids*?", "template": browseTemplate, "reloadOnSearch": false }, { "when": "", - "template": browseTemplate, - "reloadOnSearch": false + "redirectTo": "/browse/" } ], "constants": [ diff --git a/platform/commonUI/browse/src/BrowseController.js b/platform/commonUI/browse/src/BrowseController.js index 005fa105a2..c1cbbc022c 100644 --- a/platform/commonUI/browse/src/BrowseController.js +++ b/platform/commonUI/browse/src/BrowseController.js @@ -28,8 +28,6 @@ define( [], function () { - var ROOT_ID = "ROOT"; - /** * The BrowseController is used to populate the initial scope in Browse * mode. It loads the root object from the objectService and makes it @@ -49,74 +47,21 @@ define( urlService, defaultPath ) { - var path = [ROOT_ID].concat( - ($route.current.params.ids || defaultPath).split("/") - ); + var initialPath = ($route.current.params.ids || defaultPath).split("/"); - function updateRoute(domainObject) { - var priorRoute = $route.current, - // Act as if params HADN'T changed to avoid page reload - unlisten; + var currentIds = $route.current.params.ids; - unlisten = $scope.$on('$locationChangeSuccess', function () { - // Checks path to make sure /browse/ is at front - // if so, change $route.current - if ($location.path().indexOf("/browse/") === 0) { - $route.current = priorRoute; - } - unlisten(); - }); - // urlService.urlForLocation used to adjust current - // path to new, addressed, path based on - // domainObject - $location.path(urlService.urlForLocation("browse", domainObject)); + $scope.treeModel = { + selectedObject: {} + }; + function idsForObject(domainObject) { + return urlService + .urlForLocation("", domainObject) + .replace('/', ''); } - function setScopeObjects(domainObject, navigationAllowed) { - if (navigationAllowed) { - $scope.navigatedObject = domainObject; - $scope.treeModel.selectedObject = domainObject; - updateRoute(domainObject); - } else { - //If navigation was unsuccessful (ie. blocked), reset - // the selected object in the tree to the currently - // navigated object - $scope.treeModel.selectedObject = $scope.navigatedObject ; - } - } - - // Callback for updating the in-scope reference to the object - // that is currently navigated-to. - function setNavigation(domainObject) { - if (domainObject === $scope.navigatedObject) { - //do nothing; - return; - } - if (domainObject) { - domainObject.getCapability("action").perform("navigate").then(setScopeObjects.bind(undefined, domainObject)); - } else { - setScopeObjects(domainObject, true); - } - } - - function navigateTo(domainObject) { - - // Check if an object has been navigated-to already... - // If not, or if an ID path has been explicitly set in the URL, - // navigate to the URL-specified object. - if (!navigationService.getNavigation() || $route.current.params.ids) { - // If not, pick a default as the last - // root-level component (usually "mine") - navigationService.setNavigation(domainObject); - $scope.navigatedObject = domainObject; - } else { - // Otherwise, just expose the currently navigated object. - $scope.navigatedObject = navigationService.getNavigation(); - updateRoute($scope.navigatedObject); - } - } - + // Find an object in an array of objects. function findObject(domainObjects, id) { var i; for (i = 0; i < domainObjects.length; i += 1) { @@ -126,63 +71,92 @@ define( } } - // Navigate to the domain object identified by path[index], - // which we expect to find in the composition of the passed - // domain object. - function doNavigate(domainObject, index) { - var composition = domainObject.useCapability("composition"); - if (composition) { - composition.then(function (c) { - var nextObject = findObject(c, path[index]); - if (nextObject) { - if (index + 1 >= path.length) { - navigateTo(nextObject); - } else { - doNavigate(nextObject, index + 1); - } - } else if (index === 1 && c.length > 0) { - // Roots are in a top-level container that we don't - // want to be selected, so if we couldn't find an - // object at the path we wanted, at least select - // one of its children. - navigateTo(c[c.length - 1]); - } else { - // Couldn't find the next element of the path - // so navigate to the last path object we did find - navigateTo(domainObject); - } + // helper, fetch a single object from the object service. + function getObject(id) { + return objectService.getObjects([id]) + .then(function (results) { + return results[id]; }); - } else { - // Similar to above case; this object has no composition, - // so navigate to it instead of subsequent path elements. - navigateTo(domainObject); + } + + // recursively locate and return an object inside of a container + // via a path. If at any point in the recursion it fails to find + // the next object, it will return the parent. + function findViaComposition(containerObject, path) { + var nextId = path.shift(); + if (!nextId) { + return containerObject; + } + return containerObject.useCapability('composition') + .then(function (composees) { + var nextObject = findObject(composees, nextId); + if (!nextObject) { + return containerObject; + } + if (!nextObject.hasCapability('composition')) { + return nextObject; + } + return findViaComposition(nextObject, path); + }); + } + + function navigateToObject(desiredObject) { + $scope.navigatedObject = desiredObject; + $scope.treeModel.selectedObject = desiredObject; + navigationService.setNavigation(desiredObject); + currentIds = idsForObject(desiredObject); + $route.current.pathParams.ids = currentIds; + $location.path('/browse/' + currentIds); + } + + function navigateToPath(path) { + return getObject('ROOT') + .then(function (root) { + return findViaComposition(root, path); + }) + .then(navigateToObject); + } + + + getObject('ROOT') + .then(function (root) { + $scope.domainObject = root; + navigateToPath(initialPath); + }); + + // Handle navigation events from view service. Only navigates + // if path has changed. + function navigateDirectlyToModel(domainObject) { + var newIds = idsForObject(domainObject); + if (currentIds !== newIds) { + currentIds = newIds; + navigateToObject(domainObject); } } - // Load the root object, put it in the scope. - // Also, load its immediate children, and (possibly) - // navigate to one of them, so that navigation state has - // a useful initial value. - objectService.getObjects([path[0]]).then(function (objects) { - $scope.domainObject = objects[path[0]]; - doNavigate($scope.domainObject, 1); - }); - - // Provide a model for the tree to modify - $scope.treeModel = { - selectedObject: navigationService.getNavigation() - }; - // Listen for changes in navigation state. - navigationService.addListener(setNavigation); + navigationService.addListener(navigateDirectlyToModel); // Also listen for changes which come from the tree. Changes in // the tree will trigger a change in browse navigation state. - $scope.$watch("treeModel.selectedObject", setNavigation); + $scope.$watch("treeModel.selectedObject", navigateDirectlyToModel); + + + // Listen for route changes which are caused by browser events + // (e.g. bookmarks to pages in OpenMCT) and prevent them. Instead, + // navigate to the path ourselves, which results in it being + // properly set. + $scope.$on('$routeChangeStart', function (event, route) { + if (route.$$route === $route.current.$$route && + route.pathParams.ids !== $route.current.pathParams.ids) { + event.preventDefault(); + navigateToPath(route.pathParams.ids.split('/')); + } + }); // Clean up when the scope is destroyed $scope.$on("$destroy", function () { - navigationService.removeListener(setNavigation); + navigationService.removeListener(navigateDirectlyToModel); }); } diff --git a/platform/commonUI/browse/src/BrowseObjectController.js b/platform/commonUI/browse/src/BrowseObjectController.js index 73640f7415..443d3ed594 100644 --- a/platform/commonUI/browse/src/BrowseObjectController.js +++ b/platform/commonUI/browse/src/BrowseObjectController.js @@ -51,24 +51,16 @@ define( } function updateQueryParam(viewKey) { - var unlisten, - priorRoute = $route.current; - - if (viewKey) { + if (viewKey && $location.search().view !== viewKey) { $location.search('view', viewKey); - unlisten = $scope.$on('$locationChangeSuccess', function () { - // Checks path to make sure /browse/ is at front - // if so, change $route.current - if ($location.path().indexOf("/browse/") === 0) { - $route.current = priorRoute; - } - unlisten(); - }); } } $scope.$watch('domainObject', setViewForDomainObject); $scope.$watch('representation.selected.key', updateQueryParam); + $scope.$on('$locationChangeSuccess', function () { + setViewForDomainObject($scope.domainObject); + }); $scope.doAction = function (action) { return $scope[action] && $scope[action](); diff --git a/platform/commonUI/browse/src/InspectorPaneController.js b/platform/commonUI/browse/src/InspectorPaneController.js index ef8e3883f7..dbefbc4d72 100644 --- a/platform/commonUI/browse/src/InspectorPaneController.js +++ b/platform/commonUI/browse/src/InspectorPaneController.js @@ -64,11 +64,11 @@ define( attachStatusListener(domainObject); } - var navigationListener = navigationService.addListener(attachStatusListener); + navigationService.addListener(attachStatusListener); $scope.$on("$destroy", function () { statusListener(); - navigationListener(); + navigationService.removeListener(attachStatusListener); }); } diff --git a/platform/commonUI/browse/test/BrowseControllerSpec.js b/platform/commonUI/browse/test/BrowseControllerSpec.js index c32232797e..18a59f0ea9 100644 --- a/platform/commonUI/browse/test/BrowseControllerSpec.js +++ b/platform/commonUI/browse/test/BrowseControllerSpec.js @@ -35,18 +35,17 @@ define( mockNavigationService, mockRootObject, mockUrlService, - mockDomainObject, + mockDefaultRootObject, + mockOtherDomainObject, mockNextObject, testDefaultRoot, - mockActionCapability, controller; - function mockPromise(value) { - return { - then: function (callback) { - return mockPromise(callback(value)); - } - }; + function waitsForNavigation() { + var calls = mockNavigationService.setNavigation.calls.length; + waitsFor(function () { + return mockNavigationService.setNavigation.calls.length > calls ; + }); } function instantiateController() { @@ -68,15 +67,27 @@ define( "$scope", ["$on", "$watch"] ); - mockRoute = { current: { params: {} } }; - mockLocation = jasmine.createSpyObj( - "$location", - ["path"] - ); + mockRoute = { current: { params: {}, pathParams: {} } }; mockUrlService = jasmine.createSpyObj( "urlService", ["urlForLocation"] ); + mockUrlService.urlForLocation.andCallFake(function (mode, object) { + if (object === mockDefaultRootObject) { + return [mode, testDefaultRoot].join('/'); + } + if (object === mockOtherDomainObject) { + return [mode, 'other'].join('/'); + } + if (object === mockNextObject) { + return [mode, testDefaultRoot, 'next'].join('/'); + } + throw new Error('Tried to get url for unexpected object'); + }); + mockLocation = jasmine.createSpyObj( + "$location", + ["path"] + ); mockObjectService = jasmine.createSpyObj( "objectService", ["getObjects"] @@ -91,62 +102,78 @@ define( ] ); mockRootObject = jasmine.createSpyObj( - "domainObject", - ["getId", "getCapability", "getModel", "useCapability"] + "rootObjectContainer", + ["getId", "getCapability", "getModel", "useCapability", "hasCapability"] ); - mockDomainObject = jasmine.createSpyObj( - "domainObject", - ["getId", "getCapability", "getModel", "useCapability"] + mockDefaultRootObject = jasmine.createSpyObj( + "defaultRootObject", + ["getId", "getCapability", "getModel", "useCapability", "hasCapability"] + ); + mockOtherDomainObject = jasmine.createSpyObj( + "otherDomainObject", + ["getId", "getCapability", "getModel", "useCapability", "hasCapability"] ); mockNextObject = jasmine.createSpyObj( - "nextObject", - ["getId", "getCapability", "getModel", "useCapability"] + "nestedDomainObject", + ["getId", "getCapability", "getModel", "useCapability", "hasCapability"] ); - - mockObjectService.getObjects.andReturn(mockPromise({ + mockObjectService.getObjects.andReturn(Promise.resolve({ ROOT: mockRootObject })); - mockRootObject.useCapability.andReturn(mockPromise([ - mockDomainObject + mockRootObject.useCapability.andReturn(Promise.resolve([ + mockOtherDomainObject, + mockDefaultRootObject ])); - mockDomainObject.useCapability.andReturn(mockPromise([ + mockRootObject.hasCapability.andReturn(true); + mockDefaultRootObject.useCapability.andReturn(Promise.resolve([ mockNextObject ])); + mockDefaultRootObject.hasCapability.andReturn(true); + mockOtherDomainObject.hasCapability.andReturn(false); mockNextObject.useCapability.andReturn(undefined); + mockNextObject.hasCapability.andReturn(false); mockNextObject.getId.andReturn("next"); - mockDomainObject.getId.andReturn(testDefaultRoot); - - mockActionCapability = jasmine.createSpyObj('actionCapability', ['perform']); + mockDefaultRootObject.getId.andReturn(testDefaultRoot); instantiateController(); + waitsForNavigation(); }); it("uses composition to set the navigated object, if there is none", function () { instantiateController(); - expect(mockNavigationService.setNavigation) - .toHaveBeenCalledWith(mockDomainObject); + waitsForNavigation(); + runs(function () { + expect(mockNavigationService.setNavigation) + .toHaveBeenCalledWith(mockDefaultRootObject); + }); }); it("navigates to a root-level object, even when default path is not found", function () { - mockDomainObject.getId + mockDefaultRootObject.getId .andReturn("something-other-than-the-" + testDefaultRoot); instantiateController(); - expect(mockNavigationService.setNavigation) - .toHaveBeenCalledWith(mockDomainObject); - }); + waitsForNavigation(); + runs(function () { + expect(mockNavigationService.setNavigation) + .toHaveBeenCalledWith(mockDefaultRootObject); + }); + + }); + // it("does not try to override navigation", function () { - mockNavigationService.getNavigation.andReturn(mockDomainObject); + mockNavigationService.getNavigation.andReturn(mockDefaultRootObject); instantiateController(); - expect(mockScope.navigatedObject).toBe(mockDomainObject); + waitsForNavigation(); + expect(mockScope.navigatedObject).toBe(mockDefaultRootObject); }); - + // it("updates scope when navigated object changes", function () { // Should have registered a listener - call it mockNavigationService.addListener.mostRecentCall.args[0]( - mockDomainObject + mockOtherDomainObject ); - expect(mockScope.navigatedObject).toEqual(mockDomainObject); + expect(mockScope.navigatedObject).toEqual(mockOtherDomainObject); }); @@ -166,9 +193,12 @@ define( it("uses route parameters to choose initially-navigated object", function () { mockRoute.current.params.ids = testDefaultRoot + "/next"; instantiateController(); - expect(mockScope.navigatedObject).toBe(mockNextObject); - expect(mockNavigationService.setNavigation) - .toHaveBeenCalledWith(mockNextObject); + waitsForNavigation(); + runs(function () { + expect(mockScope.navigatedObject).toBe(mockNextObject); + expect(mockNavigationService.setNavigation) + .toHaveBeenCalledWith(mockNextObject); + }); }); it("handles invalid IDs by going as far as possible", function () { @@ -177,9 +207,13 @@ define( // it hits an invalid ID. mockRoute.current.params.ids = testDefaultRoot + "/junk"; instantiateController(); - expect(mockScope.navigatedObject).toBe(mockDomainObject); - expect(mockNavigationService.setNavigation) - .toHaveBeenCalledWith(mockDomainObject); + waitsForNavigation(); + runs(function () { + expect(mockScope.navigatedObject).toBe(mockDefaultRootObject); + expect(mockNavigationService.setNavigation) + .toHaveBeenCalledWith(mockDefaultRootObject); + + }); }); it("handles compositionless objects by going as far as possible", function () { @@ -188,84 +222,33 @@ define( // should stop at it since remaining IDs cannot be loaded. mockRoute.current.params.ids = testDefaultRoot + "/next/junk"; instantiateController(); - expect(mockScope.navigatedObject).toBe(mockNextObject); - expect(mockNavigationService.setNavigation) - .toHaveBeenCalledWith(mockNextObject); + waitsForNavigation(); + runs(function () { + expect(mockScope.navigatedObject).toBe(mockNextObject); + expect(mockNavigationService.setNavigation) + .toHaveBeenCalledWith(mockNextObject); + }); }); it("updates the displayed route to reflect current navigation", function () { - var mockContext = jasmine.createSpyObj('context', ['getPath']), - mockUnlisten = jasmine.createSpy('unlisten'), - mockMode = "browse"; - - mockContext.getPath.andReturn( - [mockRootObject, mockDomainObject, mockNextObject] - ); - - //Return true from navigate action - mockActionCapability.perform.andReturn(mockPromise(true)); - - mockNextObject.getCapability.andCallFake(function (c) { - return (c === 'context' && mockContext) || - (c === 'action' && mockActionCapability); + // In order to trigger a route update and not a route change, + // the current route must be updated before location.path is + // called. + expect(mockRoute.current.pathParams.ids) + .not + .toBe(testDefaultRoot + '/next'); + mockLocation.path.andCallFake(function () { + expect(mockRoute.current.pathParams.ids) + .toBe(testDefaultRoot + '/next'); }); - mockScope.$on.andReturn(mockUnlisten); - // Provide a navigation change mockNavigationService.addListener.mostRecentCall.args[0]( mockNextObject ); - - // Allows the path index to be checked - // prior to setting $route.current - mockLocation.path.andReturn("/browse/"); - - mockNavigationService.setNavigation.andReturn(true); - mockActionCapability.perform.andReturn(mockPromise(true)); - - // Exercise the Angular workaround - mockNavigationService.addListener.mostRecentCall.args[0](); - mockScope.$on.mostRecentCall.args[1](); - expect(mockUnlisten).toHaveBeenCalled(); - - // location.path to be called with the urlService's - // urlFor function with the next domainObject and mode expect(mockLocation.path).toHaveBeenCalledWith( - mockUrlService.urlForLocation(mockMode, mockNextObject) + '/browse/' + testDefaultRoot + '/next' ); }); - it("after successful navigation event sets the selected tree " + - "object", function () { - mockScope.navigatedObject = mockDomainObject; - mockNavigationService.setNavigation.andReturn(true); - - mockActionCapability.perform.andReturn(mockPromise(true)); - mockNextObject.getCapability.andReturn(mockActionCapability); - - //Simulate a change in selected tree object - mockScope.treeModel = {selectedObject: mockDomainObject}; - mockScope.$watch.mostRecentCall.args[1](mockNextObject); - - expect(mockScope.treeModel.selectedObject).toBe(mockNextObject); - expect(mockScope.treeModel.selectedObject).not.toBe(mockDomainObject); - }); - - it("after failed navigation event resets the selected tree" + - " object", function () { - mockScope.navigatedObject = mockDomainObject; - - //Return false from navigation action - mockActionCapability.perform.andReturn(mockPromise(false)); - mockNextObject.getCapability.andReturn(mockActionCapability); - - //Simulate a change in selected tree object - mockScope.treeModel = {selectedObject: mockDomainObject}; - mockScope.$watch.mostRecentCall.args[1](mockNextObject); - - expect(mockScope.treeModel.selectedObject).not.toBe(mockNextObject); - expect(mockScope.treeModel.selectedObject).toBe(mockDomainObject); - }); - }); } ); diff --git a/platform/commonUI/browse/test/BrowseObjectControllerSpec.js b/platform/commonUI/browse/test/BrowseObjectControllerSpec.js index edcee97216..3d73febe2d 100644 --- a/platform/commonUI/browse/test/BrowseObjectControllerSpec.js +++ b/platform/commonUI/browse/test/BrowseObjectControllerSpec.js @@ -29,7 +29,6 @@ define( var mockScope, mockLocation, mockRoute, - mockUnlisten, controller; // Utility function; look for a $watch on scope and fire it @@ -51,9 +50,7 @@ define( "$location", ["path", "search"] ); - mockUnlisten = jasmine.createSpy("unlisten"); - - mockScope.$on.andReturn(mockUnlisten); + mockLocation.search.andReturn({}); controller = new BrowseObjectController( mockScope, @@ -69,10 +66,6 @@ define( // Allows the path index to be checked // prior to setting $route.current mockLocation.path.andReturn("/browse/"); - - // Exercise the Angular workaround - mockScope.$on.mostRecentCall.args[1](); - expect(mockUnlisten).toHaveBeenCalled(); }); it("sets the active view from query parameters", function () { diff --git a/platform/core/src/capabilities/InstantiationCapability.js b/platform/core/src/capabilities/InstantiationCapability.js index 646052dc57..81b2f954e4 100644 --- a/platform/core/src/capabilities/InstantiationCapability.js +++ b/platform/core/src/capabilities/InstantiationCapability.js @@ -68,7 +68,13 @@ define( this.instantiateFn = this.instantiateFn || this.$injector.get("instantiate"); - return this.instantiateFn(model, id); + var newObject = this.instantiateFn(model, id); + + this.contextualizeFn = this.contextualizeFn || + this.$injector.get("contextualize"); + + + return this.contextualizeFn(newObject, this.domainObject); }; /** diff --git a/platform/core/test/capabilities/InstantiationCapabilitySpec.js b/platform/core/test/capabilities/InstantiationCapabilitySpec.js index 686f211ca8..90c9341fcf 100644 --- a/platform/core/test/capabilities/InstantiationCapabilitySpec.js +++ b/platform/core/test/capabilities/InstantiationCapabilitySpec.js @@ -28,6 +28,7 @@ define( var mockInjector, mockIdentifierService, mockInstantiate, + mockContextualize, mockIdentifier, mockNow, mockDomainObject, @@ -36,6 +37,7 @@ define( beforeEach(function () { mockInjector = jasmine.createSpyObj("$injector", ["get"]); mockInstantiate = jasmine.createSpy("instantiate"); + mockContextualize = jasmine.createSpy("contextualize"); mockIdentifierService = jasmine.createSpyObj( 'identifierService', ['parse', 'generate'] @@ -50,8 +52,10 @@ define( ); mockInjector.get.andCallFake(function (key) { - return key === 'instantiate' ? - mockInstantiate : undefined; + return { + 'instantiate': mockInstantiate, + 'contextualize': mockContextualize + }[key]; }); mockIdentifierService.parse.andReturn(mockIdentifier); mockIdentifierService.generate.andReturn("some-id"); @@ -72,7 +76,7 @@ define( expect(instantiation.invoke).toBe(instantiation.instantiate); }); - it("uses the instantiate service to create domain objects", function () { + it("uses instantiate and contextualize to create domain objects", function () { var mockDomainObj = jasmine.createSpyObj('domainObject', [ 'getId', 'getModel', @@ -81,6 +85,9 @@ define( 'hasCapability' ]), testModel = { someKey: "some value" }; mockInstantiate.andReturn(mockDomainObj); + mockContextualize.andCallFake(function (x) { + return x; + }); expect(instantiation.instantiate(testModel)) .toBe(mockDomainObj); expect(mockInstantiate) @@ -88,6 +95,8 @@ define( someKey: "some value", modified: mockNow() }, jasmine.any(String)); + expect(mockContextualize) + .toHaveBeenCalledWith(mockDomainObj, mockDomainObject); }); }); diff --git a/platform/features/conductor/core/src/ui/TimeConductorController.js b/platform/features/conductor/core/src/ui/TimeConductorController.js index 10ede90d03..1d1fa937a0 100644 --- a/platform/features/conductor/core/src/ui/TimeConductorController.js +++ b/platform/features/conductor/core/src/ui/TimeConductorController.js @@ -96,6 +96,19 @@ define( this.conductor.on('timeSystem', this.changeTimeSystem); } + /** + * Used as a url search param setter in place of $location.search(...) + * + * Invokes $location.search(...) but prevents an Angular route + * change from occurring as a consequence which will cause + * controllers to reload and strangeness to ensue. + * + * @private + */ + TimeConductorController.prototype.setParam = function (name, value) { + this.$location.search(name, value); + }; + /** * @private */ @@ -185,8 +198,8 @@ define( this.setFormFromBounds(bounds); if (this.conductorViewService.mode() === 'fixed') { //Set bounds in URL on change - this.$location.search(SEARCH.START_BOUND, bounds.start); - this.$location.search(SEARCH.END_BOUND, bounds.end); + this.setParam(SEARCH.START_BOUND, bounds.start); + this.setParam(SEARCH.END_BOUND, bounds.end); } } }; @@ -299,8 +312,8 @@ define( this.conductorViewService.deltas(deltas); //Set Deltas in URL on change - this.$location.search(SEARCH.START_DELTA, deltas.start); - this.$location.search(SEARCH.END_DELTA, deltas.end); + this.setParam(SEARCH.START_DELTA, deltas.start); + this.setParam(SEARCH.END_DELTA, deltas.end); } }; @@ -315,23 +328,23 @@ define( */ TimeConductorController.prototype.setMode = function (newModeKey, oldModeKey) { //Set mode in URL on change - this.$location.search(SEARCH.MODE, newModeKey); + this.setParam(SEARCH.MODE, newModeKey); if (newModeKey !== oldModeKey) { this.conductorViewService.mode(newModeKey); this.setFormFromMode(newModeKey); if (newModeKey === "fixed") { - this.$location.search(SEARCH.START_DELTA, null); - this.$location.search(SEARCH.END_DELTA, null); + this.setParam(SEARCH.START_DELTA, undefined); + this.setParam(SEARCH.END_DELTA, undefined); } else { - this.$location.search(SEARCH.START_BOUND, null); - this.$location.search(SEARCH.END_BOUND, null); + this.setParam(SEARCH.START_BOUND, undefined); + this.setParam(SEARCH.END_BOUND, undefined); var deltas = this.conductorViewService.deltas(); if (deltas) { - this.$location.search(SEARCH.START_DELTA, deltas.start); - this.$location.search(SEARCH.END_DELTA, deltas.end); + this.setParam(SEARCH.START_DELTA, deltas.start); + this.setParam(SEARCH.END_DELTA, deltas.end); } } } @@ -363,7 +376,7 @@ define( */ TimeConductorController.prototype.changeTimeSystem = function (newTimeSystem) { //Set time system in URL on change - this.$location.search(SEARCH.TIME_SYSTEM, newTimeSystem.metadata.key); + this.setParam(SEARCH.TIME_SYSTEM, newTimeSystem.metadata.key); if (newTimeSystem && (newTimeSystem !== this.$scope.timeSystemModel.selected)) { this.setFormFromTimeSystem(newTimeSystem); diff --git a/platform/features/conductor/core/src/ui/TimeConductorControllerSpec.js b/platform/features/conductor/core/src/ui/TimeConductorControllerSpec.js index ef3a25a163..ffe0b6a893 100644 --- a/platform/features/conductor/core/src/ui/TimeConductorControllerSpec.js +++ b/platform/features/conductor/core/src/ui/TimeConductorControllerSpec.js @@ -37,6 +37,7 @@ define(['./TimeConductorController'], function (TimeConductorController) { "$watch", "$on" ]); + mockWindow = jasmine.createSpyObj("$window", ["requestAnimationFrame"]); mockTimeConductor = jasmine.createSpyObj( "TimeConductor", @@ -258,9 +259,7 @@ define(['./TimeConductorController'], function (TimeConductorController) { return mockTimeSystem; }; }); - }); - it("sets the mode on scope", function () { controller = new TimeConductorController( mockScope, mockWindow, @@ -270,7 +269,9 @@ define(['./TimeConductorController'], function (TimeConductorController) { mockTimeSystemConstructors, mockFormatService ); + }); + it("sets the mode on scope", function () { mockConductorViewService.availableTimeSystems.andReturn(mockTimeSystems); controller.setMode(mode); @@ -278,16 +279,6 @@ define(['./TimeConductorController'], function (TimeConductorController) { }); it("sets available time systems on scope when mode changes", function () { - controller = new TimeConductorController( - mockScope, - mockWindow, - mockLocation, - {conductor: mockTimeConductor}, - mockConductorViewService, - mockTimeSystemConstructors, - mockFormatService - ); - mockConductorViewService.availableTimeSystems.andReturn(mockTimeSystems); controller.setMode(mode); @@ -303,16 +294,6 @@ define(['./TimeConductorController'], function (TimeConductorController) { end: 10 }; - controller = new TimeConductorController( - mockScope, - mockWindow, - mockLocation, - {conductor: mockTimeConductor}, - mockConductorViewService, - mockTimeSystemConstructors, - mockFormatService - ); - controller.setBounds(formModel); expect(mockTimeConductor.bounds).toHaveBeenCalledWith(formModel); }); @@ -327,16 +308,6 @@ define(['./TimeConductorController'], function (TimeConductorController) { endDelta: deltas.end }; - controller = new TimeConductorController( - mockScope, - mockWindow, - mockLocation, - {conductor: mockTimeConductor}, - mockConductorViewService, - mockTimeSystemConstructors, - mockFormatService - ); - controller.setDeltas(formModel); expect(mockConductorViewService.deltas).toHaveBeenCalledWith(deltas); }); @@ -347,32 +318,17 @@ define(['./TimeConductorController'], function (TimeConductorController) { end: 6 }; var timeSystem = { - metadata: { - key: 'testTimeSystem' - }, - defaults: function () { - return { - bounds: defaultBounds - }; - } - }; - - mockTimeSystems = [ - // Wrap as constructor function - function () { - return timeSystem; + metadata: { + key: 'testTimeSystem' + }, + defaults: function () { + return { + bounds: defaultBounds + }; } - ]; + }; - controller = new TimeConductorController( - mockScope, - mockWindow, - mockLocation, - {conductor: mockTimeConductor}, - mockConductorViewService, - mockTimeSystems, - mockFormatService - ); + controller.timeSystems = [timeSystem]; controller.selectTimeSystemByKey('testTimeSystem'); expect(mockTimeConductor.timeSystem).toHaveBeenCalledWith(timeSystem, defaultBounds);