From 75c796f754555086042dab643c2e27a15f1c5a53 Mon Sep 17 00:00:00 2001 From: Mircea Lungu Date: Sun, 2 Aug 2026 00:29:43 +0300 Subject: [PATCH] Friend-share email: temporal debounce instead of first-unread-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The count-based rule (email only when unread==1) had a sharp edge: a single un-opened share silenced ALL future share emails to that friend until they zeroed their inbox. Replace with a temporal debounce — suppress only if an EARLIER share reached the recipient within SHARE_EMAIL_DEBOUNCE_MINUTES (10). Collapses a burst to one email; a genuinely later share still notifies. Keyed on id order so a simultaneous burst can't suppress all its members. Co-Authored-By: Claude Opus 4.8 --- zeeguu/core/emailer/shared_article.py | 25 ++++++--- zeeguu/core/model/shared_article.py | 21 +++++++- .../test_shared_article_email_debounce.py | 52 +++++++++++++++++++ 3 files changed, 90 insertions(+), 8 deletions(-) create mode 100644 zeeguu/core/test/test_shared_article_email_debounce.py diff --git a/zeeguu/core/emailer/shared_article.py b/zeeguu/core/emailer/shared_article.py index a2637c23..0e52a9b5 100644 --- a/zeeguu/core/emailer/shared_article.py +++ b/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 diff --git a/zeeguu/core/model/shared_article.py b/zeeguu/core/model/shared_article.py index db735ddb..a5260de9 100644 --- a/zeeguu/core/model/shared_article.py +++ b/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 ( diff --git a/zeeguu/core/test/test_shared_article_email_debounce.py b/zeeguu/core/test/test_shared_article_email_debounce.py new file mode 100644 index 00000000..e1f78cee --- /dev/null +++ b/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)