CrestApps / laravel-code-generator

An efficient Laravel code generator, saving time by automating the creation of resources such as views, controllers, routes, migrations, languages, and form-requests. Highly flexible and customizable, it includes a cross-browser compatible template and client-side validation for application modernization.
https://laravel-code-generator.crestapps.com
MIT License
738 stars 158 forks source link

Undefined variable $column Error with command "php artisan create:fields-file [table-name]" #2

Closed essemme closed 7 years ago

essemme commented 7 years ago

(Laravel 5.4, code-generator master branch)

While trying to create the definitions file from a MySql database using the command php artisan create:fields-file whatever-my-table-name I get this error (pasted from the log):

local.ERROR: exception 'ErrorException' with message 'Undefined variable: column' in [LaravelAppPath]/vendor/crestapps/laravel-code-generator/src/DatabaseParsers/ParserBase.php:96

(while comparing, with a quick look, the BaseParser / MysqlParser classes methods with the ones in SQLServerParser, it seems something is missing)

P.S. BTW, great package - I like the clean and effective code of the stubs

MikeAlhayek commented 7 years ago

Hello, First of all thank you for using this package. I hope you are enjoying and benefiting from its features.

Thank for reporting this issue! Indeed there was a bug in the code which was addressed in the this commit. However, before I release a new minor version, I would appreciate it if you can confirm that everything is working correctly for you. To get the latest changes, please do the following steps

Additionally, you can use the create:resources command to create the fields-file along with all the resources using a single command. i.e. php artisan create:resources YourModelName --table-exists for more info about the --table-exists option, please visit the documentation

Finally, since this feature is in a beta stage, I like to hear your feedback to help me improve it and make it more robust. Any challenges that you saw while using it? Any new idea you wish it would have? Any improvement to existing logic? The more info I have on the user's use case, the better this feature will become.

MikeAlhayek commented 7 years ago

Please upgrade to v1.2.1 to get the bug fixes.

essemme commented 7 years ago

Yes, I'm already requiring dev-master - I'll keep the most recent version for this project. Just a quick look after updating: the file generation works as intended. I'll try it on more complex tables later.

Just one minor thing I noticed: a mysql TINYINT (1) (from a Laravel migration boolean field) has a "html-type" value of "text"; I believe "checkbox" is what is meant to be.

I think this package is a great time saver! I'll use it in my current project, and post feedbacks / open new issues here if needed

MikeAlhayek commented 7 years ago

@essemme , please download the latest changes from dev-master. That should fix the tinyint issue you mentioned.

You'll need to publish your vendor changes after that to get the latest config changes.

essemme commented 7 years ago

Just another minor issue i noticed while generating resources/the fields definition file from an existing table: the property "is-nullable" seems to be always true: could it automatically be set to false (and "required|"added to validation rules) if the field is not nullable?

(side-question: should we open new issues for questions like this one, or is it fine to add related minor things to this issue?)

essemme commented 7 years ago

Regarding the boolean - TINYINT (1) issue: in the generated field file becomes "html-type": "radio", "options": { "0": "No", "1": "Yes" }, which makes sense (probably I'd prefer checkbox, but both will do); the problem is in the form view - only one option ("Yes") for the radio is generated. BTW, just changing the input type to checkbox in the view seems fine.

MikeAlhayek commented 7 years ago

Regarding the boolean issue. Did you publish the vendor files? By design it should be checkbox not radio https://github.com/CrestApps/laravel-code-generator/blob/master/config/codegenerator.php#L190.

php artisan vendor:publish --provider="CrestApps\CodeGenerator\CodeGeneratorServiceProvider" --tag=default --force

Additionally, this is something you can set in the config/codegenerator.phpin theeloquent_type_to_html_type` array you can control the default types if you feel the default does not work for you.

MikeAlhayek commented 7 years ago

I suggest you open new question to keep things easy to find.

Regarding your commend about nullable, it should already get set when generating the fields' file. I can try to recreate the problem and see if it is a bug. To do that, please post your SHOW CREATE TABLE your-table-name output from the database and the exact command you used to generate the fields-file.

Regarding the validation, that is something that is set/generated when the controller is generated. So if the field is does not allow null, required will automatically get added. There is lots of logic similar to this that gets automatically added based on the data type. For example, when you create a field with a string data type and the data-type-params is set to [100] we automatically add max:100 to the validation rules unless it is already provided. Either way, I may consider adding this logic to create:fields-file command to make it clear for the end user.

essemme commented 7 years ago

Yes, I did publish the vendor files (maybe, the first time, without the --force flag), default templates (and the custom copy with just some edit) seemed up to date. Thank you for reminding me the "eloquent_type_to_html_type" setting (By the way, that's great!) - it was that indeed (probably the only vendor-published-file I didn't double check). Now it defaults to checkbox.

Good to know about the validation, i noticed most of the rules being automatically added (very useful, indeed), only the "required" for the not nullable field seemed inconsistent, I'll double check.

Ok, I'll keep this one (about the "nullable" issue) in this thread, then open new ones for any new issue Table (MySQL):

CREATE TABLE `makes` (
  `id` int(10) unsigned NOT NULL AUTO_INCREMENT,
  `code` varchar(191) COLLATE utf8mb4_unicode_ci NOT NULL,
  `name_en` varchar(191) COLLATE utf8mb4_unicode_ci DEFAULT NULL,
  `created_at` timestamp NULL DEFAULT NULL,
  `updated_at` timestamp NULL DEFAULT NULL,
  `type_one` tinyint(1) NOT NULL DEFAULT '0',
  `type_two` tinyint(1) NOT NULL DEFAULT '0',
  PRIMARY KEY (`id`)
) ENGINE=InnoDB AUTO_INCREMENT=2 DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;

Command (tried a few times, this should be the relevant one): php artisan create:resources Make --table-exists --with-form-request --without-migration --force --relationships=models#hasMany#App\Models\CModel|id|make_id

MikeAlhayek commented 7 years ago

Thanks for posting your table structure. The issue with the nullable field should be fixed. Please get the latest code in dev-master to see the changes.

Thank you