moderators now have teeth, more at http://meta.discourse.org/t/moderator-permission-set/6307/5

SamSaffron · SamSaffron · commit 65cd00cf25fa · 2013-05-02T15:15:53.000+10:00
allow pms to be targetted at groups
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -1,16 +1,16 @@
 class Admin::AdminController < ApplicationController
 
   before_filter :ensure_logged_in
-  before_filter :ensure_is_admin
+  before_filter :ensure_is_moderator
 
   def index
     render nothing: true
   end
 
   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
@@ -34,6 +34,10 @@ def admin?
     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
@@ -25,8 +25,8 @@ def self.update_flagged_posts_count
                                            '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
@@ -60,8 +60,8 @@ def after_create_version(version)
   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
@@ -38,6 +38,10 @@ def self.featured_users_count
   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 @@ def initialize(user)
     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
@@ -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
@@ -51,8 +51,8 @@ def attributes
 
   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
@@ -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
@@ -88,7 +88,7 @@ def link_counts
   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 @@ def actions_summary
 
       # 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 @@ def actions_summary
       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
@@ -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
@@ -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
@@ -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 @@
   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 @@
       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
@@ -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
@@ -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
diff --git a/lib/moderator_constraint.rb b/lib/moderator_constraint.rb
diff --git a/lib/post_creator.rb b/lib/post_creator.rb
diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb
diff --git a/spec/components/post_creator_spec.rb b/spec/components/post_creator_spec.rb
