From 19b757b0587653eeba2fe516fe29c334ee79ca77 Mon Sep 17 00:00:00 2001
From: riking <rikingcoding@gmail.com>
Date: Tue, 15 Jul 2014 14:02:43 -0700
Subject: [PATCH] FEATURE: Hide deleted posts by default for staff

---
 .../components/toggle-deleted.js.es6          | 20 ++++++++
 .../discourse/controllers/topic_controller.js |  2 +-
 .../discourse/models/post_stream.js           | 10 +++-
 .../javascripts/discourse/models/topic.js     |  1 +
 .../discourse/routes/topic_route.js           |  4 +-
 .../components/toggle-deleted.js.handlebars   |  7 +++
 .../views/topic-map-container.js.es6          | 11 ++++
 .../stylesheets/desktop/topic-post.scss       | 40 ++++++---------
 app/controllers/topics_controller.rb          |  2 +-
 app/serializers/topic_view_serializer.rb      |  9 ++++
 config/locales/client.en.yml                  |  6 +++
 lib/guardian/topic_guardian.rb                | 12 +++--
 lib/topic_view.rb                             | 23 +++++++--
 spec/components/guardian_spec.rb              | 20 ++++++++
 spec/components/topic_view_spec.rb            | 51 ++++++++++++++++---
 15 files changed, 172 insertions(+), 46 deletions(-)
 create mode 100644 app/assets/javascripts/discourse/components/toggle-deleted.js.es6
 create mode 100644 app/assets/javascripts/discourse/templates/components/toggle-deleted.js.handlebars

