Merge pull request #18516 from overleaf/em-web-resolved-comment-ids

Reintroduce resolved comment ids in getDocument()

GitOrigin-RevId: 591589efc643c815c40df440d1297158901f7a79
This commit is contained in:
Eric Mc Sween 2024-05-27 08:03:04 -04:00 committed by Copybot
parent 485710538d
commit 3a1560894a
4 changed files with 69 additions and 18 deletions

View file

@ -9,8 +9,8 @@ const https = require('https')
* Make a request and return the parsed JSON response. * Make a request and return the parsed JSON response.
* *
* @param {string | URL} url - request URL * @param {string | URL} url - request URL
* @param {object} opts - fetch options * @param {any} [opts] - fetch options
* @return {Promise<object>} the parsed JSON response * @return {Promise<any>} the parsed JSON response
* @throws {RequestFailedError} if the response has a failure status code * @throws {RequestFailedError} if the response has a failure status code
*/ */
async function fetchJson(url, opts = {}) { async function fetchJson(url, opts = {}) {
@ -39,7 +39,7 @@ async function fetchJsonWithResponse(url, opts = {}) {
* If the response body is destroyed, the request is aborted. * If the response body is destroyed, the request is aborted.
* *
* @param {string | URL} url - request URL * @param {string | URL} url - request URL
* @param {object} opts - fetch options * @param {any} [opts] - fetch options
* @return {Promise<Readable>} * @return {Promise<Readable>}
* @throws {RequestFailedError} if the response has a failure status code * @throws {RequestFailedError} if the response has a failure status code
*/ */
@ -67,7 +67,7 @@ async function fetchStreamWithResponse(url, opts = {}) {
* Make a request and discard the response. * Make a request and discard the response.
* *
* @param {string | URL} url - request URL * @param {string | URL} url - request URL
* @param {object} opts - fetch options * @param {any} [opts] - fetch options
* @return {Promise<Response>} * @return {Promise<Response>}
* @throws {RequestFailedError} if the response has a failure status code * @throws {RequestFailedError} if the response has a failure status code
*/ */
@ -86,7 +86,7 @@ async function fetchNothing(url, opts = {}) {
* Make a request and extract the redirect from the response. * Make a request and extract the redirect from the response.
* *
* @param {string | URL} url - request URL * @param {string | URL} url - request URL
* @param {object} opts - fetch options * @param {any} [opts] - fetch options
* @return {Promise<string>} * @return {Promise<string>}
* @throws {RequestFailedError} if the response has a non redirect status code or missing Location header * @throws {RequestFailedError} if the response has a non redirect status code or missing Location header
*/ */
@ -115,7 +115,7 @@ async function fetchRedirect(url, opts = {}) {
* Make a request and return a string. * Make a request and return a string.
* *
* @param {string | URL} url - request URL * @param {string | URL} url - request URL
* @param {object} opts - fetch options * @param {any} [opts] - fetch options
* @return {Promise<string>} * @return {Promise<string>}
* @throws {RequestFailedError} if the response has a failure status code * @throws {RequestFailedError} if the response has a failure status code
*/ */

View file

@ -90,6 +90,13 @@ async function deleteMessage(projectId, threadId, messageId) {
) )
} }
async function getResolvedThreadIds(projectId) {
const body = await fetchJson(
chatApiUrl(`/project/${projectId}/resolved-thread-ids`)
)
return body.resolvedThreadIds
}
function chatApiUrl(path) { function chatApiUrl(path) {
return new URL(path, settings.apis.chat.internal_url) return new URL(path, settings.apis.chat.internal_url)
} }
@ -105,6 +112,7 @@ module.exports = {
deleteThread: callbackify(deleteThread), deleteThread: callbackify(deleteThread),
editMessage: callbackify(editMessage), editMessage: callbackify(editMessage),
deleteMessage: callbackify(deleteMessage), deleteMessage: callbackify(deleteMessage),
getResolvedThreadIds: callbackify(getResolvedThreadIds),
promises: { promises: {
getThreads, getThreads,
destroyProject, destroyProject,
@ -116,5 +124,6 @@ module.exports = {
deleteThread, deleteThread,
editMessage, editMessage,
deleteMessage, deleteMessage,
getResolvedThreadIds,
}, },
} }

View file

@ -1,3 +1,4 @@
const ChatApiHandler = require('../Chat/ChatApiHandler')
const ProjectGetter = require('../Project/ProjectGetter') const ProjectGetter = require('../Project/ProjectGetter')
const ProjectLocator = require('../Project/ProjectLocator') const ProjectLocator = require('../Project/ProjectLocator')
const ProjectEntityHandler = require('../Project/ProjectEntityHandler') const ProjectEntityHandler = require('../Project/ProjectEntityHandler')
@ -31,6 +32,17 @@ async function getDocument(req, res) {
{ peek } { peek }
) )
const resolvedCommentIdsInProject =
await ChatApiHandler.promises.getResolvedThreadIds(projectId)
const commentIdsInDoc = new Set(
ranges?.comments?.map(comment => comment.id) ?? []
)
const resolvedCommentIds = resolvedCommentIdsInProject.filter(commentId =>
commentIdsInDoc.has(commentId)
)
if (plain) { if (plain) {
plainTextResponse(res, lines.join('\n')) plainTextResponse(res, lines.join('\n'))
} else { } else {
@ -53,6 +65,7 @@ async function getDocument(req, res) {
projectHistoryId, projectHistoryId,
projectHistoryType, projectHistoryType,
historyRangesSupport, historyRangesSupport,
resolvedCommentIds,
}) })
} }
} }

