فهرست منبع

Merge pull request #4645 from overleaf/jpa-pdf-caching-opt-in

[misc] make PDF caching an opt-in feature

GitOrigin-RevId: 85ea5739d7bbeea3ac2517ec99f90f2beec2a768
Jakob Ackermann 5 سال پیش
والد
کامیت
006a140fb8

+ 7 - 36
services/web/app/src/Features/Project/ProjectController.js

@@ -37,7 +37,6 @@ const BrandVariationsHandler = require('../BrandVariations/BrandVariationsHandle
 const UserController = require('../User/UserController')
 const AnalyticsManager = require('../Analytics/AnalyticsManager')
 const Modules = require('../../infrastructure/Modules')
-const SplitTestHandler = require('../SplitTests/SplitTestHandler')
 const SplitTestV2Handler = require('../SplitTests/SplitTestV2Handler')
 const { getNewLogsUIVariantForUser } = require('../Helpers/NewLogsUI')
 
@@ -711,21 +710,6 @@ const ProjectController = {
         flushToTpds: cb => {
           TpdsProjectFlusher.flushProjectToTpdsIfNeeded(projectId, cb)
         },
-        pdfCachingFeatureFlag(cb) {
-          if (!Settings.enablePdfCaching) return cb(null, '')
-          if (!userId) return cb(null, 'enable-caching-only')
-          SplitTestHandler.getTestSegmentation(
-            userId,
-            'pdf_caching_full',
-            (err, segmentation) => {
-              if (err) {
-                // Do not fail loading the editor.
-                return cb(null, '')
-              }
-              cb(null, (segmentation && segmentation.variant) || '')
-            }
-          )
-        },
         sharingModalSplitTest(cb) {
           SplitTestV2Handler.assignInLocalsContext(
             res,
@@ -737,17 +721,7 @@ const ProjectController = {
           )
         },
       },
-      (
-        err,
-        {
-          project,
-          user,
-          subscription,
-          isTokenMember,
-          brandVariation,
-          pdfCachingFeatureFlag,
-        }
-      ) => {
+      (err, { project, user, subscription, isTokenMember, brandVariation }) => {
         if (err != null) {
           OError.tag(err, 'error getting details for project page')
           return next(err)
@@ -826,15 +800,10 @@ const ProjectController = {
                 // The feature is disabled globally.
                 return false
               }
-              const canSeeFeaturePreview = pdfCachingFeatureFlag.includes(flag)
-              if (!canSeeFeaturePreview) {
-                // The user is not in the target group.
-                return false
-              }
-              // Optionally let the user opt-out.
-              // The will opt-out of both caching and metrics collection,
+              // Let the user opt-in only.
+              // The flag will opt-out of both caching and metrics collection,
               //  as if this editing session never happened.
-              return shouldDisplayFeature('enable_pdf_caching', true)
+              return shouldDisplayFeature('enable_pdf_caching', false)
             }
 
             res.render('project/editor', {
@@ -909,7 +878,9 @@ const ProjectController = {
               ),
               trackPdfDownload: partOfPdfCachingRollout('collect-metrics'),
               enablePdfCaching: partOfPdfCachingRollout('enable-caching'),
-              resetServiceWorker: Boolean(Settings.resetServiceWorker),
+              resetServiceWorker:
+                Boolean(Settings.resetServiceWorker) &&
+                !shouldDisplayFeature('enable_pdf_caching', false),
             })
             timer.done()
           }

+ 26 - 0
services/web/test/unit/src/Project/ProjectControllerTests.js

@@ -1237,6 +1237,9 @@ describe('ProjectController', function () {
       })
 
       describe('during regular roll-out', function () {
+        before(function () {
+          this.skip()
+        })
         describe('disabled', function () {
           showNoVariant()
 
@@ -1285,6 +1288,29 @@ describe('ProjectController', function () {
           })
         })
       })
+
+      describe('during opt-in only', function () {
+        describe('with no query', function () {
+          expectBandwidthTrackingDisabled()
+          expectPDFCachingDisabled()
+        })
+
+        describe('with enable_pdf_caching=false', function () {
+          beforeEach(function () {
+            this.req.query.enable_pdf_caching = 'false'
+          })
+          expectBandwidthTrackingDisabled()
+          expectPDFCachingDisabled()
+        })
+
+        describe('with enable_pdf_caching=true', function () {
+          beforeEach(function () {
+            this.req.query.enable_pdf_caching = 'true'
+          })
+          expectBandwidthTrackingEnabled()
+          expectPDFCachingEnabled()
+        })
+      })
     })
 
     describe('wsUrl', function () {