Parcourir la source

Merge pull request #18538 from overleaf/ar-add-download-all-link-in-web

[web] add download all link for output files

GitOrigin-RevId: 3d574d75d53e577cb0f8fd3caa4f757d9e1b7889
Andrew Rumble il y a 2 ans
Parent
commit
d1a58e6b77

+ 2 - 0
services/web/app/src/Features/Compile/ClsiManager.js

@@ -193,6 +193,7 @@ async function _sendBuiltRequest(projectId, userId, req, options, callback) {
     status: compile.status,
     outputFiles,
     clsiServerId,
+    buildId: compile.buildId,
     stats: compile.stats,
     timings: compile.timings,
     outputUrlPrefix: compile.outputUrlPrefix,
@@ -817,6 +818,7 @@ module.exports = {
     'stats',
     'timings',
     'outputUrlPrefix',
+    'buildId',
   ]),
   sendExternalRequest: callbackifyMultiResult(sendExternalRequest, [
     'status',

+ 52 - 7
services/web/app/src/Features/Compile/CompileController.js

@@ -30,6 +30,15 @@ const pdfDownloadRateLimiter = new RateLimiter('full-pdf-download', {
   duration: 60 * 60,
 })
 
+function getOutputFilesArchiveSpecification(projectId, userId, buildId) {
+  const fileName = 'output.zip'
+  return {
+    path: fileName,
+    url: CompileController._getFileUrl(projectId, userId, buildId, fileName),
+    type: 'zip',
+  }
+}
+
 function getImageNameForProject(projectId, callback) {
   ProjectGetter.getProject(projectId, { imageName: 1 }, (err, project) => {
     if (err) return callback(err)
@@ -149,7 +158,8 @@ module.exports = CompileController = {
           validationProblems,
           stats,
           timings,
-          outputUrlPrefix
+          outputUrlPrefix,
+          buildId
         ) => {
           if (error) {
             Metrics.inc('compile-error')
@@ -187,14 +197,21 @@ module.exports = CompileController = {
               }
             )
           }
+
+          const outputFilesArchive = buildId
+            ? getOutputFilesArchiveSpecification(projectId, userId, buildId)
+            : null
+
           res.json({
             status,
             outputFiles,
+            outputFilesArchive,
             compileGroup: limits?.compileGroup,
             clsiServerId,
             validationProblems,
             stats,
             timings,
+            outputUrlPrefix,
             pdfDownloadDomain,
             pdfCachingMinChunkSize,
           })
@@ -398,6 +415,22 @@ module.exports = CompileController = {
       if (error) {
         return next(error)
       }
+
+      const qs = {}
+
+      if (req.params.file === 'output.zip' && req.query?.files) {
+        /**
+         * The output.zip file is generated on the fly and allows either:
+         *
+         * 1. All files to be downloaded (no files query parameter)
+         * 2. A specific set of files to be downloaded (files query parameter)
+         *
+         * As the frontend separates the PDF download and ignores several other output
+         * files we will generally need to tell CLSI specifically what is required.
+         */
+        qs.files = req.query.files
+      }
+
       const url = CompileController._getFileUrl(
         projectId,
         userId,
@@ -408,7 +441,7 @@ module.exports = CompileController = {
         projectId,
         'output-file',
         url,
-        {},
+        qs,
         req,
         res,
         next
@@ -573,10 +606,20 @@ module.exports = CompileController = {
           return next(err)
         }
         url = new URL(`${Settings.apis.clsi.url}${url}`)
-        url.search = new URLSearchParams({
-          ...persistenceOptions.qs,
-          ...qs,
-        }).toString()
+
+        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()
         const timer = new Metrics.Timer(
           'proxy_to_clsi',
           1,
@@ -604,7 +647,9 @@ module.exports = CompileController = {
             })
 
             for (const key of ['Content-Length', 'Content-Type']) {
-              res.setHeader(key, response.headers.get(key))
+              if (response.headers.has(key)) {
+                res.setHeader(key, response.headers.get(key))
+              }
             }
             res.writeHead(response.status)
             return pipeline(stream, res)

+ 3 - 0
services/web/app/src/Features/Compile/CompileManager.js

@@ -77,6 +77,7 @@ async function compile(projectId, userId, options = {}) {
     stats,
     timings,
     outputUrlPrefix,
+    buildId,
   } = await ClsiManager.promises.sendRequest(projectId, compileAsUser, options)
 
   return {
@@ -88,6 +89,7 @@ async function compile(projectId, userId, options = {}) {
     stats,
     timings,
     outputUrlPrefix,
+    buildId,
   }
 }
 
@@ -173,6 +175,7 @@ module.exports = CompileManager = {
     'stats',
     'timings',
     'outputUrlPrefix',
+    'buildId',
   ]),
 
   stopCompile: callbackify(stopCompile),

+ 1 - 0
services/web/frontend/extracted-translations.json

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

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

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

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

@@ -1,7 +1,12 @@
 const topFileTypes = ['bbl', 'gls', 'ind']
 const ignoreFiles = ['output.fls', 'output.fdb_latexmk']
 
-export const buildFileList = (outputFiles, clsiServerId, compileGroup) => {
+export const buildFileList = (
+  outputFiles,
+  clsiServerId,
+  compileGroup,
+  outputFilesArchive
+) => {
   const files = { top: [], other: [] }
 
   if (outputFiles) {
@@ -52,6 +57,18 @@ export const buildFileList = (outputFiles, clsiServerId, compileGroup) => {
         files.other.push(file)
       }
     }
+
+    const archivableFiles = [...files.top, ...files.other]
+
+    if (outputFilesArchive && archivableFiles.length > 0) {
+      archivableFiles.forEach(file => params.append('files', file.path))
+
+      files.archive = {
+        ...outputFilesArchive,
+        fileCount: archivableFiles.length,
+        url: `${outputFilesArchive.url}?${params.toString()}`,
+      }
+    }
   }
 
   return files

+ 7 - 1
services/web/frontend/js/shared/context/local-compile-context.tsx

@@ -54,6 +54,7 @@ export type CompileContext = {
   isProjectOwner: boolean
   logEntries?: Record<string, any>
   logEntryAnnotations?: Record<string, any>
+  outputFilesArchive?: string
   pdfDownloadUrl?: string
   pdfFile?: PdfFile
   pdfUrl?: string
@@ -368,7 +369,12 @@ export const LocalCompileProvider: FC = ({ children }) => {
         }
 
         setFileList(
-          buildFileList(outputFiles, data.clsiServerId, data.compileGroup)
+          buildFileList(
+            outputFiles,
+            data.clsiServerId,
+            data.compileGroup,
+            data.outputFilesArchive
+          )
         )
 
         // handle log files

+ 1 - 0
services/web/locales/en.json

@@ -435,6 +435,7 @@
   "dont_have_account": "Don’t have an account?",
   "dont_have_account_without_question_mark": "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",

+ 9 - 2
services/web/test/unit/src/Compile/ClsiManagerTests.js

@@ -156,19 +156,21 @@ describe('ClsiManager', function () {
 
   describe('sendRequest', function () {
     describe('with a successful compile', function () {
+      const buildId = '18fbe9e7564-30dcb2f71250c690'
+
       beforeEach(async function () {
         this.outputFiles = [
           {
             url: `/project/${this.project_id}/user/${this.user_id}/build/1234/output/output.pdf`,
             path: 'output.pdf',
             type: 'pdf',
-            build: 1234,
+            build: buildId,
           },
           {
             url: `/project/${this.project_id}/user/${this.user_id}/build/1234/output/output.log`,
             path: 'output.log',
             type: 'log',
-            build: 1234,
+            build: buildId,
           },
         ]
         this.responseBody.compile.outputFiles = this.outputFiles.map(
@@ -177,6 +179,7 @@ describe('ClsiManager', function () {
             url: `http://${CLSI_HOST}${outputFile.url}`,
           })
         )
+        this.responseBody.compile.buildId = buildId
         this.timeout = 100
         this.result = await this.ClsiManager.promises.sendRequest(
           this.project._id,
@@ -264,6 +267,10 @@ describe('ClsiManager', function () {
         )
       })
 
+      it('should return the buildId', function () {
+        expect(this.result.buildId).to.equal(buildId)
+      })
+
       it('should persist the cookie from the response', function () {
         expect(
           this.ClsiCookieManager.promises.setServerId

+ 31 - 6
services/web/test/unit/src/Compile/CompileControllerTests.js

@@ -110,9 +110,11 @@ describe('CompileController', function () {
       },
     })
     this.projectId = 'project-id'
+    this.build_id = '18fbe9e7564-30dcb2f71250c690'
     this.next = sinon.stub()
     this.req = new MockRequest()
     this.res = new MockResponse()
+    this.res = new MockResponse()
   })
 
   describe('compile', function () {
@@ -129,7 +131,14 @@ describe('CompileController', function () {
             url: `/project/${this.projectId}/user/${this.user_id}/build/id/output.pdf`,
             type: 'pdf',
           },
-        ])
+        ]),
+        undefined,
+        undefined,
+        undefined,
+        undefined,
+        undefined,
+        undefined,
+        this.build_id
       )
     })
 
@@ -156,6 +165,11 @@ describe('CompileController', function () {
                   type: 'pdf',
                 },
               ],
+              outputFilesArchive: {
+                path: 'output.zip',
+                url: `/project/${this.projectId}/user/wat/build/${this.build_id}/output/output.zip`,
+                type: 'zip',
+              },
               pdfDownloadDomain: 'https://compiles.overleaf.test',
             })
           )
@@ -181,7 +195,8 @@ describe('CompileController', function () {
             undefined, // validationProblems
             undefined, // stats
             undefined, // timings
-            '/zone/b'
+            '/zone/b',
+            this.build_id
           )
           this.CompileController.compile(this.req, this.res, this.next)
         })
@@ -198,6 +213,12 @@ describe('CompileController', function () {
                   type: 'pdf',
                 },
               ],
+              outputFilesArchive: {
+                path: 'output.zip',
+                url: `/project/${this.projectId}/user/wat/build/${this.build_id}/output/output.zip`,
+                type: 'zip',
+              },
+              outputUrlPrefix: '/zone/b',
               pdfDownloadDomain: 'https://compiles.overleaf.test/zone/b',
             })
           )
@@ -240,6 +261,11 @@ describe('CompileController', function () {
           JSON.stringify({
             status: this.status,
             outputFiles: this.outputFiles,
+            outputFilesArchive: {
+              path: 'output.zip',
+              url: `/project/${this.projectId}/user/wat/build/${this.build_id}/output/output.zip`,
+              type: 'zip',
+            },
           })
         )
       })
@@ -429,9 +455,9 @@ describe('CompileController', function () {
       })
     })
 
-    describe('when the a build-id is provided', function () {
+    describe('when a build-id is provided', function () {
       beforeEach(function (done) {
-        this.req.params.build_id = this.buildId = '1234-5678'
+        this.req.params.build_id = this.build_id
         this.CompileController.proxyToClsi = sinon
           .stub()
           .callsFake(() => done())
@@ -443,7 +469,7 @@ describe('CompileController', function () {
           .calledWith(
             this.projectId,
             'output-file',
-            `/project/${this.projectId}/user/${this.user_id}/build/${this.buildId}/output/output.pdf`,
+            `/project/${this.projectId}/user/${this.user_id}/build/${this.build_id}/output/output.pdf`,
             {},
             this.req,
             this.res,
@@ -457,7 +483,6 @@ describe('CompileController', function () {
   describe('getFileFromClsiWithoutUser', function () {
     beforeEach(function () {
       this.submission_id = 'sub-1234'
-      this.build_id = 123456
       this.file = 'project.pdf'
       this.req.params = {
         submission_id: this.submission_id,