From 344b2b2395bb305afa4312c37752dc4bdfce0aa9 Mon Sep 17 00:00:00 2001
From: nate stemen <nate.stemen@overleaf.com>
Date: Mon, 30 Mar 2020 14:12:32 -0400
Subject: [PATCH] Merge pull request #2702 from overleaf/ta-subscription-links

Fix Language Change Links

GitOrigin-RevId: e47f59dfd53102d4569c3ffeb950d3259c65215b
---
 .../Subscription/SubscriptionController.js      |  9 ++++++++-
 .../web/app/src/infrastructure/ExpressLocals.js |  4 +++-
 services/web/app/views/layout/footer.pug        |  2 +-
 .../Subscription/SubscriptionControllerTests.js | 17 ++++++++++++-----
 4 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/services/web/app/src/Features/Subscription/SubscriptionController.js b/services/web/app/src/Features/Subscription/SubscriptionController.js
index 87664f914d..8afa415c1b 100644
--- a/services/web/app/src/Features/Subscription/SubscriptionController.js
+++ b/services/web/app/src/Features/Subscription/SubscriptionController.js
@@ -81,6 +81,13 @@ module.exports = SubscriptionController = {
   paymentPage(req, res, next) {
     const user = AuthenticationController.getSessionUser(req)
     const plan = PlansLocator.findLocalPlanInSettings(req.query.planCode)
+    if (!plan) {
+      return next(
+        new HttpErrors.UnprocessableEntityError({
+          info: { public: { message: 'Plan not found' } }
+        })
+      )
+    }
     return LimitationsManager.userHasV1OrV2Subscription(user, function(
       err,
       hasSubscription
@@ -88,7 +95,7 @@ module.exports = SubscriptionController = {
       if (err != null) {
         return next(err)
       }
-      if (hasSubscription || plan == null) {
+      if (hasSubscription) {
         return res.redirect('/user/subscription?hasSubscription=true')
       } else {
         // LimitationsManager.userHasV2Subscription only checks Mongo. Double check with
diff --git a/services/web/app/src/infrastructure/ExpressLocals.js b/services/web/app/src/infrastructure/ExpressLocals.js
index b8c9cb207a..936aaacb0b 100644
--- a/services/web/app/src/infrastructure/ExpressLocals.js
+++ b/services/web/app/src/infrastructure/ExpressLocals.js
@@ -196,7 +196,9 @@ module.exports = function(webRouter, privateApiRouter, publicApiRouter) {
     }
     // Don't include the query string parameters, otherwise Google
     // treats ?nocdn=true as the canonical version
-    res.locals.currentUrl = Url.parse(req.originalUrl).pathname
+    const parsedOriginalUrl = Url.parse(req.originalUrl)
+    res.locals.currentUrl = parsedOriginalUrl.pathname
+    res.locals.currentUrlWithQueryParams = parsedOriginalUrl.path
     res.locals.capitalize = function(string) {
       if (string.length === 0) {
         return ''
diff --git a/services/web/app/views/layout/footer.pug b/services/web/app/views/layout/footer.pug
index 1120768099..a7e7a56b81 100644
--- a/services/web/app/views/layout/footer.pug
+++ b/services/web/app/views/layout/footer.pug
@@ -23,7 +23,7 @@ footer.site-footer
 							each subdomainDetails, subdomain in settings.i18n.subdomainLang
 								if !subdomainDetails.hide
 									li.lngOption
-										a.menu-indent(href=subdomainDetails.url+currentUrl)
+										a.menu-indent(href=subdomainDetails.url+currentUrlWithQueryParams)
 											figure(class="sprite-icon sprite-icon-lang sprite-icon-"+subdomainDetails.lngCode alt=translate(subdomainDetails.lngCode))
 											|  #{translate(subdomainDetails.lngCode)}
 										//- img(src="/img/flags/24/.png")
diff --git a/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js b/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js
index 6a660582b7..dec8efeddb 100644
--- a/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js
+++ b/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js
@@ -225,6 +225,7 @@ describe('SubscriptionController', function() {
 
     describe('with a user with subscription', function() {
       it('should redirect to the subscription dashboard', function(done) {
+        this.PlansLocator.findLocalPlanInSettings.returns({})
         this.LimitationsManager.userHasV1OrV2Subscription.callsArgWith(
           1,
           null,
@@ -239,18 +240,23 @@ describe('SubscriptionController', function() {
     })
 
     describe('with an invalid plan code', function() {
-      it('should redirect to the subscription dashboard', function(done) {
+      it('should return 422 error', function(done) {
         this.LimitationsManager.userHasV1OrV2Subscription.callsArgWith(
           1,
           null,
           false
         )
         this.PlansLocator.findLocalPlanInSettings.returns(null)
-        this.res.redirect = url => {
-          url.should.equal('/user/subscription?hasSubscription=true')
-          return done()
+        this.next = error => {
+          expect(error).to.exist
+          expect(error.statusCode).to.equal(422)
+          done()
         }
-        return this.SubscriptionController.paymentPage(this.req, this.res)
+        return this.SubscriptionController.paymentPage(
+          this.req,
+          this.res,
+          this.next
+        )
       })
     })
 
@@ -295,6 +301,7 @@ describe('SubscriptionController', function() {
 
     describe('with a recurly subscription already', function() {
       it('should redirect to the subscription dashboard', function(done) {
+        this.PlansLocator.findLocalPlanInSettings.returns({})
         this.LimitationsManager.userHasV1OrV2Subscription.callsArgWith(
           1,
           null,