Reputation: 3500
I want to create Thumbs Up vote system but I don't know how to do it in best way.
Here's my Entries#vote (controller/action):
def vote
if Entry.where(id: params[:entry_id]).first && Vote.where(entry_id: params[:entry_id], user_id: @current_user.id).first.nil? # Check if entry with given entry_id exist && Check if user vote previously.
Vote.create(entry_id: params[:entry_id], user_id: @current_user.id) # Create vote
ActiveRecord::Base.connection.execute("UPDATE `entries` SET `entries`.`points` = `entries`.`points` + 1 WHERE `entries`.`id` = #{params[:entry_id].to_i}") # Update entry points count.
end
render nothing: true
end
I think it's not optimal way to do it, because this action have lots of query. Here's queries logs:
Started GET "/vote/2" for 127.0.0.1 at 2012-02-20 16:20:01 +0100
Processing by EntriesController#vote as JS
Parameters: {"entry_id"=>"2"}
←[1m←[36mUser Load (1.0ms)←[0m ←[1mSELECT id, name FROM `users` WHERE `users`.`auth_token` = '6f1aa3b944d530a1d52c6f40bcb69398' LIMIT 1←[0m
←[1m←[35mEntry Load (1.0ms)←[0m SELECT `entries`.* FROM `entries` WHERE `entries`.`id` = 2 LIMIT 1
←[1m←[36mVote Load (0.0ms)←[0m ←[1mSELECT `votes`.* FROM `votes` WHERE `votes`.`entry_id` = 2 AND `votes`.`user_id` = 1 LIMIT 1←[0m
←[1m←[35m (0.0ms)←[0m BEGIN
←[1m←[36mSQL (0.0ms)←[0m ←[1mINSERT INTO `votes` (`entry_id`, `user_id`) VALUES (2, 1)←[0m
←[1m←[35m (54.0ms)←[0m COMMIT
←[1m←[36m (16.0ms)←[0m ←[1mUPDATE `entries` SET `entries`.`points` = `entries`.`points` + 1 WHERE `entries`.`id` = 2←[0m
Rendered text template (0.0ms)
Completed 200 OK in 115ms (Views: 1.0ms | ActiveRecord: 78.0ms)
Anybody have an idea how to do this in best way?
Upvotes: 0
Views: 572
Reputation: 6084
That controller logic can be cleaned up. If it's strictly just thumbs up where 1 vote = 1 point, you could use counter_cache to track points. If you also need thumbs down, then you could use update_counters instead.
Entry.rb
class Entry < ActiveRecord::Base
has_many :votes
end
Vote.rb
class Vote < ActiveRecord::Base
belongs_to :entry, :counter_cache => :points
belongs_to :user
end
User.rb
class User < ActiveRecord::Base
has_many :votes
end
EntriesController#vote
def vote
Vote.find_or_create_by_entry_id_and_user_id(params[:entry_id], @current_user.id)
render :nothing
end
For new vote SQL log is:
Vote Load (0.3ms) SELECT "votes".* FROM "votes" WHERE "votes"."entry_id" = 3 AND "votes"."user_id" = 1 LIMIT 1
(0.1ms) BEGIN
SQL (0.4ms) INSERT INTO "votes" ("created_at", "entry_id", "updated_at", "user_id") VALUES ($1, $2, $3, $4) RETURNING "id" [["created_at", Tue, 21 Feb 2012 16:51:54 UTC +00:00], ["entry_id", 3], ["updated_at", Tue, 21 Feb 2012 16:51:54 UTC +00:00], ["user_id", 1]]
Entry Load (0.2ms) SELECT "entries".* FROM "entries" WHERE "entries"."id" = 3 LIMIT 1
SQL (0.2ms) UPDATE "entries" SET "points" = COALESCE("points", 0) + 1 WHERE "entries"."id" = 3
(2.3ms) COMMIT
For existing vote:
Vote Load (0.4ms) SELECT "votes".* FROM "votes" WHERE "votes"."entry_id" = 3 AND "votes"."user_id" = 1 LIMIT 1
=> #<Vote id: 7, user_id: 1, entry_id: 3, created_at: "2012-02-21 16:51:54", updated_at: "2012-02-21 16:51:54">
Upvotes: 2
Reputation: 12245
Not a direct answer, but you can use, or look for inspiration, other gems, such as vote_fu, or thumbs up (the latter started as a fork of the first one). If you don't end up using one of them, chances are you'll learn something from their code.
Upvotes: 0
Reputation: 5320
You can use a query to check both an Entry exists and if user hasn't vote on that enrty Something like this :
SELECT enrty.* FROM enrty LEFT JOIN votes ON enrty.voteId=votes.Id AND votes.UserId=userId And enrty.Id=enrtyId WHERE vote.id IS NULL
If this query returns an enrty it means that the entry exists and the user have not voted for that enrty yet.
Upvotes: 0
Reputation: 16677
looks like your constructors all do their own queries..
you could maybe optimize a bit by doing all the checking and update logic in a database procedure that you call once from the UI... this at least would save you network round trips.
otherwise - look at different ways of construction that maybe are smarter about not loading up the smallest amount of each bit of info...
Upvotes: 0