Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

custom_profile_fields: Add "editable_by_user" setting. 

This new property allows organization administrators to specify whether
users can modify the custom profile field value on their own account.
This property is configurable for individual fields.

By default, existing and newly created fields have this property set to
true, that is, they allow users to edit the value of the fields.

Fixes part of #22883.

Co-Authored-By: Ujjawal Modi <umodi2003@gmail.com>
This commit is contained in:
tnmkr 2024-08-15 20:08:12 +05:30 committed by Tim Abbott
parent aabecf131c
commit ddecba4e1c
15 changed files with 286 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
api_docs/changelog.md
View File

@ -20,6 +20,14 @@ format used by the Zulip server that they are interacting with.
## Changes in Zulip 10.0
**Feature level 296**:
* [`POST /register`](/api/register-queue), [`GET /events`](/api/get-events),
[`POST /realm/profile_fields`](/api/create-custom-profile-field),
[`GET /realm/profile_fields`](/api/get-custom-profile-fields): Added a new
parameter `editable_by_user` to custom profile field objects, which indicates whether
regular users can edit the value of the profile field on their own account.
**Feature level 295**
* [`GET /export/realm/consents`](/api/get-realm-export-consents): Added

2
version.py
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 = 295 # Last bumped for `/export/realm/consents` endpoint.
API_FEATURE_LEVEL = 296 # Last bumped for `editable_by_user` custom profile field setting.
# 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

2
web/tests/lib/events.js
View File

@ -141,6 +141,7 @@ exports.fixtures = {
order: 1,
display_in_profile_summary: false,
required: false,
editable_by_user: true,
},
{
id: 2,
@ -151,6 +152,7 @@ exports.fixtures = {
order: 2,
display_in_profile_summary: false,
required: false,
editable_by_user: false,
},
],
},

18
zerver/actions/custom_profile_fields.py
View File

