Rails Ruby on Rails

Comments and changes to this ticket

• 

  josh July 17th, 2008 @ 01:59 AM

  • 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 July 28th, 2008 @ 05:24 PM

  • no changes were found...

  • diff.txt 2.5 KB

• You flagged this item as spam.
  

  Rich Bradley July 28th, 2008 @ 05:24 PM

  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.

• You flagged this item as spam.
  

  Rich Bradley July 28th, 2008 @ 05:26 PM

  How do I mark the ticket as no longer stale?

• 

  CancelProfileIsBroken August 3rd, 2009 @ 03:02 PM

  • Tag changed from “activerecord” to “activerecord, bugmash”

• You flagged this item as spam.
  

  Rich Bradley August 3rd, 2009 @ 05:04 PM

  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 August 3rd, 2009 @ 07:04 PM

  • 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.

• You flagged this item as spam.
  

  Dmitry Ratnikov August 9th, 2009 @ 06:53 AM

  +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?

• You flagged this item as spam.
  

  Dmitry Ratnikov August 9th, 2009 @ 07:00 AM

  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 August 9th, 2009 @ 07:26 AM

  • 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.

• You flagged this item as spam.
  

  Gabe da Silveira August 9th, 2009 @ 08:12 AM

  Testing to see if I can upload Dmitry's patch for him:

  • constantization.patch.txt 4.9 KB
• 

  Jeremy Kemper August 9th, 2009 @ 08:30 AM

  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 August 9th, 2009 @ 09:02 AM

  • State changed from “open” to “committed”

  (from [0b95a2afab88851eea78926eb91462002ff8db9a]) Fix for nested :include with namespaced models.

  [#260 state:committed] http://github.com/rails/rails/commit/0b95a2afab88851eea78926eb91462...

• 

  Repository August 9th, 2009 @ 09:02 AM

  (from [9bb8ef9edebf5c27b8a1c67ca3776d52afbc1dc4]) Fix for nested :include with namespaced models.

  [#260 state:committed] http://github.com/rails/rails/commit/9bb8ef9edebf5c27b8a1c67ca3776d...

• You flagged this item as spam.
  

  Dmitry Ratnikov August 9th, 2009 @ 09:56 AM

  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.8 KB
• 

  Jeremy Kemper August 9th, 2009 @ 10:00 AM

  • State changed from “committed” to “open”

  Thanks! Needs rebase against Rich's commit.

• You flagged this item as spam.
  

  Dmitry Ratnikov August 9th, 2009 @ 10:15 AM

  Rebased my patch against master.

  • constantization_rebased.patch 5.9 KB
• 

  Repository August 9th, 2009 @ 10:52 AM

  • State changed from “open” to “committed”

  (from [32c23552f547412081116c001b152bca78c9892e]) Changed to use klass instead of constantizing in assign_ids generated method

  [#260 state:committed]

  Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
  http://github.com/rails/rails/commit/32c23552f547412081116c001b152b...

• 

  Repository August 9th, 2009 @ 10:53 AM

  (from [314ba0433f03b66022ad41d55cc75d2bd9809fe3]) Changed to use klass instead of constantizing in assign_ids generated method

  [#260 state:committed]

  Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
  http://github.com/rails/rails/commit/314ba0433f03b66022ad41d55cc75d...

• 

  CancelProfileIsBroken August 9th, 2009 @ 11:18 AM

  • Assigned user cleared.
  • Tag changed from “activerecord, bugmash” to “activerecord”
  • Milestone cleared.

• You flagged this item as spam.
  

  Rich Bradley August 10th, 2009 @ 10:26 AM

  Thanks all.