-
Notifications
You must be signed in to change notification settings - Fork 2
ENG-448: Query Roam Discourse Nodes created or modified since the last update #232
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
ENG-448: Query Roam Discourse Nodes created or modified since the last update #232
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
📝 WalkthroughWalkthroughA new utility function, Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant getAllDiscourseNodesSince
participant RoamAPI
participant isDiscourseNode
Caller->>getAllDiscourseNodesSince: Call with "since" date string
getAllDiscourseNodesSince->>RoamAPI: Query for blocks edited after timestamp
RoamAPI-->>getAllDiscourseNodesSince: Return block data
getAllDiscourseNodesSince->>isDiscourseNode: Check each block's source_local_id
isDiscourseNode-->>getAllDiscourseNodesSince: Return boolean
getAllDiscourseNodesSince-->>Caller: Return filtered discourse graph content array
Suggested reviewers
Poem
✨ Finishing Touches
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 (
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
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
🧹 Nitpick comments (1)
apps/roam/src/utils/getAllDiscourseNodesSince.ts (1)
33-39
: Add defensive programming and consider query optimization.The filtering logic looks good, but consider these improvements:
- Add error handling for the
isDiscourseNode
function call- The filtering could be moved to the Datalog query for better performance
return result.filter( (entity) => entity.source_local_id && - isDiscourseNode(entity.source_local_id) && + (() => { + try { + return isDiscourseNode(entity.source_local_id); + } catch (error) { + console.warn(`Error checking discourse node: ${entity.source_local_id}`, error); + return false; + } + })() && entity.text && entity.text.trim() !== "", );Consider moving some filtering logic to the Datalog query itself to reduce the amount of data transferred and processed in JavaScript. For example, you could add conditions to filter out empty strings directly in the query.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/roam/src/utils/getAllDiscourseNodesSince.ts
(1 hunks)
🔇 Additional comments (1)
apps/roam/src/utils/getAllDiscourseNodesSince.ts (1)
3-10
: ```shell
#!/bin/bashDisplay the contents of the utility to inspect the Datalog query and field mappings
sed -n '1,200p' apps/roam/src/utils/getAllDiscourseNodesSince.ts
</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
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.
Some changes requested.
Here is the result of using this function
Summary by CodeRabbit