@ -27,6 +27,7 @@ def try_add_realm_default_custom_profile_field(
field_subtype: str,
display_in_profile_summary: bool = False,
required: bool = False,
editable_by_user: bool = True,
) -> CustomProfileField:
field_data = DEFAULT_EXTERNAL_ACCOUNTS[field_subtype]
custom_profile_field = CustomProfileField(
@ -37,6 +38,7 @@ def try_add_realm_default_custom_profile_field(
field_data=orjson.dumps(dict(subtype=field_subtype)).decode(),
display_in_profile_summary=display_in_profile_summary,
required=required,
editable_by_user=editable_by_user,
)
custom_profile_field.save()
custom_profile_field.order = custom_profile_field.id
@ -54,6 +56,7 @@ def try_add_realm_custom_profile_field(
field_data: ProfileFieldData | None = None,
display_in_profile_summary: bool = False,
required: bool = False,
editable_by_user: bool = True,
) -> CustomProfileField:
custom_profile_field = CustomProfileField(
realm=realm,
@ -61,6 +64,7 @@ def try_add_realm_custom_profile_field(
field_type=field_type,
display_in_profile_summary=display_in_profile_summary,
required=required,
editable_by_user=editable_by_user,
)
custom_profile_field.hint = hint
if custom_profile_field.field_type in (
@ -110,6 +114,7 @@ def try_update_realm_custom_profile_field(
field_data: ProfileFieldData | None = None,
display_in_profile_summary: bool | None = None,
required: bool | None = None,
editable_by_user: bool | None = None,
) -> None:
if name is not None:
field.name = name
@ -117,6 +122,8 @@ def try_update_realm_custom_profile_field(
field.hint = hint
if required is not None:
field.required = required
if editable_by_user is not None:
field.editable_by_user = editable_by_user
if display_in_profile_summary is not None:
field.display_in_profile_summary = display_in_profile_summary
@ -204,9 +211,18 @@ def do_update_user_custom_profile_data_if_changed(
@transaction.atomic(durable=True)
def check_remove_custom_profile_field_value(user_profile: UserProfile, field_id: int) -> None:
def check_remove_custom_profile_field_value(
user_profile: UserProfile, field_id: int, acting_user: UserProfile
) -> None:
try:
custom_profile_field = CustomProfileField.objects.get(realm=user_profile.realm, id=field_id)
if not acting_user.is_realm_admin and not custom_profile_field.editable_by_user:
raise JsonableError(
_(
"You are not allowed to change this field. Contact an administrator to update it."
)
)
field_value = CustomProfileFieldValue.objects.get(
field=custom_profile_field, user_profile=user_profile
)

1
zerver/lib/event_schema.py
View File

@ -171,6 +171,7 @@ custom_profile_field_type = DictType(
("field_data", str),
("order", int),
("required", bool),
("editable_by_user", bool),
],
optional_keys=[
("display_in_profile_summary", bool),

1
zerver/lib/types.py
View File

@ -25,6 +25,7 @@ class ProfileDataElementBase(TypedDict, total=False):
hint: str
display_in_profile_summary: bool
required: bool
editable_by_user: bool
field_data: str
order: int

9
zerver/lib/users.py
View File

@ -479,7 +479,7 @@ def validate_user_custom_profile_field(
def validate_user_custom_profile_data(
realm_id: int, profile_data: list[ProfileDataElementUpdateDict]
realm_id: int, profile_data: list[ProfileDataElementUpdateDict], acting_user: UserProfile
) -> None:
# This function validate all custom field values according to their field type.
for item in profile_data:
@ -489,6 +489,13 @@ def validate_user_custom_profile_data(
except CustomProfileField.DoesNotExist:
raise JsonableError(_("Field id {id} not found.").format(id=field_id))
if not acting_user.is_realm_admin and not field.editable_by_user:
raise JsonableError(
_(
"You are not allowed to change this field. Contact an administrator to update it."
)
)
try:
validate_user_custom_profile_field(realm_id, field, item["value"])
except ValidationError as error:

17
zerver/migrations/0586_customprofilefield_editable_by_user.py Normal file
View File

@ -0,0 +1,17 @@
# Generated by Django 5.0.6 on 2024-06-29 20:06
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
("zerver", "0585_userprofile_allow_private_data_export_and_more"),
]
operations = [
migrations.AddField(
model_name="customprofilefield",
name="editable_by_user",
field=models.BooleanField(db_default=True, default=True),
),
]

4
zerver/models/custom_profile_fields.py
View File

@ -82,6 +82,9 @@ class CustomProfileField(models.Model):
display_in_profile_summary = models.BooleanField(default=False)
required = models.BooleanField(default=False)
# Whether regular users can edit this field on their own account.
editable_by_user = models.BooleanField(default=True, db_default=True)
SHORT_TEXT = 1
LONG_TEXT = 2
SELECT = 3
@ -170,6 +173,7 @@ class CustomProfileField(models.Model):
"field_data": self.field_data,
"order": self.order,
"required": self.required,
"editable_by_user": self.editable_by_user,
}
if self.display_in_profile_summary:
data_as_dict["display_in_profile_summary"] = True

40
zerver/openapi/zulip.yaml
View File

@ -1866,6 +1866,7 @@ paths:
"field_data": "",
"order": 1,
"required": true,
"editable_by_user": true,
},
{
"id": 2,
@ -1875,6 +1876,7 @@ paths:
"field_data": "",
"order": 2,
"required": true,
"editable_by_user": true,
},
{
"id": 3,
@ -1884,6 +1886,7 @@ paths:
"field_data": "",
"order": 3,
"required": false,
"editable_by_user": true,
},
{
"id": 4,
@ -1894,6 +1897,7 @@ paths:
"order": 4,
"display_in_profile_summary": true,
"required": true,
"editable_by_user": true,
},
{
"id": 5,
@ -1903,6 +1907,7 @@ paths:
"field_data": "",
"order": 5,
"required": false,
"editable_by_user": false,
},
{
"id": 6,
@ -1913,6 +1918,7 @@ paths:
"order": 6,
"display_in_profile_summary": true,
"required": false,
"editable_by_user": true,
},
{
"id": 7,
@ -1922,6 +1928,7 @@ paths:
"field_data": "",
"order": 7,
"required": true,
"editable_by_user": false,
},
{
"id": 8,
@ -1931,6 +1938,7 @@ paths:
"field_data": '{"subtype":"github"}',
"order": 8,
"required": true,
"editable_by_user": true,
},
],
"id": 0,
@ -11060,6 +11068,7 @@ paths:
"field_data": "",
"order": 1,
"required": true,
"editable_by_user": false,
},
{
"id": 2,
@ -11069,6 +11078,7 @@ paths:
"field_data": "",
"order": 2,
"required": true,
"editable_by_user": true,
},
{
"id": 3,
@ -11078,6 +11088,7 @@ paths:
"field_data": "",
"order": 3,
"required": false,
"editable_by_user": true,
},
{
"id": 4,
@ -11088,6 +11099,7 @@ paths:
"order": 4,
"display_in_profile_summary": true,
"required": true,
"editable_by_user": true,
},
{
"id": 5,
@ -11097,6 +11109,7 @@ paths:
"field_data": "",
"order": 5,
"required": false,
"editable_by_user": false,
},
{
"id": 6,
@ -11107,6 +11120,7 @@ paths:
"order": 6,
"display_in_profile_summary": true,
"required": false,
"editable_by_user": true,
},
{
"id": 7,
@ -11116,6 +11130,7 @@ paths:
"field_data": "",
"order": 7,
"required": true,
"editable_by_user": false,
},
{
"id": 8,
@ -11125,6 +11140,7 @@ paths:
"field_data": '{"subtype":"github"}',
"order": 8,
"required": true,
"editable_by_user": true,
},
{
"id": 9,
@ -11133,6 +11149,7 @@ paths:
"hint": "What pronouns should people use to refer to you?",
"order": 9,
"required": false,
"editable_by_user": true,
},
],
}
@ -11262,6 +11279,16 @@ paths:
**Changes**: New in Zulip 9.0 (feature level 244).
type: boolean
example: true
editable_by_user:
description: |
Whether regular users can edit this profile field on their own account.
Note that organization administrators can edit custom profile fields for any user
regardless of this setting.
**Changes**: New in Zulip 10.0 (feature level 296).
type: boolean
example: true
required:
- field_type
encoding:
@ -11273,6 +11300,8 @@ paths:
contentType: application/json
required:
contentType: application/json
editable_by_user:
contentType: application/json
responses:
"200":
description: Success.
@ -21180,6 +21209,16 @@ components:
banner to any user who has not set a value for a required field.
**Changes**: New in Zulip 9.0 (feature level 244).
editable_by_user:
type: boolean
description: |
Whether regular users can edit this profile field on their own account.
Note that organization administrators can edit custom profile fields for any user
regardless of this setting.
**Changes**: New in Zulip 10.0 (feature level 296).
default: true
required:
- id
- type
@ -21187,6 +21226,7 @@ components:
- name
- hint
- required
- editable_by_user
OnboardingStep:
type: object
additionalProperties: false

162
zerver/tests/test_custom_profile_data.py
View File

@ -454,6 +454,82 @@ class DeleteCustomProfileFieldTest(CustomProfileFieldTestCase):
self.assertFalse(self.custom_field_exists_in_realm(field.id))
self.assertEqual(user_profile.customprofilefieldvalue_set.count(), self.original_count - 1)
def test_delete_value_with_editable_by_user(self) -> None:
iago = self.example_user("iago")
hamlet = self.example_user("hamlet")
realm = iago.realm
self.login("hamlet")
biography_custom_field = CustomProfileField.objects.get(name="Biography", realm=realm)
birthday_custom_field = CustomProfileField.objects.get(name="Birthday", realm=realm)
# Set and assert our initial state.
data = {}
data["editable_by_user"] = "false"
result = self.api_patch(
iago, f"/api/v1/realm/profile_fields/{birthday_custom_field.id}", info=data
)
self.assert_json_success(result)
birthday_custom_field.refresh_from_db()
self.assertFalse(birthday_custom_field.editable_by_user)
self.assertTrue(biography_custom_field.editable_by_user)
self.assertTrue(
CustomProfileFieldValue.objects.filter(
user_profile=iago, field=birthday_custom_field
).exists()
)
self.assertTrue(
CustomProfileFieldValue.objects.filter(
user_profile=hamlet, field=birthday_custom_field
).exists()
)
self.assertTrue(
CustomProfileFieldValue.objects.filter(
user_profile=hamlet, field=biography_custom_field
).exists()
)
# Users can only delete fields where editable_by_user is true.
result = self.client_delete(
"/json/users/me/profile_data",
{"data": orjson.dumps([biography_custom_field.id]).decode()},
)
self.assert_json_success(result)
self.assertFalse(
CustomProfileFieldValue.objects.filter(
user_profile=hamlet, field=biography_custom_field
).exists()
)
result = self.client_delete(
"/json/users/me/profile_data",
{"data": orjson.dumps([birthday_custom_field.id]).decode()},
)
self.assert_json_error(
result,
"You are not allowed to change this field. Contact an administrator to update it.",
)
self.assertTrue(
CustomProfileFieldValue.objects.filter(
user_profile=hamlet, field=birthday_custom_field
).exists()
)
# Admins can always delete field values regardless of editable_by_user.
result = self.api_delete(
iago,
"/api/v1/users/me/profile_data",
{"data": orjson.dumps([birthday_custom_field.id]).decode()},
)
self.assert_json_success(result)
self.assertFalse(
CustomProfileFieldValue.objects.filter(
user_profile=iago, field=birthday_custom_field
).exists()
)
class UpdateCustomProfileFieldTest(CustomProfileFieldTestCase):
def test_update(self) -> None:
@ -497,6 +573,15 @@ class UpdateCustomProfileFieldTest(CustomProfileFieldTestCase):
msg = "required is not valid JSON"
self.assert_json_error(result, msg)
result = self.client_patch(
f"/json/realm/profile_fields/{field.id}",
info={
"editable_by_user": "invalid value",
},
)
msg = "editable_by_user is not valid JSON"
self.assert_json_error(result, msg)
result = self.client_patch(
f"/json/realm/profile_fields/{field.id}",
info={
@ -504,6 +589,7 @@ class UpdateCustomProfileFieldTest(CustomProfileFieldTestCase):
"hint": "New contact number",
"display_in_profile_summary": "true",
"required": "true",
"editable_by_user": "false",
},
)
self.assert_json_success(result)
@ -514,8 +600,9 @@ class UpdateCustomProfileFieldTest(CustomProfileFieldTestCase):
self.assertEqual(field.field_type, CustomProfileField.SHORT_TEXT)
self.assertEqual(field.display_in_profile_summary, True)
self.assertEqual(field.required, True)
self.assertEqual(field.editable_by_user, False)
# Not sending required should not set it to false.
# Not sending required or editable_by_user should not reset their value to default.
result = self.client_patch(
f"/json/realm/profile_fields/{field.id}",
info={
@ -526,6 +613,7 @@ class UpdateCustomProfileFieldTest(CustomProfileFieldTestCase):
field.refresh_from_db()
self.assertEqual(field.hint, "New hint")
self.assertEqual(field.required, True)
self.assertEqual(field.editable_by_user, False)
result = self.client_patch(
f"/json/realm/profile_fields/{field.id}",
@ -942,6 +1030,78 @@ class UpdateCustomProfileFieldTest(CustomProfileFieldTestCase):
)
self.assert_json_error(result, "Default custom field cannot be updated.")
def assert_profile_field_value(
self, user: UserProfile, field_id: int, field_value: str | None
) -> None:
for field_dict in user.profile_data():
if field_dict["id"] == field_id:
self.assertEqual(field_dict["value"], field_value)
def test_update_with_editable_by_user(self) -> None:
iago = self.example_user("iago")
aaron = self.example_user("aaron")
self.login("aaron")
# Create field with editable_by_user = false
realm_profile_field_data: dict[str, Any] = {}
realm_profile_field_data["name"] = "Dummy field"
realm_profile_field_data["field_type"] = CustomProfileField.SHORT_TEXT
realm_profile_field_data["editable_by_user"] = "false"
result = self.api_post(iago, "/api/v1/realm/profile_fields", info=realm_profile_field_data)
result_json = self.assert_json_success(result)
restricted_field_id = result_json["id"]
field_data = [
{
"id": restricted_field_id,
"value": "test",
}
]
# Admins can always change their own fields
self.assert_profile_field_value(iago, restricted_field_id, None)
result = self.api_patch(
iago, "/api/v1/users/me/profile_data", {"data": orjson.dumps(field_data).decode()}
)
self.assert_json_success(result)
self.assert_profile_field_value(iago, restricted_field_id, "test")
# Admins can always change fields of others
self.assert_profile_field_value(aaron, restricted_field_id, None)
result = self.api_patch(
iago, f"/api/v1/users/{aaron.id}", {"profile_data": orjson.dumps(field_data).decode()}
)
self.assert_json_success(result)
self.assert_profile_field_value(aaron, restricted_field_id, "test")
# Users cannot update field value when editable_by_user is false.
self.assert_profile_field_value(aaron, restricted_field_id, "test")
result = self.client_patch(
"/json/users/me/profile_data", {"data": orjson.dumps(field_data).decode()}
)
self.assert_json_error(
result,
"You are not allowed to change this field. Contact an administrator to update it.",
)
self.assert_profile_field_value(aaron, restricted_field_id, "test")
# Change editable_by_user to true.
data = {}
data["editable_by_user"] = "true"
result = self.api_patch(
iago, f"/api/v1/realm/profile_fields/{restricted_field_id}", info=data
)
self.assert_json_success(result)
# Users can update field value when editable_by_user is true
self.assert_profile_field_value(aaron, restricted_field_id, "test")
field_data[0]["value"] = "test2"
result = self.client_patch(
"/json/users/me/profile_data", {"data": orjson.dumps(field_data).decode()}
)
self.assert_json_success(result)
self.assert_profile_field_value(aaron, restricted_field_id, "test2")
class ListCustomProfileFieldTest(CustomProfileFieldTestCase):
def test_list(self) -> None:

4
zerver/tests/test_events.py
View File

@ -1476,7 +1476,9 @@ class NormalActionsTest(BaseAction):
# Test event for removing custom profile data
with self.verify_action() as events:
check_remove_custom_profile_field_value(self.user_profile, field_id)
check_remove_custom_profile_field_value(
self.user_profile, field_id, acting_user=self.user_profile
)
check_realm_user_update("events[0]", events[0], "custom_profile_field")
self.assertEqual(events[0]["person"]["custom_profile_field"].keys(), {"id", "value"})

12
zerver/tests/test_users.py
View File

@ -752,6 +752,18 @@ class PermissionTest(ZulipTestCase):
new_profile_data = []
cordelia = self.example_user("cordelia")
# Setting editable_by_user to false shouldn't affect admin's ability
# to modify the profile field for other users.
biography_field = CustomProfileField.objects.get(name="Biography", realm=realm)
data = {}
data["editable_by_user"] = "false"
result = self.client_patch(f"/json/realm/profile_fields/{biography_field.id}", info=data)
self.assert_json_success(result)
biography_field.refresh_from_db()
self.assertFalse(biography_field.editable_by_user)
# Test for all type of data
fields = {
"Phone number": "short text data",

9
zerver/views/custom_profile_fields.py
View File

@ -177,6 +177,7 @@ def create_realm_custom_profile_field(
field_type: Json[int],
display_in_profile_summary: Json[bool] = False,
required: Json[bool] = False,
editable_by_user: Json[bool] = True,
) -> HttpResponse:
if field_data is None:
field_data = {}
@ -195,6 +196,7 @@ def create_realm_custom_profile_field(
field_subtype=field_subtype,
display_in_profile_summary=display_in_profile_summary,
required=required,
editable_by_user=editable_by_user,
)
return json_success(request, data={"id": field.id})
else:
@ -206,6 +208,7 @@ def create_realm_custom_profile_field(
hint=hint,
display_in_profile_summary=display_in_profile_summary,
required=required,
editable_by_user=editable_by_user,
)
return json_success(request, data={"id": field.id})
except IntegrityError:
@ -237,6 +240,7 @@ def update_realm_custom_profile_field(
field_data: Json[ProfileFieldData] | None = None,
required: Json[bool] | None = None,
display_in_profile_summary: Json[bool] | None = None,
editable_by_user: Json[bool] | None = None,
) -> HttpResponse:
realm = user_profile.realm
try:
@ -276,6 +280,7 @@ def update_realm_custom_profile_field(
field_data=field_data,
display_in_profile_summary=display_in_profile_summary,
required=required,
editable_by_user=editable_by_user,
)
except IntegrityError:
raise JsonableError(_("A field with that label already exists."))
@ -303,7 +308,7 @@ def remove_user_custom_profile_data(
data: Json[list[int]],
) -> HttpResponse:
for field_id in data:
check_remove_custom_profile_field_value(user_profile, field_id)
check_remove_custom_profile_field_value(user_profile, field_id, acting_user=user_profile)
return json_success(request)
@ -315,7 +320,7 @@ def update_user_custom_profile_data(
*,
data: Json[list[ProfileDataElementUpdateDict]],
) -> HttpResponse:
validate_user_custom_profile_data(user_profile.realm.id, data)
validate_user_custom_profile_data(user_profile.realm.id, data, acting_user=user_profile)
do_update_user_custom_profile_data_if_changed(user_profile, data)
# We need to call this explicitly otherwise constraints are not check
return json_success(request)

6
zerver/views/users.py
View File

@ -246,7 +246,7 @@ def update_user_backend(
assert not isinstance(entry.value, int)
if entry.value is None or not entry.value:
field_id = entry.id
check_remove_custom_profile_field_value(target, field_id)
check_remove_custom_profile_field_value(target, field_id, acting_user=user_profile)
else:
clean_profile_data.append(
{
@ -254,7 +254,9 @@ def update_user_backend(
"value": entry.value,
}
)
validate_user_custom_profile_data(target.realm.id, clean_profile_data)
validate_user_custom_profile_data(
target.realm.id, clean_profile_data, acting_user=user_profile
)
do_update_user_custom_profile_data_if_changed(target, clean_profile_data)
return json_success(request)