diff --git a/services/web/app/src/Features/Analytics/AnalyticsController.js b/services/web/app/src/Features/Analytics/AnalyticsController.js index 079441db94..b5fc3a3ced 100644 --- a/services/web/app/src/Features/Analytics/AnalyticsController.js +++ b/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, } diff --git a/services/web/app/src/Features/Analytics/AnalyticsRouter.js b/services/web/app/src/Features/Analytics/AnalyticsRouter.js index 15b503dee2..70c98c7298 100644 --- a/services/web/app/src/Features/Analytics/AnalyticsRouter.js +++ b/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( diff --git a/services/web/app/src/Features/Authorization/PermissionsController.js b/services/web/app/src/Features/Authorization/PermissionsController.js index dc28b37a91..40f23135df 100644 --- a/services/web/app/src/Features/Authorization/PermissionsController.js +++ b/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) } /** diff --git a/services/web/app/src/Features/StaticPages/HomeController.js b/services/web/app/src/Features/StaticPages/HomeController.js index f96142e8fa..897598a557 100644 --- a/services/web/app/src/Features/StaticPages/HomeController.js +++ b/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') } - }, - - 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, - }) - - return res.render('external/home/website-redesign/index', { - onboardingFlowVariant: onboardingFlowAssignment.variant, - hideNewsletterCheckbox: - onboardingFlowAssignment.variant === 'token-confirmation-odc', - }) - } else { - return res.redirect('/login') - } - }, - - externalPage(page, title) { - return function (req, res, next) { - if (next == null) { - next = function () {} - } - 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) - } - }) - } - }, + } else { + await home(req, res) + } +} + +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, + }) + + res.render('external/home/website-redesign/index', { + onboardingFlowVariant: onboardingFlowAssignment.variant, + hideNewsletterCheckbox: + onboardingFlowAssignment.variant === 'token-confirmation-odc', + }) + } else { + res.redirect('/login') + } +} + +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 }) + } + } 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, } diff --git a/services/web/test/unit/src/Analytics/AnalyticsControllerTests.js b/services/web/test/unit/src/Analytics/AnalyticsControllerTests.js index 16a9ad666d..4752c1e029 100644 --- a/services/web/test/unit/src/Analytics/AnalyticsControllerTests.js +++ b/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) }) })