Jelajahi Sumber

Merge pull request #8240 from overleaf/tm-teardown-persistent-upgrade-test

Teardown persistent upgrade prompt split test and fix styling

GitOrigin-RevId: 8776231c3308c5f08b91232c962c9796a5f4afa0
Thomas 4 tahun lalu
induk
melakukan
5e2933e2fe

+ 2 - 45
services/web/app/src/Features/Project/ProjectController.js

@@ -459,38 +459,14 @@ const ProjectController = {
             }
           )
         },
-
-        persistentUpgradePromptsAssignment(cb) {
-          SplitTestHandler.getAssignment(
-            req,
-            res,
-            'persistent-upgrade-prompt',
-            (err, assignment) => {
-              if (err) {
-                logger.warn(
-                  { err },
-                  'failed to get "persistent-upgrade-prompt" split test assignment'
-                )
-                cb(null, { variant: 'default' })
-              } else {
-                cb(null, assignment)
-              }
-            }
-          )
-        },
       },
       (err, results) => {
         if (err != null) {
           OError.tag(err, 'error getting data for project list page')
           return next(err)
         }
-        const {
-          notifications,
-          user,
-          userEmailsData,
-          primaryEmailCheckActive,
-          persistentUpgradePromptsAssignment,
-        } = results
+        const { notifications, user, userEmailsData, primaryEmailCheckActive } =
+          results
 
         if (
           user &&
@@ -621,7 +597,6 @@ const ProjectController = {
 
         // Persistent upgrade prompts
         const showToolbarUpgradePrompt =
-          persistentUpgradePromptsAssignment.variant === 'persistent-upgrade' &&
           !results.hasSubscription &&
           !userEmails.some(e => e.emailHasInstitutionLicence)
 
@@ -909,21 +884,6 @@ const ProjectController = {
             }
           )
         },
-        persistentUpgradePromptsAssignment(cb) {
-          SplitTestHandler.getAssignment(
-            req,
-            res,
-            'persistent-upgrade-prompt',
-            (error, assignment) => {
-              // do not fail editor load if assignment fails
-              if (error) {
-                cb(null, { variant: 'default' })
-              } else {
-                cb(null, assignment)
-              }
-            }
-          )
-        },
       },
       (
         err,
@@ -940,7 +900,6 @@ const ProjectController = {
           pdfDetachAssignment,
           pdfjsAssignment,
           dictionaryEditorAssignment,
-          persistentUpgradePromptsAssignment,
         }
       ) => {
         if (err != null) {
@@ -1053,8 +1012,6 @@ const ProjectController = {
 
             // Persistent upgrade prompts
             const showHeaderUpgradePrompt =
-              persistentUpgradePromptsAssignment.variant ===
-                'persistent-upgrade' &&
               userId &&
               !subscription &&
               !userIsMemberOfGroupSubscription &&

+ 0 - 10
services/web/app/views/project/list/side-bar.pug

@@ -133,13 +133,3 @@ if (isOverleaf)
 					a.btn.btn-info(
 						href="/user/settings"
 					) Add Affiliation
-
-		unless (showToolbarUpgradePrompt)
-			.row-spaced(ng-if="hasProjects && userHasNoSubscription && !userOnPayingUniversity", ng-cloak).text-centered
-				hr
-				p.small #{translate("on_free_sl")}
-				p
-					a(href="/user/subscription/plans" ng-click="upgradeSubscription()").btn.btn-primary #{translate("upgrade")}
-				p.small.text-centered
-					| #{translate("or_unlock_features_bonus")} 
-					a(href="/user/bonus" ng-click="share()") #{translate("sharing_sl")}.

+ 0 - 15
services/web/frontend/js/main/project-list/left-hand-menu-promo-controller.js

@@ -4,24 +4,9 @@ export default App.controller(
   'LeftHandMenuPromoController',
   function ($scope, UserAffiliationsDataService, eventTracking) {
     $scope.hasProjects = window.data.projects.length > 0
-    $scope.userHasNoSubscription = window.userHasNoSubscription
-
-    $scope.upgradeSubscription = function () {
-      eventTracking.send('subscription-funnel', 'project-page', 'upgrade')
-      eventTracking.sendMB('upgrade-button-click', {
-        source: 'dashboard',
-      })
-    }
-
-    $scope.share = function () {
-      eventTracking.send('subscription-funnel', 'project-page', 'sharing')
-    }
 
     const _userHasNoAffiliation = function () {
       $scope.withAffiliations = window.data.userAffiliations.length > 0
-      $scope.userOnPayingUniversity = window.data.userAffiliations.some(
-        affiliation => affiliation.licence && affiliation.licence !== 'free'
-      )
     }
 
     _userHasNoAffiliation()

+ 1 - 0
services/web/frontend/stylesheets/app/editor/toolbar.less

@@ -198,6 +198,7 @@
 }
 
 .toolbar-header-upgrade-prompt {
+  margin-left: 10px;
   @media (max-width: @screen-md-min) {
     display: none;
   }

+ 64 - 98
services/web/test/unit/src/Project/ProjectControllerTests.js

@@ -556,53 +556,36 @@ describe('ProjectController', function () {
     })
 
     describe('persistent upgrade prompt', function () {
-      describe('if the user has the default variant', function (done) {
-        it('should not show', function (done) {
-          this.res.render = (pageName, opts) => {
-            expect(opts.showToolbarUpgradePrompt).to.equal(false)
-            done()
-          }
-          this.ProjectController.projectListPage(this.req, this.res)
-        })
+      it('should show for a user without a subscription or only non-paid affiliations', function (done) {
+        this.res.render = (pageName, opts) => {
+          expect(opts.showToolbarUpgradePrompt).to.equal(true)
+          done()
+        }
+        this.ProjectController.projectListPage(this.req, this.res)
       })
-
-      describe('if the user has the persistent-upgrade variant', function (done) {
-        beforeEach(function () {
-          this.SplitTestHandler.getAssignment
-            .withArgs(this.req, this.res, 'persistent-upgrade-prompt')
-            .yields(null, { variant: 'persistent-upgrade' })
-        })
-        it('should show for a user without a subscription or only non-paid affiliations', function (done) {
-          this.res.render = (pageName, opts) => {
-            expect(opts.showToolbarUpgradePrompt).to.equal(true)
-            done()
-          }
-          this.ProjectController.projectListPage(this.req, this.res)
-        })
-        it('should not show for a user with a subscription', function (done) {
-          this.LimitationsManager.hasPaidSubscription = sinon
-            .stub()
-            .callsArgWith(1, null, true)
-          this.res.render = (pageName, opts) => {
-            expect(opts.showToolbarUpgradePrompt).to.equal(false)
-            done()
-          }
-          this.ProjectController.projectListPage(this.req, this.res)
-        })
-        it('should not show for a user with an affiliated paid university', function (done) {
-          const emailWithProAffiliation = {
-            email: 'pro@example.com',
-            emailHasInstitutionLicence: true,
-          }
-          this.UserGetter.getUserFullEmails = sinon
-            .stub()
-            .yields(null, [emailWithProAffiliation])
-          this.res.render = (pageName, opts) => {
-            expect(opts.showToolbarUpgradePrompt).to.equal(false)
-            done()
-          }
-          this.ProjectController.projectListPage(this.req, this.res)
-        })
+      it('should not show for a user with a subscription', function (done) {
+        this.LimitationsManager.hasPaidSubscription = sinon
+          .stub()
+          .callsArgWith(1, null, true)
+        this.res.render = (pageName, opts) => {
+          expect(opts.showToolbarUpgradePrompt).to.equal(false)
+          done()
+        }
+        this.ProjectController.projectListPage(this.req, this.res)
+      })
+      it('should not show for a user with an affiliated paid university', function (done) {
+        const emailWithProAffiliation = {
+          email: 'pro@example.com',
+          emailHasInstitutionLicence: true,
+        }
+        this.UserGetter.getUserFullEmails = sinon
+          .stub()
+          .yields(null, [emailWithProAffiliation])
+        this.res.render = (pageName, opts) => {
+          expect(opts.showToolbarUpgradePrompt).to.equal(false)
+          done()
+        }
+        this.ProjectController.projectListPage(this.req, this.res)
       })
     })
 
@@ -1509,59 +1492,42 @@ describe('ProjectController', function () {
           .stub()
           .callsArgWith(1, null, null)
       })
-      describe('if the user has the default variant', function (done) {
-        it('should not show', function (done) {
-          this.res.render = (pageName, opts) => {
-            expect(opts.showHeaderUpgradePrompt).to.equal(false)
-            done()
-          }
-          this.ProjectController.loadEditor(this.req, this.res)
-        })
+      it('should show for a user without a subscription or only non-paid affiliations', function (done) {
+        this.res.render = (pageName, opts) => {
+          expect(opts.showHeaderUpgradePrompt).to.equal(true)
+          done()
+        }
+        this.ProjectController.loadEditor(this.req, this.res)
       })
-
-      describe('if the user has the persistent-upgrade variant', function (done) {
-        beforeEach(function () {
-          this.SplitTestHandler.getAssignment
-            .withArgs(this.req, this.res, 'persistent-upgrade-prompt')
-            .yields(null, { variant: 'persistent-upgrade' })
-        })
-        it('should show for a user without a subscription or only non-paid affiliations', function (done) {
-          this.res.render = (pageName, opts) => {
-            expect(opts.showHeaderUpgradePrompt).to.equal(true)
-            done()
-          }
-          this.ProjectController.loadEditor(this.req, this.res)
-        })
-        it('should not show for a user with a personal subscription', function (done) {
-          this.SubscriptionLocator.getUsersSubscription = sinon
-            .stub()
-            .callsArgWith(1, null, {})
-          this.res.render = (pageName, opts) => {
-            expect(opts.showHeaderUpgradePrompt).to.equal(false)
-            done()
-          }
-          this.ProjectController.loadEditor(this.req, this.res)
-        })
-        it('should not show for a user who is a member of a group subscription', function (done) {
-          this.LimitationsManager.userIsMemberOfGroupSubscription = sinon
-            .stub()
-            .callsArgWith(1, null, true)
-          this.res.render = (pageName, opts) => {
-            expect(opts.showHeaderUpgradePrompt).to.equal(false)
-            done()
-          }
-          this.ProjectController.loadEditor(this.req, this.res)
-        })
-        it('should not show for a user with an affiliated paid university', function (done) {
-          this.InstitutionsFeatures.hasLicence = sinon
-            .stub()
-            .callsArgWith(1, null, true)
-          this.res.render = (pageName, opts) => {
-            expect(opts.showHeaderUpgradePrompt).to.equal(false)
-            done()
-          }
-          this.ProjectController.loadEditor(this.req, this.res)
-        })
+      it('should not show for a user with a personal subscription', function (done) {
+        this.SubscriptionLocator.getUsersSubscription = sinon
+          .stub()
+          .callsArgWith(1, null, {})
+        this.res.render = (pageName, opts) => {
+          expect(opts.showHeaderUpgradePrompt).to.equal(false)
+          done()
+        }
+        this.ProjectController.loadEditor(this.req, this.res)
+      })
+      it('should not show for a user who is a member of a group subscription', function (done) {
+        this.LimitationsManager.userIsMemberOfGroupSubscription = sinon
+          .stub()
+          .callsArgWith(1, null, true)
+        this.res.render = (pageName, opts) => {
+          expect(opts.showHeaderUpgradePrompt).to.equal(false)
+          done()
+        }
+        this.ProjectController.loadEditor(this.req, this.res)
+      })
+      it('should not show for a user with an affiliated paid university', function (done) {
+        this.InstitutionsFeatures.hasLicence = sinon
+          .stub()
+          .callsArgWith(1, null, true)
+        this.res.render = (pageName, opts) => {
+          expect(opts.showHeaderUpgradePrompt).to.equal(false)
+          done()
+        }
+        this.ProjectController.loadEditor(this.req, this.res)
       })
     })
   })