Skip to content

[Fix] Pages APIs#226

Open
GloireMutaliko21 wants to merge 2 commits intodevelopfrom
fix/pages-feature-description-fields
Open

[Fix] Pages APIs#226
GloireMutaliko21 wants to merge 2 commits intodevelopfrom
fix/pages-feature-description-fields

Conversation

@GloireMutaliko21
Copy link
Copy Markdown
Collaborator

No description provided.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 9, 2026

Summary by CodeRabbit

Release Notes

  • New Features

    • Added support for larger request payloads up to 50MB
    • Enhanced binary data handling for improved editor state synchronization
    • Unified page description updates supporting binary, HTML, and JSON formats simultaneously
  • Performance

    • Optimized API requests by consolidating page description operations into single calls

Walkthrough

The changes introduce request body parsing middleware with size limits in the application bootstrap, implement conditional binary header handling in the API client for arraybuffer responses, enhance endpoint documentation for the pages description functionality, and consolidate the description update flow from two sequential API calls into a single atomic PATCH request.

Changes

Cohort / File(s) Summary
Request Middleware Configuration
packages/plugin-plane/src/main.ts
Added Express middleware for parsing JSON and URL-encoded request bodies with 50mb size limits after cookie parsing and before global prefix configuration.
API Client Configuration
packages/plugin-plane/src/modules/api-fetch/api-fetch.service.ts
Implemented conditional header logic that switches Content-Type and Accept headers from JSON to application/octet-stream for arraybuffer response types using an isBinaryRequest constant.
Pages Description Endpoint
packages/plugin-plane/src/modules/pages/pages.controller.ts, packages/plugin-plane/src/modules/pages/pages.service.ts
Refactored description update flow to consolidate two separate PUT requests (binary and metadata) into a single atomic PATCH request to /:id/description with conditional JSON payload; updated endpoint documentation with Swagger comments describing binary state handling and payload structure; simplified GET response logic to use conditional buffer allocation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Middleware hops, headers align,
Binary paths now consolidate fine,
Two calls become one, atomic and neat,
The description dance reaches complete!
✨ Refactored with grace, the code takes flight.

🚥 Pre-merge checks | ❌ 3

❌ Failed checks (1 warning, 2 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title '[Fix] Pages APIs' is vague and doesn't clearly describe the specific changes; it lacks detail about what was fixed in the pages APIs. Make the title more specific, such as '[Fix] Update pages API description endpoints to handle binary, HTML, and JSON fields atomically' to clearly convey the main change.
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to assess whether the description relates to the changeset. Add a detailed pull request description explaining the motivation for the changes, what was fixed, and how the new implementation improves the API behavior.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/pages-feature-description-fields

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed. Inline comments: In `@packages/plugin-plane/src/main.ts`: - Line 21: main.ts only configures body-parser json with a 200mb limit while mount.ts configures both json and urlencoded with 200mb, causing inconsistent request size handling; update the Express app setup in main.ts to also register urlencoded middleware with the same limit (mirroring the configuration in mount.ts) so both standalone (main.ts) and proxy/mount modes accept the same large payloads; locate the app initialization where app.use(json({ limit: '200mb' })) is called and add a corresponding app.use(urlencoded({ limit: '200mb', extended: true })) registration to match mount.ts. In `@packages/plugin-plane/src/modules/pages/pages.service.ts`: - Around line 290-298: The null-checking for description fields is inconsistent: change the conditional that sets body['descriptionBinary'] (currently checking payload.description_binary != null) to check explicitly for !== undefined like the others so that payload.description_binary === null will be included in the body the same way description_html and description_json are; update the check in the block that sets body['descriptionBinary'] to use payload.description_binary !== undefined (keeping keys descriptionBinary/descriptionHtml/descriptionJson and the surrounding logic in the same method) to match the updatePageInputTransformer behavior. In `@packages/plugin-plane/src/mount.ts`: - Around line 234-235: The current global middleware sets body size limits to '200mb' via proxyApp.use(json(...)) and proxyApp.use(urlencoded(...)), which is risky; change this by removing the large global limit and instead apply a much smaller default (e.g., 1–5MB) for proxyApp global middleware, and move the 200MB allowance to only the specific route handlers that actually need it (attach json/urlencoded with limit: '200mb' to those route functions or routers), plus add route-level request size validation and rate-limiting middleware on the large-payload endpoints to protect against memory exhaustion (identify usages of proxyApp and the routes that accept page descriptions to scope these changes). 
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 037f1718-26d3-412e-9bd3-b7f4fb75c1e5

📥 Commits

Reviewing files that changed from the base of the PR and between 8d445af and 97ac65e.

📒 Files selected for processing (5)
  • packages/plugin-plane/src/main.ts
  • packages/plugin-plane/src/modules/api-fetch/api-fetch.service.ts
  • packages/plugin-plane/src/modules/pages/pages.controller.ts
  • packages/plugin-plane/src/modules/pages/pages.service.ts
  • packages/plugin-plane/src/mount.ts
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed. Inline comments: In `@packages/plugin-plane/src/modules/pages/pages.service.ts`: - Line 285: Add a typed payload interface and use it in updateDescription to restore compile-time checks: define an interface (e.g., UpdateDescriptionPayload with optional description_binary, description_html, description_json) and change the updateDescription signature from payload: any to payload: UpdateDescriptionPayload; if the rest of the codebase expects camelCase, add a small normalization/mapping step inside updateDescription (or in its callers) to convert descriptionBinary → description_binary (or vice versa) so property name mismatches are caught at build time. 
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2eedaba8-ce4d-4dfa-be63-9890c1b5a371

📥 Commits

Reviewing files that changed from the base of the PR and between 97ac65e and ae51411.

📒 Files selected for processing (2)
  • packages/plugin-plane/src/main.ts
  • packages/plugin-plane/src/modules/pages/pages.service.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant