From 679017cb0447464d4fc8ce527c96eea1613ddb1c Mon Sep 17 00:00:00 2001 From: Gajendra Nishad Date: Tue, 10 Feb 2026 17:09:44 +0530 Subject: [PATCH 001/128] fix: correct wiki edit links redirection --- wiki/templates/wiki/includes/sidebar.html | 8 +++++--- wiki/templates/wiki/macros/buttons.html | 3 +-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/wiki/templates/wiki/includes/sidebar.html b/wiki/templates/wiki/includes/sidebar.html index 99b80e46..8009f1c1 100644 --- a/wiki/templates/wiki/includes/sidebar.html +++ b/wiki/templates/wiki/includes/sidebar.html @@ -170,9 +170,11 @@ Alpine.store('pageContent').markdown = data.raw_markdown; } - // Update edit link - const editLink = document.getElementById('wiki-edit-link'); - if (editLink) editLink.setAttribute('href', data.edit_link); + // Update edit links + const editLinks = document.querySelectorAll('.wiki-edit-link'); + if (editLinks) { + editLinks.forEach(link => link.setAttribute('href', data.edit_link)); + } // Update last updated const lastUpdatedEl = document.getElementById('wiki-last-updated'); diff --git a/wiki/templates/wiki/macros/buttons.html b/wiki/templates/wiki/macros/buttons.html index 23435f41..281246ad 100644 --- a/wiki/templates/wiki/macros/buttons.html +++ b/wiki/templates/wiki/macros/buttons.html @@ -133,8 +133,7 @@
+ class="wiki-edit-link inline-flex items-center gap-1.5 px-3 py-1.5 text-sm font-medium text-[var(--ink-gray-7)] bg-[var(--surface-white)] hover:bg-[var(--surface-gray-1)] transition-colors duration-200"> {{ render_icon("edit", "w-4 h-4") }} {{ edit_text }} From c335a296063c53dee8400af8206bf022ded8cd88 Mon Sep 17 00:00:00 2001 From: Hussain Nagaria Date: Tue, 10 Feb 2026 17:33:02 +0530 Subject: [PATCH 002/128] test(e2e): support class-based wiki edit link selector --- e2e/tests/sidebar.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/tests/sidebar.spec.ts b/e2e/tests/sidebar.spec.ts index 2c26e79a..b8bb11a6 100644 --- a/e2e/tests/sidebar.spec.ts +++ b/e2e/tests/sidebar.spec.ts @@ -277,7 +277,7 @@ test.describe('Public Sidebar', () => { await expect(lastUpdated).toContainText('Last updated'); // Get initial edit link href - const editLinks = publicPage.locator('#wiki-edit-link'); + const editLinks = publicPage.locator('.wiki-edit-link, #wiki-edit-link'); const initialEditHref = await editLinks.first().getAttribute('href'); // Click the second page link in sidebar From edebd723f250ac7fe371009303961b20d8cd2b26 Mon Sep 17 00:00:00 2001 From: Gajendra Nishad Date: Tue, 10 Feb 2026 17:45:17 +0530 Subject: [PATCH 003/128] fix: allow Ctrl/Cmd+Click to open sidebar links in new tab --- wiki/templates/wiki/macros/sidebar_tree.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wiki/templates/wiki/macros/sidebar_tree.html b/wiki/templates/wiki/macros/sidebar_tree.html index 570a0f19..a34f5aca 100644 --- a/wiki/templates/wiki/macros/sidebar_tree.html +++ b/wiki/templates/wiki/macros/sidebar_tree.html @@ -38,7 +38,7 @@ {% else %} {# Page/leaf item #} Date: Wed, 11 Feb 2026 11:02:34 +0530 Subject: [PATCH 004/128] fix: wiki prefix missing in redirect after login --- frontend/src/router.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/router.js b/frontend/src/router.js index 89340983..e276b8c7 100644 --- a/frontend/src/router.js +++ b/frontend/src/router.js @@ -85,7 +85,7 @@ router.beforeEach(async (to, from, next) => { } if (!isLoggedIn) { - window.location.href = `/login?redirect-to=${encodeURIComponent( + window.location.href = `/login?redirect-to=/wiki/${encodeURIComponent( to.fullPath, )}`; } else { From 9f8105b8ac171009f796371442e4b9ef14bfd5c4 Mon Sep 17 00:00:00 2001 From: Hussain Nagaria Date: Wed, 11 Feb 2026 11:09:56 +0530 Subject: [PATCH 005/128] fix: prevent duplicate leaf slugs in change requests --- .../test_wiki_change_request.py | 134 ++++++++++++++++++ .../wiki_change_request.py | 111 +++++++++++++++ 2 files changed, 245 insertions(+) diff --git a/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py b/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py index 05a2a78b..d83c4b22 100644 --- a/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py +++ b/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py @@ -25,6 +25,7 @@ ) from wiki.frappe_wiki.doctype.wiki_revision.wiki_revision import ( create_revision_from_live_tree, + get_or_create_content_blob, ) @@ -72,6 +73,139 @@ def test_create_update_page_in_cr(self): self.assertEqual(item.is_deleted, 0) self.assertIsNotNone(item.content_blob) + def test_create_cr_page_prevents_duplicate_leaf_slug_under_parent(self): + space = create_test_wiki_space() + create_test_wiki_document(space.root_group, title="Page A") + cr = create_change_request(space.name, "CR dup slug") + + root_key = frappe.get_value("Wiki Document", space.root_group, "doc_key") + create_cr_page( + cr.name, + parent_key=root_key, + title="Fees", + slug="fees", + is_group=0, + is_published=1, + content="v1", + ) + + with self.assertRaises(frappe.ValidationError): + create_cr_page( + cr.name, + parent_key=root_key, + title="Fees 2", + slug="fees", + is_group=0, + is_published=1, + content="v2", + ) + + def test_update_cr_page_prevents_duplicate_leaf_slug_under_parent(self): + space = create_test_wiki_space() + create_test_wiki_document(space.root_group, title="Page A") + cr = create_change_request(space.name, "CR dup slug update") + + root_key = frappe.get_value("Wiki Document", space.root_group, "doc_key") + one = create_cr_page( + cr.name, + parent_key=root_key, + title="One", + slug="one", + is_group=0, + is_published=1, + content="v1", + ) + two = create_cr_page( + cr.name, + parent_key=root_key, + title="Two", + slug="two", + is_group=0, + is_published=1, + content="v2", + ) + + with self.assertRaises(frappe.ValidationError): + update_cr_page(cr.name, two, {"slug": "one"}) + + # Sanity: original still unchanged + item_one = get_revision_item(cr.head_revision, one) + self.assertEqual(item_one.slug, "one") + + def test_move_cr_page_prevents_duplicate_leaf_slug_under_parent(self): + space = create_test_wiki_space() + create_test_wiki_document(space.root_group, title="Page A") + cr = create_change_request(space.name, "CR dup slug move") + + root_key = frappe.get_value("Wiki Document", space.root_group, "doc_key") + group_a = create_cr_page( + cr.name, + parent_key=root_key, + title="Group A", + slug="group-a", + is_group=1, + is_published=1, + content="", + ) + group_b = create_cr_page( + cr.name, + parent_key=root_key, + title="Group B", + slug="group-b", + is_group=1, + is_published=1, + content="", + ) + + _ = create_cr_page( + cr.name, + parent_key=group_a, + title="Fees", + slug="fees", + is_group=0, + is_published=1, + content="v1", + ) + fees_b = create_cr_page( + cr.name, + parent_key=group_b, + title="Fees", + slug="fees", + is_group=0, + is_published=1, + content="v2", + ) + + with self.assertRaises(frappe.ValidationError): + move_cr_page(cr.name, fees_b, group_a) + + def test_merge_change_request_fails_fast_on_duplicate_leaf_slugs(self): + space = create_test_wiki_space() + create_test_wiki_document(space.root_group, title="Page A") + cr = create_change_request(space.name, "CR dup merge") + + root_key = frappe.get_value("Wiki Document", space.root_group, "doc_key") + blob = get_or_create_content_blob("x") + + for i in range(2): + item = frappe.new_doc("Wiki Revision Item") + item.revision = cr.head_revision + item.doc_key = frappe.generate_hash(length=12) + item.title = f"Dup {i}" + item.slug = "dup" + item.is_group = 0 + item.is_published = 1 + item.is_external_link = 0 + item.parent_key = root_key + item.order_index = 100 + i + item.content_blob = blob + item.is_deleted = 0 + item.insert(ignore_permissions=True) + + with self.assertRaises(frappe.ValidationError) as ctx: + merge_change_request(cr.name) + self.assertIn("Duplicate leaf slugs detected", str(ctx.exception)) + def test_move_reorder_in_cr(self): space = create_test_wiki_space() page1 = create_test_wiki_document(space.root_group, title="Page 1") diff --git a/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py b/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py index 154f79fe..3f53473d 100644 --- a/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py +++ b/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py @@ -47,6 +47,85 @@ def touch_change_request(name: str) -> None: ) +def _validate_unique_leaf_slug_in_revision( + revision: str, + doc_key: str, + parent_key: str | None, + slug: str | None, + *, + is_group: int | bool = 0, + is_deleted: int | bool = 0, +) -> None: + """Prevent invalid trees inside a revision. + + A leaf's route is derived from ancestor slugs + its slug. Two leaf nodes with the same + (parent_key, slug) will compute the same route and fail at merge time. + """ + if is_deleted or is_group or not slug: + return + + # Treat empty parent_key and None as equivalent. + parent_key = parent_key or None + + existing = frappe.db.get_value( + "Wiki Revision Item", + { + "revision": revision, + "parent_key": parent_key, + "slug": slug, + "is_group": 0, + "is_deleted": 0, + "doc_key": ("!=", doc_key), + }, + ["name", "doc_key", "title"], + as_dict=True, + ) + if existing: + frappe.throw( + _( + "Duplicate page slug '{0}' under the same parent in this change request. " + "Existing doc_key: {1} ({2})" + ).format(slug, existing.doc_key, (existing.title or "").strip() or existing.name), + frappe.ValidationError, + ) + + +def _validate_no_duplicate_leaf_slugs(items: dict[str, dict[str, Any]]) -> None: + """Validate the final merged tree has no duplicate leaf slugs under the same parent.""" + seen: dict[tuple[str | None, str], dict[str, Any]] = {} + dupes: dict[tuple[str | None, str], list[dict[str, Any]]] = {} + + for doc_key, item in items.items(): + if not item or item.get("is_group") or not item.get("slug"): + continue + key = (item.get("parent_key") or None, item.get("slug")) + current = {"doc_key": doc_key, "title": item.get("title")} + if key in seen: + dupes.setdefault(key, [seen[key]]).append(current) + else: + seen[key] = current + + if not dupes: + return + + lines = [] + for (parent_key, slug), rows in sorted(dupes.items(), key=lambda kv: (kv[0][0] or "", kv[0][1])): + parts = [] + for r in rows: + title = (r.get("title") or "").strip() + parts.append(f"{r.get('doc_key')}{f' ({title})' if title else ''}") + parent_label = parent_key or "" + lines.append(f"- parent_key={parent_label}, slug='{slug}': " + ", ".join(parts)) + + frappe.throw( + _( + "Duplicate leaf slugs detected in the merged tree (these would generate duplicate routes). " + "Resolve by renaming/moving/deleting one of the pages:\n{0}" + ).format("\n".join(lines)), + frappe.ValidationError, + ) + + def has_revision_changes(base_revision: str | None, head_revision: str | None) -> bool: if not base_revision or not head_revision: return False @@ -378,6 +457,14 @@ def create_cr_page( item.doc_key = frappe.generate_hash(length=12) item.title = title item.slug = slug or cleanup_page_name(title) + _validate_unique_leaf_slug_in_revision( + head_revision, + item.doc_key, + parent_key, + item.slug, + is_group=is_group, + is_deleted=0, + ) item.is_group = 1 if is_group else 0 item.is_published = 1 if is_published else 0 item.is_external_link = 1 if is_external_link else 0 @@ -419,6 +506,15 @@ def update_cr_page(name: str, doc_key: str, fields: dict[str, Any]) -> None: item.content_blob = get_or_create_content_blob(fields["content"]) if "is_deleted" in fields and fields["is_deleted"] is not None: item.is_deleted = 1 if fields["is_deleted"] else 0 + + _validate_unique_leaf_slug_in_revision( + cr.head_revision, + item.doc_key, + item.parent_key, + item.slug, + is_group=item.is_group, + is_deleted=item.is_deleted, + ) item.save() recompute_revision_hashes(cr.head_revision) @@ -436,6 +532,14 @@ def move_cr_page(name: str, doc_key: str, new_parent_key: str, new_order_index: item = frappe.get_doc("Wiki Revision Item", item_name) item.parent_key = new_parent_key + _validate_unique_leaf_slug_in_revision( + cr.head_revision, + item.doc_key, + item.parent_key, + item.slug, + is_group=item.is_group, + is_deleted=item.is_deleted, + ) if new_order_index is not None: item.order_index = new_order_index item.save() @@ -762,8 +866,15 @@ def merge_change_request(name: str) -> str: conflict_doc.status = "Open" conflict_doc.insert(ignore_permissions=True) + # In web requests, an exception triggers a rollback which would erase the + # inserted conflict rows. Persist them so the UI can show details. + if not frappe.flags.in_test: + frappe.db.commit() + frappe.throw(_("Merge conflicts detected"), frappe.ValidationError) + _validate_no_duplicate_leaf_slugs(merged_items) + merge_revision = create_merge_revision(cr, merged_items) apply_merge_revision(space, merge_revision) From 57363717055c6c55df5c046cc6a343c25411dc27 Mon Sep 17 00:00:00 2001 From: Hussain Nagaria Date: Wed, 11 Feb 2026 11:45:29 +0530 Subject: [PATCH 006/128] fix: link in editor styling Closes #509 --- frontend/src/components/WikiEditor.vue | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/frontend/src/components/WikiEditor.vue b/frontend/src/components/WikiEditor.vue index b7bc0c57..aeb57ac5 100644 --- a/frontend/src/components/WikiEditor.vue +++ b/frontend/src/components/WikiEditor.vue @@ -435,7 +435,7 @@ function initEditor() { handleDrop, attributes: { class: - 'prose prose-sm max-w-none prose-code:before:content-none prose-code:after:content-none prose-code:bg-transparent prose-code:p-0 prose-code:font-normal prose-table:table-fixed prose-td:p-2 prose-th:p-2 prose-td:border prose-th:border prose-td:border-outline-gray-2 prose-th:border-outline-gray-2 prose-td:relative prose-th:relative prose-th:bg-surface-gray-2 wiki-editor-content', + 'prose prose-sm max-w-none prose-code:before:content-none prose-code:after:content-none prose-code:bg-transparent prose-code:p-0 prose-code:font-normal prose-table:table-fixed prose-td:p-2 prose-th:p-2 prose-td:border prose-th:border prose-td:border-outline-gray-2 prose-th:border-outline-gray-2 prose-td:relative prose-th:relative prose-th:bg-surface-gray-2 prose-a:underline prose-a:[text-underline-offset:2px] prose-a:[word-break:break-all] hover:prose-a:text-ink-gray-7 wiki-editor-content', }, }, onUpdate: () => { @@ -665,17 +665,6 @@ onUnmounted(() => { font-style: italic; } -/* Link styling */ -.wiki-editor-content a { - color: var(--primary, #171717); - text-decoration: underline; - text-underline-offset: 2px; -} - -.wiki-editor-content a:hover { - text-decoration-thickness: 2px; -} - /* Table styling */ .wiki-editor-content table { width: 100%; From 0631f0f9f19742a077dbcb86e711588d247d7ccf Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Wed, 11 Feb 2026 14:58:15 +0530 Subject: [PATCH 007/128] fix: archive btn not visible in contribution banner --- .../src/components/ContributionBanner.vue | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/frontend/src/components/ContributionBanner.vue b/frontend/src/components/ContributionBanner.vue index 079fbaac..4d1390b7 100644 --- a/frontend/src/components/ContributionBanner.vue +++ b/frontend/src/components/ContributionBanner.vue @@ -41,17 +41,6 @@ - - + +
@@ -227,6 +227,10 @@ const canShowMerge = computed(() => { return props.canMerge && props.changeCount > 0; }); +const canShowArchive = computed(() => { + return props.changeCount > 0 && (props.changeRequestStatus === 'Draft' || props.changeRequestStatus === 'In Review' || props.changeRequestStatus === 'Changes Requested'); +}) + function getChangeIcon(changeType) { switch (changeType) { case 'added': return LucidePlus; From db714c2159df39885a22d0e9bee71b9a1b5ce7c8 Mon Sep 17 00:00:00 2001 From: Gajendra Nishad Date: Thu, 12 Feb 2026 12:57:55 +0530 Subject: [PATCH 008/128] fix: contain DiffViewer Shadow DOM within stacking context --- frontend/src/pages/ContributionReview.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/pages/ContributionReview.vue b/frontend/src/pages/ContributionReview.vue index 058ee9c6..a8a4e1c9 100644 --- a/frontend/src/pages/ContributionReview.vue +++ b/frontend/src/pages/ContributionReview.vue @@ -112,7 +112,7 @@
-
+
Date: Mon, 16 Feb 2026 10:41:19 +0530 Subject: [PATCH 009/128] fix: reset page on archiving change request --- frontend/src/pages/SpaceDetails.vue | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/frontend/src/pages/SpaceDetails.vue b/frontend/src/pages/SpaceDetails.vue index 7fca99ed..1821dc17 100644 --- a/frontend/src/pages/SpaceDetails.vue +++ b/frontend/src/pages/SpaceDetails.vue @@ -456,6 +456,10 @@ async function handleArchiveChangeRequest() { try { await archiveChangeRequest(); toast.success(__('Change request archived')); + currentChangeRequest.value = null; + await initChangeRequest(); + await loadChanges(); + await refreshTree(); } catch (error) { toast.error(error.messages?.[0] || __('Error archiving change request')); } From 430237ae8810248d3f6409c7a3bd0aa88df92810 Mon Sep 17 00:00:00 2001 From: Gajendra Nishad Date: Mon, 16 Feb 2026 12:34:36 +0530 Subject: [PATCH 010/128] Revert "fix: prevent duplicate leaf slugs in change requests" This reverts commit 9f8105b8ac171009f796371442e4b9ef14bfd5c4. --- .../test_wiki_change_request.py | 134 ------------------ .../wiki_change_request.py | 111 --------------- 2 files changed, 245 deletions(-) diff --git a/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py b/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py index d83c4b22..05a2a78b 100644 --- a/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py +++ b/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py @@ -25,7 +25,6 @@ ) from wiki.frappe_wiki.doctype.wiki_revision.wiki_revision import ( create_revision_from_live_tree, - get_or_create_content_blob, ) @@ -73,139 +72,6 @@ def test_create_update_page_in_cr(self): self.assertEqual(item.is_deleted, 0) self.assertIsNotNone(item.content_blob) - def test_create_cr_page_prevents_duplicate_leaf_slug_under_parent(self): - space = create_test_wiki_space() - create_test_wiki_document(space.root_group, title="Page A") - cr = create_change_request(space.name, "CR dup slug") - - root_key = frappe.get_value("Wiki Document", space.root_group, "doc_key") - create_cr_page( - cr.name, - parent_key=root_key, - title="Fees", - slug="fees", - is_group=0, - is_published=1, - content="v1", - ) - - with self.assertRaises(frappe.ValidationError): - create_cr_page( - cr.name, - parent_key=root_key, - title="Fees 2", - slug="fees", - is_group=0, - is_published=1, - content="v2", - ) - - def test_update_cr_page_prevents_duplicate_leaf_slug_under_parent(self): - space = create_test_wiki_space() - create_test_wiki_document(space.root_group, title="Page A") - cr = create_change_request(space.name, "CR dup slug update") - - root_key = frappe.get_value("Wiki Document", space.root_group, "doc_key") - one = create_cr_page( - cr.name, - parent_key=root_key, - title="One", - slug="one", - is_group=0, - is_published=1, - content="v1", - ) - two = create_cr_page( - cr.name, - parent_key=root_key, - title="Two", - slug="two", - is_group=0, - is_published=1, - content="v2", - ) - - with self.assertRaises(frappe.ValidationError): - update_cr_page(cr.name, two, {"slug": "one"}) - - # Sanity: original still unchanged - item_one = get_revision_item(cr.head_revision, one) - self.assertEqual(item_one.slug, "one") - - def test_move_cr_page_prevents_duplicate_leaf_slug_under_parent(self): - space = create_test_wiki_space() - create_test_wiki_document(space.root_group, title="Page A") - cr = create_change_request(space.name, "CR dup slug move") - - root_key = frappe.get_value("Wiki Document", space.root_group, "doc_key") - group_a = create_cr_page( - cr.name, - parent_key=root_key, - title="Group A", - slug="group-a", - is_group=1, - is_published=1, - content="", - ) - group_b = create_cr_page( - cr.name, - parent_key=root_key, - title="Group B", - slug="group-b", - is_group=1, - is_published=1, - content="", - ) - - _ = create_cr_page( - cr.name, - parent_key=group_a, - title="Fees", - slug="fees", - is_group=0, - is_published=1, - content="v1", - ) - fees_b = create_cr_page( - cr.name, - parent_key=group_b, - title="Fees", - slug="fees", - is_group=0, - is_published=1, - content="v2", - ) - - with self.assertRaises(frappe.ValidationError): - move_cr_page(cr.name, fees_b, group_a) - - def test_merge_change_request_fails_fast_on_duplicate_leaf_slugs(self): - space = create_test_wiki_space() - create_test_wiki_document(space.root_group, title="Page A") - cr = create_change_request(space.name, "CR dup merge") - - root_key = frappe.get_value("Wiki Document", space.root_group, "doc_key") - blob = get_or_create_content_blob("x") - - for i in range(2): - item = frappe.new_doc("Wiki Revision Item") - item.revision = cr.head_revision - item.doc_key = frappe.generate_hash(length=12) - item.title = f"Dup {i}" - item.slug = "dup" - item.is_group = 0 - item.is_published = 1 - item.is_external_link = 0 - item.parent_key = root_key - item.order_index = 100 + i - item.content_blob = blob - item.is_deleted = 0 - item.insert(ignore_permissions=True) - - with self.assertRaises(frappe.ValidationError) as ctx: - merge_change_request(cr.name) - self.assertIn("Duplicate leaf slugs detected", str(ctx.exception)) - def test_move_reorder_in_cr(self): space = create_test_wiki_space() page1 = create_test_wiki_document(space.root_group, title="Page 1") diff --git a/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py b/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py index 3f53473d..154f79fe 100644 --- a/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py +++ b/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py @@ -47,85 +47,6 @@ def touch_change_request(name: str) -> None: ) -def _validate_unique_leaf_slug_in_revision( - revision: str, - doc_key: str, - parent_key: str | None, - slug: str | None, - *, - is_group: int | bool = 0, - is_deleted: int | bool = 0, -) -> None: - """Prevent invalid trees inside a revision. - - A leaf's route is derived from ancestor slugs + its slug. Two leaf nodes with the same - (parent_key, slug) will compute the same route and fail at merge time. - """ - if is_deleted or is_group or not slug: - return - - # Treat empty parent_key and None as equivalent. - parent_key = parent_key or None - - existing = frappe.db.get_value( - "Wiki Revision Item", - { - "revision": revision, - "parent_key": parent_key, - "slug": slug, - "is_group": 0, - "is_deleted": 0, - "doc_key": ("!=", doc_key), - }, - ["name", "doc_key", "title"], - as_dict=True, - ) - if existing: - frappe.throw( - _( - "Duplicate page slug '{0}' under the same parent in this change request. " - "Existing doc_key: {1} ({2})" - ).format(slug, existing.doc_key, (existing.title or "").strip() or existing.name), - frappe.ValidationError, - ) - - -def _validate_no_duplicate_leaf_slugs(items: dict[str, dict[str, Any]]) -> None: - """Validate the final merged tree has no duplicate leaf slugs under the same parent.""" - seen: dict[tuple[str | None, str], dict[str, Any]] = {} - dupes: dict[tuple[str | None, str], list[dict[str, Any]]] = {} - - for doc_key, item in items.items(): - if not item or item.get("is_group") or not item.get("slug"): - continue - key = (item.get("parent_key") or None, item.get("slug")) - current = {"doc_key": doc_key, "title": item.get("title")} - if key in seen: - dupes.setdefault(key, [seen[key]]).append(current) - else: - seen[key] = current - - if not dupes: - return - - lines = [] - for (parent_key, slug), rows in sorted(dupes.items(), key=lambda kv: (kv[0][0] or "", kv[0][1])): - parts = [] - for r in rows: - title = (r.get("title") or "").strip() - parts.append(f"{r.get('doc_key')}{f' ({title})' if title else ''}") - parent_label = parent_key or "" - lines.append(f"- parent_key={parent_label}, slug='{slug}': " + ", ".join(parts)) - - frappe.throw( - _( - "Duplicate leaf slugs detected in the merged tree (these would generate duplicate routes). " - "Resolve by renaming/moving/deleting one of the pages:\n{0}" - ).format("\n".join(lines)), - frappe.ValidationError, - ) - - def has_revision_changes(base_revision: str | None, head_revision: str | None) -> bool: if not base_revision or not head_revision: return False @@ -457,14 +378,6 @@ def create_cr_page( item.doc_key = frappe.generate_hash(length=12) item.title = title item.slug = slug or cleanup_page_name(title) - _validate_unique_leaf_slug_in_revision( - head_revision, - item.doc_key, - parent_key, - item.slug, - is_group=is_group, - is_deleted=0, - ) item.is_group = 1 if is_group else 0 item.is_published = 1 if is_published else 0 item.is_external_link = 1 if is_external_link else 0 @@ -506,15 +419,6 @@ def update_cr_page(name: str, doc_key: str, fields: dict[str, Any]) -> None: item.content_blob = get_or_create_content_blob(fields["content"]) if "is_deleted" in fields and fields["is_deleted"] is not None: item.is_deleted = 1 if fields["is_deleted"] else 0 - - _validate_unique_leaf_slug_in_revision( - cr.head_revision, - item.doc_key, - item.parent_key, - item.slug, - is_group=item.is_group, - is_deleted=item.is_deleted, - ) item.save() recompute_revision_hashes(cr.head_revision) @@ -532,14 +436,6 @@ def move_cr_page(name: str, doc_key: str, new_parent_key: str, new_order_index: item = frappe.get_doc("Wiki Revision Item", item_name) item.parent_key = new_parent_key - _validate_unique_leaf_slug_in_revision( - cr.head_revision, - item.doc_key, - item.parent_key, - item.slug, - is_group=item.is_group, - is_deleted=item.is_deleted, - ) if new_order_index is not None: item.order_index = new_order_index item.save() @@ -866,15 +762,8 @@ def merge_change_request(name: str) -> str: conflict_doc.status = "Open" conflict_doc.insert(ignore_permissions=True) - # In web requests, an exception triggers a rollback which would erase the - # inserted conflict rows. Persist them so the UI can show details. - if not frappe.flags.in_test: - frappe.db.commit() - frappe.throw(_("Merge conflicts detected"), frappe.ValidationError) - _validate_no_duplicate_leaf_slugs(merged_items) - merge_revision = create_merge_revision(cr, merged_items) apply_merge_revision(space, merge_revision) From 81f0debff19da0753c85423aff3e3e839de7b1e2 Mon Sep 17 00:00:00 2001 From: Hussain Nagaria Date: Mon, 16 Feb 2026 22:50:19 +0530 Subject: [PATCH 011/128] refactor: remove dead WikiPagePatch code and unused utility functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WikiPagePatch references the obsolete "Wiki Page" doctype (replaced by Wiki Document in v3). Replace with a stub class for backward compat. Remove apply_markdown_diff, apply_changes, highlight_changes from wiki/utils.py — no other code calls them. Clean up wiki_page_patch.js. Phase 3 of the wiki contribution & merge flow refactor plan. Co-Authored-By: Claude Opus 4.6 --- wiki/REFACTOR.md | 523 ++++++++++++++++++ wiki/utils.py | 144 ----- .../wiki_page_patch/wiki_page_patch.js | 38 -- .../wiki_page_patch/wiki_page_patch.py | 109 +--- 4 files changed, 524 insertions(+), 290 deletions(-) create mode 100644 wiki/REFACTOR.md diff --git a/wiki/REFACTOR.md b/wiki/REFACTOR.md new file mode 100644 index 00000000..8e6a7892 --- /dev/null +++ b/wiki/REFACTOR.md @@ -0,0 +1,523 @@ +# Wiki Contribution & Merge Flow — Refactor Plan + +## Executive Summary + +The wiki app's contribution and merge system is overengineered for what it does. A git-style full-tree-snapshot revision model was built, but without the optimizations that make git fast. The result: + +- **Data integrity bug**: Desk edits bypass the revision system entirely — changes made from Frappe's admin UI are silently lost when CRs are created or merged. +- **O(N) operations that should be O(1)**: Creating a Change Request clones every revision item in the space (500 pages → 500 DB inserts for a 1-page edit). Every CR mutation recomputes hashes over the entire tree. +- **O(N) merge for O(1) changes**: Merging loads all content blobs for 3 full revisions and iterates every doc_key, even when only 1 page changed. Then `apply_merge_revision` does individual `get_doc()` + `save()` for every document in the tree. +- **4 overlapping merge strategies** tried in sequence with a duplicated retry block — hard to reason about, hard to test. +- **Dead code** from the old `WikiPagePatch` system still ships. + +This document proposes 6 independently-shippable phases to fix all of the above. + +--- + +## Current Architecture + +### Data Model + +``` +Wiki Space + ├── root_group → Wiki Document (NestedSet root) + ├── main_revision → Wiki Revision (current published state) + └── route + +Wiki Document (NestedSet) + ├── doc_key (immutable identifier) + ├── title, slug, route + ├── content (live markdown) + ├── parent_wiki_document, lft, rgt, sort_order + └── is_group, is_published + +Wiki Revision (full tree snapshot) + ├── wiki_space, change_request + ├── parent_revision + ├── tree_hash, content_hash + ├── is_working, is_merge + └── has N → Wiki Revision Items + +Wiki Revision Item (one per doc per revision) + ├── revision (link to Wiki Revision) + ├── doc_key, title, slug + ├── parent_key, order_index + ├── content_blob → Wiki Content Blob + └── is_deleted, is_group, is_published + +Wiki Content Blob (content-addressed storage) + ├── hash (SHA256) + └── content (markdown text) + +Wiki Change Request + ├── base_revision (snapshot when CR was created) + ├── head_revision (working revision with edits) + ├── merge_revision (created on merge) + └── status: Draft → In Review → Approved → Merged +``` + +### Edit Flows + +**Editor UI (Change Request flow):** +``` +User types → WikiEditor.vue auto-save → update_cr_page() API + → updates Wiki Revision Item in head_revision + → updates Wiki Content Blob + → recompute_revision_hashes() over entire tree +``` + +**Desk (direct save — NO revision tracking):** +``` +User edits Wiki Document in Frappe admin → doc.save() + → WikiDocument.validate() runs (slug, route, etc.) + → saves directly to tabWiki Document + → revision system is NEVER notified +``` + +**Merge flow:** +``` +merge_change_request() + → load ALL items for base_revision, main_revision, head_revision + → load ALL content blobs for all 3 revisions + → iterate every doc_key across all 3 sets + → for each: try 4 merge strategies in sequence + → create merge_revision with ALL items + → apply_merge_revision: load + save EVERY Wiki Document individually +``` + +### Key Files + +| File | Lines | Purpose | +|------|-------|---------| +| `frappe_wiki/doctype/wiki_change_request/wiki_change_request.py` | 1341 | CR CRUD, diff, 3-way merge, 4 content-merge strategies, apply to live tree | +| `frappe_wiki/doctype/wiki_revision/wiki_revision.py` | 278 | Revision creation, cloning, hash computation, content blobs | +| `frappe_wiki/doctype/wiki_document/wiki_document.py` | 594 | Wiki Document model, validation, rendering, tree building | +| `api/wiki_space.py` | 225 | Live tree reorder, sync to revision system | +| `frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py` | 580 | 20 test cases covering CR lifecycle | + +--- + +## Findings + +### P1: Desk Edits Bypass Revision System (Critical — Data Integrity) + +**The problem**: Wiki Document can be edited directly from Frappe Desk. There is no `doc_events` hook for `Wiki Document` in `hooks.py:111-113` — only `User` has a hook. The only place that syncs desk edits to the revision system is `_sync_main_revision_for_space()` in `api/wiki_space.py:155-170`, which is called **only** from `reorder_wiki_documents()` — never from normal content edits. + +**Impact**: Team members edit Wiki Documents from Desk. The Editor UI builds its tree from `Wiki Revision Item` records (`get_cr_tree()` at `wiki_change_request.py:268-360`). These items are cloned from `main_revision`, which was never updated after the Desk edit. The desk changes are invisible to CRs and are overwritten on the next merge. + +**Root cause**: No `on_update` / `on_trash` doc_event for `Wiki Document`. + +### P2: Full-Tree Clone on CR Creation (Performance) + +**The problem**: `create_change_request()` at `wiki_change_request.py:410-430` calls `clone_revision()` at `wiki_revision.py:97-150`. This copies **every** `Wiki Revision Item` from the base revision to a new working revision. + +**Impact**: For a wiki space with 500 pages, creating a CR inserts 500 `Wiki Revision Item` records even though the user may only edit 1 page. Each insert is an individual `frappe.new_doc().insert()` call — no bulk operations. + +**Compounding factor**: Every subsequent CR operation (`update_cr_page`, `create_cr_page`, `move_cr_page`, `delete_cr_page`, `reorder_cr_children`) calls `recompute_revision_hashes()` at `wiki_revision.py:171-217`, which queries ALL items in the revision plus ALL their content blobs, every single time. + +### P3: Full-Tree Merge Processing (Performance) + +**The problem**: `merge_change_request()` at `wiki_change_request.py:785-887`: + +1. Loads ALL items for 3 revisions via `get_revision_item_map()` — 3 queries returning N rows each +2. Loads ALL content blobs for all 3 via `get_contents_for_items()` at lines 795-797 — 3 bulk queries returning N content strings each +3. Iterates `set(base_items) | set(ours_items) | set(theirs_items)` — processes every doc_key +4. For every doc_key, runs normalization + comparison + merge logic + +Then `apply_merge_revision()` at lines 1267-1341: +1. Creates a new merge revision with ALL items (N inserts) +2. Gets all space docs (1 query) +3. For each of N doc_keys: `frappe.get_doc()` + set fields + `doc.save()` — that's N doc loads + N validates + N saves + +**Impact**: Merging a 1-page content change on a 500-page wiki loads ~1500 revision items, ~1500 content blobs, creates 500 new revision items, and individually loads + saves 500 Wiki Documents. This is where the "merge is slow" complaints come from. + +### P4: Redundant Content Merge Strategies (Complexity) + +**The problem**: There are 4 content-merge functions at `wiki_change_request.py:1034-1131`: + +1. `line_merge_fallback()` — same-length only, rstrip comparison +2. `merge_content_linewise()` — diff-based, `splitlines()` (strips endings) +3. `merge_content_disjoint()` — diff-based, `splitlines(keepends=True)` +4. `merge_content()` — tries same-length first, then diff-based + +They are called in a waterfall at `wiki_change_request.py:966-977`: +```python +merged_content, line_ok = line_merge_fallback(...) +if not line_ok: + merged_content, conflict = merge_content_linewise(...) + if conflict: + merged_content = merge_content_disjoint(...) + if merged_content is None: + merged_content, conflict = merge_content(...) +``` + +Then there's a **duplicated retry** of the exact same waterfall at lines 813-848 inside `merge_change_request()` — if `merge_items()` returns a `"content"` conflict, the same 4 strategies are tried again. + +**Impact**: `merge_content()` already subsumes the logic of the other 3. The duplication makes the code ~100 lines longer than needed and very hard to follow. When someone needs to fix a merge bug, they have to understand all 4 strategies plus the retry block. + +### P5: N+1 Query Pattern in apply_merge_revision (Performance) + +**The problem**: `apply_merge_revision()` at `wiki_change_request.py:1267-1341` processes every doc_key individually: + +```python +for doc_key in ordered_keys: + # ... + if doc_key in key_to_name: + doc = frappe.get_doc("Wiki Document", key_to_name[doc_key]) # DB read + else: + doc = frappe.new_doc("Wiki Document") + doc.title = item.get("title") + # ... set fields ... + content = frappe.get_value("Wiki Content Blob", content_blob, "content") # DB read + doc.content = content + doc.save() # DB write + validate +``` + +For N documents: N `get_doc` calls + N `get_value` calls for content + N `save()` calls (each triggering `validate()` with route computation, unique-route checks, etc.). + +**Impact**: This is the main bottleneck in merge. Content blobs could be batch-loaded in 1 query. Documents that haven't changed don't need to be loaded or saved at all. + +### P6: Dead Code — WikiPagePatch (Tech Debt) + +**The problem**: `wiki/wiki/doctype/wiki_page_patch/wiki_page_patch.py` (116 lines) references a `"Wiki Page"` doctype that no longer exists (replaced by `Wiki Document` in the v3 architecture). It imports `apply_changes`, `apply_markdown_diff`, `highlight_changes` from `wiki.utils` — these are dead utility functions. + +**Impact**: Confusing for new contributors. The old merge logic in `update_old_page()` at line 62 uses a completely different approach from the current 3-way merge, creating a false impression of how the system works. + +### P7: Overcomplicated Draft CR Logic (Complexity) + +**The problem**: `get_or_create_draft_change_request()` at `wiki_change_request.py:157-216` is 60 lines with: +- Loop through all user's drafts checking hash equality for each +- Inline hash comparison (duplicates `has_revision_changes()` logic) +- Auto-archive logic for stale drafts with no changes +- Creates new CR inside the stale-draft branch + +This function is called **every time the editor opens**. + +**Impact**: Hard to follow, hard to test. The inline hash comparison at lines 198-201 duplicates the logic already in `has_revision_changes()` at lines 129-153. + +--- + +## Refactor Plan + +### Phase 1: Desk-to-Revision Sync + +**Goal**: Any edit to a Wiki Document through Desk automatically updates `main_revision`. + +**Priority**: Critical — this is the only data-integrity bug. + +**Changes**: + +1. **`hooks.py`** — Add `Wiki Document` to `doc_events`: + ```python + doc_events = { + "User": {"after_insert": "wiki.utils.add_wiki_user_role"}, + "Wiki Document": { + "on_update": "wiki.frappe_wiki.doctype.wiki_document.wiki_document.on_wiki_document_update", + "on_trash": "wiki.frappe_wiki.doctype.wiki_document.wiki_document.on_wiki_document_trash", + }, + } + ``` + +2. **`wiki_document.py`** — Add hook handlers: + - `on_wiki_document_update(doc, method)`: Skip if `frappe.flags.in_apply_merge_revision` is set (avoid infinite loop during merge). Otherwise, find the owning Wiki Space and call `_sync_main_revision_for_space()`. + - `on_wiki_document_trash(doc, method)`: Same logic. + - Use a per-request debounce via `frappe.flags` so bulk operations only trigger 1 revision sync per space. + +3. **`wiki_change_request.py`** — Set `frappe.flags.in_apply_merge_revision = True` around the loop in `apply_merge_revision()` (wrap in try/finally). + +4. **`api/wiki_space.py`** — Set `frappe.flags.in_reorder_wiki_documents = True` around `reorder_wiki_documents()` to avoid double-sync. + +**Tests to add**: +- `test_desk_edit_syncs_to_revision_system`: Edit Wiki Document directly, verify `main_revision` updates. +- `test_desk_edit_visible_in_new_cr`: Edit via desk, create CR, verify CR's head_revision includes the desk edit. +- `test_merge_does_not_double_sync`: Merge a CR, verify the `on_update` hook doesn't re-trigger revision creation. + +**Risk**: Low. Guard flags are a well-understood pattern. The `_sync_main_revision_for_space` function already exists and is tested (it's used by reorder). + +--- + +### Phase 2: Consolidate Merge Strategies + +**Goal**: Replace 4 overlapping merge functions + duplicated retry block with 1 clean function. + +**Priority**: High — simplifies the hardest-to-understand part of the codebase, makes Phase 5 easier. + +**Changes**: + +1. **`wiki_change_request.py`** — New function `merge_content_three_way(base, ours, theirs) -> (str, bool)`: + ``` + Strategy (in order): + 1. Trivial: ours == theirs, or one side == base + 2. Normalize whitespace, re-check trivials + 3. Same-length line-by-line (rstrip comparison for whitespace tolerance) + 4. Diff-based with SequenceMatcher + disjoint-edit detection + 5. If edits overlap → conflict + ``` + +2. **Delete**: `line_merge_fallback`, `merge_content_linewise`, `merge_content_disjoint`, `merge_content` (the old 4 functions). + +3. **Delete**: The duplicated retry block at lines 813-848 of `merge_change_request()`. + +4. **Update `merge_items()`**: Replace the waterfall chain with a single call to `merge_content_three_way()`. + +5. **Delete unused helpers**: `edits_conflict`, `ranges_overlap` — after consolidation, only `edits_disjoint` is needed for the conflict check. + +**Net result**: ~150 lines removed, 1 function to understand instead of 4. + +**Tests to add**: +- `test_merge_content_three_way_trivial_cases` +- `test_merge_content_three_way_same_length_lines` +- `test_merge_content_three_way_different_length_disjoint` +- `test_merge_content_three_way_overlapping_conflict` +- `test_merge_content_three_way_whitespace_tolerance` + +All existing merge tests must continue to pass (they test the end-to-end flow, not the internal functions). + +**Risk**: Low. Pure refactor with no schema or API changes. Existing test coverage validates correctness. + +--- + +### Phase 3: Dead Code Removal ✅ DONE + +**Goal**: Remove `WikiPagePatch` active code and dead utility functions. + +**Priority**: Low effort, low risk, reduces confusion. + +**Status**: Completed. WikiPagePatch replaced with stub class. Removed `apply_changes`, `apply_markdown_diff`, `highlight_changes` from `wiki/utils.py`. Cleaned up `wiki_page_patch.js`. + +**Changes**: + +1. **`wiki/wiki/doctype/wiki_page_patch/wiki_page_patch.py`** — Replace with stub: + ```python + from frappe.model.document import Document + + class WikiPagePatch(Document): + pass + ``` + Keep the doctype JSON and `__init__.py` for backward compatibility (existing databases may have old records). + +2. **`wiki/utils.py`** — Remove `apply_changes`, `apply_markdown_diff`, `highlight_changes` if they have no other callers (verify with grep first). + +**Tests**: Existing tests should pass unchanged. No new tests needed. + +**Risk**: Very low. The old code references `"Wiki Page"` which doesn't exist in the current schema. + +--- + +### Phase 4: Delta-Based "Overlay" Revisions + +**Goal**: CR working revisions only store items that differ from the base. CR creation goes from O(N) to O(1). + +**Priority**: High impact on performance, but larger change — do after Phases 1-3 are stable. + +**Design: Overlay Revisions** + +Instead of `clone_revision()` copying all N items, a CR's `head_revision` starts empty and only accumulates items as they're edited. Reading the full tree = base items + overlay items (overlay wins). + +**Schema change**: + +Add to `Wiki Revision`: +``` +is_overlay (Check, default 0) — if set, this revision inherits from parent_revision +hashes_stale (Check, default 0, hidden) — if set, tree_hash/content_hash need recomputation +``` + +**Changes**: + +1. **`wiki_revision.py`** — New function `create_overlay_revision(base_revision)`: + - Creates a Wiki Revision with `is_overlay=1`, zero items + - Copies `tree_hash`, `content_hash`, `doc_count` from base (initially identical) + +2. **`wiki_revision.py`** — New function `get_effective_revision_item_map(revision)`: + - If `is_overlay`: return `base_items | overlay_items` (overlay wins) + - If not overlay: return `get_revision_item_map(revision)` as before + +3. **`wiki_change_request.py`** — Update `create_change_request()`: + - Replace `clone_revision(base_revision, is_working=1)` with `create_overlay_revision(base_revision, is_working=1)` + +4. **`wiki_change_request.py`** — New helper `_ensure_overlay_item(head_revision, doc_key)`: + - If item exists in overlay → return it + - If not → copy from base revision to overlay → return it + - Used by `update_cr_page`, `delete_cr_page`, `move_cr_page` + +5. **Update all CR read functions** (`get_cr_tree`, `get_cr_page`, `diff_change_request`) to use `get_effective_revision_item_map()`. + +6. **Lazy hash computation**: Replace per-mutation `recompute_revision_hashes()` calls with `frappe.db.set_value(revision, "hashes_stale", 1)`. Add `ensure_revision_hashes()` that recomputes only when needed (before merge, before `has_revision_changes()` check). + +**Data migration**: + +For existing open CRs (Draft / In Review / Changes Requested): +1. Compare head_revision items against base_revision items +2. Delete identical items from head_revision +3. Set `is_overlay = 1` on head_revision + +Add patch to `patches.txt`. + +**Tests to add**: +- `test_create_cr_creates_empty_overlay`: Verify 0 items in head_revision after CR creation +- `test_editing_promotes_item_to_overlay`: Edit 1 page, verify only 1 item in overlay +- `test_overlay_tree_shows_all_pages`: `get_cr_tree()` returns full tree despite empty overlay +- `test_overlay_diff_shows_only_changes`: `diff_change_request()` only shows edited pages +- `test_overlay_merge_works`: Full merge lifecycle with overlay revisions +- All existing tests must pass (they test the same public API) + +**Risk**: Medium. Schema change + migration. The overlay model is well-understood (it's how Docker layers work). The migration only affects open CRs. + +--- + +### Phase 5: Optimized Merge + +**Goal**: Merge only processes changed doc_keys. `apply_merge_revision` only touches changed documents. + +**Depends on**: Phase 4 (overlay revisions make changed-key detection trivial). + +**Changes**: + +1. **`wiki_change_request.py`** — Add fast-forward path to `merge_change_request()`: + ```python + if cr.base_revision == space.main_revision: + # No concurrent changes — fast-forward merge + return _fast_forward_merge(cr, space) + else: + return _three_way_merge(cr, space) + ``` + +2. **Fast-forward merge** (`_fast_forward_merge`): + - Materialize effective tree from overlay + - Create merge revision (full snapshot for main_revision) + - Only apply changes to live tree (not the entire tree) + +3. **Optimized 3-way merge** (`_three_way_merge`): + - Find changed doc_keys: `main_changed = _find_changed_keys(base, main)` + - Head overlay items are already only the changed keys + - Only load content blobs for `main_changed | head_changed` keys + - Only run merge logic for those keys + - Start with base items, apply merge results + +4. **New function `_find_changed_keys(base_items, other_items)`**: + - Compare `content_blob`, `title`, `slug`, `parent_key`, `order_index`, `is_group`, `is_published` without loading content + - Content blob comparison is a simple string compare (the blob name includes the hash) + +5. **Optimized `apply_merge_revision`** → rename to `_apply_merge_changes_only(space, merge_revision, base_revision)`: + - Find changed keys between merge_revision and base_revision + - Batch-load content blobs for changed items (1 query instead of N) + - Only `get_doc()` + `save()` for changed documents + - Unchanged documents are not touched at all + +6. **Content-only fast path**: For documents where only `content` changed (no title/slug/parent changes), use `frappe.db.set_value()` instead of `doc.save()` to skip validation. Content changes don't affect routes or tree structure. + +**Impact on a 500-page wiki with 1 changed page**: +- Before: Load 1500 items + 1500 blobs, process 500 keys, create 500 items, load+save 500 docs +- After: Load ~2 items + 1 blob, process 1 key, create 500 items (merge revision is still full), load+save 1 doc + +Note: The merge revision still needs to be a full snapshot (it becomes `main_revision`). But items can be bulk-inserted from base with the delta applied, rather than individually created. + +**Tests to add**: +- `test_fast_forward_merge_only_updates_changed_docs`: Create 10 pages, edit 1 via CR, merge, verify only 1 doc's `modified` timestamp changed +- `test_three_way_merge_only_loads_changed_content`: Mock/count DB queries during merge to verify content isn't loaded for unchanged docs +- `test_content_only_change_skips_validation`: Edit only content (no title/slug), verify route is untouched + +**Risk**: Medium. The fast-forward path is safe (it's just applying a diff). The 3-way optimization needs careful testing to ensure unchanged documents truly aren't affected. + +--- + +### Phase 6: Simplify Draft CR Logic + +**Goal**: Make `get_or_create_draft_change_request()` readable. + +**Priority**: Low — UX improvement, not a bug. + +**Changes**: + +Extract the 60-line function into 3 small functions: + +```python +@frappe.whitelist() +def get_or_create_draft_change_request(wiki_space, title=None): + cr = _find_existing_draft(wiki_space) + if cr: + if _should_replace_stale_draft(cr, wiki_space): + _archive_stale_draft(cr) + else: + return cr.as_dict() + + space = frappe.get_doc("Wiki Space", wiki_space) + default_title = title or f"Draft Changes - {space.space_name}" + return create_change_request(wiki_space, default_title).as_dict() + + +def _find_existing_draft(wiki_space): + """Find user's most relevant draft: prefer one with actual changes.""" + ... + +def _should_replace_stale_draft(cr, wiki_space): + """True if the draft is outdated AND has no changes.""" + ... + +def _archive_stale_draft(cr): + """Archive a stale empty draft.""" + ... +``` + +**Tests**: Existing tests cover `get_or_create_draft_change_request` indirectly. Add: +- `test_stale_empty_draft_is_auto_archived` +- `test_stale_draft_with_changes_is_kept` + +**Risk**: Very low. Pure refactor with no behavior change. + +--- + +## Execution Order + +``` +Phase 1 (Desk sync) ← Do first: data integrity fix + ↓ +Phase 3 (Dead code removal) ← Quick win, no dependencies + ↓ +Phase 2 (Merge consolidation) ← Simplifies code for Phase 5 + ↓ +Phase 6 (Draft CR cleanup) ← Quick win, no dependencies + ↓ +Phase 4 (Overlay revisions) ← Biggest architectural change + ↓ +Phase 5 (Optimized merge) ← Builds on Phase 4 +``` + +Phases 1, 2, 3, 6 are independent of each other and can be done in any order. +Phase 5 depends on Phase 4. +Phase 2 should be done before Phase 5 (cleaner code to optimize). + +## Dependency Graph + +``` +Phase 1 ──┐ +Phase 3 ──┤ +Phase 2 ──┼──→ Phase 4 ──→ Phase 5 +Phase 6 ──┘ +``` + +## Summary Table + +| Phase | Problem | Risk | Effort | Lines Changed (est.) | +|-------|---------|------|--------|---------------------| +| 1 | Desk sync (data integrity) | Low | Medium | +60, ~10 modified | +| 2 | 4 merge strategies → 1 | Low | Low | -150, +50 | +| 3 | Dead code removal | Very Low | Very Low | -100 | +| 4 | O(N) → O(1) CR creation | Medium | High | +120, ~80 modified | +| 5 | O(N) → O(changed) merge | Medium | Medium | +100, ~60 modified | +| 6 | Draft CR cleanup | Very Low | Low | ~40 modified | + +## Test Strategy + +All phases add tests to `test_wiki_change_request.py`. The existing 20 tests serve as regression guards — they must pass after every phase. + +Run tests with: +```bash +bench --site run-tests --app wiki --module wiki.frappe_wiki.doctype.wiki_change_request.test_wiki_change_request +``` + +Manual verification checklist: +- [ ] Edit a Wiki Document from Desk → open Editor UI → changes visible (Phase 1) +- [ ] Create a CR on a wiki with 100+ pages → should be near-instant (Phase 4) +- [ ] Merge a CR with 1 changed page → should not take noticeably longer than creating the CR (Phase 5) +- [ ] Merge with concurrent changes (main advanced while CR was open) → 3-way merge resolves cleanly or reports conflict correctly (Phase 2 + 5) diff --git a/wiki/utils.py b/wiki/utils.py index 7be425a9..e3ff9591 100644 --- a/wiki/utils.py +++ b/wiki/utils.py @@ -1,5 +1,3 @@ -import difflib - import frappe from frappe.core.doctype.file.utils import get_content_hash @@ -23,147 +21,5 @@ def check_app_permission(): return False -def apply_markdown_diff(original_md, modified_md): - """ - Compares two markdown texts, finds the differences, and applies them to the original text. - - Args: - original_md (str): The original markdown text. - modified_md (str): The modified markdown text. - - Returns: - tuple: A tuple containing the updated markdown text and a list of changes with their positions. - """ - original_lines = original_md.split("\n") - modified_lines = modified_md.split("\n") - - # Initialize the SequenceMatcher to compare the two lists of lines - matcher = difflib.SequenceMatcher(None, original_lines, modified_lines) - opcodes = matcher.get_opcodes() - - # Sort opcodes by the reverse order of the original start index to handle index shifting - sorted_opcodes = sorted(opcodes, key=lambda x: (-x[1], x[0])) - - # Create a copy of the original lines to apply changes - result = list(original_lines) - changes = [] - - for tag, i1, i2, j1, j2 in sorted_opcodes: - # Convert original line indices to 1-based for reporting - original_range = None - if i1 < i2: - original_start = i1 + 1 - original_end = i2 - original_range = (original_start, original_end) - - if tag == "delete": - # Record the deletion change - changes.append({"type": "delete", "original_lines": original_range, "content": None}) - # Apply deletion to the result - del result[i1:i2] - elif tag == "insert": - # Record the insertion change with position (1-based) - content = modified_lines[j1:j2] - position = i1 + 1 # Convert to 1-based position - changes.append( - {"type": "insert", "original_lines": None, "content": content, "position": position} - ) - # Apply insertion to the result - result[i1:i1] = content - elif tag == "replace": - # Record the replacement change - content = modified_lines[j1:j2] - changes.append({"type": "replace", "original_lines": original_range, "content": content}) - # Apply replacement: delete original lines and insert new content - del result[i1:i2] - result[i1:i1] = content - # 'equal' changes are ignored - - # Join the modified lines back into a single string - updated_md = "\n".join(result) - return updated_md, changes - - -def apply_changes(original_md, changes): - """ - Applies a list of changes to the original markdown text. - - Args: - original_md (str): The original markdown text. - changes (list): A list of changes as returned by apply_markdown_diff. - - Returns: - str: The modified markdown text after applying all changes. - """ - lines = original_md.split("\n") - - # Sort changes in reverse order of their original line numbers to handle index shifting - sorted_changes = sorted( - changes, key=lambda x: (-x["original_lines"][0] if x["original_lines"] else -x["position"]) - ) - - for change in sorted_changes: - if change["type"] == "delete": - start = change["original_lines"][0] - 1 # Convert to 0-based index - end = change["original_lines"][1] - del lines[start:end] - elif change["type"] == "insert": - position = change["position"] - 1 # Convert to 0-based index - lines[position:position] = change["content"] - elif change["type"] == "replace": - start = change["original_lines"][0] - 1 - end = change["original_lines"][1] - del lines[start:end] - lines[start:start] = change["content"] - - return "\n".join(lines) - - -def highlight_changes(original_md, changes): - """ - Highlights changes in the original markdown text using and tags. - - Args: - original_md (str): The original markdown text. - changes (list): A list of changes as returned by apply_markdown_diff. - - Returns: - str: The modified markdown text with changes highlighted. - """ - lines = original_md.split("\n") - - # Sort changes in reverse order of their original line numbers to handle index shifting - sorted_changes = sorted( - changes, key=lambda x: (-x["original_lines"][0] if x["original_lines"] else -x["position"]) - ) - - for change in sorted_changes: - if change["type"] == "delete": - start = change["original_lines"][0] - 1 # Convert to 0-based index - end = change["original_lines"][1] - # Wrap deleted lines with tags - for i in range(start, end): - lines[i] = f"{lines[i]}" - elif change["type"] == "insert": - position = change["position"] - 1 # Convert to 0-based index - # Insert new lines with tags - for line in change["content"]: - lines.insert(position, f"{line}") - position += 1 - elif change["type"] == "replace": - start = change["original_lines"][0] - 1 - end = change["original_lines"][1] - # Wrap deleted lines with tags - for i in range(start, end): - lines[i] = f"{lines[i]}" - # Insert new lines with tags after the deleted lines - insert_pos = end - for line in change["content"]: - lines.insert(insert_pos, f"{line}") - insert_pos += 1 - - return "\n".join(lines) - - def add_wiki_user_role(doc, event=None): doc.add_roles("Wiki User") diff --git a/wiki/wiki/doctype/wiki_page_patch/wiki_page_patch.js b/wiki/wiki/doctype/wiki_page_patch/wiki_page_patch.js index 87279236..0b81b212 100644 --- a/wiki/wiki/doctype/wiki_page_patch/wiki_page_patch.js +++ b/wiki/wiki/doctype/wiki_page_patch/wiki_page_patch.js @@ -1,40 +1,2 @@ // Copyright (c) 2021, Frappe and contributors // For license information, please see license.txt - -frappe.ui.form.on("Wiki Page Patch", { - refresh: function (frm) { - $('[data-fieldname="orignal_code"] pre') - .parent(".like-disabled-input") - .html(frm.doc.orignal_code); - $('[data-fieldname="new_code"] pre') - .parent(".like-disabled-input") - .html(frm.doc.new_code); - - if (!frm.doc.new && !frm.doc.__unsaved) - frappe.call({ - method: "wiki.wiki.doctype.wiki_page.wiki_page.preview", - args: { - original_code: frm.doc.orignal_code, - new_code: frm.doc.new_code, - name: frm.doc.wiki_page, - }, - callback: (r) => { - if (r.message) { - $(".wiki-diff").append(r.message); - $(".wiki-diff").append( - ``, - ); - } - }, - }); - }, -}); diff --git a/wiki/wiki/doctype/wiki_page_patch/wiki_page_patch.py b/wiki/wiki/doctype/wiki_page_patch/wiki_page_patch.py index 199b004f..2d2d8614 100644 --- a/wiki/wiki/doctype/wiki_page_patch/wiki_page_patch.py +++ b/wiki/wiki/doctype/wiki_page_patch/wiki_page_patch.py @@ -1,115 +1,8 @@ # Copyright (c) 2021, Frappe and contributors # For license information, please see license.txt - -import json -import re - -import frappe -from frappe import _ -from frappe.desk.form.utils import add_comment from frappe.model.document import Document -from frappe.website.utils import cleanup_page_name - -from wiki.utils import apply_changes, apply_markdown_diff, highlight_changes class WikiPagePatch(Document): - def before_save(self): - if not self.new: - self.orignal_code = frappe.db.get_value("Wiki Page", self.wiki_page, "content") - - def after_insert(self): - add_comment_to_patch(self.name, self.message) - frappe.db.commit() - - def on_submit(self): - if self.status == "Rejected": - return - - if self.status != "Approved": - frappe.throw(_("Please approve/ reject the request before submitting")) - - self.wiki_page_doc = frappe.get_doc("Wiki Page", self.wiki_page) - - self.clear_sidebar_cache() - - if self.new: - self.create_new_wiki_page() - self.update_sidebars() - else: - self.update_old_page() - - def clear_sidebar_cache(self): - if self.new or self.new_title != self.wiki_page_doc.title: - for key in frappe.cache().hgetall("wiki_sidebar").keys(): - frappe.cache().hdel("wiki_sidebar", key) - - def create_new_wiki_page(self): - self.new_wiki_page = frappe.new_doc("Wiki Page") - - wiki_page_dict = { - "title": self.new_title, - "content": self.new_code or "content", - "route": f"{self.wiki_page_doc.get_space_route()}/{cleanup_page_name(self.new_title)}", - "published": 1, - "allow_guest": self.wiki_page_doc.allow_guest, - } - - self.new_wiki_page.update(wiki_page_dict) - self.new_wiki_page.save() - - def update_old_page(self): - original_md = self.wiki_page_doc.content or "" - modified_md = self.new_code or "" - - merge_old_content = apply_markdown_diff(self.orignal_code, modified_md)[1] - merge_new_content = apply_changes(original_md, merge_old_content) - new_modified_md = apply_markdown_diff(original_md, merge_new_content)[0] - - self.wiki_page_doc.update_page(self.new_title, new_modified_md, self.message, self.raised_by) - - def update_sidebars(self): - if not hasattr(self, "new_sidebar_items") or not self.new_sidebar_items: - self.insert_on_sidebar(self.new_sidebar_group, self.new_wiki_page.name) - return - - sidebars = json.loads(self.new_sidebar_items) - - sidebar_items = sidebars.items() - if sidebar_items: - idx = 0 - for sidebar, items in sidebar_items: - for item in items: - idx += 1 - if item["name"] == "new-wiki-page": - item["name"] = self.new_wiki_page.name - self.insert_on_sidebar(list(sidebars)[-1], self.new_wiki_page.name) - - frappe.db.set_value( - "Wiki Group Item", - {"wiki_page": str(item["name"])}, - {"parent_label": sidebar, "idx": idx}, - ) - - def insert_on_sidebar(self, parent_label: str, wiki_page: str): - wiki_space_name = frappe.get_value("Wiki Space", {"route": self.wiki_page_doc.get_space_route()}) - - wiki_space = frappe.get_doc("Wiki Space", wiki_space_name) - wiki_space.append( - "wiki_sidebars", - { - "wiki_page": wiki_page, - "parent_label": parent_label, - }, - ) - wiki_space.save() - - -@frappe.whitelist() -def add_comment_to_patch(reference_name, content): - email = frappe.session.user - name = frappe.db.get_value("User", frappe.session.user, ["first_name"], as_dict=True).get("first_name") - comment = add_comment("Wiki Page Patch", reference_name, content, email, name) - comment.timepassed = frappe.utils.pretty_date(comment.creation) - return comment + pass From 4a26a44d45283d509bd11bccca2a06a9f8b67bf0 Mon Sep 17 00:00:00 2001 From: Hussain Nagaria Date: Mon, 16 Feb 2026 23:08:25 +0530 Subject: [PATCH 012/128] fix: sync desk edits to revision system (Phase 1) Wiki Documents edited from Frappe Desk now automatically update the space's main_revision via on_update/on_trash doc_events hooks. Guard flags in apply_merge_revision() and reorder_wiki_documents() prevent infinite loops. Three new tests verify desk-to-revision sync, CR visibility, and no double-sync during merge. Co-Authored-By: Claude Opus 4.6 --- wiki/REFACTOR.md | 4 +- wiki/api/wiki_space.py | 24 +++--- .../test_wiki_change_request.py | 75 +++++++++++++++++++ .../wiki_change_request.py | 8 ++ .../doctype/wiki_document/wiki_document.py | 30 ++++++++ wiki/hooks.py | 4 + 6 files changed, 134 insertions(+), 11 deletions(-) diff --git a/wiki/REFACTOR.md b/wiki/REFACTOR.md index 8e6a7892..9ae17530 100644 --- a/wiki/REFACTOR.md +++ b/wiki/REFACTOR.md @@ -199,12 +199,14 @@ This function is called **every time the editor opens**. ## Refactor Plan -### Phase 1: Desk-to-Revision Sync +### Phase 1: Desk-to-Revision Sync ✅ DONE **Goal**: Any edit to a Wiki Document through Desk automatically updates `main_revision`. **Priority**: Critical — this is the only data-integrity bug. +**Status**: Completed. Added `Wiki Document` doc_events (`on_update`, `on_trash`) in `hooks.py`. Hook handlers in `wiki_document.py` sync desk edits to the revision system via `_sync_main_revision_for_space()`. Guard flags in `apply_merge_revision()` and `reorder_wiki_documents()` prevent infinite loops. Three new tests added and all 28 tests pass. + **Changes**: 1. **`hooks.py`** — Add `Wiki Document` to `doc_events`: diff --git a/wiki/api/wiki_space.py b/wiki/api/wiki_space.py index e39f8f39..4887ce70 100644 --- a/wiki/api/wiki_space.py +++ b/wiki/api/wiki_space.py @@ -85,16 +85,20 @@ def reorder_wiki_documents( # Direct reorder for users with write permission parent_changed = doc.parent_wiki_document != new_parent - if parent_changed: - frappe.db.set_value("Wiki Document", doc_name, "parent_wiki_document", new_parent) - - # Batch update sort_order for all siblings - _batch_update_sort_order(siblings_list) - - # Only rebuild the tree if parent changed (structural change) - # For simple reorders, sort_order is sufficient - if parent_changed: - rebuild_wiki_tree() + frappe.flags.in_reorder_wiki_documents = True + try: + if parent_changed: + frappe.db.set_value("Wiki Document", doc_name, "parent_wiki_document", new_parent) + + # Batch update sort_order for all siblings + _batch_update_sort_order(siblings_list) + + # Only rebuild the tree if parent changed (structural change) + # For simple reorders, sort_order is sufficient + if parent_changed: + rebuild_wiki_tree() + finally: + frappe.flags.in_reorder_wiki_documents = False _sync_main_revision_for_space(_get_wiki_space_for_document(doc.name)) diff --git a/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py b/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py index d83c4b22..738265cf 100644 --- a/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py +++ b/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py @@ -528,6 +528,81 @@ def test_merge_preserves_existing_routes(self): f"Route of {doc_name} changed after CR merge: {old_route!r} -> {current_route!r}", ) + def test_desk_edit_syncs_to_revision_system(self): + """Phase 1: Editing a Wiki Document via desk should update main_revision.""" + space = create_test_wiki_space() + page = create_test_wiki_document(space.root_group, title="Page A", content="v1") + + # Ensure main_revision exists + from wiki.frappe_wiki.doctype.wiki_revision.wiki_revision import ( + create_revision_from_live_tree, + get_revision_item_map, + ) + + main_rev = create_revision_from_live_tree(space.name, message="Initial") + frappe.db.set_value("Wiki Space", space.name, "main_revision", main_rev.name) + old_main = main_rev.name + + # Edit via desk (direct save) + page.content = "v2-desk-edit" + page.save() + + # main_revision should have advanced + new_main = frappe.db.get_value("Wiki Space", space.name, "main_revision") + self.assertNotEqual(new_main, old_main, "main_revision should advance after desk edit") + + # The new revision should reflect the desk edit + items = get_revision_item_map(new_main) + page_key = frappe.db.get_value("Wiki Document", page.name, "doc_key") + self.assertIn(page_key, items) + + def test_desk_edit_visible_in_new_cr(self): + """Phase 1: A desk edit should be visible when a new CR is created afterwards.""" + space = create_test_wiki_space() + page = create_test_wiki_document(space.root_group, title="Page A", content="original") + + from wiki.frappe_wiki.doctype.wiki_revision.wiki_revision import ( + create_revision_from_live_tree, + ) + + main_rev = create_revision_from_live_tree(space.name, message="Initial") + frappe.db.set_value("Wiki Space", space.name, "main_revision", main_rev.name) + + # Desk edit + page.content = "desk-edit-content" + page.save() + + # Create a CR after the desk edit + cr = create_change_request(space.name, "CR after desk edit") + + # The CR's head_revision should contain the desk-edited content + page_key = frappe.db.get_value("Wiki Document", page.name, "doc_key") + page_data = frappe.db.get_value( + "Wiki Revision Item", + {"revision": cr.head_revision, "doc_key": page_key}, + "content_blob", + ) + content = frappe.db.get_value("Wiki Content Blob", page_data, "content") or "" + self.assertEqual(content, "desk-edit-content") + + def test_merge_does_not_double_sync(self): + """Phase 1: Merging a CR should not re-trigger revision sync via the on_update hook.""" + space = create_test_wiki_space() + page = create_test_wiki_document(space.root_group, title="Page A", content="v1") + cr = create_change_request(space.name, "CR no double sync") + + page_key = frappe.get_value("Wiki Document", page.name, "doc_key") + update_cr_page(cr.name, page_key, {"content": "v2"}) + + merge_change_request(cr.name) + + # The merge sets main_revision to the merge_revision. + # If on_update fired during merge, it would create yet another revision + # and overwrite main_revision. Verify main_revision == merge_revision. + cr_doc = frappe.get_doc("Wiki Change Request", cr.name) + current_main = frappe.db.get_value("Wiki Space", space.name, "main_revision") + self.assertEqual(current_main, cr_doc.merge_revision) + def test_archive_change_request_sets_status(self): space = create_test_wiki_space() create_test_wiki_document(space.root_group, title="Page A") diff --git a/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py b/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py index 3f53473d..a36a0a78 100644 --- a/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py +++ b/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py @@ -1265,6 +1265,14 @@ def create_merge_revision(cr: Document, merged_items: dict[str, dict[str, Any]]) def apply_merge_revision(space: Document, revision: Document) -> None: + frappe.flags.in_apply_merge_revision = True + try: + _apply_merge_revision(space, revision) + finally: + frappe.flags.in_apply_merge_revision = False + + +def _apply_merge_revision(space: Document, revision: Document) -> None: items = get_revision_item_map(revision.name) ordered_keys = build_tree_order(items) root_doc_key = frappe.get_value("Wiki Document", space.root_group, "doc_key") diff --git a/wiki/frappe_wiki/doctype/wiki_document/wiki_document.py b/wiki/frappe_wiki/doctype/wiki_document/wiki_document.py index 728cb7a9..c9a3f24a 100644 --- a/wiki/frappe_wiki/doctype/wiki_document/wiki_document.py +++ b/wiki/frappe_wiki/doctype/wiki_document/wiki_document.py @@ -550,6 +550,36 @@ def get_page_data(route: str) -> dict: return doc.get_web_context() +def on_wiki_document_update(doc, method): + """Sync desk edits to the revision system so CRs stay aligned with the live tree.""" + _sync_document_to_revision(doc) + + +def on_wiki_document_trash(doc, method): + """Sync desk deletions to the revision system.""" + _sync_document_to_revision(doc) + + +def _sync_document_to_revision(doc): + """Find the owning Wiki Space and refresh its main_revision. + + Skips when called during merge or reorder — those flows manage revisions + themselves via guard flags. + """ + if getattr(frappe.flags, "in_apply_merge_revision", False): + return + if getattr(frappe.flags, "in_reorder_wiki_documents", False): + return + + from wiki.api.wiki_space import _get_wiki_space_for_document, _sync_main_revision_for_space + + space_name = _get_wiki_space_for_document(doc.name) + if not space_name: + return + + _sync_main_revision_for_space(space_name) + + def get_adjacent_documents(nested_tree: list, current_route: str) -> dict: """ Get the previous and next documents based on the flattened tree order. diff --git a/wiki/hooks.py b/wiki/hooks.py index 161b140c..6cb52de7 100644 --- a/wiki/hooks.py +++ b/wiki/hooks.py @@ -110,6 +110,10 @@ doc_events = { "User": {"after_insert": "wiki.utils.add_wiki_user_role"}, + "Wiki Document": { + "on_update": "wiki.frappe_wiki.doctype.wiki_document.wiki_document.on_wiki_document_update", + "on_trash": "wiki.frappe_wiki.doctype.wiki_document.wiki_document.on_wiki_document_trash", + }, } # Scheduled Tasks From 8d26ef5f8626aa5a7cdd5954814dd988070e03fc Mon Sep 17 00:00:00 2001 From: Hussain Nagaria Date: Mon, 16 Feb 2026 23:15:34 +0530 Subject: [PATCH 013/128] refactor: consolidate 4 merge strategies into merge_content_three_way (Phase 2) Replace line_merge_fallback, merge_content_linewise, merge_content_disjoint, and merge_content with a single merge_content_three_way() function. Delete the duplicated retry block in merge_change_request() and unused helpers edits_conflict/ranges_overlap. Net ~50 lines removed. Co-Authored-By: Claude Opus 4.6 --- wiki/REFACTOR.md | 4 +- .../test_wiki_change_request.py | 51 +++++ .../wiki_change_request.py | 185 ++++-------------- 3 files changed, 94 insertions(+), 146 deletions(-) diff --git a/wiki/REFACTOR.md b/wiki/REFACTOR.md index 9ae17530..d51324a2 100644 --- a/wiki/REFACTOR.md +++ b/wiki/REFACTOR.md @@ -238,12 +238,14 @@ This function is called **every time the editor opens**. --- -### Phase 2: Consolidate Merge Strategies +### Phase 2: Consolidate Merge Strategies ✅ DONE **Goal**: Replace 4 overlapping merge functions + duplicated retry block with 1 clean function. **Priority**: High — simplifies the hardest-to-understand part of the codebase, makes Phase 5 easier. +**Status**: Completed. Replaced `line_merge_fallback`, `merge_content_linewise`, `merge_content_disjoint`, and `merge_content` with a single `merge_content_three_way()` function. Deleted the duplicated retry block in `merge_change_request()`. Removed unused helpers `edits_conflict` and `ranges_overlap`. Five new unit tests added and all 33 tests pass. + **Changes**: 1. **`wiki_change_request.py`** — New function `merge_content_three_way(base, ours, theirs) -> (str, bool)`: diff --git a/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py b/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py index 738265cf..5cce86af 100644 --- a/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py +++ b/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py @@ -16,6 +16,7 @@ get_cr_tree, list_change_requests, merge_change_request, + merge_content_three_way, move_cr_page, reorder_cr_children, request_review, @@ -613,6 +614,56 @@ def test_archive_change_request_sets_status(self): self.assertEqual(archived.status, "Archived") self.assertIsNotNone(archived.archived_at) + # --- Phase 2: merge_content_three_way unit tests --- + + def test_merge_content_three_way_trivial_cases(self): + # ours == theirs + result, conflict = merge_content_three_way("base", "same", "same") + self.assertFalse(conflict) + self.assertEqual(result, "same") + + # ours == base (theirs changed) + result, conflict = merge_content_three_way("base", "base", "theirs") + self.assertFalse(conflict) + self.assertEqual(result, "theirs") + + # theirs == base (ours changed) + result, conflict = merge_content_three_way("base", "ours", "base") + self.assertFalse(conflict) + self.assertEqual(result, "ours") + + def test_merge_content_three_way_same_length_lines(self): + base = "line1\nline2\nline3\n" + ours = "line1-ours\nline2\nline3\n" + theirs = "line1\nline2\nline3-theirs\n" + result, conflict = merge_content_three_way(base, ours, theirs) + self.assertFalse(conflict) + self.assertEqual(result, "line1-ours\nline2\nline3-theirs\n") + + def test_merge_content_three_way_different_length_disjoint(self): + base = "line1\nline2\nline3\n" + ours = "line1-ours\nline2\nline3\n" + theirs = "line1\nline2\nline3\nnew-line\n" + result, conflict = merge_content_three_way(base, ours, theirs) + self.assertFalse(conflict) + self.assertIn("line1-ours", result) + self.assertIn("new-line", result) + + def test_merge_content_three_way_overlapping_conflict(self): + base = "line1\nline2\nline3\n" + ours = "line1-ours\nline2\nline3\n" + theirs = "line1-theirs\nline2\nline3\n" + result, conflict = merge_content_three_way(base, ours, theirs) + self.assertTrue(conflict) + + def test_merge_content_three_way_whitespace_tolerance(self): + base = "line1\nline2\nline3\n" + ours = "line1 \nline2\nline3\n" + theirs = "line1\nline2\nline3-theirs\n" + result, conflict = merge_content_three_way(base, ours, theirs) + self.assertFalse(conflict) + self.assertIn("line3-theirs", result) + # Helpers diff --git a/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py b/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py index a36a0a78..3127b6fb 100644 --- a/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py +++ b/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py @@ -810,42 +810,6 @@ def merge_change_request(name: str) -> str: theirs_content = theirs_contents.get(key, "") result, conflict_type = merge_items(base, ours, theirs, base_content, ours_content, theirs_content) - if conflict_type == "content" and base and ours and theirs: - normalized_base = normalize_merge_text(base_content) - normalized_ours = normalize_merge_text(ours_content) - normalized_theirs = normalize_merge_text(theirs_content) - merged_content, line_ok = line_merge_fallback(normalized_base, normalized_ours, normalized_theirs) - if not line_ok: - merged_content, conflict = merge_content_linewise( - normalized_base, normalized_ours, normalized_theirs - ) - line_ok = not conflict - if not line_ok: - merged_content = merge_content_disjoint( - normalized_base, normalized_ours, normalized_theirs - ) - line_ok = merged_content is not None - if not line_ok: - merged_content, conflict = merge_content( - normalized_base, normalized_ours, normalized_theirs - ) - line_ok = not conflict - if line_ok: - merged = { - "doc_key": ours.get("doc_key"), - "title": resolve_field(base.get("title"), ours.get("title"), theirs.get("title")), - "slug": resolve_field(base.get("slug"), ours.get("slug"), theirs.get("slug")), - "is_group": resolve_field( - base.get("is_group"), ours.get("is_group"), theirs.get("is_group") - ), - "is_published": resolve_field( - base.get("is_published"), ours.get("is_published"), theirs.get("is_published") - ), - "parent_key": ours.get("parent_key"), - "order_index": ours.get("order_index"), - } - merged_items[key] = with_content_blob(merged, merged_content) - continue if conflict_type: conflicts.append( {"doc_key": key, "type": conflict_type, "base": base, "ours": ours, "theirs": theirs} @@ -966,15 +930,9 @@ def merge_items( normalized_base = normalize_merge_text(base_content) normalized_ours = normalize_merge_text(ours_content) normalized_theirs = normalize_merge_text(theirs_content) - merged_content, line_ok = line_merge_fallback(normalized_base, normalized_ours, normalized_theirs) - if not line_ok: - merged_content, conflict = merge_content_linewise(normalized_base, normalized_ours, normalized_theirs) - if conflict: - merged_content = merge_content_disjoint(normalized_base, normalized_ours, normalized_theirs) - if merged_content is None: - merged_content, conflict = merge_content(normalized_base, normalized_ours, normalized_theirs) - if conflict: - return None, "content" + merged_content, conflict = merge_content_three_way(normalized_base, normalized_ours, normalized_theirs) + if conflict: + return None, "content" merged = { "doc_key": ours.get("doc_key"), @@ -1031,7 +989,16 @@ def resolve_field(base_value: Any, ours_value: Any, theirs_value: Any) -> Any: return ours_value -def merge_content(base: str, ours: str, theirs: str) -> tuple[str, bool]: +def merge_content_three_way(base: str, ours: str, theirs: str) -> tuple[str, bool]: + """Three-way content merge. Returns (merged_content, has_conflict). + + Strategies tried in order: + 1. Trivial: ours == theirs, or one side == base + 2. Same-length line-by-line merge (rstrip for whitespace tolerance) + 3. Diff-based merge with disjoint-edit detection + 4. If edits overlap -> conflict + """ + # 1. Trivial cases if ours == theirs: return ours, False if ours == base: @@ -1039,90 +1006,41 @@ def merge_content(base: str, ours: str, theirs: str) -> tuple[str, bool]: if theirs == base: return ours, False - base_lines = base.splitlines(keepends=True) - ours_lines = ours.splitlines(keepends=True) - theirs_lines = theirs.splitlines(keepends=True) - - if len(base_lines) == len(ours_lines) == len(theirs_lines): - merged_lines: list[str] = [] - for base_line, ours_line, theirs_line in zip(base_lines, ours_lines, theirs_lines, strict=False): - if ours_line == theirs_line: - merged_lines.append(ours_line) - elif ours_line == base_line: - merged_lines.append(theirs_line) - elif theirs_line == base_line: - merged_lines.append(ours_line) - else: - merged_lines = [] - break - if merged_lines: - return "".join(merged_lines), False - - ours_edits = diff_edits(base_lines, ours_lines) - theirs_edits = diff_edits(base_lines, theirs_lines) - - if edits_conflict(ours_edits, theirs_edits) and not edits_disjoint(ours_edits, theirs_edits): - return "", True - - merged_lines = apply_edits(base_lines, combine_edits(ours_edits, theirs_edits)) - return "".join(merged_lines), False - - -def merge_content_linewise(base: str, ours: str, theirs: str) -> tuple[str, bool]: + # 2. Same-length line-by-line merge with rstrip tolerance base_lines = base.splitlines() ours_lines = ours.splitlines() theirs_lines = theirs.splitlines() - ours_edits = diff_edits(base_lines, ours_lines) - theirs_edits = diff_edits(base_lines, theirs_lines) - - if not edits_disjoint(ours_edits, theirs_edits): - return "", True - - merged_lines = apply_edits(base_lines, combine_edits(ours_edits, theirs_edits)) - ending = "\n" if base.endswith("\n") or ours.endswith("\n") or theirs.endswith("\n") else "" - return "\n".join(merged_lines) + ending, False - - -def merge_content_disjoint(base: str, ours: str, theirs: str) -> str | None: - base_lines = base.splitlines(keepends=True) - ours_lines = ours.splitlines(keepends=True) - theirs_lines = theirs.splitlines(keepends=True) - - ours_edits = diff_edits(base_lines, ours_lines) - theirs_edits = diff_edits(base_lines, theirs_lines) - - if not edits_disjoint(ours_edits, theirs_edits): - return None - - merged_lines = apply_edits(base_lines, combine_edits(ours_edits, theirs_edits)) - return "".join(merged_lines) + if len(base_lines) == len(ours_lines) == len(theirs_lines): + merged: list[str] = [] + for b, o, t in zip(base_lines, ours_lines, theirs_lines, strict=False): + if o.rstrip() == t.rstrip(): + merged.append(o) + elif o.rstrip() == b.rstrip(): + merged.append(t) + elif t.rstrip() == b.rstrip(): + merged.append(o) + else: + merged = [] + break + if merged: + ending = "\n" if base.endswith("\n") or ours.endswith("\n") or theirs.endswith("\n") else "" + return "\n".join(merged) + ending, False + # 3. Diff-based merge with disjoint-edit check + base_lines_ke = base.splitlines(keepends=True) + ours_lines_ke = ours.splitlines(keepends=True) + theirs_lines_ke = theirs.splitlines(keepends=True) -def line_merge_fallback(base: str, ours: str, theirs: str) -> tuple[str, bool]: - base_lines = base.splitlines() - ours_lines = ours.splitlines() - theirs_lines = theirs.splitlines() + ours_edits = diff_edits(base_lines_ke, ours_lines_ke) + theirs_edits = diff_edits(base_lines_ke, theirs_lines_ke) - if len(base_lines) != len(ours_lines) or len(base_lines) != len(theirs_lines): - return "", False - - merged: list[str] = [] - for base_line, ours_line, theirs_line in zip(base_lines, ours_lines, theirs_lines, strict=False): - base_cmp = base_line.rstrip() - ours_cmp = ours_line.rstrip() - theirs_cmp = theirs_line.rstrip() - if ours_cmp == theirs_cmp: - merged.append(ours_line) - elif ours_cmp == base_cmp: - merged.append(theirs_line) - elif theirs_cmp == base_cmp: - merged.append(ours_line) - else: - return "", False + if edits_disjoint(ours_edits, theirs_edits): + merged_lines = apply_edits(base_lines_ke, combine_edits(ours_edits, theirs_edits)) + return "".join(merged_lines), False - ending = "\n" if base.endswith("\n") or ours.endswith("\n") or theirs.endswith("\n") else "" - return "\n".join(merged) + ending, True + # 4. Edits overlap -> conflict + return "", True def normalize_merge_text(content: str) -> str: @@ -1143,29 +1061,6 @@ def diff_edits(base_lines: list[str], new_lines: list[str]) -> list[tuple[int, i return edits -def edits_conflict( - ours_edits: list[tuple[int, int, list[str]]], - theirs_edits: list[tuple[int, int, list[str]]], -) -> bool: - for i1, i2, o_lines in ours_edits: - for j1, j2, t_lines in theirs_edits: - if i1 == i2 and j1 == j2 and i1 == j1: - if o_lines != t_lines: - return True - continue - if ranges_overlap(i1, i2, j1, j2): - return True - if i1 == i2 and j1 <= i1 < j2: - return True - if j1 == j2 and i1 <= j1 < i2: - return True - return False - - -def ranges_overlap(a_start: int, a_end: int, b_start: int, b_end: int) -> bool: - return max(a_start, b_start) < min(a_end, b_end) - - def edits_disjoint( ours_edits: list[tuple[int, int, list[str]]], theirs_edits: list[tuple[int, int, list[str]]], From 8a4c6e8b4805e4c74925926db16980002eedfd9a Mon Sep 17 00:00:00 2001 From: Hussain Nagaria Date: Mon, 16 Feb 2026 23:26:05 +0530 Subject: [PATCH 014/128] refactor: simplify get_or_create_draft_change_request into focused helpers (Phase 6) Extract the 60-line monolithic function into _find_existing_draft(), _is_stale_empty_draft(), and _archive_stale_draft(). Replace duplicated inline hash comparison with a call to has_revision_changes(). Co-Authored-By: Claude Opus 4.6 --- wiki/REFACTOR.md | 4 +- .../test_wiki_change_request.py | 50 ++++++++++ .../wiki_change_request.py | 93 +++++++++---------- 3 files changed, 98 insertions(+), 49 deletions(-) diff --git a/wiki/REFACTOR.md b/wiki/REFACTOR.md index d51324a2..a99d9053 100644 --- a/wiki/REFACTOR.md +++ b/wiki/REFACTOR.md @@ -425,12 +425,14 @@ Note: The merge revision still needs to be a full snapshot (it becomes `main_rev --- -### Phase 6: Simplify Draft CR Logic +### Phase 6: Simplify Draft CR Logic ✅ DONE **Goal**: Make `get_or_create_draft_change_request()` readable. **Priority**: Low — UX improvement, not a bug. +**Status**: Completed. Extracted the 60-line `get_or_create_draft_change_request()` into 3 focused helpers: `_find_existing_draft()`, `_is_stale_empty_draft()`, `_archive_stale_draft()`. Replaced duplicated inline hash comparison with a call to `has_revision_changes()`. Two new tests added and all 35 tests pass. + **Changes**: Extract the 60-line function into 3 small functions: diff --git a/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py b/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py index 5cce86af..ec401e78 100644 --- a/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py +++ b/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py @@ -14,6 +14,7 @@ diff_change_request, get_change_request, get_cr_tree, + get_or_create_draft_change_request, list_change_requests, merge_change_request, merge_content_three_way, @@ -604,6 +605,55 @@ def test_merge_does_not_double_sync(self): current_main = frappe.db.get_value("Wiki Space", space.name, "main_revision") self.assertEqual(current_main, cr_doc.merge_revision) + def test_stale_empty_draft_is_auto_archived(self): + """Phase 6: An outdated draft with no changes should be archived and replaced.""" + space = create_test_wiki_space() + create_test_wiki_document(space.root_group, title="Page A", content="v1") + + # Create a draft CR (this also initializes main_revision) + cr = create_change_request(space.name, "Stale Draft") + old_cr_name = cr.name + + # Advance main_revision so the draft becomes outdated + new_main = create_revision_from_live_tree(space.name, message="advance main") + frappe.db.set_value("Wiki Space", space.name, "main_revision", new_main.name) + + # The draft has no changes (head == base hashes), so it should be archived + result = get_or_create_draft_change_request(space.name) + + old_cr = frappe.get_doc("Wiki Change Request", old_cr_name) + self.assertEqual(old_cr.status, "Archived") + self.assertIsNotNone(old_cr.archived_at) + + # A new CR should have been created + self.assertNotEqual(result.get("name"), old_cr_name) + new_cr = frappe.get_doc("Wiki Change Request", result.get("name")) + self.assertEqual(new_cr.status, "Draft") + + def test_stale_draft_with_changes_is_kept(self): + """Phase 6: An outdated draft that has actual changes should NOT be archived.""" + space = create_test_wiki_space() + page = create_test_wiki_document(space.root_group, title="Page A", content="v1") + + cr = create_change_request(space.name, "Draft With Changes") + cr_name = cr.name + + # Make an edit in the CR so it has real changes + page_key = frappe.get_value("Wiki Document", page.name, "doc_key") + update_cr_page(cr.name, page_key, {"content": "v2-from-cr"}) + + # Advance main_revision so the draft becomes outdated + new_main = create_revision_from_live_tree(space.name, message="advance main") + frappe.db.set_value("Wiki Space", space.name, "main_revision", new_main.name) + + # The draft has changes, so it should be kept (not archived) + result = get_or_create_draft_change_request(space.name) + + self.assertEqual(result.get("name"), cr_name) + kept_cr = frappe.get_doc("Wiki Change Request", cr_name) + self.assertNotEqual(kept_cr.status, "Archived") + self.assertEqual(kept_cr.outdated, 1) + def test_archive_change_request_sets_status(self): space = create_test_wiki_space() create_test_wiki_document(space.root_group, title="Page A") diff --git a/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py b/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py index 3127b6fb..f41a83e4 100644 --- a/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py +++ b/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py @@ -155,6 +155,20 @@ def has_revision_changes(base_revision: str | None, head_revision: str | None) - @frappe.whitelist() def get_or_create_draft_change_request(wiki_space: str, title: str | None = None) -> dict[str, Any]: + cr = _find_existing_draft(wiki_space) + if cr: + if _is_stale_empty_draft(cr, wiki_space): + _archive_stale_draft(cr) + else: + return cr.as_dict() + + space = frappe.get_doc("Wiki Space", wiki_space) + default_title = title or f"Draft Changes - {space.space_name}" + return create_change_request(wiki_space, default_title).as_dict() + + +def _find_existing_draft(wiki_space: str) -> Document | None: + """Find user's most relevant draft: prefer one with actual changes.""" existing = frappe.get_all( "Wiki Change Request", filters={ @@ -165,55 +179,38 @@ def get_or_create_draft_change_request(wiki_space: str, title: str | None = None fields=["name", "base_revision", "head_revision", "modified"], order_by="modified desc", ) - if existing: - selected = None - for row in existing: - if has_revision_changes(row.get("base_revision"), row.get("head_revision")): - selected = row - break - if not selected: - selected = existing[0] - cr = frappe.get_doc("Wiki Change Request", selected["name"]) - cr.check_permission("read") - main_revision = frappe.get_value("Wiki Space", wiki_space, "main_revision") - if main_revision and cr.base_revision and cr.base_revision != main_revision: - frappe.db.set_value("Wiki Change Request", cr.name, "outdated", 1) - base_hash = ( - frappe.get_value( - "Wiki Revision", - cr.base_revision, - ["tree_hash", "content_hash"], - as_dict=True, - ) - or {} - ) - head_hash = ( - frappe.get_value( - "Wiki Revision", - cr.head_revision, - ["tree_hash", "content_hash"], - as_dict=True, - ) - or {} - ) - if base_hash.get("tree_hash") == head_hash.get("tree_hash") and base_hash.get( - "content_hash" - ) == head_hash.get("content_hash"): - frappe.db.set_value( - "Wiki Change Request", - cr.name, - {"status": "Archived", "archived_at": now_datetime()}, - ) - space = frappe.get_doc("Wiki Space", wiki_space) - default_title = title or f"Draft Changes - {space.space_name}" - new_cr = create_change_request(wiki_space, default_title) - return new_cr.as_dict() - return cr.as_dict() + if not existing: + return None - space = frappe.get_doc("Wiki Space", wiki_space) - default_title = title or f"Draft Changes - {space.space_name}" - cr = create_change_request(wiki_space, default_title) - return cr.as_dict() + selected = None + for row in existing: + if has_revision_changes(row.get("base_revision"), row.get("head_revision")): + selected = row + break + if not selected: + selected = existing[0] + + cr = frappe.get_doc("Wiki Change Request", selected["name"]) + cr.check_permission("read") + return cr + + +def _is_stale_empty_draft(cr: Document, wiki_space: str) -> bool: + """True if the draft is outdated AND has no changes.""" + main_revision = frappe.get_value("Wiki Space", wiki_space, "main_revision") + if not main_revision or not cr.base_revision or cr.base_revision == main_revision: + return False + frappe.db.set_value("Wiki Change Request", cr.name, "outdated", 1) + return not has_revision_changes(cr.base_revision, cr.head_revision) + + +def _archive_stale_draft(cr: Document) -> None: + """Archive a stale empty draft.""" + frappe.db.set_value( + "Wiki Change Request", + cr.name, + {"status": "Archived", "archived_at": now_datetime()}, + ) @frappe.whitelist() From abc3de2af2965d9caf7924881fadcfcda50e6977 Mon Sep 17 00:00:00 2001 From: Hussain Nagaria Date: Mon, 16 Feb 2026 23:49:03 +0530 Subject: [PATCH 015/128] refactor: delta-based overlay revisions for O(1) CR creation (Phase 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CR working revisions now start empty and only accumulate items as they are edited (copy-on-write). This eliminates the O(N) clone on CR creation — a 500-page wiki no longer inserts 500 revision items just to open the editor. Key changes: - create_overlay_revision() replaces clone_revision() for CRs - get_effective_revision_item_map() merges base + overlay transparently - ensure_overlay_item() handles copy-on-write for mutations - Lazy hash computation defers recompute until merge/diff time - All read/write CR functions updated for overlay awareness - Data migration patch converts existing open CRs Co-Authored-By: Claude Opus 4.6 --- wiki/REFACTOR.md | 4 +- .../test_wiki_change_request.py | 132 +++++++++++++- .../wiki_change_request.py | 160 ++++++++++------- .../doctype/wiki_revision/patches/__init__.py | 0 .../patches/convert_open_crs_to_overlay.py | 75 ++++++++ .../doctype/wiki_revision/wiki_revision.json | 15 ++ .../doctype/wiki_revision/wiki_revision.py | 168 +++++++++++++++--- wiki/patches.txt | 3 +- 8 files changed, 457 insertions(+), 100 deletions(-) create mode 100644 wiki/frappe_wiki/doctype/wiki_revision/patches/__init__.py create mode 100644 wiki/frappe_wiki/doctype/wiki_revision/patches/convert_open_crs_to_overlay.py diff --git a/wiki/REFACTOR.md b/wiki/REFACTOR.md index a99d9053..1989d2ba 100644 --- a/wiki/REFACTOR.md +++ b/wiki/REFACTOR.md @@ -308,12 +308,14 @@ All existing merge tests must continue to pass (they test the end-to-end flow, n --- -### Phase 4: Delta-Based "Overlay" Revisions +### Phase 4: Delta-Based "Overlay" Revisions ✅ DONE **Goal**: CR working revisions only store items that differ from the base. CR creation goes from O(N) to O(1). **Priority**: High impact on performance, but larger change — do after Phases 1-3 are stable. +**Status**: Completed. CR creation now produces an empty overlay revision (0 items) instead of cloning all N items. Copy-on-write via `ensure_overlay_item()` promotes items to the overlay only when mutated. All read functions (`get_cr_tree`, `get_cr_page`, `diff_change_request`) use `get_effective_revision_item_map()` to merge base + overlay. Lazy hash computation defers `recompute_revision_hashes()` until needed (before merge/diff checks). Data migration patch converts existing open CRs. Five new tests added and all 40 tests pass. + **Design: Overlay Revisions** Instead of `clone_revision()` copying all N items, a CR's `head_revision` starts empty and only accumulates items as they're edited. Reading the full tree = base items + overlay items (overlay wins). diff --git a/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py b/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py index ec401e78..f2b800e9 100644 --- a/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py +++ b/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py @@ -48,9 +48,17 @@ def test_create_change_request_initializes_revisions(self): head_revision = frappe.get_doc("Wiki Revision", cr.head_revision) self.assertEqual(head_revision.is_working, 1) + self.assertEqual(head_revision.is_overlay, 1) - item_count = frappe.db.count("Wiki Revision Item", {"revision": cr.head_revision}) - self.assertEqual(item_count, 2) # root + page + # Overlay has 0 items (inherits from base) + overlay_count = frappe.db.count("Wiki Revision Item", {"revision": cr.head_revision}) + self.assertEqual(overlay_count, 0) + + # But the effective tree has all items (root + page) + from wiki.frappe_wiki.doctype.wiki_revision.wiki_revision import get_effective_revision_item_map + + effective = get_effective_revision_item_map(cr.head_revision) + self.assertEqual(len(effective), 2) def test_create_update_page_in_cr(self): space = create_test_wiki_space() @@ -565,6 +573,7 @@ def test_desk_edit_visible_in_new_cr(self): from wiki.frappe_wiki.doctype.wiki_revision.wiki_revision import ( create_revision_from_live_tree, + get_effective_revision_item_map, ) main_rev = create_revision_from_live_tree(space.name, message="Initial") @@ -577,14 +586,13 @@ def test_desk_edit_visible_in_new_cr(self): # Create a CR after the desk edit cr = create_change_request(space.name, "CR after desk edit") - # The CR's head_revision should contain the desk-edited content + # The CR's effective tree should contain the desk-edited content page_key = frappe.db.get_value("Wiki Document", page.name, "doc_key") - page_data = frappe.db.get_value( - "Wiki Revision Item", - {"revision": cr.head_revision, "doc_key": page_key}, - "content_blob", - ) - content = frappe.db.get_value("Wiki Content Blob", page_data, "content") or "" + effective = get_effective_revision_item_map(cr.head_revision) + item = effective.get(page_key) + self.assertIsNotNone(item) + content_blob = item.get("content_blob") + content = frappe.db.get_value("Wiki Content Blob", content_blob, "content") or "" self.assertEqual(content, "desk-edit-content") def test_merge_does_not_double_sync(self): @@ -714,6 +722,112 @@ def test_merge_content_three_way_whitespace_tolerance(self): self.assertFalse(conflict) self.assertIn("line3-theirs", result) + # --- Phase 4: Overlay revision tests --- + + def test_create_cr_creates_empty_overlay(self): + """Phase 4: CR creation should produce an overlay revision with 0 items.""" + space = create_test_wiki_space() + create_test_wiki_document(space.root_group, title="Page A") + create_test_wiki_document(space.root_group, title="Page B") + + cr = create_change_request(space.name, "CR overlay") + + head_revision = frappe.get_doc("Wiki Revision", cr.head_revision) + self.assertEqual(head_revision.is_overlay, 1) + self.assertEqual(head_revision.parent_revision, cr.base_revision) + + # Zero items in the overlay itself + overlay_item_count = frappe.db.count("Wiki Revision Item", {"revision": cr.head_revision}) + self.assertEqual(overlay_item_count, 0) + + # Base still has all items + base_item_count = frappe.db.count("Wiki Revision Item", {"revision": cr.base_revision}) + self.assertGreater(base_item_count, 0) + + def test_editing_promotes_item_to_overlay(self): + """Phase 4: Editing 1 page should copy-on-write only that item into the overlay.""" + space = create_test_wiki_space() + page_a = create_test_wiki_document(space.root_group, title="Page A", content="v1") + create_test_wiki_document(space.root_group, title="Page B", content="other") + + cr = create_change_request(space.name, "CR promote") + + page_a_key = frappe.get_value("Wiki Document", page_a.name, "doc_key") + update_cr_page(cr.name, page_a_key, {"content": "v2"}) + + # Only 1 item promoted to overlay + overlay_count = frappe.db.count("Wiki Revision Item", {"revision": cr.head_revision}) + self.assertEqual(overlay_count, 1) + + # And it's the right one + overlay_item = frappe.get_all( + "Wiki Revision Item", + filters={"revision": cr.head_revision}, + fields=["doc_key"], + ) + self.assertEqual(overlay_item[0]["doc_key"], page_a_key) + + def test_overlay_tree_shows_all_pages(self): + """Phase 4: get_cr_tree should return full tree despite empty overlay.""" + space = create_test_wiki_space() + page_a = create_test_wiki_document(space.root_group, title="Page A") + group = create_test_wiki_document(space.root_group, title="Group", is_group=1) + child = create_test_wiki_document(group.name, title="Child") + + cr = create_change_request(space.name, "CR tree") + + tree = get_cr_tree(cr.name) + children = tree.get("children") or [] + + # Should show all pages even though overlay is empty + page_a_key = frappe.get_value("Wiki Document", page_a.name, "doc_key") + group_key = frappe.get_value("Wiki Document", group.name, "doc_key") + child_key = frappe.get_value("Wiki Document", child.name, "doc_key") + + child_keys = {node["doc_key"] for node in children} + self.assertSetEqual(child_keys, {page_a_key, group_key}) + + group_node = next(n for n in children if n["doc_key"] == group_key) + grandchild_keys = {n["doc_key"] for n in group_node.get("children") or []} + self.assertSetEqual(grandchild_keys, {child_key}) + + def test_overlay_diff_shows_only_changes(self): + """Phase 4: diff_change_request should only show edited pages.""" + space = create_test_wiki_space() + page_a = create_test_wiki_document(space.root_group, title="Page A", content="v1") + create_test_wiki_document(space.root_group, title="Page B", content="other") + + cr = create_change_request(space.name, "CR diff") + + page_a_key = frappe.get_value("Wiki Document", page_a.name, "doc_key") + update_cr_page(cr.name, page_a_key, {"content": "v2"}) + + summary = diff_change_request(cr.name, scope="summary") + changed_keys = {row["doc_key"] for row in summary} + + # Only page_a should show as changed + self.assertEqual(changed_keys, {page_a_key}) + + def test_overlay_merge_works(self): + """Phase 4: Full merge lifecycle with overlay revisions.""" + space = create_test_wiki_space() + page = create_test_wiki_document(space.root_group, title="Page A", content="original") + + cr = create_change_request(space.name, "CR merge overlay") + + page_key = frappe.get_value("Wiki Document", page.name, "doc_key") + update_cr_page(cr.name, page_key, {"content": "updated via cr"}) + + merge_change_request(cr.name) + + # Live tree should be updated + updated = frappe.get_doc("Wiki Document", page.name) + self.assertEqual(updated.content, "updated via cr") + + cr_doc = frappe.get_doc("Wiki Change Request", cr.name) + self.assertEqual(cr_doc.status, "Merged") + self.assertIsNotNone(cr_doc.merge_revision) + # Helpers diff --git a/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py b/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py index f41a83e4..43a98b00 100644 --- a/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py +++ b/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py @@ -14,10 +14,14 @@ from wiki.frappe_wiki.doctype.wiki_revision.wiki_revision import ( build_tree_order, - clone_revision, + create_overlay_revision, create_revision_from_live_tree, + ensure_overlay_item, + ensure_revision_hashes, + get_effective_revision_item_map, get_or_create_content_blob, get_revision_item_map, + mark_hashes_stale, recompute_revision_hashes, ) @@ -67,6 +71,7 @@ def _validate_unique_leaf_slug_in_revision( # Treat empty parent_key and None as equivalent. parent_key = parent_key or None + # Check in the current revision existing = frappe.db.get_value( "Wiki Revision Item", { @@ -88,6 +93,37 @@ def _validate_unique_leaf_slug_in_revision( ).format(slug, existing.doc_key, (existing.title or "").strip() or existing.name), frappe.ValidationError, ) + return + + # For overlay revisions, also check base items not overridden by the overlay + rev_info = frappe.db.get_value("Wiki Revision", revision, ["is_overlay", "parent_revision"], as_dict=True) + if not (rev_info and rev_info.is_overlay and rev_info.parent_revision): + return + + overlay_keys = set(frappe.get_all("Wiki Revision Item", filters={"revision": revision}, pluck="doc_key")) + + base_matches = frappe.get_all( + "Wiki Revision Item", + filters={ + "revision": rev_info.parent_revision, + "parent_key": parent_key, + "slug": slug, + "is_group": 0, + "is_deleted": 0, + }, + fields=["name", "doc_key", "title"], + ) + for match in base_matches: + if match["doc_key"] == doc_key or match["doc_key"] in overlay_keys: + continue + frappe.throw( + _( + "Duplicate page slug '{0}' under the same parent in this change request. " + "Existing doc_key: {1} ({2})" + ).format(slug, match["doc_key"], (match.get("title") or "").strip() or match["name"]), + frappe.ValidationError, + ) + return def _validate_no_duplicate_leaf_slugs(items: dict[str, dict[str, Any]]) -> None: @@ -129,6 +165,8 @@ def _validate_no_duplicate_leaf_slugs(items: dict[str, dict[str, Any]]) -> None: def has_revision_changes(base_revision: str | None, head_revision: str | None) -> bool: if not base_revision or not head_revision: return False + ensure_revision_hashes(base_revision) + ensure_revision_hashes(head_revision) base_hash = ( frappe.get_value( "Wiki Revision", @@ -271,25 +309,10 @@ def get_cr_tree(name: str) -> dict[str, Any]: return {"children": [], "root_group": None} root_key = frappe.get_value("Wiki Document", root_group, "doc_key") - items = frappe.get_all( - "Wiki Revision Item", - fields=[ - "doc_key", - "title", - "slug", - "is_group", - "is_published", - "is_external_link", - "external_url", - "parent_key", - "order_index", - "is_deleted", - ], - filters={"revision": cr.head_revision}, - ) + effective_items = get_effective_revision_item_map(cr.head_revision) doc_map: dict[str, dict[str, Any]] = {} - for item in items: + for item in effective_items.values(): if item.get("is_deleted"): continue doc_map[item["doc_key"]] = { @@ -361,24 +384,38 @@ def sort_children(nodes: list[dict[str, Any]]) -> list[dict[str, Any]]: def get_cr_page(name: str, doc_key: str) -> dict[str, Any]: cr = frappe.get_doc("Wiki Change Request", name) cr.check_permission("read") + + _item_fields = [ + "doc_key", + "title", + "slug", + "is_group", + "is_published", + "is_external_link", + "external_url", + "parent_key", + "order_index", + "content_blob", + "is_deleted", + ] item = frappe.db.get_value( "Wiki Revision Item", {"revision": cr.head_revision, "doc_key": doc_key}, - [ - "doc_key", - "title", - "slug", - "is_group", - "is_published", - "is_external_link", - "external_url", - "parent_key", - "order_index", - "content_blob", - "is_deleted", - ], + _item_fields, as_dict=True, ) + # For overlay revisions, fall back to base if item isn't in the overlay + if not item: + rev_info = frappe.db.get_value( + "Wiki Revision", cr.head_revision, ["is_overlay", "parent_revision"], as_dict=True + ) + if rev_info and rev_info.is_overlay and rev_info.parent_revision: + item = frappe.db.get_value( + "Wiki Revision Item", + {"revision": rev_info.parent_revision, "doc_key": doc_key}, + _item_fields, + as_dict=True, + ) if not item or item.get("is_deleted"): frappe.throw(_("Document not found in change request")) @@ -412,7 +449,7 @@ def create_change_request(wiki_space: str, title: str, description: str | None = space.main_revision = main_revision.name base_revision = space.main_revision - head_revision = clone_revision(base_revision, is_working=1) + head_revision = create_overlay_revision(base_revision, is_working=1) cr = frappe.new_doc("Wiki Change Request") cr.title = title @@ -443,7 +480,7 @@ def create_cr_page( cr = frappe.get_doc("Wiki Change Request", name) head_revision = cr.head_revision - item_map = get_revision_item_map(head_revision) + item_map = get_effective_revision_item_map(head_revision) max_order = max( [item.get("order_index") or 0 for item in item_map.values() if item.get("parent_key") == parent_key] or [0] @@ -472,7 +509,7 @@ def create_cr_page( item.is_deleted = 0 item.insert() - recompute_revision_hashes(head_revision) + mark_hashes_stale(head_revision) touch_change_request(cr.name) return item.doc_key @@ -480,9 +517,7 @@ def create_cr_page( @frappe.whitelist() def update_cr_page(name: str, doc_key: str, fields: dict[str, Any]) -> None: cr = frappe.get_doc("Wiki Change Request", name) - item_name = frappe.get_value( - "Wiki Revision Item", {"revision": cr.head_revision, "doc_key": doc_key}, "name" - ) + item_name = ensure_overlay_item(cr.head_revision, doc_key) if not item_name: frappe.throw(_("Document not found in change request")) @@ -514,16 +549,14 @@ def update_cr_page(name: str, doc_key: str, fields: dict[str, Any]) -> None: ) item.save() - recompute_revision_hashes(cr.head_revision) + mark_hashes_stale(cr.head_revision) touch_change_request(cr.name) @frappe.whitelist() def move_cr_page(name: str, doc_key: str, new_parent_key: str, new_order_index: int | None = None) -> None: cr = frappe.get_doc("Wiki Change Request", name) - item_name = frappe.get_value( - "Wiki Revision Item", {"revision": cr.head_revision, "doc_key": doc_key}, "name" - ) + item_name = ensure_overlay_item(cr.head_revision, doc_key) if not item_name: frappe.throw(_("Document not found in change request")) @@ -541,7 +574,7 @@ def move_cr_page(name: str, doc_key: str, new_parent_key: str, new_order_index: item.order_index = new_order_index item.save() - recompute_revision_hashes(cr.head_revision) + mark_hashes_stale(cr.head_revision) touch_change_request(cr.name) @@ -549,25 +582,22 @@ def move_cr_page(name: str, doc_key: str, new_parent_key: str, new_order_index: def reorder_cr_children(name: str, parent_key: str, ordered_doc_keys: list[str]) -> None: cr = frappe.get_doc("Wiki Change Request", name) for index, doc_key in enumerate(ordered_doc_keys): - item_name = frappe.get_value( - "Wiki Revision Item", - {"revision": cr.head_revision, "doc_key": doc_key, "parent_key": parent_key}, - "name", - ) + item_name = ensure_overlay_item(cr.head_revision, doc_key) if not item_name: continue + actual_parent = frappe.db.get_value("Wiki Revision Item", item_name, "parent_key") + if actual_parent != parent_key: + continue frappe.db.set_value("Wiki Revision Item", item_name, "order_index", index) - recompute_revision_hashes(cr.head_revision) + mark_hashes_stale(cr.head_revision) touch_change_request(cr.name) @frappe.whitelist() def delete_cr_page(name: str, doc_key: str) -> None: cr = frappe.get_doc("Wiki Change Request", name) - item_name = frappe.get_value( - "Wiki Revision Item", {"revision": cr.head_revision, "doc_key": doc_key}, "name" - ) + item_name = ensure_overlay_item(cr.head_revision, doc_key) if not item_name: frappe.throw(_("Document not found in change request")) @@ -575,28 +605,26 @@ def delete_cr_page(name: str, doc_key: str) -> None: item.is_deleted = 1 item.save() - items = frappe.get_all( - "Wiki Revision Item", - fields=["name", "doc_key", "parent_key"], - filters={"revision": cr.head_revision}, - ) - children: dict[str | None, list[dict[str, str]]] = {} - for row in items: - children.setdefault(row.get("parent_key"), []).append(row) + # Use effective items to find all descendants (overlay + base) + effective_items = get_effective_revision_item_map(cr.head_revision) + children: dict[str | None, list[str]] = {} + for key, item_data in effective_items.items(): + children.setdefault(item_data.get("parent_key"), []).append(key) to_visit = [doc_key] seen = {doc_key} while to_visit: current = to_visit.pop() - for child in children.get(current, []): - child_key = child.get("doc_key") - if not child_key or child_key in seen: + for child_key in children.get(current, []): + if child_key in seen: continue seen.add(child_key) - frappe.db.set_value("Wiki Revision Item", child.get("name"), "is_deleted", 1) + child_item_name = ensure_overlay_item(cr.head_revision, child_key) + if child_item_name: + frappe.db.set_value("Wiki Revision Item", child_item_name, "is_deleted", 1) to_visit.append(child_key) - recompute_revision_hashes(cr.head_revision) + mark_hashes_stale(cr.head_revision) touch_change_request(cr.name) @@ -604,7 +632,7 @@ def delete_cr_page(name: str, doc_key: str) -> None: def diff_change_request(name: str, scope: str = "summary", doc_key: str | None = None): cr = frappe.get_doc("Wiki Change Request", name) base_items = get_revision_item_map(cr.base_revision) - head_items = get_revision_item_map(cr.head_revision) + head_items = get_effective_revision_item_map(cr.head_revision) base_contents: dict[str, str] = {} head_contents: dict[str, str] = {} @@ -788,7 +816,7 @@ def merge_change_request(name: str) -> str: base_items = get_revision_item_map(cr.base_revision) ours_items = get_revision_item_map(space.main_revision) - theirs_items = get_revision_item_map(cr.head_revision) + theirs_items = get_effective_revision_item_map(cr.head_revision) base_contents = get_contents_for_items(base_items) ours_contents = get_contents_for_items(ours_items) theirs_contents = get_contents_for_items(theirs_items) diff --git a/wiki/frappe_wiki/doctype/wiki_revision/patches/__init__.py b/wiki/frappe_wiki/doctype/wiki_revision/patches/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/wiki/frappe_wiki/doctype/wiki_revision/patches/convert_open_crs_to_overlay.py b/wiki/frappe_wiki/doctype/wiki_revision/patches/convert_open_crs_to_overlay.py new file mode 100644 index 00000000..906d8aad --- /dev/null +++ b/wiki/frappe_wiki/doctype/wiki_revision/patches/convert_open_crs_to_overlay.py @@ -0,0 +1,75 @@ +"""Convert existing open Change Requests to the overlay revision model. + +For each open CR (Draft / In Review / Changes Requested): +1. Compare head_revision items against base_revision items +2. Delete identical items from head_revision +3. Set is_overlay=1 on head_revision +""" + +import frappe + + +def execute(): + open_crs = frappe.get_all( + "Wiki Change Request", + filters={"status": ("in", ["Draft", "In Review", "Changes Requested"])}, + fields=["name", "base_revision", "head_revision"], + ) + + compare_fields = [ + "title", + "slug", + "is_group", + "is_published", + "is_external_link", + "external_url", + "parent_key", + "order_index", + "content_blob", + "is_deleted", + ] + + for cr in open_crs: + if not cr.base_revision or not cr.head_revision: + continue + + # Already converted + if frappe.db.get_value("Wiki Revision", cr.head_revision, "is_overlay"): + continue + + base_items = {} + for item in frappe.get_all( + "Wiki Revision Item", + filters={"revision": cr.base_revision}, + fields=["doc_key", *compare_fields], + ): + base_items[item["doc_key"]] = item + + head_items = frappe.get_all( + "Wiki Revision Item", + filters={"revision": cr.head_revision}, + fields=["name", "doc_key", *compare_fields], + ) + + identical_names = [] + for item in head_items: + base = base_items.get(item["doc_key"]) + if not base: + continue # New item in CR, keep in overlay + if all(item.get(f) == base.get(f) for f in compare_fields): + identical_names.append(item["name"]) + + if identical_names: + frappe.db.delete("Wiki Revision Item", {"name": ("in", identical_names)}) + + frappe.db.set_value( + "Wiki Revision", + cr.head_revision, + { + "is_overlay": 1, + "parent_revision": cr.base_revision, + "hashes_stale": 0, + }, + ) + + frappe.db.commit() diff --git a/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.json b/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.json index 5d063691..cbf0f2e7 100644 --- a/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.json +++ b/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.json @@ -13,6 +13,8 @@ "message", "is_merge", "is_working", + "is_overlay", + "hashes_stale", "section_break_meta", "created_by", "created_at", @@ -61,6 +63,19 @@ "fieldtype": "Check", "label": "Is Working" }, + { + "default": "0", + "fieldname": "is_overlay", + "fieldtype": "Check", + "label": "Is Overlay" + }, + { + "default": "0", + "fieldname": "hashes_stale", + "fieldtype": "Check", + "hidden": 1, + "label": "Hashes Stale" + }, { "fieldname": "section_break_meta", "fieldtype": "Section Break", diff --git a/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.py b/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.py index f8e84bf4..58b19fbd 100644 --- a/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.py +++ b/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.py @@ -94,6 +94,31 @@ def create_revision_from_live_tree( return revision +def create_overlay_revision( + base_revision: str, + change_request: str | None = None, + is_working: int = 0, +) -> Document: + """Create an empty overlay revision that inherits items from base_revision.""" + base = frappe.get_doc("Wiki Revision", base_revision) + revision = frappe.new_doc("Wiki Revision") + revision.wiki_space = base.wiki_space + revision.change_request = change_request + revision.parent_revision = base_revision + revision.message = base.message + revision.is_merge = 0 + revision.is_working = 1 if is_working else 0 + revision.is_overlay = 1 + revision.hashes_stale = 0 + revision.created_by = frappe.session.user + revision.created_at = now_datetime() + revision.tree_hash = base.tree_hash + revision.content_hash = base.content_hash + revision.doc_count = base.doc_count + revision.insert() + return revision + + def clone_revision( base_revision: str, change_request: str | None = None, @@ -169,25 +194,37 @@ def get_or_create_content_blob(content: str, content_type: str = "markdown") -> def recompute_revision_hashes(revision: str) -> None: - items = frappe.get_all( - "Wiki Revision Item", - fields=["doc_key", "parent_key", "order_index", "slug", "content_blob", "is_deleted"], - filters={"revision": revision}, - ) + is_overlay = frappe.db.get_value("Wiki Revision", revision, "is_overlay") - blob_names = {item["content_blob"] for item in items if item.get("content_blob")} - blob_hashes = { - blob["name"]: blob["hash"] - for blob in frappe.get_all( - "Wiki Content Blob", - fields=["name", "hash"], - filters={"name": ("in", list(blob_names))}, + if is_overlay: + # For overlay revisions, compute from the effective (merged) item set + effective = get_effective_revision_item_map(revision) + items = list(effective.values()) + # Reuse content_hash already loaded by get_effective_revision_item_map + blob_hashes = { + item["content_blob"]: item.get("content_hash") or "" for item in items if item.get("content_blob") + } + else: + items = frappe.get_all( + "Wiki Revision Item", + fields=["doc_key", "parent_key", "order_index", "slug", "content_blob", "is_deleted"], + filters={"revision": revision}, ) - } + blob_names = {item["content_blob"] for item in items if item.get("content_blob")} + blob_hashes = {} + if blob_names: + blob_hashes = { + blob["name"]: blob["hash"] + for blob in frappe.get_all( + "Wiki Content Blob", + fields=["name", "hash"], + filters={"name": ("in", list(blob_names))}, + ) + } tree_parts = [] content_parts = [] - for item in sorted(items, key=lambda x: x["doc_key"]): + for item in sorted(items, key=lambda x: x.get("doc_key") or ""): if item.get("is_deleted"): continue tree_parts.append( @@ -206,15 +243,15 @@ def recompute_revision_hashes(revision: str) -> None: tree_hash = hashlib.sha256("\n".join(tree_parts).encode("utf-8")).hexdigest() content_hash = hashlib.sha256("\n".join(content_parts).encode("utf-8")).hexdigest() - frappe.db.set_value( - "Wiki Revision", - revision, - { - "tree_hash": tree_hash, - "content_hash": content_hash, - "doc_count": len([item for item in items if not item.get("is_deleted")]), - }, - ) + update_fields = { + "tree_hash": tree_hash, + "content_hash": content_hash, + "doc_count": len([item for item in items if not item.get("is_deleted")]), + } + if is_overlay: + update_fields["hashes_stale"] = 0 + + frappe.db.set_value("Wiki Revision", revision, update_fields) def get_revision_item_map(revision: str) -> dict[str, dict[str, Any]]: @@ -257,6 +294,91 @@ def get_revision_item_map(revision: str) -> dict[str, dict[str, Any]]: return item_map +def get_effective_revision_item_map(revision: str) -> dict[str, dict[str, Any]]: + """Get the effective item map, resolving overlay inheritance. + + For overlay revisions: base items + overlay items (overlay wins). + For full revisions: same as get_revision_item_map(). + """ + rev_info = frappe.db.get_value("Wiki Revision", revision, ["is_overlay", "parent_revision"], as_dict=True) + if not (rev_info and rev_info.is_overlay and rev_info.parent_revision): + return get_revision_item_map(revision) + + base_items = get_revision_item_map(rev_info.parent_revision) + overlay_items = get_revision_item_map(revision) + + effective = dict(base_items) + effective.update(overlay_items) + return effective + + +def ensure_overlay_item(revision: str, doc_key: str) -> str | None: + """Copy-on-write: ensure doc_key has an item in the overlay revision. + + If the item already exists in the overlay, return its name. + If the revision is an overlay and the item exists in the base, copy it. + Returns the item name, or None if the item doesn't exist anywhere. + """ + existing = frappe.db.get_value("Wiki Revision Item", {"revision": revision, "doc_key": doc_key}, "name") + if existing: + return existing + + rev_info = frappe.db.get_value("Wiki Revision", revision, ["is_overlay", "parent_revision"], as_dict=True) + if not (rev_info and rev_info.is_overlay and rev_info.parent_revision): + return None + + base_item = frappe.db.get_value( + "Wiki Revision Item", + {"revision": rev_info.parent_revision, "doc_key": doc_key}, + [ + "doc_key", + "title", + "slug", + "is_group", + "is_published", + "is_external_link", + "external_url", + "parent_key", + "order_index", + "content_blob", + "is_deleted", + ], + as_dict=True, + ) + if not base_item: + return None + + new_item = frappe.new_doc("Wiki Revision Item") + new_item.revision = revision + new_item.doc_key = base_item.doc_key + new_item.title = base_item.title + new_item.slug = base_item.slug + new_item.is_group = base_item.is_group + new_item.is_published = base_item.is_published + new_item.is_external_link = base_item.is_external_link + new_item.external_url = base_item.external_url + new_item.parent_key = base_item.parent_key + new_item.order_index = base_item.order_index + new_item.content_blob = base_item.content_blob + new_item.is_deleted = base_item.is_deleted + new_item.insert() + return new_item.name + + +def mark_hashes_stale(revision: str) -> None: + """Mark a revision's hashes as needing recomputation.""" + frappe.db.set_value("Wiki Revision", revision, "hashes_stale", 1) + + +def ensure_revision_hashes(revision: str) -> None: + """Recompute hashes if they are stale.""" + if not revision: + return + is_stale = frappe.db.get_value("Wiki Revision", revision, "hashes_stale") + if is_stale: + recompute_revision_hashes(revision) + + def build_tree_order(items: dict[str, dict[str, Any]]) -> list[str]: children: dict[str | None, list[str]] = {} for key, item in items.items(): diff --git a/wiki/patches.txt b/wiki/patches.txt index a7b484f0..fcbc2dec 100644 --- a/wiki/patches.txt +++ b/wiki/patches.txt @@ -13,4 +13,5 @@ wiki.wiki.doctype.wiki_page.patches.update_escaped_chars wiki.wiki.doctype.wiki_space.patches.wiki_navbar_app_switcher_migration wiki.wiki.doctype.wiki_space.patches.v3.migrate_to_new_tree_document_structure wiki.wiki.doctype.wiki_space.patches.v3.assign_wiki_user_to_active_users -wiki.wiki.doctype.wiki_space.patches.v3.migrate_orphan_pages_to_wiki_document \ No newline at end of file +wiki.wiki.doctype.wiki_space.patches.v3.migrate_orphan_pages_to_wiki_document +wiki.frappe_wiki.doctype.wiki_revision.patches.convert_open_crs_to_overlay \ No newline at end of file From 63117defbc369b06f295bde0a331626ad986ca9d Mon Sep 17 00:00:00 2001 From: Hussain Nagaria Date: Tue, 17 Feb 2026 00:04:04 +0530 Subject: [PATCH 016/128] refactor: optimized merge with fast-forward path and delta-only apply (Phase 5) Merge now only processes changed doc_keys instead of the entire tree. Fast-forward merges skip 3-way logic entirely. Content-only changes use frappe.db.set_value() to bypass validation. For a 500-page wiki with 1 changed page, this reduces merge from O(N) to O(changed). Co-Authored-By: Claude Opus 4.6 --- wiki/REFACTOR.md | 4 +- .../test_wiki_change_request.py | 103 ++++++ .../wiki_change_request.py | 300 ++++++++++++++++-- 3 files changed, 384 insertions(+), 23 deletions(-) diff --git a/wiki/REFACTOR.md b/wiki/REFACTOR.md index 1989d2ba..af9e26c7 100644 --- a/wiki/REFACTOR.md +++ b/wiki/REFACTOR.md @@ -371,12 +371,14 @@ Add patch to `patches.txt`. --- -### Phase 5: Optimized Merge +### Phase 5: Optimized Merge ✅ DONE **Goal**: Merge only processes changed doc_keys. `apply_merge_revision` only touches changed documents. **Depends on**: Phase 4 (overlay revisions make changed-key detection trivial). +**Status**: Completed. Added fast-forward merge path (`_fast_forward_merge`) for when no concurrent changes exist. Added optimized three-way merge (`_three_way_merge`) that only loads content for changed keys. Added `_find_changed_keys()` for O(1) metadata comparison without loading content. Added `_classify_changes()` to separate content-only, structural, added, and deleted changes. Replaced `apply_merge_revision` with `_apply_merge_changes_only()` that only touches changed documents and uses `frappe.db.set_value()` for content-only changes. Three new tests added and all 43 tests pass. + **Changes**: 1. **`wiki_change_request.py`** — Add fast-forward path to `merge_change_request()`: diff --git a/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py b/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py index f2b800e9..1cdf4ab5 100644 --- a/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py +++ b/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py @@ -828,6 +828,109 @@ def test_overlay_merge_works(self): self.assertEqual(cr_doc.status, "Merged") self.assertIsNotNone(cr_doc.merge_revision) + # --- Phase 5: Optimized merge tests --- + + def test_fast_forward_merge_only_updates_changed_docs(self): + """Phase 5: Fast-forward merge only touches documents that changed.""" + space = create_test_wiki_space() + pages = [] + for i in range(5): + pages.append( + create_test_wiki_document(space.root_group, title=f"Page {i}", content=f"content-{i}") + ) + + cr = create_change_request(space.name, "CR fast-forward") + + # Only edit page 2 + target_key = frappe.get_value("Wiki Document", pages[2].name, "doc_key") + update_cr_page(cr.name, target_key, {"content": "updated-content"}) + + # Snapshot state before merge + snapshot = {} + for page in pages: + page.reload() + snapshot[page.name] = { + "title": page.title, + "content": page.content, + "route": page.route, + "modified": page.modified, + } + + merge_change_request(cr.name) + + # Only page 2 should have new content; others should be completely untouched + for page in pages: + page.reload() + if page.name == pages[2].name: + self.assertEqual(page.content, "updated-content") + else: + self.assertEqual(page.content, snapshot[page.name]["content"]) + self.assertEqual(page.route, snapshot[page.name]["route"]) + self.assertEqual(page.title, snapshot[page.name]["title"]) + self.assertEqual( + page.modified, + snapshot[page.name]["modified"], + f"Unchanged doc '{page.title}' should not have been modified", + ) + + def test_optimized_three_way_merge_produces_correct_result(self): + """Phase 5: Three-way merge with concurrent changes produces correct results.""" + space = create_test_wiki_space() + pages = [] + for i in range(5): + pages.append( + create_test_wiki_document( + space.root_group, title=f"Page {i}", content="line1\nline2\nline3\n" + ) + ) + + cr = create_change_request(space.name, "CR three-way") + + # CR edits page 0 + page0_key = frappe.get_value("Wiki Document", pages[0].name, "doc_key") + update_cr_page(cr.name, page0_key, {"content": "line1-cr\nline2\nline3\n"}) + + # Main edits page 1 (advance main_revision) + pages[1].content = "page1-main-edit" + pages[1].save() + new_main = create_revision_from_live_tree(space.name, message="main update") + frappe.db.set_value("Wiki Space", space.name, "main_revision", new_main.name) + + merge_change_request(cr.name) + + # Page 0 should have CR's changes + pages[0].reload() + self.assertEqual(pages[0].content, "line1-cr\nline2\nline3\n") + + # Page 1 should retain main's changes + pages[1].reload() + self.assertEqual(pages[1].content, "page1-main-edit") + + # Pages 2-4 should be untouched + for i in range(2, 5): + pages[i].reload() + self.assertEqual(pages[i].content, "line1\nline2\nline3\n") + + def test_content_only_change_preserves_route(self): + """Phase 5: Content-only changes use fast path and don't affect routes.""" + space = create_test_wiki_space() + group = create_test_wiki_document(space.root_group, title="Guide", is_group=1) + page = create_test_wiki_document(group.name, title="Installation", content="v1") + + page.reload() + route_before = page.route + self.assertTrue(route_before) + + cr = create_change_request(space.name, "CR content only") + page_key = frappe.get_value("Wiki Document", page.name, "doc_key") + update_cr_page(cr.name, page_key, {"content": "v2 - updated content"}) + + merge_change_request(cr.name) + + page.reload() + self.assertEqual(page.content, "v2 - updated content") + self.assertEqual(page.route, route_before, "Route should not change for content-only edit") + # Helpers diff --git a/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py b/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py index 43a98b00..c51cb4df 100644 --- a/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py +++ b/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py @@ -814,19 +814,72 @@ def merge_change_request(name: str) -> str: cr = frappe.get_doc("Wiki Change Request", name) space = frappe.get_doc("Wiki Space", cr.wiki_space) + if cr.base_revision == space.main_revision: + return _fast_forward_merge(cr, space) + return _three_way_merge(cr, space) + + +@frappe.whitelist() +def check_outdated(name: str) -> int: + cr = frappe.get_doc("Wiki Change Request", name) + main_revision = frappe.get_value("Wiki Space", cr.wiki_space, "main_revision") + outdated = 1 if main_revision and main_revision != cr.base_revision else 0 + frappe.db.set_value("Wiki Change Request", cr.name, "outdated", outdated) + return outdated + + +def _fast_forward_merge(cr: Document, space: Document) -> str: + """Fast-forward merge: no concurrent changes since CR was created.""" + base_items = get_revision_item_map(cr.base_revision) + effective_items = get_effective_revision_item_map(cr.head_revision) + + merged_items: dict[str, dict[str, Any]] = {} + for key, item in effective_items.items(): + normalized = normalize_item(item) + if normalized: + merged_items[key] = normalized + + _validate_no_duplicate_leaf_slugs(merged_items) + merge_revision = create_merge_revision(cr, merged_items) + + frappe.flags.in_apply_merge_revision = True + try: + _apply_merge_changes_only(space, merge_revision, base_items) + finally: + frappe.flags.in_apply_merge_revision = False + + _finalize_merge(cr, merge_revision) + return merge_revision.name + + +def _three_way_merge(cr: Document, space: Document) -> str: + """Three-way merge: concurrent changes require conflict resolution.""" base_items = get_revision_item_map(cr.base_revision) ours_items = get_revision_item_map(space.main_revision) theirs_items = get_effective_revision_item_map(cr.head_revision) - base_contents = get_contents_for_items(base_items) - ours_contents = get_contents_for_items(ours_items) - theirs_contents = get_contents_for_items(theirs_items) - conflicts = [] - merged_items: dict[str, dict[str, Any]] = {} + # Only load content for keys that actually changed in either branch + main_changed = _find_changed_keys(base_items, ours_items) + head_changed = _find_changed_keys(base_items, theirs_items) + changed_keys = main_changed | head_changed - all_keys = set(base_items) | set(ours_items) | set(theirs_items) + base_subset = {k: base_items[k] for k in changed_keys if k in base_items} + ours_subset = {k: ours_items[k] for k in changed_keys if k in ours_items} + theirs_subset = {k: theirs_items[k] for k in changed_keys if k in theirs_items} - for key in all_keys: + base_contents = get_contents_for_items(base_subset) + ours_contents = get_contents_for_items(ours_subset) + theirs_contents = get_contents_for_items(theirs_subset) + + # Start with current main state as baseline for merged result + merged_items: dict[str, dict[str, Any]] = {} + for key, item in ours_items.items(): + normalized = normalize_item(item) + if normalized: + merged_items[key] = normalized + + conflicts = [] + for key in changed_keys: base = normalize_item(base_items.get(key)) ours = normalize_item(ours_items.get(key)) theirs = normalize_item(theirs_items.get(key)) @@ -842,6 +895,8 @@ def merge_change_request(name: str) -> str: continue if result: merged_items[key] = result + else: + merged_items.pop(key, None) if conflicts: for conflict in conflicts: @@ -855,35 +910,236 @@ def merge_change_request(name: str) -> str: conflict_doc.status = "Open" conflict_doc.insert(ignore_permissions=True) - # In web requests, an exception triggers a rollback which would erase the - # inserted conflict rows. Persist them so the UI can show details. if not frappe.flags.in_test: frappe.db.commit() frappe.throw(_("Merge conflicts detected"), frappe.ValidationError) _validate_no_duplicate_leaf_slugs(merged_items) - merge_revision = create_merge_revision(cr, merged_items) - apply_merge_revision(space, merge_revision) + frappe.flags.in_apply_merge_revision = True + try: + _apply_merge_changes_only(space, merge_revision, ours_items) + finally: + frappe.flags.in_apply_merge_revision = False + + _finalize_merge(cr, merge_revision) + return merge_revision.name + + +def _find_changed_keys( + base_items: dict[str, dict[str, Any]], other_items: dict[str, dict[str, Any]] +) -> set[str]: + """Find doc_keys that differ between two revision item maps. + + Compares metadata and content_blob without loading actual content text. + """ + changed: set[str] = set() + all_keys = set(base_items) | set(other_items) + compare_fields = [ + "title", + "slug", + "parent_key", + "order_index", + "is_group", + "is_published", + "is_external_link", + "external_url", + "content_blob", + "is_deleted", + ] + + for key in all_keys: + base = base_items.get(key) + other = other_items.get(key) + if base is None or other is None: + changed.add(key) + continue + + for field in compare_fields: + if base.get(field) != other.get(field): + changed.add(key) + break + + return changed + + +def _classify_changes( + prev_items: dict[str, dict[str, Any]], + new_items: dict[str, dict[str, Any]], + changed_keys: set[str], +) -> tuple[set[str], set[str], set[str], set[str]]: + """Classify changed keys into (content_only, structural, added, deleted).""" + content_only: set[str] = set() + structural: set[str] = set() + added: set[str] = set() + deleted: set[str] = set() + metadata_fields = [ + "title", + "slug", + "parent_key", + "order_index", + "is_group", + "is_published", + "is_external_link", + "external_url", + ] + + for key in changed_keys: + prev = prev_items.get(key) + new = new_items.get(key) + + prev_exists = prev is not None and not prev.get("is_deleted") + new_exists = new is not None and not new.get("is_deleted") + + if not prev_exists and new_exists: + added.add(key) + elif prev_exists and not new_exists: + deleted.add(key) + elif prev_exists and new_exists: + if all(prev.get(f) == new.get(f) for f in metadata_fields): + content_only.add(key) + else: + structural.add(key) + + return content_only, structural, added, deleted + + +def _apply_merge_changes_only( + space: Document, + merge_revision: Document, + prev_items: dict[str, dict[str, Any]], +) -> None: + """Apply only changed documents to the live tree. + + Instead of loading and saving every document, finds the delta between + prev_items (the old main_revision state) and the merge_revision, then: + - Deletes removed docs + - Uses frappe.db.set_value for content-only changes (skips validation) + - Does full doc.save() only for structural changes and additions + """ + new_items = get_revision_item_map(merge_revision.name) + changed_keys = _find_changed_keys(prev_items, new_items) + + if not changed_keys: + frappe.db.set_value("Wiki Space", space.name, "main_revision", merge_revision.name) + return + + root_doc_key = frappe.get_value("Wiki Document", space.root_group, "doc_key") + + root_lft, root_rgt = frappe.get_value("Wiki Document", space.root_group, ["lft", "rgt"]) + space_docs = frappe.get_all( + "Wiki Document", + fields=["name", "doc_key"], + filters=[ + ["lft", ">=", root_lft], + ["rgt", "<=", root_rgt], + ["doc_key", "is", "set"], + ], + ) + key_to_name = {doc["doc_key"]: doc["name"] for doc in space_docs} + + content_only_keys, structural_keys, added_keys, deleted_keys = _classify_changes( + prev_items, new_items, changed_keys + ) + + # Handle deletions + for doc_key in deleted_keys: + if doc_key == root_doc_key: + continue + if doc_key in key_to_name: + frappe.delete_doc("Wiki Document", key_to_name[doc_key], force=True) + del key_to_name[doc_key] + + # Batch-load content blobs for all non-deleted changed items (1 query) + need_content_keys = content_only_keys | structural_keys | added_keys + blob_names: set[str] = set() + for k in need_content_keys: + item = new_items.get(k) + if item and item.get("content_blob"): + blob_names.add(item["content_blob"]) + + blob_contents: dict[str, str] = {} + if blob_names: + blobs = frappe.get_all( + "Wiki Content Blob", + fields=["name", "content"], + filters={"name": ("in", list(blob_names))}, + ) + blob_contents = {blob["name"]: blob.get("content") or "" for blob in blobs} + + # Content-only fast path: direct DB update, skip doc.save() validation + for doc_key in content_only_keys: + if doc_key not in key_to_name: + continue + item = new_items[doc_key] + content_blob = item.get("content_blob") + content = blob_contents.get(content_blob, "") if content_blob else "" + frappe.db.set_value("Wiki Document", key_to_name[doc_key], "content", content) + + # Structural changes and additions need full save (process in tree order) + full_save_keys = structural_keys | added_keys + if full_save_keys: + ordered_keys = build_tree_order(new_items) + for doc_key in ordered_keys: + if doc_key not in full_save_keys: + continue + + item = new_items[doc_key] + if item.get("is_deleted"): + continue + + parent_name = None + if doc_key == root_doc_key: + parent_name = None + elif item.get("parent_key"): + parent_name = key_to_name.get(item.get("parent_key")) + elif space.root_group: + parent_name = space.root_group + + if doc_key in key_to_name: + doc = frappe.get_doc("Wiki Document", key_to_name[doc_key]) + else: + existing_name = frappe.db.get_value("Wiki Document", {"doc_key": doc_key}, "name") + if existing_name: + doc = frappe.get_doc("Wiki Document", existing_name) + else: + doc = frappe.new_doc("Wiki Document") + doc.doc_key = doc_key + + doc.title = item.get("title") + doc.slug = item.get("slug") or cleanup_page_name(item.get("title") or "") + doc.is_group = item.get("is_group") + doc.is_published = item.get("is_published") + doc.is_external_link = item.get("is_external_link") + doc.external_url = item.get("external_url") + if doc_key == root_doc_key: + doc.parent_wiki_document = None + else: + doc.parent_wiki_document = parent_name or space.root_group + doc.sort_order = item.get("order_index") or 0 + + content_blob = item.get("content_blob") + doc.content = blob_contents.get(content_blob, "") if content_blob else "" + + if doc.is_new(): + doc.insert() + key_to_name[doc_key] = doc.name + else: + doc.save() + + frappe.db.set_value("Wiki Space", space.name, "main_revision", merge_revision.name) + + +def _finalize_merge(cr: Document, merge_revision: Document) -> None: + """Update CR status after successful merge.""" cr.status = "Merged" cr.merge_revision = merge_revision.name cr.merged_by = frappe.session.user cr.merged_at = now_datetime() cr.save() - return merge_revision.name - - -@frappe.whitelist() -def check_outdated(name: str) -> int: - cr = frappe.get_doc("Wiki Change Request", name) - main_revision = frappe.get_value("Wiki Space", cr.wiki_space, "main_revision") - outdated = 1 if main_revision and main_revision != cr.base_revision else 0 - frappe.db.set_value("Wiki Change Request", cr.name, "outdated", outdated) - return outdated - def normalize_item(item: dict[str, Any] | None) -> dict[str, Any] | None: if not item or item.get("is_deleted"): From e7674ba23448079ffc0e7315e12e40dbd6113c08 Mon Sep 17 00:00:00 2001 From: Hussain Nagaria Date: Tue, 17 Feb 2026 00:13:39 +0530 Subject: [PATCH 017/128] chore: trigger merge status refresh From e0c62dc410520a7db6d2863161ab91120886710a Mon Sep 17 00:00:00 2001 From: Hussain Nagaria Date: Tue, 17 Feb 2026 07:14:25 +0530 Subject: [PATCH 018/128] fix: delete wiki documents after saves to prevent NestedSetChildExistsError When merging a CR, deletions must happen after structural saves so that children are reparented away before their parent group is deleted. Extracted shared deletion logic into _delete_wiki_documents helper. Co-Authored-By: Claude Opus 4.6 --- .../test_wiki_change_request.py | 77 +++++++++++++++++++ .../wiki_change_request.py | 38 ++++++--- 2 files changed, 103 insertions(+), 12 deletions(-) diff --git a/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py b/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py index 407728e1..6ec4eab7 100644 --- a/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py +++ b/wiki/frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py @@ -797,6 +797,83 @@ def test_content_only_change_preserves_route(self): self.assertEqual(page.content, "v2 - updated content") self.assertEqual(page.route, route_before, "Route should not change for content-only edit") + def test_merge_with_orphan_group_in_live_tree(self): + """Merge succeeds when live tree has groups not tracked in any revision. + + Reproduces production bug: intermediate group docs inserted directly into + the live tree (bypassing revisions) caused NestedSetChildExistsError because + apply_merge_revision tried to delete parents before reparenting children. + """ + space = create_test_wiki_space() + stock = create_test_wiki_document(space.root_group, title="Stock", is_group=1) + page_a = create_test_wiki_document(stock.name, title="Page A", content="v1") + page_b = create_test_wiki_document(stock.name, title="Page B", content="v1") + + # Snapshot the tree into main_revision — pages are children of Stock + cr_init = create_change_request(space.name, "Init") + merge_change_request(cr_init.name) + + # Now insert an intermediate group directly into the live tree (bypassing revisions), + # and reparent the pages under it — simulating the production data inconsistency. + orphan_group = frappe.new_doc("Wiki Document") + orphan_group.title = "Help Articles" + orphan_group.is_group = 1 + orphan_group.parent_wiki_document = stock.name + frappe.flags.in_apply_merge_revision = True + try: + orphan_group.insert() + finally: + frappe.flags.in_apply_merge_revision = False + + # Reparent page_a under the orphan group in the live tree + page_a.reload() + page_a.parent_wiki_document = orphan_group.name + frappe.flags.in_apply_merge_revision = True + try: + page_a.save() + finally: + frappe.flags.in_apply_merge_revision = False + + # Create a CR that makes a trivial content change — no deletions + cr = create_change_request(space.name, "Trivial edit") + page_b_key = frappe.get_value("Wiki Document", page_b.name, "doc_key") + update_cr_page(cr.name, page_b_key, {"content": "v2"}) + + # This should NOT raise NestedSetChildExistsError + merge_change_request(cr.name) + + page_b.reload() + self.assertEqual(page_b.content, "v2") + + def test_merge_delete_group_with_reparented_children(self): + """Merge deletes a group whose children are reparented in the same CR. + + If deletions ran before saves, the parent would still have children attached + and frappe.delete_doc would raise NestedSetChildExistsError. + """ + space = create_test_wiki_space() + group = create_test_wiki_document(space.root_group, title="Old Group", is_group=1) + page = create_test_wiki_document(group.name, title="Page Under Group", content="v1") + + cr_init = create_change_request(space.name, "Init") + merge_change_request(cr_init.name) + + root_key = frappe.get_value("Wiki Document", space.root_group, "doc_key") + page_key = frappe.get_value("Wiki Document", page.name, "doc_key") + group_key = frappe.get_value("Wiki Document", group.name, "doc_key") + + # Create a CR that reparents the page to root and deletes the group + cr = create_change_request(space.name, "Reparent + delete") + move_cr_page(cr.name, page_key, new_parent_key=root_key) + delete_cr_page(cr.name, group_key) + + # This should NOT raise NestedSetChildExistsError + merge_change_request(cr.name) + + page.reload() + self.assertEqual(page.parent_wiki_document, space.root_group) + self.assertFalse(frappe.db.exists("Wiki Document", group.name)) + # Helpers diff --git a/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py b/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py index 554271ec..0b922237 100644 --- a/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py +++ b/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py @@ -827,6 +827,22 @@ def _find_changed_keys( return changed +def _delete_wiki_documents(doc_keys: Iterable[str], key_to_name: dict[str, str], root_doc_key: str) -> None: + """Delete Wiki Documents by doc_key, children-first (highest lft first).""" + delete_docs = [] + for doc_key in doc_keys: + if doc_key == root_doc_key: + continue + if doc_key in key_to_name: + doc_name = key_to_name[doc_key] + lft = frappe.db.get_value("Wiki Document", doc_name, "lft") or 0 + delete_docs.append((lft, doc_key, doc_name)) + delete_docs.sort(key=lambda x: x[0], reverse=True) + for _lft, doc_key, doc_name in delete_docs: + frappe.delete_doc("Wiki Document", doc_name, force=True) + del key_to_name[doc_key] + + def _classify_changes( prev_items: dict[str, dict[str, Any]], new_items: dict[str, dict[str, Any]], @@ -906,14 +922,6 @@ def _apply_merge_changes_only( prev_items, new_items, changed_keys ) - # Handle deletions - for doc_key in deleted_keys: - if doc_key == root_doc_key: - continue - if doc_key in key_to_name: - frappe.delete_doc("Wiki Document", key_to_name[doc_key], force=True) - del key_to_name[doc_key] - # Batch-load content blobs for all non-deleted changed items (1 query) need_content_keys = content_only_keys | structural_keys | added_keys blob_names: set[str] = set() @@ -991,6 +999,9 @@ def _apply_merge_changes_only( else: doc.save() + # Delete AFTER saves — children must be reparented before parents are deleted + _delete_wiki_documents(deleted_keys, key_to_name, root_doc_key) + frappe.db.set_value("Wiki Space", space.name, "main_revision", merge_revision.name) @@ -1328,13 +1339,13 @@ def _apply_merge_revision(space: Document, revision: Document) -> None: ) key_to_name = {doc["doc_key"]: doc["name"] for doc in space_docs} - # Handle deletions: docs that exist in space but not in merge revision - for doc_key, doc_name in list(key_to_name.items()): + # Collect docs to delete (exist in space but not in merge revision) + delete_doc_keys: set[str] = set() + for doc_key in list(key_to_name): if doc_key == root_doc_key: continue if doc_key not in items: - frappe.delete_doc("Wiki Document", doc_name, force=True) - del key_to_name[doc_key] + delete_doc_keys.add(doc_key) for doc_key in ordered_keys: item = items[doc_key] @@ -1383,4 +1394,7 @@ def _apply_merge_revision(space: Document, revision: Document) -> None: else: doc.save() + # Delete AFTER saves — children must be reparented before parents are deleted + _delete_wiki_documents(delete_doc_keys, key_to_name, root_doc_key) + frappe.db.set_value("Wiki Space", space.name, "main_revision", revision.name) From 1065401702b8579494b69666a568c2a9dd6ec0e8 Mon Sep 17 00:00:00 2001 From: Hussain Nagaria <34810212+NagariaHussain@users.noreply.github.com> Date: Tue, 17 Feb 2026 07:16:46 +0530 Subject: [PATCH 019/128] chore: delete docs file --- wiki/REFACTOR.md | 533 ----------------------------------------------- 1 file changed, 533 deletions(-) delete mode 100644 wiki/REFACTOR.md diff --git a/wiki/REFACTOR.md b/wiki/REFACTOR.md deleted file mode 100644 index af9e26c7..00000000 --- a/wiki/REFACTOR.md +++ /dev/null @@ -1,533 +0,0 @@ -# Wiki Contribution & Merge Flow — Refactor Plan - -## Executive Summary - -The wiki app's contribution and merge system is overengineered for what it does. A git-style full-tree-snapshot revision model was built, but without the optimizations that make git fast. The result: - -- **Data integrity bug**: Desk edits bypass the revision system entirely — changes made from Frappe's admin UI are silently lost when CRs are created or merged. -- **O(N) operations that should be O(1)**: Creating a Change Request clones every revision item in the space (500 pages → 500 DB inserts for a 1-page edit). Every CR mutation recomputes hashes over the entire tree. -- **O(N) merge for O(1) changes**: Merging loads all content blobs for 3 full revisions and iterates every doc_key, even when only 1 page changed. Then `apply_merge_revision` does individual `get_doc()` + `save()` for every document in the tree. -- **4 overlapping merge strategies** tried in sequence with a duplicated retry block — hard to reason about, hard to test. -- **Dead code** from the old `WikiPagePatch` system still ships. - -This document proposes 6 independently-shippable phases to fix all of the above. - ---- - -## Current Architecture - -### Data Model - -``` -Wiki Space - ├── root_group → Wiki Document (NestedSet root) - ├── main_revision → Wiki Revision (current published state) - └── route - -Wiki Document (NestedSet) - ├── doc_key (immutable identifier) - ├── title, slug, route - ├── content (live markdown) - ├── parent_wiki_document, lft, rgt, sort_order - └── is_group, is_published - -Wiki Revision (full tree snapshot) - ├── wiki_space, change_request - ├── parent_revision - ├── tree_hash, content_hash - ├── is_working, is_merge - └── has N → Wiki Revision Items - -Wiki Revision Item (one per doc per revision) - ├── revision (link to Wiki Revision) - ├── doc_key, title, slug - ├── parent_key, order_index - ├── content_blob → Wiki Content Blob - └── is_deleted, is_group, is_published - -Wiki Content Blob (content-addressed storage) - ├── hash (SHA256) - └── content (markdown text) - -Wiki Change Request - ├── base_revision (snapshot when CR was created) - ├── head_revision (working revision with edits) - ├── merge_revision (created on merge) - └── status: Draft → In Review → Approved → Merged -``` - -### Edit Flows - -**Editor UI (Change Request flow):** -``` -User types → WikiEditor.vue auto-save → update_cr_page() API - → updates Wiki Revision Item in head_revision - → updates Wiki Content Blob - → recompute_revision_hashes() over entire tree -``` - -**Desk (direct save — NO revision tracking):** -``` -User edits Wiki Document in Frappe admin → doc.save() - → WikiDocument.validate() runs (slug, route, etc.) - → saves directly to tabWiki Document - → revision system is NEVER notified -``` - -**Merge flow:** -``` -merge_change_request() - → load ALL items for base_revision, main_revision, head_revision - → load ALL content blobs for all 3 revisions - → iterate every doc_key across all 3 sets - → for each: try 4 merge strategies in sequence - → create merge_revision with ALL items - → apply_merge_revision: load + save EVERY Wiki Document individually -``` - -### Key Files - -| File | Lines | Purpose | -|------|-------|---------| -| `frappe_wiki/doctype/wiki_change_request/wiki_change_request.py` | 1341 | CR CRUD, diff, 3-way merge, 4 content-merge strategies, apply to live tree | -| `frappe_wiki/doctype/wiki_revision/wiki_revision.py` | 278 | Revision creation, cloning, hash computation, content blobs | -| `frappe_wiki/doctype/wiki_document/wiki_document.py` | 594 | Wiki Document model, validation, rendering, tree building | -| `api/wiki_space.py` | 225 | Live tree reorder, sync to revision system | -| `frappe_wiki/doctype/wiki_change_request/test_wiki_change_request.py` | 580 | 20 test cases covering CR lifecycle | - ---- - -## Findings - -### P1: Desk Edits Bypass Revision System (Critical — Data Integrity) - -**The problem**: Wiki Document can be edited directly from Frappe Desk. There is no `doc_events` hook for `Wiki Document` in `hooks.py:111-113` — only `User` has a hook. The only place that syncs desk edits to the revision system is `_sync_main_revision_for_space()` in `api/wiki_space.py:155-170`, which is called **only** from `reorder_wiki_documents()` — never from normal content edits. - -**Impact**: Team members edit Wiki Documents from Desk. The Editor UI builds its tree from `Wiki Revision Item` records (`get_cr_tree()` at `wiki_change_request.py:268-360`). These items are cloned from `main_revision`, which was never updated after the Desk edit. The desk changes are invisible to CRs and are overwritten on the next merge. - -**Root cause**: No `on_update` / `on_trash` doc_event for `Wiki Document`. - -### P2: Full-Tree Clone on CR Creation (Performance) - -**The problem**: `create_change_request()` at `wiki_change_request.py:410-430` calls `clone_revision()` at `wiki_revision.py:97-150`. This copies **every** `Wiki Revision Item` from the base revision to a new working revision. - -**Impact**: For a wiki space with 500 pages, creating a CR inserts 500 `Wiki Revision Item` records even though the user may only edit 1 page. Each insert is an individual `frappe.new_doc().insert()` call — no bulk operations. - -**Compounding factor**: Every subsequent CR operation (`update_cr_page`, `create_cr_page`, `move_cr_page`, `delete_cr_page`, `reorder_cr_children`) calls `recompute_revision_hashes()` at `wiki_revision.py:171-217`, which queries ALL items in the revision plus ALL their content blobs, every single time. - -### P3: Full-Tree Merge Processing (Performance) - -**The problem**: `merge_change_request()` at `wiki_change_request.py:785-887`: - -1. Loads ALL items for 3 revisions via `get_revision_item_map()` — 3 queries returning N rows each -2. Loads ALL content blobs for all 3 via `get_contents_for_items()` at lines 795-797 — 3 bulk queries returning N content strings each -3. Iterates `set(base_items) | set(ours_items) | set(theirs_items)` — processes every doc_key -4. For every doc_key, runs normalization + comparison + merge logic - -Then `apply_merge_revision()` at lines 1267-1341: -1. Creates a new merge revision with ALL items (N inserts) -2. Gets all space docs (1 query) -3. For each of N doc_keys: `frappe.get_doc()` + set fields + `doc.save()` — that's N doc loads + N validates + N saves - -**Impact**: Merging a 1-page content change on a 500-page wiki loads ~1500 revision items, ~1500 content blobs, creates 500 new revision items, and individually loads + saves 500 Wiki Documents. This is where the "merge is slow" complaints come from. - -### P4: Redundant Content Merge Strategies (Complexity) - -**The problem**: There are 4 content-merge functions at `wiki_change_request.py:1034-1131`: - -1. `line_merge_fallback()` — same-length only, rstrip comparison -2. `merge_content_linewise()` — diff-based, `splitlines()` (strips endings) -3. `merge_content_disjoint()` — diff-based, `splitlines(keepends=True)` -4. `merge_content()` — tries same-length first, then diff-based - -They are called in a waterfall at `wiki_change_request.py:966-977`: -```python -merged_content, line_ok = line_merge_fallback(...) -if not line_ok: - merged_content, conflict = merge_content_linewise(...) - if conflict: - merged_content = merge_content_disjoint(...) - if merged_content is None: - merged_content, conflict = merge_content(...) -``` - -Then there's a **duplicated retry** of the exact same waterfall at lines 813-848 inside `merge_change_request()` — if `merge_items()` returns a `"content"` conflict, the same 4 strategies are tried again. - -**Impact**: `merge_content()` already subsumes the logic of the other 3. The duplication makes the code ~100 lines longer than needed and very hard to follow. When someone needs to fix a merge bug, they have to understand all 4 strategies plus the retry block. - -### P5: N+1 Query Pattern in apply_merge_revision (Performance) - -**The problem**: `apply_merge_revision()` at `wiki_change_request.py:1267-1341` processes every doc_key individually: - -```python -for doc_key in ordered_keys: - # ... - if doc_key in key_to_name: - doc = frappe.get_doc("Wiki Document", key_to_name[doc_key]) # DB read - else: - doc = frappe.new_doc("Wiki Document") - doc.title = item.get("title") - # ... set fields ... - content = frappe.get_value("Wiki Content Blob", content_blob, "content") # DB read - doc.content = content - doc.save() # DB write + validate -``` - -For N documents: N `get_doc` calls + N `get_value` calls for content + N `save()` calls (each triggering `validate()` with route computation, unique-route checks, etc.). - -**Impact**: This is the main bottleneck in merge. Content blobs could be batch-loaded in 1 query. Documents that haven't changed don't need to be loaded or saved at all. - -### P6: Dead Code — WikiPagePatch (Tech Debt) - -**The problem**: `wiki/wiki/doctype/wiki_page_patch/wiki_page_patch.py` (116 lines) references a `"Wiki Page"` doctype that no longer exists (replaced by `Wiki Document` in the v3 architecture). It imports `apply_changes`, `apply_markdown_diff`, `highlight_changes` from `wiki.utils` — these are dead utility functions. - -**Impact**: Confusing for new contributors. The old merge logic in `update_old_page()` at line 62 uses a completely different approach from the current 3-way merge, creating a false impression of how the system works. - -### P7: Overcomplicated Draft CR Logic (Complexity) - -**The problem**: `get_or_create_draft_change_request()` at `wiki_change_request.py:157-216` is 60 lines with: -- Loop through all user's drafts checking hash equality for each -- Inline hash comparison (duplicates `has_revision_changes()` logic) -- Auto-archive logic for stale drafts with no changes -- Creates new CR inside the stale-draft branch - -This function is called **every time the editor opens**. - -**Impact**: Hard to follow, hard to test. The inline hash comparison at lines 198-201 duplicates the logic already in `has_revision_changes()` at lines 129-153. - ---- - -## Refactor Plan - -### Phase 1: Desk-to-Revision Sync ✅ DONE - -**Goal**: Any edit to a Wiki Document through Desk automatically updates `main_revision`. - -**Priority**: Critical — this is the only data-integrity bug. - -**Status**: Completed. Added `Wiki Document` doc_events (`on_update`, `on_trash`) in `hooks.py`. Hook handlers in `wiki_document.py` sync desk edits to the revision system via `_sync_main_revision_for_space()`. Guard flags in `apply_merge_revision()` and `reorder_wiki_documents()` prevent infinite loops. Three new tests added and all 28 tests pass. - -**Changes**: - -1. **`hooks.py`** — Add `Wiki Document` to `doc_events`: - ```python - doc_events = { - "User": {"after_insert": "wiki.utils.add_wiki_user_role"}, - "Wiki Document": { - "on_update": "wiki.frappe_wiki.doctype.wiki_document.wiki_document.on_wiki_document_update", - "on_trash": "wiki.frappe_wiki.doctype.wiki_document.wiki_document.on_wiki_document_trash", - }, - } - ``` - -2. **`wiki_document.py`** — Add hook handlers: - - `on_wiki_document_update(doc, method)`: Skip if `frappe.flags.in_apply_merge_revision` is set (avoid infinite loop during merge). Otherwise, find the owning Wiki Space and call `_sync_main_revision_for_space()`. - - `on_wiki_document_trash(doc, method)`: Same logic. - - Use a per-request debounce via `frappe.flags` so bulk operations only trigger 1 revision sync per space. - -3. **`wiki_change_request.py`** — Set `frappe.flags.in_apply_merge_revision = True` around the loop in `apply_merge_revision()` (wrap in try/finally). - -4. **`api/wiki_space.py`** — Set `frappe.flags.in_reorder_wiki_documents = True` around `reorder_wiki_documents()` to avoid double-sync. - -**Tests to add**: -- `test_desk_edit_syncs_to_revision_system`: Edit Wiki Document directly, verify `main_revision` updates. -- `test_desk_edit_visible_in_new_cr`: Edit via desk, create CR, verify CR's head_revision includes the desk edit. -- `test_merge_does_not_double_sync`: Merge a CR, verify the `on_update` hook doesn't re-trigger revision creation. - -**Risk**: Low. Guard flags are a well-understood pattern. The `_sync_main_revision_for_space` function already exists and is tested (it's used by reorder). - ---- - -### Phase 2: Consolidate Merge Strategies ✅ DONE - -**Goal**: Replace 4 overlapping merge functions + duplicated retry block with 1 clean function. - -**Priority**: High — simplifies the hardest-to-understand part of the codebase, makes Phase 5 easier. - -**Status**: Completed. Replaced `line_merge_fallback`, `merge_content_linewise`, `merge_content_disjoint`, and `merge_content` with a single `merge_content_three_way()` function. Deleted the duplicated retry block in `merge_change_request()`. Removed unused helpers `edits_conflict` and `ranges_overlap`. Five new unit tests added and all 33 tests pass. - -**Changes**: - -1. **`wiki_change_request.py`** — New function `merge_content_three_way(base, ours, theirs) -> (str, bool)`: - ``` - Strategy (in order): - 1. Trivial: ours == theirs, or one side == base - 2. Normalize whitespace, re-check trivials - 3. Same-length line-by-line (rstrip comparison for whitespace tolerance) - 4. Diff-based with SequenceMatcher + disjoint-edit detection - 5. If edits overlap → conflict - ``` - -2. **Delete**: `line_merge_fallback`, `merge_content_linewise`, `merge_content_disjoint`, `merge_content` (the old 4 functions). - -3. **Delete**: The duplicated retry block at lines 813-848 of `merge_change_request()`. - -4. **Update `merge_items()`**: Replace the waterfall chain with a single call to `merge_content_three_way()`. - -5. **Delete unused helpers**: `edits_conflict`, `ranges_overlap` — after consolidation, only `edits_disjoint` is needed for the conflict check. - -**Net result**: ~150 lines removed, 1 function to understand instead of 4. - -**Tests to add**: -- `test_merge_content_three_way_trivial_cases` -- `test_merge_content_three_way_same_length_lines` -- `test_merge_content_three_way_different_length_disjoint` -- `test_merge_content_three_way_overlapping_conflict` -- `test_merge_content_three_way_whitespace_tolerance` - -All existing merge tests must continue to pass (they test the end-to-end flow, not the internal functions). - -**Risk**: Low. Pure refactor with no schema or API changes. Existing test coverage validates correctness. - ---- - -### Phase 3: Dead Code Removal ✅ DONE - -**Goal**: Remove `WikiPagePatch` active code and dead utility functions. - -**Priority**: Low effort, low risk, reduces confusion. - -**Status**: Completed. WikiPagePatch replaced with stub class. Removed `apply_changes`, `apply_markdown_diff`, `highlight_changes` from `wiki/utils.py`. Cleaned up `wiki_page_patch.js`. - -**Changes**: - -1. **`wiki/wiki/doctype/wiki_page_patch/wiki_page_patch.py`** — Replace with stub: - ```python - from frappe.model.document import Document - - class WikiPagePatch(Document): - pass - ``` - Keep the doctype JSON and `__init__.py` for backward compatibility (existing databases may have old records). - -2. **`wiki/utils.py`** — Remove `apply_changes`, `apply_markdown_diff`, `highlight_changes` if they have no other callers (verify with grep first). - -**Tests**: Existing tests should pass unchanged. No new tests needed. - -**Risk**: Very low. The old code references `"Wiki Page"` which doesn't exist in the current schema. - ---- - -### Phase 4: Delta-Based "Overlay" Revisions ✅ DONE - -**Goal**: CR working revisions only store items that differ from the base. CR creation goes from O(N) to O(1). - -**Priority**: High impact on performance, but larger change — do after Phases 1-3 are stable. - -**Status**: Completed. CR creation now produces an empty overlay revision (0 items) instead of cloning all N items. Copy-on-write via `ensure_overlay_item()` promotes items to the overlay only when mutated. All read functions (`get_cr_tree`, `get_cr_page`, `diff_change_request`) use `get_effective_revision_item_map()` to merge base + overlay. Lazy hash computation defers `recompute_revision_hashes()` until needed (before merge/diff checks). Data migration patch converts existing open CRs. Five new tests added and all 40 tests pass. - -**Design: Overlay Revisions** - -Instead of `clone_revision()` copying all N items, a CR's `head_revision` starts empty and only accumulates items as they're edited. Reading the full tree = base items + overlay items (overlay wins). - -**Schema change**: - -Add to `Wiki Revision`: -``` -is_overlay (Check, default 0) — if set, this revision inherits from parent_revision -hashes_stale (Check, default 0, hidden) — if set, tree_hash/content_hash need recomputation -``` - -**Changes**: - -1. **`wiki_revision.py`** — New function `create_overlay_revision(base_revision)`: - - Creates a Wiki Revision with `is_overlay=1`, zero items - - Copies `tree_hash`, `content_hash`, `doc_count` from base (initially identical) - -2. **`wiki_revision.py`** — New function `get_effective_revision_item_map(revision)`: - - If `is_overlay`: return `base_items | overlay_items` (overlay wins) - - If not overlay: return `get_revision_item_map(revision)` as before - -3. **`wiki_change_request.py`** — Update `create_change_request()`: - - Replace `clone_revision(base_revision, is_working=1)` with `create_overlay_revision(base_revision, is_working=1)` - -4. **`wiki_change_request.py`** — New helper `_ensure_overlay_item(head_revision, doc_key)`: - - If item exists in overlay → return it - - If not → copy from base revision to overlay → return it - - Used by `update_cr_page`, `delete_cr_page`, `move_cr_page` - -5. **Update all CR read functions** (`get_cr_tree`, `get_cr_page`, `diff_change_request`) to use `get_effective_revision_item_map()`. - -6. **Lazy hash computation**: Replace per-mutation `recompute_revision_hashes()` calls with `frappe.db.set_value(revision, "hashes_stale", 1)`. Add `ensure_revision_hashes()` that recomputes only when needed (before merge, before `has_revision_changes()` check). - -**Data migration**: - -For existing open CRs (Draft / In Review / Changes Requested): -1. Compare head_revision items against base_revision items -2. Delete identical items from head_revision -3. Set `is_overlay = 1` on head_revision - -Add patch to `patches.txt`. - -**Tests to add**: -- `test_create_cr_creates_empty_overlay`: Verify 0 items in head_revision after CR creation -- `test_editing_promotes_item_to_overlay`: Edit 1 page, verify only 1 item in overlay -- `test_overlay_tree_shows_all_pages`: `get_cr_tree()` returns full tree despite empty overlay -- `test_overlay_diff_shows_only_changes`: `diff_change_request()` only shows edited pages -- `test_overlay_merge_works`: Full merge lifecycle with overlay revisions -- All existing tests must pass (they test the same public API) - -**Risk**: Medium. Schema change + migration. The overlay model is well-understood (it's how Docker layers work). The migration only affects open CRs. - ---- - -### Phase 5: Optimized Merge ✅ DONE - -**Goal**: Merge only processes changed doc_keys. `apply_merge_revision` only touches changed documents. - -**Depends on**: Phase 4 (overlay revisions make changed-key detection trivial). - -**Status**: Completed. Added fast-forward merge path (`_fast_forward_merge`) for when no concurrent changes exist. Added optimized three-way merge (`_three_way_merge`) that only loads content for changed keys. Added `_find_changed_keys()` for O(1) metadata comparison without loading content. Added `_classify_changes()` to separate content-only, structural, added, and deleted changes. Replaced `apply_merge_revision` with `_apply_merge_changes_only()` that only touches changed documents and uses `frappe.db.set_value()` for content-only changes. Three new tests added and all 43 tests pass. - -**Changes**: - -1. **`wiki_change_request.py`** — Add fast-forward path to `merge_change_request()`: - ```python - if cr.base_revision == space.main_revision: - # No concurrent changes — fast-forward merge - return _fast_forward_merge(cr, space) - else: - return _three_way_merge(cr, space) - ``` - -2. **Fast-forward merge** (`_fast_forward_merge`): - - Materialize effective tree from overlay - - Create merge revision (full snapshot for main_revision) - - Only apply changes to live tree (not the entire tree) - -3. **Optimized 3-way merge** (`_three_way_merge`): - - Find changed doc_keys: `main_changed = _find_changed_keys(base, main)` - - Head overlay items are already only the changed keys - - Only load content blobs for `main_changed | head_changed` keys - - Only run merge logic for those keys - - Start with base items, apply merge results - -4. **New function `_find_changed_keys(base_items, other_items)`**: - - Compare `content_blob`, `title`, `slug`, `parent_key`, `order_index`, `is_group`, `is_published` without loading content - - Content blob comparison is a simple string compare (the blob name includes the hash) - -5. **Optimized `apply_merge_revision`** → rename to `_apply_merge_changes_only(space, merge_revision, base_revision)`: - - Find changed keys between merge_revision and base_revision - - Batch-load content blobs for changed items (1 query instead of N) - - Only `get_doc()` + `save()` for changed documents - - Unchanged documents are not touched at all - -6. **Content-only fast path**: For documents where only `content` changed (no title/slug/parent changes), use `frappe.db.set_value()` instead of `doc.save()` to skip validation. Content changes don't affect routes or tree structure. - -**Impact on a 500-page wiki with 1 changed page**: -- Before: Load 1500 items + 1500 blobs, process 500 keys, create 500 items, load+save 500 docs -- After: Load ~2 items + 1 blob, process 1 key, create 500 items (merge revision is still full), load+save 1 doc - -Note: The merge revision still needs to be a full snapshot (it becomes `main_revision`). But items can be bulk-inserted from base with the delta applied, rather than individually created. - -**Tests to add**: -- `test_fast_forward_merge_only_updates_changed_docs`: Create 10 pages, edit 1 via CR, merge, verify only 1 doc's `modified` timestamp changed -- `test_three_way_merge_only_loads_changed_content`: Mock/count DB queries during merge to verify content isn't loaded for unchanged docs -- `test_content_only_change_skips_validation`: Edit only content (no title/slug), verify route is untouched - -**Risk**: Medium. The fast-forward path is safe (it's just applying a diff). The 3-way optimization needs careful testing to ensure unchanged documents truly aren't affected. - ---- - -### Phase 6: Simplify Draft CR Logic ✅ DONE - -**Goal**: Make `get_or_create_draft_change_request()` readable. - -**Priority**: Low — UX improvement, not a bug. - -**Status**: Completed. Extracted the 60-line `get_or_create_draft_change_request()` into 3 focused helpers: `_find_existing_draft()`, `_is_stale_empty_draft()`, `_archive_stale_draft()`. Replaced duplicated inline hash comparison with a call to `has_revision_changes()`. Two new tests added and all 35 tests pass. - -**Changes**: - -Extract the 60-line function into 3 small functions: - -```python -@frappe.whitelist() -def get_or_create_draft_change_request(wiki_space, title=None): - cr = _find_existing_draft(wiki_space) - if cr: - if _should_replace_stale_draft(cr, wiki_space): - _archive_stale_draft(cr) - else: - return cr.as_dict() - - space = frappe.get_doc("Wiki Space", wiki_space) - default_title = title or f"Draft Changes - {space.space_name}" - return create_change_request(wiki_space, default_title).as_dict() - - -def _find_existing_draft(wiki_space): - """Find user's most relevant draft: prefer one with actual changes.""" - ... - -def _should_replace_stale_draft(cr, wiki_space): - """True if the draft is outdated AND has no changes.""" - ... - -def _archive_stale_draft(cr): - """Archive a stale empty draft.""" - ... -``` - -**Tests**: Existing tests cover `get_or_create_draft_change_request` indirectly. Add: -- `test_stale_empty_draft_is_auto_archived` -- `test_stale_draft_with_changes_is_kept` - -**Risk**: Very low. Pure refactor with no behavior change. - ---- - -## Execution Order - -``` -Phase 1 (Desk sync) ← Do first: data integrity fix - ↓ -Phase 3 (Dead code removal) ← Quick win, no dependencies - ↓ -Phase 2 (Merge consolidation) ← Simplifies code for Phase 5 - ↓ -Phase 6 (Draft CR cleanup) ← Quick win, no dependencies - ↓ -Phase 4 (Overlay revisions) ← Biggest architectural change - ↓ -Phase 5 (Optimized merge) ← Builds on Phase 4 -``` - -Phases 1, 2, 3, 6 are independent of each other and can be done in any order. -Phase 5 depends on Phase 4. -Phase 2 should be done before Phase 5 (cleaner code to optimize). - -## Dependency Graph - -``` -Phase 1 ──┐ -Phase 3 ──┤ -Phase 2 ──┼──→ Phase 4 ──→ Phase 5 -Phase 6 ──┘ -``` - -## Summary Table - -| Phase | Problem | Risk | Effort | Lines Changed (est.) | -|-------|---------|------|--------|---------------------| -| 1 | Desk sync (data integrity) | Low | Medium | +60, ~10 modified | -| 2 | 4 merge strategies → 1 | Low | Low | -150, +50 | -| 3 | Dead code removal | Very Low | Very Low | -100 | -| 4 | O(N) → O(1) CR creation | Medium | High | +120, ~80 modified | -| 5 | O(N) → O(changed) merge | Medium | Medium | +100, ~60 modified | -| 6 | Draft CR cleanup | Very Low | Low | ~40 modified | - -## Test Strategy - -All phases add tests to `test_wiki_change_request.py`. The existing 20 tests serve as regression guards — they must pass after every phase. - -Run tests with: -```bash -bench --site run-tests --app wiki --module wiki.frappe_wiki.doctype.wiki_change_request.test_wiki_change_request -``` - -Manual verification checklist: -- [ ] Edit a Wiki Document from Desk → open Editor UI → changes visible (Phase 1) -- [ ] Create a CR on a wiki with 100+ pages → should be near-instant (Phase 4) -- [ ] Merge a CR with 1 changed page → should not take noticeably longer than creating the CR (Phase 5) -- [ ] Merge with concurrent changes (main advanced while CR was open) → 3-way merge resolves cleanly or reports conflict correctly (Phase 2 + 5) From 60be8c0c3814495602aa3cfe1c5b59739a862be6 Mon Sep 17 00:00:00 2001 From: Hussain Nagaria Date: Tue, 17 Feb 2026 09:30:24 +0530 Subject: [PATCH 020/128] chore: move reorder perf test to separate benchmark module The 800-page/80-group perf test was timing out CI with too many writes. Moved to wiki/benchmarks/ so it only runs on-demand. Co-Authored-By: Claude Opus 4.6 --- wiki/benchmarks/__init__.py | 0 wiki/benchmarks/bench_reorder.py | 109 +++++++++++++++++++++++++++++++ wiki/test_api.py | 95 --------------------------- 3 files changed, 109 insertions(+), 95 deletions(-) create mode 100644 wiki/benchmarks/__init__.py create mode 100644 wiki/benchmarks/bench_reorder.py diff --git a/wiki/benchmarks/__init__.py b/wiki/benchmarks/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/wiki/benchmarks/bench_reorder.py b/wiki/benchmarks/bench_reorder.py new file mode 100644 index 00000000..ac04c982 --- /dev/null +++ b/wiki/benchmarks/bench_reorder.py @@ -0,0 +1,109 @@ +# Copyright (c) 2025, Frappe and Contributors +# See license.txt + +""" +Performance benchmark for wiki reorder operations. + +NOT part of the regular test suite — run manually: + + bench --site run-tests --app wiki --module wiki.benchmarks.bench_reorder +""" + +import json +import time + +import frappe +from frappe.tests.utils import FrappeTestCase + +from wiki.test_api import create_test_wiki_space, create_wiki_document + + +class TestReorderPerformance(FrappeTestCase): + """Performance test for reorder with a large wiki tree (80 groups, ~800 pages).""" + + def setUp(self): + frappe.set_user("Administrator") + + def tearDown(self): + frappe.set_user("Administrator") + frappe.db.rollback() + + def test_reorder_performance_large_tree(self): + """Create 80 groups (up to 3 levels deep), ~800 pages, reorder, and measure time.""" + from wiki.api.wiki_space import reorder_wiki_documents + + space = create_test_wiki_space() + root = space.root_group + + all_groups = [] + all_pages = [] + page_count = 0 + + # Level 1: 20 groups directly under root + level1_groups = [] + for i in range(20): + g = create_wiki_document(root, f"L1 Group {i}", is_group=True) + level1_groups.append(g) + all_groups.append(g) + + # Level 2: 2 sub-groups under each L1 group (20 * 2 = 40) + level2_groups = [] + for g1 in level1_groups: + for j in range(2): + g = create_wiki_document(g1.name, f"L2 Group {g1.name[-6:]}-{j}", is_group=True) + level2_groups.append(g) + all_groups.append(g) + + # Level 3: 1 sub-group under each L2 group (40 * 1 = 40, but cap at 20 to hit 80 total) + level3_groups = [] + for g2 in level2_groups[:20]: + g = create_wiki_document(g2.name, f"L3 Group {g2.name[-6:]}", is_group=True) + level3_groups.append(g) + all_groups.append(g) + + # Total groups: 20 + 40 + 20 = 80 + self.assertEqual(len(all_groups), 80) + + # Distribute ~800 pages across all groups (10 per group) + for group in all_groups: + for _k in range(10): + p = create_wiki_document(group.name, f"Page {page_count}", content=f"Content {page_count}") + all_pages.append(p) + page_count += 1 + + self.assertEqual(len(all_pages), 800) + frappe.db.commit() # nosemgrep + + # Pick a group with 10 children and reverse their order + target_group = level1_groups[0] + children = frappe.get_all( + "Wiki Document", + filters={"parent_wiki_document": target_group.name, "is_group": 0}, + fields=["name"], + order_by="sort_order asc, name asc", + ) + siblings_reversed = [c["name"] for c in reversed(children)] + + start = time.monotonic() + reorder_wiki_documents( + doc_name=siblings_reversed[0], + new_parent=target_group.name, + new_index=0, + siblings=json.dumps(siblings_reversed), + ) + elapsed = time.monotonic() - start + + # Verify the reorder actually took effect + children_after = frappe.get_all( + "Wiki Document", + filters={"parent_wiki_document": target_group.name, "is_group": 0}, + fields=["name", "sort_order"], + order_by="sort_order asc", + ) + names_after = [c["name"] for c in children_after] + self.assertEqual(names_after, siblings_reversed) + + print(f"\n[PERF] Reorder within same parent (800 pages, 80 groups): {elapsed:.3f}s") + + # Fail if unreasonably slow (> 30s is a red flag) + self.assertLess(elapsed, 30, f"Reorder took {elapsed:.3f}s which exceeds 30s threshold") diff --git a/wiki/test_api.py b/wiki/test_api.py index 869fa938..63ffd046 100644 --- a/wiki/test_api.py +++ b/wiki/test_api.py @@ -864,101 +864,6 @@ def test_rebuild_respects_sort_order(self): self.assertLess(page_b.lft, page_a.lft) -class TestReorderPerformance(FrappeTestCase): - """Performance test for reorder with a large wiki tree (80 groups, ~800 pages).""" - - def setUp(self): - frappe.set_user("Administrator") - - def tearDown(self): - frappe.set_user("Administrator") - frappe.db.rollback() - - def test_reorder_performance_large_tree(self): - """Create 80 groups (up to 3 levels deep), ~800 pages, reorder, and measure time.""" - import time - - from wiki.api.wiki_space import reorder_wiki_documents - - space = create_test_wiki_space() - root = space.root_group - - all_groups = [] - all_pages = [] - page_count = 0 - - # Level 1: 20 groups directly under root - level1_groups = [] - for i in range(20): - g = create_wiki_document(root, f"L1 Group {i}", is_group=True) - level1_groups.append(g) - all_groups.append(g) - - # Level 2: 2 sub-groups under each L1 group (20 * 2 = 40) - level2_groups = [] - for g1 in level1_groups: - for j in range(2): - g = create_wiki_document(g1.name, f"L2 Group {g1.name[-6:]}-{j}", is_group=True) - level2_groups.append(g) - all_groups.append(g) - - # Level 3: 1 sub-group under each L2 group (40 * 1 = 40, but cap at 20 to hit 80 total) - level3_groups = [] - for g2 in level2_groups[:20]: - g = create_wiki_document(g2.name, f"L3 Group {g2.name[-6:]}", is_group=True) - level3_groups.append(g) - all_groups.append(g) - - # Total groups: 20 + 40 + 20 = 80 - self.assertEqual(len(all_groups), 80) - - # Distribute ~800 pages across all groups (10 per group) - for group in all_groups: - for _k in range(10): - p = create_wiki_document(group.name, f"Page {page_count}", content=f"Content {page_count}") - all_pages.append(p) - page_count += 1 - - self.assertEqual(len(all_pages), 800) - frappe.db.commit() # nosemgrep - - # Pick a group with 10 children and reverse their order - target_group = level1_groups[0] - children = frappe.get_all( - "Wiki Document", - filters={"parent_wiki_document": target_group.name, "is_group": 0}, - fields=["name"], - order_by="sort_order asc, name asc", - ) - siblings_reversed = [c["name"] for c in reversed(children)] - - start = time.monotonic() - reorder_wiki_documents( - doc_name=siblings_reversed[0], - new_parent=target_group.name, - new_index=0, - siblings=json.dumps(siblings_reversed), - ) - elapsed = time.monotonic() - start - - # Verify the reorder actually took effect - children_after = frappe.get_all( - "Wiki Document", - filters={"parent_wiki_document": target_group.name, "is_group": 0}, - fields=["name", "sort_order"], - order_by="sort_order asc", - ) - names_after = [c["name"] for c in children_after] - self.assertEqual(names_after, siblings_reversed) - - print(f"\n[PERF] Reorder within same parent (800 pages, 80 groups): {elapsed:.3f}s") - - # Fail if unreasonably slow (> 30s is a red flag) - self.assertLess(elapsed, 30, f"Reorder took {elapsed:.3f}s which exceeds 30s threshold") - - # Profiling tests removed — kept only the main perf test above. - - # Helper functions From e216fd614f26541219e33d8ccd02746a2b5d9469 Mon Sep 17 00:00:00 2001 From: vishwajeet-13 Date: Tue, 17 Feb 2026 11:31:28 +0530 Subject: [PATCH 021/128] fix: add ignore permission --- wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.py b/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.py index 58b19fbd..18848f33 100644 --- a/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.py +++ b/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.py @@ -115,7 +115,7 @@ def create_overlay_revision( revision.tree_hash = base.tree_hash revision.content_hash = base.content_hash revision.doc_count = base.doc_count - revision.insert() + revision.insert(ignore_permissions=True) return revision @@ -361,7 +361,7 @@ def ensure_overlay_item(revision: str, doc_key: str) -> str | None: new_item.order_index = base_item.order_index new_item.content_blob = base_item.content_blob new_item.is_deleted = base_item.is_deleted - new_item.insert() + new_item.insert(ignore_permissions=True) return new_item.name From ff189dd3e7a1612f395a0f3dddb78dc7e2908eda Mon Sep 17 00:00:00 2001 From: vishwajeet-13 Date: Tue, 17 Feb 2026 12:15:37 +0530 Subject: [PATCH 022/128] fix: no change request error --- .../wiki_change_request.js | 8 ++++++ .../wiki_change_request.json | 20 ++++++++++--- .../wiki_change_request.py | 28 +++++++++++++++++++ .../wiki_revision/test_wiki_revision.py | 20 +++++++++++++ .../doctype/wiki_revision/wiki_revision.js | 8 ++++++ .../doctype/wiki_revision/wiki_revision.json | 21 +++++++++++--- .../doctype/wiki_revision/wiki_revision.py | 2 +- .../test_wiki_revision_item.py | 20 +++++++++++++ .../wiki_revision_item/wiki_revision_item.js | 8 ++++++ .../wiki_revision_item.json | 24 +++++++++++++--- .../wiki_revision_item/wiki_revision_item.py | 22 +++++++++++++++ 11 files changed, 168 insertions(+), 13 deletions(-) create mode 100644 wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.js create mode 100644 wiki/frappe_wiki/doctype/wiki_revision/test_wiki_revision.py create mode 100644 wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.js create mode 100644 wiki/frappe_wiki/doctype/wiki_revision_item/test_wiki_revision_item.py create mode 100644 wiki/frappe_wiki/doctype/wiki_revision_item/wiki_revision_item.js diff --git a/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.js b/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.js new file mode 100644 index 00000000..e359773f --- /dev/null +++ b/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.js @@ -0,0 +1,8 @@ +// Copyright (c) 2026, Frappe and contributors +// For license information, please see license.txt + +// frappe.ui.form.on("Wiki Change Request", { +// refresh(frm) { + +// }, +// }); diff --git a/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.json b/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.json index 36c73a91..c7a5aedf 100644 --- a/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.json +++ b/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.json @@ -1,8 +1,7 @@ { "actions": [], - "allow_rename": 0, "autoname": "hash", - "creation": "2026-01-22 00:00:00.000000", + "creation": "2026-01-22 00:00:00", "doctype": "DocType", "engine": "InnoDB", "field_order": [ @@ -132,9 +131,8 @@ "options": "Wiki CR Participant" } ], - "index_web_pages_for_search": 0, "links": [], - "modified": "2026-01-22 00:00:00.000000", + "modified": "2026-02-17 11:54:18.272204", "modified_by": "Administrator", "module": "Frappe Wiki", "name": "Wiki Change Request", @@ -183,8 +181,22 @@ "read": 1, "role": "All", "write": 1 + }, + { + "create": 1, + "delete": 1, + "email": 1, + "export": 1, + "if_owner": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "Wiki User", + "share": 1, + "write": 1 } ], + "row_format": "Dynamic", "show_title_field_in_link": 1, "sort_field": "modified", "sort_order": "DESC", diff --git a/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py b/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py index 0b922237..2813eec1 100644 --- a/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py +++ b/wiki/frappe_wiki/doctype/wiki_change_request/wiki_change_request.py @@ -27,6 +27,34 @@ class WikiChangeRequest(Document): + # begin: auto-generated types + # This code is auto-generated. Do not modify anything in this block. + + from typing import TYPE_CHECKING + + if TYPE_CHECKING: + from frappe.types import DF + + from wiki.frappe_wiki.doctype.wiki_cr_participant.wiki_cr_participant import WikiCRParticipant + from wiki.frappe_wiki.doctype.wiki_cr_reviewer.wiki_cr_reviewer import WikiCRReviewer + + archived_at: DF.Datetime | None + base_revision: DF.Link + description: DF.SmallText | None + head_revision: DF.Link + merge_revision: DF.Link | None + merged_at: DF.Datetime | None + merged_by: DF.Link | None + outdated: DF.Check + participants: DF.Table[WikiCRParticipant] + reviewers: DF.Table[WikiCRReviewer] + status: DF.Literal[ + "Draft", "Open", "In Review", "Changes Requested", "Approved", "Merged", "Archived" + ] + title: DF.Data + wiki_space: DF.Link + # end: auto-generated types + pass diff --git a/wiki/frappe_wiki/doctype/wiki_revision/test_wiki_revision.py b/wiki/frappe_wiki/doctype/wiki_revision/test_wiki_revision.py new file mode 100644 index 00000000..a2a07ff5 --- /dev/null +++ b/wiki/frappe_wiki/doctype/wiki_revision/test_wiki_revision.py @@ -0,0 +1,20 @@ +# Copyright (c) 2026, Frappe and Contributors +# See license.txt + +# import frappe +from frappe.tests import IntegrationTestCase + +# On IntegrationTestCase, the doctype test records and all +# link-field test record dependencies are recursively loaded +# Use these module variables to add/remove to/from that list +EXTRA_TEST_RECORD_DEPENDENCIES = [] # eg. ["User"] +IGNORE_TEST_RECORD_DEPENDENCIES = [] # eg. ["User"] + + +class IntegrationTestWikiRevision(IntegrationTestCase): + """ + Integration tests for WikiRevision. + Use this class for testing interactions between multiple components. + """ + + pass diff --git a/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.js b/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.js new file mode 100644 index 00000000..a972b972 --- /dev/null +++ b/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.js @@ -0,0 +1,8 @@ +// Copyright (c) 2026, Frappe and contributors +// For license information, please see license.txt + +// frappe.ui.form.on("Wiki Revision", { +// refresh(frm) { + +// }, +// }); diff --git a/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.json b/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.json index cbf0f2e7..2b546096 100644 --- a/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.json +++ b/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.json @@ -1,8 +1,7 @@ { "actions": [], - "allow_rename": 0, "autoname": "hash", - "creation": "2026-01-22 00:00:00.000000", + "creation": "2026-01-22 00:00:00", "doctype": "DocType", "engine": "InnoDB", "field_order": [ @@ -26,6 +25,7 @@ { "fieldname": "wiki_space", "fieldtype": "Link", + "in_list_view": 1, "label": "Wiki Space", "options": "Wiki Space", "reqd": 1 @@ -114,9 +114,8 @@ "read_only": 1 } ], - "index_web_pages_for_search": 0, "links": [], - "modified": "2026-01-22 00:00:00.000000", + "modified": "2026-02-17 11:57:07.163673", "modified_by": "Administrator", "module": "Frappe Wiki", "name": "Wiki Revision", @@ -158,8 +157,22 @@ "role": "Wiki Approver", "share": 1, "write": 1 + }, + { + "create": 1, + "delete": 1, + "email": 1, + "export": 1, + "if_owner": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "Wiki User", + "share": 1, + "write": 1 } ], + "row_format": "Dynamic", "show_title_field_in_link": 1, "sort_field": "modified", "sort_order": "DESC", diff --git a/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.py b/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.py index 58b19fbd..5397b1df 100644 --- a/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.py +++ b/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.py @@ -169,7 +169,7 @@ def clone_revision( new_item.order_index = item.get("order_index") new_item.content_blob = item.get("content_blob") new_item.is_deleted = item.get("is_deleted") - new_item.insert() + new_item.insert(ignore_permissions=True) recompute_revision_hashes(new_revision.name) return new_revision diff --git a/wiki/frappe_wiki/doctype/wiki_revision_item/test_wiki_revision_item.py b/wiki/frappe_wiki/doctype/wiki_revision_item/test_wiki_revision_item.py new file mode 100644 index 00000000..95ba4819 --- /dev/null +++ b/wiki/frappe_wiki/doctype/wiki_revision_item/test_wiki_revision_item.py @@ -0,0 +1,20 @@ +# Copyright (c) 2026, Frappe and Contributors +# See license.txt + +# import frappe +from frappe.tests import IntegrationTestCase + +# On IntegrationTestCase, the doctype test records and all +# link-field test record dependencies are recursively loaded +# Use these module variables to add/remove to/from that list +EXTRA_TEST_RECORD_DEPENDENCIES = [] # eg. ["User"] +IGNORE_TEST_RECORD_DEPENDENCIES = [] # eg. ["User"] + + +class IntegrationTestWikiRevisionItem(IntegrationTestCase): + """ + Integration tests for WikiRevisionItem. + Use this class for testing interactions between multiple components. + """ + + pass diff --git a/wiki/frappe_wiki/doctype/wiki_revision_item/wiki_revision_item.js b/wiki/frappe_wiki/doctype/wiki_revision_item/wiki_revision_item.js new file mode 100644 index 00000000..1552183f --- /dev/null +++ b/wiki/frappe_wiki/doctype/wiki_revision_item/wiki_revision_item.js @@ -0,0 +1,8 @@ +// Copyright (c) 2026, Frappe and contributors +// For license information, please see license.txt + +// frappe.ui.form.on("Wiki Revision Item", { +// refresh(frm) { + +// }, +// }); diff --git a/wiki/frappe_wiki/doctype/wiki_revision_item/wiki_revision_item.json b/wiki/frappe_wiki/doctype/wiki_revision_item/wiki_revision_item.json index db74b77e..04c3880d 100644 --- a/wiki/frappe_wiki/doctype/wiki_revision_item/wiki_revision_item.json +++ b/wiki/frappe_wiki/doctype/wiki_revision_item/wiki_revision_item.json @@ -1,8 +1,7 @@ { "actions": [], - "allow_rename": 0, "autoname": "hash", - "creation": "2026-01-22 00:00:00.000000", + "creation": "2026-01-22 00:00:00", "doctype": "DocType", "engine": "InnoDB", "field_order": [ @@ -24,6 +23,7 @@ { "fieldname": "revision", "fieldtype": "Link", + "in_list_view": 1, "label": "Revision", "options": "Wiki Revision", "reqd": 1 @@ -31,6 +31,7 @@ { "fieldname": "doc_key", "fieldtype": "Data", + "in_list_view": 1, "label": "Doc Key", "reqd": 1 }, @@ -49,11 +50,13 @@ "fieldtype": "Column Break" }, { + "default": "0", "fieldname": "is_group", "fieldtype": "Check", "label": "Is Group" }, { + "default": "0", "fieldname": "is_published", "fieldtype": "Check", "label": "Is Published" @@ -92,9 +95,8 @@ "label": "Is Deleted" } ], - "index_web_pages_for_search": 0, "links": [], - "modified": "2026-01-22 00:00:00.000000", + "modified": "2026-02-17 11:54:37.644118", "modified_by": "Administrator", "module": "Frappe Wiki", "name": "Wiki Revision Item", @@ -136,8 +138,22 @@ "role": "Wiki Approver", "share": 1, "write": 1 + }, + { + "create": 1, + "delete": 1, + "email": 1, + "export": 1, + "if_owner": 1, + "print": 1, + "read": 1, + "report": 1, + "role": "Wiki User", + "share": 1, + "write": 1 } ], + "row_format": "Dynamic", "sort_field": "modified", "sort_order": "DESC", "states": [], diff --git a/wiki/frappe_wiki/doctype/wiki_revision_item/wiki_revision_item.py b/wiki/frappe_wiki/doctype/wiki_revision_item/wiki_revision_item.py index e1ad8931..14d148b9 100644 --- a/wiki/frappe_wiki/doctype/wiki_revision_item/wiki_revision_item.py +++ b/wiki/frappe_wiki/doctype/wiki_revision_item/wiki_revision_item.py @@ -5,4 +5,26 @@ class WikiRevisionItem(Document): + # begin: auto-generated types + # This code is auto-generated. Do not modify anything in this block. + + from typing import TYPE_CHECKING + + if TYPE_CHECKING: + from frappe.types import DF + + content_blob: DF.Link | None + doc_key: DF.Data + external_url: DF.Data | None + is_deleted: DF.Check + is_external_link: DF.Check + is_group: DF.Check + is_published: DF.Check + order_index: DF.Int + parent_key: DF.Data | None + revision: DF.Link + slug: DF.Data | None + title: DF.Data | None + # end: auto-generated types + pass From b9dd71c848f4638f766a2b64a6f3ac45a72a162f Mon Sep 17 00:00:00 2001 From: Rahul Agrawal <12agrawalrahul@gmail.com> Date: Tue, 17 Feb 2026 13:55:33 +0530 Subject: [PATCH 023/128] feat: add table dropdown in markdown toolbar --- .../tiptap-extensions/WikiTableDropdown.vue | 90 +++++++++++++++++++ .../tiptap-extensions/WikiToolbar.vue | 20 +---- 2 files changed, 93 insertions(+), 17 deletions(-) create mode 100644 frontend/src/components/tiptap-extensions/WikiTableDropdown.vue diff --git a/frontend/src/components/tiptap-extensions/WikiTableDropdown.vue b/frontend/src/components/tiptap-extensions/WikiTableDropdown.vue new file mode 100644 index 00000000..a1e329b5 --- /dev/null +++ b/frontend/src/components/tiptap-extensions/WikiTableDropdown.vue @@ -0,0 +1,90 @@ + + + diff --git a/frontend/src/components/tiptap-extensions/WikiToolbar.vue b/frontend/src/components/tiptap-extensions/WikiToolbar.vue index 78ac4fa1..a0249a12 100644 --- a/frontend/src/components/tiptap-extensions/WikiToolbar.vue +++ b/frontend/src/components/tiptap-extensions/WikiToolbar.vue @@ -128,14 +128,8 @@
- - + +