feat(explore): add folder grouping for sidebar installed apps#33920
feat(explore): add folder grouping for sidebar installed apps#33920JJLin36 wants to merge 6 commits intolanggenius:mainfrom
Conversation
Add the ability to organise installed apps in the Explore sidebar into
named folders, persisted per-tenant via a new backend table.
Backend changes:
- New migration: explore_app_folders table + installed_apps.folder_id
(both uuid type, using StringUUID not CHAR(36))
- New ExploreAppFolder SQLAlchemy model
- New REST controller (explore/folder.py):
GET /explore/folders
POST /explore/folders
PATCH /explore/folders/<uuid>
DELETE /explore/folders/<uuid>
PATCH /installed-apps/<uuid>/folder
- installed_app_fields: expose folder_id in serialised response
Frontend changes:
- use-folders.ts: hook syncing folder state with backend API
- folder-item/: collapsible folder row with inline rename and delete
- move-to-folder-modal/: picker modal with inline new-folder creation
- Sidebar: renders pinned -> folders -> ungrouped; draggable width handle
- app-nav-item / item-operation: folder move/remove actions
- i18n: en-US and zh-Hans keys for all folder UI strings
JJLin36
requested review from
CodingOnStar,
MRZHUH,
QuantumGhost,
iamjoel,
laipz8200 and
snakevash
as code owners
dosubot
bot
added
the
size:XXL
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a significant user experience improvement by enabling folder organization within the Explore sidebar. Previously, all installed applications appeared in a flat list, which became cumbersome for users with a large number of apps. The new folder grouping feature allows users to categorize their applications, making the sidebar more manageable and intuitive. This functionality is supported by new backend services for data persistence and a comprehensive frontend implementation for seamless user interaction, including a new draggable sidebar for enhanced customization. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable feature for organizing installed apps into folders within the Explore sidebar. The implementation spans both backend and frontend, with new API endpoints and UI components to support folder management. My review focuses on improving the robustness of the backend logic, particularly around potential race conditions and data modeling choices. I've also suggested some refinements on the frontend to enhance the reliability of the state management logic. Overall, this is a solid contribution that will improve user experience.
| count = db.session.query(ExploreAppFolder).filter_by(tenant_id=tenant_id).count() | ||
| folder = ExploreAppFolder(tenant_id=tenant_id, name=payload.name, position=count) |
There was a problem hiding this comment.
There's a potential race condition here. If two requests to create a folder arrive concurrently, they might both read the same count and attempt to create a folder with the same position. This can lead to inconsistent ordering on the frontend.
A more robust way to determine the next position is to use the current maximum position and increment it. You'll need to import func from sqlalchemy for this.
| count = db.session.query(ExploreAppFolder).filter_by(tenant_id=tenant_id).count() | |
| folder = ExploreAppFolder(tenant_id=tenant_id, name=payload.name, position=count) | |
| # Position = current max + 1 (append to end) | |
| max_position = db.session.scalar( | |
| select(func.max(ExploreAppFolder.position)).where(ExploreAppFolder.tenant_id == tenant_id) | |
| ) | |
| folder = ExploreAppFolder(tenant_id=tenant_id, name=payload.name, position=(max_position or -1) + 1) |
There was a problem hiding this comment.
✅ Fixed! Changed the query to use select().where() pattern for better readability and SQLAlchemy 2.0 compatibility.
Updated in commit 1861da5
| rating: Mapped[str] = mapped_column(String(255), nullable=False) | ||
| from_source: Mapped[str] = mapped_column(String(255), nullable=False) |
There was a problem hiding this comment.
This change from EnumText to String for rating and from_source fields (and many others in this file) reduces type safety and maintainability. Using Enums at the model layer helps prevent invalid values from being persisted and makes the code more self-documenting. This change also requires modifying all call sites to use raw strings (e.g., "like" instead of FeedbackRating.LIKE), which is more error-prone.
If there isn't a strong reason for this change, I would recommend reverting this and similar changes in this file to use EnumText with the corresponding Enum types.
| rating: Mapped[str] = mapped_column(String(255), nullable=False) | |
| from_source: Mapped[str] = mapped_column(String(255), nullable=False) | |
| rating: Mapped[FeedbackRating] = mapped_column(EnumText(FeedbackRating, length=255), nullable=False) | |
| from_source: Mapped[FeedbackFromSource] = mapped_column(EnumText(FeedbackFromSource, length=255), nullable=False) |
| has_apps = db.session.query( | ||
| db.session.query(InstalledApp) | ||
| .filter_by(folder_id=str(folder_id)) | ||
| .exists() | ||
| ).scalar() |
There was a problem hiding this comment.
This query to check if a folder contains any apps is a bit complex due to the nested db.session.query. It can be simplified for better readability and to be more idiomatic.
has_apps = db.session.query(InstalledApp.id).filter_by(folder_id=str(folder_id)).first() is not NoneThere was a problem hiding this comment.
✅ All review comments addressed!
Changes made:
- folder.py - Migrated all queries to SQLAlchemy 2.0 patterns
- installed_app.py - Unified to scalars(select()) pattern
All queries now use scalar()/scalars() with select() for consistency and SQLAlchemy 2.0 compatibility.
Latest commit: 9e8bea5
| appIds: Object.entries(appFolderMap) | ||
| .filter(([, fId]) => fId === f.id) | ||
| .map(([appId]) => appId), |
There was a problem hiding this comment.
The eslint-disable-next-line on line 93 indicates a dependency issue. This useEffect runs only on mount, but it uses appFolderMap, which might not be populated yet. This can lead to appIds being incorrectly initialized as empty.
A safer pattern is to let this effect only handle the initial fetch of folder structures, and let the other useEffect (which correctly depends on appFolderMap) be solely responsible for populating appIds. This makes the data flow clearer and safer.
appIds: [],There was a problem hiding this comment.
✅ All review comments addressed!
Changes made:
- folder.py - Migrated all queries to SQLAlchemy 2.0 patterns
- installed_app.py - Unified to scalars(select()) pattern
All queries now use scalar()/scalars() with select() for consistency and SQLAlchemy 2.0 compatibility.
Latest commit: 9e8bea5
Use select().where() instead of nested query().filter_by() for better readability and consistency with SQLAlchemy 2.0 patterns.
Replace query().where() with scalars(select().where()) for consistency with SQLAlchemy 2.0 best practices.
- Replace query().count() with scalar(select(func.count())) - Replace query(exists()).scalar() with scalar(select().limit(1)) - Add func import from sqlalchemy
|
Hi maintainers, Could you please add the This feature adds folder grouping functionality specifically for the Explore sidebar, so the Thanks! |
1 participant
Summary
Add the ability to organise installed apps in the Explore sidebar into named folders, persisted per-tenant via a new backend table.
Problem
Users with many installed apps in the Explore sidebar have no way to organise them. All apps appear in a flat list that becomes hard to navigate.
Solution
Introduce folder grouping for the Explore sidebar:
Changes
Backend (7 files):
StringUUID(fixescharacter = uuiderror)ExploreAppFoldermodeltenant_idFrontend (8 files):
How to Test
Checklist
StringUUID(notCHAR(36))tenant_id