[ENG-1860] Add Roam manual refresh for imported nodes - #1279
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR size/scope checkThis PR is over our review-size guideline.
Please split this into smaller PRs unless there is a clear reason the changes need to land together. If keeping it as one PR, please add a brief justification covering:
|
|
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}
![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: 4cef87b2f2
ℹ️ 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".
| .from("my_contents") | ||
| .select(DIRECT_CONTENT_COLUMNS) | ||
| .eq("space_id", space.id) | ||
| .eq("source_local_id", sourceLocalId) | ||
| .eq("variant", "direct"), |
There was a problem hiding this comment.
When a source has a derived (original = NULL) direct representation alongside its original direct content—a layout explicitly allowed by the content uniqueness indexes—this query returns both rows. buildSharedNodes then collapses them using last-write-wins map insertion with no ordering, so a refresh can nondeterministically use the derived text as the page title and rename the imported page incorrectly. Restrict this lookup to original = true, as the subsequent full-Markdown fetch already does.
Useful? React with 👍 / 👎.
4cef87b to
11446f6
Compare
11446f6 to
9f74a1a
Compare
ENG-2022 made node-sharing features sync-independent; the import and publish entry points gate on Enable node sharing, so the refresh button for imported nodes must too.
2f76c20 to
b47167e
Compare
1 participant
eng-1860.mp4
Adds a manual refresh path for imported cross-app nodes in Roam. Pages carrying stored
importedFromsource identity get aRefreshbutton in the page-title additions (gated on the same node-sharing flag as the import entry point). Clicking it re-fetches the latest shared content by the storedsourceNodeRid— never by the local title — and overwrites the local page through the existingmaterializeSharedNodeupdate path, which preserves identity props, renames on title drift, and never duplicates. This mirrors the Obsidian per-note refresh (refreshImportedFile).Two pieces of infra:
getSharedNodeByRidinpackages/databasefetches one shared node by rid with targeted queries (space by url, then the concept/direct/full rows), reusingbuildSharedNodes; andmaterializeSharedNodegains an opt-inforcethat bypasses the up-to-date skip, since a manual refresh must restore source content even when stored timestamps say current (MVP0: imported local edits are overwritten by refresh). The outcome surfaces as one toast; materialization failures and unexpected errors also go to PostHog. If the stored rid resolves to a different page holding the same identity, refresh reports that instead of claiming success on the clicked page.The button check runs outside the discourse-node gate in the H1 observer so imported pages whose titles don't match local node formats still get the button. Refreshing a source with no full content yields a title-only page, matching import's existing (tested) behavior. Refresh-all is ENG-1861, provenance indicators ENG-1875.
Scope check
$scope-checkagainst ENG-1860 and the final diff.Done When: None.