Rate-limit emails sent by the same user in a period of time

Author: mtomilov

Diff for: h/services/email.py

@@ -17,7 +17,7 @@
 logger = get_task_logger(__name__)
 
 # Limit for the number of mention emails sent by a single user in a day to prevent abuse
-DAILY_SENDER_MENTION_LIMIT = 5
+DAILY_SENDER_MENTION_LIMIT = 100
 
 
 @dataclass(frozen=True)
@@ -91,8 +91,10 @@ def _allow_sending(self, task_data: TaskData) -> bool:
             and self._sender_limit_reached(task_data)
         ):
             logger.warning(
-                "Email not sent for sender_id=%s: limit reached",
+                "Not sending email: tag=%r sender_id=%s recipient_ids=%s. Sender limit reached.",
+                task_data.tag,
                 task_data.sender_id,
+                task_data.recipient_ids,
             )
             return False
         return True

Diff for: tests/unit/h/services/email_test.py

@@ -69,26 +69,22 @@ def test_send_creates_email_message_with_subaccount(
             extra_headers={"X-MC-Tags": EmailTag.TEST, "X-MC-Subaccount": "subaccount"},
         )
 
-    def test_send_creates_email_with_mention(
-        self, email_data, task_data, email_service, pyramid_mailer, task_done_service
+    def test_send_creates_mention_email_when_sender_limit_not_reached(
+        self,
+        mention_email_data,
+        mention_task_data,
+        email_service,
+        pyramid_mailer,
+        task_done_service,
     ):
-        email_data = EmailData(
-            recipients=["[email protected]"],
-            subject="My email subject",
-            body="Some text body",
-            tag=EmailTag.MENTION_NOTIFICATION,
-        )
-        task_data = TaskData(
-            tag=email_data.tag,
-            sender_id=123,
-            recipient_ids=[124],
+        task_done_service.sender_mention_count.return_value = (
+            DAILY_SENDER_MENTION_LIMIT - 1
         )
-        task_done_service.sender_mention_count.return_value = DAILY_SENDER_MENTION_LIMIT
 
-        email_service.send(email_data, task_data)
+        email_service.send(mention_email_data, mention_task_data)
 
         task_done_service.sender_mention_count.assert_called_once_with(
-            task_data.sender_id, mock.ANY
+            mention_task_data.sender_id, mock.ANY
         )
         pyramid_mailer.message.Message.assert_called_once_with(
             recipients=["[email protected]"],
@@ -98,28 +94,20 @@ def test_send_creates_email_with_mention(
             extra_headers={"X-MC-Tags": EmailTag.MENTION_NOTIFICATION},
         )
 
-    def test_send_does_not_create_email_with_mention(
-        self, email_data, task_data, email_service, pyramid_mailer, task_done_service
+    def test_send_does_not_create_mention_email_when_sender_limit_reached(
+        self,
+        mention_email_data,
+        mention_task_data,
+        email_service,
+        pyramid_mailer,
+        task_done_service,
     ):
-        email_data = EmailData(
-            recipients=["[email protected]"],
-            subject="My email subject",
-            body="Some text body",
-            tag=EmailTag.MENTION_NOTIFICATION,
-        )
-        task_data = TaskData(
-            tag=email_data.tag,
-            sender_id=123,
-            recipient_ids=[124],
-        )
-        task_done_service.sender_mention_count.return_value = (
-            DAILY_SENDER_MENTION_LIMIT + 1
-        )
+        task_done_service.sender_mention_count.return_value = DAILY_SENDER_MENTION_LIMIT
 
-        email_service.send(email_data, task_data)
+        email_service.send(mention_email_data, mention_task_data)
 
         task_done_service.sender_mention_count.assert_called_once_with(
-            task_data.sender_id, mock.ANY
+            mention_task_data.sender_id, mock.ANY
         )
         pyramid_mailer.message.Message.assert_not_called()
 
@@ -159,12 +147,29 @@ def test_send_logging_with_extra(self, email_data, email_service, info_caplog):
             recipient_ids=[recipient_id],
             extra={"annotation_id": annotation_id},
         )
+
         email_service.send(email_data, task_data)
 
         assert info_caplog.messages == [
             f"Sent email: tag={task_data.tag!r}, sender_id={sender_id}, recipient_ids={[recipient_id]}, annotation_id={annotation_id!r}"
         ]
 
+    def test_sender_limit_reached_logging(
+        self,
+        mention_email_data,
+        mention_task_data,
+        email_service,
+        task_done_service,
+        info_caplog,
+    ):
+        task_done_service.sender_mention_count.return_value = DAILY_SENDER_MENTION_LIMIT
+
+        email_service.send(mention_email_data, mention_task_data)
+
+        assert info_caplog.messages == [
+            f"Not sending email: tag={mention_task_data.tag!r} sender_id={mention_task_data.sender_id} recipient_ids={mention_task_data.recipient_ids}. Sender limit reached."
+        ]
+
     def test_send_creates_task_done(
         self, email_data, task_data, email_service, task_done_service
     ):
@@ -174,6 +179,7 @@ def test_send_creates_task_done(
             recipient_ids=[124],
             extra={"annotation_id": "annotation_id"},
         )
+
         email_service.send(email_data, task_data)
 
         task_done_service.create.assert_called_once_with(task_data)
@@ -195,6 +201,23 @@ def task_data(self):
             recipient_ids=[124],
         )
 
+    @pytest.fixture
+    def mention_email_data(self):
+        return EmailData(
+            recipients=["[email protected]"],
+            subject="My email subject",
+            body="Some text body",
+            tag=EmailTag.MENTION_NOTIFICATION,
+        )
+
+    @pytest.fixture
+    def mention_task_data(self):
+        return TaskData(
+            tag=EmailTag.MENTION_NOTIFICATION,
+            sender_id=123,
+            recipient_ids=[124],
+        )
+
     @pytest.fixture
     def email_service(self, pyramid_request, pyramid_mailer, task_done_service):
         request_mailer = pyramid_mailer.get_mailer.return_value