diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 2e7a037eef..929a2311ad 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -865,9 +865,6 @@ def get_recipient_info(recipient, sender_id): def get_service_bot_events(sender, service_bot_tuples, mentioned_user_ids, recipient_type): # type: (UserProfile, List[Tuple[int, int]], Set[int], int) -> Dict[str, List[Dict[str, Any]]] - # TODO: Right now, service bots need to be subscribed to a stream in order to - # receive messages when mentioned; we will want to change that structure. - # Prepare to collect service queue events triggered by the message. event_dict = defaultdict(list) # type: Dict[str, List[Dict[str, Any]]] @@ -876,6 +873,27 @@ def get_service_bot_events(sender, service_bot_tuples, mentioned_user_ids, recip if sender.is_bot: return event_dict + if recipient_type == Recipient.STREAM: + known_ids = {user_profile_id for user_profile_id, bot_type in service_bot_tuples} + unknown_ids = mentioned_user_ids - known_ids + if unknown_ids: + ''' + If we mention a service bot in a stream message, we should notify + them. If the bot also happens to be subscribed to the stream, then + they would have already been in `service_bot_tuples`, but if + they are not subscribed to the stream, we need to go find them + in the follow up query below. Also, it's important to filter + for service bots only. + ''' + query = UserProfile.objects.filter( + id__in=unknown_ids, + is_active=True, + is_bot=True, + bot_type__in=UserProfile.SERVICE_BOT_TYPES, + ).values('id', 'bot_type') + tups = [(row['id'], row['bot_type']) for row in query] + service_bot_tuples += tups + for user_profile_id, bot_type in service_bot_tuples: if bot_type == UserProfile.OUTGOING_WEBHOOK_BOT: queue_name = 'outgoing_webhooks' diff --git a/zerver/tests/test_service_bot_system.py b/zerver/tests/test_service_bot_system.py index b129c11b6b..e2e5c6a9b8 100644 --- a/zerver/tests/test_service_bot_system.py +++ b/zerver/tests/test_service_bot_system.py @@ -84,6 +84,42 @@ class TestServiceBotBasics(ZulipTestCase): self.assertEqual(event_dict, expected) + def test_service_events_for_unsubscribed_stream_mentions(self): + # type: () -> None + sender = self.example_user('hamlet') + assert(not sender.is_bot) + + outgoing_bot = self._get_outgoing_bot() + + ''' + If an outgoing bot is mentioned on a stream message, we will + create an event for it even if it is not subscribed to the + stream and not part of our original `service_bot_tuples`. + + Note that we add Cordelia as a red herring value that the + code should ignore, since she is not a bot. + ''' + + cordelia = self.example_user('cordelia') + + event_dict = get_service_bot_events( + sender=sender, + service_bot_tuples=[], + mentioned_user_ids={ + outgoing_bot.id, + cordelia.id, # should be excluded, not a service bot + }, + recipient_type=Recipient.STREAM, + ) + + expected = dict( + outgoing_webhooks=[ + dict(trigger='mention', user_profile_id=outgoing_bot.id), + ], + ) + + self.assertEqual(event_dict, expected) + class TestServiceBotEventTriggers(ZulipTestCase): def setUp(self):