Merge pull request #3014 from overleaf/ns-delete-project-history-cleanup

Decaf cleanup HistoryController

GitOrigin-RevId: e5df4cde30d8b9e65062e1484699326e96c4eb92

services/web/app/src/Features/History/HistoryController.js

@@ -1,22 +1,5 @@
-/* eslint-disable
-    camelcase,
-    handle-callback-err,
-    max-len,
-    no-undef,
-    no-unused-vars,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * DS103: Rewrite code to no longer use __guard__
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
 let HistoryController
 const OError = require('@overleaf/o-error')
-const _ = require('lodash')
 const async = require('async')
 const logger = require('logger-sharelatex')
 const request = require('request')
@@ -32,45 +15,32 @@ const { pipeline } = require('stream')
 
 module.exports = HistoryController = {
   selectHistoryApi(req, res, next) {
-    if (next == null) {
-      next = function(error) {}
-    }
-    const project_id = req.params != null ? req.params.Project_id : undefined
+    const { Project_id: projectId } = req.params
     // find out which type of history service this project uses
-    return ProjectDetailsHandler.getDetails(project_id, function(err, project) {
-      if (err != null) {
+    ProjectDetailsHandler.getDetails(projectId, function(err, project) {
+      if (err) {
         return next(err)
       }
-      const history =
-        project.overleaf != null ? project.overleaf.history : undefined
-      if (
-        (history != null ? history.id : undefined) != null &&
-        (history != null ? history.display : undefined)
-      ) {
+      const history = project.overleaf && project.overleaf.history
+      if (history && history.id && history.display) {
         req.useProjectHistory = true
       } else {
         req.useProjectHistory = false
       }
-      return next()
+      next()
     })
   },
 
   ensureProjectHistoryEnabled(req, res, next) {
-    if (next == null) {
-      next = function(error) {}
-    }
-    if (req.useProjectHistory != null) {
-      return next()
+    if (req.useProjectHistory) {
+      next()
     } else {
-      return res.sendStatus(404)
+      res.sendStatus(404)
     }
   },
 
   proxyToHistoryApi(req, res, next) {
-    if (next == null) {
-      next = function(error) {}
-    }
-    const user_id = AuthenticationController.getLoggedInUserId(req)
+    const userId = AuthenticationController.getLoggedInUserId(req)
     const url =
       HistoryController.buildHistoryServiceUrl(req.useProjectHistory) + req.url
 
@@ -78,41 +48,38 @@ module.exports = HistoryController = {
       url,
       method: req.method,
       headers: {
-        'X-User-Id': user_id
+        'X-User-Id': userId
       }
     })
     getReq.pipe(res)
-    return getReq.on('error', function(error) {
-      logger.warn({ url, err: error }, 'history API error')
-      return next(error)
+    getReq.on('error', function(err) {
+      logger.warn({ url, err }, 'history API error')
+      next(err)
     })
   },
 
   proxyToHistoryApiAndInjectUserDetails(req, res, next) {
-    if (next == null) {
-      next = function(error) {}
-    }
-    const user_id = AuthenticationController.getLoggedInUserId(req)
+    const userId = AuthenticationController.getLoggedInUserId(req)
     const url =
       HistoryController.buildHistoryServiceUrl(req.useProjectHistory) + req.url
-    return HistoryController._makeRequest(
+    HistoryController._makeRequest(
       {
         url,
         method: req.method,
         json: true,
         headers: {
-          'X-User-Id': user_id
+          'X-User-Id': userId
         }
       },
-      function(error, body) {
-        if (error != null) {
-          return next(error)
+      function(err, body) {
+        if (err) {
+          return next(err)
         }
-        return HistoryManager.injectUserDetails(body, function(error, data) {
-          if (error != null) {
-            return next(error)
+        HistoryManager.injectUserDetails(body, function(err, data) {
+          if (err) {
+            return next(err)
           }
-          return res.json(data)
+          res.json(data)
         })
       }
     )
@@ -129,37 +96,32 @@ module.exports = HistoryController = {
   },
 
   resyncProjectHistory(req, res, next) {
-    if (next == null) {
-      next = function(error) {}
-    }
-    const project_id = req.params.Project_id
-    return ProjectEntityUpdateHandler.resyncProjectHistory(project_id, function(
-      error
-    ) {
-      if (error instanceof Errors.ProjectHistoryDisabledError) {
+    const projectId = req.params.Project_id
+    ProjectEntityUpdateHandler.resyncProjectHistory(projectId, function(err) {
+      if (err instanceof Errors.ProjectHistoryDisabledError) {
         return res.sendStatus(404)
       }
-      if (error != null) {
-        return next(error)
+      if (err) {
+        return next(err)
       }
-      return res.sendStatus(204)
+      res.sendStatus(204)
     })
   },
 
   restoreFileFromV2(req, res, next) {
-    const { project_id } = req.params
+    const { project_id: projectId } = req.params
     const { version, pathname } = req.body
-    const user_id = AuthenticationController.getLoggedInUserId(req)
-    return RestoreManager.restoreFileFromV2(
-      user_id,
-      project_id,
+    const userId = AuthenticationController.getLoggedInUserId(req)
+    RestoreManager.restoreFileFromV2(
+      userId,
+      projectId,
       version,
       pathname,
-      function(error, entity) {
-        if (error != null) {
-          return next(error)
+      function(err, entity) {
+        if (err) {
+          return next(err)
         }
-        return res.json({
+        res.json({
           type: entity.type,
           id: entity._id
         })
@@ -168,19 +130,19 @@ module.exports = HistoryController = {
   },
 
   restoreDocFromDeletedDoc(req, res, next) {
-    const { project_id, doc_id } = req.params
+    const { project_id: projectId, doc_id: docId } = req.params
     const { name } = req.body
-    const user_id = AuthenticationController.getLoggedInUserId(req)
+    const userId = AuthenticationController.getLoggedInUserId(req)
     if (name == null) {
       return res.sendStatus(400) // Malformed request
     }
-    return RestoreManager.restoreDocFromDeletedDoc(
-      user_id,
-      project_id,
-      doc_id,
+    RestoreManager.restoreDocFromDeletedDoc(
+      userId,
+      projectId,
+      docId,
       name,
-      (error, doc) => {
-        if (error != null) return next(error)
+      (err, doc) => {
+        if (err) return next(err)
         res.json({
           doc_id: doc._id
         })
@@ -189,51 +151,48 @@ module.exports = HistoryController = {
   },
 
   getLabels(req, res, next) {
-    const project_id = req.params.Project_id
-    const user_id = AuthenticationController.getLoggedInUserId(req)
-    return HistoryController._makeRequest(
+    const projectId = req.params.Project_id
+    HistoryController._makeRequest(
       {
         method: 'GET',
-        url: `${
-          settings.apis.project_history.url
-        }/project/${project_id}/labels`,
+        url: `${settings.apis.project_history.url}/project/${projectId}/labels`,
         json: true
       },
-      function(error, labels) {
-        if (error != null) {
-          return next(error)
+      function(err, labels) {
+        if (err) {
+          return next(err)
         }
         HistoryController._enrichLabels(labels, (err, labels) => {
           if (err) {
             return next(err)
           }
-          return res.json(labels)
+          res.json(labels)
         })
       }
     )
   },
 
   createLabel(req, res, next) {
-    const project_id = req.params.Project_id
+    const projectId = req.params.Project_id
     const { comment, version } = req.body
-    const user_id = AuthenticationController.getLoggedInUserId(req)
-    return HistoryController._makeRequest(
+    const userId = AuthenticationController.getLoggedInUserId(req)
+    HistoryController._makeRequest(
       {
         method: 'POST',
         url: `${
           settings.apis.project_history.url
-        }/project/${project_id}/user/${user_id}/labels`,
+        }/project/${projectId}/user/${userId}/labels`,
         json: { comment, version }
       },
-      function(error, label) {
-        if (error != null) {
-          return next(error)
+      function(err, label) {
+        if (err) {
+          return next(err)
         }
         HistoryController._enrichLabel(label, (err, label) => {
           if (err) {
             return next(err)
           }
-          return res.json(label)
+          res.json(label)
         })
       }
     )
@@ -293,7 +252,7 @@ module.exports = HistoryController = {
     if (user == null) {
       return 'Anonymous'
     }
-    if (user.name != null) {
+    if (user.name) {
       return user.name
     }
     let name = [user.first_name, user.last_name]
@@ -303,67 +262,66 @@ module.exports = HistoryController = {
     if (name === '') {
       name = user.email.split('@')[0]
     }
-    if (name == null || name === '') {
+    if (!name) {
       return '?'
     }
     return name
   },
 
   deleteLabel(req, res, next) {
-    const project_id = req.params.Project_id
-    const { label_id } = req.params
-    const user_id = AuthenticationController.getLoggedInUserId(req)
-    return HistoryController._makeRequest(
+    const { Project_id: projectId, label_id: labelId } = req.params
+    const userId = AuthenticationController.getLoggedInUserId(req)
+    HistoryController._makeRequest(
       {
         method: 'DELETE',
         url: `${
           settings.apis.project_history.url
-        }/project/${project_id}/user/${user_id}/labels/${label_id}`
+        }/project/${projectId}/user/${userId}/labels/${labelId}`
       },
-      function(error) {
-        if (error != null) {
-          return next(error)
+      function(err) {
+        if (err) {
+          return next(err)
         }
-        return res.sendStatus(204)
+        res.sendStatus(204)
       }
     )
   },
 
   _makeRequest(options, callback) {
-    return request(options, function(error, response, body) {
-      if (error != null) {
-        return callback(error)
+    return request(options, function(err, response, body) {
+      if (err) {
+        return callback(err)
       }
       if (response.statusCode >= 200 && response.statusCode < 300) {
-        return callback(null, body)
+        callback(null, body)
       } else {
-        error = new Error(
+        err = new Error(
           `history api responded with non-success code: ${response.statusCode}`
         )
-        return callback(error)
+        callback(err)
       }
     })
   },
 
   downloadZipOfVersion(req, res, next) {
-    const { project_id, version } = req.params
-    return ProjectDetailsHandler.getDetails(project_id, function(err, project) {
-      if (err != null) {
+    const { project_id: projectId, version } = req.params
+    ProjectDetailsHandler.getDetails(projectId, function(err, project) {
+      if (err) {
         return next(err)
       }
-      const v1_id = __guard__(
-        project.overleaf != null ? project.overleaf.history : undefined,
-        x => x.id
-      )
-      if (v1_id == null) {
-        logger.err(
-          { project_id, version },
+      const v1Id =
+        project.overleaf &&
+        project.overleaf.history &&
+        project.overleaf.history.id
+      if (v1Id == null) {
+        logger.error(
+          { projectId, version },
           'got request for zip version of non-v1 history project'
         )
         return res.sendStatus(402)
       }
-      return HistoryController._pipeHistoryZipToResponse(
-        v1_id,
+      HistoryController._pipeHistoryZipToResponse(
+        v1Id,
         version,
         `${project.name} (Version ${version})`,
         req,
@@ -373,7 +331,7 @@ module.exports = HistoryController = {
     })
   },
 
-  _pipeHistoryZipToResponse(v1_project_id, version, name, req, res, next) {
+  _pipeHistoryZipToResponse(v1ProjectId, version, name, req, res, next) {
     if (req.aborted) {
       // client has disconnected -- skip project history api call and download
       return
@@ -382,7 +340,7 @@ module.exports = HistoryController = {
     res.setTimeout(6 * 60 * 1000)
     const url = `${
       settings.apis.v1_history.url
-    }/projects/${v1_project_id}/version/${version}/zip`
+    }/projects/${v1ProjectId}/version/${version}/zip`
     const options = {
       auth: {
         user: settings.apis.v1_history.user,
@@ -392,10 +350,10 @@ module.exports = HistoryController = {
       method: 'post',
       url
     }
-    return request(options, function(err, response, body) {
+    request(options, function(err, response, body) {
       if (err) {
         OError.tag(err, 'history API error', {
-          v1_project_id,
+          v1ProjectId,
           version
         })
         return next(err)
@@ -407,7 +365,7 @@ module.exports = HistoryController = {
       let retryAttempt = 0
       let retryDelay = 2000
       // retry for about 6 minutes starting with short delay
-      return async.retry(
+      async.retry(
         40,
         callback =>
           setTimeout(function() {
@@ -448,39 +406,33 @@ module.exports = HistoryController = {
               pipeline(response, res, err => {
                 if (err) {
                   logger.warn(
-                    { err, v1_project_id, version, retryAttempt },
+                    { err, v1ProjectId, version, retryAttempt },
                     'history s3 proxying error'
                   )
                 }
               })
               callback()
             })
-            return getReq.on('error', function(err) {
+            getReq.on('error', function(err) {
               logger.warn(
-                { err, v1_project_id, version, retryAttempt },
+                { err, v1ProjectId, version, retryAttempt },
                 'history s3 download error'
               )
               cleanupAbortTrigger()
-              return callback(err)
+              callback(err)
             })
           }, retryDelay),
         function(err) {
           if (err) {
             OError.tag(err, 'history s3 download failed', {
-              v1_project_id,
+              v1ProjectId,
               version,
               retryAttempt
             })
-            return next(err)
+            next(err)
           }
         }
       )
     })
   }
 }
-
-function __guard__(value, transform) {
-  return typeof value !== 'undefined' && value !== null
-    ? transform(value)
-    : undefined
-}

services/web/test/unit/src/History/HistoryControllerTests.js

@@ -59,7 +59,7 @@ describe('HistoryController', function() {
 
   describe('selectHistoryApi', function() {
     beforeEach(function() {
-      this.req = { url: '/mock/url', method: 'POST' }
+      this.req = { url: '/mock/url', method: 'POST', params: {} }
       this.res = 'mock-res'
       return (this.next = sinon.stub())
     })