-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Notion - new sources #18202
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
Notion - new sources #18202
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughAdds Notion webhook support: a shared webhook base for verification and token storage, a generic instant webhook source, a Page Properties Updated (Instant) source with optional property filtering and dynamic options, test fixtures for both sources, and bumps the Notion component version to 0.10.0. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Notion as Notion Webhooks
participant Base as Base Webhook Source
participant DB as Token Store
participant Src as Page Properties Updated Source
participant NotionAPI as Notion API
Notion->>Base: POST /webhook { body, headers }
Base->>DB: _getToken()
alt First-time verification (body.verification_token && no token)
Base->>DB: _setToken(verification_token)
Base-->>Notion: 200 OK (verification saved)
else Normal flow
Base->>Base: verifyWebhook(token, body, headers)
alt Signature invalid
Base-->>Notion: 401 Invalid webhook signature
else Signature valid
Base->>Src: processEvent(body)
alt event.type == "page.properties_updated"
Src->>NotionAPI: retrievePage(entity.id)
NotionAPI-->>Src: page data
Src-->>Base: $emit(enrichedPage, meta)
else other event
Src-->>Base: $emit(event, meta)
end
Base-->>Notion: 200 OK
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
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. ✨ 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: 6
🧹 Nitpick comments (6)
components/notion/sources/new-webhook-event-instant/new-webhook-event-instant.mjs (1)
14-20
: Use event timestamp for deterministic metadata.Prefer the event’s timestamp over Date.now() so emitted records sort consistently and match delivery time.
Apply:
_generateMeta(event) { return { id: event.id, summary: `Webhook event: ${event.type}`, - ts: Date.now(), + ts: event?.timestamp + ? new Date(event.timestamp).getTime() + : Date.now(), }; },components/notion/sources/common/base-webhook.mjs (1)
44-52
: Optional: clearer behavior before verification completes.If no token is stored yet (pre-verification), consider short-circuiting with a friendly log instead of throwing “Invalid webhook signature”.
- if (!this.verifyWebhook(token, body, headers)) { - throw new Error("Invalid webhook signature"); - } + if (!token) { + console.warn("Notion webhook not verified yet (no token stored). Ignoring event until verification completes."); + return; + } + if (!this.verifyWebhook(token, body, headers)) { + throw new Error("Invalid webhook signature"); + }components/notion/sources/page-properties-updated-instant/page-properties-updated-instant.mjs (4)
25-37
: Guard dynamic options when no database selected; sort options for UXReturn early if
databaseId
isn’t set and sort properties alphabetically.Apply this diff:
- async options() { - try { - const { properties } = await this.notion.retrieveDatabase(this.databaseId); - const propEntries = Object.entries(properties); - return propEntries.map((prop) => ({ - label: prop[1].name, - value: prop[1].id, - })); - } catch (error) { - console.log(error); - return []; - } - }, + async options() { + try { + if (!this.databaseId) return []; + const { properties } = await this.notion.retrieveDatabase(this.databaseId); + const propEntries = Object.entries(properties) + .sort((a, b) => a[1].name.localeCompare(b[1].name)); + return propEntries.map(([, prop]) => ({ + label: prop.name, + value: prop.id, + })); + } catch (error) { + console.log(error); + return []; + } + },
34-35
: Standardize logging to your component loggerReplace
console.log
with the preferred logger (e.g.,this.logger.debug(...)
) used across Notion sources for consistent observability.Also applies to: 54-55, 59-60
20-38
: UX: Clarify the “Properties” prop help textConsider clarifying that when no properties are selected, all property updates will emit events.
Apply this diff:
- description: "Only emit events when one or more of the selected properties have changed", + description: "Emit events only when one or more of the selected properties have changed. If none are selected, all property updates will emit.",
52-86
: Add tests around filter and dedupe behaviorRecommend tests for:
- Emits on any property when no filter is set.
- Emits only when selected property IDs are updated (including percent-encoded keys).
- Dedupes properly on webhook retries (same event.id).
I can add fixture-driven tests and a retry simulation—want me to open a follow-up PR?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
components/notion/package.json
(1 hunks)components/notion/sources/common/base-webhook.mjs
(1 hunks)components/notion/sources/new-webhook-event-instant/new-webhook-event-instant.mjs
(1 hunks)components/notion/sources/new-webhook-event-instant/test-event.mjs
(1 hunks)components/notion/sources/page-properties-updated-instant/page-properties-updated-instant.mjs
(1 hunks)components/notion/sources/page-properties-updated-instant/test-event.mjs
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-08T15:33:38.240Z
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#12697
File: components/salesforce_rest_api/sources/common-webhook-methods.mjs:1-71
Timestamp: 2024-10-08T15:33:38.240Z
Learning: The `common-webhook-methods.mjs` object is designed to be extended, similar to an abstract class, and intentionally does not implement certain methods like `generateWebhookMeta` and `getEventType` to enforce implementation in subclasses.
Applied to files:
components/notion/sources/new-webhook-event-instant/new-webhook-event-instant.mjs
components/notion/sources/common/base-webhook.mjs
components/notion/sources/page-properties-updated-instant/page-properties-updated-instant.mjs
🧬 Code graph analysis (2)
components/notion/sources/new-webhook-event-instant/new-webhook-event-instant.mjs (2)
components/notion/sources/common/base-webhook.mjs (1)
event
(38-40)components/notion/sources/page-properties-updated-instant/page-properties-updated-instant.mjs (1)
meta
(81-81)
components/notion/sources/page-properties-updated-instant/page-properties-updated-instant.mjs (2)
components/notion/sources/common/base-webhook.mjs (1)
event
(38-40)components/notion/sources/new-webhook-event-instant/new-webhook-event-instant.mjs (1)
meta
(22-22)
⏰ 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: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
🔇 Additional comments (5)
components/notion/package.json (1)
3-3
: Version bump looks appropriate.New public sources justify a minor version bump to 0.10.0. No dep changes; LGTM.
components/notion/sources/new-webhook-event-instant/test-event.mjs (1)
1-31
: Fixture is representative and consistent.Fields align with Notion webhook samples (type, entity, parent, updated_properties). Good for sampleEmit.
components/notion/sources/common/base-webhook.mjs (1)
13-16
: Copy guidance matches Notion’s flow.Logging the one-time verification_token for users to paste in the Notion UI mirrors the official verification steps. (developers.notion.com)
components/notion/sources/page-properties-updated-instant/page-properties-updated-instant.mjs (2)
4-12
: Solid source skeleton and props wiringExtends the common webhook base correctly, defines props cleanly, and sets up a dedicated source key. Good foundation.
58-61
: Keep usingevent.data.parent.id
—Notion webhooks don’t include adatabase_id
field in the event payload.The official webhook payload for
page.properties_updated
always usesdata.parent.id
(plustype
) to identify the parent entity; there is nodata.parent.database_id
in webhook events (developers.notion.com). Thedatabase_id
property appears only in the Page object returned by the Notion API, not in webhook notifications.No code changes required here.
Likely an incorrect or invalid review comment.
components/notion/sources/page-properties-updated-instant/page-properties-updated-instant.mjs
Outdated
Show resolved
Hide resolved
components/notion/sources/page-properties-updated-instant/page-properties-updated-instant.mjs
Show resolved
Hide resolved
components/notion/sources/page-properties-updated-instant/page-properties-updated-instant.mjs
Show resolved
Hide resolved
components/notion/sources/page-properties-updated-instant/test-event.mjs
Show resolved
Hide resolved
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 (3)
components/notion/sources/page-properties-updated-instant/page-properties-updated-instant.mjs (3)
8-8
: Description grammar fix looks good“set up” is correct. Thanks for addressing this.
42-51
: Fix dedupe: use webhook event id and timestamp in metaWith dedupe: "unique", meta.id must be stable across retries. Using Date.now() changes per delivery and can emit duplicates. Use event.id and event.timestamp instead. Notion’s event object includes both fields. (developers.notion.com)
Apply this diff:
- _generateMeta(page) { + _generateMeta(page, event) { const { id } = page; - const title = this.notion.extractPageTitle(page); - const ts = Date.now(); + const title = this.notion.extractPageTitle(page); + const ts = event?.timestamp + ? (Date.parse(event.timestamp) || Date.now()) + : Date.now(); + const eventId = event?.id; return { - id: `${id}-${ts}`, - summary: `Page updated: ${title}`, + id: eventId || `${id}-${ts}`, + summary: `Page properties updated: ${title || id}`, ts, }; },
63-77
: Normalize and decode updated_properties before filteringNotion delivers
updated_properties
as percent-encoded IDs (and may deliver an object keyed by IDs). Decode and support both shapes; then intersect with the selected property IDs. Otherwise events can be dropped. (developers.notion.com)Apply this diff:
- const updatedProperties = event.data.updated_properties; - if (!updatedProperties?.length) { - return; - } - - let propertyHasChanged = false; - for (const propertyName of updatedProperties) { - if (!this.properties || this.properties.includes(propertyName)) { - propertyHasChanged = true; - } - } - - if (!propertyHasChanged) { - return; - } + const updatedPropertiesRaw = event?.data?.updated_properties; + // Normalize to array of property IDs (handle array or object-keyed input) + let updatedPropIds = Array.isArray(updatedPropertiesRaw) + ? updatedPropertiesRaw + : Object.keys(updatedPropertiesRaw || {}); + // Decode percent-encoded IDs + updatedPropIds = updatedPropIds.map((p) => { + try { return decodeURIComponent(p); } catch { return p; } + }); + if (!updatedPropIds.length) return; + + // If user selected specific properties, require at least one match + if (this.properties?.length) { + const selected = new Set(this.properties.map((p) => { + try { return decodeURIComponent(p); } catch { return p; } + })); + const matchesSelection = updatedPropIds.some((p) => selected.has(p)); + if (!matchesSelection) return; + }
🧹 Nitpick comments (1)
components/notion/sources/page-properties-updated-instant/page-properties-updated-instant.mjs (1)
28-32
: Sort property options for UX; guard against missing namesMinor: return options sorted by name; fall back to id when name is absent.
Apply this diff:
- const propEntries = Object.entries(properties); - return propEntries.map((prop) => ({ - label: prop[1].name, - value: prop[1].id, - })); + return Object + .values(properties) + .map((prop) => ({ + label: prop?.name || prop?.id, + value: prop?.id, + })) + .sort((a, b) => a.label.localeCompare(b.label));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
components/notion/sources/page-properties-updated-instant/page-properties-updated-instant.mjs
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-10T19:18:27.998Z
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#14265
File: components/the_magic_drip/sources/common.mjs:35-43
Timestamp: 2024-10-10T19:18:27.998Z
Learning: In `components/the_magic_drip/sources/common.mjs`, when processing items in `getAndProcessData`, `savedIds` is intentionally updated with IDs of both emitted and non-emitted items to avoid emitting retroactive events upon first deployment and ensure only new events are emitted as they occur.
Applied to files:
components/notion/sources/page-properties-updated-instant/page-properties-updated-instant.mjs
📚 Learning: 2024-10-08T15:33:38.240Z
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#12697
File: components/salesforce_rest_api/sources/common-webhook-methods.mjs:1-71
Timestamp: 2024-10-08T15:33:38.240Z
Learning: The `common-webhook-methods.mjs` object is designed to be extended, similar to an abstract class, and intentionally does not implement certain methods like `generateWebhookMeta` and `getEventType` to enforce implementation in subclasses.
Applied to files:
components/notion/sources/page-properties-updated-instant/page-properties-updated-instant.mjs
🧬 Code graph analysis (1)
components/notion/sources/page-properties-updated-instant/page-properties-updated-instant.mjs (2)
components/notion/sources/common/base-webhook.mjs (1)
event
(38-40)components/notion/sources/new-webhook-event-instant/new-webhook-event-instant.mjs (1)
meta
(22-22)
⏰ 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: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
🔇 Additional comments (1)
components/notion/sources/page-properties-updated-instant/page-properties-updated-instant.mjs (1)
52-56
: Event-type gating is correctChecking for
page.properties_updated
aligns with the supported Notion webhook event types. (developers.notion.com)
components/notion/sources/page-properties-updated-instant/page-properties-updated-instant.mjs
Show resolved
Hide resolved
components/notion/sources/page-properties-updated-instant/page-properties-updated-instant.mjs
Show resolved
Hide resolved
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.
LGTM! Great work communicating the webhook setup!
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
♻️ Duplicate comments (1)
components/notion/sources/common/base-webhook.mjs (1)
27-34
: Harden signature verification: handle missing token/header, header casing, body type, and length mismatch.Current code can throw when token/signature are absent and doesn’t normalize header case; timingSafeEqual also throws on unequal Buffer lengths. Implement safe checks and treat body as string or object.
Apply:
- verifyWebhook(token, body, headers) { - const calculatedSignature = `sha256=${createHmac("sha256", token).update(JSON.stringify(body)) - .digest("hex")}`; - return timingSafeEqual( - Buffer.from(calculatedSignature), - Buffer.from(headers["x-notion-signature"]), - ); - }, + verifyWebhook(token, body, headers) { + try { + if (!token || !headers) return false; + const headerSig = headers["x-notion-signature"] || headers["X-Notion-Signature"]; + if (!headerSig) return false; + const payload = typeof body === "string" ? body : JSON.stringify(body); + const calculatedSignature = `sha256=${ + createHmac("sha256", token) + .update(payload, "utf8") + .digest("hex") + }`; + const a = Buffer.from(calculatedSignature, "utf8"); + const b = Buffer.from(headerSig, "utf8"); + if (a.length !== b.length) return false; + return timingSafeEqual(a, b); + } catch (err) { + console.warn("Webhook signature verification error:", err?.message); + return false; + } + },Reference: Notion signs payloads with X-Notion-Signature using the verification_token from Step 2. (developers.notion.com)
🧹 Nitpick comments (1)
components/notion/sources/common/base-webhook.mjs (1)
39-57
: Always ACK HTTP requests and avoid throws on untrusted payloads.To prevent Notion retries on non-2xx and “ignore” untrusted events per docs, respond 200 on all non-error paths (including invalid signatures and verification handshake), and only 5xx on real handler errors.
Apply:
async run(event) { - const { - body, headers, - } = event; - if (!body) { - return; - } + const { body, headers } = event; + if (!body) { + this.http.respond({ status: 200 }); + return; + } const token = this._getToken(); if (body.verification_token && !token) { this._setToken(body.verification_token); - console.log(`Verification token: ${body.verification_token}. Enter this in your Notion webhook settings.`); - return; + console.log(`Verification token: ${body.verification_token}. Paste this in your Notion webhook verification UI.`); + this.http.respond({ status: 200, body: "Verification token stored." }); + return; } - if (!this.verifyWebhook(token, body, headers)) { - throw new Error("Invalid webhook signature"); - } - - await this.processEvent(body); + if (!this.verifyWebhook(token, body, headers)) { + // Ignore untrusted payloads to avoid retries. + this.http.respond({ status: 200 }); + return; + } + try { + await this.processEvent(body); + this.http.respond({ status: 200 }); + } catch (err) { + this.http.respond({ status: 500 }); + throw err; + } },Notion retries up to 8 times on non-2xx; acknowledging with 200 for invalid signatures avoids noisy retries. Please confirm Pipedream’s default HTTP behavior if this.http.respond isn’t called. (developers.notion.com)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
components/notion/sources/common/base-webhook.mjs
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-07-24T02:06:47.016Z
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#12697
File: components/salesforce_rest_api/sources/common-webhook-methods.mjs:1-71
Timestamp: 2024-07-24T02:06:47.016Z
Learning: The `common-webhook-methods.mjs` object is designed to be extended, similar to an abstract class, and intentionally does not implement certain methods like `generateWebhookMeta` and `getEventType` to enforce implementation in subclasses.
Applied to files:
components/notion/sources/common/base-webhook.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: Verify TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
🔇 Additional comments (2)
components/notion/sources/common/base-webhook.mjs (2)
15-18
: Instructions align with Notion’s verification flow.Storing the one-time verification_token and asking the user to paste it back into Notion’s UI matches Step 2 of Notion’s webhook setup. (developers.notion.com)
35-37
: Abstract method contract is good.Throwing ConfigurationError to enforce overriding processEvent is consistent with the “base/abstract” pattern used in other sources.
/approve |
Resolves #18182
Summary by CodeRabbit
New Features
Chores