Display related records, if they exist, in Rails view

Question

Is this much logic in view is justified to check for the presence of record?

# device model
class Device < ApplicationRecord
 validates :abbr, uniqueness: { case_sensitive: false }, presence: true
 validates :name, presence: true
 def abbr=(value)
 self[:abbr] = value.to_s.strip
 end
end

Here is the code for DeviceVendor that also saves abbr column of Device to uniquely identify the device:

# model code 
class DeviceVendor < ApplicationRecord
 validates :vendor, uniqueness: {scope: :device}
end
# controller code
def index
 @device_vendors = DeviceVendor.order(:device, :vendor)
end
# view code
<% @device_vendors.each do |device_vendor| %>
 <% device = Device.where(abbr: device_vendor.device).first %>
 <% vendor = Vendor.where(abbr: device_vendor.vendor).first %>
 <tr>
 <td><%= device.name if device %></td>
 <td><%= vendor.name if vendor %></td>
 <td><%= link_to 'Show', device_vendor %></td>
 <td><%= link_to 'Edit', edit_device_vendor_path(device_vendor) %></td>
 </tr>
<% end %>

What is the rails way to do this?

• Pavan: What are the associations between Device and DeviceVendor?
• Sergio Tulentsev: Yes, it is too much logic in views.
• Rajkaran Mishra: @Pavan Currently Device has abbr column which will be unique and this will be saved in DeviceVendor not device_id
• Pavan: Just update the associations please
• puneet18: @devel check my answer, it will work.

Answer 1

There's a lot of ways to check the presence of an object, i prefer to use try :

device.try(&:name)

• Sergio Tulentsev: try doesn't accept a block, though.

Answer 2

You could also use first_or_initialize. It tries to find the record and, if not found, creates a new unsaved object (so that you always have an object and don't have to branch. This is called "confident code").

<% device = Device.where(abbr: device_vendor.device).first_or_initialize %>
<% vendor = Vendor.where(abbr: device_vendor.vendor).first_or_initialize %>
<tr>
 <td><%= device.name %></td>
 <td><%= vendor.name %></td>

Although I'm not a fan of queries in views like that. You should make proper activerecord relations there. Or, at least, move the queries to DeviceVendor's methods.

<td><%= device_vendor.device_or_default.name %></td>
<td><%= device_vendor.vendor_or_default.name %></td>

Answer 3

I can see three problems with this example.

1. You are doing DB queries in the View template

A simple rule of thumb with Rails views: avoid triggering DB queries in your views at all costs.

The Rails convention is to do all your data preparation in the Controller, and your Views should only use what has already been gathered.

An exception to this rule is lazy-loaded data, but that would still be initialized somewhere else.

2. You are performing queries in a loop

By performing a DB query inside a loop, you are generating far more requests than is likely necessary.

3. You are (it appears) relying on a uniqueness constraint in the model

Never rely on validation alone to prevent duplicate records in your database. It is possible that you've already accounted for this and simply omitted the relevant code, but I want to make that point in case you haven't because it is important.

---

Honestly it seems like you should find a way to make abbr an actual foreign key so that you can rely on the framework to do lookups for you. Non-numeric keys are a thing. This looks like a standard many-many relationship, why are you reinventing the wheel here?

If that is not possible, here's how I would approach this:

Controller

def index
 @device_vendors = DeviceVendor.order(:device, :vendor)
 @devices = Device.where(abbr: @device_vendors.map(&:device).uniq).index_by(&:abbr)
 @vendors = Vendor.where(abbr: @device_vendors.map(&:vendor).uniq).index_by(&:abbr)
end

Using index_by in this way creates a Hash keyed to the abbr value. If abbr is non-unique and there are multiple records with the same value, this gets slightly more complicated but the principle is the same: pull in every record you need up front with as few queries as feasible.

View

<% @device_vendors.each do |device_vendor| %>
 <tr>
 <td><%= @devices[device_vendor.device].try(:name) %></td>
 <td><%= @vendors[device_vendor.vendor].try(:name) %></td>
 <td><%= link_to 'Show', device_vendor %></td>
 <td><%= link_to 'Edit', edit_device_vendor_path(device_vendor) %></td>
 </tr>
<% end %>

Using try covers the case where the abbr key wasn't found.

• Sergio Tulentsev: "You are creating an n+2 query problem" - it's 2N + 1, actually :)
• Adam Lassek: @SergioTulentsev I should have known invoking O-notation would bite me.

Answer 4

Is this much logic in view is justified to check for the presence of record?

It is justified to check for a presence of a record in the view but you are writing AR queries in the view which will trigger DB calls and its not the Rails way to do it! You should instead move those to corresponding view helper or define it as model methods. I would do

#view_helper
def get_device_name_and_vendor_name(dv)
 device = Device.where(abbr: dv.device).first
 device_name = device.blank? ? nil : device.name
 vendor = Vendor.where(abbr: dv.vendor).first
 vendor_name = vendor.blank? ? nil : vendor.name
 return device_name,vendor_name
end

And in the view, call that method to get device name and vendor name

<td><%= get_device_name_and_vendor_name(device_vendor)[0] %></td>
<td><%= get_device_name_and_vendor_name(device_vendor)[1] %></td>

Note: I would also instead apply associations to the models to cleanup those queries.

• Rajkaran Mishra: Can you also explain how association can be applied in this scenario as device_id is not getting used so that I can seed all the data to avoid association
• Pavan: @devel Associations in your case is a long shot,since you need to add corresponding Foreign Key in the tables and the data in the db need to be altered too to map the corresponding records. But they provide more flexibility. My suggestion, take it or leave it :)
• Sergio Tulentsev: @devel: foreign key column does not have to be device_id. You can customize it. Something along the lines of belongs_to :device, foreign_key: :abbr. Consult docs for the exact syntax.
• Sergio Tulentsev: @Pavan: you realize your proposed code makes 4 queries?
• Pavan: @SergioTulentsev Yeah! Since you already pointed out in your answer, I have to think of another way and the final result is this answer :)

