Association Hash Conditions

Hi All,

I've got a patch (http://rails.lighthouseapp.com/projects/8994/tickets/467-hash-conditions-belongs_to-sugar) that allows you to do:

Person.find(:all, :conditions => { :university => victoria_wellington })

I'm planning on making it work for belongs_to and has_one (including polymorphic association).

The discussion was basically around whether or not we should tie hash conditions to the database field names.

There was also some discussion about find_by_association, i.e

  Person.find_by_university(victoria_wellington)

Josh has a patch that I can merge into all this stuff pretty easily.

Pratik asked me to restart the discussion here, so I'd be interested to hear what everybody thinks.

Thanks!

Nik

Person.find(:all, :conditions => { :university => victoria_wellington })

I'm quite suspicious where you're using find_by_some_association, couldn't you be walking the associations back in the other direction.

victoria_wellington.people

I'm planning on making it work for belongs_to and has_one (including polymorphic association).

How are you planning on making it work for has_one, what would the query look like?

The discussion was basically around whether or not we should tie hash conditions to the database field names.

There was also some discussion about find_by_association, i.e

Person.find_by_university(victoria_wellington)

Josh has a patch that I can merge into all this stuff pretty easily.

Pratik asked me to restart the discussion here, so I'd be interested to hear what everybody thinks.

I'd be a little concerned about adding some special case code to the sanitize_sql logic, but if some work was undertaken to rationalise all that stuff at the same time then this kind of thing could be a nice addition.

Good stuff

Person.find(:all, :conditions => { :university => victoria_wellington })

I'm quite suspicious where you're using find_by_some_association,
couldn't you be walking the associations back in the other direction.
victoria_wellington.people

Can you explain this some more?

Why does has_many acts as a named_scope and belongs_to not?

I think it would be nice to be able to do:

Comment.university(boston).creator(current_user).commentable(post).all

Instead of:

Comment.find_all_by_university_id_and_creator_id_and_commentable_type_and_commentable_id(boston, current_user, post.class.base_class.name.to_s, post.id)

It’s main advantage would be that it allows easy formation of dynamic queries based on input:

scope = Comment.scoped({:include => [:creator, :commentable]})

scope = scope.commentable(post) if post

scope = scope.university(params[:uni]) if params[:uni]

scope = scope.creator(current_user) if params[:created_by_me] == "1"

scope = scope.country(params[:country]) if params[:country]

@comments = scope.all

Lawrence

I think it would be nice to be able to do:

  Comment.university(boston).creator(current_user).commentable(post).all

Indeed, that does look pretty nice, and very useful in some circumstances. But it could be implemented without touching the query generation code at all. You could automatically add proc based named scopes to the class when you define the associations. and allow those procs to take either ids or instances.

I'd definitely prefer that approach to supporting hash keys referring to associations inside the query generation code.

Comment.find_all_by_university_id_and_creator_id_and_commentable_type_and_commentable_id(boston, current_user, post.class.base_class.name.to_s, post.id)

I'd never advocate anything quite as ugly as that. but something like

post.comments_by_user_about(current_user, boston)

would work fine too.

It's main advantage would be that it allows easy formation of dynamic queries based on input:

    scope = Comment.scoped({:include => [:creator, :commentable]})     scope = scope.commentable(post) if post     scope = scope.university(params[:uni]) if params[:uni]     scope = scope.creator(current_user) if params[:created_by_me] == "1"     scope = scope.country(params[:country]) if params[:country]     @comments = scope.all

This is a nice use case, care to work with nik on implementing it?

Person.find(:all, :conditions => { :university => victoria_wellington })

I'm quite suspicious where you're using find_by_some_association, couldn't you be walking the associations back in the other direction.

victoria_wellington.people

You could, but that's less easy when you've got other conditions. It's just meant to replace:

Person.find(:all, :conditions => { :university_id => victoria_wellington })

Rails will automatically use the id for "victoria_wellington", but it seems weird to say "_id" in the hash when you _actually_ mean the association. And it gets even weirder when your foreign keys aren't named the same as your association.

I'm planning on making it work for belongs_to and has_one (including polymorphic association).

How are you planning on making it work for has_one, what would the query look like?

At first it seemed hard as the has_one has to query against a seperate table, but Pratik introduced a patch that allows you to specify multiple tables in the conditions hash. I was planning on using that, but I'll freely admit I haven't looked into it properly yet :wink:

The discussion was basically around whether or not we should tie hash conditions to the database field names.

There was also some discussion about find_by_association, i.e

Person.find_by_university(victoria_wellington)

Josh has a patch that I can merge into all this stuff pretty easily.

Pratik asked me to restart the discussion here, so I'd be interested to hear what everybody thinks.

I'd be a little concerned about adding some special case code to the sanitize_sql logic, but if some work was undertaken to rationalise all that stuff at the same time then this kind of thing could be a nice addition.

What kind of rationalisation needs to happen? I'm happy to take some of that on.

That does look cool. I'll email you off-list.