From 9fb29cce5ea5f67c43d2aa5926a4f7a8aa4ba81c Mon Sep 17 00:00:00 2001
From: c0bra <c0bra@users.noreply.github.com>
Date: Tue, 7 Oct 2014 09:00:00 -0500
Subject: [PATCH] fix(uiGridCell): Use promises for tmpl processing

Issue #1712 demonstrates a race condition where a column's
compiledElementFn is attempted to be called before the cellTemplate is
present, e.g. in the case of a template fetched over the network. This
causes the function to not be present and an exception to be thrown.

This changes fixes this by added a getCompiledElementFn() method to
GridColumn, which returns a promise that is created in the
defaultColumnBuilder(). It gets resolved in
Grid.preCompileCellTemplates(). uiGridCell now uses this promise if the
function is not present at pre-link time.

Added tests for getCompiledElementFn() and getCellTemplate()

Fixes #1712
---
 misc/demo/cellTemplate.html                 |   3 +
 misc/demo/grid-directive.html               |  14 ++-
 src/js/core/directives/ui-grid-cell.js      |  13 ++-
 src/js/core/factories/Grid.js               |   4 +
 src/js/core/factories/GridColumn.js         |  12 ++
 src/js/core/services/gridClassFactory.js    |   8 +-
 test/unit/core/factories/GridColumn.spec.js | 116 +++++++++++++++++++-
 7 files changed, 160 insertions(+), 10 deletions(-)
 create mode 100644 misc/demo/cellTemplate.html

diff --git a/misc/demo/cellTemplate.html b/misc/demo/cellTemplate.html
new file mode 100644
index 0000000000..ce7160826e
--- /dev/null
+++ b/misc/demo/cellTemplate.html
@@ -0,0 +1,3 @@
+<div class="ui-grid-cell-contents">
+    Testing $http template: {{COL_FIELD CUSTOM_FILTERS}}
+</div>
\ No newline at end of file
diff --git a/misc/demo/grid-directive.html b/misc/demo/grid-directive.html
index 9d5bfdeceb..9cb57699e8 100644
--- a/misc/demo/grid-directive.html
+++ b/misc/demo/grid-directive.html
@@ -46,12 +46,20 @@ <h2>Grid</h2>
 
       <script>
 
-      var app = angular.module('test', ['ui.grid', 'ui.grid.selection']);
+      var app = angular.module('test', ['ui.grid', 'ui.grid.selection', 'ui.grid.expandable']);
 
       app.controller('Main', function($scope, $http) {
           // $scope.gridOptions = { data: 'data' };
-          $scope.gridOptions = {};
-          // $scope.gridOptions.columnDefs = [ {name:"First Name", field:"firstName"} ];
+          $scope.gridOptions = {
+            enableFiltering: true
+          };
+          $scope.gridOptions.columnDefs = [
+            {
+              name : "Name",
+              field: "name",
+              cellTemplate: '/misc/demo/cellTemplate.html'
+            }
+          ];
 
           $http.get('/misc/site/data/100.json')
             .success(function (data) {
diff --git a/src/js/core/directives/ui-grid-cell.js b/src/js/core/directives/ui-grid-cell.js
index 8ff0aaf316..c0ad738047 100644
--- a/src/js/core/directives/ui-grid-cell.js
+++ b/src/js/core/directives/ui-grid-cell.js
@@ -21,11 +21,20 @@ angular.module('ui.grid').directive('uiGridCell', ['$compile', '$log', '$parse',
           // No controller, compile the element manually (for unit tests)
           else {
             if ( uiGridCtrl && !$scope.col.compiledElementFn ){
-              $log.error('Render has been called before precompile.  Please log a ui-grid issue');  
-            } else {
+              // $log.error('Render has been called before precompile.  Please log a ui-grid issue');  
+
+              $scope.col.getCompiledElementFn()
+                .then(function (compiledElementFn) {
+                  compiledElementFn($scope, function(clonedElement, scope) {
+                    $elm.append(clonedElement);
+                  });
+                });
+            }
+            else {
               var html = $scope.col.cellTemplate
                 .replace(uiGridConstants.MODEL_COL_FIELD, 'row.entity.' + gridUtil.preEval($scope.col.field))
                 .replace(uiGridConstants.COL_FIELD, 'grid.getCellValue(row, col)');
+
               var cellElement = $compile(html)($scope);
               $elm.append(cellElement);
             }
diff --git a/src/js/core/factories/Grid.js b/src/js/core/factories/Grid.js
index 20cb959e48..dcf10af7ae 100644
--- a/src/js/core/factories/Grid.js
+++ b/src/js/core/factories/Grid.js
@@ -429,6 +429,10 @@ angular.module('ui.grid')
 
       var compiledElementFn = $compile(html);
       col.compiledElementFn = compiledElementFn;
+
+      if (col.compiledElementFnDefer) {
+        col.compiledElementFnDefer.resolve(col.compiledElementFn);
+      }
     });
   };
 
diff --git a/src/js/core/factories/GridColumn.js b/src/js/core/factories/GridColumn.js
index 3e89f901d1..3418873162 100644
--- a/src/js/core/factories/GridColumn.js
+++ b/src/js/core/factories/GridColumn.js
@@ -661,6 +661,18 @@ angular.module('ui.grid')
       }
     };
 
+    GridColumn.prototype.getCellTemplate = function () {
+      var self = this;
+
+      return self.cellTemplatePromise;
+    };
+
+    GridColumn.prototype.getCompiledElementFn = function () {
+      var self = this;
+      
+      return self.compiledElementFnDefer.promise;
+    };
+
     return GridColumn;
   }]);
 
