custom_profile_fields: Make name & hint optional during update.

Fixes #23022.
Added a new validation function that validates updates to custom
profile fields.
This commit is contained in:
Shubham Padia 2024-03-30 14:26:26 +05:30 committed by Tim Abbott
parent 4a2a9176c2
commit c6c392bcc7
5 changed files with 60 additions and 29 deletions

View File

@ -20,6 +20,13 @@ format used by the Zulip server that they are interacting with.
## Changes in Zulip 9.0 ## Changes in Zulip 9.0
**Feature level 252**
* `PATCH /realm/profile_fields/{field_id}`: `name` and `hint` fields are now
optional during an update. Previously we required the clients to populate
the fields in the PATCH request even if there was no change to those fields'
values.
**Feature level 251** **Feature level 251**
* [`POST /register`](/api/register-queue): Fixed `realm_upload_quota_mib` * [`POST /register`](/api/register-queue): Fixed `realm_upload_quota_mib`

View File

@ -101,14 +101,17 @@ def remove_custom_profile_field_value_if_required(
def try_update_realm_custom_profile_field( def try_update_realm_custom_profile_field(
realm: Realm, realm: Realm,
field: CustomProfileField, field: CustomProfileField,
name: str, name: Optional[str] = None,
hint: str = "", hint: Optional[str] = None,
field_data: Optional[ProfileFieldData] = None, field_data: Optional[ProfileFieldData] = None,
display_in_profile_summary: bool = False, display_in_profile_summary: bool = False,
required: bool = False, required: bool = False,
) -> None: ) -> None:
field.name = name if name is not None:
field.hint = hint field.name = name
if hint is not None:
field.hint = hint
field.display_in_profile_summary = display_in_profile_summary field.display_in_profile_summary = display_in_profile_summary
field.required = required field.required = required
if field.field_type in (CustomProfileField.SELECT, CustomProfileField.EXTERNAL_ACCOUNT): if field.field_type in (CustomProfileField.SELECT, CustomProfileField.EXTERNAL_ACCOUNT):

View File

@ -494,7 +494,6 @@ class UpdateCustomProfileFieldTest(CustomProfileFieldTestCase):
result = self.client_patch( result = self.client_patch(
f"/json/realm/profile_fields/{field.id}", f"/json/realm/profile_fields/{field.id}",
info={ info={
"name": "New phone number",
"hint": "*" * 81, "hint": "*" * 81,
}, },
) )
@ -504,8 +503,6 @@ class UpdateCustomProfileFieldTest(CustomProfileFieldTestCase):
result = self.client_patch( result = self.client_patch(
f"/json/realm/profile_fields/{field.id}", f"/json/realm/profile_fields/{field.id}",
info={ info={
"name": "New phone number",
"hint": "New contact number",
"display_in_profile_summary": "invalid value", "display_in_profile_summary": "invalid value",
}, },
) )
@ -515,8 +512,6 @@ class UpdateCustomProfileFieldTest(CustomProfileFieldTestCase):
result = self.client_patch( result = self.client_patch(
f"/json/realm/profile_fields/{field.id}", f"/json/realm/profile_fields/{field.id}",
info={ info={
"name": "New phone number",
"hint": "New contact number",
"required": "invalid value", "required": "invalid value",
}, },
) )
@ -554,7 +549,7 @@ class UpdateCustomProfileFieldTest(CustomProfileFieldTestCase):
field = CustomProfileField.objects.get(name="Favorite editor", realm=realm) field = CustomProfileField.objects.get(name="Favorite editor", realm=realm)
result = self.client_patch( result = self.client_patch(
f"/json/realm/profile_fields/{field.id}", f"/json/realm/profile_fields/{field.id}",
info={"name": "Favorite editor", "field_data": "invalid"}, info={"field_data": "invalid"},
) )
self.assert_json_error(result, 'Argument "field_data" is not valid JSON.') self.assert_json_error(result, 'Argument "field_data" is not valid JSON.')
@ -566,7 +561,7 @@ class UpdateCustomProfileFieldTest(CustomProfileFieldTestCase):
).decode() ).decode()
result = self.client_patch( result = self.client_patch(
f"/json/realm/profile_fields/{field.id}", f"/json/realm/profile_fields/{field.id}",
info={"name": "Favorite editor", "field_data": field_data}, info={"field_data": field_data},
) )
self.assert_json_error(result, "field_data is not a dict") self.assert_json_error(result, "field_data is not a dict")
@ -580,7 +575,6 @@ class UpdateCustomProfileFieldTest(CustomProfileFieldTestCase):
result = self.client_patch( result = self.client_patch(
f"/json/realm/profile_fields/{field.id}", f"/json/realm/profile_fields/{field.id}",
info={ info={
"name": "Favorite editor",
"field_data": field_data, "field_data": field_data,
"display_in_profile_summary": "true", "display_in_profile_summary": "true",
}, },
@ -591,8 +585,6 @@ class UpdateCustomProfileFieldTest(CustomProfileFieldTestCase):
result = self.client_patch( result = self.client_patch(
f"/json/realm/profile_fields/{field.id}", f"/json/realm/profile_fields/{field.id}",
info={ info={
"name": field.name,
"hint": field.hint,
"display_in_profile_summary": "true", "display_in_profile_summary": "true",
}, },
) )
@ -600,6 +592,15 @@ class UpdateCustomProfileFieldTest(CustomProfileFieldTestCase):
result, "Only 2 custom profile fields can be displayed in the profile summary." result, "Only 2 custom profile fields can be displayed in the profile summary."
) )
# Empty string for hint should set it to an empty string
result = self.client_patch(
f"/json/realm/profile_fields/{field.id}",
info={"hint": ""},
)
self.assert_json_success(result)
field.refresh_from_db()
self.assertEqual(field.hint, "")
def test_update_is_aware_of_uniqueness(self) -> None: def test_update_is_aware_of_uniqueness(self) -> None:
self.login("iago") self.login("iago")
realm = get_realm("zulip") realm = get_realm("zulip")

