diff --git a/app/models/user_history.rb b/app/models/user_history.rb
index 980ba8728f0..7bd0f68587b 100644
--- a/app/models/user_history.rb
+++ b/app/models/user_history.rb
@@ -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
diff --git a/app/services/staff_action_logger.rb b/app/services/staff_action_logger.rb
index 6f57021af12..6261e655809 100644
--- a/app/services/staff_action_logger.rb
+++ b/app/services/staff_action_logger.rb
@@ -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)
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index dc948cbbdb6..823fd110480 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -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."
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index d698d82fbab..694378b9181 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -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."
diff --git a/config/site_settings.yml b/config/site_settings.yml
index e02399e1515..1b19897afcc 100644
--- a/config/site_settings.yml
+++ b/config/site_settings.yml
@@ -425,6 +425,7 @@ users:
hide_suspension_reasons:
default: false
client: true
+ log_personal_messages_views: false
groups:
enable_group_directory:
diff --git a/lib/topic_view.rb b/lib/topic_view.rb
index 1ad15dc8798..041f52fe3ef 100644
--- a/lib/topic_view.rb
+++ b/lib/topic_view.rb
@@ -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
diff --git a/spec/components/topic_view_spec.rb b/spec/components/topic_view_spec.rb
index 3e8897ae533..1291da2e355 100644
--- a/spec/components/topic_view_spec.rb
+++ b/spec/components/topic_view_spec.rb
@@ -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
diff --git a/spec/services/staff_action_logger_spec.rb b/spec/services/staff_action_logger_spec.rb
index b8159b28fda..7d8535fe3c5 100644
--- a/spec/services/staff_action_logger_spec.rb
+++ b/spec/services/staff_action_logger_spec.rb
@@ -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