diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index 32114f33699..aa5ddd5d59f 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/app/controllers/admin/groups_controller.rb @@ -133,7 +133,7 @@ class Admin::GroupsController < Admin::AdminController return can_not_modify_automatic if group.automatic - existing_domains = group.automatic_membership_email_domains.split("|") + existing_domains = group.automatic_membership_email_domains&.split("|") || [] domains -= existing_domains end diff --git a/app/models/group.rb b/app/models/group.rb index 6448f46512c..b207f1c02fb 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -91,6 +91,8 @@ class Group < ActiveRecord::Base everyone: 99 } + VALID_DOMAIN_REGEX = /\A[a-z0-9]+([\-\.]{1}[a-z0-9]+)*\.[a-z]{2,24}(:[0-9]{1,5})?(\/.*)?\Z/i + def self.visibility_levels @visibility_levels = Enum.new( public: 0, @@ -777,7 +779,7 @@ class Group < ActiveRecord::Base return if self.automatic_membership_email_domains.blank? domains = Group.get_valid_email_domains(self.automatic_membership_email_domains) do |domain| - self.errors.add :base, (I18n.t('groups.errors.invalid_domain', domain: domain)) unless domain =~ /\A[a-z0-9]+([\-\.]{1}[a-z0-9]+)*\.[a-z]{2,24}(:[0-9]{1,5})?(\/.*)?\Z/i + self.errors.add :base, (I18n.t('groups.errors.invalid_domain', domain: domain)) end self.automatic_membership_email_domains = domains.join("|") @@ -862,10 +864,10 @@ class Group < ActiveRecord::Base domain.sub!(/^https?:\/\//, '') domain.sub!(/\/.*$/, '') - if domain =~ /\A[a-z0-9]+([\-\.]{1}[a-z0-9]+)*\.[a-z]{2,24}(:[0-9]{1,5})?(\/.*)?\Z/i + if domain =~ Group::VALID_DOMAIN_REGEX valid_domains << domain else - yield domain + yield domain if block_given? end end diff --git a/spec/requests/admin/groups_controller_spec.rb b/spec/requests/admin/groups_controller_spec.rb index 1279d33ed3d..c8cedd06bac 100644 --- a/spec/requests/admin/groups_controller_spec.rb +++ b/spec/requests/admin/groups_controller_spec.rb @@ -223,4 +223,31 @@ RSpec.describe Admin::GroupsController do end end end + + describe '#automatic_membership_count' do + it 'returns count of users whose emails match the domain' do + Fabricate(:user, email: 'user1@somedomain.org') + Fabricate(:user, email: 'user1@somedomain.com') + Fabricate(:user, email: 'user1@notsomedomain.com') + group = Fabricate(:group) + + put "/admin/groups/automatic_membership_count.json", params: { + automatic_membership_email_domains: 'somedomain.org|somedomain.com', + id: group.id + } + expect(response.status).to eq(200) + expect(response.parsed_body["user_count"]).to eq(2) + end + + it "doesn't responde with 500 if domain is invalid" do + group = Fabricate(:group) + + put "/admin/groups/automatic_membership_count.json", params: { + automatic_membership_email_domains: '@somedomain.org|@somedomain.com', + id: group.id + } + expect(response.status).to eq(200) + expect(response.parsed_body["user_count"]).to eq(0) + end + end end