From 19d5362c6bfdc0e8f0d5cd6ebe1c20a18361ea68 Mon Sep 17 00:00:00 2001
From: David McClure <dave@xerotrope.org>
Date: Mon, 13 Oct 2014 01:18:49 -0700
Subject: [PATCH] FEATURE: ability to hide or show specific post revisions

---
 .../discourse/controllers/history.js.es6      | 20 ++++
 .../javascripts/discourse/models/_post.js     |  8 ++
 .../discourse/templates/modal/history.hbs     |  6 ++
 app/controllers/posts_controller.rb           | 14 +++
 app/models/post_revision.rb                   | 92 ++++++++++++++-----
 app/serializers/post_revision_serializer.rb   |  9 +-
 config/locales/client.en.yml                  |  2 +
 config/routes.rb                              |  2 +
 ...41014032859_add_hidden_to_post_revision.rb |  5 +
 lib/guardian/post_guardian.rb                 |  8 ++
 spec/models/post_revision_spec.rb             | 68 +++++++++++---
 11 files changed, 195 insertions(+), 39 deletions(-)
 create mode 100644 db/migrate/20141014032859_add_hidden_to_post_revision.rb

diff --git a/app/assets/javascripts/discourse/controllers/history.js.es6 b/app/assets/javascripts/discourse/controllers/history.js.es6
index eb35021486e..7b8cb412db5 100644
--- a/app/assets/javascripts/discourse/controllers/history.js.es6
+++ b/app/assets/javascripts/discourse/controllers/history.js.es6
@@ -25,6 +25,20 @@ export default ObjectController.extend(ModalFunctionality, {
     });
   },
 
+  hide: function(postId, postVersion) {
+    var self = this;
+    Discourse.Post.hideRevision(postId, postVersion).then(function (result) {
+      self.refresh(postId, postVersion);
+    });
+  },
+
+  show: function(postId, postVersion) {
+    var self = this;
+    Discourse.Post.showRevision(postId, postVersion).then(function (result) {
+      self.refresh(postId, postVersion);
+    });
+  },
+
   createdAtDate: function() { return moment(this.get("created_at")).format("LLLL"); }.property("created_at"),
 
   previousVersion: function() { return this.get("version") - 1; }.property("version"),
