Allow generic bots to be mentioned.

The original PR to allow generic bots to be mentioned had
some merge issues that we detected about a week after the
fact.  This commit restores the logic from the original PR.

The reason we didn't detect this bug earlier is that the
merge issues didn't break any existing behavior.  Instead,
they made it so that only UserMessage rows got written for
bots, but no events were being set.  The part of the commit
that got lost is restored here, so now events get sent as
well.

Thanks to @derAnfaenger for reporting this and being patient
as we tracked it down.

Fixes #7140

zerver/lib/actions.py

@@ -1153,6 +1153,20 @@ def do_send_messages(messages_maybe_none):
             presence_idle_user_ids=presence_idle_user_ids,
         )
 
+        '''
+        TODO:  We may want to limit user_ids to only those users who have
+               UserMessage rows, if only for minor performance reasons.
+
+               For now we queue events for all subscribers/sendees of the
+               message, since downstream code may still do notifications
+               that don't require UserMessage rows.
+
+               Our automated tests have gotten better on this codepath,
+               but we may have coverage gaps, so we should be careful
+               about changing the next line.
+        '''
+        user_ids = message['active_user_ids'] | set(user_flags.keys())
+
         users = [
             dict(
                 id=user_id,
@@ -1160,7 +1174,7 @@ def do_send_messages(messages_maybe_none):
                 always_push_notify=(user_id in message['push_notify_user_ids']),
                 stream_push_notify=(user_id in message['stream_push_user_ids']),
             )
-            for user_id in message['active_user_ids']
+            for user_id in user_ids
         ]
 
         if message['message'].is_stream_message():

zerver/tests/test_messages.py

@@ -68,7 +68,7 @@ import DNS
 import mock
 import time
 import ujson
-from typing import Any, Dict, List, Optional, Text
+from typing import Any, Dict, List, Optional, Set, Text
 
 from collections import namedtuple
 
@@ -593,6 +593,19 @@ class StreamMessagesTest(ZulipTestCase):
         message = most_recent_message(user_profile)
         assert(UserMessage.objects.get(user_profile=user_profile, message=message).flags.mentioned.is_set)
 
+    def _send_stream_message(self, email, stream_name, content):
+        # type: (Text, Text, Text) -> Set[int]
+        with mock.patch('zerver.lib.actions.send_event') as m:
+            self.send_stream_message(
+                email,
+                stream_name,
+                content=content
+            )
+        self.assertEqual(m.call_count, 1)
+        users = m.call_args[0][1]
+        user_ids = {u['id'] for u in users}
+        return user_ids
+
     def test_unsub_mention(self):
         # type: () -> None
         cordelia = self.example_user('cordelia')
@@ -607,14 +620,15 @@ class StreamMessagesTest(ZulipTestCase):
         ).delete()
 
         def mention_cordelia():
-            # type: () -> None
+            # type: () -> Set[int]
             content = 'test @**Cordelia Lear** rules'
 
-            self.send_stream_message(
-                hamlet.email,
-                stream_name,
-                content=content,
+            user_ids = self._send_stream_message(
+                email=hamlet.email,
+                stream_name=stream_name,
+                content=content
             )
+            return user_ids
 
         def num_cordelia_messages():
             # type: () -> int
@@ -622,15 +636,16 @@ class StreamMessagesTest(ZulipTestCase):
                 user_profile=cordelia
             ).count()
 
-        mention_cordelia()
-
+        user_ids = mention_cordelia()
         self.assertEqual(0, num_cordelia_messages())
+        self.assertNotIn(cordelia.id, user_ids)
 
         # Make sure test isn't too brittle-subscribing
         # Cordelia and mentioning her should give her a
         # message.
         self.subscribe(cordelia, stream_name)
-        mention_cordelia()
+        user_ids = mention_cordelia()
+        self.assertIn(cordelia.id, user_ids)
         self.assertEqual(1, num_cordelia_messages())
 
     def test_message_bot_mentions(self):
@@ -655,12 +670,13 @@ class StreamMessagesTest(ZulipTestCase):
 
         content = 'test @**Normal Bot** rules'
 
-        self.send_stream_message(
-            hamlet.email,
-            stream_name,
+        user_ids = self._send_stream_message(
+            email=hamlet.email,
+            stream_name=stream_name,
             content=content
         )
 
+        self.assertIn(normal_bot.id, user_ids)
         user_message = most_recent_usermessage(normal_bot)
         self.assertEqual(user_message.message.content, content)
         self.assertTrue(user_message.flags.mentioned)