From 5db53029a590284f63414b2172c6bfd1c661d1d5 Mon Sep 17 00:00:00 2001 From: Vishnu KS Date: Fri, 28 May 2021 16:21:50 +0530 Subject: [PATCH] api: Include is_billing_admin as an attribute in user response. This is sufficiently useful that it should be made available to clients. --- frontend_tests/node_tests/lib/events.js | 1 + frontend_tests/node_tests/people.js | 4 +++ frontend_tests/node_tests/user_events.js | 21 +++++++++++ static/js/user_events.js | 7 ++++ templates/zerver/api/changelog.md | 11 ++++++ version.py | 2 +- zerver/lib/actions.py | 4 +++ zerver/lib/cache.py | 1 + zerver/lib/event_schema.py | 8 +++++ zerver/lib/events.py | 6 +++- zerver/lib/users.py | 1 + zerver/openapi/zulip.yaml | 46 ++++++++++++++++++++++++ zerver/tests/test_custom_profile_data.py | 3 ++ zerver/tests/test_events.py | 13 +++++++ zerver/tests/test_home.py | 4 +++ 15 files changed, 130 insertions(+), 2 deletions(-) diff --git a/frontend_tests/node_tests/lib/events.js b/frontend_tests/node_tests/lib/events.js index cf9650e00b..01288cf6b9 100644 --- a/frontend_tests/node_tests/lib/events.js +++ b/frontend_tests/node_tests/lib/events.js @@ -489,6 +489,7 @@ exports.fixtures = { is_admin: false, is_active: true, is_owner: false, + is_billing_admin: false, role: 400, is_bot: false, is_guest: false, diff --git a/frontend_tests/node_tests/people.js b/frontend_tests/node_tests/people.js index 697eae3a56..e449c98280 100644 --- a/frontend_tests/node_tests/people.js +++ b/frontend_tests/node_tests/people.js @@ -90,6 +90,7 @@ const realm_admin = { is_admin: true, is_guest: false, is_moderator: false, + is_billing_admin: true, is_bot: false, role: 200, }; @@ -102,6 +103,7 @@ const guest = { is_admin: false, is_guest: true, is_moderator: false, + is_billing_admin: false, is_bot: false, role: 600, }; @@ -114,6 +116,7 @@ const realm_owner = { is_admin: true, is_guest: false, is_moderator: false, + is_billing_admin: false, is_bot: false, role: 100, }; @@ -133,6 +136,7 @@ const moderator = { is_owner: false, is_admin: false, is_guest: false, + is_billing_admin: false, is_moderator: true, is_bot: false, role: 300, diff --git a/frontend_tests/node_tests/user_events.js b/frontend_tests/node_tests/user_events.js index 94cc8ae2b8..7bd839714d 100644 --- a/frontend_tests/node_tests/user_events.js +++ b/frontend_tests/node_tests/user_events.js @@ -57,6 +57,7 @@ const me = { user_id: 30, full_name: "Me Myself", is_admin: true, + role: settings_config.user_role_values.member.code, }; function initialize() { @@ -118,6 +119,26 @@ run_test("updates", () => { assert.equal(person.is_owner, true); assert.equal(person.role, settings_config.user_role_values.owner.code); + user_events.update_person({user_id: me.user_id, is_billing_admin: true}); + person = people.get_by_email(me.email); + assert(person.is_billing_admin); + assert.equal(person.role, settings_config.user_role_values.member.code); + assert(page_params.is_billing_admin); + + user_events.update_person({user_id: me.user_id, is_billing_admin: false}); + person = people.get_by_email(me.email); + assert.equal(person.user_id, me.user_id); + assert(!person.is_billing_admin); + assert.equal(person.role, settings_config.user_role_values.member.code); + assert(!page_params.is_billing_admin); + + user_events.update_person({user_id: isaac.user_id, is_billing_admin: false}); + person = people.get_by_email(isaac.email); + assert.equal(person.user_id, isaac.user_id); + assert(!person.is_billing_admin); + assert.equal(person.role, settings_config.user_role_values.owner.code); + assert(!page_params.is_billing_admin); + let user_id; let full_name; message_live_update.update_user_full_name = (user_id_arg, full_name_arg) => { diff --git a/static/js/user_events.js b/static/js/user_events.js index 447995c160..48db800388 100644 --- a/static/js/user_events.js +++ b/static/js/user_events.js @@ -96,6 +96,13 @@ export const update_person = function update(person) { } } + if (Object.prototype.hasOwnProperty.call(person, "is_billing_admin")) { + person_obj.is_billing_admin = person.is_billing_admin; + if (people.is_my_user_id(person.user_id)) { + page_params.is_billing_admin = person_obj.is_billing_admin; + } + } + if (Object.prototype.hasOwnProperty.call(person, "avatar_url")) { const url = person.avatar_url; person_obj.avatar_url = url; diff --git a/templates/zerver/api/changelog.md b/templates/zerver/api/changelog.md index 0f591a065a..70b4e15a2f 100644 --- a/templates/zerver/api/changelog.md +++ b/templates/zerver/api/changelog.md @@ -11,6 +11,17 @@ below features are supported. ## Changes in Zulip 5.0 +**Feature level 73** + +* [`GET /users`](/api/get-users), [`GET /users/{user_id}`](/api/get-user), + [`GET /users/{email}`](/api/get-user-by-email) and + [`GET /users/me`](/api/get-own-user): Added `is_billing_admin` field to + returned user objects. +* [`GET /events`](/api/get-events): Added `is_billing_admin` field to + user objects sent in `realm_user` events. +* [`POST /register`](/api/register-queue): Added `is_billing_admin` field + in the user objects returned in the `realm_users` field. + **Feature level 72** * [`POST /register`](/api/register-queue): Renamed `max_icon_file_size` to diff --git a/version.py b/version.py index 90681232f8..1ba3fb3e43 100644 --- a/version.py +++ b/version.py @@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.4.3" # Changes should be accompanied by documentation explaining what the # new level means in templates/zerver/api/changelog.md, as well as # "**Changes**" entries in the endpoint's documentation in `zulip.yaml`. -API_FEATURE_LEVEL = 72 +API_FEATURE_LEVEL = 73 # 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 diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index b7152ebf08..d2fe26d1c3 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -4562,6 +4562,10 @@ def do_change_user_role( def do_make_user_billing_admin(user_profile: UserProfile) -> None: user_profile.is_billing_admin = True user_profile.save(update_fields=["is_billing_admin"]) + event = dict( + type="realm_user", op="update", person=dict(user_id=user_profile.id, is_billing_admin=True) + ) + send_event(user_profile.realm, event, active_user_ids(user_profile.realm_id)) def do_change_can_forge_sender(user_profile: UserProfile, value: bool) -> None: diff --git a/zerver/lib/cache.py b/zerver/lib/cache.py index d7cd8781ef..35334543de 100644 --- a/zerver/lib/cache.py +++ b/zerver/lib/cache.py @@ -519,6 +519,7 @@ realm_user_dict_fields: List[str] = [ "avatar_version", "is_active", "role", + "is_billing_admin", "is_bot", "realm_id", "timezone", diff --git a/zerver/lib/event_schema.py b/zerver/lib/event_schema.py index 8b6f2afd57..6df982bb01 100644 --- a/zerver/lib/event_schema.py +++ b/zerver/lib/event_schema.py @@ -1010,6 +1010,7 @@ realm_user_type = DictType( ("avatar_version", int), ("full_name", str), ("is_admin", bool), + ("is_billing_admin", bool), ("is_owner", bool), ("is_bot", bool), ("is_guest", bool), @@ -1100,6 +1101,13 @@ realm_user_person_types = dict( ("full_name", str), ], ), + is_billing_admin=DictType( + required_keys=[ + # vertical formatting + ("user_id", int), + ("is_billing_admin", bool), + ], + ), role=DictType( required_keys=[ # vertical formatting diff --git a/zerver/lib/events.py b/zerver/lib/events.py index f2f11a7231..60867d46a6 100644 --- a/zerver/lib/events.py +++ b/zerver/lib/events.py @@ -339,6 +339,7 @@ def fetch_initial_state_data( # restrictions apply to these users as well, and it lets # us avoid unnecessary conditionals. role=UserProfile.ROLE_GUEST, + is_billing_admin=False, avatar_source=UserProfile.AVATAR_FROM_GRAVATAR, # ID=0 is not used in real Zulip databases, ensuring this is unique. id=0, @@ -375,6 +376,7 @@ def fetch_initial_state_data( state["is_owner"] = settings_user.is_realm_owner state["is_moderator"] = settings_user.is_moderator state["is_guest"] = settings_user.is_guest + state["is_billing_admin"] = settings_user.is_billing_admin state["user_id"] = settings_user.id state["enter_sends"] = settings_user.enter_sends state["email"] = settings_user.email @@ -671,7 +673,7 @@ def apply_event( user_profile, include_all_active=user_profile.is_realm_admin ) - for field in ["delivery_email", "email", "full_name"]: + for field in ["delivery_email", "email", "full_name", "is_billing_admin"]: if field in person and field in state: state[field] = person[field] @@ -706,6 +708,8 @@ def apply_event( p["is_admin"] = is_administrator_role(person["role"]) p["is_owner"] = person["role"] == UserProfile.ROLE_REALM_OWNER p["is_guest"] = person["role"] == UserProfile.ROLE_GUEST + if "is_billing_admin" in person: + p["is_billing_admin"] = person["is_billing_admin"] if "custom_profile_field" in person: custom_field_id = person["custom_profile_field"]["id"] custom_field_new_value = person["custom_profile_field"]["value"] diff --git a/zerver/lib/users.py b/zerver/lib/users.py index 309dda19bf..d4a0162f11 100644 --- a/zerver/lib/users.py +++ b/zerver/lib/users.py @@ -395,6 +395,7 @@ def format_user_row( is_admin=is_admin, is_owner=is_owner, is_guest=is_guest, + is_billing_admin=row["is_billing_admin"], role=row["role"], is_bot=is_bot, full_name=row["full_name"], diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 208a00557e..115f57385b 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -396,6 +396,21 @@ paths: - 300 - 400 - 600 + - type: object + additionalProperties: false + description: | + When billing role of a user changes. + properties: + user_id: + type: integer + description: | + The ID of the user affected by this change. + is_billing_admin: + type: boolean + description: | + A boolean specifying whether the user is now a billing administrator. + + **Changes**: New in Zulip 5.0 (feature level 73). - type: object additionalProperties: false description: | @@ -818,6 +833,7 @@ paths: "is_admin": false, "is_owner": false, "is_guest": false, + "is_billing_admin": false, "role": 400, "is_bot": false, "full_name": "full name", @@ -5099,6 +5115,7 @@ paths: "email": "AARON@zulip.com", "is_admin": false, "is_owner": false, + "is_billing_admin": false, "role": 400, "avatar_url": "https://secure.gravatar.com/avatar/818c212b9f8830dfef491b3f7da99a14?d=identicon&version=1", "bot_type": null, @@ -5143,6 +5160,7 @@ paths: "timezone": "", "is_admin": false, "is_owner": false, + "is_billing_admin": false, "role": 400, "avatar_url": "https://secure.gravatar.com/avatar/6d8cad0fd00256e7b40691d27ddfd466?d=identicon&version=1", "is_active": true, @@ -5158,6 +5176,7 @@ paths: "avatar_url": "https://secure.gravatar.com/avatar/7328586831cdbb1627649bd857b1ee8c?d=identicon&version=1", "is_admin": false, "is_owner": false, + "is_billing_admin": false, "role": 400, "user_id": 23, "bot_type": 1, @@ -5407,6 +5426,14 @@ paths: **Changes**: New in Zulip 3.0 (feature level 8). example: false + is_billing_admin: + type: boolean + description: | + A boolean indicating if the requesting user is + a billing administrator. + + **Changes**: New in Zulip 5.0 (feature level 73). + example: false role: type: integer enum: @@ -5496,6 +5523,7 @@ paths: "is_owner": false, "role": 200, "is_guest": false, + "is_billing_admin": false, "is_bot": false, "is_active": true, "timezone": "", @@ -6719,6 +6747,7 @@ paths: "timezone": "", "is_admin": false, "is_owner": false, + "is_billing_admin": false, "role": 400, "avatar_url": "https://secure.gravatar.com/avatar/6d8cad0fd00256e7b40691d27ddfd466?d=identicon&version=1", "is_active": true, @@ -6795,6 +6824,7 @@ paths: "timezone": "", "is_admin": false, "is_owner": false, + "is_billing_admin": false, "role": 400, "avatar_url": "https://secure.gravatar.com/avatar/6d8cad0fd00256e7b40691d27ddfd466?d=identicon&version=1", "is_active": true, @@ -9049,6 +9079,14 @@ paths: Present if `realm_user` is present in `fetch_event_types`. Whether the current user is an [organization owner](/help/roles-and-permissions). + is_billing_admin: + type: boolean + description: | + Present if `realm_user` is present in `fetch_event_types`. + + Whether the current user is a billing administrator. + + **Changes**: New in Zulip 5.0 (feature level 73). is_moderator: type: boolean description: | @@ -9120,6 +9158,7 @@ paths: full_name: {} is_admin: {} is_owner: {} + is_billing_admin: {} role: {} bot_type: {} user_id: {} @@ -11814,6 +11853,7 @@ components: full_name: {} is_admin: {} is_owner: {} + is_billing_admin: {} role: {} bot_type: {} user_id: {} @@ -11872,6 +11912,12 @@ components: If true, is_admin will also be true. **Changes**: New in Zulip 3.0 (feature level 8). + is_billing_admin: + type: boolean + description: | + A boolean specifying whether the user is a billing administrator. + + **Changes**: New in Zulip 5.0 (feature level 73). role: type: integer enum: diff --git a/zerver/tests/test_custom_profile_data.py b/zerver/tests/test_custom_profile_data.py index 15e7ed2eb8..db1fb0e71e 100644 --- a/zerver/tests/test_custom_profile_data.py +++ b/zerver/tests/test_custom_profile_data.py @@ -780,6 +780,7 @@ class ListCustomProfileFieldTest(CustomProfileFieldTestCase): "avatar_version", "is_admin", "is_guest", + "is_billing_admin", "is_bot", "is_owner", "role", @@ -802,6 +803,7 @@ class ListCustomProfileFieldTest(CustomProfileFieldTestCase): "is_guest", "is_bot", "is_owner", + "is_billing_admin", "role", "full_name", "timezone", @@ -834,6 +836,7 @@ class ListCustomProfileFieldTest(CustomProfileFieldTestCase): "is_bot", "is_admin", "is_owner", + "is_billing_admin", "role", "profile_data", "avatar_version", diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index 3cfb451c03..f6a1589ae4 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -58,6 +58,7 @@ from zerver.lib.actions import ( do_deactivate_user, do_delete_messages, do_invite_users, + do_make_user_billing_admin, do_mark_hotspot_as_read, do_mute_topic, do_mute_user, @@ -1289,6 +1290,18 @@ class NormalActionsTest(BaseAction): check_realm_user_update("events[0]", events[0], "role") self.assertEqual(events[0]["person"]["role"], role) + def test_change_is_billing_admin(self) -> None: + reset_emails_in_zulip_realm() + + # Important: We need to refresh from the database here so that + # we don't have a stale UserProfile object with an old value + # for email being passed into this next function. + self.user_profile.refresh_from_db() + + events = self.verify_action(lambda: do_make_user_billing_admin(self.user_profile)) + check_realm_user_update("events[0]", events[0], "is_billing_admin") + self.assertEqual(events[0]["person"]["is_billing_admin"], True) + def test_change_is_owner(self) -> None: reset_emails_in_zulip_realm() diff --git a/zerver/tests/test_home.py b/zerver/tests/test_home.py index 65c1c2faff..19cb133f73 100644 --- a/zerver/tests/test_home.py +++ b/zerver/tests/test_home.py @@ -93,6 +93,7 @@ class HomeTest(ZulipTestCase): "hotspots", "insecure_desktop_app", "is_admin", + "is_billing_admin", "is_guest", "is_moderator", "is_owner", @@ -635,6 +636,7 @@ class HomeTest(ZulipTestCase): is_bot=True, is_admin=False, is_owner=False, + is_billing_admin=False, role=email_gateway_bot.role, is_cross_realm_bot=True, is_guest=False, @@ -650,6 +652,7 @@ class HomeTest(ZulipTestCase): is_bot=True, is_admin=False, is_owner=False, + is_billing_admin=False, role=notification_bot.role, is_cross_realm_bot=True, is_guest=False, @@ -665,6 +668,7 @@ class HomeTest(ZulipTestCase): is_bot=True, is_admin=False, is_owner=False, + is_billing_admin=False, role=welcome_bot.role, is_cross_realm_bot=True, is_guest=False,