feat: Add move command to relocate pages#32
Conversation
This commit adds a new `move` command that allows users to move Confluence pages to a different parent page, optionally renaming them in the process. ## Changes - Add `confluence move <pageId> <newParentId> [--title]` command to CLI - Implement `movePage()` method in ConfluenceClient class - Support moving pages by updating the ancestors array in the Confluence API ## Usage ```bash # Move a page to a new parent confluence move 123456789 987654321 # Move and rename a page confluence move 123456789 987654321 --title "New Page Title" ``` ## Implementation Details The move operation: 1. Fetches the current page to get version and content 2. Updates the page with new ancestors array pointing to new parent 3. Optionally updates the title if provided 4. Increments version number as required by Confluence API This feature enables better page organization and restructuring workflows within Confluence spaces. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds comprehensive test coverage for the movePage functionality to prevent regressions and document expected behavior. Tests verify basic page moves and moves with title changes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Refactor movePage method in ConfluenceClient to use destructuring and reduce verbosity while maintaining identical functionality. Remove unnecessary intermediate variables and simplify data extraction. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This reverts commit 6a0c6c0.
There was a problem hiding this comment.
Pull request overview
Adds support for relocating Confluence pages by introducing a move command backed by a new ConfluenceClient.movePage() API, with accompanying tests.
Changes:
- Added
movePage(pageId, newParentId, newTitle)to update a page’s ancestors (and optionally title). - Added unit tests covering moving a page and moving+renaming.
- Incremented page version during the move update request.
Reviewed changes
Copilot reviewed 2 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
lib/confluence-client.js |
Implements movePage() by GETing the page, building an updated payload (ancestors/version/title/body), and PUTing it back. |
tests/confluence-client.test.js |
Adds tests exercising movePage() for move-only and move+rename scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * Move a page to a new parent location | ||
| */ | ||
| async movePage(pageId, newParentId, newTitle = null) { |
There was a problem hiding this comment.
newTitle || title treats an empty string as falsy and will silently fall back to the original title. If callers intentionally pass an empty string (or other falsy values), this won’t be honored. Prefer nullish coalescing (newTitle ?? title) and consider defaulting newTitle to undefined to cleanly represent “not provided”.
| const pageData = { | ||
| id: pageId, | ||
| type: 'page', | ||
| title: newTitle || title, |
There was a problem hiding this comment.
newTitle || title treats an empty string as falsy and will silently fall back to the original title. If callers intentionally pass an empty string (or other falsy values), this won’t be honored. Prefer nullish coalescing (newTitle ?? title) and consider defaulting newTitle to undefined to cleanly represent “not provided”.
| const result = await client.movePage('123456789', '987654321'); | ||
|
|
||
| expect(result.id).toBe('123456789'); | ||
| expect(result.version.number).toBe(6); | ||
| expect(result.ancestors).toEqual([{ id: '987654321' }]); |
There was a problem hiding this comment.
These tests only assert the mocked response, not that movePage() sends the correct PUT payload (e.g., ancestors set to newParentId, version incremented, title overridden when provided, and body/space preserved). Add assertions against the captured request (e.g., via mock.history.put[0].data or an onPut matcher) to validate the behavior being introduced.
| mock.onGet('/content/123456789').reply(200, { | ||
| id: '123456789', | ||
| title: 'Original Title', | ||
| version: { number: 5 }, | ||
| body: { storage: { value: '<p>Original content</p>' } }, | ||
| space: { key: 'TEST' } | ||
| }); | ||
|
|
||
| mock.onPut('/content/123456789').reply(200, { | ||
| id: '123456789', | ||
| title: 'Original Title', | ||
| version: { number: 6 }, | ||
| ancestors: [{ id: '987654321' }] | ||
| }); | ||
|
|
||
| const result = await client.movePage('123456789', '987654321'); | ||
|
|
||
| expect(result.id).toBe('123456789'); | ||
| expect(result.version.number).toBe(6); | ||
| expect(result.ancestors).toEqual([{ id: '987654321' }]); | ||
|
|
||
| mock.restore(); | ||
| }); | ||
|
|
||
| test('should move a page with new title', async () => { | ||
| const mock = new MockAdapter(client.client); | ||
|
|
||
| mock.onGet('/content/555666777').reply(200, { | ||
| id: '555666777', | ||
| title: 'Old Title', | ||
| version: { number: 2 }, | ||
| body: { storage: { value: '<p>Page content</p>' } }, | ||
| space: { key: 'DOCS' } | ||
| }); | ||
|
|
||
| mock.onPut('/content/555666777').reply(200, { | ||
| id: '555666777', | ||
| title: 'New Title', | ||
| version: { number: 3 }, | ||
| ancestors: [{ id: '888999000' }] | ||
| }); | ||
|
|
||
| const result = await client.movePage('555666777', '888999000', 'New Title'); | ||
|
|
||
| expect(result.title).toBe('New Title'); | ||
| expect(result.version.number).toBe(3); | ||
| expect(result.ancestors).toEqual([{ id: '888999000' }]); | ||
|
|
||
| mock.restore(); |
There was a problem hiding this comment.
Calling mock.restore() at the end of the test can leak the mock if the test throws/early-fails before reaching that line. Move cleanup to an afterEach(() => mock.restore()), or wrap the test body in try/finally to ensure restore always runs.
| mock.onGet('/content/123456789').reply(200, { | |
| id: '123456789', | |
| title: 'Original Title', | |
| version: { number: 5 }, | |
| body: { storage: { value: '<p>Original content</p>' } }, | |
| space: { key: 'TEST' } | |
| }); | |
| mock.onPut('/content/123456789').reply(200, { | |
| id: '123456789', | |
| title: 'Original Title', | |
| version: { number: 6 }, | |
| ancestors: [{ id: '987654321' }] | |
| }); | |
| const result = await client.movePage('123456789', '987654321'); | |
| expect(result.id).toBe('123456789'); | |
| expect(result.version.number).toBe(6); | |
| expect(result.ancestors).toEqual([{ id: '987654321' }]); | |
| mock.restore(); | |
| }); | |
| test('should move a page with new title', async () => { | |
| const mock = new MockAdapter(client.client); | |
| mock.onGet('/content/555666777').reply(200, { | |
| id: '555666777', | |
| title: 'Old Title', | |
| version: { number: 2 }, | |
| body: { storage: { value: '<p>Page content</p>' } }, | |
| space: { key: 'DOCS' } | |
| }); | |
| mock.onPut('/content/555666777').reply(200, { | |
| id: '555666777', | |
| title: 'New Title', | |
| version: { number: 3 }, | |
| ancestors: [{ id: '888999000' }] | |
| }); | |
| const result = await client.movePage('555666777', '888999000', 'New Title'); | |
| expect(result.title).toBe('New Title'); | |
| expect(result.version.number).toBe(3); | |
| expect(result.ancestors).toEqual([{ id: '888999000' }]); | |
| mock.restore(); | |
| try { | |
| mock.onGet('/content/123456789').reply(200, { | |
| id: '123456789', | |
| title: 'Original Title', | |
| version: { number: 5 }, | |
| body: { storage: { value: '<p>Original content</p>' } }, | |
| space: { key: 'TEST' } | |
| }); | |
| mock.onPut('/content/123456789').reply(200, { | |
| id: '123456789', | |
| title: 'Original Title', | |
| version: { number: 6 }, | |
| ancestors: [{ id: '987654321' }] | |
| }); | |
| const result = await client.movePage('123456789', '987654321'); | |
| expect(result.id).toBe('123456789'); | |
| expect(result.version.number).toBe(6); | |
| expect(result.ancestors).toEqual([{ id: '987654321' }]); | |
| } finally { | |
| mock.restore(); | |
| } | |
| }); | |
| test('should move a page with new title', async () => { | |
| const mock = new MockAdapter(client.client); | |
| try { | |
| mock.onGet('/content/555666777').reply(200, { | |
| id: '555666777', | |
| title: 'Old Title', | |
| version: { number: 2 }, | |
| body: { storage: { value: '<p>Page content</p>' } }, | |
| space: { key: 'DOCS' } | |
| }); | |
| mock.onPut('/content/555666777').reply(200, { | |
| id: '555666777', | |
| title: 'New Title', | |
| version: { number: 3 }, | |
| ancestors: [{ id: '888999000' }] | |
| }); | |
| const result = await client.movePage('555666777', '888999000', 'New Title'); | |
| expect(result.title).toBe('New Title'); | |
| expect(result.version.number).toBe(3); | |
| expect(result.ancestors).toEqual([{ id: '888999000' }]); | |
| } finally { | |
| mock.restore(); | |
| } |
| mock.onGet('/content/123456789').reply(200, { | ||
| id: '123456789', | ||
| title: 'Original Title', | ||
| version: { number: 5 }, | ||
| body: { storage: { value: '<p>Original content</p>' } }, | ||
| space: { key: 'TEST' } | ||
| }); | ||
|
|
||
| mock.onPut('/content/123456789').reply(200, { | ||
| id: '123456789', | ||
| title: 'Original Title', | ||
| version: { number: 6 }, | ||
| ancestors: [{ id: '987654321' }] | ||
| }); | ||
|
|
||
| const result = await client.movePage('123456789', '987654321'); | ||
|
|
||
| expect(result.id).toBe('123456789'); | ||
| expect(result.version.number).toBe(6); | ||
| expect(result.ancestors).toEqual([{ id: '987654321' }]); | ||
|
|
||
| mock.restore(); | ||
| }); | ||
|
|
||
| test('should move a page with new title', async () => { | ||
| const mock = new MockAdapter(client.client); | ||
|
|
||
| mock.onGet('/content/555666777').reply(200, { | ||
| id: '555666777', | ||
| title: 'Old Title', | ||
| version: { number: 2 }, | ||
| body: { storage: { value: '<p>Page content</p>' } }, | ||
| space: { key: 'DOCS' } | ||
| }); | ||
|
|
||
| mock.onPut('/content/555666777').reply(200, { | ||
| id: '555666777', | ||
| title: 'New Title', | ||
| version: { number: 3 }, | ||
| ancestors: [{ id: '888999000' }] | ||
| }); | ||
|
|
||
| const result = await client.movePage('555666777', '888999000', 'New Title'); | ||
|
|
||
| expect(result.title).toBe('New Title'); | ||
| expect(result.version.number).toBe(3); | ||
| expect(result.ancestors).toEqual([{ id: '888999000' }]); | ||
|
|
||
| mock.restore(); |
There was a problem hiding this comment.
Calling mock.restore() at the end of the test can leak the mock if the test throws/early-fails before reaching that line. Move cleanup to an afterEach(() => mock.restore()), or wrap the test body in try/finally to ensure restore always runs.
| mock.onGet('/content/123456789').reply(200, { | |
| id: '123456789', | |
| title: 'Original Title', | |
| version: { number: 5 }, | |
| body: { storage: { value: '<p>Original content</p>' } }, | |
| space: { key: 'TEST' } | |
| }); | |
| mock.onPut('/content/123456789').reply(200, { | |
| id: '123456789', | |
| title: 'Original Title', | |
| version: { number: 6 }, | |
| ancestors: [{ id: '987654321' }] | |
| }); | |
| const result = await client.movePage('123456789', '987654321'); | |
| expect(result.id).toBe('123456789'); | |
| expect(result.version.number).toBe(6); | |
| expect(result.ancestors).toEqual([{ id: '987654321' }]); | |
| mock.restore(); | |
| }); | |
| test('should move a page with new title', async () => { | |
| const mock = new MockAdapter(client.client); | |
| mock.onGet('/content/555666777').reply(200, { | |
| id: '555666777', | |
| title: 'Old Title', | |
| version: { number: 2 }, | |
| body: { storage: { value: '<p>Page content</p>' } }, | |
| space: { key: 'DOCS' } | |
| }); | |
| mock.onPut('/content/555666777').reply(200, { | |
| id: '555666777', | |
| title: 'New Title', | |
| version: { number: 3 }, | |
| ancestors: [{ id: '888999000' }] | |
| }); | |
| const result = await client.movePage('555666777', '888999000', 'New Title'); | |
| expect(result.title).toBe('New Title'); | |
| expect(result.version.number).toBe(3); | |
| expect(result.ancestors).toEqual([{ id: '888999000' }]); | |
| mock.restore(); | |
| try { | |
| mock.onGet('/content/123456789').reply(200, { | |
| id: '123456789', | |
| title: 'Original Title', | |
| version: { number: 5 }, | |
| body: { storage: { value: '<p>Original content</p>' } }, | |
| space: { key: 'TEST' } | |
| }); | |
| mock.onPut('/content/123456789').reply(200, { | |
| id: '123456789', | |
| title: 'Original Title', | |
| version: { number: 6 }, | |
| ancestors: [{ id: '987654321' }] | |
| }); | |
| const result = await client.movePage('123456789', '987654321'); | |
| expect(result.id).toBe('123456789'); | |
| expect(result.version.number).toBe(6); | |
| expect(result.ancestors).toEqual([{ id: '987654321' }]); | |
| } finally { | |
| mock.restore(); | |
| } | |
| }); | |
| test('should move a page with new title', async () => { | |
| const mock = new MockAdapter(client.client); | |
| try { | |
| mock.onGet('/content/555666777').reply(200, { | |
| id: '555666777', | |
| title: 'Old Title', | |
| version: { number: 2 }, | |
| body: { storage: { value: '<p>Page content</p>' } }, | |
| space: { key: 'DOCS' } | |
| }); | |
| mock.onPut('/content/555666777').reply(200, { | |
| id: '555666777', | |
| title: 'New Title', | |
| version: { number: 3 }, | |
| ancestors: [{ id: '888999000' }] | |
| }); | |
| const result = await client.movePage('555666777', '888999000', 'New Title'); | |
| expect(result.title).toBe('New Title'); | |
| expect(result.version.number).toBe(3); | |
| expect(result.ancestors).toEqual([{ id: '888999000' }]); | |
| } finally { | |
| mock.restore(); | |
| } |
pchuri
left a comment
There was a problem hiding this comment.
Thanks for the PR — the new move command and ConfluenceClient.movePage() implementation look solid, and local npm test / npm run lint are green.
A few suggestions:
-
Scope/behavior for cross-space moves
movePage()always sendsspace: { key: space.key }from the current page in the update payload. If cross-space moves are not intended/supported, it might be worth explicitly guarding/documenting “same-space only” (or surfacing a clearer error when the new parent is in a different space). If cross-space moves are intended, the payload/validation likely needs more thought. -
Docs
README’s Commands table and examples don’t mentionmoveyet. Adding an entry + a short usage example (including--title) would help discoverability. -
CLI argument consistency
Many commands accept<pageId_or_url>, butmovecurrently requires numeric IDs for bothpageIdandnewParentId. Consider either:
- supporting URLs by reusing the existing ID extraction helper, or
- documenting clearly that
moveexpects IDs.
Addresses PR pchuri#32 review feedback: - Add URL support for both pageId and newParentId parameters for consistency with other commands - Add validation to prevent cross-space moves with clear error messages - Add comprehensive documentation to README including usage examples and command table entry - Add test coverage for URL support and cross-space validation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
# [1.16.0](v1.15.1...v1.16.0) (2026-02-13) ### Features * Add move command to relocate pages ([#32](#32)) ([a37f9b8](a37f9b8))
|
🎉 This PR is included in version 1.16.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary
Adds a
movecommand to relocate Confluence pages to a new parent location.Changes
confluence move <pageId> <newParentId>command with optional--titleflagmovePage()method in ConfluenceClientUsage
Move a page:
Move and rename:
confluence move 123456789 987654321 --title "New Title"🤖 Generated with Claude Code