diff --git a/zerver/lib/drafts.py b/zerver/lib/drafts.py index dee6d657a9..afd073b046 100644 --- a/zerver/lib/drafts.py +++ b/zerver/lib/drafts.py @@ -24,6 +24,7 @@ from zerver.lib.validator import ( check_union, ) from zerver.models import Draft, UserProfile +from zerver.tornado.django_api import send_event VALID_DRAFT_TYPES: Set[str] = {"", "private", "stream"} @@ -117,6 +118,14 @@ def do_create_drafts(draft_dicts: List[Dict[str, Any]], user_profile: UserProfil ) created_draft_objects = Draft.objects.bulk_create(draft_objects) + + event = { + "type": "drafts", + "op": "add", + "drafts": [draft.to_dict() for draft in created_draft_objects], + } + send_event(user_profile.realm, event, [user_profile.id]) + return created_draft_objects @@ -135,6 +144,9 @@ def do_edit_draft(draft_id: int, draft_dict: Dict[str, Any], user_profile: UserP draft_object.last_edit_time = valid_draft_dict["last_edit_time"] draft_object.save() + event = {"type": "drafts", "op": "update", "draft": draft_object.to_dict()} + send_event(user_profile.realm, event, [user_profile.id]) + def do_delete_draft(draft_id: int, user_profile: UserProfile) -> None: """Delete a draft belonging to a particular user.""" @@ -142,4 +154,9 @@ def do_delete_draft(draft_id: int, user_profile: UserProfile) -> None: draft_object = Draft.objects.get(id=draft_id, user_profile=user_profile) except Draft.DoesNotExist: raise ResourceNotFoundError(_("Draft does not exist")) + + draft_id = draft_object.id draft_object.delete() + + event = {"type": "drafts", "op": "remove", "draft_id": draft_id} + send_event(user_profile.realm, event, [user_profile.id]) diff --git a/zerver/lib/events.py b/zerver/lib/events.py index 47d74006dc..0d954f6a0c 100644 --- a/zerver/lib/events.py +++ b/zerver/lib/events.py @@ -54,6 +54,7 @@ from zerver.models import ( MAX_TOPIC_NAME_LENGTH, Client, CustomProfileField, + Draft, Message, Realm, Stream, @@ -169,6 +170,17 @@ def fetch_initial_state_data( else: state["max_message_id"] = -1 + if want("drafts"): + # Note: if a user ever disables synching drafts then all of + # their old drafts stored on the server will be deleted and + # simply retained in local storage. In which case user_drafts + # would just be an empty queryset. + user_draft_objects = Draft.objects.filter(user_profile=user_profile).order_by( + "-last_edit_time" + )[: settings.MAX_DRAFTS_IN_REGISTER_RESPONSE] + user_draft_dicts = [draft.to_dict() for draft in user_draft_objects] + state["drafts"] = user_draft_dicts + if want("muted_topics"): state["muted_topics"] = [] if user_profile is None else get_topic_mutes(user_profile) @@ -618,6 +630,34 @@ def apply_event( # It may be impossible for a heartbeat event to actually reach # this code path. But in any case, they're noops. pass + + elif event["type"] == "drafts": + if event["op"] == "add": + state["drafts"].extend(event["drafts"]) + else: + if event["op"] == "update": + event_draft_idx = event["draft"]["id"] + + def _draft_update_action(i: int) -> None: + state["drafts"][i] = event["draft"] + + elif event["op"] == "remove": + event_draft_idx = event["draft_id"] + + def _draft_update_action(i: int) -> None: + del state["drafts"][i] + + # We have to perform a linear search for the draft that + # was either edited or removed since we have a list + # ordered by the last edited timestamp and not id. + state_draft_idx = None + for idx, draft in enumerate(state["drafts"]): + if draft["id"] == event_draft_idx: + state_draft_idx = idx + break + assert state_draft_idx is not None + _draft_update_action(state_draft_idx) + elif event["type"] == "hotspots": state["hotspots"] = event["hotspots"] elif event["type"] == "custom_profile_fields": diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 93f3d25c66..1755a0c63d 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -3818,6 +3818,102 @@ paths: }, "id": 0, } + - type: object + additionalProperties: false + description: | + Event containing details of newly created drafts. + properties: + id: + $ref: "#/components/schemas/EventIdSchema" + type: + allOf: + - $ref: "#/components/schemas/EventTypeSchema" + - enum: + - "drafts" + op: + type: string + enum: + - "add" + drafts: + type: array + description: | + An array containing objects for the newly created drafts. + items: + $ref: "#/components/schemas/Draft" + example: + { + "type": "drafts", + "op": "add", + "drafts": + [ + { + "id": 17, + "type": "private", + "to": [6], + "topic": "", + "content": "Hello there!", + "timestamp": 15954790200, + }, + ], + } + - type: object + additionalProperties: false + description: | + Event containing details for an edited draft. + properties: + id: + $ref: "#/components/schemas/EventIdSchema" + type: + allOf: + - $ref: "#/components/schemas/EventTypeSchema" + - enum: + - "drafts" + op: + type: string + enum: + - "update" + draft: + $ref: "#/components/schemas/Draft" + example: + { + "type": "drafts", + "op": "update", + "draft": + { + "id": 17, + "type": "private", + "to": [6, 7, 8, 9, 10], + "topic": "", + "content": "Hello everyone!", + "timestamp": 15954790200, + }, + } + - type: object + additionalProperties: false + description: | + Event containing the id of a deleted draft. + properties: + id: + $ref: "#/components/schemas/EventIdSchema" + type: + allOf: + - $ref: "#/components/schemas/EventTypeSchema" + - enum: + - "drafts" + op: + type: string + enum: + - "remove" + draft_id: + type: integer + description: | + The ID of the draft that was just deleted. + example: + { + "type": "drafts", + "op": "update", + "draft_id": 17, + } queue_id: type: string description: | @@ -7980,6 +8076,14 @@ paths: type: string description: | The name of the custom profile field type. + drafts: + type: array + description: | + An array containing draft objects for the user. These drafts are being + stored on the backend for the purpose of syncing across devices. This + array will be empty if `enable_drafts_synchronization` is set to `false`. + items: + $ref: "#/components/schemas/Draft" hotspots: type: array description: | @@ -8835,7 +8939,8 @@ paths: description: | Present if `update_display_settings` is present in `fetch_event_types`. - Whether drafts synchronization is enabled for the user. + Whether drafts synchronization is enabled for the user. If disabled, + clients will receive an error when trying to use the `drafts` endpoints. See [PATCH /settings](/api/update-settings) for details on the meaning of this setting. @@ -12800,6 +12905,59 @@ components: description: | Whether the client is capable of showing mobile/push notifications to the user. + Draft: + type: object + description: | + A dictionary for representing a message draft. + properties: + id: + type: integer + description: | + The unique ID of the draft. It will only used whenever the drafts are + fetched. This field should not be specified when the draft is being + created or edited. + type: + type: string + description: | + The type of the draft. Either unaddressed (empty string), "stream", + or "private" (for PMs and private group messages). + enum: + - "" + - "stream" + - "private" + to: + type: array + description: | + An array of the tentative target audience IDs. For "stream" + messages, this should contain exactly 1 ID, the ID of the + target stream. For private messages, this should be an array + of target user IDs. For unaddressed drafts this is ignored + so it's best to leave it as an empty array. + items: + type: integer + topic: + type: string + description: | + For stream message drafts, the tentative topic name. For private + or unaddressed messages this will be ignored and should ideally + be an empty string. Should not contain null bytes. + content: + type: string + description: | + The body of the draft. Should not contain null bytes. + timestamp: + type: number + description: | + A Unix timestamp (seconds only) representing when the draft was + last edited. When creating a draft, this key need not be present + and it will be filled in automatically by the server. + example: 1595479019 + additionalProperties: false + required: + - type + - to + - topic + - content User: allOf: - $ref: "#/components/schemas/UserBase" diff --git a/zerver/tests/test_event_system.py b/zerver/tests/test_event_system.py index 206e91f512..0d4eaf6525 100644 --- a/zerver/tests/test_event_system.py +++ b/zerver/tests/test_event_system.py @@ -980,13 +980,14 @@ class FetchQueriesTest(ZulipTestCase): with mock.patch("zerver.lib.events.always_want") as want_mock: fetch_initial_state_data(user) - self.assert_length(queries, 33) + self.assert_length(queries, 34) expected_counts = dict( alert_words=1, custom_profile_fields=1, default_streams=1, default_stream_groups=1, + drafts=1, hotspots=0, message=1, muted_topics=1, diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index 8dbab9d0f4..043ad03f2d 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -101,6 +101,7 @@ from zerver.lib.actions import ( try_add_realm_custom_profile_field, try_update_realm_custom_profile_field, ) +from zerver.lib.drafts import do_create_drafts, do_delete_draft, do_edit_draft from zerver.lib.event_schema import ( check_alert_words, check_attachment_add, @@ -2308,6 +2309,45 @@ class DraftActionTest(BaseAction): def do_disable_drafts_synchronization(self, user_profile: UserProfile) -> None: do_set_user_display_setting(user_profile, "enable_drafts_synchronization", False) + def test_draft_create_event(self) -> None: + self.do_enable_drafts_synchronization(self.user_profile) + dummy_draft = { + "type": "draft", + "to": "", + "topic": "", + "content": "Sample draft content", + "timestamp": 1596820995, + } + action = lambda: do_create_drafts([dummy_draft], self.user_profile) + self.verify_action(action) + + def test_draft_edit_event(self) -> None: + self.do_enable_drafts_synchronization(self.user_profile) + dummy_draft = { + "type": "draft", + "to": "", + "topic": "", + "content": "Sample draft content", + "timestamp": 1596820995, + } + draft_id = do_create_drafts([dummy_draft], self.user_profile)[0].id + dummy_draft["content"] = "Some more sample draft content" + action = lambda: do_edit_draft(draft_id, dummy_draft, self.user_profile) + self.verify_action(action) + + def test_draft_delete_event(self) -> None: + self.do_enable_drafts_synchronization(self.user_profile) + dummy_draft = { + "type": "draft", + "to": "", + "topic": "", + "content": "Sample draft content", + "timestamp": 1596820995, + } + draft_id = do_create_drafts([dummy_draft], self.user_profile)[0].id + action = lambda: do_delete_draft(draft_id, self.user_profile) + self.verify_action(action) + def test_enable_syncing_drafts(self) -> None: self.do_disable_drafts_synchronization(self.user_profile) action = lambda: self.do_enable_drafts_synchronization(self.user_profile) diff --git a/zerver/tests/test_home.py b/zerver/tests/test_home.py index 1550e41a7d..4b50bfb0a9 100644 --- a/zerver/tests/test_home.py +++ b/zerver/tests/test_home.py @@ -24,6 +24,7 @@ from zerver.lib.test_classes import ZulipTestCase from zerver.lib.test_helpers import get_user_messages, queries_captured from zerver.models import ( DefaultStream, + Draft, Realm, UserActivity, UserProfile, @@ -64,6 +65,7 @@ class HomeTest(ZulipTestCase): "dense_mode", "desktop_icon_count_display", "development_environment", + "drafts", "email", "email_notifications_batching_period_seconds", "emojiset", @@ -271,7 +273,7 @@ class HomeTest(ZulipTestCase): set(result["Cache-Control"].split(", ")), {"must-revalidate", "no-store", "no-cache"} ) - self.assert_length(queries, 42) + self.assert_length(queries, 43) self.assert_length(cache_mock.call_args_list, 5) html = result.content.decode("utf-8") @@ -351,7 +353,7 @@ class HomeTest(ZulipTestCase): result = self._get_home_page() self.check_rendered_logged_in_app(result) self.assert_length(cache_mock.call_args_list, 6) - self.assert_length(queries, 39) + self.assert_length(queries, 40) def test_num_queries_with_streams(self) -> None: main_user = self.example_user("hamlet") @@ -382,7 +384,7 @@ class HomeTest(ZulipTestCase): with queries_captured() as queries2: result = self._get_home_page() - self.assert_length(queries2, 37) + self.assert_length(queries2, 38) # Do a sanity check that our new streams were in the payload. html = result.content.decode("utf-8") @@ -1041,3 +1043,35 @@ class HomeTest(ZulipTestCase): page_params = self._get_page_params(result) self.assertEqual(page_params["default_language"], "es") + + @override_settings(MAX_DRAFTS_IN_REGISTER_RESPONSE=5) + def test_limit_drafts(self) -> None: + draft_objects = [] + hamlet = self.example_user("hamlet") + base_time = timezone_now() + + step_value = timedelta(seconds=1) + # Create 11 drafts. + for i in range(0, settings.MAX_DRAFTS_IN_REGISTER_RESPONSE + 1): + draft_objects.append( + Draft( + user_profile=hamlet, + recipient=None, + topic="", + content="sample draft", + last_edit_time=base_time + i * step_value, + ) + ) + Draft.objects.bulk_create(draft_objects) + + # Now fetch the drafts part of the initial state and make sure + # that we only got back settings.MAX_DRAFTS_IN_REGISTER_RESPONSE. + # No more. Also make sure that the drafts returned are the most + # recently edited ones. + self.login("hamlet") + page_params = self._get_page_params(self._get_home_page()) + self.assertEqual(page_params["enable_drafts_synchronization"], True) + self.assert_length(page_params["drafts"], settings.MAX_DRAFTS_IN_REGISTER_RESPONSE) + self.assertEqual(Draft.objects.count(), settings.MAX_DRAFTS_IN_REGISTER_RESPONSE + 1) + for draft in page_params["drafts"]: + self.assertNotEqual(draft["timestamp"], base_time) diff --git a/zproject/default_settings.py b/zproject/default_settings.py index c8465c51fd..45b2db097a 100644 --- a/zproject/default_settings.py +++ b/zproject/default_settings.py @@ -455,3 +455,8 @@ OUTGOING_WEBHOOK_TIMEOUT_SECONDS = 10 # Any message content exceeding this limit will be truncated. # See: `_internal_prep_message` function in zerver/lib/actions.py. MAX_MESSAGE_LENGTH = 10000 + +# The maximum number of drafts to send in the response to /register. +# More drafts, should they exist for some crazy reason, could be +# fetched in a separate request. +MAX_DRAFTS_IN_REGISTER_RESPONSE = 1000