From 19fa73891e23792cf7524ea33969cb848e334c8e Mon Sep 17 00:00:00 2001
From: Eeshan Garg <jerryguitarist@gmail.com>
Date: Sat, 5 May 2018 17:47:33 -0230
Subject: [PATCH] actions: Refactor rate-limiting logic out of
 send_pm_if_empty_stream.

It makes sense to refactor out the last_reminder logic out of
send_pm_if_empty_stream and have a generic function that can send
rate-limited PM notifications to a bot owner and can be used by
methods other than send_pm_if_empty_stream.
---
 zerver/lib/actions.py | 47 +++++++++++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 17 deletions(-)

diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py
index cd1c9613cd..2454721fd5 100644
--- a/zerver/lib/actions.py
+++ b/zerver/lib/actions.py
@@ -1835,13 +1835,13 @@ def check_default_stream_group_name(group_name: Text) -> None:
             raise JsonableError(_("Default stream group name '%s' contains NULL (0x00) characters."
                                 % (group_name)))
 
-def send_pm_if_empty_stream(sender: UserProfile,
-                            stream: Optional[Stream],
-                            stream_name: Text,
-                            realm: Realm) -> None:
-    """If a bot sends a message to a stream that doesn't exist or has no
-    subscribers, sends a notification to the bot owner (if not a
-    cross-realm bot) so that the owner can correct the issue."""
+def send_rate_limited_pm_notification_to_bot_owner(sender: UserProfile,
+                                                   realm: Realm,
+                                                   content: Text) -> None:
+    """
+    Sends a PM error notification to a bot's owner if one hasn't already
+    been sent in the last 5 minutes.
+    """
     if sender.realm.is_zephyr_mirror_realm or sender.realm.deactivated:
         return
 
@@ -1855,11 +1855,6 @@ def send_pm_if_empty_stream(sender: UserProfile,
     if sender.realm != realm:
         return
 
-    if stream is not None:
-        num_subscribers = num_subscribers_for_stream_id(stream.id)
-        if num_subscribers > 0:
-            return
-
     # We warn the user once every 5 minutes to avoid a flood of
     # PMs on a misconfigured integration, re-using the
     # UserProfile.last_reminder field, which is not used for bots.
@@ -1868,6 +1863,28 @@ def send_pm_if_empty_stream(sender: UserProfile,
     if last_reminder and timezone_now() - last_reminder <= waitperiod:
         return
 
+    internal_send_private_message(realm, get_system_bot(settings.NOTIFICATION_BOT),
+                                  sender.bot_owner, content)
+
+    sender.last_reminder = timezone_now()
+    sender.save(update_fields=['last_reminder'])
+
+
+def send_pm_if_empty_stream(sender: UserProfile,
+                            stream: Optional[Stream],
+                            stream_name: Text,
+                            realm: Realm) -> None:
+    """If a bot sends a message to a stream that doesn't exist or has no
+    subscribers, sends a notification to the bot owner (if not a
+    cross-realm bot) so that the owner can correct the issue."""
+    if not sender.is_bot or sender.bot_owner is None:
+        return
+
+    if stream is not None:
+        num_subscribers = num_subscribers_for_stream_id(stream.id)
+        if num_subscribers > 0:
+            return
+
     if stream is None:
         error_msg = "that stream does not yet exist. To create it, "
     else:
@@ -1879,11 +1896,7 @@ def send_pm_if_empty_stream(sender: UserProfile,
                "click the gear in the left-side stream list." %
                (sender.full_name, stream_name, error_msg))
 
-    internal_send_private_message(realm, get_system_bot(settings.NOTIFICATION_BOT),
-                                  sender.bot_owner, content)
-
-    sender.last_reminder = timezone_now()
-    sender.save(update_fields=['last_reminder'])
+    send_rate_limited_pm_notification_to_bot_owner(sender, realm, content)
 
 # check_message:
 # Returns message ready for sending with do_send_message on success or the error message (string) on error.