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?

Answer 1

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

device.try(&:name)

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.

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.

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

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.