View file

@ -15,7 +15,26 @@ describe('DocumentController', function () {
this.doc = { _id: 'doc-id-123' } this.doc = { _id: 'doc-id-123' }
this.doc_lines = ['one', 'two', 'three'] this.doc_lines = ['one', 'two', 'three']
this.version = 42 this.version = 42
this.ranges = { mock: 'ranges' } this.ranges = {
comments: [
{
id: 'comment1',
op: {
c: 'foo',
p: 123,
t: 'comment1',
},
},
{
id: 'comment2',
op: {
c: 'bar',
p: 456,
t: 'comment2',
},
},
],
}
this.pathname = '/a/b/c/file.tex' this.pathname = '/a/b/c/file.tex'
this.lastUpdatedAt = new Date().getTime() this.lastUpdatedAt = new Date().getTime()
this.lastUpdatedBy = 'fake-last-updater-id' this.lastUpdatedBy = 'fake-last-updater-id'
@ -29,6 +48,10 @@ describe('DocumentController', function () {
}, },
}, },
} }
this.resolvedThreadIds = [
'comment2',
'comment4', // Comment in project but not in doc
]
this.ProjectGetter = { this.ProjectGetter = {
promises: { promises: {
@ -58,6 +81,12 @@ describe('DocumentController', function () {
}, },
} }
this.ChatApiHandler = {
promises: {
getResolvedThreadIds: sinon.stub().resolves(this.resolvedThreadIds),
},
}
this.DocumentController = SandboxedModule.require(MODULE_PATH, { this.DocumentController = SandboxedModule.require(MODULE_PATH, {
requires: { requires: {
'../Project/ProjectGetter': this.ProjectGetter, '../Project/ProjectGetter': this.ProjectGetter,
@ -65,6 +94,7 @@ describe('DocumentController', function () {
'../Project/ProjectEntityHandler': this.ProjectEntityHandler, '../Project/ProjectEntityHandler': this.ProjectEntityHandler,
'../Project/ProjectEntityUpdateHandler': '../Project/ProjectEntityUpdateHandler':
this.ProjectEntityUpdateHandler, this.ProjectEntityUpdateHandler,
'../Chat/ChatApiHandler': this.ChatApiHandler,
}, },
}) })
}) })
@ -87,17 +117,16 @@ describe('DocumentController', function () {
it('should return the history id and display setting to the client as JSON', function () { it('should return the history id and display setting to the client as JSON', function () {
this.res.type.should.equal('application/json') this.res.type.should.equal('application/json')
this.res.body.should.equal( JSON.parse(this.res.body).should.deep.equal({
JSON.stringify({ lines: this.doc_lines,
lines: this.doc_lines, version: this.version,
version: this.version, ranges: this.ranges,
ranges: this.ranges, pathname: this.pathname,
pathname: this.pathname, projectHistoryId: this.project.overleaf.history.id,
projectHistoryId: this.project.overleaf.history.id, projectHistoryType: 'project-history',
projectHistoryType: 'project-history', resolvedCommentIds: ['comment2'],
historyRangesSupport: false, historyRangesSupport: false,
}) })
)
}) })
}) })