Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

FEATURE: log private message views

This commit is contained in:
Arpit Jalan 2018-01-27 18:21:22 +05:30
parent 4c19088084
commit 1f6adbea5c
8 changed files with 93 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
app/models/user_history.rb
View File

@ -65,7 +65,8 @@ class UserHistory < ActiveRecord::Base
notified_about_get_a_room: 47,
change_name: 48,
post_locked: 49,
post_unlocked: 50)
post_unlocked: 50,
check_personal_message: 51)
end
# Staff actions is a subset of all actions, used to audit actions taken by staff users.
@ -108,7 +109,8 @@ class UserHistory < ActiveRecord::Base
:backup_download,
:backup_destroy,
:post_locked,
:post_unlocked]
:post_unlocked,
:check_personal_message]
end
def self.staff_action_ids

7
app/services/staff_action_logger.rb
View File

@ -379,6 +379,13 @@ class StaffActionLogger
new_value: state))
end
def log_check_personal_message(topic, opts = {})
raise Discourse::InvalidParameters.new(:topic) unless topic && topic.is_a?(Topic)
UserHistory.create(params(opts).merge(action: UserHistory.actions[:check_personal_message],
topic_id: topic.id,
context: topic.relative_url))
end
private
def params(opts = nil)

1
config/locales/client.en.yml
View File

@ -3227,6 +3227,7 @@ en:
custom_staff: "plugin custom action"
post_locked: "post locked"
post_unlocked: "post unlocked"
check_personal_message: "check personal message"
screened_emails:
title: "Screened Emails"
description: "When someone tries to create a new account, the following email addresses will be checked and the registration will be blocked, or some other action performed."

3
config/locales/server.en.yml
View File

@ -1463,6 +1463,9 @@ en:
show_inactive_accounts: "Allow logged in users to browse profiles of inactive accounts."
hide_suspension_reasons: "Don't display suspension reasons publically on user profiles."
log_personal_messages_views: "Log personal message views by Admin for other users/groups."
user_website_domains_whitelist: "User website will be verified against these domains. Pipe-delimited list."
allow_profile_backgrounds: "Allow users to upload profile backgrounds."

1
config/site_settings.yml
View File

@ -425,6 +425,7 @@ users:
hide_suspension_reasons:
default: false
client: true
log_personal_messages_views: false
groups:
enable_group_directory:

5
lib/topic_view.rb
View File

@ -489,8 +489,9 @@ class TopicView
raise Discourse::NotFound if @topic.blank?
# Special case: If the topic is private and the user isn't logged in, ask them
# to log in!
if @topic.present? && @topic.private_message? && @user.blank?
raise Discourse::NotLoggedIn.new
if @topic.present? && @topic.private_message?
raise Discourse::NotLoggedIn.new if @user.blank?
StaffActionLogger.new(@user).log_check_personal_message(@topic) if SiteSetting.log_personal_messages_views && @topic.all_allowed_users.where(id: @user.id).blank?
end
raise Discourse::InvalidAccess.new("can't see #{@topic}", @topic) unless @guardian.can_see?(@topic)
end

99
spec/components/topic_view_spec.rb
View File

