FactoryGenerator: No need to import fqcn foreign keys. #642
Conversation
|
I wonder what affect this has on other areas of the code: migrations, controllers, etc. |
There was a problem hiding this comment.
Hey @Jehong-Ahn ,
We appreciate the time you invested in preparing this pull request.
I will be reviewing this patch now.
Thank you for your contribution.
| use Illuminate\Database\Eloquent\Factories\Factory; | ||
| use Illuminate\Support\Str; | ||
| use App\Models\Post; | ||
|
|
There was a problem hiding this comment.
import use My\User; fully-qualified class name.
| { | ||
| return [ | ||
| 'title' => $this->faker->sentence(4), | ||
| 'author_id' => \My\User::factory(), |
There was a problem hiding this comment.
simplify \My\User::factory() fully-qualified class name to User::factory().
| if (!Str::startsWith($class, '\\')) { | ||
| $reference = $this->fullyQualifyModelReference($class) ?? $model; | ||
| $this->addImport($model, $reference->fullyQualifiedNamespace() . '\\' . $class); | ||
| } |
There was a problem hiding this comment.
This is wrong.
I believe you want to remove the application's namespace prefix for fully-qualified class names that start with '\'.
eg. for foreign:\Foo\Bar
- use App\Models\\Foo\Bar;
+ use Foo\Bar;To achieve this we need to change:
$class = Str::studly(Str::singular($table));
$reference = $this->fullyQualifyModelReference($class) ?? $model;
- $this->addImport($model, $reference->fullyQualifiedNamespace() . '\\' . $class);
+ if (Str::startsWith($class, '\\')) {
+ $this->addImport($model, $class);
+ } else {
+ $this->addImport($model, $reference->fullyQualifiedNamespace() . '\\' . $class);
+ }This should correctly import all fully-qualified class names.
|
Thanks, @ghostwriter. I've learned a lot from your review. |
|
What's the status of this PR. Seems conversations are not resolved. |
No need
use App\Models\\Foo\Bar;forforeign:\Foo\Bar.