From 65cd00cf25fad45e31add6974cc5863d02092267 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Thu, 2 May 2013 15:15:17 +1000
Subject: [PATCH] moderators now have teeth, more at
 http://meta.discourse.org/t/moderator-permission-set/6307/5 allow pms to be
 targetted at groups

---
 .../controllers/admin_dashboard_controller.js |  3 +-
 .../admin/templates/admin.js.handlebars       | 12 +++--
 .../admin/templates/user.js.handlebars        |  2 +
 app/assets/javascripts/discourse.js           |  2 +-
 .../discourse/controllers/user_controller.js  |  2 +-
 .../discourse/templates/header.js.handlebars  |  2 +-
 .../discourse/templates/topic.js.handlebars   |  2 +-
 .../templates/user/user.js.handlebars         |  2 +-
 app/controllers/admin/admin_controller.rb     |  6 +--
 app/helpers/application_helper.rb             |  4 ++
 app/models/post_action.rb                     |  4 +-
 app/models/post_alert_observer.rb             |  4 +-
 app/models/topic.rb                           | 10 ++++
 app/models/topic_allowed_group.rb             |  7 +++
 app/models/topic_list.rb                      |  4 +-
 app/serializers/current_user_serializer.rb    |  9 +++-
 app/serializers/post_serializer.rb            |  6 +--
 .../common/_discourse_javascript.html.erb     |  2 +-
 .../common/_discourse_stylesheet.html.erb     |  2 +-
 config/routes.rb                              | 32 +++++++------
 ...20130430052751_add_topic_allowed_groups.rb |  1 +
 ...20130501105651_fix_topic_allowed_groups.rb |  6 +++
 lib/guardian.rb                               | 21 ++++----
 lib/moderator_constraint.rb                   | 10 ++++
 lib/post_creator.rb                           | 48 +++++++++++++++----
 spec/components/guardian_spec.rb              |  4 +-
 spec/components/post_creator_spec.rb          | 30 ++++++++++++
 27 files changed, 176 insertions(+), 61 deletions(-)
 create mode 100644 app/models/topic_allowed_group.rb
 create mode 100644 db/migrate/20130501105651_fix_topic_allowed_groups.rb
 create mode 100644 lib/moderator_constraint.rb

