From 0d198294ebb98855f5afa5f319be7224d600d1be Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Sun, 7 Jun 2026 12:09:44 -0400 Subject: [PATCH] fix: Improve patch merging of links in table cells (#12614) * fix: Improve merging of links in table cells through patch * PR feedback --- server/commands/documentUpdater.test.ts | 78 ++++++++++++++++++++++++ server/models/helpers/DocumentHelper.tsx | 14 ++++- 2 files changed, 90 insertions(+), 2 deletions(-) diff --git a/server/commands/documentUpdater.test.ts b/server/commands/documentUpdater.test.ts index 0ee4db3c94..156a961304 100644 --- a/server/commands/documentUpdater.test.ts +++ b/server/commands/documentUpdater.test.ts @@ -3,6 +3,7 @@ import * as Y from "yjs"; import { TextEditMode } from "@shared/types"; import { APIUpdateExtension } from "@server/collaboration/APIUpdateExtension"; import { Event } from "@server/models"; +import { parser } from "@server/editor"; import { DocumentHelper } from "@server/models/helpers/DocumentHelper"; import { ProsemirrorHelper } from "@server/models/helpers/ProsemirrorHelper"; import { buildDocument, buildUser } from "@server/test/factories"; @@ -1481,5 +1482,82 @@ describe("documentUpdater", () => { "Second item" ); }); + + it("should apply a link when wrapping existing table cell text", async () => { + const user = await buildUser(); + const document = await buildDocument({ teamId: user.teamId }); + + document.content = parser + .parse("| Name | Notes |\n|------|-------|\n| Alpha | see |\n") + .toJSON(); + await document.save(); + + const result = DocumentHelper.applyMarkdownToDocument( + document, + "[see](https://example.com/docs)", + TextEditMode.Patch, + "see" + ); + + // The cell text is unchanged but should now carry a link mark — it must + // not be silently dropped during the merge. + const cellText = + result.content!.content![0].content![1].content![1].content![0] + .content![0]; + expect(cellText.text).toEqual("see"); + expect(cellText.marks).toEqual([ + expect.objectContaining({ + type: "link", + attrs: expect.objectContaining({ href: "https://example.com/docs" }), + }), + ]); + }); + + it("should preserve other table cells when adding a link to one cell", async () => { + const user = await buildUser(); + const document = await buildDocument({ teamId: user.teamId }); + + document.content = parser + .parse( + "| Name | Notes |\n|------|-------|\n| Alpha | see |\n| Beta | other |\n" + ) + .toJSON(); + await document.save(); + + // Capture the untouched (Beta) row BEFORE patching so we can assert its + // structure and attrs are preserved exactly, not just its text. + const beforeDoc = DocumentHelper.toProsemirror(document).toJSON(); + const untouchedRow = beforeDoc.content[0].content[2]; + + const result = DocumentHelper.applyMarkdownToDocument( + document, + "see [docs](https://example.com/d)", + TextEditMode.Patch, + "see" + ); + + // The patched cell gained the link + expect(result.text).toContain("[docs](https://example.com/d)"); + // The untouched row node must remain identical + expect(result.content!.content![0].content![2]).toEqual(untouchedRow); + }); + + it("should apply a mark when wrapping existing list item text", async () => { + const user = await buildUser(); + const document = await buildDocument({ teamId: user.teamId }); + + document.content = parser.parse("- clickme\n- other\n").toJSON(); + await document.save(); + + const result = DocumentHelper.applyMarkdownToDocument( + document, + "[clickme](https://example.com)", + TextEditMode.Patch, + "clickme" + ); + + expect(result.text).toContain("[clickme](https://example.com)"); + expect(result.text).toContain("other"); + }); }); }); diff --git a/server/models/helpers/DocumentHelper.tsx b/server/models/helpers/DocumentHelper.tsx index 78bfd7f5be..08813948c5 100644 --- a/server/models/helpers/DocumentHelper.tsx +++ b/server/models/helpers/DocumentHelper.tsx @@ -817,8 +817,18 @@ export class DocumentHelper { const textSame = oldChild.textContent === newChild.textContent; if (textSame && oldChild.sameMarkup(newChild)) { - // Fully unchanged — keep original with its rich content - merged.push(oldChild); + // Compare against the round-tripped baseline: when the + // updated child is identical to a plain round-trip of the original, + // the patch did not touch it + if (!rtChild || newChild.eq(rtChild)) { + merged.push(oldChild); + } else if (!oldChild.isTextblock && !oldChild.isLeaf) { + // Container child changed deeper down — recurse to preserve rich + // content in the parts that did not change. + merged.push(DocumentHelper.mergeNodes(oldChild, newChild, rtChild)); + } else { + merged.push(newChild); + } } else if (textSame) { // Attrs changed (e.g. checked state) but content same — merge attrs // so that non-markdown-representable values (colwidth, highlight