diff --git a/app/controllers/moderator/post/approvals_controller.rb b/app/controllers/moderator/post/approvals_controller.rb index c17f5c5fe..1da5998e2 100644 --- a/app/controllers/moderator/post/approvals_controller.rb +++ b/app/controllers/moderator/post/approvals_controller.rb @@ -3,14 +3,13 @@ module Moderator class ApprovalsController < ApplicationController before_filter :approver_only skip_before_filter :api_check + respond_to :json, :xml, :js def create cookies.permanent[:moderated] = Time.now.to_i - @post = ::Post.find(params[:post_id]) - if @post.is_deleted? || @post.is_flagged? || @post.is_pending? - @post.approve! - end - rescue ::Post::ApprovalError + post = ::Post.find(params[:post_id]) + @approval = post.approve! + respond_with(@approval) end end end diff --git a/app/models/post.rb b/app/models/post.rb index cb1d7493a..f29271d1d 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -46,6 +46,7 @@ class Post < ActiveRecord::Base has_many :notes, :dependent => :destroy has_many :comments, lambda {includes(:creator, :updater).order("comments.id")}, :dependent => :destroy has_many :children, lambda {order("posts.id")}, :class_name => "Post", :foreign_key => "parent_id" + has_many :approvals, :class_name => "PostApproval", :dependent => :destroy has_many :disapprovals, :class_name => "PostDisapproval", :dependent => :destroy has_many :favorites, :dependent => :destroy @@ -290,8 +291,8 @@ class Post < ActiveRecord::Base end module ApprovalMethods - def is_approvable? - !is_status_locked? && (is_pending? || is_flagged? || is_deleted?) && !PostApproval.approved?(CurrentUser.id, id) + def is_approvable?(user = CurrentUser.user) + !is_status_locked? && (is_pending? || is_flagged? || is_deleted?) && !approved_by?(user) end def flag!(reason, options = {}) @@ -320,35 +321,12 @@ class Post < ActiveRecord::Base end end - def approve! - if is_status_locked? - errors.add(:is_status_locked, "; post cannot be approved") - raise ApprovalError.new("Post is locked and cannot be approved") - end + def approve!(approver = CurrentUser.user) + approvals.create(user: approver) + end - if uploader_id == CurrentUser.id - errors.add(:base, "You cannot approve a post you uploaded") - raise ApprovalError.new("You cannot approve a post you uploaded") - end - - if approver_id == CurrentUser.id || PostApproval.approved?(CurrentUser.id, id) - errors.add(:approver, "have already approved this post") - raise ApprovalError.new("You have previously approved this post and cannot approve it again") - end - - flags.each {|x| x.resolve!} - self.is_flagged = false - self.is_pending = false - self.is_deleted = false - self.approver_id = CurrentUser.id - - PostApproval.create(user_id: CurrentUser.id, post_id: id) - - if is_deleted_was == true - ModAction.log("undeleted post ##{id}") - end - - save! + def approved_by?(user) + approver == user || approvals.where(user: user).exists? end def disapproved_by?(user) @@ -1396,7 +1374,7 @@ class Post < ActiveRecord::Base end if !CurrentUser.is_admin? - if approver_id == CurrentUser.id || PostApproval.approved?(CurrentUser.id, id) + if approved_by?(CurrentUser.user) raise ApprovalError.new("You have previously approved this post and cannot undelete it") elsif uploader_id == CurrentUser.id raise ApprovalError.new("You cannot undelete a post you uploaded") diff --git a/app/models/post_approval.rb b/app/models/post_approval.rb index 7fbfe741f..47f476699 100644 --- a/app/models/post_approval.rb +++ b/app/models/post_approval.rb @@ -1,12 +1,36 @@ class PostApproval < ActiveRecord::Base belongs_to :user - belongs_to :post + belongs_to :post, inverse_of: :approvals + + validate :validate_approval + after_create :approve_post def self.prune! where("created_at < ?", 1.month.ago).delete_all end - def self.approved?(user_id, post_id) - where(user_id: user_id, post_id: post_id).exists? + def validate_approval + if post.is_status_locked? + errors.add(:post, "is locked and cannot be approved") + end + + if post.status == "active" + errors.add(:post, "is already active and cannot be approved") + end + + if post.uploader == user + errors.add(:base, "You cannot approve a post you uploaded") + end + + if post.approved_by?(user) + errors.add(:base, "You have previously approved this post and cannot approve it again") + end + end + + def approve_post + ModAction.log("undeleted post ##{id}") if post.is_deleted + + post.flags.each(&:resolve!) + post.update({ approver: user, is_flagged: false, is_pending: false, is_deleted: false }, without_protection: true) end end diff --git a/app/models/user.rb b/app/models/user.rb index c253907c5..8512f9bda 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -77,6 +77,7 @@ class User < ActiveRecord::Base #after_create :notify_sock_puppets has_many :feedback, :class_name => "UserFeedback", :dependent => :destroy has_many :posts, :foreign_key => "uploader_id" + has_many :post_approvals, :dependent => :destroy has_many :post_votes has_many :bans, lambda {order("bans.id desc")} has_one :recent_ban, lambda {order("bans.id desc")}, :class_name => "Ban" diff --git a/app/views/moderator/post/approvals/create.js.erb b/app/views/moderator/post/approvals/create.js.erb index 515a6bcc4..52586b311 100644 --- a/app/views/moderator/post/approvals/create.js.erb +++ b/app/views/moderator/post/approvals/create.js.erb @@ -1,6 +1,6 @@ -<% if @post.errors.any? %> +<% if @approval.errors.any? %> -Danbooru.error("Error: " + <%= @post.errors.full_messages.join("; ").to_json.html_safe %>); +Danbooru.error("Error: " + <%= @approval.errors.full_messages.join("; ").to_json.html_safe %>); <% else %> @@ -10,7 +10,7 @@ $("#c-posts #flag").show(); $("#pending-approval-notice").hide(); -$("#c-moderator-post-queues #post-<%= @post.id %>").hide(); +$("#c-moderator-post-queues #post-<%= @approval.post.id %>").hide(); Danbooru.notice("Post was approved"); Danbooru.ModQueue.increment_processed(); diff --git a/test/models/post_approval_test.rb b/test/models/post_approval_test.rb deleted file mode 100644 index 835324f68..000000000 --- a/test/models/post_approval_test.rb +++ /dev/null @@ -1,63 +0,0 @@ -require 'test_helper' - -class PostApprovalTest < ActiveSupport::TestCase - context "a pending post" do - setup do - @user = FactoryGirl.create(:user) - CurrentUser.user = @user - CurrentUser.ip_addr = "127.0.0.1" - - @post = FactoryGirl.create(:post, uploader_id: @user.id, is_pending: true) - - @approver = FactoryGirl.create(:user) - @approver.can_approve_posts = true - @approver.save - CurrentUser.user = @approver - - CurrentUser.stubs(:can_approve_posts?).returns(true) - end - - teardown do - CurrentUser.user = nil - CurrentUser.ip_addr = nil - end - - should "allow approval" do - assert_equal(false, PostApproval.approved?(@approver.id, @post.id)) - end - - context "That is approved" do - should "create a postapproval record" do - assert_difference("PostApproval.count") do - @post.approve! - end - end - - context "that is then flagged" do - setup do - @user2 = FactoryGirl.create(:user) - @user3 = FactoryGirl.create(:user) - @approver2 = FactoryGirl.create(:user) - @approver2.can_approve_posts = true - @approver2.save - end - - should "prevent the first approver from approving again" do - @post.approve! - CurrentUser.user = @user2 - @post.flag!("blah") - CurrentUser.user = @approver2 - @post.approve! - assert_not_equal(@approver.id, @post.approver_id) - CurrentUser.user = @user3 - @post.flag!("blah blah") - CurrentUser.user = @approver - - assert_raises(Post::ApprovalError) do - @post.approve! - end - end - end - end - end -end diff --git a/test/unit/post_approval_test.rb b/test/unit/post_approval_test.rb new file mode 100644 index 000000000..c718feb37 --- /dev/null +++ b/test/unit/post_approval_test.rb @@ -0,0 +1,60 @@ +require 'test_helper' + +class PostApprovalTest < ActiveSupport::TestCase + context "a pending post" do + setup do + @user = FactoryGirl.create(:user) + CurrentUser.user = @user + CurrentUser.ip_addr = "127.0.0.1" + + @post = FactoryGirl.create(:post, uploader_id: @user.id, is_pending: true) + + @approver = FactoryGirl.create(:user) + @approver.can_approve_posts = true + @approver.save + CurrentUser.user = @approver + + CurrentUser.stubs(:can_approve_posts?).returns(true) + end + + teardown do + CurrentUser.user = nil + CurrentUser.ip_addr = nil + end + + should "allow approval" do + assert_equal(false, @post.approved_by?(@approver)) + end + + context "That is approved" do + should "create a postapproval record" do + assert_difference("PostApproval.count") do + @post.approve! + end + end + + context "that is then flagged" do + setup do + @user2 = FactoryGirl.create(:user) + @user3 = FactoryGirl.create(:user) + @approver2 = FactoryGirl.create(:user) + @approver2.can_approve_posts = true + @approver2.save + end + + should "prevent the first approver from approving again" do + @post.approve!(@approver) + CurrentUser.user = @user2 + @post.flag!("blah") + @post.approve!(@approver2) + assert_not_equal(@approver.id, @post.approver_id) + CurrentUser.user = @user3 + @post.flag!("blah blah") + + approval = @post.approve!(@approver) + assert_includes(approval.errors.full_messages, "You have previously approved this post and cannot approve it again") + end + end + end + end +end diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index 151afe9a2..1f702cce0 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -412,13 +412,10 @@ class PostTest < ActiveSupport::TestCase end should "not allow person X to approve that post" do - assert_raises(Post::ApprovalError) do - CurrentUser.scoped(@post.uploader, "127.0.0.1") do - @post.approve! - end - end + approval = @post.approve!(@post.uploader) - assert_equal(["You cannot approve a post you uploaded"], @post.errors.full_messages) + assert(@post.invalid?) + assert_includes(approval.errors.full_messages, "You cannot approve a post you uploaded") end end @@ -431,19 +428,13 @@ class PostTest < ActiveSupport::TestCase end should "not allow person X to reapprove that post" do - CurrentUser.scoped(@user, "127.0.0.1") do - assert_raises(Post::ApprovalError) do - @post.approve! - end - end + approval = @post.approve!(@user) + assert_includes(approval.errors.full_messages, "You have previously approved this post and cannot approve it again") end should "allow person Y to approve the post" do - CurrentUser.scoped(@user2, "127.0.0.1") do - assert_nothing_raised do - @post.approve! - end - end + @post.approve!(@user2) + assert(@post.valid?) end end @@ -479,9 +470,8 @@ class PostTest < ActiveSupport::TestCase end should "not allow approval" do - assert_raises(Post::ApprovalError) do - @post.approve! - end + approval = @post.approve! + assert_includes(approval.errors.full_messages, "Post is locked and cannot be approved") end end end