Merge pull request #2182 from overleaf/ta-remove-user-stubs

Remove Usages of UserStub GitOrigin-RevId: 6896d0d3594d12ffa06211838ae2274661c77f4f
2025-04-17 06:17:40 +00:00 · 2019-10-01 12:27:15 +01:00 · 2019-10-01 12:27:15 +01:00 · 2eb1f510c1
commit 2eb1f510c1
parent 962c5cc273
10 changed files with 31 additions and 127 deletions
--- a/services/web/app/src/Features/Collaborators/CollaboratorsHandler.js
+++ b/services/web/app/src/Features/Collaborators/CollaboratorsHandler.js
@ -164,7 +164,7 @@ const CollaboratorsHandler = {
      members,
      3,
      (member, cb) =>
-        UserGetter.getUserOrUserStubById(
+        UserGetter.getUser(
          member.id,
          CollaboratorsHandler.USER_PROJECTION,
          function(error, user) {
--- a/services/web/app/src/Features/Project/ProjectController.js
+++ b/services/web/app/src/Features/Project/ProjectController.js
@ -933,7 +933,7 @@ const ProjectController = {
    async.eachSeries(
      userIds,
      (userId, cb) => {
-        UserGetter.getUserOrUserStubById(
+        UserGetter.getUser(
          userId,
          { first_name: 1, last_name: 1, email: 1 },
          (error, user) => {
--- a/services/web/app/src/Features/Project/ProjectCreationHandler.js
+++ b/services/web/app/src/Features/Project/ProjectCreationHandler.js
@ -117,10 +117,7 @@ const ProjectCreationHandler = {
      err,
      user
    ) {
-      if (user != null) {
-        // It's possible the owner_id is a UserStub
-        project.spellCheckLanguage = user.ace.spellCheckLanguage
-      }
+      project.spellCheckLanguage = user.ace.spellCheckLanguage
      return project.save(function(err) {
        if (err != null) {
          return callback(err)
--- a/services/web/app/src/Features/Subscription/SubscriptionUpdater.js
+++ b/services/web/app/src/Features/Subscription/SubscriptionUpdater.js
@ -96,16 +96,7 @@ const SubscriptionUpdater = {
        if (err != null) {
          return callback(err)
        }
-
-        // Only apply features updates to users, not user stubs
-        UserGetter.getUsers(memberIds, { _id: 1 }, function(err, users) {
-          if (err != null) {
-            return callback(err)
-          }
-
-          const userIds = users.map(u => u._id.toString())
-          async.map(userIds, FeaturesUpdater.refreshFeatures, callback)
-        })
+        async.map(memberIds, FeaturesUpdater.refreshFeatures, callback)
      }
    )
  },
@ -131,17 +122,10 @@ const SubscriptionUpdater = {
        )
        return callback(err)
      }
-      UserGetter.getUserOrUserStubById(userId, {}, function(
-        error,
-        user,
-        isStub
-      ) {
+      UserGetter.getUser(userId, function(error, user) {
        if (error) {
          return callback(error)
        }
-        if (isStub) {
-          return callback()
-        }
        FeaturesUpdater.refreshFeatures(userId, callback)
      })
    })
--- a/services/web/app/src/Features/User/UserGetter.js
+++ b/services/web/app/src/Features/User/UserGetter.js
@ -136,29 +136,6 @@ const UserGetter = {
    db.users.find({ _id: { $in: userIds } }, projection, callback)
  },

-  getUserOrUserStubById(userId, projection, callback) {
-    let query
-    try {
-      query = { _id: ObjectId(userId.toString()) }
-    } catch (e) {
-      return callback(new Error(e))
-    }
-    db.users.findOne(query, projection, function(error, user) {
-      if (error) {
-        return callback(error)
-      }
-      if (user) {
-        return callback(null, user, false)
-      }
-      db.userstubs.findOne(query, projection, function(error, user) {
-        if (error || user) {
-          return callback(error, user)
-        }
-        callback(null, user, true)
-      })
-    })
-  },
-
  // check for duplicate email address. This is also enforced at the DB level
  ensureUniqueEmailAddress(newEmail, callback) {
    this.getUserByAnyEmail(newEmail, function(error, user) {
@ -192,7 +169,6 @@ var decorateFullEmails = (defaultEmail, emailsData, affiliationsData) =>
  'getUserByMainEmail',
  'getUserByAnyEmail',
  'getUsers',
-  'getUserOrUserStubById',
  'ensureUniqueEmailAddress'
 ].map(method =>
  metrics.timeAsyncMethod(UserGetter, method, 'mongo.UserGetter', logger)
--- a/services/web/app/src/Features/UserMembership/UserMembershipViewModel.js
+++ b/services/web/app/src/Features/UserMembership/UserMembershipViewModel.js
@ -34,17 +34,10 @@ module.exports = UserMembershipViewModel = {

    const userId = userOrIdOrEmail
    const projection = { email: 1, first_name: 1, last_name: 1 }
-    return UserGetter.getUserOrUserStubById(userId, projection, function(
-      error,
-      user,
-      isStub
-    ) {
+    return UserGetter.getUser(userId, projection, function(error, user) {
      if (error != null || user == null) {
        return callback(null, buildUserViewModelWithId(userId.toString()))
      }
-      if (isStub) {
-        return callback(null, buildUserViewModelWithStub(user))
-      }
      return callback(null, buildUserViewModel(user))
    })
  }
@ -65,8 +58,4 @@ var buildUserViewModel = function(user, isInvite) {

 var buildUserViewModelWithEmail = email => buildUserViewModel({ email }, true)

-var buildUserViewModelWithStub = user =>
-  // user stubs behave as invites
-  buildUserViewModel(user, true)
-
 var buildUserViewModelWithId = id => buildUserViewModel({ _id: id }, false)
--- a/services/web/test/unit/src/Collaborators/CollaboratorsHandlerTests.js
+++ b/services/web/test/unit/src/Collaborators/CollaboratorsHandlerTests.js
@ -206,22 +206,20 @@ describe('CollaboratorsHandler', function() {
            source: 'invite'
          }
        ])
-      this.UserGetter.getUserOrUserStubById = sinon.stub()
-      this.UserGetter.getUserOrUserStubById
+      this.UserGetter.getUser = sinon.stub()
+      this.UserGetter.getUser
        .withArgs('read-only-ref-1')
        .yields(null, { _id: 'read-only-ref-1' })
-      this.UserGetter.getUserOrUserStubById
+      this.UserGetter.getUser
        .withArgs('read-only-ref-2')
        .yields(null, { _id: 'read-only-ref-2' })
-      this.UserGetter.getUserOrUserStubById
+      this.UserGetter.getUser
        .withArgs('read-write-ref-1')
        .yields(null, { _id: 'read-write-ref-1' })
-      this.UserGetter.getUserOrUserStubById
+      this.UserGetter.getUser
        .withArgs('read-write-ref-2')
        .yields(null, { _id: 'read-write-ref-2' })
-      this.UserGetter.getUserOrUserStubById
-        .withArgs('doesnt-exist')
-        .yields(null, null)
+      this.UserGetter.getUser.withArgs('doesnt-exist').yields(null, null)
      return this.CollaboratorHandler.getMembersWithPrivilegeLevels(
        this.project_id,
        this.callback
@ -272,22 +270,20 @@ describe('CollaboratorsHandler', function() {
            source: 'invite'
          }
        ])
-      this.UserGetter.getUserOrUserStubById = sinon.stub()
-      this.UserGetter.getUserOrUserStubById
+      this.UserGetter.getUser = sinon.stub()
+      this.UserGetter.getUser
        .withArgs('read-only-ref-1')
        .yields(null, { _id: 'read-only-ref-1' })
-      this.UserGetter.getUserOrUserStubById
+      this.UserGetter.getUser
        .withArgs('read-only-ref-2')
        .yields(null, { _id: 'read-only-ref-2' })
-      this.UserGetter.getUserOrUserStubById
+      this.UserGetter.getUser
        .withArgs('read-write-ref-1')
        .yields(null, { _id: 'read-write-ref-1' })
-      this.UserGetter.getUserOrUserStubById
+      this.UserGetter.getUser
        .withArgs('read-write-ref-2')
        .yields(null, { _id: 'read-write-ref-2' })
-      this.UserGetter.getUserOrUserStubById
-        .withArgs('doesnt-exist')
-        .yields(null, null)
+      this.UserGetter.getUser.withArgs('doesnt-exist').yields(null, null)
      return this.CollaboratorHandler.getInvitedMembersWithPrivilegeLevels(
        this.project_id,
        this.callback
@ -336,22 +332,20 @@ describe('CollaboratorsHandler', function() {
            source: 'invite'
          }
        ])
-      this.UserGetter.getUserOrUserStubById = sinon.stub()
-      this.UserGetter.getUserOrUserStubById
+      this.UserGetter.getUser = sinon.stub()
+      this.UserGetter.getUser
        .withArgs('read-only-ref-1')
        .yields(null, { _id: 'read-only-ref-1' })
-      this.UserGetter.getUserOrUserStubById
+      this.UserGetter.getUser
        .withArgs('read-only-ref-2')
        .yields(null, { _id: 'read-only-ref-2' })
-      this.UserGetter.getUserOrUserStubById
+      this.UserGetter.getUser
        .withArgs('read-write-ref-1')
        .yields(null, { _id: 'read-write-ref-1' })
-      this.UserGetter.getUserOrUserStubById
+      this.UserGetter.getUser
        .withArgs('read-write-ref-2')
        .yields(null, { _id: 'read-write-ref-2' })
-      this.UserGetter.getUserOrUserStubById
-        .withArgs('doesnt-exist')
-        .yields(null, null)
+      this.UserGetter.getUser.withArgs('doesnt-exist').yields(null, null)
      return this.CollaboratorHandler.getTokenMembersWithPrivilegeLevels(
        this.project_id,
        this.callback
--- a/services/web/test/unit/src/Project/ProjectControllerTests.js
+++ b/services/web/test/unit/src/Project/ProjectControllerTests.js
@ -97,8 +97,7 @@ describe('ProjectController', function() {
    this.UserGetter = {
      getUser: sinon
        .stub()
-        .callsArgWith(2, null, { lastLoginIp: '192.170.18.2' }),
-      getUserOrUserStubById: sinon.stub().callsArgWith(2, null, {})
+        .callsArgWith(2, null, { lastLoginIp: '192.170.18.2' })
    }
    this.Modules = {
      hooks: {
@ -404,7 +403,7 @@ describe('ProjectController', function() {
      this.UserModel.findById = (id, fields, callback) => {
        callback(null, this.users[id])
      }
-      this.UserGetter.getUserOrUserStubById = (id, fields, callback) => {
+      this.UserGetter.getUser = (id, fields, callback) => {
        callback(null, this.users[id])
      }

--- a/services/web/test/unit/src/Subscription/SubscriptionUpdaterTests.js
+++ b/services/web/test/unit/src/Subscription/SubscriptionUpdaterTests.js
@ -19,10 +19,6 @@ describe('SubscriptionUpdater', function() {
    this.adminUser = { _id: (this.adminuser_id = '5208dd34438843e2db000007') }
    this.otherUserId = '5208dd34438842e2db000005'
    this.allUserIds = ['13213', 'dsadas', 'djsaiud89']
-    this.userStub = {
-      _id: 'mock-user-stub-id',
-      email: 'mock-stub-email@baz.com'
-    }
    this.subscription = {
      _id: '111111111111111111111111',
      admin_id: this.adminUser._id,
@ -86,7 +82,7 @@ describe('SubscriptionUpdater', function() {
        const users = memberIds.map(id => ({ _id: id }))
        callback(null, users)
      },
-      getUserOrUserStubById: sinon.stub()
+      getUser: sinon.stub()
    }

    this.ReferalFeatures = { getBonusFeatures: sinon.stub().callsArgWith(1) }
@ -392,7 +388,7 @@ describe('SubscriptionUpdater', function() {
  describe('removeUserFromGroups', function() {
    beforeEach(function() {
      this.FeaturesUpdater.refreshFeatures = sinon.stub().callsArgWith(1)
-      this.UserGetter.getUserOrUserStubById.yields(null, {}, false)
+      this.UserGetter.getUser.yields(null, {})
      this.fakeSubscriptions = [{ _id: 'fake-id-1' }, { _id: 'fake-id-2' }]
      this.SubscriptionLocator.getMemberSubscriptions.yields(
        null,
@ -436,18 +432,6 @@ describe('SubscriptionUpdater', function() {
        }
      )
    })
-
-    it('should not update features for user stubs', function(done) {
-      this.UserGetter.getUserOrUserStubById.yields(null, {}, true)
-      this.SubscriptionUpdater.removeUserFromGroup(
-        this.subscription._id,
-        this.userStub._id,
-        () => {
-          this.FeaturesUpdater.refreshFeatures.called.should.equal(false)
-          done()
-        }
-      )
-    })
  })

  describe('deleteSubscription', function() {
--- a/services/web/test/unit/src/UserMembership/UserMembershipViewModelTests.js
+++ b/services/web/test/unit/src/UserMembership/UserMembershipViewModelTests.js
@ -24,7 +24,7 @@ const SandboxedModule = require('sandboxed-module')

 describe('UserMembershipViewModel', function() {
  beforeEach(function() {
-    this.UserGetter = { getUserOrUserStubById: sinon.stub() }
+    this.UserGetter = { getUser: sinon.stub() }
    this.UserMembershipViewModel = SandboxedModule.require(modulePath, {
      globals: {
        console: console
@ -40,10 +40,6 @@ describe('UserMembershipViewModel', function() {
      email: 'mock-email@baz.com',
      first_name: 'Name'
    }
-    return (this.userStub = {
-      _id: 'mock-user-stub-id',
-      email: 'mock-stub-email@baz.com'
-    })
  })

  describe('build', function() {
@ -92,7 +88,7 @@ describe('UserMembershipViewModel', function() {
    })

    it('build user id', function(done) {
-      this.UserGetter.getUserOrUserStubById.yields(null, this.user, false)
+      this.UserGetter.getUser.yields(null, this.user)
      return this.UserMembershipViewModel.buildAsync(
        ObjectId(),
        (error, viewModel) => {
@ -108,23 +104,8 @@ describe('UserMembershipViewModel', function() {
      )
    })

-    it('build user stub id', function(done) {
-      this.UserGetter.getUserOrUserStubById.yields(null, this.userStub, true)
-      return this.UserMembershipViewModel.buildAsync(
-        ObjectId(),
-        (error, viewModel) => {
-          should.not.exist(error)
-          assertNotCalled(this.UserMembershipViewModel.build)
-          expect(viewModel._id).to.equal(this.userStub._id)
-          expect(viewModel.email).to.equal(this.userStub.email)
-          expect(viewModel.invite).to.equal(true)
-          return done()
-        }
-      )
-    })
-
    it('build user id with error', function(done) {
-      this.UserGetter.getUserOrUserStubById.yields(new Error('nope'))
+      this.UserGetter.getUser.yields(new Error('nope'))
      const userId = ObjectId()
      return this.UserMembershipViewModel.buildAsync(
        userId,