jenseng/immigrant

better handling of invalid associations

jenseng opened this issue · 4 comments

although immigrant ignores associations where the model doesn't exist (e.g. belongs_to :invalid_model), it won't ignore ones where the table or column doesn't exist (e.g. belongs_to :foo, :foreign_key => 'bad_col_id') and will generate invalid add_foreign_key statements for them.

immigrant should ignore any association that would result in an invalid add_foreign_key call, and issue a warning so that the user knows to go fix/remove the broken association

I've noticed something similar with polymorphic associations. It would be nice for the system to skip over polymorphic associations and avoid creating foreign keys that can't possibly work.

@dkubb it ought to skip polymorphic associations (there's even a test for it) ... do you have an example where that fails for you?

@jenseng sorry for not responding earlier. I had to put this on the back burner while focusing on something else.

I recently attempted to reflect the associations in Spree 2.2, and for the most part immigrant handled them all except for this one:

https://github.com/bloomcrush/spree/blob/master/core/app/models/spree/stock_movement.rb#L4

It added the following in the migration:

add_foreign_key "spree_stock_movements", "spree_variants", name: "spree_stock_movements_variant_id_fk", column: "variant_id"

Yet, the schema for this table does not include the variant_id column, eg:

create_table "spree_stock_movements", force: true do |t|
  t.integer  "stock_item_id"
  t.integer  "quantity",        default: 0
  t.string   "action"
  t.datetime "created_at",                  null: false
  t.datetime "updated_at",                  null: false
  t.integer  "originator_id"
  t.string   "originator_type"
end

EDIT: I suspect this bug is caused by the use of a polymorphic belongs_to association. The tests referenced above only show tests for polymorphic has_many associations.

I'll poke around at the spree schema, but my first guess is one of its models has an invalid association. Note that the polymorphic association won't be picked up by immigrant, since it cannot be backed by a foreign key. That test file has changed since my earlier comment, so the corresponding belongs_to polymorphic test is actually here