fixes #69: Comment voting non functional

app/controllers/comment_votes_controller.rb

@@ -1,15 +1,9 @@
 class CommentVotesController < ApplicationController
-  rescue_from CommentVote::Error, :with => :error
+  respond_to :js
   
   def create
     @comment = Comment.find(params[:comment_id])
-    @comment.vote!(params[:score])
+    @comment_vote = @comment.vote!(params[:score])
-    render :nothing => true
+    respond_with(@comment_vote)
-  end
-  
-private
-  def error(exception)
-    @exception = exception
-    render :action => "error", :status => 500
   end
 end

app/models/comment.rb

@@ -38,15 +38,18 @@ class Comment < ActiveRecord::Base
   end
   
   def vote!(score)
-    vote = votes.create(:score => score)
+    numerical_score = score == "up" ? 1 : -1
+    vote = votes.create(:score => numerical_score)
     
-    if vote.errors.any?
+    if vote.errors.empty?
-      raise CommentVote::Error.new(vote.errors.full_messages.join("; "))
+      if vote.is_positive?
-    elsif vote.is_positive?
+        increment!(:score)
-      increment!(:score)
+      elsif vote.is_negative?
-    elsif vote.is_negative?
+        decrement!(:score)
-      decrement!(:score)
+      end
     end
+
+    return vote
   end
   
   def creator_name

app/models/comment_vote.rb

@@ -5,7 +5,7 @@ class CommentVote < ActiveRecord::Base
   belongs_to :user
   before_validation :initialize_user, :on => :create
   validates_presence_of :user_id, :comment_id, :score
-  validates_uniqueness_of :user_id, :scope => :comment_id
+  validates_uniqueness_of :user_id, :scope => :comment_id, :message => "has already voted for this comment"
   validate :validate_user_can_vote
   validate :validate_comment_can_be_down_voted
   validates_inclusion_of :score, :in => [-1, 1], :message => "must be 1 or -1"

app/models/user.rb

@@ -26,7 +26,8 @@ class User < ActiveRecord::Base
   validate :validate_ip_addr_is_not_banned, :on => :create
   before_validation :convert_blank_email_to_null
   before_validation :normalize_blacklisted_tags
-  before_save :encrypt_password
+  before_create :encrypt_password_on_create
+  before_update :encrypt_password_on_update
   after_save :update_cache
   before_create :promote_to_admin_if_first_user
   has_many :feedback, :class_name => "UserFeedback", :dependent => :destroy
@@ -99,9 +100,21 @@ class User < ActiveRecord::Base
   end
   
   module PasswordMethods
-    def encrypt_password
+    def encrypt_password_on_create
-      puts password.inspect
+      self.password_hash = User.sha1(password)
-      self.password_hash = self.class.sha1(password) if password.present?
+    end
+    
+    def encrypt_password_on_update
+      return if password.blank?
+      return if old_password.blank?
+      
+      if User.sha1(old_password) == password_hash
+        self.password_hash = User.sha1(password)
+        return true
+      else
+        errors[:old_password] << "is incorrect"
+        return false
+      end
     end
 
     def reset_password
@@ -270,7 +283,6 @@ class User < ActiveRecord::Base
       if is_contributor?
         true
       elsif created_at > 1.second.ago
-        # TODO: change this to 1 week
         false
       else
         upload_limit > 0
@@ -298,7 +310,7 @@ class User < ActiveRecord::Base
     def upload_limit
       deleted_count = Post.for_user(id).deleted.count
       pending_count = Post.for_user(id).pending.count
-      approved_count = Post.where("is_flagged = false and is_pending = false and user_id = ?", id).count
+      approved_count = Post.where("is_flagged = false and is_pending = false and uploader_id = ?", id).count
       
       if base_upload_limit
         limit = base_upload_limit - pending_count

app/views/comment_votes/create.js.erb

@@ -0,0 +1,5 @@
+<% if @comment_vote.errors.any? %>
+  Danbooru.j_error("<%= j @comment_vote.errors.full_messages.join('; ') %>");
+<% elsif @comment_vote.is_negative? %>
+  $(".comment[data-comment-id=<%= @comment.id %>]").remove();
+<% end %>

