From 30ebad91b75acb77ee60f373fbae00135dc31cbe Mon Sep 17 00:00:00 2001 From: Domagoj Kriskovic Date: Mon, 13 Jan 2025 14:26:13 +0100 Subject: [PATCH] Allow reviewers to resolve their own comments (#22582) * Allow reviewers to resolve their own comments * check if reviewer is comment author * add missing translation * add CommentsController tests * added DocumentManagerTests * added HttpControllerTests * Add AuthorizationManagerTests * added AuthorizationMiddlewareTests * added DocumentUpdaterHandler test * fix test descriptions * remove returns from CommentsControllerTests * use ensureUserCanResolveThread in authorizationMiddleware * move canResolveThread to AuthorizationManager * commentId as param in NotFoundError * refactor canUserResolveThread GitOrigin-RevId: 131c3d1eb9ac916eaaa9221d351a92bc07b80cdc --- services/document-updater/app.js | 4 + .../app/js/DocumentManager.js | 26 +++++ .../document-updater/app/js/HttpController.js | 24 +++++ .../DocumentManager/DocumentManagerTests.js | 71 ++++++++++++ .../js/HttpController/HttpControllerTests.js | 59 ++++++++++ .../Authorization/AuthorizationManager.js | 44 ++++++++ .../Authorization/AuthorizationMiddleware.js | 49 +++++++++ .../DocumentUpdater/DocumentUpdaterHandler.js | 19 ++++ services/web/locales/en.json | 4 +- .../AuthorizationManagerTests.js | 102 ++++++++++++++++++ .../AuthorizationMiddlewareTests.js | 51 +++++++++ .../DocumentUpdaterHandlerTests.js | 34 ++++++ 12 files changed, 486 insertions(+), 1 deletion(-) diff --git a/services/document-updater/app.js b/services/document-updater/app.js index 9466c188ac..2932bba87d 100644 --- a/services/document-updater/app.js +++ b/services/document-updater/app.js @@ -135,6 +135,10 @@ app.use((req, res, next) => { }) app.get('/project/:project_id/doc/:doc_id', HttpController.getDoc) +app.get( + '/project/:project_id/doc/:doc_id/comment/:comment_id', + HttpController.getComment +) app.get('/project/:project_id/doc/:doc_id/peek', HttpController.peekDoc) // temporarily keep the GET method for backwards compatibility app.get('/project/:project_id/doc', HttpController.getProjectDocsAndFlushIfOld) diff --git a/services/document-updater/app/js/DocumentManager.js b/services/document-updater/app/js/DocumentManager.js index 1b5598aab8..540a8a254c 100644 --- a/services/document-updater/app/js/DocumentManager.js +++ b/services/document-updater/app/js/DocumentManager.js @@ -367,6 +367,21 @@ const DocumentManager = { } }, + async getComment(projectId, docId, commentId) { + const { ranges } = await DocumentManager.getDoc(projectId, docId) + + const comment = ranges?.comments?.find(comment => comment.id === commentId) + + if (!comment) { + throw new Errors.NotFoundError({ + message: 'comment not found', + info: { commentId }, + }) + } + + return { comment } + }, + async deleteComment(projectId, docId, commentId, userId) { const { lines, version, ranges, pathname, historyRangesSupport } = await DocumentManager.getDoc(projectId, docId) @@ -500,6 +515,16 @@ const DocumentManager = { ) }, + async getCommentWithLock(projectId, docId, commentId) { + const UpdateManager = require('./UpdateManager') + return await UpdateManager.promises.lockUpdatesAndDo( + DocumentManager.getComment, + projectId, + docId, + commentId + ) + }, + async getDocAndRecentOpsWithLock(projectId, docId, fromVersion) { const UpdateManager = require('./UpdateManager') return await UpdateManager.promises.lockUpdatesAndDo( @@ -676,6 +701,7 @@ module.exports = { 'pathname', 'projectHistoryId', ], + getCommentWithLock: ['comment'], }, }), promises: DocumentManager, diff --git a/services/document-updater/app/js/HttpController.js b/services/document-updater/app/js/HttpController.js index 2d6e81eebb..d8f2d0c5d3 100644 --- a/services/document-updater/app/js/HttpController.js +++ b/services/document-updater/app/js/HttpController.js @@ -49,6 +49,29 @@ function getDoc(req, res, next) { ) } +function getComment(req, res, next) { + const docId = req.params.doc_id + const projectId = req.params.project_id + const commentId = req.params.comment_id + + logger.debug({ projectId, docId, commentId }, 'getting comment via http') + + DocumentManager.getCommentWithLock( + projectId, + docId, + commentId, + (error, comment) => { + if (error) { + return next(error) + } + if (comment == null) { + return next(new Errors.NotFoundError('comment not found')) + } + res.json(comment) + } + ) +} + // return the doc from redis if present, but don't load it from mongo function peekDoc(req, res, next) { const docId = req.params.doc_id @@ -506,4 +529,5 @@ module.exports = { flushQueuedProjects, blockProject, unblockProject, + getComment, } diff --git a/services/document-updater/test/unit/js/DocumentManager/DocumentManagerTests.js b/services/document-updater/test/unit/js/DocumentManager/DocumentManagerTests.js index 5dc3d1c88f..e9d68ee414 100644 --- a/services/document-updater/test/unit/js/DocumentManager/DocumentManagerTests.js +++ b/services/document-updater/test/unit/js/DocumentManager/DocumentManagerTests.js @@ -835,6 +835,77 @@ describe('DocumentManager', function () { }) }) + describe('getComment', function () { + beforeEach(function () { + this.ranges.comments = [ + { + id: 'mock-comment-id-1', + }, + { + id: 'mock-comment-id-2', + }, + ] + this.DocumentManager.promises.getDoc = sinon.stub().resolves({ + lines: this.lines, + version: this.version, + ranges: this.ranges, + }) + }) + + describe('when comment exists', function () { + beforeEach(async function () { + await expect( + this.DocumentManager.promises.getComment( + this.project_id, + this.doc_id, + 'mock-comment-id-1' + ) + ).to.eventually.deep.equal({ + comment: { id: 'mock-comment-id-1' }, + }) + }) + + it("should get the document's current ranges", function () { + this.DocumentManager.promises.getDoc + .calledWith(this.project_id, this.doc_id) + .should.equal(true) + }) + }) + + describe('when comment doesnt exists', function () { + beforeEach(async function () { + await expect( + this.DocumentManager.promises.getComment( + this.project_id, + this.doc_id, + 'mock-comment-id-x' + ) + ).to.be.rejectedWith(Errors.NotFoundError) + }) + + it("should get the document's current ranges", function () { + this.DocumentManager.promises.getDoc + .calledWith(this.project_id, this.doc_id) + .should.equal(true) + }) + }) + + describe('when the doc is not found', function () { + beforeEach(async function () { + this.DocumentManager.promises.getDoc = sinon + .stub() + .resolves({ lines: null, version: null, ranges: null }) + await expect( + this.DocumentManager.promises.acceptChanges( + this.project_id, + this.doc_id, + [this.change_id] + ) + ).to.be.rejectedWith(Errors.NotFoundError) + }) + }) + }) + describe('deleteComment', function () { beforeEach(function () { this.comment_id = 'mock-comment-id' diff --git a/services/document-updater/test/unit/js/HttpController/HttpControllerTests.js b/services/document-updater/test/unit/js/HttpController/HttpControllerTests.js index d6aa03ab52..2b8d288ef8 100644 --- a/services/document-updater/test/unit/js/HttpController/HttpControllerTests.js +++ b/services/document-updater/test/unit/js/HttpController/HttpControllerTests.js @@ -184,6 +184,65 @@ describe('HttpController', function () { }) }) + describe('getComment', function () { + beforeEach(function () { + this.ranges = { + changes: 'mock', + comments: [ + { + id: 'comment-id-1', + }, + { + id: 'comment-id-2', + }, + ], + } + this.req = { + params: { + project_id: this.project_id, + doc_id: this.doc_id, + comment_id: this.comment_id, + }, + query: {}, + body: {}, + } + }) + + beforeEach(function () { + this.DocumentManager.getCommentWithLock = sinon + .stub() + .callsArgWith(3, null, this.ranges.comments[0]) + this.HttpController.getComment(this.req, this.res, this.next) + }) + + it('should get the comment', function () { + this.DocumentManager.getCommentWithLock + .calledWith(this.project_id, this.doc_id, this.comment_id) + .should.equal(true) + }) + + it('should return the comment as JSON', function () { + this.res.json + .calledWith({ + id: 'comment-id-1', + }) + .should.equal(true) + }) + + it('should log the request', function () { + this.logger.debug + .calledWith( + { + projectId: this.project_id, + docId: this.doc_id, + commentId: this.comment_id, + }, + 'getting comment via http' + ) + .should.equal(true) + }) + }) + describe('setDoc', function () { beforeEach(function () { this.lines = ['one', 'two', 'three'] diff --git a/services/web/app/src/Features/Authorization/AuthorizationManager.js b/services/web/app/src/Features/Authorization/AuthorizationManager.js index 565788c3ba..c28ddf363d 100644 --- a/services/web/app/src/Features/Authorization/AuthorizationManager.js +++ b/services/web/app/src/Features/Authorization/AuthorizationManager.js @@ -10,6 +10,7 @@ const PublicAccessLevels = require('./PublicAccessLevels') const Errors = require('../Errors/Errors') const { hasAdminAccess } = require('../Helpers/AdminAuthorizationHelper') const Settings = require('@overleaf/settings') +const DocumentUpdaterHandler = require('../DocumentUpdater/DocumentUpdaterHandler') function isRestrictedUser( userId, @@ -190,6 +191,19 @@ async function canUserReadProject(userId, projectId, token) { ].includes(privilegeLevel) } +async function canUserReviewProjectContent(userId, projectId, token) { + const privilegeLevel = await getPrivilegeLevelForProject( + userId, + projectId, + token + ) + return [ + PrivilegeLevels.OWNER, + PrivilegeLevels.READ_AND_WRITE, + PrivilegeLevels.REVIEW, + ].includes(privilegeLevel) +} + async function canUserWriteProjectContent(userId, projectId, token) { const privilegeLevel = await getPrivilegeLevelForProject( userId, @@ -240,9 +254,37 @@ async function isUserSiteAdmin(userId) { return hasAdminAccess(user) } +async function canUserResolveThread(userId, projectId, docId, threadId, token) { + const privilegeLevel = await getPrivilegeLevelForProject( + userId, + projectId, + token, + { ignorePublicAccess: true } + ) + if ( + privilegeLevel === PrivilegeLevels.OWNER || + privilegeLevel === PrivilegeLevels.READ_AND_WRITE + ) { + return true + } + + if (privilegeLevel !== PrivilegeLevels.REVIEW) { + return false + } + + const comment = await DocumentUpdaterHandler.promises.getComment( + projectId, + docId, + threadId + ) + return comment.metadata.user_id === userId +} + module.exports = { canUserReadProject: callbackify(canUserReadProject), canUserWriteProjectContent: callbackify(canUserWriteProjectContent), + canUserReviewProjectContent: callbackify(canUserReviewProjectContent), + canUserResolveThread: callbackify(canUserResolveThread), canUserWriteProjectSettings: callbackify(canUserWriteProjectSettings), canUserRenameProject: callbackify(canUserRenameProject), canUserAdminProject: callbackify(canUserAdminProject), @@ -253,6 +295,8 @@ module.exports = { promises: { canUserReadProject, canUserWriteProjectContent, + canUserReviewProjectContent, + canUserResolveThread, canUserWriteProjectSettings, canUserRenameProject, canUserAdminProject, diff --git a/services/web/app/src/Features/Authorization/AuthorizationMiddleware.js b/services/web/app/src/Features/Authorization/AuthorizationMiddleware.js index 6b2d2ab920..93db5af1c1 100644 --- a/services/web/app/src/Features/Authorization/AuthorizationMiddleware.js +++ b/services/web/app/src/Features/Authorization/AuthorizationMiddleware.js @@ -103,6 +103,32 @@ async function ensureUserCanWriteProjectSettings(req, res, next) { next() } +async function ensureUserCanResolveThread(req, res, next) { + const projectId = _getProjectId(req) + const docId = _getDocId(req) + const threadId = _getThreadId(req) + const userId = _getUserId(req) + const token = TokenAccessHandler.getRequestToken(req, projectId) + const canResolveThread = + await AuthorizationManager.promises.canUserResolveThread( + userId, + projectId, + docId, + threadId, + token + ) + if (canResolveThread) { + logger.debug({ userId, projectId }, 'allowing user resolve comment thread') + return next() + } + + logger.debug( + { userId, projectId, threadId }, + 'denying user to resolve comment thread' + ) + return HttpErrorHandler.forbidden(req, res) +} + async function ensureUserCanWriteProjectContent(req, res, next) { const projectId = _getProjectId(req) const userId = _getUserId(req) @@ -166,6 +192,28 @@ function _getProjectId(req) { return projectId } +function _getDocId(req) { + const docId = req.params.doc_id + if (!docId) { + throw new Error('Expected doc_id in request parameters') + } + if (!ObjectId.isValid(docId)) { + throw new Errors.NotFoundError(`invalid docId: ${docId}`) + } + return docId +} + +function _getThreadId(req) { + const threadId = req.params.thread_id + if (!threadId) { + throw new Error('Expected thread_id in request parameters') + } + if (!ObjectId.isValid(threadId)) { + throw new Errors.NotFoundError(`invalid threadId: ${threadId}`) + } + return threadId +} + function _getUserId(req) { return ( SessionManager.getLoggedInUserId(req.session) || @@ -200,6 +248,7 @@ module.exports = { ensureUserCanWriteProjectSettings: expressify( ensureUserCanWriteProjectSettings ), + ensureUserCanResolveThread: expressify(ensureUserCanResolveThread), ensureUserCanWriteProjectContent: expressify( ensureUserCanWriteProjectContent ), diff --git a/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js b/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js index dbb8ce9c74..70e6770053 100644 --- a/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js +++ b/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js @@ -80,6 +80,23 @@ function deleteDoc(projectId, docId, ignoreFlushErrors, callback) { ) } +function getComment(projectId, docId, commentId, callback) { + _makeRequest( + { + path: `/project/${projectId}/doc/${docId}/comment/${commentId}`, + json: true, + }, + projectId, + 'get-comment', + function (error, comment) { + if (error) { + return callback(error) + } + callback(null, comment) + } + ) +} + function getDocument(projectId, docId, fromVersion, callback) { _makeRequest( { @@ -548,6 +565,7 @@ module.exports = { flushProjectToMongoAndDelete, flushDocToMongo, deleteDoc, + getComment, getDocument, setDocument, appendToDocument, @@ -567,6 +585,7 @@ module.exports = { flushProjectToMongoAndDelete: promisify(flushProjectToMongoAndDelete), flushDocToMongo: promisify(flushDocToMongo), deleteDoc: promisify(deleteDoc), + getComment: promisify(getComment), getDocument: promisifyMultiResult(getDocument, [ 'lines', 'version', diff --git a/services/web/locales/en.json b/services/web/locales/en.json index 422526725f..3969d99f57 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -573,7 +573,7 @@ "easily_manage_your_project_files_everywhere": "Easily manage your project files, everywhere", "easy_collaboration_for_students": "Easy collaboration for students. Supports longer or more complex projects.", "edit": "Edit", - "edit_comment_error_message": "There was an error editing your comment. Please try again in a few moments.", + "edit_comment_error_message": "There was an error editing the comment. Please try again in a few moments.", "edit_comment_error_title": "Edit Comment Error", "edit_dictionary": "Edit Dictionary", "edit_dictionary_empty": "Your custom dictionary is empty.", @@ -1813,6 +1813,8 @@ "resize": "Resize", "resolve": "Resolve", "resolve_comment": "Resolve comment", + "resolve_comment_error_message": "There was an error resolving the comment. Please try again in a few moments.", + "resolve_comment_error_title": "Resolve Comment Error", "resolved_comments": "Resolved comments", "restore": "Restore", "restore_file": "Restore file", diff --git a/services/web/test/unit/src/Authorization/AuthorizationManagerTests.js b/services/web/test/unit/src/Authorization/AuthorizationManagerTests.js index baa48001ea..4c272387d2 100644 --- a/services/web/test/unit/src/Authorization/AuthorizationManagerTests.js +++ b/services/web/test/unit/src/Authorization/AuthorizationManagerTests.js @@ -12,6 +12,8 @@ describe('AuthorizationManager', function () { beforeEach(function () { this.user = { _id: new ObjectId() } this.project = { _id: new ObjectId() } + this.doc = { _id: new ObjectId() } + this.thread = { _id: new ObjectId() } this.token = 'some-token' this.ProjectGetter = { @@ -46,6 +48,14 @@ describe('AuthorizationManager', function () { }, } + this.DocumentUpdaterHandler = { + promises: { + getComment: sinon + .stub() + .resolves({ metadata: { user_id: new ObjectId() } }), + }, + } + this.AuthorizationManager = SandboxedModule.require(modulePath, { requires: { 'mongodb-legacy': { ObjectId }, @@ -54,6 +64,8 @@ describe('AuthorizationManager', function () { '../Project/ProjectGetter': this.ProjectGetter, '../../models/User': { User: this.User }, '../TokenAccess/TokenAccessHandler': this.TokenAccessHandler, + '../DocumentUpdater/DocumentUpdaterHandler': + this.DocumentUpdaterHandler, '@overleaf/settings': { passwordStrengthOptions: {}, adminPrivilegeAvailable: true, @@ -70,6 +82,7 @@ describe('AuthorizationManager', function () { ['id', 'readAndWrite', true, true], ['id', 'readOnly', false, false], ['id', 'readOnly', false, true], + ['id', 'review', false, true], ] const restrictedScenarios = [ [null, 'readOnly', false, false], @@ -432,6 +445,7 @@ describe('AuthorizationManager', function () { siteAdmin: true, owner: true, readAndWrite: true, + review: true, readOnly: true, publicReadAndWrite: true, publicReadOnly: true, @@ -439,6 +453,15 @@ describe('AuthorizationManager', function () { tokenReadOnly: true, }) + testPermission('canUserReviewProjectContent', { + siteAdmin: true, + owner: true, + readAndWrite: true, + review: true, + publicReadAndWrite: true, + tokenReadAndWrite: true, + }) + testPermission('canUserWriteProjectContent', { siteAdmin: true, owner: true, @@ -503,6 +526,80 @@ describe('AuthorizationManager', function () { }) }) }) + + describe('canUserReviewThread', function () { + it('should return true when user has write permissions', async function () { + this.CollaboratorsGetter.promises.getMemberIdPrivilegeLevel + .withArgs(this.user._id, this.project._id) + .resolves(PrivilegeLevels.READ_AND_WRITE) + + const canResolve = + await this.AuthorizationManager.promises.canUserResolveThread( + this.user._id, + this.project._id, + this.doc._id, + this.thread._id, + this.token + ) + + expect(canResolve).to.equal(true) + }) + + it('should return false when user has read permission', async function () { + this.CollaboratorsGetter.promises.getMemberIdPrivilegeLevel + .withArgs(this.user._id, this.project._id) + .resolves(PrivilegeLevels.READ_ONLY) + + const canResolve = + await this.AuthorizationManager.promises.canUserResolveThread( + this.user._id, + this.project._id, + this.doc._id, + this.thread._id, + this.token + ) + + expect(canResolve).to.equal(false) + }) + + describe('when user has review permission', function () { + beforeEach(function () { + this.CollaboratorsGetter.promises.getMemberIdPrivilegeLevel + .withArgs(this.user._id, this.project._id) + .resolves(PrivilegeLevels.REVIEW) + }) + + it('should return false when user is not the comment author', async function () { + const canResolve = + await this.AuthorizationManager.promises.canUserResolveThread( + this.user._id, + this.project._id, + this.doc._id, + this.thread._id, + this.token + ) + + expect(canResolve).to.equal(false) + }) + + it('should return true when user is the comment author', async function () { + this.DocumentUpdaterHandler.promises.getComment + .withArgs(this.project._id, this.doc._id, this.thread._id) + .resolves({ metadata: { user_id: this.user._id } }) + + const canResolve = + await this.AuthorizationManager.promises.canUserResolveThread( + this.user._id, + this.project._id, + this.doc._id, + this.thread._id, + this.token + ) + + expect(canResolve).to.equal(true) + }) + }) + }) }) function testPermission(permission, privilegeLevels) { @@ -525,6 +622,11 @@ function testPermission(permission, privilegeLevels) { expectPermission(permission, privilegeLevels.readAndWrite || false) }) + describe('when user has review access', function () { + setupUserPrivilegeLevel(PrivilegeLevels.REVIEW) + expectPermission(permission, privilegeLevels.review || false) + }) + describe('when user has read-only access', function () { setupUserPrivilegeLevel(PrivilegeLevels.READ_ONLY) expectPermission(permission, privilegeLevels.readOnly || false) diff --git a/services/web/test/unit/src/Authorization/AuthorizationMiddlewareTests.js b/services/web/test/unit/src/Authorization/AuthorizationMiddlewareTests.js index 6ae930048c..43c5d4bf0c 100644 --- a/services/web/test/unit/src/Authorization/AuthorizationMiddlewareTests.js +++ b/services/web/test/unit/src/Authorization/AuthorizationMiddlewareTests.js @@ -11,6 +11,8 @@ describe('AuthorizationMiddleware', function () { beforeEach(function () { this.userId = new ObjectId().toString() this.project_id = new ObjectId().toString() + this.doc_id = new ObjectId().toString() + this.thread_id = new ObjectId().toString() this.token = 'some-token' this.AuthenticationController = {} this.SessionManager = { @@ -23,8 +25,10 @@ describe('AuthorizationMiddleware', function () { canUserReadProject: sinon.stub(), canUserWriteProjectSettings: sinon.stub(), canUserWriteProjectContent: sinon.stub(), + canUserResolveThread: sinon.stub(), canUserAdminProject: sinon.stub(), canUserRenameProject: sinon.stub(), + canUserReviewProjectContent: sinon.stub(), isUserSiteAdmin: sinon.stub(), isRestrictedUserForProject: sinon.stub(), }, @@ -35,6 +39,11 @@ describe('AuthorizationMiddleware', function () { this.TokenAccessHandler = { getRequestToken: sinon.stub().returns(this.token), } + this.DocumentUpdaterHandler = { + promises: { + getComment: sinon.stub().resolves(), + }, + } this.AuthorizationMiddleware = SandboxedModule.require(MODULE_PATH, { requires: { './AuthorizationManager': this.AuthorizationManager, @@ -47,6 +56,8 @@ describe('AuthorizationMiddleware', function () { '../Helpers/AdminAuthorizationHelper': { canRedirectToAdminDomain: sinon.stub().returns(false), }, + '../DocumentUpdater/DocumentUpdaterHandler': + this.DocumentUpdaterHandler, }, }) this.req = { @@ -75,6 +86,46 @@ describe('AuthorizationMiddleware', function () { ) }) + describe('ensureUserCanResolveThread', function () { + beforeEach(function () { + this.req.params.doc_id = this.doc_id + this.req.params.thread_id = this.thread_id + }) + describe('when user has permission', function () { + beforeEach(function () { + this.AuthorizationManager.promises.canUserResolveThread + .withArgs( + this.userId, + this.project_id, + this.doc_id, + this.thread_id, + this.token + ) + .resolves(true) + }) + + invokeMiddleware('ensureUserCanResolveThread') + expectNext() + }) + + describe("when user doesn't have permission", function () { + beforeEach(function () { + this.AuthorizationManager.promises.canUserResolveThread + .withArgs( + this.userId, + this.project_id, + this.doc_id, + this.thread_id, + this.token + ) + .resolves(false) + }) + + invokeMiddleware('ensureUserCanResolveThread') + expectForbidden() + }) + }) + describe('ensureUserCanWriteProjectSettings', function () { describe('when renaming a project', function () { beforeEach(function () { diff --git a/services/web/test/unit/src/DocumentUpdater/DocumentUpdaterHandlerTests.js b/services/web/test/unit/src/DocumentUpdater/DocumentUpdaterHandlerTests.js index b215d6b42c..3e9d2ee9d9 100644 --- a/services/web/test/unit/src/DocumentUpdater/DocumentUpdaterHandlerTests.js +++ b/services/web/test/unit/src/DocumentUpdater/DocumentUpdaterHandlerTests.js @@ -459,6 +459,40 @@ describe('DocumentUpdaterHandler', function () { }) }) + describe('getComment', function () { + describe('successfully', function () { + beforeEach(function () { + this.comment = { + id: 'mock-comment-id-1', + } + this.body = this.comment + this.request.callsArgWith(1, null, { statusCode: 200 }, this.body) + this.handler.getComment( + this.project_id, + this.doc_id, + this.comment.id, + this.callback + ) + }) + + it('should get the comment from the document updater', function () { + const url = `${this.settings.apis.documentupdater.url}/project/${this.project_id}/doc/${this.doc_id}/comment/${this.comment.id}` + this.request + .calledWith({ + url, + method: 'GET', + json: true, + timeout: 30 * 1000, + }) + .should.equal(true) + }) + + it('should call the callback with the comment', function () { + this.callback.calledWithExactly(null, this.comment).should.equal(true) + }) + }) + }) + describe('getDocument', function () { describe('successfully', function () { beforeEach(function () {