ENG-1280: Port Discourse node: Query builder and editor component for block props#764
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughThe changes implement synchronization of query builder state to discourse node block properties. A new optional Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant QueryEditor
participant DebounceTimer
participant IndexSchema
participant DiscourseNode
User->>QueryEditor: Update conditions/selections/custom
QueryEditor->>DebounceTimer: Clear previous timeout
DebounceTimer->>DebounceTimer: Start new 500ms timer
DebounceTimer->>IndexSchema: Validate query state
IndexSchema-->>IndexSchema: safeParse(data)
alt Validation succeeds
IndexSchema-->>QueryEditor: Valid data
QueryEditor->>DiscourseNode: setDiscourseNodeSetting("index", validatedData)
DiscourseNode-->>QueryEditor: Updated
else Validation fails
IndexSchema-->>QueryEditor: Parse error (silently ignored)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/roam/src/components/settings/DiscourseNodeIndex.tsx (1)
27-68:⚠️ Potential issue | 🟠 MajorNo error handling on the
createBlockpromise — spinner may hang indefinitely on failure.If
createBlockrejects,setShowQuery(true)is never called, leaving the user stuck on a<Spinner />with no recovery path. Consider adding a.catch()to handle the failure gracefully.Proposed fix
void createBlock({ parentUid: initialQueryArgs.conditionsNodesUid, node: { text: "clause", children: [ { text: "source", children: [{ text: DEFAULT_RETURN_NODE }], }, { text: "relation", children: [{ text: "is a" }], }, { text: "target", children: [ { text: node.text, }, ], }, ], }, }).then(() => { setDiscourseNodeSetting(node.type, ["index"], { conditions: [ { type: "clause", source: DEFAULT_RETURN_NODE, relation: "is a", target: node.text, }, ], selections: [], }); setShowQuery(true); - }); + }).catch((error) => { + console.error("Failed to create initial query block:", error); + setShowQuery(true); + });
🧹 Nitpick comments (1)
apps/roam/src/components/settings/DiscourseNodeIndex.tsx (1)
53-63: Minor inconsistency: initial index write omitscustomfield.The
IndexSchemanow includescustom: z.string().default(""), andQueryEditor's sync logic serializescustom. This initial write tosetDiscourseNodeSettingomits it. It works because Zod fills in the default on read, but addingcustom: ""here would be consistent with the schema and the sync logic inQueryEditor.Proposed fix
setDiscourseNodeSetting(node.type, ["index"], { conditions: [ { type: "clause", source: DEFAULT_RETURN_NODE, relation: "is a", target: node.text, }, ], selections: [], + custom: "", });
898a0bd to
adab8d8
Compare
ee13988 to
3b51bcc
Compare
adab8d8 to
cc9fd40
Compare
2700c52 to
7cd4348
Compare
3be01d4 to
72de51b
Compare
7cd4348 to
70a27fd
Compare
28d44a4 to
5a3bbba
Compare
ed44054 to
f2c0787
Compare
5a3bbba to
7eda278
Compare
7eda278 to
ff23661
Compare
f2c0787 to
26971c9
Compare
7d6f127
into
eng-1225-discourse-node-migrate-settings-prod
These PRs were squash-merged into parent branches instead of main due to a Graphite stack merge race condition. Their changes never landed on main despite showing as merged in Graphite. Includes: - #764 ENG-1280: Port query builder/editor for block props - #765 ENG-1291: Port discourse node specification (dual-write) - #769 ENG-1290: Port discourse node config panel - #793 ENG-1440: Port page groups settings in suggestive mode - #794 ENG-1438: Port keyboard shortcut keys/triggers - #805 ENG-1328: Port small blueprint misc settings
specification, config panel) The initial recovery commit claimed to include these but only contained Chain 2 (#793, #794, #805, #804). Chain 1 lives on separate branches that weren't in the merge source. Applied using `gh pr diff` for each PR in order (#764 → #765 → #769). init.ts required manual merge — #765 and #769 both modified it from the same base. Kept #765's safeParse checks inside #769's restructured initializeSettingsBlockProps. - #764 ENG-1280: Port query builder and editor component for block props - #765 ENG-1291: Port discourse node specification - #769 ENG-1290: Port discourse node config panel
…#826) * Land orphaned Graphite stack PRs (#764, #765, #769, #793, #794, #805) These PRs were squash-merged into parent branches instead of main due to a Graphite stack merge race condition. Their changes never landed on main despite showing as merged in Graphite. Includes: - #764 ENG-1280: Port query builder/editor for block props - #765 ENG-1291: Port discourse node specification (dual-write) - #769 ENG-1290: Port discourse node config panel - #793 ENG-1440: Port page groups settings in suggestive mode - #794 ENG-1438: Port keyboard shortcut keys/triggers - #805 ENG-1328: Port small blueprint misc settings * Add #804 ENG-1217: Port section component in left sidebar * Add #764, #765, #769 (Chain 1 orphaned PRs: query builder, specification, config panel) The initial recovery commit claimed to include these but only contained Chain 2 (#793, #794, #805, #804). Chain 1 lives on separate branches that weren't in the merge source. Applied using `gh pr diff` for each PR in order (#764 → #765 → #769). init.ts required manual merge — #765 and #769 both modified it from the same base. Kept #765's safeParse checks inside #769's restructured initializeSettingsBlockProps. - #764 ENG-1280: Port query builder and editor component for block props - #765 ENG-1291: Port discourse node specification - #769 ENG-1290: Port discourse node config panel * fix lint * prettier
These PRs were squash-merged into parent branches instead of main due to a Graphite stack merge race condition. Their changes never landed on main despite showing as merged in Graphite. Includes: - #764 ENG-1280: Port query builder/editor for block props - #765 ENG-1291: Port discourse node specification (dual-write) - #769 ENG-1290: Port discourse node config panel - #793 ENG-1440: Port page groups settings in suggestive mode - #794 ENG-1438: Port keyboard shortcut keys/triggers - #805 ENG-1328: Port small blueprint misc settings

https://www.loom.com/share/92b399a36f4d4e3f8e49a95e5c65661b
Summary by CodeRabbit