Why doesn't this Ruby on Rails code work as I intend it to?
So I attempted to build what I asked about in this question: Fix voting mechanism
However, this solution doesn't work. A user can still vote however many times he or she wants. How could I fix this and/or refactor?
def create
@video = Video.find(params[:video_id])
@vote = @video.video_votes.new
@vote.user = current_user
if params[:type] == "up"
@vote.value = 1
else
@vote.value = -1
end
if @previous_vote.nil?
if @vote.save
respond_to do |format|
format.html { redirect_to @video }
format.js
end
else
respond_to do |format|
format.html { redirect_to @video }
format.js {render 'fail_create.js.erb'}
end
end
elsif @previous_vote.value == params[:type]
@previous_vote.destroy
else
@previous_vote.destroy
if @vote.save
respond_to do |format|
format.html { redirect_to @video }
format.js
end
else
respond_to do |format|
format.html { redirect_to @video }
format.js {render 'fail_create.js.erb'}
end
end
end
@previous_vote = VideoVote.where(:video_id => params[:video_开发者_开发问答id], :user_id => current_user.id).first
end
@previous_vote
seems to be nil at the beginning of every request?
I would personally do away with all the logic in the controller and put uniqueness contraints at Model or database level where they belong.
Updated
This is probably full of errors but treat it as pseudo-code
Models something like:
class Video < ActiveRecord::Base
has_many :votes
end
class Vote < ActiveRecord::Base
belongs_to :user
belongs_to :video
validates_uniqueness_of :user_id, :scope => :video_id # only one vote per person per video
end
class User < ActiveRecord::Base
has_many :votes
end
Controller:
def create
@video = Video.find(params[:video_id])
@vote = current_user.votes.find_or_create_by_video_id(@video.id)
# change this next block of code so you assign value to the vote based on whatever logic you need
if you_need_to_do_anything_to_change_its_value
@vote.value = :whatever
end
if @vote.save
redirect_to @video
else
render :whatever_is_appropriate
end
end
精彩评论