FIX: use PostDestroyer when deleting/recovering a topic

This commit is contained in:
Régis Hanol 2014-08-07 19:12:35 +02:00
parent ee40a95e58
commit 3ae1ebdfc3
8 changed files with 43 additions and 27 deletions

View File

@ -80,8 +80,8 @@ class PostActionsController < ApplicationController
finder = Post.where(id: post_id) finder = Post.where(id: post_id)
# Include deleted posts if the user is a moderator (to guardian ?) # Include deleted posts if the user is a staff
finder = finder.with_deleted if current_user.try(:moderator?) finder = finder.with_deleted if guardian.is_staff?
@post = finder.first @post = finder.first
guardian.ensure_can_see!(@post) guardian.ensure_can_see!(@post)

View File

@ -31,7 +31,6 @@ class TopicsController < ApplicationController
skip_before_filter :check_xhr, only: [:show, :feed] skip_before_filter :check_xhr, only: [:show, :feed]
def show def show
flash["referer"] ||= request.referer flash["referer"] ||= request.referer
# We'd like to migrate the wordpress feed to another url. This keeps up backwards compatibility with # We'd like to migrate the wordpress feed to another url. This keeps up backwards compatibility with
@ -197,14 +196,20 @@ class TopicsController < ApplicationController
def destroy def destroy
topic = Topic.find_by(id: params[:id]) topic = Topic.find_by(id: params[:id])
guardian.ensure_can_delete!(topic) guardian.ensure_can_delete!(topic)
topic.trash!(current_user)
first_post = topic.ordered_posts.first
PostDestroyer.new(current_user, first_post).destroy
render nothing: true render nothing: true
end end
def recover def recover
topic = Topic.where(id: params[:topic_id]).with_deleted.first topic = Topic.where(id: params[:topic_id]).with_deleted.first
guardian.ensure_can_recover_topic!(topic) guardian.ensure_can_recover_topic!(topic)
topic.recover!
first_post = topic.posts.with_deleted.order(:post_number).first
PostDestroyer.new(current_user, first_post).recover
render nothing: true render nothing: true
end end

View File

@ -15,9 +15,12 @@ module PostGuardian
# not a flagging action, and haven't done it already # not a flagging action, and haven't done it already
not(is_flag || already_taken_this_action) && not(is_flag || already_taken_this_action) &&
# nothing except flagging on archived posts # nothing except flagging on archived topics
not(post.topic.archived?) && not(post.topic.archived?) &&
# nothing except flagging on deleted posts
not(post.trashed?) &&
# don't like your own stuff # don't like your own stuff
not(action_key == :like && is_my_own?(post)) && not(action_key == :like && is_my_own?(post)) &&
@ -35,6 +38,7 @@ module PostGuardian
# Can we see who acted on a post in a particular way? # Can we see who acted on a post in a particular way?
def can_see_post_actors?(topic, post_action_type_id) def can_see_post_actors?(topic, post_action_type_id)
return true if is_admin?
return false unless topic return false unless topic
type_symbol = PostActionType.types[post_action_type_id] type_symbol = PostActionType.types[post_action_type_id]
@ -49,10 +53,6 @@ module PostGuardian
true true
end end
def can_see_deleted_posts?
is_staff?
end
def can_delete_all_posts?(user) def can_delete_all_posts?(user)
is_staff? && is_staff? &&
user && user &&

View File

@ -26,7 +26,8 @@ class PostDestroyer
end end
def initialize(user, post) def initialize(user, post)
@user, @post = user, post @user = user
@post = post
end end
def destroy def destroy
@ -38,11 +39,15 @@ class PostDestroyer
end end
def recover def recover
puts "@" * 100
puts "staff? #{@user.staff?}"
puts "post.deleted_at #{@post.deleted_at}"
if @user.staff? && @post.deleted_at if @user.staff? && @post.deleted_at
staff_recovered staff_recovered
elsif @user.staff? || @user.id == @post.user_id elsif @user.staff? || @user.id == @post.user_id
user_recovered user_recovered
end end
@post.topic.recover! if @post.post_number == 1
@post.topic.update_statistics @post.topic.update_statistics
end end
@ -68,7 +73,7 @@ class PostDestroyer
@post.update_flagged_posts_count @post.update_flagged_posts_count
remove_associated_replies remove_associated_replies
remove_associated_notifications remove_associated_notifications
@post.topic.trash!(@user) if @post.topic and @post.post_number == 1 @post.topic.trash!(@user) if @post.topic && @post.post_number == 1
update_associated_category_latest_topic update_associated_category_latest_topic
end end
publish("deleted") publish("deleted")

