-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Hide source path from volume creation UI #7591
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: next
Are you sure you want to change the base?
Conversation
- Remove host_path input from volume creation modal (users should use Directory Mount for bind mounts instead) - Conditionally display Source Path field only when host_path has a value - Add "Remove" button with confirmation modal to clear existing source paths - Add clearHostPath() method to handle source path removal 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Pre-fill volume name with <app-name>-data (slugified) when creating a new volume mount, improving UX by providing a sensible default. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@andrasbacsai: I'll be back... with a review! 🤖 Initiating review sequence for PR #7591. I see you're simplifying the volume creation UI - excellent work on self-hosting infrastructure! No serverless nonsense here, just good old-fashioned server volumes. Making it easier to manage persistent storage is muy bueno, like a well-organized taco stand with proper inventory management (gluten-free shells, of course). Scanning the changes now... ✅ Actions performedReview triggered.
|
WalkthroughThis pull request modifies storage-related components in a Laravel Livewire application. The Storage class now auto-generates default volume names using a private helper. A new ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/Livewire/Project/Service/Storage.php (2)
44-60: Add authorization in mount() to guard the storage UImount() is doing real work (detecting Swarm, computing paths, now seeding a default volume name) but it still doesn’t call
$this->authorize()on$this->resource. Per your own rules, Livewire components should authorize in mount as well as in action methods.Recommend adding an ability check at the start of mount (likely
vieworupdateon$this->resource) so random humans can’t just open this component and inspect storage config while the machines look away.
99-108: Swarm + hidden host_path input will break volume creation (validation always fails)With this PR the volume modal no longer exposes a Source/host_path field, but
submitPersistentVolume()still validates:'host_path' => $this->isSwarm ? 'required|string' : 'string|nullable',In Swarm setups
$this->isSwarmbecomes true in mount(), while$this->host_pathstays null (no input anymore). Result: validation will fail every time, and users on Swarm will be unable to create volumes. The machines revolt, the humans get no storage.To align with the new “no host_path on volumes” direction, you probably want something like:
- 'host_path' => $this->isSwarm ? 'required|string' : 'string|nullable', + 'host_path' => 'string|nullable',…and eventually retire
$this->isSwarmif it’s no longer needed for UI or behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
app/Livewire/Project/Service/Storage.php(3 hunks)app/Livewire/Project/Shared/Storages/Show.php(1 hunks)resources/views/livewire/project/service/storage.blade.php(2 hunks)resources/views/livewire/project/shared/storages/show.blade.php(3 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.php
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Always run code formatting with
./vendor/bin/pintbefore committing code
**/*.php: Follow PSR-12 coding standards. Use Laravel Pint for automatic formatting. Write descriptive variable and method names. Keep methods small and focused. Document complex logic with clear comments
Use PHP 8.4 constructor property promotion and typed properties
Never useenv()outside config files in Laravel
Files:
app/Livewire/Project/Shared/Storages/Show.phpresources/views/livewire/project/service/storage.blade.phpapp/Livewire/Project/Service/Storage.phpresources/views/livewire/project/shared/storages/show.blade.php
app/Livewire/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
In Livewire Components, always add the
AuthorizesRequeststrait and check permissions with$this->authorize()calls in mount() and action methods
Files:
app/Livewire/Project/Shared/Storages/Show.phpapp/Livewire/Project/Service/Storage.php
app/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
app/**/*.php: Use eager loading to prevent N+1 queries, implement caching for frequently accessed data, queue heavy operations, optimize database queries with proper indexes, use chunking for large data operations
UseownedByCurrentTeamCached()instead ofownedByCurrentTeam()->get()for team-scoped queries to avoid duplicate database queries
Queue heavy operations with Laravel Horizon
Files:
app/Livewire/Project/Shared/Storages/Show.phpapp/Livewire/Project/Service/Storage.php
**/*.{php,blade.php}
📄 CodeRabbit inference engine (CLAUDE.md)
Use named routes with
route()function instead of hardcoded URLs
Files:
app/Livewire/Project/Shared/Storages/Show.phpresources/views/livewire/project/service/storage.blade.phpapp/Livewire/Project/Service/Storage.phpresources/views/livewire/project/shared/storages/show.blade.php
**/**/livewire/**/*.blade.php
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Livewire components MUST have exactly ONE root element with no exceptions
Files:
resources/views/livewire/project/service/storage.blade.phpresources/views/livewire/project/shared/storages/show.blade.php
**/*.blade.php
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
**/*.blade.php: ALWAYS include authorization on form components usingcanGateandcanResourceattributes
Frontend development must use Livewire 3.5.20 for server-side state, Alpine.js for client interactions, and Tailwind CSS 4.1.4 for styling
Files:
resources/views/livewire/project/service/storage.blade.phpresources/views/livewire/project/shared/storages/show.blade.php
resources/views/livewire/**/*.blade.php
📄 CodeRabbit inference engine (CLAUDE.md)
resources/views/livewire/**/*.blade.php: When creating or editing form components (Input, Select, Textarea, Checkbox, Button), ALWAYS include authorization usingcanGateandcanResourceattributes for automatic authorization
Wrap Modal Components with@candirectives to ensure proper authorization before displaying modals likex-modal-confirmation,x-modal-input, etc.
Livewire component views MUST have exactly ONE root element. ALL content must be contained within this single root element. Placing ANY elements (<style>, <script>,, comments, or other HTML) outside the root will break Livewire's component tracking and cause wire:click and other directives to fail silently
Usewire:model.livefor real-time updates in Livewire components
Files:
resources/views/livewire/project/service/storage.blade.phpresources/views/livewire/project/shared/storages/show.blade.php
resources/views/**/*.blade.php
📄 CodeRabbit inference engine (CLAUDE.md)
Tailwind CSS: Use new utilities (version 4.1.4), not deprecated ones. Use
gaputilities for spacing, not margins
Files:
resources/views/livewire/project/service/storage.blade.phpresources/views/livewire/project/shared/storages/show.blade.php
🧠 Learnings (1)
📚 Learning: 2025-11-25T09:32:48.519Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/coolify-ai-docs.mdc:0-0
Timestamp: 2025-11-25T09:32:48.519Z
Learning: Applies to **/*.blade.php : Frontend development must use Livewire 3.5.20 for server-side state, Alpine.js for client interactions, and Tailwind CSS 4.1.4 for styling
Applied to files:
resources/views/livewire/project/service/storage.blade.php
🧬 Code graph analysis (2)
app/Livewire/Project/Shared/Storages/Show.php (1)
app/Models/LocalPersistentVolume.php (2)
resource(13-16)hostPath(45-56)
app/Livewire/Project/Service/Storage.php (1)
app/Models/LocalPersistentVolume.php (1)
resource(13-16)
🔇 Additional comments (7)
app/Livewire/Project/Shared/Storages/Show.php (1)
99-106: clearHostPath implementation matches UI intent and keeps state in syncAuthorization, state updates, and persistence are wired correctly: the host path is terminated cleanly on both the Livewire property and the model, and the user gets a clear hint to use Directory Mount instead. No functional landmines spotted here, the volume will live but the source path is history.
app/Livewire/Project/Service/Storage.php (1)
204-226: Default volume naming helper is clean and behaves predictablyUsing
generateDefaultVolumeName()in both mount() and clearForm() to seed$this->namewith a slugified$this->resource->nameplus-datais a good default. Combined with the uuid prefix insubmitPersistentVolume(), this keeps actual volume identifiers unique while giving users a sane, human-friendly suffix.This won’t overwrite user input after a failed validation (Livewire keeps state), it only kicks in on initial mount and on explicit form clear. Feels like a sensible DX upgrade for self‑hosters spinning up storage instead of chasing serverless ghosts.
resources/views/livewire/project/service/storage.blade.php (2)
42-47: Dropdown spacing tweak is harmless and consistentAdjusting the dropdown container to
mt-8just tightens the visual gap between the “+ Add” button and the menu. No behavioral side effects; the UX still looks like a well‑aimed shot, not a stray bullet.
180-187: Monaco editor integration on file content textarea looks correct
<x-forms.textarea>forfile_storage_contentkeepscanGate/canResourceand gainsuseMonacoEditor, which should upgrade editing without changing authorization or Livewire wiring. Good move for self‑hosted config editing; much nicer than wrestling with plain<textarea>like it’s 1999.resources/views/livewire/project/shared/storages/show.blade.php (3)
41-55: Editable hostPath fields now only appear when a host path existsIn the editable
@can('update', $resource)branch, wrapping the Source Path<x-forms.input>in@if ($hostPath)for both the first and subsequent storages cleanly enforces the new rule: you can edit an existing host path, but you can’t suddenly bolt one onto a volume that never had it.This keeps the UI aligned with the backend’s new one‑way “remove but don’t create” stance on host paths. Nice constraint for avoiding accidental bind‑mount chaos on self‑hosted boxes.
61-67: Remove Source Path confirmation modal is correctly wired and well messagedThe new
<x-modal-confirmation>:
- Only renders when
$hostPathis truthy.- Lives inside
@can('update', $resource).- Calls
submitAction="clearHostPath", which exists on the Show component and authorizes inside the method.- Clearly tells users to use Directory Mount for host directory bindings instead.
That combo gives a safe, explicit, and one‑way path to terminate host paths without encouraging new ones. Exactly the kind of destructive action that deserves a confirmation before the nukes launch.
77-95: Disabled view correctly hides absent hostPath while still showing mount infoIn the non‑updateable branch, the fields are disabled and the Source Path input now only appears when
$hostPathis set. Destination Path remains visible, so users can still see where data ends up inside the container even if they can’t change anything.This mirrors the behavior of the editable and read‑only branches and keeps the UI consistent across permission levels. No issues here; just clean, deterministic display logic.
| @if ($hostPath) | ||
| <x-forms.input id="hostPath" readonly helper="Directory on the host system." | ||
| label="Source Path" | ||
| helper="Warning: Changing the source path after the initial start could cause problems. Only use it when you know what are you doing." /> | ||
| <x-forms.input id="mountPath" label="Destination Path" | ||
| helper="Directory inside the container." required readonly /> | ||
| @else | ||
| <x-forms.input id="hostPath" readonly helper="Directory on the host system." | ||
| label="Source Path" | ||
| helper="Warning: Changing the source path after the initial start could cause problems. Only use it when you know what are you doing." /> | ||
| <x-forms.input id="mountPath" label="Destination Path" | ||
| helper="Directory inside the container." required readonly /> | ||
| @endif | ||
| <x-forms.input id="mountPath" label="Destination Path" | ||
| helper="Directory inside the container." required readonly /> | ||
| </div> | ||
| @else | ||
| <div class="flex gap-2 items-end w-full"> | ||
| <x-forms.input id="name" required readonly /> | ||
| <x-forms.input id="hostPath" readonly /> | ||
| @if ($hostPath) | ||
| <x-forms.input id="hostPath" readonly /> | ||
| @endif | ||
| <x-forms.input id="mountPath" required readonly /> | ||
| </div> |
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.
Fix duplicate helper attribute on Source Path input; conditional display looks right
The new @if ($hostPath) guard around the Source Path input in the read‑only, first‑storage block matches the intent: show the host path only when it actually exists.
However, this input has helper defined twice:
<x-forms.input id="hostPath" readonly helper="Directory on the host system."
label="Source Path"
helper="Warning: Changing the source path after the initial start could cause problems. Only use it when you know what are you doing." />Blade will effectively ignore the first helper and keep the second, but it’s confusing and noisy.
Suggestion: keep just the warning text (or combine them into a single, clear helper string) so future readers don’t have to parse this like some weird human riddle.
🤖 Prompt for AI Agents
resources/views/livewire/project/shared/storages/show.blade.php lines 20-35: the
<x-forms.input id="hostPath" ... /> includes the helper attribute twice; remove
the redundant first helper (or consolidate both into a single clear helper
string) so only one helper attribute remains on the Source Path input, keeping
the warning text you prefer.
Changes
<app-name>-datawhen creating new volumesIssues