Forum: Rails-core (closed, excessive spam) Association Hash Conditions

Hi All,

I've got a patch
(http://rails.lighthouseapp.com/projects/8994/ticke...)
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

--

Nik Wakelin
+64 27 424 5433

* Blog: http://codetocustomer.com/blog
* WWR: http://workingwithrails.com/person/7401-nicholas-wakelin
* Company: http://codetocustomer.com

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

> 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

--
Cheers

Koz

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

--
Cheers

Koz

On Tue, Jul 15, 2008 at 8:42 PM, Michael Koziarski
<michael@koziarski.com> wrote:
>
>>  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 ;)

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

>
> Good stuff
>
> --
> Cheers
>
> Koz
>
> >
>



--

Nik Wakelin
+64 27 424 5433

* Blog: http://codetocustomer.com/blog
* WWR: http://workingwithrails.com/person/7401-nicholas-wakelin
* Company: http://codetocustomer.com

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

On Tue, Jul 15, 2008 at 11:28 PM, Michael Koziarski
<michael@koziarski.com> wrote:
>
> would work fine too.
>
> This is a nice use case,  care to work with nik on implementing it?
>
> --
> Cheers
>
> Koz
>
> >
>



--

Nik Wakelin
+64 27 424 5433

* Blog: http://codetocustomer.com/blog
* WWR: http://workingwithrails.com/person/7401-nicholas-wakelin
* Company: http://codetocustomer.com