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

Ensure matching column message when missing Enum #908

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions spec/avram/schema_enforcer/ensure_matching_columns_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,18 @@ private class ModelWithMissingButSimilarlyNamedColumn < BaseModel
end
end

private class ModelWithMissingColumn < BaseModel
table :users do
column first_name : String
end
end

private class ModelWithMissingEnumColumn < BaseModel
table :users do
column gender : Enum
end
end

private class ModelWithOptionalAttributeOnRequiredColumn < BaseModel
table :users do
column name : String?
Expand All @@ -25,6 +37,16 @@ describe Avram::SchemaEnforcer::EnsureMatchingColumns do
expect_schema_mismatch "wants to use the column 'mickname' but it does not exist. Did you mean 'nickname'?" do
ModelWithMissingButSimilarlyNamedColumn.ensure_correct_column_mappings!
end

# Partial message assertion for brevity
expect_schema_mismatch "wants to use the column 'first_name' but it does not exist." do
ModelWithMissingColumn.ensure_correct_column_mappings!
end

# Partial message assertion for brevity
expect_schema_mismatch "add gender : Int32" do
ModelWithMissingEnumColumn.ensure_correct_column_mappings!
end
end

it "raises on nilable column with required columns" do
Expand Down
6 changes: 4 additions & 2 deletions src/avram/schema_enforcer/ensure_matching_columns.cr
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ class Avram::SchemaEnforcer::EnsureMatchingColumns < Avram::SchemaEnforcer::Vali
if best_match
message += " Did you mean '#{best_match.colorize.yellow.bold}'?\n\n"
else
# Enum column are mapped to Int32 in the database layer
missing_attribute_type_hint = "#{missing_attribute[:name]} : #{missing_attribute[:type].sub("Enum", "Int32")}"
Copy link
Member

Choose a reason for hiding this comment

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

This is nice, but it's possible to define an Enum as Int64 as well. #856

Copy link
Member

Choose a reason for hiding this comment

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

Coming back to this, we may eventually want to add all of the Int. Like if you had a Colors Enum that you wanted to be Int8 or maybe Int16. I'm not sure the best way to handle this just yet, but definitely having Int32 static in here may be an issue.

message += <<-TEXT


Expand All @@ -62,10 +64,10 @@ class Avram::SchemaEnforcer::EnsureMatchingColumns < Avram::SchemaEnforcer::Vali

alter :#{table_name} do
#{"# Add the column:".colorize.dim}
add #{missing_attribute[:name]} : #{missing_attribute[:type]}
add #{missing_attribute_type_hint}

#{"# Or if this is a column for a belongs_to relationship:".colorize.dim}
add_belongs_to #{missing_attribute[:name]} : #{missing_attribute[:type]}
add_belongs_to #{missing_attribute_type_hint}
end

Or, you can skip schema checks for this model:
Expand Down