Merge pull request #489 from jamestalmage/remove-bug

Fix bug in $firebase.remove.

Author: katowulf

src/firebase.js

@@ -72,33 +72,36 @@
           },
 
           $remove: function (key) {
-            var ref = this._ref, self = this, promise;
+            var ref = this._ref, self = this;
             var def = $firebaseUtils.defer();
             if (arguments.length > 0) {
               ref = ref.ref().child(key);
             }
             if( angular.isFunction(ref.remove) ) {
               // self is not a query, just do a flat remove
               ref.remove(self._handle(def, ref));
-              promise = def.promise;
             }
             else {
-              var promises = [];
               // self is a query so let's only remove the
               // items in the query and not the entire path
               ref.once('value', function(snap) {
+                var promises = [];
                 snap.forEach(function(ss) {
                   var d = $firebaseUtils.defer();
-                  promises.push(d);
-                  ss.ref().remove(self._handle(d, ss.ref()));
+                  promises.push(d.promise);
+                  ss.ref().remove(self._handle(d));
                 }, self);
+                $firebaseUtils.allPromises(promises)
+                  .then(function() {
+                    def.resolve(ref);
+                  },
+                  function(err){
+                    def.reject(err);
+                  }
+                );
               });
-              promise = $firebaseUtils.allPromises(promises)
-                .then(function() {
-                  return ref;
-                });
             }
-            return promise;
+            return def.promise;
           },
 
           $update: function (key, data) {

tests/unit/firebase.spec.js

@@ -239,7 +239,8 @@ describe('$firebase', function () {
       var ref = new Firebase('Mock://').child('ordered').limit(2);
       var $fb = $firebase(ref);
       $fb.$remove().then(spy);
-      flushAll();
+      flushAll(ref);
+      flushAll(ref);
       expect(spy).toHaveBeenCalledWith(ref);
     });
 
@@ -305,6 +306,20 @@ describe('$firebase', function () {
         }
       });
     });
+
+    it('should wait to resolve promise until data is actually deleted',function(){
+      var ref = new Firebase('Mock://').child('ordered').limit(2);
+      var $fb = $firebase(ref);
+      var resolved = false;
+      $fb.$remove().then(function(){
+        resolved = true;
+      });
+      try {$timeout.flush();} catch(e){} //this may actually throw an error
+      expect(resolved).toBe(false);
+      flushAll(ref);
+      flushAll(ref);
+      expect(resolved).toBe(true);
+    });
   });
 
   describe('$update', function() {