api: Update `update_message` event required fields.

Makes `edit_timestamp` and `user_id` required fields for all
`update_message` events.

Adds `rendering_only` as another required field to signal if
events are only updating the rendered content of the message,
which is currently the case for adding inline url previews.

Updates `test_event.py` so that `do_update_message` and
`do_update_embedded_data` refer to the same testing schema
for `update_message` events, and therefore reflect the same
required fields for the `update_message` event.

The OpenAPI definition for `update_message` events is also
updated to reflect the required field and descriptions of
various properties are updated for the addition of the
`rendering_only` property.
This commit is contained in:
Lauryn Menard 2022-01-14 15:23:49 +01:00 committed by Tim Abbott
parent 7077871111
commit aaa627229e
6 changed files with 111 additions and 37 deletions

View File

@ -20,6 +20,19 @@ format used by the Zulip server that they are interacting with.
## Changes in Zulip 5.0 ## Changes in Zulip 5.0
**Feature level 114**
* [`GET /events`](/api/get-events): Added `rendering_only` field to
`update_message` event type to indicate if the message change only
updated the rendering of the message or if it was the result of a
user interaction.
* [`GET /events`](/api/get-events): Updated `update_message` event type
to always include `edit_timestamp` and `user_id` fields. If the event
only updates the rendering of the message, then the `user_id` field
will be present, but with a value of null as the update was not the
result of a user interaction.
**Feature level 113** **Feature level 113**
* `GET /realm/emoji`, `POST /realm/emoji/{emoji_name}`, [`GET * `GET /realm/emoji`, `POST /realm/emoji/{emoji_name}`, [`GET

View File

@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.4.3"
# Changes should be accompanied by documentation explaining what the # Changes should be accompanied by documentation explaining what the
# new level means in templates/zerver/api/changelog.md, as well as # new level means in templates/zerver/api/changelog.md, as well as
# "**Changes**" entries in the endpoint's documentation in `zulip.yaml`. # "**Changes**" entries in the endpoint's documentation in `zulip.yaml`.
API_FEATURE_LEVEL = 113 API_FEATURE_LEVEL = 114
# Bump the minor PROVISION_VERSION to indicate that folks should provision # 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 # only when going from an old version of the code to a newer version. Bump

View File

@ -6561,7 +6561,14 @@ def do_update_embedded_data(
content: Optional[str], content: Optional[str],
rendering_result: MessageRenderingResult, rendering_result: MessageRenderingResult,
) -> None: ) -> None:
event: Dict[str, Any] = {"type": "update_message", "message_id": message.id} timestamp = timezone_now()
event: Dict[str, Any] = {
"type": "update_message",
"user_id": None,
"edit_timestamp": datetime_to_timestamp(timestamp),
"message_id": message.id,
"rendering_only": True,
}
changed_messages = [message] changed_messages = [message]
rendered_content: Optional[str] = None rendered_content: Optional[str] = None
@ -6630,6 +6637,7 @@ def do_update_message(
"user_id": user_profile.id, "user_id": user_profile.id,
"edit_timestamp": datetime_to_timestamp(timestamp), "edit_timestamp": datetime_to_timestamp(timestamp),
"message_id": target_message.id, "message_id": target_message.id,
"rendering_only": False,
} }
edit_history_event: Dict[str, Any] = { edit_history_event: Dict[str, Any] = {

View File

@ -1516,13 +1516,15 @@ def check_update_global_notifications(
assert isinstance(setting, setting_type) assert isinstance(setting, setting_type)
# user_id field is null for embedded variant of update_message
update_message_required_fields = [ update_message_required_fields = [
("type", Equals("update_message")), ("type", Equals("update_message")),
("user_id", int), ("user_id", OptionalType(int)),
("edit_timestamp", int), ("edit_timestamp", int),
("message_id", int), ("message_id", int),
("flags", ListType(str)), ("flags", ListType(str)),
("message_ids", ListType(int)), ("message_ids", ListType(int)),
("rendering_only", bool),
] ]
update_message_stream_fields: List[Tuple[str, object]] = [ update_message_stream_fields: List[Tuple[str, object]] = [
@ -1531,11 +1533,14 @@ update_message_stream_fields: List[Tuple[str, object]] = [
] ]
update_message_content_fields: List[Tuple[str, object]] = [ update_message_content_fields: List[Tuple[str, object]] = [
("content", str),
("is_me_message", bool), ("is_me_message", bool),
("orig_content", str), ("orig_content", str),
("orig_rendered_content", str), ("orig_rendered_content", str),
("prev_rendered_content_version", int), ("prev_rendered_content_version", int),
]
update_message_content_or_embedded_data_fields: List[Tuple[str, object]] = [
("content", str),
("rendered_content", str), ("rendered_content", str),
] ]
@ -1566,14 +1571,13 @@ update_message_change_stream_or_topic_fields: List[Tuple[str, object]] = [
update_message_optional_fields = ( update_message_optional_fields = (
update_message_stream_fields update_message_stream_fields
+ update_message_content_fields + update_message_content_fields
+ update_message_content_or_embedded_data_fields
+ update_message_topic_fields + update_message_topic_fields
+ update_message_change_stream_fields + update_message_change_stream_fields
+ update_message_change_stream_or_topic_fields + update_message_change_stream_or_topic_fields
) )
# The schema here does not include the "embedded" # The schema here includes the embedded variant of update_message
# variant of update_message; it is for message
# and topic editing.
update_message_event = event_dict_type( update_message_event = event_dict_type(
required_keys=update_message_required_fields, required_keys=update_message_required_fields,
optional_keys=update_message_optional_fields, optional_keys=update_message_optional_fields,
@ -1588,6 +1592,7 @@ def check_update_message(
has_content: bool, has_content: bool,
has_topic: bool, has_topic: bool,
has_new_stream_id: bool, has_new_stream_id: bool,
is_embedded_update_only: bool,
) -> None: ) -> None:
# Always check the basic schema first. # Always check the basic schema first.
_check_update_message(var_name, event) _check_update_message(var_name, event)
@ -1601,6 +1606,7 @@ def check_update_message(
if has_content: if has_content:
expected_keys.update(tup[0] for tup in update_message_content_fields) expected_keys.update(tup[0] for tup in update_message_content_fields)
expected_keys.update(tup[0] for tup in update_message_content_or_embedded_data_fields)
if has_topic: if has_topic:
expected_keys.update(tup[0] for tup in update_message_topic_fields) expected_keys.update(tup[0] for tup in update_message_topic_fields)
@ -1610,21 +1616,16 @@ def check_update_message(
expected_keys.update(tup[0] for tup in update_message_change_stream_fields) expected_keys.update(tup[0] for tup in update_message_change_stream_fields)
expected_keys.update(tup[0] for tup in update_message_change_stream_or_topic_fields) expected_keys.update(tup[0] for tup in update_message_change_stream_or_topic_fields)
if is_embedded_update_only:
expected_keys.update(tup[0] for tup in update_message_content_or_embedded_data_fields)
assert event["user_id"] is None
else:
assert isinstance(event["user_id"], int)
assert event["rendering_only"] == is_embedded_update_only
assert expected_keys == actual_keys assert expected_keys == actual_keys
update_message_embedded_event = event_dict_type(
required_keys=[
("type", Equals("update_message")),
("flags", ListType(str)),
("content", str),
("message_id", int),
("message_ids", ListType(int)),
("rendered_content", str),
]
)
check_update_message_embedded = make_checker(update_message_embedded_event)
update_message_flags_add_event = event_dict_type( update_message_flags_add_event = event_dict_type(
required_keys=[ required_keys=[
("type", Equals("update_message_flags")), ("type", Equals("update_message_flags")),

View File

@ -1966,9 +1966,14 @@ paths:
- type: object - type: object
additionalProperties: false additionalProperties: false
description: | description: |
Event sent when a message has been edited. Event sent when a message's content, topic and/or
stream has been edited or when a message's content
has a rendering update, such as for an
[inline URL preview][inline-url-previews].
Sent to all users who had received the original Sent to all users who had received the original
message. message.
[inline-url-previews]: https://zulip.readthedocs.io/en/latest/subsystems/sending-messages.html#inline-url-previews
properties: properties:
id: id:
$ref: "#/components/schemas/EventIdSchema" $ref: "#/components/schemas/EventIdSchema"
@ -1979,20 +1984,42 @@ paths:
- update_message - update_message
user_id: user_id:
type: integer type: integer
nullable: true
description: | description: |
The ID of the user who sent the message. The ID of the user who sent the message.
Not present if update_message event is for providing inline URL Null when event is for a rendering update of the original message,
preview in message. such as for an [inline URL preview][inline-url-previews].
**Changes**: As of Zulip 5.0 (feature level 114), this field
is present for all `update_message` events. Previously, this
field was omitted for [inline URL preview][inline-url-previews]
updates.
rendering_only:
type: boolean
description: |
Whether the event only updates the rendered content of the message.
This field should be used by clients to determine if the event
only provides a rendering update to the message content,
such as for an [inline URL preview][inline-url-previews].
When `True`, the event does not reflect a user-generated edit
and does not modify the message history.
**Changes**: New in Zulip 5.0 (feature level 114). Clients can
correctly identify these rendering update event with earlier
Zulip versions by checking whether the `user_id` field was omitted.
message_id: message_id:
type: integer type: integer
description: | description: |
The ID of the message which was edited. The ID of the message which was edited or updated.
This field should be used to apply content edits to the client's This field should be used to apply content edits to the client's
cached message history. If the stream or topic was changed, the cached message history, or to apply rendered content updates.
set of moved messages is encoded in the separate `message_ids`
field, which is guaranteed to include `message_id`. If the stream or topic was changed, the set of moved messages is
encoded in the separate `message_ids` field, which is guaranteed
to include `message_id`.
message_ids: message_ids:
type: array type: array
items: items:
@ -2033,19 +2060,21 @@ paths:
The time when this message edit operation was processed by the The time when this message edit operation was processed by the
server. server.
Not present if update_message event is for providing inline URL **Changes**: As of Zulip 5.0 (feature level 114), this field
preview in message. is present for all `update_message` events. Previously, this
field was omitted for [inline URL preview][inline-url-previews]
updates.
stream_name: stream_name:
type: string type: string
description: | description: |
Only present if the message was originally sent to a stream. Only present if the message was edited and originally sent to a stream.
The name of the stream that the message was sent to. Clients The name of the stream that the message was sent to. Clients
are recommended to use the `stream_id` field instead. are recommended to use the `stream_id` field instead.
stream_id: stream_id:
type: integer type: integer
description: | description: |
Only present if the message was originally sent to a stream. Only present if the message was edited and originally sent to a stream.
The pre-edit stream for all of the messages with IDs in The pre-edit stream for all of the messages with IDs in
`message_ids`. `message_ids`.
@ -2068,9 +2097,12 @@ paths:
topic and/or stream. topic and/or stream.
The choice the editing user made about which messages should be The choice the editing user made about which messages should be
affected by a stream/topic edit: just the one indicated in affected by a stream/topic edit:
`message_id`, messages in the same topic that had been sent
after this one, or all messages in that topic. - `change_one` => Just change the one indicated in `message_id`.
- `change_later` => Change messages in the same topic that had
been sent after this one.
- `change_all`=> Change all messages in that topic.
This parameter should be used to decide whether to change This parameter should be used to decide whether to change
navigation and compose box state in response to the edit. For navigation and compose box state in response to the edit. For
@ -2159,14 +2191,18 @@ paths:
content: content:
type: string type: string
description: | description: |
Only present if this event changed the message content. Only present if this event changed the message content or
updated the message content for an
[inline URL preview][inline-url-previews].
The new content of the message with ID `message_id`, in the The new content of the message with ID `message_id`, in the
original Markdown. original Markdown.
rendered_content: rendered_content:
type: string type: string
description: | description: |
Only present if this event changed the message content. Only present if this event changed the message content or
updated the message content for an
[inline URL preview][inline-url-previews].
The new content of the message with ID `message_id`, The new content of the message with ID `message_id`,
rendered in HTML. rendered in HTML.
@ -2182,9 +2218,12 @@ paths:
required: required:
- "type" - "type"
- "id" - "id"
- "user_id"
- "message_id" - "message_id"
- "message_ids" - "message_ids"
- "flags" - "flags"
- "edit_timestamp"
- "rendering_only"
example: example:
{ {
"type": "update_message", "type": "update_message",
@ -2205,6 +2244,7 @@ paths:
"topic_links": [], "topic_links": [],
"message_ids": [58, 57], "message_ids": [58, 57],
"flags": [], "flags": [],
"rendering_only": false,
"id": 0, "id": 0,
} }
- type: object - type: object

View File

@ -154,7 +154,6 @@ from zerver.lib.event_schema import (
check_update_display_settings, check_update_display_settings,
check_update_global_notifications, check_update_global_notifications,
check_update_message, check_update_message,
check_update_message_embedded,
check_update_message_flags_add, check_update_message_flags_add,
check_update_message_flags_remove, check_update_message_flags_remove,
check_user_group_add, check_user_group_add,
@ -466,6 +465,7 @@ class NormalActionsTest(BaseAction):
has_content=True, has_content=True,
has_topic=False, has_topic=False,
has_new_stream_id=False, has_new_stream_id=False,
is_embedded_update_only=False,
) )
def test_huddle_send_message_events(self) -> None: def test_huddle_send_message_events(self) -> None:
@ -526,6 +526,7 @@ class NormalActionsTest(BaseAction):
has_content=True, has_content=True,
has_topic=False, has_topic=False,
has_new_stream_id=False, has_new_stream_id=False,
is_embedded_update_only=False,
) )
# Verify stream message editing - topic only # Verify stream message editing - topic only
@ -555,15 +556,25 @@ class NormalActionsTest(BaseAction):
has_content=False, has_content=False,
has_topic=True, has_topic=True,
has_new_stream_id=False, has_new_stream_id=False,
is_embedded_update_only=False,
) )
# Verify special case of embedded content update
content = "embed_content" content = "embed_content"
rendering_result = render_markdown(message, content) rendering_result = render_markdown(message, content)
events = self.verify_action( events = self.verify_action(
lambda: do_update_embedded_data(self.user_profile, message, content, rendering_result), lambda: do_update_embedded_data(self.user_profile, message, content, rendering_result),
state_change_expected=False, state_change_expected=False,
) )
check_update_message_embedded("events[0]", events[0]) check_update_message(
"events[0]",
events[0],
is_stream_message=False,
has_content=False,
has_topic=False,
has_new_stream_id=False,
is_embedded_update_only=True,
)
# Verify move topic to different stream. # Verify move topic to different stream.
@ -602,6 +613,7 @@ class NormalActionsTest(BaseAction):
has_content=False, has_content=False,
has_topic=False, has_topic=False,
has_new_stream_id=True, has_new_stream_id=True,
is_embedded_update_only=False,
) )
def test_update_message_flags(self) -> None: def test_update_message_flags(self) -> None: