{{i18n deleted_filter.disabled_description}}
+ +{{else}} +{{i18n deleted_filter.enabled_description}}
+ +{{/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 a09cf1d68..2f70b05cb 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 85fa5ed2d..edc70c168 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 0949e9d48..51cc047c3 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 b6ec57c43..349983206 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 187f6df61..1c594bb77 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 1f767fbc5..a486df515 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 07f215c12..8b826c94f 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 81c6c884a..639dd5b63 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 836e61cb6..2736c6f02 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