-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Notion - update to New or Updated Page in Database source #17762
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
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
WalkthroughThe Notion package version was incremented to 0.8.0. The existing updated-page source was renamed and versioned to "New or Updated Page in Database (By Property)". A new source, "New or Updated Page in Database (By Timestamp)", was added, emitting events based on page last edited timestamps with options to exclude new pages. A test event was also added. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant NotionSource
participant NotionAPI
User->>NotionSource: Configure source (databaseId, includeNewPages)
NotionSource->>NotionSource: Retrieve last updated timestamp checkpoint
loop Poll for updates
NotionSource->>NotionAPI: Query pages updated since checkpoint
NotionAPI-->>NotionSource: Stream pages sorted by last edited time
NotionSource->>NotionSource: For each page, update checkpoint timestamp
alt Page is new and includeNewPages is false
NotionSource->>NotionSource: Skip emitting event
else
NotionSource->>User: Emit event with page metadata and summary
end
end
NotionSource->>NotionSource: Update stored last updated timestamp
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
components/notion/sources/updated-page/updated-page.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/notion/sources/updated-page-by-timestamp/test-event.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/notion/sources/updated-page-by-timestamp/updated-page-by-timestamp.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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.
Hi @michelle0927 lgtm! Ready for QA!
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
🧹 Nitpick comments (1)
components/notion/sources/updated-page-by-timestamp/updated-page-by-timestamp.mjs (1)
64-67
: Consider using >= for consistency in timestamp comparison.The break condition uses
>
but for strict timestamp-based deduplication, consider using>=
to ensure pages with exactly the same timestamp as the last checkpoint are not processed twice.- if (lastUpdatedTimestamp > Date.parse(page.last_edited_time)) { + if (lastUpdatedTimestamp >= Date.parse(page.last_edited_time)) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
components/notion/package.json
(1 hunks)components/notion/sources/updated-page-by-timestamp/test-event.mjs
(1 hunks)components/notion/sources/updated-page-by-timestamp/updated-page-by-timestamp.mjs
(1 hunks)components/notion/sources/updated-page/updated-page.mjs
(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- components/notion/sources/updated-page/updated-page.mjs
- components/notion/sources/updated-page-by-timestamp/test-event.mjs
- components/notion/package.json
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: GTFalcao
PR: PipedreamHQ/pipedream#15376
File: components/monday/sources/name-updated/name-updated.mjs:6-6
Timestamp: 2025-01-23T03:55:15.166Z
Learning: Source names in Monday.com components don't need to start with "New" if they emit events for updated items (e.g., "Name Updated", "Column Value Updated") rather than new items. This follows the component guidelines exception where the "New" prefix is only required when emits are limited to new items.
components/notion/sources/updated-page-by-timestamp/updated-page-by-timestamp.mjs (2)
Learnt from: GTFalcao
PR: #15376
File: components/monday/sources/name-updated/name-updated.mjs:6-6
Timestamp: 2025-01-23T03:55:15.166Z
Learning: Source names in Monday.com components don't need to start with "New" if they emit events for updated items (e.g., "Name Updated", "Column Value Updated") rather than new items. This follows the component guidelines exception where the "New" prefix is only required when emits are limited to new items.
Learnt from: GTFalcao
PR: #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.
🧬 Code Graph Analysis (1)
components/notion/sources/updated-page-by-timestamp/updated-page-by-timestamp.mjs (2)
components/notion/notion.app.mjs (1)
notion
(361-361)components/notion/sources/updated-page/updated-page.mjs (10)
obj
(112-112)title
(113-113)ts
(114-114)isNewPage
(157-157)meta
(122-124)newLastUpdatedTimestamp
(151-151)params
(53-53)params
(142-150)pagesStream
(54-54)pagesStream
(153-153)
⏰ 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
🔇 Additional comments (6)
components/notion/sources/updated-page-by-timestamp/updated-page-by-timestamp.mjs (6)
1-13
: LGTM! Component structure follows established patterns.The imports, component metadata, and basic structure are well-organized and follow the expected conventions for Notion source components.
14-28
: LGTM! Props are well-designed with clear user control.The
includeNewPages
prop provides useful flexibility for users who want to filter out new pages and only receive update events. The default value oftrue
maintains backward compatibility.
31-40
: LGTM! Metadata generation uses appropriate timestamp.Good use of
Date.parse(obj.last_edited_time)
instead ofDate.now()
to reflect the actual page edit time. The composite ID ensures proper deduplication across different edit timestamps.
41-46
: LGTM! Event emission follows established patterns.The conditional metadata generation based on
isNewPage
flag properly distinguishes between new and updated pages, consistent with other Notion sources.
48-62
: LGTM! API query parameters are correctly constructed.The filter and sort parameters properly use the Notion API's
last_edited_time
field withon_or_after
filtering, following established patterns from other Notion sources.
69-87
: LGTM! Main processing logic is well-implemented.The timestamp tracking, new page detection logic (
last_edited_time === created_time
), and conditional processing based onincludeNewPages
are all correctly implemented. The debugging output and final timestamp persistence complete a solid implementation.
/approve |
Resolves #16811
Summary by CodeRabbit
New Features
Chores