Answer 5

Using try method of ruby, exception will not be raised and nil will be returned instead, if the receiving object is a nil object.

Try below code in view:

# view code
<% @device_vendors.each do |device_vendor| %>
 <% device = Device.where(abbr: device_vendor.device).first %>
 <% vendor = Vendor.where(abbr: device_vendor.vendor).first %>
 <tr>
 <td><%= device.try(:name) %></td>
 <td><%= vendor.try(:name) %></td>
 <td><%= link_to 'Show', device_vendor %></td>
 <td><%= link_to 'Edit', edit_device_vendor_path(device_vendor) %></td>
 </tr>
<% end %>

OR correct way is to add association

Device has_many_and_belongs_to Vendor
Vendor has_many_and_belongs_to Device

DeviceVendor is Join table of Device and Vendor

• Pavan: So whats the difference between your answer and Othmane El kesri's answer?
• puneet18: @Pavan I am explaining whats the try method of ruby and how to use it.
• puneet18: @SergioTulentsev Thanks for your suggestion. I updated my answer

Answer 6

You can still create relationships on keys that are not called id (although I wouldn't recommend it. I would recommend using id, at some point someone will want to change an abbreviation and it will not be easy. Joins on integers are also faster than joins on strings). I also suggest you name your join fields on DeviceVendor as device_abbr and vendor_abbr so that you can add logical relationships called device and vendor without causing a conflict.

If you want to join on abbreviation you can do something like:

class Device < ApplicationRecord
 validates :abbr, uniqueness: { case_sensitive: false }, presence: true
 validates :name, presence: true
 has_many :device_vendors, foreign_key: :vendor_abbr, primary_key: :abbr
 # You can do something similar with has_and_belongs_to_many
 def abbr=(value)
 self[:abbr] = value.to_s.strip
 end
end
class DeviceVendor < ApplicationRecord
 validates :device_abbr, presence:true
 validates :vendor_abbr, presence:true, uniqueness: {scope: :device_abbr}
 belongs_to :vendor, foreign_key: :vendor_abbr, primary_key: :abbr
 belongs_to :device, foreign_key: :device_abbr, primary_key: :abbr
end
# controller code
def index
 @device_vendors = DeviceVendor.order(:device, :vendor).include(:device, :vendor)
 # You could also simplify the above with a `scope` on DeviceVendor
end
# view code
<% @device_vendors.each do |device_vendor| %>
 <tr>
 <td><%= device_vendor.device.name if device_vendor.device %></td>
 <td><%= device_vendor.vendor.name if device_vendor.vendor %></td>
 <td><%= link_to 'Show', device_vendor %></td>
 <td><%= link_to 'Edit', edit_device_vendor_path(device_vendor) %></td>
 </tr>
<% end %>

Answer 7

Another solution would be to add device_name and vendor_name methods to the DeviceModel and encapsulate that logic there.