app/views/comment_votes/error.js.erb

@@ -1 +0,0 @@
-Danbooru.Utility.j_alert(<%= @exception.to_s.to_json);

app/views/users/edit.html.erb

@@ -19,8 +19,8 @@
     <p>For security purposes, changing the following settings requires you to re-enter your password. You can leave the new password field blank to keep your current one.</p>
 
     <%= f.input :name %>
-    <%= f.input :password, :title => "What you want your new password to be (leave blank if you don't want to change your password)", :label => "New password" %>
+    <%= f.input :password, :title => "What you want your new password to be (leave blank if you don't want to change your password)", :label => "New password", :input_html => {:autocomplete => "off"} %>
-    <%= f.input :old_password, :title => "Your old password (you must enter your password if updating your name or password)", :as => :password %>
+    <%= f.input :old_password, :title => "Your old password (you must enter your password if updating your name or password)", :as => :password, :input_html => {:autocomplete => "off"} %>
   </fieldset>
   
   <%= f.button :submit %>

test/functional/comment_votes_controller_test.rb

@@ -22,8 +22,10 @@ class CommentVotesControllerTest < ActionController::TestCase
     
     should "fail silently on errors" do
       Factory.create(:comment_vote, :comment => @comment)
-      post :create, {:format => "js", :comment_id => @comment.id, :score => 1}, {:user_id => @user.id}
+      assert_difference("CommentVote.count", 0) do
-      assert_response :error
+        post :create, {:format => "js", :comment_id => @comment.id, :score => 1}, {:user_id => @user.id}
+        assert_response :success
+      end
     end
   end
 end

test/unit/comment_test.rb

@@ -45,12 +45,15 @@ class CommentTest < ActiveSupport::TestCase
       user = Factory.create(:user)
       post = Factory.create(:post)
       c1 = Factory.create(:comment, :post => post)
-      assert_nothing_raised {c1.vote!(true)}
+      comment_vote = c1.vote!(true)
-      assert_raise(CommentVote::Error) {c1.vote!(true)}
+      assert_equal([], comment_vote.errors.full_messages)
+      comment_vote = c1.vote!(true)
+      assert_equal(["User has already voted for this comment"], comment_vote.errors.full_messages)
       assert_equal(1, CommentVote.count)
     
       c2 = Factory.create(:comment, :post => post)
-      assert_nothing_raised {c2.vote!(true)}
+      comment_vote = c2.vote!(true)
+      assert_equal([], comment_vote.errors.full_messages)
       assert_equal(2, CommentVote.count)
     end
     

test/unit/user_test.rb

@@ -206,6 +206,30 @@ class UserTest < ActiveSupport::TestCase
         assert(User.authenticate(@user.name, new_pass), "Authentication should have succeeded")
       end
       
+      should "not change the password if the password and old password are blank" do
+        @user = Factory.create(:user, :password => "67890")
+        @user.update_attributes(:password => "", :old_password => "")
+        assert_equal(User.sha1("67890"), @user.password_hash)
+      end
+      
+      should "not change the password if the old password is incorrect" do
+        @user = Factory.create(:user, :password => "67890")
+        @user.update_attributes(:password => "12345", :old_password => "abcdefg")
+        assert_equal(User.sha1("67890"), @user.password_hash)
+      end
+      
+      should "not change the password if the old password is blank" do
+        @user = Factory.create(:user, :password => "67890")
+        @user.update_attributes(:password => "12345", :old_password => "")
+        assert_equal(User.sha1("67890"), @user.password_hash)
+      end
+      
+      should "change the password if the old password is correct" do
+        @user = Factory.create(:user, :password => "67890")
+        @user.update_attributes(:password => "12345", :old_password => "67890")
+        assert_equal(User.sha1("12345"), @user.password_hash)
+      end
+      
       context "in the json representation" do
         setup do
           @user = Factory.create(:user)