Fix orderBy for fields containing Date

Internally date is now stored as Date until returned, where it is
converted to Timestamp. This fixes orderBy.

Author: op

package-lock.json

@@ -35,6 +35,7 @@
     "lodash.clone": "^4.5.0",
     "lodash.clonedeep": "^4.5.0",
     "lodash.clonedeepwith": "^4.5.0",
+    "lodash.clonewith": "^4.5.0",
     "lodash.compact": "^3.0.1",
     "lodash.difference": "^4.5.0",
     "lodash.every": "^4.6.0",

package.json

@@ -695,7 +695,7 @@ function extractName(path) {
 }
 
 function render(datum) {
-  if (datum && _.isPlainObject(datum)) {
+  if (datum && _.isObject(datum)) {
     var keys = _.keys(datum);
 
     if (_.every(keys, RegExp.prototype.test.bind(/^\d+$/))) {
@@ -714,10 +714,10 @@ function render(datum) {
         return array;
       }
     } else {
-      return _.cloneDeep(datum);
+      return _.cloneDeepWith(datum, utils.cloneCustomizer);
     }
   } else {
-    return _.clone(datum);
+    return _.cloneWith(datum, utils.cloneCustomizer);
   }
 }
 

src/firebase.js

@@ -1,13 +1,14 @@
 'use strict';
 
 var _ = require('./lodash');
+var utils = require('./utils');
 
 function MockFirestoreDocumentSnapshot (id, ref, data) {
   this.id = id;
   this.ref = ref;
   this._snapshotdata = _.cloneDeep(data) || null;
   this.data = function() {
-    return _.cloneDeep(this._snapshotdata);
+    return _.cloneDeepWith(this._snapshotdata, utils.cloneCustomizer);
   };
   this.exists = this._snapshotdata !== null;
 }

src/firestore-document-snapshot.js

@@ -111,8 +111,8 @@ MockFirestoreDocument.prototype.create = function (data, callback) {
       var base = self._getData();
       err = err || self._validateDoesNotExist(base);
       if (err === null) {
-        var time = Timestamp.fromMillis(utils.getServerTime());
-        var result = new WriteResult(time);
+        var timestamp = Timestamp.fromMillis(utils.getServerTime());
+        var result = new WriteResult(timestamp);
         data = utils.removeEmptyFirestoreProperties(data);
         self._dataChanged(data);
         resolve(result);

src/firestore-document.js

@@ -75,7 +75,7 @@ MockFirestoreQuery.prototype.get = function () {
           if (self.orderedProperties.length === 0) {
             _.forEach(self.data, function(data, key) {
               if (self.limited <= 0 || limit < self.limited) {
-                results[key] = _.cloneDeep(data);
+                results[key] = _.cloneDeepWith(data, utils.cloneCustomizer);
                 limit++;
               }
             });
@@ -92,7 +92,7 @@ MockFirestoreQuery.prototype.get = function () {
 
             queryable.forEach(function(q) {
               if (self.limited <= 0 || limit < self.limited) {
-                results[q.key] = _.cloneDeep(q.data);
+                results[q.key] = _.cloneDeepWith(q.data, utils.cloneCustomizer);
                 limit++;
               }
             });

src/firestore-query.js

@@ -2,6 +2,7 @@ module.exports = {
   assign: require('lodash.assign'),
   bind: require('lodash.bind'),
   clone: require('lodash.clone'),
+  cloneWith: require('lodash.clonewith'),
   cloneDeep: require('lodash.clonedeep'),
   cloneDeepWith: require('lodash.clonedeepwith'),
   compact: require('lodash.compact'),

src/lodash.js

@@ -138,9 +138,10 @@ exports.removeEmptyFirestoreProperties = function removeEmptyFirestoreProperties
       var value = removeEmptyFirestoreProperties(obj[s]);
       if (FieldValue.delete().isEqual(value)) {
         delete obj[s];
-      }
-      if (FieldValue.serverTimestamp().isEqual(value)) {
-        obj[s] = Timestamp.fromMillis(exports.getServerTime());
+      } else if (FieldValue.serverTimestamp().isEqual(value)) {
+        obj[s] = new Date(exports.getServerTime());
+      } else if (value instanceof Timestamp) {
+        obj[s] = value.toDate();
       }
     }
   }
@@ -223,3 +224,9 @@ exports.createThenableReference = function(reference, promise) {
   };
   return reference;
 };
+
+exports.cloneCustomizer = function(value) {
+  if (value instanceof Date) {
+    return Timestamp.fromMillis(value.getTime());
+  }
+};

src/utils.js

@@ -10,6 +10,7 @@ chai.use(require('sinon-chai'));
 var expect = chai.expect;
 var _ = require('../../src/lodash');
 var Firestore = require('../../').MockFirestore;
+var Timestamp = require('../../src/timestamp');
 
 describe('MockFirestoreCollection', function () {
 
@@ -332,6 +333,36 @@ describe('MockFirestoreCollection', function () {
         done();
       }).catch(done);
     });
+
+    it('returns documents ordered by date', function(done) {
+      db.collection('group').doc().create({
+        name: 'a',
+        date: Timestamp.fromMillis(1000)
+      }).catch(done);
+      db.flush();
+      db.collection('group').add({
+        name: 'b',
+        date: Timestamp.fromMillis(2000)
+      }).catch(done);
+      db.flush();
+
+      db.collection('group').orderBy('date', 'asc').get().then(function (snap) {
+        expect(snap.size).to.equal(2);
+        expect(snap.docs[0].data().name).to.equal('a');
+        expect(snap.docs[0].data().date).to.have.property('seconds');
+        expect(snap.docs[1].data().name).to.equal('b');
+        expect(snap.docs[1].data().date).to.have.property('seconds');
+
+        db.collection('group').orderBy('date', 'desc').get().then(function (snap) {
+          expect(snap.size).to.equal(2);
+          expect(snap.docs[0].data().name).to.equal('b');
+          expect(snap.docs[1].data().name).to.equal('a');
+          done();
+        }).catch(done);
+        db.flush();
+      }).catch(done);
+      db.flush();
+    });
   });
 
   describe('#limit', function () {

test/unit/firestore-collection.js

@@ -303,7 +303,7 @@ describe('MockFirestoreDocument', function () {
 
       doc.get().then(function (snap) {
         expect(snap.exists).to.equal(true);
-        expect(snap.get('date').getTime()).to.equal(nextDate.getTime());
+        expect(snap.get('date').toDate().getTime()).to.equal(nextDate.getTime());
         done();
       }).catch(done);
 
@@ -432,7 +432,7 @@ describe('MockFirestoreDocument', function () {
 
       doc.get()
         .then(function (snap) {
-          expect(snap.get('date').getTime()).to.equal(nextDate.getTime());
+          expect(snap.get('date').toDate().getTime()).to.equal(nextDate.getTime());
           done();
         })
         .catch(done);