users: Return true for admins for is_moderator.

This change is for consistency with how is_admin works.

API design discussion at https://chat.zulip.org/#narrow/channel/378-api-design/topic/Should.20is_moderator.20have.20admins
This commit is contained in:
Shubham Padia 2025-04-17 12:43:07 +00:00 committed by Tim Abbott
parent 8efeecbd1f
commit 51d382f717
18 changed files with 58 additions and 40 deletions

View File

@ -20,6 +20,13 @@ format used by the Zulip server that they are interacting with.
## Changes in Zulip 11.0
**Feature level 380**
* [`POST /register`](/api/register-queue), [`GET
/events`](/api/get-events): The `is_moderator` convenience field now
is true for organization administrators, matching how `is_admin`
works for organization owners.
**Feature level 379**
* [`PATCH /messages/{message_id}`](/api/update-message): Added

View File

@ -36,7 +36,8 @@ by the [events API](/api/get-events).
Note that [`POST /register`](/api/register-queue) also returns an
`is_moderator` boolean property specifying whether the current user is
an organization moderator.
at least an organization moderator. The property will be true for admins
and owners too.
Additionally, user account data include an `is_billing_admin` property
specifying whether the user is a billing administrator for the Zulip

View File

@ -34,7 +34,7 @@ DESKTOP_WARNING_VERSION = "5.9.3"
# new level means in api_docs/changelog.md, as well as "**Changes**"
# entries in the endpoint's documentation in `zulip.yaml`.
API_FEATURE_LEVEL = 379
API_FEATURE_LEVEL = 380
# Bump the minor PROVISION_VERSION to indicate that folks should provision
# only when going from an old version of the code to a newer version. Bump

View File

@ -117,7 +117,7 @@ export function is_topic_editable(message: Message, edit_limit_seconds_buffer =
// Organization admins and moderators can edit message topics indefinitely,
// irrespective of the topic editing deadline, if they are in the
// can_move_messages_between_topics_group.
if (current_user.is_admin || current_user.is_moderator) {
if (current_user.is_moderator) {
return true;
}
@ -270,7 +270,7 @@ export function is_stream_editable(message: Message, edit_limit_seconds_buffer =
// Organization admins and moderators can edit stream indefinitely,
// irrespective of the stream editing deadline, if they are in the
// can_move_messages_between_channels_group.
if (current_user.is_admin || current_user.is_moderator) {
if (current_user.is_moderator) {
return true;
}

View File

@ -1432,7 +1432,13 @@ export function _add_user(person: User): void {
our realm (like cross-realm bots).
*/
person.is_moderator = false;
if (person.role === settings_config.user_role_values.moderator.code) {
if (
[
settings_config.user_role_values.moderator.code,
settings_config.user_role_values.admin.code,
settings_config.user_role_values.owner.code,
].includes(person.role)
) {
person.is_moderator = true;
}
if (person.user_id) {

View File

@ -109,7 +109,8 @@ export const update_person = function update(event: UserUpdate): void {
user.is_owner = event.role === settings_config.user_role_values.owner.code;
user.is_admin = event.role === settings_config.user_role_values.admin.code || user.is_owner;
user.is_guest = event.role === settings_config.user_role_values.guest.code;
user.is_moderator = event.role === settings_config.user_role_values.moderator.code;
user.is_moderator =
user.is_admin || event.role === settings_config.user_role_values.moderator.code;
settings_users.update_user_data(event.user_id, event);
user_profile.update_profile_modal_ui(user, event);

View File

@ -95,7 +95,7 @@ run_test("is_topic_editable", ({override}) => {
};
override(realm, "realm_allow_message_editing", true);
override(settings_data, "user_can_move_messages_to_another_topic", () => true);
override(current_user, "is_admin", true);
override(current_user, "is_moderator", true);
assert.equal(message_edit.is_topic_editable(message), false);
@ -112,7 +112,7 @@ run_test("is_topic_editable", ({override}) => {
override(settings_data, "user_can_move_messages_to_another_topic", () => false);
assert.equal(message_edit.is_topic_editable(message), false);
override(current_user, "is_admin", false);
override(current_user, "is_moderator", false);
assert.equal(message_edit.is_topic_editable(message), false);
message.topic = "translated: (no topic)";
@ -149,7 +149,7 @@ run_test("is_stream_editable", ({override}) => {
};
override(realm, "realm_allow_message_editing", true);
override(settings_data, "user_can_move_messages_between_streams", () => true);
override(current_user, "is_admin", true);
override(current_user, "is_moderator", true);
assert.equal(message_edit.is_stream_editable(message), false);
@ -166,7 +166,7 @@ run_test("is_stream_editable", ({override}) => {
override(settings_data, "user_can_move_messages_between_streams", () => false);
assert.equal(message_edit.is_stream_editable(message), false);
override(current_user, "is_admin", false);
override(current_user, "is_moderator", false);
assert.equal(message_edit.is_stream_editable(message), false);
override(realm, "realm_move_messages_between_streams_limit_seconds", 259200);

View File

@ -122,7 +122,7 @@ const realm_admin = {
is_owner: false,
is_admin: true,
is_guest: false,
is_moderator: false,
is_moderator: true,
is_bot: false,
role: 200,
};
@ -146,7 +146,7 @@ const realm_owner = {
is_owner: true,
is_admin: true,
is_guest: false,
is_moderator: false,
is_moderator: true,
is_bot: false,
role: 100,
};
@ -686,7 +686,9 @@ test_people("user_type", () => {
people.add_active_user(realm_admin);
people.add_active_user(guest);
people.add_active_user(realm_owner);
console.log("AAAaaAAAAAAAAA");
people.add_active_user(moderator);
console.log("BABSDBSDBSDBBDSBSDB");
people.add_active_user(bot_botson);
assert.equal(people.get_user_type(me.user_id), $t({defaultMessage: "Member"}));
assert.equal(people.get_user_type(realm_admin.user_id), $t({defaultMessage: "Administrator"}));

View File

@ -133,7 +133,7 @@ run_test("updates", ({override}) => {
});
person = people.get_by_email(isaac.email);
assert.equal(person.full_name, "Isaac Newton");
assert.equal(person.is_moderator, false);
assert.equal(person.is_moderator, true);
assert.equal(person.is_admin, true);
assert.equal(person.role, settings_config.user_role_values.admin.code);

View File

@ -1438,7 +1438,6 @@ def check_update_message(
# and the time limit for editing topics is passed, raise an error.
if (
user_profile.realm.move_messages_within_stream_limit_seconds is not None
and not user_profile.is_realm_admin
and not user_profile.is_moderator
):
deadline_seconds = (
@ -1504,7 +1503,6 @@ def check_update_message(
if (
user_profile.realm.move_messages_between_streams_limit_seconds is not None
and not user_profile.is_realm_admin
and not user_profile.is_moderator
):
deadline_seconds = (
@ -1518,7 +1516,6 @@ def check_update_message(
if (
propagate_mode == "change_all"
and not user_profile.is_realm_admin
and not user_profile.is_moderator
and message_edit_request.is_message_moved
and not message_edit_request.topic_resolved

View File

@ -75,6 +75,7 @@ from zerver.lib.users import (
get_data_for_inaccessible_user,
get_users_for_api,
is_administrator_role,
is_moderator_role,
max_message_id_for_user,
)
from zerver.lib.utils import optional_bytes_to_mib
@ -1096,7 +1097,7 @@ def apply_event(
if "role" in person:
state["is_admin"] = is_administrator_role(person["role"])
state["is_owner"] = person["role"] == UserProfile.ROLE_REALM_OWNER
state["is_moderator"] = person["role"] == UserProfile.ROLE_MODERATOR
state["is_moderator"] = is_moderator_role(person["role"])
state["is_guest"] = person["role"] == UserProfile.ROLE_GUEST
# Recompute properties based on is_admin/is_guest
state["can_create_private_streams"] = user_profile.can_create_private_streams()

View File

@ -1234,10 +1234,8 @@ def check_user_has_permission_by_role(
if system_group_name == SystemGroups.ADMINISTRATORS:
return user.is_realm_admin
# is_moderator returns False for realm admins and
# owners.
if system_group_name == SystemGroups.MODERATORS:
return user.is_realm_admin or user.is_moderator
return user.is_moderator
# Handle full members case.
return user.role != UserProfile.ROLE_MEMBER or not user.is_provisional_member

View File

@ -216,6 +216,10 @@ def is_administrator_role(role: int) -> bool:
return role in {UserProfile.ROLE_REALM_ADMINISTRATOR, UserProfile.ROLE_REALM_OWNER}
def is_moderator_role(role: int) -> bool:
return is_administrator_role(role) or role == UserProfile.ROLE_MODERATOR
def bulk_get_cross_realm_bots() -> dict[str, UserProfile]:
emails = list(settings.CROSS_REALM_BOT_EMAILS)

View File

@ -776,7 +776,7 @@ class UserProfile(AbstractBaseUser, PermissionsMixin, UserBaseSettings):
@property
def is_moderator(self) -> bool:
return self.role == UserProfile.ROLE_MODERATOR
return self.is_realm_admin or self.role == UserProfile.ROLE_MODERATOR
@is_moderator.setter
def is_moderator(self, value: bool) -> None:

View File

@ -19208,7 +19208,7 @@ paths:
description: |
Present if `realm_user` is present in `fetch_event_types`.
Whether the current user is an [organization administrator](/api/roles-and-permissions).
Whether the current user is at least an [organization administrator](/api/roles-and-permissions).
is_owner:
type: boolean
description: |
@ -19222,9 +19222,12 @@ paths:
description: |
Present if `realm_user` is present in `fetch_event_types`.
Whether the current user is an [organization moderator](/api/roles-and-permissions).
Whether the current user is at least an [organization moderator](/api/roles-and-permissions).
**Changes**: New in Zulip 4.0 (feature level 60).
**Changes**: Prior to Zulip 11.0 (feature level 380), this was only true
for users whose role was exactly the moderator role.
New in Zulip 4.0 (feature level 60).
is_guest:
type: boolean
description: |

View File

@ -5628,10 +5628,6 @@ class SubscriptionAPITest(ZulipTestCase):
realm, "can_add_subscribers_group", moderators_group, acting_user=None
)
# Moderators, Admins and owners are always full members.
do_change_user_role(self.test_user, UserProfile.ROLE_MODERATOR, acting_user=None)
self.assertFalse(self.test_user.is_provisional_member)
do_change_user_role(self.test_user, UserProfile.ROLE_MEMBER, acting_user=None)
# Make sure that we are checking the permission with a full member,
# as full member is the user just below moderator in the role hierarchy.
@ -5694,6 +5690,15 @@ class SubscriptionAPITest(ZulipTestCase):
)
self.assert_json_error(result, "Insufficient permission")
# Moderators, Admins and owners are always full members.
self.assertTrue(user_profile.is_provisional_member)
do_change_user_role(self.test_user, UserProfile.ROLE_MODERATOR, acting_user=None)
self.assertFalse(self.test_user.is_provisional_member)
do_change_user_role(self.test_user, UserProfile.ROLE_REALM_ADMINISTRATOR, acting_user=None)
self.assertFalse(self.test_user.is_provisional_member)
do_change_user_role(self.test_user, UserProfile.ROLE_REALM_OWNER, acting_user=None)
self.assertFalse(self.test_user.is_provisional_member)
do_set_realm_property(realm, "waiting_period_threshold", 0, acting_user=None)
self.subscribe_via_post(
self.test_user, ["stream2"], {"principals": orjson.dumps([invitee_user_id]).decode()}

View File

@ -116,7 +116,7 @@ class PermissionTest(ZulipTestCase):
self.assertEqual(user_profile.role, UserProfile.ROLE_REALM_ADMINISTRATOR)
user_profile.is_moderator = False
self.assertEqual(user_profile.is_moderator, False)
self.assertEqual(user_profile.is_moderator, True)
self.assertEqual(user_profile.role, UserProfile.ROLE_REALM_ADMINISTRATOR)
user_profile.is_realm_admin = False
@ -593,21 +593,16 @@ class PermissionTest(ZulipTestCase):
user_profile.is_realm_admin
and not user_profile.is_guest
and not user_profile.is_realm_owner
and not user_profile.is_moderator
)
elif role == UserProfile.ROLE_REALM_OWNER:
return (
user_profile.is_realm_owner
and user_profile.is_realm_admin
and not user_profile.is_moderator
and not user_profile.is_guest
)
elif role == UserProfile.ROLE_MODERATOR:
return (
user_profile.is_moderator
and not user_profile.is_realm_owner
and not user_profile.is_realm_admin
and not user_profile.is_guest
return user_profile.is_moderator or (
user_profile.is_realm_admin and not user_profile.is_guest
)
if role == UserProfile.ROLE_MEMBER:

View File

@ -58,10 +58,8 @@ def add_dev_login_context(realm: Realm | None, context: dict[str, Any]) -> None:
context["direct_owners"] = sort([u for u in users if u.is_realm_owner])
context["direct_admins"] = sort([u for u in users if u.is_realm_admin and not u.is_realm_owner])
context["guest_users"] = sort([u for u in users if u.is_guest])
context["direct_moderators"] = sort([u for u in users if u.is_moderator])
context["direct_users"] = sort(
[u for u in users if not (u.is_realm_admin or u.is_guest or u.is_moderator)]
)
context["direct_moderators"] = sort([u for u in users if u.role == UserProfile.ROLE_MODERATOR])
context["direct_users"] = sort([u for u in users if not (u.is_guest or u.is_moderator)])
@csrf_exempt