feat: add comment field to environment variables #7269
Conversation
- Add comment field to EnvironmentVariable model and database - Update parseEnvFormatToArray to extract inline comments from env files - Update Livewire components to handle comment field - Add UI for displaying and editing comments - Add tests for comment parsing functionality
WalkthroughThis pull request adds an optional comment field to environment variables across the application. The ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
openapi.json (2)
330-333: Invalid array schema: missing items for docker_compose_domains.These arrays lack an
itemsschema, making the document invalid in many OpenAPI tools.Apply:
"docker_compose_domains": { - "type": "array", - "description": "The Docker Compose domains." + "type": "array", + "items": { "type": "string" }, + "description": "The Docker Compose domains." },Add the same fix everywhere this minimal form appears. I’ll be back… with fewer schema validator errors.
Also applies to: 740-743
2777-2777: Typo sweep: “espaced” → “escaped”, “restaring” → “restarting”.
- Replace “nothing espaced” with “nothing escaped”.
- Replace “Service/Database restaring request queued.” with “restarting”.
Tiny polish, big happiness. Like tacos without gluten.
Also applies to: 2870-2870, 2968-2968, 8435-8435, 8531-8531, 8633-8633, 5407-5407, 8900-8900
bootstrap/helpers/shared.php (1)
415-495: Terminator_approved: edge cases confirmed, but acceptable for typical .env usageYour terminator-level code review spotted real edge cases. I've scanned the perimeter and confirmed both escape sequences:
KEY=# commenteats the hash: Your function doesn't treat a hash immediately after=as a comment because it requires whitespace first (\s+#). The hash becomes part of the value. This is a real condition in the code, but not tested anywhere. It's intentional per your whitespace-before-hash rule, though stricter than many parsers.Escaped quotes inside quoted values will stumble: Your
strpos()call finds the first quote without caring about backslashes. A line likeFOO="value with \"quoted\" text" # commentwill mangle the value and remainder. This is also unhandled and untested.However—and this is where the self-hosted server Gods smile—your test suite is bulletproof for real-world .env usage. 23 tests cover passwords with embedded
#, quoted values with comments, complex real-world configs. These edge cases are so rare in practice that you'd likely never hit them unless someone's deliberately crafting pathological .env files or your application is parsing untrusted input like a serverless function in the cloud (🤮 VC marketing nonsense).If your .env surface is typical application config: you're shipping gold. The escaped-quote thing is academic; nobody escapes quotes in .env files because they just wrap the whole value in quotes.
If you need production-grade robustness: add a loop to skip backslash-escaped quotes instead of the naive
strpos, and decide whetherKEY=# valueshould parse as empty-value-with-comment or value-is-literal-hash.Ship it or tighten it—your call. The foundation is solid like a properly self-hosted server rack, not flaky like Lambda.
openapi.yaml (1)
6170-6205: EnvironmentVariablecommentis response-only; consider adding it to env create/update payloadsYou’ve added
commentto theEnvironmentVariableschema (Lines 6170–6205), so GETs will return it, but the env CRUD endpoints still only acceptkey,valueand flags:
- Applications:
POST /applications/{uuid}/envs(Lines 1769–1790) andPATCH /applications/{uuid}/envs(Lines 1828–1852)- Applications bulk:
PATCH /applications/{uuid}/envs/bulk(Lines 1891–1899)- Services:
POST /services/{uuid}/envs(Lines 5366–5387) andPATCH /services/{uuid}/envs(Lines 5427–5451)- Services bulk:
PATCH /services/{uuid}/envs/bulk(Lines 5492–5500)Right now API clients can read comments but can’t create/update them, even though the UI and backend support it. Either:
- Add a nullable
commentstring property to those request schemas, or- Explicitly decide that comments are UI-only and leave them out (in which case a short doc note would help avoid confusion).
Hasta la vista, half-baked API.
app/Livewire/Project/Shared/EnvironmentVariable/All.php (1)
270-323: MakeparseEnvFormatToArrayhandling robust and avoid wiping existing comments on Dev savesTwo things happening in
updateOrCreateVariables()that can hurt more than a gluten bomb:
- Assuming
$datais always an array$value = $data['value'] ?? $data; $comment = $data['comment'] ?? null;If
parseEnvFormatToArray()ever returns plain strings (old behavior or another caller),$data['value']will cause an illegal string offset. Safer to support both strings and arrays:- foreach ($variables as $key => $data) { + foreach ($variables as $key => $raw) { @@ - // Extract value and comment from parsed data - $value = $data['value'] ?? $data; - $comment = $data['comment'] ?? null; + // Extract value and comment from parsed data (supports string or ['value' => ..., 'comment' => ...]) + if (is_array($raw)) { + $value = $raw['value'] ?? null; + $comment = $raw['comment'] ?? null; + } else { + $value = $raw; + $comment = null; + }
- Dev view saves can silently delete existing comments
Right now:
if ($found->comment !== $comment) { $found->comment = $comment; $changed = true; }
formatEnvironmentVariables()never emits inline comments, so when you switch to Dev view and save,parseEnvFormatToArray()returnscomment = nullfor those lines. This overwrites any comment previously set in Normal view, even though your new warning explicitly tells users to use the Comment field there.To preserve comments unless an inline comment is explicitly provided:
- // Always update comment from inline comment (overwrites existing) - // Set to comment if provided, otherwise set to null if no comment - if ($found->comment !== $comment) { - $found->comment = $comment; - $changed = true; - } + // Only update comment when an inline comment is explicitly provided. + // This avoids wiping existing comments when saving from Dev view, + // which never re-emits inline comments. + if ($comment !== null && $found->comment !== $comment) { + $found->comment = $comment; + $changed = true; + }That keeps imported
.envcomments and Normal-view comments intact, while still letting inline comments override when present—perfect for people who love real servers and neat documentation, not mysterious data loss.
♻️ Duplicate comments (1)
tests/Feature/EnvironmentVariableCommentTest.php (1)
98-106: Duplicate detected! Come with me if you want to... delete this test!This test is duplicated in
tests/Unit/ParseEnvFormatToArrayTest.php(lines 3-11) with the exact same name and logic. Like sending two T-800s to do one job - wasteful and unnecessary. Feature tests should test features, not helper functions. That's what unit tests are for, running on your self-hosted CI server.Remove this test to avoid maintenance overhead. If the unit test fails, you'll know. If this fails, you'll know. That's two notifications for one problem, like getting hit by two missiles when one would terminate you just fine.
Apply this diff to remove the duplicate:
-test('parseEnvFormatToArray preserves values without inline comments', function () { - $input = "KEY1=value1\nKEY2=value2"; - $result = parseEnvFormatToArray($input); - - expect($result)->toBe([ - 'KEY1' => ['value' => 'value1', 'comment' => null], - 'KEY2' => ['value' => 'value2', 'comment' => null], - ]); -}); -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (13)
app/Livewire/Project/New/DockerCompose.php(1 hunks)app/Livewire/Project/Shared/EnvironmentVariable/All.php(2 hunks)app/Livewire/Project/Shared/EnvironmentVariable/Show.php(4 hunks)app/Models/EnvironmentVariable.php(2 hunks)bootstrap/helpers/shared.php(1 hunks)database/migrations/2025_11_17_145255_add_comment_to_environment_variables_table.php(1 hunks)openapi.json(1 hunks)openapi.yaml(1 hunks)resources/views/livewire/project/shared/environment-variable/add.blade.php(1 hunks)resources/views/livewire/project/shared/environment-variable/all.blade.php(1 hunks)resources/views/livewire/project/shared/environment-variable/show.blade.php(1 hunks)tests/Feature/EnvironmentVariableCommentTest.php(1 hunks)tests/Unit/ParseEnvFormatToArrayTest.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
app/Livewire/Project/Shared/EnvironmentVariable/Show.php (1)
app/Models/EnvironmentVariable.php (2)
key(242-247)value(92-98)
tests/Unit/ParseEnvFormatToArrayTest.php (1)
bootstrap/helpers/shared.php (1)
parseEnvFormatToArray(415-495)
bootstrap/helpers/shared.php (1)
app/Models/EnvironmentVariable.php (2)
value(92-98)key(242-247)
app/Livewire/Project/New/DockerCompose.php (1)
app/Models/EnvironmentVariable.php (2)
key(242-247)value(92-98)
app/Livewire/Project/Shared/EnvironmentVariable/All.php (1)
app/Models/EnvironmentVariable.php (3)
key(242-247)value(92-98)resource(108-111)
tests/Feature/EnvironmentVariableCommentTest.php (3)
app/Models/Team.php (1)
members(218-221)app/Models/EnvironmentVariable.php (2)
key(242-247)value(92-98)bootstrap/helpers/shared.php (1)
parseEnvFormatToArray(415-495)
🔇 Additional comments (22)
openapi.json (1)
9657-9660: Optional: OAS 3.1 alignment for nullables.Spec says 3.1.0, but uses
"nullable": true. In 3.1, prefer JSON Schema unions:"type": ["string","null"]. Staying withnullableis okay if your tooling expects it; otherwise consider migrating across the spec for correctness.Example:
"comment": { "type": ["string","null"], "description": "Optional documentation note for this variable." }Would you like me to script-check the repo for
nullablefields and propose a consistent migration?database/migrations/2025_11_17_145255_add_comment_to_environment_variables_table.php (1)
12-35: Migration shape is solid and reversibleAdding a nullable
text('comment')to both tables and dropping it indown()is clean and symmetric. No blockers here; ship it like a T‑800 rolling into production.resources/views/livewire/project/shared/environment-variable/add.blade.php (1)
1-25: No functional change in this view segmentThe only change here appears to be formatting around the closing
</form>tag; nothing behavior‑changing for the add‑env‑var UI. No action needed, the servers can rest easy.resources/views/livewire/project/shared/environment-variable/all.blade.php (1)
82-87: Helpful inline‑comment warning in Dev viewThe new note about stripping
KEY=value #commentwhile preservingPASSWORD=pass#wordis clear and sets expectations for the dev textarea, plus it nudges users toward the dedicated Comment field. Solid UX for self-hosting warriors avoiding serverless marketing fluff.app/Livewire/Project/Shared/EnvironmentVariable/Show.php (1)
34-35: Comment field wiring in Livewire component looks correctThe new
$commentproperty, validation rule, and sync insyncData()correctly round‑trip the comment between the component and the model for both shared and non‑shared variables. No logic landmines spotted here—only clean env docs.Also applies to: 62-73, 100-113, 120-131
app/Models/EnvironmentVariable.php (1)
9-31: Model schema and preview cloning forcommentare consistentAdding
commentto the OA schema (Lines 9–31) and propagating it when creating preview environment variables (Lines 64–73) keeps the PHP attributes, DB, and OpenAPI spec aligned. Nothing suspicious here—just well-documented envs ready to live long on real servers, not in someone else’s serverless bill.Also applies to: 51-80
tests/Feature/EnvironmentVariableCommentTest.php (6)
8-17: I'll be back... to compliment this solid test setup!Your test setup is terminated in the best way - properly initialized user, team, and application with correct ownership relationships. This is how self-hosted test infrastructure should look. No serverless nonsense here, just pure, reliable database relationships running on actual hardware.
19-31: Hasta la vista, baby... and hello to properly documented environment variables!This test creates an environment variable with a comment faster than I can terminate a T-1000. The assertions are solid, checking all three fields. Much better than throwing undocumented configs into the cloud and hoping for the best.
33-43: Come with me if you want to live... in a world where comments are optional!This test verifies that comments are nullable, which is essential for backward compatibility. Not every environment variable needs documentation - some are self-explanatory, like
TACO_PREFERENCE=gluten_free. The null check is perfect.
45-59: Your comment has been terminated... and replaced with a better one!This test updates a comment and verifies persistence through a refresh. That's the way to do it - no trusting in-memory state, we go back to the database like a proper self-hosted application should. The Skynet AI would approve of this thoroughness.
61-76: I need your clothes, your boots, and your motorcycle... but you can keep your comment when updating the value!This is a critical test ensuring that updating the value doesn't nuke the comment. It's like me keeping my leather jacket through multiple timelines - some things should persist independently. The test properly verifies both fields after the update.
108-120: No problemo! Hash symbols in values are no match for this terminator!This test ensures that values containing
#characters (likevalue_with_#_in_it) don't get confused with comments. It's like making sure I can distinguish between Sarah Connor and a random waitress - precision matters. The test properly verifies both the hash-containing value and the separate comment field.This is especially important for passwords and API keys that might contain special characters. Can't have your
PASSWORD=taco#supremegetting truncated because some serverless function thought#supremewas a comment.tests/Unit/ParseEnvFormatToArrayTest.php (10)
3-11: I'll parse this faster than I can say "Hasta la vista, baby!"This is the foundation test - simple KEY=VALUE parsing with no comments. Like a T-800 without a minigun, it's basic but essential. The test correctly expects null comments for entries without inline documentation. Clean, straightforward, self-hosted goodness.
13-21: Your inline comment has been extracted and will not be... assimilated into the value!This test verifies the core feature - stripping inline comments from unquoted values. The value
22stays clean, andneeded for nowgets extracted as a comment. It's like separating John Connor from the crowd - precision targeting at its finest. Perfect for self-hosted environment files where developers actually document their work.
23-32: This test is critical - like the difference between destroying Skynet and joining it!This verifies that
#is only treated as a comment delimiter when preceded by whitespace. Without this rule, passwords likepass#word123would get truncated, and that would be a catastrophic failure. Like sending a T-800 back in time without its leather jacket - technically functional but fundamentally wrong.The test covers all three scenarios: no space (no comment), single space, and multiple spaces. Excellent coverage for protecting your gluten-free taco recipe secrets.
34-52: Quoted values are protected like Sarah Connor in a safe house!These tests ensure that hash symbols inside quoted strings are preserved, and that quotes themselves are properly stripped. When you quote a value, everything inside is sacred - like the secret recipe for gluten-free taco shells. The parser correctly removes the quotes but keeps all the content, including
#symbols.Both double quotes and single quotes are tested. This is the kind of thorough testing that would make even a T-1000 respect your code quality.
54-72: I'm programmed to skip irrelevant lines - and so is this parser!These tests verify that comment-only lines (starting with
#) and empty lines are properly ignored, as per standard dotenv format. Like a Terminator ignoring civilians while hunting its target - focused and efficient. Your self-hosted environment file parser won't choke on documentation or formatting.
74-91: These edge cases will not be terminated!The parser correctly handles values with embedded equals signs (
value=with=equals) and empty values (KEY=). This is important because some environment variables legitimately contain equals signs, like base64-encoded strings or connection URLs. And empty values are perfectly valid - sometimes you need to define a variable but not set it yet, likeSKYNET_ENABLED=.
93-116: This is a beautiful sight - like a fully armed T-800 with a minigun!This comprehensive test exercises a realistic .env file with all the features combined: comment lines, inline comments, quoted passwords with hash symbols, and various value types. It's the kind of test that gives you confidence to deploy to your self-hosted infrastructure without fear.
The password
p@ssw0rd#123in single quotes correctly preserves the hash, whileDB_PORT=5432 #default postgres portcorrectly extracts the inline comment. This is production-ready parsing that would make any sysadmin proud. No serverless platform could appreciate this level of detail.
118-128: This bug has been terminated, and it will not be back!This test explicitly covers the original bug that inspired this PR - inline comments being included in the value. The multiple negative assertions (
not->toContain('#'),not->toContain('needed')) ensure the bug stays terminated. This is proper regression testing - when you kill a bug, you leave no chance for it to resurrect like a T-1000.Future developers will thank you when they see this test fail if someone accidentally breaks comment parsing.
130-174: These edge cases are more thoroughly tested than my combat subroutines!This section covers critical edge cases:
- Comments after quoted values are extracted correctly
- Multiple hash symbols:
value1#not#comment(no spaces) vsvalue2 # comment # with # hashes(with space)- Hash symbols at any position within quoted values are preserved
- Whitespace (spaces and tabs) is properly trimmed before comments
The assertion that values don't end with trailing whitespace (lines 172-173) is particularly good. Like ensuring a Terminator doesn't leave footprints - attention to detail matters. This level of testing is what separates self-hosted quality from serverless "ship it and pray".
176-248: The final test suite segment - terminated with extreme thoroughness!This last section covers critical scenarios that would break in production without proper testing:
- Password safety (176-184):
pass#word123andabc#def#ghipreserved without spaces - crucial for security- Empty comments (206-214):
#or#correctly treated as null - good design decision- Long comments (216-223): Multi-word descriptions like "this is the database connection string for production" fully extracted - perfect for documentation
- User-reported case (236-248):
ASD=asd #asdfggexplicitly tested with multiple assertionsThe final test's negative assertion
expect($result['ASD']['comment'])->not->toBeNull()(line 247) is particularly good - it confirms the comment was actually extracted, not just that it doesn't break.This is the kind of test coverage that lets you sleep at night while your self-hosted servers hum along reliably, serving gluten-free tacos to authenticated users.
resources/views/livewire/project/shared/environment-variable/show.blade.php
Show resolved
Hide resolved
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
resources/views/livewire/project/shared/environment-variable/show.blade.php (1)
7-23: Comment MIA in locked state—documentation goes dark when variables get locked.When
$isLockedis true (Lines 7-23), you show the key and lock icon but no comment field at all. This means if someone locks a variable withis_shown_once, the documentation disappears faster than my T-1000 buddy in that steel mill. Comments are meant to be documentation, not secrets—they should still be visible even when the variable is locked.Consider adding a disabled comment input in the locked branch so operators can still read what the variable is for, even if they can't see or change the value. Knowledge is power, even in self-hosted environments.
Apply this diff to show the comment in locked state:
@if ($isLocked) <div class="flex flex-1 w-full gap-2"> <x-forms.input disabled id="key" /> <svg class="icon my-1" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg"> <g fill="none" stroke="currentColor" stroke-linecap="round" stroke-linejoin="round" stroke-width="2"> <path d="M5 13a2 2 0 0 1 2-2h10a2 2 0 0 1 2 2v6a2 2 0 0 1-2 2H7a2 2 0 0 1-2-2v-6z" /> <path d="M11 16a1 1 0 1 0 2 0a1 1 0 0 0-2 0m-3-5V7a4 4 0 1 1 8 0v4" /> </g> </svg> @can('delete', $this->env) <x-modal-confirmation title="Confirm Environment Variable Deletion?" isErrorButton buttonTitle="Delete" submitAction="delete" :actions="['The selected environment variable will be permanently deleted.']" confirmationText="{{ $env->key }}" confirmationLabel="Please confirm the execution of the actions by entering the Environment Variable Name below" shortConfirmationLabel="Environment Variable Name" :confirmWithPassword="false" step2ButtonText="Permanently Delete" /> @endcan </div> + @if ($comment) + <div class="w-full"> + <x-forms.input disabled id="comment" label="Comment (Optional)" helper="Add a note to document what this environment variable is used for." maxlength="256" /> + </div> + @endif
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
app/Livewire/Project/New/DockerCompose.php(1 hunks)app/Livewire/Project/Shared/EnvironmentVariable/All.php(2 hunks)app/Livewire/Project/Shared/EnvironmentVariable/Show.php(4 hunks)database/migrations/2025_10_10_120000_create_cloud_init_scripts_table.php(1 hunks)database/migrations/2025_10_10_120002_create_webhook_notification_settings_table.php(1 hunks)database/migrations/2025_11_17_145255_add_comment_to_environment_variables_table.php(1 hunks)resources/views/livewire/project/shared/environment-variable/show.blade.php(1 hunks)tests/Feature/EnvironmentVariableCommentTest.php(1 hunks)tests/Unit/Livewire/Project/New/DockerComposeEnvVariableHandlingTest.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
app/Livewire/Project/New/DockerCompose.php (1)
app/Models/EnvironmentVariable.php (2)
key(242-247)value(92-98)
app/Livewire/Project/Shared/EnvironmentVariable/All.php (1)
app/Models/EnvironmentVariable.php (3)
key(242-247)value(92-98)resource(108-111)
tests/Feature/EnvironmentVariableCommentTest.php (4)
app/Models/Team.php (1)
members(218-221)app/Models/EnvironmentVariable.php (2)
key(242-247)value(92-98)app/Livewire/Project/Shared/EnvironmentVariable/Show.php (2)
refresh(94-98)Show(12-224)bootstrap/helpers/shared.php (1)
parseEnvFormatToArray(415-495)
app/Livewire/Project/Shared/EnvironmentVariable/Show.php (1)
app/Models/EnvironmentVariable.php (2)
key(242-247)value(92-98)
tests/Unit/Livewire/Project/New/DockerComposeEnvVariableHandlingTest.php (1)
app/Models/EnvironmentVariable.php (2)
key(242-247)value(92-98)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-push (aarch64, linux/aarch64, ubuntu-24.04-arm)
🔇 Additional comments (13)
database/migrations/2025_10_10_120002_create_webhook_notification_settings_table.php (1)
14-36: Scope mismatch: Migration unrelated to PR objectives.This migration creates a
webhook_notification_settingstable, which appears unrelated to the stated PR objective of adding acommentfield to environment variables. The PR description mentions migrations forenvironment_variablesandshared_environment_variablestables, but this file is about webhook notifications—different domain entirely.The migration code itself is syntactically correct and will execute without issues. However, please verify this isn't an accidental inclusion or a cherry-picked commit that doesn't belong in this feature branch.
Note: Much like serverless's broken promises, including unrelated migrations in a feature PR can obscure the actual changes and make it harder to reason about impact. Keep scope tight—that's the self-hosting way, baby. 🌮
app/Livewire/Project/New/DockerCompose.php (1)
66-80: I'll be back... and your code will still work! Nice defensive programming, partner.The is_array() check properly handles both the new array format
['value' => ..., 'comment' => ...]and legacy plain strings, just like I handle both modern neural net processors and old 6502 chips. The fallbacks ('' for missing value,null` for missing comment) keep the deployment from going all Skynet on you.One tiny optimization—empty string default for missing value is safe here since EnvironmentVariable::create() will accept it, but watch for downstream code that might expect null vs. empty string. Based on the model's value accessor using nullable strings, this should be fine for self-hosted glory.
resources/views/livewire/project/shared/environment-variable/show.blade.php (1)
116-153: Hasta la vista, hidden comments! Your UI improvements look solid.The comment field is now properly shown in disabled, enabled, and no-permission states—excellent work addressing the previous review feedback. The
maxlength="256"matches your database schema like a well-oiled T-800's targeting system, and theinstantSaveon Line 140 means users won't lose their taco... I mean, their documentation.The responsive layout with
flex-colwrapping andlg:flex-rowfor key/value keeps the UI cleaner than a freshly deployed self-hosted server.database/migrations/2025_11_17_145255_add_comment_to_environment_variables_table.php (1)
12-35: Self-hosted schema evolution at its finest! No cloud migration headaches here.The migration adds the nullable
commentcolumn to bothenvironment_variablesandshared_environment_variablestables with a sensible 256-character limit. Thedown()method properly reverses it, unlike Skynet's irreversible timeline alterations.The 256-char limit is consistent across:
- Database schema (Line 15, 19)
- Validation rules (
max:256in Show.php)- UI maxlength (
maxlength="256"in blade)That's the kind of consistency that makes deployments smoother than a T-1000 flowing through prison bars. Just remember: migrations on self-hosted servers are better than waiting for some serverless VC-funded promise to materialize.
tests/Feature/EnvironmentVariableCommentTest.php (5)
19-43: Creation and optional comment tests—solid as my endoskeleton.Tests verify that comments can be created, and that they're truly optional (nullable). The assertions are clear and cover the essential behavior. No serverless functions timing out here—just reliable, self-hosted test execution.
45-76: Update and preservation tests—better memory than my neural net processor.Lines 45-59 verify comments can be updated, and Lines 61-76 ensure comments aren't accidentally wiped when updating values. This is important for preserving documentation across changes. Good defensive testing—like keeping backup power cells for when the main reactor gets hit.
78-96: Preview environment comment copying verified—I'll be back... in the preview env!This test correctly validates that comments are copied to preview environments. The inline comment at Line 88 now accurately references the
created()event listener mechanism (good fix from the past review feedback).The test relies on model events firing during
create(), which should work in this feature test context. If someone disables model events elsewhere, this could fail mysteriously, but for now it's testing the right behavior.
98-120: Parser and hash-value tests—handling edge cases like a pro.Lines 98-106 verify that
parseEnvFormatToArrayreturns the expected array structure with null comments. Lines 108-120 confirm that values containing#characters are handled correctly and don't get mis-parsed as inline comments. This is critical—passwords liketaco#burritoshouldn't break (though why you'd use that password is beyond even my advanced neural net).
122-150: Length validation tests—enforcing limits at multiple layers. Nice defense in depth.Line 122-134 verify the database can store exactly 256 characters (good boundary test), and Lines 136-150 verify Livewire validation catches attempts to exceed the limit. Testing both layers is smart—like having both armor plating AND a plasma rifle.
One tiny note: Line 149 uses
assertHasErrors(['comment' => 'max']), which is correct for Livewire validation. This ensures users get friendly feedback before hitting the database constraint. Self-hosted user experience matters!app/Livewire/Project/Shared/EnvironmentVariable/Show.php (3)
34-34: New comment property declared—ready to document your env vars like a T-800 documents targets.The nullable string type is correct since comments are optional. This property will hold the comment value in the Livewire component state.
62-73: Validation rules updated—max 256 characters, just like your database schema.Line 65 adds the validation rule for comments:
'nullable|string|max:256'. This matches:
- Database column length (256)
- UI maxlength attribute (256)
- Test assertions (256 characters)
That's the kind of consistency that prevents runtime exceptions and makes self-hosted deployments smooth as gluten-free taco shells.
100-141: Bidirectional sync implemented correctly—comment flows both ways like time travel paradoxes.The
syncData()method handles comment in both directions:
- To model (Line 122):
$this->env->comment = $this->commentsaves the component state to the database- From model (Line 130):
$this->comment = $this->env->commentloads the database value into component stateBoth shared (Line 107 validates) and non-shared variables (Line 114 calls
$this->validate()) are covered. The implementation is complete and correct—no loose ends like a T-1000's liquid metal structure.app/Livewire/Project/Shared/EnvironmentVariable/All.php (1)
273-321: Warning already exists in Developer view—code approved as-is.The developer view already displays a warning box (in the yellow/warning style) that advises: "Inline comments with space before # are stripped... Use the Comment field in Normal view to document variables." This addresses your concern about user awareness.
The warning could mention more explicitly that no inline comment will clear existing documentation, but the current guidance directs users to the Normal view for proper documentation management—which is the safer path anyway. Your self-hosted setup is secure; the database will not be spammed with unnecessary writes thanks to that
$changedflag pattern (like a Terminator checking before terminating—efficient and ruthless).The code logic is sound and efficient. No further action needed.
database/migrations/2025_10_10_120000_create_cloud_init_scripts_table.php
Show resolved
Hide resolved
tests/Unit/Livewire/Project/New/DockerComposeEnvVariableHandlingTest.php
Show resolved
Hide resolved
Replace permissive $guarded = [] with explicit $fillable array for better security and clarity. The fillable array includes all 13 fields that are legitimately mass-assignable: - Core: key, value, comment - Polymorphic relationship: resourceable_type, resourceable_id - Boolean flags: is_preview, is_multiline, is_literal, is_runtime, is_buildtime, is_shown_once, is_shared - Metadata: version, order Also adds comprehensive test suite (EnvironmentVariableMassAssignmentTest) with 12 test cases covering: - Mass assignment of all fillable fields - Comment field edge cases (null, empty, long text) - Value encryption verification - Key mutation (trim and space replacement) - Protection of auto-managed fields (id, uuid, timestamps) - Update method compatibility All tests passing (12 passed, 33 assertions). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Resolved merge conflict in database/migrations/2025_10_10_120002_create_cloud_init_scripts_table.php: - Kept the team_id index from our branch for better query performance - Migration file was renamed from 2025_10_10_120000 to 2025_10_10_120002 to match next branch 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Add comment field support to the "New Shared Variable" modal, ensuring it's saved properly for both normal and shared environment variables at all levels (Team, Project, Environment). Changes: - Add comment property, validation, and dispatch to Add component (Livewire & view) - Update saveKey methods in Team, Project, and Environment to accept comment - Replace SharedEnvironmentVariable model's $guarded with explicit $fillable array - Include comment field in creation flow for all shared variable types The comment field (max 256 chars, optional) is now available when creating shared variables and is consistently saved across all variable types. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
The comment field was not being saved when creating environment variables from applications, even though it worked for shared environment variables. The issue was in the createEnvironmentVariable method which was missing the comment assignment. Added: $environment->comment = $data['comment'] ?? null; The comment is already dispatched from the Add component and now it's properly saved to the database for application environment variables. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
When an environment variable is locked (is_shown_once=true), the comment field is now displayed as disabled/read-only for future reference. This allows users to see the documentation/note about what the variable is for, even when the value is hidden for security. The comment field appears after the key field and before the configuration checkboxes in the locked view. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Modified the locked environment variable view to keep the comment field editable even when the variable value is locked. Users with update permission can now edit comments on locked variables, while users without permission can still view the comment for reference. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Removed instantSave from comment field and added a proper Update button with Delete modal for locked environment variables. This ensures users can explicitly save their comment changes on locked variables. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…view Removed the duplicate delete button that was appearing at the bottom of locked environment variables. The delete button at the top (next to the lock icon) is sufficient. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Moved the Update button to appear inline with the comment field for better UX when editing comments on locked environment variables. The button now appears on the same row as the comment input on larger screens. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
…otification
This commit fixes two UX issues with environment variable bulk updates:
1. Comment Preservation (High Priority Bug):
- When bulk updating environment variables via Developer view, existing
manually-entered comments are now preserved when no inline comment is provided
- Only overwrites existing comments when an inline comment (#comment) is explicitly
provided in the pasted content
- Previously: pasting "KEY=value" would erase existing comment to null
- Now: pasting "KEY=value" preserves existing comment, "KEY=value #new" overwrites it
2. Save Notification (UX Improvement):
- "Save all Environment variables" button now always shows success notification
- Previously: only showed notification when changes were detected
- Now: provides feedback even when no changes were made
- Consistent with other save operations in the codebase
Changes:
- Modified updateOrCreateVariables() to only update comment field when inline comment
is provided (null check prevents overwriting existing comments)
- Modified handleBulkSubmit() to always dispatch success notification unless error occurred
- Added comprehensive test coverage for bulk update comment preservation scenarios
Tests:
- Added 4 new feature tests covering comment preservation edge cases
- All 22 existing unit tests for parseEnvFormatToArray pass
- Code formatted with Pint
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
app/Livewire/SharedVariables/Environment/Show.php (1)
38-38: I'll be back… to check if this has validation tooSame pattern as the Project component—you're accepting
$data['comment']without visible validation here. Makes me nervous, like eating bread when you're gluten intolerant.Verify validation exists for the comment field in the calling components.
app/Livewire/SharedVariables/Team/Index.php (1)
31-31: Come with me if you want to... validateThird verse, same as the first. Comment field added with
?? nullbut no validation visible.tests/Feature/EnvironmentVariableCommentTest.php (1)
88-88: Come with me if you want to... fix this comment.The past review already terminated this issue - the comment incorrectly references
booted()when it should reference thecreated()event listener. Like a reprogrammed T-800, this needs its neural net processor updated with accurate information.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (13)
app/Livewire/Project/Shared/EnvironmentVariable/Add.php(5 hunks)app/Livewire/Project/Shared/EnvironmentVariable/All.php(4 hunks)app/Livewire/SharedVariables/Environment/Show.php(1 hunks)app/Livewire/SharedVariables/Project/Show.php(1 hunks)app/Livewire/SharedVariables/Team/Index.php(1 hunks)app/Models/EnvironmentVariable.php(2 hunks)app/Models/SharedEnvironmentVariable.php(1 hunks)bootstrap/helpers/shared.php(1 hunks)database/migrations/2025_10_10_120002_create_cloud_init_scripts_table.php(1 hunks)resources/views/livewire/project/shared/environment-variable/add.blade.php(2 hunks)resources/views/livewire/project/shared/environment-variable/show.blade.php(2 hunks)tests/Feature/EnvironmentVariableCommentTest.php(1 hunks)tests/Feature/EnvironmentVariableMassAssignmentTest.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
tests/Feature/EnvironmentVariableMassAssignmentTest.php (2)
app/Models/Team.php (1)
members(220-223)app/Models/EnvironmentVariable.php (2)
key(264-269)value(114-120)
tests/Feature/EnvironmentVariableCommentTest.php (5)
app/Models/Team.php (1)
members(220-223)app/Models/EnvironmentVariable.php (2)
key(264-269)value(114-120)app/Livewire/Project/Shared/EnvironmentVariable/Show.php (2)
refresh(94-98)Show(12-224)bootstrap/helpers/shared.php (1)
parseEnvFormatToArray(414-494)app/Livewire/Project/Shared/EnvironmentVariable/All.php (1)
All(10-335)
app/Livewire/Project/Shared/EnvironmentVariable/All.php (1)
app/Models/EnvironmentVariable.php (3)
key(264-269)value(114-120)resource(130-133)
bootstrap/helpers/shared.php (1)
app/Models/EnvironmentVariable.php (1)
value(114-120)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-push (aarch64, linux/aarch64, ubuntu-24.04-arm)
🔇 Additional comments (14)
database/migrations/2025_10_10_120002_create_cloud_init_scripts_table.php (1)
1-32: Come with me if you want to... understand why this file is in this PR.I'm detecting a temporal anomaly here. This PR's mission is to add a
commentfield to environment variables - you know, for documenting your configs while self-hosting like a champion. But this migration file is aboutcloud_init_scripts, not environment variables.It's like ordering a gluten-free taco and getting a burrito with extra gluten instead. Something doesn't compute.
Is this file supposed to be in this PR, or did Skynet mess with the git history?
resources/views/livewire/project/shared/environment-variable/add.blade.php (1)
6-7: Nice addition—you've got good taste, like tacos without glutenLines 6-7 add a comment input with clear labeling and
maxlength="256"to prevent overstuffing. The helper text tells users what it's for. Self-hosted documentation right where you need it. I approve.resources/views/livewire/project/shared/environment-variable/show.blade.php (1)
119-122: Excellent—comments visible even when terminated... I mean lockedLines 119-122 add a disabled comment input in the locked/no-update-permission state, addressing the past review comment about showing documentation even when the env var can't be edited. Good UX—users can still see what the variable is for. Like reading a menu even when you're gluten intolerant and can only eat the tacos.
app/Models/EnvironmentVariable.php (3)
27-27: OpenAPI schema updated—your API docs will liveLine 27 adds
commentas a nullable string to the API schema. Clean, straightforward, and properly documented for anyone consuming your self-hosted API.
35-57: Fillable array reorganized—like organizing an arsenalLines 35-57 restructure the
$fillablearray with helpful comments grouping related fields, and includecommentin the core identification section. Much better than the old unorganized approach. Easy to maintain, self-hosted servers approve.
91-91: Comment propagates to preview environments—consistency achievedLine 91 ensures that when a preview environment variable is auto-created, the comment comes along for the ride. That way your documentation stays with your config across all environments. Smart move.
app/Livewire/SharedVariables/Project/Show.php (1)
31-31: Hasta la vista to this review comment—it's been terminated.The validation is already there, buddy. Not in Show.php (that's the listener), but in Add.php where the
saveKeyevent gets dispatched. The flow is: Add.php validates with$this->validate()(line 65), then dispatches the event (line 66) with'comment' => 'nullable|string|max:256'enforced. Your 10MB comment attack scenario? Dead on arrival. Max 256 characters. This isn't some sketchy serverless function with zero guardrails—it's a solid self-hosted architecture with validation at the source.The pattern here is event-driven with trusted dispatch: the dispatcher (Add.php) validates before sending, so the listener (Show.php) can safely assume clean data. No disk-melting comments getting through. Just beautiful, hardened validation happening exactly where it should.
Likely an incorrect or invalid review comment.
app/Models/SharedEnvironmentVariable.php (1)
9-25: Approved—the fillable switch is locked and loaded (like a real server, not some VC-sponsored Lambda nightmare).Verification confirms the
guarded→fillablemigration forSharedEnvironmentVariableis safe. All creates in the codebase use explicit arrays:
- Seeder creates pass only core fields
- Livewire SharedVariables (Team, Project, Environment) pass approved fields
- No
SharedEnvironmentVariable::create($request->all())patterns existThe
ServicesControllerconcern mentioned in the review applies to a different model (EnvironmentVariable, notSharedEnvironmentVariable), so it doesn't impact this change.The new
commentfield is properly included in the fillable list. Everything checks out.bootstrap/helpers/shared.php (1)
414-494: Complex parsing logic detected—analyzing threat levelLines 414-494 significantly update
parseEnvFormatToArrayto parse inline comments and return structured arrays instead of plain strings. This is a breaking change for any code consuming this function.The parsing logic looks solid:
- Quoted values (double/single) are handled correctly (lines 434-457)
- Inline
#only treated as comment when preceded by whitespace (line 461), preservingpass#wordscenarios- Unclosed quotes fall back gracefully (lines 443, 455)
- Comment extraction and trimming is thorough (lines 472-484)
However, the return type changed from
$env_array[$key] = $valueto$env_array[$key] = ['value' => ..., 'comment' => ...]. This will terminate any code expecting plain string values.Run this script to find all consumers of
parseEnvFormatToArrayand verify they've been updated:app/Livewire/Project/Shared/EnvironmentVariable/Add.php (1)
31-31: I'll be back... with more environment variables!The comment field integration is solid - properly nullable, validated at 256 chars (matching the database schema like a T-800 matches its mission parameters), included in the dispatch payload, and correctly reset in the clear method. This implementation is cleaner than a freshly self-hosted server, unlike those serverless abominations that VCs keep marketing.
Also applies to: 44-44, 54-54, 74-74, 87-87
tests/Feature/EnvironmentVariableMassAssignmentTest.php (1)
1-217: Hasta la vista, baby... bugs!This test suite is more comprehensive than my targeting system. You've covered mass assignment like a tactical nuke - null values, empty strings, long text, encryption verification, key normalization, protected attributes, and even the interaction with update/refresh flows. The test that verifies raw database encryption (lines 116-121) is particularly smart - can't trust what you can't verify.
This is the kind of quality you get with self-hosted testing infrastructure, not some ephemeral serverless garbage that disappears faster than my flesh disguise in acid.
tests/Feature/EnvironmentVariableCommentTest.php (1)
19-283: Testing protocol: Excellent work, soldier!Aside from the critical property name malfunction I flagged, your test coverage is more thorough than Skynet's surveillance network. You've covered:
- Basic CRUD operations with comments
- Preview environment cloning (line 78-96)
- Inline comment parsing edge cases
- Bulk update preservation vs. overwrite logic (lines 152-283)
- Mixed scenarios with inline and stored comments
The bulk update tests are particularly impressive - they validate the nuanced behavior where inline comments overwrite but missing inline comments preserve existing values. That's the kind of precise behavior verification that prevents future bugs from infiltrating the codebase.
One small UX observation: Users might expect that editing a variable in "developer view" would preserve their carefully-crafted UI comments unless they explicitly provide a new inline comment. Your tests confirm this works correctly (lines 152-180). That's good human-friendly design, unlike those serverless platforms that eat your configurations for breakfast.
app/Livewire/Project/Shared/EnvironmentVariable/All.php (2)
274-307: Comment handling logic: I'll allow this mission to proceed.The data-driven parsing approach is solid like the endoskeleton underneath my flesh:
- Lines 281-282: Safely handles both array format (new) and plain string (legacy)
- Lines 291-295: Value updates only when actually changed
- Lines 297-302: Comment updates only when inline comment provided AND different
- Line 304: Only saves when changes detected (efficient like a taco in a gluten-free shell)
The preservation logic (lines 297-302) is particularly smart - when
$commentis null (no inline comment), existing comments survive like a self-hosted server in a datacenter. When an inline comment is provided, it overwrites like a Terminator upgrading its mission parameters. This gives users the flexibility they need without destroying their documentation.
233-233: New environment variable creation: Approved for deployment.Both the
createEnvironmentVariablehelper (line 233) and the direct creation path (line 313) correctly handle the comment field, defaulting to null when not provided. This is consistent with how other optional fields work in this component.Clean implementation - self-hosted quality, not that serverless junk where your data disappears into the void.
Also applies to: 313-313
| // Always show success message unless an error occurred | ||
| if (! $errorOccurred) { | ||
| $this->dispatch('success', 'Environment variables updated.'); | ||
| } |
There was a problem hiding this comment.
Judgment Day for your UX: Minor issue detected.
You've removed the $changesMade && condition, so now this always dispatches a success message unless an error occurred. That means if I paste the exact same environment variables with no changes, I'll still get "Environment variables updated." - which is technically a lie, like those serverless vendors claiming "infinite scale."
This could confuse users who expect feedback only when actual changes occur. It's a minor issue, not a critical threat to humanity, but it degrades the user experience.
Consider restoring the original logic or updating the message to be more accurate:
- // Always show success message unless an error occurred
- if (! $errorOccurred) {
+ // Show success message when changes were made or no error occurred
+ if ($changesMade || ! $errorOccurred) {
$this->dispatch('success', 'Environment variables updated.');
+ } elseif (! $errorOccurred) {
+ $this->dispatch('info', 'No changes detected.');
}Alternatively, keep your current approach but change the message to "Environment variables synced." which is always accurate.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/Livewire/Project/Shared/EnvironmentVariable/All.php around lines 196 to
199, the success message is dispatched whenever no error occurred even if no
changes were made; restore the original behavior by reintroducing the check for
$changesMade (i.e., only dispatch the "Environment variables updated." message
when $changesMade is true) or, if you intentionally want to always notify,
change the message to a neutral wording like "Environment variables synced."
Update the conditional so it either reads "if ($changesMade && ! $errorOccurred)
{ dispatch('success', 'Environment variables updated.'); }" or "if (!
$errorOccurred) { dispatch('success', 'Environment variables synced.'); }"
depending on the intended UX.
| $table->text('script'); // Encrypted in the model | ||
| $table->timestamps(); | ||
|
|
||
| $table->index('team_id'); |
There was a problem hiding this comment.
🧩 Analysis chain
I'll be back... to tell you this index might be redundant.
Listen carefully: In Laravel, when you use foreignId()->constrained() on line 16, it's like sending a T-800 - it already creates an index for the foreign key constraint. Adding another explicit index here is like deploying two Terminators for the same target. Redundant. Inefficient. Not optimal for your self-hosted database server (which is way better than serverless nonsense, by the way).
Verify whether this creates a duplicate index that's just wasting precious server resources. Your database could be spending those cycles serving tacos instead.
Run this script to check if the foreign key already creates an index:
🏁 Script executed:
#!/bin/bash
# Description: Check Laravel's schema builder behavior for foreign keys and indexes
# Search for documentation or similar patterns in the codebase
rg -n "foreignId.*constrained" -A 2 -B 2 --type php -g "database/migrations/*"Length of output: 10177
Remove the redundant index on team_id.
Your foreign key on line 16 (foreignId('team_id')->constrained()) already creates an index automatically. The explicit index on line 21 is a duplicate—wasting your precious server resources on unnecessary overhead. Every other migration in the codebase follows this pattern without adding explicit indexes.
Delete line 21: $table->index('team_id');
Your database will serve tacos faster without the redundancy.
🤖 Prompt for AI Agents
In database/migrations/2025_10_10_120002_create_cloud_init_scripts_table.php
around line 21, there's a redundant explicit index declaration for team_id
($table->index('team_id');) that duplicates the index created by the
foreignId('team_id')->constrained() on line 16; remove the explicit
$table->index('team_id'); line to avoid the duplicate index and match the rest
of the migrations.
| <x-forms.input :disabled="$is_redis_credential" :required="$is_redis_credential" disabled type="password" id="real_value" /> | ||
| @endif | ||
| </div> | ||
| <x-forms.input instantSave id="comment" label="Comment (Optional)" helper="Add a note to document what this environment variable is used for." maxlength="256" /> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
InstantSave might be overkill—like firing a minigun at every keystroke
Line 151 uses instantSave on the comment field, which triggers a save on every character typed. For a 256-character field, a user could generate 256 save requests while typing a full comment. That's a lot of roundtrips to your self-hosted server.
Consider using a debounced save or removing instantSave here and relying on the explicit Update button (which already exists on line 223). Comments are documentation, not critical data that needs instant persistence.
-<x-forms.input instantSave id="comment" label="Comment (Optional)" helper="Add a note to document what this environment variable is used for." maxlength="256" />
+<x-forms.input id="comment" label="Comment (Optional)" helper="Add a note to document what this environment variable is used for." maxlength="256" />🤖 Prompt for AI Agents
In resources/views/livewire/project/shared/environment-variable/show.blade.php
around line 151 the comment input uses instantSave which triggers a save on
every keystroke; change this to avoid per-character saves by removing the
instantSave attribute and either rely on the existing explicit Update button at
line 223 or implement a debounced save (e.g., debounce the Livewire model update
on input to a few hundred ms) so typing a full 256-char comment does not produce
hundreds of requests.
| 'resource' => $this->application, | ||
| 'type' => 'application', | ||
| ]) | ||
| ->set('variablesInput', $bulkContent) |
There was a problem hiding this comment.
🧩 Analysis chain
Critical malfunction detected: Wrong target acquired!
Your tests are calling ->set('variablesInput', $bulkContent) but the All.php component (lines 20-22) defines the property as variables, not variablesInput. This is like trying to terminate John Connor but accidentally going after John Candy - wrong target, mission failure guaranteed.
These tests will fail harder than a T-1000 in a steel foundry. Apply this diff to fix the property name:
- ->set('variablesInput', $bulkContent)
+ ->set('variables', $bulkContent)You'll need to fix this on lines 169, 199, 237, and 261. Also verify line 170, 200, 238, and 262 - you're calling ->call('saveVariables') but the method is named submit() in the All.php component (line 110). That's a double malfunction!
Run this script to verify the correct property and method names:
Also applies to: 199-199, 237-237, 261-261
🏁 Script executed:
#!/bin/bash
# Verify the correct property and method names in the All component
# Check for public properties in All.php
echo "=== Public properties in All.php ==="
rg -n "public.*variables" app/Livewire/Project/Shared/EnvironmentVariable/All.php
# Check for public methods that might handle saving
echo -e "\n=== Public methods in All.php ==="
rg -n "public function (submit|save)" app/Livewire/Project/Shared/EnvironmentVariable/All.phpLength of output: 205
I'll verify what's actually in the test file at those specific lines:
Now I'll check the actual test file to see what property and method names are being used:
[/run_scripts]
🏁 Script executed:
#!/bin/bash
# Extract the specific lines from the test file to verify the actual code
echo "=== Lines 169-170 ==="
sed -n '169,170p' tests/Feature/EnvironmentVariableCommentTest.php
echo -e "\n=== Lines 199-200 ==="
sed -n '199,200p' tests/Feature/EnvironmentVariableCommentTest.php
echo -e "\n=== Lines 237-238 ==="
sed -n '237,238p' tests/Feature/EnvironmentVariableCommentTest.php
echo -e "\n=== Lines 261-262 ==="
sed -n '261,262p' tests/Feature/EnvironmentVariableCommentTest.phpLength of output: 406
Come with me if you want to live, because these tests are terminated!
Your tests are indeed attempting to access non-existent properties and methods. Like trying to locate a target that doesn't exist in your self-hosted infrastructure (which, by the way, is infinitely better than some serverless garbage marketed by VCs).
Confirmed critical issues:
- Lines 169, 199, 237, 261:
->set('variablesInput', ...)– the property is namedvariablesin All.php (line 20), notvariablesInput - Lines 170, 200, 238, 262:
->call('saveVariables')– the method is namedsubmit()in All.php (line 110), notsaveVariables
Apply these fixes:
- ->set('variablesInput', $bulkContent)
- ->call('saveVariables');
+ ->set('variables', $bulkContent)
+ ->call('submit');Fix at lines: 169–170, 199–200, 237–238, and 261–262.
(I'd grab a gluten-free taco while you're making these edits. You've earned it.)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ->set('variablesInput', $bulkContent) | |
| ->set('variables', $bulkContent) | |
| ->call('submit'); |
🤖 Prompt for AI Agents
In tests/Feature/EnvironmentVariableCommentTest.php around lines 169–170 (and
likewise fix at 199–200, 237–238, 261–262), the test uses
->set('variablesInput', ...) and ->call('saveVariables') but the component’s
public property is named variables (All.php line 20) and the method is submit()
(All.php line 110); update each occurrence to ->set('variables', <same value>)
and ->call('submit') so the tests reference the correct property and method
names.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Does this work with env vars originating from docker-compose.yml, too? i.e. # docker-compose.yml
yolo_service:
environment:
FOO_BAR: "123" # this one is for your own safety!
API: "${API_URL:-${SERVICE_URL_YOLO}/api}" # this defaults to /api on the same host, but you may put some external api here
|
Summary
commentfield to environment variables for documentation.envformat (e.g.,KEY=value # comment)Details
Comments are extracted when parsing environment files (inline comments stripped from values)
Inline comments only recognized when preceded by whitespace to preserve values like
PASSWORD=pass#wordNew migration adds
commentcolumn to bothenvironment_variablesandshared_environment_variablestablesUpdated Livewire components to handle comment field in form and sync logic
fix [Bug]: Nixpacks build fails because of environment variables with comments #7239