more group progress, UI getting there, controller mostly done

changed it so notify moderators goes to the moderators group
allow admins to grant self moderation and revoke self moderation

app/assets/javascripts/admin/controllers/admin_groups_controller.js

@@ -1,9 +1,9 @@
-Discourse.AdminGroupsController = Ember.ArrayController.extend({
+Discourse.AdminGroupsController = Ember.Controller.extend({
   itemController: 'adminGroup',
 
   edit: function(group){
     this.get('model').select(group);
-    group.loadUsers();
+    group.load();
   },
 
   refreshAutoGroups: function(){
@@ -14,9 +14,31 @@ Discourse.AdminGroupsController = Ember.ArrayController.extend({
       controller.set('model', Discourse.Group.findAll());
       controller.set('refreshingAutoGroups',false);
     });
+  },
+
+  newGroup: function(){
+    var group = Discourse.Group.create();
+    group.set("loaded", true);
+    var model = this.get("model");
+    model.addObject(group);
+    model.select(group);
+  },
+
+  save: function(group){
+    if(!group.get("id")){
+      group.create();
+    } else {
+      group.save();
+    }
+  },
+
+  destroy: function(group){
+    var list = this.get("model");
+    if(group.get("id")){
+      group.destroy().then(function(){
+        list.removeObject(group);
+      });
+    }
   }
 });
 
-Discourse.AdminGroupController = Ember.Controller.extend({
-
-});

app/assets/javascripts/admin/models/flagged_post.js

@@ -26,7 +26,8 @@ Discourse.FlaggedPost = Discourse.Post.extend({
       if (a.message) {
         return r.push({
           user: _this.userLookup[a.user_id],
-          message: a.message
+          message: a.message,
+          permalink: a.permalink
         });
       }
     });

app/assets/javascripts/admin/models/group.js

@@ -1,4 +1,6 @@
 Discourse.Group = Discourse.Model.extend({
+  loaded: false,
+
   userCountDisplay: function(){
     var c = this.get('user_count');
     // don't display zero its ugly
@@ -7,16 +9,19 @@ Discourse.Group = Discourse.Model.extend({
     }
   }.property('user_count'),
 
-  loadUsers: function() {
+  load: function() {
+    var id = this.get('id');
+    if(id && !this.get('loaded')) {
       var group = this;
-
       Discourse.ajax('/admin/groups/' + this.get('id') + '/users').then(function(payload){
         var users = Em.A()
         payload.each(function(user){
           users.addObject(Discourse.User.create(user));
         });
         group.set('users', users)
+        group.set('loaded', true)
       });
+    }
   },
 
   usernames: function() {
@@ -28,7 +33,32 @@ Discourse.Group = Discourse.Model.extend({
       }).join(',')
     }
     return usernames;
-  }.property('users')
+  }.property('users'),
+
+  destroy: function(){
+    var group = this;
+    group.set('disableSave', true);
+
+    return Discourse.ajax("/admin/groups/" + this.get("id"), {type: "DELETE"})
+      .then(function(){
+        group.set('disableSave', false);
+      });
+  },
+
+  create: function(){
+    var group = this;
+    group.set('disableSave', true);
+
+    return Discourse.ajax("/admin/groups", {type: "POST", data: {
+      group: {
+        name: this.get('name'),
+        usernames: this.get('usernames')
+      }
+    }}).then(function(r){
+      group.set('disableSave', false);
+      group.set('id', r.id);
+    });
+  }
 
 });
 

app/assets/javascripts/admin/templates/flags.js.handlebars

@@ -38,7 +38,7 @@
         <tr>
           <td></td>
           <td class='message'>
-            <div>{{#linkTo 'adminUser' user}}{{avatar user imageSize="small"}}{{/linkTo}} {{message}}</div>
+            <div>{{#linkTo 'adminUser' user}}{{avatar user imageSize="small"}}{{/linkTo}} {{message}} <a href="{{unbound permalink}}">{{i18n admin.flags.view_message}}</a></div>
           </td>
           <td></td>
           <td></td>

app/assets/javascripts/admin/templates/groups.js.handlebars

@@ -1,5 +1,5 @@
 <!-- work in progress, please ignore -->
-<div class='row'>
+<div class='row groups'>
   <div class='content-list span6'>
     <h3>{{i18n admin.groups.edit}}</h3>
     <ul>
@@ -9,19 +9,35 @@
         </li>
       {{/each}}
     </ul>
-    <div>
-      <button {{bindAttr disabled="refreshingAutoGroups"}} {{action "refreshAutoGroups"}}>Refresh Automatic Groups</button>
+    <div class='controls'>
+      <button class='btn' {{bindAttr disabled="refreshingAutoGroups"}} {{action "refreshAutoGroups"}}>Refresh</button>
+      <button class='btn' {{action newGroup}}>New</button>
     </div>
   </div>
 
   <div class='content-editor'>
     {{#if model.active}}
+      {{#if model.active.loaded}}
         {{#with model.active}}
+          {{#if automatic}}
             <h3>{{name}}</h3>
-        {{view Discourse.UserSelector id="private-message-users" class="span8" placeholderKey="admin.groups.selector_placeholder" tabindex="1" usernamesBinding="usernames"}}
-        <button {{bindAttr disabled="allowSave"}}>Save</button>
+          {{else}}
+            {{view Discourse.TextField valueBinding="name" placeholderKey="admin.groups.name_placeholder"}}
+          {{/if}}
 
+          {{view Discourse.UserSelector id="group-users" placeholderKey="admin.groups.selector_placeholder" tabindex="1" usernamesBinding="usernames"}}
+          <div class='controls'>
+            <button {{action save this}} {{bindAttr disabled="disableSave"}} class='btn'>{{i18n admin.customize.save}}</button>
+            {{#unless automatic}}
+              {{#if id}}
+                <a {{action destroy this}} class='delete-link'>{{i18n admin.customize.delete}}</a>
+              {{/if}}
+            {{/unless}}
+          </div>
         {{/with}}
+      {{else}}
+        <div class='spinner'>{{i18n loading}}</div>
+      {{/if}}
     {{else}}
       nothing here yet
     {{/if}}

app/assets/javascripts/discourse/models/selectable_array.js

@@ -18,5 +18,13 @@ Discourse.SelectableArray = Em.ArrayProxy.extend({
       }
     });
     this.set("active", selected);
+  },
+  removeObject: function(object) {
+    if(object === this.get("active")){
+      this.set("active", null);
+      Em.set(object, "active", false);
+    }
+
+    this._super(object);
   }
 });

app/assets/javascripts/discourse/templates/topic_summary/private_message.js.handlebars

@@ -1,5 +1,10 @@
 <h3><i class='icon icon-envelope-alt'></i> {{i18n private_message_info.title}}</h3>
 <div class='participants clearfix'>
+  {{#each content.allowed_groups}}
+  <div class='user group'>
+    #{{unbound name}}
+  </div>
+  {{/each}}
   {{#each content.allowed_users}}
   <div class='user'>
     <a href='/users/{{lower username}}'>

app/assets/stylesheets/admin/admin_base.scss

@@ -654,3 +654,12 @@ table {
   }
 
 }
+
+.row.groups {
+  input[type='text'] {
+    width: 500px;
+  }
+  input#group-users {
+    width: 600px;
+  }
+}

app/assets/stylesheets/application/topic.css.scss

@@ -451,7 +451,12 @@ kbd {
 .private_message .participants .user {
   float: left;
   display: block;
+  line-height: 35px;
   margin-right: 15px;
+  margin-top: 8px;
+  &.group {
+    color: black;
+  }
 }
 
 .posts-wrapper .spinner {

app/controllers/admin/flags_controller.rb

@@ -56,20 +56,23 @@ limit 100
       map[p["id"]] = p
     }
 
-    sql = SqlBuilder.new "select a.id, a.user_id, post_action_type_id, a.created_at, post_id, a.message
+    sql = SqlBuilder.new "select a.id, a.user_id, post_action_type_id, a.created_at, post_id, a.message, p.topic_id, t.slug
 from post_actions a
+left join posts p on p.id = related_post_id
+left join topics t on t.id = p.topic_id
 /*where*/
 "
     sql.where("post_action_type_id in (:flag_types)", flag_types: PostActionType.notify_flag_types.values)
     sql.where("post_id in (:posts)", posts: posts.map{|p| p["id"].to_i})
 
     if params[:filter] == 'old'
-      sql.where('deleted_at is not null')
+      sql.where('a.deleted_at is not null')
     else
-      sql.where('deleted_at is null')
+      sql.where('a.deleted_at is null')
     end
 
     sql.exec.each do |action|
+      action["permalink"] = Topic.url(action["topic_id"],action["slug"]) if action["slug"].present?
       p = map[action["post_id"]]
       p[:post_actions] ||= []
       p[:post_actions] << action

app/controllers/admin/groups_controller.rb

@@ -1,7 +1,7 @@
 class Admin::GroupsController < Admin::AdminController
   def index
     groups = Group.order(:name).all
-    render_serialized(groups, AdminGroupSerializer)
+    render_serialized(groups, BasicGroupSerializer)
   end
 
   def refresh_automatic_groups
@@ -11,7 +11,7 @@ class Admin::GroupsController < Admin::AdminController
 
   def users
     group = Group.find(params[:group_id].to_i)
-    render_serialized(group.users, BasicUserSerializer)
+    render_serialized(group.users.limit(100).to_a, BasicUserSerializer)
   end
 
   def update
@@ -28,7 +28,7 @@ class Admin::GroupsController < Admin::AdminController
     group.name = params[:group][:name]
     group.usernames = params[:group][:usernames] if params[:group][:usernames]
     group.save!
-    render_serialized(group, AdminGroupSerializer)
+    render_serialized(group, BasicGroupSerializer)
   end
 
   def destroy

app/models/group.rb

@@ -7,6 +7,8 @@ class Group < ActiveRecord::Base
 
   after_save :destroy_deletions
 
+  validate :name_format_validator
+
   AUTO_GROUPS = {
     :admins => 1,
     :moderators => 2,
@@ -26,8 +28,8 @@ class Group < ActiveRecord::Base
 
     id = AUTO_GROUPS[name]
 
-    unless group = self[name]
-      group = Group.new(name: "", automatic: true)
+    unless group = self.lookup_group(name)
+      group = Group.new(name: name.to_s, automatic: true)
       group.id = id
       group.save!
     end
@@ -61,6 +63,8 @@ class Group < ActiveRecord::Base
 
     # we want to ensure consistency
     Group.reset_counters(group.id, :group_users)
+
+    group
   end
 
   def self.refresh_automatic_groups!(*args)
@@ -73,9 +77,15 @@ class Group < ActiveRecord::Base
   end
 
   def self.[](name)
-    raise ArgumentError, "unknown group" unless id = AUTO_GROUPS[name]
+    unless g = lookup_group(name)
+      g = refresh_automatic_group!(name)
+    end
+    g
+  end
 
-    Group.where(id: id).first
+  def self.lookup_group(name)
+    raise ArgumentError, "unknown group" unless id = AUTO_GROUPS[name]
+    g = Group.where(id: id).first
   end
 
 
@@ -84,7 +94,7 @@ class Group < ActiveRecord::Base
 
     GroupUser.where(group_id: trust_group_ids, user_id: user_id).delete_all
 
-    if group = Group[name]
+    if group = lookup_group(name)
       group.group_users.build(user_id: user_id)
       group.save!
     else
@@ -92,6 +102,7 @@ class Group < ActiveRecord::Base
     end
   end
 
+
   def self.builtin
     Enum.new(:moderators, :admins, :trust_level_1, :trust_level_2)
   end
@@ -131,9 +142,15 @@ class Group < ActiveRecord::Base
   def add(user)
     self.users.push(user)
   end
-
   protected
 
+  def name_format_validator
+    validator = UsernameValidator.new(name)
+    unless validator.valid_format?
+      validator.errors.each { |e| errors.add(:name, e) }
+    end
+  end
+
   # hack around AR
   def destroy_deletions
     if @deletions

app/models/post_action.rb

@@ -74,12 +74,16 @@ class PostAction < ActiveRecord::Base
 
   def self.act(user, post, post_action_type_id, message = nil)
     begin
-      title, target_usernames, subtype, body = nil
+      title, target_usernames, target_group_names, subtype, body = nil
 
       if message
         [:notify_moderators, :notify_user].each do |k|
           if post_action_type_id == PostActionType.types[k]
-            target_usernames = k == :notify_moderators ? target_moderators(user) : post.user.username
+            if k == :notify_moderators
+              target_group_names = target_moderators
+            else
+              target_usernames = post.user.username
+            end
             title = I18n.t("post_action_types.#{k}.email_title",
                             title: post.topic.title)
             body = I18n.t("post_action_types.#{k}.email_body",
@@ -91,9 +95,10 @@ class PostAction < ActiveRecord::Base
       end
 
       related_post_id = nil
-      if target_usernames.present?
+      if target_usernames.present? || target_group_names.present?
         related_post_id = PostCreator.new(user,
                               target_usernames: target_usernames,
+                              target_group_names: target_group_names,
                               archetype: Archetype.private_message,
                               subtype: subtype,
                               title: title,
@@ -209,13 +214,8 @@ class PostAction < ActiveRecord::Base
 
   protected
 
-  def self.target_moderators(me)
-    User
-      .where("moderator = 't' or admin = 't'")
-      .where('id <> ?', [me.id])
-      .select('username')
-      .map{|u| u.username}
-      .join(',')
+  def self.target_moderators
+    Group[:moderators].name
   end
 
 end

app/models/topic.rb

@@ -641,9 +641,16 @@ class Topic < ActiveRecord::Base
     "/t/#{slug}/#{id}/#{posts_count}"
   end
 
+
+  def self.url(id, slug, post_number=nil)
+    url = "#{Discourse.base_url}/t/#{slug}/#{id}"
+    url << "/#{post_number}" if post_number.to_i > 1
+    url
+  end
+
   def relative_url(post_number=nil)
     url = "/t/#{slug}/#{id}"
-    url << "/#{post_number}" if post_number.present? && post_number.to_i > 1
+    url << "/#{post_number}" if post_number.to_i > 1
     url
   end
 

app/serializers/basic_group_serializer.rb

@@ -1,3 +1,3 @@
-class AdminGroupSerializer < ApplicationSerializer
+class BasicGroupSerializer < ApplicationSerializer
   attributes :id, :automatic, :name, :user_count
 end

app/serializers/topic_view_serializer.rb

@@ -50,6 +50,7 @@ class TopicViewSerializer < ApplicationSerializer
   has_one :created_by, serializer: BasicUserSerializer, embed: :objects
   has_one :last_poster, serializer: BasicUserSerializer, embed: :objects
   has_many :allowed_users, serializer: BasicUserSerializer, embed: :objects
+  has_many :allowed_groups, serializer: BasicGroupSerializer, embed: :objects
 
   has_many :links, serializer: TopicLinkSerializer, embed: :objects
   has_many :participants, serializer: TopicPostCountSerializer, embed: :objects
@@ -172,6 +173,10 @@ class TopicViewSerializer < ApplicationSerializer
     object.topic.allowed_users
   end
 
+  def allowed_groups
+    object.topic.allowed_groups
+  end
+
   def include_links?
     object.links.present?
   end

config/locales/client.en.yml

@@ -895,11 +895,13 @@ en:
         delete_title: "delete post (if its the first post delete topic)"
         flagged_by: "Flagged by"
         error: "Something went wrong"
+        view_message: "view message"
 
       groups:
         title: "Groups"
         edit: "Edit Groups"
         selector_placeholder: "add users"
+        name_placeholder: "Group name, no spaces, same as username rule"
 
       api:
         title: "API"

db/migrate/20130509040248_update_sequence_for_groups.rb

@@ -0,0 +1,11 @@
+class UpdateSequenceForGroups < ActiveRecord::Migration
+  def up
+    # even if you alter a sequence you still need to set the seq
+    execute <<SQL
+    SELECT setval('groups_id_seq', 40)
+SQL
+  end
+
+  def down
+  end
+end

db/migrate/20130509041351_add_unique_name_to_groups.rb

@@ -0,0 +1,5 @@
+class AddUniqueNameToGroups < ActiveRecord::Migration
+  def change
+    add_index :groups, [:name], unique: true
+  end
+end

lib/guardian.rb

@@ -141,15 +141,16 @@ class Guardian
   def can_revoke_moderation?(moderator)
     return false unless is_admin?
     return false if moderator.blank?
-    return false if @user.id == moderator.id
+    return false if @user.id == moderator.id && !is_admin?
+    return false unless moderator.moderator?
     true
   end
 
   def can_grant_moderation?(user)
     return false unless is_admin?
-    return false if user.blank?
-    return false if @user.id == user.id
-    return false if user.staff?
+    return false unless user
+    return false if @user.id == user.id && !is_admin?
+    return false if user.moderator?
     true
   end
 

lib/post_creator.rb

@@ -21,6 +21,7 @@ class PostCreator
   #     archetype             - Topic archetype
   #     category              - Category to assign to topic
   #     target_usernames      - comma delimited list of usernames for membership (private message)
+  #     target_group_names    - comma delimited list of groups for membership (private message)
   #     meta_data             - Topic meta data hash
   def initialize(user, opts)
     # TODO: we should reload user in case it is tainted, should take in a user_id as opposed to user

spec/fabricators/group_fabricator.rb

@@ -1,3 +1,3 @@
 Fabricator(:group) do
-  name 'my group'
+  name 'my_group'
 end

spec/models/group_spec.rb

@@ -2,7 +2,22 @@ require 'spec_helper'
 
 describe Group do
 
+  describe "validation" do
+    let(:group) { build(:group) }
+
+    it "is invalid for blank" do
+      group.name = ""
+      group.valid?.should be_false
+    end
+
+    it "is valid for a longer name" do
+      group.name = "this_is_a_name"
+      group.valid?.should be_true
+    end
+  end
+
   it "Can update moderator/staff/admin groups correctly" do
+
     admin = Fabricate(:admin)
     moderator = Fabricate(:moderator)
 

spec/models/post_action_spec.rb

@@ -37,7 +37,7 @@ describe PostAction do
     describe 'notify_moderators' do
       before do
         PostAction.stubs(:create)
-        PostAction.expects(:target_moderators).returns("bob")
+        PostAction.expects(:target_moderators).returns("moderators")
       end
 
       it "sends an email to all moderators if selected" do