From a70ede6c75baf445cea8a2d229938fc672e93d83 Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Tue, 24 Oct 2017 11:08:19 -0700 Subject: [PATCH] Allow "default" bots to see mentions on all streams. This change allows normal bots to get UserMessage rows when they are mentioned on a stream, even if they are not actually subscribed to the stream. Fixes #7140. --- zerver/lib/actions.py | 33 ++++++++++++++++++++++++++++++++- zerver/tests/test_messages.py | 15 +++------------ zerver/tests/test_users.py | 20 ++++++++++++++++++++ 3 files changed, 55 insertions(+), 13 deletions(-) diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 989d9d6951..355e9e5837 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -812,6 +812,7 @@ RecipientInfoResult = TypedDict('RecipientInfoResult', { 'stream_push_user_ids': Set[int], 'um_eligible_user_ids': Set[int], 'long_term_idle_user_ids': Set[int], + 'default_bot_user_ids': Set[int], 'service_bot_tuples': List[Tuple[int, int]], }) @@ -933,6 +934,22 @@ def get_recipient_info(recipient, sender_id, stream_topic, possibly_mentioned_us lambda r: r['long_term_idle'] ) + # These two bot data structures need to filter from the full set + # of users who either are receiving the message or might have been + # mentioned in it, and so can't use get_ids_for. + # + # Further in the do_send_messages code path, once + # `mentioned_user_ids` has been computed via bugdown, we'll filter + # these data structures for just those users who are either a + # direct recipient or were mentioned; for now, we're just making + # sure we have the data we need for that without extra database + # queries. + default_bot_user_ids = set([ + row['id'] + for row in rows + if row['is_bot'] and row['bot_type'] == UserProfile.DEFAULT_BOT + ]) + service_bot_tuples = [ (row['id'], row['bot_type']) for row in rows @@ -945,11 +962,13 @@ def get_recipient_info(recipient, sender_id, stream_topic, possibly_mentioned_us stream_push_user_ids=stream_push_user_ids, um_eligible_user_ids=um_eligible_user_ids, long_term_idle_user_ids=long_term_idle_user_ids, + default_bot_user_ids=default_bot_user_ids, service_bot_tuples=service_bot_tuples ) # type: RecipientInfoResult return info -def get_service_bot_events(sender, service_bot_tuples, mentioned_user_ids, active_user_ids, recipient_type): +def get_service_bot_events(sender, service_bot_tuples, mentioned_user_ids, + active_user_ids, recipient_type): # type: (UserProfile, List[Tuple[int, int]], Set[int], Set[int], int) -> Dict[str, List[Dict[str, Any]]] event_dict = defaultdict(list) # type: Dict[str, List[Dict[str, Any]]] @@ -1040,6 +1059,7 @@ def do_send_messages(messages_maybe_none): message['stream_push_user_ids'] = info['stream_push_user_ids'] message['um_eligible_user_ids'] = info['um_eligible_user_ids'] message['long_term_idle_user_ids'] = info['long_term_idle_user_ids'] + message['default_bot_user_ids'] = info['default_bot_user_ids'] message['service_bot_tuples'] = info['service_bot_tuples'] links_for_embed = set() # type: Set[Text] @@ -1058,6 +1078,17 @@ def do_send_messages(messages_maybe_none): message['message'].rendered_content_version = bugdown_version links_for_embed |= message['message'].links_for_preview + ''' + Once we have the actual list of mentioned ids from message + rendering, we can patch in "default bots" (aka normal bots) + who were directly mentioned in this message as eligible to + get UserMessage rows. + ''' + mentioned_user_ids = message['message'].mentions_user_ids + default_bot_user_ids = message['default_bot_user_ids'] + mentioned_bot_user_ids = default_bot_user_ids & mentioned_user_ids + message['um_eligible_user_ids'] |= mentioned_bot_user_ids + for message in messages: message['message'].update_calculated_fields() diff --git a/zerver/tests/test_messages.py b/zerver/tests/test_messages.py index 2776b71c1a..1c273184af 100644 --- a/zerver/tests/test_messages.py +++ b/zerver/tests/test_messages.py @@ -658,10 +658,6 @@ class StreamMessagesTest(ZulipTestCase): bot_owner=cordelia, ) - UserMessage.objects.filter( - user_profile=normal_bot - ).delete() - content = 'test @**Normal Bot** rules' self.send_message( @@ -671,14 +667,9 @@ class StreamMessagesTest(ZulipTestCase): content=content ) - # As of now, we don't support mentioning - # bots who aren't subscribed. - self.assertEqual( - UserMessage.objects.filter( - user_profile=normal_bot - ).count(), - 0 - ) + user_message = most_recent_usermessage(normal_bot) + self.assertEqual(user_message.message.content, content) + self.assertTrue(user_message.flags.mentioned) def test_stream_message_mirroring(self): # type: () -> None diff --git a/zerver/tests/test_users.py b/zerver/tests/test_users.py index cc2d05f8a1..bde511c2b8 100644 --- a/zerver/tests/test_users.py +++ b/zerver/tests/test_users.py @@ -409,6 +409,7 @@ class RecipientInfoTest(ZulipTestCase): stream_push_user_ids={hamlet.id}, um_eligible_user_ids=all_user_ids, long_term_idle_user_ids=set(), + default_bot_user_ids=set(), service_bot_tuples=[], ) @@ -451,6 +452,25 @@ class RecipientInfoTest(ZulipTestCase): (service_bot.id, UserProfile.EMBEDDED_BOT), ]) + # Add a normal bot. + normal_bot = do_create_user( + email='normal-bot@zulip.com', + password='', + realm=realm, + full_name='', + short_name='', + active=True, + bot_type=UserProfile.DEFAULT_BOT, + ) + + info = get_recipient_info( + recipient=recipient, + sender_id=hamlet.id, + stream_topic=stream_topic, + possibly_mentioned_user_ids={service_bot.id, normal_bot.id} + ) + self.assertEqual(info['default_bot_user_ids'], {normal_bot.id}) + class BulkUsersTest(ZulipTestCase): def test_client_gravatar_option(self): # type: () -> None