Przeglądaj źródła

Merge pull request #4248 from overleaf/tm-ta-new-subscription-currency-fixes

Fix getCurrencyCode behavior and add fallback in frontend for invalid currencies

GitOrigin-RevId: 5cc40b524148f88c0f110d2cf4bb4d3b69844f1b
Thomas 5 lat temu
rodzic
commit
be73463e35

+ 4 - 2
services/web/app/src/Features/Subscription/SubscriptionController.js

@@ -26,7 +26,9 @@ const SUBSCRIPTION_PAGE_SPLIT_TEST = 'subscription-page'
 async function plansPage(req, res) {
   const plans = SubscriptionViewModelBuilder.buildPlansList()
 
-  const recommendedCurrency = await GeoIpLookup.promises.getCurrencyCode(
+  const {
+    currencyCode: recommendedCurrency,
+  } = await GeoIpLookup.promises.getCurrencyCode(
     (req.query ? req.query.ip : undefined) || req.ip
   )
 
@@ -66,7 +68,7 @@ async function paymentPage(req, res) {
         ? req.query.currency.toUpperCase()
         : undefined
       const {
-        recomendedCurrency: recommendedCurrency,
+        currencyCode: recommendedCurrency,
         countryCode,
       } = await GeoIpLookup.promises.getCurrencyCode(
         (req.query ? req.query.ip : undefined) || req.ip

+ 5 - 2
services/web/app/src/infrastructure/GeoIpLookup.js

@@ -3,7 +3,7 @@ const settings = require('settings-sharelatex')
 const _ = require('underscore')
 const logger = require('logger-sharelatex')
 const URL = require('url')
-const { promisify } = require('../util/promises')
+const { promisify, promisifyMultiResult } = require('../util/promises')
 
 const currencyMappings = {
   GB: 'GBP',
@@ -91,6 +91,9 @@ module.exports = {
   getCurrencyCode,
   promises: {
     getDetails: promisify(getDetails),
-    getCurrencyCode: promisify(getCurrencyCode),
+    getCurrencyCode: promisifyMultiResult(getCurrencyCode, [
+      'currencyCode',
+      'countryCode',
+    ]),
   },
 }

+ 23 - 9
services/web/frontend/js/main/new-subscription.js

@@ -103,15 +103,29 @@ export default App.controller(
     const pricing = recurly.Pricing()
     window.pricing = pricing
 
-    pricing
-      .plan(window.plan_code, { quantity: 1 })
-      .address({
-        country: $scope.data.country,
-      })
-      .tax({ tax_code: 'digital', vat_number: '' })
-      .currency($scope.currencyCode)
-      .coupon($scope.data.coupon)
-      .done()
+    function setupPricing() {
+      pricing
+        .plan(window.plan_code, { quantity: 1 })
+        .address({
+          country: $scope.data.country,
+        })
+        .tax({ tax_code: 'digital', vat_number: '' })
+        .currency($scope.currencyCode)
+        .coupon($scope.data.coupon)
+        .catch(function (err) {
+          if (
+            $scope.currencyCode !== 'USD' &&
+            err.name === 'invalid-currency'
+          ) {
+            $scope.currencyCode = 'USD'
+            setupPricing()
+          } else {
+            throw err
+          }
+        })
+        .done()
+    }
+    setupPricing()
 
     pricing.on('change', () => {
       $scope.planName = pricing.items.plan.name

+ 4 - 4
services/web/test/unit/src/Subscription/SubscriptionControllerTests.js

@@ -161,9 +161,9 @@ describe('SubscriptionController', function () {
   describe('plansPage', function () {
     beforeEach(function () {
       this.req.ip = '1234.3123.3131.333 313.133.445.666 653.5345.5345.534'
-      return this.GeoIpLookup.promises.getCurrencyCode.resolves(
-        this.stubbedCurrencyCode
-      )
+      return this.GeoIpLookup.promises.getCurrencyCode.resolves({
+        currencyCode: this.stubbedCurrencyCode,
+      })
     })
   })
 
@@ -174,7 +174,7 @@ describe('SubscriptionController', function () {
         .stub()
         .resolves(true)
       return this.GeoIpLookup.promises.getCurrencyCode.resolves({
-        recomendedCurrency: this.stubbedCurrencyCode,
+        currencyCode: this.stubbedCurrencyCode,
       })
     })
 

+ 36 - 21
services/web/test/unit/src/infrastructure/GeoIpLookupTest.js

@@ -2,6 +2,7 @@ const SandboxedModule = require('sandboxed-module')
 const assert = require('assert')
 const path = require('path')
 const sinon = require('sinon')
+const { expect } = require('chai')
 const modulePath = path.join(
   __dirname,
   '../../../../app/src/infrastructure/GeoIpLookup'
@@ -204,62 +205,76 @@ describe('GeoIpLookup', function () {
     describe('async', function () {
       it('should return GBP for GB country', async function () {
         this.request.get.callsArgWith(1, null, null, this.stubbedResponse)
-        const currencyCode = await this.GeoIpLookup.promises.getCurrencyCode(
-          this.ipAddress
-        )
+        const {
+          currencyCode,
+          countryCode,
+        } = await this.GeoIpLookup.promises.getCurrencyCode(this.ipAddress)
         currencyCode.should.equal('GBP')
+        countryCode.should.equal('GB')
       })
 
       it('should return GBP for gb country', async function () {
         this.stubbedResponse.country_code = 'gb'
         this.request.get.callsArgWith(1, null, null, this.stubbedResponse)
-        const currencyCode = await this.GeoIpLookup.promises.getCurrencyCode(
-          this.ipAddress
-        )
+        const {
+          currencyCode,
+          countryCode,
+        } = await this.GeoIpLookup.promises.getCurrencyCode(this.ipAddress)
         currencyCode.should.equal('GBP')
+        countryCode.should.equal('GB')
       })
 
       it('should return USD for US', async function () {
         this.stubbedResponse.country_code = 'US'
         this.request.get.callsArgWith(1, null, null, this.stubbedResponse)
-        const currencyCode = await this.GeoIpLookup.promises.getCurrencyCode(
-          this.ipAddress
-        )
+        const {
+          currencyCode,
+          countryCode,
+        } = await this.GeoIpLookup.promises.getCurrencyCode(this.ipAddress)
         currencyCode.should.equal('USD')
+        countryCode.should.equal('US')
       })
 
       it('should return EUR for DE', async function () {
         this.stubbedResponse.country_code = 'DE'
         this.request.get.callsArgWith(1, null, null, this.stubbedResponse)
-        const currencyCode = await this.GeoIpLookup.promises.getCurrencyCode(
-          this.ipAddress
-        )
+        const {
+          currencyCode,
+          countryCode,
+        } = await this.GeoIpLookup.promises.getCurrencyCode(this.ipAddress)
         currencyCode.should.equal('EUR')
+        countryCode.should.equal('DE')
       })
 
       it('should default to USD if there is an error', async function () {
         this.request.get.callsArgWith(1, null, null, { error: true })
-        const currencyCode = await this.GeoIpLookup.promises.getCurrencyCode(
-          this.ipAddress
-        )
+        const {
+          currencyCode,
+          countryCode,
+        } = await this.GeoIpLookup.promises.getCurrencyCode(this.ipAddress)
         currencyCode.should.equal('USD')
+        expect(countryCode).to.be.undefined
       })
 
       it('should default to USD if there are no details', async function () {
         this.request.get.callsArgWith(1, null, null, {})
-        const currencyCode = await this.GeoIpLookup.promises.getCurrencyCode(
-          this.ipAddress
-        )
+        const {
+          currencyCode,
+          countryCode,
+        } = await this.GeoIpLookup.promises.getCurrencyCode(this.ipAddress)
         currencyCode.should.equal('USD')
+        expect(countryCode).to.be.undefined
       })
 
       it('should default to USD if there is no match for their country', async function () {
         this.stubbedResponse.country_code = 'Non existant'
         this.request.get.callsArgWith(1, null, null, this.stubbedResponse)
-        const currencyCode = await this.GeoIpLookup.promises.getCurrencyCode(
-          this.ipAddress
-        )
+        const {
+          currencyCode,
+          countryCode,
+        } = await this.GeoIpLookup.promises.getCurrencyCode(this.ipAddress)
         currencyCode.should.equal('USD')
+        countryCode.should.equal('NON EXISTANT')
       })
     })
   })