Merge pull request #17426 from overleaf/msm-expressify-controllers

[web] Expressify controller methods

GitOrigin-RevId: 9784176b53a89beed09f9b38915872a6e7fae465

services/web/app/src/Features/Analytics/AnalyticsController.js

@@ -3,6 +3,7 @@ const AnalyticsManager = require('./AnalyticsManager')
 const SessionManager = require('../Authentication/SessionManager')
 const GeoIpLookup = require('../../infrastructure/GeoIpLookup')
 const Features = require('../../infrastructure/Features')
+const { expressify } = require('@overleaf/promise-utils')
 
 async function updateEditingSession(req, res, next) {
   if (!Features.hasFeature('analytics')) {
@@ -46,6 +47,6 @@ function recordEvent(req, res, next) {
 }
 
 module.exports = {
-  updateEditingSession,
+  updateEditingSession: expressify(updateEditingSession),
   recordEvent,
 }

services/web/app/src/Features/Analytics/AnalyticsRouter.js

@@ -3,7 +3,6 @@ const AnalyticsController = require('./AnalyticsController')
 const AnalyticsProxy = require('./AnalyticsProxy')
 const { RateLimiter } = require('../../infrastructure/RateLimiter')
 const RateLimiterMiddleware = require('../Security/RateLimiterMiddleware')
-const { expressify } = require('@overleaf/promise-utils')
 
 const rateLimiters = {
   recordEvent: new RateLimiter('analytics-record-event', {
@@ -33,7 +32,7 @@ module.exports = {
       RateLimiterMiddleware.rateLimit(rateLimiters.updateEditingSession, {
         params: ['projectId'],
       }),
-      expressify(AnalyticsController.updateEditingSession)
+      AnalyticsController.updateEditingSession
     )
 
     publicApiRouter.use(

services/web/app/src/Features/Authorization/PermissionsController.js

@@ -5,13 +5,14 @@ const {
 } = require('./PermissionsManager')
 const { checkUserPermissions } = require('./PermissionsManager').promises
 const Modules = require('../../infrastructure/Modules')
+const { expressify } = require('@overleaf/promise-utils')
 
 /**
  * Function that returns middleware to add an `assertPermission` function to the request object to check if the user has a specific capability.
  * @returns {Function} The middleware function that adds the `assertPermission` function to the request object.
  */
 function useCapabilities() {
-  return async function (req, res, next) {
+  const middleware = async function (req, res, next) {
     // attach the user's capabilities to the request object
     req.capabilitySet = new Set()
     // provide a function to assert that a capability is present
@@ -57,6 +58,7 @@ function useCapabilities() {
       }
     }
   }
+  return expressify(middleware)
 }
 
 /**

services/web/app/src/Features/StaticPages/HomeController.js

@@ -1,18 +1,3 @@
-/* eslint-disable
-    n/handle-callback-err,
-    max-len,
-    no-unused-vars,
-    n/no-deprecated-api,
-*/
-// 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
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-let HomeController
 const Features = require('../../infrastructure/Features')
 const AnalyticsManager = require('../Analytics/AnalyticsManager')
 
@@ -23,6 +8,7 @@ const ErrorController = require('../Errors/ErrorController')
 const SessionManager = require('../Authentication/SessionManager')
 
 const SplitTestHandler = require('../SplitTests/SplitTestHandler')
+const { expressify } = require('@overleaf/promise-utils')
 const logger = require('@overleaf/logger')
 
 const homepageExists = fs.existsSync(
@@ -32,55 +18,57 @@ const homepageExists = fs.existsSync(
   )
 )
 
-module.exports = HomeController = {
-  index(req, res) {
-    if (SessionManager.isUserLoggedIn(req.session)) {
-      if (req.query.scribtex_path != null) {
-        return res.redirect(`/project?scribtex_path=${req.query.scribtex_path}`)
-      } else {
-        return res.redirect('/project')
-      }
+async function index(req, res) {
+  if (SessionManager.isUserLoggedIn(req.session)) {
+    if (req.query.scribtex_path != null) {
+      res.redirect(`/project?scribtex_path=${req.query.scribtex_path}`)
     } else {
-      return HomeController.home(req, res)
+      res.redirect('/project')
     }
-  },
+  } else {
+    await home(req, res)
+  }
+}
 
-  async home(req, res) {
-    if (Features.hasFeature('homepage') && homepageExists) {
-      const onboardingFlowAssignment =
-        await SplitTestHandler.promises.getAssignment(
-          req,
-          res,
-          'onboarding-flow'
-        )
-      AnalyticsManager.recordEventForSession(req.session, 'home-page-view', {
-        page: req.path,
-      })
+async function home(req, res) {
+  if (Features.hasFeature('homepage') && homepageExists) {
+    const onboardingFlowAssignment =
+      await SplitTestHandler.promises.getAssignment(req, res, 'onboarding-flow')
+    AnalyticsManager.recordEventForSession(req.session, 'home-page-view', {
+      page: req.path,
+    })
 
-      return res.render('external/home/website-redesign/index', {
-        onboardingFlowVariant: onboardingFlowAssignment.variant,
-        hideNewsletterCheckbox:
-          onboardingFlowAssignment.variant === 'token-confirmation-odc',
-      })
-    } else {
-      return res.redirect('/login')
-    }
-  },
+    res.render('external/home/website-redesign/index', {
+      onboardingFlowVariant: onboardingFlowAssignment.variant,
+      hideNewsletterCheckbox:
+        onboardingFlowAssignment.variant === 'token-confirmation-odc',
+    })
+  } else {
+    res.redirect('/login')
+  }
+}
 
-  externalPage(page, title) {
-    return function (req, res, next) {
-      if (next == null) {
-        next = function () {}
+function externalPage(page, title) {
+  const middleware = async function (req, res, next) {
+    const path = Path.join(__dirname, `/../../../views/external/${page}.pug`)
+    try {
+      const stats = await fs.promises.stat(path)
+      if (!stats.isFile()) {
+        logger.error({ stats, path }, 'Error serving external page')
+        ErrorController.notFound(req, res, next)
+      } else {
+        res.render(`external/${page}.pug`, { title })
       }
-      const path = Path.join(__dirname, `/../../../views/external/${page}.pug`)
-      return fs.exists(path, function (exists) {
-        // No error in this callback - old method in Node.js!
-        if (exists) {
-          return res.render(`external/${page}.pug`, { title })
-        } else {
-          return ErrorController.notFound(req, res, next)
-        }
-      })
+    } catch (error) {
+      logger.error({ path }, 'Error serving external page: file not found')
+      ErrorController.notFound(req, res, next)
     }
-  },
+  }
+  return expressify(middleware)
+}
+
+module.exports = {
+  index: expressify(index),
+  home: expressify(home),
+  externalPage,
 }

services/web/test/unit/src/Analytics/AnalyticsControllerTests.js

@@ -5,6 +5,7 @@ const modulePath = path.join(
   '../../../../app/src/Features/Analytics/AnalyticsController'
 )
 const sinon = require('sinon')
+const MockResponse = require('../helpers/MockResponse')
 
 describe('AnalyticsController', function () {
   beforeEach(function () {
@@ -32,10 +33,7 @@ describe('AnalyticsController', function () {
       },
     })
 
-    this.res = {
-      send() {},
-      sendStatus() {},
-    }
+    this.res = new MockResponse()
   })
 
   describe('updateEditingSession', function () {
@@ -56,16 +54,19 @@ describe('AnalyticsController', function () {
         .resolves({ country_code: 'XY' })
     })
 
-    it('delegates to the AnalyticsManager', async function () {
+    it('delegates to the AnalyticsManager', function (done) {
       this.SessionManager.getLoggedInUserId.returns('1234')
-      await this.controller.updateEditingSession(this.req, this.res)
-      sinon.assert.calledWith(
-        this.AnalyticsManager.updateEditingSession,
-        '1234',
-        'a project id',
-        'XY',
-        { editorType: 'abc' }
-      )
+      this.res.callback = () => {
+        sinon.assert.calledWith(
+          this.AnalyticsManager.updateEditingSession,
+          '1234',
+          'a project id',
+          'XY',
+          { editorType: 'abc' }
+        )
+        done()
+      }
+      this.controller.updateEditingSession(this.req, this.res)
     })
   })