Merge pull request #23763 from overleaf/td-bs5-user-activation

Migrate user activation page to BS5

GitOrigin-RevId: 3f4c9173cf480aafc5d874bfe81bbc8503a2e5be

services/web/modules/user-activate/app/src/UserActivateController.mjs

@@ -3,68 +3,67 @@ import { fileURLToPath } from 'node:url'
 import UserGetter from '../../../../app/src/Features/User/UserGetter.js'
 import UserRegistrationHandler from '../../../../app/src/Features/User/UserRegistrationHandler.js'
 import ErrorController from '../../../../app/src/Features/Errors/ErrorController.js'
+import { expressify } from '@overleaf/promise-utils'
 
 const __dirname = Path.dirname(fileURLToPath(import.meta.url))
 
-export default {
-  registerNewUser(req, res, next) {
-    res.render(Path.resolve(__dirname, '../views/user/register'))
-  },
+function registerNewUser(req, res, next) {
+  res.render(Path.resolve(__dirname, '../views/user/register'))
+}
 
-  register(req, res, next) {
-    const { email } = req.body
-    if (email == null || email === '') {
-      return res.sendStatus(422) // Unprocessable Entity
-    }
-    UserRegistrationHandler.registerNewUserAndSendActivationEmail(
-      email,
-      (error, user, setNewPasswordUrl) => {
-        if (error != null) {
-          return next(error)
-        }
-        res.json({
-          email: user.email,
-          setNewPasswordUrl,
-        })
-      }
+async function register(req, res, next) {
+  const { email } = req.body
+  if (email == null || email === '') {
+    return res.sendStatus(422) // Unprocessable Entity
+  }
+  const { user, setNewPasswordUrl } =
+    await UserRegistrationHandler.promises.registerNewUserAndSendActivationEmail(
+      email
     )
-  },
+  res.json({
+    email: user.email,
+    setNewPasswordUrl,
+  })
+}
 
-  activateAccountPage(req, res, next) {
-    // An 'activation' is actually just a password reset on an account that
-    // was set with a random password originally.
-    if (req.query.user_id == null || req.query.token == null) {
-      return ErrorController.notFound(req, res)
-    }
+async function activateAccountPage(req, res, next) {
+  // An 'activation' is actually just a password reset on an account that
+  // was set with a random password originally.
+  if (req.query.user_id == null || req.query.token == null) {
+    return ErrorController.notFound(req, res)
+  }
 
-    if (typeof req.query.user_id !== 'string') {
-      return ErrorController.forbidden(req, res)
-    }
+  if (typeof req.query.user_id !== 'string') {
+    return ErrorController.forbidden(req, res)
+  }
 
-    UserGetter.getUser(
-      req.query.user_id,
-      { email: 1, loginCount: 1 },
-      (error, user) => {
-        if (error != null) {
-          return next(error)
-        }
-        if (!user) {
-          return ErrorController.notFound(req, res)
-        }
-        if (user.loginCount > 0) {
-          // Already seen this user, so account must be activate
-          // This lets users keep clicking the 'activate' link in their email
-          // as a way to log in which, if I know our users, they will.
-          res.redirect(`/login`)
-        } else {
-          req.session.doLoginAfterPasswordReset = true
-          res.render(Path.resolve(__dirname, '../views/user/activate'), {
-            title: 'activate_account',
-            email: user.email,
-            token: req.query.token,
-          })
-        }
-      }
-    )
-  },
+  const user = await UserGetter.promises.getUser(req.query.user_id, {
+    email: 1,
+    loginCount: 1,
+  })
+
+  if (!user) {
+    return ErrorController.notFound(req, res)
+  }
+
+  if (user.loginCount > 0) {
+    // Already seen this user, so account must be activated.
+    // This lets users keep clicking the 'activate' link in their email
+    // as a way to log in which, if I know our users, they will.
+    return res.redirect(`/login`)
+  }
+
+  req.session.doLoginAfterPasswordReset = true
+
+  res.render(Path.resolve(__dirname, '../views/user/activate'), {
+    title: 'activate_account',
+    email: user.email,
+    token: req.query.token,
+  })
+}
+
+export default {
+  registerNewUser,
+  register: expressify(register),
+  activateAccountPage: expressify(activateAccountPage),
 }

services/web/modules/user-activate/app/views/user/activate.pug

@@ -1,67 +1,72 @@
-extends ../../../../../app/views/layout-marketing
+extends ../../../../../app/views/layout-website-redesign-bootstrap-5
 
 block content
 	main.content.content-alt#main-content
 		.container
-			.row
-				.col-md-6.col-md-offset-3.col-lg-4.col-lg-offset-4
-					.alert.alert-success #{translate("nearly_activated")}
-			.row
-				.col-md-6.col-md-offset-3.col-lg-4.col-lg-offset-4
-					.card
-						.page-header
-							h1 #{translate("please_set_a_password")}
-						form(
-							data-ol-async-form
-							name="activationForm",
-							action="/user/password/set",
-							method="POST",
-						)
-							+formMessages()
+			div.col-lg-6.col-xl-4.m-auto
+				.notification-list
+					.notification.notification-type-success(aria-live="off" role="alert")
+						.notification-content-and-cta
+							.notification-icon
+								span.material-symbols(aria-hidden="true")
+									| check_circle
+							.notification-content
+								p
+									| #{translate("nearly_activated")}
+
+				h1.h3 #{translate("please_set_a_password")}
+
+				form(
+					data-ol-async-form
+					name="activationForm",
+					action="/user/password/set",
+					method="POST",
+				)
+					+formMessages()
 
-							+customFormMessage('token-expired', 'danger')
-								| #{translate("activation_token_expired")}
+					+customFormMessage('token-expired', 'danger')
+						| #{translate("activation_token_expired")}
 
-							+customFormMessage('invalid-password', 'danger')
-								| #{translate('invalid_password')}
+					+customFormMessage('invalid-password', 'danger')
+						| #{translate('invalid_password')}
 
-							input(name='_csrf', type='hidden', value=csrfToken)
-							input(
-								type="hidden",
-								name="passwordResetToken",
-								value=token
-							)
+					input(name='_csrf', type='hidden', value=csrfToken)
+					input(
+						type="hidden",
+						name="passwordResetToken",
+						value=token
+					)
 
-							.form-group
-								label(for='email') #{translate("email")}
-								input.form-control(
-									aria-label="email",
-									type='email',
-									name='email',
-									placeholder="email@example.com",
-									autocomplete="username"
-									value=email
-									required,
-									readonly
-								)
-							.form-group
-								label(for='password') #{translate("password")}
-								input.form-control#passwordField(
-									type='password',
-									name='password',
-									placeholder="********",
-									autocomplete="new-password",
-									autofocus,
-									required,
-									minlength=settings.passwordStrengthOptions.length.min
-								)
-							.actions
-								button.btn.btn-primary(
-									type='submit',
-									data-ol-disabled-inflight
-									aria-label=translate('activate')
-								)
-									span(data-ol-inflight="idle")
-										| #{translate('activate')}
-									span(hidden data-ol-inflight="pending")
-										| #{translate('activating')}…
+					.form-group
+						label(for='emailField') #{translate("email")}
+						input.form-control#emailField(
+							aria-label="email",
+							type='email',
+							name='email',
+							placeholder="email@example.com",
+							autocomplete="username"
+							value=email
+							required,
+							disabled
+						)
+					.form-group
+						label(for='passwordField') #{translate("password")}
+						input.form-control#passwordField(
+							type='password',
+							name='password',
+							placeholder="********",
+							autocomplete="new-password",
+							autofocus,
+							required,
+							minlength=settings.passwordStrengthOptions.length.min
+						)
+					.actions
+						button.btn.btn-primary(
+							type='submit',
+							data-ol-disabled-inflight
+							aria-label=translate('activate')
+						)
+							span(data-ol-inflight="idle")
+								| #{translate('activate')}
+							span(hidden data-ol-inflight="pending")
+								| #{translate('activating')}…

