Observers on numerically sorted hierarchical models trigger load error about closure_tree HOT 7 CLOSED

I had to do something similarly kludgy with chromotype: I introduced the idea of a migration_safe_on_load that would ignore issues arising from tables not existing yet.

Do you think it's OK to "lie" and say that order isn't numeric just because the table doesn't exist? Would returning nil be better? (Nothing will work otherwise, so we're limping in any event).

def order_is_numeric
  return nil unless self.table_exists?
  return false unless order_option
  c = ct_class.columns_hash[order_option]
  c && c.type == :integer
end

I think I like yours better, but I agree with your hate with && and unless.

Happy to take your pull request! I'm finishing issue 31 right now, but if I can bang it out, I'll update this issue.

from closure_tree.

Technically it isn't numeric. I mean, it isn't there! I don't see a difference between nil and false so it's up to you on that one. I would say, if the method should return 'boolean' then have it do so.

I'll put something together and send it your way. I like the explicit returns. Easier to follow.

For what it's worth, I spent some time digging in Rails in the hopes of coming up with a workaround that we could just document and not have to do something janky. You can't disable them from instantiating although you can stop them from notifying. I even experimented with putting checks in application.rb. It worked but made me want to take a shower.

# active record observers
config.active_record.observers = []
if ActiveRecord::Base.connected? && ActiveRecord::Base.connection.table_exists?(:tags)
  config.active_record.observers << :tag_observer
end

from closure_tree.

To prevent unnecessary showering, why not use a "mixin" or "concern" module that you subsequently include in your Tag model? You don't have to do janky observers array mucking about then.

I'll add the twiddle in your initial comment now, in any event, though.

Thanks!

from closure_tree.

Ah, you're too fast. You coulda had a pull request. We could have had something together!

In the app where this came up I was able to just inline the observer callbacks right into the models, which worked for our application.

If time doesn't work out for you let me know if I can get that pull request done.

from closure_tree.

Oh, we have something, sir, we have something.

You are now the proud father of v3.6.7. Thanks for your help.

from closure_tree.

(BTW, I negated the unless, just in the hopes that it would make you happy)

from closure_tree.

💞

from closure_tree.