Couldn't find User without an ID, rails 4

Question

I have two user types: Artist and Fan. I want Fans to be able to follow Artists. So far following them does not work, but unfollowing does. I have create and destroy set up the same way, but can't seem to get it to work. I get the error Couldn't find Artist without an ID when trying to create a Relationship. Anyway I can find the Artist's ID?

Code below:

relationships_controller.rb

class RelationshipsController < ApplicationController

  before_action :authenticate_fan!

  def create
    @relationship = Relationship.new
    @relationship.fan_id = current_fan.id
    @relationship.artist_id = Artist.find(params[:id]).id #the error
    if @relationship.save
      redirect_to (:back)
    else
      redirect_to root_url
    end
  end

  def destroy
    current_fan.unfollow(Artist.find(params[:id]))
    redirect_to (:back)
  end

end

artists_controller.rb

def show
  @artist = Artist.find(params[:id])
end

artists/show.html.erb

<% if fan_signed_in? && current_fan.following?(@artist) %>
  <%= button_to "unfollow", relationship_path, method: :delete, class: "submit-button" %>
<% elsif fan_signed_in? %>
  <%= form_for(Relationship.new, url: relationships_path) do |f| %>
    <%= f.submit "follow", class: "submit-button" %>
  <% end %>
<% end %>

models/fan.rb

has_many :relationships, dependent: :destroy
has_many :artists, through: :relationships
belongs_to :artist

def following?(artist)
  Relationship.exists? fan_id: id, artist_id: artist.id
end

def unfollow(artist)
  Relationship.find_by(fan_id: id, artist_id: artist.id).destroy
end

models/artists.rb

has_many :relationships, dependent: :destroy
has_many :fans, through: :relationships
belongs_to :fan

routes.rb

resources :relationships, only: [:create, :destroy]

Answer 1

Basically, you need to send artist_id to the action. Change your form to this. There is a lot of refactoring required but this one will work for you:

<%= form_for(Relationship.new, url: relationships_path) do |f| %>
  <%= hidden_field_tag :artist_id, @artist.id %>
  <%= f.submit "follow", class: "submit-button" %>
<% end %>

In controller, you can access it like:

@relationship.artist_id = Artist.find(params[:artist_id]).id

Answer 2

I would consider solving this with a nested route instead:

resources :artists, shallow: true do
  resources :relationships, only: [:create, :destroy]
end

this will create these routes in addition to the regular CRUD routes for artist:

              Prefix Verb   URI Pattern                                 Controller#Action
artist_relationships POST   /artists/:artist_id/relationships(.:format) relationships#create
        relationship DELETE /relationships/:id(.:format)                relationships#destroy

Notice that we use shallow: true which scopes the create route under /artists/:artist_id but not the destroy route.

You can then change your form:

<%= form_for(Relationship.new, url: artist_relationships_path(artist_id: @artist)) do |f| %>
  <%= f.submit "follow", class: "submit-button" %>
<% end %>

And your controller:

class RelationshipsController < ApplicationController

  before_action :authenticate_fan!

  def create
    current_fan.relationships.build(artist: Artist.find(params[:artist_id]))
    if @relationship.save
      redirect_to(:back)
    else
      redirect_to root_url # makes more sense to redirect back to @artist ?
    end
  end

  def destroy
    @relationship = current_fan.relationships.find(params[:id])
    @relationship.destroy
    redirect_to(:back)
    # or redirect back to the artist page.
    # redirect_to @relationship.artist
  end
end

Notice how we also refactor the destroy action - You never want to have a route with an :id param which points to a completely different resource. Thats just poor REST design. We don't even need to know the artist ID if we know the id of a relationship. Instead here the ID refers to the Relationship resource.

To create a link to destroy the relationship you would do:

<%= link_to 'Unfollow', relationship_path(@relationship), method: :delete %>

And lets get rid of the Fan#unfollow method.

While we are at it we can fix the Fan#following? method.

def following?(artist)
  relationships.exists?(artist: artist)
end

By using the relationship (in the ActiveRecord sense of the word!) instead of querying the Relationship model directly you can use eager loading to avoid additional queries and also you don't have to specify the fan.