Many issues with the generator

BurningDog commented 3 years ago

Hi! Firstly, thanks for making an awesome product! I'm finding it quite useful and have recommended it to others.

That said, I found many issues in normal usage when following the installation and usage documentation at https://www.infyom.com/open-source/laravelgenerator/docs/8.0/ which seem to be major bugs. I could list each of these as a new issue on GitHub, but I didn't want to overwhelm the issue queue.

I'm using latest Laravel and latest version of the Infyom Generator. I'm not using the boilerplate, although I've had to refer to it in many cases.

[x] Issue 1
Missing laravelcollective/html

https://www.infyom.com/open-source/laravelgenerator/docs/8.0/installation#add-aliases says to add the following to config/app.php:

'Form'      => Collective\Html\FormFacade::class,
'Html'      => Collective\Html\HtmlFacade::class,
'Flash'     => Laracasts\Flash\Flash::class,

but neglects to mention that there is a package which needs to be installed in order for them to work:

composer require laravelcollective/html

This package is already installed in the composer.json in the boilerplate

[x] Issue 2
Missing files

A number of base files are missing, perhaps I missed somewhere in the setup documentation where they're generated? These files are all present in the adminlte-generator boilerplate - I had to copy them manually into my project:

App\Http\Controllers\AppBaseController
App\Repositories\BaseRepository
Tests\ApiTestTrait
[x] Issue 3
Handling file uploads

This seems to be left as an exercise to the developer? When I make the htmlType for a field to be file then an HTML file input is used for the generated CRUD controller, but instead of saving the file correctly, it merely leaves it in /private/tmp as the temporary filename. I was expecting to see it saved as a file within Laravel.

[x] Issue 4
Rollback command doesn't roll back all files

The rollback command only partially works.

[x] Issue 5
The generated migration is very basic

The generated migration doesn't specify a primary key (even if "primary": true, is set in the field json, and --fieldsFile= is used as the scaffold). This must be added manually.

In addition, there's no ability to set fields as nullable when using a fields file, so this needs to manually done in the migration. It would be useful if the documentation mentioned that.

[x] Issue 6
Incorrect path

https://www.infyom.com/open-source/laravelgenerator/docs/8.0/generator-options#fields-from-file says that the path to the sample json file is vendor\infyom\laravel-generator\samples\fields_sample.json which is incorrect, it's vendor/infyomlabs/laravel-generator/samples/fields_sample.json

[x] Issue 7
Undocumented model_schema directory

At https://www.infyom.com/open-source/laravelgenerator/docs/8.0/generator-options#fields-from-file it should be made clear that resources/model_schemas/ can be used to put generated json files in to use for model generation. I had to read the code to work that out.

[x] Issue 8
Generated models

The generated models have extra, unnecessary line breaks in the file which I need to manually remove.

When Swagger comments are added as documentation, they overwrite the comments left which document all of the model's attributes. If I want those comments, I have to run 2 separate Infyom generation commands, copying the attributes after the first one and manually adding them back in after the 2nd.

[x] Issue 9
Other spacing issues

When a controller is generated, the first lines in the class read:

    /** @var  ExampleRepository */
    private $exampleRepository;

It should be:

    /** @var ExampleRepository */
    private $exampleRepository;

or even better,

    private ExampleRepository $exampleRepository;

Indentation in generated factories

The indentation of generated factories is incorrect - the first attribute is correct, all of the following aren't.

[x] Issue 10
Route prefix incorrect

In api.php I have

Route::group(['prefix' => 'admin'], function () {
    Route::resource('about_us_pages', App\Http\Controllers\API\AboutUsPageAPIController::class);
});

but actually it should be

Route::group(['prefix' => 'v1'], function () {
    Route::resource('aboutUsPages', App\Http\Controllers\API\AboutUsPageAPIController::class);
});

[x] Issue 11
Tests

Neither generated API tests nor Repository tests work out of the box. Firstly, phpunit.xml has to be configured to include them:

        <testsuite name="API">
            <directory suffix="Test.php">./tests/APIs</directory>
        </testsuite>
        <testsuite name="Repositories">
            <directory suffix="Test.php">./tests/Repositories</directory>
        </testsuite>

Then, each test needs to use DatabaseMigrations.

[x] Issue 12
Generated API urls

Regarding the API url generated - e.g. /api/v1/blogPosts - https://blog.restcase.com/5-basic-rest-api-design-guidelines/ notes that

It is recommended to use the spinal-case (which is highlighted by RFC3986), this case is used by Google, PayPal, and other big companies.

The url generated should be /api/v1/blog-posts

[x] Issue 13
v1 isn't in URL

It's not clear that v1 won't be in the url by looking at the config:

    /*
    |--------------------------------------------------------------------------
    | API routes prefix & version
    |--------------------------------------------------------------------------
    |
    */

    'api_prefix'  => 'api',

    'api_version' => 'v1',

In what sense is v1 a route prefix if it's not used in the actual route?

In fact, route must be set to v1 in prefixes like so:

    'prefixes' => [

        'route' => 'v1',  // using admin will create route('admin.?.index') type routes

        'path' => '',

        'view' => '',  // using backend will create return view('backend.?.index') type the backend views directory

        'public' => 'public',
    ],

It's not clear to me each option does, and the comments don't help explain it either. Please be more descriptive.

[x] Issue 14
Laravel style guide

The generated Blade templates should follow the Laravel style - see https://github.com/laravel/laravel/blob/8.x/resources/views/welcome.blade.php which uses 4 spaces, not 2. Since I have my IDE set up correctly, as soon as I edit one of those files it gets entirely reformatted.

[x] Issue 15
appointer/swaggervel is abandoned

https://github.com/appointer/swaggervel is abandoned. The package to use instead is probably https://github.com/DarkaOnLine/L5-Swagger

Overall workflow

It's nearly impossible that a developer knows exactly what all fields will be upfront, and only uses the generation process once. It would be useful to show a non-trivial example where:

some initial fields are created
the model etc is generated
the API docs are viewed via Swagger, and Postman is used to query the API
a new field is added
the initial scaffolding is rolled back (it's not obvious exactly what gets rolled back and what doesn't)
a new generation command, including what to ignore this time around (e.g. --skip=menu)
work on the migration file, then on next generation do a --skip=migration