Skip to content

Commit

Permalink
fix STI migrations. Fixes doctest, and hopefully ticket #397.
Browse files Browse the repository at this point in the history
  • Loading branch information
bryanlarsen committed Apr 7, 2009
1 parent 2e28dbb commit 03c514b
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 2 deletions.
9 changes: 7 additions & 2 deletions hobofields/lib/hobo_fields/migration_generator.rb
Expand Up @@ -138,8 +138,13 @@ def always_ignore_tables

def generate
models, db_tables = models_and_tables
models_by_table_name = models.index_by {|m| m.table_name}
model_table_names = models.*.table_name
models_by_table_name = {}
models.each do |m|
if m.inheritance_base? or !models_by_table_name.has_key?(m.table_name)

This comment has been minimized.

Copy link
@tslocke

tslocke Apr 8, 2009

Coding style – prefer || over ‘or’ as ‘or’ has very low precedence. I think of ‘or’ as a control flow operator.

Could use ||= as the assignment to simplify?

models_by_table_name[m.table_name] = m
end
end
model_table_names = models_by_table_name.keys

to_create = model_table_names - db_tables
to_drop = db_tables - model_table_names - always_ignore_tables
Expand Down
5 changes: 5 additions & 0 deletions hobofields/lib/hobo_fields/model_extensions.rb
Expand Up @@ -15,8 +15,13 @@ module HoboFields
# and speeds things up a little.
inheriting_cattr_reader :field_specs => HashWithIndifferentAccess.new

# is true if is the base for STI
def self.inheritance_base?

This comment has been minimized.

Copy link
@tslocke

tslocke Apr 8, 2009

Might be worth digging in the AR source to see if a method like this already exists. Seems a shame to add an extra instance variable to every model class.

@is_inheritance_base
end

def self.inherited(klass)
@is_inheritance_base = true
fields do |f|
f.field(inheritance_column, :string)
end
Expand Down

1 comment on commit 03c514b

@bryanlarsen
Copy link
Owner Author

Choose a reason for hiding this comment

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

I did some digging before committing. The only thing I found was “superclass”. So I almost used (m.superclass==models_by_table_name[m.table_name].superclass.superclass) as the test, but I wasn’t convinced it was as robust.

Please sign in to comment.