From 34b68ef5e84155ca4a7f3bcc7e375e447ad3a2a2 Mon Sep 17 00:00:00 2001
From: Jeff Cross <middlefloor@gmail.com>
Date: Tue, 21 Oct 2014 05:40:43 +0200
Subject: [PATCH] fix(select): assign result of track exp to element value

Fixes a regression where the option/select values would always be set to
the key or index of a value within the corresponding collection. Prior to
some 1.3.0 refactoring, the result of the track expression would be bound
to the value, but this behavior was not documented or explicitly tested.

This commit adds one explicit test for this behavior, and changes
several other trackBy tests to reflect the desired behavior as well.

Fixes #9592
---
 src/ng/directive/select.js      | 22 ++++++--
 test/ng/directive/selectSpec.js | 91 ++++++++++++++++++++-------------
 2 files changed, 73 insertions(+), 40 deletions(-)

diff --git a/src/ng/directive/select.js b/src/ng/directive/select.js
index 2f3266b2a131..9ed6d30656cc 100644
--- a/src/ng/directive/select.js
+++ b/src/ng/directive/select.js
@@ -38,7 +38,8 @@ var ngOptionsMinErr = minErr('ngOptions');
  * <div class="alert alert-info">
  * **Note:** Using `select as` will bind the result of the `select as` expression to the model, but
  * the value of the `<select>` and `<option>` html elements will be either the index (for array data sources)
- * or property name (for object data sources) of the value within the collection.
+ * or property name (for object data sources) of the value within the collection. If a `track by` expression
+ * is used, the result of that expression will be set as the value of the `option` and `select` elements.
  * </div>
  *
  * **Note:** Using `select as` together with `trackexpr` is not recommended.
@@ -407,24 +408,35 @@ var selectDirective = ['$compile', '$parse', function($compile,   $parse) {
             if (multiple) {
               viewValue = [];
               forEach(selectElement.val(), function(selectedKey) {
-                viewValue.push(getViewValue(selectedKey, collection[selectedKey]));
+                viewValue.push(getViewValue(selectedKey, collection));
               });
             } else {
               var selectedKey = selectElement.val();
-              viewValue = getViewValue(selectedKey, collection[selectedKey]);
+              viewValue = getViewValue(selectedKey, collection);
             }
             ctrl.$setViewValue(viewValue);
             render();
           });
         }
 
