ENG-1197: fix personal settings migration, for all users insted of only the user who instantiated it

[sid597]

Summary by CodeRabbit

• Refactor
  • Enhanced the left sidebar configuration system with improved architecture to support comprehensive multi-user personal section aggregation and management.
  • Updated sidebar settings computation and migration logic to process expanded personal section tracking across users for better section organization.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[linear]

ENG-1197 Caching issue causes the plugin to break, some settings are migrated some are not for some users

[supabase]

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

---

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

[sid597]

• ENG-1197: fix personal settings migration, for all users insted of only the user who instantiated it #639 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[sid597]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

📝 Walkthrough

Walkthrough

Extends the Left Sidebar settings utilities to support multi-user aggregation of personal sections. Introduces AllUsersPersonalSections type and new getAllLeftSidebarPersonalSectionConfigs API; modifies getLeftSidebarPersonalSectionConfig to accept an optional user identifier; updates getLeftSidebarSettings to compute and expose aggregated personal sections; and extends migration logic to handle the new nested data structure.

Changes

Cohort / File(s)	Change Summary
Left Sidebar Settings Core apps/roam/src/utils/getLeftSidebarSettings.ts	Adds AllUsersPersonalSections type for multi-user personal section aggregation. Introduces getAllLeftSidebarPersonalSectionConfigs() function to compute per-user personal sections. Modifies getLeftSidebarPersonalSectionConfig() to accept optional userUid parameter with internal targetUserUid variable. Updates getLeftSidebarSettings() to compute and include allPersonalSections in returned config.
Left Sidebar Settings Migration apps/roam/src/utils/migrateLeftSidebarSettings.ts	Extends migration logic to iterate over allPersonalSections and migrate child sections for each user where children are present.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Review the implementation of getAllLeftSidebarPersonalSectionConfigs() to ensure correct iteration and aggregation logic across all users
• Verify that the optional userUid parameter in getLeftSidebarPersonalSectionConfig() correctly defaults and applies the targetUserUid logic
• Ensure the nested loop structure in migrateLeftSidebarSettings() correctly handles the new allPersonalSections hierarchy

Possibly related PRs

• ENG-851 Get left sidebar sections and settings from config page #439: Directly modifies the same getLeftSidebarSettings module and introduces the LeftSidebarConfig types that this PR extends with multi-user aggregation.
• ENG-853 left sidebar global and personal settings #441: Modifies personal and global sections utilities that form the foundation for this PR's per-user personal section handling.
• ENG-882 Let user rearrange personal section and section items in place in left sidebar #456: Adjusts personal section fields in the same getLeftSidebarSettings.ts file that this PR extends with multi-user aggregation capabilities.

Pre-merge checks

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main objective of the PR: fixing personal settings migration to apply to all users instead of just the user who instantiated it, which aligns with the code changes that add multi-user aggregation for personal sections.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (4)

apps/roam/src/utils/migrateLeftSidebarSettings.ts (2)

54-73: Potential double migration of current user's personal sections.