diff --git a/src/js/core/services/gridClassFactory.js b/src/js/core/services/gridClassFactory.js
index a703a38b7b..4747b5ad0b 100644
--- a/src/js/core/services/gridClassFactory.js
+++ b/src/js/core/services/gridClassFactory.js
@@ -124,9 +124,11 @@
            */
           if (!colDef.cellTemplate) {
             colDef.cellTemplate = 'ui-grid/uiGridCell';
+            col.cellTemplatePromise = gridUtil.getTemplate(colDef.cellTemplate);
           }
 
-          templateGetPromises.push(gridUtil.getTemplate(colDef.cellTemplate)
+          col.cellTemplatePromise = gridUtil.getTemplate(colDef.cellTemplate);
+          templateGetPromises.push(col.cellTemplatePromise
             .then(
               function (template) {
                 col.cellTemplate = template.replace(uiGridConstants.CUSTOM_FILTERS, col.cellFilter ? "|" + col.cellFilter : "");
@@ -136,7 +138,6 @@
               })
           );
 
-
           templateGetPromises.push(gridUtil.getTemplate(colDef.headerCellTemplate)
               .then(
               function (template) {
@@ -147,6 +148,9 @@
               })
           );
 
+          // Create a promise for the compiled element function
+          col.compiledElementFnDefer = $q.defer();
+
           return $q.all(templateGetPromises);
         }
 
diff --git a/test/unit/core/factories/GridColumn.spec.js b/test/unit/core/factories/GridColumn.spec.js
index e1179081f4..d657839fd7 100644
--- a/test/unit/core/factories/GridColumn.spec.js
+++ b/test/unit/core/factories/GridColumn.spec.js
@@ -1,14 +1,14 @@
 describe('GridColumn factory', function () {
-  var $q, $scope, cols, grid, gridCol, Grid, GridColumn, gridClassFactory, GridRenderContainer, uiGridConstants;
+  var $q, $scope, cols, grid, gridCol, Grid, GridColumn, gridClassFactory, GridRenderContainer, uiGridConstants, $httpBackend;
 
   beforeEach(module('ui.grid'));
 
   function buildCols() {
-    grid.buildColumns();
+    return grid.buildColumns();
   }
 
 
-  beforeEach(inject(function (_$q_, _$rootScope_, _Grid_, _GridColumn_, _gridClassFactory_, _GridRenderContainer_, _uiGridConstants_) {
+  beforeEach(inject(function (_$q_, _$rootScope_, _Grid_, _GridColumn_, _gridClassFactory_, _GridRenderContainer_, _uiGridConstants_, _$httpBackend_) {
     $q = _$q_;
     $scope = _$rootScope_;
     Grid = _Grid_;
@@ -16,6 +16,7 @@ describe('GridColumn factory', function () {
     gridClassFactory = _gridClassFactory_;
     GridRenderContainer = _GridRenderContainer_;
     uiGridConstants = _uiGridConstants_;
+    $httpBackend = _$httpBackend_;
 
     cols = [
       { field: 'firstName' }
@@ -220,4 +221,113 @@ describe('GridColumn factory', function () {
     });
   });
 
+  describe('getCompiledElementFn()', function () {
+    var col;
+
+    beforeEach(function () {
+      col = grid.columns[0];
+    });
+
+    it('should return a promise', function () {
+      expect(col.getCompiledElementFn().hasOwnProperty('then')).toBe(true);
+    });
+
+    it('should return a promise that is resolved when the cellTemplate is compiled', function () {
+      var resolved = false;
+
+      runs(function () {
+        buildCols().then(function () {
+          grid.preCompileCellTemplates();
+        });
+      });
+
+      runs(function () {
+        col.getCompiledElementFn().then(function () {
+          resolved = true;
+        });
+
+        $scope.$digest();
+      });
+
+      // $scope.$digest();
+
+      runs(function () {
+        expect(resolved).toBe(true);
+      });
+    });
+
+    it('should return a promise that is resolved when a URL-based cellTemplate is available', function () {
+      var resolved = false;
+
+      var url = 'http://www.a-really-fake-url.com/template.html';
+      cols[0].cellTemplate = url;
+
+      $httpBackend.when('GET', url).respond('<div>foo</div>');
+
+      runs(function () {
+        buildCols().then(function () {
+          grid.preCompileCellTemplates();
+        });
+
+        col.getCompiledElementFn().then(function () {
+          resolved = true;
+        });
+
+        expect(resolved).toBe(false);
+
+        $httpBackend.flush();
+      });
+
+      runs(function () {
+        $scope.$digest();
+      });
+
+      runs(function () {
+        expect(resolved).toBe(true);
+      });
+    });
+  });
+
+  describe('getCellTemplate()', function () {
+    var col;
+
+    beforeEach(function () {
+      col = grid.columns[0];
+    });
+
+    it('should return a promise', function () {
+      expect(col.getCellTemplate().hasOwnProperty('then')).toBe(true);
+    });
+
+    it('should return a promise that is resolved when a URL-based cellTemplate is available', function () {
+      var resolved = false;
+
+      var url = 'http://www.a-really-fake-url.com/template.html';
+      cols[0].cellTemplate = url;
+
+      $httpBackend.when('GET', url).respond('<div>foo</div>');
+
+      runs(function () {
+        buildCols().then(function () {
+          grid.preCompileCellTemplates();
+        });
+
+        col.getCellTemplate().then(function () {
+          resolved = true;
+        });
+
+        expect(resolved).toBe(false);
+
+        $httpBackend.flush();
+      });
+
+      runs(function () {
+        $scope.$digest();
+      });
+
+      runs(function () {
+        expect(resolved).toBe(true);
+      });
+    });
+  });
 });
\ No newline at end of file