ENG-1549: Add convert existing page to discourse node command#987
Merged
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
mdroidian
requested changes
May 5, 2026
Address review comments on PR #987: - DRY the SmartBlocks template-handling logic by extracting insertTemplateBlocks into createDiscourseNode.ts and reusing it from convertPageToNodeFromCommand. - Tighten createOverride return type from Promise<string | void> to Promise<string> so the dialog can collapse the override fallback into a single const.
Image insertion belongs with template insertion (matches the original pre-refactor coupling). Extract handleImageCreation to a module-level insertCanvasKeyImage and call it from the helper after template work. createDiscourseNode no longer needs a separate image step; convert flow's image branch is a no-op via undefined imageUrl.
Pure extraction — the function body is identical to the original closure. The signature now takes pageUid, discourseNodes, configPageUid, imageUrl, extensionAPI, and text as params instead of picking them up via closure. createDiscourseNode passes its already-computed discourseNodes through (no duplicate fetch).
sid597
commented
May 12, 2026
| return false; | ||
| }; | ||
|
|
||
| const handleImageCreation = async ({ |
Collaborator
Author
There was a problem hiding this comment.
Pulled out function from line 53, nothing changed in the implementation of this.
Step back from the insertTemplateBlocks orchestrator. The PR is just two function extractions: - hasSmartBlockSyntax and createBlocksFromTemplate are exported from createDiscourseNode.ts at module scope. - handleImageCreation stays at module scope (used only by createDiscourseNode). createDiscourseNode keeps its original if/else SmartBlocks orchestration; convertPageToNodeFromCommand keeps its inline orchestration too. Both sites now call the shared helpers.
sid597
commented
May 12, 2026
| const results = await runQuery({ | ||
| extensionAPI, | ||
| parentUid, | ||
| // eslint-disable-next-line @typescript-eslint/naming-convention |
Collaborator
Author
There was a problem hiding this comment.
extra line to fix lint
sid597
commented
May 12, 2026
| }) => { | ||
| const createBlocksFromTemplate = async () => { | ||
| await Promise.all( | ||
| stripUid(templateNode.children).map((node, templateOrder) => |
Collaborator
Author
There was a problem hiding this comment.
- Removed the
uidfrom the place we pulled this from (line153).
Because: stripUid(...) already removes uid, so the destructuring was redundant and caused an unused uid lint warning
- Updated from
ordertotemplateOrder, defaults to 0. We need to add template to last block if we convert a page using the command palette command
sid597
commented
May 12, 2026
| }); | ||
| await createBlocksFromTemplate(); | ||
| } else if (useSmartBlocks && window.roamjs?.extension?.smartblocks) { | ||
| void window.roamjs.extension.smartblocks?.triggerSmartblock({ |
Collaborator
Author
There was a problem hiding this comment.
void That is only to mark the promise as intentionally not awaited. Runtime intent is the same
mdroidian
approved these changes
May 13, 2026
…ting-page-to-node-roam # Conflicts: # apps/roam/src/utils/createDiscourseNode.ts
…ting-page-to-node-roam # Conflicts: # apps/roam/src/utils/createDiscourseNode.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://www.loom.com/share/ace42b9d8d66415b9143d4d4a164cfc1