From 4fb274fb9d8d8ecc4fc2db56a90d57263b3fef4e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Tue, 4 Feb 2014 20:05:50 +0100
Subject: [PATCH] BUGFIX: history link doesn't work on deleted posts

---
 app/controllers/posts_controller.rb          |  2 +
 lib/guardian.rb                              |  2 +-
 lib/guardian/post_guardian.rb                |  4 +-
 spec/controllers/posts_controller_spec.rb    | 61 +++++++++++++++++++-
 spec/fabricators/post_revision_fabricator.rb |  8 +++
 spec/models/post_revision_spec.rb            |  9 +++
 spec/models/topic_revision_spec.rb           |  9 +++
 7 files changed, 92 insertions(+), 3 deletions(-)
 create mode 100644 spec/fabricators/post_revision_fabricator.rb
 create mode 100644 spec/models/post_revision_spec.rb
 create mode 100644 spec/models/topic_revision_spec.rb

diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb
index 58039f27aa9..69172b1c514 100644
--- a/app/controllers/posts_controller.rb
+++ b/app/controllers/posts_controller.rb
@@ -216,6 +216,8 @@ class PostsController < ApplicationController
     raise Discourse::InvalidParameters.new(:revision) if revision < 2
 
     post_revision = PostRevision.where(post_id: post_id, number: revision).first
+    post_revision.post = find_post_from_params
+
     guardian.ensure_can_see!(post_revision)
     post_revision
   end
diff --git a/lib/guardian.rb b/lib/guardian.rb
index a22340fc93c..d6709b40cf0 100644
--- a/lib/guardian.rb
+++ b/lib/guardian.rb
@@ -8,7 +8,7 @@ class Guardian
   include CategoryGuardian
   include PostGuardain
   include TopicGuardian
-    
+
   class AnonymousUser
     def blank?; true; end
     def admin?; false; end
diff --git a/lib/guardian/post_guardian.rb b/lib/guardian/post_guardian.rb
index 9c9f660209b..7c1d041627d 100644
--- a/lib/guardian/post_guardian.rb
+++ b/lib/guardian/post_guardian.rb
@@ -104,7 +104,9 @@ module PostGuardain
   end
 
   def can_see_post_revision?(post_revision)
-    post_revision.present? && (is_staff? || can_see_post?(post_revision.post))
+    return false if post_revision.nil?
+    return true if SiteSetting.edit_history_visible_to_public
+    authenticated? && (is_staff? || can_see_post?(post_revision.post))
   end
 
   def can_vote?(post, opts={})
diff --git a/spec/controllers/posts_controller_spec.rb b/spec/controllers/posts_controller_spec.rb
index 745f51b32ad..1780423346c 100644
--- a/spec/controllers/posts_controller_spec.rb
+++ b/spec/controllers/posts_controller_spec.rb
@@ -2,7 +2,6 @@ require 'spec_helper'
 
 describe PostsController do
 
-
   describe 'short_link' do
     it 'logs the incoming link once' do
       IncomingLink.expects(:add).once.returns(true)
@@ -386,4 +385,64 @@ describe PostsController do
     end
   end
 
+  describe "revisions" do
+
+    let(:post_revision) { Fabricate(:post_revision) }
+
+    it "throws an exception when revision is < 2" do
+      expect {
+        xhr :get, :revisions, post_id: post_revision.post_id, revision: 1
+      }.to raise_error(Discourse::InvalidParameters)
+    end
+
+    context "when edit history is not visible to the public" do
+
+      before { SiteSetting.stubs(:edit_history_visible_to_public).returns(false) }
+
+      it "ensures anonymous can not see the revisions" do
+        xhr :get, :revisions, post_id: post_revision.post_id, revision: post_revision.number
+        response.should be_forbidden
+      end
+
+      it "ensures staff can see the revisions" do
+        log_in(:admin)
+        xhr :get, :revisions, post_id: post_revision.post_id, revision: post_revision.number
+        response.should be_success
+      end
+
+      it "ensures poster can see the revisions" do
+        user = log_in(:active_user)
+        pr = Fabricate(:post_revision, user: user)
+        xhr :get, :revisions, post_id: pr.post_id, revision: pr.number
+        response.should be_success
+      end
+
+    end
+
+    context "when edit history is visible to everyone" do
+
+      before { SiteSetting.stubs(:edit_history_visible_to_public).returns(true) }
+
+      it "ensures anyone can see the revisions" do
+        xhr :get, :revisions, post_id: post_revision.post_id, revision: post_revision.number
+        response.should be_success
+      end
+
+    end
+
+    context "deleted post" do
+      let(:admin) { log_in(:admin) }
+      let(:deleted_post) { Fabricate(:post, user: admin) }
+      let(:deleted_post_revision) { Fabricate(:post_revision, user: admin, post: deleted_post) }
+
+      before { deleted_post.trash!(admin) }
+
+      it "also work on deleted post" do
+        xhr :get, :revisions, post_id: deleted_post_revision.post_id, revision: deleted_post_revision.number
+        response.should be_success
+      end
+    end
+
+  end
+
 end
diff --git a/spec/fabricators/post_revision_fabricator.rb b/spec/fabricators/post_revision_fabricator.rb
new file mode 100644
index 00000000000..5a977b23829
--- /dev/null
+++ b/spec/fabricators/post_revision_fabricator.rb
@@ -0,0 +1,8 @@
+Fabricator(:post_revision) do
+  post
+  user
+  number 3
+  modifications do
+    { "cooked" => ["<p>BEFORE</p>", "<p>AFTER</p>"], "raw" => ["BEFORE", "AFTER"] }
+  end
+end
diff --git a/spec/models/post_revision_spec.rb b/spec/models/post_revision_spec.rb
new file mode 100644
index 00000000000..25c8e47bcee
--- /dev/null
+++ b/spec/models/post_revision_spec.rb
@@ -0,0 +1,9 @@
+require 'spec_helper'
+require_dependency 'post_revision'
+
+describe PostRevision do
+
+  it { should belong_to :user }
+  it { should belong_to :post }
+
+end
diff --git a/spec/models/topic_revision_spec.rb b/spec/models/topic_revision_spec.rb
new file mode 100644
index 00000000000..f1717ea9ee2
--- /dev/null
+++ b/spec/models/topic_revision_spec.rb
@@ -0,0 +1,9 @@
+require 'spec_helper'
+require_dependency 'topic_revision'
+
+describe TopicRevision do
+
+  it { should belong_to :user }
+  it { should belong_to :topic }
+
+end