From fca67382121ad722bb6efa273eddd6bc92608e7b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Mon, 12 May 2014 16:30:10 +0200
Subject: [PATCH] BUGFIX: could not see the revisions of a post in a deleted
 topic

---
 app/controllers/posts_controller.rb       |  3 ++-
 lib/guardian.rb                           |  3 ++-
 lib/guardian/post_guardian.rb             | 12 ++++++------
 lib/guardian/topic_guardian.rb            | 22 ++++++++++------------
 spec/controllers/posts_controller_spec.rb | 14 ++++++++++++++
 5 files changed, 34 insertions(+), 20 deletions(-)

diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb
index 68abb60c4e0..221748535cb 100644
--- a/app/controllers/posts_controller.rb
+++ b/app/controllers/posts_controller.rb
@@ -192,7 +192,6 @@ class PostsController < ApplicationController
 
   def revisions
     post_revision = find_post_revision_from_params
-    guardian.ensure_can_see!(post_revision)
     post_revision_serializer = PostRevisionSerializer.new(post_revision, scope: guardian, root: false)
     render_json_dump(post_revision_serializer)
   end
@@ -302,6 +301,8 @@ class PostsController < ApplicationController
     # Include deleted posts if the user is staff
     finder = finder.with_deleted if current_user.try(:staff?)
     post = finder.first
+    # load deleted topic
+    post.topic = Topic.with_deleted.find(post.topic_id) if current_user.try(:staff?)
     guardian.ensure_can_see!(post)
     post
   end
diff --git a/lib/guardian.rb b/lib/guardian.rb
index 150f7a2b291..f19a18d93db 100644
--- a/lib/guardian.rb
+++ b/lib/guardian.rb
@@ -8,7 +8,7 @@ require_dependency 'guardian/user_guardian'
 class Guardian
   include EnsureMagic
   include CategoryGuardian
-  include PostGuardain
+  include PostGuardian
   include TopicGuardian
   include UserGuardian
 
@@ -23,6 +23,7 @@ class Guardian
     def has_trust_level?(level); false; end
     def email; nil; end
   end
+
   def initialize(user=nil)
     @user = user.presence || AnonymousUser.new
   end
diff --git a/lib/guardian/post_guardian.rb b/lib/guardian/post_guardian.rb
index b6d0a0dd588..2149494380f 100644
--- a/lib/guardian/post_guardian.rb
+++ b/lib/guardian/post_guardian.rb
@@ -1,9 +1,8 @@
 #mixin for all guardian methods dealing with post permissions
-module PostGuardain
+module PostGuardian
   # Can the user act on the post in a particular way.
   #  taken_actions = the list of actions the user has already taken
   def post_can_act?(post, action_key, opts={})
-
     taken = opts[:taken_actions].try(:keys).to_a
     is_flag = PostActionType.is_flag?(action_key)
     already_taken_this_action = taken.any? && taken.include?(PostActionType.types[action_key])
@@ -110,16 +109,17 @@ module PostGuardain
   end
 
   def can_see_post_revision?(post_revision)
-    return false if post_revision.nil?
+    return false unless post_revision
     can_view_post_revisions?(post_revision.post)
   end
 
   def can_view_post_revisions?(post)
-    return false if post.nil?
+    return false unless post
     return true if SiteSetting.edit_history_visible_to_public && !post.hidden
+
     authenticated? &&
-      (is_staff? || @user.has_trust_level?(:elder) || @user.id == post.user_id) &&
-      can_see_post?(post)
+    (is_staff? || @user.has_trust_level?(:elder) || @user.id == post.user_id) &&
+    can_see_post?(post)
   end
 
   def can_vote?(post, opts={})
diff --git a/lib/guardian/topic_guardian.rb b/lib/guardian/topic_guardian.rb
index 3e08af064ba..2885f5553c5 100644
--- a/lib/guardian/topic_guardian.rb
+++ b/lib/guardian/topic_guardian.rb
@@ -45,20 +45,18 @@ module TopicGuardian
   end
 
   def can_see_topic?(topic)
-    if topic
-      is_staff? ||
+    return false unless topic
+    return true if is_staff?
+    return false if topic.deleted_at
 
-      topic.deleted_at.nil? &&
+    # NOTE
+    # At the moment staff can see PMs, there is some talk of restricting this, however
+    # we still need to allow staff to join PMs for the case of flagging ones
 
-      # not secure, or I can see it
-      (not(topic.read_restricted_category?) || can_see_category?(topic.category)) &&
+    # not secure, or I can see it
+    (not(topic.read_restricted_category?) || can_see_category?(topic.category)) &&
+    # not private, or I am allowed (or is staff)
+    (not(topic.private_message?) || (authenticated? && (is_staff? || topic.all_allowed_users.where(id: @user.id).exists?)))
 
-      # NOTE
-      # At the moment staff can see PMs, there is some talk of restricting this, however
-      # we still need to allow staff to join PMs for the case of flagging ones
-
-      # not private, or I am allowed (or is staff)
-      (not(topic.private_message?) || authenticated? && (topic.all_allowed_users.where(id: @user.id).exists? || is_staff?))
-    end
   end
 end
diff --git a/spec/controllers/posts_controller_spec.rb b/spec/controllers/posts_controller_spec.rb
index ec3941454b3..18dd8609c42 100644
--- a/spec/controllers/posts_controller_spec.rb
+++ b/spec/controllers/posts_controller_spec.rb
@@ -508,6 +508,20 @@ describe PostsController do
       end
     end
 
+    context "deleted topic" do
+      let(:admin) { log_in(:admin) }
+      let(:deleted_topic) { Fabricate(:topic, user: admin) }
+      let(:post) { Fabricate(:post, user: admin, topic: deleted_topic) }
+      let(:post_revision) { Fabricate(:post_revision, user: admin, post: post) }
+
+      before { deleted_topic.trash!(admin) }
+
+      it "also work on deleted topic" do
+        xhr :get, :revisions, post_id: post_revision.post_id, revision: post_revision.number
+        response.should be_success
+      end
+    end
+
   end
 
   describe 'expandable embedded posts' do