Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Defaults value in database migration doesn't work (infyom:scaffold) #291

Closed
briano-jabico opened this issue Sep 9, 2016 · 3 comments

Comments

Projects
None yet
2 participants
@briano-jabico
Copy link

commented Sep 9, 2016

Greetings,

I'm using the 5.3 command-line interface to generate a migration for a line_items table. I've got a few fields for which I'd like to add database defaults. This is something Laravel supports, and the InfyOm generator almost supports it.

Here's the command I'm running.

php artisan infyom:scaffold LineItem --fieldsFile=database/infyom-plans/LineItem.json --skip=controllers,repository,requests,scaffold_requests,routes,views,tests,menu

Here's an excerpt from that file.

{"name": "name",         "dbType": "string",            "validations": "required", "inIndex": false},
{"name": "person_count", "dbType": "integer:default,1", "validations": "required|max:9999", "inIndex": false},

Here's an excerpt from what it's generating.

$table->string('name');
$table->integer('person_count')->default(, 1);

Here's what I'd like it to be generating: no comma before the 1 in ->default(

$table->string('name');
$table->integer('person_count')->default(1);

I believe this is the reason: https://github.com/InfyOmLabs/laravel-generator/blob/5.3/src/Common/GeneratorField.php#L108-L110

And I believe this would fix it, though I'm not sure if it would impact anything else:

$this->migrationText .= implode(', ', $inputParams);

Can you take a look?

Thanks!

@mitulgolakiya mitulgolakiya added the bug label Sep 10, 2016

@mitulgolakiya

This comment has been minimized.

Copy link
Member

commented Sep 10, 2016

@briano-jabico Great catch. Yes, that would be a possible fix. Will fix it.

mitulgolakiya added a commit that referenced this issue Sep 10, 2016

@mitulgolakiya

This comment has been minimized.

Copy link
Member

commented Sep 10, 2016

mitulgolakiya added a commit that referenced this issue Sep 10, 2016

mitulgolakiya added a commit that referenced this issue Sep 10, 2016

@briano-jabico

This comment has been minimized.

Copy link
Author

commented Sep 15, 2016

Sorry about being slow to get back to you; I've been involved in other things. I can confirm that the fix works. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.