fix(ngOptions): fix model<->option interaction when using track by

src/ng/directive/ngOptions.js

@@ -335,6 +335,11 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
           selectValueMap: selectValueMap,
           getOptionFromViewValue: function(value) {
             return selectValueMap[getTrackByValue(value, getLocals(value))];
+          },
+          getViewValueFromOption: function(option) {
+            // If the viewValue could be an object that may be mutated by the application,
+            // we need to make a copy and not return the reference to the value on the option.
+            return trackBy ? angular.copy(option.viewValue) : option.viewValue;
           }
         };
       }
@@ -428,7 +433,7 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
         if (selectedOption && !selectedOption.disabled) {
           removeEmptyOption();
           removeUnknownOption();
-          return selectedOption.viewValue;
+          return options.getViewValueFromOption(selectedOption);
         }
         return null;
       };
@@ -462,7 +467,7 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
 
           forEach(selectedValues, function(value) {
             var option = options.selectValueMap[value];
-            if (!option.disabled) selections.push(option.viewValue);
+            if (!option.disabled) selections.push(options.getViewValueFromOption(option));
           });
 
           return selections;
@@ -493,6 +498,10 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
       // We will re-render the option elements if the option values or labels change
       scope.$watchCollection(ngOptions.getWatchables, updateOptions);
 
+      // We also need to watch to see if the internals of the model changes, since
+      // ngModel only watches for object identity change
+      scope.$watch(attr.ngModel, function() { ngModelCtrl.$render(); }, true);
+
       // ------------------------------------------------------------------ //
 
 

test/ng/directive/ngOptionsSpec.js

@@ -362,6 +362,7 @@ describe('ngOptions', function() {
     expect(options.eq(2)).toEqualOption(scope.values[2], 'D');
   });
 
+
   it('should preserve pre-existing empty option', function() {
     createSingleSelect(true);
 
@@ -855,6 +856,87 @@ describe('ngOptions', function() {
       expect(options.eq(2)).toEqualTrackedOption(20, 'twenty');
     });
 
+
+    it('should update the selected option even if only the tracked property on the selected object changes (single)', function() {
+      createSelect({
+        'ng-model': 'selected',
+        'ng-options': 'item.label for item in arr track by item.id'
+      });
+
+      scope.$apply(function() {
+        scope.selected = {id: 10, label: 'ten'};
+      });
+
+      expect(element.val()).toEqual('10');
+
+      // Update the properties on the selected object, rather than replacing the whole object
+      scope.$apply(function() {
+        scope.selected.id = 20;
+        scope.selected.label = 'new twenty';
+      });
+
+      // The value of the select should change since the id property changed
+      expect(element.val()).toEqual('20');
+
+      // But the label of the selected option does not change
+      var option = element.find('option').eq(1);
+      expect(option.prop('selected')).toEqual(true);
+      expect(option.text()).toEqual('twenty'); // not 'new twenty'
+    });
+
+
+    it('should update the selected options even if only the tracked properties on the objects in the ' +
+        'selected collection change (multi)', function() {
+      createSelect({
+        'ng-model': 'selected',
+        'multiple': true,
+        'ng-options': 'item.label for item in arr track by item.id'
+      });
+
+      scope.$apply(function() {
+        scope.selected = [{id: 10, label: 'ten'}];
+      });
+
+      expect(element.val()).toEqual(['10']);
+
+      // Update the properties on the object in the selected array, rather than replacing the whole object
+      scope.$apply(function() {
+        scope.selected[0].id = 20;
+        scope.selected[0].label = 'new twenty';
+      });
+
+      // The value of the select should change since the id property changed
+      expect(element.val()).toEqual(['20']);
+
+      // But the label of the selected option does not change
+      var option = element.find('option').eq(1);
+      expect(option.prop('selected')).toEqual(true);
+      expect(option.text()).toEqual('twenty'); // not 'new twenty'
+    });
+
+
+    it('should prevent changes to the selected object from modifying the options objects (single)', function() {
+
+      createSelect({
+        'ng-model': 'selected',
+        'ng-options': 'item.label for item in arr track by item.id'
+      });
+
+      element.val('10');
+      browserTrigger(element, 'change');
+
+      expect(scope.selected).toEqual(scope.arr[0]);
+
+      scope.$apply(function() {
+        scope.selected.id = 20;
+      });
+
+      expect(scope.selected).not.toEqual(scope.arr[0]);
+      expect(element.val()).toEqual('20');
+      expect(scope.arr).toEqual([{id: 10, label: 'ten'}, {id:20, label: 'twenty'}]);
+    });
+
+
     it('should preserve value even when reference has changed (single&array)', function() {
       createSelect({
         'ng-model': 'selected',
@@ -917,7 +999,7 @@ describe('ngOptions', function() {
       expect(element.val()).toBe('10');
 
       setSelectValue(element, 1);
-      expect(scope.selected).toBe(scope.obj['2']);
+      expect(scope.selected).toEqual(scope.obj['2']);
     });
 
 
@@ -996,7 +1078,7 @@ describe('ngOptions', function() {
 
       element.val('10');
       browserTrigger(element, 'change');
-      expect(scope.selected).toBe(scope.arr[0].subItem);
+      expect(scope.selected).toEqual(scope.arr[0].subItem);
 
       // Now reload the array
       scope.$apply(function() {
@@ -1575,7 +1657,7 @@ describe('ngOptions', function() {
         scope.values.pop();
       });
 
-      expect(element.val()).toEqualUnknownValue();
+      expect(element.val()).toEqual('');
       expect(scope.selected).toEqual(null);
 
       // Check after model change
@@ -1589,7 +1671,7 @@ describe('ngOptions', function() {
         scope.values.pop();
       });
 
-      expect(element.val()).toEqualUnknownValue();
+      expect(element.val()).toEqual('');
       expect(scope.selected).toEqual(null);
     });