This project is archived and is in readonly mode.
ActiveRecord: nested "include" hints fail if record classes are in modules
ActiveRecord 2.0.2 can't load records whose classes are in modules when using nested "include"s.
e.g.
@edition = Edition.find_by_id(edition_id, {:include => {:published_item => :user}})
gives an error:
C:/Ruby/lib/ruby/gems/1.8/gems/activesupport-2.0.2/lib/active_support/dependencies.rb:266:in
`load_missing_constant': uninitialized constant PublishedItem (NameError)
This appears to be because "constantize" is called on the string "PublishedItem", which isn't a constant, although "YuduLibrary::Model::PublishedItem" is.
I've attached a monkey patch which I'm currently using to work around the problem.
I would submit a proper patch to fix the issue, but I can't get the ActiveRecord unit tests to run on my machine (win32).
-
josh- State changed from new to stale
- Tag set to activerecord
Closing this ticket as stale. If this is still an issue for you, feel free to reopen this ticket or create a new one with an updated description. Remember those unit tests too ;)
-
Rich Bradley-
diff.txt
2.53 KB
-
-
Rich Bradley
Here's a patch, which fixes the issue, and adds a new unit test to demonstrate the problem.
This bug is harder to reproduce on HEAD than on 2.0.2, because it only occurs in the "join" eager load model, which has been replaced by a sequential eager load model as per: http://dev.rubyonrails.org/ticke...
Let me know if you need anything else.
-
Rich Bradley
How do I mark the ticket as no longer stale?
-
CancelProfileIsBroken
- Tag changed from activerecord to activerecord, bugmash
-
Rich Bradley
This bug is still present, one year after I submitted a patch with unit tests :-(
It probably crops up in other places -- have a search for ".class_name.constantize" -- I think that all of these ought to use ".klass" instead. There's at least one other instance in lib/active_record/associations.rb
Mike, thanks for tagging this "bugmash". Maybe it'll get looked at?
I think all this needs is committing? It doesn't seem to fit any of 1-4 at http://weblog.rubyonrails.org/2009/7/28/rails-bugmash
(Well, maybe it fails '3' -- "a patch that applies cleanly to the current Rails source"; I haven't checked if the patch still applies cleanly to HEAD -- it's been a while since I wrote it :-)
If anyone watching this ticket has any advice on what I could have done differently here to get this committed back in Jul 2008, I'd love to hear it.
I think I followed all the steps at http://drnicwilliams.com/2007/06/01/8-steps-for-fixing-other-people...
.. something's just occurred to me: I've been waiting for someone with "commit rights" to commit this. Is that not how it works on Rails?
-
CancelProfileIsBroken
- State changed from stale to open
Rich - Yes, flagging this for a look by someone during the upcoming BugMash (http://wiki.railsbridge.org/projects/railsbridge/wiki/BugMash). We're going to make a concerted effort to clean up some of the stuff that's been hanging around for a long while.
If you wanted to participate yourself, that'd be great. Otherwise with any luck someone will volunteer to look up.
And yes, in Rails it needs someone with commit rights to commit. We'll have committers involved.
-
Dmitry Ratnikov
+1 Seems to apply cleanly to both master and 2-3-stable.
I have extended the patch to fix the class_name.constantize call in _ids= assignment. I have also attached a test to test it, but perhaps someone could take a look at it and make it a bit neater?
-
Dmitry Ratnikov
For some reason I fail to update the ticket with the attacked patch, so I put it on drop.io for now: http://drop.io/urxdaxz If someone has better luck attaching things to LH, please do. :)
-- D
-
Jeremy Kemper- Assigned user set to Jeremy Kemper
- Milestone set to 2.3.4
Rich - sorry this lingered here, incorrectly stale. Thanks BugMash for the archaeology exercise.
-
Gabe da Silveira
Testing to see if I can upload Dmitry's patch for him:
-
constantization.patch.txt
4.86 KB
-
-
Jeremy Kemper
Dmitry - good find on the _ids= assignment. Could you rebase and update the test? I see it can raise an exception, but the test has no assertions.
-
Repository
- State changed from open to committed
(from [0b95a2afab88851eea78926eb91462002ff8db9a]) Fix for nested :include with namespaced models.
[#260 ActiveRecord: nested "include" hints fail if record classes are in modules state:committed] http://github.com/rails/rails/commit/0b95a2afab88851eea78926eb91462...
-
Repository
(from [9bb8ef9edebf5c27b8a1c67ca3776d52afbc1dc4]) Fix for nested :include with namespaced models.
[#260 ActiveRecord: nested "include" hints fail if record classes are in modules state:committed] http://github.com/rails/rails/commit/9bb8ef9edebf5c27b8a1c67ca3776d...
-
Dmitry Ratnikov
Here's an updated patch.
I made the tests actually fail rather than blow up.
Also, I had to resort to removing constants possibly defined by requiring models/company.rb This allows the test to fail when run as test-suite.-
constantization_fix.patch
7.79 KB
-
-
-
-
Repository
- State changed from open to committed
(from [32c23552f547412081116c001b152bca78c9892e]) Changed to use klass instead of constantizing in assign_ids generated method
[#260 ActiveRecord: nested "include" hints fail if record classes are in modules state:committed]
Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
http://github.com/rails/rails/commit/32c23552f547412081116c001b152b... -
Repository
(from [314ba0433f03b66022ad41d55cc75d2bd9809fe3]) Changed to use klass instead of constantizing in assign_ids generated method
[#260 ActiveRecord: nested "include" hints fail if record classes are in modules state:committed]
Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
http://github.com/rails/rails/commit/314ba0433f03b66022ad41d55cc75d... -
CancelProfileIsBroken
- Assigned user cleared.
- Tag changed from activerecord, bugmash to activerecord
- Milestone cleared.
-
Rich Bradley
Thanks all.