Merge pull request #9789 from overleaf/ab-tags-controller-async-await

[web] Cleanup and move TagsController to async/await

GitOrigin-RevId: 9684e341146e533e5a06eb6001997adf56ecced0

services/web/app/src/Features/Tags/TagsController.js

@@ -1,93 +1,71 @@
 const TagsHandler = require('./TagsHandler')
 const SessionManager = require('../Authentication/SessionManager')
 const Errors = require('../Errors/Errors')
+const { expressify } = require('../../util/promises')
 
-const TagsController = {
-  _getTags(userId, _req, res, next) {
-    if (!userId) {
-      return next(new Errors.NotFoundError())
-    }
-    TagsHandler.getAllTags(userId, function (error, allTags) {
-      if (error != null) {
-        return next(error)
-      }
-      res.json(allTags)
-    })
-  },
+async function _getTags(userId, _req, res) {
+  if (!userId) {
+    throw new Errors.NotFoundError()
+  }
+  const allTags = await TagsHandler.promises.getAllTags(userId)
+  res.json(allTags)
+}
 
-  apiGetAllTags(req, res, next) {
-    const { userId } = req.params
-    TagsController._getTags(userId, req, res, next)
-  },
+async function apiGetAllTags(req, res) {
+  const { userId } = req.params
+  await _getTags(userId, req, res)
+}
 
-  getAllTags(req, res, next) {
-    const userId = SessionManager.getLoggedInUserId(req.session)
-    TagsController._getTags(userId, req, res, next)
-  },
+async function getAllTags(req, res) {
+  const userId = SessionManager.getLoggedInUserId(req.session)
+  await _getTags(userId, req, res)
+}
 
-  createTag(req, res, next) {
-    const userId = SessionManager.getLoggedInUserId(req.session)
-    const { name } = req.body
-    TagsHandler.createTag(userId, name, function (error, tag) {
-      if (error != null) {
-        return next(error)
-      }
-      res.json(tag)
-    })
-  },
+async function createTag(req, res) {
+  const userId = SessionManager.getLoggedInUserId(req.session)
+  const { name } = req.body
+  const tag = await TagsHandler.promises.createTag(userId, name)
+  res.json(tag)
+}
 
-  addProjectToTag(req, res, next) {
-    const userId = SessionManager.getLoggedInUserId(req.session)
-    const { tagId, projectId } = req.params
-    TagsHandler.addProjectToTag(userId, tagId, projectId, function (error) {
-      if (error) {
-        return next(error)
-      }
-      res.status(204).end()
-    })
-  },
+async function addProjectToTag(req, res) {
+  const userId = SessionManager.getLoggedInUserId(req.session)
+  const { tagId, projectId } = req.params
+  await TagsHandler.promises.addProjectToTag(userId, tagId, projectId)
+  res.status(204).end()
+}
 
-  removeProjectFromTag(req, res, next) {
-    const userId = SessionManager.getLoggedInUserId(req.session)
-    const { tagId, projectId } = req.params
-    TagsHandler.removeProjectFromTag(
-      userId,
-      tagId,
-      projectId,
-      function (error) {
-        if (error) {
-          return next(error)
-        }
-        res.status(204).end()
-      }
-    )
-  },
+async function removeProjectFromTag(req, res, next) {
+  const userId = SessionManager.getLoggedInUserId(req.session)
+  const { tagId, projectId } = req.params
+  await TagsHandler.promises.removeProjectFromTag(userId, tagId, projectId)
+  res.status(204).end()
+}
 
-  deleteTag(req, res, next) {
-    const userId = SessionManager.getLoggedInUserId(req.session)
-    const { tagId } = req.params
-    TagsHandler.deleteTag(userId, tagId, function (error) {
-      if (error) {
-        return next(error)
-      }
-      res.status(204).end()
-    })
-  },
+async function deleteTag(req, res) {
+  const userId = SessionManager.getLoggedInUserId(req.session)
+  const { tagId } = req.params
+  await TagsHandler.promises.deleteTag(userId, tagId)
+  res.status(204).end()
+}
 
-  renameTag(req, res, next) {
-    const userId = SessionManager.getLoggedInUserId(req.session)
-    const { tagId } = req.params
-    const name = req.body != null ? req.body.name : undefined
-    if (!name) {
-      return res.status(400).end()
-    }
-    TagsHandler.renameTag(userId, tagId, name, function (error) {
-      if (error) {
-        return next(error)
-      }
-      res.status(204).end()
-    })
-  },
+async function renameTag(req, res) {
+  const userId = SessionManager.getLoggedInUserId(req.session)
+  const { tagId } = req.params
+  const name = req.body?.name
+  if (!name) {
+    return res.status(400).end()
+  }
+  await TagsHandler.promises.renameTag(userId, tagId, name)
+  res.status(204).end()
 }
 
-module.exports = TagsController
+module.exports = {
+  apiGetAllTags: expressify(apiGetAllTags),
+  getAllTags: expressify(getAllTags),
+  createTag: expressify(createTag),
+  addProjectToTag: expressify(addProjectToTag),
+  removeProjectFromTag: expressify(removeProjectFromTag),
+  deleteTag: expressify(deleteTag),
+  renameTag: expressify(renameTag),
+}

services/web/test/unit/src/Tags/TagsControllerTests.js

@@ -1,19 +1,6 @@
-/* eslint-disable
-    camelcase,
-    max-len,
-    no-return-assign,
-    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
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
 const SandboxedModule = require('sandboxed-module')
-const assert = require('assert')
 const sinon = require('sinon')
+const { assert } = require('chai')
 const modulePath = require('path').join(
   __dirname,
   '../../../../app/src/Features/Tags/TagsController.js'
@@ -22,24 +9,25 @@ const modulePath = require('path').join(
 describe('TagsController', function () {
   const userId = '123nd3ijdks'
   const projectId = '123njdskj9jlk'
-  const tag = 'some_class101'
 
   beforeEach(function () {
-    this.handler = {
-      addProjectToTag: sinon.stub().callsArgWith(3),
-      removeProjectFromTag: sinon.stub().callsArgWith(3),
-      deleteTag: sinon.stub().callsArg(2),
-      renameTag: sinon.stub().callsArg(3),
-      createTag: sinon.stub(),
+    this.TagsHandler = {
+      promises: {
+        addProjectToTag: sinon.stub().resolves(),
+        removeProjectFromTag: sinon.stub().resolves(),
+        deleteTag: sinon.stub().resolves(),
+        renameTag: sinon.stub().resolves(),
+        createTag: sinon.stub().resolves(),
+      },
     }
     this.SessionManager = {
       getLoggedInUserId: session => {
         return session.user._id
       },
     }
-    this.controller = SandboxedModule.require(modulePath, {
+    this.TagsController = SandboxedModule.require(modulePath, {
       requires: {
-        './TagsHandler': this.handler,
+        './TagsHandler': this.TagsHandler,
         '../Authentication/SessionManager': this.SessionManager,
       },
     })
@@ -57,138 +45,142 @@ describe('TagsController', function () {
     this.res = {}
     this.res.status = sinon.stub().returns(this.res)
     this.res.end = sinon.stub()
-    return (this.res.json = sinon.stub())
+    this.res.json = sinon.stub()
   })
 
-  describe('getAllTags', function () {
-    it('should ask the handler for all tags', function (done) {
-      const allTags = [{ name: 'tag', projects: ['123423', '423423'] }]
-      this.handler.getAllTags = sinon.stub().callsArgWith(1, null, allTags)
-      return this.controller.getAllTags(this.req, {
-        json: body => {
-          body.should.equal(allTags)
-          this.handler.getAllTags.calledWith(userId).should.equal(true)
-          return done()
-        },
-      })
+  it('get all tags', function (done) {
+    const allTags = [{ name: 'tag', projects: ['123423', '423423'] }]
+    this.TagsHandler.promises.getAllTags = sinon.stub().resolves(allTags)
+    this.TagsController.getAllTags(this.req, {
+      json: body => {
+        body.should.equal(allTags)
+        sinon.assert.calledWith(this.TagsHandler.promises.getAllTags, userId)
+        done()
+        return {
+          end: () => {},
+        }
+      },
     })
   })
 
-  describe('createTag', function () {
-    beforeEach(function () {
-      this.handler.createTag.callsArgWith(2, null, (this.tag = { mock: 'tag' }))
-      this.req.session.user._id = this.userId = 'user-id-123'
-      this.req.body = { name: (this.name = 'tag-name') }
-      return this.controller.createTag(this.req, this.res)
-    })
-
-    it('should create the tag in the backend', function () {
-      return this.handler.createTag
-        .calledWith(this.userId, this.name)
-        .should.equal(true)
-    })
-
-    it('should return the tag', function () {
-      return this.res.json.calledWith(this.tag).should.equal(true)
+  it('create a tag', function (done) {
+    this.tag = { mock: 'tag' }
+    this.TagsHandler.promises.createTag = sinon.stub().resolves(this.tag)
+    this.req.session.user._id = this.userId = 'user-id-123'
+    this.req.body = { name: (this.name = 'tag-name') }
+    this.TagsController.createTag(this.req, {
+      json: () => {
+        sinon.assert.calledWith(
+          this.TagsHandler.promises.createTag,
+          this.userId,
+          this.name
+        )
+        done()
+        return {
+          end: () => {},
+        }
+      },
     })
   })
 
-  describe('deleteTag', function () {
-    beforeEach(function () {
-      this.req.params.tagId = this.tagId = 'tag-id-123'
-      this.req.session.user._id = this.userId = 'user-id-123'
-      return this.controller.deleteTag(this.req, this.res)
-    })
-
-    it('should delete the tag in the backend', function () {
-      return this.handler.deleteTag
-        .calledWith(this.userId, this.tagId)
-        .should.equal(true)
-    })
-
-    it('should return 204 status code', function () {
-      this.res.status.calledWith(204).should.equal(true)
-      return this.res.end.called.should.equal(true)
+  it('delete a tag', function (done) {
+    this.req.params.tagId = this.tagId = 'tag-id-123'
+    this.req.session.user._id = this.userId = 'user-id-123'
+    this.TagsController.deleteTag(this.req, {
+      status: code => {
+        assert.equal(code, 204)
+        sinon.assert.calledWith(
+          this.TagsHandler.promises.deleteTag,
+          this.userId,
+          this.tagId
+        )
+        done()
+        return {
+          end: () => {},
+        }
+      },
     })
   })
 
-  describe('renameTag', function () {
+  describe('rename a tag', function () {
     beforeEach(function () {
       this.req.params.tagId = this.tagId = 'tag-id-123'
-      return (this.req.session.user._id = this.userId = 'user-id-123')
+      this.req.session.user._id = this.userId = 'user-id-123'
     })
 
-    describe('with a name', function () {
-      beforeEach(function () {
-        this.req.body = { name: (this.name = 'new-name') }
-        return this.controller.renameTag(this.req, this.res)
-      })
-
-      it('should delete the tag in the backend', function () {
-        return this.handler.renameTag
-          .calledWith(this.userId, this.tagId, this.name)
-          .should.equal(true)
-      })
-
-      it('should return 204 status code', function () {
-        this.res.status.calledWith(204).should.equal(true)
-        return this.res.end.called.should.equal(true)
+    it('with a name', function (done) {
+      this.req.body = { name: (this.name = 'new-name') }
+      this.TagsController.renameTag(this.req, {
+        status: code => {
+          assert.equal(code, 204)
+          sinon.assert.calledWith(
+            this.TagsHandler.promises.renameTag,
+            this.userId,
+            this.tagId,
+            this.name
+          )
+          done()
+          return {
+            end: () => {},
+          }
+        },
       })
     })
 
-    describe('without a name', function () {
-      beforeEach(function () {
-        return this.controller.renameTag(this.req, this.res)
-      })
-
-      it('should not call the backend', function () {
-        return this.handler.renameTag.called.should.equal(false)
-      })
-
-      it('should return 400 (bad request) status code', function () {
-        this.res.status.calledWith(400).should.equal(true)
-        return this.res.end.called.should.equal(true)
+    it('without a name', function (done) {
+      this.req.body = { name: undefined }
+      this.TagsController.renameTag(this.req, {
+        status: code => {
+          assert.equal(code, 400)
+          sinon.assert.notCalled(this.TagsHandler.promises.renameTag)
+          done()
+          return {
+            end: () => {},
+          }
+        },
       })
     })
   })
 
-  describe('addProjectToTag', function () {
-    beforeEach(function () {
-      this.req.params.tagId = this.tagId = 'tag-id-123'
-      this.req.params.projectId = this.projectId = 'project-id-123'
-      this.req.session.user._id = this.userId = 'user-id-123'
-      return this.controller.addProjectToTag(this.req, this.res)
-    })
-
-    it('should add the tag to the project in the backend', function () {
-      return this.handler.addProjectToTag
-        .calledWith(this.userId, this.tagId, this.projectId)
-        .should.equal(true)
-    })
-
-    it('should return 204 status code', function () {
-      this.res.status.calledWith(204).should.equal(true)
-      return this.res.end.called.should.equal(true)
+  it('add a project to a tag', function (done) {
+    this.req.params.tagId = this.tagId = 'tag-id-123'
+    this.req.params.projectId = this.projectId = 'project-id-123'
+    this.req.session.user._id = this.userId = 'user-id-123'
+    this.TagsController.addProjectToTag(this.req, {
+      status: code => {
+        assert.equal(code, 204)
+        sinon.assert.calledWith(
+          this.TagsHandler.promises.addProjectToTag,
+          this.userId,
+          this.tagId,
+          this.projectId
+        )
+        done()
+        return {
+          end: () => {},
+        }
+      },
     })
   })
 
-  describe('removeProjectFromTag', function () {
-    beforeEach(function () {
-      this.req.params.tagId = this.tagId = 'tag-id-123'
-      this.req.params.projectId = this.projectId = 'project-id-123'
-      this.req.session.user._id = this.userId = 'user-id-123'
-      return this.controller.removeProjectFromTag(this.req, this.res)
-    })
-
-    it('should remove the tag from the project in the backend', function () {
-      return this.handler.removeProjectFromTag
-        .calledWith(this.userId, this.tagId, this.projectId)
-        .should.equal(true)
-    })
-
-    it('should return 204 status code', function () {
-      this.res.status.calledWith(204).should.equal(true)
-      return this.res.end.called.should.equal(true)
+  it('remove a project from a tag', function (done) {
+    this.req.params.tagId = this.tagId = 'tag-id-123'
+    this.req.params.projectId = this.projectId = 'project-id-123'
+    this.req.session.user._id = this.userId = 'user-id-123'
+    this.TagsController.removeProjectFromTag(this.req, {
+      status: code => {
+        assert.equal(code, 204)
+        sinon.assert.calledWith(
+          this.TagsHandler.promises.removeProjectFromTag,
+          this.userId,
+          this.tagId,
+          this.projectId
+        )
+        done()
+        return {
+          end: () => {},
+        }
+      },
     })
   })
 })