Merge pull request #1976 from overleaf/hb-v1-history-metadata-from-mongo

Fetch history metadata for v1 users

GitOrigin-RevId: a65971dbef2be02c17ff07b1bf22c0f7b15c1780

services/web/app/src/Features/History/HistoryManager.js

@@ -130,7 +130,7 @@ module.exports = HistoryManager = {
     // 	diff: [{
     // 		i: "foo",
     // 		meta: {
-    // 			users: ["user_id", { first_name: "James", ... }, ...]
+    // 			users: ["user_id", v1_user_id, ...]
     // 			...
     // 		}
     // 	}, ...]
@@ -140,7 +140,7 @@ module.exports = HistoryManager = {
     // 	updates: [{
     // 		pathnames: ["main.tex"]
     // 		meta: {
-    // 			users: ["user_id", { first_name: "James", ... }, ...]
+    // 			users: ["user_id", v1_user_id, ...]
     // 			...
     // 		},
     // 		...
@@ -148,50 +148,65 @@ module.exports = HistoryManager = {
     // }
     // Either way, the top level key points to an array of objects with a meta.users property
     // that we need to replace user_ids with populated user objects.
-    // Note that some entries in the users arrays may already have user objects from the v1 history
-    // service
+    // Note that some entries in the users arrays may be v1 ids returned by the v1 history
+    // service. v1 ids will be `numbers`
     let entry, user
     if (callback == null) {
       callback = function(error, data_with_users) {}
     }
     let user_ids = new Set()
+    let v1_user_ids = new Set()
     for (entry of Array.from(data.diff || data.updates || [])) {
       for (user of Array.from(
         (entry.meta != null ? entry.meta.users : undefined) || []
       )) {
         if (typeof user === 'string') {
           user_ids.add(user)
+        } else if (typeof user === 'number') {
+          v1_user_ids.add(user)
         }
       }
     }
     user_ids = Array.from(user_ids)
-    return UserGetter.getUsers(
-      user_ids,
-      { first_name: 1, last_name: 1, email: 1 },
-      function(error, users_array) {
-        if (error != null) {
-          return callback(error)
-        }
-        const users = {}
-        for (user of Array.from(users_array || [])) {
-          users[user._id.toString()] = HistoryManager._userView(user)
-        }
-        for (entry of Array.from(data.diff || data.updates || [])) {
-          if (entry.meta != null) {
-            entry.meta.users = (
-              (entry.meta != null ? entry.meta.users : undefined) || []
-            ).map(function(user) {
-              if (typeof user === 'string') {
-                return users[user]
-              } else {
-                return user
-              }
-            })
+    v1_user_ids = Array.from(v1_user_ids)
+    const projection = { first_name: 1, last_name: 1, email: 1 }
+    return UserGetter.getUsers(user_ids, projection, function(
+      error,
+      users_array
+    ) {
+      if (error != null) {
+        return callback(error)
+      }
+      const v1_query = { 'overleaf.id': v1_user_ids }
+      const users = {}
+      for (user of Array.from(users_array || [])) {
+        users[user._id.toString()] = HistoryManager._userView(user)
+      }
+      projection.overleaf = 1
+      UserGetter.getUsersByV1Ids(
+        v1_user_ids,
+        projection,
+        (error, v1_identified_users_array) => {
+          for (user of Array.from(v1_identified_users_array || [])) {
+            users[user.overleaf.id] = HistoryManager._userView(user)
+          }
+          for (entry of Array.from(data.diff || data.updates || [])) {
+            if (entry.meta != null) {
+              entry.meta.users = (
+                (entry.meta != null ? entry.meta.users : undefined) || []
+              ).map(function(user) {
+                if (typeof user === 'string' || typeof user === 'number') {
+                  return users[user]
+                } else {
+                  return user
+                }
+              })
+            }
           }
+          return callback(null, data)
         }
-        return callback(null, data)
-      }
-    )
+      )
+    })
   },
 
   _userView(user) {

services/web/app/src/Features/User/UserGetter.js

@@ -132,6 +132,20 @@ module.exports = UserGetter = {
     })
   },
 
+  getUsersByV1Ids(v1Ids, projection, callback) {
+    if (callback == null) {
+      callback = function(error, user) {}
+    }
+    if (arguments.length === 2) {
+      callback = projection
+      projection = {}
+    }
+    const query = { 'overleaf.id': { $in: v1Ids } }
+    return db.users.find(query, projection, (error, users) => {
+      return callback(error, users)
+    })
+  },
+
   getUsersByHostname(hostname, projection, callback) {
     if (callback == null) {
       callback = function(error, users) {}

services/web/test/unit/src/History/HistoryManagerTests.js

@@ -152,7 +152,8 @@ describe('HistoryManager', function() {
         _id: (this.user_id1 = '123456'),
         first_name: 'Jane',
         last_name: 'Doe',
-        email: 'jane@example.com'
+        email: 'jane@example.com',
+        overleaf: { id: 5011 }
       }
       this.user1_view = {
         id: this.user_id1,
@@ -172,6 +173,7 @@ describe('HistoryManager', function() {
         last_name: 'Doe',
         email: 'john@example.com'
       }
+      this.UserGetter.getUsersByV1Ids = sinon.stub().yields(null, [this.user1])
       return (this.UserGetter.getUsers = sinon
         .stub()
         .yields(null, [this.user1, this.user2]))
@@ -205,6 +207,33 @@ describe('HistoryManager', function() {
         )
       })
 
+      it('should handle v1 user ids', function(done) {
+        return this.HistoryManager.injectUserDetails(
+          {
+            diff: [
+              {
+                i: 'foo',
+                meta: {
+                  users: [5011]
+                }
+              },
+              {
+                i: 'bar',
+                meta: {
+                  users: [this.user_id2]
+                }
+              }
+            ]
+          },
+          (error, diff) => {
+            expect(error).to.be.null
+            expect(diff.diff[0].meta.users).to.deep.equal([this.user1_view])
+            expect(diff.diff[1].meta.users).to.deep.equal([this.user2_view])
+            return done()
+          }
+        )
+      })
+
       it('should leave user objects', function(done) {
         return this.HistoryManager.injectUserDetails(
           {

services/web/test/unit/src/User/UserGetterTests.js

@@ -294,6 +294,24 @@ describe('UserGetter', function() {
       )
     }))
 
+  describe('getUsersByV1Id', () =>
+    it('should find users by list of v1 ids', function(done) {
+      const v1Ids = [501]
+      const expectedQuery = {
+        'overleaf.id': { $in: v1Ids }
+      }
+      const projection = { emails: 1 }
+      return this.UserGetter.getUsersByV1Ids(
+        v1Ids,
+        projection,
+        (error, users) => {
+          this.find.calledOnce.should.equal(true)
+          this.find.calledWith(expectedQuery, projection).should.equal(true)
+          return done()
+        }
+      )
+    }))
+
   describe('ensureUniqueEmailAddress', function() {
     beforeEach(function() {
       return (this.UserGetter.getUserByAnyEmail = sinon.stub())