multiple has_many breaks owner actions
Reported by David Mathers | November 15th, 2009 @ 12:59 PM | in Beyond Hobo 1.0
To reproduce:
$ hobo project
$ script/generate hobo_model_resource story name:string
$ script/generate hobo_model_resource task name:string hard:boolean
story model (order is important):
has_many :hard_tasks, :class_name => "Task", :scope => :hard
has_many :tasks, :dependent => :destroy
task model:
belongs_to :story
task controller:
auto_actions_for :story, [ :index, :new, :create ]
Now create a story and then from the story page click the new task link.
The link sends you to /stories/1/tasks/new but the generated route is /stories/1/hard_tasks/new
I was going to try and fix this but couldn't think of how it should work, even in theory.
A thought occurred to me just now that maybe the association with ":dependent => :destroy" could be anointed as the "auto_actions_for" association. Since that's what causes the collection to show up on the page it seems reasonable that it be required in order for "auto_actions_for" to work at all.
Comments and changes to this ticket
-
David Mathers November 15th, 2009 @ 01:43 PM
I just discovered Model#dependent_on, but it doesn't work because it also depends on Model#reverse_reflection, which has already discarded the dependent association. Model#reverse_reflection essentially chooses an association at random if there's more than one to choose from.
(I think) Model#dependent_on needs to get all the reverse associations and then select the dependent association. Then the model_router could call that, instead of calling Model#reverse_reflection as it does now, and that would fix the above problem.
-
David Mathers November 15th, 2009 @ 03:08 PM
That didn't work. I had to replace ".find" with ".try.find" and although the route is now correct the collection is still wrong.
-
Matt Jones November 15th, 2009 @ 09:56 PM
- Assigned user set to Matt Jones
The real solution to this is waiting in Rails 3 - they've added an :inverse_of option to the associations that allows the inverse (which we currently guess) to be specified explicitly.
Patch is here:
https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets...There's a lot of additional plumbing in the patch to get identity-map-esque behavior for the associations (see the ticket for details). It's possible that the basic function of the option could get back-ported for Hobo on Rails 2.x; the move to Rails 3 is very much a post-1.0 kind of thing.
In your particular case, it may be sufficient to amend the test around line 347 of model.rb to skip associations that have :scope defined. The current code already skips associations with :conditions set.
I'll take a look at this over the next couple days.
-
Matt Jones November 15th, 2009 @ 09:56 PM
- State changed from new to investigating
-
David Mathers November 16th, 2009 @ 12:41 AM
"In your particular case, it may be sufficient to amend the test around line 347 of model.rb to skip associations that have :scope defined. The current code already skips associations with :conditions set."
Yes, that sounds like the best idea. Just add 1 line to fix. I'll try it now.
-
Matt Jones November 22nd, 2009 @ 10:47 PM
- State changed from investigating to open
I've committed the filtering on :scope (commit 3e9b0d684f990af75ef0f559e0fa5a5a0337e339). Can you take a look and see if it fixes your issue?
-
Matt Jones November 30th, 2009 @ 03:35 PM
- Milestone set to Beyond Hobo 1.0
The :inverse_of option is nice to have, but it can wait till after 1.0; the check for both :conditions and :scope means that there's not many ways left to confuse the reverse_reflection code.
-
Bryan Larsen February 19th, 2010 @ 07:17 PM
- Tag changed from associations, auto_actions_for, routes to associations, auto_actions_for, enhancement, routes
Please Sign in or create a free account to add a new ticket.
With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป