diff --git a/app/controllers/comment_votes_controller.rb b/app/controllers/comment_votes_controller.rb index 9364278bc..6e00f63b6 100644 --- a/app/controllers/comment_votes_controller.rb +++ b/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]) - render :nothing => true - end - -private - def error(exception) - @exception = exception - render :action => "error", :status => 500 + @comment_vote = @comment.vote!(params[:score]) + respond_with(@comment_vote) end end diff --git a/app/models/comment.rb b/app/models/comment.rb index 8f58c5835..b99567311 100644 --- a/app/models/comment.rb +++ b/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? - raise CommentVote::Error.new(vote.errors.full_messages.join("; ")) - elsif vote.is_positive? - increment!(:score) - elsif vote.is_negative? - decrement!(:score) + if vote.errors.empty? + if vote.is_positive? + increment!(:score) + elsif vote.is_negative? + decrement!(:score) + end end + + return vote end def creator_name diff --git a/app/models/comment_vote.rb b/app/models/comment_vote.rb index 661bed6f2..ba2001998 100644 --- a/app/models/comment_vote.rb +++ b/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" diff --git a/app/models/user.rb b/app/models/user.rb index 61d06eabe..d49b9f22b 100644 --- a/app/models/user.rb +++ b/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 - puts password.inspect - self.password_hash = self.class.sha1(password) if password.present? + def encrypt_password_on_create + self.password_hash = User.sha1(password) + 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 diff --git a/app/views/comment_votes/create.js.erb b/app/views/comment_votes/create.js.erb new file mode 100644 index 000000000..729743de0 --- /dev/null +++ b/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 %> diff --git a/app/views/comment_votes/error.js.erb b/app/views/comment_votes/error.js.erb deleted file mode 100644 index 3cbad8c29..000000000 --- a/app/views/comment_votes/error.js.erb +++ /dev/null @@ -1 +0,0 @@ -Danbooru.Utility.j_alert(<%= @exception.to_s.to_json); \ No newline at end of file diff --git a/app/views/users/edit.html.erb b/app/views/users/edit.html.erb index 8be267b71..04d26ecee 100644 --- a/app/views/users/edit.html.erb +++ b/app/views/users/edit.html.erb @@ -19,8 +19,8 @@

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.

<%= 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 :old_password, :title => "Your old password (you must enter your password if updating your name or password)", :as => :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, :input_html => {:autocomplete => "off"} %> <%= f.button :submit %> diff --git a/test/functional/comment_votes_controller_test.rb b/test/functional/comment_votes_controller_test.rb index ea98afbe9..94ec342af 100644 --- a/test/functional/comment_votes_controller_test.rb +++ b/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_response :error + assert_difference("CommentVote.count", 0) do + post :create, {:format => "js", :comment_id => @comment.id, :score => 1}, {:user_id => @user.id} + assert_response :success + end end end end diff --git a/test/unit/comment_test.rb b/test/unit/comment_test.rb index c45272998..52de77aa9 100644 --- a/test/unit/comment_test.rb +++ b/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)} - assert_raise(CommentVote::Error) {c1.vote!(true)} + comment_vote = 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 diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index b3a767a33..6ba19f71e 100644 --- a/test/unit/user_test.rb +++ b/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)