group progress, never email banned users

app/controllers/admin/groups_controller.rb

@@ -9,11 +9,32 @@ class Admin::GroupsController < Admin::AdminController
     render json: "ok"
   end
 
-  def show
-  end
-
   def users
     group = Group.find(params[:group_id].to_i)
     render_serialized(group.users, BasicUserSerializer)
   end
+
+  def update
+    group = Group.find(params[:id].to_i)
+    render_json_error if group.automatic
+    group.usernames = params[:group][:usernames]
+    group.name = params[:group][:name] if params[:name]
+    group.save!
+    render json: "ok"
+  end
+
+  def create
+    group = Group.new
+    group.name = params[:group][:name]
+    group.usernames = params[:group][:usernames] if params[:group][:usernames]
+    group.save!
+    render_serialized(group, AdminGroupSerializer)
+  end
+
+  def destroy
+    group = Group.find(params[:id].to_i)
+    render_json_error if group.automatic
+    group.destroy
+    render json: "ok"
+  end
 end

app/models/group.rb

@@ -1,10 +1,12 @@
 class Group < ActiveRecord::Base
   has_many :category_groups
-  has_many :group_users
+  has_many :group_users, dependent: :destroy
 
   has_many :categories, through: :category_groups
   has_many :users, through: :group_users
 
+  after_save :destroy_deletions
+
   AUTO_GROUPS = {
     :admins => 1,
     :moderators => 2,
@@ -90,15 +92,56 @@ class Group < ActiveRecord::Base
     end
   end
 
-  def user_ids
-    users.select('users.id').map(&:id)
-  end
-
   def self.builtin
     Enum.new(:moderators, :admins, :trust_level_1, :trust_level_2)
   end
 
+  def usernames=(val)
+    current = usernames.split(",")
+    expected = val.split(",")
+
+    additions = expected - current
+    deletions = current - expected
+
+    map = Hash[*User.where(username: additions+deletions)
+                 .select('id,username')
+                 .map{|u| [u.username,u.id]}.flatten]
+
+    deletions = Set.new(deletions.map{|d| map[d]})
+
+    @deletions = []
+    group_users.delete_if do |gu|
+      @deletions << gu if deletions.include?(gu.user_id)
+    end
+
+    additions.each do |a|
+      group_users.build(user_id: map[a])
+    end
+
+  end
+
+  def usernames
+    users.select("username").map(&:username).join(",")
+  end
+
+  def user_ids
+    users.select('users.id').map(&:id)
+  end
+
   def add(user)
     self.users.push(user)
   end
+
+  protected
+
+  # hack around AR
+  def destroy_deletions
+    if @deletions
+      @deletions.each do |gu|
+        gu.destroy
+      end
+    end
+    @deletions = nil
+  end
+
 end

lib/jobs/user_email.rb

@@ -13,7 +13,7 @@ module Jobs
 
       # Find the user
       user = User.where(id: args[:user_id]).first
-      return unless user.present?
+      return if !user || user.is_banned?
 
       seen_recently = (user.last_seen_at.present? && user.last_seen_at > SiteSetting.email_time_window_mins.minutes.ago)
 

spec/controllers/admin/groups_controller_spec.rb

@@ -28,4 +28,49 @@ describe Admin::GroupsController do
     xhr :post, :refresh_automatic_groups
     response.status.should == 200
   end
+
+  it "is able to destroy a group" do
+    log_in(:admin)
+    group = Fabricate(:group)
+
+    xhr :delete, :destroy, id: group.id
+    response.status.should == 200
+
+    Group.count.should == 0
+  end
+
+  it "is able to create a group" do
+    a = log_in(:admin)
+
+    xhr :post, :create, group: {
+      usernames: a.username,
+      name: "bob"
+    }
+
+    response.status.should == 200
+
+    groups = Group.all.to_a
+
+    groups.count.should == 1
+    groups[0].usernames.should == a.username
+    groups[0].name.should == "bob"
+
+  end
+
+  it "is able to update group members" do
+    user1 = Fabricate(:user)
+    user2 = Fabricate(:user)
+    group = Fabricate(:group)
+    log_in(:admin)
+
+    xhr :put, :update, id: group.id, name: 'fred', group: {
+        name: 'fred',
+        usernames: "#{user1.username},#{user2.username}"
+    }
+
+    group.reload
+    group.users.count.should == 2
+    group.name.should == 'fred'
+
+  end
 end

spec/models/group_spec.rb

@@ -80,4 +80,38 @@ describe Group do
 
   end
 
+  it "can set members via usernames helper" do
+    g = Fabricate(:group)
+    u1 = Fabricate(:user)
+    u2 = Fabricate(:user)
+    u3 = Fabricate(:user)
+
+    g.add(u1)
+    g.save!
+
+    usernames = "#{u2.username},#{u3.username}"
+
+    # no side effects please
+    g.usernames = usernames
+    g.reload
+    g.users.count.should == 1
+
+    g.usernames = usernames
+    g.save!
+
+    g.usernames.split(",").sort.should == usernames.split(",").sort
+  end
+
+  it "correctly destroys groups" do
+    g = Fabricate(:group)
+    u1 = Fabricate(:user)
+    g.add(u1)
+    g.save!
+
+    g.destroy
+
+    User.where(id: u1.id).count.should == 1
+    GroupUser.count.should == 0
+  end
+
 end