From 2422289c8b7e70c74fc4e4c70597f449296be532 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Fri, 25 Sep 2015 10:15:58 +1000
Subject: [PATCH] FIX: whispers should not be revealed in reply to, or reply
expansion FEATURE: mark whisper as experimental FIX: badges should never
apply to whispers
---
app/controllers/posts_controller.rb | 5 +++--
app/models/post.rb | 16 ++++++++------
app/models/topic_featured_users.rb | 4 ++--
config/locales/server.en.yml | 2 +-
...0925000915_exclude_whispers_from_badges.rb | 22 +++++++++++++++++++
spec/controllers/posts_controller_spec.rb | 5 +++--
6 files changed, 40 insertions(+), 14 deletions(-)
create mode 100644 db/migrate/20150925000915_exclude_whispers_from_badges.rb
diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb
index b7c321ed9..44ac6dc8d 100644
--- a/app/controllers/posts_controller.rb
+++ b/app/controllers/posts_controller.rb
@@ -181,7 +181,7 @@ class PostsController < ApplicationController
def reply_history
post = find_post_from_params
- render_serialized(post.reply_history(params[:max_replies].to_i), PostSerializer)
+ render_serialized(post.reply_history(params[:max_replies].to_i, guardian), PostSerializer)
end
def destroy
@@ -237,7 +237,8 @@ class PostsController < ApplicationController
# Direct replies to this post
def replies
post = find_post_from_params
- render_serialized(post.replies, PostSerializer)
+ replies = post.replies.secured(guardian)
+ render_serialized(replies, PostSerializer)
end
def revisions
diff --git a/app/models/post.rb b/app/models/post.rb
index 5693369e8..16a2ce178 100644
--- a/app/models/post.rb
+++ b/app/models/post.rb
@@ -61,6 +61,7 @@ class Post < ActiveRecord::Base
scope :private_posts, -> { joins(:topic).where('topics.archetype = ?', Archetype.private_message) }
scope :with_topic_subtype, ->(subtype) { joins(:topic).where('topics.subtype = ?', subtype) }
scope :visible, -> { joins(:topic).where('topics.visible = true').where(hidden: false) }
+ scope :secured, lambda { |guardian| where('posts.post_type in (?)', Topic.visible_post_types(guardian && guardian.user))}
delegate :username, to: :user
@@ -108,12 +109,11 @@ class Post < ActiveRecord::Base
type: type
}
- # Whispers should not be published to everyone
- if post_type == Post.types[:whisper]
+ if Topic.visible_post_types.include?(post_type)
+ MessageBus.publish(channel, msg, group_ids: topic.secure_group_ids)
+ else
user_ids = User.where('admin or moderator or id = ?', user_id).pluck(:id)
MessageBus.publish(channel, msg, user_ids: user_ids)
- else
- MessageBus.publish(channel, msg, group_ids: topic.secure_group_ids)
end
end
@@ -521,7 +521,7 @@ class Post < ActiveRecord::Base
private_posts.with_topic_subtype(topic_subtype).where('posts.created_at > ?', since_days_ago.days.ago).group('date(posts.created_at)').order('date(posts.created_at)').count
end
- def reply_history(max_replies=100)
+ def reply_history(max_replies=100, guardian=nil)
post_ids = Post.exec_sql("WITH RECURSIVE breadcrumb(id, reply_to_post_number) AS (
SELECT p.id, p.reply_to_post_number FROM posts AS p
WHERE p.id = :post_id
@@ -537,7 +537,7 @@ class Post < ActiveRecord::Base
# [1,2,3][-10,-1] => nil
post_ids = (post_ids[(0-max_replies)..-1] || post_ids)
- Post.where(id: post_ids).includes(:user, :topic).order(:id).to_a
+ Post.secured(guardian).where(id: post_ids).includes(:user, :topic).order(:id).to_a
end
def revert_to(number)
@@ -586,7 +586,9 @@ class Post < ActiveRecord::Base
return if post.nil?
post_reply = post.post_replies.new(reply_id: id)
if post_reply.save
- Post.where(id: post.id).update_all ['reply_count = reply_count + 1']
+ if Topic.visible_post_types.include?(self.post_type)
+ Post.where(id: post.id).update_all ['reply_count = reply_count + 1']
+ end
end
end
diff --git a/app/models/topic_featured_users.rb b/app/models/topic_featured_users.rb
index ee461cbe6..9d6297a4c 100644
--- a/app/models/topic_featured_users.rb
+++ b/app/models/topic_featured_users.rb
@@ -38,7 +38,7 @@ WITH cte as (
JOIN posts p ON p.topic_id = t.id
WHERE p.deleted_at IS NULL AND
NOT p.hidden AND
- p.post_type <> #{Post.types[:whisper]} AND
+ p.post_type in (#{Topic.visible_post_types.join(",")}) AND
p.user_id <> t.user_id AND
p.user_id <> t.last_post_user_id
#{filter}
@@ -82,7 +82,7 @@ SQL
private
def update_participant_count
- count = topic.posts.where('NOT hidden AND post_type <> ?', Post.types[:whisper]).count('distinct user_id')
+ count = topic.posts.where('NOT hidden AND post_type in (?)', Topic.visible_post_types).count('distinct user_id')
topic.update_columns(participant_count: count)
end
end
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 945846f67..d6cffcd20 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -897,7 +897,7 @@ en:
email_token_grace_period_hours: "Forgot password / activate account tokens are still valid for a grace period of (n) hours after being redeemed."
enable_badges: "Enable the badge system"
- enable_whispers: "Allow users to whisper to staff"
+ enable_whispers: "Allow staff private communication within topic. (experimental)"
allow_index_in_robots_txt: "Specify in robots.txt that this site is allowed to be indexed by web search engines."
email_domains_blacklist: "A pipe-delimited list of email domains that users are not allowed to register accounts with. Example: mailinator.com|trashmail.net"
diff --git a/db/migrate/20150925000915_exclude_whispers_from_badges.rb b/db/migrate/20150925000915_exclude_whispers_from_badges.rb
new file mode 100644
index 000000000..a7326e8ee
--- /dev/null
+++ b/db/migrate/20150925000915_exclude_whispers_from_badges.rb
@@ -0,0 +1,22 @@
+class ExcludeWhispersFromBadges < ActiveRecord::Migration
+ def up
+ execute "DROP VIEW badge_posts"
+
+ execute "CREATE VIEW badge_posts AS
+ SELECT p.*
+ FROM posts p
+ JOIN topics t ON t.id = p.topic_id
+ JOIN categories c ON c.id = t.category_id
+ WHERE c.allow_badges AND
+ p.deleted_at IS NULL AND
+ t.deleted_at IS NULL AND
+ NOT c.read_restricted AND
+ t.visible AND
+ p.post_type IN (1,2,3)
+ "
+ end
+
+ def down
+ # nada, nothing to do just keep good view
+ end
+end
diff --git a/spec/controllers/posts_controller_spec.rb b/spec/controllers/posts_controller_spec.rb
index 748cfca7c..cc42bf48e 100644
--- a/spec/controllers/posts_controller_spec.rb
+++ b/spec/controllers/posts_controller_spec.rb
@@ -141,8 +141,9 @@ describe PostsController do
end
it 'asks post for replies' do
- Post.any_instance.expects(:replies)
- xhr :get, :replies, post_id: post.id
+ p1 = Fabricate(:post)
+ xhr :get, :replies, post_id: p1.id
+ expect(response.status).to eq(200)
end
end