diff --git a/web/src/settings_components.ts b/web/src/settings_components.ts index a6a2344e76..58c387dd35 100644 --- a/web/src/settings_components.ts +++ b/web/src/settings_components.ts @@ -1043,7 +1043,7 @@ export function populate_data_for_realm_settings_request( continue; } - const realm_group_settings_using_new_api_format = new Set([ + const realm_group_settings = new Set([ "can_access_all_users_group", "can_add_custom_emoji_group", "can_create_groups", @@ -1058,7 +1058,7 @@ export function populate_data_for_realm_settings_request( "direct_message_initiator_group", "direct_message_permission_group", ]); - if (realm_group_settings_using_new_api_format.has(property_name)) { + if (realm_group_settings.has(property_name)) { const old_value = get_realm_settings_property_value( // eslint-disable-next-line @typescript-eslint/consistent-type-assertions ("realm_" + property_name) as RealmSettingProperty, diff --git a/zerver/lib/events.py b/zerver/lib/events.py index 2cbe2f9ed4..cfe243821a 100644 --- a/zerver/lib/events.py +++ b/zerver/lib/events.py @@ -292,16 +292,9 @@ def fetch_initial_state_data( for property_name in Realm.property_types: state["realm_" + property_name] = getattr(realm, property_name) - for ( - setting_name, - permission_configuration, - ) in Realm.REALM_PERMISSION_GROUP_SETTINGS.items(): - if setting_name in Realm.REALM_PERMISSION_GROUP_SETTINGS_WITH_NEW_API_FORMAT: - setting_value = getattr(realm, setting_name) - state["realm_" + setting_name] = get_group_setting_value_for_api(setting_value) - continue - - state["realm_" + setting_name] = getattr(realm, permission_configuration.id_field_name) + for setting_name in Realm.REALM_PERMISSION_GROUP_SETTINGS: + setting_value = getattr(realm, setting_name) + state["realm_" + setting_name] = get_group_setting_value_for_api(setting_value) state["realm_create_public_stream_policy"] = ( get_corresponding_policy_value_for_group_setting( @@ -1114,7 +1107,7 @@ def apply_event( if user_id != person_user_id ] - for setting_name in Realm.REALM_PERMISSION_GROUP_SETTINGS_WITH_NEW_API_FORMAT: + for setting_name in Realm.REALM_PERMISSION_GROUP_SETTINGS: if not isinstance(state["realm_" + setting_name], int): state["realm_" + setting_name].direct_members = [ user_id diff --git a/zerver/models/realms.py b/zerver/models/realms.py index 997f846702..7c0e2d87b1 100644 --- a/zerver/models/realms.py +++ b/zerver/models/realms.py @@ -807,22 +807,6 @@ class Realm(models.Model): # type: ignore[django-manager-missing] # django-stub ), ) - REALM_PERMISSION_GROUP_SETTINGS_WITH_NEW_API_FORMAT = [ - "create_multiuse_invite_group", - "can_access_all_users_group", - "can_add_custom_emoji_group", - "can_create_groups", - "can_create_private_channel_group", - "can_create_public_channel_group", - "can_create_web_public_channel_group", - "can_delete_any_message_group", - "can_delete_own_message_group", - "can_manage_all_groups", - "can_move_messages_between_channels_group", - "direct_message_initiator_group", - "direct_message_permission_group", - ] - DIGEST_WEEKDAY_VALUES = [0, 1, 2, 3, 4, 5, 6] # Icon is the square mobile icon. diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index 0eb68a205e..8a0bbe496e 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -4038,13 +4038,11 @@ class RealmPropertyActionTest(BaseAction): for prop in Realm.REALM_PERMISSION_GROUP_SETTINGS: with self.settings(SEND_DIGEST_EMAILS=True): self.do_set_realm_permission_group_setting_test(prop) - - for prop in Realm.REALM_PERMISSION_GROUP_SETTINGS_WITH_NEW_API_FORMAT: if Realm.REALM_PERMISSION_GROUP_SETTINGS[prop].require_system_group: # Anonymous system groups aren't relevant when # restricted to system groups. continue - with self.settings(SEND_DIGEST_EMAILs=True): + with self.settings(SEND_DIGEST_EMAILS=True): self.do_set_realm_permission_group_setting_to_anonymous_groups_test(prop) def do_set_realm_user_default_setting_test(self, name: str) -> None: diff --git a/zerver/tests/test_realm.py b/zerver/tests/test_realm.py index bb8abe749d..ae53ca74c6 100644 --- a/zerver/tests/test_realm.py +++ b/zerver/tests/test_realm.py @@ -1745,13 +1745,11 @@ class RealmAPITest(ZulipTestCase): self.assertEqual(getattr(realm, setting_name), default_group.usergroup_ptr) for user_group in all_system_user_groups: - value = orjson.dumps(user_group.id).decode() - if setting_name in Realm.REALM_PERMISSION_GROUP_SETTINGS_WITH_NEW_API_FORMAT: - value = orjson.dumps( - { - "new": user_group.id, - } - ).decode() + value = orjson.dumps( + { + "new": user_group.id, + } + ).decode() if ( ( @@ -1788,62 +1786,55 @@ class RealmAPITest(ZulipTestCase): if setting_permission_configuration.require_system_group: leadership_group = NamedUserGroup.objects.get(name="leadership", realm=realm) - value = orjson.dumps(leadership_group.id).decode() - if setting_name in Realm.REALM_PERMISSION_GROUP_SETTINGS_WITH_NEW_API_FORMAT: - value = orjson.dumps( - { - "new": leadership_group.id, - } - ).decode() + value = orjson.dumps( + { + "new": leadership_group.id, + } + ).decode() result = self.client_patch("/json/realm", {setting_name: value}) self.assert_json_error(result, f"'{setting_name}' must be a system user group.") - if setting_name in Realm.REALM_PERMISSION_GROUP_SETTINGS_WITH_NEW_API_FORMAT: - admins_group = NamedUserGroup.objects.get( - name=SystemGroups.ADMINISTRATORS, realm=realm + admins_group = NamedUserGroup.objects.get(name=SystemGroups.ADMINISTRATORS, realm=realm) + moderators_group = NamedUserGroup.objects.get(name=SystemGroups.MODERATORS, realm=realm) + value = orjson.dumps( + { + "new": { + "direct_members": [], + "direct_subgroups": [admins_group.id, leadership_group.id], + } + } + ).decode() + result = self.client_patch("/json/realm", {setting_name: value}) + self.assert_json_error(result, f"'{setting_name}' must be a system user group.") + + value = orjson.dumps( + { + "new": { + "direct_members": [], + "direct_subgroups": [admins_group.id, moderators_group.id], + } + } + ).decode() + result = self.client_patch("/json/realm", {setting_name: value}) + self.assert_json_error(result, f"'{setting_name}' must be a system user group.") + + group = admins_group + if setting_permission_configuration.allowed_system_groups: + group = NamedUserGroup.objects.get( + name=setting_permission_configuration.allowed_system_groups[0], realm=realm ) - moderators_group = NamedUserGroup.objects.get( - name=SystemGroups.MODERATORS, realm=realm - ) - value = orjson.dumps( - { - "new": { - "direct_members": [], - "direct_subgroups": [admins_group.id, leadership_group.id], - } - } - ).decode() - result = self.client_patch("/json/realm", {setting_name: value}) - self.assert_json_error(result, f"'{setting_name}' must be a system user group.") - value = orjson.dumps( - { - "new": { - "direct_members": [], - "direct_subgroups": [admins_group.id, moderators_group.id], - } + value = orjson.dumps( + { + "new": { + "direct_members": [], + "direct_subgroups": [group.id], } - ).decode() - result = self.client_patch("/json/realm", {setting_name: value}) - self.assert_json_error(result, f"'{setting_name}' must be a system user group.") - - group = admins_group - if setting_permission_configuration.allowed_system_groups: - group = NamedUserGroup.objects.get( - name=setting_permission_configuration.allowed_system_groups[0], realm=realm - ) - - value = orjson.dumps( - { - "new": { - "direct_members": [], - "direct_subgroups": [group.id], - } - } - ).decode() - realm = self.update_with_api(setting_name, value) - self.assertEqual(getattr(realm, setting_name), group.usergroup_ptr) + } + ).decode() + realm = self.update_with_api(setting_name, value) + self.assertEqual(getattr(realm, setting_name), group.usergroup_ptr) def do_test_realm_permission_group_setting_update_api_with_anonymous_groups( self, setting_name: str @@ -2037,13 +2028,10 @@ class RealmAPITest(ZulipTestCase): for prop in Realm.REALM_PERMISSION_GROUP_SETTINGS: with self.subTest(property=prop): self.do_test_realm_permission_group_setting_update_api(prop) - - for prop in Realm.REALM_PERMISSION_GROUP_SETTINGS_WITH_NEW_API_FORMAT: - if Realm.REALM_PERMISSION_GROUP_SETTINGS[prop].require_system_group: - # Anonymous system groups aren't relevant when - # restricted to system groups. - continue - with self.subTest(property=prop): + if Realm.REALM_PERMISSION_GROUP_SETTINGS[prop].require_system_group: + # Anonymous system groups aren't relevant when + # restricted to system groups. + continue self.do_test_realm_permission_group_setting_update_api_with_anonymous_groups(prop) # Not in Realm.property_types because org_type has diff --git a/zerver/views/realm.py b/zerver/views/realm.py index e84664ebb5..fc7597975b 100644 --- a/zerver/views/realm.py +++ b/zerver/views/realm.py @@ -356,26 +356,18 @@ def update_realm( data[k] = v for setting_name, permission_configuration in Realm.REALM_PERMISSION_GROUP_SETTINGS.items(): - setting_group_id_name = permission_configuration.id_field_name - expected_current_setting_value = None - if setting_name in Realm.REALM_PERMISSION_GROUP_SETTINGS_WITH_NEW_API_FORMAT: - assert setting_name in req_group_setting_vars - if req_group_setting_vars[setting_name] is None: - continue + assert setting_name in req_group_setting_vars + if req_group_setting_vars[setting_name] is None: + continue - setting_value = req_group_setting_vars[setting_name] - new_setting_value = parse_group_setting_value(setting_value.new, setting_name) + setting_value = req_group_setting_vars[setting_name] + new_setting_value = parse_group_setting_value(setting_value.new, setting_name) - if setting_value.old is not None: - expected_current_setting_value = parse_group_setting_value( - setting_value.old, setting_name - ) - else: - assert setting_group_id_name in req_group_setting_vars - if req_group_setting_vars[setting_group_id_name] is None: - continue - new_setting_value = req_group_setting_vars[setting_group_id_name] + if setting_value.old is not None: + expected_current_setting_value = parse_group_setting_value( + setting_value.old, setting_name + ) current_value = getattr(realm, setting_name) current_setting_api_value = get_group_setting_value_for_api(current_value)