Reimplement Drag & Drop for file uploading 🎉 #1858
Conversation
📝 WalkthroughWalkthroughAdds a Blade/Alpine drag-and-drop upload UI and a full-screen upload component; extends the Filament ListFiles page with view wiring, JWT upload URL/size endpoints, folder-creation and upload-logging methods; adds upload translations and bumps Vite devDependency. Changes
Sequence Diagram(s)%%{init: {"themeVariables":{"actorBackground":"#F3F8FF","sequenceNumber":"#666"}}}%%
sequenceDiagram
actor User
participant UI as Upload UI (Blade + Alpine)
participant Livewire as ListFiles (Livewire)
participant Repo as DaemonFileRepository
participant Auth as Authorization
User->>UI: Drag & drop files/folders
UI->>Livewire: getUploadSizeLimit()
Livewire-->>UI: size limit
loop ensure folders exist
UI->>Livewire: createFolder(path)
Livewire->>Auth: authorize create-directory
alt authorized
Livewire->>Repo: createDirectory(path)
Repo-->>Livewire: success / exists
Livewire-->>UI: ack / error
else unauthorized
Livewire-->>UI: 403
end
end
UI->>Livewire: getUploadUrl()
Livewire->>Auth: authorize file:create
alt authorized
Livewire-->>UI: signed JWT upload URL
else unauthorized
Livewire-->>UI: 403
end
loop per file (concurrent up to 3)
UI->>UI: start XHR upload (JWT + directory)
UI-->>Livewire: upload response (success/error)
UI->>UI: update progress/status
end
alt all succeeded
UI->>User: success notification
UI->>Livewire: logUploadedFiles(files)
else partial/failed
UI->>User: partial/failure notification
UI->>Livewire: logUploadedFiles(files)
end
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/Filament/Server/Resources/Files/Pages/ListFiles.php(4 hunks)lang/en/server/file.php(1 hunks)resources/views/filament/server/pages/list-files.blade.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/Filament/Server/Resources/Files/Pages/ListFiles.php (8)
app/Services/Nodes/NodeJWTService.php (4)
NodeJWTService(16-100)setExpiresAt(50-55)setUser(43-48)setClaims(32-37)app/Repositories/Daemon/DaemonFileRepository.php (2)
DaemonFileRepository(12-248)createDirectory(86-100)app/helpers.php (1)
user(127-130)app/Models/Permission.php (1)
Permission(11-221)app/Models/Node.php (1)
getConnectionAddress(178-181)app/Facades/Activity.php (1)
Activity(8-14)app/Services/Activity/ActivityLogService.php (3)
event(48-53)property(109-117)log(136-155)app/Exceptions/Repository/FileExistsException.php (1)
FileExistsException(7-7)
🔇 Additional comments (1)
resources/views/filament/server/pages/list-files.blade.php (1)
198-204: Align failure notifications with defined translations.The notifications reference
server/file.actions.upload.error_all/error_partial, which are not defined. Until the translation file gains those keys, these will surface raw translation paths to users. Please either add the missing language strings (see translation file comment) or point these notifications at the newly addedfailedkey once it exists.⛔ Skipped due to learnings
Learnt from: notAreYouScared Repo: pelican-dev/panel PR: 1671 File: lang/en/server/database.php:22-25 Timestamp: 2025-09-06T20:20:48.706Z Learning: In the pelican-dev/panel project, translation workflows for non-English languages are handled in separate PRs and processes, so missing translations in other locales should be ignored during code reviews.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/Filament/Server/Resources/Files/Pages/ListFiles.php (1)
670-695: Nested folder paths still need to be split before callingcreateDirectory.This issue was flagged in a previous review but hasn't been addressed. When
$folderPathcontains nested paths (e.g.,photos/2023), passing it directly tocreateDirectorywill fail because the daemon expects only the folder basename as the first parameter, not a path with slashes.The previous review suggested splitting the path into its parent segments and basename. For example:
try { - $this->getDaemonFileRepository()->createDirectory($folderPath, $this->path); + $fullPath = trim($folderPath, '/'); + $segments = array_values(array_filter(explode('/', $fullPath), static fn (string $segment): bool => $segment !== '')); + $name = array_pop($segments); + + if ($name === null) { + return; + } + + $targetPath = $this->path; + + if (!empty($segments)) { + $targetPath = join_paths($this->path, implode('/', $segments)); + } + + $this->getDaemonFileRepository()->createDirectory($name, $targetPath); Activity::event('server:file.create-directory') - ->property(['directory' => $this->path, 'name' => $folderPath]) + ->property(['directory' => $targetPath, 'name' => $name]) ->log(); } catch (FileExistsException) {
🧹 Nitpick comments (1)
app/Filament/Server/Resources/Files/Pages/ListFiles.php (1)
665-694: Update exception handling to match method signature.The method signature declares
@throws ConnectionException, but the exception is caught and not rethrown (Lines 688-694). This creates an inconsistency between the documented behavior and actual implementation.Choose one of the following approaches:
Option 1: Remove the
@throws ConnectionExceptionannotation if catching and showing a notification is the desired behavior:/** - * @throws ConnectionException * @throws FileExistsException * @throws \Throwable */Option 2: Rethrow the exception after sending the notification if callers need to handle it:
} catch (ConnectionException $e) { Notification::make() ->body($e->getMessage()) ->danger() ->send(); + + throw $e; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/Filament/Server/Resources/Files/Pages/ListFiles.php(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/Filament/Server/Resources/Files/Pages/ListFiles.php (8)
app/Services/Nodes/NodeJWTService.php (4)
NodeJWTService(16-100)setExpiresAt(50-55)setUser(43-48)setClaims(32-37)app/helpers.php (1)
user(127-130)app/Models/Node.php (1)
getConnectionAddress(178-181)app/Filament/Server/Resources/Files/Pages/EditFiles.php (1)
getDaemonFileRepository(270-277)app/Repositories/Daemon/DaemonFileRepository.php (2)
createDirectory(86-100)DaemonFileRepository(12-248)app/Facades/Activity.php (1)
Activity(8-14)app/Services/Activity/ActivityLogService.php (3)
event(48-53)property(109-117)log(136-155)app/Exceptions/Repository/FileExistsException.php (1)
FileExistsException(7-7)
🔇 Additional comments (1)
app/Filament/Server/Resources/Files/Pages/ListFiles.php (1)
657-663: LGTM!The conversion from MiB to bytes is correct, and the implementation is straightforward.
|
@notAreYouScared This won't fix the upload issue on the panel (#1855) |
Referring to our previous discussion in discord, im well aware. The only reason it still exists is due to me nlt having a solution for mobile users yet. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
resources/views/filament/server/pages/list-files.blade.php (1)
175-192: Fix the concurrency limiter cleanup
Promiseobjects never expose.status, soactiveUploads.filter(p => p.status …)never removes settled uploads. After the firstPromise.racethe array keeps the resolved promises and the limiter stops working, letting every remaining upload start at once. Wrap each upload promise in afinallythat removes itself fromactiveUploadsso the cap actually holds.- for (let i = 0; i < this.uploadQueue.length; i++) { - const uploadPromise = this.uploadFile(i) - .then(() => { completedCount++; this.currentFileIndex = completedCount; }) - .catch(() => { completedCount++; this.currentFileIndex = completedCount; }); - - activeUploads.push(uploadPromise); - - if (activeUploads.length >= maxConcurrent) { - await Promise.race(activeUploads); - activeUploads = activeUploads.filter(p => p.status !== 'fulfilled' && p.status !== 'rejected'); - } - } - - await Promise.allSettled(activeUploads); + for (let i = 0; i < this.uploadQueue.length; i++) { + const uploadPromise = this.uploadFile(i) + .then(() => { completedCount++; this.currentFileIndex = completedCount; }) + .catch(() => { completedCount++; this.currentFileIndex = completedCount; }); + + const trackedPromise = uploadPromise.finally(() => { + activeUploads = activeUploads.filter(p => p !== trackedPromise); + }); + + activeUploads.push(trackedPromise); + + if (activeUploads.length >= maxConcurrent) { + await Promise.race(activeUploads); + } + } + + await Promise.allSettled(activeUploads);app/Filament/Server/Resources/Files/Pages/ListFiles.php (1)
661-666: Split nested folder paths before calling the daemon
DaemonFileRepository::createDirectory()expects a folder name and its parent path separately. Passing"foo/bar"as$nameyields a 400 from the daemon, so uploading directories with nesting always fails. Normalize the incoming relative path, take the basename as the name, and join the remaining segments onto$this->pathbefore delegating.- try { - $this->getDaemonFileRepository()->createDirectory($folderPath, $this->path); - - Activity::event('server:file.create-directory') - ->property(['directory' => $this->path, 'name' => $folderPath]) - ->log(); + $fullPath = trim($folderPath, '/'); + + if ($fullPath === '') { + return; + } + + $segments = array_values(array_filter(explode('/', $fullPath), static fn (string $segment): bool => $segment !== '')); + $name = array_pop($segments); + + if ($name === null) { + return; + } + + $targetPath = $this->path; + + if ($segments !== []) { + $targetPath = join_paths($this->path, implode('/', $segments)); + } + + try { + $this->getDaemonFileRepository()->createDirectory($name, $targetPath); + + Activity::event('server:file.create-directory') + ->property(['directory' => $targetPath, 'name' => $name]) + ->log();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/Filament/Server/Resources/Files/Pages/ListFiles.php(6 hunks)resources/views/filament/server/pages/file-upload.blade.php(1 hunks)resources/views/filament/server/pages/list-files.blade.php(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-15T11:55:53.461Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1801
File: app/Extensions/OAuth/Schemas/AuthentikSchema.php:7-10
Timestamp: 2025-10-15T11:55:53.461Z
Learning: In Filament v4, the Forms component Placeholder was deprecated and removed. Use TextEntry from Filament\Infolists\Components\TextEntry in forms instead, binding values with ->state(). For HTML content, use ->html().
Applied to files:
app/Filament/Server/Resources/Files/Pages/ListFiles.php
📚 Learning: 2025-10-15T11:55:53.461Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1801
File: app/Extensions/OAuth/Schemas/AuthentikSchema.php:7-10
Timestamp: 2025-10-15T11:55:53.461Z
Learning: In Filament v4, Wizard Step components use the Filament\Schemas namespace (Filament\Schemas\Components\Wizard\Step), not Filament\Forms.
Applied to files:
app/Filament/Server/Resources/Files/Pages/ListFiles.php
🧬 Code graph analysis (1)
app/Filament/Server/Resources/Files/Pages/ListFiles.php (6)
app/Services/Nodes/NodeJWTService.php (4)
NodeJWTService(16-100)setExpiresAt(50-55)setUser(43-48)setClaims(32-37)app/helpers.php (1)
user(127-130)app/Models/Permission.php (1)
Permission(11-221)app/Models/Node.php (1)
getConnectionAddress(178-181)app/Repositories/Daemon/DaemonFileRepository.php (1)
createDirectory(86-100)app/Exceptions/Repository/FileExistsException.php (1)
FileExistsException(7-7)
|
Drag and drop is now working only when you're inside the upload button. Not by default on the ListFiles view. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
app/Filament/Server/Resources/Files/Pages/ListFiles.php (1)
655-680: Nested folder paths must be split before callingcreateDirectory.Line 665 passes
$folderPathdirectly as the$nameparameter, butDaemonFileRepository::createDirectory(string $name, string $path)expects a single folder name, not a nested path. When uploading nested directories (e.g.,photos/2023), the daemon will reject this with a validation error.Split the incoming
$folderPathinto parent segments and basename, then create each level recursively or pass the correct parent path and folder name.resources/views/filament/server/pages/list-files.blade.php (1)
175-193: Fix the concurrency limiter—settled promises never vacate their slots.Line 188 attempts to filter promises by checking a
.statusproperty that doesn't exist on Promise objects. As a result,activeUploadsnever shrinks, and the concurrency limit breaks after the first batch. Wrap each upload promise with a.finally()hook that removes itself from the array once settled.
🧹 Nitpick comments (1)
resources/views/filament/server/pages/list-files.blade.php (1)
217-281: Consider reusing upload tokens across files to reduce server round-trips.Line 221 calls
getUploadUrl()for every file, generating a new JWT token each time. For large folder uploads (hundreds of files), this creates unnecessary server requests. Since tokens have a 15-minute expiry, you could fetch one token and reuse it across all files in the batch, refreshing only when needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/Filament/Server/Resources/Files/Pages/ListFiles.php(6 hunks)resources/views/filament/server/pages/list-files.blade.php(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-15T11:55:53.461Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1801
File: app/Extensions/OAuth/Schemas/AuthentikSchema.php:7-10
Timestamp: 2025-10-15T11:55:53.461Z
Learning: In Filament v4, the Forms component Placeholder was deprecated and removed. Use TextEntry from Filament\Infolists\Components\TextEntry in forms instead, binding values with ->state(). For HTML content, use ->html().
Applied to files:
app/Filament/Server/Resources/Files/Pages/ListFiles.php
📚 Learning: 2025-10-15T11:55:53.461Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1801
File: app/Extensions/OAuth/Schemas/AuthentikSchema.php:7-10
Timestamp: 2025-10-15T11:55:53.461Z
Learning: In Filament v4, Wizard Step components use the Filament\Schemas namespace (Filament\Schemas\Components\Wizard\Step), not Filament\Forms.
Applied to files:
app/Filament/Server/Resources/Files/Pages/ListFiles.php
🧬 Code graph analysis (1)
app/Filament/Server/Resources/Files/Pages/ListFiles.php (5)
app/Services/Nodes/NodeJWTService.php (4)
NodeJWTService(16-100)setExpiresAt(50-55)setUser(43-48)setClaims(32-37)app/helpers.php (1)
user(127-130)app/Models/Permission.php (1)
Permission(11-221)app/Repositories/Daemon/DaemonFileRepository.php (1)
createDirectory(86-100)app/Exceptions/Repository/FileExistsException.php (1)
FileExistsException(7-7)
🔇 Additional comments (4)
app/Filament/Server/Resources/Files/Pages/ListFiles.php (2)
620-640: LGTM! JWT-based upload URL generation is properly secured.The permission check, token expiry, and URL construction are all correct. The 15-minute expiry provides a reasonable window for uploads while maintaining security.
642-648: LGTM! Upload size limit conversion is correct.The MiB-to-bytes conversion is accurate, and exposing this limit allows client-side validation before upload attempts.
resources/views/filament/server/pages/list-files.blade.php (2)
84-120: LGTM! Recursive directory traversal correctly handles large directories.The continuation loop in
readEntriesproperly handles the fact that entries are returned in batches. The Promise-based recursive approach cleanly collects all nested files while preserving their relative paths.
13-57: LGTM! Drag-and-drop handlers are well-structured.The
dragCounterpattern correctly handles nested drag events, the modal check prevents conflicts, and the feature detection with fallback ensures broad compatibility while supporting folder uploads where available.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
resources/views/filament/server/pages/list-files.blade.php (2)
149-155: Folder creation failures are silently ignored, leading to subsequent upload failures.When
createFolderfails (e.g., for nested paths likephotos/2023), Line 153 only logs a warning and continues. Files targeting non-existent folders will then fail during upload with unclear error messages. Surface folder-creation failures more prominently or halt the upload workflow if critical folders can't be created.
171-189: Fix the concurrency limiter so settled uploads vacate their slots.
Promiseinstances never expose a.statusfield, so Line 184's filter never removes settled promises fromactiveUploads. After the first race resolves, the array keeps growing and themaxConcurrentcap becomes ineffective. Wrap each upload promise with afinallyhook that removes itself once settled.Apply this diff to fix the concurrency limiter:
- for (let i = 0; i < this.uploadQueue.length; i++) { - const uploadPromise = this.uploadFile(i) - .then(() => { completedCount++; this.currentFileIndex = completedCount; }) - .catch(() => { completedCount++; this.currentFileIndex = completedCount; }); - - activeUploads.push(uploadPromise); - - if (activeUploads.length >= maxConcurrent) { - await Promise.race(activeUploads); - activeUploads = activeUploads.filter(p => p.status !== 'fulfilled' && p.status !== 'rejected'); - } - } - - await Promise.allSettled(activeUploads); + for (let i = 0; i < this.uploadQueue.length; i++) { + const uploadPromise = this.uploadFile(i) + .then(() => { completedCount++; this.currentFileIndex = completedCount; }) + .catch(() => { completedCount++; this.currentFileIndex = completedCount; }); + + const trackedPromise = uploadPromise.finally(() => { + activeUploads = activeUploads.filter(p => p !== trackedPromise); + }); + + activeUploads.push(trackedPromise); + + if (activeUploads.length >= maxConcurrent) { + await Promise.race(activeUploads); + } + } + + await Promise.allSettled(activeUploads);app/Filament/Server/Resources/Files/Pages/ListFiles.php (1)
652-677: Split nested folder paths before callingcreateDirectory.
DaemonFileRepository::createDirectory()expects the folder name and its parent path as separate arguments (see snippet showing signaturecreateDirectory(string $name, string $path)). Passing a nested path likephotos/2023as$folderPathresults in daemon-side validation errors, so nested directory uploads fail. Split the incoming relative path into parent segments and basename before delegating to the repository.Apply this diff to properly handle nested folder paths:
+ $fullPath = trim($folderPath, '/'); + $segments = array_values(array_filter(explode('/', $fullPath), static fn (string $segment): bool => $segment !== '')); + $name = array_pop($segments); + + if ($name === null) { + return; + } + + $targetPath = $this->path; + + if (!empty($segments)) { + $targetPath = join_paths($this->path, implode('/', $segments)); + } + try { - $this->getDaemonFileRepository()->createDirectory($folderPath, $this->path); + $this->getDaemonFileRepository()->createDirectory($name, $targetPath); Activity::event('server:file.create-directory') - ->property(['directory' => $this->path, 'name' => $folderPath]) + ->property(['directory' => $targetPath, 'name' => $name]) ->log();
🧹 Nitpick comments (1)
resources/views/filament/server/pages/list-files.blade.php (1)
190-211: LGTM!The completion handling correctly categorizes outcomes (all success, partial failure, all failure) and triggers appropriate notifications. The auto-close timer is properly cleaned up.
Consider increasing the auto-close delay from 1 second to 2-3 seconds to give users more time to review the upload results, especially for large batches:
- this.autoCloseTimer = setTimeout(() => { - this.isUploading = false; - this.uploadQueue = []; - },1000); + this.autoCloseTimer = setTimeout(() => { + this.isUploading = false; + this.uploadQueue = []; + }, 2000);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/Filament/Server/Resources/Files/Pages/ListFiles.php(6 hunks)resources/views/filament/server/pages/file-upload.blade.php(1 hunks)resources/views/filament/server/pages/list-files.blade.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- resources/views/filament/server/pages/file-upload.blade.php
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-15T11:55:53.461Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1801
File: app/Extensions/OAuth/Schemas/AuthentikSchema.php:7-10
Timestamp: 2025-10-15T11:55:53.461Z
Learning: In Filament v4, the Forms component Placeholder was deprecated and removed. Use TextEntry from Filament\Infolists\Components\TextEntry in forms instead, binding values with ->state(). For HTML content, use ->html().
Applied to files:
app/Filament/Server/Resources/Files/Pages/ListFiles.php
📚 Learning: 2025-10-15T11:55:53.461Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1801
File: app/Extensions/OAuth/Schemas/AuthentikSchema.php:7-10
Timestamp: 2025-10-15T11:55:53.461Z
Learning: In Filament v4, Wizard Step components use the Filament\Schemas namespace (Filament\Schemas\Components\Wizard\Step), not Filament\Forms.
Applied to files:
app/Filament/Server/Resources/Files/Pages/ListFiles.php
🧬 Code graph analysis (1)
app/Filament/Server/Resources/Files/Pages/ListFiles.php (7)
app/Services/Nodes/NodeJWTService.php (4)
NodeJWTService(16-100)setExpiresAt(50-55)setUser(43-48)setClaims(32-37)app/helpers.php (1)
user(127-130)app/Models/Permission.php (1)
Permission(11-221)app/Models/Node.php (1)
getConnectionAddress(178-181)app/Repositories/Daemon/DaemonFileRepository.php (1)
createDirectory(86-100)app/Services/Activity/ActivityLogService.php (3)
event(48-53)property(109-117)log(136-155)app/Exceptions/Repository/FileExistsException.php (1)
FileExistsException(7-7)
🔇 Additional comments (9)
resources/views/filament/server/pages/list-files.blade.php (5)
13-28: LGTM!The drag event handlers correctly use a counter-based approach to manage nested drag enter/leave events, preventing premature dismissal of the drag overlay when hovering over child elements.
29-53: LGTM!The drop handler correctly prioritizes the FileSystem Access API (
webkitGetAsEntry) for folder support, with a fallback to the standardFileListfor browsers that don't support it. This progressive enhancement approach ensures broad compatibility.
55-78: LGTM!The file extraction logic properly handles both file and directory entries with appropriate fallbacks, and uses
Promise.allto ensure all asynchronous traversals complete before proceeding.
80-116: LGTM!The recursive directory traversal correctly implements the
readEntries()loop pattern (required for handling directories with many entries) and properly builds nested paths for uploaded files.
213-277: LGTM!The upload implementation correctly uses
XMLHttpRequest(rather thanfetch) to enable upload progress tracking, properly constructs the upload path, and handles both success and error cases. The speed calculation throttling (Line 242) prevents excessive updates.app/Filament/Server/Resources/Files/Pages/ListFiles.php (4)
15-15: LGTM!The new imports and view property binding are appropriate for the drag-and-drop upload functionality. The custom view replaces the default Filament list page template to enable the Alpine.js-powered upload UI.
Also applies to: 18-18, 41-41, 48-48, 61-61
617-637: LGTM!The upload URL generation correctly enforces permissions, creates short-lived JWT tokens (15-minute expiry), and constructs URLs pointing directly to the Wings node. This enables the client-side code to upload directly to Wings, bypassing Panel and avoiding PHP memory limits.
Ensure your deployment uses HTTPS for Panel-to-Wings communication, as the JWT token is transmitted in the URL query string. This is a deployment requirement rather than a code issue.
639-645: LGTM!The size limit retrieval correctly converts the node's upload size from MiB to bytes for client-side validation.
541-541: LGTM!The upload action changes correctly reflect the new architecture where file uploads are handled via drag-and-drop in the main view, while the Upload button modal now primarily serves URL-based imports. The "files" tab now uses a custom View component instead of the built-in FileUpload.
Also applies to: 559-559
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
resources/views/filament/server/pages/list-files.blade.php (3)
149-155: Folder creation failures should halt the upload with a clear error message.When
createFolderfails (line 151), the code logs a warning but continues. If folders cannot be created due to permission issues or other errors, subsequent file uploads targeting those folders will fail with less clear error messages, confusing users.
171-189: Critical: Concurrency limiter never frees slots because Promises lack a.statusproperty.Line 184 filters promises using
.status, but JavaScript Promise objects don't expose this property. TheactiveUploadsarray never shrinks, so after the firstPromise.raceresolves, all promises remain in the array and themaxConcurrentlimit stops working.
213-277: Escape key cancellation doesn't abort in-flight XHR uploads.The XHR instance created at line 228 is not stored in
fileData, so when the user presses Escape (handled at lines 289-294), the UI clears but active uploads continue in the background. This can cause unexpected server load and user confusion.
🧹 Nitpick comments (2)
lang/en/server/file.php (1)
20-24: Consider adding differentiated failure messages for partial vs. complete upload failures.The Blade view (lines 193-199) uses the same
failedtranslation key whether some files fail or all files fail, making it unclear to users what happened. Addingerror_partialanderror_allkeys would improve the user experience.Add differentiated keys to provide clearer feedback:
'drop_files' => 'Drop files to upload', 'success' => 'Files uploaded successfully', 'failed' => 'Failed to upload files', + 'error_partial' => 'Some files failed to upload', + 'error_all' => 'All files failed to upload', 'header' => 'Uploading Files', 'error' => 'An error occurred while uploading',Then update the Blade file to use them:
// In list-files.blade.php around lines 195-198 } else if (failed.length === this.totalFiles) { new window.FilamentNotification().title('{{ trans('server/file.actions.upload.error_all') }}').danger().send(); } else { new window.FilamentNotification().title('{{ trans('server/file.actions.upload.error_partial') }}').danger().send(); }resources/views/filament/server/pages/list-files.blade.php (1)
332-390: Consider adding accessibility features to the upload modal.The upload progress modal (lines 332-390) lacks accessibility features:
- No focus trap to prevent keyboard users from tabbing outside the modal
- Missing
role="dialog"andaria-modal="true"attributes- No
aria-labeloraria-labelledbyfor screen readers- Escape key closes the modal but doesn't announce the action
Consider enhancing accessibility:
<div x-show="isUploading" x-cloak + role="dialog" + aria-modal="true" + aria-labelledby="upload-modal-title" class="fixed inset-0 z-50 flex items-center justify-center bg-gray-900/50 dark:bg-gray-100/20 p-4" > <div class="rounded-lg bg-white shadow-xl dark:bg-gray-800 max-w-[50%] max-h-[50vh] overflow-hidden flex flex-col"> <div class="px-6 py-4 border-b border-gray-200 dark:border-gray-700 flex items-center justify-center"> - <h3 class="text-lg font-semibold text-gray-900 dark:text-gray-100"> + <h3 id="upload-modal-title" class="text-lg font-semibold text-gray-900 dark:text-gray-100">Additionally, consider implementing focus trapping using Alpine.js's
x-trapdirective or a similar solution.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lang/en/server/file.php(1 hunks)resources/views/filament/server/pages/list-files.blade.php(1 hunks)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
app/Filament/Server/Resources/Files/Pages/ListFiles.php (1)
652-677: Split nested folder paths before callingcreateDirectory.The same issue identified in the previous review remains:
DaemonFileRepository::createDirectory()expects a single folder name and its parent path separately. Passing a nested path likephotos/2023as$folderPathwill cause daemon-side validation errors, preventing nested directory uploads from working.Break the incoming relative path into parent segments and basename before delegating to the repository, as suggested in the previous review.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/Filament/Server/Resources/Files/Pages/ListFiles.php(6 hunks)resources/views/filament/server/pages/file-upload.blade.php(1 hunks)resources/views/filament/server/pages/list-files.blade.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- resources/views/filament/server/pages/list-files.blade.php
- resources/views/filament/server/pages/file-upload.blade.php
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-15T11:55:53.461Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1801
File: app/Extensions/OAuth/Schemas/AuthentikSchema.php:7-10
Timestamp: 2025-10-15T11:55:53.461Z
Learning: In Filament v4, the Forms component Placeholder was deprecated and removed. Use TextEntry from Filament\Infolists\Components\TextEntry in forms instead, binding values with ->state(). For HTML content, use ->html().
Applied to files:
app/Filament/Server/Resources/Files/Pages/ListFiles.php
📚 Learning: 2025-10-15T11:55:53.461Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1801
File: app/Extensions/OAuth/Schemas/AuthentikSchema.php:7-10
Timestamp: 2025-10-15T11:55:53.461Z
Learning: In Filament v4, Wizard Step components use the Filament\Schemas namespace (Filament\Schemas\Components\Wizard\Step), not Filament\Forms.
Applied to files:
app/Filament/Server/Resources/Files/Pages/ListFiles.php
🧬 Code graph analysis (1)
app/Filament/Server/Resources/Files/Pages/ListFiles.php (7)
app/Services/Nodes/NodeJWTService.php (4)
NodeJWTService(16-100)setExpiresAt(50-55)setUser(43-48)setClaims(32-37)app/helpers.php (1)
user(127-130)app/Models/Node.php (1)
getConnectionAddress(178-181)app/Filament/Server/Resources/Files/Pages/EditFiles.php (1)
getDaemonFileRepository(270-277)app/Repositories/Daemon/DaemonFileRepository.php (1)
createDirectory(86-100)app/Services/Activity/ActivityLogService.php (3)
event(48-53)property(109-117)log(136-155)app/Exceptions/Repository/FileExistsException.php (1)
FileExistsException(7-7)
🔇 Additional comments (5)
app/Filament/Server/Resources/Files/Pages/ListFiles.php (5)
15-15: LGTM!The new imports support the direct-to-Wings upload functionality and are all utilized in the new methods.
Also applies to: 18-18, 41-41, 48-48
61-61: LGTM!Setting a custom view is the correct approach to integrate the drag-and-drop upload component into the file list page.
541-541: LGTM!Transitioning from server-side FileUpload to a View-based component correctly implements direct-to-Wings uploads, bypassing Panel as a file relay.
Also applies to: 559-559
617-637: LGTM!The JWT token generation for direct uploads includes proper permission checks, follows established codebase patterns for token identifiers, and correctly constructs the Wings upload URL.
639-645: LGTM!The upload size limit conversion from megabytes to bytes is correct and will properly enforce node-level upload constraints.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
resources/views/filament/server/pages/list-files.blade.php (1)
181-208: Fix the concurrency limiter so settled uploads vacate their slots.
Promiseinstances never expose a.statusfield, so line 203's filter never removes settled promises fromactiveUploads. After the first 3 uploads start, the array keeps all promises, effectively breaking the intended concurrent upload limit.Apply this fix to properly track and remove settled promises:
- for (let i = 0; i < this.uploadQueue.length; i++) { - const uploadPromise = this.uploadFile(i) - .then(() => { - completedCount++; - this.currentFileIndex = completedCount; - const item = this.uploadQueue[i]; - const relativePath = (item.path ? item.path.replace(/^\/+/, '') + '/' : '') + item.name; - uploadedFiles.push(relativePath); - }) - .catch(() => { - completedCount++; - this.currentFileIndex = completedCount; - }); - - activeUploads.push(uploadPromise); - - if (activeUploads.length >= maxConcurrent) { - await Promise.race(activeUploads); - activeUploads = activeUploads.filter(p => p.status !== 'fulfilled' && p.status !== 'rejected'); - } - } - - await Promise.allSettled(activeUploads); + for (let i = 0; i < this.uploadQueue.length; i++) { + const uploadPromise = this.uploadFile(i) + .then(() => { + completedCount++; + this.currentFileIndex = completedCount; + const item = this.uploadQueue[i]; + const relativePath = (item.path ? item.path.replace(/^\/+/, '') + '/' : '') + item.name; + uploadedFiles.push(relativePath); + }) + .catch(() => { + completedCount++; + this.currentFileIndex = completedCount; + }); + + const trackedPromise = uploadPromise.finally(() => { + activeUploads = activeUploads.filter(p => p !== trackedPromise); + }); + + activeUploads.push(trackedPromise); + + if (activeUploads.length >= maxConcurrent) { + await Promise.race(activeUploads); + } + } + + await Promise.allSettled(activeUploads);resources/views/filament/server/pages/file-upload.blade.php (1)
132-158: Fix the concurrency limiter so settled uploads vacate their slots.
Promiseinstances never expose a.statusfield, so line 154's filter never removes settled promises fromactiveUploads. This breaks the intended 3-concurrent-upload limit, causing uploads to effectively serialize after the first 3 start.Apply this fix:
- for (let i = 0; i < this.uploadQueue.length; i++) { - const uploadPromise = this.uploadFile(i) - .then(() => { - completedCount++; - this.currentFileIndex = completedCount; - const item = this.uploadQueue[i]; - const relativePath = (item.path ? item.path.replace(/^\/+/, '') + '/' : '') + item.name; - uploadedFiles.push(relativePath); - }) - .catch(() => { - completedCount++; - this.currentFileIndex = completedCount; - }); - - activeUploads.push(uploadPromise); - - if (activeUploads.length >= maxConcurrent) { - await Promise.race(activeUploads); - activeUploads = activeUploads.filter(p => p.status !== 'fulfilled' && p.status !== 'rejected'); - } - } - - await Promise.allSettled(activeUploads); + for (let i = 0; i < this.uploadQueue.length; i++) { + const uploadPromise = this.uploadFile(i) + .then(() => { + completedCount++; + this.currentFileIndex = completedCount; + const item = this.uploadQueue[i]; + const relativePath = (item.path ? item.path.replace(/^\/+/, '') + '/' : '') + item.name; + uploadedFiles.push(relativePath); + }) + .catch(() => { + completedCount++; + this.currentFileIndex = completedCount; + }); + + const trackedPromise = uploadPromise.finally(() => { + activeUploads = activeUploads.filter(p => p !== trackedPromise); + }); + + activeUploads.push(trackedPromise); + + if (activeUploads.length >= maxConcurrent) { + await Promise.race(activeUploads); + } + } + + await Promise.allSettled(activeUploads);
🧹 Nitpick comments (3)
resources/views/filament/server/pages/list-files.blade.php (1)
159-165: Consider surfacing folder creation errors to users.When folder creation fails (line 163), only a console warning is logged. If Wings doesn't auto-create parent folders, subsequent file uploads to those paths will fail with potentially unclear error messages, degrading the user experience.
Consider surfacing folder-creation failures or collecting them to show a single aggregated notification if any critical paths can't be created.
resources/views/filament/server/pages/file-upload.blade.php (2)
9-69: Extract duplicated folder traversal logic into a shared module.The
extractFilesFromItemsandtraverseFileTreefunctions (lines 9-69) are duplicated identically inresources/views/filament/server/pages/list-files.blade.php(lines 58-119). This 60+ line duplication makes maintenance harder and increases the risk of inconsistent behavior if one copy is updated but not the other.Consider extracting these functions into a shared JavaScript module or Alpine.js store that both components can import, or create a shared Alpine.js data object that can be reused across components.
71-207: Consider refactoring duplicated upload logic.The entire
uploadFilesWithFoldersmethod (137 lines) is nearly identical to the one inlist-files.blade.php(lines 120-256), differing only in minor error message details. This represents substantial code duplication that makes maintenance harder.Consider extracting this shared upload logic into a reusable Alpine.js component, store, or JavaScript module that both components can use.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
package.json(1 hunks)resources/views/filament/server/pages/file-upload.blade.php(1 hunks)resources/views/filament/server/pages/list-files.blade.php(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 1858
File: resources/views/filament/server/pages/list-files.blade.php:304-309
Timestamp: 2025-11-07T11:30:21.662Z
Learning: In resources/views/filament/server/pages/list-files.blade.php, the handleEscapeKey function intentionally does not abort active XMLHttpRequest uploads when Escape is pressed. The UI is hidden but uploads continue in the background by design.
📚 Learning: 2025-11-07T11:30:21.662Z
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 1858
File: resources/views/filament/server/pages/list-files.blade.php:304-309
Timestamp: 2025-11-07T11:30:21.662Z
Learning: In resources/views/filament/server/pages/list-files.blade.php, the handleEscapeKey function intentionally does not abort active XMLHttpRequest uploads when Escape is pressed. The UI is hidden but uploads continue in the background by design.
Applied to files:
resources/views/filament/server/pages/list-files.blade.phpresources/views/filament/server/pages/file-upload.blade.php
📚 Learning: 2025-11-05T22:21:31.863Z
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 1865
File: app/Filament/Admin/Resources/Nodes/Pages/EditNode.php:682-682
Timestamp: 2025-11-05T22:21:31.863Z
Learning: In app/Filament/Admin/Resources/Nodes/Pages/EditNode.php, the diagnostics tab's upload action intentionally does not use the iconButton() modifier, while the pull action does. This UI difference is intentional.
Applied to files:
resources/views/filament/server/pages/list-files.blade.phpresources/views/filament/server/pages/file-upload.blade.php
📚 Learning: 2025-11-03T23:44:24.737Z
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 1858
File: resources/views/filament/server/pages/list-files.blade.php:338-338
Timestamp: 2025-11-03T23:44:24.737Z
Learning: In the pelican-dev/panel repository, Tailwind CSS v4 is used, which supports fractional utility class syntax such as `max-w-1/2` (equivalent to `max-width: calc(1/2 * 100%)`). This syntax is valid and should not be flagged as invalid.
Applied to files:
resources/views/filament/server/pages/list-files.blade.php
📚 Learning: 2025-10-07T14:44:18.583Z
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 1779
File: app/Filament/Admin/Resources/Users/Pages/EditUser.php:51-51
Timestamp: 2025-10-07T14:44:18.583Z
Learning: In the Pelican Panel codebase, when using Filament's FileUpload component for the avatar field in UserResource, the 'avatar' key is intentionally unset from the data array in EditUser::handleRecordUpdate before passing to UserUpdateService. This is by design because the avatar is not stored in the database directly—Filament's FileUpload component handles file storage, retrieval, and deletion through its own lifecycle hooks (formatStateUsing, deleteUploadedFileUsing, etc.) independently of the database update service.
Applied to files:
resources/views/filament/server/pages/list-files.blade.php
🔇 Additional comments (1)
package.json (1)
17-17: Verify and address Vite 7 breaking changes before merging.Vite 7.1.11 is a valid version with no known security vulnerabilities. However, the upgrade from 6.2.6 introduces multiple breaking changes that require action:
- Node version requirement: Upgrade Node to one of the supported minimums (20.19+ or 22.12+)
- Config changes: Rename build.target → baseline-widely-available and update tooling that reads build.target
- Build changes: Remove splitVendorChunkPlugin usages and switch to alternative chunking strategies
- Sass changes: Update code that depended on the legacy Sass integration; Vite 7 uses the Sass compiler API
- Plugin and SSR updates: Update plugins to versions compatible with Vite 7 and remove experimental.skipSsrTransform and HotBroadcaster-related code
- Test tooling: Upgrade Vitest to a version with Vite 7 support (3.2+ recommended)
Verify your vite.config.js and package.json against these requirements before merging.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (8)
resources/views/filament/server/pages/list-files.blade.php (3)
181-208: Fix the concurrency limiter so settled uploads vacate their slots.
Promiseinstances never expose a.statusfield, so theactiveUploadsarray never shrinks. After the first race resolves you still keep all settled promises, effectively disabling the intendedmaxConcurrentcap.Apply this diff to wrap each upload promise with a
finallyhook that removes itself once settled:- for (let i = 0; i < this.uploadQueue.length; i++) { - const uploadPromise = this.uploadFile(i) - .then(() => { completedCount++; this.currentFileIndex = completedCount; }) - .catch(() => { completedCount++; this.currentFileIndex = completedCount; }); - - activeUploads.push(uploadPromise); - - if (activeUploads.length >= maxConcurrent) { - await Promise.race(activeUploads); - activeUploads = activeUploads.filter(p => p.status !== 'fulfilled' && p.status !== 'rejected'); - } - } - - await Promise.allSettled(activeUploads); + for (let i = 0; i < this.uploadQueue.length; i++) { + const uploadPromise = this.uploadFile(i) + .then(() => { completedCount++; this.currentFileIndex = completedCount; }) + .catch(() => { completedCount++; this.currentFileIndex = completedCount; }); + + const trackedPromise = uploadPromise.finally(() => { + activeUploads = activeUploads.filter(p => p !== trackedPromise); + }); + + activeUploads.push(trackedPromise); + + if (activeUploads.length >= maxConcurrent) { + await Promise.race(activeUploads); + } + } + + await Promise.allSettled(activeUploads);
159-165: Folder creation failures are silently ignored, leading to subsequent upload failures.When
createFolderfails for nested paths (e.g.,photos/2023), Line 163 logs a warning but continues. Files targeting those non-existent folders will then fail during upload with less clear error messages.Consider surfacing folder-creation failures more prominently, or halt the upload workflow if critical folders can't be created.
223-227: Partial failure notification uses the same message as total failure.When some (but not all) uploads fail, the notification on Line 224 displays
'server/file.actions.upload.failed', which is identical to the total failure message (Line 219). This doesn't clearly communicate to users that some files uploaded successfully.Consider using a distinct translation key like
'server/file.actions.upload.partial'or modifying the message to indicate partial success.resources/views/filament/server/pages/file-upload.blade.php (3)
136-158: Restore active upload slot reclamation.Filtering on
p.statusnever removes settled promises, soactiveUploadsonly grows and the three-upload cap disappears after the first completion.Apply this diff to clean up each tracked promise in a
finallyso the limiter keeps working:- for (let i = 0; i < this.uploadQueue.length; i++) { - const uploadPromise = this.uploadFile(i) - .then(() => { - completedCount++; - this.currentFileIndex = completedCount; - const item = this.uploadQueue[i]; - const relativePath = (item.path ? item.path.replace(/^\/+/, '') + '/' : '') + item.name; - uploadedFiles.push(relativePath); - }) - .catch(() => { - completedCount++; - this.currentFileIndex = completedCount; - }); - - activeUploads.push(uploadPromise); - - if (activeUploads.length >= maxConcurrent) { - await Promise.race(activeUploads); - activeUploads = activeUploads.filter(p => p.status !== 'fulfilled' && p.status !== 'rejected'); - } - } - - await Promise.allSettled(activeUploads); + for (let i = 0; i < this.uploadQueue.length; i++) { + const uploadPromise = this.uploadFile(i) + .then(() => { + completedCount++; + this.currentFileIndex = completedCount; + const item = this.uploadQueue[i]; + const relativePath = (item.path ? item.path.replace(/^\/+/, '') + '/' : '') + item.name; + uploadedFiles.push(relativePath); + }) + .catch(() => { + completedCount++; + this.currentFileIndex = completedCount; + }); + + const trackedPromise = uploadPromise.finally(() => { + activeUploads = activeUploads.filter(p => p !== trackedPromise); + }); + + activeUploads.push(trackedPromise); + + if (activeUploads.length >= maxConcurrent) { + await Promise.race(activeUploads); + } + } + + await Promise.allSettled(activeUploads);
110-116: Folder creation failures are silently ignored.Same issue as in
list-files.blade.php: failed folder creation is only logged to console, which may lead to less clear upload errors later.
173-178: Partial failure notification uses the same message as total failure.Identical issue as in
list-files.blade.php: partial success (some files uploaded, some failed) displays the same'server/file.actions.upload.failed'message as total failure.app/Filament/Server/Resources/Files/Pages/ListFiles.php (2)
668-676: Add permission check tologUploadedFilesmethod—currently exploitable from frontend.This public method is callable directly from frontend JavaScript without permission validation. While
getUploadUrlprotects the actual file upload endpoint,logUploadedFileshas no such check. An attacker could invokelogUploadedFiles(['fake-file.txt'])directly to create false activity log entries without uploading any files, polluting the audit trail.Add a permission check matching the pattern used in
createFolder(Line 643):public function logUploadedFiles(array $files): void { + /** @var Server $server */ + $server = Filament::getTenant(); + + if (!user()?->can(Permission::ACTION_FILE_CREATE, $server)) { + abort(403, 'You do not have permission to log uploads.'); + } + $filesCollection = collect($files); Activity::event('server:files.uploaded') ->property('directory', $this->path) ->property('files', $filesCollection) ->log(); }
672-672: Use consistent event naming convention.The event name
'server:files.uploaded'uses plural "files" while all other activity events in this file use the singular form:'server:file.rename','server:file.copy','server:file.write','server:file.pull', etc. This inconsistency could cause confusion when filtering or searching activity logs.Apply this diff to use consistent singular naming:
- Activity::event('server:files.uploaded') + Activity::event('server:file.upload') ->property('directory', $this->path) ->property('files', $filesCollection) ->log();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/Filament/Server/Resources/Files/Pages/ListFiles.php(5 hunks)resources/views/filament/server/pages/file-upload.blade.php(1 hunks)resources/views/filament/server/pages/list-files.blade.php(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-11-07T11:30:21.662Z
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 1858
File: resources/views/filament/server/pages/list-files.blade.php:304-309
Timestamp: 2025-11-07T11:30:21.662Z
Learning: In resources/views/filament/server/pages/list-files.blade.php, the handleEscapeKey function intentionally does not abort active XMLHttpRequest uploads when Escape is pressed. The UI is hidden but uploads continue in the background by design.
Applied to files:
app/Filament/Server/Resources/Files/Pages/ListFiles.phpresources/views/filament/server/pages/list-files.blade.phpresources/views/filament/server/pages/file-upload.blade.php
📚 Learning: 2025-10-15T11:55:53.461Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1801
File: app/Extensions/OAuth/Schemas/AuthentikSchema.php:7-10
Timestamp: 2025-10-15T11:55:53.461Z
Learning: In Filament v4, the Forms component Placeholder was deprecated and removed. Use TextEntry from Filament\Infolists\Components\TextEntry in forms instead, binding values with ->state(). For HTML content, use ->html().
Applied to files:
app/Filament/Server/Resources/Files/Pages/ListFiles.php
📚 Learning: 2025-10-15T11:55:53.461Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1801
File: app/Extensions/OAuth/Schemas/AuthentikSchema.php:7-10
Timestamp: 2025-10-15T11:55:53.461Z
Learning: In Filament v4, Wizard Step components use the Filament\Schemas namespace (Filament\Schemas\Components\Wizard\Step), not Filament\Forms.
Applied to files:
app/Filament/Server/Resources/Files/Pages/ListFiles.php
📚 Learning: 2025-11-05T22:21:31.863Z
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 1865
File: app/Filament/Admin/Resources/Nodes/Pages/EditNode.php:682-682
Timestamp: 2025-11-05T22:21:31.863Z
Learning: In app/Filament/Admin/Resources/Nodes/Pages/EditNode.php, the diagnostics tab's upload action intentionally does not use the iconButton() modifier, while the pull action does. This UI difference is intentional.
Applied to files:
app/Filament/Server/Resources/Files/Pages/ListFiles.phpresources/views/filament/server/pages/list-files.blade.phpresources/views/filament/server/pages/file-upload.blade.php
📚 Learning: 2025-11-03T23:44:24.737Z
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 1858
File: resources/views/filament/server/pages/list-files.blade.php:338-338
Timestamp: 2025-11-03T23:44:24.737Z
Learning: In the pelican-dev/panel repository, Tailwind CSS v4 is used, which supports fractional utility class syntax such as `max-w-1/2` (equivalent to `max-width: calc(1/2 * 100%)`). This syntax is valid and should not be flagged as invalid.
Applied to files:
resources/views/filament/server/pages/list-files.blade.php
📚 Learning: 2025-10-07T14:44:18.583Z
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 1779
File: app/Filament/Admin/Resources/Users/Pages/EditUser.php:51-51
Timestamp: 2025-10-07T14:44:18.583Z
Learning: In the Pelican Panel codebase, when using Filament's FileUpload component for the avatar field in UserResource, the 'avatar' key is intentionally unset from the data array in EditUser::handleRecordUpdate before passing to UserUpdateService. This is by design because the avatar is not stored in the database directly—Filament's FileUpload component handles file storage, retrieval, and deletion through its own lifecycle hooks (formatStateUsing, deleteUploadedFileUsing, etc.) independently of the database update service.
Applied to files:
resources/views/filament/server/pages/list-files.blade.php
🧬 Code graph analysis (1)
app/Filament/Server/Resources/Files/Pages/ListFiles.php (4)
app/Services/Nodes/NodeJWTService.php (4)
NodeJWTService(16-100)setExpiresAt(50-55)setUser(43-48)setClaims(32-37)app/Repositories/Daemon/DaemonFileRepository.php (2)
pull(215-226)createDirectory(86-100)app/Models/Node.php (1)
getConnectionAddress(178-181)app/Exceptions/Repository/FileExistsException.php (1)
FileExistsException(7-7)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/Filament/Server/Resources/Files/Pages/ListFiles.php (1)
668-676: Critical: Add permission check to prevent unauthorized activity log manipulation.This public method remains callable from the frontend without permission validation, allowing malicious users to inject false upload activity entries. While
getUploadUrl(line 608) protects the actual upload endpoint, this logging method has no such guard.Add the permission check:
public function logUploadedFiles(array $files): void { + /** @var Server $server */ + $server = Filament::getTenant(); + + if (!user()?->can(Permission::ACTION_FILE_CREATE, $server)) { + abort(403, 'You do not have permission to log uploads.'); + } + $filesCollection = collect($files); Activity::event('server:files.uploaded') ->property('directory', $this->path) ->property('files', $filesCollection) ->log(); }
Fix event naming inconsistency.
The event name
'server:files.uploaded'(line 672) uses plural form, breaking the established convention. All other file activity events use singular:'server:file.rename','server:file.copy','server:file.write', etc.Apply this diff:
- Activity::event('server:files.uploaded') + Activity::event('server:file.uploaded') ->property('directory', $this->path) ->property('files', $filesCollection) ->log();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/Filament/Server/Resources/Files/Pages/ListFiles.php(6 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 1858
File: resources/views/filament/server/pages/list-files.blade.php:304-309
Timestamp: 2025-11-07T11:30:21.662Z
Learning: In resources/views/filament/server/pages/list-files.blade.php, the handleEscapeKey function intentionally does not abort active XMLHttpRequest uploads when Escape is pressed. The UI is hidden but uploads continue in the background by design.
📚 Learning: 2025-11-05T22:21:31.863Z
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 1865
File: app/Filament/Admin/Resources/Nodes/Pages/EditNode.php:682-682
Timestamp: 2025-11-05T22:21:31.863Z
Learning: In app/Filament/Admin/Resources/Nodes/Pages/EditNode.php, the diagnostics tab's upload action intentionally does not use the iconButton() modifier, while the pull action does. This UI difference is intentional.
Applied to files:
app/Filament/Server/Resources/Files/Pages/ListFiles.php
📚 Learning: 2025-11-07T11:30:21.662Z
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 1858
File: resources/views/filament/server/pages/list-files.blade.php:304-309
Timestamp: 2025-11-07T11:30:21.662Z
Learning: In resources/views/filament/server/pages/list-files.blade.php, the handleEscapeKey function intentionally does not abort active XMLHttpRequest uploads when Escape is pressed. The UI is hidden but uploads continue in the background by design.
Applied to files:
app/Filament/Server/Resources/Files/Pages/ListFiles.php
📚 Learning: 2025-10-15T11:55:53.461Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1801
File: app/Extensions/OAuth/Schemas/AuthentikSchema.php:7-10
Timestamp: 2025-10-15T11:55:53.461Z
Learning: In Filament v4, the Forms component Placeholder was deprecated and removed. Use TextEntry from Filament\Infolists\Components\TextEntry in forms instead, binding values with ->state(). For HTML content, use ->html().
Applied to files:
app/Filament/Server/Resources/Files/Pages/ListFiles.php
📚 Learning: 2025-10-15T11:55:53.461Z
Learnt from: rmartinoscar
Repo: pelican-dev/panel PR: 1801
File: app/Extensions/OAuth/Schemas/AuthentikSchema.php:7-10
Timestamp: 2025-10-15T11:55:53.461Z
Learning: In Filament v4, Wizard Step components use the Filament\Schemas namespace (Filament\Schemas\Components\Wizard\Step), not Filament\Forms.
Applied to files:
app/Filament/Server/Resources/Files/Pages/ListFiles.php
🧬 Code graph analysis (1)
app/Filament/Server/Resources/Files/Pages/ListFiles.php (6)
app/Services/Nodes/NodeJWTService.php (4)
NodeJWTService(16-100)setExpiresAt(50-55)setUser(43-48)setClaims(32-37)app/helpers.php (1)
user(127-130)app/Filament/Server/Resources/Files/Pages/EditFiles.php (1)
getDaemonFileRepository(274-281)app/Repositories/Daemon/DaemonFileRepository.php (2)
pull(215-226)createDirectory(86-100)app/Models/Node.php (1)
getConnectionAddress(178-181)app/Exceptions/Repository/FileExistsException.php (1)
FileExistsException(7-7)
🔇 Additional comments (6)
app/Filament/Server/Resources/Files/Pages/ListFiles.php (6)
15-15: LGTM!The new imports support the direct-to-Wings upload functionality: NodeJWTService for token generation, CarbonImmutable for expiry timestamps, and ConnectionException for network error handling.
Also applies to: 18-18, 45-45
58-58: LGTM!Setting a custom view property is the correct approach to wire the drag-and-drop Blade template for this page.
508-508: LGTM!Refactoring to use the
createFoldermethod eliminates duplication and ensures consistent error handling, addressing the previous review suggestion.
532-556: LGTM!The refactoring cleanly separates direct file upload (now handled via the Blade view at line 534) from URL-based pulls (uploadURL). Both actions maintain appropriate authorization checks.
603-623: LGTM!The method correctly generates a JWT token for direct browser-to-Wings uploads with appropriate permission checks and a reasonable 15-minute expiry window. This implementation enables the bypass of PHP memory limits for large uploads.
638-663: LGTM!The method correctly handles folder creation with appropriate permission checks, comprehensive error handling, and activity logging. The idempotent behavior (silently ignoring FileExistsException at line 655) is appropriate for the drag-and-drop upload flow where folders may be created multiple times.
Reimplements Drag and Dropping for file uploads when viewing the file manager list. Fixes #1855 and Closes #1186
** Only Supported if the browser supports it! **
Adds support for folders 🥳 ( Also supports folderception )
Uploads directly to wings from the users pc, instead of user->panel->wings
Known Issues / Things that would be nice...
~ Check user permissions BEFORE showing the d&d
Video is outdated.... Feel free to Pull this PR and test it youself :)
SPOILER_final-form.mp4