Skip to content

feat(cli): add --clear flag to delete subpages before upload#60

Open
geopanther wants to merge 1 commit into
mainfrom
feat/clear-subpages
Open

feat(cli): add --clear flag to delete subpages before upload#60
geopanther wants to merge 1 commit into
mainfrom
feat/clear-subpages

Conversation

@geopanther

@geopanther geopanther commented May 13, 2026

Copy link
Copy Markdown
Owner

Summary

Add --clear flag that recursively deletes all child pages under the specified --parent-id before uploading new content. Useful for CI workflows where you want to ensure a clean slate.

Adapted from iamjackg/md2cf#114 by @stevengoossensB.

Changes

  • New CLI flag --clear (requires --parent-id)
  • Confirmation prompt before deletion — shows count of top-level children, skippable with --yes/-y
  • delete_page() and get_child_pages() added to MinimalConfluence API
  • get_child_pages() paginates — follows _links.next to fetch all children (Confluence default limit is 25)
  • Handle HTTP 204 no-content responses in _request()
  • Recursive deletion with error handling per page

Differences from upstream

  • Requires --parent-id — errors explicitly instead of silently doing nothing
  • Confirmation prompt with --yes to skip (upstream had none)
  • Paginated child page fetching (upstream only got first 25)
  • Proper 204 handling in base _request()

Tests (6 new)

  • test_delete_subpages_empty_parent — no children, no deletes
  • test_delete_subpages_flat_children — direct children deleted
  • test_delete_subpages_nested_children — depth-first recursive deletion
  • test_delete_subpages_partial_failure — continues on HTTPError
  • test_get_child_pages_pagination — follows _links.next
  • test_get_child_pages_empty — returns empty list

All 156 unit tests pass.

@geopanther

Copy link
Copy Markdown
Owner Author

Maintainer Review

Verdict: ⚠️ Request changes

The feature is useful for CI workflows, but this is a destructive operation that needs more safeguards before merging.

Blocking concerns:

  1. No confirmation prompt — Running --clear with the wrong --parent-id could wipe an entire Confluence subtree. Should add a confirmation prompt (skippable via --yes or --force) or at minimum a dry-run preview showing what would be deleted
  2. No tests — The recursive deletion logic should have unit tests mocking the API. Key scenarios: empty parent, nested children, partial failure
  3. Pagination missingget_child_pages() only fetches the first page of results (Confluence API default: 25). Large subtrees will be partially deleted. Need to paginate or use limit=500 + follow _links.next

Non-blocking suggestions:

  1. Should log the count of pages to delete before starting
  2. Consider --clear as a standalone subcommand rather than a flag, to make the destructive nature more explicit
  3. The recursive deletion could hit rate limits on large trees — consider adding a small delay between deletes

Good:

  • Requiring --parent-id is correct (upstream silently does nothing)
  • 204 handling in _request() is clean and benefits other potential DELETE/PUT operations
  • Error handling per page (doesn't abort on single failure)

Add --clear flag that recursively deletes all child pages under the
specified --parent-id before uploading new content. Requires
--parent-id to be set. Also adds delete_page() and get_child_pages()
to the API client, and handles HTTP 204 no-content responses.

Adapted from iamjackg/md2cf#114 by @stevengoossensB.
@geopanther geopanther force-pushed the feat/clear-subpages branch from a7e64c6 to e89f6d5 Compare May 13, 2026 16:23
@geopanther

Copy link
Copy Markdown
Owner Author

Updated — addressed all review feedback:

  1. Confirmation prompt — shows count of top-level children before deletion. Skippable with --yes/-y for CI use
  2. 6 unit tests added covering: empty parent, flat children, nested (depth-first), partial failure, pagination, empty results
  3. Paginationget_child_pages() now follows _links.next with limit=100 per request, fetching all children regardless of count

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant