diff --git a/templates/zerver/api/changelog.md b/templates/zerver/api/changelog.md index 7c2e39edd2..6e26a135e6 100644 --- a/templates/zerver/api/changelog.md +++ b/templates/zerver/api/changelog.md @@ -20,6 +20,14 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 6.0 +**Feature level 146** + +* [`POST /realm/profile_fields`](/api/create-custom-profile-field), +[`GET /realm/profile_fields`](/api/get-custom-profile-fields): Added a +new parameter `display_in_profile_summary`, which clients use to +decide whether to display the field in a small/summary section of the +user's profile. + **Feature level 145** * [`DELETE users/me/subscriptions`](/api/unsubscribe): Normal users can diff --git a/version.py b/version.py index d044bb2e17..97524b849f 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 = 145 +API_FEATURE_LEVEL = 146 # 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/actions/custom_profile_fields.py b/zerver/actions/custom_profile_fields.py index a9a69a4994..7fb0ad31cf 100644 --- a/zerver/actions/custom_profile_fields.py +++ b/zerver/actions/custom_profile_fields.py @@ -26,7 +26,9 @@ def notify_realm_custom_profile_fields(realm: Realm) -> None: def try_add_realm_default_custom_profile_field( - realm: Realm, field_subtype: str + realm: Realm, + field_subtype: str, + display_in_profile_summary: bool = False, ) -> CustomProfileField: field_data = DEFAULT_EXTERNAL_ACCOUNTS[field_subtype] custom_profile_field = CustomProfileField( @@ -35,6 +37,7 @@ def try_add_realm_default_custom_profile_field( field_type=CustomProfileField.EXTERNAL_ACCOUNT, hint=field_data.hint, field_data=orjson.dumps(dict(subtype=field_subtype)).decode(), + display_in_profile_summary=display_in_profile_summary, ) custom_profile_field.save() custom_profile_field.order = custom_profile_field.id @@ -49,8 +52,14 @@ def try_add_realm_custom_profile_field( field_type: int, hint: str = "", field_data: Optional[ProfileFieldData] = None, + display_in_profile_summary: bool = False, ) -> CustomProfileField: - custom_profile_field = CustomProfileField(realm=realm, name=name, field_type=field_type) + custom_profile_field = CustomProfileField( + realm=realm, + name=name, + field_type=field_type, + display_in_profile_summary=display_in_profile_summary, + ) custom_profile_field.hint = hint if ( custom_profile_field.field_type == CustomProfileField.SELECT @@ -95,9 +104,11 @@ def try_update_realm_custom_profile_field( name: str, hint: str = "", field_data: Optional[ProfileFieldData] = None, + display_in_profile_summary: bool = False, ) -> None: field.name = name field.hint = hint + field.display_in_profile_summary = display_in_profile_summary if ( field.field_type == CustomProfileField.SELECT or field.field_type == CustomProfileField.EXTERNAL_ACCOUNT diff --git a/zerver/lib/event_schema.py b/zerver/lib/event_schema.py index 8de368bb1f..e8fa731776 100644 --- a/zerver/lib/event_schema.py +++ b/zerver/lib/event_schema.py @@ -171,6 +171,9 @@ custom_profile_field_type = DictType( ("field_data", str), ("order", int), ], + optional_keys=[ + ("display_in_profile_summary", bool), + ], ) custom_profile_fields_event = event_dict_type( diff --git a/zerver/lib/types.py b/zerver/lib/types.py index abffc35c35..4a7f4e9a26 100644 --- a/zerver/lib/types.py +++ b/zerver/lib/types.py @@ -29,11 +29,12 @@ RealmUserValidator = Callable[[int, object, bool], List[int]] ProfileDataElementValue = Union[str, List[int]] -class ProfileDataElementBase(TypedDict): +class ProfileDataElementBase(TypedDict, total=False): id: int name: str type: int hint: str + display_in_profile_summary: bool field_data: str order: int diff --git a/zerver/migrations/0412_customprofilefield_display_in_profile_summary.py b/zerver/migrations/0412_customprofilefield_display_in_profile_summary.py new file mode 100644 index 0000000000..be21ae528c --- /dev/null +++ b/zerver/migrations/0412_customprofilefield_display_in_profile_summary.py @@ -0,0 +1,18 @@ +# Generated by Django 4.0.7 on 2022-09-19 17:28 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("zerver", "0411_alter_muteduser_muted_user_and_more"), + ] + + operations = [ + migrations.AddField( + model_name="customprofilefield", + name="display_in_profile_summary", + field=models.BooleanField(default=False), + ), + ] diff --git a/zerver/models.py b/zerver/models.py index 7a1749c47a..123adc6c7b 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -4540,13 +4540,20 @@ class CustomProfileField(models.Model): HINT_MAX_LENGTH = 80 NAME_MAX_LENGTH = 40 + MAX_DISPLAY_IN_PROFILE_SUMMARY_FIELDS = 2 id: int = models.AutoField(auto_created=True, primary_key=True, verbose_name="ID") realm: Realm = models.ForeignKey(Realm, on_delete=CASCADE) name: str = models.CharField(max_length=NAME_MAX_LENGTH) hint: str = models.CharField(max_length=HINT_MAX_LENGTH, default="") + + # Sort order for display of custom profile fields. order: int = models.IntegerField(default=0) + # Whether the field should be displayed in smaller summary + # sections of a page displaying custom profile fields. + display_in_profile_summary: bool = models.BooleanField(default=False) + SHORT_TEXT = 1 LONG_TEXT = 2 SELECT = 3 @@ -4619,7 +4626,7 @@ class CustomProfileField(models.Model): unique_together = ("realm", "name") def as_dict(self) -> ProfileDataElementBase: - return { + data_as_dict: ProfileDataElementBase = { "id": self.id, "name": self.name, "type": self.field_type, @@ -4627,6 +4634,10 @@ class CustomProfileField(models.Model): "field_data": self.field_data, "order": self.order, } + if self.display_in_profile_summary: + data_as_dict["display_in_profile_summary"] = True + + return data_as_dict def is_renderable(self) -> bool: if self.field_type in [CustomProfileField.SHORT_TEXT, CustomProfileField.LONG_TEXT]: diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index cf927a0956..a2a651e1c6 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -1666,6 +1666,7 @@ paths: "hint": "", "field_data": '{"0":{"text":"Vim","order":"1"},"1":{"text":"Emacs","order":"2"}}', "order": 4, + "display_in_profile_summary": true, }, { "id": 5, @@ -1682,6 +1683,7 @@ paths: "hint": "Or your personal blog's URL", "field_data": "", "order": 6, + "display_in_profile_summary": true, }, { "id": 7, @@ -8250,6 +8252,7 @@ paths: "hint": "", "field_data": '{"0":{"text":"Vim","order":"1"},"1":{"text":"Emacs","order":"2"}}', "order": 4, + "display_in_profile_summary": true, }, { "id": 5, @@ -8266,6 +8269,7 @@ paths: "hint": "Or your personal blog's URL", "field_data": "", "order": 6, + "display_in_profile_summary": true, }, { "id": 7, @@ -8378,6 +8382,25 @@ paths: "python": {"text": "Python", "order": "1"}, "java": {"text": "Java", "order": "2"}, } + - name: display_in_profile_summary + in: query + description: | + Whether clients should display this profile field in a summary section of a + user's profile (or in a more easily accessible "small profile"). + + At most 2 profile fields may have this property be true in a given + organization. The "Long text" [profile field types][profile-field-types] + profile field types cannot be selected to be displayed in profile summaries. + + The "Person picker" profile field is also not supported, but that is likely to + be temporary. + + [profile-field-types]: /help/add-custom-profile-fields#profile-field-types + + **Changes**: New in Zulip 6.0 (feature level 146). + schema: + type: boolean + example: true responses: "200": description: Success. @@ -15418,6 +15441,15 @@ components: dropdown UI for individual users to select an option. The interface for field type 7 is not yet stabilized. + display_in_profile_summary: + type: boolean + description: | + Whether the custom profile field, display or not in the user profile summary. + + Currently it's value not allowed to be `true` of `Long text` and `Person picker` + [profile field types](/help/add-custom-profile-fields#profile-field-types). + + **Changes**: New in Zulip 6.0 (feature level 146). Hotspot: type: object additionalProperties: false diff --git a/zerver/tests/test_custom_profile_data.py b/zerver/tests/test_custom_profile_data.py index 8386f4f77e..ab38708f79 100644 --- a/zerver/tests/test_custom_profile_data.py +++ b/zerver/tests/test_custom_profile_data.py @@ -60,11 +60,19 @@ class CreateCustomProfileFieldTest(CustomProfileFieldTestCase): data["hint"] = "*" * 81 data["field_type"] = CustomProfileField.SHORT_TEXT result = self.client_post("/json/realm/profile_fields", info=data) - msg = "hint is too long (limit: 80 characters)" - self.assert_json_error(result, msg) + self.assert_json_error(result, "hint is too long (limit: 80 characters)") data["name"] = "Phone" data["hint"] = "Contact number" + data["field_type"] = CustomProfileField.LONG_TEXT + data["display_in_profile_summary"] = "true" + result = self.client_post("/json/realm/profile_fields", info=data) + self.assert_json_error(result, "Field type not supported for display in profile summary.") + + data["field_type"] = CustomProfileField.USER + result = self.client_post("/json/realm/profile_fields", info=data) + self.assert_json_error(result, "Field type not supported for display in profile summary.") + data["field_type"] = CustomProfileField.SHORT_TEXT result = self.client_post("/json/realm/profile_fields", info=data) self.assert_json_success(result) @@ -75,12 +83,19 @@ class CreateCustomProfileFieldTest(CustomProfileFieldTestCase): data["name"] = "Name " data["hint"] = "Some name" data["field_type"] = CustomProfileField.SHORT_TEXT + data["display_in_profile_summary"] = "true" result = self.client_post("/json/realm/profile_fields", info=data) self.assert_json_success(result) field = CustomProfileField.objects.get(name="Name", realm=realm) self.assertEqual(field.id, field.order) + result = self.client_post("/json/realm/profile_fields", info=data) + self.assert_json_error( + result, "Only 2 custom profile fields can be displayed in the profile summary." + ) + + data["display_in_profile_summary"] = "false" result = self.client_post("/json/realm/profile_fields", info=data) self.assert_json_error(result, "A field with that label already exists.") @@ -202,7 +217,7 @@ class CreateCustomProfileFieldTest(CustomProfileFieldTestCase): ) self.assert_json_success(result) - # Default external account field data cannot be updated + # Default external account field data cannot be updated except "display_in_profile_summary" field field = CustomProfileField.objects.get(name="Twitter username", realm=realm) result = self.client_patch( f"/json/realm/profile_fields/{field.id}", @@ -210,6 +225,18 @@ class CreateCustomProfileFieldTest(CustomProfileFieldTestCase): ) self.assert_json_error(result, "Default custom field cannot be updated.") + result = self.client_patch( + f"/json/realm/profile_fields/{field.id}", + info={ + "name": field.name, + "hint": field.hint, + "field_type": field_type, + "field_data": field_data, + "display_in_profile_summary": "true", + }, + ) + self.assert_json_success(result) + result = self.client_delete(f"/json/realm/profile_fields/{field.id}") self.assert_json_success(result) @@ -470,6 +497,18 @@ class UpdateCustomProfileFieldTest(CustomProfileFieldTestCase): info={ "name": "New phone number", "hint": "New contact number", + "display_in_profile_summary": "invalid value", + }, + ) + msg = 'Argument "display_in_profile_summary" is not valid JSON.' + self.assert_json_error(result, msg) + + result = self.client_patch( + f"/json/realm/profile_fields/{field.id}", + info={ + "name": "New phone number", + "hint": "New contact number", + "display_in_profile_summary": "true", }, ) self.assert_json_success(result) @@ -479,14 +518,16 @@ class UpdateCustomProfileFieldTest(CustomProfileFieldTestCase): self.assertEqual(field.name, "New phone number") self.assertEqual(field.hint, "New contact number") self.assertEqual(field.field_type, CustomProfileField.SHORT_TEXT) + self.assertEqual(field.display_in_profile_summary, True) result = self.client_patch( f"/json/realm/profile_fields/{field.id}", - info={"name": "Name "}, + info={"name": "Name ", "display_in_profile_summary": "true"}, ) self.assert_json_success(result) field.refresh_from_db() self.assertEqual(field.name, "Name") + self.assertEqual(field.display_in_profile_summary, True) field = CustomProfileField.objects.get(name="Favorite editor", realm=realm) result = self.client_patch( @@ -516,10 +557,27 @@ class UpdateCustomProfileFieldTest(CustomProfileFieldTestCase): ).decode() result = self.client_patch( f"/json/realm/profile_fields/{field.id}", - info={"name": "Favorite editor", "field_data": field_data}, + info={ + "name": "Favorite editor", + "field_data": field_data, + "display_in_profile_summary": "true", + }, ) self.assert_json_success(result) + field = CustomProfileField.objects.get(name="Birthday", realm=realm) + result = self.client_patch( + f"/json/realm/profile_fields/{field.id}", + info={ + "name": field.name, + "hint": field.hint, + "display_in_profile_summary": "true", + }, + ) + self.assert_json_error( + result, "Only 2 custom profile fields can be displayed in the profile summary." + ) + def test_update_is_aware_of_uniqueness(self) -> None: self.login("iago") realm = get_realm("zulip") diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index 0cf2bc3a33..8fa52b2d21 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -985,9 +985,12 @@ class NormalActionsTest(BaseAction): field = realm.customprofilefield_set.get(realm=realm, name="Biography") name = field.name hint = "Biography of the user" + display_in_profile_summary = False events = self.verify_action( - lambda: try_update_realm_custom_profile_field(realm, field, name, hint=hint) + lambda: try_update_realm_custom_profile_field( + realm, field, name, hint=hint, display_in_profile_summary=display_in_profile_summary + ) ) check_custom_profile_fields("events[0]", events[0]) diff --git a/zerver/views/custom_profile_fields.py b/zerver/views/custom_profile_fields.py index 9e281628f7..ff14d7c5c8 100644 --- a/zerver/views/custom_profile_fields.py +++ b/zerver/views/custom_profile_fields.py @@ -1,4 +1,4 @@ -from typing import List, cast +from typing import List, Optional, cast import orjson from django.core.exceptions import ValidationError @@ -23,6 +23,7 @@ from zerver.lib.response import json_success from zerver.lib.types import ProfileDataElementUpdateDict, ProfileFieldData, Validator from zerver.lib.users import validate_user_custom_profile_data from zerver.lib.validator import ( + check_bool, check_capped_string, check_dict, check_dict_only, @@ -70,6 +71,19 @@ def validate_custom_field_data(field_type: int, field_data: ProfileFieldData) -> raise JsonableError(error.message) +def validate_display_in_profile_summary_field( + field_type: int, display_in_profile_summary: bool +) -> None: + if not display_in_profile_summary: + return + + # The LONG_TEXT field type doesn't make sense visually for profile + # field summaries. The USER field type will require some further + # client support. + if field_type == CustomProfileField.LONG_TEXT or field_type == CustomProfileField.USER: + raise JsonableError(_("Field type not supported for display in profile summary.")) + + def is_default_external_field(field_type: int, field_data: ProfileFieldData) -> bool: if field_type != CustomProfileField.EXTERNAL_ACCOUNT: return False @@ -79,7 +93,11 @@ def is_default_external_field(field_type: int, field_data: ProfileFieldData) -> def validate_custom_profile_field( - name: str, hint: str, field_type: int, field_data: ProfileFieldData + name: str, + hint: str, + field_type: int, + field_data: ProfileFieldData, + display_in_profile_summary: bool, ) -> None: # Validate field data validate_custom_field_data(field_type, field_data) @@ -94,12 +112,36 @@ def validate_custom_profile_field( if field_type not in field_types: raise JsonableError(_("Invalid field type.")) + validate_display_in_profile_summary_field(field_type, display_in_profile_summary) + check_profile_field_data: Validator[ProfileFieldData] = check_dict( value_validator=check_union([check_dict(value_validator=check_string), check_string]) ) +def update_only_display_in_profile_summary( + requested_name: str, + requested_hint: str, + requested_field_data: ProfileFieldData, + existing_field: CustomProfileField, +) -> bool: + if ( + requested_name != existing_field.name + or requested_hint != existing_field.hint + or requested_field_data != orjson.loads(existing_field.field_data) + ): + return False + return True + + +def display_in_profile_summary_limit_reached(profile_field_id: Optional[int] = None) -> bool: + query = CustomProfileField.objects.filter(display_in_profile_summary=True) + if profile_field_id is not None: + query = query.exclude(id=profile_field_id) + return query.count() >= CustomProfileField.MAX_DISPLAY_IN_PROFILE_SUMMARY_FIELDS + + @require_realm_admin @has_request_variables def create_realm_custom_profile_field( @@ -109,8 +151,14 @@ def create_realm_custom_profile_field( hint: str = REQ(default=""), field_data: ProfileFieldData = REQ(default={}, json_validator=check_profile_field_data), field_type: int = REQ(json_validator=check_int), + display_in_profile_summary: bool = REQ(default=False, json_validator=check_bool), ) -> HttpResponse: - validate_custom_profile_field(name, hint, field_type, field_data) + if display_in_profile_summary and display_in_profile_summary_limit_reached(): + raise JsonableError( + _("Only 2 custom profile fields can be displayed in the profile summary.") + ) + + validate_custom_profile_field(name, hint, field_type, field_data, display_in_profile_summary) try: if is_default_external_field(field_type, field_data): field_subtype = field_data["subtype"] @@ -118,6 +166,7 @@ def create_realm_custom_profile_field( field = try_add_realm_default_custom_profile_field( realm=user_profile.realm, field_subtype=field_subtype, + display_in_profile_summary=display_in_profile_summary, ) return json_success(request, data={"id": field.id}) else: @@ -127,6 +176,7 @@ def create_realm_custom_profile_field( field_data=field_data, field_type=field_type, hint=hint, + display_in_profile_summary=display_in_profile_summary, ) return json_success(request, data={"id": field.id}) except IntegrityError: @@ -155,6 +205,7 @@ def update_realm_custom_profile_field( name: str = REQ(default="", converter=lambda var_name, x: x.strip()), hint: str = REQ(default=""), field_data: ProfileFieldData = REQ(default={}, json_validator=check_profile_field_data), + display_in_profile_summary: bool = REQ(default=False, json_validator=check_bool), ) -> HttpResponse: realm = user_profile.realm try: @@ -162,13 +213,34 @@ def update_realm_custom_profile_field( except CustomProfileField.DoesNotExist: raise JsonableError(_("Field id {id} not found.").format(id=field_id)) + if display_in_profile_summary and display_in_profile_summary_limit_reached(field.id): + raise JsonableError( + _("Only 2 custom profile fields can be displayed in the profile summary.") + ) + if field.field_type == CustomProfileField.EXTERNAL_ACCOUNT: - if is_default_external_field(field.field_type, orjson.loads(field.field_data)): + # HACK: Allow changing the display_in_profile_summary property + # of default external account types, but not any others. + # + # TODO: Make the name/hint/field_data parameters optional, and + # just require that None was passed for all of them for this case. + if 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): raise JsonableError(_("Default custom field cannot be updated.")) - validate_custom_profile_field(name, hint, field.field_type, field_data) + validate_custom_profile_field( + name, hint, field.field_type, field_data, display_in_profile_summary + ) try: - try_update_realm_custom_profile_field(realm, field, name, hint=hint, field_data=field_data) + try_update_realm_custom_profile_field( + realm, + field, + name, + hint=hint, + field_data=field_data, + display_in_profile_summary=display_in_profile_summary, + ) except IntegrityError: raise JsonableError(_("A field with that label already exists.")) return json_success(request)