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.