@@ -35,6 +49,9 @@ export default ObjectController.extend(ModalFunctionality, {
   displayGoToNext: function() { return this.get("version") < this.get("revisions_count"); }.property("version", "revisions_count"),
   displayGoToLast: function() { return this.get("version") < this.get("revisions_count") - 1; }.property("version", "revisions_count"),
 
+  displayShow: function() { return this.get("hidden") && Discourse.User.currentProp('staff'); }.property("hidden"),
+  displayHide: function() { return !this.get("hidden") && Discourse.User.currentProp('staff'); }.property("hidden"),
+
   displayingInline: Em.computed.equal("viewMode", "inline"),
   displayingSideBySide: Em.computed.equal("viewMode", "side_by_side"),
   displayingSideBySideMarkdown: Em.computed.equal("viewMode", "side_by_side_markdown"),
@@ -118,6 +135,9 @@ export default ObjectController.extend(ModalFunctionality, {
     loadNextVersion: function() { this.refresh(this.get("post_id"), this.get("version") + 1); },
     loadLastVersion: function() { this.refresh(this.get("post_id"), this.get("revisions_count")); },
 
+    hideVersion: function() { this.hide(this.get("post_id"), this.get("version")); },
+    showVersion: function() { this.show(this.get("post_id"), this.get("version")); },
+
     displayInline: function() { this.set("viewMode", "inline"); },
     displaySideBySide: function() { this.set("viewMode", "side_by_side"); },
     displaySideBySideMarkdown: function() { this.set("viewMode", "side_by_side_markdown"); }
diff --git a/app/assets/javascripts/discourse/models/_post.js b/app/assets/javascripts/discourse/models/_post.js
index 58094963c20..620baf40138 100644
--- a/app/assets/javascripts/discourse/models/_post.js
+++ b/app/assets/javascripts/discourse/models/_post.js
@@ -471,6 +471,14 @@ Discourse.Post.reopenClass({
     });
   },
 
+  hideRevision: function(postId, version) {
+    return Discourse.ajax("/posts/" + postId + "/revisions/" + version + "/hide", { type: 'PUT' });
+  },
+
+  showRevision: function(postId, version) {
+    return Discourse.ajax("/posts/" + postId + "/revisions/" + version + "/show", { type: 'PUT' });
+  },
+
   loadQuote: function(postId) {
     return Discourse.ajax("/posts/" + postId + ".json").then(function (result) {
       var post = Discourse.Post.create(result);
diff --git a/app/assets/javascripts/discourse/templates/modal/history.hbs b/app/assets/javascripts/discourse/templates/modal/history.hbs
index 5ef0ef98366..783f7e60e78 100644
--- a/app/assets/javascripts/discourse/templates/modal/history.hbs
+++ b/app/assets/javascripts/discourse/templates/modal/history.hbs
@@ -6,6 +6,12 @@
       <div id="revision-numbers" {{bind-attr class="displayRevisions::invisible"}}>{{{boundI18n revisionsTextKey previousBinding="previousVersion" currentBinding="version" totalBinding="revisions_count"}}}</div>
       <button title="{{i18n post.revisions.controls.next}}" {{bind-attr class=":btn :standard displayGoToNext::invisible" disabled=loading}} {{action "loadNextVersion"}}><i class="fa fa-forward"></i></button>
       <button title="{{i18n post.revisions.controls.last}}" {{bind-attr class=":btn :standard displayGoToLast::invisible" disabled=loading}} {{action "loadLastVersion"}}><i class="fa fa-fast-forward"></i></button>
+      {{#if displayHide}}
+        <button title="{{i18n post.revisions.controls.hide}}" {{bind-attr class=":btn :standard" disabled=loading}} {{action "hideVersion"}}><i class="fa fa-trash-o"></i></button>
+      {{/if}}
+      {{#if displayShow}}
+        <button title="{{i18n post.revisions.controls.show}}" {{bind-attr class=":btn :standard" disabled=loading}} {{action "showVersion"}}><i class="fa fa-undo"></i></button>
+      {{/if}}
     </div>
     {{#if loading}}<div id='revision-loading'><i class='fa fa-spinner fa-spin'></i>{{i18n loading}}</div>{{/if}}
     <div id="display-modes">
diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb
index ee9eb605220..e52b5f0fd5b 100644
--- a/app/controllers/posts_controller.rb
+++ b/app/controllers/posts_controller.rb
@@ -216,6 +216,20 @@ class PostsController < ApplicationController
     render_json_dump(post_revision_serializer)
   end
 
+  def hide_revision
+    post_revision = find_post_revision_from_params
+    guardian.ensure_can_hide_post_revision! post_revision
+    post_revision.hide!
+    render nothing: true
+  end
+
+  def show_revision
+    post_revision = find_post_revision_from_params
+    guardian.ensure_can_show_post_revision! post_revision
+    post_revision.show!
+    render nothing: true
+  end
+
   def bookmark
     post = find_post_from_params
     if current_user
diff --git a/app/models/post_revision.rb b/app/models/post_revision.rb
index b757efbfac5..99ac5e3aa5e 100644
--- a/app/models/post_revision.rb
+++ b/app/models/post_revision.rb
@@ -29,8 +29,8 @@ class PostRevision < ActiveRecord::Base
   end
 
   def wiki_changes
-    prev = lookup("wiki", 0)
-    cur = lookup("wiki", 1)
+    prev = previous("wiki")
+    cur = current("wiki")
     return if prev == cur
 
     {
@@ -40,8 +40,8 @@ class PostRevision < ActiveRecord::Base
   end
 
   def post_type_changes
-    prev = lookup("post_type", 0)
-    cur = lookup("post_type", 1)
+    prev = previous("post_type")
+    cur = current("post_type")
     return if prev == cur
 
     {
@@ -75,48 +75,96 @@ class PostRevision < ActiveRecord::Base
   end
 
   def previous(field)
-    lookup_with_fallback(field, 0)
+    val = lookup(field)
+    if val.nil?
+      val = lookup_in_previous_revisions(field)
+    end
+
+    if val.nil?
+      val = lookup_in_post(field)
+    end
+
+    val
   end
 
   def current(field)
-    lookup_with_fallback(field, 1)
+    val = lookup_in_next_revision(field)
+    if val.nil?
+      val = lookup_in_post(field)
+    end
+
+    if val.nil?
+      val = lookup(field)
+    end
+
+    if val.nil?
+      val = lookup_in_previous_revisions(field)
+    end
+
+    return val
   end
 
   def previous_revisions
-    @previous_revs ||= PostRevision.where("post_id = ? AND number < ?", post_id, number)
+    @previous_revs ||= PostRevision.where("post_id = ? AND number < ? AND hidden = ?", post_id, number, false)
                                    .order("number desc")
                                    .to_a
   end
 
+  def next_revision
+    @next_revision ||= PostRevision.where("post_id = ? AND number > ? AND hidden = ?", post_id, number, false)
+                                   .order("number asc")
+                                   .to_a.first
+  end
+
   def has_topic_data?
     post && post.post_number == 1
   end
 
-  def lookup_with_fallback(field, index)
-
-    unless val = lookup(field, index)
-      previous_revisions.each do |v|
-        break if val = v.lookup(field, 1)
-      end
+  def lookup_in_previous_revisions(field)
+    previous_revisions.each do |v|
+      val = v.lookup(field)
+      return val unless val.nil?
     end
 
-    unless val
-      if ["cooked", "raw"].include?(field)
-        val = post.send(field)
-      else
-        val = post.topic.send(field)
-      end
+    nil
+  end
+
+  def lookup_in_next_revision(field)
+    if next_revision
+      return next_revision.lookup(field)
+    end
+  end
+
+  def lookup_in_post(field)
+    if !post
+      return
+    elsif ["cooked", "raw"].include?(field)
+      val = post.send(field)
+    elsif ["title", "category_id"].include?(field)
+      val = post.topic.send(field)
     end
 
     val
   end
 
-  def lookup(field, index)
-    if mod = modifications[field]
-      mod[index]
+  def lookup(field)
+    return nil if hidden
+    mod = modifications[field]
+    unless mod.nil?
+      mod[0]
     end
   end
 
+  def hide!
+    self.hidden = true
+    self.save!
+  end
+
+  def show!
+    self.hidden = false
+    self.save!
+  end
+
 end
 
 # == Schema Information
diff --git a/app/serializers/post_revision_serializer.rb b/app/serializers/post_revision_serializer.rb
index bd51b6765b9..4f50c3ebcf4 100644
--- a/app/serializers/post_revision_serializer.rb
+++ b/app/serializers/post_revision_serializer.rb
@@ -12,7 +12,8 @@ class PostRevisionSerializer < ApplicationSerializer
              :category_changes,
              :user_changes,
              :wiki_changes,
-             :post_type_changes
+             :post_type_changes,
+             :hidden
 
   def include_title_changes?
     object.has_topic_data?
@@ -22,6 +23,10 @@ class PostRevisionSerializer < ApplicationSerializer
     object.has_topic_data?
   end
 
+  def hidden
+    object.hidden
+  end
+
   def version
     object.number
   end
@@ -43,7 +48,7 @@ class PostRevisionSerializer < ApplicationSerializer
   end
 
   def edit_reason
-    object.lookup("edit_reason", 1)
+    object.current("edit_reason")
   end
 
   def user_changes
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 3d55b4dc11e..e6daac438d1 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -1233,6 +1233,8 @@ en:
           previous: "Previous revision"
           next: "Next revision"
           last: "Last revision"
+          hide: "Hide revision"
+          show: "Show revision"
           comparing_previous_to_current_out_of_total: "<strong>{{previous}}</strong> vs <strong>{{current}}</strong> / {{total}}"
         displays:
           inline:
diff --git a/config/routes.rb b/config/routes.rb
index 66c373631bc..eab50d805af 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -277,6 +277,8 @@ Discourse::Application.routes.draw do
     put "unhide"
     get "replies"
     get "revisions/:revision" => "posts#revisions"
+    put "revisions/:revision/hide" => "posts#hide_revision"
+    put "revisions/:revision/show" => "posts#show_revision"
     put "recover"
     collection do
       delete "destroy_many"
diff --git a/db/migrate/20141014032859_add_hidden_to_post_revision.rb b/db/migrate/20141014032859_add_hidden_to_post_revision.rb
new file mode 100644
index 00000000000..93423e2190a
--- /dev/null
+++ b/db/migrate/20141014032859_add_hidden_to_post_revision.rb
@@ -0,0 +1,5 @@
+class AddHiddenToPostRevision < ActiveRecord::Migration
+  def change
+    add_column :post_revisions, :hidden, :boolean, null: false, default: false
+  end
+end
diff --git a/lib/guardian/post_guardian.rb b/lib/guardian/post_guardian.rb
index 901e641d7f6..4da4acf03d8 100644
--- a/lib/guardian/post_guardian.rb
+++ b/lib/guardian/post_guardian.rb
@@ -157,6 +157,14 @@ module PostGuardian
     can_see_post?(post)
   end
 
+  def can_hide_post_revision?(post_revision)
+    is_staff?
+  end
+
+  def can_show_post_revision?(post_revision)
+    is_staff?
+  end
+
   def can_vote?(post, opts={})
     post_can_act?(post,:vote, opts)
   end
diff --git a/spec/models/post_revision_spec.rb b/spec/models/post_revision_spec.rb
index 44500bc088f..b19bacb0639 100644
--- a/spec/models/post_revision_spec.rb
+++ b/spec/models/post_revision_spec.rb
@@ -12,18 +12,19 @@ describe PostRevision do
     PostRevision.create!(post_id: post_id, user_id: 1, number: @number, modifications: modifications)
   end
 
-  it "can grab history from current object" do
+  it "ignores deprecated current values in history" do
     p = PostRevision.new(modifications: {"foo" => ["bar", "bar1"]})
     p.previous("foo").should == "bar"
-    p.current("foo").should == "bar1"
+    p.current("foo").should == "bar"
   end
 
   it "can fallback to previous revisions if needed" do
-    create_rev("foo" => ["A", "B"])
-    r2 = create_rev("bar" => ["C", "D"])
+    r1 = create_rev("foo" => ["A", "B"])
+    r2 = create_rev("foo" => ["C", "D"])
 
-    r2.current("foo").should == "B"
-    r2.previous("foo").should == "B"
+    r1.current("foo").should == "C"
+    r2.current("foo").should == "C"
+    r2.previous("foo").should == "C"
   end
 
   it "can fallback to post if needed" do
@@ -36,6 +37,16 @@ describe PostRevision do
     r.previous("cooked").should == post.cooked
   end
 
+  it "can fallback to post for current rev only if needed" do
+    post = Fabricate(:post)
+    r = create_rev({"raw" => ["A"], "cooked" => ["AA"]}, post.id)
+
+    r.current("raw").should == post.raw
+    r.previous("raw").should == "A"
+    r.current("cooked").should == post.cooked
+    r.previous("cooked").should == "AA"
+  end
+
   it "can fallback to topic if needed" do
     post = Fabricate(:post)
     r = create_rev({"foo" => ["A", "B"]}, post.id)
@@ -45,37 +56,64 @@ describe PostRevision do
   end
 
   it "can find title changes" do
-    r = create_rev({"title" => ["hello", "frog"]})
-    r.title_changes[:inline].should =~ /frog.*hello/
-    r.title_changes[:side_by_side].should =~ /hello.*frog/
+    r1 = create_rev({"title" => ["hello"]})
+    r2 = create_rev({"title" => ["frog"]})
+    r1.title_changes[:inline].should =~ /frog.*hello/
+    r1.title_changes[:side_by_side].should =~ /hello.*frog/
   end
 
   it "can find category changes" do
     cat1 = Fabricate(:category, name: "cat1")
     cat2 = Fabricate(:category, name: "cat2")
 
-    r = create_rev({"category_id" => [cat1.id, cat2.id]})
+    r1 = create_rev({"category_id" => [cat1.id, cat2.id]})
+    r2 = create_rev({"category_id" => [cat2.id, cat1.id]})
 
-    changes = r.category_changes
+    changes = r1.category_changes
     changes[:previous_category_id].should == cat1.id
     changes[:current_category_id].should == cat2.id
 
   end
 
   it "can find wiki changes" do
-    r = create_rev("wiki" => [false, true])
+    r1 = create_rev("wiki" => [false])
+    r2 = create_rev("wiki" => [true])
 
-    changes = r.wiki_changes
+    changes = r1.wiki_changes
     changes[:previous_wiki].should == false
     changes[:current_wiki].should == true
   end
 
   it "can find post_type changes" do
-    r = create_rev("post_type" => [1, 2])
+    r1 = create_rev("post_type" => [1])
+    r2 = create_rev("post_type" => [2])
 
-    changes = r.post_type_changes
+    changes = r1.post_type_changes
     changes[:previous_post_type].should == 1
     changes[:current_post_type].should == 2
   end
 
+  it "hides revisions that were hidden" do
+    r1 = create_rev({"raw" => ["one"]})
+    r2 = create_rev({"raw" => ["two"]})
+    r3 = create_rev({"raw" => ["three"]})
+
+    r2.hide!
+
+    r1.current("raw").should == "three"
+    r2.previous("raw").should == "one"
+  end
+
+  it "shows revisions that were shown" do
+    r1 = create_rev({"raw" => ["one"]})
+    r2 = create_rev({"raw" => ["two"]})
+    r3 = create_rev({"raw" => ["three"]})
+
+    r2.hide!
+    r2.show!
+
+    r2.previous("raw").should == "two"
+    r1.current("raw").should == "two"
+  end
+
 end