ENG-2068 Sync and publish node schemas while publishing nodes - #1249
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
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}
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
eacdc23 to
2f999dc
Compare
ab026af to
05ce8da
Compare
2f999dc to
c642663
Compare
4e8d0a4 to
9792f5d
Compare
dec5cf5 to
2f1ab86
Compare
|
@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: 2f1ab864d6
ℹ️ 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".
2f1ab86 to
9a61a87
Compare
3d44a71 to
7a69089
Compare
| } | ||
|
|
||
| result.okGroupIds = groupIds; | ||
| result.publishedNodeSchemaUids = [...nodeSchemaUids]; |
There was a problem hiding this comment.
Logic Bug: publishedNodeSchemaUids includes failed schemas
This line assigns ALL node schema UIDs to publishedNodeSchemaUids, including ones that failed to sync (tracked in failedSyncedUids). This is inconsistent with how publishedNodeUids works (line 157), which only includes successfully synced nodes.
Fix: Exclude failed schemas:
result.publishedNodeSchemaUids = [...nodeSchemaUids].filter(
(uid) => !result.failedSyncedUids.includes(uid)
);| result.publishedNodeSchemaUids = [...nodeSchemaUids]; | |
| result.publishedNodeSchemaUids = [...nodeSchemaUids].filter( | |
| (uid) => !result.failedSyncedUids.includes(uid) | |
| ); | |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
2 participants
https://linear.app/discourse-graphs/issue/ENG-2068/sync-and-publish-node-schemas-while-publishing-nodes
explanation:
https://www.loom.com/share/d92d2c80d10b4ec98745a6223c5778e5
testing:
https://www.loom.com/share/b65354b89d154f8bbfe08e66c8a52116