[ranges-tracker] add legacy orderedRejections field in tracked-changes (#36565)

* [docstore] use more realistic test stubs for ranges

* [monorepo] define ranges schemas in @overleaf/ranges-tracker

* [ranges-tracker] add legacy orderedRejections field in tracked-changes

* [monorepo] resolve @overleaf/ranges-tracker subpath imports in typescript

* [real-time] validate comment thread ids and tracked change id seeds

* [ranges-tracker] require ObjectId ids and tracked change metadata in ranges

* [monorepo] require ObjectId tracked change ids on the accept routes

* [web] use ObjectId comment thread ids in the DocumentController tests

GitOrigin-RevId: 58b20dec311d85b917ef63f9efbe745a265a2de6

services/docstore/app/js/HttpController.js

@@ -6,7 +6,7 @@ import Errors from './Errors.js'
 import Settings from '@overleaf/settings'
 import { expressify } from '@overleaf/promise-utils'
 import { parseReq, z, zz } from '@overleaf/validation-tools'
-import * as schemas from './schemas.js'
+import rangesSchemas from '@overleaf/ranges-tracker/schemas.js'
 
 const projectParamsSchema = z.object({
   params: z.strictObject({
@@ -73,7 +73,7 @@ const updateDocSchema = z.object({
   body: z.strictObject({
     lines: z.array(z.string()),
     version: z.number(),
-    ranges: schemas.ranges,
+    ranges: rangesSchemas.ranges,
   }),
 })
 

services/docstore/app/js/schemas.js

@@ -1,66 +0,0 @@
-// @ts-check
-import { z } from '@overleaf/validation-tools'
-
-// Zod schemas for the ranges data (RangesTracker format) accepted by
-// updateDoc.
-//
-// Ranges reach docstore via web from several writers and are stored (and
-// later round-tripped, e.g. by project clone) without normalization, so the
-// schemas model wire reality rather than the ideal shape:
-// - document-updater flushes RangesTracker data: change/comment ids are
-//   RangesTracker ids (seed + increment) or thread ids, i.e. strings that are
-//   not always ObjectIds (and old fixtures/documents carry arbitrary string
-//   thread ids), see libraries/ranges-tracker/index.cjs
-// - history restores send id-less changes and comment ops with a `resolved`
-//   flag (stripped again by RangeManager), and detached comments without an
-//   id, see overleaf-editor-core/lib/doc_updater_compatible_ranges.js
-// - old tracked changes created while the fixedRemoveChange flag existed
-//   (removed in https://github.com/overleaf/internal/pull/23993) still carry
-//   it in their op until accepted/rejected
-
-const insertOp = z.strictObject({
-  i: z.string(),
-  p: z.number().int().min(0),
-  u: z.boolean().optional(),
-  fixedRemoveChange: z.boolean().optional(),
-})
-
-const deleteOp = z.strictObject({
-  d: z.string(),
-  p: z.number().int().min(0),
-  u: z.boolean().optional(),
-  fixedRemoveChange: z.boolean().optional(),
-})
-
-const commentOp = z.strictObject({
-  c: z.string().optional(),
-  p: z.number().int().min(0).optional(),
-  // comment thread ids are usually ObjectIds, but legacy documents carry
-  // arbitrary strings
-  t: z.string().optional(),
-  u: z.boolean().optional(),
-  // sent by history restores; removed again by RangeManager
-  resolved: z.boolean().optional(),
-})
-
-const rangeMetadata = z.strictObject({
-  user_id: z.string().optional(),
-  ts: z.string().optional(),
-})
-
-const comment = z.strictObject({
-  id: z.string().optional(),
-  op: commentOp,
-  metadata: rangeMetadata.optional(),
-})
-
-const trackedChange = z.strictObject({
-  id: z.string().optional(),
-  op: insertOp.or(deleteOp).optional(),
-  metadata: rangeMetadata.optional(),
-})
-
-export const ranges = z.strictObject({
-  comments: z.array(comment).optional(),
-  changes: z.array(trackedChange).optional(),
-})

services/docstore/package.json

@@ -23,6 +23,7 @@
     "@overleaf/o-error": "workspace:*",
     "@overleaf/object-persistor": "workspace:*",
     "@overleaf/promise-utils": "workspace:*",
+    "@overleaf/ranges-tracker": "workspace:*",
     "@overleaf/settings": "workspace:*",
     "@overleaf/stream-utils": "^0.1.0",
     "@overleaf/validation-tools": "workspace:*",

services/docstore/test/acceptance/js/GettingAllDocsTests.js

@@ -17,12 +17,23 @@ describe('Getting all docs', function () {
         lines: ['one', 'two', 'three'],
         ranges: {
           comments: [
-            { id: new ObjectId().toString(), op: { t: this.threadId1 } },
+            {
+              id: new ObjectId().toString(),
+              op: {
+                c: 'c',
+                p: 0,
+                t: this.threadId1,
+              },
+            },
           ],
           changes: [
             {
               id: new ObjectId().toString(),
-              metadata: { user_id: 'user-id-1' },
+              op: { d: 'd', p: 1 },
+              metadata: {
+                user_id: 'user-id-1',
+                ts: new Date().toJSON(),
+              },
             },
           ],
         },
@@ -35,7 +46,11 @@ describe('Getting all docs', function () {
           changes: [
             {
               id: new ObjectId().toString(),
-              metadata: { user_id: 'user-id-2' },
+              op: { i: 'd', p: 1 },
+              metadata: {
+                user_id: 'user-id-2',
+                ts: new Date().toJSON(),
+              },
             },
           ],
         },
@@ -46,12 +61,19 @@ describe('Getting all docs', function () {
         lines: ['111', '222', '333'],
         ranges: {
           comments: [
-            { id: new ObjectId().toString(), op: { t: this.threadId2 } },
+            {
+              id: new ObjectId().toString(),
+              op: { c: 'c', p: 1, t: this.threadId2 },
+            },
           ],
           changes: [
             {
               id: new ObjectId().toString(),
-              metadata: { user_id: 'anonymous-user' },
+              op: { d: 'x', p: 1 },
+              metadata: {
+                user_id: 'anonymous-user',
+                ts: new Date().toJSON(),
+              },
             },
           ],
         },
@@ -71,9 +93,21 @@ describe('Getting all docs', function () {
       _id: new ObjectId(),
       lines: ['deleted'],
       ranges: {
-        comments: [{ id: new ObjectId().toString(), op: { t: 'thread-id-3' } }],
+        comments: [
+          {
+            id: new ObjectId().toString(),
+            op: { c: 'c', p: 1, t: new ObjectId().toString() },
+          },
+        ],
         changes: [
-          { id: new ObjectId().toString(), metadata: { user_id: 'user-id-3' } },
+          {
+            id: new ObjectId().toString(),
+            op: { d: 'd', p: 0 },
+            metadata: {
+              user_id: 'user-id-3',
+              ts: new Date().toJSON(),
+            },
+          },
         ],
       },
       rev: 8,

services/docstore/test/unit/js/schemas.test.js

@@ -1,27 +0,0 @@
-import { describe, it, expect } from 'vitest'
-import { ranges } from '../../../app/js/schemas.js'
-
-describe('schemas', function () {
-  describe('ranges', function () {
-    it('accepts a legacy fixedRemoveChange flag on an insert op', function () {
-      const result = ranges.safeParse({
-        changes: [{ op: { i: 'foo', p: 0, fixedRemoveChange: true } }],
-      })
-      expect(result.success).to.equal(true)
-    })
-
-    it('accepts a legacy fixedRemoveChange flag on a delete op', function () {
-      const result = ranges.safeParse({
-        changes: [{ op: { d: 'foo', p: 0, fixedRemoveChange: true } }],
-      })
-      expect(result.success).to.equal(true)
-    })
-
-    it('rejects an unrecognized key on an insert op', function () {
-      const result = ranges.safeParse({
-        changes: [{ op: { i: 'foo', p: 0, somethingElse: true } }],
-      })
-      expect(result.success).to.equal(false)
-    })
-  })
-})