users: Modify update user API endpoint to accept role as parameter.

This commit changes the update user API endpoint to accept role
as parameter instead of the bool parameters is_guest and is_admin.

User role dropdown in user info modal is also modified to use
"dropdown_options_widget".

Modified by tabbott to document the API change.

static/js/settings_config.js

@@ -158,6 +158,21 @@ exports.msg_delete_limit_dropdown_values = time_limit_dropdown_values;
 
 exports.retain_message_forever = -1;
 
+exports.user_role_values = {
+    guest: {
+        code: 600,
+        description: i18n.t("Guest"),
+    },
+    member: {
+        code: 400,
+        description: i18n.t("Member"),
+    },
+    admin: {
+        code: 200,
+        description: i18n.t("Administrator"),
+    },
+};
+
 // NOTIFICATIONS
 
 exports.general_notifications_table_labels = {

static/js/settings_users.js

@@ -1,4 +1,5 @@
 const settings_data = require("./settings_data");
+const settings_config = require("./settings_config");
 const render_admin_user_list = require("../templates/admin_user_list.hbs");
 const render_admin_human_form = require('../templates/admin_human_form.hbs');
 const render_admin_bot_form = require('../templates/admin_bot_form.hbs');
@@ -66,6 +67,16 @@ function get_user_info_row(user_id) {
     return $("tr.user_row[data-user-id='" + user_id + "']");
 }
 
+function set_user_role_dropdown(person) {
+    let role_value = settings_config.user_role_values.member.code;
+    if (person.is_admin) {
+        role_value = settings_config.user_role_values.admin.code;
+    } else if (person.is_guest) {
+        role_value = settings_config.user_role_values.guest.code;
+    }
+    $('#user-role-select').val(role_value);
+}
+
 function update_view_on_deactivate(row) {
     const button = row.find("button.deactivate");
     const user_role = row.find(".user_role");
@@ -377,14 +388,13 @@ function open_human_form(person) {
         user_id: user_id,
         email: person.email,
         full_name: person.full_name,
-        is_admin: person.is_admin,
-        is_guest: person.is_guest,
-        is_member: !person.is_admin && !person.is_guest,
+        user_role_values: settings_config.user_role_values,
     });
     const div = $(html);
     const modal_container = $('#user-info-form-modal-container');
     modal_container.empty().append(div);
     overlays.open_modal('#admin-human-form');
+    set_user_role_dropdown(person);
 
     const element = "#admin-human-form .custom-profile-field-form";
     $(element).html("");
@@ -587,15 +597,14 @@ function handle_human_form(tbody, status_field) {
             e.preventDefault();
             e.stopPropagation();
 
-            const user_role_select_value = modal.find('#user-role-select').val();
+            const role = parseInt(modal.find('#user-role-select').val().trim(), 10);
             const full_name = modal.find("input[name='full_name']");
             const profile_data = get_human_profile_data(fields_user_pills);
 
             const url = "/json/users/" + encodeURIComponent(user_id);
             const data = {
                 full_name: JSON.stringify(full_name.val()),
-                is_admin: JSON.stringify(user_role_select_value === 'admin'),
-                is_guest: JSON.stringify(user_role_select_value === 'guest'),
+                role: JSON.stringify(role),
                 profile_data: JSON.stringify(profile_data),
             };
 

static/templates/admin_human_form.hbs

@@ -17,10 +17,8 @@
                 </div>
                 <div class="input-group">
                     <label class="input-label" for="user-role-select">{{t 'User role' }}</label>
-                    <select name="user-role-select" id="user-role-select">
-                        <option value="guest" {{#if is_guest}}selected{{/if}}>{{t 'Guest'}}</option>
-                        <option value="regular" {{#if is_member}}selected{{/if}}>{{t 'Member' }}</option>
-                        <option value="admin" {{#if is_admin}}selected{{/if}}>{{t 'Administrator' }}</option>
+                    <select name="user-role-select" id="user-role-select" data-setting-widget-type="number">
+                        {{> settings/dropdown_options_widget option_values=user_role_values}}
                     </select>
                 </div>
                 <div class="custom-profile-field-form"></div>

templates/zerver/api/changelog.md

@@ -16,6 +16,9 @@ below features are supported.
   the user; use the `user_id` field instead.  Previously, some (but
   not all) events of these types contained an `email` key in addition to
   to `user_id`) for identifying the modified user.
+* [`PATCH /users/{user_id}`](/api/update-user): The `is_admin` and
+  `is_guest` parameters were removed in favor of the more general
+  `role` parameter for specifying a change in user role.
 
 **Feature level 6**
 * [`GET /events`](/api/get-events-from-queue): `realm_user` events to

zerver/models.py

@@ -880,6 +880,12 @@ class UserProfile(AbstractBaseUser, PermissionsMixin):
     ROLE_GUEST = 600
     role: int = models.PositiveSmallIntegerField(default=ROLE_MEMBER, db_index=True)
 
+    ROLE_TYPES = [
+        ROLE_REALM_ADMINISTRATOR,
+        ROLE_MEMBER,
+        ROLE_GUEST,
+    ]
+
     # Whether the user has been "soft-deactivated" due to weeks of inactivity.
     # For these users we avoid doing UserMessage table work, as an optimization
     # for large Zulip organizations with lots of single-visit users.

zerver/openapi/zulip.yaml

@@ -1579,21 +1579,24 @@ paths:
               type: string
             example: NewName
         required: false
-      - name: is_admin
+      - name: role
         in: query
         description: |
-          Whether the target user is an administrator.
+          New [role](/help/roles-and-permissions) for the user.  Supported roles include:
+            * Organization owner: 100
+            * Organization administrator: 200
+            * Member: 400
+            * Guest: 600
+
+          Only organization owners can add or remove the owner permission.
+
+          The owner permission cannot be removed from the only organization owner.
+
+          **Changes**: New in Zulip 2.2 (feature level 8), replacing the previous
+          pair of `is_admin` and `is_guest` boolean parameters.
         schema:
-          type: boolean
-        example: false
-        required: false
-      - name: is_guest
-        in: query
-        description: |
-          Whether the target user is a guest.
-        schema:
-          type: boolean
-        example: true
+          type: integer
+        example: 400
         required: false
       - name: profile_data
         in: query

zerver/tests/test_users.py

@@ -138,7 +138,7 @@ class PermissionTest(ZulipTestCase):
         self.assertFalse(othello_dict['is_admin'])
 
         # Giveth
-        req = dict(is_admin=ujson.dumps(True))
+        req = dict(role=ujson.dumps(UserProfile.ROLE_REALM_ADMINISTRATOR))
 
         events: List[Mapping[str, Any]] = []
         with tornado_redirected_to_list(events):
@@ -151,7 +151,7 @@ class PermissionTest(ZulipTestCase):
         self.assertEqual(person['is_admin'], True)
 
         # Taketh away
-        req = dict(is_admin=ujson.dumps(False))
+        req = dict(role=ujson.dumps(UserProfile.ROLE_MEMBER))
         events = []
         with tornado_redirected_to_list(events):
             result = self.client_patch('/json/users/{}'.format(othello.id), req)
@@ -164,7 +164,7 @@ class PermissionTest(ZulipTestCase):
 
         # Cannot take away from last admin
         self.login('iago')
-        req = dict(is_admin=ujson.dumps(False))
+        req = dict(role=ujson.dumps(UserProfile.ROLE_MEMBER))
         events = []
         with tornado_redirected_to_list(events):
             result = self.client_patch('/json/users/{}'.format(desdemona.id), req)
@@ -355,15 +355,7 @@ class PermissionTest(ZulipTestCase):
         hamlet = self.example_user("hamlet")
         self.assertFalse(hamlet.is_guest)
 
-        # Test failure of making user both admin and guest
-        req = dict(is_guest=ujson.dumps(True), is_admin=ujson.dumps(True))
-        result = self.client_patch('/json/users/{}'.format(hamlet.id), req)
-        self.assert_json_error(result, 'Guests cannot be organization administrators')
-        self.assertFalse(hamlet.is_guest)
-        self.assertFalse(hamlet.is_realm_admin)
-        hamlet = self.example_user("hamlet")
-
-        req = dict(is_guest=ujson.dumps(True))
+        req = dict(role=ujson.dumps(UserProfile.ROLE_GUEST))
         events: List[Mapping[str, Any]] = []
         with tornado_redirected_to_list(events):
             result = self.client_patch('/json/users/{}'.format(hamlet.id), req)
@@ -382,7 +374,7 @@ class PermissionTest(ZulipTestCase):
 
         polonius = self.example_user("polonius")
         self.assertTrue(polonius.is_guest)
-        req = dict(is_guest=ujson.dumps(False))
+        req = dict(role=ujson.dumps(UserProfile.ROLE_MEMBER))
         events: List[Mapping[str, Any]] = []
         with tornado_redirected_to_list(events):
             result = self.client_patch('/json/users/{}'.format(polonius.id), req)
@@ -402,15 +394,10 @@ class PermissionTest(ZulipTestCase):
         self.assertFalse(hamlet.is_guest)
         self.assertTrue(hamlet.is_realm_admin)
 
-        # Test failure of making a admin to guest without revoking admin status
-        req = dict(is_guest=ujson.dumps(True))
-        result = self.client_patch('/json/users/{}'.format(hamlet.id), req)
-        self.assert_json_error(result, 'Guests cannot be organization administrators')
-
         # Test changing a user from admin to guest and revoking admin status
         hamlet = self.example_user("hamlet")
         self.assertFalse(hamlet.is_guest)
-        req = dict(is_admin=ujson.dumps(False), is_guest=ujson.dumps(True))
+        req = dict(role=ujson.dumps(UserProfile.ROLE_GUEST))
         events: List[Mapping[str, Any]] = []
         with tornado_redirected_to_list(events):
             result = self.client_patch('/json/users/{}'.format(hamlet.id), req)
@@ -435,15 +422,10 @@ class PermissionTest(ZulipTestCase):
         self.assertTrue(polonius.is_guest)
         self.assertFalse(polonius.is_realm_admin)
 
-        # Test failure of making a guest to admin without revoking guest status
-        req = dict(is_admin=ujson.dumps(True))
-        result = self.client_patch('/json/users/{}'.format(polonius.id), req)
-        self.assert_json_error(result, 'Guests cannot be organization administrators')
-
         # Test changing a user from guest to admin and revoking guest status
         polonius = self.example_user("polonius")
         self.assertFalse(polonius.is_realm_admin)
-        req = dict(is_admin=ujson.dumps(True), is_guest=ujson.dumps(False))
+        req = dict(role=ujson.dumps(UserProfile.ROLE_REALM_ADMINISTRATOR))
         events: List[Mapping[str, Any]] = []
         with tornado_redirected_to_list(events):
             result = self.client_patch('/json/users/{}'.format(polonius.id), req)

zerver/views/users.py

@@ -24,7 +24,8 @@ from zerver.lib.request import has_request_variables, REQ
 from zerver.lib.response import json_error, json_success
 from zerver.lib.streams import access_stream_by_name
 from zerver.lib.upload import upload_avatar_image
-from zerver.lib.validator import check_bool, check_string, check_int, check_url, check_dict, check_list
+from zerver.lib.validator import check_bool, check_string, check_int, check_url, check_dict, check_list, \
+    check_int_in
 from zerver.lib.url_encoding import add_query_arg_to_redirect_url
 from zerver.lib.users import check_valid_bot_type, check_bot_creation_policy, \
     check_full_name, check_short_name, check_valid_interface_type, check_valid_bot_config, \
@@ -80,36 +81,16 @@ def reactivate_user_backend(request: HttpRequest, user_profile: UserProfile,
 @has_request_variables
 def update_user_backend(request: HttpRequest, user_profile: UserProfile, user_id: int,
                         full_name: Optional[str]=REQ(default="", validator=check_string),
-                        is_admin: Optional[bool]=REQ(default=None, validator=check_bool),
-                        is_guest: Optional[bool]=REQ(default=None, validator=check_bool),
+                        role: Optional[int]=REQ(default=None, validator=check_int_in(
+                            UserProfile.ROLE_TYPES)),
                         profile_data: Optional[List[Dict[str, Union[int, str, List[int]]]]]=
                         REQ(default=None,
                             validator=check_list(check_dict([('id', check_int)])))) -> HttpResponse:
     target = access_user_by_id(user_profile, user_id, allow_deactivated=True, allow_bots=True)
 
-    # Historically, UserProfile had two fields, is_guest and is_realm_admin.
-    # This condition protected against situations where update_user_backend
-    # could cause both is_guest and is_realm_admin to be set.
-    # Once we update the frontend to just send a 'role' value, we can remove this check.
-    if (((is_guest is None and target.is_guest) or is_guest) and
-            ((is_admin is None and target.is_realm_admin) or is_admin)):
-        return json_error(_("Guests cannot be organization administrators"))
-
-    role = None
-    if is_admin is not None and target.is_realm_admin != is_admin:
-        if not is_admin and check_last_admin(user_profile):
-            return json_error(_('Cannot remove the only organization administrator'))
-        role = UserProfile.ROLE_MEMBER
-        if is_admin:
-            role = UserProfile.ROLE_REALM_ADMINISTRATOR
-
-    if is_guest is not None and target.is_guest != is_guest:
-        if is_guest:
-            role = UserProfile.ROLE_GUEST
-        if role is None:
-            role = UserProfile.ROLE_MEMBER
-
     if role is not None and target.role != role:
+        if target.role == UserProfile.ROLE_REALM_ADMINISTRATOR and check_last_admin(user_profile):
+            return json_error(_('Cannot remove the only organization administrator'))
         do_change_user_role(target, role)
 
     if (full_name is not None and target.full_name != full_name and