diff --git a/services/web/app/src/Features/History/HistoryManager.js b/services/web/app/src/Features/History/HistoryManager.js index 83282356b9..9748e82506 100644 --- a/services/web/app/src/Features/History/HistoryManager.js +++ b/services/web/app/src/Features/History/HistoryManager.js @@ -4,7 +4,7 @@ const settings = require('@overleaf/settings') const OError = require('@overleaf/o-error') const UserGetter = require('../User/UserGetter') -async function initializeProject() { +async function initializeProject(projectId) { if ( !( settings.apis.project_history && @@ -16,8 +16,10 @@ async function initializeProject() { const response = await fetch(`${settings.apis.project_history.url}/project`, { method: 'POST', headers: { + 'Content-Type': 'application/json', Accept: 'application/json', }, + body: JSON.stringify({ historyId: projectId.toString() }), }) if (!response.ok) { throw new OError('failed to initialize project history', { diff --git a/services/web/app/src/Features/Project/ProjectCreationHandler.js b/services/web/app/src/Features/Project/ProjectCreationHandler.js index 07cfaea0d4..47935ca1f8 100644 --- a/services/web/app/src/Features/Project/ProjectCreationHandler.js +++ b/services/web/app/src/Features/Project/ProjectCreationHandler.js @@ -152,33 +152,32 @@ async function _createBlankProject( const timer = new metrics.Timer('project-creation') await ProjectDetailsHandler.promises.validateProjectName(projectName) - if (!attributes.overleaf) { - const historyId = await HistoryManager.promises.initializeProject() - if (historyId != null) { - attributes.overleaf = { - history: { id: historyId }, - } - } - } - const rootFolder = new Folder({ name: 'rootFolder' }) attributes.lastUpdatedBy = attributes.owner_ref = new ObjectId(ownerId) attributes.name = projectName const project = new Project(attributes) - Object.assign(project, attributes) + if (project.overleaf.history.id == null) { + const historyId = await HistoryManager.promises.initializeProject( + project._id + ) + if (historyId != null) { + project.overleaf.history.id = historyId + } + } // only display full project history when the project has the overleaf history id attribute // (to allow scripted creation of projects without full project history) - const historyId = _.get(attributes, ['overleaf', 'history', 'id']) + const historyId = project.overleaf.history.id if ( Features.hasFeature('history-v1') && Settings.apis.project_history.displayHistoryForNewProjects && - historyId + historyId != null ) { project.overleaf.history.display = true } + if (Settings.currentImageName) { // avoid clobbering any imageName already set in attributes (e.g. importedImageName) if (!project.imageName) { diff --git a/services/web/app/src/Features/Project/ProjectHistoryHandler.js b/services/web/app/src/Features/Project/ProjectHistoryHandler.js index fcea9eaf73..3c16a2eb11 100644 --- a/services/web/app/src/Features/Project/ProjectHistoryHandler.js +++ b/services/web/app/src/Features/Project/ProjectHistoryHandler.js @@ -26,8 +26,8 @@ const ProjectHistoryHandler = { if (callback == null) { callback = function () {} } - if (!history_id || typeof history_id !== 'number') { - return callback(new Error('invalid history id')) + if (history_id == null) { + return callback(new Error('missing history id')) } // use $exists:false to prevent overwriting any existing history id, atomically return Project.updateOne( @@ -154,32 +154,35 @@ const ProjectHistoryHandler = { if (history_id != null) { return callback() } // history already exists, success - return HistoryManager.initializeProject(function (err, historyId) { - if (err != null) { - return callback(err) - } - if (historyId == null) { - return callback(new Error('failed to initialize history id')) - } - return ProjectHistoryHandler.setHistoryId( - project_id, - historyId, - function (err) { - if (err != null) { - return callback(err) - } - return ProjectEntityUpdateHandler.resyncProjectHistory( - project_id, - function (err) { - if (err != null) { - return callback(err) - } - return HistoryManager.flushProject(project_id, callback) - } - ) + return HistoryManager.initializeProject( + project_id, + function (err, historyId) { + if (err != null) { + return callback(err) } - ) - }) + if (historyId == null) { + return callback(new Error('failed to initialize history id')) + } + return ProjectHistoryHandler.setHistoryId( + project_id, + historyId, + function (err) { + if (err != null) { + return callback(err) + } + return ProjectEntityUpdateHandler.resyncProjectHistory( + project_id, + function (err) { + if (err != null) { + return callback(err) + } + return HistoryManager.flushProject(project_id, callback) + } + ) + } + ) + } + ) } ) }, diff --git a/services/web/app/src/models/Project.js b/services/web/app/src/models/Project.js index b8160fc32e..55bda02268 100644 --- a/services/web/app/src/models/Project.js +++ b/services/web/app/src/models/Project.js @@ -88,7 +88,7 @@ const ProjectSchema = new Schema({ token: { type: String }, read_token: { type: String }, history: { - id: { type: Number }, + id: { type: Schema.Types.Mixed }, display: { type: Boolean }, upgradedAt: { type: Date }, allowDowngrade: { type: Boolean }, diff --git a/services/web/scripts/history/HistoryUpgradeHelper.js b/services/web/scripts/history/HistoryUpgradeHelper.js index 30d48d93e9..4fb48364cb 100644 --- a/services/web/scripts/history/HistoryUpgradeHelper.js +++ b/services/web/scripts/history/HistoryUpgradeHelper.js @@ -176,7 +176,7 @@ async function doUpgradeForNoneWithoutConversion(project) { // of a resync request to doc-updater let historyId = await ProjectHistoryHandler.promises.getHistoryId(projectId) if (historyId == null) { - historyId = await HistoryManager.promises.initializeProject() + historyId = await HistoryManager.promises.initializeProject(projectId) if (historyId != null) { await ProjectHistoryHandler.promises.setHistoryId(projectId, historyId) } diff --git a/services/web/scripts/history/upgrade_none_without_conversion_if_no_sl_history.js b/services/web/scripts/history/upgrade_none_without_conversion_if_no_sl_history.js index 2567adb1a5..6a6e5eb212 100644 --- a/services/web/scripts/history/upgrade_none_without_conversion_if_no_sl_history.js +++ b/services/web/scripts/history/upgrade_none_without_conversion_if_no_sl_history.js @@ -121,7 +121,7 @@ async function doUpgradeForNoneWithoutConversion(project) { projectId ) if (historyId == null) { - historyId = await HistoryManager.promises.initializeProject() + historyId = await HistoryManager.promises.initializeProject(projectId) if (historyId != null) { await ProjectHistoryHandler.promises.setHistoryId( projectId, diff --git a/services/web/test/unit/src/History/HistoryManagerTests.js b/services/web/test/unit/src/History/HistoryManagerTests.js index 0e4f81956a..91d6180db8 100644 --- a/services/web/test/unit/src/History/HistoryManagerTests.js +++ b/services/web/test/unit/src/History/HistoryManagerTests.js @@ -8,6 +8,7 @@ const MODULE_PATH = '../../../../app/src/Features/History/HistoryManager' describe('HistoryManager', function () { beforeEach(function () { this.user_id = 'user-id-123' + this.historyId = ObjectId().toString() this.AuthenticationController = { getLoggedInUserId: sinon.stub().returns(this.user_id), } @@ -61,9 +62,10 @@ describe('HistoryManager', function () { describe('project history returns a successful response', function () { beforeEach(async function () { - this.overleaf_id = 1234 - this.response.json.resolves({ project: { id: this.overleaf_id } }) - this.result = await this.HistoryManager.promises.initializeProject() + this.response.json.resolves({ project: { id: this.historyId } }) + this.result = await this.HistoryManager.promises.initializeProject( + this.historyId + ) }) it('should call the project history api', function () { @@ -74,23 +76,25 @@ describe('HistoryManager', function () { }) it('should return the overleaf id', function () { - expect(this.result).to.deep.equal(this.overleaf_id) + expect(this.result).to.equal(this.historyId) }) }) describe('project history returns a response without the project id', function () { it('should throw an error', async function () { this.response.json.resolves({ project: {} }) - await expect(this.HistoryManager.promises.initializeProject()).to.be - .rejected + await expect( + this.HistoryManager.promises.initializeProject(this.historyId) + ).to.be.rejected }) }) describe('project history errors', function () { it('should propagate the error', async function () { this.fetch.rejects(new Error('problem connecting')) - await expect(this.HistoryManager.promises.initializeProject()).to.be - .rejected + await expect( + this.HistoryManager.promises.initializeProject(this.historyId) + ).to.be.rejected }) }) }) @@ -98,8 +102,9 @@ describe('HistoryManager', function () { describe('with project history disabled', function () { it('should return without errors', async function () { this.settings.apis.project_history.initializeHistoryForNewProjects = false - await expect(this.HistoryManager.promises.initializeProject()).to.be - .fulfilled + await expect( + this.HistoryManager.promises.initializeProject(this.historyId) + ).to.be.fulfilled }) }) }) diff --git a/services/web/test/unit/src/Project/ProjectHistoryHandlerTests.js b/services/web/test/unit/src/Project/ProjectHistoryHandlerTests.js index 56e4f44919..e594aa1d84 100644 --- a/services/web/test/unit/src/Project/ProjectHistoryHandlerTests.js +++ b/services/web/test/unit/src/Project/ProjectHistoryHandlerTests.js @@ -39,6 +39,7 @@ describe('ProjectHistoryHandler', function () { return Project })() this.project = new this.ProjectModel() + this.historyId = this.project._id.toString() this.callback = sinon.stub() @@ -57,10 +58,9 @@ describe('ProjectHistoryHandler', function () { describe('starting history for an existing project', function () { beforeEach(function () { - this.newHistoryId = 123456789 this.HistoryManager.initializeProject = sinon .stub() - .callsArgWith(0, null, this.newHistoryId) + .yields(null, this.historyId) this.HistoryManager.flushProject = sinon.stub().callsArg(1) return (this.ProjectEntityUpdateHandler.resyncProjectHistory = sinon .stub() @@ -96,7 +96,7 @@ describe('ProjectHistoryHandler', function () { return this.ProjectModel.updateOne .calledWith( { _id: project_id, 'overleaf.history.id': { $exists: false } }, - { 'overleaf.history.id': this.newHistoryId } + { 'overleaf.history.id': this.historyId } ) .should.equal(true) })