Friend-share email: temporal debounce (fix silent suppression)

zeeguu/core/emailer/shared_article.py

@@ -6,15 +6,22 @@
 
 WEB_URL = "https://zeeguu.org"
 
+# Collapse a burst of shares to the same recipient into one email: a share is
+# only skipped if an earlier one reached them within this window. Generous
+# enough to cover a deliberate "share a few articles in one sitting" session,
+# short enough that a genuinely later share re-notifies.
+SHARE_EMAIL_DEBOUNCE_MINUTES = 10
+
 
 def send_shared_article_notification(to_user_id, from_user_id, shared_article_id):
     """Email the recipient that a friend shared an article. Best-effort; meant to
     run off the request thread via run_in_background (re-fetches everything by id).
 
-    Debounce without a cron or extra state: only the recipient's **first** unread
-    share triggers an email. Once they have an unread share they've been told;
-    further shares just accumulate in their inbox until they clear it, and then
-    the next share re-notifies. That collapses a burst of shares into one email.
+    Temporal debounce (no cron, no extra state): collapse a *burst* of shares to
+    the same recipient into one email, while still notifying a genuinely new
+    share later. We suppress only if an earlier share reached this recipient
+    within the last SHARE_EMAIL_DEBOUNCE_MINUTES — unlike the old count-based
+    rule, a single un-opened share no longer silences all future notifications.
 
     Globally gated by EMAIL_SENDING_ENABLED (checked inside ZeeguuMailer.send —
     the env-level "send real email at all" switch), and per-user by the
@@ -26,15 +33,19 @@ def send_shared_article_notification(to_user_id, from_user_id, shared_article_id
             return
         if not UserPreference.is_email_on_article_shared_enabled(recipient):
             return
-        # Only the first unread share triggers an email; the rest batch in-app.
-        if SharedArticle.unread_count_for(to_user_id) != 1:
-            return
 
         sharer = User.find_by_id(from_user_id)
         shared = SharedArticle.find_by_id(shared_article_id)
         if not shared:
             return
 
+        # Collapse a burst: skip the email if an earlier share reached this
+        # recipient in the debounce window (that one already notified them).
+        if SharedArticle.has_earlier_recent_share_to(
+            to_user_id, shared.id, within_minutes=SHARE_EMAIL_DEBOUNCE_MINUTES
+        ):
+            return
+
         # Prefer the recipient's personalized copy: its title is in *their*
         # language (so a cross-language share isn't a foreign headline), and its
         # id deep-links straight to their adapted read. Fall back to the

zeeguu/core/model/shared_article.py

@@ -1,4 +1,4 @@
-from datetime import datetime
+from datetime import datetime, timedelta
 
 from sqlalchemy import Column, Integer, String, DateTime, ForeignKey, func
 from sqlalchemy.orm import relationship
@@ -169,6 +169,25 @@ def inbox_for(cls, user_id: int):
             .all()
         )
 
+    @classmethod
+    def has_earlier_recent_share_to(cls, user_id: int, before_id: int, within_minutes: int) -> bool:
+        """Whether an EARLIER share reached this recipient within the last
+        ``within_minutes`` — used to collapse a burst into one notification email.
+
+        Keyed on ``id < before_id`` (not just "any other"), so it's asymmetric:
+        the first share of a simultaneous burst has no earlier neighbour and
+        notifies; the rest are suppressed. That avoids the race where every
+        member sees a sibling and none notifies.
+        """
+        cutoff = datetime.now() - timedelta(minutes=within_minutes)
+        return (
+            cls.query.filter(cls.to_user_id == user_id)
+            .filter(cls.id < before_id)
+            .filter(cls.created_at >= cutoff)
+            .count()
+            > 0
+        )
+
     @classmethod
     def unread_count_for(cls, user_id: int) -> int:
         return (

zeeguu/core/test/test_shared_article_email_debounce.py

@@ -0,0 +1,52 @@
+"""Temporal debounce for the share-notification email: a burst to the same
+recipient collapses to one email, but a genuinely later share still notifies.
+See emailer/shared_article.py.
+"""
+from datetime import datetime, timedelta
+from unittest import TestCase
+
+from zeeguu.core.test.model_test_mixin import ModelTestMixIn
+from zeeguu.core.test.rules.user_rule import UserRule
+from zeeguu.core.test.rules.article_rule import ArticleRule
+
+import zeeguu.core
+from zeeguu.core.model.shared_article import SharedArticle
+
+session = zeeguu.core.model.db.session
+
+
+class SharedArticleEmailDebounceTest(ModelTestMixIn, TestCase):
+    def setUp(self):
+        super().setUp()
+        self.sender = UserRule().user
+        self.recipient = UserRule().user
+        self.article = ArticleRule().article
+
+    def _share_at(self, when, to_user=None):
+        shared = SharedArticle.create(
+            session, self.sender.id, (to_user or self.recipient).id, self.article.id
+        )
+        shared.created_at = when  # explicit, so the test doesn't depend on DB clock/tz
+        session.add(shared)
+        session.commit()
+        return shared
+
+    def test_first_of_a_burst_notifies_rest_suppressed(self):
+        now = datetime.now()
+        first = self._share_at(now)
+        second = self._share_at(now)
+        # First has no earlier neighbour → it notifies; second is suppressed.
+        assert not SharedArticle.has_earlier_recent_share_to(self.recipient.id, first.id, 10)
+        assert SharedArticle.has_earlier_recent_share_to(self.recipient.id, second.id, 10)
+
+    def test_share_after_the_window_notifies_again(self):
+        self._share_at(datetime.now() - timedelta(minutes=30))
+        later = self._share_at(datetime.now())
+        # The earlier share is outside the 10-min window → the later one notifies.
+        assert not SharedArticle.has_earlier_recent_share_to(self.recipient.id, later.id, 10)
+
+    def test_other_recipients_do_not_count(self):
+        other = UserRule().user
+        self._share_at(datetime.now())  # to self.recipient
+        to_other = self._share_at(datetime.now(), to_user=other)
+        assert not SharedArticle.has_earlier_recent_share_to(other.id, to_other.id, 10)