settings: Migrate to create_stream_policy structure.

This commit replaces the `create_stream_by_admins_only` setting with a
new `create_stream_policy` setting, which mirroring the structure of
the existing `invite_to_stream_policy`.

This is important preparation for migrating the waiting period feature
to be its own independent setting.

Fixes #12236.
This commit is contained in:
David Wood 2019-05-06 15:34:31 +01:00 committed by Tim Abbott
parent 3ecdabdc77
commit 34d810aac3
17 changed files with 245 additions and 67 deletions

View File

@ -193,11 +193,11 @@ var event_fixtures = {
// Please keep this next section un-nested, as we want this to partly
// be simple documentation on the formats of individual events.
realm__update__create_stream_by_admins_only: {
realm__update__create_stream_policy: {
type: 'realm',
op: 'update',
property: 'create_stream_by_admins_only',
value: false,
property: 'create_stream_policy',
value: 2,
},
realm__update__invite_to_stream_policy: {
@ -872,8 +872,8 @@ with_overrides(function (override) {
assert.equal(page_params[parameter_name], true);
}
var event = event_fixtures.realm__update__create_stream_by_admins_only;
test_realm_boolean(event, 'realm_create_stream_by_admins_only');
var event = event_fixtures.realm__update__create_stream_policy;
test_realm_boolean(event, 'realm_create_stream_policy');
event = event_fixtures.realm__update__invite_to_stream_policy;
test_realm_boolean(event, 'realm_invite_to_stream_policy');
@ -903,9 +903,6 @@ with_overrides(function (override) {
event = event_fixtures.realm__update__disallow_disposable_email_addresses;
test_realm_boolean(event, 'realm_disallow_disposable_email_addresses');
event = event_fixtures.realm__update__create_stream_by_admins_only;
test_realm_boolean(event, 'realm_create_stream_by_admins_only');
event = event_fixtures.realm__update_notifications_stream_id;
override('settings_org.render_notifications_stream_ui', noop);
dispatch(event);

View File

@ -269,7 +269,7 @@ function test_submit_settings_form(submit_form) {
invite_to_stream_policy: '1',
email_address_visibility: '1',
add_emoji_by_admins_only: false,
create_stream_by_admins_only: false,
create_stream_policy: '1',
waiting_period_threshold: 0,
};
assert.deepEqual(data, expected_value);
@ -532,10 +532,10 @@ function test_sync_realm_settings() {
const waiting_period_input_parent = $.create('stub-waiting-period-input-parent');
$("#id_realm_waiting_period_threshold").set_parent(waiting_period_input_parent);
page_params.realm_create_stream_by_admins_only = false;
page_params.realm_create_stream_policy = 3;
page_params.realm_waiting_period_threshold = 3;
settings_org.sync_realm_settings('create_stream_by_admins_only');
settings_org.sync_realm_settings('create_stream_policy');
assert.equal($("#id_realm_create_stream_permission").val(), "by_admin_user_with_three_days_old");
assert.equal(waiting_period_input_parent.visible(), false);
}
@ -558,7 +558,7 @@ function test_sync_realm_settings() {
property_elem.length = 1;
property_elem.attr('id', 'id_realm_message_content_edit_limit_minutes');
page_params.realm_create_stream_by_admins_only = false;
page_params.realm_create_stream_policy = 1;
page_params.realm_message_content_edit_limit_seconds = 120;
settings_org.sync_realm_settings('message_content_edit_limit_seconds');
@ -593,7 +593,7 @@ function test_sync_realm_settings() {
property_elem.length = 1;
property_elem.attr('id', 'id_realm_message_content_edit_limit_minutes');
page_params.realm_create_stream_by_admins_only = false;
page_params.realm_create_stream_policy = 1;
page_params.realm_message_content_edit_limit_seconds = 120;
settings_org.sync_realm_settings('message_content_edit_limit_seconds');

View File

@ -32,7 +32,7 @@ exports.build_page = function () {
realm_inline_url_embed_preview: page_params.realm_inline_url_embed_preview,
server_inline_url_embed_preview: page_params.server_inline_url_embed_preview,
realm_authentication_methods: page_params.realm_authentication_methods,
realm_create_stream_by_admins_only: page_params.realm_create_stream_by_admins_only,
realm_create_stream_policy: page_params.realm_create_stream_policy,
realm_invite_to_stream_by_admins_only: page_params.realm_invite_to_stream_by_admins_only,
realm_name_changes_disabled: page_params.realm_name_changes_disabled,
realm_email_changes_disabled: page_params.realm_email_changes_disabled,

View File

@ -92,7 +92,7 @@ exports.dispatch_normal_event = function dispatch_normal_event(event) {
allow_community_topic_editing: noop,
avatar_changes_disabled: settings_account.update_avatar_change_display,
bot_creation_policy: settings_bots.update_bot_permissions_ui,
create_stream_by_admins_only: noop,
create_stream_policy: noop,
invite_to_stream_policy: noop,
default_language: noop,
default_twenty_four_hour_time: noop,
@ -128,10 +128,11 @@ exports.dispatch_normal_event = function dispatch_normal_event(event) {
page_params['realm_' + event.property] = event.value;
realm_settings[event.property]();
settings_org.sync_realm_settings(event.property);
if (event.property === 'create_stream_by_admins_only') {
if (event.property === 'create_stream_policy') {
if (!page_params.is_admin) {
// TODO: Add waiting_period_threshold logic here.
page_params.can_create_streams =
!page_params.realm_create_stream_by_admins_only;
page_params.realm_create_stream_policy === 1;
}
} else if (event.property === 'invite_to_stream_policy') {
if (!page_params.is_admin) {

View File

@ -71,13 +71,14 @@ function get_property_value(property_name) {
}
if (property_name === 'realm_create_stream_permission') {
if (page_params.realm_create_stream_by_admins_only) {
if (page_params.realm_create_stream_policy === 2) {
return "by_admins_only";
}
if (page_params.realm_waiting_period_threshold === 0) {
if (page_params.realm_create_stream_policy === 1) {
return "by_anyone";
}
if (page_params.realm_waiting_period_threshold === 3) {
if (page_params.realm_create_stream_policy === 3 &&
page_params.realm_waiting_period_threshold === 3) {
return "by_admin_user_with_three_days_old";
}
return "by_admin_user_with_custom_time";
@ -455,7 +456,7 @@ exports.sync_realm_settings = function (property) {
if (property === 'message_content_edit_limit_seconds') {
property = 'message_content_edit_limit_minutes';
} else if (property === 'create_stream_by_admins_only') {
} else if (property === 'create_stream_policy') {
property = 'create_stream_permission';
} else if (property === 'invite_to_stream_policy') {
property = 'invite_to_stream_policy';
@ -724,15 +725,15 @@ exports.build_page = function () {
}
if (create_stream_permission === "by_admins_only") {
data.create_stream_by_admins_only = true;
data.create_stream_policy = 2;
} else if (create_stream_permission === "by_admin_user_with_three_days_old") {
data.create_stream_by_admins_only = false;
data.create_stream_policy = 3;
data.waiting_period_threshold = 3;
} else if (create_stream_permission === "by_admin_user_with_custom_time") {
data.create_stream_by_admins_only = false;
data.create_stream_policy = 3;
data.waiting_period_threshold = $("#id_realm_waiting_period_threshold").val();
} else if (create_stream_permission === "by_anyone") {
data.create_stream_by_admins_only = false;
data.create_stream_policy = 1;
data.waiting_period_threshold = 0;
}

View File

@ -568,7 +568,7 @@ def apply_event(state: Dict[str, Any],
state[field] = event['value']
# Tricky interaction: Whether we can create streams can get changed here.
if (field in ['realm_create_stream_by_admins_only',
if (field in ['realm_create_stream_policy',
'realm_waiting_period_threshold']) and 'can_create_streams' in state:
state['can_create_streams'] = user_profile.can_create_streams()

View File

@ -0,0 +1,19 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.11.20 on 2019-05-06 13:15
from __future__ import unicode_literals
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('zerver', '0215_realm_avatar_changes_disabled'),
]
operations = [
migrations.AddField(
model_name='realm',
name='create_stream_policy',
field=models.PositiveSmallIntegerField(default=1),
),
]

View File

@ -0,0 +1,30 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.11.20 on 2019-05-06 13:15
from __future__ import unicode_literals
from django.db import migrations
from django.db.backends.postgresql_psycopg2.schema import DatabaseSchemaEditor
from django.db.migrations.state import StateApps
def upgrade_create_stream_policy(apps: StateApps, schema_editor: DatabaseSchemaEditor) -> None:
Realm = apps.get_model('zerver', 'Realm')
Realm.objects.filter(waiting_period_threshold__exact=0) \
.filter(create_stream_by_admins_only=False) \
.update(create_stream_policy=1) # CREATE_STREAM_POLICY_MEMBERS
Realm.objects.filter(create_stream_by_admins_only=True) \
.update(create_stream_policy=2) # CREATE_STREAM_POLICY_ADMINS
Realm.objects.filter(waiting_period_threshold__gt=0) \
.filter(create_stream_by_admins_only=False) \
.update(create_stream_policy=3) # CREATE_STREAM_POLICY_WAITING_PERIOD
class Migration(migrations.Migration):
dependencies = [
('zerver', '0216_add_create_stream_policy'),
]
operations = [
migrations.RunPython(upgrade_create_stream_policy,
reverse_code=migrations.RunPython.noop),
]

View File

@ -0,0 +1,18 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.11.20 on 2019-05-06 13:15
from __future__ import unicode_literals
from django.db import migrations
class Migration(migrations.Migration):
dependencies = [
('zerver', '0217_migrate_create_stream_policy'),
]
operations = [
migrations.RemoveField(
model_name='realm',
name='create_stream_by_admins_only',
),
]

View File

@ -196,18 +196,24 @@ class Realm(models.Model):
message_content_allowed_in_email_notifications = models.BooleanField(default=True) # type: bool
mandatory_topics = models.BooleanField(default=False) # type: bool
create_stream_by_admins_only = models.BooleanField(default=False) # type: bool
add_emoji_by_admins_only = models.BooleanField(default=False) # type: bool
name_changes_disabled = models.BooleanField(default=False) # type: bool
email_changes_disabled = models.BooleanField(default=False) # type: bool
avatar_changes_disabled = models.BooleanField(default=False) # type: bool
# Who in the organization is allowed to create streams.
CREATE_STREAM_POLICY_MEMBERS = 1
CREATE_STREAM_POLICY_ADMINS = 2
CREATE_STREAM_POLICY_WAITING_PERIOD = 3
create_stream_policy = models.PositiveSmallIntegerField(
default=CREATE_STREAM_POLICY_MEMBERS) # type: int
# Who in the organization is allowed to invite other users to streams.
INVITE_TO_STREAM_POLICY_MEMBERS = 1
INVITE_TO_STREAM_POLICY_ADMINS = 2
INVITE_TO_STREAM_POLICY_WAITING_PERIOD = 3
invite_to_stream_policy = models.PositiveSmallIntegerField(
default=INVITE_TO_STREAM_POLICY_MEMBERS) # type: bool
default=INVITE_TO_STREAM_POLICY_MEMBERS) # type: int
# Who in the organization has access to users' actual email
# addresses. Controls whether the UserProfile.email field is the
@ -298,7 +304,7 @@ class Realm(models.Model):
allow_edit_history=bool,
allow_message_deleting=bool,
bot_creation_policy=int,
create_stream_by_admins_only=bool,
create_stream_policy=int,
invite_to_stream_policy=int,
default_language=str,
default_twenty_four_hour_time = bool,
@ -1004,11 +1010,14 @@ class UserProfile(AbstractBaseUser, PermissionsMixin):
def can_create_streams(self) -> bool:
if self.is_realm_admin:
return True
if self.realm.create_stream_by_admins_only:
if self.realm.create_stream_policy == Realm.CREATE_STREAM_POLICY_ADMINS:
return False
if self.is_guest:
return False
if self.realm.create_stream_policy == Realm.CREATE_STREAM_POLICY_MEMBERS:
return True
diff = (timezone_now() - self.date_joined).days
if diff >= self.realm.waiting_period_threshold:
return True

View File

@ -357,7 +357,7 @@ paths:
* `realm_bots`:
* `realm_create_stream_by_admins_only`:
* `realm_create_stream_policy`:
* `realm_invite_to_stream_policy`:
@ -892,8 +892,8 @@ definitions:
type: string
realm_bots:
type: string
realm_create_stream_by_admins_only:
type: boolean
realm_create_stream_policy:
type: int64
realm_invite_to_stream_policy:
type: int64
realm_default_language:

View File

@ -1574,6 +1574,7 @@ class EventsRegisterTest(ZulipTestCase):
message_retention_days=[10, 20],
name=[u'Zulip', u'New Name'],
waiting_period_threshold=[10, 20],
create_stream_policy=[3, 2, 1],
invite_to_stream_policy=[3, 2, 1],
email_address_visibility=[Realm.EMAIL_ADDRESS_VISIBILITY_ADMINS],
bot_creation_policy=[Realm.BOT_CREATION_EVERYONE],

View File

@ -125,7 +125,7 @@ class HomeTest(ZulipTestCase):
"realm_bot_creation_policy",
"realm_bot_domain",
"realm_bots",
"realm_create_stream_by_admins_only",
"realm_create_stream_policy",
"realm_default_language",
"realm_default_stream_groups",
"realm_default_streams",

View File

@ -64,3 +64,74 @@ class EmojiName2IdTestCase(MigrationsTestCase):
realm_id=reaction.user_profile.realm_id,
name=reaction.emoji_name)
self.assertEqual(reaction.emoji_code, str(realm_emoji.id))
class CreateStreamPolicyMembersAndAdmins(MigrationsTestCase):
migrate_from = '0215_realm_avatar_changes_disabled'
migrate_to = '0218_remove_create_stream_by_admins_only'
@use_db_models
def setUpBeforeMigration(self, apps: StateApps) -> None:
user = self.example_user('iago')
realm = user.realm
# Test migration of "Members and admins"
realm.create_stream_by_admins_only = False
realm.waiting_period_threshold = 0
realm.save()
def test_policy_migrated(self) -> None:
"""Test runs after the migration, and verifies the data was migrated correctly"""
user = self.example_user('iago')
realm = user.realm
# Test migration of "Members and admins"
self.assertEqual(realm.create_stream_policy, realm.CREATE_STREAM_POLICY_MEMBERS)
class CreateStreamPolicyAdmins(MigrationsTestCase):
migrate_from = '0215_realm_avatar_changes_disabled'
migrate_to = '0218_remove_create_stream_by_admins_only'
@use_db_models
def setUpBeforeMigration(self, apps: StateApps) -> None:
user = self.example_user('iago')
realm = user.realm
# Test migration of "Admins only"
realm.create_stream_by_admins_only = True
realm.save()
def test_policy_migrated(self) -> None:
"""Test runs after the migration, and verifies the data was migrated correctly"""
user = self.example_user('iago')
realm = user.realm
# Test migration of "Admins only"
self.assertEqual(realm.create_stream_policy, realm.CREATE_STREAM_POLICY_ADMINS)
class CreateStreamPolicyWaitingPeriod(MigrationsTestCase):
migrate_from = '0215_realm_avatar_changes_disabled'
migrate_to = '0218_remove_create_stream_by_admins_only'
@use_db_models
def setUpBeforeMigration(self, apps: StateApps) -> None:
user = self.example_user('iago')
realm = user.realm
# Test migration of "All admins, and members with accounts at least N days old."
realm.create_stream_by_admins_only = False
realm.waiting_period_threshold = 6
realm.save()
def test_policy_migrated(self) -> None:
"""Test runs after the migration, and verifies the data was migrated correctly"""
user = self.example_user('iago')
realm = user.realm
# Test migration of "All admins, and members with accounts at least N days old."
self.assertEqual(realm.create_stream_policy, realm.CREATE_STREAM_POLICY_WAITING_PERIOD)

View File

@ -548,6 +548,9 @@ class RealmAPITest(ZulipTestCase):
message_retention_days=[10, 20],
name=[u'Zulip', u'New Name'],
waiting_period_threshold=[10, 20],
create_stream_policy=[Realm.CREATE_STREAM_POLICY_ADMINS,
Realm.CREATE_STREAM_POLICY_MEMBERS,
Realm.CREATE_STREAM_POLICY_WAITING_PERIOD],
invite_to_stream_policy=[Realm.INVITE_TO_STREAM_POLICY_ADMINS,
Realm.INVITE_TO_STREAM_POLICY_MEMBERS,
Realm.INVITE_TO_STREAM_POLICY_WAITING_PERIOD],

View File

@ -967,28 +967,16 @@ class StreamAdminTest(ZulipTestCase):
self.assertEqual(len(json["removed"]), 1)
self.assertEqual(len(json["not_subscribed"]), 0)
def test_create_stream_by_admins_only_setting(self) -> None:
def test_create_stream_policy_setting(self) -> None:
"""
When realm.create_stream_by_admins_only setting is active and
the number of days since the user had joined is less than waiting period
threshold, non admin users shouldn't be able to create new streams.
"""
user_profile = self.example_user('hamlet')
email = user_profile.email
self.login(email)
do_set_realm_property(user_profile.realm, 'create_stream_by_admins_only', True)
When realm.create_stream_policy setting is Realm.CREATE_STREAM_POLICY_MEMBERS then
test that any user can create a stream.
stream_name = ['adminsonlysetting']
result = self.common_subscribe_to_streams(
email,
stream_name
)
self.assert_json_error(result, 'User cannot create streams.')
When realm.create_stream_policy setting is Realm.CREATE_STREAM_POLICY_ADMINS then
test that only admins can create a stream.
def test_create_stream_by_waiting_period_threshold(self) -> None:
"""
Non admin users with account age greater or equal to waiting period
threshold should be able to create new streams.
When realm.create_stream_policy setting is Realm.CREATE_STREAM_POLICY_WAITING_PERIOD then
test that admins and users with accounts older than the waiting period can create a stream.
"""
user_profile = self.example_user('hamlet')
user_profile.date_joined = timezone_now()
@ -997,21 +985,60 @@ class StreamAdminTest(ZulipTestCase):
self.login(email)
do_change_is_admin(user_profile, False)
# Allow all members to create streams.
do_set_realm_property(user_profile.realm, 'create_stream_policy',
Realm.CREATE_STREAM_POLICY_MEMBERS)
# Set waiting period to 10 days.
do_set_realm_property(user_profile.realm, 'waiting_period_threshold', 10)
stream_name = ['waitingperiodtest']
result = self.common_subscribe_to_streams(
email,
stream_name
)
# Can successfully create stream despite being less than waiting period and not an admin,
# due to create stream policy.
stream_name = ['all_members']
result = self.common_subscribe_to_streams(email, stream_name)
self.assert_json_success(result)
# Allow only administrators to create streams.
do_set_realm_property(user_profile.realm, 'create_stream_policy',
Realm.CREATE_STREAM_POLICY_ADMINS)
# Cannot create stream because not an admin.
stream_name = ['admins_only']
result = self.common_subscribe_to_streams(email, stream_name)
self.assert_json_error(result, 'User cannot create streams.')
do_set_realm_property(user_profile.realm, 'waiting_period_threshold', 0)
# Make current user an admin.
do_change_is_admin(user_profile, True)
result = self.common_subscribe_to_streams(
email,
stream_name
)
# Can successfully create stream as user is now an admin.
stream_name = ['admins_only']
result = self.common_subscribe_to_streams(email, stream_name)
self.assert_json_success(result)
# Allow users older than the waiting period to create streams.
do_set_realm_property(user_profile.realm, 'create_stream_policy',
Realm.CREATE_STREAM_POLICY_WAITING_PERIOD)
# Can successfully create stream despite being under waiting period because user is admin.
stream_name = ['waiting_period_as_admin']
result = self.common_subscribe_to_streams(email, stream_name)
self.assert_json_success(result)
# Make current user no longer an admin.
do_change_is_admin(user_profile, False)
# Cannot create stream because user is not an admin and is not older than the waiting
# period.
stream_name = ['waiting_period']
result = self.common_subscribe_to_streams(email, stream_name)
self.assert_json_error(result, 'User cannot create streams.')
# Make user account 11 days old..
user_profile.date_joined = timezone_now() - timedelta(days=11)
user_profile.save()
# Can successfully create stream now that account is old enough.
stream_name = ['waiting_period']
result = self.common_subscribe_to_streams(email, stream_name)
self.assert_json_success(result)
def test_invite_to_stream_by_invite_period_threshold(self) -> None:
@ -2127,15 +2154,16 @@ class SubscriptionAPITest(ZulipTestCase):
self.assertTrue(othello.can_create_streams())
othello.is_realm_admin = False
othello.realm.create_stream_by_admins_only = True
othello.realm.create_stream_policy = Realm.CREATE_STREAM_POLICY_ADMINS
self.assertFalse(othello.can_create_streams())
othello.realm.create_stream_by_admins_only = False
othello.realm.create_stream_policy = Realm.CREATE_STREAM_POLICY_MEMBERS
othello.is_guest = True
self.assertFalse(othello.can_create_streams())
othello.is_guest = False
othello.realm.waiting_period_threshold = 1000
othello.realm.create_stream_policy = Realm.CREATE_STREAM_POLICY_WAITING_PERIOD
othello.date_joined = timezone_now() - timedelta(days=(othello.realm.waiting_period_threshold - 1))
self.assertFalse(othello.can_create_streams())
@ -2151,7 +2179,7 @@ class SubscriptionAPITest(ZulipTestCase):
invitee_email = user_profile.email
realm = user_profile.realm
do_set_realm_property(realm, "create_stream_by_admins_only", False)
do_set_realm_property(realm, "create_stream_policy", Realm.CREATE_STREAM_POLICY_MEMBERS)
do_set_realm_property(realm, "invite_to_stream_policy",
Realm.INVITE_TO_STREAM_POLICY_ADMINS)
result = self.common_subscribe_to_streams(

View File

@ -42,7 +42,6 @@ def update_realm(
avatar_changes_disabled: Optional[bool]=REQ(validator=check_bool, default=None),
inline_image_preview: Optional[bool]=REQ(validator=check_bool, default=None),
inline_url_embed_preview: Optional[bool]=REQ(validator=check_bool, default=None),
create_stream_by_admins_only: Optional[bool]=REQ(validator=check_bool, default=None),
add_emoji_by_admins_only: Optional[bool]=REQ(validator=check_bool, default=None),
allow_message_deleting: Optional[bool]=REQ(validator=check_bool, default=None),
message_content_delete_limit_seconds: Optional[int]=REQ(converter=to_non_negative_int, default=None),
@ -62,6 +61,7 @@ def update_realm(
message_content_allowed_in_email_notifications:
Optional[bool]=REQ(validator=check_bool, default=None),
bot_creation_policy: Optional[int]=REQ(converter=to_not_negative_int_or_none, default=None),
create_stream_policy: Optional[int]=REQ(validator=check_int, default=None),
invite_to_stream_policy: Optional[int]=REQ(validator=check_int, default=None),
email_address_visibility: Optional[int]=REQ(converter=to_not_negative_int_or_none, default=None),
default_twenty_four_hour_time: Optional[bool]=REQ(validator=check_bool, default=None),