Reputation: 61
I have a Books Controller with the following methods:
books_controller.rb
def set_as_reading
if (user_signed_in?)
if (current_user.user_type == "0")
@book = Book.find(params[:id])
@reading = Reading.new
@reading.book_id = @book.id
@reading.profile_id = session[:current_profile]["id"]
@reading.is_finished = false
@reading.save
else
redirect_to books_path
end
else
redirect_to root_path
end
end
def remove_from_reading
if (user_signed_in?)
if (current_user.user_type == "0")
@book = Book.find(params[:id])
@reading = Reading.where(profile_id: session[:current_profile]["id"]).where(book_id: @book.id)
@reading.is_finished = true
@reading.save
else
redirect_to books_path
end
else
redirect_to root_path
end
end
and I've defined the following routes:
routes.rb
post 'books/:id', to: 'books#set_as_reading', as: :set_as_reading
post 'books/:id', to: 'books#remove_from_reading', as: :remove_from_reading
I use these methods in the show view as follows:
show.html.erb
<% if current_user.user_type == "0" %>
<%= link_to "Add to Reading list", @book, method: :set_as_reading, class: 'btn btn-danger',
style: 'margin-right:10px' %>
<%= link_to "Finished", @book, method: :remove_from_reading, class: 'btn btn-danger',
style: 'margin-right:10px' %>
<% end %>
Whenever I click on "Add to Reading list" it works properly, it calls the set_as_reading
method. But if I want to set as "Finished", instead of calling remove_from_reading
, it calls the set_as_reading
.
How can I fix this? Thank you in advance.
EDIT
rake routes output:
books GET /books(.:format) books#index
POST /books(.:format) books#create
new_book GET /books/new(.:format) books#new
edit_book GET /books/:id/edit(.:format) books#edit
book GET /books/:id(.:format) books#show
PATCH /books/:id(.:format) books#update
PUT /books/:id(.:format) books#update
DELETE /books/:id(.:format) books#destroy
set_as_reading POST /books/:id(.:format) books#set_as_reading
remove_from_reading POST /books/:id(.:format) books#remove_from_reading
Upvotes: 1
Views: 68
Reputation: 4686
The problem is that you have one route pointing to two different places. Rails just grabs the first one it sees.
Each route needs to be unique:
post 'books/:id/reading', to: 'books#set_as_reading', as: :set_as_reading
post 'books/:id/finished', to: 'books#remove_from_reading', as: :remove_from_reading
Also, your nested if statements don't always result in a redirect.
If I'm signed in and my user_type is "0", where should I be sent after @reading.save
completes?
And your code is pretty fat. It can be a lot cleaner.
I suggest something like this:
class BooksController < ApplicationController
before_action :set_book, only: %w[set_as_reading remove_from_reading]
before_action :check_user, only: %w[set_as_reading remove_from_reading]
def set_as_reading
@reading = Reading.create(
book: @book,
profile_id: session[:current_profile]["id"], # is this the same as current_user?? If so, just use current_user.id
is_finished: false
)
redirect_to somewhere_path # this is missing currently
end
def remove_from_reading
# is this the same as current_user?? If so, just use current_user.id
@reading = Reading.where(profile_id: session[:current_profile]["id"]).where(book_id: @book.id)
# what happens if no record is found??
redirect_to somewhere_path and return unless @reading.present?
@reading.update(is_finished: true)
redirect_to somewhere_path and return # this is missing currently
end
private
def set_book
@book = Book.find(params[:id])
end
def check_user
redirect_to root_path unless user_signed_in? and return
redirect_to books_path unless current_user.user_type == '0' and return
end
end
Upvotes: 1