diff --git a/e2e/tests/page-version-history.spec.ts b/e2e/tests/page-version-history.spec.ts new file mode 100644 index 000000000..06ee425e6 --- /dev/null +++ b/e2e/tests/page-version-history.spec.ts @@ -0,0 +1,111 @@ +import { type Page, expect, test } from '@playwright/test'; + +/** + * Page version history UI (issue #622). + * + * Seeds two published revisions of a page through the self-serve editor merge + * flow (add v1, then edit to v2), then opens the history browser from the page + * kebab and asserts the revision list and the content diff render. + */ + +/** Set the open editor's content via the exposed wikiEditor and save. */ +async function setEditorContentAndSave(page: Page, content: string) { + const editor = page.locator('.ProseMirror, [contenteditable="true"]').first(); + await expect(editor).toBeVisible({ timeout: 10000 }); + await page.waitForFunction(() => window.wikiEditor !== undefined, { + timeout: 10000, + }); + await page.evaluate((c) => { + window.wikiEditor.commands.setContent(c, { contentType: 'markdown' }); + }, content); + await editor.click(); + await page.getByRole('button', { name: 'Save' }).click(); + await page.waitForTimeout(500); +} + +/** Self-serve publish from the editor (submit -> approve -> merge under the hood). */ +async function mergeFromEditor(page: Page) { + await page.getByRole('button', { name: 'Merge', exact: true }).click(); + await expect(page.locator('text=Change request merged').first()).toBeVisible({ + timeout: 15000, + }); + await page.waitForURL(/\/page\//, { timeout: 10000 }); +} + +test.describe('Page Version History', () => { + test('kebab → View history lists revisions and shows a content diff', async ({ + page, + }) => { + const ts = Date.now(); + const spaceName = `History Space ${ts}`; + const spaceRoute = `history-space-${ts}`; + const pageTitle = `history-page-${ts}`; + // Single-token contents so the word-level diff renders each as one node. + const v1 = `alphacontent${ts}`; + const v2 = `bravocontent${ts}`; + + // New space. + await page.goto('/wiki/spaces'); + await page.waitForLoadState('networkidle'); + await page.getByRole('button', { name: 'New Space' }).click(); + await page.waitForSelector('[role="dialog"]', { state: 'visible' }); + await page.getByLabel('Space Name').fill(spaceName); + await page.getByLabel('Route').fill(spaceRoute); + await page + .getByRole('dialog') + .getByRole('button', { name: 'Create' }) + .click(); + await page.waitForLoadState('networkidle'); + await expect(page).toHaveURL(/\/wiki\/spaces\//); + + // First page as a draft. + const createFirstPage = page.getByRole('button', { + name: 'Create First Page', + }); + if (await createFirstPage.isVisible({ timeout: 2000 }).catch(() => false)) { + await createFirstPage.click(); + } else { + await page.getByRole('button', { name: 'New Page' }).click(); + } + await page.getByLabel('Title').fill(pageTitle); + await page + .getByRole('dialog') + .getByRole('button', { name: 'Save' }) + .click(); + await page.waitForTimeout(500); + await page.locator('aside').getByText(pageTitle, { exact: true }).click(); + await page.waitForURL(/\/draft\/[^/?#]+/); + + // Publish v1 (revision: "added"). + await setEditorContentAndSave(page, v1); + await mergeFromEditor(page); + + // Edit to v2 and publish again (revision: "edited"). + await setEditorContentAndSave(page, v2); + await mergeFromEditor(page); + + // Open history from the page kebab. The contribution banner has its own + // "More actions" kebab, so scope to the document panel header. + await page + .locator('[class*="border-b-gray-500/20"]') + .getByRole('button', { name: 'More actions' }) + .click(); + await page.getByRole('menuitem', { name: 'View history' }).click(); + await expect(page).toHaveURL(/\/page\/[^/]+\/history$/, { timeout: 10000 }); + + // The list: newest-first, an "Edited" entry above an "Added" entry. + await expect(page.getByText('Version history')).toBeVisible(); + await expect(page.getByText('Edited', { exact: true }).first()).toBeVisible( + { + timeout: 10000, + }, + ); + await expect(page.getByText('Added', { exact: true })).toBeVisible(); + + // The detail: the newest revision auto-selects and its diff shows the new + // (v2) content against the previous (v1) version. + await expect(page.getByText(v2, { exact: false }).first()).toBeVisible({ + timeout: 10000, + }); + }); +}); diff --git a/frontend/src/components/PageHistory.vue b/frontend/src/components/PageHistory.vue new file mode 100644 index 000000000..199f5b590 --- /dev/null +++ b/frontend/src/components/PageHistory.vue @@ -0,0 +1,298 @@ + + + + diff --git a/frontend/src/components/WikiDocumentPanel.vue b/frontend/src/components/WikiDocumentPanel.vue index 55d861255..34830fe0d 100644 --- a/frontend/src/components/WikiDocumentPanel.vue +++ b/frontend/src/components/WikiDocumentPanel.vue @@ -154,11 +154,13 @@ import { Dropdown, FormControl, createDocumentResource, + createResource, getCachedDocumentResource, toast, usePageMeta, } from 'frappe-ui'; import { computed, ref, shallowRef, watch } from 'vue'; +import { useRouter } from 'vue-router'; import LucideExternalLink from '~icons/lucide/external-link'; import LucideMoreVertical from '~icons/lucide/more-vertical'; import LucidePencil from '~icons/lucide/pencil'; @@ -192,10 +194,28 @@ const showRouteDialog = ref(false); const isSavingRoute = ref(false); const showPageSettingsDialog = ref(false); +const router = useRouter(); const crStore = useChangeRequestStore(); const draftStore = useDraftWorkspaceStore(); const userStore = useUserStore(); +// Gate the "View history" entry point on contribute capability — history is an +// editor tool, not a reader feature (mirrors the backend endpoint's gate). +const canContribute = ref(false); +const capabilitiesResource = createResource({ + url: 'wiki.api.get_space_capabilities', + onSuccess: (data) => { + canContribute.value = Boolean(data?.can_contribute); + }, +}); +watch( + () => props.spaceId, + (space) => { + if (space) capabilitiesResource.submit({ space }); + }, + { immediate: true }, +); + // frappe-ui caches document resources by (doctype, name), so revisiting an // already-opened page renders instantly from the cached doc while `auto` // kicks off a background revalidation (stale-while-revalidate). One resource @@ -440,6 +460,17 @@ const menuOptions = computed(() => { }, }, ]; + if (canContribute.value && props.spaceId && wikiDoc.value.doc?.name) { + options.push({ + label: __('View history'), + icon: 'clock', + onClick: () => + router.push({ + name: 'PageHistory', + params: { spaceId: props.spaceId, pageId: wikiDoc.value.doc.name }, + }), + }); + } if (githubEditUrl.value) { options.push({ label: __('Edit on GitHub'), diff --git a/frontend/src/router.js b/frontend/src/router.js index 7e298bff2..003691264 100644 --- a/frontend/src/router.js +++ b/frontend/src/router.js @@ -49,6 +49,12 @@ const routes = [ component: () => import('@/components/WikiDocumentPanel.vue'), props: true, }, + { + path: 'page/:pageId/history', + name: 'PageHistory', + component: () => import('@/components/PageHistory.vue'), + props: true, + }, { path: 'draft/:docKey', name: 'DraftChangeRequest', diff --git a/specs/page_version_history_ui.md b/specs/page_version_history_ui.md new file mode 100644 index 000000000..971582d94 --- /dev/null +++ b/specs/page_version_history_ui.md @@ -0,0 +1,197 @@ +# Page Version History UI + +Date: 2026-07-07 +Status: **Implemented.** Addresses issue [#622](https://github.com/frappe/wiki/issues/622) ("UI for Version History"). Backend revision *data model* already exists (v3); this spec adds the **read endpoints** and the **history browser UI** for a single page. + +### Progress log +- 2026-07-07 — Spec committed. Starting tracer-bullet slice 1 (list end-to-end). +- 2026-07-07 — All slices shipped: + - Backend `wiki/frappe_wiki/doctype/wiki_revision/history.py` — `get_page_history` + `diff_page_revisions`, editor-gated on `can_contribute_to_space`. Timeline reconstructed from the time-ordered published snapshot set (never a `parent_revision` walk). + - Indexes: composite `wiki_space_published_history` on `Wiki Revision` (via `on_doctype_update`) and `search_index` on `Wiki Revision Item.doc_key`. `EXPLAIN` confirms the history query is index-covered (no filesort). **Gotcha:** `on_doctype_update` only runs when the doctype re-syncs, so `wiki_revision.json`'s `modified` was bumped to force the re-sync on migrate (existing sites won't create the index otherwise). + - Frontend: `PageHistory.vue` (master–detail, change-type badges, CR links, author avatars, relative time, split/unified diff toggle, empty/loading/error states), nested route `PageHistory`, and a contribute-gated "View history" kebab item in `WikiDocumentPanel.vue`. + - Tests: 7 backend unit tests (`test_wiki_revision.py`, incl. the concurrent-merge guard, verified by temp-revert) + 1 Playwright e2e (`e2e/tests/page-version-history.spec.ts`). All green. + +## Goal + +Let an **editor** (contributor) open a wiki page and browse its version history: a chronological list of every published revision that changed the page, and a content diff between a selected revision and its predecessor. **Read-only browse** — no restore/revert in this iteration. Scope is **per-page** (one `Wiki Document` / `doc_key`), not a space-wide timeline. + +This is an **editor-facing** tool, not a reader feature: it is gated on contribute capability and surfaced only in the editing context, not on the public rendered page. Plain readers never see it. + +Decisions (locked with maintainer): +- Audience: **editors / contributors** — gated on `can_contribute_to_space`, **not** `can_read_space`. (Tighten to `can_write_space` if history should be limited to direct writers rather than CR proposers.) +- Granularity: **page history only** (deferred: space-wide timeline). +- Capability: **read-only browse** (deferred: restore-to-revision, which would open a seeded Change Request). +- Placement: **dedicated nested route** `/spaces/:spaceId/page/:pageId/history`, reached only from an editor-context entry point. + +## Current State + +### What exists (the "backend is there" from the issue) + +The v3 revision engine (`wiki/frappe_wiki/`) is a git-like content-addressed model: + +- **`Wiki Revision`** — an immutable snapshot of a whole space tree. Fields: `wiki_space`, `parent_revision`, `change_request`, `message`, `is_merge`, `is_working`, `is_overlay`, `created_by`, `created_at`, `tree_hash`, `content_hash`, `doc_count`. +- **`Wiki Revision Item`** — one row per page inside a revision. Keyed by stable `doc_key`. Holds metadata (`title`, `slug`, `route`, `is_group`, `is_published`, `is_external_link`, `external_url`, `parent_key`, `order_index`, `is_deleted`) and a `content_blob` link. +- **`Wiki Content Blob`** — deduplicated content, addressed by SHA-256 `hash`. Identical content across revisions ⇒ same blob ⇒ same hash. +- **`Wiki Space.main_revision`** — pointer to the current published head revision. + +Published history of a space advances by appending non-working, non-overlay `Wiki Revision`s and moving `main_revision`: +- Bootstrap: `_bootstrap_main_revision` → `create_revision_from_live_tree(...)` (`is_working=0`, `is_overlay=0`). +- CR merge: `create_merge_revision` (`is_merge=1`, `is_working=0`, `is_overlay=0`) then `main_revision = merge_revision`. +- Git sync / direct advance: `create_revision_from_live_tree` (`wiki/wiki/git_sync.py`, `wiki/api/wiki_space.py`). +- CR working heads are `is_overlay=1` (excluded from published history). + +### What's missing + +- **No read endpoint** to list a page's history or diff two revisions. `wiki_change_request.diff_change_request` diffs a CR's `base`↔`head` only — coupled to a CR, not to two arbitrary revisions. The only `get_revisions` in the repo (`wiki/wiki/doctype/wiki_page_revision/`) is the **legacy v2** model — unrelated. +- **No UI.** Frontend (`frontend/src`) has `DiffViewer.vue` (wraps `@pierre/diffs`) and the CR review flow, but nothing surfacing per-page history. + +## Key design decision: how to reconstruct one page's history + +The `parent_revision` pointer is **not** a clean linked list. In a three-way merge, `create_merge_revision` sets `parent_revision = cr.base_revision` (the CR's base, i.e. old main), **not** the current main — so walking `parent_revision` from `main_revision` can skip concurrently-merged revisions. Do **not** reconstruct history by walking parents. + +**Robust approach — query the published snapshot set by time:** + +``` +revisions = Wiki Revision WHERE wiki_space = X AND is_working = 0 AND is_overlay = 0 + ORDER BY created_at ASC +``` + +This is exactly the set of published states (bootstrap + git-sync advances + CR merges). CR working overlays are excluded (`is_overlay=1`). No non-test caller creates stray non-working/non-overlay revisions (`clone_revision` has no production callers; `create_revision_from_live_tree` is only ever used to seed/advance main). A rejected CR never reaches `create_merge_revision`, so there are no orphan merge snapshots. + +For the target `doc_key`, walk that time-ordered list, comparing each revision's item against the previous one that contained the page. Diffing consecutive materialized states is correct **even across the merge-parent quirk**: a three-way merge builds its item set starting from current main, so `M(prev) → M(next)` shows exactly what that merge changed for the page — no more, no less. + +## Backend + +New module: `wiki/frappe_wiki/doctype/wiki_revision/history.py`. Reuse helpers already in `wiki_revision.py`: `get_revision_item_map`, `get_contents_for_items` (in `wiki_change_request.py` — move/import). + +### Indexes (performance) + +The history query `WHERE wiki_space = X AND is_working = 0 AND is_overlay = 0 ORDER BY created_at` currently has no supporting index (checked the `Wiki Revision` JSON — no `search_index`/composite). Without one it's a full-table scan + filesort that degrades as revisions accumulate. Add composite indexes via module-level `on_doctype_update()` (Frappe runs it on every `bench migrate`; `frappe.db.add_index` is idempotent — composite indexes **cannot** be declared through DocType JSON field flags). + +- **`Wiki Revision`** — `(wiki_space, is_working, is_overlay, created_at)`. `wiki_space` + the two booleans are equality predicates (leftmost), `created_at` serves both the range and the `ORDER BY` — the whole WHERE+sort is index-covered. + ```python + # wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.py + def on_doctype_update(): + frappe.db.add_index( + "Wiki Revision", + ["wiki_space", "is_working", "is_overlay", "created_at"], + index_name="wiki_space_published_history", + ) + ``` + (Leaner alternative if index width is a concern: `(wiki_space, created_at)` — the booleans become cheap residual filters on the already-narrow per-space set.) +- **`Wiki Revision Item`** — index on `doc_key` so the per-page batch load (`WHERE revision IN (...) AND doc_key = X`) narrows to one page's items directly instead of scanning. `doc_key` is highly selective; set `"search_index": 1` on the field in the DocType JSON (single-column — JSON flag is fine). Optionally composite `(doc_key, revision)` via `on_doctype_update` if the `IN (...)` set is large for old spaces. + +Confirm with `EXPLAIN` on a seeded space that the history query uses `wiki_space_published_history` (no `Using filesort`). + +### Endpoint 1 — `get_page_history(page: str)` + +`@frappe.whitelist()`. `page` = `Wiki Document.name` (matches the `:pageId` route param). Resolve `doc_key` + `wiki_space` server-side; permission-gate on `can_contribute_to_space(wiki_space)` — **editor gate, not read gate** (throw `PermissionError` otherwise). + +Algorithm: +1. Load the time-ordered published revision set (query above), fields: `name, change_request, message, created_by, created_at`. +2. Batch-load `Wiki Revision Item` for these revisions **and** this `doc_key` in one query; join `content_blob → hash`. +3. Walk chronologically tracking previous `(content_hash, metadata)`. Emit an entry only when the page **changed**: + - **added** — first revision containing the page (item present, not deleted, no prior). + - **edited** — `content_hash` differs from prior. + - **renamed / moved / (un)published** — content unchanged but a tracked metadata field (`title`, `slug`, `route`, `parent_key`, `is_published`, external link) differs. (Classify as `edited` if both content and metadata changed.) + - **deleted** — item becomes `is_deleted` or disappears after having existed. + - Skip revisions where nothing about the page changed (dedup via content_hash collapses no-op churn automatically). +4. Enrich: `Wiki Change Request.title` for `change_request` (if set), and author (`User.full_name`, `user_image`) for `created_by`. + +Returns newest-first list: +```python +[ + { + "revision": "abc123", # Wiki Revision name + "change_request": "CR-0142", # or None (git-sync / bootstrap) + "cr_title": "Fix install docs", # or None + "message": "Merge CR-0142", + "change_type": "edited", # added | edited | renamed | deleted + "title": "Installation", # page title at this revision + "author": {"name": "hussain@…", "full_name": "Hussain", "user_image": "/files/…"}, + "timestamp": "2026-07-07 10:11:12", + }, + ... +] +``` + +### Endpoint 2 — `diff_page_revisions(page: str, revision: str, base_revision: str | None = None)` + +`@frappe.whitelist()`. Same `can_contribute_to_space` gate. Diffs the page between `revision` and its predecessor. If `base_revision` is omitted, derive the predecessor = the previous **history entry's** revision (the last published revision before `revision` where the page changed); `None` predecessor ⇒ page was added, base content is empty. + +Return shape mirrors `diff_change_request(scope="page")` so `DiffViewer.vue` consumes it unchanged: +```python +{ + "doc_key": "…", + "base": {"title": …, "content": …, "route": …, "is_published": …} | None, + "head": {"title": …, "content": …, "route": …, "is_published": …} | None, +} +``` +Resolve `content` from `content_blob` via `Wiki Content Blob.content`. + +## Frontend + +### Route + +Add a child under `/spaces/:spaceId` in `frontend/src/router.js`, sibling of `SpacePage`: +```js +{ + path: 'page/:pageId/history', + name: 'PageHistory', + component: () => import('@/components/PageHistory.vue'), + props: true, +} +``` + +### Entry point + +Add a **"View history"** item to the page kebab (⋯) menu in `WikiDocumentPanel.vue` (near `PageSettings`), **shown only when the user can contribute** — reuse the existing `capabilitiesResource.can_contribute` pattern from `ContributionReview.vue`. Not rendered for plain readers. On click: `router.push({ name: 'PageHistory', params: { spaceId, pageId } })`. The panel already has `spaceId` (prop) and the document (`wikiDoc.doc`). + +### `PageHistory.vue` + +Master–detail, styled to match `ContributionReview.vue` (header bar, `Badge`, avatars, responsive collapse): + +- **Header** — Back button (returns to the page), page title, breadcrumb. +- **Left list** — `get_page_history(page=pageId)` via `createResource`. Each row: author avatar, relative time, `change_type` badge, and `cr_title`/`message` with a link to the CR review (`ChangeRequestReview`) when `change_request` is set. Newest first; auto-select the newest on load. +- **Right detail** — on select, `diff_page_revisions(page, revision)` → feed `base.content`/`head.content` into `DiffViewer` (`oldContent`/`newContent`, `fileName` = page title, split/unified toggle reused from the CR review). +- **States** — loading skeletons; **empty state** when the page has only one version ("No earlier versions of this page yet."); error surface. +- **Mobile** — full-screen; list stacks above the diff (mirror `ContributionReview` breakpoints). i18n every string via `__()`. + +Change-type badge themes: `added` green, `edited` blue, `renamed` gray, `deleted` red. + +## Tracer bullets (build order) + +Each slice is a thin vertical cut through all layers — ship and eyeball before the next. + +1. **List end-to-end.** `get_page_history` + route + `PageHistory.vue` rendering the revision list only (no diff). Kebab entry point. → Click History, see the timeline. +2. **Diff.** `diff_page_revisions` + wire `DiffViewer` into the detail pane; auto-select newest. → Select an entry, see the content diff. +3. **Polish.** change-type badges, author enrichment, CR links, empty/loading/error states, mobile layout, i18n sweep. + +Commit the spec first, then one commit per slice (per CLAUDE.md). + +## Tests + +Per CLAUDE.md regression protocol (temp-revert to confirm the test bites): + +- **Backend unit** (`test_wiki_revision.py` / history module): + - Page edited across N merges ⇒ history length = number of changing revisions; unchanged revisions skipped. + - Classification: added / edited / renamed / deleted each produce the right `change_type`. + - Identical content re-saved (same blob) ⇒ **no** spurious entry. + - Three-way merge with a concurrent merge ⇒ the concurrently-merged revision still appears (guards the "don't walk parent_revision" decision). + - `diff_page_revisions` returns correct base/head content; omitted `base_revision` picks the right predecessor; added-page ⇒ empty base. + - Permission: **non-contributor** (read-only user) ⇒ `PermissionError` on both endpoints; contributor ⇒ allowed. +- **E2E** (Playwright): space + page, make 2–3 edits via CR merges, open History, assert entries render and the diff shows the change. (Local run: `BASE_URL=http://wiki.localhost:8000` — see memory.) + +## Edge cases + +- **`pageId` = Wiki Document name**, resolved server-side to `doc_key` + `wiki_space` (centralizes permission + space lookup). +- **Deleted pages** — history of a live page only; browsing history of an already-deleted page is out of scope this iteration. +- **Never-changed page** — exactly one entry (added); empty diff state. +- **Non-CR revisions** (bootstrap / git-sync) — `change_request` is `None`; show `message` instead of a CR link. +- **Overlay/working revisions never leak** — filter excludes them, so unmerged drafts never appear in published history. + +## Out of scope (future issues) + +- Space-wide history timeline. +- Restore/revert a page to a past revision (would seed a Change Request). +- "View at this point" standalone render, cross-page compare, blame/annotate. diff --git a/wiki/frappe_wiki/doctype/wiki_revision/history.py b/wiki/frappe_wiki/doctype/wiki_revision/history.py new file mode 100644 index 000000000..ae4103a7e --- /dev/null +++ b/wiki/frappe_wiki/doctype/wiki_revision/history.py @@ -0,0 +1,318 @@ +# Copyright (c) 2026, Frappe and contributors +# For license information, please see license.txt + +"""Read endpoints for a single page's published version history. + +A page's history is the set of published space snapshots (`Wiki Revision` with +`is_working = 0` and `is_overlay = 0`) in which that page changed. We reconstruct +it by materializing the page's item in each published revision, in time order, +and emitting an entry only when the page actually changed — never by walking the +`parent_revision` chain, which skips concurrently-merged revisions (see the spec +`specs/page_version_history_ui.md`). +""" + +from __future__ import annotations + +from typing import Any + +import frappe +from frappe import _ + +# Content-identical revisions where one of these differs count as a "renamed" +# style entry (a rename, move, route change, or (un)publish) rather than an edit. +_TRACKED_META_FIELDS = ( + "title", + "slug", + "route", + "parent_key", + "is_published", + "is_external_link", + "external_url", +) + +_ITEM_FIELDS = [ + "revision", + "title", + "slug", + "route", + "parent_key", + "is_published", + "is_external_link", + "external_url", + "is_deleted", + "content_blob", +] + + +@frappe.whitelist() +def get_page_history(page: str) -> list[dict[str, Any]]: + """Newest-first list of published revisions in which `page` changed. + + `page` is a `Wiki Document.name` (the `:pageId` route param). Gated on + contribute capability — this is an editor tool, not a reader feature. + """ + doc_key, wiki_space = _resolve_page(page) + timeline = _load_page_timeline(doc_key, wiki_space) + return _to_history_entries(timeline) + + +@frappe.whitelist() +def diff_page_revisions(page: str, revision: str, base_revision: str | None = None) -> dict[str, Any]: + """Diff `page` between `revision` and its predecessor. + + When `base_revision` is omitted the predecessor is the previous history + entry's revision (the last published revision before `revision` where the + page changed); no predecessor means the page was added, so `base` is None. + Mirrors `diff_change_request(scope="page")` so `DiffViewer.vue` consumes it + unchanged. + """ + doc_key, wiki_space = _resolve_page(page) + + if base_revision is None: + base_revision = _predecessor_revision(doc_key, wiki_space, revision) + + head = _resolve_page_at_revision(revision, doc_key) + base = _resolve_page_at_revision(base_revision, doc_key) if base_revision else None + return {"doc_key": doc_key, "base": base, "head": head} + + +# --- Resolution + permission gate -------------------------------------------- + + +def _resolve_page(page: str) -> tuple[str, str]: + """Resolve a Wiki Document name to (doc_key, wiki_space) and gate access. + + Centralizes the space lookup and the permission check for both endpoints. + Editors gate: `can_contribute_to_space`, not `can_read_space`. + """ + row = frappe.db.get_value("Wiki Document", page, ["doc_key", "wiki_space"], as_dict=True) + if not row: + frappe.throw(_("Page not found."), frappe.DoesNotExistError) + + doc_key = row.doc_key + wiki_space = row.wiki_space + if not wiki_space: + # `wiki_space` isn't always denormalized onto the document; fall back to + # the nested-set root-group lookup (mirrors WikiDocument.check_space_access). + wiki_space = (frappe.get_doc("Wiki Document", page).get_wiki_space() or {}).get("name") + + if not doc_key or not wiki_space: + frappe.throw(_("This page has no version history yet.")) + + from wiki.permissions import can_contribute_to_space + + if not can_contribute_to_space(wiki_space): + frappe.throw( + _("You are not allowed to view this page's history."), + frappe.PermissionError, + ) + return doc_key, wiki_space + + +# --- Timeline reconstruction ------------------------------------------------- + + +def _published_revisions(wiki_space: str) -> list[dict[str, Any]]: + """Time-ordered published snapshots of a space (oldest first). + + This is the exact set of published states — bootstrap, git-sync advances, and + CR merges. Overlay/working revisions are excluded, so unmerged drafts never + leak. `creation` breaks ties when two revisions share a `created_at` second. + """ + return frappe.get_all( + "Wiki Revision", + filters={"wiki_space": wiki_space, "is_working": 0, "is_overlay": 0}, + fields=["name", "change_request", "message", "created_by", "created_at"], + order_by="created_at asc, creation asc", + ) + + +def _load_page_timeline(doc_key: str, wiki_space: str) -> list[dict[str, Any]]: + """Ordered (oldest-first) list of revisions in which the page changed. + + Walks the published snapshot set, comparing each revision's item for the page + against the previous one that contained it. Diffing consecutive materialized + states is correct even across the merge-parent quirk: a three-way merge builds + its item set from current main, so M(prev) -> M(next) shows exactly what that + merge changed for the page. + """ + revisions = _published_revisions(wiki_space) + if not revisions: + return [] + + rev_names = [rev.name for rev in revisions] + items = frappe.get_all( + "Wiki Revision Item", + filters={"revision": ("in", rev_names), "doc_key": doc_key}, + fields=_ITEM_FIELDS, + ) + _attach_content_hashes(items) + item_by_rev = {item.revision: item for item in items} + + timeline: list[dict[str, Any]] = [] + prev: dict[str, Any] | None = None # last revision in which the page was present + for rev in revisions: + item = item_by_rev.get(rev.name) + present = bool(item) and not item.get("is_deleted") + change_type = _classify(prev, item, present) + if change_type: + snapshot = item if present else prev + timeline.append( + { + "revision": rev.name, + "change_request": rev.change_request, + "message": rev.message, + "created_by": rev.created_by, + "created_at": rev.created_at, + "change_type": change_type, + "title": (snapshot or {}).get("title"), + } + ) + prev = item if present else None + return timeline + + +def _classify(prev: dict[str, Any] | None, item: dict[str, Any] | None, present: bool) -> str | None: + """Change type of `item` relative to the page's previous present state. + + Returns None when nothing about the page changed (deduped away). + """ + if present and prev is None: + return "added" + if present and prev is not None: + if _content_key(item) != _content_key(prev): + return "edited" + if _meta_key(item) != _meta_key(prev): + return "renamed" + return None + if not present and prev is not None: + return "deleted" + return None + + +def _content_key(item: dict[str, Any]) -> str: + # Prefer the blob hash (dedup key); fall back to the blob name so identical + # content always collapses even if a hash is somehow missing. + return item.get("content_hash") or item.get("content_blob") or "" + + +def _meta_key(item: dict[str, Any]) -> tuple: + return tuple(item.get(field) for field in _TRACKED_META_FIELDS) + + +def _attach_content_hashes(items: list[dict[str, Any]]) -> None: + blob_names = {item.content_blob for item in items if item.get("content_blob")} + hashes: dict[str, str] = {} + if blob_names: + hashes = { + blob.name: blob.hash + for blob in frappe.get_all( + "Wiki Content Blob", + fields=["name", "hash"], + filters={"name": ("in", list(blob_names))}, + ) + } + for item in items: + item["content_hash"] = hashes.get(item.get("content_blob")) + + +# --- Diff resolution --------------------------------------------------------- + + +def _predecessor_revision(doc_key: str, wiki_space: str, revision: str) -> str | None: + """The published revision the page changed in just before `revision`.""" + timeline = _load_page_timeline(doc_key, wiki_space) + revs = [entry["revision"] for entry in timeline] + if revision in revs: + index = revs.index(revision) + return revs[index - 1] if index > 0 else None + + # `revision` isn't itself a change point (the UI never sends one, but stay + # robust): pick the last change strictly before it in time. + target_created = frappe.db.get_value("Wiki Revision", revision, "created_at") + predecessor = None + for entry in timeline: + if entry["created_at"] and target_created and entry["created_at"] < target_created: + predecessor = entry["revision"] + return predecessor + + +def _resolve_page_at_revision(revision: str, doc_key: str) -> dict[str, Any] | None: + """Materialize the page's content + key metadata at a published revision. + + None when the page is absent or deleted there (an empty side of the diff). + Published revisions are full (non-overlay) snapshots, so a direct item lookup + resolves the state — no overlay inheritance to walk. + """ + item = frappe.db.get_value( + "Wiki Revision Item", + {"revision": revision, "doc_key": doc_key}, + ["title", "route", "is_published", "content_blob", "is_deleted"], + as_dict=True, + ) + if not item or item.is_deleted: + return None + + content = "" + if item.content_blob: + content = frappe.db.get_value("Wiki Content Blob", item.content_blob, "content") or "" + + return { + "title": item.title, + "content": content, + "route": item.route, + "is_published": item.is_published, + } + + +# --- Enrichment -------------------------------------------------------------- + + +def _to_history_entries(timeline: list[dict[str, Any]]) -> list[dict[str, Any]]: + """Newest-first display entries with CR titles and author details resolved.""" + entries = list(reversed(timeline)) + + cr_names = {entry["change_request"] for entry in entries if entry["change_request"]} + cr_titles: dict[str, str] = {} + if cr_names: + cr_titles = { + cr.name: cr.title + for cr in frappe.get_all( + "Wiki Change Request", + filters={"name": ("in", list(cr_names))}, + fields=["name", "title"], + ) + } + + user_names = {entry["created_by"] for entry in entries if entry["created_by"]} + users: dict[str, Any] = {} + if user_names: + users = { + user.name: user + for user in frappe.get_all( + "User", + filters={"name": ("in", list(user_names))}, + fields=["name", "full_name", "user_image"], + ) + } + + result: list[dict[str, Any]] = [] + for entry in entries: + author = users.get(entry["created_by"]) or {} + result.append( + { + "revision": entry["revision"], + "change_request": entry["change_request"], + "cr_title": cr_titles.get(entry["change_request"]), + "message": entry["message"], + "change_type": entry["change_type"], + "title": entry["title"], + "author": { + "name": entry["created_by"], + "full_name": author.get("full_name") or entry["created_by"], + "user_image": author.get("user_image"), + }, + "timestamp": entry["created_at"], + } + ) + return result diff --git a/wiki/frappe_wiki/doctype/wiki_revision/test_wiki_revision.py b/wiki/frappe_wiki/doctype/wiki_revision/test_wiki_revision.py index a2a07ff5a..b110982aa 100644 --- a/wiki/frappe_wiki/doctype/wiki_revision/test_wiki_revision.py +++ b/wiki/frappe_wiki/doctype/wiki_revision/test_wiki_revision.py @@ -1,8 +1,28 @@ # Copyright (c) 2026, Frappe and Contributors # See license.txt -# import frappe +import frappe +from frappe.core.doctype.user_permission.test_user_permission import create_user from frappe.tests import IntegrationTestCase +from frappe.tests.utils import FrappeTestCase + +from wiki.frappe_wiki.doctype.wiki_change_request.test_wiki_change_request import ( + _approve_and_merge, + create_test_wiki_document, + create_test_wiki_space, +) +from wiki.frappe_wiki.doctype.wiki_change_request.wiki_change_request import ( + create_change_request, + create_cr_page, + delete_cr_page, + update_cr_page, +) +from wiki.frappe_wiki.doctype.wiki_revision.history import ( + _classify, + _load_page_timeline, + diff_page_revisions, + get_page_history, +) # On IntegrationTestCase, the doctype test records and all # link-field test record dependencies are recursively loaded @@ -18,3 +38,191 @@ class IntegrationTestWikiRevision(IntegrationTestCase): """ pass + + +class TestPageHistory(FrappeTestCase): + """Read endpoints for a single page's published version history.""" + + def tearDown(self): + frappe.db.rollback() + + def _doc_key(self, name): + return frappe.get_value("Wiki Document", name, "doc_key") + + def _root_key(self, space): + return frappe.get_value("Wiki Document", space.root_group, "doc_key") + + # --- Classification -------------------------------------------------------- + + def test_classify_change_types(self): + base = { + "content_hash": "h1", + "content_blob": "b1", + "title": "A", + "slug": "a", + "route": "a", + "parent_key": None, + "is_published": 1, + "is_external_link": 0, + "external_url": None, + } + self.assertEqual(_classify(None, base, True), "added") + self.assertEqual(_classify(base, {**base, "content_hash": "h2"}, True), "edited") + self.assertEqual(_classify(base, {**base, "title": "B"}, True), "renamed") + self.assertIsNone(_classify(base, base, True)) + self.assertEqual(_classify(base, None, False), "deleted") + self.assertIsNone(_classify(None, None, False)) + + # --- Timeline reconstruction ---------------------------------------------- + + def test_history_lists_only_changing_revisions(self): + space = create_test_wiki_space() + page = create_test_wiki_document(space.root_group, title="Install", content="v1") + page_key = self._doc_key(page.name) + + # The first CR bootstraps main_revision — the page's first published + # snapshot (change_type "added"). + cr1 = create_change_request(space.name, "edit to v2") + update_cr_page(cr1.name, page_key, {"content": "v2"}) + _approve_and_merge(cr1.name) + + cr2 = create_change_request(space.name, "edit to v3") + update_cr_page(cr2.name, page_key, {"content": "v3"}) + _approve_and_merge(cr2.name) + + # A merge that never touches this page must not add an entry for it. + cr3 = create_change_request(space.name, "add sibling") + create_cr_page(cr3.name, self._root_key(space), "Other", content="x") + skip_rev = _approve_and_merge(cr3.name) + + history = get_page_history(page.name) + self.assertEqual([e["change_type"] for e in history], ["edited", "edited", "added"]) + self.assertNotIn(skip_rev, [e["revision"] for e in history]) + + def test_identical_content_produces_no_entry(self): + space = create_test_wiki_space() + page = create_test_wiki_document(space.root_group, title="Doc", content="same") + page_key = self._doc_key(page.name) + + cr = create_change_request(space.name, "resave same") + # Re-save byte-identical content: dedups to the same blob, so no real change. + update_cr_page(cr.name, page_key, {"content": "same"}) + # A sibling gives the CR real changes to merge without touching this page. + create_cr_page(cr.name, self._root_key(space), "Sibling", content="x") + _approve_and_merge(cr.name) + + history = get_page_history(page.name) + self.assertEqual([e["change_type"] for e in history], ["added"]) + + def test_deleted_page_shows_in_timeline(self): + space = create_test_wiki_space() + page = create_test_wiki_document(space.root_group, title="Temp", content="v1") + page_key = self._doc_key(page.name) + + cr1 = create_change_request(space.name, "edit") + update_cr_page(cr1.name, page_key, {"content": "v2"}) + _approve_and_merge(cr1.name) + + cr2 = create_change_request(space.name, "delete") + delete_cr_page(cr2.name, page_key) + _approve_and_merge(cr2.name) + + # The live document is gone, so query the timeline by (doc_key, space) + # directly — browsing a deleted page's history via the endpoint is out of + # scope, but the delete must still register as a change point. + timeline = _load_page_timeline(page_key, space.name) + self.assertEqual([e["change_type"] for e in timeline], ["added", "edited", "deleted"]) + + def test_concurrent_merge_revision_appears(self): + """Guards the "don't walk parent_revision" decision. + + Two CRs branch from the same main M0; merging the second is a three-way + merge whose revision points parent_revision at M0, skipping the first + merge M1. Walking parents from the head would miss M1 — the time-ordered + query must still surface it in the page's history. + """ + space = create_test_wiki_space() + page_a = create_test_wiki_document(space.root_group, title="A", content="a1") + page_b = create_test_wiki_document(space.root_group, title="B", content="b1") + a_key = self._doc_key(page_a.name) + b_key = self._doc_key(page_b.name) + + # Both CRs branch off the same bootstrap main (M0). + cr_a = create_change_request(space.name, "edit A") + cr_b = create_change_request(space.name, "edit B") + m0 = cr_a.base_revision + self.assertEqual(cr_b.base_revision, m0) + + update_cr_page(cr_a.name, a_key, {"content": "a2"}) + update_cr_page(cr_b.name, b_key, {"content": "b2"}) + + m1 = _approve_and_merge(cr_a.name) # fast-forward: main -> M1 + m2 = _approve_and_merge(cr_b.name) # three-way: parent_revision = M0 + + # The quirk exists: M2 skips M1 in the parent chain. + self.assertEqual(frappe.db.get_value("Wiki Revision", m2, "parent_revision"), m0) + + a_revs = [e["revision"] for e in get_page_history(page_a.name)] + self.assertIn(m1, a_revs) # a parent-walk from the head would miss this + + b_revs = [e["revision"] for e in get_page_history(page_b.name)] + self.assertIn(m2, b_revs) + + # --- Diff ------------------------------------------------------------------ + + def test_diff_page_revisions(self): + space = create_test_wiki_space() + page = create_test_wiki_document(space.root_group, title="Doc", content="v1") + page_key = self._doc_key(page.name) + + cr1 = create_change_request(space.name, "v2") + update_cr_page(cr1.name, page_key, {"content": "v2"}) + m1 = _approve_and_merge(cr1.name) + + cr2 = create_change_request(space.name, "v3") + update_cr_page(cr2.name, page_key, {"content": "v3"}) + m2 = _approve_and_merge(cr2.name) + + history = get_page_history(page.name) + self.assertEqual(len(history), 3) # added + two edits + bootstrap_rev = history[-1]["revision"] + + # Explicit base revision. + diff = diff_page_revisions(page.name, m2, base_revision=m1) + self.assertEqual(diff["base"]["content"], "v2") + self.assertEqual(diff["head"]["content"], "v3") + + # Omitted base → predecessor derived from the history walk. + diff2 = diff_page_revisions(page.name, m1) + self.assertEqual(diff2["base"]["content"], "v1") + self.assertEqual(diff2["head"]["content"], "v2") + + # The added revision has no predecessor → empty base. + diff3 = diff_page_revisions(page.name, bootstrap_rev) + self.assertIsNone(diff3["base"]) + self.assertEqual(diff3["head"]["content"], "v1") + + # --- Permission ------------------------------------------------------------ + + def test_history_requires_contribute_permission(self): + space = create_test_wiki_space() + page = create_test_wiki_document(space.root_group, title="Doc", content="v1") + page_key = self._doc_key(page.name) + cr = create_change_request(space.name, "v2") + update_cr_page(cr.name, page_key, {"content": "v2"}) + m1 = _approve_and_merge(cr.name) + + # Read-only user on a space that no longer accepts contributions. + frappe.db.set_value("Wiki Space", space.name, "allow_contributions", 0) + reader = create_user("history-reader@example.com", "Wiki User") + frappe.set_user(reader.name) + try: + with self.assertRaises(frappe.PermissionError): + get_page_history(page.name) + with self.assertRaises(frappe.PermissionError): + diff_page_revisions(page.name, m1) + finally: + frappe.set_user("Administrator") + + # A manager (Administrator) can always view. + self.assertTrue(get_page_history(page.name)) diff --git a/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.json b/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.json index 2b546096a..390799eda 100644 --- a/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.json +++ b/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.json @@ -115,7 +115,7 @@ } ], "links": [], - "modified": "2026-02-17 11:57:07.163673", + "modified": "2026-07-07 12:00:00.000000", "modified_by": "Administrator", "module": "Frappe Wiki", "name": "Wiki Revision", diff --git a/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.py b/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.py index a1cb1b1f6..19529cb52 100644 --- a/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.py +++ b/wiki/frappe_wiki/doctype/wiki_revision/wiki_revision.py @@ -16,6 +16,19 @@ class WikiRevision(Document): pass +def on_doctype_update(): + # The per-page history query filters published snapshots of a space and sorts + # them by time: WHERE wiki_space = X AND is_working = 0 AND is_overlay = 0 + # ORDER BY created_at. The leftmost three columns are equality predicates and + # `created_at` covers both the range and the sort, so the whole query is + # index-served (no filesort). See history.py / the version-history spec. + frappe.db.add_index( + "Wiki Revision", + ["wiki_space", "is_working", "is_overlay", "created_at"], + index_name="wiki_space_published_history", + ) + + def create_revision_from_live_tree( wiki_space: str, message: str | None = None, 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 2f0c486e0..5c3e78d72 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 @@ -34,7 +34,8 @@ "fieldtype": "Data", "in_list_view": 1, "label": "Doc Key", - "reqd": 1 + "reqd": 1, + "search_index": 1 }, { "fieldname": "title",