Drop user created server's resource entry minimum limit to 0 #86
Conversation
The rest of the code is already set up to treat 0 as "unlimited", as far as I can tell, but the UI elements were limiting the minimum to 1. This change allows users to specify unlimited resources for a specific server if the user is unlimited in that resource.
📝 WalkthroughWalkthroughUpdated minValue logic for CPU, memory, and disk input fields to be conditional: set to 1 if the user's corresponding resource limit is greater than 0, otherwise 0. MaxValue, hints, and other validations remain unchanged across two Filament files. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code graph analysis (1)user-creatable-servers/src/Filament/Components/Actions/CreateServerAction.php (1)
🔇 Additional comments (1)
✏️ Tip: You can disable this entire section by setting 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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
user-creatable-servers/src/Filament/Server/Pages/ServerResourcePage.php (1)
65-93: Apply the same conditional minimum here to avoid “unlimited” bypass.The edit form now permits
0regardless of whether the user has finite limits. If0means “unlimited,” this can exceed quotas. Consider mirroring the conditional minimums here as well.✅ Suggested adjustment
TextInput::make('cpu') ->label(trans('user-creatable-servers::strings.cpu')) ->required() ->live(onBlur: true) ->hint(fn ($state) => $userResourceLimits->cpu > 0 ? ($maxCpu - $state . '% ' . trans('user-creatable-servers::strings.left')) : trans('user-creatable-servers::strings.unlimited')) ->hintColor(fn ($state) => $userResourceLimits->cpu > 0 && $maxCpu - $state < 0 ? 'danger' : null) ->numeric() - ->minValue(0) + ->minValue($userResourceLimits->cpu > 0 ? 1 : 0) ->maxValue($userResourceLimits->cpu > 0 ? $maxCpu : null) ->suffix('%'), TextInput::make('memory') ->label(trans('user-creatable-servers::strings.memory')) ->required() ->live(onBlur: true) ->hint(fn ($state) => $userResourceLimits->memory > 0 ? ($maxMemory - $state . $suffix . ' ' . trans('user-creatable-servers::strings.left')) : trans('user-creatable-servers::strings.unlimited')) ->hintColor(fn ($state) => $userResourceLimits->memory > 0 && $maxMemory - $state < 0 ? 'danger' : null) ->numeric() - ->minValue(0) + ->minValue($userResourceLimits->memory > 0 ? 1 : 0) ->maxValue($userResourceLimits->memory > 0 ? $maxMemory : null) ->suffix($suffix), TextInput::make('disk') ->label(trans('user-creatable-servers::strings.disk')) ->required() ->live(onBlur: true) ->hint(fn ($state) => $userResourceLimits->disk > 0 ? ($maxDisk - $state . $suffix . ' ' . trans('user-creatable-servers::strings.left')) : trans('user-creatable-servers::strings.unlimited')) ->hintColor(fn ($state) => $userResourceLimits->disk > 0 && $maxDisk - $state < 0 ? 'danger' : null) ->numeric() - ->minValue(0) + ->minValue($userResourceLimits->disk > 0 ? 1 : 0) ->maxValue($userResourceLimits->disk > 0 ? $maxDisk : null) ->suffix($suffix),user-creatable-servers/src/Filament/Components/Actions/CreateServerAction.php (1)
58-77: Align minValue with backend validation for finite user quotas.When a user has a finite resource limit, the backend
canCreateServer()enforces per-server values> 0by rejecting<= 0. However, the UI currently allowsminValue(0)unconditionally, creating a mismatch: users can submit 0, which the backend will reject. Make minValue conditional to match backend behavior.✅ Suggested adjustment
TextInput::make('cpu') ->label(trans('user-creatable-servers::strings.cpu')) ->required() ->numeric() - ->minValue(0) + ->minValue($userResourceLimits->cpu > 0 ? 1 : 0) ->maxValue($userResourceLimits->getCpuLeft()) ->suffix('%'), TextInput::make('memory') ->label(trans('user-creatable-servers::strings.memory')) ->required() ->numeric() - ->minValue(0) + ->minValue($userResourceLimits->memory > 0 ? 1 : 0) ->maxValue($userResourceLimits->getMemoryLeft()) ->suffix(config('panel.use_binary_prefix') ? 'MiB' : 'MB'), TextInput::make('disk') ->label(trans('user-creatable-servers::strings.disk')) ->required() ->numeric() - ->minValue(0) + ->minValue($userResourceLimits->disk > 0 ? 1 : 0) ->maxValue($userResourceLimits->getDiskLeft()) ->suffix(config('panel.use_binary_prefix') ? 'MiB' : 'MB'),
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
user-creatable-servers/src/Filament/Components/Actions/CreateServerAction.phpuser-creatable-servers/src/Filament/Server/Pages/ServerResourcePage.php
🧰 Additional context used
🧬 Code graph analysis (1)
user-creatable-servers/src/Filament/Components/Actions/CreateServerAction.php (1)
user-creatable-servers/src/Models/UserResourceLimits.php (2)
getCpuLeft(40-49)getMemoryLeft(51-60)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Boy132
left a comment
Boy132
left a comment
There was a problem hiding this comment.
I agree with coderabbit, the minValue should be conditional.
…ave a global limit in that resource
|
Requested changes have been made. |
Boy132
left a comment
Boy132
left a comment
There was a problem hiding this comment.
Thank you for your contribution!
2 participants
The rest of the code is already set up to treat 0 as "unlimited", as far as I can tell, but the UI elements were limiting the minimum to 1. This change allows users to specify unlimited resources for a specific server if the user is not globally limited in that resource. I've tested this on my local Pelican instance, and it works as intended.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.