From 75890aed2631ed1858d8d6c756b81d7edb735542 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Fri, 10 Apr 2015 12:17:28 +1000
Subject: [PATCH] FEATURE: allow admins to choose a group as a primary group
 FEATURE: allow admins to set a default title for a group

---
 .../javascripts/admin/templates/group.hbs     | 16 +++++
 .../javascripts/discourse/models/group.js.es6 |  4 +-
 app/controllers/admin/groups_controller.rb    | 25 +++----
 app/models/group.rb                           | 55 +++++++++++++++
 app/models/group_user.rb                      | 43 ++++++++++++
 app/serializers/basic_group_serializer.rb     |  4 +-
 config/locales/client.en.yml                  |  2 +
 ...50410002033_add_primary_group_to_groups.rb |  5 ++
 .../20150410002551_add_title_to_groups.rb     |  5 ++
 .../admin/groups_controller_spec.rb           | 12 ++--
 spec/models/group_spec.rb                     | 68 ++++++++++++++++++-
 11 files changed, 220 insertions(+), 19 deletions(-)
 create mode 100644 db/migrate/20150410002033_add_primary_group_to_groups.rb
 create mode 100644 db/migrate/20150410002551_add_title_to_groups.rb

diff --git a/app/assets/javascripts/admin/templates/group.hbs b/app/assets/javascripts/admin/templates/group.hbs
index 2800ac2b9c1..d17167a66f5 100644
--- a/app/assets/javascripts/admin/templates/group.hbs
+++ b/app/assets/javascripts/admin/templates/group.hbs
@@ -38,6 +38,15 @@
     </label>
   </div>
 
+  {{#unless automatic}}
+  <div>
+    <label for="primary_group">
+      {{input type="checkbox" checked=primary_group}}
+      {{i18n 'admin.groups.primary_group'}}
+    </label>
+  </div>
+  {{/unless}}
+
   <div>
     <label for="alias">{{i18n 'groups.alias_levels.title'}}</label>
     {{combo-box name="alias" valueAttribute="value" value=alias_level content=aliasLevelOptions}}
@@ -52,6 +61,13 @@
         {{i18n 'admin.groups.automatic_membership_retroactive'}}
       </label>
     </div>
+
+    <div>
+      <label for="title">
+        {{i18n 'admin.groups.default_title'}}
+      </label>
+        {{input value=title}}
+    </div>
   {{/unless}}
 
   <div class='buttons'>
diff --git a/app/assets/javascripts/discourse/models/group.js.es6 b/app/assets/javascripts/discourse/models/group.js.es6
index eaa58e8e0cb..6f426073aa7 100644
--- a/app/assets/javascripts/discourse/models/group.js.es6
+++ b/app/assets/javascripts/discourse/models/group.js.es6
@@ -61,7 +61,9 @@ const Group = Discourse.Model.extend({
       alias_level: this.get('alias_level'),
       visible: !!this.get('visible'),
       automatic_membership_email_domains: this.get('emailDomains'),
-      automatic_membership_retroactive: !!this.get('automatic_membership_retroactive')
+      automatic_membership_retroactive: !!this.get('automatic_membership_retroactive'),
+      title: this.get('title'),
+      primary_group: !!this.get('primary_group')
     };
   },
 
diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb
index 5389b578d5a..69288921f46 100644
--- a/app/controllers/admin/groups_controller.rb
+++ b/app/controllers/admin/groups_controller.rb
@@ -23,16 +23,7 @@ class Admin::GroupsController < Admin::AdminController
     group = Group.new
 
     group.name = (params[:name] || '').strip
-    group.alias_level = params[:alias_level].to_i if params[:alias_level].present?
-    group.visible = params[:visible] == "true"
-    group.automatic_membership_email_domains = params[:automatic_membership_email_domains]
-    group.automatic_membership_retroactive = params[:automatic_membership_retroactive] == "true"
-
-    if group.save
-      render_serialized(group, BasicGroupSerializer)
-    else
-      render_json_error group
-    end
+    save_group(group)
   end
 
   def update
@@ -40,10 +31,20 @@ class Admin::GroupsController < Admin::AdminController
 
     # group rename is ignored for automatic groups
     group.name = params[:name] if params[:name] && !group.automatic
+    save_group(group)
+  end
+
+  def save_group(group)
     group.alias_level = params[:alias_level].to_i if params[:alias_level].present?
     group.visible = params[:visible] == "true"
-    group.automatic_membership_email_domains = params[:automatic_membership_email_domains]
-    group.automatic_membership_retroactive = params[:automatic_membership_retroactive] == "true"
+
+    group.automatic_membership_email_domains = params[:automatic_membership_email_domains] unless group.automatic
+    group.automatic_membership_retroactive = params[:automatic_membership_retroactive] == "true" unless group.automatic
+
+    group.primary_group = group.automatic ? false : params["primary_group"] == "true"
+
+    title = params[:title] if params[:title].present?
+    group.title = group.automatic ? nil : title
 
     if group.save
       render_serialized(group, BasicGroupSerializer)
diff --git a/app/models/group.rb b/app/models/group.rb
index c8329eb5c2f..8d1e38f42a0 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -12,6 +12,8 @@ class Group < ActiveRecord::Base
 
   after_save :destroy_deletions
   after_save :automatic_group_membership
+  after_save :update_primary_group
+  after_save :update_title
 
   validate :name_format_validator
   validates_uniqueness_of :name, case_sensitive: false
@@ -301,6 +303,59 @@ class Group < ActiveRecord::Base
       end
     end
 
+    def update_title
+      return if new_record? && !self.title.present?
+
+      if self.title_changed?
+        sql = <<SQL
+        UPDATE users SET title = :title
+        WHERE (title = :title_was OR
+              title = '' OR
+              title IS NULL) AND
+              COALESCE(title,'') <> COALESCE(:title,'') AND
+              id IN (
+                SELECT user_id
+                FROM group_users
+                WHERE group_id = :id
+              )
+SQL
+
+        self.class.exec_sql(sql,
+              title: title,
+              title_was: title_was,
+              id: id
+        )
+      end
+    end
+
+    def update_primary_group
+      return if new_record? && !self.primary_group?
+
+      if self.primary_group_changed?
+        sql = <<SQL
+        UPDATE users
+        /*set*/
+        /*where*/
+SQL
+
+        builder = SqlBuilder.new(sql)
+        builder.where("
+              id IN (
+                SELECT user_id
+                FROM group_users
+                WHERE group_id = :id
+              )", id: id)
+
+        if primary_group
+          builder.set("primary_group_id = :id")
+        else
+          builder.set("primary_group_id = NULL")
+          builder.where("primary_group_id = :id")
+        end
+
+        builder.exec
+      end
+    end
 end
 
 # == Schema Information
diff --git a/app/models/group_user.rb b/app/models/group_user.rb
index 53ace2df1fd..38351f52b1e 100644
--- a/app/models/group_user.rb
+++ b/app/models/group_user.rb
@@ -1,6 +1,49 @@
 class GroupUser < ActiveRecord::Base
   belongs_to :group, counter_cache: "user_count"
   belongs_to :user
+
+  after_save :update_title
+  after_destroy :remove_title
+
+  after_save :set_primary_group
+  after_destroy :remove_primary_group
+
+  protected
+
+  def set_primary_group
+    if group.primary_group
+        self.class.exec_sql("UPDATE users
+                             SET primary_group_id = :id
+                             WHERE id = :user_id",
+                          id: group.id, user_id: user_id)
+    end
+  end
+
+  def remove_primary_group
+      self.class.exec_sql("UPDATE users
+                           SET primary_group_id = NULL
+                           WHERE id = :user_id AND primary_group_id = :id",
+                        id: group.id, user_id: user_id)
+
+  end
+
+  def remove_title
+    if group.title.present?
+        self.class.exec_sql("UPDATE users SET title = NULL
+                          WHERE title = :title AND id = :id",
+                          id: user_id,
+                          title: group.title)
+    end
+  end
+
+  def update_title
+    if group.title.present?
+      self.class.exec_sql("UPDATE users SET title = :title
+                          WHERE (title IS NULL OR title = '') AND id = :id",
+                          id: user_id,
+                          title: group.title)
+    end
+  end
 end
 
 # == Schema Information
diff --git a/app/serializers/basic_group_serializer.rb b/app/serializers/basic_group_serializer.rb
index 0e22994212c..9f6ad6b7a2f 100644
--- a/app/serializers/basic_group_serializer.rb
+++ b/app/serializers/basic_group_serializer.rb
@@ -6,5 +6,7 @@ class BasicGroupSerializer < ApplicationSerializer
              :alias_level,
              :visible,
              :automatic_membership_email_domains,
-             :automatic_membership_retroactive
+             :automatic_membership_retroactive,
+             :primary_group,
+             :title
 end
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 87747847c34..c2d95b258ef 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -1728,6 +1728,8 @@ en:
         automatic: "Automatic"
         automatic_membership_email_domains: "Users who register with an email domain that exactly matches one in this list will be automatically added to this group:"
         automatic_membership_retroactive: "Apply the same email domain rule to add existing registered users"
+        default_title: "Default title for all users in this group"
+        primary_group: "Automatically set as primary group"
 
       api:
         generate_master: "Generate Master API Key"
diff --git a/db/migrate/20150410002033_add_primary_group_to_groups.rb b/db/migrate/20150410002033_add_primary_group_to_groups.rb
new file mode 100644
index 00000000000..23e4e87138e
--- /dev/null
+++ b/db/migrate/20150410002033_add_primary_group_to_groups.rb
@@ -0,0 +1,5 @@
+class AddPrimaryGroupToGroups < ActiveRecord::Migration
+  def change
+    add_column :groups, :primary_group, :boolean, default: false, null: false
+  end
+end
diff --git a/db/migrate/20150410002551_add_title_to_groups.rb b/db/migrate/20150410002551_add_title_to_groups.rb
new file mode 100644
index 00000000000..eb90e98efa5
--- /dev/null
+++ b/db/migrate/20150410002551_add_title_to_groups.rb
@@ -0,0 +1,5 @@
+class AddTitleToGroups < ActiveRecord::Migration
+  def change
+    add_column :groups, :title, :string
+  end
+end
diff --git a/spec/controllers/admin/groups_controller_spec.rb b/spec/controllers/admin/groups_controller_spec.rb
index 2e362fd23ad..b8c543f4cc7 100644
--- a/spec/controllers/admin/groups_controller_spec.rb
+++ b/spec/controllers/admin/groups_controller_spec.rb
@@ -27,7 +27,9 @@ describe Admin::GroupsController do
         "alias_level"=>0,
         "visible"=>true,
         "automatic_membership_email_domains"=>nil,
-        "automatic_membership_retroactive"=>false
+        "automatic_membership_retroactive"=>false,
+        "title"=>nil,
+        "primary_group"=>false
       }])
     end
 
@@ -61,13 +63,15 @@ describe Admin::GroupsController do
 
     it "doesn't launch the 'automatic group membership' job when it's not retroactive" do
       Jobs.expects(:enqueue).never
-      xhr :put, :update, id: 1, automatic_membership_retroactive: "false"
+      group = Fabricate(:group)
+      xhr :put, :update, id: group.id, automatic_membership_retroactive: "false"
       expect(response).to be_success
     end
 
     it "launches the 'automatic group membership' job when it's retroactive" do
-      Jobs.expects(:enqueue).with(:automatic_group_membership, group_id: 1)
-      xhr :put, :update, id: 1, automatic_membership_retroactive: "true"
+      group = Fabricate(:group)
+      Jobs.expects(:enqueue).with(:automatic_group_membership, group_id: group.id)
+      xhr :put, :update, id: group.id, automatic_membership_retroactive: "true"
       expect(response).to be_success
     end
 
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index f5fc34e9424..6941e90cf92 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -47,7 +47,73 @@ describe Group do
     Group[:staff].user_ids - [-1]
   end
 
-  it "Correctly handles primary group" do
+  it "Correctly handles primary groups" do
+    group = Fabricate(:group, primary_group: true)
+    user = Fabricate(:user)
+
+    group.add(user)
+
+    user.reload
+    expect(user.primary_group_id).to eq group.id
+
+    group.remove(user)
+
+    user.reload
+    expect(user.primary_group_id).to eq nil
+
+    group.add(user)
+    group.primary_group = false
+    group.save
+
+    user.reload
+    expect(user.primary_group_id).to eq nil
+
+  end
+
+  it "Correctly handles title" do
+
+    group = Fabricate(:group, title: 'Super Awesome')
+    user = Fabricate(:user)
+
+    expect(user.title).to eq nil
+
+    group.add(user)
+    user.reload
+
+    expect(user.title).to eq 'Super Awesome'
+
+    group.title = 'BOOM'
+    group.save
+
+    user.reload
+    expect(user.title).to eq 'BOOM'
+
+    group.title = nil
+    group.save
+
+    user.reload
+    expect(user.title).to eq nil
+
+    group.title = "BOB"
+    group.save
+
+    user.reload
+    expect(user.title).to eq "BOB"
+
+    group.remove(user)
+
+    user.reload
+    expect(user.title).to eq nil
+
+    group.add(user)
+    group.destroy
+
+    user.reload
+    expect(user.title).to eq nil
+
+  end
+
+  it "Correctly handles removal of primary group" do
     group = Fabricate(:group)
     user = Fabricate(:user)
     group.add(user)