diff --git a/app/assets/javascripts/discourse/components/toggle-deleted.js.es6 b/app/assets/javascripts/discourse/components/toggle-deleted.js.es6
new file mode 100644
index 00000000000..a672696f780
--- /dev/null
+++ b/app/assets/javascripts/discourse/components/toggle-deleted.js.es6
@@ -0,0 +1,20 @@
+/**
+  The controls for toggling the supression of deleted posts
+
+  @class ToggleDeletedComponent
+  @extends Ember.Component
+  @namespace Discourse
+  @module Discourse
+**/
+export default Ember.Component.extend({
+  layoutName: 'components/toggle-deleted',
+  tagName: 'section',
+  classNames: ['information'],
+  postStream: Em.computed.alias('topic.postStream'),
+
+  actions: {
+    toggleDeleted: function() {
+      this.get('postStream').toggleDeleted();
+    }
+  }
+});
diff --git a/app/assets/javascripts/discourse/controllers/topic_controller.js b/app/assets/javascripts/discourse/controllers/topic_controller.js
index c52c72e469f..a6d2b29903b 100644
--- a/app/assets/javascripts/discourse/controllers/topic_controller.js
+++ b/app/assets/javascripts/discourse/controllers/topic_controller.js
@@ -13,7 +13,7 @@ Discourse.TopicController = Discourse.ObjectController.extend(Discourse.Selected
   editingTopic: false,
   selectedPosts: null,
   selectedReplies: null,
-  queryParams: ['filter', 'username_filters'],
+  queryParams: ['filter', 'username_filters', 'show_deleted'],
 
   contextChanged: function(){
     this.set('controllers.search.searchContext', this.get('model.searchContext'));
diff --git a/app/assets/javascripts/discourse/models/post_stream.js b/app/assets/javascripts/discourse/models/post_stream.js
index a8ff38bf377..da79ba37d87 100644
--- a/app/assets/javascripts/discourse/models/post_stream.js
+++ b/app/assets/javascripts/discourse/models/post_stream.js
@@ -120,6 +120,7 @@ Discourse.PostStream = Em.Object.extend({
   streamFilters: function() {
     var result = {};
     if (this.get('summary')) { result.filter = "summary"; }
+    if (this.get('show_deleted')) { result.show_deleted = true; }
 
     var userFilters = this.get('userFilters');
     if (!Em.isEmpty(userFilters)) {
@@ -128,7 +129,7 @@ Discourse.PostStream = Em.Object.extend({
     }
 
     return result;
-  }.property('userFilters.[]', 'summary'),
+  }.property('userFilters.[]', 'summary', 'show_deleted'),
 
   hasNoFilters: function() {
     var streamFilters = this.get('streamFilters');
@@ -186,6 +187,7 @@ Discourse.PostStream = Em.Object.extend({
   **/
   cancelFilter: function() {
     this.set('summary', false);
+    this.set('show_deleted', false);
     this.get('userFilters').clear();
   },
 
@@ -200,6 +202,11 @@ Discourse.PostStream = Em.Object.extend({
     return this.refresh();
   },
 
+  toggleDeleted: function() {
+    this.toggleProperty('show_deleted');
+    return this.refresh();
+  },
+
   /**
     Filter the stream to a particular user.
 
@@ -208,6 +215,7 @@ Discourse.PostStream = Em.Object.extend({
   toggleParticipant: function(username) {
     var userFilters = this.get('userFilters');
     this.set('summary', false);
+    this.set('show_deleted', true);
     if (userFilters.contains(username)) {
       userFilters.remove(username);
     } else {
diff --git a/app/assets/javascripts/discourse/models/topic.js b/app/assets/javascripts/discourse/models/topic.js
index cb6943d9b56..4de71e7435c 100644
--- a/app/assets/javascripts/discourse/models/topic.js
+++ b/app/assets/javascripts/discourse/models/topic.js
@@ -396,6 +396,7 @@ Discourse.Topic.reopenClass({
       opts.userFilters.forEach(function(username) {
         data.username_filters.push(username);
       });
+      data.show_deleted = true;
     }
 
     // Add the summary of filter if we have it
diff --git a/app/assets/javascripts/discourse/routes/topic_route.js b/app/assets/javascripts/discourse/routes/topic_route.js
index 5722b19ff4d..9063c8b2b47 100644
--- a/app/assets/javascripts/discourse/routes/topic_route.js
+++ b/app/assets/javascripts/discourse/routes/topic_route.js
@@ -11,7 +11,8 @@ Discourse.TopicRoute = Discourse.Route.extend({
 
   queryParams: {
     filter: { replace: true },
-    username_filters: { replace: true }
+    username_filters: { replace: true },
+    show_deleted: { replace: true }
   },
 
   actions: {
@@ -96,6 +97,7 @@ Discourse.TopicRoute = Discourse.Route.extend({
   setupParams: function(topic, params) {
     var postStream = topic.get('postStream');
     postStream.set('summary', Em.get(params, 'filter') === 'summary');
+    postStream.set('show_deleted', !!Em.get(params, 'show_deleted'));
 
     var usernames = Em.get(params, 'username_filters'),
         userFilters = postStream.get('userFilters');
diff --git a/app/assets/javascripts/discourse/templates/components/toggle-deleted.js.handlebars b/app/assets/javascripts/discourse/templates/components/toggle-deleted.js.handlebars
new file mode 100644
index 00000000000..c4c3497c6fe
--- /dev/null
+++ b/app/assets/javascripts/discourse/templates/components/toggle-deleted.js.handlebars
@@ -0,0 +1,7 @@
+{{#if postStream.show_deleted}}
+  <p>{{i18n deleted_filter.disabled_description}}</p>
+  <button class='btn btn-danger' {{action toggleDeleted}}>{{i18n deleted_filter.enable}}</button>
+{{else}}
+  <p>{{i18n deleted_filter.enabled_description}}</p>
+  <button class='btn btn-danger' {{action toggleDeleted}}>{{i18n deleted_filter.disable}}</button>
+{{/if}}
diff --git a/app/assets/javascripts/discourse/views/topic-map-container.js.es6 b/app/assets/javascripts/discourse/views/topic-map-container.js.es6
index a09cf1d6851..2f70b05cb3d 100644
--- a/app/assets/javascripts/discourse/views/topic-map-container.js.es6
+++ b/app/assets/javascripts/discourse/views/topic-map-container.js.es6
@@ -10,6 +10,7 @@
 import PrivateMessageMapComponent from 'discourse/components/private-message-map';
 import TopicMapComponent from 'discourse/components/topic-map';
 import ToggleSummaryComponent from 'discourse/components/toggle-summary';
+import ToggleDeletedComponent from 'discourse/components/toggle-deleted';
 
 export default Discourse.ContainerView.extend({
   classNameBindings: ['hidden', ':topic-map'],
@@ -43,6 +44,16 @@ export default Discourse.ContainerView.extend({
       }, ToggleSummaryComponent);
     }
 
+    if (Discourse.User.currentProp('staff')) {
+      // If we have deleted post filtering
+      if (topic.get('has_deleted')) {
+        container.attachViewWithArgs({
+          topic: topic,
+          filterBinding: 'controller.filter'
+        }, ToggleDeletedComponent);
+      }
+    }
+
     // If we have a private message
     if (this.get('topic.isPrivateMessage')) {
       container.attachViewWithArgs({ topic: topic, showPrivateInviteAction: 'showPrivateInvite' }, PrivateMessageMapComponent);
diff --git a/app/assets/stylesheets/desktop/topic-post.scss b/app/assets/stylesheets/desktop/topic-post.scss
index 85fa5ed2dcd..edc70c16857 100644
--- a/app/assets/stylesheets/desktop/topic-post.scss
+++ b/app/assets/stylesheets/desktop/topic-post.scss
@@ -1,24 +1,3 @@
-.gap {
-  background-color: scale-color-diff();
-  padding: 5px 0;
-  color: lighten($primary, 35%);
-  cursor: pointer;
-  text-align: center;
-  width: 80%;
-
-  &:hover {
-    background-color: scale-color-diff();
-  }
-
-  @include medium-width {
-    width: 795px;
-  }
-
-  @include small-width {
-    width: 815px;
-  }
-}
-
 .container {
   @extend .clearfix;
   max-width: $large-width;
@@ -759,12 +738,16 @@ blockquote > *:last-child {
   }
 }
 
+// variables are used to calculate the width of .gap
+$topic-body-width: 690px;
+$topic-body-width-padding: 11px;
+$topic-avatar-width: 45px;
 .topic-body {
-  width: 690px;
+  width: $topic-body-width;
   float: left;
   position: relative;
   border-top: 1px solid scale-color-diff();
-  padding: 12px 11px 15px 11px;
+  padding: 12px $topic-body-width-padding 15px $topic-body-width-padding;
   &.highlighted {
     background-color: scale-color($tertiary, $lightness: 85%);
   }
@@ -776,7 +759,7 @@ blockquote > *:last-child {
 .topic-avatar {
  border-top: 1px solid scale-color-diff();
  padding-top: 16px;
- width: 45px;
+ width: $topic-avatar-width;
  float: left;
 
  .wiki {
@@ -786,6 +769,15 @@ blockquote > *:last-child {
  }
 }
 
+.gap {
+  background-color: scale-color-diff();
+  padding: 5px 0;
+  color: lighten($primary, 30%);
+  cursor: pointer;
+  text-align: center;
+  width: calc(#{$topic-avatar-width} + #{$topic-body-width} + 2 * #{$topic-body-width-padding});
+}
+
 .posts-wrapper {
   position: relative;
 }
diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb
index 0949e9d484d..51cc047c3db 100644
--- a/app/controllers/topics_controller.rb
+++ b/app/controllers/topics_controller.rb
@@ -35,7 +35,7 @@ class TopicsController < ApplicationController
     # existing installs.
     return wordpress if params[:best].present?
 
-    opts = params.slice(:username_filters, :filter, :page, :post_number)
+    opts = params.slice(:username_filters, :filter, :page, :post_number, :show_deleted)
     username_filters = opts[:username_filters]
 
     opts[:username_filters] = username_filters.split(',') if username_filters.is_a?(String)
diff --git a/app/serializers/topic_view_serializer.rb b/app/serializers/topic_view_serializer.rb
index b6ec57c43f0..34998320690 100644
--- a/app/serializers/topic_view_serializer.rb
+++ b/app/serializers/topic_view_serializer.rb
@@ -39,6 +39,7 @@ class TopicViewSerializer < ApplicationSerializer
              :highest_post_number,
              :last_read_post_number,
              :deleted_by,
+             :has_deleted,
              :actions_summary,
              :expandable_first_post
 
@@ -177,6 +178,14 @@ class TopicViewSerializer < ApplicationSerializer
     result
   end
 
+  def has_deleted
+    object.has_deleted?
+  end
+
+  def include_has_deleted?
+    object.guardian.can_see_deleted_posts?
+  end
+
   def expandable_first_post
     true
   end
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 187f6df6155..1c594bb774d 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -514,6 +514,12 @@ en:
       enable: 'Summarize This Topic'
       disable: 'Show All Posts'
 
+    deleted_filter:
+      enabled_description: "This topic contains deleted posts, which have been hidden. To see all posts again, click below."
+      disabled_description: "Deleted posts in the topic are shown. To hide them again, click below."
+      enable: "Hide Deleted Posts"
+      disable: "Unhide Deleted Posts"
+
     private_message_info:
       title: "Private Message"
       invite: "Invite Others..."
diff --git a/lib/guardian/topic_guardian.rb b/lib/guardian/topic_guardian.rb
index 1f767fbc50f..a486df515bb 100644
--- a/lib/guardian/topic_guardian.rb
+++ b/lib/guardian/topic_guardian.rb
@@ -45,14 +45,16 @@ module TopicGuardian
     authenticated? && topic && not(topic.private_message?) && @user.has_trust_level?(:basic)
   end
 
+  def can_see_deleted_topics?
+    is_staff?
+  end
+
   def can_see_topic?(topic)
     return false unless topic
+    # Admins can see everything
     return true if is_admin?
-    return false if topic.deleted_at
-
-    # 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
+    # Deleted topics
+    return false if topic.deleted_at && !can_see_deleted_topics?
 
     # not secure, or I can see it
     (not(topic.read_restricted_category?) || can_see_category?(topic.category)) &&
diff --git a/lib/topic_view.rb b/lib/topic_view.rb
index 07f215c1293..8b826c94f2b 100644
--- a/lib/topic_view.rb
+++ b/lib/topic_view.rb
@@ -11,8 +11,8 @@ class TopicView
 
   def initialize(topic_id, user=nil, options={})
     @user = user
-    @topic = find_topic(topic_id)
     @guardian = Guardian.new(@user)
+    @topic = find_topic(topic_id)
     check_and_raise_exceptions
 
     options.each do |key, value|
@@ -187,6 +187,10 @@ class TopicView
     read_posts_set.include?(post_number)
   end
 
+  def has_deleted?
+    @predelete_filtered_posts.with_deleted.where("deleted_at IS NOT NULL").exists?
+  end
+
   def topic_user
     @topic_user ||= begin
       return nil if @user.blank?
@@ -281,7 +285,7 @@ class TopicView
                  .includes(:user)
                  .includes(:reply_to_user)
                  .order('sort_order')
-    @posts = @posts.with_deleted if @user.try(:staff?)
+    @posts = @posts.with_deleted if @guardian.can_see_deleted_posts?
     @posts
   end
 
@@ -300,13 +304,13 @@ class TopicView
 
   def find_topic(topic_id)
     finder = Topic.where(id: topic_id).includes(:category)
-    finder = finder.with_deleted if @user.try(:staff?)
+    finder = finder.with_deleted if @guardian.can_see_deleted_topics?
     finder.first
   end
 
   def unfiltered_posts
     result = @topic.posts
-    result = result.with_deleted if @user.try(:staff?)
+    result = result.with_deleted if @guardian.can_see_deleted_posts?
     result = @topic.posts.where("user_id IS NOT NULL") if @exclude_deleted_users
     result
   end
@@ -316,7 +320,6 @@ class TopicView
     # Certain filters might leave gaps between posts. If that's true, we can return a gap structure
     @contains_gaps = false
     @filtered_posts = unfiltered_posts
-    @filtered_posts = @filtered_posts.with_deleted if @user.try(:staff?)
 
     # Filters
     if @filter == 'summary'
@@ -329,12 +332,22 @@ class TopicView
       @contains_gaps = true
     end
 
+    # Username filters
     if @username_filters.present?
       usernames = @username_filters.map{|u| u.downcase}
       @filtered_posts = @filtered_posts.where('post_number = 1 or user_id in (select u.id from users u where username_lower in (?))', usernames)
       @contains_gaps = true
     end
 
+    # Deleted
+    # This should be last - don't want to tell the admin about deleted posts that clicking the button won't show
+    # copy the filter for has_deleted? method
+    @predelete_filtered_posts = @filtered_posts.spawn
+    if @guardian.can_see_deleted_posts? && !@show_deleted && has_deleted?
+      @filtered_posts = @filtered_posts.where(deleted_at: nil)
+      @contains_gaps = true
+    end
+
   end
 
   def check_and_raise_exceptions
diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb
index 81c6c884a92..639dd5b6310 100644
--- a/spec/components/guardian_spec.rb
+++ b/spec/components/guardian_spec.rb
@@ -326,6 +326,15 @@ describe Guardian do
         Guardian.new(user).can_see?(topic).should be_true
       end
 
+      it "restricts deleted topics" do
+        topic = Fabricate(:topic)
+        topic.trash!(moderator)
+
+        Guardian.new(build(:user)).can_see?(topic).should be_false
+        Guardian.new(moderator).can_see?(topic).should be_true
+        Guardian.new(admin).can_see?(topic).should be_true
+      end
+
       it "restricts private topics" do
         user.save!
         private_topic = Fabricate(:private_message_topic, user: user)
@@ -334,6 +343,17 @@ describe Guardian do
         Guardian.new(moderator).can_see?(private_topic).should be_false
         Guardian.new(admin).can_see?(private_topic).should be_true
       end
+
+      it "restricts private deleted topics" do
+        user.save!
+        private_topic = Fabricate(:private_message_topic, user: user)
+        private_topic.trash!(admin)
+
+        Guardian.new(private_topic.user).can_see?(private_topic).should be_false
+        Guardian.new(build(:user)).can_see?(private_topic).should be_false
+        Guardian.new(moderator).can_see?(private_topic).should be_false
+        Guardian.new(admin).can_see?(private_topic).should be_true
+      end
     end
 
     describe 'a Post' do
diff --git a/spec/components/topic_view_spec.rb b/spec/components/topic_view_spec.rb
index 836e61cb692..2736c6f02c7 100644
--- a/spec/components/topic_view_spec.rb
+++ b/spec/components/topic_view_spec.rb
@@ -218,13 +218,14 @@ describe TopicView do
     let!(:p6) { Fabricate(:post, topic: topic, user: Fabricate(:user), deleted_at: Time.now)}
     let!(:p4) { Fabricate(:post, topic: topic, user: coding_horror, deleted_at: Time.now)}
     let!(:p1) { Fabricate(:post, topic: topic, user: first_poster)}
+    let!(:p7) { Fabricate(:post, topic: topic, user: coding_horror, deleted_at: Time.now)}
     let!(:p3) { Fabricate(:post, topic: topic, user: first_poster)}
 
     before do
       SiteSetting.posts_per_page = 3
 
       # Update them to the sort order we're checking for
-      [p1, p2, p3, p4, p5, p6].each_with_index do |p, idx|
+      [p1, p2, p3, p4, p5, p6, p7].each_with_index do |p, idx|
         p.sort_order = idx + 1
         p.save
       end
@@ -277,40 +278,64 @@ describe TopicView do
 
     describe "filter_posts_near" do
 
-      def topic_view_near(post)
-        TopicView.new(topic.id, coding_horror, post_number: post.post_number)
+      def topic_view_near(post, show_deleted=false)
+        TopicView.new(topic.id, coding_horror, post_number: post.post_number, show_deleted: show_deleted)
       end
 
       it "snaps to the lower boundary" do
         near_view = topic_view_near(p1)
         near_view.desired_post.should == p1
         near_view.posts.should == [p1, p2, p3]
+        near_view.contains_gaps?.should be_false
       end
 
       it "snaps to the upper boundary" do
         near_view = topic_view_near(p5)
         near_view.desired_post.should == p5
         near_view.posts.should == [p2, p3, p5]
+        near_view.contains_gaps?.should be_false
       end
 
       it "returns the posts in the middle" do
         near_view = topic_view_near(p2)
         near_view.desired_post.should == p2
         near_view.posts.should == [p1, p2, p3]
+        near_view.contains_gaps?.should be_false
       end
 
-      it "returns deleted posts to an admin" do
+      it "gaps deleted posts to an admin" do
         coding_horror.admin = true
         near_view = topic_view_near(p3)
         near_view.desired_post.should == p3
-        near_view.posts.should == [p2, p3, p4]
+        near_view.posts.should == [p2, p3, p5]
+        near_view.gaps.before.should == {p5.id => [p4.id]}
+        near_view.gaps.after.should == {p5.id => [p6.id, p7.id]}
       end
 
-      it "returns deleted posts by nuked users to an admin" do
+      it "returns deleted posts to an admin with show_deleted" do
+        coding_horror.admin = true
+        near_view = topic_view_near(p3, true)
+        near_view.desired_post.should == p3
+        near_view.posts.should == [p2, p3, p4]
+        near_view.contains_gaps?.should be_false
+      end
+
+      it "gaps deleted posts by nuked users to an admin" do
         coding_horror.admin = true
         near_view = topic_view_near(p5)
         near_view.desired_post.should == p5
+        # note: both p4 and p6 get skipped
+        near_view.posts.should == [p2, p3, p5]
+        near_view.gaps.before.should == {p5.id => [p4.id]}
+        near_view.gaps.after.should == {p5.id => [p6.id, p7.id]}
+      end
+
+      it "returns deleted posts by nuked users to an admin with show_deleted" do
+        coding_horror.admin = true
+        near_view = topic_view_near(p5, true)
+        near_view.desired_post.should == p5
         near_view.posts.should == [p4, p5, p6]
+        near_view.contains_gaps?.should be_false
       end
 
       context "when 'posts per page' exceeds the number of posts" do
@@ -319,12 +344,22 @@ describe TopicView do
         it 'returns all the posts' do
           near_view = topic_view_near(p5)
           near_view.posts.should == [p1, p2, p3, p5]
+          near_view.contains_gaps?.should be_false
+        end
+
+        it 'gaps deleted posts to admins' do
+          coding_horror.admin = true
+          near_view = topic_view_near(p5)
+          near_view.posts.should == [p1, p2, p3, p5]
+          near_view.gaps.before.should == {p5.id => [p4.id]}
+          near_view.gaps.after.should == {p5.id => [p6.id, p7.id]}
         end
 
         it 'returns deleted posts to admins' do
           coding_horror.admin = true
-          near_view = topic_view_near(p5)
-          near_view.posts.should == [p1, p2, p3, p4, p5, p6]
+          near_view = topic_view_near(p5, true)
+          near_view.posts.should == [p1, p2, p3, p4, p5, p6, p7]
+          near_view.contains_gaps?.should be_false
         end
       end
     end