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

Adding default values for columns in models #424

Merged
merged 9 commits into from
Jul 16, 2020
Merged

Adding default values for columns in models #424

merged 9 commits into from
Jul 16, 2020

Conversation

jwoertink
Copy link
Member

Fixes #390

This adds the ability to set your default values for columns in your models. If your columns require default values, you'll still have to do it in the migrations as well.

If this gets merged in, then the next step will be to update the SchemaEnforcer to ensure that the migration and model both have the same default value set.

Copy link
Member

@paulcsmith paulcsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Will this cause issues with the SchemaEnforcer raising when it shouldn't? I'm guessing it just doesn't check the default right? If so then I think this is good to go since there is no downside to merging!

If it causes issues with SchemaEnforcer then we can still merge but we just need to be careful not to release a patch version until the SchemaEnforcer is updated

spec/support/model_with_default_values.cr Outdated Show resolved Hide resolved
spec/save_operation_spec.cr Outdated Show resolved Hide resolved
@jwoertink
Copy link
Member Author

Ok, I added a spec for the SchemaEnforcer to make sure it doesn't just randomly bork. The other specs are all passing, so I don't think this will affect anything currently. Then in another PR I'd like to update that SchemaEnforcer to account for the default values better.

@jwoertink
Copy link
Member Author

Also, just learned this neat little trick:

This will return all of the columns that have a default value in your database.

select table_name, column_name, column_default from information_schema.COLUMNS where table_schema = 'public' and column_default is not null;

Screen Shot 2020-07-15 at 2 43 12 PM

This could be a good use for the schema enforcer

Copy link
Member

@paulcsmith paulcsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good! And nice find on the default values thing!

spec/schema_enforcer_spec.cr Show resolved Hide resolved
@jwoertink jwoertink merged commit a141b98 into master Jul 16, 2020
@jwoertink jwoertink deleted the bugs/390 branch July 16, 2020 17:22
paulcsmith pushed a commit to luckyframework/website that referenced this pull request Aug 11, 2020
This PR aims to serve as a starting point to close #189.

I'm a little bit fuzzy on how the new functionality in luckyframework/avram#424 actually works in conjunction with the `default` option you can add to a database migration, but I'm hoping this is a good enough starting point to give me a bit of guidance and take some work off of the plate of the folks actually building all of this nifty stuff.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

default is acting not like it suppose to
2 participants