Ah-ha! Didn't think of the super-without-paranthesis bug! Very subtle, good catch!
-- Dheeraj Kumar On Wednesday 22 May 2013 at 4:07 PM, Nicolás Sanguinetti wrote: > The problem with that is that you're overloading the meaning of > association(flag). When passing a boolean to an association, you are telling > it to force-reload the association (if true). If I read in the code > "post.comments(true)" I will always think you're trying to force-reload the > association, which would be weird :) > > If you want to have both versions of the association (with and without the > scope), I would just define a different method (`visible_comments`). > > There's also a subtle bug with your code: The version of super without > parentheses will invoke the superclass method *with the same arguments than > the current method*, which would pass the boolean flag to the association > per-se, which would have weird effects (since you'd force an extra query > every time you try to load the visible comments). > > You would need to call super() like so: > > def comments(show_hidden = false) > show_hidden ? super() : super().visible > end > > > Cheers, > -foca > > > > On Wed, May 22, 2013 at 5:25 AM, Dheeraj Kumar <[email protected] > (mailto:[email protected])> wrote: > > foca, in your example, I'd use Post.find(1).comments to get only the > > visible comments. How would I be able to get ALL the comments, including > > the hidden ones? I'm thinking of passing a parameter to the comments method. > > > > class Post > > has_many :comments > > > > def comments(show_hidden = false) > > show_hidden ? super : super.visible > > end > > end > > > > > > Is this the right way? > > > > -- > > Dheeraj Kumar > > > > > > On Wednesday 22 May 2013 at 6:34 AM, Nicolás Sanguinetti wrote: > > > > > IIRC with Rails 4 associations introduce their methods via a module, > > > which means you can `super` to them, so why not: > > > > > > class Post > > > has_many :comments > > > > > > def comments(*) > > > super.visible > > > end > > > end > > > > > > I'm not too keen on having moar DSL when defining a simple method > > > suffices. It also gives you more flexibility to call whatever on the > > > association. > > > > > > For example, your suggestion to avoid an Array of scopes is to define an > > > extra scope / singleton method on Comment for the sole purpose of having > > > something you can call here. But then you're defining a method on Comment > > > that you only use from Post, which is weird (and potentially > > > confusing—later one someone might change the scope passed to the > > > association, but might not know that the scope definition on Comment is > > > no longer used, so you end up with dead code). > > > > > > With this approach you can simply call whatever you want on the method > > > overriding the association, instead of having to do extra work just for > > > the sake of having a DSL. > > > > > > Cheers, > > > -foca > > > > > > > > > > > > On Wed, May 22, 2013 at 12:41 AM, Caleb Thompson <[email protected] > > > (mailto:[email protected])> wrote: > > > > I'm considering implementing a feature by which a collection > > > > association might limit the results. > > > > > > > > By default, a collection association returns all values where the > > > > foreign key on the `belongs_to` or `has_and_belongs_to` model matches > > > > the parent object's primary key. These results can be filtered using the > > > > `conditions` option, but that requires that other models have knowledge > > > > of the parent model's table structure. > > > > > > > > The feature I'm proposing is to add a `scope` option to collection > > > > associations which takes a symbol representing a scope (or class mehtod) > > > > defined on the associated model class. > > > > > > > > A basic example would look like this: > > > > > > > > class Comment > > > > belongs_to :post > > > > > > > > def self.visible > > > > where(deleted_at: nil) > > > > end > > > > end > > > > > > > > class Post > > > > has_many :posts, scope: :visible > > > > end > > > > > > > > In the above example, `post.comments` would return `Comment` instances > > > > whose `post_id = post.id (http://post.id)` and whose `deleted_at = > > > > NULL`, providing basic > > > > soft-deletion functionality. > > > > > > > > While the same effect could be achieved with a declaration such as > > > > `has_many :posts, conditions: ['deleted_at = ?', nil]`, that betrays > > > > knowledge of the implementation of deletion on `Post` and would break if > > > > the implementation were changed to a boolean value for deletion rather > > > > than a timestamp field. > > > > > > > > If I were to work on adding this functionality into ActiveRecord, is it > > > > > > > > something that core might entertain merging? > > > > > > > > Thank you, > > > > > > > > Caleb Thompson > > > > > > > > -- > > > > You received this message because you are subscribed to the Google > > > > Groups "Ruby on Rails: Core" group. > > > > To unsubscribe from this group and stop receiving emails from it, send > > > > an email to [email protected] > > > > (mailto:rubyonrails-core%[email protected]). > > > > To post to this group, send email to [email protected] > > > > (mailto:[email protected]). > > > > Visit this group at > > > > http://groups.google.com/group/rubyonrails-core?hl=en. > > > > For more options, visit https://groups.google.com/groups/opt_out. > > > > > > > > > > > > > > -- > > > You received this message because you are subscribed to the Google Groups > > > "Ruby on Rails: Core" group. > > > To unsubscribe from this group and stop receiving emails from it, send an > > > email to [email protected] > > > (mailto:[email protected]). > > > To post to this group, send email to [email protected] > > > (mailto:[email protected]). > > > Visit this group at http://groups.google.com/group/rubyonrails-core?hl=en. > > > For more options, visit https://groups.google.com/groups/opt_out. > > > > > > > > > > -- > > You received this message because you are subscribed to the Google Groups > > "Ruby on Rails: Core" group. > > To unsubscribe from this group and stop receiving emails from it, send an > > email to [email protected] > > (mailto:rubyonrails-core%[email protected]). > > To post to this group, send email to [email protected] > > (mailto:[email protected]). > > Visit this group at http://groups.google.com/group/rubyonrails-core?hl=en. > > For more options, visit https://groups.google.com/groups/opt_out. > > > > > > -- > You received this message because you are subscribed to the Google Groups > "Ruby on Rails: Core" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected] > (mailto:[email protected]). > To post to this group, send email to [email protected] > (mailto:[email protected]). > Visit this group at http://groups.google.com/group/rubyonrails-core?hl=en. > For more options, visit https://groups.google.com/groups/opt_out. > > -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/rubyonrails-core?hl=en. For more options, visit https://groups.google.com/groups/opt_out.
