ENG-1138 Migrate canvas shape node type to discourse-node#894
ENG-1138 Migrate canvas shape node type to discourse-node#894mdroidian wants to merge 4 commits into
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![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.
This comment was marked as resolved.
This comment was marked as resolved.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
mdroidian
force-pushed
the
eng-1138-migrate-canvas-shape-node-type-to-discourse-node
branch
2 times, most recently
from
458bbb7 to
a6091e9
Compare
- Replaced the use of createNodeShapeUtils with DiscourseNodeUtil across components to streamline discourse node management. - Introduced DISCOURSE_NODE_SHAPE_TYPE constant for consistent node type identification. - Updated various components to utilize nodeTypeId for better clarity and functionality in shape creation and manipulation. - Enhanced migration logic to accommodate new node type structure, ensuring backward compatibility and improved data integrity. This refactor improves the maintainability and readability of the codebase while ensuring that new features can be integrated more seamlessly.
mdroidian
force-pushed
the
eng-1138-migrate-canvas-shape-node-type-to-discourse-node
branch
from
a6091e9 to
1397ce2
Compare
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1397ce23c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return ( | ||
| getRecordTypeName(r) === "shape" && | ||
| !!recordType && | ||
| allNodeTypes.includes(recordType) |
There was a problem hiding this comment.
When a relation is also exposed as an anchor-backed node, getDiscourseNodes adds a node whose type is the relation id, so allNodeTypes can contain relation shape types too. This filter then matches existing relation arrow records as if they were legacy node records, and the up step changes those arrows to discourse-node, corrupting canvases for those configurations on migration; the filter should distinguish node records by their node props or exclude relation shape ids.
Useful? React with 👍 / 👎.
| }: TLShapeUtilCanBindOpts<DiscourseRelationShape>): boolean { | ||
| // bindings can go from arrows to shapes, but not from shapes to arrows | ||
| return toShapeType !== "arrow"; | ||
| return toShapeType === DISCOURSE_NODE_SHAPE_TYPE; |
There was a problem hiding this comment.
The Cloudflare adapter still registers legacy node-id shape utils for rooms that stream pre-migration records, but this predicate rejects any target whose shape type is still a node id such as page-node or a custom node UID. In those rooms, drawing a relation to a legacy node will not create the binding, and dragging an existing endpoint over the same legacy node can remove the binding because editor.canBindShapes returns false; allow legacy discourse node types here as well as discourse-node.
Useful? React with 👍 / 👎.
1 participant
[[Page]]into the canvas and confirm it creates a discourse node correctly.((block-uid))into the canvas and confirm it creates a discourse node correctly.Summary by CodeRabbit