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

Provide better error when primary key is missing #454

Merged
merged 1 commit into from
Sep 14, 2020

Conversation

matthewmcgarvey
Copy link
Member

@matthewmcgarvey matthewmcgarvey commented Sep 7, 2020

Pointed out in this issue comment

If a model uses skip_default_columns, they are required to call primary_key in the table block. If they forget, the error is cryptic. This updates the process to actually check for a primary key and provide a more clear error.

Before

Showing last frame. Use --error-trace for full trace.

In src/avram/model.cr:170:30

 170 | primary_key_type: {{ PRIMARY_KEY_TYPE }},
                            ^
Error: undefined constant PRIMARY_KEY_TYPE

After

Showing last frame. Use --error-trace for full trace.

There was a problem expanding macro 'table'

Code in spec/support/line_item_product.cr:4:3

 4 | table :line_items_products do
     ^
Called macro defined in src/avram/model.cr:112:3

 112 | macro table(table_name = nil)

Which expanded to:

 >  8 |  end
 >  9 | 
 > 10 |     validate_primary_key
            ^
Error: No primary key was specified.

Example:

  table do
    primary_key id : Int64
    ...
  end

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.

Nice! Yeah, this is a much nicer error.

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.

Love it!

@paulcsmith paulcsmith merged commit c787870 into luckyframework:master Sep 14, 2020
@matthewmcgarvey matthewmcgarvey deleted the primary-key-error branch September 14, 2020 22:24
# 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.

3 participants