return the block reason in should_disable()

app/email_utils.py

@@ -1118,15 +1118,17 @@ def normalize_reply_email(reply_email: str) -> str:
     return "".join(ret)
 
 
-def should_disable(alias: Alias) -> bool:
+def should_disable(alias: Alias) -> (bool, str):
-    """Disable an alias if it has too many bounces recently"""
+    """
+    Return whether an alias should be disabled and if yes, the reason why
+    """
     # Bypass the bounce rule
     if alias.cannot_be_disabled:
         LOG.w("%s cannot be disabled", alias)
-        return False
+        return False, ""
 
     if not ALIAS_AUTOMATIC_DISABLE:
-        return False
+        return False, ""
 
     yesterday = arrow.now().shift(days=-1)
     nb_bounced_last_24h = (
@@ -1141,8 +1143,7 @@ def should_disable(alias: Alias) -> bool:
     )
     # if more than 12 bounces in 24h -> disable alias
     if nb_bounced_last_24h > 12:
-        LOG.d("more than 12 bounces in the last 24h, disable alias %s", alias)
+        return True, "more than 12 bounces in the last 24h"
-        return True
 
     # if more than 5 bounces but has bounces last week -> disable alias
     elif nb_bounced_last_24h > 5:
@@ -1159,15 +1160,13 @@ def should_disable(alias: Alias) -> bool:
             .count()
         )
         if nb_bounced_7d_1d > 1:
-            LOG.d(
+            return (
-                "more than 5 bounces in the last 24h and more than 1 bounces in the last 7 days, "
+                True,
-                "disable alias %s",
+                "more than 5 bounces in the last 24h and more than 1 bounces in the last 7 days",
-                alias,
             )
-            return True
     else:
         # alias level
-        # if bounces at least 9 days in the last 10 days -> disable alias
+        # if bounces happen for at least 9 days in the last 10 days -> disable alias
         query = (
             Session.query(
                 func.date(EmailLog.created_at).label("date"),
@@ -1183,11 +1182,7 @@ def should_disable(alias: Alias) -> bool:
         )
 
         if query.count() >= 9:
-            LOG.d(
+            return True, "Bounces every day for at least 9 days in the last 10 days"
-                "Bounces every day for at least 9 days in the last 10 days, disable alias %s",
-                alias,
-            )
-            return True
 
         # account level
         query = (
@@ -1206,16 +1201,13 @@ def should_disable(alias: Alias) -> bool:
 
         # if an account has more than 10 bounces every day for at least 4 days in the last 10 days, disable alias
         date_bounces: List[Tuple[arrow.Arrow, int]] = list(query)
-        if len(date_bounces) > 4:
+        more_than_10_bounces = [
-            if all([v > 10 for _, v in date_bounces]):
+            (d, nb_bounce) for d, nb_bounce in date_bounces if nb_bounce > 10
-                LOG.d(
+        ]
-                    "+10 bounces for +4 days in the last 10 days on %s, disable alias %s",
+        if len(more_than_10_bounces) > 4:
-                    alias.user,
+            return True, "+10 bounces for +4 days in the last 10 days"
-                    alias,
-                )
-            return True
 
-    return False
+    return False, ""
 
 
 def parse_id_from_bounce(email_address: str) -> int:

email_handler.py

@@ -1396,7 +1396,8 @@ def handle_bounce_forward_phase(msg: Message, email_log: EmailLog):
     refused_email_url = f"{URL}/dashboard/refused_email?highlight_id={email_log.id}"
 
     # inform user of this bounce
-    if not should_disable(alias):
+    alias_will_be_disabled, reason = should_disable(alias)
+    if not alias_will_be_disabled:
         LOG.d(
             "Inform user %s about a bounce from contact %s to alias %s",
             user,
@@ -1447,7 +1448,9 @@ def handle_bounce_forward_phase(msg: Message, email_log: EmailLog):
             ignore_smtp_error=True,
         )
     else:
-        LOG.w("Disable alias %s %s. Last contact %s", alias, user, contact)
+        LOG.w(
+            f"Disable alias {alias} because {reason}. {alias.mailboxes} {alias.user}. Last contact {contact}"
+        )
         alias.enabled = False
         Session.commit()
 

tests/test_email_utils.py

@@ -610,7 +610,7 @@ def test_should_disable(flask_client):
     alias = Alias.create_new_random(user)
     Session.commit()
 
-    assert not should_disable(alias)
+    assert not should_disable(alias)[0]
 
     # create a lot of bounce on this alias
     contact = Contact.create(
@@ -629,12 +629,12 @@ def test_should_disable(flask_client):
             bounced=True,
         )
 
-    assert should_disable(alias)
+    assert should_disable(alias)[0]
 
     # should not affect another alias
     alias2 = Alias.create_new_random(user)
     Session.commit()
-    assert not should_disable(alias2)
+    assert not should_disable(alias2)[0]
 
 
 def test_should_disable_bounces_every_day(flask_client):
@@ -643,7 +643,7 @@ def test_should_disable_bounces_every_day(flask_client):
     alias = Alias.create_new_random(user)
     Session.commit()
 
-    assert not should_disable(alias)
+    assert not should_disable(alias)[0]
 
     # create a lot of bounce on this alias
     contact = Contact.create(
@@ -663,7 +663,7 @@ def test_should_disable_bounces_every_day(flask_client):
             created_at=arrow.now().shift(days=-i),
         )
 
-    assert should_disable(alias)
+    assert should_disable(alias)[0]
 
 
 def test_should_disable_bounces_account(flask_client):
@@ -682,8 +682,8 @@ def test_should_disable_bounces_account(flask_client):
         commit=True,
     )
 
-    for day in range(6):
+    for day in range(5):
-        for _ in range(10):
+        for _ in range(11):
             EmailLog.create(
                 user_id=user.id,
                 contact_id=contact.id,
@@ -694,7 +694,7 @@ def test_should_disable_bounces_account(flask_client):
             )
 
     alias2 = Alias.create_new_random(user)
-    assert should_disable(alias2)
+    assert should_disable(alias2)[0]
 
 
 def test_should_disable_bounce_consecutive_days(flask_client):
@@ -719,7 +719,7 @@ def test_should_disable_bounce_consecutive_days(flask_client):
             commit=True,
             bounced=True,
         )
-    assert not should_disable(alias)
+    assert not should_disable(alias)[0]
 
     # create 2 bounces in the last 7 days: alias should be disabled
     for _ in range(2):
@@ -731,7 +731,7 @@ def test_should_disable_bounce_consecutive_days(flask_client):
             bounced=True,
             created_at=arrow.now().shift(days=-3),
         )
-    assert should_disable(alias)
+    assert should_disable(alias)[0]
 
 
 def test_parse_id_from_bounce():