diff --git a/app/assets/javascripts/admin/controllers/admin_dashboard_controller.js b/app/assets/javascripts/admin/controllers/admin_dashboard_controller.js
index 2f85858e8c8..96eba0d5b42 100644
--- a/app/assets/javascripts/admin/controllers/admin_dashboard_controller.js
+++ b/app/assets/javascripts/admin/controllers/admin_dashboard_controller.js
@@ -12,10 +12,11 @@ Discourse.AdminDashboardController = Ember.Controller.extend({
   problemsCheckInterval: '1 minute ago',
 
   foundProblems: function() {
-    return(this.get('problems') && this.get('problems').length > 0);
+    return(Discourse.currentUser.admin && this.get('problems') && this.get('problems').length > 0);
   }.property('problems'),
 
   thereWereProblems: function() {
+    if(!Discourse.currentUser.admin) { return false }
     if( this.get('foundProblems') ) {
       this.set('hadProblems', true);
       return true;
diff --git a/app/assets/javascripts/admin/templates/admin.js.handlebars b/app/assets/javascripts/admin/templates/admin.js.handlebars
index 4d66db86c9f..240909f3c52 100644
--- a/app/assets/javascripts/admin/templates/admin.js.handlebars
+++ b/app/assets/javascripts/admin/templates/admin.js.handlebars
@@ -4,14 +4,18 @@
 
       <ul class="nav nav-pills">
         <li>{{#linkTo 'admin.dashboard'}}{{i18n admin.dashboard.title}}{{/linkTo}}</li>
-        <li>{{#linkTo 'admin.site_settings'}}{{i18n admin.site_settings.title}}{{/linkTo}}</li>
-        <li>{{#linkTo 'adminSiteContents'}}{{i18n admin.site_content.title}}{{/linkTo}}</li>
+        {{#if Discourse.currentUser.admin}}
+          <li>{{#linkTo 'admin.site_settings'}}{{i18n admin.site_settings.title}}{{/linkTo}}</li>
+          <li>{{#linkTo 'adminSiteContents'}}{{i18n admin.site_content.title}}{{/linkTo}}</li>
+        {{/if}}
         <li>{{#linkTo 'adminUsersList.active'}}{{i18n admin.users.title}}{{/linkTo}}</li>
         <!--<li>{{#linkTo 'admin.groups'}}{{i18n admin.groups.title}}{{/linkTo}}</li>-->
         <li>{{#linkTo 'admin.email_logs'}}{{i18n admin.email_logs.title}}{{/linkTo}}</li>
         <li>{{#linkTo 'adminFlags.active'}}{{i18n admin.flags.title}}{{/linkTo}}</li>
-        <li>{{#linkTo 'admin.customize'}}{{i18n admin.customize.title}}{{/linkTo}}</li>
-        <li>{{#linkTo 'admin.api'}}{{i18n admin.api.title}}{{/linkTo}}</li>
+        {{#if Discourse.currentUser.admin}}
+          <li>{{#linkTo 'admin.customize'}}{{i18n admin.customize.title}}{{/linkTo}}</li>
+          <li>{{#linkTo 'admin.api'}}{{i18n admin.api.title}}{{/linkTo}}</li>
+        {{/if}}
       </ul>
 
       <div class='boxed white admin-content'>
diff --git a/app/assets/javascripts/admin/templates/user.js.handlebars b/app/assets/javascripts/admin/templates/user.js.handlebars
index 57a132b3551..30dc561e156 100644
--- a/app/assets/javascripts/admin/templates/user.js.handlebars
+++ b/app/assets/javascripts/admin/templates/user.js.handlebars
@@ -31,9 +31,11 @@
     <div class='field'>{{i18n user.ip_address.title}}</div>
     <div class='value'>{{content.ip_address}}</div>
     <div class='controls'>
+      {{#if Discourse.currentUser.admin}}
       <button class='btn' {{action refreshBrowsers target="content"}}>
         {{i18n admin.user.refresh_browsers}}
       </button>
+      {{/if}}
     </div>
   </div>
 
diff --git a/app/assets/javascripts/discourse.js b/app/assets/javascripts/discourse.js
index 55d4dce31aa..9986bd15d03 100644
--- a/app/assets/javascripts/discourse.js
+++ b/app/assets/javascripts/discourse.js
@@ -60,7 +60,7 @@ Discourse = Ember.Application.createWithMixins({
     if (user) {
       bus.callbackInterval = Discourse.SiteSettings.polling_interval;
       bus.enableLongPolling = true;
-      if (user.admin) {
+      if (user.admin || user.moderator) {
         bus.subscribe("/flagged_counts", function(data) {
           user.set('site_flagged_posts_count', data.total);
         });
diff --git a/app/assets/javascripts/discourse/controllers/user_controller.js b/app/assets/javascripts/discourse/controllers/user_controller.js
index 8574c4df77f..2b56d4bfab5 100644
--- a/app/assets/javascripts/discourse/controllers/user_controller.js
+++ b/app/assets/javascripts/discourse/controllers/user_controller.js
@@ -13,7 +13,7 @@ Discourse.UserController = Discourse.ObjectController.extend({
   }).property('content.username', 'Discourse.currentUser.username'),
 
   canSeePrivateMessages: (function() {
-    return this.get('viewingSelf') || Discourse.get('currentUser.admin');
+    return this.get('viewingSelf') || Discourse.get('currentUser.moderator');
   }).property('viewingSelf', 'Discourse.currentUser')
 
 });
diff --git a/app/assets/javascripts/discourse/templates/header.js.handlebars b/app/assets/javascripts/discourse/templates/header.js.handlebars
index 26931aaeef7..6f0f7284abb 100644
--- a/app/assets/javascripts/discourse/templates/header.js.handlebars
+++ b/app/assets/javascripts/discourse/templates/header.js.handlebars
@@ -65,7 +65,7 @@
 
       <section class='d-dropdown' id='site-map-dropdown'>
       <ul>
-        {{#if Discourse.currentUser.admin}}
+        {{#if Discourse.currentUser.moderator}}
           <li><a href="/admin"><i class='icon-cog'></i>{{i18n admin_title}}</a></li>
           <li><a href="/admin/flags/active"><i class='icon-flag'></i>{{i18n flags_title}}</a></li>
         {{/if}}
diff --git a/app/assets/javascripts/discourse/templates/topic.js.handlebars b/app/assets/javascripts/discourse/templates/topic.js.handlebars
index 4afd93ee27a..505dcb95e01 100644
--- a/app/assets/javascripts/discourse/templates/topic.js.handlebars
+++ b/app/assets/javascripts/discourse/templates/topic.js.handlebars
@@ -134,6 +134,6 @@
 {{render share}}
 {{render quoteButton}}
 
-{{#if Discourse.currentUser.admin}}
+{{#if Discourse.currentUser.moderator}}
   {{render topicAdminMenu content}}
 {{/if}}
diff --git a/app/assets/javascripts/discourse/templates/user/user.js.handlebars b/app/assets/javascripts/discourse/templates/user/user.js.handlebars
index eb29b6fe49c..d74d70414f8 100644
--- a/app/assets/javascripts/discourse/templates/user/user.js.handlebars
+++ b/app/assets/javascripts/discourse/templates/user/user.js.handlebars
@@ -7,7 +7,7 @@
       {{#if viewingSelf}}
         <button {{action "logout" target="Discourse"}} class='btn'>{{i18n user.log_out}}</button>
       {{/if}}
-      {{#if Discourse.currentUser.admin}}
+      {{#if Discourse.currentUser.moderator}}
         <a href="{{unbound content.adminPath}}" class='btn'><i class="icon-wrench"></i>&nbsp;{{i18n admin.user.show_admin_profile}}</a>
       {{/if}}
       <ul class="nav nav-pills">
diff --git a/app/controllers/admin/admin_controller.rb b/app/controllers/admin/admin_controller.rb
index 5da5ac7a13e..622ae0af1cd 100644
--- a/app/controllers/admin/admin_controller.rb
+++ b/app/controllers/admin/admin_controller.rb
@@ -1,7 +1,7 @@
 class Admin::AdminController < ApplicationController
 
   before_filter :ensure_logged_in
-  before_filter :ensure_is_admin
+  before_filter :ensure_is_moderator
 
   def index
     render nothing: true
@@ -9,8 +9,8 @@ class Admin::AdminController < ApplicationController
 
   protected
 
-    def ensure_is_admin
-      raise Discourse::InvalidAccess.new unless current_user.admin?
+    def ensure_is_moderator
+      raise Discourse::InvalidAccess.new unless current_user.moderator?
     end
 
 end
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 6da3fdda8c1..1f0dced370d 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -34,6 +34,10 @@ module ApplicationHelper
     current_user.try(:admin?)
   end
 
+  def moderator?
+    current_user.try(:moderator?)
+  end
+
   # Creates open graph and twitter card meta data
   def crawlable_meta_data(opts=nil)
 
diff --git a/app/models/post_action.rb b/app/models/post_action.rb
index 9ef15291259..4b2f2773779 100644
--- a/app/models/post_action.rb
+++ b/app/models/post_action.rb
@@ -25,8 +25,8 @@ class PostAction < ActiveRecord::Base
                                            'topics.deleted_at' => nil).count('DISTINCT posts.id')
 
     $redis.set('posts_flagged_count', posts_flagged_count)
-    admins = User.admins.select(:id).map {|u| u.id}
-    MessageBus.publish('/flagged_counts', { total: posts_flagged_count }, { user_ids: admins })
+    user_ids = User.where("admin = 't' or moderator = 't'").select(:id).map {|u| u.id}
+    MessageBus.publish('/flagged_counts', { total: posts_flagged_count }, { user_ids: user_ids })
   end
 
   def self.flagged_posts_count
diff --git a/app/models/post_alert_observer.rb b/app/models/post_alert_observer.rb
index b377230bf00..e5763c6e3d5 100644
--- a/app/models/post_alert_observer.rb
+++ b/app/models/post_alert_observer.rb
@@ -60,8 +60,8 @@ class PostAlertObserver < ActiveRecord::Observer
   def after_create_post(post)
     if post.topic.private_message?
       # If it's a private message, notify the topic_allowed_users
-      post.topic.topic_allowed_users.reject{ |a| a.user_id == post.user_id }.each do |a|
-        create_notification(a.user, Notification.types[:private_message], post)
+      post.topic.all_allowed_users.reject{ |a| a.id == post.user_id }.each do |a|
+        create_notification(a, Notification.types[:private_message], post)
       end
     else
       # If it's not a private message, notify the users
diff --git a/app/models/topic.rb b/app/models/topic.rb
index fa9e06e47bd..1fa8e1d9398 100644
--- a/app/models/topic.rb
+++ b/app/models/topic.rb
@@ -38,6 +38,10 @@ class Topic < ActiveRecord::Base
   belongs_to :category
   has_many :posts
   has_many :topic_allowed_users
+  has_many :topic_allowed_groups
+
+  has_many :allowed_group_users, through: :allowed_groups, source: :users
+  has_many :allowed_groups, through: :topic_allowed_groups, source: :group
   has_many :allowed_users, through: :topic_allowed_users, source: :user
 
   has_one :hot_topic
@@ -94,6 +98,12 @@ class Topic < ActiveRecord::Base
     end
   end
 
+  # all users (in groups or directly targetted) that are going to get the pm
+  def all_allowed_users
+    # TODO we should probably change this from 3 queries to 1
+    User.where('id in (?)', allowed_users.select('users.id').to_a + allowed_group_users.select('users.id').to_a)
+  end
+
   # Additional rate limits on topics: per day and private messages per day
   def limit_topics_per_day
     RateLimiter.new(user, "topics-per-day:#{Date.today.to_s}", SiteSetting.max_topics_per_day, 1.day.to_i)
diff --git a/app/models/topic_allowed_group.rb b/app/models/topic_allowed_group.rb
new file mode 100644
index 00000000000..e5606596fad
--- /dev/null
+++ b/app/models/topic_allowed_group.rb
@@ -0,0 +1,7 @@
+class TopicAllowedGroup < ActiveRecord::Base
+  belongs_to :topic
+  belongs_to :group
+  attr_accessible :group_id, :user_id
+
+  validates_uniqueness_of :topic_id, scope: :group_id
+end
diff --git a/app/models/topic_list.rb b/app/models/topic_list.rb
index 5087d58494b..1f87bfcb6f3 100644
--- a/app/models/topic_list.rb
+++ b/app/models/topic_list.rb
@@ -51,8 +51,8 @@ class TopicList
 
   def has_rank_details?
 
-    # Only admins can see rank details
-    return false unless @current_user.try(:admin?)
+    # Only moderators can see rank details
+    return false unless @current_user.try(:moderator?)
 
     # Only show them on 'Hot'
     return @filter == :hot
diff --git a/app/serializers/current_user_serializer.rb b/app/serializers/current_user_serializer.rb
index 988525510b1..45f0ef9c11b 100644
--- a/app/serializers/current_user_serializer.rb
+++ b/app/serializers/current_user_serializer.rb
@@ -15,8 +15,15 @@ class CurrentUserSerializer < BasicUserSerializer
 
   # we probably want to move this into site, but that json is cached so hanging it off current user seems okish
 
+  def moderator
+    # TODO we probably want better terminology
+    #
+    # we have admins / moderators and users who are either moderators or admins denoted by moderator?
+    object.moderator?
+  end
+
   def include_site_flagged_posts_count?
-    object.admin
+    object.moderator?
   end
 
   def topic_count
diff --git a/app/serializers/post_serializer.rb b/app/serializers/post_serializer.rb
index d8b62a1f7f9..18f581adb81 100755
--- a/app/serializers/post_serializer.rb
+++ b/app/serializers/post_serializer.rb
@@ -88,7 +88,7 @@ class PostSerializer < ApplicationSerializer
   end
 
   def cooked
-    if object.hidden && !scope.is_admin?
+    if object.hidden && !scope.is_moderator?
       if scope.current_user && object.user_id == scope.current_user.id
         I18n.t('flagging.you_must_edit')
       else
@@ -154,7 +154,7 @@ class PostSerializer < ApplicationSerializer
 
       # The following only applies if you're logged in
       if action_summary[:can_act] && scope.current_user.present?
-        action_summary[:can_clear_flags] = scope.is_admin? && PostActionType.flag_types.values.include?(id)
+        action_summary[:can_clear_flags] = scope.is_moderator? && PostActionType.flag_types.values.include?(id)
       end
 
       if post_actions.present? && post_actions.has_key?(id)
@@ -163,7 +163,7 @@ class PostSerializer < ApplicationSerializer
       end
 
       # anonymize flags
-      if !scope.is_admin? && PostActionType.flag_types.values.include?(id)
+      if !scope.is_moderator? && PostActionType.flag_types.values.include?(id)
         action_summary[:count] = action_summary[:acted] ? 1 : 0
       end
 
diff --git a/app/views/common/_discourse_javascript.html.erb b/app/views/common/_discourse_javascript.html.erb
index f70fbba4912..883d4c6cdb9 100644
--- a/app/views/common/_discourse_javascript.html.erb
+++ b/app/views/common/_discourse_javascript.html.erb
@@ -18,7 +18,7 @@
 <%# load the selected locale before any other scripts %>
 <%= javascript_include_tag "locales/#{I18n.locale}" %>
 <%= javascript_include_tag "application" %>
-<%- if admin? %>
+<%- if moderator? %>
   <%= javascript_include_tag "admin"%>
 <%- end %>
 
diff --git a/app/views/common/_discourse_stylesheet.html.erb b/app/views/common/_discourse_stylesheet.html.erb
index 87b248f024e..cca9c1ebebf 100644
--- a/app/views/common/_discourse_stylesheet.html.erb
+++ b/app/views/common/_discourse_stylesheet.html.erb
@@ -2,7 +2,7 @@
   <%=stylesheet_link_tag "application"%>
 <%- end %>
 
-<%- if admin? %>
+<%- if moderator? %>
   <%= stylesheet_link_tag "admin"%>
 <%-end%>
 <%=SiteCustomization.custom_stylesheet(session[:preview_style])%>
diff --git a/config/routes.rb b/config/routes.rb
index eabab717f49..181cdeb4ae5 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -1,6 +1,7 @@
 require 'sidekiq/web'
 
 require_dependency 'admin_constraint'
+require_dependency 'moderator_constraint'
 require_dependency 'homepage_constraint'
 
 # This used to be User#username_format, but that causes a preload of the User object
@@ -21,13 +22,14 @@ Discourse::Application.routes.draw do
   end
   get 'srv/status' => 'forums#status'
 
-  namespace :admin, constraints: AdminConstraint.new do
+  namespace :admin, constraints: ModeratorConstraint.new do
     get '' => 'admin#index'
 
-    resources :site_settings
+    resources :site_settings, constraints: AdminConstraint.new
+
     get 'reports/:type' => 'reports#show'
 
-    resources :groups
+    resources :groups, constraints: AdminConstraint.new
     resources :users, id: USERNAME_ROUTE_FORMAT do
       collection do
         get 'list/:query' => 'users#index'
@@ -36,35 +38,35 @@ Discourse::Application.routes.draw do
       put 'ban'
       put 'delete_all_posts'
       put 'unban'
-      put 'revoke_admin'
-      put 'grant_admin'
-      put 'revoke_moderation'
-      put 'grant_moderation'
+      put 'revoke_admin', constraints: AdminConstraint.new
+      put 'grant_admin', constraints: AdminConstraint.new
+      put 'revoke_moderation', constraints: AdminConstraint.new
+      put 'grant_moderation', constraints: AdminConstraint.new
       put 'approve'
-      post 'refresh_browsers'
+      post 'refresh_browsers', constraints: AdminConstraint.new
     end
 
-    resources :impersonate
+    resources :impersonate, constraints: AdminConstraint.new
     resources :email_logs do
       collection do
         post 'test'
       end
     end
-    get 'customize' => 'site_customizations#index'
+    get 'customize' => 'site_customizations#index', constraints: AdminConstraint.new
     get 'flags' => 'flags#index'
     get 'flags/:filter' => 'flags#index'
     post 'flags/clear/:id' => 'flags#clear'
-    resources :site_customizations
-    resources :site_contents
-    resources :site_content_types
-    resources :export
+    resources :site_customizations, constraints: AdminConstraint.new
+    resources :site_contents, constraints: AdminConstraint.new
+    resources :site_content_types, constraints: AdminConstraint.new
+    resources :export, constraints: AdminConstraint.new
     get 'version_check' => 'versions#show'
     resources :dashboard, only: [:index] do
       collection do
         get 'problems'
       end
     end
-    resources :api, only: [:index] do
+    resources :api, only: [:index], constraints: AdminConstraint.new do
       collection do
         post 'generate_key'
       end
diff --git a/db/migrate/20130430052751_add_topic_allowed_groups.rb b/db/migrate/20130430052751_add_topic_allowed_groups.rb
index 45de8bd7fec..f4bf13aeef7 100644
--- a/db/migrate/20130430052751_add_topic_allowed_groups.rb
+++ b/db/migrate/20130430052751_add_topic_allowed_groups.rb
@@ -1,6 +1,7 @@
 class AddTopicAllowedGroups < ActiveRecord::Migration
   def change
     create_table :topic_allowed_groups do |t|
+      # oops
       t.integer :group_id, :integer, null: false
       t.integer :topic_id, :integer, null: false
     end
diff --git a/db/migrate/20130501105651_fix_topic_allowed_groups.rb b/db/migrate/20130501105651_fix_topic_allowed_groups.rb
new file mode 100644
index 00000000000..cf12cedff9d
--- /dev/null
+++ b/db/migrate/20130501105651_fix_topic_allowed_groups.rb
@@ -0,0 +1,6 @@
+class FixTopicAllowedGroups < ActiveRecord::Migration
+  def change
+    # big oops
+    remove_column :topic_allowed_groups, :integer
+  end
+end
diff --git a/lib/guardian.rb b/lib/guardian.rb
index 3e4967f1868..7679a6b2daa 100644
--- a/lib/guardian.rb
+++ b/lib/guardian.rb
@@ -207,7 +207,7 @@ class Guardian
   end
 
   def can_see_private_messages?(user_id)
-    return true if is_admin?
+    return true if is_moderator?
     return false if @user.blank?
     @user.id == user_id
   end
@@ -263,7 +263,7 @@ class Guardian
 
   def can_edit_user?(user)
     return true if user == @user
-    @user.admin?
+    @user.moderator?
   end
 
   def can_edit_topic?(topic)
@@ -311,12 +311,12 @@ class Guardian
     return post_action.created_at > SiteSetting.post_undo_action_window_mins.minutes.ago
   end
 
-  def can_send_private_message?(target_user)
-    return false unless User === target_user
+  def can_send_private_message?(target)
+    return false unless User === target || Group === target
     return false if @user.blank?
 
     # Can't send message to yourself
-    return false if @user.id == target_user.id
+    return false if User === target && @user.id == target.id
 
     # Have to be a basic level at least
     return false unless @user.has_trust_level?(:basic)
@@ -336,15 +336,15 @@ class Guardian
     return false unless topic
 
     return true if @user && @user.moderator?
-    return false if topic.deleted_at.present?
+    return false if topic.deleted_at
 
     if topic.category && topic.category.secure
       return false unless @user && can_see_category?(topic.category)
     end
 
     if topic.private_message?
-      return false if @user.blank?
-      return true if topic.allowed_users.include?(@user)
+      return false unless @user
+      return true if topic.all_allowed_users.where(id: @user.id).exists?
       return is_admin?
     end
     true
@@ -375,11 +375,11 @@ class Guardian
   def post_can_act?(post, action_key, opts={})
     return false if @user.blank?
     return false if post.blank?
-    return false if post.topic.archived?
 
     taken = opts[:taken_actions]
     taken = taken.keys if taken
 
+    # we always allow flagging
     if PostActionType.is_flag?(action_key)
       return false unless @user.has_trust_level?(:basic)
 
@@ -390,6 +390,9 @@ class Guardian
       return false if taken && taken.include?(PostActionType.types[action_key])
     end
 
+    # nothing else on archived posts
+    return false if post.topic.archived?
+
     case action_key
     when :like
       return false if post.user == @user
diff --git a/lib/moderator_constraint.rb b/lib/moderator_constraint.rb
new file mode 100644
index 00000000000..86edbc056c6
--- /dev/null
+++ b/lib/moderator_constraint.rb
@@ -0,0 +1,10 @@
+require_dependency 'current_user'
+
+class ModeratorConstraint
+
+  def matches?(request)
+    return false unless request.session[:current_user_id].present?
+    User.where("admin = 't' or moderator = 't'").where(id: request.session[:current_user_id].to_i).exists?
+  end
+
+end
diff --git a/lib/post_creator.rb b/lib/post_creator.rb
index 264a0307a0b..b100b3cb02a 100644
--- a/lib/post_creator.rb
+++ b/lib/post_creator.rb
@@ -56,17 +56,14 @@ class PostCreator
 
           topic.subtype = TopicSubtype.user_to_user unless topic.subtype
 
-          usernames = @opts[:target_usernames].split(',')
-          User.where(username: usernames).each do |u|
-
-            unless guardian.can_send_private_message?(u)
-              topic.errors.add(:archetype, :cant_send_pm)
-              @errors = topic.errors
-              raise ActiveRecord::Rollback.new
-            end
-
-            topic.topic_allowed_users.build(user_id: u.id)
+          unless @opts[:target_usernames].present? || @opts[:target_group_names].present?
+            topic.errors.add(:archetype, :cant_send_pm)
+            @errors = topic.errors
+            raise ActiveRecord::Rollback.new
           end
+
+          add_users(topic,@opts[:target_usernames])
+          add_groups(topic,@opts[:target_group_names])
           topic.topic_allowed_users.build(user_id: @user.id)
         end
 
@@ -148,4 +145,35 @@ class PostCreator
     PostCreator.new(user, opts).create
   end
 
+  protected
+
+  def add_users(topic, usernames)
+    return unless usernames
+    usernames = usernames.split(',')
+    User.where(username: usernames).each do |u|
+
+      unless guardian.can_send_private_message?(u)
+        topic.errors.add(:archetype, :cant_send_pm)
+        @errors = topic.errors
+        raise ActiveRecord::Rollback.new
+      end
+
+      topic.topic_allowed_users.build(user_id: u.id)
+    end
+  end
+
+  def add_groups(topic, groups)
+    return unless groups
+    groups = groups.split(',')
+    Group.where(name: groups).each do |g|
+
+      unless guardian.can_send_private_message?(g)
+        topic.errors.add(:archetype, :cant_send_pm)
+        @errors = topic.errors
+        raise ActiveRecord::Rollback.new
+      end
+
+      topic.topic_allowed_groups.build(group_id: g.id)
+    end
+  end
 end
diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb
index 5637a5ec551..ff9e70875ea 100644
--- a/spec/components/guardian_spec.rb
+++ b/spec/components/guardian_spec.rb
@@ -485,8 +485,8 @@ describe Guardian do
         Guardian.new(user).can_edit?(user).should be_true
       end
 
-      it 'returns false as a moderator' do
-        Guardian.new(moderator).can_edit?(user).should be_false
+      it 'returns true as a moderator' do
+        Guardian.new(moderator).can_edit?(user).should be_true
       end
 
       it 'returns true as an admin' do
diff --git a/spec/components/post_creator_spec.rb b/spec/components/post_creator_spec.rb
index 9a38e29cc11..d159e5f7ebd 100644
--- a/spec/components/post_creator_spec.rb
+++ b/spec/components/post_creator_spec.rb
@@ -190,5 +190,35 @@ describe PostCreator do
     end
   end
 
+  context 'private message to group' do
+    let(:target_user1) { Fabricate(:coding_horror) }
+    let(:target_user2) { Fabricate(:moderator) }
+    let(:group) do
+      g = Fabricate.build(:group)
+      g.add(target_user1)
+      g.add(target_user2)
+      g.save
+      g
+    end
+    let(:unrelated) { Fabricate(:user) }
+    let(:post) do
+      PostCreator.create(user, title: 'hi there welcome to my topic',
+                               raw: "this is my awesome message @#{unrelated.username_lower}",
+                               archetype: Archetype.private_message,
+                               target_group_names: group.name)
+    end
+
+    it 'acts correctly' do
+      post.topic.archetype.should == Archetype.private_message
+      post.topic.topic_allowed_users.count.should == 1
+      post.topic.topic_allowed_groups.count.should == 1
+
+      # does not notify an unrelated user
+      unrelated.notifications.count.should == 0
+      post.topic.subtype.should == TopicSubtype.user_to_user
+      target_user1.notifications.count.should == 1
+      target_user2.notifications.count.should == 1
+    end
+  end
 end