-
-
Notifications
You must be signed in to change notification settings - Fork 254
EditServer select_startup refactor #1983
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
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughFixed the startup command selector validation in the server edit form. Refactored the options source to use a simpler closure, updated the state formatter to map values correctly, and modified the afterStateUpdated handler to conditionally update the startup state based on whether the custom option is selected. Changes
Possibly related PRs
Pre-merge checks✅ Passed checks (5 passed)
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
🧹 Nitpick comments (2)
app/Filament/Admin/Resources/Servers/Pages/EditServer.php (2)
801-805: Consider adding a null guard for defensive programming.If
Egg::find()returns null (e.g., due to data corruption or race conditions during deletion), accessing$egg->startup_commandson line 804 would throw an error.Apply this diff to add a null check:
->options(function (Get $get) { $egg = Egg::find($get('egg_id')); - return array_flip($egg->startup_commands ?? []) + ['custom' => 'Custom Startup']; + return $egg + ? array_flip($egg->startup_commands ?? []) + ['custom' => 'Custom Startup'] + : ['custom' => 'Custom Startup']; })
806-806: Excellent fix for the invalid default value issue—consider null-safe operator.This correctly validates the startup command against the egg's list and falls back to 'custom' when not found, which directly addresses issue #1977.
For additional robustness, consider using the null-safe operator:
-->formatStateUsing(fn (Server $server) => in_array($server->startup, $server->egg->startup_commands) ? $server->startup : 'custom') +->formatStateUsing(fn (Server $server) => in_array($server->startup, $server->egg?->startup_commands ?? []) ? $server->startup : 'custom')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/Filament/Admin/Resources/Servers/Pages/EditServer.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/Filament/Admin/Resources/Servers/Pages/EditServer.php (2)
app/Models/Server.php (2)
egg(298-301)Server(134-525)app/Models/Egg.php (1)
Egg(60-332)
🔇 Additional comments (1)
app/Filament/Admin/Resources/Servers/Pages/EditServer.php (1)
807-812: LGTM! Clean conditional logic.The conditional update prevents overwriting custom startup values, and unconditionally clearing the
previewingflag ensures the preview state is properly reset when the selection changes.
Closes #1977