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

add API routes for comment likes #8439

Merged
merged 2 commits into from
Jun 4, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
88 changes: 80 additions & 8 deletions app/controllers/api/v1/likes_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ def show
post = post_service.find!(params.require(:post_id))
raise ActiveRecord::RecordInvalid unless post.public? || private_read?

likes_query = like_service.find_for_post(params[:post_id])
likes_query = find_likes

return unless likes_query

likes_page = index_pager(likes_query).response
likes_page[:data] = likes_page[:data].map {|x| like_json(x) }
render_paged_api_response likes_page
Expand All @@ -33,31 +36,42 @@ def create
post = post_service.find!(params.require(:post_id))
raise ActiveRecord::RecordInvalid unless post.public? || private_read?

like_service.create_for_post(params[:post_id])
if params[:comment_id].present?
create_for_comment
else
create_for_post
end
rescue ActiveRecord::RecordInvalid => e
if e.message == "Validation failed: Target has already been taken"
return render_error 409, "Like already exists"
end

raise
else
head :no_content
end

def destroy
post = post_service.find!(params.require(:post_id))
raise ActiveRecord::RecordInvalid unless post.public? || private_read?

success = like_service.unlike_post(params[:post_id])
if success
head :no_content
if params[:comment_id].present?
destroy_for_comment
else
render_error 410, "Like doesn’t exist"
destroy_for_post
end
end

private

def find_likes
if params[:comment_id].present?
return unless comment_and_post_validate(params[:post_id], params[:comment_id])

like_service.find_for_comment(params[:comment_id])
else
like_service.find_for_post(params[:post_id])
end
end

def like_service
@like_service ||= LikeService.new(current_user)
end
Expand All @@ -66,9 +80,67 @@ def post_service
@post_service ||= PostService.new(current_user)
end

def comment_service
@comment_service ||= CommentService.new(current_user)
end

def like_json(like)
LikesPresenter.new(like).as_api_json
end

def create_for_post
like_service.create_for_post(params[:post_id])

head :no_content
end

def create_for_comment
return unless comment_and_post_validate(params[:post_id], params[:comment_id])

like_service.create_for_comment(params[:comment_id])

head :no_content
end

def destroy_for_post
if like_service.unlike_post(params[:post_id])
head :no_content
else
render_error 410, "Like doesn’t exist"
end
end

def destroy_for_comment
return unless comment_and_post_validate(params[:post_id], params[:comment_id])

if like_service.unlike_comment(params[:comment_id])
head :no_content
else
render_error 410, "Like doesn’t exist"
end
end

def comment_and_post_validate(post_guid, comment_guid)
if !comment_exists(comment_guid) || !comment_is_for_post(post_guid, comment_guid)
render_error 404, "Comment not found for the given post"
false
else
true
end
end

def comment_exists(comment_guid)
comment = comment_service.find!(comment_guid)
comment ? true : false
rescue ActiveRecord::RecordNotFound
false
end

def comment_is_for_post(post_guid, comment_guid)
comments = comment_service.find_for_post(post_guid)
comment = comments.find {|comment| comment[:guid] == comment_guid }
comment ? true : false
end
Copy link
Member

Choose a reason for hiding this comment

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

The comment_exists is kinda redundant, if the comment doesn't exist, it also doesn't exist "for the post", so only checking comment_is_for_post is enough.

Also in comment_is_for_post you can use .exists? directly on an SQL level, instead of looping through all comments with find.

Like this:

Suggested change
def comment_and_post_validate(post_guid, comment_guid)
if !comment_exists(comment_guid) || !comment_is_for_post(post_guid, comment_guid)
render_error 404, "Comment not found for the given post"
false
else
true
end
end
def comment_exists(comment_guid)
comment = comment_service.find!(comment_guid)
comment ? true : false
rescue ActiveRecord::RecordNotFound
false
end
def comment_is_for_post(post_guid, comment_guid)
comments = comment_service.find_for_post(post_guid)
comment = comments.find {|comment| comment[:guid] == comment_guid }
comment ? true : false
end
def comment_and_post_validate(post_guid, comment_guid)
if comment_is_for_post(post_guid, comment_guid)
true
else
render_error 404, "Comment not found for the given post"
false
end
end
def comment_is_for_post(post_guid, comment_guid)
comments = comment_service.find_for_post(post_guid)
comments.exists?(guid: comment_guid)
end

This way the whole validation is just a single sql-query, instead of 2 queries and looping through all comments in ruby.

end
end
end
3 changes: 2 additions & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,8 @@
resources :photos, only: %i[show index create destroy]
resources :posts, only: %i[show create destroy] do
resources :comments, only: %i[create index destroy] do
post "report" => "comments#report"
resource :likes, only: %i[show create destroy]
post :report
end
resource :reshares, only: %i[show create]
resource :likes, only: %i[show create destroy]
Expand Down
2 changes: 1 addition & 1 deletion spec/integration/api/comments_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@
end
end

describe "#read" do
describe "#index" do
before do
@comment_text1 = "This is a comment"
@comment_text2 = "This is a comment 2"
Expand Down