dont include this in your gemfile #36

Closed
omricohen opened this Issue Jul 14, 2011 · 9 comments

Projects

None yet

6 participants

accidentally did, and since it overwrites method_missing for active_record was ignoring all the beautiful built in stuff.

Took some debugging to find it, just in case anyone else runs into this problem

Contributor
MrJoy commented Jul 15, 2011

What do you mean "all the beautiful built in stuff"?

I mean Model.find_by_* and find_or_create_by, etc.

On Jul 14, 2011, at 6:38 PM, MrJoyreply@reply.github.com wrote:

What do you mean "all the beautiful built in stuff"?

Reply to this email directly or view it on GitHub:
#36 (comment)

Collaborator
alexch commented Jul 15, 2011

I think I know how to fix this...

Contributor
MrJoy commented Jul 15, 2011

That explains a lot. I was thinking I was misremembering how find_or_create_by_* worked...

alexch, havent gone through the code, but for that function, just check the method name for it to be whatever it is you are trying to catch and throw in a super:

 module ::ActiveRecord
   class Base
     def self.method_missing(name, *args)
        super unless (name == "whatever_the_method_is")
     end
   end
 end
Collaborator
alexch commented Jul 20, 2011

partial fix here: alexch@2f6dbdc

Unfortunately, this is kind of a sticky situation to fix. As omricohen points out, the method_missing override is only a problem when you're including annotate inside a real app (or test), which you should never do. So perhaps instead of my fix, we should detect the environment and abort if we're not being called from inside a rake task.

Alternately we could only put in the method_missing patch during a run, and remove it afterwards.

There is a unit test but we should also test that macros work properly in a "real" environment.

Contributor
rtlong commented Sep 14, 2011

I get around this by simply adding :require => false to the end of my gem "annotate" line. I do this for many gems anyway, as it avoids loading them when they are only need for a binary, or when I know they will be explicitly required later.

jacqui commented Sep 16, 2011

Out of curiosity... why is method_missing being overwritten here?

Collaborator
alexch commented Sep 17, 2011

Because it was written to be run without a full Rails environment loaded, one model file/class at a time. But that meant that certain class methods (has_many type macros) were not available, especially ones provided by plugins. So to capture the exceptions I hacked in a method_missing override... This was back in like 2007, when this was just a rake task people were copying and pasting without any real version control, and I'm not ready to defend the decision but we are where we are now.

It does feel like a rewrite may be in order, but I'm not the one to do it since I don't even use ActiveRecord any more except when I'm teaching...

@ctran ctran closed this Nov 30, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment