Tricky Ruby/Rails loop and query logic. Advice needed. SQL join or group?

Question

I'm still pretty green in Ruby, Rails, and SQL (and programming in general) but I know the basics. Here's my little puzzle involving a wine cellar organization app:

I have models. Wine - with the attributes :name and :vintage, Winery - which Wine belongs to, User - who has many wines, Location - the wine's cellar location which is a string attribute :name, and Bottle - every single bottle has a record with user, wine, and location ids. Here's my gist with models, schema, the controller, and the view I want to work with: https://gist.github.com/mpron/8725840

Here's what I want to do:

• Show rows of a user's wine sorted alphabetically by wine name
• I want to show multiple rows of the same wine name if bottles of that wine exist at multiple locations (if there are bottles with the same wine name in 3 different locations, then I want to show three separate rows for each location)
• I need to show the number of wine bottles at each location on each of those rows for that one wine name (so a bottle '.count' for the bottles with the same wine.name).

Here is my rough, slightly pseudocode solution that I have so far:

- if @bottles.count > 0
  You have 
  = @bottles.count
  bottles
  %table.table
    %tr
      %th Location
      %th Vintage
      %th Name
      %th Winery
      %th Quantity
      %th Actions
    - @wines.each do |wine|
      - @locations each do |loc|
        - if wine.@bottles.each exists in loc
          %tr
            %td
              = "#{loc}"
            %td
              = "#{wine.vintage}"
            %td
              = "#{wine.name}"
            %td
              = "#{wine.winery.name}"
            %td
              = "#{loc.bottles.where(name: wine.name).count}"
            %td
              = link_to "Delete", bottle_path(bottle), :method => "delete", :data => {:confirm => "Are you sure?"}, :class => "btn btn-danger btn-sm"

This is also in the gist along with other relevant files.
My questions are: 1. What is the best way to query the database efficiently for this data? 2. Does this belong in the view, or should it exist somewhere else, maybe a partial?

If there's a more efficient way to model this, I'm open to significantly revamping this. I just don't know enough SQL to figure out how I might use something like GROUP BY or a JOIN to make this work well. A friend suggested something like Location.joins(:bottles).where(name: wine.name) but later decided it might result in bad performance.

(this is for a personal project, not homework or anything. I'm just stumped.)

Answer 1

@Beartech helped get me thinking about how I could utilize the has_many through: associations, so after re-studying those, I came up with my solution:

wine.rb

class Wine < ActiveRecord::Base

  belongs_to :winery
  has_many :bottles
  has_many :locations, through: :bottles
end

user.rb

class User < ActiveRecord::Base
  # Include default devise modules. Others available are:
  # :confirmable, :lockable, :timeoutable and :omniauthable
  devise :database_authenticatable, :registerable,
  :recoverable, :rememberable, :trackable, :validatable

  has_many :wines, through: :bottles
  has_many :bottles
end

location.rb

class Location < ActiveRecord::Base
    has_many :bottles
  has_many :wines, through: :bottles

end

bottle.rb

class Bottle < ActiveRecord::Base

  belongs_to :wine
  belongs_to :user
  belongs_to :location

end

section of users_controller.rb

  def show
    @user_wines = @user.wines.order('wines.name')
    @bottles = @user.bottles
  end

show.html.haml

  %table.table
    %tr
      %th Location
      %th Vintage
      %th Name
      %th Winery
      %th Quantity
      %th Actions
    - @user_wines.each do |wine|
      - wine.locations.each do |loc|
        %tr
          %td= loc
          %td= wine.vintage
          %td= wine.name
          %td= wine.winery.name
          %td= loc.bottles.where(:wine_id => wine.id).count

That will run through a much narrower subset of the wines in the database, and an even smaller subset of the locations in the database so that my nested loop isn't querying as much.

Answer 2

My initial answer to your question would be to go with better associations. Refactor how your associations are structured. For instance, remove the association of belongs_to :user from bottle:

class Bottle < ActiveRecord::Base

  belongs_to :wine
  # belongs_to :user
  belongs_to :location

end

Add a belongs_to in the wine model:

class Wine < ActiveRecord::Base
  belongs_to :user
  belongs_to :winery
  has_many :bottles
  #has_many :locations
end

And add a has_many through: to user:

class User < ActiveRecord::Base
  # Include default devise modules. Others available are:
  # :confirmable, :lockable, :timeoutable and :omniauthable
  devise :database_authenticatable, :registerable,
  :recoverable, :rememberable, :trackable, :validatable
  has_many :wines
  has_many :bottles, through :wines
end

You will have to adjust your columns accordingly to get rid of foreign keys you don't need and add any new ones you need. You don't need an explicit key for has_many through as Rails will do the work for you. user -> wine -> bottles See this as a decent example: Getting count by drilling down through relations

This will give you the ability to just pull a User:

@user = User.first

and you will automatically get joins that can be access with methods like:

my_wines = @user.wines # wines in a user's cellar
my_bottles = @user.bottles # bottles in a user's cellar

these all return ActiveRecord Relations that represent arrays of AR objects. You can then access those object like:

my_bottles.each do |bottle|
   puts bottle.name
   puts bottle.location
end

I actually needed to learn more about using the .joins method because I had learned the has_many through so early on I didn't even realize it was doing joins for me. At the very least do some tutorials on has_many through and anything you can find on model and helper methods. The CodeSchool.com link I posted in the comments is a HUGE help. It's $25 for a month but worth every cent.

.joins is good for when you need to bring together information in separate tables that doesn't have the explicit associations you've defined. Or when you want to pull the information from two table, but just the info that relates to a certain user. For performance I would watch out for doing any kind of SQL request inside of a loop. I didn't see any here, but just a heads up.

Your view will be much cleaner by removing all of the nested loops and laying out your logic in associations and in a helper method or two. Think of it this way, that triple nested loop in your view is basically a layered object. You should leverage your associations and build some methods that encapsulate that logic so you only have one .each statement in your view. Refactor you associations, and if you still don't see an easier way to encapsulate your view logic let me know and I can give some more direction.