# LikesControler for a Post

I have this controller that I call via Ajax, where I update the likes of a post.

I have two methods in the user model:

• already_like_post?: Where I pass the posted ID and return true or false if the post is already liked.
• dislike_post: Delete the user like for given post.

class LikesController < ApplicationController
def create
like = Like. new user: current_user, post_id: params[:post_id]
if !like.save
render json: like.errors
end
else
current_user.dislike_post params[:post_id]
end
amount_of_likes = Post.find(params[:post_id]).likes.count
render json:{status:"success",likes: amount_of_likes}
end
end


These are the methods in the user model:

def already_liked_post?(post_id)
self.likes.where(post_id: post_id).size == 1
end

def dislike_post(post_id)
self.likes.find_by(post_id: post_id).destroy
end


Can you give a review of this? I don't know if this looks good or if it is the Rails way.

• How do you record dislikes, I believe you also have a Dislike model? or keep it under the Like model but having an attribute to determine whether it was "liked" or "disliked"(bloody). Aug 20, 2015 at 8:25

Some notes:

• Use 2-space indentation.
• Move all the logic to the model.
• DB deletes can also fail.
• No need for self.
• If you use object instead of object_id as argument, code is usually more declarative.
• In case of error, you are performing a double render.

I'd write:

class LikesController < ApplicationController
def create
post = Post.find(params[:post_id])

if current_user.toggle_like(post)
render(json: {status: "success", likes: post.likes.count})
else
render(json: {status: "error"}, status: :unprocessable_entity)
end
end
end

class User < ActiveRecord::Model
# Toggle like for post. Returns boolean with the status of the operation.
def toggle_like(post)
likes_for_post = likes.where(post_id: post.id)

if likes_for_post.exists?
likes_for_post.destroy_all.all?(&:destroyed?)
else
likes.new(post: post).save
end
end
end

• Thanks, I really liked that I even did the refactoring. Two notes: 1. The method exist?, its named exists?. 2. It's throwed an error when I called the destroy method instead I had to use the destroy_all method. Thanks. Aug 5, 2015 at 12:44
• You are spot-on on both notes, updated. Aug 5, 2015 at 15:11
• @Nivla: I've refactored toggle_like to make sure it returns a meaningful boolean. Aug 6, 2015 at 9:34
• Great, what means .all?(&:destroyed?), the current record already destroyed? Aug 6, 2015 at 15:04
• Yes, it makes sure all the Like objects (usually, just one), have been destroyed. "destroy" may not suceed (it returns false) for example if a before_destroy callbacks stops it. Aug 6, 2015 at 15:50

Architecturally this looks fine. The database end of might need some tweaking, however.

amount_of_likes = Post.find(params[:post_id]).likes.count


This results in two queries:

SELECT * FROM posts WHERE post_id = ...
SELECT COUNT(*) FROM likes WHERE post_id = ...


This will load the Post, then do a count(*) on the likes relation. Why not get the count by the Like model instead:

amount_of_likes = Like.where(user_id: current_user.id).count


(assuming the Like and User models are related in that way)

This does a SELECT COUNT(*) FROM likes WHERE user_id = ... which would avoid loading the Post object.

The only other thing I would recommend is to return a 422 HTTP response code if the Like failed to save:

    if !like.save
render json: like.errors, status: 422
end
else
...
end
...
render json: { likes: amount_of_likes }


This would eliminate the need for a "status" flag. Switching the HTTP response code would prevent this route in your application from being used as a JSONP request, but if it's just normal same-domain AJAX or cross-domain AJAX with the proper permissions in place, then the status code would be ideal.

The error handler in JavaScript on the client would handle the validation errors.

• According to a deleted answer I converted to a comment on the question: You forgot to add post_id on the where query. def already_liked_post?(post_id) Like.where(user_id: current_user.id, post_id: params[:post_id]).count > 0 end (I personally don't know if this is correct or not) Aug 20, 2015 at 10:22