fix(export): /export/etherpad honors the :rev URL segment

[JohnMcLear]

Summary

Fixes #5071. The /p/:pad/:rev/export/etherpad route has always ignored the :rev URL segment and returned the full pad history, unlike the txt/html exports on the same route which do respect rev. This broke timeslider's "Export current version (as Etherpad)" button and any integration trying to produce a rev-bounded .etherpad backup or inspect a particular snapshot.

The original maintainer reply on the issue (#5071 (comment)) concluded: "OK I think you have a point here. I'd consider it a feature request to make the etherpad export more useful to different use cases." — implementing that now.

Change

• exportEtherpad.getPadRaw(padId, readOnlyId, revNum?) takes an optional revNum. When supplied it clamps to min(revNum, pad.head), iterates only revs 0..effectiveHead, and ships a shallow-cloned pad object whose head and atext reflect the requested snapshot. The original live Pad is still passed to the exportEtherpad hook so plugin callbacks see the real document.
• ExportHandler threads req.params.rev through on the etherpad type, matching the existing behavior of txt/html.
• Chat is left full (not rev-anchored).

Out of scope

The API-side getHTML(padID, rev) call reported in the original issue is already honoring rev in current code (exportHtml.getPadHTML threads the parameter through via pad.getInternalRevisionAText). That concern appears to have been fixed between 1.8.13 (the version the reporter was on) and today; this PR does not touch it.

Test plan

• Three new regression tests in src/tests/backend/specs/ExportEtherpad.ts:
  • default (no revNum) still exports the full history
  • explicit revNum limits exported revs and rewrites the serialized head
  • revNum above head falls back to full history
• pnpm run ts-check clean locally
• CI: backend + playwright

Closes #5071

🤖 Generated with Claude Code

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Fix /export/etherpad to honor :rev URL segment for revision-bounded exports

🐞 Bug fix

Walkthroughs

Description

• /export/etherpad now respects the :rev URL segment to export bounded pad history
• getPadRaw() accepts optional revNum parameter clamping exported revisions
• Serialized pad object rewritten with bounded head and atext for correct re-import
• Three regression tests added covering full history, bounded revisions, and edge cases

Diagram

flowchart LR
  A["ExportHandler receives :rev param"] -->|passes revNum| B["getPadRaw with revNum"]
  B -->|clamps to min| C["effectiveHead = min(revNum, pad.head)"]
  C -->|iterates 0..effectiveHead| D["Filtered revisions exported"]
  C -->|fetches bounded atext| E["Serialized pad with rewritten head/atext"]
  E -->|preserves live pad| F["exportEtherpad hook sees full document"]
  D --> G["Rev-bounded .etherpad file"]
  F --> G

Loading

File Changes

1. src/node/handler/ExportHandler.ts 🐞 Bug fix +4/-1

Thread rev parameter through etherpad export

• Passes req.params.rev to getPadRaw() for etherpad export type
• Aligns etherpad export behavior with existing txt/html export handling
• Added clarifying comment referencing issue #5071

src/node/handler/ExportHandler.ts

---

2. src/node/utils/ExportEtherpad.ts 🐞 Bug fix +19/-3

Add revision bounding logic to getPadRaw export function

• getPadRaw() signature extended with optional revNum parameter
• Calculates effectiveHead by clamping revNum to pad.head
• Fetches bounded atext using pad.getInternalRevisionAText() when rev-limited
• Exports only revisions 0..effectiveHead instead of full history
• Serializes shallow-cloned pad object with rewritten head and atext for bounded exports
• Preserves original live Pad instance for exportEtherpad hook callbacks

src/node/utils/ExportEtherpad.ts

---

3. src/tests/backend/specs/ExportEtherpad.ts 🧪 Tests +48/-0

Add regression tests for revision-bounded etherpad exports

• Added test suite "revNum bounding (issue #5071)" with three regression tests
• Test 1: Verifies full history export when revNum is omitted
• Test 2: Validates revision limiting and head rewriting when revNum is supplied
• Test 3: Confirms revNum above pad.head falls back to full history
• Helper function addRevs() generates distinct revisions for testing

src/tests/backend/specs/ExportEtherpad.ts

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (1) 📎 Requirement gaps (0)

1. Rev-bounded export undocumented 📘 Rule violation ⚙ Maintainability

Description

The PR changes public behavior by making /p/:pad/:rev/export/etherpad honor :rev and by
sometimes serializing data["pad:<id>"] as a plain object rather than a Pad instance, but no
documentation in doc/ is updated to reflect this. This can break or confuse API/plugin consumers
who rely on the prior export semantics.

Code

src/node/handler/ExportHandler.ts[R70-74]

+    // Honor the :rev URL segment on `.etherpad` exports the same way the
+    // other formats already do — revNum limits the serialized pad to revs
+    // 0..rev (issue #5071).
+    const pad = await exportEtherpad.getPadRaw(padId, readOnlyId, req.params.rev);
    res.send(pad);

Evidence

Rule 12 requires documenting public API or hook behavior changes in the same PR. The diff shows the
export endpoint now threads req.params.rev into .etherpad export and
ExportEtherpad.getPadRaw() conditionally serializes the exported pad record as a shallow JSON
object; however, the existing server-side hook documentation for exportEtherpad does not mention
revision-bounded exports or that data[pad:<id>] may not be a Pad instance.

src/node/handler/ExportHandler.ts[70-74]
src/node/utils/ExportEtherpad.ts[24-73]
doc/api/hooks_server-side.md[970-991]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR changes `.etherpad` export semantics to honor `:rev` and changes the shape/semantics of the exported `data` payload (the exported pad record can be a plain JSON object when rev-bounded), but there is no corresponding documentation update under `doc/`.

## Issue Context
This behavior is externally observable via the `/p/:pad/:rev/export/etherpad` route and can also affect plugin authors via the `exportEtherpad` hook’s `data` object.

## Fix Focus Areas
- doc/api/hooks_server-side.md[970-991]
- src/node/handler/ExportHandler.ts[70-74]
- src/node/utils/ExportEtherpad.ts[24-73]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Chat clamp not implemented 🐞 Bug ⚙ Maintainability

Description

ExportEtherpad.getPadRaw() adds a comment claiming chat will be clamped when revNum is provided,
but it still exports chat messages through pad.chatHead regardless of revNum. This makes the
comment incorrect and can cause users/devs to incorrectly assume rev-bounded .etherpad exports
exclude post-revision chat content.

Code

src/node/utils/ExportEtherpad.ts[62]

    for (let i = 0; i <= pad.chatHead; ++i) yield [`${dstPfx}:chat:${i}`, pad.getChatMessage(i)];

Evidence

The newly added comment explicitly states chat will be clamped for rev-bounded exports, but the chat
export loop is still unconditional and iterates up to pad.chatHead rather than a rev-derived
boundary.

src/node/utils/ExportEtherpad.ts[30-34]
src/node/utils/ExportEtherpad.ts[61-63]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`ExportEtherpad.getPadRaw()` states (in a newly added comment) that chat will be clamped when `revNum` is provided, but the code still exports all chat messages up to `pad.chatHead`.

### Issue Context
You can fix this in one of two ways:
1) **Update the comment** to reflect actual behavior (chat remains unbounded), or
2) **Implement chat clamping** to match the comment (and, if necessary, adjust the serialized pad’s `chatHead` accordingly so imports remain consistent).

### Fix Focus Areas
- src/node/utils/ExportEtherpad.ts[30-38]
- src/node/utils/ExportEtherpad.ts[61-63]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[qodo-free-for-open-source-projects]

1. Rev-bounded export undocumented 📘 Rule violation ⚙ Maintainability

The PR changes public behavior by making /p/:pad/:rev/export/etherpad honor :rev and by
sometimes serializing data["pad:<id>"] as a plain object rather than a Pad instance, but no
documentation in doc/ is updated to reflect this. This can break or confuse API/plugin consumers
who rely on the prior export semantics.

Agent Prompt

## Issue description
The PR changes `.etherpad` export semantics to honor `:rev` and changes the shape/semantics of the exported `data` payload (the exported pad record can be a plain JSON object when rev-bounded), but there is no corresponding documentation update under `doc/`.

## Issue Context
This behavior is externally observable via the `/p/:pad/:rev/export/etherpad` route and can also affect plugin authors via the `exportEtherpad` hook’s `data` object.

## Fix Focus Areas
- doc/api/hooks_server-side.md[970-991]
- src/node/handler/ExportHandler.ts[70-74]
- src/node/utils/ExportEtherpad.ts[24-73]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools