ENG-1171 Remove migrateLeftSideBarSettings - #1419
Conversation
The left sidebar ran two one-time migrations on every mount. Each self-skips via a marker block, but the check itself still costs `getLeftSidebarSettings()` plus `getPageUidByPageTitle()` calls on every load. `migrateLeftSidebarSettings` converted sidebar entries stored as page titles into page UIDs. Current write paths (`migrateFavorites`, `addBlockToGlobalSection`, `addBlockToPersonalSection`) all store UIDs, so the conversion has nothing left to do. Removes the migration and the config fields that existed only to serve it. `migrateFavorites` is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
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. |
| let root = wrapper.querySelector(`#${id}`) as HTMLDivElement; | ||
| if (!root) { | ||
| await migrateFavorites(); | ||
| await migrateLeftSidebarSettings(); |
There was a problem hiding this comment.
🟡 Legacy sidebar entries become unusable
Graphs upgrading directly from a pre-migration release keep title-encoded entries because mountLeftSidebar no longer converts them. ChildRow treats every page entry as a UID, leaving these entries blank and unable to open.
Prompt for agents
Preserve compatibility for graphs whose legacy Left Sidebar children still contain page titles. The removed call in apps/roam/src/components/LeftSidebarView.tsx previously converted global and every user's personal sidebar children before rendering and before block-prop migration could copy those values. Restore an equivalent one-time conversion path, or make the legacy and block-prop readers normalize titles to page UIDs before ChildRow receives them. Keep the fast path for already migrated UID data, and cover direct upgrades from releases predating ENG-1127, including users whose sidebar was not mounted while the old migration existed.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
@coderabbitai full review |
|
|
Warning Review limit reachedNext included review available in 59 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@sid597 could you review this PR please? |
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
mdroidian
left a comment
There was a problem hiding this comment.
Could you run dg-delegated-full-review on these changes please? Thanks!
Left Sidebar setting is intact and functional
Scope check
Ran
$scope-checkagainst the ENG ticket and final diff.Scope beyond
Done When: None.The removed
sidebarMigrated/allPersonalSectionsfields,getAllLeftSidebarPersonalSectionConfigsandAllUsersPersonalSectionshad no consumers outside the deleted migration.🤖 Generated with Claude Code