Merge pull request #2546 from riking/hide_deleted

Hide deleted posts by default for staff

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();
+    }
+  }
+});

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'));

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 {

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

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');

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}}

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);

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;
 }

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)

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

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..."

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)) &&

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

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

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