From 9419cc3b37941473f2bc4bbf8994811d4b980c1e Mon Sep 17 00:00:00 2001 From: Antoine Clausse Date: Tue, 14 May 2024 10:20:35 +0200 Subject: [PATCH] [web] Add tests to `collect_paypal_past_due_invoice.js` + update logging (#18310) * Fix: Invoices collected array length comparison Update the code with the correct condition to respect the intent of the previous implementation ("exit with non zero code when no invoicess were processed"). See https://github.com/overleaf/internal/commit/5476f39984f9cbb6edaeec3bef35e9e5207c5823 However, I'm not sure if erroring when no invoices are collected is actually what we want to do. * Wrap `collect_paypal_past_due_invoice` script and export the function * Fixup typo `accoutns` * Log invoices collection data before throwing * Add note: `handleAPIError` is silencing the errors * Create a test on `collect_paypal_past_due_invoice` * Replace `console.log` by `@overleaf/logger` (bunyan) Our `console.warn` show up as Errors (in red) in GCP. For example the following is an error in GCP: ``` Errors in attemptInvoiceCollection with id=2693634 OError: Recurly API returned with status code: 400 ``` https://github.com/overleaf/internal/blob/5476f39/services/web/scripts/recurly/collect_paypal_past_due_invoice.js#L9 --- Does it correctly set the levels as warnings if we use `@overleaf/logger` GitOrigin-RevId: 37c8bdf4afd8cef4706700aafb44480ec8966a74 --- .../Features/Subscription/RecurlyWrapper.js | 2 +- .../collect_paypal_past_due_invoice.js | 204 ++++++++------- .../src/CollectPayPalPastDueInvoiceTest.js | 235 ++++++++++++++++++ 3 files changed, 355 insertions(+), 86 deletions(-) create mode 100644 services/web/test/acceptance/src/CollectPayPalPastDueInvoiceTest.js diff --git a/services/web/app/src/Features/Subscription/RecurlyWrapper.js b/services/web/app/src/Features/Subscription/RecurlyWrapper.js index 6714da86f1..830b3a047e 100644 --- a/services/web/app/src/Features/Subscription/RecurlyWrapper.js +++ b/services/web/app/src/Features/Subscription/RecurlyWrapper.js @@ -583,7 +583,7 @@ const RecurlyWrapper = { } return RecurlyWrapper._parseXml(body, function (err, data) { if (err) { - logger.warn({ err }, 'could not get accoutns') + logger.warn({ err }, 'could not get accounts') return callback(err) } const items = data[resource] diff --git a/services/web/scripts/recurly/collect_paypal_past_due_invoice.js b/services/web/scripts/recurly/collect_paypal_past_due_invoice.js index 8ede1d5278..89a5702bb4 100644 --- a/services/web/scripts/recurly/collect_paypal_past_due_invoice.js +++ b/services/web/scripts/recurly/collect_paypal_past_due_invoice.js @@ -1,102 +1,136 @@ const RecurlyWrapper = require('../../app/src/Features/Subscription/RecurlyWrapper') const async = require('async') const minimist = require('minimist') +const logger = require('@overleaf/logger') -const slowCallback = (callback, error, data) => - setTimeout(() => callback(error, data), 80) +const slowCallback = + require.main === module + ? (callback, error, data) => setTimeout(() => callback(error, data), 80) + : (callback, error, data) => callback(error, data) +// NOTE: Errors are not propagated to the caller const handleAPIError = (source, id, error, callback) => { - console.warn(`Errors in ${source} with id=${id}`, error) + logger.warn(`Errors in ${source} with id=${id}`, error) if (typeof error === 'string' && error.match(/429$/)) { return setTimeout(callback, 1000 * 60 * 5) } slowCallback(callback) } -const attemptInvoiceCollection = (invoice, callback) => { - isAccountUsingPaypal(invoice, (error, isPaypal) => { - if (error || !isPaypal) { - return callback(error) - } - const accountId = invoice.account.url.match(/accounts\/(.*)/)[1] - if (USERS_COLLECTED.indexOf(accountId) > -1) { - console.warn(`Skipping duplicate user ${accountId}`) - return callback() - } - INVOICES_COLLECTED.push(invoice.invoice_number) - USERS_COLLECTED.push(accountId) - if (DRY_RUN) { - return callback() - } - RecurlyWrapper.attemptInvoiceCollection( - invoice.invoice_number, - (error, response) => { - if (error) { - return handleAPIError( - 'attemptInvoiceCollection', - invoice.invoice_number, - error, - callback - ) - } - INVOICES_COLLECTED_SUCCESS.push(invoice.invoice_number) - slowCallback(callback, null) - } - ) - }) -} - -const isAccountUsingPaypal = (invoice, callback) => { - const accountId = invoice.account.url.match(/accounts\/(.*)/)[1] - RecurlyWrapper.getBillingInfo(accountId, (error, response) => { - if (error) { - return handleAPIError('billing info', accountId, error, callback) - } - if (response.billing_info.paypal_billing_agreement_id) { - return slowCallback(callback, null, true) - } - slowCallback(callback, null, false) - }) -} - -const attemptInvoicesCollection = callback => { - RecurlyWrapper.getPaginatedEndpoint( - 'invoices', - { state: 'past_due' }, - (error, invoices) => { - console.log('invoices', invoices.length) - if (error) { +/** + * @returns {Promise<{ + * INVOICES_COLLECTED: string[], + * INVOICES_COLLECTED_SUCCESS: string[], + * USERS_COLLECTED: string[], + * }>} + */ +const main = async () => { + const attemptInvoiceCollection = (invoice, callback) => { + isAccountUsingPaypal(invoice, (error, isPaypal) => { + if (error || !isPaypal) { return callback(error) } - async.eachSeries(invoices, attemptInvoiceCollection, callback) - } - ) + const accountId = invoice.account.url.match(/accounts\/(.*)/)[1] + if (USERS_COLLECTED.indexOf(accountId) > -1) { + logger.warn(`Skipping duplicate user ${accountId}`) + return callback() + } + INVOICES_COLLECTED.push(invoice.invoice_number) + USERS_COLLECTED.push(accountId) + if (DRY_RUN) { + return callback() + } + RecurlyWrapper.attemptInvoiceCollection( + invoice.invoice_number, + (error, response) => { + if (error) { + return handleAPIError( + 'attemptInvoiceCollection', + invoice.invoice_number, + error, + callback + ) + } + INVOICES_COLLECTED_SUCCESS.push(invoice.invoice_number) + slowCallback(callback, null) + } + ) + }) + } + + const isAccountUsingPaypal = (invoice, callback) => { + const accountId = invoice.account.url.match(/accounts\/(.*)/)[1] + RecurlyWrapper.getBillingInfo(accountId, (error, response) => { + if (error) { + return handleAPIError('billing info', accountId, error, callback) + } + if (response.billing_info.paypal_billing_agreement_id) { + return slowCallback(callback, null, true) + } + slowCallback(callback, null, false) + }) + } + + const attemptInvoicesCollection = callback => { + RecurlyWrapper.getPaginatedEndpoint( + 'invoices', + { state: 'past_due' }, + (error, invoices) => { + logger.info('invoices', invoices.length) + if (error) { + return callback(error) + } + async.eachSeries(invoices, attemptInvoiceCollection, callback) + } + ) + } + const argv = minimist(process.argv.slice(2)) + const DRY_RUN = argv.n !== undefined + const INVOICES_COLLECTED = [] + const INVOICES_COLLECTED_SUCCESS = [] + const USERS_COLLECTED = [] + + return new Promise(resolve => { + attemptInvoicesCollection(error => { + logger.info( + `DONE (DRY_RUN=${DRY_RUN}). ${INVOICES_COLLECTED.length} invoices collection attempts for ${USERS_COLLECTED.length} users. ${INVOICES_COLLECTED_SUCCESS.length} successful collections` + ) + console.dir( + { + INVOICES_COLLECTED, + INVOICES_COLLECTED_SUCCESS, + USERS_COLLECTED, + }, + { maxArrayLength: null } + ) + + if (error) { + throw error + } + + if (INVOICES_COLLECTED_SUCCESS.length === 0) { + throw new Error('No invoices collected') + } + + resolve({ + INVOICES_COLLECTED, + INVOICES_COLLECTED_SUCCESS, + USERS_COLLECTED, + }) + }) + }) } -const argv = minimist(process.argv.slice(2)) -const DRY_RUN = argv.n !== undefined -const INVOICES_COLLECTED = [] -const INVOICES_COLLECTED_SUCCESS = [] -const USERS_COLLECTED = [] -attemptInvoicesCollection(error => { - if (error) { - throw error - } - console.log( - `DONE (DRY_RUN=${DRY_RUN}). ${INVOICES_COLLECTED.length} invoices collection attempts for ${USERS_COLLECTED.length} users. ${INVOICES_COLLECTED_SUCCESS.length} successful collections` - ) - console.dir( - { - INVOICES_COLLECTED, - INVOICES_COLLECTED_SUCCESS, - USERS_COLLECTED, - }, - { maxArrayLength: null } - ) +if (require.main === module) { + main() + .then(() => { + logger.error('Done.') + process.exit(0) + }) + .catch(err => { + logger.error('Error', err) + process.exit(1) + }) +} - if (INVOICES_COLLECTED_SUCCESS === 0) { - process.exit(1) - } else { - process.exit() - } -}) +module.exports = { main } diff --git a/services/web/test/acceptance/src/CollectPayPalPastDueInvoiceTest.js b/services/web/test/acceptance/src/CollectPayPalPastDueInvoiceTest.js new file mode 100644 index 0000000000..57ba6b85ef --- /dev/null +++ b/services/web/test/acceptance/src/CollectPayPalPastDueInvoiceTest.js @@ -0,0 +1,235 @@ +const sinon = require('sinon') +const chai = require('chai') +const { expect } = require('chai') + +chai.use(require('chai-as-promised')) +chai.use(require('sinon-chai')) + +const { + main, +} = require('../../../scripts/recurly/collect_paypal_past_due_invoice') + +const RecurlyWrapper = require('../../../app/src/Features/Subscription/RecurlyWrapper') +const OError = require('@overleaf/o-error') + +// from https://recurly.com/developers/api-v2/v2.21/#operation/listInvoices +const invoicesXml = invoiceIds => ` + + ${invoiceIds + .map( + invoiceId => ` + + + +
+ + + + + + + +
+ + Lon Doner + 221B Baker St. + + London + + W1K 6AH + GB + + + 421f7b7d414e4c6792938e7c49d552e9 + paid + + ${invoiceId} + + + 2000 + 0 + 2018-01-30T21:11:50Z + 0 + charge + purchase + + 2000 + + + 0 + 1200 + USD + 2016-06-25T12:00:00Z + + + + + usst + CA + 0 + 0 + automatic + + + + + + + +
` + ) + .join('')} +
+` + +// from https://recurly.com/developers/api-v2/v2.21/#operation/lookupAccountsBillingInfo +const billingInfoXml = ` + + PAYPAL_BILLING_AGREEMENT_ID + + Verena + Example + + 123 Main St. + + San Francisco + CA + 94105 + US + + + 127.0.0.1 + + Visa + 2019 + 11 + 411111 + 1111 + 2017-02-17T15:38:53Z + +` + +// from https://recurly.com/developers/api-v2/v2.21/#operation/collectAnInvoice +const invoiceCollectXml = ` + + + +
+ 123 Main St. + + San Francisco + CA + 94105 + US + +
+ 374a37924f83c733b9c9814e9580496a + pending + + 1000 + + + 5000 + 438 + 5438 + USD + 2016-07-11T19:25:57Z + 2016-07-11T19:25:57Z + + + + usst + CA + 0.0875 + 0 + automatic + + + + + + + + + +
+` + +// from our logs +const invoiceCollectErrXml2 = ` + + + not_found + Couldn't find BillingInfo with account_code = abcdef87654321 + +` + +describe('CollectPayPalPastDueInvoice', function () { + let apiRequestStub + const fakeApiRequests = invoiceIdsAndReturnCode => { + apiRequestStub = sinon.stub(RecurlyWrapper, 'apiRequest') + apiRequestStub.callsFake((options, callback) => { + switch (options.url) { + case 'invoices': + callback( + null, + { statusCode: 200, headers: {} }, + invoicesXml(invoiceIdsAndReturnCode) + ) + return + case 'accounts/200/billing_info': + case 'accounts/404/billing_info': + callback(null, { statusCode: 200, headers: {} }, billingInfoXml) + return + case 'invoices/200/collect': + callback(null, { statusCode: 200, headers: {} }, invoiceCollectXml) + return + case 'invoices/404/collect': + callback( + new OError(`Recurly API returned with status code: 404`, { + statusCode: 404, + }), + { statusCode: 404, headers: {} }, + invoiceCollectErrXml2 + ) + return + default: + throw new Error(`Unexpected URL: ${options.url}`) + } + }) + } + + afterEach(function () { + apiRequestStub?.restore() + }) + + it('collects one valid invoice', async function () { + fakeApiRequests([200]) + const r = await main() + await expect(r).to.eql({ + INVOICES_COLLECTED: [200], + INVOICES_COLLECTED_SUCCESS: [200], + USERS_COLLECTED: ['200'], + }) + }) + + it('rejects with no invoices are processed because of errors', async function () { + fakeApiRequests([404]) + await expect(main()).to.be.rejectedWith('No invoices collected') + }) + + it('rejects when there are no invoices', async function () { + fakeApiRequests([]) + await expect(main()).to.be.rejectedWith('No invoices collected') + }) + + it('resolves when some invoices are partially successful', async function () { + fakeApiRequests([200, 404]) + const r = await main() + await expect(r).to.eql({ + INVOICES_COLLECTED: [200, 404], + INVOICES_COLLECTED_SUCCESS: [200], + USERS_COLLECTED: ['200', '404'], + }) + }) +})