fix(tagsInput): Fix makeObjectArray function

Fix makeObjectArray function so it returns a new array instead of changing the provided one and messing with the model bounded to the tagsInput directive.
mbenford · Apr 9, 2015 · b5dc57f · b5dc57f
1 parent 32845a0
commit b5dc57f
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 21 deletions.
diff --git a/src/tags-input.js b/src/tags-input.js
@@ -203,7 +203,7 @@ tagsInput.directive('tagsInput', function($timeout, $document, $window, tagsInpu
                         input[0].focus();
                     },
                     getTags: function() {
-                        return $scope.tags;
+                        return $scope.tagList.items;
                     },
                     getCurrentTagText: function() {
                         return $scope.newTag.text;
@@ -242,9 +242,8 @@ tagsInput.directive('tagsInput', function($timeout, $document, $window, tagsInpu
                 setElementValidity;
 
             setElementValidity = function() {
-                var tagsLength = (scope.tags && scope.tags.length) || 0;
-                ngModelCtrl.$setValidity('maxTags', tagsLength <= options.maxTags);
-                ngModelCtrl.$setValidity('minTags', tagsLength >= options.minTags);
+                ngModelCtrl.$setValidity('maxTags', tagList.items.length <= options.maxTags);
+                ngModelCtrl.$setValidity('minTags', tagList.items.length >= options.minTags);
                 ngModelCtrl.$setValidity('leftoverText', scope.hasFocus || options.allowLeftoverText ? true : !scope.newTag.text);
             };
 
@@ -266,7 +265,13 @@ tagsInput.directive('tagsInput', function($timeout, $document, $window, tagsInpu
             };
 
             scope.$watch('tags', function(value) {
-                tagList.items = tiUtil.makeObjectArray(value, options.displayProperty);
+                if (value) {
+                    tagList.items = tiUtil.makeObjectArray(value, options.displayProperty);
+                    scope.tags = tagList.items;
+                }
+                else {
+                    tagList.items = [];
+                }
             });
 
             scope.$watch('tags.length', function() {

diff --git a/src/util.js b/src/util.js
@@ -21,14 +21,17 @@ tagsInput.factory('tiUtil', function($timeout) {
     };
 
     self.makeObjectArray = function(array, key) {
-        array = array || [];
-        if (array.length > 0 && !angular.isObject(array[0])) {
-            array.forEach(function(item, index) {
-                array[index] = {};
-                array[index][key] = item;
-            });
+        if (!angular.isArray(array) || array.length === 0 || angular.isObject(array[0])) {
+            return array;
         }
-        return array;
+
+        var newArray = [];
+        array.forEach(function(item) {
+            var obj = {};
+            obj[key] = item;
+            newArray.push(obj);
+        });
+        return newArray;
     };
 
     self.findInObjectArray = function(array, obj, key, comparer) {

diff --git a/test/tags-input.spec.js b/test/tags-input.spec.js
@@ -1883,6 +1883,26 @@ describe('tags-input directive', function() {
             expect(autocompleteObj.getTags()).toEqual([{ text: 'Tag1' }, { text: 'Tag2' }]);
         });
 
+        it('returns an empty list of tags when the model is undefined', function() {
+            // Arrange
+            $scope.$digest();
+
+            // Act/Assert
+            expect(autocompleteObj.getTags()).toEqual([]);
+        });
+
+        it('returns an empty list of tags when the model becomes undefined', function() {
+            // Arrange
+            $scope.tags = ['Tag1'];
+            $scope.$digest();
+
+            delete $scope.tags;
+            $scope.$digest();
+
+            // Act/Assert
+            expect(autocompleteObj.getTags()).toEqual([]);
+        });
+
         it('returns the current tag text', function() {
             // Arrange
             changeInputValue('ABC');

diff --git a/test/util.spec.js b/test/util.spec.js
@@ -93,21 +93,27 @@ describe('tiUtil factory', function() {
 
     describe('makeObjectArray', function() {
         it('converts a non-object array into an object array using the provided key', function() {
-            expect(tiUtil.makeObjectArray(['a', 'b', 'c'], 'prop')).toEqual([
+            var array = ['a', 'b', 'c'],
+                result = tiUtil.makeObjectArray(array, 'prop');
+
+            expect(result).not.toBe(array);
+            expect(result).toEqual([
                 { prop: 'a' },
                 { prop: 'b' },
                 { prop: 'c' }
             ]);
         });
 
-        it('returns the provided array if it is empty', function() {
-            var array = [];
-            expect(tiUtil.makeObjectArray(array, 'prop')).toBe(array);
-        });
-
-        it('returns the provided array if its first element is an object', function() {
-            var array = [{}];
-            expect(tiUtil.makeObjectArray(array, 'prop')).toBe(array);
+        [
+            { desc: 'an empty array', value: [] },
+            { desc: 'an array of objects', value: [{}] },
+            { desc: 'null', value: null },
+            { desc: 'a number', value: 1 },
+            { desc: 'a string', value: 'a' }
+        ].forEach(function(item) {
+            it('returns the provided argument itself if it is ' + item.desc, function() {
+                expect(tiUtil.makeObjectArray(item.value, 'prop')).toBe(item.value);
+            });
         });
     });