The existing logic at lines 54-60 migrates leftSidebarSettings.personal.sections (current user's sections), and the new loop at lines 64-73 iterates over allPersonalSections which includes all users—including the current user. This means the current user's sections may be migrated twice.

While migrateSectionChildren appears idempotent (it skips items that are already UIDs via getPageTitleByPageUid check at line 16), processing the same data twice is inefficient.

Consider either:

1. Removing the original personal sections migration (lines 54-60) since allPersonalSections now covers all users, or
2. Filtering out the current user from allPersonalSections iteration.

Option 1: Remove redundant original personal sections migration

-  const personalSections = leftSidebarSettings.personal.sections;
-  for (const section of personalSections) {
-    const children = section.children || [];
-    if (children.length > 0) {
-      await migrateSectionChildren(children);
-    }
-  }
-
   const allPersonalSections = leftSidebarSettings.allPersonalSections;

   for (const [_, userPersonalSection] of Object.entries(

---

39-39: Add explicit return type for the function.

As per coding guidelines, functions should have explicit return types.

Proposed fix

-export const migrateLeftSidebarSettings = async () => {
+export const migrateLeftSidebarSettings = async (): Promise<void> => {

apps/roam/src/utils/getLeftSidebarSettings.ts (2)

185-201: Add blank line before function and explicit return type.

Minor formatting and coding guidelines improvements:

1. Add a blank line before the function declaration for better readability.
2. Add explicit return type as per coding guidelines.

Proposed fix

+
 export const getAllLeftSidebarPersonalSectionConfigs = (
   leftSidebarChildren: RoamBasicNode[],
-): AllUsersPersonalSections => {
+): AllUsersPersonalSections => {
   const result: AllUsersPersonalSections = {};

Note: The return type is already present, so only the blank line needs to be added before line 185.

---

212-214: Track the TODO for removal after migration.

The TODO comment indicates allPersonalSections should be removed after migration is complete. Consider creating an issue to track this cleanup task to ensure it doesn't get forgotten.

Would you like me to open an issue to track the removal of allPersonalSections after the migration is complete?

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 670fe22 and 0144dcf.

📒 Files selected for processing (2)

• apps/roam/src/utils/getLeftSidebarSettings.ts
• apps/roam/src/utils/migrateLeftSidebarSettings.ts

🧰 Additional context used

📓 Path-based instructions (5)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/main.mdc)

**/*.{ts,tsx}: Use Tailwind CSS for styling where possible
When refactoring inline styles, use tailwind classes
Prefer type over interface in TypeScript
Use explicit return types for functions
Avoid any types when possible
Prefer arrow functions over regular function declarations
Use named parameters (object destructuring) when a function has more than 2 parameters
Use PascalCase for components and types
Use camelCase for variables and functions
Use UPPERCASE for constants
Function names should describe their purpose clearly
Prefer early returns over nested conditionals for better readability

Files:

• apps/roam/src/utils/migrateLeftSidebarSettings.ts
• apps/roam/src/utils/getLeftSidebarSettings.ts

apps/roam/**/*.{js,ts,tsx,jsx,json}

📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)

Prefer existing dependencies from package.json when working on the Roam Research extension

Files:

• apps/roam/src/utils/migrateLeftSidebarSettings.ts
• apps/roam/src/utils/getLeftSidebarSettings.ts

apps/roam/**/*.{ts,tsx,jsx,js,css,scss}

📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)

Use BlueprintJS 3 components and Tailwind CSS for platform-native UI in the Roam Research extension

Files:

• apps/roam/src/utils/migrateLeftSidebarSettings.ts
• apps/roam/src/utils/getLeftSidebarSettings.ts

apps/roam/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)

apps/roam/**/*.{ts,tsx,js,jsx}: Use the roamAlphaApi docs from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj when implementing Roam functionality
Use Roam Depot/Extension API docs from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU when implementing extension functionality

Files:

• apps/roam/src/utils/migrateLeftSidebarSettings.ts
• apps/roam/src/utils/getLeftSidebarSettings.ts

apps/roam/**

📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)

Implement the Discourse Graph protocol in the Roam Research extension

Files:

• apps/roam/src/utils/migrateLeftSidebarSettings.ts
• apps/roam/src/utils/getLeftSidebarSettings.ts

🔇 Additional comments (3)

apps/roam/src/utils/getLeftSidebarSettings.ts (3)

129-134: LGTM!

The type definition is clean, uses type over interface as per guidelines, and the index signature appropriately models the dynamic user UID keys.

---

136-144: LGTM!

The optional userUid parameter with nullish coalescing fallback to the current user maintains backward compatibility while enabling per-user section retrieval. Clean implementation.

---

223-230: LGTM!

The integration of allPersonalSections into the return object is correct and type-safe, properly matching the updated LeftSidebarConfig type.