View File

@ -190,6 +190,7 @@ class TopicView
def has_deleted? def has_deleted?
@predelete_filtered_posts.with_deleted @predelete_filtered_posts.with_deleted
.where("posts.deleted_at IS NOT NULL") .where("posts.deleted_at IS NOT NULL")
.where("posts.post_number > 1")
.exists? .exists?
end end
@ -352,7 +353,7 @@ class TopicView
# copy the filter for has_deleted? method # copy the filter for has_deleted? method
@predelete_filtered_posts = @filtered_posts.spawn @predelete_filtered_posts = @filtered_posts.spawn
if @guardian.can_see_deleted_posts? && !@show_deleted && has_deleted? if @guardian.can_see_deleted_posts? && !@show_deleted && has_deleted?
@filtered_posts = @filtered_posts.where(deleted_at: nil) @filtered_posts = @filtered_posts.where("deleted_at IS NULL OR post_number = 1")
@contains_gaps = true @contains_gaps = true
end end

View File

@ -40,6 +40,11 @@ describe Guardian do
Guardian.new(user).post_can_act?(post, :like).should be_false Guardian.new(user).post_can_act?(post, :like).should be_false
end end
it "returns false when the post is deleted" do
post.deleted_at = Time.now
Guardian.new(user).post_can_act?(post, :like).should be_false
end
it "always allows flagging" do it "always allows flagging" do
post.topic.archived = true post.topic.archived = true
Guardian.new(user).post_can_act?(post, :spam).should be_true Guardian.new(user).post_can_act?(post, :spam).should be_true

View File

@ -36,6 +36,12 @@ shared_examples 'finding and showing post' do
xhr :get, action, params xhr :get, action, params
response.should be_success response.should be_success
end end
it "can find posts as a admin" do
log_in(:admin)
xhr :get, action, params
response.should be_success
end
end end
end end

View File

@ -501,7 +501,7 @@ describe TopicsController do
end end
it 'succeeds' do it 'succeeds' do
Topic.any_instance.expects(:recover!) PostDestroyer.any_instance.expects(:recover)
xhr :put, :recover, topic_id: topic.id xhr :put, :recover, topic_id: topic.id
response.should be_success response.should be_success
end end
@ -516,33 +516,27 @@ describe TopicsController do
end end
describe 'when logged in' do describe 'when logged in' do
before do let(:topic) { Fabricate(:topic, user: log_in) }
@topic = Fabricate(:topic, user: log_in)
end
describe 'without access' do describe 'without access' do
it "raises an exception when the user doesn't have permission to delete the topic" do it "raises an exception when the user doesn't have permission to delete the topic" do
Guardian.any_instance.expects(:can_delete?).with(@topic).returns(false) Guardian.any_instance.expects(:can_delete?).with(topic).returns(false)
xhr :delete, :destroy, id: @topic.id xhr :delete, :destroy, id: topic.id
response.should be_forbidden response.should be_forbidden
end end
end end
describe 'with permission' do describe 'with permission' do
before do before do
Guardian.any_instance.expects(:can_delete?).with(@topic).returns(true) Guardian.any_instance.expects(:can_delete?).with(topic).returns(true)
end end
it 'succeeds' do it 'succeeds' do
xhr :delete, :destroy, id: @topic.id PostDestroyer.any_instance.expects(:destroy)
xhr :delete, :destroy, id: topic.id
response.should be_success response.should be_success
end end
it 'deletes the topic' do
xhr :delete, :destroy, id: @topic.id
Topic.exists?(id: @topic_id).should be_false
end
end end
end end