diff --git a/frontend/src/pages/ContributionReview.vue b/frontend/src/pages/ContributionReview.vue index a8a4e1c9..d36e0016 100644 --- a/frontend/src/pages/ContributionReview.vue +++ b/frontend/src/pages/ContributionReview.vue @@ -22,16 +22,24 @@
- - +
@@ -60,77 +68,173 @@ -
-

- {{ __('Changes') }} ({{ changes.data?.length || 0 }}) -

- -
- + +
+
+ +
+

{{ __('Merge Conflicts') }}

+

+ {{ __('The following documents have conflicting changes. Choose which version to keep for each conflict.') }} +

+

+ {{ resolvedCount }}/{{ conflicts.length }} {{ __('resolved') }} +

+
+
-
-
+
+ + + + +
@@ -176,15 +280,13 @@ import { userResource } from '@/data/user'; import { isWikiManager, currentChangeRequest } from '@/composables/useChangeRequest'; import DiffViewer from '@/components/DiffViewer.vue'; import LucideChevronDown from '~icons/lucide/chevron-down'; -import LucideCheck from '~icons/lucide/check'; -import LucideX from '~icons/lucide/x'; import LucideAlertCircle from '~icons/lucide/alert-circle'; +import LucideAlertTriangle from '~icons/lucide/alert-triangle'; import LucideArrowUpDown from '~icons/lucide/arrow-up-down'; import LucidePlus from '~icons/lucide/plus'; import LucidePencil from '~icons/lucide/pencil'; import LucideTrash2 from '~icons/lucide/trash-2'; import LucideFileText from '~icons/lucide/file-text'; -import LucideLink from '~icons/lucide/link'; const props = defineProps({ changeRequestId: { @@ -198,6 +300,18 @@ const rejectComment = ref(''); const expandedChanges = reactive(new Set()); const diffsByDocKey = reactive({}); +// Conflict resolution state +const hasConflicts = ref(false); +const conflicts = ref([]); +const resolutions = reactive({}); +const expandedConflicts = reactive(new Set()); +const resolvingMerge = ref(false); + +const resolvedCount = computed(() => + Object.values(resolutions).filter((v) => v === 'ours' || v === 'theirs').length, +); +const allResolved = computed(() => conflicts.value.length > 0 && resolvedCount.value === conflicts.value.length); + const changeRequest = createDocumentResource({ doctype: 'Wiki Change Request', name: props.changeRequestId, @@ -218,6 +332,18 @@ const mergeResource = createResource({ url: 'wiki.frappe_wiki.doctype.wiki_change_request.wiki_change_request.merge_change_request', }); +const conflictsResource = createResource({ + url: 'wiki.frappe_wiki.doctype.wiki_change_request.wiki_change_request.get_merge_conflicts', +}); + +const resolveResource = createResource({ + url: 'wiki.frappe_wiki.doctype.wiki_change_request.wiki_change_request.resolve_merge_conflict', +}); + +const retryResource = createResource({ + url: 'wiki.frappe_wiki.doctype.wiki_change_request.wiki_change_request.retry_merge_after_resolution', +}); + const rejectResource = createResource({ url: 'wiki.frappe_wiki.doctype.wiki_change_request.wiki_change_request.review_action', }); @@ -250,6 +376,37 @@ const reviewNote = computed(() => { }; }); +function setResolution(conflictName, value) { + if (resolutions[conflictName] === value) { + delete resolutions[conflictName]; + } else { + resolutions[conflictName] = value; + } +} + +function toggleConflict(conflictName) { + if (expandedConflicts.has(conflictName)) { + expandedConflicts.delete(conflictName); + } else { + expandedConflicts.add(conflictName); + } +} + +async function fetchConflicts() { + try { + const result = await conflictsResource.submit({ name: props.changeRequestId }); + conflicts.value = result || []; + // Default all resolutions to 'theirs' (Keep Your Changes) + for (const key in resolutions) delete resolutions[key]; + for (const conflict of conflicts.value) { + resolutions[conflict.name] = 'theirs'; + } + hasConflicts.value = conflicts.value.length > 0; + } catch (error) { + toast.error(error.messages?.[0] || __('Error loading conflicts')); + } +} + async function toggleChange(docKey) { if (expandedChanges.has(docKey)) { expandedChanges.delete(docKey); @@ -280,7 +437,48 @@ async function handleApprove() { changeRequest.reload(); await changes.submit({ name: props.changeRequestId, scope: 'summary' }); } catch (error) { - toast.error(error.messages?.[0] || __('Error merging change request')); + const msg = error.messages?.[0] || ''; + if (error.exc_type === 'ValidationError') { + await fetchConflicts(); + if (!hasConflicts.value) { + toast.error(msg || __('Error merging change request')); + } + } else { + toast.error(msg || __('Error merging change request')); + } + } +} + +async function handleResolveAndMerge() { + resolvingMerge.value = true; + try { + for (const conflict of conflicts.value) { + try { + await resolveResource.submit({ + conflict_name: conflict.name, + resolution: resolutions[conflict.name], + }); + } catch (e) { + const errMsg = e.messages?.[0] || ''; + if (errMsg.includes('already resolved')) { + continue; + } + throw e; + } + } + await retryResource.submit({ name: props.changeRequestId }); + toast.success(__('Conflicts resolved and change request merged')); + hasConflicts.value = false; + conflicts.value = []; + if (currentChangeRequest.value?.name === props.changeRequestId) { + currentChangeRequest.value = null; + } + changeRequest.reload(); + await changes.submit({ name: props.changeRequestId, scope: 'summary' }); + } catch (error) { + toast.error(error.messages?.[0] || __('Error resolving conflicts')); + } finally { + resolvingMerge.value = false; } } @@ -385,6 +583,15 @@ function getChangeDescription(changeType, isGroup, isExternalLink) { } } +function getConflictTheme(type) { + switch (type) { + case 'content': return 'blue'; + case 'meta': return 'orange'; + case 'tree': return 'red'; + default: return 'gray'; + } +} + function formatDate(dateStr) { if (!dateStr) return ''; const date = new Date(dateStr); 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 6ec4eab7..2fb2ec1b 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_merge_conflicts, get_or_create_draft_change_request, list_change_requests, merge_change_request, @@ -21,6 +22,8 @@ move_cr_page, reorder_cr_children, request_review, + resolve_merge_conflict, + retry_merge_after_resolution, review_action, update_change_request, update_cr_page, @@ -874,6 +877,47 @@ def test_merge_delete_group_with_reparented_children(self): self.assertEqual(page.parent_wiki_document, space.root_group) self.assertFalse(frappe.db.exists("Wiki Document", group.name)) + def test_resolve_and_retry_merge_end_to_end(self): + """Phase 1 tracer bullet: detect conflict, resolve it, retry merge successfully.""" + 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 conflict resolve") + + page_key = frappe.get_value("Wiki Document", page.name, "doc_key") + update_cr_page(cr.name, page_key, {"content": "cr-change"}) + + # Advance main so three-way merge is needed + page.content = "main-change" + page.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 should fail with conflict + with self.assertRaises(frappe.ValidationError): + merge_change_request(cr.name) + + # get_merge_conflicts should return the conflict + conflicts = get_merge_conflicts(cr.name) + self.assertEqual(len(conflicts), 1) + conflict = conflicts[0] + self.assertEqual(conflict["doc_key"], page_key) + + # Resolve with "theirs" (CR's version) + resolve_merge_conflict(conflict["name"], "theirs") + + resolved = frappe.get_doc("Wiki Merge Conflict", conflict["name"]) + self.assertEqual(resolved.status, "Resolved") + self.assertEqual(resolved.resolution, "theirs") + + # Retry merge + retry_merge_after_resolution(cr.name) + + cr_doc = frappe.get_doc("Wiki Change Request", cr.name) + self.assertEqual(cr_doc.status, "Merged") + + page.reload() + self.assertEqual(page.content, "cr-change") + # 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 2813eec1..35649451 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 @@ -711,6 +711,203 @@ def merge_change_request(name: str) -> str: return _three_way_merge(cr, space) +@frappe.whitelist() +def get_merge_conflicts(name: str) -> list[dict[str, Any]]: + """Return open merge conflicts for a change request.""" + if not _is_manager_or_approver(): + frappe.throw(_("Only Wiki Managers or Approvers can view merge conflicts."), frappe.PermissionError) + + conflicts = frappe.get_all( + "Wiki Merge Conflict", + filters={"change_request": name, "status": "Open"}, + fields=["name", "doc_key", "conflict_type", "ours_payload", "theirs_payload"], + ) + + for conflict in conflicts: + ours = conflict.get("ours_payload") + theirs = conflict.get("theirs_payload") + if isinstance(ours, str): + ours = frappe.parse_json(ours) + if isinstance(theirs, str): + theirs = frappe.parse_json(theirs) + conflict["ours_title"] = (ours or {}).get("title", "") + conflict["theirs_title"] = (theirs or {}).get("title", "") + + # Resolve content from content_blob references + ours_blob = (ours or {}).get("content_blob") + theirs_blob = (theirs or {}).get("content_blob") + conflict["ours_content"] = ( + (frappe.get_value("Wiki Content Blob", ours_blob, "content") or "") if ours_blob else "" + ) + conflict["theirs_content"] = ( + (frappe.get_value("Wiki Content Blob", theirs_blob, "content") or "") if theirs_blob else "" + ) + + return conflicts + + +@frappe.whitelist() +def resolve_merge_conflict(conflict_name: str, resolution: str) -> None: + """Resolve a single merge conflict by choosing 'ours' or 'theirs'.""" + if not _is_manager_or_approver(): + frappe.throw(_("Only Wiki Managers or Approvers can resolve conflicts."), frappe.PermissionError) + + if resolution not in ("ours", "theirs"): + frappe.throw(_("Resolution must be 'ours' or 'theirs'."), frappe.ValidationError) + + conflict = frappe.get_doc("Wiki Merge Conflict", conflict_name) + + # Validate the parent change request is in an allowed state + cr = frappe.get_doc("Wiki Change Request", conflict.change_request) + if cr.status in ("Merged", "Archived"): + frappe.throw(_("Cannot resolve conflicts for a finalized Change Request."), frappe.ValidationError) + + if conflict.status == "Resolved": + frappe.throw(_("Conflict is already resolved."), frappe.ValidationError) + + payload = conflict.ours_payload if resolution == "ours" else conflict.theirs_payload + if isinstance(payload, str): + payload = frappe.parse_json(payload) + + conflict.resolution = resolution + conflict.resolved_payload = payload + conflict.resolved_by = frappe.session.user + conflict.resolved_at = now_datetime() + conflict.status = "Resolved" + conflict.save(ignore_permissions=True) + + +@frappe.whitelist() +def retry_merge_after_resolution(name: str) -> str: + """Retry a merge after all conflicts have been resolved.""" + if not _is_manager_or_approver(): + frappe.throw(_("Only Wiki Managers or Approvers can merge."), frappe.PermissionError) + + cr = frappe.get_doc("Wiki Change Request", name) + space = frappe.get_doc("Wiki Space", cr.wiki_space) + + # Verify all conflicts are resolved + open_conflicts = frappe.db.count("Wiki Merge Conflict", {"change_request": name, "status": "Open"}) + if open_conflicts: + frappe.throw( + _("{0} conflict(s) still unresolved.").format(open_conflicts), + frappe.ValidationError, + ) + + resolved_conflicts = frappe.get_all( + "Wiki Merge Conflict", + filters={"change_request": name, "status": "Resolved"}, + fields=["doc_key", "resolved_payload", "resolution"], + ) + resolved_map: dict[str, dict[str, Any]] = {} + resolution_side: dict[str, str] = {} + for rc in resolved_conflicts: + payload = rc.get("resolved_payload") + if isinstance(payload, str): + payload = frappe.parse_json(payload) + resolved_map[rc["doc_key"]] = payload + resolution_side[rc["doc_key"]] = rc.get("resolution", "") + + # Rebuild the merge: start from current main, apply non-conflicting + resolved + 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) + + main_changed = _find_changed_keys(base_items, ours_items) + head_changed = _find_changed_keys(base_items, theirs_items) + changed_keys = main_changed | head_changed + + 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} + + 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 + merged_items: dict[str, dict[str, Any]] = {} + for key, item in ours_items.items(): + normalized = normalize_item(item) + if normalized: + merged_items[key] = normalized + + new_conflicts = [] + for key in changed_keys: + if key in resolved_map: + # Use the resolved payload (with content_blob already set) + resolved = resolved_map[key] + if resolved: + # Ensure content_blob exists — resolve from the chosen side's payload + if not resolved.get("content_blob"): + # Re-derive content blob from ours/theirs content + res = resolution_side.get(key) + if res: + content = ( + ours_contents.get(key, "") if res == "ours" else theirs_contents.get(key, "") + ) + resolved = with_content_blob(resolved, content) + merged_items[key] = resolved + else: + merged_items.pop(key, None) + continue + + base = normalize_item(base_items.get(key)) + ours = normalize_item(ours_items.get(key)) + theirs = normalize_item(theirs_items.get(key)) + base_content = base_contents.get(key, "") + ours_content = ours_contents.get(key, "") + theirs_content = theirs_contents.get(key, "") + + result, _conflict_type = merge_items(base, ours, theirs, base_content, ours_content, theirs_content) + if _conflict_type: + new_conflicts.append( + { + "doc_key": key, + "type": _conflict_type, + "base": base, + "ours": ours, + "theirs": theirs, + } + ) + continue + if result: + merged_items[key] = result + else: + merged_items.pop(key, None) + + if new_conflicts: + for conflict_data in new_conflicts: + conflict_doc = frappe.new_doc("Wiki Merge Conflict") + conflict_doc.change_request = cr.name + conflict_doc.doc_key = conflict_data["doc_key"] + conflict_doc.conflict_type = conflict_data["type"] + conflict_doc.base_payload = conflict_data["base"] + conflict_doc.ours_payload = conflict_data["ours"] + conflict_doc.theirs_payload = conflict_data["theirs"] + conflict_doc.status = "Open" + conflict_doc.insert(ignore_permissions=True) + + if not frappe.flags.in_test: + frappe.db.commit() + + frappe.throw( + _("{0} new conflict(s) detected during merge retry.").format(len(new_conflicts)), + frappe.ValidationError, + ) + + merge_revision = create_merge_revision(cr, merged_items) + + 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 + + @frappe.whitelist() def check_outdated(name: str) -> int: cr = frappe.get_doc("Wiki Change Request", name)