notify moderators now goes to the "community" user, that saves our poor mods from a flood of pms

if any staff respond to a pm they are automatically added to the list of recipients and will start
getting email notifications
This commit is contained in:
Sam 2013-09-06 14:07:23 +10:00
parent c495a0b996
commit 41a1b6942d
15 changed files with 635 additions and 583 deletions

View File

@ -96,10 +96,10 @@ class PostAction < ActiveRecord::Base
return unless opts[:message] && [:notify_moderators, :notify_user].include?(post_action_type) return unless opts[:message] && [:notify_moderators, :notify_user].include?(post_action_type)
target_group_names, target_usernames = nil target_usernames = nil
if post_action_type == :notify_moderators if post_action_type == :notify_moderators
target_group_names = target_moderators target_usernames = "community"
else else
target_usernames = post.user.username target_usernames = post.user.username
end end
@ -111,10 +111,9 @@ class PostAction < ActiveRecord::Base
subtype = post_action_type == :notify_moderators ? TopicSubtype.notify_moderators : TopicSubtype.notify_user subtype = post_action_type == :notify_moderators ? TopicSubtype.notify_moderators : TopicSubtype.notify_user
if target_usernames.present? || target_group_names.present? if target_usernames.present?
PostCreator.new(user, PostCreator.new(user,
target_usernames: target_usernames, target_usernames: target_usernames,
target_group_names: target_group_names,
archetype: Archetype.private_message, archetype: Archetype.private_message,
subtype: subtype, subtype: subtype,
title: title, title: title,

View File

@ -38,7 +38,7 @@ class Report
end end
def self.report_signups(report) def self.report_signups(report)
report_about report, User, :count_by_signup_date report_about report, User.real, :count_by_signup_date
end end
def self.report_topics(report) def self.report_topics(report)
@ -76,7 +76,7 @@ class Report
def self.report_users_by_trust_level(report) def self.report_users_by_trust_level(report)
report.data = [] report.data = []
User.counts_by_trust_level.each do |level, count| User.real.group('trust_level').count.each do |level, count|
report.data << {x: level.to_i, y: count} report.data << {x: level.to_i, y: count}
end end
end end

View File

@ -76,6 +76,8 @@ class User < ActiveRecord::Base
scope :blocked, -> { where(blocked: true) } # no index scope :blocked, -> { where(blocked: true) } # no index
scope :banned, -> { where('banned_till IS NOT NULL AND banned_till > ?', Time.zone.now) } # no index scope :banned, -> { where('banned_till IS NOT NULL AND banned_till > ?', Time.zone.now) } # no index
scope :not_banned, -> { where('banned_till IS NULL') } scope :not_banned, -> { where('banned_till IS NULL') }
# excluding fake users like the community user
scope :real, -> { where('id > 0') }
module NewTopicDuration module NewTopicDuration
ALWAYS = -1 ALWAYS = -1
@ -475,10 +477,6 @@ class User < ActiveRecord::Base
where('created_at > ?', sinceDaysAgo.days.ago).group('date(created_at)').order('date(created_at)').count where('created_at > ?', sinceDaysAgo.days.ago).group('date(created_at)').order('date(created_at)').count
end end
def self.counts_by_trust_level
group('trust_level').count
end
def update_topic_reply_count def update_topic_reply_count
self.topic_reply_count = self.topic_reply_count =
Topic Topic

21
db/fixtures/users.rb Normal file
View File

@ -0,0 +1,21 @@
user = User.where("id <> -1 and username_lower = 'community'").first
if user
user.username = UserNameSuggester.suggest('community')
user.save
end
User.seed do |u|
u.id = -1
u.name = 'Community'
u.username = 'community'
u.username_lower = 'community'
u.email = 'no_email'
u.password = SecureRandom.hex
u.bio_raw = 'I am a community user, I clean up the forum and make sure it runs well.'
u.active = true
u.admin = true
u.moderator = true
u.email_direct = false
u.approved = true
u.email_private_messages = false
end

View File

View File

@ -131,8 +131,7 @@ module Discourse
# Either returns the system_username user or the first admin. # Either returns the system_username user or the first admin.
def self.system_user def self.system_user
user = User.where(username_lower: SiteSetting.system_username).first if SiteSetting.system_username.present? user = User.where(username_lower: SiteSetting.system_username).first if SiteSetting.system_username.present?
user = User.admins.order(:id).first if user.blank? user ||= User.admins.real.order(:id).first
user
end end
def self.store def self.store

View File

@ -40,8 +40,12 @@ class Guardian
def is_developer? def is_developer?
@user && @user &&
is_admin? && is_admin? &&
(Rails.env.development? ||
(
Rails.configuration.respond_to?(:developer_emails) && Rails.configuration.respond_to?(:developer_emails) &&
Rails.configuration.developer_emails.include?(@user.email) Rails.configuration.developer_emails.include?(@user.email)
)
)
end end
# Can the user see the object? # Can the user see the object?
@ -348,8 +352,12 @@ class Guardian
# not secure, or I can see it # not secure, or I can see it
(not(topic.read_restricted_category?) || can_see_category?(topic.category)) && (not(topic.read_restricted_category?) || can_see_category?(topic.category)) &&
# not private, or I am allowed (or an admin) # NOTE
(not(topic.private_message?) || authenticated? && (topic.all_allowed_users.where(id: @user.id).exists? || is_admin?)) # 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
# not private, or I am allowed (or is staff)
(not(topic.private_message?) || authenticated? && (topic.all_allowed_users.where(id: @user.id).exists? || is_staff?))
end end
end end

View File

@ -12,7 +12,8 @@ module Jobs
def target_user_ids def target_user_ids
# Users who want to receive emails and haven't been emailed in the last day # Users who want to receive emails and haven't been emailed in the last day
query = User.where(email_digests: true, active: true) query = User.real
.where(email_digests: true, active: true)
.where("COALESCE(last_emailed_at, '2010-01-01') <= CURRENT_TIMESTAMP - ('1 DAY'::INTERVAL * digest_after_days)") .where("COALESCE(last_emailed_at, '2010-01-01') <= CURRENT_TIMESTAMP - ('1 DAY'::INTERVAL * digest_after_days)")
.where("COALESCE(last_seen_at, '2010-01-01') <= CURRENT_TIMESTAMP - ('1 DAY'::INTERVAL * digest_after_days)") .where("COALESCE(last_seen_at, '2010-01-01') <= CURRENT_TIMESTAMP - ('1 DAY'::INTERVAL * digest_after_days)")

View File

@ -69,6 +69,7 @@ class PostCreator
update_topic_stats update_topic_stats
update_user_counts update_user_counts
publish publish
ensure_in_allowed_users if guardian.is_staff?
@post.advance_draft_sequence @post.advance_draft_sequence
@post.save_reply_relationships @post.save_reply_relationships
end end
@ -104,6 +105,14 @@ class PostCreator
protected protected
def ensure_in_allowed_users
return unless @topic.private_message?
unless @topic.topic_allowed_users.where(user_id: @user.id).exists?
@topic.topic_allowed_users.create!(user_id: @user.id)
end
end
def secure_group_ids(topic) def secure_group_ids(topic)
@secure_group_ids ||= if topic.category && topic.category.read_restricted? @secure_group_ids ||= if topic.category && topic.category.read_restricted?
topic.category.secure_group_ids topic.category.secure_group_ids

View File

@ -2,6 +2,10 @@ require 'spec_helper'
require_dependency 'admin_user_index_query' require_dependency 'admin_user_index_query'
describe AdminUserIndexQuery do describe AdminUserIndexQuery do
def real_users_count(query)
query.find_users_query.where('users.id > 0').count
end
describe "sql order" do describe "sql order" do
it "has default" do it "has default" do
query = ::AdminUserIndexQuery.new({}) query = ::AdminUserIndexQuery.new({})
@ -19,19 +23,20 @@ describe AdminUserIndexQuery do
TrustLevel.levels.each do |key, value| TrustLevel.levels.each do |key, value|
it "#{key} returns no records" do it "#{key} returns no records" do
query = ::AdminUserIndexQuery.new({ query: key.to_s }) query = ::AdminUserIndexQuery.new({ query: key.to_s })
expect(query.find_users.count).to eq(0) expect(real_users_count(query)).to eq(0)
end end
end end
end end
describe "users with trust level" do describe "users with trust level" do
TrustLevel.levels.each do |key, value| TrustLevel.levels.each do |key, value|
it "finds user with trust #{key}" do it "finds user with trust #{key}" do
Fabricate(:user, trust_level: TrustLevel.levels[key]) Fabricate(:user, trust_level: TrustLevel.levels[key])
query = ::AdminUserIndexQuery.new({ query: key.to_s }) query = ::AdminUserIndexQuery.new({ query: key.to_s })
expect(query.find_users.count).to eq(1) expect(real_users_count(query)).to eq(1)
end end
end end
@ -62,7 +67,7 @@ describe AdminUserIndexQuery do
it "finds the admin" do it "finds the admin" do
query = ::AdminUserIndexQuery.new({ query: 'admins' }) query = ::AdminUserIndexQuery.new({ query: 'admins' })
expect(query.find_users.count).to eq(1) expect(real_users_count(query)).to eq(1)
end end
end end
@ -73,7 +78,7 @@ describe AdminUserIndexQuery do
it "finds the moderator" do it "finds the moderator" do
query = ::AdminUserIndexQuery.new({ query: 'moderators' }) query = ::AdminUserIndexQuery.new({ query: 'moderators' })
expect(query.find_users.count).to eq(1) expect(real_users_count(query)).to eq(1)
end end
end end

View File

@ -47,7 +47,6 @@ describe Discourse do
end end
end end
context '#system_user' do context '#system_user' do
let!(:admin) { Fabricate(:admin) } let!(:admin) { Fabricate(:admin) }

File diff suppressed because it is too large Load Diff

View File

@ -310,6 +310,14 @@ describe PostCreator do
# does not notify an unrelated user # does not notify an unrelated user
unrelated.notifications.count.should == 0 unrelated.notifications.count.should == 0
post.topic.subtype.should == TopicSubtype.user_to_user post.topic.subtype.should == TopicSubtype.user_to_user
# if a mod replies they should be added to the allowed user list
mod = Fabricate(:moderator)
PostCreator.create(mod, raw: 'hi there welcome topic, I am a mod',
topic_id: post.topic_id)
post.topic.reload
post.topic.topic_allowed_users.where(user_id: mod.id).count.should == 1
end end
end end

View File

@ -21,6 +21,18 @@ describe Group do
end end
end end
def real_admins
Group[:admins].user_ids - [-1]
end
def real_moderators
Group[:moderators].user_ids - [-1]
end
def real_staff
Group[:staff].user_ids - [-1]
end
it "Can update moderator/staff/admin groups correctly" do it "Can update moderator/staff/admin groups correctly" do
admin = Fabricate(:admin) admin = Fabricate(:admin)
@ -28,33 +40,33 @@ describe Group do
Group.refresh_automatic_groups!(:admins, :staff, :moderators) Group.refresh_automatic_groups!(:admins, :staff, :moderators)
Group[:admins].user_ids.should == [admin.id] real_admins.should == [admin.id]
Group[:moderators].user_ids.should == [moderator.id] real_moderators.should == [moderator.id]
Group[:staff].user_ids.sort.should == [moderator.id,admin.id].sort real_staff.sort.should == [moderator.id,admin.id].sort
admin.admin = false admin.admin = false
admin.save admin.save
Group.refresh_automatic_group!(:admins) Group.refresh_automatic_group!(:admins)
Group[:admins].user_ids.should == [] real_admins.should == []
moderator.revoke_moderation! moderator.revoke_moderation!
admin.grant_admin! admin.grant_admin!
Group[:admins].user_ids.should == [admin.id] real_admins.should == [admin.id]
Group[:staff].user_ids.should == [admin.id] real_staff.should == [admin.id]
admin.revoke_admin! admin.revoke_admin!
Group[:admins].user_ids.should == [] real_admins.should == []
Group[:staff].user_ids.should == [] real_staff.should == []
admin.grant_moderation! admin.grant_moderation!
Group[:moderators].user_ids.should == [admin.id] real_moderators.should == [admin.id]
Group[:staff].user_ids.should == [admin.id] real_staff.should == [admin.id]
admin.revoke_moderation! admin.revoke_moderation!
Group[:admins].user_ids.should == [] real_admins.should == []
Group[:staff].user_ids.should == [] real_staff.should == []
end end
it "Correctly updates automatic trust level groups" do it "Correctly updates automatic trust level groups" do
@ -87,12 +99,12 @@ describe Group do
groups.count.should == Group::AUTO_GROUPS.count groups.count.should == Group::AUTO_GROUPS.count
g = groups.find{|g| g.id == Group::AUTO_GROUPS[:admins]} g = groups.find{|g| g.id == Group::AUTO_GROUPS[:admins]}
g.users.count.should == 1 g.users.count.should == 2
g.user_count.should == 1 g.user_count.should == 2
g = groups.find{|g| g.id == Group::AUTO_GROUPS[:staff]} g = groups.find{|g| g.id == Group::AUTO_GROUPS[:staff]}
g.users.count.should == 1 g.users.count.should == 2
g.user_count.should == 1 g.user_count.should == 2
g = groups.find{|g| g.id == Group::AUTO_GROUPS[:trust_level_2]} g = groups.find{|g| g.id == Group::AUTO_GROUPS[:trust_level_2]}
g.users.count.should == 1 g.users.count.should == 1

View File

@ -41,10 +41,9 @@ describe PostAction do
describe 'notify_moderators' do describe 'notify_moderators' do
before do before do
PostAction.stubs(:create) PostAction.stubs(:create)
PostAction.expects(:target_moderators).returns("moderators")
end end
it "sends an email to all moderators if selected" do it "sends an email to community if selected" do
post = build(:post, id: 1000) post = build(:post, id: 1000)
PostCreator.any_instance.expects(:create).returns(post) PostCreator.any_instance.expects(:create).returns(post)
PostAction.act(build(:user), build(:post), PostActionType.types[:notify_moderators], message: "this is my special message"); PostAction.act(build(:user), build(:post), PostActionType.types[:notify_moderators], message: "this is my special message");