NewbieOnRails
NewbieOnRails

Reputation: 61

Problem calling a controller method from view

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

Answers (1)

Chiperific
Chiperific

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

Related Questions