[227]: use winter/laravel-config-writer to append lang validation #233
Conversation
There was a problem hiding this comment.
Pull request overview
This PR modernizes the translation file manipulation in the entity generator by replacing manual string manipulation with the winter/laravel-config-writer library. The change eliminates the need for a separate translation_not_found stub and regex-based content appending, replacing it with a cleaner ArrayFile-based API for modifying PHP configuration arrays.
Key changes:
- Replaced regex-based file manipulation with
winter/laravel-config-writerArrayFile API for safer and more maintainable code - Removed the
translation_not_foundstub configuration as it's no longer needed - Updated test coverage to reflect the new implementation approach, replacing the stub existence test with a test for appending to existing exceptions
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Generators/TranslationsGenerator.php | Refactored to use ArrayFile library instead of manual string manipulation; removed appendNotFoundException() and isTranslationMissed() methods |
| config/entity-generator.php | Removed obsolete translation_not_found stub configuration |
| composer.json | Added winter/laravel-config-writer dependency (>=1.2.1) |
| composer.lock | Updated with winter/laravel-config-writer package metadata and dependencies |
| tests/TranslationGeneratorTest.php | Renamed and refactored test to cover appending when exceptions exist; updated mock setup to accept validation stub parameter |
| tests/Support/Translation/TranslationMockTrait.php | Modified mockFilesystemForAppend() to accept validation stub filename as parameter for more flexible testing |
| tests/Support/Translation/validation_with_exceptions.php | Added new test fixture with pre-existing exceptions array for testing append behavior |
| tests/fixtures/TranslationGeneratorTest/validation_append_not_found_exception.php | Updated expected output format to match ArrayFile-generated files; removed trailing whitespace |
| tests/fixtures/TranslationGeneratorTest/validation_append_not_found_with_exceptions.php | Added complete file structure including PHP tags for testing append to existing exceptions |
| tests/fixtures/CommandTest/changed_config.json | Removed obsolete translation_not_found stub reference |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!file_exists($this->translationPath) && $this->isStubExists('validation')) { | ||
| $this->createTranslate(); | ||
| } | ||
|
|
||
| if (file_exists($this->translationPath) && $this->isTranslationMissed('validation.exceptions.not_found') && $this->isStubExists('translation_not_found')) { | ||
| $this->appendNotFoundException(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
If the translation file doesn't exist and the validation stub is not configured or missing, the code will attempt to open a non-existent file with ArrayFile::open() on line 28, which will likely throw an exception. The logic should handle the case where the file doesn't exist, regardless of stub availability. Consider changing the condition to:
if (!file_exists($this->translationPath)) {
if ($this->isStubExists('validation')) {
$this->createTranslate();
}
return;
}
There was a problem hiding this comment.
@neellii please take a look https://github.com/RonasIT/laravel-entity-generator/pull/233/files#r2585557366 it seems we really need to process this case and cover it by autotests
| if (!file_exists($this->translationPath) && $this->isStubExists('validation')) { | ||
| $this->createTranslate(); | ||
| } | ||
|
|
||
| if (file_exists($this->translationPath) && $this->isTranslationMissed('validation.exceptions.not_found') && $this->isStubExists('translation_not_found')) { | ||
| $this->appendNotFoundException(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
@neellii please take a look https://github.com/RonasIT/laravel-entity-generator/pull/233/files#r2585557366 it seems we really need to process this case and cover it by autotests
refs: #227