-
Notifications
You must be signed in to change notification settings - Fork 5.5k
17823 notion #18005
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
17823 notion #18005
Conversation
- Added actions for creating, completing, sending, and retrieving file uploads. - Introduced actions for creating and updating databases. - Enhanced user management with actions to list all users and retrieve user details. - Updated existing actions to improve functionality and consistency. - Bumped version numbers for various actions to reflect changes. - Updated dependencies in package.json to the latest versions.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
WalkthroughAdds many new Notion actions (database CRUD, file upload lifecycle, block update/delete, user list/retrieve), extends the Notion app client with pagination and multiple API wrappers, changes utils.parseObject to return null for falsy inputs, bumps package/dependencies, and applies numerous version increments and import reorderings. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Action_Create as Create File Upload Action
participant Action_Send as Send File Upload Action
participant Action_Complete as Complete File Upload Action
participant App as Notion App Client
participant Notion as Notion API
User->>Action_Create: Provide mode/filename/parts/URL
Action_Create->>App: createFileUpload(payload)
App->>Notion: POST /v1/file_uploads
Notion-->>App: 201 {id,...}
App-->>Action_Create: {id,...}
Action_Create-->>User: Summary with file upload ID
User->>Action_Send: Provide fileUploadId + file
Action_Send->>App: sendFileUpload({file_upload_id,file})
App->>Notion: POST /v1/file_uploads/{id}/send
Notion-->>App: 200 {id,...}
App-->>Action_Send: {id,...}
Action_Send-->>User: Summary sent
User->>Action_Complete: Provide fileUploadId
Action_Complete->>App: completeFileUpload({file_upload_id})
App->>Notion: POST /v1/file_uploads/{id}/complete
Notion-->>App: 200 {id,status}
App-->>Action_Complete: {id,status}
Action_Complete-->>User: Summary completed
sequenceDiagram
participant User
participant Action_UpdateDB as Update Database Action
participant App as Notion App Client
participant Notion as Notion API
User->>Action_UpdateDB: Select databaseId
Action_UpdateDB->>App: retrieveDatabase(databaseId)
App->>Notion: GET /v1/databases/{id}
Notion-->>App: 200 {title,description,properties}
App-->>Action_UpdateDB: Database payload
Action_UpdateDB-->>User: Prefilled defaults
User->>Action_UpdateDB: Submit edits
Action_UpdateDB->>App: updateDatabase({database_id,title,description,properties})
App->>Notion: PATCH /v1/databases/{id}
Notion-->>App: 200 {id,...}
App-->>Action_UpdateDB: Response
Action_UpdateDB-->>User: Summary updated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Out-of-scope changes
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (4)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 7
🔭 Outside diff range comments (4)
components/notion/sources/updated-page/updated-page.mjs (1)
80-84: Guard against missing/corrupt state when inflating propertyValues.If db has no “propertyValues” (first run, reset, or corruption), Buffer.from(undefined) will throw. Add a defensive fallback.
_getPropertyValues() { - const compressed = this.db.get("propertyValues"); - const buffer = Buffer.from(compressed, "base64"); - const decompressed = zlib.inflateSync(buffer).toString(); - return JSON.parse(decompressed); + const compressed = this.db.get("propertyValues"); + if (!compressed) { + return {}; + } + try { + const buffer = Buffer.from(compressed, "base64"); + const decompressed = zlib.inflateSync(buffer).toString(); + return JSON.parse(decompressed); + } catch (err) { + console.warn("Failed to decode stored propertyValues; resetting cache.", err?.message); + return {}; + } }components/notion/actions/append-block/append-block.mjs (2)
66-76: Guard against undefined blockTypes in additionalProps to prevent TypeError
blockTypes.includeswill throw ifblockTypesis undefined during prop evaluation. Default to an empty array.Apply:
additionalProps(currentProps) { - const { blockTypes } = this; + const { blockTypes } = this; + const types = Array.isArray(blockTypes) ? blockTypes : []; for (let prop of [ "blockIds", "markdownContents", "imageUrls", ]) { - currentProps[prop].hidden = !blockTypes.includes(prop); + currentProps[prop].hidden = !types.includes(prop); } return {}; },
90-114: Same undefined-guard needed in run() for blockTypes checksThree conditionals call
blockTypes.includes. Default to an empty array once to avoid exceptions.async run({ $ }) { - const { blockTypes } = this; + const { blockTypes } = this; + const types = Array.isArray(blockTypes) ? blockTypes : []; const children = []; // add blocks from blockIds - if (blockTypes.includes("blockIds") && this.blockIds?.length > 0) { + if (types.includes("blockIds") && this.blockIds?.length > 0) { ... - if (blockTypes.includes("markdownContents") && this.markdownContents?.length > 0) { + if (types.includes("markdownContents") && this.markdownContents?.length > 0) { ... - if (blockTypes.includes("imageUrls") && this.imageUrls?.length) { + if (types.includes("imageUrls") && this.imageUrls?.length) {Also applies to: 112-124
components/notion/actions/create-page-from-database/create-page-from-database.mjs (1)
68-71: Ensure parseObject return value is safely defaulted
TheparseObjecthelper now returnsnullfor falsy inputs, which can lead to runtime errors when downstream code assumes an object or array. Please update call sites in the Notion integration to guard againstnull:• components/notion/actions/create-page-from-database/create-page-from-database.mjs
- this.properties = utils.parseObject(this.properties); + // parseObject returns null for falsy inputs; default to empty object + this.properties = utils.parseObject(this.properties) || {};• components/notion/actions/query-database/query-database.mjs
- sorts: utils.parseObject(sorts), + // default to empty array when parseObject returns null + sorts: utils.parseObject(sorts) || [],• components/notion/actions/update-database/update-database.mjs
- properties: utils.parseObject(this.properties), + properties: utils.parseObject(this.properties) || {},• components/notion/actions/create-database/create-database.mjs
- properties: utils.parseObject(this.properties), + properties: utils.parseObject(this.properties) || {},Also audit any other
utils.parseObject(…)calls in the Notion code (and beyond) to ensure they handle anullreturn value appropriately.
🧹 Nitpick comments (7)
components/notion/actions/retrieve-page/retrieve-page.mjs (1)
25-29: Use generic title lookup instead of hardcoding "Name"Notion page title property can be named arbitrarily. Prefer detecting the first
title-type property.- const title = response?.properties?.Name?.title[0]?.plain_text; + const titleProp = Object.values(response?.properties || {}).find((p) => p?.type === "title"); + const title = titleProp?.title?.[0]?.plain_text;components/notion/actions/retrieve-user/retrieve-user.mjs (1)
1-23: LGTM: new Retrieve User actionReads cleanly and aligns with the new app method
getUser. Consider adding a minimal try/catch for parity with other actions, but not required.components/notion/actions/send-file-upload/send-file-upload.mjs (2)
24-28: Clarify file input labeling, add safe content-type fallback, and fix summary text
- The label/description imply images only; this action sends any file.
- Add a fallback for
metadata.contentType.- Summary currently says “created” but this action “sends”.
- file: { - type: "string", - label: "Image File Path or URL", - description: "The image to process. Provide either a file URL or a path to a file in the `/tmp` directory (for example, `/tmp/myImage.jpg`).", - }, + file: { + type: "string", + label: "File Path or URL", + description: "The file to upload. Provide either a file URL or a path to a file in the `/tmp` directory (for example, `/tmp/myFile.pdf`).", + },- ], { - type: metadata.contentType, - }), + ], { + type: metadata.contentType || "application/octet-stream", + }),- $.export("$summary", `Successfully created file upload with ID ${response.id}`); + $.export("$summary", `Successfully sent file upload with ID ${response.id}`);Also applies to: 53-53, 59-59
40-45: Large file memory usage; consider streaming if API supports itReading the entire file into memory before creating a Blob can be costly for large uploads. If Notion’s
sendFileUploadsupports streams or chunked sending, prefer streaming to reduce memory footprint.components/notion/actions/create-file-upload/create-file-upload.mjs (1)
58-65: Add minimal mode-based validation and explicit defaultHelp users avoid 400s by validating mode-specific requirements and explicitly defaulting to
single_part.async run({ $ }) { - const response = await this.notion.createFileUpload({ - mode: this.mode, + const mode = this.mode || "single_part"; + + if (mode === "external_url" && !this.external_url) { + throw new Error("external_url is required when mode is 'external_url'."); + } + if (mode === "multi_part") { + if (!this.number_of_parts || this.number_of_parts < 1 || this.number_of_parts > 1000) { + throw new Error("number_of_parts must be between 1 and 1000 when mode is 'multi_part'."); + } + } + + const response = await this.notion.createFileUpload({ + mode, filename: this.filename, content_type: this.content_type, number_of_parts: this.number_of_parts, external_url: this.external_url, });components/notion/actions/update-database/update-database.mjs (2)
44-45: Add defensive checks for database fieldsWhile
titleanddescriptionare typically present in Notion database responses, consider adding defensive checks to prevent potential runtime errors if the API response structure changes or fields are missing.- props.title.default = database.title.map((text) => text.text.content).join(" "); - props.description.default = database.description.map((text) => text.plain_text).join(" "); + props.title.default = database.title?.map((text) => text.text?.content || "").join(" ") || ""; + props.description.default = database.description?.map((text) => text.plain_text || "").join(" ") || "";
24-24: Clarify prop descriptions to match actual input typeThe descriptions mention "An array of rich text objects" but the actual prop type is
string. This could confuse users about what format to provide.- description: "Title of database as it appears in Notion. An array of [rich text objects](https://developers.notion.com/reference/rich-text).", + description: "Title of database as it appears in Notion. Provide a plain text string that will be converted to the required format.",- description: "An array of [rich text objects](https://developers.notion.com/reference/rich-text) that represents the description of the database that is displayed in the Notion UI. If omitted, then the database description remains unchanged.", + description: "Description of the database that is displayed in the Notion UI. Provide a plain text string that will be converted to the required format. If omitted, the database description remains unchanged.",Also applies to: 30-30
📜 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 (34)
components/notion/actions/append-block/append-block.mjs(1 hunks)components/notion/actions/complete-file-upload/complete-file-upload.mjs(1 hunks)components/notion/actions/create-comment/create-comment.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(1 hunks)components/notion/actions/create-page/create-page.mjs(1 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-all-users/list-all-users.mjs(1 hunks)components/notion/actions/list-file-uploads/list-file-uploads.mjs(1 hunks)components/notion/actions/query-database/query-database.mjs(2 hunks)components/notion/actions/retrieve-block/retrieve-block.mjs(1 hunks)components/notion/actions/retrieve-database-content/retrieve-database-content.mjs(1 hunks)components/notion/actions/retrieve-database-schema/retrieve-database-schema.mjs(1 hunks)components/notion/actions/retrieve-file-upload/retrieve-file-upload.mjs(1 hunks)components/notion/actions/retrieve-page-property-item/retrieve-page-property-item.mjs(1 hunks)components/notion/actions/retrieve-page/retrieve-page.mjs(1 hunks)components/notion/actions/retrieve-user/retrieve-user.mjs(1 hunks)components/notion/actions/search/search.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/common/utils.mjs(1 hunks)components/notion/notion.app.mjs(5 hunks)components/notion/package.json(2 hunks)components/notion/sources/new-comment-created/new-comment-created.mjs(1 hunks)components/notion/sources/new-database/new-database.mjs(1 hunks)components/notion/sources/new-page/new-page.mjs(1 hunks)components/notion/sources/page-or-subpage-updated/page-or-subpage-updated.mjs(1 hunks)components/notion/sources/updated-page-by-timestamp/updated-page-by-timestamp.mjs(1 hunks)components/notion/sources/updated-page-id/updated-page-id.mjs(1 hunks)components/notion/sources/updated-page/updated-page.mjs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (10)
components/notion/actions/retrieve-file-upload/retrieve-file-upload.mjs (17)
components/notion/notion.app.mjs (1)
notion(421-421)components/notion/actions/complete-file-upload/complete-file-upload.mjs (1)
response(24-26)components/notion/actions/create-database/create-database.mjs (1)
response(35-49)components/notion/actions/create-comment/create-comment.mjs (1)
response(41-53)components/notion/actions/create-file-upload/create-file-upload.mjs (1)
response(59-65)components/notion/actions/create-page-from-database/create-page-from-database.mjs (1)
response(85-88)components/notion/actions/create-page/create-page.mjs (1)
response(99-99)components/notion/actions/delete-block/delete-block.mjs (1)
response(25-25)components/notion/actions/list-all-users/list-all-users.mjs (1)
response(13-15)components/notion/actions/list-file-uploads/list-file-uploads.mjs (1)
response(15-17)components/notion/actions/retrieve-database-schema/retrieve-database-schema.mjs (1)
response(19-19)components/notion/actions/query-database/query-database.mjs (1)
response(34-37)components/notion/actions/retrieve-page-property-item/retrieve-page-property-item.mjs (1)
response(28-31)components/notion/actions/retrieve-page/retrieve-page.mjs (1)
response(24-24)components/notion/actions/retrieve-user/retrieve-user.mjs (1)
response(19-19)components/notion/actions/update-block/update-block.mjs (1)
response(30-33)components/notion/actions/send-file-upload/send-file-upload.mjs (1)
response(47-57)
components/notion/actions/retrieve-user/retrieve-user.mjs (5)
components/notion/notion.app.mjs (1)
notion(421-421)components/notion/actions/create-page/create-page.mjs (1)
response(99-99)components/notion/actions/list-all-users/list-all-users.mjs (1)
response(13-15)components/notion/actions/retrieve-database-schema/retrieve-database-schema.mjs (1)
response(19-19)components/notion/actions/retrieve-page/retrieve-page.mjs (1)
response(24-24)
components/notion/actions/update-block/update-block.mjs (5)
components/notion/actions/create-page-from-database/create-page-from-database.mjs (1)
response(85-88)components/notion/actions/create-page/create-page.mjs (1)
response(99-99)components/notion/actions/delete-block/delete-block.mjs (1)
response(25-25)components/notion/actions/query-database/query-database.mjs (1)
response(34-37)components/notion/actions/retrieve-page/retrieve-page.mjs (1)
response(24-24)
components/notion/actions/send-file-upload/send-file-upload.mjs (5)
components/notion/notion.app.mjs (1)
notion(421-421)components/notion/actions/complete-file-upload/complete-file-upload.mjs (1)
response(24-26)components/notion/actions/create-file-upload/create-file-upload.mjs (1)
response(59-65)components/notion/actions/list-file-uploads/list-file-uploads.mjs (1)
response(15-17)components/notion/actions/retrieve-file-upload/retrieve-file-upload.mjs (1)
response(21-23)
components/notion/actions/list-all-users/list-all-users.mjs (2)
components/notion/actions/list-file-uploads/list-file-uploads.mjs (1)
response(15-17)components/notion/notion.app.mjs (1)
users(135-135)
components/notion/actions/delete-block/delete-block.mjs (2)
components/notion/actions/retrieve-page/retrieve-page.mjs (1)
response(24-24)components/notion/actions/update-block/update-block.mjs (1)
response(30-33)
components/notion/actions/create-file-upload/create-file-upload.mjs (4)
components/notion/actions/complete-file-upload/complete-file-upload.mjs (1)
response(24-26)components/notion/actions/list-file-uploads/list-file-uploads.mjs (1)
response(15-17)components/notion/actions/retrieve-file-upload/retrieve-file-upload.mjs (1)
response(21-23)components/notion/actions/send-file-upload/send-file-upload.mjs (1)
response(47-57)
components/notion/actions/query-database/query-database.mjs (1)
components/notion/actions/create-database/create-database.mjs (1)
response(35-49)
components/notion/actions/update-database/update-database.mjs (8)
components/notion/notion.app.mjs (1)
notion(421-421)components/notion/sources/new-comment-created/new-comment-created.mjs (1)
text(30-30)components/notion/actions/create-database/create-database.mjs (1)
response(35-49)components/notion/actions/create-page-from-database/create-page-from-database.mjs (1)
response(85-88)components/notion/actions/create-page/create-page.mjs (1)
response(99-99)components/notion/actions/retrieve-database-schema/retrieve-database-schema.mjs (1)
response(19-19)components/notion/actions/query-database/query-database.mjs (1)
response(34-37)components/notion/actions/update-block/update-block.mjs (1)
response(30-33)
components/notion/actions/create-database/create-database.mjs (5)
components/notion/notion.app.mjs (1)
notion(421-421)components/notion/actions/create-page-from-database/create-page-from-database.mjs (1)
response(85-88)components/notion/actions/create-page/create-page.mjs (1)
response(99-99)components/notion/actions/retrieve-database-schema/retrieve-database-schema.mjs (1)
response(19-19)components/notion/actions/query-database/query-database.mjs (1)
response(34-37)
⏰ 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). (4)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
🔇 Additional comments (27)
components/notion/sources/new-comment-created/new-comment-created.mjs (1)
8-8: Version bump only — no logic changes detected. LGTM.components/notion/package.json (1)
3-15: SDK upgrade: review breaking changes in @notionhq/client v4Please verify the following before bumping to
^4.0.1:
- Legacy page‐properties shorthand has been removed (e.g.
"numberId": 24). All requests must now use the polymorphic form:(See Notion SDK v4 PR #580.)"numberId": { type: "number", number: 24 }- TypeScript surface changes: duplicate/no-op union entries and certain endpoint/type exports were refactored—audit any inline unions or imported type names for compatibility. (See PR #579.)
- Node.js engine requirement is now ≥18. Confirm that the Pipedream runtime in use supports Node.js 18+.
components/notion/sources/updated-page-by-timestamp/updated-page-by-timestamp.mjs (1)
11-11: Version bump only — logic remains unchanged. LGTM.components/notion/sources/updated-page-id/updated-page-id.mjs (1)
10-10: Version bump only — logic remains unchanged. LGTM.components/notion/sources/new-page/new-page.mjs (1)
4-4: Import reorder + version bump — behavior unchanged. LGTM.Also applies to: 11-11
components/notion/sources/updated-page/updated-page.mjs (1)
1-1: Import tidy-up + version bump — no functional diffs observed. LGTM.Also applies to: 5-5, 12-12
components/notion/sources/new-database/new-database.mjs (1)
3-3: Import reorder + version bump — behavior unchanged. LGTM.Also applies to: 10-10
components/notion/actions/append-block/append-block.mjs (1)
10-10: Version bump only — no functional changesLooks good.
components/notion/actions/create-page/create-page.mjs (1)
10-10: Version bump only — no functional changesAll good.
components/notion/actions/retrieve-page-property-item/retrieve-page-property-item.mjs (1)
7-7: Version bump only — no functional changesLGTM.
components/notion/actions/retrieve-block/retrieve-block.mjs (1)
7-7: Version bump only — no functional changesLooks good.
components/notion/actions/retrieve-page/retrieve-page.mjs (1)
7-7: Version bump only — no functional changesApproved.
components/notion/actions/search/search.mjs (1)
8-8: Version bump only — no functional changesLGTM.
components/notion/actions/retrieve-database-schema/retrieve-database-schema.mjs (1)
7-7: Version bump only — no functional changesApproved.
components/notion/actions/create-page-from-database/create-page-from-database.mjs (1)
3-4: Import reorder + version bumpNo functional changes here.
Also applies to: 11-11
components/notion/sources/page-or-subpage-updated/page-or-subpage-updated.mjs (1)
3-3: LGTM on import reorder and version bumpNo functional changes; version bump is appropriate.
Also applies to: 10-10
components/notion/actions/create-comment/create-comment.mjs (1)
2-2: LGTM on import order tweak and version bumpNo behavioral changes introduced.
Also applies to: 8-8
components/notion/actions/duplicate-page/duplicate-page.mjs (1)
1-1: LGTM on centralizing utils import and version bumpAligns with repo patterns; no runtime impact observed.
Also applies to: 10-10
components/notion/actions/update-page/update-page.mjs (1)
1-1: LGTM on import reordering forpickand version bumpNo logic changes; action behavior remains the same.
Also applies to: 10-10
components/notion/actions/retrieve-database-content/retrieve-database-content.mjs (1)
7-7: LGTM on version bumpNo code changes affecting behavior.
components/notion/actions/list-all-users/list-all-users.mjs (1)
13-15: Verifypaginate’s invocation context forgetUsersBefore applying a manual
.bind, confirm how your Notion client’spaginatemethod invokes the passed function:• Inspect the implementation of
paginatein your Notion client wrapper to see whether it preservesthiswhen callingfn.
• If it does (e.g. usingfn.call(this, …)or ifgetUsersis defined as an arrow function), no manual bind is needed.
• Otherwise, wrap the call in an arrow or use.bind(this.notion)only if you encounter a lost-context runtime error.components/notion/actions/retrieve-file-upload/retrieve-file-upload.mjs (1)
20-27: LGTMStraightforward wrapper around the app method with clear summary and correct propDefinition usage.
components/notion/actions/update-block/update-block.mjs (1)
23-27: Verify JSON parsing forcontentin update-block actionEnsure that the
contentinput (declared as type"string") incomponents/notion/actions/update-block/update-block.mjsis parsed into a JSON object before calling the Notion API:
In the
async run({ $ })method, replace direct use ofthis.contentwith:+ let data = this.content; + if (typeof data === 'string') { + try { + data = JSON.parse(data); + } catch { + throw new Error( + "Invalid JSON in Content. Provide a valid block object per Notion docs." + ); + } + } const response = await this.notion.updateBlock({ block_id: this.blockId, - data: this.content, + data, });Apply the same guard for lines 29–36 if they perform a similar update.
Confirm that the file path and existing implementation match and that clients expect to pass a JSON‐stringified block object.
Please verify these changes against your codebase.
components/notion/actions/delete-block/delete-block.mjs (1)
24-28: LGTMSimple and correct wrapper with clear summary and inputs.
components/notion/actions/complete-file-upload/complete-file-upload.mjs (1)
23-30: LGTMAccurately completes a multipart upload with appropriate propDefinition and summary.
components/notion/notion.app.mjs (2)
435-465: LGTM! Well-implemented pagination helperThe paginate generator method is well-structured and follows best practices for async generators. It properly handles cursor-based pagination and respects the maxResults limit.
412-413: No changes needed for Notion’s getUsers wrapperThe
getUsersmethod incomponents/notion/notion.app.mjsnow returns the full Notion SDK response, and its sole consumer in this file correctly destructures theresultsarray:
- In
async options()(around line 386), we have:which continues to work as expected.const { results: users } = await this.getUsers();No updates required.
michelle0927
left a comment
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.
Looks good! Just a couple questions/comments.
components/notion/actions/create-file-upload/create-file-upload.mjs
Outdated
Show resolved
Hide resolved
…d.mjs Co-authored-by: michelle0927 <[email protected]>
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.
Actionable comments posted: 2
🔭 Outside diff range comments (4)
components/notion/notion.app.mjs (4)
389-397: Infinite loop risk: using next_cursor as input without setting start_cursorYou store next_cursor into params.next_cursor and loop while that is truthy, but blocks.children.list expects start_cursor. This will keep requesting the first page.
Apply this diff to pass the correct cursor field and loop on it:
do { const { results, next_cursor: nextCursor, } = await this.listBlockChildren(block.id, params); children.push(...(results.filter((child) => !subpagesOnly || child.type === "child_page"))); - params.next_cursor = nextCursor; - } while (params.next_cursor); + params.start_cursor = nextCursor; + } while (params.start_cursor);
214-218: Guard against undefined in _checkOptionsContextvalue.match(...) will throw if value is undefined (e.g., when the dependent field isn’t set yet in options context).
Apply this diff to make it safe:
_checkOptionsContext(value, name) { - if (value.match(/{{\s?steps/)) { + if (typeof value === "string" && value.match(/{{\s?steps/)) { throw new ConfigurationError(`Please use a custom expression to reference a previous value, since you are also using one for \`${name}\``); } },
49-56: RefactorqueryDatabasecall to use supported parameters
components/notion/notion.app.mjs:49-56Notion’s
databases.queryAPI does not support aqueryfield—it only acceptsfilter,sorts,page_size, andstart_cursor. Passingquerywill be ignored or cause errors, so the current “useQuery” logic won’t work.Suggested server-side filter by title:
- const response = await this.queryDatabase(databaseId, { - query, - start_cursor: prevContext.nextPageParameters ?? undefined, - }); + const response = await this.queryDatabase(databaseId, { + filter: { + property: "Name", // adjust to your database’s title property + title: { contains: query }, + }, + start_cursor: prevContext.nextPageParameters ?? undefined, + });Alternative full-text search approach:
- Call
notion.search({ query, filter: { property: "object", value: "page" } })- Then
.filter(page => page.parent.database_id === databaseId)I can open a PR for either option—please advise which you’d prefer.
220-223: Upgrade Notion API Version for File UploadsThe current
notionVersion: "2022-02-22"predates Notion’s file upload endpoints, which were introduced on 2022-06-28 (seeUploading small files guide¹). Without bumping the version, any attempts to upload files will result in 4xx errors.• File needing update:
– components/notion/notion.app.mjs (around lines 220–223)Suggested change:
_getNotionClient() { return new notion.Client({ auth: this.$auth.oauth_access_token, - notionVersion: "2022-02-22", + notionVersion: "2022-06-28", }); },¹ https://developers.notion.com/docs/uploading-small-files?utm_source=chatgpt.com
♻️ Duplicate comments (1)
components/notion/notion.app.mjs (1)
143-170: Undefined “status” in fileUploadId options (still present)The options() signature destructures a status that is never provided, and passes it to listFileUploads. This causes undefined to be sent and can break or mask issues.
Apply this diff to remove the undefined parameter:
- async options({ - prevContext, status, - }) { + async options({ + prevContext, + }) { const { - results, next_cursor: nextCursor, + results, next_cursor: nextCursor, } = await this.listFileUploads({ - status, page_size: 100, start_cursor: prevContext.nextPageParameters ?? undefined, });
🧹 Nitpick comments (2)
components/notion/notion.app.mjs (2)
106-116: Minor: Misleading error context labelWhen validating parentId for propertyTypes, the error message references “Database ID” even when parentType may be “page”.
Apply this diff to make the message generic:
- this._checkOptionsContext(parentId, "Database ID"); + this._checkOptionsContext(parentId, "Parent ID");
289-317: Verify @notionhq/client fileUploads supportI ran a search in the codebase and saw your calls to
fileUploads.*, but there’s no vendorednode_modules/@notionhq/clientin the repo to confirm those methods actually exist in your installed SDK version. Please:• Check your package.json or lockfile to see which version of @notionhq/client you’re using.
• Verify that this version’s API surface includesfileUploads.create,.list,.send,.complete, and.retrieve.
• If those methods aren’t present, add a thin HTTP wrapper against these endpoints using the same auth token and Notion-Version header:
- POST /v1/file_uploads
- POST /v1/file_uploads/{id}/send
- POST /v1/file_uploads/{id}/complete
- GET /v1/file_uploads/{id}
- GET /v1/file_uploads
📜 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 (1)
components/notion/notion.app.mjs(5 hunks)
⏰ 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). (4)
- GitHub Check: pnpm publish
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (2)
components/notion/notion.app.mjs (2)
4-4: LGTM: centralizing meta options via NOTION_METAImport looks correct and keeps meta type options single-sourced.
412-418: LGTM: exposing users.list / users.retrieve wrappersThe method signatures align with @notionhq/client usage and enable reuse across actions.
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.
Actionable comments posted: 2
♻️ Duplicate comments (1)
components/notion/notion.app.mjs (1)
143-170: Undefined status in options context for fileUploadIdThe options({ prevContext, status }) destructures status but it’s not defined anywhere. This will always be undefined and may confuse future readers. Prior review already flagged this.
Apply this diff to remove the unused status parameter:
- async options({ - prevContext, status, - }) { + async options({ + prevContext, + }) { const { results, next_cursor: nextCursor, } = await this.listFileUploads({ - status, page_size: 100, start_cursor: prevContext.nextPageParameters ?? undefined, });Optional consistency nit: you could also use _buildPaginatedOptions similar to other props:
- return { - options: results.map(({ id: value, filename: label }) => ({ label, value })), - context: { nextPageParameters: nextCursor }, - }; + const options = results.map(({ id: value, filename: label }) => ({ label, value })); + return this._buildPaginatedOptions(options, nextCursor);
🧹 Nitpick comments (2)
components/notion/notion.app.mjs (2)
130-141: User options: add pagination and handle null namesLarge workspaces can exceed the default page size; also user.name can be null. Paginate and add a safe label fallback.
Apply this diff within these lines:
- userId: { - type: "string", - label: "User ID", - description: "Select a user, or provide a user ID", - async options() { - const { results: users } = await this.getUsers(); - - return users.map((user) => ({ - label: user.name, - value: user.id, - })); - }, - }, + userId: { + type: "string", + label: "User ID", + description: "Select a user, or provide a user ID", + async options({ prevContext }) { + const { + results: users, + next_cursor: nextCursor, + } = await this.getUsers({ + page_size: 100, + start_cursor: prevContext?.nextPageParameters ?? undefined, + }); + + const options = users.map((user) => ({ + label: user?.name || user?.type || user?.id, + value: user.id, + })); + return this._buildPaginatedOptions(options, nextCursor); + }, + },
412-418: Users wrappers: LGTM; consider documenting paging expectationsThe signatures align with the SDK. Callers should pass page_size/start_cursor as needed.
Add brief JSDoc noting accepted params (page_size, start_cursor) to guide action authors.
📜 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 (20)
components/notion/actions/append-block/append-block.mjs(1 hunks)components/notion/actions/create-comment/create-comment.mjs(1 hunks)components/notion/actions/create-page-from-database/create-page-from-database.mjs(1 hunks)components/notion/actions/create-page/create-page.mjs(1 hunks)components/notion/actions/duplicate-page/duplicate-page.mjs(1 hunks)components/notion/actions/retrieve-block/retrieve-block.mjs(1 hunks)components/notion/actions/retrieve-database-content/retrieve-database-content.mjs(1 hunks)components/notion/actions/retrieve-database-schema/retrieve-database-schema.mjs(1 hunks)components/notion/actions/retrieve-page-property-item/retrieve-page-property-item.mjs(1 hunks)components/notion/actions/retrieve-page/retrieve-page.mjs(1 hunks)components/notion/actions/search/search.mjs(1 hunks)components/notion/actions/update-page/update-page.mjs(1 hunks)components/notion/notion.app.mjs(5 hunks)components/notion/sources/new-comment-created/new-comment-created.mjs(1 hunks)components/notion/sources/new-database/new-database.mjs(1 hunks)components/notion/sources/new-page/new-page.mjs(1 hunks)components/notion/sources/page-or-subpage-updated/page-or-subpage-updated.mjs(1 hunks)components/notion/sources/updated-page-by-timestamp/updated-page-by-timestamp.mjs(1 hunks)components/notion/sources/updated-page-id/updated-page-id.mjs(1 hunks)components/notion/sources/updated-page/updated-page.mjs(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- components/notion/sources/updated-page-id/updated-page-id.mjs
- components/notion/sources/new-database/new-database.mjs
🚧 Files skipped from review as they are similar to previous changes (16)
- components/notion/sources/updated-page-by-timestamp/updated-page-by-timestamp.mjs
- components/notion/sources/new-comment-created/new-comment-created.mjs
- components/notion/actions/retrieve-page-property-item/retrieve-page-property-item.mjs
- components/notion/actions/append-block/append-block.mjs
- components/notion/actions/create-page/create-page.mjs
- components/notion/actions/retrieve-block/retrieve-block.mjs
- components/notion/actions/retrieve-database-content/retrieve-database-content.mjs
- components/notion/actions/retrieve-database-schema/retrieve-database-schema.mjs
- components/notion/actions/duplicate-page/duplicate-page.mjs
- components/notion/actions/update-page/update-page.mjs
- components/notion/actions/search/search.mjs
- components/notion/actions/retrieve-page/retrieve-page.mjs
- components/notion/sources/updated-page/updated-page.mjs
- components/notion/sources/new-page/new-page.mjs
- components/notion/actions/create-page-from-database/create-page-from-database.mjs
- components/notion/actions/create-comment/create-comment.mjs
🧰 Additional context used
🧬 Code Graph Analysis (1)
components/notion/notion.app.mjs (10)
components/notion/actions/duplicate-page/duplicate-page.mjs (2)
results(66-66)block(38-38)components/notion/actions/append-block/append-block.mjs (3)
results(131-131)block(96-96)block(106-106)components/notion/actions/update-database/update-database.mjs (1)
database(42-42)components/notion/sources/new-database/new-database.mjs (1)
params(22-22)components/notion/sources/new-page/new-page.mjs (1)
params(37-45)components/notion/sources/page-or-subpage-updated/page-or-subpage-updated.mjs (2)
params(45-45)hasMore(48-48)components/notion/sources/updated-page/updated-page.mjs (2)
params(53-53)params(142-150)components/notion/sources/updated-page-by-timestamp/updated-page-by-timestamp.mjs (1)
params(52-60)components/notion/actions/retrieve-block/retrieve-block.mjs (1)
block(49-49)components/notion/sources/new-comment-created/new-comment-created.mjs (1)
cursor(46-46)
⏰ 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 (5)
components/notion/sources/page-or-subpage-updated/page-or-subpage-updated.mjs (2)
10-10: LGTM: Patch version bump to 0.0.11.No behavioral changes introduced here; the bump aligns with the broader Notion component updates in this PR.
3-3: ✅sampleEmitimport verified
Confirmed thatcomponents/notion/sources/page-or-subpage-updated/test-event.mjsexists and provides a default export—no further changes required.components/notion/notion.app.mjs (3)
4-4: Import reordering is fineNo functional impact. Import of NOTION_META looks correct.
289-294: Database create/update wrappers: LGTMUsing databases.create and databases.update matches the Notion SDK.
310-317: Block update/delete wrappers: LGTMThese map correctly to blocks.update and blocks.delete with expected parameters.
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.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
components/notion/actions/common/base-page-builder.mjs (2)
80-88: Preserve falsy values and align filter with optional chaining (fix checkbox=false, number=0 cases).The filter still drops falsy values from
this.properties(e.g.,false,0,"") because it uses a truthy check. Also, using||forvaluewill override valid falsy user inputs with the secondary source. Withutils.parseObjectnow returningnull,this.propertiescan be nullish; the filter should mirror the optional chaining added to the value.Apply nullish checks and coalescing to avoid silently omitting valid values and to keep semantics consistent:
_filterProps(properties = {}) { return Object.keys(properties) - .filter((property) => this[property] != null - || (this.properties && this.properties[property])) + .filter((property) => this[property] != null + || this.properties?.[property] != null) .map((property) => ({ type: properties[property]?.type ?? property, label: property, - value: this[property] || this.properties?.[property], + value: this[property] ?? this.properties?.[property], })); },Impact:
- Ensures
checkbox: falseandnumber: 0are retained.- Avoids unintended fallback when
this[property]is a valid falsy value.- Fully compatible with the new null-returning parser.
170-189: Fix video type detection: dot mismatch and query/case handling cause false negatives.Currently,
supportedTypesinclude a leading dot (e.g., ".mp4") whileextensionis computed without it (e.g., "mp4"). Also, query strings/fragments and case differences are not handled. This leads to valid videos being incorrectly filtered out innotValid().Refactor to normalize both extension and supported types:
isSupportedVideoType(url) { if (!url) { return false; } - const supportedTypes = [ - ".mkv", - ".flv", - ".gifv", - ".avi", - ".mov", - ".qt", - ".wmv", - ".asf", - ".amv", - ".mp4", - ".m4v", - ".mpeg", - ".mpv", - ".mpg", - ".f4v", - ]; - const extension = url.split(".").pop(); - return supportedTypes.includes(extension); + const supportedTypes = new Set([ + "mkv", "flv", "gifv", "avi", "mov", "qt", "wmv", "asf", "amv", + "mp4", "m4v", "mpeg", "mpv", "mpg", "f4v", + ]); + // Strip query/fragment and normalize case + const clean = url.split("?")[0].split("#")[0].toLowerCase(); + const parts = clean.split("."); + const extension = parts.length > 1 ? parts.pop() : ""; + return extension && supportedTypes.has(extension); },Note: Many external video URLs (e.g., YouTube/Vimeo) lack file extensions. If you intend to allow those, consider whitelisting domains or removing this check and deferring to Notion API validation.
🧹 Nitpick comments (1)
components/notion/actions/common/base-page-builder.mjs (1)
11-17: JSDoc mismatch:blocksparameter documented but not accepted or used.The comment references selected block children and a
blocksparam, butbuildAdditionalPropsonly accepts{ properties, meta }. Please update the doc to avoid confusion./** - * Creates additional props for page properties and the selected block children + * Creates additional props for page properties * @param properties - The selected (database) properties from the page obtained from Notion * @param meta - The selected meta properties - * @param blocks - The selected block children from the workflow UI * @returns additional props */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/notion/actions/common/base-page-builder.mjs(1 hunks)components/notion/actions/update-block/update-block.mjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/notion/actions/update-block/update-block.mjs
⏰ 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). (4)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
…parsing and improve block description formatting
|
/approve |
michelle0927
left a comment
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.
This is failing one of the PR Checks. I think you need to update the pnpm-lock.yaml file.
|
/approve |
Resolves #17823
Summary by CodeRabbit
New Features
Chores