Created
April 28, 2009 13:53
-
-
Save aub/103168 to your computer and use it in GitHub Desktop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
############### Named filter issues. | |
# | |
# 1) The table aliases for explicit joins are hacky. Obviously this is an easy fix, but we should | |
# talk about the best alternative. In this example, the resulting alias should not be "posts_blogs". | |
# | |
Post.filter do | |
join(Blog, :left, :posts_blogs) do | |
on(:blog_id => :id) | |
with(:name, 'Test Name') | |
end | |
end.inspect | |
Post.last_find[:joins].should == %q(LEFT JOIN "blogs" AS posts_blogs ON "posts".blog_id = posts_blogs.id) | |
# | |
# 2) Join type support is spotty. Right now we have inner, left, left_outer, and outer. We just | |
# need to make sure that we support all of the reasonable types. | |
# | |
# | |
# 3) Custom SQL isn't supported. Worth discussing the correct API for this, given the large number | |
# of ways in which you might want to use it. | |
# | |
# | |
# 4) Accessing local variables can be clumsy with name conflicts. Since the methods called inside | |
# of filters and named_filters are scoped to the filter DSL, there can be name conflicts when trying | |
# to use local variables with the same name. I think think this is just a fact of life, but worth | |
# thinking about. Take this example, from a content block presenter, where we have to alias the 'limited' | |
# value. Unfortunately, this is a general case for these presenters. | |
# | |
class FeaturedListingsContentBlockPresenter < ContentBlockPresenter | |
def items | |
@items ||= begin | |
scope = current_site.listings.featured.published | |
scope = scope.for_category(category) if category | |
scope = scope_for_deduplication(scope, Listing) | |
limited = limit | |
scope.filter do | |
order({ :feature => :published_at }, :desc) | |
limit(limited) | |
end | |
end | |
end | |
end | |
# | |
# 5) The explicit join API can be clunky. Consider this case... I found the combination of hashes and | |
# argument lists a bit confusing when creating them. It obviously makes sense but isn't necessarily | |
# intuitive. It's also not clear which column name should go on the left of the hash declaration | |
# and which one on the right, and the fact that the argument list form switches them isn't easy on the | |
# eyes (hey, I still need to read that wikipedia article on joins, obviously). | |
# | |
named_filter(:for_category) do |category| | |
join(ClassificationsObject, :inner) do | |
on(:id => :classifiable_id) | |
on(:classifiable_type, filter_class.name) | |
on(:classification_type, 'Category') | |
on(:deleted_at, nil) | |
join(Category, :inner) do | |
on(:classification_id => :id) | |
with(:lft).between(category.lft, category.rgt) | |
end | |
end | |
end | |
# | |
# 6) Also in the code example above, I added the filter_class method so that we can get access to the | |
# class being filtered, but it's not very intuitive to use (or that you should have to use it). Once nice | |
# thing about named scopes is that because they are evaled in context of the filtering class they don't | |
# have to do this. I might look into delegating all methods that aren't in the DSL API to the underlying | |
# model class to fix this. | |
# | |
# | |
# 7) We need to make sure we're handling all the wacky cases for AR associations. There are a lot of | |
# ways to customize the associations, and we are probably only handling a small percentage of them. | |
# | |
# | |
# 8) The pending test about using named filters within named filters is still pending. I can take | |
# another look at implementing that now that I have a better grip on things than I did last time I | |
# looked. | |
# |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment