FIX: Groups that do not have any owners should not allow membership requests.

This commit is contained in:
Guo Xiang Tan
2017-06-02 23:15:57 +09:00
parent a10c939775
commit ac6c1acbed
5 changed files with 62 additions and 9 deletions

View File

@ -36,6 +36,7 @@ class Group < ActiveRecord::Base
validates_uniqueness_of :name, case_sensitive: false validates_uniqueness_of :name, case_sensitive: false
validate :automatic_membership_email_domains_format_validator validate :automatic_membership_email_domains_format_validator
validate :incoming_email_validator validate :incoming_email_validator
validate :can_allow_membership_requests
validates :flair_url, url: true, if: Proc.new { |g| g.flair_url && g.flair_url[0,3] != 'fa-' } validates :flair_url, url: true, if: Proc.new { |g| g.flair_url && g.flair_url[0,3] != 'fa-' }
AUTO_GROUPS = { AUTO_GROUPS = {
@ -511,6 +512,12 @@ SQL
private private
def can_allow_membership_requests
if self.allow_membership_requests && !self.group_users.where(owner: true).exists?
self.errors.add(:base, I18n.t('groups.errors.cant_allow_membership_requests'))
end
end
def enqueue_update_mentions_job def enqueue_update_mentions_job
Jobs.enqueue(:update_group_mentions, Jobs.enqueue(:update_group_mentions,
previous_name: self.name_was, previous_name: self.name_was,

View File

@ -264,6 +264,7 @@ en:
invalid_incoming_email: "'%{email}' is not a valid email address." invalid_incoming_email: "'%{email}' is not a valid email address."
email_already_used_in_group: "'%{email}' is already used by the group '%{group_name}'." email_already_used_in_group: "'%{email}' is already used by the group '%{group_name}'."
email_already_used_in_category: "'%{email}' is already used by the category '%{category_name}'." email_already_used_in_category: "'%{email}' is already used by the category '%{category_name}'."
cant_allow_membership_requests: "You cannot allow membership requests for a group without any owners."
default_names: default_names:
everyone: "everyone" everyone: "everyone"
admins: "admins" admins: "admins"

View File

@ -0,0 +1,13 @@
class FixGroupAllowMembershipRequests < ActiveRecord::Migration
def up
execute <<~SQL
UPDATE groups g
SET allow_membership_requests = 'f'
WHERE NOT EXISTS (SELECT 1 FROM group_users gu WHERE gu.owner = 't' AND gu.group_id = g.id LIMIT 1)
SQL
end
def down
raise ActiveRecord::IrreversibleMigration
end
end

View File

@ -1,6 +1,8 @@
require 'rails_helper' require 'rails_helper'
describe Admin::GroupsController do describe Admin::GroupsController do
let(:user) { Fabricate(:user) }
let(:group) { Fabricate(:group) }
before do before do
@admin = log_in(:admin) @admin = log_in(:admin)
@ -77,7 +79,7 @@ describe Admin::GroupsController do
end end
end end
context ".create" do context "#create" do
it "strip spaces on the group name" do it "strip spaces on the group name" do
xhr :post, :create, { group: { name: " bob " } } xhr :post, :create, { group: { name: " bob " } }
@ -92,23 +94,33 @@ describe Admin::GroupsController do
end end
context ".update" do context "#update" do
it 'should update a group' do
group.add_owner(user)
expect do
xhr :put, :update, { id: group.id, group: {
visible: "false",
allow_membership_requests: "true"
} }
end.to change { GroupHistory.count }.by(2)
expect(response).to be_success
group.reload
expect(group.visible).to eq(false)
expect(group.allow_membership_requests).to eq(true)
end
it "ignore name change on automatic group" do it "ignore name change on automatic group" do
expect do expect do
xhr :put, :update, { id: 1, group: { xhr :put, :update, { id: 1, group: { name: "WAT" } }
name: "WAT",
visible: "true",
allow_membership_requests: "true"
} }
end.to change { GroupHistory.count }.by(1) end.to change { GroupHistory.count }.by(1)
expect(response).to be_success expect(response).to be_success
group = Group.find(1) group = Group.find(1)
expect(group.name).not_to eq("WAT") expect(group.name).not_to eq("WAT")
expect(group.visible).to eq(true)
expect(group.allow_membership_requests).to eq(true)
end end
it "doesn't launch the 'automatic group membership' job when it's not retroactive" do it "doesn't launch the 'automatic group membership' job when it's not retroactive" do

View File

@ -77,6 +77,26 @@ describe Group do
group.incoming_email = "foo@bar.org" group.incoming_email = "foo@bar.org"
expect(group.valid?).to eq(true) expect(group.valid?).to eq(true)
end end
context 'when a group has no owners' do
it 'should not allow membership requests' do
group.allow_membership_requests = true
expect(group.valid?).to eq(false)
expect(group.errors.full_messages).to include(I18n.t(
"groups.errors.cant_allow_membership_requests"
))
group.allow_membership_requests = false
group.save!
group.add_owner(user)
group.allow_membership_requests = true
expect(group.valid?).to eq(true)
end
end
end end
def real_admins def real_admins