CODEWITHSUNDEEP
ruby-on-railsruby
echan00
echan00

Reputation: 2807

Any better way to execute something like this?

I'm trying to list all the user's products with a probable association where a flag 'notification' is set to zero.

user.probable_associations.where(:notified => 0).collect{|a| Product.where(:id => a.product_id).collect{|p| p.name}}.to_sentence

It seems like using a where and collect method twice within the statement isn't very good. Is there a better way to go about this?

Also, the result is something like

"[\"Product A\"] and [\"Product B\"]"

which is pretty ugly...and I still need to remove the extra punctuation "[\" \"] instead of something clean like

"Product A and Product B"

EDIT based on Rich's Answer, still have issues because notified is a field in associations NOT product:

  has_many :probable_associations, -> { where "associations.category = 3"}, class_name: 'Association', before_add: :set_probable_category
  has_many :probable_products, class_name: 'Product', through: :probable_associations, source: :product do
    def not_notified
        select(:name).where(notified: 0)
    end  
  end

Upvotes: 0

Views: 58

Answers (3)

tompave
tompave

Reputation: 12402

Assuming that probable_associations is just an ActiveRecord has_many association, and that you want to end up with a list of titles for Product records, you can use this:

ids = user.probable_associations
          .where(notified: 0)
          .pluck(:product_id)

result = Product.where(id: ids).pluck(:name).to_sentence

It's similar to @spikermann's answer, but pluck(:column_name) is faster than using a block and only extracts the required column from the DB.

Also, the reason your code produces that string is that, by the time you call to_sentence, you have an Array of sub-arrays. Each sub-array contains a single element: a product name.

That's because the second collect is sent to an ActiveRecord::Relation containing just one record.

You could have solved that problem with flatten, but the whole operation could just be refactored.

Upvotes: 0

spickermann
spickermann

Reputation: 106802

Without knowing what probable_associations does would I rewrite the code to something like this:

product_ids = user.probable_associations.where(:notified => 0).map(&:product_id)
Product.where(:id => product_ids).map(&:name).to_sentence

Upvotes: 0

Richard Peck
Richard Peck

Reputation: 76774

I'd use an ActiveRecord Association extension:

#app/models/user.rb
Class User < ActiveRecord::Base
    has_many :products do
        def not_notified
            select(:name).where(notified: 0)
        end
    end
end

#-> @user.products.not_notified

That's my contribution, but you could then use @spickermann & @tompave's controbutions and use .flatten.to_sentence

Upvotes: 1

Related Questions