-        function getViewValue(key, value) {
+        function getViewValue(key, collection) {
           if (key === '?') {
             return undefined;
           } else if (key === '') {
             return null;
           } else {
             var viewValueFn = selectAsFn ? selectAsFn : valueFn;
+            var result;
+            var value = collection[key];
+            if (trackFn) {
+              for (var i in collection) {
+                result = callExpression(trackFn, key, collection[i]);
+                if (result.toString() === key) {
+                  value = collection[i];
+                  break;
+                }
+              }
+            }
             return callExpression(viewValueFn, key, value);
           }
         }
@@ -558,7 +570,7 @@ var selectDirective = ['$compile', '$parse', function($compile,   $parse) {
             label = isDefined(label) ? label : '';
             optionGroup.push({
               // either the index into array or key from object
-              id: (keyName ? keys[index] : index),
+              id: trackFn? trackFn(scope, locals) : (keyName ? keys[index] : index),
               label: label,
               selected: selected                   // determine if we should be selected
             });
diff --git a/test/ng/directive/selectSpec.js b/test/ng/directive/selectSpec.js
index 30ad415b8423..62c388e8cc47 100644
--- a/test/ng/directive/selectSpec.js
+++ b/test/ng/directive/selectSpec.js
@@ -722,7 +722,29 @@ describe('select', function() {
     describe('trackBy expression', function() {
       beforeEach(function() {
         scope.arr = [{id: 10, label: 'ten'}, {id:20, label: 'twenty'}];
-        scope.obj = {'10': {score: 10, label: 'ten'}, '20': {score: 20, label: 'twenty'}};
+        scope.obj = {'1': {score: 10, label: 'ten'}, '2': {score: 20, label: 'twenty'}};
+      });
+
+
+      it('should set the result of track by expression to element value', function() {
+        createSelect({
+          'ng-model': 'selected',
+          'ng-options': 'item.label for item in arr track by item.id'
+        });
+
+        scope.$apply(function() {
+          scope.selected = scope.arr[0];
+        });
+        expect(element.val()).toBe('10');
+
+        scope.$apply(function() {
+          scope.arr[0] = {id: 10, label: 'new ten'};
+        });
+        expect(element.val()).toBe('10');
+
+        element.children()[1].selected = 'selected';
+        browserTrigger(element, 'change');
+        expect(scope.selected).toEqual(scope.arr[1]);
       });
 
 
@@ -735,14 +757,14 @@ describe('select', function() {
         scope.$apply(function() {
           scope.selected = scope.arr[0];
         });
-        expect(element.val()).toBe('0');
+        expect(element.val()).toBe('10');
 
         scope.$apply(function() {
           scope.arr[0] = {id: 10, label: 'new ten'};
         });
-        expect(element.val()).toBe('0');
+        expect(element.val()).toBe('10');
 
-        element.children()[1].selected = 1;
+        element.children()[1].selected = 'selected';
         browserTrigger(element, 'change');
         expect(scope.selected).toEqual(scope.arr[1]);
       });
@@ -758,12 +780,12 @@ describe('select', function() {
         scope.$apply(function() {
           scope.selected = scope.arr;
         });
-        expect(element.val()).toEqual(['0','1']);
+        expect(element.val()).toEqual(['10','20']);
 
         scope.$apply(function() {
           scope.arr[0] = {id: 10, label: 'new ten'};
         });
-        expect(element.val()).toEqual(['0','1']);
+        expect(element.val()).toEqual(['10','20']);
 
         element.children()[0].selected = false;
         browserTrigger(element, 'change');
@@ -778,18 +800,18 @@ describe('select', function() {
         });
 
         scope.$apply(function() {
-          scope.selected = scope.obj['10'];
+          scope.selected = scope.obj['1'];
         });
         expect(element.val()).toBe('10');
 
         scope.$apply(function() {
-          scope.obj['10'] = {score: 10, label: 'ten'};
+          scope.obj['1'] = {score: 10, label: 'ten'};
         });
         expect(element.val()).toBe('10');
 
         element.val('20');
         browserTrigger(element, 'change');
-        expect(scope.selected).toBe(scope.obj[20]);
+        expect(scope.selected).toBe(scope.obj['2']);
       });
 
 
@@ -801,18 +823,18 @@ describe('select', function() {
         });
 
         scope.$apply(function() {
-          scope.selected = [scope.obj['10']];
+          scope.selected = [scope.obj['1']];
         });
         expect(element.val()).toEqual(['10']);
 
         scope.$apply(function() {
-          scope.obj['10'] = {score: 10, label: 'ten'};
+          scope.obj['1'] = {score: 10, label: 'ten'};
         });
         expect(element.val()).toEqual(['10']);
 
         element.children()[1].selected = 'selected';
         browserTrigger(element, 'change');
-        expect(scope.selected).toEqual([scope.obj[10], scope.obj[20]]);
+        expect(scope.selected).toEqual([scope.obj['1'], scope.obj['2']]);
       });
     });
 
@@ -824,7 +846,7 @@ describe('select', function() {
     describe('selectAs+trackBy expression', function() {
       beforeEach(function() {
         scope.arr = [{subItem: {label: 'ten', id: 10}}, {subItem: {label: 'twenty', id: 20}}];
-        scope.obj = {'10': {subItem: {id: 10, label: 'ten'}}, '20': {subItem: {id: 20, label: 'twenty'}}};
+        scope.obj = {'1': {subItem: {id: 10, label: 'ten'}}, '2': {subItem: {id: 20, label: 'twenty'}}};
       });
 
 
@@ -840,16 +862,16 @@ describe('select', function() {
         scope.$apply(function() {
           scope.selected = scope.arr[0].subItem;
         });
-        expect(element.val()).toEqual('0');
+        expect(element.val()).toEqual('10');
 
         scope.$apply(function() {
           scope.selected = scope.arr[1].subItem;
         });
-        expect(element.val()).toEqual('1');
+        expect(element.val()).toEqual('20');
 
         // Now test view -> model
 
-        element.val('0');
+        element.val('10');
         browserTrigger(element, 'change');
         expect(scope.selected).toBe(scope.arr[0].subItem);
 
@@ -861,7 +883,7 @@ describe('select', function() {
             subItem: {label: 'new twenty', id: 20}
           }];
         });
-        expect(element.val()).toBe('0');
+        expect(element.val()).toBe('10');
         expect(scope.selected.id).toBe(10);
       });
 
@@ -879,12 +901,12 @@ describe('select', function() {
         scope.$apply(function() {
           scope.selected = [scope.arr[0].subItem];
         });
-        expect(element.val()).toEqual(['0']);
+        expect(element.val()).toEqual(['10']);
 
         scope.$apply(function() {
           scope.selected = [scope.arr[1].subItem];
         });
-        expect(element.val()).toEqual(['1']);
+        expect(element.val()).toEqual(['20']);
 
         // Now test view -> model
 
@@ -901,7 +923,7 @@ describe('select', function() {
             subItem: {label: 'new twenty', id: 20}
           }];
         });
-        expect(element.val()).toEqual(['0']);
+        expect(element.val()).toEqual(['10']);
         expect(scope.selected[0].id).toEqual(10);
         expect(scope.selected.length).toBe(1);
       });
@@ -918,13 +940,12 @@ describe('select', function() {
         // First test model -> view
 
         scope.$apply(function() {
-          scope.selected = [scope.obj['10'].subItem];
+          scope.selected = [scope.obj['1'].subItem];
         });
         expect(element.val()).toEqual(['10']);
 
-
         scope.$apply(function() {
-          scope.selected = [scope.obj['10'].subItem];
+          scope.selected = [scope.obj['1'].subItem];
         });
         expect(element.val()).toEqual(['10']);
 
@@ -933,15 +954,15 @@ describe('select', function() {
         element.find('option')[0].selected = true;
         element.find('option')[1].selected = false;
         browserTrigger(element, 'change');
-        expect(scope.selected).toEqual([scope.obj['10'].subItem]);
+        expect(scope.selected).toEqual([scope.obj['1'].subItem]);
 
         // Now reload the object
         scope.$apply(function() {
           scope.obj = {
-            '10': {
+            '1': {
               subItem: {label: 'new ten', id: 10}
             },
-            '20': {
+            '2': {
               subItem: {label: 'new twenty', id: 20}
             }
           };
@@ -962,13 +983,13 @@ describe('select', function() {
         // First test model -> view
 
         scope.$apply(function() {
-          scope.selected = scope.obj['10'].subItem;
+          scope.selected = scope.obj['1'].subItem;
         });
         expect(element.val()).toEqual('10');
 
 
         scope.$apply(function() {
-          scope.selected = scope.obj['10'].subItem;
+          scope.selected = scope.obj['1'].subItem;
         });
         expect(element.val()).toEqual('10');
 
@@ -976,15 +997,15 @@ describe('select', function() {
 
         element.find('option')[0].selected = true;
         browserTrigger(element, 'change');
-        expect(scope.selected).toEqual(scope.obj['10'].subItem);
+        expect(scope.selected).toEqual(scope.obj['1'].subItem);
 
         // Now reload the object
         scope.$apply(function() {
           scope.obj = {
-            '10': {
+            '1': {
               subItem: {label: 'new ten', id: 10}
             },
-            '20': {
+            '2': {
               subItem: {label: 'new twenty', id: 20}
             }
           };
@@ -1338,20 +1359,20 @@ describe('select', function() {
           scope.selected = scope.values[1];
         });
 
-        expect(element.val()).toEqual('1');
+        expect(element.val()).toEqual('2');
 
         var first = jqLite(element.find('option')[0]);
         expect(first.text()).toEqual('first');
-        expect(first.attr('value')).toEqual('0');
+        expect(first.attr('value')).toEqual('1');
         var forth = jqLite(element.find('option')[3]);
         expect(forth.text()).toEqual('forth');
-        expect(forth.attr('value')).toEqual('3');
+        expect(forth.attr('value')).toEqual('4');
 
         scope.$apply(function() {
           scope.selected = scope.values[3];
         });
 
-        expect(element.val()).toEqual('3');
+        expect(element.val()).toEqual('4');
       });