-
Notifications
You must be signed in to change notification settings - Fork 25
Drop user created server's resource entry minimum limit to 0 #86
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: main
Are you sure you want to change the base?
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.
📝 WalkthroughWalkthroughThis pull request modifies validation rules for server resource fields (CPU, memory, and disk) across two Filament component files, changing the minimum allowed value from 1 to 0, thereby permitting zero resource allocation entries. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 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
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧬 Code graph analysis (1)user-creatable-servers/src/Filament/Components/Actions/CreateServerAction.php (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 |
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: 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.
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.