Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create likes #3

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Create likes #3

wants to merge 6 commits into from

Conversation

AshleyFoster
Copy link
Owner

@AshleyFoster AshleyFoster commented May 4, 2018

This sets up likes. I would like to be able to use the likes? method from the user model on the front end to correctly show whether the user has liked the specific post. I'm just not sure how to go about it.

@AshleyFoster AshleyFoster requested review from BlakeWilliams and removed request for BlakeWilliams May 4, 2018 19:14
@like = @post.likes.where(user_id: current_user.id).first_or_create

unless @like.save
puts @like.errors.inspect
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove comment?

has_many :posts, through: :likes

def likes?(post)
post.likes.where(user_id: id).any?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could be: likes.find_by(post_id: post.id).present?

end

def destroy
@like = @post.likes.where(user_id: current_user.id).destroy_all
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be:

@like = current_user.likes.find_by(post_id: @post.id)
@like.destroy

head 204

The first change because you always want to think about operating on the current users likes because if you don't, you could be operating on any like. You cover that case here but I feel like this format makes the behavior/intention more clear.

The head 204 is nice because you're saying there's no content to send down

before_action :set_post

def create
@like = @post.likes.where(user_id: current_user.id).first_or_create
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment here, putting the focus on the current user.

@@ -9,5 +9,10 @@ class User < ApplicationRecord
validates :password, presence: true
validates :authentication_token, uniqueness: true

has_many :posts
has_many :likes
has_many :posts, through: :likes
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is what I want to be doing here?

@AshleyFoster AshleyFoster force-pushed the master branch 2 times, most recently from dd3a047 to 121314d Compare June 26, 2018 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants