Notion improvements - use notion-helper #18797
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
💡 Enable Vercel Agent with $100 free credit for automated AI reviews |
WalkthroughThis PR integrates the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AppendBlock as append-block action
participant Helper as notion-helper
participant NotionAPI as Notion API
User->>AppendBlock: append blocks to page
AppendBlock->>AppendBlock: prepare block data
AppendBlock->>Helper: appendBlocks(client, block_id, children)
Note over Helper: Internal batch splitting logic
Helper->>Helper: split blocks respecting API limits<br/>(nesting depth, array size, total blocks, character limits)
loop For each batch request
Helper->>NotionAPI: append_block_children API call
NotionAPI-->>Helper: response
end
Helper-->>AppendBlock: return apiResponses array
AppendBlock-->>User: success summary + responses
sequenceDiagram
participant User
participant CreatePage as create-page action
participant PageBuilder as base-page-builder
participant Helper as notion-helper
participant NotionAPI as Notion API
User->>CreatePage: create page (pageContent, metadata)
CreatePage->>PageBuilder: buildPageFromDataSource({pageContent, parentPageId, properties, icon, cover})
PageBuilder->>PageBuilder: convert Markdown to blocks
PageBuilder->>Helper: createNotionBuilder()
Helper-->>PageBuilder: page builder instance
PageBuilder->>PageBuilder: apply properties to builder
PageBuilder->>PageBuilder: set icon & cover
PageBuilder->>Helper: createPage(client, pageBuilder)
Note over Helper: Internal request splitting<br/>if needed
Helper->>NotionAPI: create_page API call(s)
NotionAPI-->>Helper: response(s)
Helper-->>PageBuilder: API response
PageBuilder-->>CreatePage: response
CreatePage-->>User: created page
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The changes mix homogeneous version bumps (low effort) with heterogeneous functional refactoring (higher effort). The core integration logic in Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/notion/actions/complete-file-upload/complete-file-upload.mjs (1)
2-5: Based on my verification:
base-page-builder content: I confirmed it exports only page/property/block building methods (buildPageFromDataSource, buildPageProperties, formatChildBlocks, etc.)—none related to file uploads.
complete-file-upload purpose: This API finalizes a mode=multi_part file upload after all parts have been sent successfully—a simple file upload completion operation.
Mismatch confirmed: The action spreads properties from a page-builder base into a file-upload-only action, creating semantic misalignment and unused inheritance.
Use the appropriate base for this action—likely a generic app-only base or no base at all if the action only requires the Notion app connection.
Verify that the
complete-file-upload.mjsaction:
- Only accepts a
file_upload_idprop- Calls the Notion API's
completeFileUpload()endpoint- Does not need or use any page-building, property-handling, or block-formatting methods from
base-page-builderIf confirmed, remove the
base-page-builderimport and spread, then add only thenotionapp import to the action's props.
🧹 Nitpick comments (3)
components/notion/actions/common/base-page-builder.mjs (3)
305-307: Consider validating mutually exclusive parent parameters.The method accepts both
parentDataSourceIdandparentPageId. If both are provided, both will be set on the builder (lines 323-328). Verify that notion-helper handles this correctly, or add validation to ensure only one parent type is specified.async buildPageFromDataSource({ pageContent, parentDataSourceId, parentPageId, properties = [], icon, cover, }) { + if (parentDataSourceId && parentPageId) { + throw new ConfigurationError("Cannot specify both parentDataSourceId and parentPageId"); + }
330-333: Add error handling for invalid property types.The dynamic method invocation on line 332 will throw if
propertyTypeCamelCasedoesn't correspond to a valid builder method. This could happen with unsupported or malformed property types.for (const property of properties) { const propertyTypeCamelCase = property.type.replace(/_([a-z])/g, (_, c) => c.toUpperCase()); - pageBuilder = pageBuilder[propertyTypeCamelCase](property.label, property.value); + if (typeof pageBuilder[propertyTypeCamelCase] !== 'function') { + throw new ConfigurationError(`Unsupported property type: ${property.type}`); + } + try { + pageBuilder = pageBuilder[propertyTypeCamelCase](property.label, property.value); + } catch (error) { + throw new ConfigurationError(`Failed to set property "${property.label}" of type "${property.type}": ${error.message}`); + } }
309-315: Improve error message handling.The error handler assumes
error.messageexists, but ifmarkdownToBlocksthrows a non-Error object, this could fail. Use optional chaining or ensure the error is properly formatted.try { pageBlocks = markdownToBlocks(pageContent); } catch (error) { - throw new ConfigurationError(`Failed to convert Markdown content to Notion blocks: ${error.message}`); + throw new ConfigurationError(`Failed to convert Markdown content to Notion blocks: ${error?.message || error}`); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
components/notion/actions/append-block/append-block.mjs(2 hunks)components/notion/actions/common/base-page-builder.mjs(2 hunks)components/notion/actions/complete-file-upload/complete-file-upload.mjs(1 hunks)components/notion/actions/create-database/create-database.mjs(1 hunks)components/notion/actions/create-file-upload/create-file-upload.mjs(1 hunks)components/notion/actions/create-page-from-database/create-page-from-database.mjs(2 hunks)components/notion/actions/create-page/create-page.mjs(2 hunks)components/notion/actions/delete-block/delete-block.mjs(1 hunks)components/notion/actions/duplicate-page/duplicate-page.mjs(1 hunks)components/notion/actions/list-file-uploads/list-file-uploads.mjs(1 hunks)components/notion/actions/retrieve-file-upload/retrieve-file-upload.mjs(1 hunks)components/notion/actions/send-file-upload/send-file-upload.mjs(1 hunks)components/notion/actions/update-block/update-block.mjs(1 hunks)components/notion/actions/update-database/update-database.mjs(1 hunks)components/notion/actions/update-page/update-page.mjs(1 hunks)components/notion/package.json(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
components/notion/actions/common/base-page-builder.mjs (2)
components/notion/actions/create-page-from-database/create-page-from-database.mjs (3)
properties(95-95)properties(113-113)response(114-120)components/notion/actions/create-page/create-page.mjs (1)
response(52-64)
components/notion/actions/create-page/create-page.mjs (2)
components/notion/actions/common/base-page-builder.mjs (1)
response(353-356)components/notion/actions/create-page-from-database/create-page-from-database.mjs (1)
response(114-120)
components/notion/actions/create-page-from-database/create-page-from-database.mjs (2)
components/notion/actions/update-page/update-page.mjs (2)
properties(94-94)response(133-133)components/notion/actions/create-page/create-page.mjs (1)
response(52-64)
components/notion/actions/append-block/append-block.mjs (1)
components/notion/actions/common/base-page-builder.mjs (1)
response(353-356)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (16)
components/notion/actions/list-file-uploads/list-file-uploads.mjs (1)
1-32: Version bump is appropriate; no functional changes needed.This file contains only a cosmetic version bump (0.0.6 → 0.0.7) and no functional changes. Since this is a read-only action (
readOnlyHint: true), it does not integratenotion-helper's batch-splitting capabilities—which are intended for write operations likecreate-pageandappend-block. The change aligns with the overall PR scope of adding the dependency and bumping versions across Notion actions.components/notion/actions/update-database/update-database.mjs (1)
10-10: Clarify the scope of this version bump.This action updates a data source but is not listed in the PR objectives as a core action being refactored (only
append-block,create-page, andcreate-page-from-databaseare mentioned). The version bump alone—without functional changes—is unclear in this context.Should this file be modified in this PR? If so:
- Does it need to integrate
notion-helperfor handling API limits?- If not, this version bump should be reverted or applied in a separate maintenance PR.
If this is a broad version bump across all Notion actions for consistency, that should be stated explicitly in the PR description.
components/notion/actions/update-block/update-block.mjs (1)
10-10: Version bump is appropriate as part of this PR's coordinated updates.The version increment from 0.0.6 to 0.0.7 aligns with the PR's dependency updates and refactors to related Notion actions. Since
update-blockonly updates individual blocks (not creating pages or appending blocks), it does not require notion-helper integration and the current implementation remains unchanged and correct.components/notion/actions/retrieve-file-upload/retrieve-file-upload.mjs (1)
2-2: Verify the base module and confirm no functional changes.This file imports and spreads a new
basemodule frombase-page-builder.mjs, which is not provided for review. The version is appropriately bumped to 0.0.7, but I cannot fully verify the impact of the base module integration.The action's core logic (lines 26–31) remains unchanged, which is good; however, spreading the base module could introduce unexpected properties or methods that might conflict or alter behavior.
Please confirm:
- The
base-page-builder.mjsmodule does not override or conflict with any properties defined in this action (e.g.,key,name,props,run).- The action's behavior is unchanged and passes all tests (especially the file-upload retrieval flow).
Also applies to: 5-5, 9-9
components/notion/actions/delete-block/delete-block.mjs (1)
9-9: LGTM: Version bump is appropriate.The version increment aligns with the PR's broader update pattern. No notion-helper integration is required for this action since delete operations don't encounter the API request limits (deeply nested blocks, 100-block arrays, 1,000-block-per-request limits) that the PR addresses for create-page and append-blocks actions.
components/notion/actions/duplicate-page/duplicate-page.mjs (1)
10-10: Version bump approved as part of PR-wide synchronization.The version is bumped from 0.0.21 to 0.0.22 as part of the PR-wide metadata update across Notion actions. This aligns with the integration of notion-helper into the Notion module infrastructure.
Since this action passes potentially large
formattedChildrenarrays tothis.notion.createPage(page), please verify that the Notion app module'screatePagemethod has been updated to use notion-helper's request splitting logic for handling nested blocks, character limits, and array size constraints.components/notion/actions/complete-file-upload/complete-file-upload.mjs (1)
9-9: LGTM! Appropriate version bump.The version increment aligns with the broader PR updates to Notion components and dependencies.
components/notion/actions/send-file-upload/send-file-upload.mjs (2)
11-11: Version bump is appropriate.This patch-level version increment aligns with the release cycle for integrating notion-helper into Notion actions. This action correctly does not require notion-helper integration since it only handles file uploads, not page or block creation.
1-67: The review comment is based on incorrect assumptions and should be disregarded.The latest published version of notion-helper is 1.3.25 (Sep 24, 2025), so the concern about 1.3.27 is technically valid. However, this codebase contains no references to notion-helper anywhere. The file under review (
components/notion/actions/send-file-upload/send-file-upload.mjs) imports from@pipedream/platformand a local Notion app configuration—not from notion-helper. The review comment's concern about a notion-helper version mismatch does not apply to this PR.Likely an incorrect or invalid review comment.
components/notion/actions/create-page-from-database/create-page-from-database.mjs (2)
97-108: Clarify property identification: name vs. ID.The code uses
property.nameas the label (line 101) when building the properties array, but checks forproperties[property.id]on line 99. This mixing of property name and ID could cause issues if Notion's API expects property IDs in certain contexts.According to the related code in
update-page.mjs(line 93), property labels are typically set toproperty.id. Consider usingproperty.idconsistently as the label unless property names are explicitly required bybuildPageFromDataSource.Based on learnings
111-123: Clean refactor to centralized page building.The simplified flow delegates complex block batching and page construction to
buildPageFromDataSource, which aligns with the PR objective to leverage notion-helper for API limit handling.components/notion/actions/create-page/create-page.mjs (1)
51-67: Excellent simplification.Removing the complex page-building logic and delegating to
buildPageFromDataSourcemakes the code much more maintainable and aligns with the PR objective to use notion-helper for handling API limits.components/notion/package.json (1)
17-17: No action required—notion-helper version 1.3.27 exists and is valid.Verification confirms that version 1.3.27 is published on npm and is the latest available release. The dependency specification
^1.3.27is correct and will not fail during installation.components/notion/actions/append-block/append-block.mjs (2)
127-127: No issues found — method verified.The
_getNotionClient()method exists atcomponents/notion/notion.app.mjs:219and correctly returnsnew notion.Client({...})with the proper authentication and Notion version configuration, confirming compatibility with@notionhq/clientexpectations.
126-133: Verify the return value structure for backward compatibility.The return value changed to
response.apiResponses. Without access to previous versions or tests showing the original return structure, carefully review any existing workflows consuming this action to ensure they expect the current output format. Consider checking git history to confirm what the previous return value structure was.components/notion/actions/common/base-page-builder.mjs (1)
352-358: Verify the page builder output structure and createPage API contract.The code assumes
page.contentandresponse.apiResponseproperties exist based on builder and helper function contracts. Without definitive verification of these internal API guarantees, silent failures remain possible if the builder returns an unexpected structure or the helper function changes its response format.
1 participant
Resolves #18764
Summary by CodeRabbit
New Features
Chores