-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
feat: S3 restore #7085
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
Draft
andrasbacsai
wants to merge
20
commits into
next
Choose a base branch
from
s3-restore
base: next
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
feat: S3 restore #7085
+866
−190
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit introduces functionality for integrating S3 storage into the import process. It allows users to select S3 storage, check for file existence, and download files directly from S3. This enhancement improves the flexibility of the import feature by enabling users to work with files stored in S3, addressing a common use case for teams that utilize cloud storage solutions.
- Add Alpine.js entangle bindings for s3StorageId and s3Path to enable reactive button state without server requests - Change button disabled binding from PHP :disabled to Alpine x-bind:disabled for client-side reactivity using deferred wire:model inputs - Replace S3Storage::findOrFail with ownedByCurrentTeam()->findOrFail in checkS3File() and downloadFromS3() methods - Remove redundant manual team verification since ownedByCurrentTeam scope automatically filters to current team 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add unique wire keys to activity-monitor components (s3-download-monitor and database-restore-monitor) - Update dispatch calls to target specific components using ->to() method - This prevents both activity monitors from listening to the same activityMonitor event and displaying identical output - S3 download now shows in s3-download-monitor component - Database restore now shows in database-restore-monitor component 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…tent" This reverts commit d07cc48.
- Add currentActivityId property to track the active process - Replace event dispatching with property assignment for cleaner state management - S3 download monitor only renders during download and is removed when complete - Database restore monitor only renders during restore operation - Both monitors now share the same activity-monitor component instance with proper lifecycle management - When user starts restore after S3 download, S3 monitor is removed from DOM - Fixes issue where S3 download and database restore showed identical output 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add S3DownloadFinished event listener to Import component - Add handleS3DownloadFinished method to set s3DownloadInProgress to false - This ensures the 'Downloading from S3...' message is hidden when download completes - The success message now properly displays after download finishes 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add importRunning to x-data entangle bindings - Change S3 download activity monitor from @if to x-show for real-time visibility - Change database restore activity monitor from @if to x-show for real-time visibility - Activity monitors now display reactively as state changes instead of only on page load - Both monitors now visible immediately when processes start 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add updatedActivityId method to watch for changes to activityId property - When activityId is set/updated, automatically hydrate the activity and enable polling - This allows the activity monitor to display content when activityId is bound from parent component - Fixes issue where activity monitor was empty because activity wasn't loaded 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…nitor
Root cause analysis:
- Changed from dispatch to property binding broke the activity monitor completely
- ActivityMonitor component expects activityMonitor event, not property binding
- Original approach was correct: use dispatch + event listeners
Solution:
- Revert to original dispatch('activityMonitor', $activity->id) calls
- Use @if conditionals to render only one monitor at a time (removes from DOM)
- Add unique wire:key to each monitor instance to prevent conflicts
- S3 download monitor: wire:key="s3-download-{{ $resource->uuid }}"
- Database restore monitor: wire:key="database-restore-{{ $resource->uuid }}"
This ensures:
- Activity monitors display correctly when processes start
- Only one monitor is rendered at a time (S3 download OR database restore)
- Each monitor has unique identity via wire:key
- Event listeners work as designed
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Problem: - "Downloading from S3..." message stayed visible after download finished - @if conditional only evaluates on server-side render, not reactive - Event listener sets s3DownloadInProgress=false but view doesn't update Solution: - Wrap outer container with x-show="s3DownloadInProgress" for reactive hiding - Keep @if for activity-monitor to control when it's rendered in DOM - Message and success state now toggle reactively via Alpine.js entangle - When download finishes, message hides immediately without page refresh 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add debugging to understand why the download message stays visible after completion. This will help us see if: 1. The event is being dispatched by ActivityMonitor 2. The event is being received by Import component 3. The property is being set to false 4. The entangle is syncing to Alpine properly 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The formatBytes function was used in the view but never defined, causing a runtime error. This function was needed to display S3 file sizes in human-readable format (e.g., "1.5 MB" instead of "1572864"). Added formatBytes() helper to bootstrap/helpers/shared.php: - Converts bytes to human-readable format (B, KB, MB, GB, TB, PB) - Uses base 1024 for proper binary conversion - Configurable precision (defaults to 2 decimal places) - Handles zero bytes case 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Create S3DownloadFinished event to cleanup MinIO containers - Create S3RestoreJobFinished event to cleanup temp files and S3 downloads - Add formatBytes() helper function for human-readable file sizes - Update Import component to use full Event class names in callEventOnFinish - Fix activity monitor visibility issues with proper event dispatching 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Remove App\\Events\\ prefix from event class names - RunRemoteProcess already prepends App\\Events\\ to the class name - Use 'S3DownloadFinished' instead of 'App\\Events\\S3DownloadFinished' - Use 'S3RestoreJobFinished' instead of 'App\\Events\\S3RestoreJobFinished' - Fixes "Class 'App\Events\App\Events\S3DownloadFinished' not found" error 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Make S3DownloadFinished implement ShouldBroadcast - Add listener in Import component to handle S3DownloadFinished event - Set s3DownloadInProgress to false when download completes - This hides "Downloading from S3..." message after download finishes - Follows the same pattern as DatabaseStatusChanged event 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The event was broadcasting to the first user in the team instead of the actual user who triggered the download. This caused the download message to never hide for other team members. - Pass userId in S3DownloadFinished event data - Use the specific userId from event data for broadcasting - Remove unused User model import - Ensures broadcast reaches the correct user's private channel 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The s3DownloadedFile was being set immediately when download started, causing the "Restore" button to appear while still downloading and the download message to not hide properly. - Remove immediate setting of s3DownloadedFile in downloadFromS3() - Set s3DownloadedFile only in handleS3DownloadFinished() event handler - Add broadcastWith() to S3DownloadFinished to send downloadPath - Store downloadPath as public property for broadcasting - Now download message hides and restore button shows only when complete 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…wnload The first click did nothing because instant_remote_process() blocked the Livewire response, preventing UI state updates. The button also remained visible during download, allowing multiple clicks. - Replace blocking instant_remote_process() with async command in queue - Add container cleanup to command queue with error suppression - Hide "Download & Prepare" button when s3DownloadInProgress is true - Button now properly disappears when clicked, preventing double-clicks - No more blocking operations in downloadFromS3() method 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…onitor The ActivityMonitor component was never rendered because: 1. x-show hides elements with CSS but doesn't affect DOM rendering 2. @if on ActivityMonitor evaluated to false on initial page load 3. When s3DownloadInProgress became true, x-show showed the div 4. But ActivityMonitor was never in the DOM to receive events 5. dispatch('activityMonitor') event was lost Changed to use @if exclusively for all S3 download UI states: - Button visibility controlled by @if instead of x-show - Download progress section controlled by @if - Downloaded file section controlled by @if - Livewire automatically re-renders when state changes - ActivityMonitor is properly added to DOM and receives events 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
a8b2fd2 to
d9f89b8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixed the S3 restore button that was always disabled by implementing proper client-side reactivity using Alpine.js, and improved security by using the
ownedByCurrentTeam()scope instead of manual team verification.Changes
View File (
resources/views/livewire/project/database/import.blade.php)entangle()bindings fors3StorageIdands3Path:disabledto Alpinex-bind:disabledfor reactive updateswire:model(no.livemodifier) to avoid unnecessary server requestsComponent (
app/Livewire/Project/Database/Import.php)checkS3File()method: UseS3Storage::ownedByCurrentTeam()->findOrFail()instead of::findOrFail()downloadFromS3()method: UseS3Storage::ownedByCurrentTeam()->findOrFail()instead of::findOrFail()Why This Works
The button now uses Alpine.js reactivity via `$wire.entangle()` bindings, which:
The security improvements use the existing `ownedByCurrentTeam()` scope which:
Testing
The button should now: