From 256519dddfb97743b71f9295708769edfbaf0176 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Fri, 23 Jan 2015 18:25:43 +0100 Subject: [PATCH] FEATURE: automatic group membership based on email address --- .../javascripts/admin/templates/group.hbs | 11 +++++++++ .../javascripts/discourse/models/group.js | 4 +++- .../stylesheets/common/admin/admin_base.scss | 8 +++++++ app/controllers/admin/groups_controller.rb | 7 +++++- .../regular/automatic_group_membership.rb | 23 +++++++++++++++++++ app/models/group.rb | 7 ++++++ app/models/user.rb | 12 ++++++++++ app/serializers/basic_group_serializer.rb | 13 ++++++++++- config/locales/client.en.yml | 2 ++ ...45128_add_automatic_membership_to_group.rb | 6 +++++ .../admin/groups_controller_spec.rb | 16 ++++++++++++- spec/jobs/automatic_group_membership_spec.rb | 22 ++++++++++++++++++ spec/models/user_spec.rb | 11 +++++++++ 13 files changed, 138 insertions(+), 4 deletions(-) create mode 100644 app/jobs/regular/automatic_group_membership.rb create mode 100644 db/migrate/20150123145128_add_automatic_membership_to_group.rb create mode 100644 spec/jobs/automatic_group_membership_spec.rb diff --git a/app/assets/javascripts/admin/templates/group.hbs b/app/assets/javascripts/admin/templates/group.hbs index 03d9d697e67..ac76d7056ad 100644 --- a/app/assets/javascripts/admin/templates/group.hbs +++ b/app/assets/javascripts/admin/templates/group.hbs @@ -43,6 +43,17 @@ {{combo-box name="alias" valueAttribute="value" value=alias_level content=aliasLevelOptions}} + {{#unless automatic}} +
+ + {{list-setting name="automatic_membership" settingValue=automatic_membership_email_domains}} + +
+ {{/unless}} +
{{#unless automatic}} diff --git a/app/assets/javascripts/discourse/models/group.js b/app/assets/javascripts/discourse/models/group.js index c492cce5455..a598b9822b4 100644 --- a/app/assets/javascripts/discourse/models/group.js +++ b/app/assets/javascripts/discourse/models/group.js @@ -68,7 +68,9 @@ Discourse.Group = Discourse.Model.extend({ return { name: this.get('name'), alias_level: this.get('alias_level'), - visible: !!this.get('visible') + visible: !!this.get('visible'), + automatic_membership_email_domains: this.get('automatic_membership_email_domains'), + automatic_membership_retroactive: !!this.get('automatic_membership_retroactive') }; }, diff --git a/app/assets/stylesheets/common/admin/admin_base.scss b/app/assets/stylesheets/common/admin/admin_base.scss index dca9a5403bf..33ce6d2ef9a 100644 --- a/app/assets/stylesheets/common/admin/admin_base.scss +++ b/app/assets/stylesheets/common/admin/admin_base.scss @@ -463,6 +463,7 @@ section.details { .groups { .ac-wrap { width: 100% !important; + border-color: scale-color($primary, $lightness: 75%); .item { width: 190px; margin-right: 0 !important; @@ -480,6 +481,13 @@ section.details { .controls { margin-top: 10px; } + .select2-container { + width: 100%; + } + .select2-choices { + width: 100%; + border-color: scale-color($primary, $lightness: 75%); + } } // Customise area diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index 0cdf0413691..65b737392ea 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/app/controllers/admin/groups_controller.rb @@ -21,8 +21,11 @@ class Admin::GroupsController < Admin::AdminController def create group = Group.new + group.name = (params[:name] || '').strip 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) @@ -36,11 +39,13 @@ class Admin::GroupsController < Admin::AdminController 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 rename is ignored for automatic groups group.name = params[:name] if params[:name] && !group.automatic if group.save - render json: success_json + render_serialized(group, BasicGroupSerializer) else render_json_error group end diff --git a/app/jobs/regular/automatic_group_membership.rb b/app/jobs/regular/automatic_group_membership.rb new file mode 100644 index 00000000000..d49d5b37989 --- /dev/null +++ b/app/jobs/regular/automatic_group_membership.rb @@ -0,0 +1,23 @@ +module Jobs + + class AutomaticGroupMembership < Jobs::Base + + def execute(args) + group_id = args[:group_id] + + raise Discourse::InvalidParameters.new(:group_id) if group_id.blank? + + group = Group.find(group_id) + + return unless group.automatic_membership_retroactive + + domains = group.automatic_membership_email_domains.gsub('.', '\.') + + User.where("email ~* '@(#{domains})'").find_each do |user| + group.add(user) rescue ActiveRecord::RecordNotUnique + end + end + + end + +end diff --git a/app/models/group.rb b/app/models/group.rb index d9985367df1..33ed718aed6 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -11,6 +11,7 @@ class Group < ActiveRecord::Base has_many :managers, through: :group_managers after_save :destroy_deletions + after_save :automatic_group_membership validate :name_format_validator validates_uniqueness_of :name, case_sensitive: false @@ -301,6 +302,12 @@ class Group < ActiveRecord::Base @deletions = nil end + def automatic_group_membership + if self.automatic_membership_retroactive + Jobs.enqueue(:automatic_group_membership, group_id: self.id) + end + end + end # == Schema Information diff --git a/app/models/user.rb b/app/models/user.rb index 8b597298f6c..64e9749bed8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -79,6 +79,7 @@ class User < ActiveRecord::Base after_create :create_user_stat after_create :create_user_profile after_create :ensure_in_trust_level_group + after_create :automatic_group_membership before_save :update_username_lower before_save :ensure_password_is_hashed @@ -715,6 +716,17 @@ class User < ActiveRecord::Base Group.user_trust_level_change!(id, trust_level) end + def automatic_group_membership + Group.where(automatic: false) + .where("LENGTH(COALESCE(automatic_membership_email_domains, '')) > 0") + .each do |group| + domains = group.automatic_membership_email_domains.gsub('.', '\.') + if self.email =~ Regexp.new("@(#{domains})", true) + group.add(self) rescue ActiveRecord::RecordNotUnique + end + end + end + def create_user_stat stat = UserStat.new(new_since: Time.now) stat.user_id = id diff --git a/app/serializers/basic_group_serializer.rb b/app/serializers/basic_group_serializer.rb index 8f1574cb97b..2bf2d111be6 100644 --- a/app/serializers/basic_group_serializer.rb +++ b/app/serializers/basic_group_serializer.rb @@ -1,3 +1,14 @@ class BasicGroupSerializer < ApplicationSerializer - attributes :id, :automatic, :name, :user_count, :alias_level, :visible + attributes :id, + :automatic, + :name, + :user_count, + :alias_level, + :visible, + :automatic_membership_email_domains, + :automatic_membership_retroactive + + def automatic_membership_email_domains + object.automatic_membership_email_domains.presence || "" + end end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index d24229b9cbc..3c37e443d23 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1633,6 +1633,8 @@ en: add_members: "Add members" custom: "Custom" automatic: "Automatic" + automatic_membership_email_domains: "Users registering with an email from that list will automatically be a member of this group" + automatic_membership_retroactive: "Registered users matching this list are also member of the group" api: generate_master: "Generate Master API Key" diff --git a/db/migrate/20150123145128_add_automatic_membership_to_group.rb b/db/migrate/20150123145128_add_automatic_membership_to_group.rb new file mode 100644 index 00000000000..2ea654656e2 --- /dev/null +++ b/db/migrate/20150123145128_add_automatic_membership_to_group.rb @@ -0,0 +1,6 @@ +class AddAutomaticMembershipToGroup < ActiveRecord::Migration + def change + add_column :groups, :automatic_membership_email_domains, :text + add_column :groups, :automatic_membership_retroactive, :boolean, default: false + end +end diff --git a/spec/controllers/admin/groups_controller_spec.rb b/spec/controllers/admin/groups_controller_spec.rb index 8c83768e0d5..8efd09bd5ed 100644 --- a/spec/controllers/admin/groups_controller_spec.rb +++ b/spec/controllers/admin/groups_controller_spec.rb @@ -25,7 +25,9 @@ describe Admin::GroupsController do "user_count"=>1, "automatic"=>false, "alias_level"=>0, - "visible"=>true + "visible"=>true, + "automatic_membership_email_domains"=>"", + "automatic_membership_retroactive"=>false }]) end @@ -57,6 +59,18 @@ describe Admin::GroupsController do expect(group.visible).to eq(true) end + 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" + 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" + expect(response).to be_success + end + end context ".destroy" do diff --git a/spec/jobs/automatic_group_membership_spec.rb b/spec/jobs/automatic_group_membership_spec.rb new file mode 100644 index 00000000000..fe80c04cae2 --- /dev/null +++ b/spec/jobs/automatic_group_membership_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' +require_dependency 'jobs/regular/automatic_group_membership' + +describe Jobs::AutomaticGroupMembership do + + it "raises an error when the group id is missing" do + expect { Jobs::AutomaticGroupMembership.new.execute({}) }.to raise_error(Discourse::InvalidParameters) + end + + it "updates the membership" do + user1 = Fabricate(:user, email: "foo@wat.com") + user2 = Fabricate(:user, email: "foo@bar.com") + group = Fabricate(:group, automatic_membership_email_domains: "wat.com", automatic_membership_retroactive: true) + + Jobs::AutomaticGroupMembership.new.execute(group_id: group.id) + + group.reload + expect(group.users.include?(user1)).to eq(true) + expect(group.users.include?(user2)).to eq(false) + end + +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 96aa8286c59..b918d44467e 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1213,4 +1213,15 @@ describe User do end + describe "automatic group membership" do + + it "is automatically added to a group when the email matches" do + group = Fabricate(:group, automatic_membership_email_domains: "bar.com|wat.com") + user = Fabricate(:user, email: "foo@bar.com") + group.reload + expect(group.users.include?(user)).to eq(true) + end + + end + end