Allow multiple startup commands per egg #1656
Conversation
📝 WalkthroughWalkthroughReplaces the single egg Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Filament UI
participant API as Backend
participant DB as Database
Note over UI: User selects Egg → UI loads egg.startup_commands
User->>UI: choose Egg
UI->>API: GET /egg (includes `startup_commands`)
API->>DB: SELECT egg.startup_commands
DB-->>API: return startup_commands
API-->>UI: return options list
Note over UI: User selects predefined startup or types custom
User->>UI: select_startup / edit startup textarea
UI-->>UI: sync select_startup ↔ startup textarea
Note over UI,API: Persist change
UI->>API: PATCH server.startup
API->>API: StartupCommandService.handle(server, startup)
API->>DB: UPDATE servers.startup
DB-->>API: updated
API-->>User: emit activity + send notification
sequenceDiagram
participant User
participant Preview as PreviewStartupAction
participant Store as Filament Store
participant Service as StartupCommandService
User->>Preview: click Preview
Preview->>Store: get('startup')
Store-->>Preview: current startup
Preview->>Service: handle(server, startup)
Service-->>Preview: rendered preview
Preview-->>User: display preview toggle/result
Possibly related PRs
Pre-merge checks❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
…rtup-commands # Conflicts: # app/Filament/Admin/Resources/Servers/Pages/EditServer.php # app/Filament/Server/Pages/Startup.php
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (18)
lang/en/activity.php (1)
115-116: Make quoting consistent within the startup group.Currently, only :new is quoted. Quote both values (like the nearby "edit" entry) or neither. Suggest quoting both for clarity.
- 'command' => 'Updated the Startup Command for the server from <b>:old</b> to <b>:new</b>', + 'command' => 'Updated the Startup Command for the server from "<b>:old</b>" to "<b>:new</b>"',app/Services/Servers/StartupModificationService.php (1)
88-90: Prefer explicit "Default" startup/image when present; fall back safely.Associative order can vary. Use the "Default" key when available, then first value, then current server values.
- $data['docker_image'] ??= Arr::first($egg->docker_images); - $data['startup'] ??= Arr::first($egg->startup_commands); + $data['docker_image'] ??= Arr::get($egg->docker_images, 'Default') + ?? Arr::first($egg->docker_images); + $data['startup'] ??= Arr::get($egg->startup_commands, 'Default') + ?? Arr::first($egg->startup_commands) + ?? $server->startup;Minor: use the already-normalized $eggId variable above instead of $data['egg_id'] for consistency.
database/Seeders/eggs/rust/egg-rust.yaml (1)
22-23: Complex command string: validate escaping.The single-quoted YAML with embedded " and command substitution looks valid, but easy to regress. Consider adding a seed parse/smoke test to catch quoting errors.
Would you like a tiny CI check that parses all egg YAML and asserts startup_commands contain non-empty strings?
database/Seeders/eggs/source-engine/egg-garrys-mod.yaml (1)
21-22: Prefer POSIX-compatible test operator in embedded shell.Use single = for broader shell compatibility (dash/ash), since the startup is executed under sh -c in many images.
Apply this diff in the exporter so generated seeds use POSIX =:
- Default: './srcds_run -game garrysmod -console -port {{SERVER_PORT}} +ip 0.0.0.0 +host_workshop_collection {{WORKSHOP_ID}} +map {{SRCDS_MAP}} +gamemode {{GAMEMODE}} -strictportbind -norestart +sv_setsteamaccount {{STEAM_ACC}} +maxplayers {{MAX_PLAYERS}} -tickrate {{TICKRATE}} $( [ "$LUA_REFRESH" == "1" ] || printf %s ''-disableluarefresh'' )' + Default: './srcds_run -game garrysmod -console -port {{SERVER_PORT}} +ip 0.0.0.0 +host_workshop_collection {{WORKSHOP_ID}} +map {{SRCDS_MAP}} +gamemode {{GAMEMODE}} -strictportbind -norestart +sv_setsteamaccount {{STEAM_ACC}} +maxplayers {{MAX_PLAYERS}} -tickrate {{TICKRATE}} $( [ "$LUA_REFRESH" = "1" ] || printf %s ''-disableluarefresh'' )'database/Seeders/eggs/minecraft/egg-vanilla-minecraft.yaml (1)
27-28: Consider appending 'nogui' to the default Java invocation.Prevents attempting to launch a GUI and matches common vanilla server practice.
- Default: 'java -Xms128M -XX:MaxRAMPercentage=95.0 -jar {{SERVER_JARFILE}}' + Default: 'java -Xms128M -XX:MaxRAMPercentage=95.0 -jar {{SERVER_JARFILE}} nogui'database/Factories/EggFactory.php (1)
26-34: Use a named map for startup_commands to mirror production seeds/UI.Factory currently sets a numeric array; switching to a name=>command map helps catch shape issues in tests and aligns with KeyValue UI.
- 'startup_commands' => ['java -jar test.jar'], + // Use named commands to mirror production seeds and UI expectations. + 'startup_commands' => ['Default' => 'java -jar test.jar'],app/Services/Eggs/EggChangerService.php (1)
25-27: Guard against empty docker_images/startup_commands when switching eggs.Arr::first can return null; avoid persisting null image/startup or regress to prior values.
- $server->forceFill([ - 'egg_id' => $newEgg->id, - 'image' => Arr::first($newEgg->docker_images), - 'startup' => Arr::first($newEgg->startup_commands), - ])->saveOrFail(); + $defaultImage = Arr::first($newEgg->docker_images) ?? $server->image; + $defaultStartup = Arr::first($newEgg->startup_commands) ?? $server->startup; + $server->forceFill([ + 'egg_id' => $newEgg->id, + 'image' => $defaultImage, + 'startup' => $defaultStartup, + ])->saveOrFail();If you prefer a hard failure instead of fallback, I can wire a DisplayException with a localized message.
app/Filament/Admin/Resources/Eggs/Pages/EditEgg.php (1)
83-91: Switch to KeyValue startup_commands is good; consider UX placeholdersOptional: add key/value placeholders (e.g., "Default" and a typical command) to match CreateEgg for consistency.
app/Transformers/Api/Application/EggTransformer.php (1)
51-52: Transformer defaults — LGTM; tiny comment nit.Code looks correct. Optional: clarify the inline comments for readability.
Apply this diff to tighten comments:
- 'docker_image' => Arr::first($model->docker_images, default: ''), // docker_images, use startup_commands + 'docker_image' => Arr::first($model->docker_images, default: ''), // first image is the default @@ - 'startup' => Arr::first($model->startup_commands, default: ''), // deprecated, use startup_commands + 'startup' => Arr::first($model->startup_commands, default: ''), // deprecated; prefer startup_commandsAlso applies to: 61-63
database/migrations/2025_09_03_090706_support_multiple_startup_commands.php (1)
17-18: Note: Column positioning with after() may fail on SQLite.If your CI uses SQLite for tests, after() is ignored/unsupported. Not a blocker, but be aware.
Would you like a test-friendly variant that skips after() on SQLite?
Also applies to: 35-36
app/Filament/Admin/Resources/Servers/Pages/EditServer.php (2)
616-633: Don’t persist select_startup; add dehydrated(false).select_startup is a helper UI field; prevent accidental persistence/validation.
Select::make('select_startup') ->label(trans('admin/server.startup_cmd')) ->live() ->afterStateUpdated(fn (Set $set, $state) => $set('startup', $state)) + ->dehydrated(false)
637-653: Reduce reactive churn on free-typing in startup.live() + afterStateUpdated runs on every keystroke. Debounce to avoid flicker when syncing select_startup.
- ->live() + ->live(debounce: 500)app/Filament/Admin/Resources/Servers/Pages/CreateServer.php (3)
323-324: Also reset select_startup when egg changes.Prevents stale selection before options() re-evaluates.
- $set('startup', ''); + $set('startup', ''); + $set('select_startup', '');
397-417: Do not dehydrate select_startup; UI-only field.Avoid persisting it to ServerCreationService payload.
Select::make('select_startup') ->label(trans('admin/server.startup_cmd')) ->hidden(fn (Get $get) => $get('egg_id') === null) ->live() ->afterStateUpdated(fn (Set $set, $state) => $set('startup', $state)) + ->dehydrated(false)
418-436: Debounce live typing to reduce unnecessary recomputations.Same rationale as EditServer; improves UX on slower devices.
- ->live() + ->live(debounce: 500)app/Services/Eggs/Sharing/EggImporterService.php (1)
184-187: Reserved env redaction applied to each startup command.Approach is sound. Minor: consider using preg_quote with delimiter to be explicit.
- $pattern = '/\b(' . collect($forbidden)->map(fn ($variable) => preg_quote($variable['env_variable']))->join('|') . ')\b/'; + $pattern = '/\b(' . collect($forbidden)->map(fn ($v) => preg_quote($v['env_variable'], '/'))->join('|') . ')\b/';app/Filament/Admin/Resources/Eggs/Pages/CreateEgg.php (1)
86-96: Great UX for multiple commands; add a hint about uniqueness.Because the server UI flips command values, duplicate command strings will collapse options. Add a brief helper note to warn admins that command values must be unique. Model-side distinct rule recommended in Egg::$validationRules (see other comment).
app/Filament/Server/Pages/Startup.php (1)
45-49: Show a snippet of the custom command instead of a static string.This improves clarity without exposing a huge command (full command still shown below).
- TextInput::make('custom_startup') + TextInput::make('custom_startup') ->label(trans('server/startup.command')) - ->readOnly() - ->visible(fn (Server $server) => !in_array($server->startup, $server->egg->startup_commands)) - ->formatStateUsing(fn () => 'Custom Startup'), + ->readOnly() + ->visible(fn (Server $server) => !in_array($server->startup, $server->egg->startup_commands)) + ->formatStateUsing(fn (Server $server) => mb_strimwidth($server->startup ?? '', 0, 120, '…')),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
app/Filament/Admin/Resources/Eggs/Pages/CreateEgg.php(1 hunks)app/Filament/Admin/Resources/Eggs/Pages/EditEgg.php(1 hunks)app/Filament/Admin/Resources/Servers/Pages/CreateServer.php(2 hunks)app/Filament/Admin/Resources/Servers/Pages/EditServer.php(1 hunks)app/Filament/Components/Actions/PreviewStartupAction.php(1 hunks)app/Filament/Server/Pages/Startup.php(3 hunks)app/Models/Egg.php(6 hunks)app/Services/Eggs/EggChangerService.php(2 hunks)app/Services/Eggs/Sharing/EggExporterService.php(1 hunks)app/Services/Eggs/Sharing/EggImporterService.php(5 hunks)app/Services/Servers/ServerCreationService.php(1 hunks)app/Services/Servers/StartupCommandService.php(2 hunks)app/Services/Servers/StartupModificationService.php(1 hunks)app/Transformers/Api/Application/EggTransformer.php(2 hunks)app/Transformers/Api/Client/ServerTransformer.php(1 hunks)database/Factories/EggFactory.php(1 hunks)database/Seeders/eggs/minecraft/egg-bungeecord.yaml(2 hunks)database/Seeders/eggs/minecraft/egg-forge-minecraft.yaml(2 hunks)database/Seeders/eggs/minecraft/egg-paper.yaml(2 hunks)database/Seeders/eggs/minecraft/egg-sponge.yaml(2 hunks)database/Seeders/eggs/minecraft/egg-vanilla-minecraft.yaml(2 hunks)database/Seeders/eggs/rust/egg-rust.yaml(2 hunks)database/Seeders/eggs/source-engine/egg-custom-source-engine-game.yaml(2 hunks)database/Seeders/eggs/source-engine/egg-garrys-mod.yaml(2 hunks)database/Seeders/eggs/source-engine/egg-insurgency.yaml(2 hunks)database/Seeders/eggs/source-engine/egg-team-fortress2.yaml(2 hunks)database/Seeders/eggs/voice-servers/egg-mumble-server.yaml(2 hunks)database/Seeders/eggs/voice-servers/egg-teamspeak3-server.yaml(2 hunks)database/migrations/2025_09_03_090706_support_multiple_startup_commands.php(1 hunks)lang/en/activity.php(1 hunks)lang/en/admin/egg.php(2 hunks)lang/en/admin/server.php(1 hunks)lang/en/server/startup.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
app/Services/Servers/StartupCommandService.php (5)
app/Services/Servers/ServerCreationService.php (1)
handle(54-134)app/Services/Servers/StartupModificationService.php (1)
handle(30-64)app/Services/Servers/VariableValidatorService.php (1)
handle(25-56)app/Services/Servers/ServerConfigurationStructureService.php (1)
handle(23-36)app/Models/Server.php (1)
Server(132-504)
app/Services/Servers/StartupModificationService.php (1)
app/Models/Server.php (1)
egg(295-298)
app/Transformers/Api/Client/ServerTransformer.php (1)
app/Services/Servers/ServerConfigurationStructureService.php (1)
handle(23-36)
app/Services/Eggs/Sharing/EggImporterService.php (1)
app/Models/Egg.php (1)
Egg(59-329)
app/Filament/Server/Pages/Startup.php (5)
app/Models/Server.php (2)
Server(132-504)egg(295-298)app/Models/User.php (1)
can(360-373)app/Models/Permission.php (1)
Permission(11-221)app/Facades/Activity.php (1)
Activity(8-14)app/Services/Activity/ActivityLogService.php (3)
event(48-53)property(109-117)log(136-155)
🪛 YAMLlint (1.37.1)
database/Seeders/eggs/source-engine/egg-garrys-mod.yaml
[warning] 20-20: too many spaces inside empty braces
(braces)
🔇 Additional comments (40)
lang/en/admin/server.php (1)
29-31: Translations added look good; align with UI usage.Keys read clearly and match existing naming (e.g., image_placeholder). No issues.
database/Seeders/eggs/minecraft/egg-bungeecord.yaml (2)
3-5: Version/export timestamp bump to PLCN_v3 is fine.Matches the schema shift to startup_commands.
29-30: startup_commands migration looks correct.Default command preserved verbatim. No YAML quoting issues spotted.
database/Seeders/eggs/voice-servers/egg-mumble-server.yaml (2)
3-5: PLCN_v3/version bump is consistent with the migration.
18-19: startup_commands Default preserved; looks good.database/Seeders/eggs/rust/egg-rust.yaml (2)
3-5: PLCN_v3/version bump OK.
20-20: Labeling docker image as "Rust" improves UX.No functional concerns.
database/Seeders/eggs/voice-servers/egg-teamspeak3-server.yaml (2)
3-5: PLCN_v3/version bump OK.
18-19: startup_commands migration looks correct.Default command matches prior scalar.
database/Seeders/eggs/minecraft/egg-sponge.yaml (1)
23-24: startup_commands Default retained; looks good.database/Seeders/eggs/source-engine/egg-garrys-mod.yaml (2)
3-5: Exporter version bump looks consistent with schema migration.PLCN_v3 and updated exported_at align with the startup_commands migration. No action needed.
19-19: docker_images mapping change is OK.Switching to a labeled map (Source => image) works with Arr::first usage downstream.
database/Seeders/eggs/source-engine/egg-team-fortress2.yaml (2)
3-5: v3 export + docker_images map LGTM.Matches model changes and Arr::first usage.
Also applies to: 19-19
21-22: startup_commands migration looks correct.Default command preserved; no issues spotted.
database/Seeders/eggs/minecraft/egg-vanilla-minecraft.yaml (1)
3-5: PLCN_v3 export metadata is consistent.No action needed.
app/Transformers/Api/Client/ServerTransformer.php (1)
63-64: Approve — named-argument usage is correct.Signature present: app/Services/Servers/StartupCommandService.php:12 — public function handle(Server $server, ?string $startup = null, bool $hideAllValues = false): string. Using hideAllValues by name is safe and future‑proof.
lang/en/server/startup.php (1)
6-7: New notification strings read well.Clear and consistent with existing Docker image notifications.
app/Services/Eggs/Sharing/EggExporterService.php (1)
36-36: Export field rename to startup_commands — LGTMThe exporter now emits startup_commands from the model. Looks correct and consistent with the migration.
database/Seeders/eggs/minecraft/egg-forge-minecraft.yaml (1)
3-5: Seed migrated to PLCN_v3 with startup_commands mapping — LGTMVersion bump and Default startup command mapping look correct.
Also applies to: 25-26
lang/en/admin/egg.php (1)
40-45: New startup_commands strings — LGTMWording is clear and matches behavior (first entry is default).
Also applies to: 57-57
app/Services/Servers/StartupCommandService.php (1)
12-29: Make allocation access null-safe; callers verified
- Apply the nullsafe change in StartupCommandService (diff below).
- I searched the repo: no positional boolean second-arg calls to StartupCommandService->handle were found. Call sites checked: app/Transformers/Api/Client/ServerTransformer.php (uses named param), app/Filament/Components/Actions/PreviewStartupAction.php (passes $startup string), app/Http/Controllers/Api/Client/Servers/StartupController.php (single-arg). No signature-breaking callers detected.
- $server->allocation->ip ?? '127.0.0.1', - (string) ($server->allocation->port ?? '0'), + $server->allocation?->ip ?? '127.0.0.1', + (string) ($server->allocation?->port ?? '0'),database/Seeders/eggs/minecraft/egg-paper.yaml (1)
3-5: Paper seed migrated to PLCN_v3 with Default startup mapping — LGTMMatches the new schema and default-selection semantics.
Also applies to: 23-24
database/Seeders/eggs/source-engine/egg-insurgency.yaml (1)
3-3: Seeder aligns with PLCN_v3 and startup_commands. LGTM.Meta bump, docker_images restructuring, and Default startup_commands mapping look consistent with UI/transformer expectations.
Also applies to: 5-5, 19-19, 21-22
database/Seeders/eggs/source-engine/egg-custom-source-engine-game.yaml (1)
3-3: Seeder migration to startup_commands and named docker image is consistent.Default startup preserved; docker_images now keyed. Matches UI that flips arrays for labels/values.
Also applies to: 5-5, 18-18, 20-21
app/Filament/Admin/Resources/Servers/Pages/EditServer.php (2)
631-632: Guard against duplicate startup command strings collapsing options.array_flip will drop duplicates if two entries share identical command strings.
Are all values in Egg::$startup_commands guaranteed unique per egg? If not, consider returning ['name' => 'command'] and mapping state by name instead of command.
621-622: Ensure Egg::$casts includes 'startup_commands' => 'array'.Without a cast, $egg->startup_commands may be a JSON string; in_array/array_flip would error.
If missing, add in App\Models\Egg: protected $casts = ['startup_commands' => 'array', 'docker_images' => 'array'];
Also applies to: 644-645
app/Filament/Admin/Resources/Servers/Pages/CreateServer.php (2)
403-414: Potential duplicate-value collapse in array_flip.Duplicate command strings will be lost.
If duplicates are possible, return options keyed by the command name and set the select’s state to the name, not the command string.
403-405: Confirm model casts for Egg startup_commands/docker_images.Both components expect arrays; ensure casts exist.
If not already present, add: protected $casts = ['startup_commands' => 'array', 'docker_images' => 'array'];
Also applies to: 426-427
app/Models/Egg.php (4)
25-25: Docblocks updated correctly.Types for docker_images and startup_commands look right for downstream static analysis.
Also applies to: 34-34
92-92: Fillable updated for startup_commands.Correct replacement of startup with startup_commands.
146-147: Cast added.Casting startup_commands to array is correct.
73-73: Update docs/exporters to PLCN_v3; legacy import mappings are presentImporter still matches older versions (app/Services/Eggs/Sharing/EggImporterService.php lines 134–139: 'PTDL_v1','PTDL_v2','PLCN_v1','PLCN_v2' => convertToV3). Egg::EXPORT_VERSION is used for current exports — ensure docs and any external importers/exporters are updated to 'PLCN_v3'.
app/Services/Eggs/Sharing/EggImporterService.php (3)
136-139: Import version routing LGTM.PTDL_v1 → legacy + v3; PTDL_v2/PLCN_v1/PLCN_v2 → v3; PLCN_v3 passthrough. Good layering.
210-210: Model mapping updated to startup_commands.Looks correct and matches Egg::$fillable and casts.
221-238: Legacy image conversion rename is fine.No functional concerns; clear separation from v3 conversion.
app/Filament/Server/Pages/Startup.php (5)
20-20: Set import added.Good.
39-39: Form columns change.OK.
81-81: Custom image formatter.Good: shows actual image.
106-107: No functional change.Formatting-only diff near options() closing brace.
107-113: Read-only startup textarea with preview.Nice separation of selection vs. preview.
database/migrations/2025_09_03_090706_support_multiple_startup_commands.php
Show resolved
Hide resolved
database/migrations/2025_09_03_090706_support_multiple_startup_commands.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
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 (1)
database/Seeders/eggs/minecraft/egg-sponge.yaml (1)
98-102: Checksum comparison is incorrect and will always fail.
basename $(md5sum ...)returns the whole "hash filename" string. Compare only the first field (the digest).- if [ $(basename $(md5sum ${SERVER_JARFILE})) = ${CHECKSUM} ] ; then + if [ "$(md5sum "${SERVER_JARFILE}" | awk '{print $1}')" = "${CHECKSUM}" ] ; then echo "Checksum passed" else echo "Checksum failed" fi
🧹 Nitpick comments (1)
database/Seeders/eggs/minecraft/egg-sponge.yaml (1)
3-5: Approve — update_url is valid; optional: prefer canonical raw hostBoth endpoints returned HTTP 200; prefer raw.githubusercontent.com to reduce redirects and improve caching.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/Models/Egg.php(6 hunks)database/Seeders/eggs/minecraft/egg-sponge.yaml(2 hunks)database/migrations/2025_09_03_090706_support_multiple_startup_commands.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- database/migrations/2025_09_03_090706_support_multiple_startup_commands.php
🔇 Additional comments (6)
app/Models/Egg.php (6)
25-25: Docblock type for docker_images is accurate.Using
array<string, string>better reflects the label=>image mapping.
35-35: Docblock for startup_commands added correctly.The map of label=>command is clearly typed.
92-92: Mass-assignable startup_commands added.Necessary for creates/updates via forms and import.
113-115: Validation updated; distinct values prevent UI collisions.Good catch adding
distinctto avoid duplicate command strings collapsing in selection.
146-146: Casts include startup_commands as array.Ensures consistent serialization.
73-73: Approve: EXPORT_VERSION bumped to PLCN_v3 — no action requiredBump matches YAML meta.version; the only PLCN_v2 occurrence is the backward-compatibility mapping in app/Services/Eggs/Sharing/EggImporterService.php (maps 'PLCN_v2' => $this->convertToV3(...)), so nothing to change.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
app/Filament/Admin/Resources/Eggs/Pages/CreateEgg.php (3)
86-96: Normalize and validate entries (trim, drop empties, require ≥1).Prevents blank keys/values and enforces at least one command. Also marks the field as an array for validation.
Apply this diff:
KeyValue::make('startup_commands') ->label(trans('admin/egg.startup_commands')) - ->live() ->columnSpanFull() - ->required() + ->rules(['required', 'array', 'min:1']) + ->dehydrateStateUsing(fn ($state) => collect($state ?? []) + ->mapWithKeys(fn ($v, $k) => [trim((string) $k) => is_string($v) ? trim($v) : $v]) + ->reject(fn ($v, $k) => $k === '' || $v === '') + ->all() + ) ->addActionLabel(trans('admin/egg.add_startup')) ->keyLabel(trans('admin/egg.startup_name')) ->keyPlaceholder('Default') ->valueLabel(trans('admin/egg.startup_command')) ->valuePlaceholder('java -Xms128M -XX:MaxRAMPercentage=95.0 -jar {{SERVER_JARFILE}}') ->helperText(trans('admin/egg.startup_help')),
86-96: Seed a sensible default entry.Pre-populating a single “Default” row reduces empty-form errors and clarifies intent.
Apply this diff:
->keyLabel(trans('admin/egg.startup_name')) ->keyPlaceholder('Default') ->valueLabel(trans('admin/egg.startup_command')) ->valuePlaceholder('java -Xms128M -XX:MaxRAMPercentage=95.0 -jar {{SERVER_JARFILE}}') + ->default(['Default' => '']) ->helperText(trans('admin/egg.startup_help')),
86-96: Drop live() unless something else reacts to this field.
->live()can cause unnecessary re-renders; remove if no reactive dependencies rely on this state.- ->live()app/Filament/Admin/Resources/Servers/Pages/EditServer.php (3)
617-636: Consider handling edge cases in startup command selection logic.The implementation handles selection and synchronization well, but there are a few edge cases to consider:
- When no egg is found (
$eggis null), accessing$egg->startup_commandscould cause an error- The empty string key (
'') for "Custom Startup" could be confusing; consider using a more descriptive constantConsider adding null checks and using a constant for the custom startup key:
Select::make('select_startup') ->label(trans('admin/server.startup_cmd')) ->live() ->afterStateUpdated(fn (Set $set, $state) => $set('startup', $state)) ->options(function ($state, Get $get, Set $set) { $egg = Egg::query()->find($get('egg_id')); - $startups = $egg->startup_commands ?? []; + if (!$egg) { + return []; + } + $startups = $egg->startup_commands ?? []; $currentStartup = $get('startup'); if (!$currentStartup && $startups) { $currentStartup = collect($startups)->first(); $set('startup', $currentStartup); $set('select_startup', $currentStartup); } - return array_flip($startups) + ['' => 'Custom Startup']; + const CUSTOM_STARTUP_KEY = '__custom__'; + return array_flip($startups) + [self::CUSTOM_STARTUP_KEY => 'Custom Startup']; })
638-654: Startup textarea synchronization looks good with one suggestion.The bidirectional synchronization between
startuptextarea andselect_startupis well-implemented. The live updates and proper state management ensure consistency.Consider adding debouncing to the
afterStateUpdatedcallback to reduce unnecessary state updates during typing:Textarea::make('startup') ->hiddenLabel() ->required() ->live() + ->debounce(500) ->autosize() ->afterStateUpdated(function ($state, Get $get, Set $set) { $egg = Egg::query()->find($get('egg_id')); $startups = $egg->startup_commands ?? []; if (in_array($state, $startups)) { $set('select_startup', $state); } else { $set('select_startup', ''); } })
643-652: Add null check for egg retrieval in textarea callback.Similar to the select field, the textarea's
afterStateUpdatedcallback should handle the case where the egg is not found.Apply this fix to prevent potential errors:
->afterStateUpdated(function ($state, Get $get, Set $set) { $egg = Egg::query()->find($get('egg_id')); + if (!$egg) { + $set('select_startup', ''); + return; + } $startups = $egg->startup_commands ?? []; if (in_array($state, $startups)) { $set('select_startup', $state); } else { $set('select_startup', ''); } })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/Filament/Admin/Resources/Eggs/Pages/CreateEgg.php(1 hunks)app/Filament/Admin/Resources/Eggs/Pages/EditEgg.php(1 hunks)app/Filament/Admin/Resources/Servers/Pages/CreateServer.php(2 hunks)app/Filament/Admin/Resources/Servers/Pages/EditServer.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/Filament/Admin/Resources/Servers/Pages/CreateServer.php
- app/Filament/Admin/Resources/Eggs/Pages/EditEgg.php
🧰 Additional context used
🧬 Code graph analysis (1)
app/Filament/Admin/Resources/Servers/Pages/EditServer.php (1)
app/Models/Egg.php (1)
Egg(59-329)
🔇 Additional comments (4)
app/Filament/Admin/Resources/Eggs/Pages/CreateEgg.php (2)
86-96: Translations/help copy: ensure keys exist and reflect “multiple commands.”Confirm new i18n keys exist:
admin/egg.startup_commands,admin/egg.add_startup,admin/egg.startup_name,admin/egg.startup_command. Also verifyadmin/egg.startup_helptext no longer implies a single command.
86-96: Persisting startup_commands verified — no action required.Egg model casts
startup_commandsto an array and includes it in$fillable; a migration creates a nullable JSONstartup_commandscolumn and seeds/factories use it.
- app/Models/Egg.php — docblock,
$fillableand cast present.- database/migrations/2025_09_03_090706_support_multiple_startup_commands.php — creates/populates
startup_commands.app/Filament/Admin/Resources/Servers/Pages/EditServer.php (2)
614-616: LGTM! Proper state management for startup preview.The addition of the
previewinghidden field is appropriate for tracking the internal state during startup-related UI interactions.
621-630: No action required — startup_commands are guaranteed non-empty. Migration populates startup_commands from the legacy startup field and model validation enforces 'startup_commands' as a required array with min:1, so the UI's conditional defaulting is correct.
There was a problem hiding this comment.
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 (1)
app/Filament/Server/Pages/Startup.php (1)
44-113: Restore ability to change custom startup commandsWith
startup_selecthidden whenever the server’s current command is not one of the egg presets, and thestartuptextarea now read-only, anyone using a bespoke command can no longer switch back to a predefined option or edit the command at all. That’s a regression from the previous behaviour and blocks normal administration flows. Please keep the selector visible (e.g. include a “Custom” option) and make the textarea editable for users who holdstartup.update, updatingselect_startupwhen they type so both paths stay in sync.
🧹 Nitpick comments (1)
app/Filament/Admin/Resources/Eggs/Pages/CreateEgg.php (1)
86-96: Consider adding key validation for startup commands.The KeyValue field implementation is well-structured with good labels and placeholders. However, consider adding explicit validation to ensure startup command names (keys) are not empty and are unique within the set.
While Filament's KeyValue component may handle some of these cases by default, you can add explicit validation for clarity:
KeyValue::make('startup_commands') ->label(trans('admin/egg.startup_commands')) ->live() ->columnSpanFull() ->required() ->addActionLabel(trans('admin/egg.add_startup')) ->keyLabel(trans('admin/egg.startup_name')) ->keyPlaceholder('Default') ->valueLabel(trans('admin/egg.startup_command')) ->valuePlaceholder('java -Xms128M -XX:MaxRAMPercentage=95.0 -jar {{SERVER_JARFILE}}') + ->rules([ + function () { + return function (string $attribute, $value, $fail) { + if (is_array($value)) { + $keys = array_keys($value); + if (in_array('', $keys, true)) { + $fail('Startup command names cannot be empty.'); + } + if (count($keys) !== count(array_unique($keys))) { + $fail('Startup command names must be unique.'); + } + } + }; + }, + ]) ->helperText(trans('admin/egg.startup_help')),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
app/Filament/Admin/Resources/Eggs/Pages/CreateEgg.php(1 hunks)app/Filament/Admin/Resources/Eggs/Pages/EditEgg.php(1 hunks)app/Filament/Admin/Resources/Servers/Pages/CreateServer.php(2 hunks)app/Filament/Admin/Resources/Servers/Pages/EditServer.php(1 hunks)app/Filament/Server/Pages/Startup.php(3 hunks)app/Models/Egg.php(6 hunks)app/Services/Eggs/Sharing/EggExporterService.php(1 hunks)app/Services/Eggs/Sharing/EggImporterService.php(5 hunks)app/Services/Servers/ServerCreationService.php(1 hunks)app/Services/Servers/StartupModificationService.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- app/Services/Servers/ServerCreationService.php
- app/Services/Servers/StartupModificationService.php
- app/Services/Eggs/Sharing/EggExporterService.php
🧰 Additional context used
🧬 Code graph analysis (4)
app/Filament/Admin/Resources/Servers/Pages/CreateServer.php (1)
app/Models/Egg.php (1)
Egg(59-329)
app/Services/Eggs/Sharing/EggImporterService.php (1)
app/Models/Egg.php (1)
Egg(59-329)
app/Filament/Admin/Resources/Servers/Pages/EditServer.php (2)
app/Models/Server.php (1)
egg(295-298)app/Models/Egg.php (1)
Egg(59-329)
app/Filament/Server/Pages/Startup.php (5)
app/Models/Server.php (2)
Server(132-516)egg(295-298)app/Models/User.php (1)
can(357-370)app/Models/Permission.php (1)
Permission(11-221)app/Facades/Activity.php (1)
Activity(8-14)app/Services/Activity/ActivityLogService.php (3)
event(48-53)property(109-117)log(136-155)
🔇 Additional comments (1)
app/Filament/Admin/Resources/Servers/Pages/EditServer.php (1)
637-653: Approve startup field logic as-is. The null-coalescing ($egg->startup_commands ?? []) guarantees$startupsis always an array, and Filament’safterStateUpdatedonly fires when the state actually changes—preventing any infinite-loop risk.
notAreYouScared
left a comment
notAreYouScared
left a comment
There was a problem hiding this comment.
Seems to work in the testing I've done.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
app/Filament/Admin/Resources/Servers/Pages/EditServer.php (1)
623-635: Refactor state initialization out of the options closure.The options closure contains side effects (lines 628-631) that set form state during options computation. This is problematic because Filament may call the options closure multiple times during the form lifecycle (on re-renders, validation, etc.), leading to unexpected state resets.
Move the initialization logic to a
default()closure:Select::make('select_startup') ->label(trans('admin/server.startup_cmd')) ->live() + ->default(function (Get $get, Set $set) { + $egg = Egg::find($get('egg_id')); + $startups = $egg->startup_commands ?? []; + + if (empty($startups)) { + return ''; + } + + $currentStartup = $get('startup'); + if (!$currentStartup) { + $defaultStartup = collect($startups)->first(); + $set('startup', $defaultStartup); + return $defaultStartup; + } + + return in_array($currentStartup, $startups) ? $currentStartup : ''; + }) ->afterStateUpdated(fn (Set $set, $state) => $set('startup', $state)) ->options(function ($state, Get $get, Set $set) { $egg = Egg::query()->find($get('egg_id')); $startups = $egg->startup_commands ?? []; - - $currentStartup = $get('startup'); - if (!$currentStartup && $startups) { - $currentStartup = collect($startups)->first(); - $set('startup', $currentStartup); - $set('select_startup', $currentStartup); - } return array_flip($startups) + ['' => 'Custom Startup']; }) ->selectablePlaceholder(false) ->columnSpanFull() ->hintAction(PreviewStartupAction::make('preview')),app/Filament/Server/Pages/Startup.php (1)
56-68: Guard against undefined index errors in activity logging.Lines 64-66 perform
array_flipand direct array access that can fail if:
startup_commandsis null (though unlikely given validation, data integrity issues can occur)$originalis a custom startup value (possible in edge cases despite visibility logic)Apply defensive coding:
if ($original !== $server->startup) { - $startups = array_flip($server->egg->startup_commands); + $startups = array_flip($server->egg->startup_commands ?? []); Activity::event('server:startup.command') - ->property(['old' => $startups[$original], 'new' => $startups[$state]]) + ->property(['old' => $startups[$original] ?? $original, 'new' => $startups[$state] ?? $state]) ->log(); }This ensures the activity log gracefully falls back to the raw command if the label lookup fails.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/Filament/Admin/Resources/Servers/Pages/EditServer.php(1 hunks)app/Filament/Components/Actions/PreviewStartupAction.php(1 hunks)app/Filament/Server/Pages/Startup.php(3 hunks)lang/en/server/startup.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lang/en/server/startup.php
🧰 Additional context used
🧬 Code graph analysis (3)
app/Filament/Components/Actions/PreviewStartupAction.php (2)
app/Models/Server.php (1)
Server(132-516)app/Services/Servers/StartupCommandService.php (1)
StartupCommandService(7-30)
app/Filament/Admin/Resources/Servers/Pages/EditServer.php (3)
app/Models/Server.php (1)
egg(295-298)app/Models/Egg.php (1)
Egg(59-329)app/Filament/Components/Actions/PreviewStartupAction.php (1)
PreviewStartupAction(11-31)
app/Filament/Server/Pages/Startup.php (5)
app/Models/Server.php (4)
Server(132-516)egg(295-298)user(259-262)send(452-457)app/Filament/Components/Actions/PreviewStartupAction.php (1)
PreviewStartupAction(11-31)app/Models/Permission.php (1)
Permission(11-221)app/Facades/Activity.php (1)
Activity(8-14)app/Services/Activity/ActivityLogService.php (3)
event(48-53)property(109-117)log(136-155)
🔇 Additional comments (1)
app/Filament/Components/Actions/PreviewStartupAction.php (1)
22-29: Preview toggle logic correctly fixed.The action now properly restores
$server->startup(the persisted database value) when disabling preview, avoiding the state corruption issue flagged in the previous review. When enabling preview, it correctly sets a closure that calls the service to render variables.Minor UX consideration: In the EditServer context where the form is saved as a whole, if a user types a new startup command but hasn't saved the form yet, clicking preview will show the previously saved command rather than the typed value. This appears intentional—preview reflects the currently persisted state—but consider whether this might confuse users expecting to preview unsaved changes.
Closes #1655
Example with
Garry's Modegg:Egg:
Create Server:
Edit Server:
Client Area:
Activity log when user changes startup
Shown changes to the gmod egg are not included!