Rails like and unlike posts

Question

I have the following two methods which handle the liking and unliking of posts in my Rails application:

def create
  @post = Post.find(params[:id])
  if @post.present?
    @like = Like.new(user_id: current_user.id, post_id: @post.id)
    if @like.save
      redirect_to post_redirect(@post), :notice => 'Liked!'
    else
      redirect_to post_redirect(@post), :alert => 'An error prevented you from liking this post!'
    end
  else
    redirect_to post_redirect(@post), :alert => 'Invalid post!'
  end
end

def destroy
  @post = Post.find(params[:id])
  if @post.present?
    @like = Like.where(user_id: current_user.id, post_id: @post.id).first
    if @like.destroy
      redirect_to post_redirect(@post), :notice => 'Unliked!'
    else
      redirect_to post_redirect(@post), :alert => 'An error prevented you from unliking this post!'
    end
  else
    redirect_to p, :alert => 'Invalid post!'
  end
end

They both check that the post exists first and then creates or destroys the like record containing the post id and user id and redirects to the post with a message depending on the outcome.

In the Like model I prevent duplicate likes with: validates :user_id, uniqueness: {scope: :post_id}

However the methods feel quite bloated... is there a better way to handle this?

Answer 1

You're right. The code can be improved. Here is how I'd write this.

1. We can extract post lookup into a method or before hook.
2. There is no need to check if Post was found. In production environment find will raise an ActiveRecord::RecordNotFound exception (I'm assuming you're using AR)
3. The destroy action can be a little more optimistic. When somebody unlikes the post, which cannot be found or which is not liked, we can just show the success. After all there is no like for the post.
4. We can use associations on models to write less.
5. AR understands relations in conditions. where(user: current_user) instead of where(user_id: current_user.id)
6. Use find_by instead of where().first

For example:

class LikesController < ApplicationController
  # before_action :find_post

  def create
    if post.likes.create(user: current_user)
      redirect_to post_redirect(post), :notice => 'Liked!'
    else
      redirect_to post_redirect(post), :alert => 'An error prevented you from liking this post!'
    end
  end

  def destroy
    post.likes.where(user: current_user).destroy_all
    redirect_to post_redirect(post), :notice => 'Unliked!'
  end

  private

  def post
    @post ||= Post.find(params[:id])
  end

  # def find_post
  #   @post = Post.find(params[:id])
  # end
end

Answer 2

to expand a bit on @cutalion 's good answer:

class LikesController < ApplicationController
  # before_action :find_post

  after_action :redirect, only: [:create, :destroy]

  def create
    set_flash notice: 'Liked!' if like.save
  end

  def destroy
    set_flash notice: 'Unliked!' if likes.destroy_all
  end

  private

  def post
    @post ||= Post.find(params[:id])
  end

  def likes
    post.likes.where(user: current_user)
  end

  def like
    likes.build
  end

  def redirect
    redirect_to post_redirect(post), flash_message
  end

  def flash_message
    @flash_message ||= error_flash
  end

  def error_flash
    { alert: 'An Error Occurred.' }
  end

  def set_flash(message)
    @flash_message = message
  end
end