Skip to content

Conversation

@nickbrowne
Copy link
Contributor

Previously, if there were not enough ops to "fill" an entire version, they would not be included in the returned array, now the incomplete version is included in the results.

This does mean each version is no longer guaranteed to return the same result every time, but in practice this shouldn't be an issue with how we use this API.

To test, start up the demo server:

./bin/demoserver

Open the demo app in your browser: http://localhost:8000/

Add enough text to make one full version, and one partial version, with versions grouped every 10 ops, eg:

yo this is dog

curl the versions api, you should now see two versions returned:

curl http://localhost:8000/doc/blag/versions?every=10 | jq
[
  {
    "v": 10,
    "type": "text",
    "snapshot": "yo this is",
    "meta": {
      "source": "2d3f398f8bb6b1a2fba14ff2ae0867b6",
      "ts": 1762920066706
    }
  },
  {
    "v": 14,
    "type": "text",
    "snapshot": "yo this is dog",
    "meta": {
      "source": "2d3f398f8bb6b1a2fba14ff2ae0867b6",
      "ts": 1762920067193
    }
  }
]

Previously, if there were not enough ops to "fill" an entire
version, they would not be included in the returned array, now
the incomplete version is included in the results.

This does mean each version is no longer guaranteed to return
the same result every time, but in practice this shouldn't be
an issue.

buildVersions = (callback, ops, results = []) ->
if ops.length
lastOpV = ops[ops.length - 1].v
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works as described. How hard would it be to add tests for it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit tedious because they are still nodeunit tests, maybe I'll see if a robot can do it

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what Junie came up with

Index: test/model.coffee
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/test/model.coffee b/test/model.coffee
--- a/test/model.coffee	(revision e48f4cfcaccd54498d1cdc241d62ea509d0a101b)
+++ b/test/model.coffee	(date 1762923393660)
@@ -1190,3 +1190,53 @@
             test.deepEqual data.snapshot, "Hello mum"
             test.deepEqual data.v, 3
             test.done()
+
+  'getVersions returns every nth version and includes the latest if not a multiple': (test) ->
+    name = newDocName()
+
+    @model.create name, types.text, (error) =>
+      test.equal error, null
+
+      applyOps @model, name, 0, [
+        [{ p: 0, i: 'Hi' }],        # v1: "Hi"
+        [{ p: 2, i: ' mum' }],      # v2: "Hi mum"
+        [{ p: 1, d: 'i' }],         # v3: "H mum"
+        [{ p: 1, i: 'ello' }],      # v4: "Hello mum"
+      ], (error) =>
+        test.equal error, null
+
+        # n = 2 -> expect versions 2 and 4
+        @model.getVersions name, 2, (error, versions) =>
+          test.equal error, null
+          test.ok Array.isArray versions
+          test.strictEqual versions.length, 2
+
+          test.deepEqual versions[0].v, 2
+          test.strictEqual versions[0].type, 'text'
+          test.deepEqual versions[0].snapshot, 'Hi mum'
+          test.strictEqual typeof versions[0].meta, 'object'
+          test.strictEqual typeof versions[0].meta.ts, 'number'
+
+          test.deepEqual versions[1].v, 4
+          test.strictEqual versions[1].type, 'text'
+          test.deepEqual versions[1].snapshot, 'Hello mum'
+          test.strictEqual typeof versions[1].meta, 'object'
+          test.strictEqual typeof versions[1].meta.ts, 'number'
+
+          # n = 3 -> expect versions 3 and 4 (include latest even if not multiple)
+          @model.getVersions name, 3, (error, versions3) =>
+            test.equal error, null
+            test.ok Array.isArray versions3
+            test.strictEqual versions3.length, 2
+            test.deepEqual versions3.map((d) -> d.v), [3, 4]
+            test.deepEqual versions3.map((d) -> d.snapshot), ['H mum', 'Hello mum']
+            test.done()
+
+  'getVersions on an empty document returns an empty list': (test) ->
+    name = newDocName()
+    @model.create name, types.text, (error) =>
+      test.equal error, null
+      @model.getVersions name, 2, (error, versions) ->
+        test.equal error, null
+        test.deepEqual versions, []
+        test.done()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took a bit of fiddling, but I've added some tests to at least ensure this continues to work the way it's supposed to.

@nickbrowne nickbrowne merged commit 1a285fa into master Nov 13, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants