Why can I not get data from an associated table?

Question

In my Ruby on Rails application I am trying to display the first_name and last_name of the user who made a booking.

On the bookings/index.html.erb I have this:

<% @bookings.each do |booking| %>
                                    <tr>
                                        <td><%= booking.user.first_name %> <%#= booking.user.last_name %></td>
                                        <td><%= booking.showing.film.title %></td>
                                        <td><%= booking.showing.show_date %></td>
                                        <td><%= booking.showing.show_time.strftime("%H:%M") %></td>
                                        <td></td>
                                        <td><%= booking.seat.row_letter %><%= booking.seat.number %></td>
                                        <td style="padding-right:65px">
                                            <%#= link_to 'Edit', edit_user_path(user) %>  
                                            <%#= link_to 'Show', user_path(user) %>  
                                            <%#= link_to 'Destroy', user_path(user), method: :delete, data: { confirm: 'Are you sure?' } %>
                                        </td>
                                    </tr>
                                <% end %>

The booking.showing.film.title and the code below that works but with the booking.user.first_name I get the error:

NoMethodError in Bookings#index     
undefined method `first_name' for nil:NilClass

But my models I think are set up correctly, user.rb:

has_many :bookings

bookings.rb:

belongs_to :user

bookings_controller:

class BookingsController < ApplicationController
  before_action :set_booking, only: [:show, :edit, :update, :destroy]

  # GET /bookings
  # GET /bookings.json
  def index
    @bookings = Booking.all
  end

  # GET /bookings/1
  # GET /bookings/1.json
  def show
  end

  # GET /bookings/new
  def new
    @booking = Booking.new
  end

  # GET /bookings/1/edit
  def edit
  end

  # POST /bookings
  # POST /bookings.json
  def create
    @booking = Booking.new(booking_params)

    respond_to do |format|
      if @booking.save
        format.html { redirect_to @booking, notice: 'Booking was successfully created.' }
        format.json { render :show, status: :created, location: @booking }
      else
        format.html { render :new }
        format.json { render json: @booking.errors, status: :unprocessable_entity }
      end
    end
  end

  # PATCH/PUT /bookings/1
  # PATCH/PUT /bookings/1.json
  def update
     respond_to do |format|
      if @booking.update(booking_params)
        format.html { redirect_to @booking, notice: 'Booking was successfully updated.' }
        format.json { render :show, status: :ok, location: @booking }
      else
        format.html { render :edit }
        format.json { render json: @booking.errors, status: :unprocessable_entity }
      end
    end
  end

  # DELETE /bookings/1
  # DELETE /bookings/1.json
  def destroy
     @booking.destroy
    respond_to do |format|
      format.html { redirect_to bookings_url, notice: 'Booking was successfully destroyed.' }
      format.json { head :no_content }
    end
  end

  private
    # Use callbacks to share common setup or constraints between actions.
    def set_booking
      @booking = Booking.find(params[:id])
    end

   # Never trust parameters from the scary internet, only allow the white list through.
    def booking_params
      params.require(:booking).permit(:showing_id, :user_id, :seat_id)
    end
end

Am I missing something? The attribute names and table names are correct.

Answer 1

Using more than one dot is a violation of Law of Demeter and is considered as a bad practice. To follow the Law of Demeter, you can delegate the name to the user model and create a instance method in the user class to return the full name,

booking class

class Booking < ActiveRecord::Base
  belongs_to :user
  delegate :full_name, to: :user, prefix: true, allow_nil: true
end

user class

class User < ActiveRecord::Base
  has_many :bookings

  def full_name
    "#{first_name} #{last_name}"
  end
end

the view

<td><%= booking.user_full_name %></td>

If the user is nil this function will return gracefully return nil without complaining about not having a user.

It's always better to delegate the method calls ( don't mean delegate as a keyword specifically ) rather than call wit so many dots, take booking.showing.film.title for example, there's so many things that could go wrong with this, if booking or showing or film changed any attribute you'll need to keep iterating on all your views and fix every one of them every time, instead try to create a centralized function called for example booking.film_title, then you only need to maintain it in a single place.