Skip to content
New issue

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

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

Already on GitHub? # to your account

Schema enforcer update #556

Merged

Conversation

matthewmcgarvey
Copy link
Member

Related to #453
Related to #555

In order for views to work with SchemaEnforcer, EnsureMatchingColumns has to have a configuration to not check required vs. nilable on columns since all columns are nilable in views. This update also changes how we set up validations for SchemaEnforcer so that tables could have different validations than views and there's a way to add/remove/customize validations

This allows us to new up a validation before we want to use it and avoid the database penalty
This allows users to override and change which validations are included

def initialize(@model_class)
@database_info = @model_class.database.database_info
Copy link
Member Author

Choose a reason for hiding this comment

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

This could no longer live here because #database_info causes a database query which must happen after the database has been configured. Moving it into the class level of models meant that it is no longer the case.

Copy link
Member

@jwoertink jwoertink left a comment

Choose a reason for hiding this comment

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

I've tried to review this PR 4 times now, and I keep getting side tracked 2 seconds in 😅 I think it's good 😂 If not, we'll find out pretty quick

@matthewmcgarvey matthewmcgarvey merged commit 5c750d5 into luckyframework:master Dec 8, 2020
@matthewmcgarvey matthewmcgarvey deleted the schema-enforcer-update branch December 8, 2020 20:32
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants