0

I'm having trouble with my routes. I want my user to be able to edit a guitar that they've added. I have two problems:

1.The url for the edit guitar view is incorrect. The IDs of the user and guitar are switched and I'm not sure why http://localhost:3000/users/23/guitars/26/edit The 23 is the guitar id and the 26 is the user id.

2.When a user is able to update a guitar, it updates all the guitars. I'm not sure why this is happening either.

I've spent some time trying to fix this but can't figure it out. Here's the relevant code:

My routes:

Rails.application.routes.draw do
devise_for :registrations
root to: 'pages#home'

resources :users, only: [ :show, :new, :create, :edit, :update, :destroy ] do
resources :guitars, only: [ :edit, :update ]
end

resources :guitars, only: [ :show, :new, :create, :destroy ]
# For details on the DSL available within this file, see 
http://guides.rubyonrails.org/routing.html
end

Rails Routes

Guitar model:

class Guitar < ApplicationRecord
  belongs_to :user
  has_many :transactions, dependent: :destroy
end

User model:

class User < ApplicationRecord
 has_many :guitars, dependent: :destroy
 has_many :transactions, dependent: :destroy
 belongs_to :registration
end

Guitar Controller:

def edit
 @user = User.find(params[:id])
 @guitar = @user.guitars.find(params[:id])
end

def update
 @guitar = Guitar.find(params[:id].to_i)
 @guitar = Guitar.update(guitar_params)
 if @guitar
   redirect_to user_path(current_user)
 else
   render :new
 end
end

private

def guitar_params
  params.require(:guitar).permit(:brand, :model, :condition, :finish, :year, 
  :description, :price)
end

Link to edit guitar:

 <% @guitars.each do |guitar|%>
  <p> <%= guitar.brand %> </p>
  <p><%= link_to 'Edit guitar', edit_user_guitar_path(guitar) %></p>
 <% end %>

Edit guitar view:

<h1>Edit your guitar</h1>
<%= simple_form_for  @guitar do |f| %>
    <%= f.error_notification %>
    <%= f.input :brand, required: true, autofocus: true, input_html: {value: 'brand'} %>
    <%= f.input :model, required: true, autofocus: true, input_html: {value: 'model'} %>
    <%= f.input :condition, required: true, autofocus: true, input_html: {value: 'condition'} %>
    <%= f.input :finish, required: true, autofocus: true, input_html: {value: 'finish'} %>
    <%= f.input :year, required: true, autofocus: true, input_html: {value: 'year'} %>
    <%= f.input :description, required: true, autofocus: true, input_html: {value: 'description'} %>
    <%= f.input :price, required: true, autofocus: true, input_html: {value: 'price'} %>
    <%= f.button :submit, :class => "btn btn-primary" %>
<% end %>
Jawaka72
  • 65
  • 8

1 Answers1

1

There are a couple of problems with the above - in your controller code:

def edit
 @user = User.find(params[:id])
 @guitar = @user.guitars.find(params[:id])
end

def update
 @guitar = Guitar.find(params[:id].to_i)
 @guitar = Guitar.update(guitar_params)
 if @guitar
   redirect_to user_path(current_user)
 else
   render :new
 end
end

You're only ever referencing params[:id], but the user id will be coming in under the nested route as :user_id, so the User.find should be User.find(params[:user_id]).

I'd also point out that unless you want users to edit other users guitars, I'd personally do away with the user_id being in the route at all, and instead just use current_user, assuming that's something that is exposed by your authorization setup:

def edit
  @guitar = current_user.guitars.find(params[:id])
end

Otherwise, someone could potentially guess a user_id and then an id for a guitar belonging to that user, and edit it.

The other issue is if you do keep your nested route, then your link code will need updating:

<p><%= link_to 'Edit guitar', edit_user_guitar_path(guitar) %></p>

As you have a nested route, you need to specify all of the parts of the route, in order - in this case, user, then guitar:

<p><%= link_to 'Edit guitar', edit_user_guitar_path(guitar.user, guitar) %></p>

As I mentioned though, this highlights how the nested route is a little redundant if users can only edit their own guitars.

Hope that helps!

ejdraper
  • 439
  • 2
  • 6
  • Thank you @ejdraper. That fixed my routing issue. I was using current_user before, but was trying different things to solve the problem. The edit_user_guitar_path(guitar.user, guitar) did the trick. I'm still having the problem of updating only one guitar. After a use edits his guitar and presses update, all the guitars are changed. – Jawaka72 Jan 13 '18 at 18:37
  • I figured it out. Just a silly mistake on my part. Thanks again for your help! – Jawaka72 Jan 13 '18 at 20:48