services/web/modules/user-activate/test/unit/src/UserActivateControllerTests.mjs

@@ -17,15 +17,26 @@ describe('UserActivateController', function () {
       email: 'joe@example.com',
     }
 
-    this.UserGetter = { getUser: sinon.stub() }
-    this.UserRegistrationHandler = {}
+    this.UserGetter = {
+      promises: {
+        getUser: sinon.stub(),
+      },
+    }
+    this.UserRegistrationHandler = { promises: {} }
     this.ErrorController = { notFound: sinon.stub() }
+    this.SplitTestHandler = {
+      promises: {
+        getAssignment: sinon.stub().resolves({ variant: 'default' }),
+      },
+    }
     this.UserActivateController = await esmock(MODULE_PATH, {
       '../../../../../app/src/Features/User/UserGetter.js': this.UserGetter,
       '../../../../../app/src/Features/User/UserRegistrationHandler.js':
         this.UserRegistrationHandler,
       '../../../../../app/src/Features/Errors/ErrorController.js':
         this.ErrorController,
+      '../../../../../app/src/Features/SplitTests/SplitTestHandler':
+        this.SplitTestHandler,
     })
     this.req = {
       body: {},
@@ -41,12 +52,12 @@ describe('UserActivateController', function () {
 
   describe('activateAccountPage', function () {
     beforeEach(function () {
-      this.UserGetter.getUser = sinon.stub().callsArgWith(2, null, this.user)
+      this.UserGetter.promises.getUser = sinon.stub().resolves(this.user)
       this.req.query.user_id = this.user_id
       this.req.query.token = this.token = 'mock-token-123'
     })
 
-    it('should 404 without a user_id', function (done) {
+    it('should 404 without a user_id', async function (done) {
       delete this.req.query.user_id
       this.ErrorController.notFound = () => done()
       this.UserActivateController.activateAccountPage(this.req, this.res)
@@ -59,7 +70,7 @@ describe('UserActivateController', function () {
     })
 
     it('should 404 without a valid user_id', function (done) {
-      this.UserGetter.getUser = sinon.stub().callsArgWith(2, null, null)
+      this.UserGetter.promises.getUser = sinon.stub().resolves(null)
       this.ErrorController.notFound = () => done()
       this.UserActivateController.activateAccountPage(this.req, this.res)
     })
@@ -73,7 +84,7 @@ describe('UserActivateController', function () {
     it('should redirect activated users to login', function (done) {
       this.user.loginCount = 1
       this.res.redirect = url => {
-        this.UserGetter.getUser.calledWith(this.user_id).should.equal(true)
+        sinon.assert.calledWith(this.UserGetter.promises.getUser, this.user_id)
         url.should.equal('/login')
         return done()
       }
@@ -93,17 +104,20 @@ describe('UserActivateController', function () {
   })
 
   describe('register', function () {
-    beforeEach(function () {
-      this.UserRegistrationHandler.registerNewUserAndSendActivationEmail = sinon
-        .stub()
-        .callsArgWith(1, null, this.user, (this.url = 'mock/url'))
+    beforeEach(async function () {
+      this.UserRegistrationHandler.promises.registerNewUserAndSendActivationEmail =
+        sinon.stub().resolves({
+          user: this.user,
+          setNewPasswordUrl: (this.url = 'mock/url'),
+        })
       this.req.body.email = this.user.email = this.email = 'email@example.com'
-      this.UserActivateController.register(this.req, this.res)
+      await this.UserActivateController.register(this.req, this.res)
     })
 
     it('should register the user and send them an email', function () {
       sinon.assert.calledWith(
-        this.UserRegistrationHandler.registerNewUserAndSendActivationEmail,
+        this.UserRegistrationHandler.promises
+          .registerNewUserAndSendActivationEmail,
         this.email
       )
     })