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