-
Notifications
You must be signed in to change notification settings - Fork 78
feat(FR-1655): migrate SFTP server launch feature to React #4610
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
Conversation
Coverage report for
|
St.❔ |
Category | Percentage | Covered / Total |
|---|---|---|---|
| 🔴 | Statements | 4.69% | 547/11664 |
| 🔴 | Branches | 3.82% | 314/8215 |
| 🔴 | Functions | 2.91% | 104/3577 |
| 🔴 | Lines | 4.64% | 529/11405 |
Test suite run success
125 tests passing in 14 suites.
Report generated by 🧪jest coverage report action from 971d534
eeeeac7 to
e601415
Compare
c6f460a to
1432a1d
Compare
1432a1d to
5c9adde
Compare
e601415 to
113d240
Compare
5c9adde to
22d8d15
Compare
22d8d15 to
3400bf9
Compare
113d240 to
4c0bf66
Compare
3400bf9 to
6322cdf
Compare
6322cdf to
76f6aca
Compare
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 migrates the SFTP server launch functionality from Lit-based components to React, aligning with the modern architecture patterns in the codebase. The migration introduces proper permission checking, validation for SFTP scaling groups, and enhanced error handling.
Key Changes
- Created a new
SFTPServerButtonReact component with comprehensive permission and resource validation - Consolidated file browser and SFTP image hooks into a single
useDefaultImagesWithFallback.tsfile - Added three new i18n keys across 21 language files for permission-related error messages
Reviewed Changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
react/src/components/SFTPServerButton.tsx |
New React component implementing SFTP session launch with permission checks and validation |
react/src/components/FolderExplorerHeader.tsx |
Updated to use new SFTPServerButton component with Suspense wrapper |
react/src/components/FileBrowserButton.tsx |
Enhanced with permission checking for volume access |
react/src/hooks/useDefaultImagesWithFallback.ts |
Consolidated file browser and SSH/SFTP image hooks |
react/src/hooks/useDefaultFileBrowserImageWithFallback.ts |
Removed (functionality moved to consolidated hook) |
react/src/hooks/useStartSession.tsx |
Improved session creation handling with notification for existing sessions |
react/src/pages/SessionLauncherPage.tsx |
Added 'system' session type to interface |
resources/i18n/*.json |
Added three new translation keys for permission error messages |
Comments suppressed due to low confidence (3)
react/src/components/SFTPServerButton.tsx:33
- Unknown directive: 'use memo'.
'use memo';
react/src/hooks/useDefaultImagesWithFallback.ts:50
- Unknown directive: 'use memo'.
'use memo';
react/src/hooks/useDefaultImagesWithFallback.ts:97
- Unknown directive: 'use memo'.
'use memo';
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
76f6aca to
52e13e4
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.
52e13e4 to
4f4fff8
Compare
yomybaby
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
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.
Please re-request this PR after resolving this issue.
https://app.graphite.com/github/pr/lablup/backend.ai-webui/4610/feat(FR-1655)-migrate-SFTP-server-launch-feature-to-React#review-PRR_kwDOCRTcws7NyM8O
4f4fff8 to
75e5bb8
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
75e5bb8 to
5e23245
Compare
Merge activity
|
Resolves [FR-1655](https://lablup.atlassian.net/browse/FR-1655) ## Summary Migrates the SFTP/SSH server launch functionality from Lit-based components to React, following the modern architecture patterns established in the codebase. ## Changes - Extracted `FolderExplorerHeaderActions` as a new React component containing both File Browser and SFTP launch actions - Migrated SFTP session creation logic to use the `useStartSession` hook for consistency - Implemented comprehensive permission checking for SFTP volume access - Added validation for SFTP scaling groups by current project - Enhanced error handling with clear user feedback when SFTP is unavailable - Updated all 21 language files with new permission error messages ## Technical Details - Uses React Suspense and lazy loading for better performance - Leverages Relay for GraphQL data fetching - Follows the established pattern of using `useStartSession` for session creation - Maintains backward compatibility with existing functionality ## Testing The process for determining whether an SFTP session can be executed is as follows: 1. Determine whether the current project has a host that supports SFTP and is accessible. 2. If a host supporting SFTP exists, determine whether the user has access permissions for that host. 3. If access permissions are also present, determine whether an image supporting SFTP exists. - [ ] File browser launch functionality works as before - [ ] SFTP session creation works with proper permissions - [ ] Error messages display correctly when SFTP is unavailable - [ ] Permission checks properly validate volume access - [ ] All translations are properly loaded [FR-1655]: https://lablup.atlassian.net/browse/FR-1655?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
5e23245 to
971d534
Compare


Resolves FR-1655
Summary
Migrates the SFTP/SSH server launch functionality from Lit-based components to React, following the modern architecture patterns established in the codebase.
Changes
FolderExplorerHeaderActionsas a new React component containing both File Browser and SFTP launch actionsuseStartSessionhook for consistencyTechnical Details
useStartSessionfor session creationTesting
The process for determining whether an SFTP session can be executed is as follows: