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 @@ + {{#unless automatic}} +
+ +
+ {{/unless}} +
{{combo-box name="alias" valueAttribute="value" value=alias_level content=aliasLevelOptions}} @@ -52,6 +61,13 @@ {{i18n 'admin.groups.automatic_membership_retroactive'}}
+ +
+ + {{input value=title}} +
{{/unless}}
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 = < 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 = <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)