# Article voting schema

System I'm building has Users and Articles. Users can upvote/downvote Articles and cancel their votes if they want to. Each vote changes article's score. Score equals to sum of all vote values together.

So I created three models: User, Article, ArticleVote. Even though the most natural way for me would be to be able to do user.upvote(article), for separation of concerns reasons I've put it in ArticleVote model:

class ArticleVote < ActiveRecord::Base
belongs_to :article
belongs_to :user

before_destroy :before_destroy

validates :value, inclusion: {in: [1, -1]}

def self.upvote(user, article)
cast_vote(user, article, 1)
end

def self.downvote(user, article)
cast_vote(user, article, -1)
end

private

def self.cast_vote(user, article, value)
vote = ArticleVote.where(user_id: user.id, article_id: article.id).first_or_initialize
vote.value = value
vote.save!
article.score += value
article.save!
end

def before_destroy
article.score -= value
article.save
end
end


So now I call upvote via ArticleVote.upvote(user, article) which doesn't look too bad, but because of Rails' behavior, this doesn't work very good.

If I want to cancel an upvote for example:

article = Article.find(528953)
# article.score = 5 at this point (for example)
vote = ArticleVote.where(user: current_user, article: article).first.destroy
# article.score = 5 !!!!
article.reload
# article.score = 4


So even though I already fetched an Article from database, and used instance of it to find ArticleVote I have to fetch it again to get the actual score value. I would expect ActiveRecord to automatically set vote.article to reference my article, so when I modify it's state, I wouldn't have to fetch it once again.

Is there a way I can avoid this unnecessary article.reload call and still have valid article.score value?

-
Could you just tell which rails version are you using? –  BroiSatse Mar 30 at 23:36
Rails 4, not sure about exact version number, typing from phone –  Igor Pantović Mar 30 at 23:38

## 2 Answers

I would start with creating two subclasses of your votes, using rails STI (you need to add one string 'type' column to your model - You can get rid of value though) and use rails counter_cache feature for both subclasses.

class ArticleVote < ActiveRecord::Base

def initialize(*args)
raise 'This class cannot be initialized' if self.class == ArticleVote
super
end

validate :user, presence: true

end

class UpVote < ArticleVote
belong_to :article, counter_cache: true
end

class DownVote < ArticleVote
belongs_to: article, counter_cache: true
end


With this in place, I would create three associations on your Article model, and define score method:

class Article < ActiveRecord::Base
has_many :votes, class_name: 'ArticleVotes'
has_many :up_votes
has_many: down_votes

def score
(up_votes_count || 0) - (down_votes_count || 0)
end
end


You can create new votes using article.up_votes.create(user_id: user.id) (similar for down_votes).

Counter cache will keep track of a number of down and up votes (including destroying existing votes) and this is extremely cheap to subtract those values. Note that you need two extra columns in you article model (up_votes_count and down_votes_count, no score column needed).

Now the only thing left is to keep track of user changing their votes. To do this, I would add before_create filter on Vote model:

class ArticleVote < ActiveRecord::Base
belongs_to :article

def initialize(*args)
raise 'This class cannot be initialized' if self.class == ArticleVote
super
end

validate :user, presence: true

before_create(prepend: true) do
existing_record = article.article_votes.find_by(user_id: user_id)
return false if existing_record && existing_record.class == self.class
existing_record && existing_record.destroy
true
end
end


This filter will check whether given user already voted on given article. If he did and he is trying to vote again, do nothing (return false) otherwise remove previous vote and create new one.

There is a potential problem here with condition race. The only way to ensure record uniqueness is to create database constraint like unique index.

Note that, while testing it in the console, counters columns behaves as normal column - it is stored in memory and will not checked whether the value changed in a database or not. Hence you need to reload the model to see the changes:

article.score #=> 0
article.up_vote.create(user_id: User.first.id)
article.score #=> 0         # Counters not updated
article.reload.score #=> 1  # But column in a database has been updated

-
I went with this in the end and it worksgreat :) –  Igor Pantović Apr 21 at 19:12

Rails just isn't taking chances, so when you call vote.article it's performing a new DB query, and thus creating a new instance rather than grabbing a cache. And that's a good thing! You might run into trouble otherwise, since you call save. If it was the same Article instance everywhere, you'd suddenly be calling save on it from a non-obvious point in the code.

I.e. pretend that Rails does reuse the same Article instance, and that instance has other unsaved changes. Perhaps these changes aren't valid, and in that case, the save call in ArticleVote would fail although you're only trying to change the article's score. It would also update the article's updated_at timestamp, although that may not be relevant; an article receiving or losing a vote doesn't really mean that the article itself was updated.

So, for one I'd recommend using article.update_column(:score, article.score - value) in you before_destroy method. And, yes, you'll still need to call reload elsewhere.

However, the underlying issue is that you're modifying an article "from the outside". I'd recommend a different approach altogether, with no articles.score column in the database, and using Rails' caching API instead:

class Article
has_many :votes, class_name: "ArticleVotes"

# score is not part of the DB schema;
# it's computed and cached as necessary
def score
# get or update the cached score
Rails.cache.fetch(score_cache_key) { votes.sum(:value) }
end

def flush_score_cache
Rails.cache.delete(score_cache_key)
end

# let's not use or override the built-in
# #cache_key method for this, since that
# relies on the updated_at timestamp.
def score_cache_key
"article/#{id}/score" # or something similar with the ID in it
end
end


(In the above, you may want to handle the possibility of the record being new, and thus having a nil id.)

And in ArticleVote:

class ArticleVote
after_commit :flush_article_score

private

def flush_article_score
article.flush_score_cache
end
end


Now whenever a vote is added, removed or updated, the article's score cache will be busted, forcing a article.score to do a proper recount if/when it's called.

This also has the advantage that the article's score remains consistent with the actual votes: It's computed entirely from what's in the database. Alternatively, you can simply increment/decrement the cached value instead of flushing it, and still avoid having a database column.

Of course, there are some drawbacks too. Since it is a cache the usual disclaimers apply (you have to be diligent about busting it when necessary, or you'll hit a stale cache, etc..). And if you've got heavy voting traffic you might end up busting the cache so often it just always does the SUM() query. In that case, you could still go with your current solution of having a database column that you increment and decrement, but I might still want to stick that in a different table to avoid the aforementioned issues with implicitly calling save on an article (or, like now, overwriting an updated score with a older one if you forget to reload). You could also consider a more "raw SQL" approach of actually incrementing or decrementing rather than writing the value directly, i.e. UPDATE ... SET score=score + 1 WHERE .... That would avoid race conditions. Optimally, you might do a bit of both: A database column holding the canonical score of an article on disk, and a memory cache you use most of the time for reads, so you only have to hit the database for writes.

-