Last active
January 11, 2022 12:37
-
-
Save biancadanforth/4c98d108325f05f9f714fe7b8a54e7c5 to your computer and use it in GitHub Desktop.
FXA-4418 chore(auth): set state as "unknown" if it can't be determined
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
diff --git a/packages/fxa-auth-server/lib/payments/stripe.ts b/packages/fxa-auth-server/lib/payments/stripe.ts | |
index 23ef5812a..84be94e75 100644 | |
--- a/packages/fxa-auth-server/lib/payments/stripe.ts | |
+++ b/packages/fxa-auth-server/lib/payments/stripe.ts | |
@@ -833,24 +833,9 @@ export class StripeHelper { | |
customerId: string; | |
postalCode: string; | |
country: string; | |
- }): Promise<boolean> { | |
- try { | |
- const state = await this.googleMapsService.getStateFromZip( | |
- postalCode, | |
- country | |
- ); | |
- | |
- await this.updateCustomerBillingAddress(customerId, { | |
- line1: '', | |
- line2: '', | |
- city: '', | |
- state, | |
- country, | |
- postalCode, | |
- }); | |
- | |
- return true; | |
- } catch (err: unknown) { | |
+ }): Promise<void> { | |
+ let state; | |
+ const reportToSentry = (err: unknown) => { | |
Sentry.withScope((scope) => { | |
scope.setContext('setCustomerLocation', { | |
customer: { id: customerId }, | |
@@ -860,8 +845,24 @@ export class StripeHelper { | |
Sentry.captureException(err); | |
}); | |
} | |
- | |
- return false; | |
+ try { | |
+ state = await this.googleMapsService.getStateFromZip(postalCode, country); | |
+ } catch (err: unknown) { | |
+ reportToSentry(err); | |
+ } finally { | |
+ try { | |
+ await this.updateCustomerBillingAddress(customerId, { | |
+ line1: '', | |
+ line2: '', | |
+ city: '', | |
+ state: state || 'unknown', | |
+ country, | |
+ postalCode, | |
+ }); | |
+ } catch (err: unknown) { | |
+ reportToSentry(err); | |
+ } | |
+ } | |
} | |
/** | |
diff --git a/packages/fxa-auth-server/lib/routes/subscriptions/paypal.ts b/packages/fxa-auth-server/lib/routes/subscriptions/paypal.ts | |
index c35c33d51..c2eed4523 100644 | |
--- a/packages/fxa-auth-server/lib/routes/subscriptions/paypal.ts | |
+++ b/packages/fxa-auth-server/lib/routes/subscriptions/paypal.ts | |
@@ -398,18 +398,25 @@ export class PayPalHandler extends StripeWebhookHandler { | |
const accountCustomer = await getAccountCustomerByUid(uid); | |
if (accountCustomer.stripeCustomerId) { | |
let locationDetails = {} as any; | |
+ // Record the state (short name) if in a needed country | |
+ const country = Object.keys(COUNTRIES_LONG_NAME_TO_SHORT_NAME_MAP).find( | |
+ (key) => | |
+ COUNTRIES_LONG_NAME_TO_SHORT_NAME_MAP[key] === | |
+ agreementDetails.countryCode | |
+ ); | |
if (agreementDetails.countryCode === options.location?.countryCode) { | |
- // Convert the state long name to a short name | |
const state = options.location?.state; | |
- const country = Object.keys(COUNTRIES_LONG_NAME_TO_SHORT_NAME_MAP).find( | |
- (key) => | |
- COUNTRIES_LONG_NAME_TO_SHORT_NAME_MAP[key] === | |
- agreementDetails.countryCode | |
- ); | |
if (country && stateNames[country][state]) { | |
locationDetails.state = stateNames[country][state]; | |
} | |
} | |
+ if (country && !locationDetails.state) { | |
+ this.log.info('createAndVerifyBillingAgreement.unknownState', { | |
+ message: 'Could not infer state for the PayPal customer.', | |
+ customerId: accountCustomer.stripeCustomerId, | |
+ }); | |
+ locationDetails.state = "unknown"; | |
+ } | |
this.stripeHelper.updateCustomerBillingAddress( | |
accountCustomer.stripeCustomerId, | |
{ | |
diff --git a/packages/fxa-auth-server/test/local/payments/stripe.js b/packages/fxa-auth-server/test/local/payments/stripe.js | |
index 9281326c0..c63438fcd 100644 | |
--- a/packages/fxa-auth-server/test/local/payments/stripe.js | |
+++ b/packages/fxa-auth-server/test/local/payments/stripe.js | |
@@ -4778,12 +4778,11 @@ describe('StripeHelper', () => { | |
it('updates the Stripe customer address', async () => { | |
sandbox.stub(stripeHelper, 'updateCustomerBillingAddress').resolves(); | |
- const actual = await stripeHelper.setCustomerLocation({ | |
+ await stripeHelper.setCustomerLocation({ | |
customerId: customer1.id, | |
postalCode: expectedAddressArg.postalCode, | |
country: expectedAddressArg.country, | |
}); | |
- assert.isTrue(actual); | |
sinon.assert.calledOnceWithExactly( | |
stripeHelper.googleMapsService.getStateFromZip, | |
'99999', | |
@@ -4797,20 +4796,22 @@ describe('StripeHelper', () => { | |
}); | |
it('fails when an error is thrown by Google Maps service', async () => { | |
+ const expectedAddressArgUnknownState = { | |
+ ...deepCopy(expectedAddressArg), | |
+ state: 'unknown', | |
+ } | |
sandbox.stub(stripeHelper, 'updateCustomerBillingAddress').resolves(); | |
mockGoogleMapsService.getStateFromZip = sandbox.stub().rejects(err); | |
- const actual = await stripeHelper.setCustomerLocation({ | |
+ await stripeHelper.setCustomerLocation({ | |
customerId: customer1.id, | |
postalCode: expectedAddressArg.postalCode, | |
country: expectedAddressArg.country, | |
}); | |
- assert.isFalse(actual); | |
sinon.assert.calledOnceWithExactly( | |
stripeHelper.googleMapsService.getStateFromZip, | |
'99999', | |
'GD' | |
); | |
- sinon.assert.notCalled(stripeHelper.updateCustomerBillingAddress); | |
sinon.assert.calledOnce(Sentry.withScope); | |
sinon.assert.calledOnceWithExactly( | |
sentryScope.setContext, | |
@@ -4822,16 +4823,20 @@ describe('StripeHelper', () => { | |
} | |
); | |
sinon.assert.calledOnceWithExactly(Sentry.captureException, err); | |
+ sinon.assert.calledOnceWithExactly( | |
+ stripeHelper.updateCustomerBillingAddress, | |
+ customer1.id, | |
+ expectedAddressArgUnknownState, | |
+ ); | |
}); | |
it('fails when an error is thrown while updating the customer address', async () => { | |
sandbox.stub(stripeHelper, 'updateCustomerBillingAddress').rejects(err); | |
- const actual = await stripeHelper.setCustomerLocation({ | |
+ await stripeHelper.setCustomerLocation({ | |
customerId: customer1.id, | |
postalCode: expectedAddressArg.postalCode, | |
country: expectedAddressArg.country, | |
}); | |
- assert.isFalse(actual); | |
sinon.assert.calledOnceWithExactly( | |
stripeHelper.googleMapsService.getStateFromZip, | |
'99999', | |
diff --git a/packages/fxa-auth-server/test/local/routes/subscriptions/paypal.js b/packages/fxa-auth-server/test/local/routes/subscriptions/paypal.js | |
index 660e33972..9e791c3c4 100644 | |
--- a/packages/fxa-auth-server/test/local/routes/subscriptions/paypal.js | |
+++ b/packages/fxa-auth-server/test/local/routes/subscriptions/paypal.js | |
@@ -87,6 +87,12 @@ describe('subscriptions payPalRoutes', () => { | |
email: `${TEST_EMAIL}`, | |
}, | |
}, | |
+ geo: { | |
+ location: { | |
+ countryCode: 'CA', | |
+ state: 'Ontario', | |
+ }, | |
+ }, | |
}; | |
const authDbModule = require('fxa-shared/db/models/auth'); | |
const accountCustomer = { stripeCustomerId: 'accountCustomer' }; | |
@@ -375,7 +381,7 @@ describe('subscriptions payPalRoutes', () => { | |
line1: undefined, | |
line2: undefined, | |
postalCode: undefined, | |
- state: undefined, | |
+ state: 'ON', | |
} | |
); | |
}); | |
@@ -409,7 +415,7 @@ describe('subscriptions payPalRoutes', () => { | |
line1: undefined, | |
line2: undefined, | |
postalCode: undefined, | |
- state: undefined, | |
+ state: 'ON', | |
} | |
); | |
}); | |
@@ -496,7 +502,7 @@ describe('subscriptions payPalRoutes', () => { | |
line1: undefined, | |
line2: undefined, | |
postalCode: undefined, | |
- state: undefined, | |
+ state: 'ON', | |
} | |
); | |
}); | |
@@ -515,6 +521,32 @@ describe('subscriptions payPalRoutes', () => { | |
sinon.assert.calledOnce(payPalHelper.cancelBillingAgreement); | |
} | |
}); | |
+ | |
+ it('should record \'unknown\' as the state if it cannot be inferred', async () => { | |
+ const requestOptions = deepCopy(defaultRequestOptions); | |
+ requestOptions.geo = { | |
+ location: { | |
+ countryCode: 'DE', | |
+ state: 'Saxony-Anhalt', | |
+ }, | |
+ }; | |
+ await runTest('/oauth/subscriptions/active/new-paypal', { | |
+ ...requestOptions, | |
+ payload: { token }, | |
+ }); | |
+ sinon.assert.calledOnceWithExactly( | |
+ stripeHelper.updateCustomerBillingAddress, | |
+ accountCustomer.stripeCustomerId, | |
+ { | |
+ city: undefined, | |
+ country: 'CA', | |
+ line1: undefined, | |
+ line2: undefined, | |
+ postalCode: undefined, | |
+ state: 'unknown', | |
+ } | |
+ ); | |
+ }); | |
}); | |
describe('new subscription with an existing billing agreement', () => { |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment