-
Notifications
You must be signed in to change notification settings - Fork 78
feat(FR-1296): Improve error message for deleting in-use vfolder #4706
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat(FR-1296): Improve error message for deleting in-use vfolder #4706
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has required the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Coverage report for
|
St.❔ |
Category | Percentage | Covered / Total |
|---|---|---|---|
| 🔴 | Statements | 4.38% (-0.01% 🔻) |
517/11802 |
| 🔴 | Branches | 3.57% (-0.01% 🔻) |
297/8317 |
| 🔴 | Functions | 2.55% (-0% 🔻) |
92/3612 |
| 🔴 | Lines | 4.36% (-0.01% 🔻) |
503/11543 |
Show new covered files 🐣
St.❔ |
File | Statements | Branches | Functions | Lines |
|---|---|---|---|---|---|
| 🔴 | ... / BAINotificationBackgroundItem.tsx |
0% | 0% | 0% | 0% |
| 🔴 | ... / BAIVirtualFolderNodeNotificationItem.tsx |
0% | 0% | 0% | 0% |
Test suite run success
118 tests passing in 13 suites.
Report generated by 🧪jest coverage report action from 7742549
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enhances the error handling and notification system for virtual folder operations, specifically improving the user experience when attempting to delete a folder that is currently in use. The key improvement is showing users which sessions are mounted to the folder, allowing them to navigate directly to those sessions.
Key changes:
- Extended notification system to support React nodes in
extraDescriptionfield for richer content display - Added new reusable notification components for virtual folders and background tasks
- Enhanced error handling to parse and display mounted session information when folder deletion fails
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
react/src/hooks/useBAINotification.tsx |
Changed extraDescription type from string to ReactNode to support richer notification content; added 'use memo' directive |
react/src/components/VFolderNodes.tsx |
Enhanced error handling to parse mounted session IDs and display them as clickable links in notifications |
react/src/components/BAIVirtualFolderNodeNotificationItem.tsx |
New component for displaying virtual folder-specific notifications with folder details and navigation |
react/src/components/BAINotificationBackgroundItem.tsx |
New reusable component extracted for displaying background task progress indicators |
react/src/components/BAINodeNotificationItem.tsx |
Updated to support VirtualFolderNode notifications using the new notification item component |
react/src/components/BAIGeneralNotificationItem.tsx |
Refactored to use new BAINotificationBackgroundItem component and support ReactNode in extraDescription |
resources/i18n/*.json |
Added translations for "MountedSessions" and "Folder" keys across all supported languages |
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
ab031bd to
c6bef5a
Compare
c6bef5a to
55d04bf
Compare
agatha197
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
55d04bf to
7742549
Compare
agatha197
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
nowgnuesLee
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM


resolves (FR-1296)
Improve vfolder notification and error handling
This PR adds support for virtual folder notifications in the UI, allowing users to see detailed information about folder operations and errors.
Key changes:
BAIVirtualFolderNodeNotificationprovides a common interface for notifications targeting vfolders. Currently, actions for the vfolder explorer are not included; only the message, description, progress, and extraDescription features are provided.BAIVirtualFolderNodeNotificationItemcomponent to display folder-specific notificationsBAINotificationBackgroundItemcomponent to show progress indicatorsextraDescriptionChecklist: