Merge pull request #18397 from overleaf/em-revert-download-all-link

Revert "Merge pull request #18190 from overleaf/ar-add-download-all-l…

GitOrigin-RevId: 681eb2734636d76558e682dc85083bfcaa6b7d2d

services/clsi/app/js/CompileController.js

@@ -12,17 +12,6 @@ function timeSinceLastSuccessfulCompile() {
   return Date.now() - lastSuccessfulCompileTimestamp
 }
 
-function addUrlToOutputFile(outputFile, projectId, userId) {
-  return {
-    url:
-      `${Settings.apis.clsi.url}/project/${projectId}` +
-      (userId != null ? `/user/${userId}` : '') +
-      (outputFile.build != null ? `/build/${outputFile.build}` : '') +
-      `/output/${outputFile.path}`,
-    ...outputFile,
-  }
-}
-
 function compile(req, res, next) {
   const timer = new Metrics.Timer('compile-request')
   RequestParser.parse(req.body, function (error, request) {
@@ -41,7 +30,7 @@ function compile(req, res, next) {
           return next(error)
         }
         CompileManager.doCompileWithLock(request, (error, result) => {
-          let { buildId, outputFiles, stats, timings } = result || {}
+          let { outputFiles, stats, timings } = result || {}
           let code, status
           if (outputFiles == null) {
             outputFiles = []
@@ -109,7 +98,6 @@ function compile(req, res, next) {
 
           if (error) {
             outputFiles = error.outputFiles || []
-            buildId = error.buildId
           }
 
           timer.done()
@@ -120,28 +108,14 @@ function compile(req, res, next) {
               stats,
               timings,
               outputUrlPrefix: Settings.apis.clsi.outputUrlPrefix,
-              outputFiles:
-                outputFiles.length === 0
-                  ? []
-                  : outputFiles
-                      .map(file =>
-                        addUrlToOutputFile(
-                          file,
-                          request.project_id,
-                          request.user_id
-                        )
-                      )
-                      .concat(
-                        addUrlToOutputFile(
-                          {
-                            build: buildId,
-                            path: 'output.zip',
-                            type: 'zip',
-                          },
-                          request.project_id,
-                          request.user_id
-                        )
-                      ),
+              outputFiles: outputFiles.map(file => ({
+                url:
+                  `${Settings.apis.clsi.url}/project/${request.project_id}` +
+                  (request.user_id != null ? `/user/${request.user_id}` : '') +
+                  (file.build != null ? `/build/${file.build}` : '') +
+                  `/output/${file.path}`,
+                ...file,
+              })),
             },
           })
         })

services/clsi/app/js/CompileManager.js

@@ -208,7 +208,7 @@ async function doCompile(request) {
       Metrics.inc('compiles-timeout', 1, request.metricsOpts)
     }
 
-    const { outputFiles, allEntries, buildId } = await _saveOutputFiles({
+    const { outputFiles, allEntries } = await _saveOutputFiles({
       request,
       compileDir,
       resourceList,
@@ -216,7 +216,7 @@ async function doCompile(request) {
       timings,
     })
     error.outputFiles = outputFiles // return output files so user can check logs
-    error.buildId = buildId
+
     // Clear project if this compile was abruptly terminated
     if (error.terminated || error.timedout) {
       await clearProjectWithListing(
@@ -280,7 +280,7 @@ async function doCompile(request) {
   // Emit compile time.
   timings.compile = ts
 
-  const { outputFiles, buildId } = await _saveOutputFiles({
+  const { outputFiles } = await _saveOutputFiles({
     request,
     compileDir,
     resourceList,
@@ -296,7 +296,7 @@ async function doCompile(request) {
     emitPdfStats(stats, timings, request)
   }
 
-  return { outputFiles, stats, timings, buildId }
+  return { outputFiles, stats, timings }
 }
 
 async function _saveOutputFiles({
@@ -316,24 +316,20 @@ async function _saveOutputFiles({
   let { outputFiles, allEntries } =
     await OutputFileFinder.promises.findOutputFiles(resourceList, compileDir)
 
-  let buildId
-
   try {
-    const saveResult = await OutputCacheManager.promises.saveOutputFiles(
+    outputFiles = await OutputCacheManager.promises.saveOutputFiles(
       { request, stats, timings },
       outputFiles,
       compileDir,
       outputDir
     )
-    buildId = saveResult.buildId
-    outputFiles = saveResult.outputFiles
   } catch (err) {
     const { project_id: projectId, user_id: userId } = request
     logger.err({ projectId, userId, err }, 'failed to save output files')
   }
 
   timings.output = timer.done()
-  return { outputFiles, allEntries, buildId }
+  return { outputFiles, allEntries }
 }
 
 async function stopCompile(projectId, userId) {

services/clsi/app/js/OutputCacheManager.js

@@ -164,7 +164,7 @@ module.exports = OutputCacheManager = {
             outputDir,
             stats,
             (err, outputFiles) => {
-              if (err) return callback(err, { outputFiles, buildId })
+              if (err) return callback(err, outputFiles)
 
               const enablePdfCaching = request.enablePdfCaching
               const enablePdfCachingDark =
@@ -173,7 +173,7 @@ module.exports = OutputCacheManager = {
                 !Settings.enablePdfCaching ||
                 (!enablePdfCaching && !enablePdfCachingDark)
               ) {
-                return callback(null, { outputFiles, buildId })
+                return callback(null, outputFiles)
               }
 
               OutputCacheManager.saveStreamsInContentDir(
@@ -191,9 +191,9 @@ module.exports = OutputCacheManager = {
                       { err, outputDir, stats, timings },
                       'pdf caching failed'
                     )
-                    return callback(null, { outputFiles, buildId })
+                    return callback(null, outputFiles)
                   }
-                  callback(err, { outputFiles, buildId })
+                  callback(err, outputFiles)
                 }
               )
             }

services/clsi/test/unit/js/CompileControllerTests.js

@@ -50,7 +50,6 @@ function tryImageNameValidation(method, imageNameField) {
 
 describe('CompileController', function () {
   beforeEach(function () {
-    this.buildId = 'build-id-123'
     this.CompileController = SandboxedModule.require(modulePath, {
       requires: {
         './CompileManager': (this.CompileManager = {}),
@@ -119,7 +118,6 @@ describe('CompileController', function () {
           outputFiles: this.output_files,
           stats: this.stats,
           timings: this.timings,
-          buildId: this.buildId,
         })
         this.CompileController.compile(this.req, this.res)
       })
@@ -142,32 +140,21 @@ describe('CompileController', function () {
 
       it('should return the JSON response', function () {
         this.res.status.calledWith(200).should.equal(true)
-        console.log(this.res.send.args[0][0].compile)
-        sinon.assert.calledWith(
-          this.res.send,
-          sinon.match.has(
-            'compile',
-            sinon.match({
+        this.res.send
+          .calledWith({
+            compile: {
               status: 'success',
               error: null,
               stats: this.stats,
               timings: this.timings,
               outputUrlPrefix: '/zone/b',
-              outputFiles: [
-                ...this.output_files.map(file => ({
-                  url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`,
-                  ...file,
-                })),
-                {
-                  url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${this.buildId}/output/output.zip`,
-                  build: this.buildId,
-                  path: 'output.zip',
-                  type: 'zip',
-                },
-              ],
-            })
-          )
-        )
+              outputFiles: this.output_files.map(file => ({
+                url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`,
+                ...file,
+              })),
+            },
+          })
+          .should.equal(true)
       })
     })
 
@@ -178,7 +165,6 @@ describe('CompileController', function () {
           outputFiles: this.output_files,
           stats: this.stats,
           timings: this.timings,
-          buildId: this.buildId,
         })
         this.CompileController.compile(this.req, this.res)
       })
@@ -193,18 +179,10 @@ describe('CompileController', function () {
               stats: this.stats,
               timings: this.timings,
               outputUrlPrefix: '',
-              outputFiles: [
-                ...this.output_files.map(file => ({
-                  url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`,
-                  ...file,
-                })),
-                {
-                  url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${this.buildId}/output/output.zip`,
-                  build: this.buildId,
-                  path: 'output.zip',
-                  type: 'zip',
-                },
-              ],
+              outputFiles: this.output_files.map(file => ({
+                url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`,
+                ...file,
+              })),
             },
           })
           .should.equal(true)
@@ -229,7 +207,6 @@ describe('CompileController', function () {
           outputFiles: this.output_files,
           stats: this.stats,
           timings: this.timings,
-          buildId: this.buildId,
         })
         this.CompileController.compile(this.req, this.res)
       })
@@ -244,18 +221,10 @@ describe('CompileController', function () {
               stats: this.stats,
               timings: this.timings,
               outputUrlPrefix: '/zone/b',
-              outputFiles: [
-                ...this.output_files.map(file => ({
-                  url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`,
-                  ...file,
-                })),
-                {
-                  url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${this.buildId}/output/output.zip`,
-                  build: this.buildId,
-                  path: 'output.zip',
-                  type: 'zip',
-                },
-              ],
+              outputFiles: this.output_files.map(file => ({
+                url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`,
+                ...file,
+              })),
             },
           })
           .should.equal(true)
@@ -281,7 +250,6 @@ describe('CompileController', function () {
           outputFiles: this.output_files,
           stats: this.stats,
           timings: this.timings,
-          buildId: this.buildId,
         })
         this.CompileController.compile(this.req, this.res)
       })
@@ -296,18 +264,10 @@ describe('CompileController', function () {
               stats: this.stats,
               timings: this.timings,
               outputUrlPrefix: '/zone/b',
-              outputFiles: [
-                ...this.output_files.map(file => ({
-                  url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`,
-                  ...file,
-                })),
-                {
-                  url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${this.buildId}/output/output.zip`,
-                  build: this.buildId,
-                  path: 'output.zip',
-                  type: 'zip',
-                },
-              ],
+              outputFiles: this.output_files.map(file => ({
+                url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`,
+                ...file,
+              })),
             },
           })
           .should.equal(true)
@@ -316,11 +276,9 @@ describe('CompileController', function () {
 
     describe('with an error', function () {
       beforeEach(function () {
-        const error = new Error((this.message = 'error message'))
-        error.buildId = this.buildId
         this.CompileManager.doCompileWithLock = sinon
           .stub()
-          .callsArgWith(1, error, null)
+          .callsArgWith(1, new Error((this.message = 'error message')), null)
         this.CompileController.compile(this.req, this.res)
       })
 

services/clsi/test/unit/js/CompileManagerTests.js

@@ -35,7 +35,6 @@ describe('CompileManager', function () {
         build: 1234,
       },
     ]
-    this.buildId = 'build-id-123'
     this.commandOutput = 'Dummy output'
     this.compileBaseDir = '/compile/dir'
     this.outputBaseDir = '/output/dir'
@@ -62,9 +61,7 @@ describe('CompileManager', function () {
     }
     this.OutputCacheManager = {
       promises: {
-        saveOutputFiles: sinon
-          .stub()
-          .resolves({ outputFiles: this.buildFiles, buildId: this.buildId }),
+        saveOutputFiles: sinon.stub().resolves(this.buildFiles),
       },
     }
     this.Settings = {

services/web/app/src/Features/Compile/CompileController.js

@@ -398,13 +398,6 @@ module.exports = CompileController = {
       if (error) {
         return next(error)
       }
-
-      const qs = {}
-
-      if (req.params.file === 'output.zip') {
-        qs.files = req.query.files
-      }
-
       const url = CompileController._getFileUrl(
         projectId,
         userId,
@@ -415,7 +408,7 @@ module.exports = CompileController = {
         projectId,
         'output-file',
         url,
-        qs,
+        {},
         req,
         res,
         next
@@ -580,20 +573,10 @@ module.exports = CompileController = {
           return next(err)
         }
         url = new URL(`${Settings.apis.clsi.url}${url}`)
-
-        const params = new URLSearchParams(persistenceOptions.qs)
-
-        for (const [key, value] of Object.entries(qs)) {
-          if (Array.isArray(value)) {
-            for (const v of value) {
-              params.append(key, v)
-            }
-            continue
-          }
-          params.append(key, value)
-        }
-
-        url.search = params.toString()
+        url.search = new URLSearchParams({
+          ...persistenceOptions.qs,
+          ...qs,
+        }).toString()
         const timer = new Metrics.Timer(
           'proxy_to_clsi',
           1,
@@ -621,10 +604,7 @@ module.exports = CompileController = {
             })
 
             for (const key of ['Content-Length', 'Content-Type']) {
-              const headerValue = response.headers.get(key)
-              if (headerValue) {
-                res.setHeader(key, headerValue)
-              }
+              res.setHeader(key, response.headers.get(key))
             }
             res.writeHead(response.status)
             return pipeline(stream, res)

services/web/frontend/extracted-translations.json

@@ -304,7 +304,6 @@
   "doing_this_will_verify_affiliation_and_allow_log_in_2": "",
   "done": "",
   "download": "",
-  "download_all": "",
   "download_metadata": "",
   "download_pdf": "",
   "download_zip_file": "",

services/web/frontend/js/features/pdf-preview/components/pdf-file-list.jsx

@@ -33,18 +33,6 @@ function PdfFileList({ fileList }) {
           <b>{file.path}</b>
         </MenuItem>
       ))}
-
-      {fileList.other.length + fileList.top.length > 0 && fileList.archive && (
-        <MenuItem
-          download={basename(fileList.archive)}
-          href={fileList.archive.url}
-          key={fileList.archive.path}
-        >
-          <b>
-            {t('download_all')} ({fileList.other.length + fileList.top.length})
-          </b>
-        </MenuItem>
-      )}
     </>
   )
 }
@@ -60,10 +48,6 @@ PdfFileList.propTypes = {
   fileList: PropTypes.shape({
     top: FilesArray,
     other: FilesArray,
-    archive: PropTypes.shape({
-      path: PropTypes.string.isRequired,
-      url: PropTypes.string.isRequired,
-    }),
   }),
 }
 

services/web/frontend/js/features/pdf-preview/util/file-list.js

@@ -1,5 +1,5 @@
 const topFileTypes = ['bbl', 'gls', 'ind']
-const ignoreFiles = ['output.fls', 'output.fdb_latexmk', 'output.zip']
+const ignoreFiles = ['output.fls', 'output.fdb_latexmk']
 
 export const buildFileList = (outputFiles, clsiServerId, compileGroup) => {
   const files = { top: [], other: [] }
@@ -18,8 +18,6 @@ export const buildFileList = (outputFiles, clsiServerId, compileGroup) => {
 
     const allFiles = []
 
-    let outputArchiveFile
-
     // filter out ignored files and set some properties
     for (const file of outputFiles.values()) {
       if (!ignoreFiles.includes(file.path)) {
@@ -30,8 +28,6 @@ export const buildFileList = (outputFiles, clsiServerId, compileGroup) => {
         }
 
         allFiles.push(file)
-      } else if (file.type === 'zip') {
-        outputArchiveFile = file
       }
     }
 
@@ -56,14 +52,6 @@ export const buildFileList = (outputFiles, clsiServerId, compileGroup) => {
         files.other.push(file)
       }
     }
-
-    if (outputArchiveFile) {
-      allFiles.forEach(
-        file => file.type !== 'pdf' && params.append('files', file.path)
-      )
-      outputArchiveFile.url += `?${params.toString()}`
-      files.archive = outputArchiveFile
-    }
   }
 
   return files

services/web/locales/en.json

@@ -430,7 +430,6 @@
   "done": "Done",
   "dont_have_account": "Don’t have an account?",
   "download": "Download",
-  "download_all": "Download all",
   "download_metadata": "Download Overleaf metadata",
   "download_pdf": "Download PDF",
   "download_zip_file": "Download .zip file",