From 4e5887d9ecdda7e46cc3243b214fa4e1d8c99504 Mon Sep 17 00:00:00 2001
From: Victor Woeltjen <victor.woeltjen@nasa.gov>
Date: Mon, 15 Aug 2016 15:14:17 -0700
Subject: [PATCH] [Tree] Check for change before scope.$apply

TreeView's observers will be called when the selected domain object
changes, which can occur for one of two reasons:

1. Because a new value was set externally, from mct-tree.
2. Because a new value was selected, by the user.

In the latter case a $apply is needed, but in the former it is not
(and causes an error.) However, when that case occurs, the value in
scope will be up to date already (it was a watch that triggered the
call to treeView.value) so no assignment or $apply is necessary.

Fixes #1114.
---
 .../general/src/directives/MCTTree.js         |  6 ++--
 .../general/test/directives/MCTTreeSpec.js    | 36 ++++++++++++++++++-
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/platform/commonUI/general/src/directives/MCTTree.js b/platform/commonUI/general/src/directives/MCTTree.js
index 987b733ba6..3b08691125 100644
--- a/platform/commonUI/general/src/directives/MCTTree.js
+++ b/platform/commonUI/general/src/directives/MCTTree.js
@@ -28,8 +28,10 @@ define([
         function link(scope, element) {
             var treeView = new TreeView(gestureService),
                 unobserve = treeView.observe(function (domainObject) {
-                    scope.mctModel = domainObject;
-                    scope.$apply();
+                    if (scope.mctModel !== domainObject) {
+                        scope.mctModel = domainObject;
+                        scope.$apply();
+                    }
                 });
 
             element.append(angular.element(treeView.elements()));
diff --git a/platform/commonUI/general/test/directives/MCTTreeSpec.js b/platform/commonUI/general/test/directives/MCTTreeSpec.js
index 0cc64949c2..bd8e17117a 100644
--- a/platform/commonUI/general/test/directives/MCTTreeSpec.js
+++ b/platform/commonUI/general/test/directives/MCTTreeSpec.js
@@ -29,6 +29,18 @@ define([
             mockExpr,
             mctTree;
 
+        function makeMockDomainObject(id) {
+            var mockDomainObject = jasmine.createSpyObj('domainObject-' + id, [
+                'getId',
+                'getModel',
+                'getCapability',
+                'hasCapability'
+            ]);
+            mockDomainObject.getId.andReturn(id);
+            mockDomainObject.getModel.andReturn({});
+            return mockDomainObject;
+        }
+
         beforeEach(function () {
             mockGestureService = jasmine.createSpyObj(
                 'gestureService',
@@ -56,7 +68,8 @@ define([
                 testAttrs;
 
             beforeEach(function () {
-                mockScope = jasmine.createSpyObj('$scope', ['$watch', '$on']);
+                mockScope =
+                    jasmine.createSpyObj('$scope', ['$watch', '$on', '$apply']);
                 mockElement = jasmine.createSpyObj('element', ['append']);
                 testAttrs = { mctModel: "some-expression" };
                 mockScope.$parent =
@@ -88,6 +101,27 @@ define([
                     jasmine.any(Function)
                 );
             });
+
+            // https://github.com/nasa/openmct/issues/1114
+            it("does not trigger $apply during $watches", function () {
+                mockScope.mctObject = makeMockDomainObject('root');
+                mockScope.mctMode = makeMockDomainObject('selection');
+                mockScope.$watch.calls.forEach(function (call) {
+                    call.args[1](mockScope[call.args[0]]);
+                });
+                expect(mockScope.$apply).not.toHaveBeenCalled();
+            });
+            it("does trigger $apply from other value changes", function () {
+                // White-boxy; we know this is the setter for the tree's value
+                var treeValueFn = mockScope.$watch.calls[0].args[1];
+
+                mockScope.mctObject = makeMockDomainObject('root');
+                mockScope.mctMode = makeMockDomainObject('selection');
+
+                treeValueFn(makeMockDomainObject('other'));
+
+                expect(mockScope.$apply).toHaveBeenCalled();
+            });
         });
     });