From 45f6c8d27ffebc357b35c967d79ed919372824dc Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Mon, 19 Jul 2021 13:06:58 +0000 Subject: [PATCH] page load: Remove sender_ids in unread messages for streams. --- templates/zerver/api/changelog.md | 9 +++++++++ zerver/lib/message.py | 28 ++++++---------------------- zerver/openapi/zulip.yaml | 7 ------- zerver/tests/test_message_flags.py | 6 ------ zerver/tests/test_subs.py | 3 --- 5 files changed, 15 insertions(+), 38 deletions(-) diff --git a/templates/zerver/api/changelog.md b/templates/zerver/api/changelog.md index 7c155eb0c9..8d5ab6e580 100644 --- a/templates/zerver/api/changelog.md +++ b/templates/zerver/api/changelog.md @@ -11,6 +11,15 @@ below features are supported. ## Changes in Zulip 5.0 +======= +**Feature level 90** + +* We no longer include `sender_ids` in the `streams` section of + `UnreadMessagesResult`, which becomes the `unread_msgs` section of + the events that clients fetch when registering events at page + load time. (We don't believe this feature has ever been used in + a meaningful way.) + **Feature level 89** * [`GET /events`](/api/get-events): Introduced the `user_settings` diff --git a/zerver/lib/message.py b/zerver/lib/message.py index 9896b80140..cd508f286a 100644 --- a/zerver/lib/message.py +++ b/zerver/lib/message.py @@ -69,7 +69,6 @@ class RawReactionRow(TypedDict): class RawUnreadStreamDict(TypedDict): stream_id: int topic: str - sender_id: int class RawUnreadPrivateMessageDict(TypedDict): @@ -871,7 +870,7 @@ def huddle_users(recipient_id: int) -> str: def aggregate_message_dict( - input_dict: Dict[int, Any], lookup_fields: List[str], collect_senders: bool + input_dict: Dict[int, Any], lookup_fields: List[str] ) -> List[Dict[str, Any]]: lookup_dict: Dict[Tuple[Any, ...], Dict[str, Any]] = {} @@ -879,21 +878,20 @@ def aggregate_message_dict( A concrete example might help explain the inputs here: input_dict = { - 1002: dict(stream_id=5, topic='foo', sender_id=40), - 1003: dict(stream_id=5, topic='foo', sender_id=41), - 1004: dict(stream_id=6, topic='baz', sender_id=99), + 1002: dict(stream_id=5, topic='foo'), + 1003: dict(stream_id=5, topic='foo'), + 1004: dict(stream_id=6, topic='baz'), } lookup_fields = ['stream_id', 'topic'] The first time through the loop: - attribute_dict = dict(stream_id=5, topic='foo', sender_id=40) - lookup_dict = (5, 'foo') + attribute_dict = dict(stream_id=5, topic='foo') + lookup_key = (5, 'foo') lookup_dict = { (5, 'foo'): dict(stream_id=5, topic='foo', unread_message_ids=[1002, 1003], - sender_ids=[40, 41], ), ... } @@ -901,7 +899,6 @@ def aggregate_message_dict( result = [ dict(stream_id=5, topic='foo', unread_message_ids=[1002, 1003], - sender_ids=[40, 41], ), ... ] @@ -914,19 +911,13 @@ def aggregate_message_dict( for f in lookup_fields: obj[f] = attribute_dict[f] obj["unread_message_ids"] = [] - if collect_senders: - obj["sender_ids"] = set() lookup_dict[lookup_key] = obj bucket = lookup_dict[lookup_key] bucket["unread_message_ids"].append(message_id) - if collect_senders: - bucket["sender_ids"].add(attribute_dict["sender_id"]) for dct in lookup_dict.values(): dct["unread_message_ids"].sort() - if collect_senders: - dct["sender_ids"] = sorted(dct["sender_ids"]) sorted_keys = sorted(lookup_dict.keys()) @@ -1073,7 +1064,6 @@ def extract_unread_data_from_um_rows( stream_dict[message_id] = dict( stream_id=stream_id, topic=topic, - sender_id=sender_id, ) if not is_row_muted(stream_id, recipient_id, topic): unmuted_stream_msgs.add(message_id) @@ -1137,7 +1127,6 @@ def aggregate_unread_data(raw_data: RawUnreadMessagesResult) -> UnreadMessagesRe lookup_fields=[ "sender_id", ], - collect_senders=False, ) stream_objects = aggregate_message_dict( @@ -1146,7 +1135,6 @@ def aggregate_unread_data(raw_data: RawUnreadMessagesResult) -> UnreadMessagesRe "stream_id", "topic", ], - collect_senders=True, ) huddle_objects = aggregate_message_dict( @@ -1154,7 +1142,6 @@ def aggregate_unread_data(raw_data: RawUnreadMessagesResult) -> UnreadMessagesRe lookup_fields=[ "user_ids_string", ], - collect_senders=False, ) result: UnreadMessagesResult = dict( @@ -1187,15 +1174,12 @@ def apply_unread_message_event( else: raise AssertionError("Invalid message type {}".format(message["type"])) - sender_id = message["sender_id"] - if message_type == "stream": stream_id = message["stream_id"] topic = message[TOPIC_NAME] state["stream_dict"][message_id] = RawUnreadStreamDict( stream_id=stream_id, topic=topic, - sender_id=sender_id, ) if stream_id not in state["muted_stream_ids"]: diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 37c128fec6..f1da968ef7 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -8864,13 +8864,6 @@ paths: message with the message_id as the key. additionalProperties: false properties: - sender_ids: - type: array - items: - type: integer - description: | - Array containing the id of the users who have sent recent messages - on this stream under the given topic which have been unread by the user. topic: type: string description: | diff --git a/zerver/tests/test_message_flags.py b/zerver/tests/test_message_flags.py index 4b38c7e45f..585036e0c5 100644 --- a/zerver/tests/test_message_flags.py +++ b/zerver/tests/test_message_flags.py @@ -645,7 +645,6 @@ class GetUnreadMsgsTest(ZulipTestCase): self.assertEqual( stream_dict[message_ids["lunch"][0]], dict( - sender_id=cordelia.id, stream_id=get_stream("social", realm).id, topic="lunch", ), @@ -870,19 +869,16 @@ class GetUnreadMsgsTest(ZulipTestCase): unread_pm = result["pms"][0] self.assertEqual(unread_pm["sender_id"], sender_id) self.assertEqual(unread_pm["unread_message_ids"], [pm1_message_id, pm2_message_id]) - self.assertTrue("sender_ids" not in unread_pm) unread_stream = result["streams"][0] self.assertEqual(unread_stream["stream_id"], get_stream("Denmark", user_profile.realm).id) self.assertEqual(unread_stream["topic"], "muted-topic") self.assertEqual(unread_stream["unread_message_ids"], [muted_topic_message_id]) - self.assertEqual(unread_stream["sender_ids"], [sender_id]) unread_stream = result["streams"][1] self.assertEqual(unread_stream["stream_id"], get_stream("Denmark", user_profile.realm).id) self.assertEqual(unread_stream["topic"], "test") self.assertEqual(unread_stream["unread_message_ids"], [stream_message_id]) - self.assertEqual(unread_stream["sender_ids"], [sender_id]) unread_stream = result["streams"][2] self.assertEqual( @@ -890,7 +886,6 @@ class GetUnreadMsgsTest(ZulipTestCase): ) self.assertEqual(unread_stream["topic"], "test") self.assertEqual(unread_stream["unread_message_ids"], [muted_stream_message_id]) - self.assertEqual(unread_stream["sender_ids"], [sender_id]) huddle_string = ",".join( str(uid) for uid in sorted([sender_id, user_profile.id, othello.id]) @@ -899,7 +894,6 @@ class GetUnreadMsgsTest(ZulipTestCase): unread_huddle = result["huddles"][0] self.assertEqual(unread_huddle["user_ids_string"], huddle_string) self.assertEqual(unread_huddle["unread_message_ids"], [huddle_message_id]) - self.assertTrue("sender_ids" not in unread_huddle) self.assertEqual(result["mentions"], []) diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index 4ad25297b6..af6ea540b9 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -83,7 +83,6 @@ from zerver.models import ( get_default_stream_groups, get_realm, get_stream, - get_system_bot, get_user, get_user_profile_by_id_in_realm, ) @@ -279,7 +278,6 @@ class TestCreateStreams(ZulipTestCase): self.subscribe(iago, announce_stream.name) self.subscribe(hamlet, announce_stream.name) - notification_bot = get_system_bot(settings.NOTIFICATION_BOT, realm.id) self.login_user(iago) initial_message_count = Message.objects.count() @@ -336,7 +334,6 @@ class TestCreateStreams(ZulipTestCase): # According to the code in zerver/views/streams/add_subscriptions_backend # the notification stream message is sent first, then the new stream's message. - self.assertEqual(hamlet_unread_messages[0]["sender_ids"][0], notification_bot.id) self.assertEqual(hamlet_unread_messages[1]["stream_id"], stream_id) # But it should be marked as read for Iago, the stream creator.