-
Notifications
You must be signed in to change notification settings - Fork 3
Request child pages using pagination (#61) #62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
* fix(#60): Use manual request to get children pages paginated
|
Check SPDX headers ✅ 242 files have valid headers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements paginated fetching of child pages in both markdown-confluence-sync and confluence-sync packages, adds new mock routes and collections for confluence-get-page-children, updates tests to validate pagination logic, and bumps package versions.
- Bump versions to 2.1.1 (
markdown-confluence-sync) and 2.0.2 (confluence-sync) with accompanying CHANGELOG entries. - Add
_getChildPagesrecursive pagination method inCustomConfluenceClientand integrate it intogetPage. - Introduce
getPageChildrenMiddleware, addconfluence-get-page-childrenroutes and collections, and update unit/component tests.
Reviewed Changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| components/markdown-confluence-sync/package.json | Bumped version to 2.1.1 |
| components/markdown-confluence-sync/mocks/routes/Confluence.ts | Added getPageChildrenMiddleware and route variants |
| components/markdown-confluence-sync/mocks/collections.ts | Included confluence-get-page-children in mock collections |
| components/markdown-confluence-sync/CHANGELOG.md | Documented pagination fix in 2.1.1 |
| components/confluence-sync/src/confluence/CustomConfluenceClient.ts | Introduced _getChildPages, updated getPage logic |
| components/confluence-sync/package.json | Bumped version to 2.0.2 |
| components/confluence-sync/mocks/routes/Confluence.ts | Added getPageChildrenMiddleware and route variants |
| components/confluence-sync/mocks/collections.ts | Included confluence-get-page-children in mock collections |
| components/confluence-sync/CHANGELOG.md | Documented pagination fix in 2.0.2 |
| components/confluence-sync/test/unit/specs/confluence/CustomConfluenceClient.test.ts | Extended tests for pagination and errors |
| components/confluence-sync/test/component/specs/Sync.spec.ts | Updated component tests to assert child‐page fetch order |
| .github/ISSUE_TEMPLATE/BUG.yml | Updated default component/version options |
| .github/workflows/publish.yml | Removed trailing whitespace |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (3)
components/confluence-sync/src/confluence/CustomConfluenceClient.ts:61
- Confluence’s REST API for fetching page children specifically uses
/rest/api/content/{id}/child/page. Consider updating the endpoint path to/child/pageto avoid ambiguity with other child types.
`${this._config.url}/rest/api/content/${parentId}/child`,
components/markdown-confluence-sync/mocks/routes/Confluence.ts:70
- [nitpick] The log message says "to Confluence", but this middleware is receiving from Confluence. For clarity, change to "Requested page children with id ${req.params.pageId} from Confluence".
`Requested page children with id ${req.params.pageId} to Confluence`,
components/markdown-confluence-sync/CHANGELOG.md:18
- [nitpick] The bullet starts with "fix: Fix"—consider removing the redundant prefix and capitalizing consistently (e.g., "* Fix issue when a page has more than 25 children...").
* fix: Fix issue when a page has more than 25 children. There was an error when trying to update a children page that had 25 brothers. The API was not returning it as a child of the parent page, so the library was trying to create it as a new page instead of updating it. Now, we use another API call to get all children pages.
components/confluence-sync/test/unit/specs/confluence/CustomConfluenceClient.test.ts
Show resolved
Hide resolved
|
Check License Compliance ✅ There are 1437 dependencies with allowed licenses.
✅ Result: Valid licenses |
Request child pages using pagination
Description
closes #60
Agreement
Please check the following boxes after you have read and understood each item.
In case this is your first contribution to this project, you will also have to add a comment with the following text: "I have read the CLA Document and I hereby sign the CLA", otherwise the PR status will fail and our bot will request you to add it. Once you have signed it in a PR, you will not have to sign it again for future contributions.