Skip to content

Conversation

@andrasbacsai
Copy link
Member

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)

  • Added Alpine.js entangle() bindings for s3StorageId and s3Path
  • Changed button disabled binding from PHP :disabled to Alpine x-bind:disabled for reactive updates
  • Kept inputs as deferred wire:model (no .live modifier) to avoid unnecessary server requests

Component (app/Livewire/Project/Database/Import.php)

  • Updated checkS3File() method: Use S3Storage::ownedByCurrentTeam()->findOrFail() instead of ::findOrFail()
  • Updated downloadFromS3() method: Use S3Storage::ownedByCurrentTeam()->findOrFail() instead of ::findOrFail()
  • Removed redundant manual team verification since the scope handles it automatically

Why This Works

The button now uses Alpine.js reactivity via `$wire.entangle()` bindings, which:

  • Syncs Livewire properties to Alpine state without hitting the server
  • Allows `x-bind:disabled` to reactively update based on user input
  • Avoids the overhead of `.live` modifier which would trigger server requests on every keystroke

The security improvements use the existing `ownedByCurrentTeam()` scope which:

  • Automatically filters S3 storages to only those belonging to the current team
  • Eliminates redundant manual team ID verification code
  • Follows the same pattern already used in `loadAvailableS3Storages()`

Testing

The button should now:

  • Be disabled when the page loads (no storage selected, no path)
  • Enable when both S3 Storage and S3 File Path are filled in
  • Remain responsive as the user types/selects without server latency

@coollabsio coollabsio deleted a comment from coderabbitai bot Nov 2, 2025
@andrasbacsai andrasbacsai changed the title fix: S3 restore button disabled state and security scopes feat: S3 restore Nov 3, 2025
@coollabsio coollabsio deleted a comment from coderabbitai bot Nov 3, 2025
@andrasbacsai andrasbacsai changed the base branch from next to v4.x November 3, 2025 07:58
@andrasbacsai andrasbacsai changed the base branch from v4.x to next November 3, 2025 07:58
andrasbacsai and others added 20 commits November 3, 2025 09:00
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>
- 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>
@peaklabs-dev peaklabs-dev marked this pull request as ready for review November 5, 2025 22:17
@peaklabs-dev peaklabs-dev marked this pull request as draft November 5, 2025 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants