ENG-1865 Publish Roam stored relations for shared nodes - #1221
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
df8bd66 to
7e9a33d
Compare
7e9a33d to
e15e0eb
Compare
![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.
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
e15e0eb to
9e93709
Compare
9e93709 to
123104f
Compare
123104f to
ae3123f
Compare
ae3123f to
f71e309
Compare
c2ee00a to
1127f9a
Compare
f71e309 to
a96cd20
Compare
a96cd20 to
af01ede
Compare
af01ede to
abacebf
Compare
51ca3bc to
dc87ee0
Compare
a9b80ec to
fdd0a3d
Compare
fdd0a3d to
17a85f8
Compare
dbb1311 to
3af849e
Compare
3af849e to
4df42a3
Compare
4df42a3 to
5b550bd
Compare
5b550bd to
9dcd771
Compare
9dcd771 to
126f6f0
Compare
126f6f0 to
3d55378
Compare
3d55378 to
9f6663d
Compare
9f6663d to
6ab0a38
Compare
6ab0a38 to
ff86870
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: 476c18f5eb
ℹ️ 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".
| const sourceId = | ||
| sourceSpaceUri === undefined | ||
| ? r.sourceUid | ||
| : spaceUriAndLocalIdToRid(sourceSpaceUri, r.sourceUid); |
There was a problem hiding this comment.
When a relation endpoint is an imported node, readImportedSourceIdentity already provides its canonical sourceNodeRid, but this conversion keeps only the space URI and combines it with the local Roam page UID. For example, an imported .../node-1 stored on page-uid becomes .../page-uid, so the database cannot resolve the relation to the remote concept. Return the stored RID directly, or reconstruct it with the parsed source local ID.
Useful? React with 👍 / 👎.
| (publishedIds.has(r.sourceUid) || | ||
| (forNodeIds ? forNodeIds.has(r.sourceUid) : false) || | ||
| groupSpaceIds.has(isImportedFrom(r.sourceUid) || 0)) && | ||
| (publishedIds.has(r.destinationUid) || | ||
| (forNodeIds ? forNodeIds.has(r.destinationUid) : false) || | ||
| groupSpaceIds.has(isImportedFrom(r.destinationUid) || 0)), |
There was a problem hiding this comment.
When a selected node has not synced yet, this condition still treats it as an available relation endpoint via forNodeIds. The later syncedUids intersection removes that node from the published node accesses, but the already-gathered relation is still upserted and granted, leaving it with an unresolved local endpoint. Relation gathering should use only the selected node IDs confirmed by my_concepts, or otherwise discard relations touching skipped nodes.
Useful? React with 👍 / 👎.
| const groupResourceIds = [ | ||
| ...resourceIds, | ||
| ...groupRelationIds, | ||
| ...groupRelationTripleSchemaIds, |
There was a problem hiding this comment.
If a stored relation references a deleted schema, or its converter returns null, it is removed from the returned relations array but remains in relevantRelationIdsPerGroupId. Spreading the unfiltered IDs here therefore inserts a ResourceAccess row for a concept that was never upserted; this table has no resource foreign key, so the orphan persists. Intersect these IDs with the successfully converted relations before creating access rows.
Useful? React with 👍 / 👎.
2 participants
https://linear.app/discourse-graphs/issue/ENG-1865/publish-roam-stored-relations-for-shared-nodes
PR walkthrough:
https://www.loom.com/share/c8a96f0cdbc74dcf8fada181ca2f7144
Note: In the loom, I mention a bit of code waiting for 1856. That got merged, and I did make the adjustments accordingly, so disregard that.
Demo:
https://www.loom.com/share/2d93fd0570784f2eb25fac7c514911a3