@ -4,13 +4,13 @@ require 'topic_view'
describe TopicView do
let(:topic) { create_topic }
let(:coding_horror) { Fabricate(:coding_horror) }
let(:evil_trout) { Fabricate(:evil_trout) }
let(:first_poster) { topic.user }
let(:topic_view) { TopicView.new(topic.id, coding_horror) }
let(:topic_view) { TopicView.new(topic.id, evil_trout) }
it "raises a not found error if the topic doesn't exist" do
expect { TopicView.new(1231232, coding_horror) }.to raise_error(Discourse::NotFound)
expect { TopicView.new(1231232, evil_trout) }.to raise_error(Discourse::NotFound)
end
# see also spec/controllers/topics_controller_spec.rb TopicsController::show::permission errors
@ -28,23 +28,23 @@ describe TopicView do
context "chunk_size" do
it "returns `chunk_size` by default" do
expect(TopicView.new(topic.id, coding_horror).chunk_size).to eq(TopicView.chunk_size)
expect(TopicView.new(topic.id, evil_trout).chunk_size).to eq(TopicView.chunk_size)
end
it "returns `slow_chunk_size` when slow_platform is true" do
tv = TopicView.new(topic.id, coding_horror, slow_platform: true)
tv = TopicView.new(topic.id, evil_trout, slow_platform: true)
expect(tv.chunk_size).to eq(TopicView.slow_chunk_size)
end
it "returns `print_chunk_size` when print param is true" do
tv = TopicView.new(topic.id, coding_horror, print: true)
tv = TopicView.new(topic.id, evil_trout, print: true)
expect(tv.chunk_size).to eq(TopicView.print_chunk_size)
end
end
context "with a few sample posts" do
let!(:p1) { Fabricate(:post, topic: topic, user: first_poster, percent_rank: 1) }
let!(:p2) { Fabricate(:post, topic: topic, user: coding_horror, percent_rank: 0.5) }
let!(:p2) { Fabricate(:post, topic: topic, user: evil_trout, percent_rank: 0.5) }
let!(:p3) { Fabricate(:post, topic: topic, user: first_poster, percent_rank: 0) }
let(:moderator) { Fabricate(:moderator) }
@ -52,7 +52,7 @@ describe TopicView do
it "it can find the best responses" do
best2 = TopicView.new(topic.id, coding_horror, best: 2)
best2 = TopicView.new(topic.id, evil_trout, best: 2)
expect(best2.posts.count).to eq(2)
expect(best2.posts[0].id).to eq(p2.id)
expect(best2.posts[1].id).to eq(p3.id)
@ -67,7 +67,7 @@ describe TopicView do
expect(best.posts.pluck(:id)).to match_array([p2.id, p3.id])
# should get no results for trust level too low
best = TopicView.new(topic.id, nil, best: 99, min_trust_level: coding_horror.trust_level + 1)
best = TopicView.new(topic.id, nil, best: 99, min_trust_level: evil_trout.trust_level + 1)
expect(best.posts.count).to eq(0)
# should filter out the posts with a score that is too low
@ -81,11 +81,11 @@ describe TopicView do
# should punch through posts if the score is high enough
p2.update_column(:score, 100)
best = TopicView.new(topic.id, nil, best: 99, bypass_trust_level_score: 100, min_trust_level: coding_horror.trust_level + 1)
best = TopicView.new(topic.id, nil, best: 99, bypass_trust_level_score: 100, min_trust_level: evil_trout.trust_level + 1)
expect(best.posts.count).to eq(1)
# 0 means ignore
best = TopicView.new(topic.id, nil, best: 99, bypass_trust_level_score: 0, min_trust_level: coding_horror.trust_level + 1)
best = TopicView.new(topic.id, nil, best: 99, bypass_trust_level_score: 0, min_trust_level: evil_trout.trust_level + 1)
expect(best.posts.count).to eq(0)
# If we restrict to posts a moderator liked, return none
@ -109,6 +109,19 @@ describe TopicView do
expect { TopicView.new(topic.id, nil) }.to raise_error(Discourse::NotLoggedIn)
end
it "logs personal message views if log_check_personal_message is enabled" do
SiteSetting.log_personal_messages_views = true
private_message = Fabricate(:private_message_topic)
allowed_user = private_message.topic_allowed_users.first.user
TopicView.new(private_message.id, allowed_user)
expect(UserHistory.where(action: UserHistory.actions[:check_personal_message]).count).to eq(0)
evil_trout.admin = true
TopicView.new(private_message.id, evil_trout)
expect(UserHistory.where(action: UserHistory.actions[:check_personal_message]).count).to eq(1)
end
it "provides an absolute url" do
expect(topic_view.absolute_url).to be_present
end
@ -161,11 +174,11 @@ describe TopicView do
context '.post_counts_by_user' do
it 'returns the two posters with their appropriate counts' do
Fabricate(:post, topic: topic, user: coding_horror, post_type: Post.types[:whisper])
Fabricate(:post, topic: topic, user: evil_trout, post_type: Post.types[:whisper])
expect(topic_view.post_counts_by_user.to_a).to match_array([[first_poster.id, 2], [coding_horror.id, 2]])
expect(topic_view.post_counts_by_user.to_a).to match_array([[first_poster.id, 2], [evil_trout.id, 2]])
expect(TopicView.new(topic.id, first_poster).post_counts_by_user.to_a).to match_array([[first_poster.id, 2], [coding_horror.id, 1]])
expect(TopicView.new(topic.id, first_poster).post_counts_by_user.to_a).to match_array([[first_poster.id, 2], [evil_trout.id, 1]])
end
it "doesn't return counts for posts with authors who have been deleted" do
@ -178,7 +191,7 @@ describe TopicView do
context '.participants' do
it 'returns the two participants hashed by id' do
expect(topic_view.participants.to_a).to match_array([[first_poster.id, first_poster], [coding_horror.id, coding_horror]])
expect(topic_view.participants.to_a).to match_array([[first_poster.id, first_poster], [evil_trout.id, evil_trout]])
end
end
@ -188,7 +201,7 @@ describe TopicView do
end
it 'returns the like' do
PostAction.act(coding_horror, p1, PostActionType.types[:like])
PostAction.act(evil_trout, p1, PostActionType.types[:like])
expect(topic_view.all_post_actions[p1.id][PostActionType.types[:like]]).to be_present
end
end
@ -200,14 +213,14 @@ describe TopicView do
it 'returns the active flags' do
PostAction.act(moderator, p1, PostActionType.types[:off_topic])
PostAction.act(coding_horror, p1, PostActionType.types[:off_topic])
PostAction.act(evil_trout, p1, PostActionType.types[:off_topic])
expect(topic_view.all_active_flags[p1.id][PostActionType.types[:off_topic]].count).to eq(2)
end
it 'returns only the active flags' do
PostAction.act(moderator, p1, PostActionType.types[:off_topic])
PostAction.act(coding_horror, p1, PostActionType.types[:off_topic])
PostAction.act(evil_trout, p1, PostActionType.types[:off_topic])
PostAction.defer_flags!(p1, moderator)
@ -223,12 +236,12 @@ describe TopicView do
# random user has nothing
expect(topic_view.read?(1)).to eq(false)
coding_horror.created_at = 2.days.ago
evil_trout.created_at = 2.days.ago
# a real user that just read it should have it marked
PostTiming.process_timings(coding_horror, topic.id, 1, [[1, 1000]])
expect(TopicView.new(topic.id, coding_horror).read?(1)).to eq(true)
expect(TopicView.new(topic.id, coding_horror).topic_user).to be_present
PostTiming.process_timings(evil_trout, topic.id, 1, [[1, 1000]])
expect(TopicView.new(topic.id, evil_trout).read?(1)).to eq(true)
expect(TopicView.new(topic.id, evil_trout).topic_user).to be_present
end
end
@ -262,11 +275,11 @@ describe TopicView do
context 'whispers' do
it "handles their visibility properly" do
p1 = Fabricate(:post, topic: topic, user: coding_horror)
p2 = Fabricate(:post, topic: topic, user: coding_horror, post_type: Post.types[:whisper])
p3 = Fabricate(:post, topic: topic, user: coding_horror)
p1 = Fabricate(:post, topic: topic, user: evil_trout)
p2 = Fabricate(:post, topic: topic, user: evil_trout, post_type: Post.types[:whisper])
p3 = Fabricate(:post, topic: topic, user: evil_trout)
ch_posts = TopicView.new(topic.id, coding_horror).posts
ch_posts = TopicView.new(topic.id, evil_trout).posts
expect(ch_posts.map(&:id)).to eq([p1.id, p2.id, p3.id])
anon_posts = TopicView.new(topic.id).posts
@ -280,12 +293,12 @@ describe TopicView do
context '.posts' do
# Create the posts in a different order than the sort_order
let!(:p5) { Fabricate(:post, topic: topic, user: coding_horror) }
let!(:p2) { Fabricate(:post, topic: topic, user: coding_horror) }
let!(:p5) { Fabricate(:post, topic: topic, user: evil_trout) }
let!(:p2) { Fabricate(:post, topic: topic, user: evil_trout) }
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!(:p4) { Fabricate(:post, topic: topic, user: evil_trout, 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!(:p7) { Fabricate(:post, topic: topic, user: evil_trout, deleted_at: Time.now) }
let!(:p3) { Fabricate(:post, topic: topic, user: first_poster) }
before do
@ -305,11 +318,11 @@ describe TopicView do
# does not contain contains_gaps with default filtering
expect(topic_view.contains_gaps?).to eq(false)
# contains contains_gaps when filtered by username" do
expect(TopicView.new(topic.id, coding_horror, username_filters: ['eviltrout']).contains_gaps?).to eq(true)
expect(TopicView.new(topic.id, evil_trout, username_filters: ['eviltrout']).contains_gaps?).to eq(true)
# contains contains_gaps when filtered by summary
expect(TopicView.new(topic.id, coding_horror, filter: 'summary').contains_gaps?).to eq(true)
expect(TopicView.new(topic.id, evil_trout, filter: 'summary').contains_gaps?).to eq(true)
# contains contains_gaps when filtered by best
expect(TopicView.new(topic.id, coding_horror, best: 5).contains_gaps?).to eq(true)
expect(TopicView.new(topic.id, evil_trout, best: 5).contains_gaps?).to eq(true)
end
end
@ -324,10 +337,10 @@ describe TopicView do
topic.save!
expect {
TopicView.new(topic.id, coding_horror).posts.count
TopicView.new(topic.id, evil_trout).posts.count
}.to raise_error(Discourse::InvalidAccess)
expect(TopicView.new(t2.id, coding_horror, post_ids: [p1.id, p2.id]).posts.count).to eq(0)
expect(TopicView.new(t2.id, evil_trout, post_ids: [p1.id, p2.id]).posts.count).to eq(0)
end
@ -345,7 +358,7 @@ describe TopicView do
describe "filter_posts_near" do
def topic_view_near(post, show_deleted = false)
TopicView.new(topic.id, coding_horror, post_number: post.post_number, show_deleted: show_deleted)
TopicView.new(topic.id, evil_trout, post_number: post.post_number, show_deleted: show_deleted)
end
it "snaps to the lower boundary" do
@ -370,7 +383,7 @@ describe TopicView do
end
it "gaps deleted posts to an admin" do
coding_horror.admin = true
evil_trout.admin = true
near_view = topic_view_near(p3)
expect(near_view.desired_post).to eq(p3)
expect(near_view.posts).to eq([p2, p3, p5])
@ -379,7 +392,7 @@ describe TopicView do
end
it "returns deleted posts to an admin with show_deleted" do
coding_horror.admin = true
evil_trout.admin = true
near_view = topic_view_near(p3, true)
expect(near_view.desired_post).to eq(p3)
expect(near_view.posts).to eq([p2, p3, p4])
@ -387,7 +400,7 @@ describe TopicView do
end
it "gaps deleted posts by nuked users to an admin" do
coding_horror.admin = true
evil_trout.admin = true
near_view = topic_view_near(p5)
expect(near_view.desired_post).to eq(p5)
# note: both p4 and p6 get skipped
@ -397,7 +410,7 @@ describe TopicView do
end
it "returns deleted posts by nuked users to an admin with show_deleted" do
coding_horror.admin = true
evil_trout.admin = true
near_view = topic_view_near(p5, true)
expect(near_view.desired_post).to eq(p5)
expect(near_view.posts).to eq([p4, p5, p6])
@ -414,7 +427,7 @@ describe TopicView do
end
it 'gaps deleted posts to admins' do
coding_horror.admin = true
evil_trout.admin = true
near_view = topic_view_near(p5)
expect(near_view.posts).to eq([p1, p2, p3, p5])
expect(near_view.gaps.before).to eq(p5.id => [p4.id])
@ -422,7 +435,7 @@ describe TopicView do
end
it 'returns deleted posts to admins' do
coding_horror.admin = true
evil_trout.admin = true
near_view = topic_view_near(p5, true)
expect(near_view.posts).to eq([p1, p2, p3, p4, p5, p6, p7])
expect(near_view.contains_gaps?).to eq(false)
@ -435,7 +448,7 @@ describe TopicView do
let(:tag1) { Fabricate(:tag) }
let(:tag2) { Fabricate(:tag, topic_count: 2) }
subject { TopicView.new(topic.id, coding_horror).page_title }
subject { TopicView.new(topic.id, evil_trout).page_title }
context "uncategorized topic" do
context "topic_page_title_includes_category is false" do

18
spec/services/staff_action_logger_spec.rb
View File

@ -434,4 +434,22 @@ describe StaffActionLogger do
expect(user_history.previous_value).to eq('t')
end
end
describe 'log_check_personal_message' do
let(:personal_message) { Fabricate(:private_message_topic) }
subject(:log_check_personal_message) { described_class.new(admin).log_check_personal_message(personal_message) }
it 'raises an error when topic is nil' do
expect { logger.log_check_personal_message(nil) }.to raise_error(Discourse::InvalidParameters)
end
it 'raises an error when topic is not a Topic' do
expect { logger.log_check_personal_message(1) }.to raise_error(Discourse::InvalidParameters)
end
it 'creates a new UserHistory record' do
expect { log_check_personal_message }.to change { UserHistory.count }.by(1)
end
end
end