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

This problem is beset by the problem of `ngModel` expecting models  to be
atomic things (primitives/objects).

> When it was first invented it was expected that ngModel would only be
a primitive, e.g. a string or a number. Later when things like ngList and
ngOptions were added or became more complex then various hacks were put
in place to make it look like it worked well with those but it doesn't.

-------------

Just to be clear what is happening, lets name the objects:

```js
var option1 = { uid: 1, name: 'someName1' };
var option2 = { uid: 2, name: 'someName2' };
var option3 = { uid: 3, name: 'someName3' };

var initialItem = { uid: 1, name: 'someName1' };

model {
  options: [option1, option2, option3],
  selected: initialItem
};
```

Now when we begin we have:

```js
expect(model.selected).toBe(initialItem);
expect(model.selected.uid).toEqual(option1.uid);
expect(model.selected).not.toBe(option1);
```

So although `ngOptions` has found a match between an option and the
modelValue, these are not the same object.

Now if we change the properties of the `model.selected` object, we are
effectively changing the `initialItem` object.

```js
model.selected.uid = 3;
model.selected.name = 'someName3';

expect(model.selected).toBe(initialItem);
expect(model.selected.uid).toEqual(option3.uid);
expect(model.selected).not.toBe(option3);
```

At the moment `ngModel` only watches for changes to the object identity
and so it doesn't trigger an update to the `ngOptions` directive.

This commit fixes this in `ngOptions` by adding a **deep** watch on the
`attr.ngModel` expression...

```js
scope.$watch(attr.ngModel, updateOptions, true);
```

You can see that in this Plunker:
http://plnkr.co/edit/0PE7qN5FXIA23y4RwyN0?p=preview

-------

But this isn't the end of the story. Since `ngModel` and `ngOptions` did
not make copies between the model and the view, we can't go around just
changing the properties of the `model.selected` object. This is particularly
important in the situation where the user has actually chosen an option,
since the `model.selected` points directly to one of the option objects:

```js
// User selects "someName2" option
expect(model.selected).toBe(option2);
expect(model.selected.uid).toEqual(option2.uid);
expect(model.selected).not.toBe(initialOption);
```

If we now change the `model.selected` object's properties we are actually
changing the `option2` object:

```js
expect(model.selected).toBe(option2);

model.selected.uid = 3;
model.selected.name = 'someName3';

expect(model.selected).toBe(option2);
expect(model.selected).not.toBe(option3);

expect(option2.uid).toEqual(3);
expect(option2.name).toEqual('someName3');
```

which means that the options are now broken:

```js
expect(model.options).toEqual([
  { uid: 1, name: 'someName1' },
  { uid: 3, name: 'someName3' },
  { uid: 3, name: 'someName3' }
]);
```

This commit fixes this in `ngOptions` by making copies when reading the
value if `track by` is being used. If we are not using `track by` then
we really do care about the identity of the object and should not be
copying...

You can see this in the Plunker here:
http://plnkr.co/edit/YEzEf4dxHTnoW5pbeJDp?p=preview

Closes #10869
Closes #10893

Author: petebacondarwin

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

@@ -856,7 +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',
@@ -919,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']);
     });
 
 
@@ -998,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() {
@@ -1577,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
@@ -1591,7 +1671,7 @@ describe('ngOptions', function() {
         scope.values.pop();
       });
 
-      expect(element.val()).toEqualUnknownValue();
+      expect(element.val()).toEqual('');
       expect(scope.selected).toEqual(null);
     });