From 68f011378d6de634cc1ab20b3920c76e8fe141c5 Mon Sep 17 00:00:00 2001 From: David <33458145+davidmcpowell@users.noreply.github.com> Date: Thu, 8 Feb 2024 11:13:37 +0000 Subject: [PATCH] Merge pull request #16911 from overleaf/dp-mongoose-callback-referal-allocator Promisify ReferalAllocator and ReferalAllocatorTests GitOrigin-RevId: 0c68ec6176ac440504fc6501e0fad161d83d3541 --- .../src/Features/Referal/ReferalAllocator.js | 61 +++++++------------ .../unit/src/Referal/ReferalAllocatorTests.js | 58 ++++++++---------- 2 files changed, 47 insertions(+), 72 deletions(-) diff --git a/services/web/app/src/Features/Referal/ReferalAllocator.js b/services/web/app/src/Features/Referal/ReferalAllocator.js index 2346839b07..bf42fb840f 100644 --- a/services/web/app/src/Features/Referal/ReferalAllocator.js +++ b/services/web/app/src/Features/Referal/ReferalAllocator.js @@ -1,33 +1,22 @@ const OError = require('@overleaf/o-error') const { User } = require('../../models/User') const FeaturesUpdater = require('../Subscription/FeaturesUpdater') -const { promisify } = require('@overleaf/promise-utils') +const { callbackify } = require('@overleaf/promise-utils') -function allocate( - referalId, - newUserId, - referalSource, - referalMedium, - callback -) { - if (callback == null) { - callback = function () {} - } +async function allocate(referalId, newUserId, referalSource, referalMedium) { if (referalId == null) { - return callback(null) + return null } const query = { referal_id: referalId } - User.findOne(query, { _id: 1 }, function (error, user) { - if (error != null) { - return callback(error) - } - if (user == null || user._id == null) { - return callback(null) - } + const user = await User.findOne(query, { _id: 1 }).exec() + if (user == null || user._id == null) { + return null + } - if (referalSource === 'bonus') { - User.updateOne( + if (referalSource === 'bonus') { + try { + await User.updateOne( query, { $push: { @@ -37,27 +26,23 @@ function allocate( refered_user_count: 1, }, }, - {}, - function (err) { - if (err != null) { - OError.tag(err, 'something went wrong allocating referal', { - referalId, - newUserId, - }) - return callback(err) - } - FeaturesUpdater.refreshFeatures(user._id, 'referral', callback) - } - ) - } else { - callback() + {} + ).exec() + } catch (err) { + OError.tag(err, 'something went wrong allocating referal', { + referalId, + newUserId, + }) + throw err } - }) + + return await FeaturesUpdater.promises.refreshFeatures(user._id, 'referral') + } } module.exports = { - allocate, + allocate: callbackify(allocate), promises: { - allocate: promisify(allocate), + allocate, }, } diff --git a/services/web/test/unit/src/Referal/ReferalAllocatorTests.js b/services/web/test/unit/src/Referal/ReferalAllocatorTests.js index 9adeb74466..ecc0c6ea89 100644 --- a/services/web/test/unit/src/Referal/ReferalAllocatorTests.js +++ b/services/web/test/unit/src/Referal/ReferalAllocatorTests.js @@ -16,28 +16,30 @@ describe('ReferalAllocator', function () { '@overleaf/settings': (this.Settings = {}), }, }) - this.callback = sinon.stub() this.referal_id = 'referal-id-123' this.referal_medium = 'twitter' this.user_id = 'user-id-123' this.new_user_id = 'new-user-id-123' - this.FeaturesUpdater.refreshFeatures = sinon.stub().yields() - this.User.updateOne = sinon.stub().callsArgWith(3, null) - this.User.findOne = sinon - .stub() - .callsArgWith(2, null, { _id: this.user_id }) + this.FeaturesUpdater.promises = { + refreshFeatures: sinon.stub().resolves(), + } + this.User.updateOne = sinon.stub().returns({ + exec: sinon.stub().resolves(), + }) + this.User.findOne = sinon.stub().returns({ + exec: sinon.stub().resolves({ _id: this.user_id }), + }) }) describe('allocate', function () { describe('when the referal was a bonus referal', function () { - beforeEach(function () { + beforeEach(async function () { this.referal_source = 'bonus' - this.ReferalAllocator.allocate( + await this.ReferalAllocator.promises.allocate( this.referal_id, this.new_user_id, this.referal_source, - this.referal_medium, - this.callback + this.referal_medium ) }) @@ -66,27 +68,24 @@ describe('ReferalAllocator', function () { }) it("should refresh the user's subscription", function () { - this.FeaturesUpdater.refreshFeatures + this.FeaturesUpdater.promises.refreshFeatures .calledWith(this.user_id) .should.equal(true) }) - - it('should call the callback', function () { - this.callback.called.should.equal(true) - }) }) describe('when there is no user for the referal id', function () { - beforeEach(function () { + beforeEach(async function () { this.referal_source = 'bonus' this.referal_id = 'wombat' - this.User.findOne = sinon.stub().callsArgWith(2, null, null) - this.ReferalAllocator.allocate( + this.User.findOne = sinon.stub().returns({ + exec: sinon.stub().resolves(null), + }) + await this.ReferalAllocator.promises.allocate( this.referal_id, this.new_user_id, this.referal_source, - this.referal_medium, - this.callback + this.referal_medium ) }) @@ -101,23 +100,18 @@ describe('ReferalAllocator', function () { }) it('should not assign the user a bonus', function () { - this.FeaturesUpdater.refreshFeatures.called.should.equal(false) - }) - - it('should call the callback', function () { - this.callback.called.should.equal(true) + this.FeaturesUpdater.promises.refreshFeatures.called.should.equal(false) }) }) describe('when the referal is not a bonus referal', function () { - beforeEach(function () { + beforeEach(async function () { this.referal_source = 'public_share' - this.ReferalAllocator.allocate( + await this.ReferalAllocator.promises.allocate( this.referal_id, this.new_user_id, this.referal_source, - this.referal_medium, - this.callback + this.referal_medium ) }) @@ -132,11 +126,7 @@ describe('ReferalAllocator', function () { }) it('should not assign the user a bonus', function () { - this.FeaturesUpdater.refreshFeatures.called.should.equal(false) - }) - - it('should call the callback', function () { - this.callback.called.should.equal(true) + this.FeaturesUpdater.promises.refreshFeatures.called.should.equal(false) }) }) })