View File

@ -1390,7 +1390,11 @@ class NormalActionsTest(BaseAction):
events = self.verify_action( events = self.verify_action(
lambda: try_update_realm_custom_profile_field( lambda: try_update_realm_custom_profile_field(
realm, field, name, hint=hint, display_in_profile_summary=display_in_profile_summary realm=realm,
field=field,
name=name,
hint=hint,
display_in_profile_summary=display_in_profile_summary,
) )
) )
check_custom_profile_fields("events[0]", events[0]) check_custom_profile_fields("events[0]", events[0])
@ -1416,7 +1420,9 @@ class NormalActionsTest(BaseAction):
hint = "What pronouns should people use to refer you?" hint = "What pronouns should people use to refer you?"
events = self.verify_action( events = self.verify_action(
lambda: try_update_realm_custom_profile_field(realm, field, name, hint=hint), lambda: try_update_realm_custom_profile_field(
realm=realm, field=field, name=name, hint=hint
),
pronouns_field_type_supported=False, pronouns_field_type_supported=False,
) )
check_custom_profile_fields("events[0]", events[0]) check_custom_profile_fields("events[0]", events[0])

View File

@ -116,20 +116,36 @@ def validate_custom_profile_field(
validate_display_in_profile_summary_field(field_type, display_in_profile_summary) validate_display_in_profile_summary_field(field_type, display_in_profile_summary)
def validate_custom_profile_field_update(
field: CustomProfileField,
field_data: ProfileFieldData,
display_in_profile_summary: bool,
name: Optional[str] = None,
hint: Optional[str] = None,
) -> None:
if name is None:
name = field.name
if hint is None:
hint = field.hint
validate_custom_profile_field(
name, hint, field.field_type, field_data, display_in_profile_summary
)
check_profile_field_data: Validator[ProfileFieldData] = check_dict( check_profile_field_data: Validator[ProfileFieldData] = check_dict(
value_validator=check_union([check_dict(value_validator=check_string), check_string]) value_validator=check_union([check_dict(value_validator=check_string), check_string])
) )
def update_only_display_in_profile_summary( def update_only_display_in_profile_summary(
requested_name: str,
requested_hint: str,
requested_field_data: ProfileFieldData, requested_field_data: ProfileFieldData,
existing_field: CustomProfileField, existing_field: CustomProfileField,
requested_name: Optional[str] = None,
requested_hint: Optional[str] = None,
) -> bool: ) -> bool:
if ( if (
requested_name != existing_field.name (requested_name is not None and requested_name != existing_field.name)
or requested_hint != existing_field.hint or (requested_hint is not None and requested_hint != existing_field.hint)
or requested_field_data != orjson.loads(existing_field.field_data) or requested_field_data != orjson.loads(existing_field.field_data)
): ):
return False return False
@ -208,8 +224,8 @@ def update_realm_custom_profile_field(
request: HttpRequest, request: HttpRequest,
user_profile: UserProfile, user_profile: UserProfile,
field_id: int, field_id: int,
name: str = REQ(default="", converter=lambda var_name, x: x.strip()), name: Optional[str] = REQ(default=None, converter=lambda var_name, x: x.strip()),
hint: str = REQ(default=""), hint: Optional[str] = REQ(default=None),
field_data: ProfileFieldData = REQ(default={}, json_validator=check_profile_field_data), field_data: ProfileFieldData = REQ(default={}, json_validator=check_profile_field_data),
display_in_profile_summary: bool = REQ(default=False, json_validator=check_bool), display_in_profile_summary: bool = REQ(default=False, json_validator=check_bool),
required: bool = REQ(default=False, json_validator=check_bool), required: bool = REQ(default=False, json_validator=check_bool),
@ -235,18 +251,16 @@ def update_realm_custom_profile_field(
# TODO: Make the name/hint/field_data parameters optional, and # TODO: Make the name/hint/field_data parameters optional, and
# just require that None was passed for all of them for this case. # just require that None was passed for all of them for this case.
and is_default_external_field(field.field_type, orjson.loads(field.field_data)) and is_default_external_field(field.field_type, orjson.loads(field.field_data))
and not update_only_display_in_profile_summary(name, hint, field_data, field) and not update_only_display_in_profile_summary(field_data, field, name, hint)
): ):
raise JsonableError(_("Default custom field cannot be updated.")) raise JsonableError(_("Default custom field cannot be updated."))
validate_custom_profile_field( validate_custom_profile_field_update(field, field_data, display_in_profile_summary, name, hint)
name, hint, field.field_type, field_data, display_in_profile_summary
)
try: try:
try_update_realm_custom_profile_field( try_update_realm_custom_profile_field(
realm, realm=realm,
field, field=field,
name, name=name,
hint=hint, hint=hint,
field_data=field_data, field_data=field_data,
display_in_profile_summary=display_in_profile_summary, display_in_profile_summary=display_in_profile_summary,