[document-updater] add track-changes support to setDoc (#35820)

GitOrigin-RevId: 5d39dd1945e710ec3f7068830fb2476e59f0f2a5
This commit is contained in:
Jakob Ackermann
2026-09-17 11:39:12 +00:00
committed by Copybot
parent a77a4782cc
commit 00a2765d01
14 changed files with 901 additions and 34 deletions
@@ -114,7 +114,14 @@ async function getDocumentWithHistoryRanges(projectId, docId) {
return doc
}
async function setDocument(projectId, docId, userId, docLines, source) {
async function setDocument(
projectId,
docId,
userId,
docLines,
source,
trackChanges = false
) {
const maybeJson = await fetchString(
`${BASE_URL}/project/${projectId}/doc/${docId}`,
{
@@ -123,6 +130,7 @@ async function setDocument(projectId, docId, userId, docLines, source) {
lines: docLines,
source,
user_id: userId,
trackChanges,
},
signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS),
}
@@ -138,7 +146,14 @@ async function setDocument(projectId, docId, userId, docLines, source) {
}
}
async function appendToDocument(projectId, docId, userId, lines, source) {
async function appendToDocument(
projectId,
docId,
userId,
lines,
source,
trackChanges = false
) {
const maybeJson = await fetchString(
`${BASE_URL}/project/${projectId}/doc/${docId}/append`,
{
@@ -147,6 +162,7 @@ async function appendToDocument(projectId, docId, userId, lines, source) {
lines,
source,
user_id: userId,
trackChanges,
},
signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS),
}
@@ -13,6 +13,7 @@ import LockManager from '../../infrastructure/LockManager.mjs'
import { Project } from '../../models/Project.mjs'
import ProjectEntityHandler from './ProjectEntityHandler.mjs'
import ProjectGetter from './ProjectGetter.mjs'
import ProjectHelper from './ProjectHelper.mjs'
import ProjectLocator from './ProjectLocator.mjs'
import ProjectOptionsHandler from './ProjectOptionsHandler.mjs'
import ProjectUpdateHandler from './ProjectUpdateHandler.mjs'
@@ -388,11 +389,19 @@ const upsertDoc = wrapWithLock(
if (!SafePath.isCleanFilename(docName)) {
throw new Errors.InvalidNameError('invalid element name')
}
const project = await ProjectGetter.promises.getProject(projectId, {
rootFolder: true,
rootDoc_id: true,
track_changes: true,
})
if (project == null) {
throw new Errors.NotFoundError('project not found')
}
let element, folderPath
try {
;({ element, path: folderPath } =
await ProjectLocator.promises.findElement({
project_id: projectId,
project,
element_id: folderId,
type: 'folder',
}))
@@ -421,7 +430,7 @@ const upsertDoc = wrapWithLock(
)
doc.rev = rev
const project =
const updatedProject =
await ProjectEntityMongoUpdateHandler.promises.replaceFileWithDoc(
projectId,
existingFile._id,
@@ -433,15 +442,15 @@ const upsertDoc = wrapWithLock(
projectId,
docId: doc._id,
path: filePath,
projectName: project.name,
projectName: updatedProject.name,
rev: existingFile.rev + 1,
folderId,
})
const projectHistoryId =
project.overleaf &&
project.overleaf.history &&
project.overleaf.history.id
updatedProject.overleaf &&
updatedProject.overleaf.history &&
updatedProject.overleaf.history.id
const newDocs = [
{
doc,
@@ -459,7 +468,7 @@ const upsertDoc = wrapWithLock(
projectId,
projectHistoryId,
userId,
{ oldFiles, newDocs, newProject: project },
{ oldFiles, newDocs, newProject: updatedProject },
source
)
@@ -471,12 +480,17 @@ const upsertDoc = wrapWithLock(
)
return { doc, isNew: true }
} else if (existingDoc) {
const trackChanges = ProjectHelper.isTrackChangesEnabledForUser(
project.track_changes,
userId
)
const result = await DocumentUpdaterHandler.promises.setDocument(
projectId,
existingDoc._id,
userId,
docLines,
source
source,
trackChanges
)
logger.debug(
{ projectId, docId: existingDoc._id },
@@ -509,18 +523,31 @@ const upsertDoc = wrapWithLock(
const appendToDoc = wrapWithLock(
async (projectId, docId, lines, source, userId) => {
const project = await ProjectGetter.promises.getProject(projectId, {
rootFolder: true,
rootDoc_id: true,
track_changes: true,
})
if (project == null) {
throw new Errors.NotFoundError('project not found')
}
const { element } = await ProjectLocator.promises.findElement({
project_id: projectId,
project,
element_id: docId,
type: 'doc',
})
const trackChanges = ProjectHelper.isTrackChangesEnabledForUser(
project.track_changes,
userId
)
return await DocumentUpdaterHandler.promises.appendToDocument(
projectId,
element._id,
userId,
lines,
source
source,
trackChanges
)
}
)
@@ -23,6 +23,7 @@ export default {
isArchived,
isTrashed,
isArchivedOrTrashed,
isTrackChangesEnabledForUser,
getAllowedImagesForUser,
ensureNameIsUnique,
}
@@ -62,6 +63,22 @@ function isArchivedOrTrashed(project, userId) {
return isArchived(project, userId) || isTrashed(project, userId)
}
/**
* Whether track changes is enabled for the given user, from the project's
* track_changes state: `true` means on for everyone (legacy format),
* otherwise it is an object keyed by user id.
*
* @param {boolean | Record<string, boolean> | undefined} trackChangesState
* @param {string | ObjectId | null} userId
* @returns {boolean}
*/
function isTrackChangesEnabledForUser(trackChangesState, userId) {
return (
trackChangesState === true ||
(userId != null && trackChangesState?.[userId.toString()] === true)
)
}
/**
* @param {string[]} nameList
* @param {string} name
@@ -1,12 +1,20 @@
import { expect } from 'chai'
import sinon from 'sinon'
import { Project } from '../../../app/src/models/Project.mjs'
import ProjectGetter from '../../../app/src/Features/Project/ProjectGetter.mjs'
import ProjectOptionsHandler from '../../../app/src/Features/Project/ProjectOptionsHandler.mjs'
import ProjectRootDocManager from '../../../app/src/Features/Project/ProjectRootDocManager.mjs'
import MockDocUpdaterApiClass from './mocks/MockDocUpdaterApi.mjs'
import request from './helpers/request.js'
import User from './helpers/User.mjs'
import { db, ObjectId } from '../../../app/src/infrastructure/mongodb.mjs'
let MockDocUpdaterApi
before(function () {
MockDocUpdaterApi = MockDocUpdaterApiClass.instance()
})
describe('TpdsUpdateTests', function () {
beforeEach(function () {
// Creating a project via TPDS schedules a root doc reset on a 30s timer,
@@ -78,6 +86,170 @@ describe('TpdsUpdateTests', function () {
})
})
describe('updating an existing file as a user', function () {
function updateMainTex(userId, done) {
request(
{
method: 'POST',
url: `/user/${userId}/update/test-project/main.tex`,
auth: {
username: 'overleaf',
password: 'password',
sendImmediately: true,
},
body: 'test one two',
},
(error, response, body) => {
if (error) {
throw error
}
expect(response.statusCode).to.equal(200)
expect(JSON.parse(response.body).status).to.equal('applied')
done()
}
)
}
describe('with track changes disabled', function () {
beforeEach(function (done) {
updateMainTex(this.owner._id, done)
})
it('should send the update to document-updater without track changes', function () {
const requests = MockDocUpdaterApi.getReceivedSetDocRequests(
this.projectId
)
expect(requests).to.have.length(1)
expect(requests[0].body.lines).to.deep.equal(['test one two'])
expect(requests[0].body.user_id).to.equal(this.owner._id.toString())
expect(requests[0].body.trackChanges).to.equal(false)
})
})
describe('with track changes enabled for the user', function () {
beforeEach(function (done) {
Project.updateOne(
{ _id: this.projectId },
{ track_changes: { [this.owner._id.toString()]: true } }
)
.then(() => updateMainTex(this.owner._id, done))
.catch(done)
})
it('should ask document-updater to record the update as tracked changes', function () {
const requests = MockDocUpdaterApi.getReceivedSetDocRequests(
this.projectId
)
expect(requests).to.have.length(1)
expect(requests[0].body.user_id).to.equal(this.owner._id.toString())
expect(requests[0].body.trackChanges).to.equal(true)
})
})
describe('with track changes enabled for another user', function () {
beforeEach(function (done) {
Project.updateOne(
{ _id: this.projectId },
{ track_changes: { '5c41deb2b4ca500153340809': true } }
)
.then(() => updateMainTex(this.owner._id, done))
.catch(done)
})
it('should not ask document-updater to record the update as tracked changes', function () {
const requests = MockDocUpdaterApi.getReceivedSetDocRequests(
this.projectId
)
expect(requests).to.have.length(1)
expect(requests[0].body.trackChanges).to.equal(false)
})
})
})
describe('updating an existing file as a user by project id', function () {
beforeEach(function (done) {
Project.updateOne(
{ _id: this.projectId },
{ track_changes: { [this.owner._id.toString()]: true } }
)
.then(() => {
request(
{
method: 'POST',
url: `/project/${this.projectId}/user/${this.owner._id}/update/main.tex`,
auth: {
username: 'overleaf',
password: 'password',
sendImmediately: true,
},
body: 'test one two',
},
(error, response, body) => {
if (error) {
throw error
}
expect(response.statusCode).to.equal(200)
expect(JSON.parse(response.body).status).to.equal('applied')
done()
}
)
})
.catch(done)
})
it('should ask document-updater to record the update as tracked changes', function () {
const requests = MockDocUpdaterApi.getReceivedSetDocRequests(
this.projectId
)
expect(requests).to.have.length(1)
expect(requests[0].body.user_id).to.equal(this.owner._id.toString())
expect(requests[0].body.trackChanges).to.equal(true)
})
})
describe('updating an existing file via the project contents route', function () {
// Updates via the project contents route (used by github-sync) carry no
// user id: the synced commits can come from any GitHub user, so they are
// never recorded as tracked changes.
beforeEach(function (done) {
Project.updateOne(
{ _id: this.projectId },
{ track_changes: { [this.owner._id.toString()]: true } }
)
.then(() => {
request(
{
method: 'POST',
url: `/project/${this.projectId}/contents/main.tex`,
auth: {
username: 'overleaf',
password: 'password',
sendImmediately: true,
},
body: 'test one two',
},
(error, response, body) => {
if (error) {
throw error
}
expect(response.statusCode).to.equal(200)
done()
}
)
})
.catch(done)
})
it('should send the update to document-updater without track changes', function () {
const requests = MockDocUpdaterApi.getReceivedSetDocRequests(
this.projectId
)
expect(requests).to.have.length(1)
expect(requests[0].body.user_id).to.equal(null)
expect(requests[0].body.trackChanges).to.equal(false)
})
})
describe('deleting a file', function () {
beforeEach(function (done) {
request(
@@ -4,6 +4,13 @@ class MockDocUpdaterApi extends AbstractMockApi {
reset() {
this.updates = {}
this.docsByProject = new Map()
this.receivedSetDocRequests = []
}
getReceivedSetDocRequests(projectId) {
return this.receivedSetDocRequests.filter(
request => request.projectId === projectId
)
}
getProjectStructureUpdates(projectId) {
@@ -63,6 +70,8 @@ class MockDocUpdaterApi extends AbstractMockApi {
)
this.app.post('/project/:projectId/doc/:doc_id', (req, res) => {
const { projectId, doc_id: docId } = req.params
this.receivedSetDocRequests.push({ projectId, docId, body: req.body })
res.sendStatus(204)
})
@@ -282,6 +282,7 @@ describe('DocumentUpdaterHandler', function () {
lines: ctx.lines,
source: ctx.source,
user_id: ctx.user_id,
trackChanges: false,
})
.reply(204)
})
@@ -298,6 +299,31 @@ describe('DocumentUpdaterHandler', function () {
})
})
describe('with track changes enabled', function () {
beforeEach(function (ctx) {
ctx.docUpdaterMock
.post(`/project/${ctx.project_id}/doc/${ctx.doc_id}`, {
lines: ctx.lines,
source: ctx.source,
user_id: ctx.user_id,
trackChanges: true,
})
.reply(204)
})
it('should forward the trackChanges flag', async function (ctx) {
await ctx.handler.promises.setDocument(
ctx.project_id,
ctx.doc_id,
ctx.user_id,
ctx.lines,
ctx.source,
true
)
expect(ctx.docUpdaterMock.isDone()).to.be.true
})
})
describe('when the document updater API returns an error', function () {
beforeEach(function (ctx) {
ctx.docUpdaterMock
@@ -305,6 +331,7 @@ describe('DocumentUpdaterHandler', function () {
lines: ctx.lines,
source: ctx.source,
user_id: ctx.user_id,
trackChanges: false,
})
.replyWithError('boom')
})
@@ -329,6 +356,7 @@ describe('DocumentUpdaterHandler', function () {
lines: ctx.lines,
source: ctx.source,
user_id: ctx.user_id,
trackChanges: false,
})
.reply(500)
})
@@ -1603,6 +1631,7 @@ describe('DocumentUpdaterHandler', function () {
lines: ctx.lines,
source: ctx.source,
user_id: ctx.user_id,
trackChanges: false,
})
.reply(200)
})
@@ -1619,6 +1648,31 @@ describe('DocumentUpdaterHandler', function () {
})
})
describe('with track changes enabled', function () {
beforeEach(function (ctx) {
ctx.docUpdaterMock
.post(`/project/${ctx.project_id}/doc/${ctx.doc_id}/append`, {
lines: ctx.lines,
source: ctx.source,
user_id: ctx.user_id,
trackChanges: true,
})
.reply(200)
})
it('should forward the trackChanges flag', async function (ctx) {
await ctx.handler.promises.appendToDocument(
ctx.project_id,
ctx.doc_id,
ctx.user_id,
ctx.lines,
ctx.source,
true
)
expect(ctx.docUpdaterMock.isDone()).to.be.true
})
})
describe('when the document updater API returns an error', function () {
beforeEach(function (ctx) {
ctx.docUpdaterMock
@@ -112,6 +112,9 @@ describe('ProjectEntityUpdateHandler', function () {
getProject: sinon.stub(),
},
}
ctx.ProjectHelper = {
isTrackChangesEnabledForUser: sinon.stub().returns(false),
}
ctx.ProjectLocator = {
promises: {
findElement: sinon.stub(),
@@ -217,6 +220,10 @@ describe('ProjectEntityUpdateHandler', function () {
default: ctx.ProjectGetter,
}))
vi.doMock('../../../../app/src/Features/Project/ProjectHelper', () => ({
default: ctx.ProjectHelper,
}))
vi.doMock('../../../../app/src/Features/Project/ProjectLocator', () => ({
default: ctx.ProjectLocator,
}))
@@ -838,6 +845,10 @@ describe('ProjectEntityUpdateHandler', function () {
})
describe('upsertDoc', function () {
beforeEach(function (ctx) {
ctx.ProjectGetter.promises.getProject.resolves(ctx.project)
})
describe('upserting into an invalid folder', function () {
beforeEach(function (ctx) {
ctx.ProjectLocator.promises.findElement.resolves({ element: null })
@@ -866,6 +877,7 @@ describe('ProjectEntityUpdateHandler', function () {
describe('updating an existing doc', function () {
let upsertDocResponse
beforeEach(async function (ctx) {
ctx.project.track_changes = { 'some-user-id': true }
ctx.existingDoc = { _id: docId, name: ctx.docName }
ctx.existingFile = {
_id: fileId,
@@ -893,16 +905,32 @@ describe('ProjectEntityUpdateHandler', function () {
)
})
it('gets the project with the track changes state', function (ctx) {
ctx.ProjectGetter.promises.getProject
.calledWith(projectId, {
rootFolder: true,
rootDoc_id: true,
track_changes: true,
})
.should.equal(true)
})
it('tries to find the folder', function (ctx) {
ctx.ProjectLocator.promises.findElement
.calledWith({
project_id: projectId,
project: ctx.project,
element_id: folderId,
type: 'folder',
})
.should.equal(true)
})
it('resolves the track changes state for the user', function (ctx) {
ctx.ProjectHelper.isTrackChangesEnabledForUser
.calledWith(ctx.project.track_changes, userId)
.should.equal(true)
})
it('updates the doc contents', function (ctx) {
ctx.DocumentUpdaterHandler.promises.setDocument
.calledWith(
@@ -910,7 +938,8 @@ describe('ProjectEntityUpdateHandler', function () {
ctx.existingDoc._id,
userId,
ctx.docLines,
ctx.source
ctx.source,
false
)
.should.equal(true)
})
@@ -921,6 +950,44 @@ describe('ProjectEntityUpdateHandler', function () {
})
})
describe('updating an existing doc with track changes enabled for the user', function () {
beforeEach(async function (ctx) {
ctx.ProjectHelper.isTrackChangesEnabledForUser.returns(true)
ctx.existingDoc = { _id: docId, name: ctx.docName }
ctx.folder = {
_id: folderId,
docs: [ctx.existingDoc],
fileRefs: [],
}
ctx.ProjectLocator.promises.findElement.resolves({
element: ctx.folder,
})
ctx.DocumentUpdaterHandler.promises.setDocument.resolves()
await ctx.ProjectEntityUpdateHandler.promises.upsertDoc(
projectId,
folderId,
ctx.docName,
ctx.docLines,
ctx.source,
userId
)
})
it('updates the doc contents with track changes', function (ctx) {
ctx.DocumentUpdaterHandler.promises.setDocument
.calledWith(
projectId,
ctx.existingDoc._id,
userId,
ctx.docLines,
ctx.source,
true
)
.should.equal(true)
})
})
describe('creating a new doc', function () {
let upsertDocResponse
@@ -948,7 +1015,7 @@ describe('ProjectEntityUpdateHandler', function () {
it('tries to find the folder', function (ctx) {
ctx.ProjectLocator.promises.findElement
.calledWith({
project_id: projectId,
project: ctx.project,
element_id: folderId,
type: 'folder',
})
@@ -1019,7 +1086,7 @@ describe('ProjectEntityUpdateHandler', function () {
ctx.filePath = '/path/to/folder/foo.tex'
ctx.ProjectLocator.promises.findElement
.withArgs({
project_id: projectId,
project: ctx.project,
element_id: ctx.folder._id,
type: 'folder',
})
@@ -3165,6 +3232,10 @@ describe('ProjectEntityUpdateHandler', function () {
})
describe('appendToDoc', function () {
beforeEach(function (ctx) {
ctx.ProjectGetter.promises.getProject.resolves(ctx.project)
})
describe('when document cannot be found', function () {
let appendToDocPromise
beforeEach(function (ctx) {
@@ -3172,7 +3243,7 @@ describe('ProjectEntityUpdateHandler', function () {
ctx.DocumentUpdaterHandler.promises.appendToDocument = sinon.stub()
ctx.ProjectLocator.promises.findElement = sinon.stub()
ctx.ProjectLocator.promises.findElement
.withArgs({ project_id: projectId, element_id: docId, type: 'doc' })
.withArgs({ project: ctx.project, element_id: docId, type: 'doc' })
.rejects(new Errors.NotFoundError())
appendToDocPromise =
ctx.ProjectEntityUpdateHandler.promises.appendToDocWithPath(
@@ -3214,6 +3285,7 @@ describe('ProjectEntityUpdateHandler', function () {
describe('when document is found', function () {
let appendToDocResult
beforeEach(async function (ctx) {
ctx.project.track_changes = { 'some-user-id': true }
ctx.appendedLines = ['5678', 'def']
ctx.DocumentUpdaterHandler.promises.appendToDocument = sinon.stub()
ctx.DocumentUpdaterHandler.promises.appendToDocument
@@ -3221,7 +3293,7 @@ describe('ProjectEntityUpdateHandler', function () {
.resolves({ rev: 1 })
ctx.ProjectLocator.promises.findElement = sinon.stub()
ctx.ProjectLocator.promises.findElement
.withArgs({ project_id: projectId, element_id: docId, type: 'doc' })
.withArgs({ project: ctx.project, element_id: docId, type: 'doc' })
.resolves({ element: { _id: docId } })
appendToDocResult =
await ctx.ProjectEntityUpdateHandler.promises.appendToDocWithPath(
@@ -3233,13 +3305,21 @@ describe('ProjectEntityUpdateHandler', function () {
)
})
it('should resolve the track changes state for the user', function (ctx) {
ctx.ProjectHelper.isTrackChangesEnabledForUser.should.have.been.calledWith(
ctx.project.track_changes,
userId
)
})
it('should forward call to DocumentUpdaterHandler.appendToDocument', function (ctx) {
ctx.DocumentUpdaterHandler.promises.appendToDocument.should.have.been.calledWith(
projectId,
docId,
userId,
ctx.appendedLines,
ctx.source
ctx.source,
false
)
})
@@ -3248,6 +3328,38 @@ describe('ProjectEntityUpdateHandler', function () {
})
})
describe('when track changes is enabled for the user', function () {
beforeEach(async function (ctx) {
ctx.ProjectHelper.isTrackChangesEnabledForUser.returns(true)
ctx.appendedLines = ['5678', 'def']
ctx.DocumentUpdaterHandler.promises.appendToDocument = sinon
.stub()
.resolves({ rev: 1 })
ctx.ProjectLocator.promises.findElement = sinon.stub()
ctx.ProjectLocator.promises.findElement
.withArgs({ project: ctx.project, element_id: docId, type: 'doc' })
.resolves({ element: { _id: docId } })
await ctx.ProjectEntityUpdateHandler.promises.appendToDocWithPath(
projectId,
docId,
ctx.appendedLines,
ctx.source,
userId
)
})
it('should forward the track changes flag', function (ctx) {
ctx.DocumentUpdaterHandler.promises.appendToDocument.should.have.been.calledWith(
projectId,
docId,
userId,
ctx.appendedLines,
ctx.source,
true
)
})
})
describe('when DocumentUpdater throws an error', function () {
beforeEach(function (ctx) {
ctx.appendedLines = ['5678', 'def']
@@ -3257,7 +3369,7 @@ describe('ProjectEntityUpdateHandler', function () {
)
ctx.ProjectLocator.promises.findElement = sinon.stub()
ctx.ProjectLocator.promises.findElement
.withArgs({ project_id: projectId, element_id: docId, type: 'doc' })
.withArgs({ project: ctx.project, element_id: docId, type: 'doc' })
.resolves({ element: { _id: docId } })
})
@@ -133,6 +133,67 @@ describe('ProjectHelper', function () {
})
})
describe('isTrackChangesEnabledForUser', function () {
const userId = '588f3ddae8ebc1bac07c9fa4'
it('returns true for the legacy on-for-everyone format', function (ctx) {
expect(
ctx.ProjectHelper.isTrackChangesEnabledForUser(true, userId)
).to.equal(true)
})
it('returns true when enabled for the user', function (ctx) {
expect(
ctx.ProjectHelper.isTrackChangesEnabledForUser(
{ [userId]: true },
userId
)
).to.equal(true)
})
it('supports ObjectId user ids', function (ctx) {
expect(
ctx.ProjectHelper.isTrackChangesEnabledForUser(
{ [userId]: true },
new ObjectId(userId)
)
).to.equal(true)
})
it('returns false when only enabled for another user', function (ctx) {
expect(
ctx.ProjectHelper.isTrackChangesEnabledForUser(
{ '5c41deb2b4ca500153340809': true },
userId
)
).to.equal(false)
})
it('returns false when only enabled for guests', function (ctx) {
expect(
ctx.ProjectHelper.isTrackChangesEnabledForUser(
{ __guests__: true },
userId
)
).to.equal(false)
})
it('returns false when disabled', function (ctx) {
expect(
ctx.ProjectHelper.isTrackChangesEnabledForUser(false, userId)
).to.equal(false)
expect(
ctx.ProjectHelper.isTrackChangesEnabledForUser(undefined, userId)
).to.equal(false)
})
it('returns false without a user id', function (ctx) {
expect(
ctx.ProjectHelper.isTrackChangesEnabledForUser({ [userId]: true }, null)
).to.equal(false)
})
})
describe('compilerFromV1Engine', function () {
it('returns the correct engine for latex_dvipdf', function (ctx) {
expect(ctx.ProjectHelper.compilerFromV1Engine('latex_dvipdf')).to.equal(