fix: Fix subscriptions sending twice (#21581)

ee/tasks/subscriptions/__init__.py

@@ -117,8 +117,8 @@ def _deliver_subscription_report(
         raise NotImplementedError(f"{subscription.target_type} is not supported")
 
     if not is_new_subscription_target:
-        subscription.set_next_delivery_date()
-        subscription.save()
+        subscription.set_next_delivery_date(subscription.next_delivery_date)
+        subscription.save(update_fields=["next_delivery_date"])
 
 
 @shared_task(queue=CeleryQueue.SUBSCRIPTION_DELIVERY.value)

ee/tasks/test/subscriptions/test_slack_subscriptions.py

@@ -13,7 +13,7 @@ from posthog.test.base import APIBaseTest
 
 
 @patch("ee.tasks.subscriptions.slack_subscriptions.SlackIntegration")
-@freeze_time("2022-02-02T08:55:00.000Z")
+@freeze_time("2022-02-02T08:30:00.000Z")
 class TestSlackSubscriptionsTasks(APIBaseTest):
     subscription: Subscription
     dashboard: Dashboard

ee/tasks/test/subscriptions/test_subscriptions.py

@@ -51,17 +51,18 @@ class TestSubscriptionsTasks(APIBaseTest):
         mock_send_email: MagicMock,
         mock_send_slack: MagicMock,
     ) -> None:
-        subscriptions = [
-            create_subscription(team=self.team, insight=self.insight, created_by=self.user),
-            create_subscription(team=self.team, insight=self.insight, created_by=self.user),
-            create_subscription(team=self.team, dashboard=self.dashboard, created_by=self.user),
-            create_subscription(
-                team=self.team,
-                dashboard=self.dashboard,
-                created_by=self.user,
-                deleted=True,
-            ),
-        ]
+        with freeze_time("2022-02-02T08:30:00.000Z"):  # Create outside of buffer before running
+            subscriptions = [
+                create_subscription(team=self.team, insight=self.insight, created_by=self.user),
+                create_subscription(team=self.team, insight=self.insight, created_by=self.user),
+                create_subscription(team=self.team, dashboard=self.dashboard, created_by=self.user),
+                create_subscription(
+                    team=self.team,
+                    dashboard=self.dashboard,
+                    created_by=self.user,
+                    deleted=True,
+                ),
+            ]
         # Modify a subscription to have its target time at least an hour ahead
         subscriptions[2].start_date = datetime(2022, 1, 1, 10, 0).replace(tzinfo=ZoneInfo("UTC"))
         subscriptions[2].save()

posthog/models/subscription.py

@@ -124,12 +124,16 @@ class Subscription(models.Model):
         )
 
     def set_next_delivery_date(self, from_dt=None):
-        self.next_delivery_date = self.rrule.after(dt=from_dt or timezone.now(), inc=False)
+        # We never want next_delivery_date to be in the past
+        now = timezone.now() + timedelta(minutes=15)  # Buffer of 15 minutes since we might run a bit early
+        self.next_delivery_date = self.rrule.after(dt=max(from_dt or now, now), inc=False)
 
     def save(self, *args, **kwargs) -> None:
         # Only if the schedule has changed do we update the next delivery date
         if not self.id or str(self._rrule) != str(self.rrule):
             self.set_next_delivery_date()
+            if "update_fields" in kwargs:
+                kwargs["update_fields"].append("next_delivery_date")
         super(Subscription, self).save(*args, **kwargs)
 
     @property

posthog/models/test/test_subscription_model.py

@@ -71,6 +71,40 @@ class TestSubscription(BaseTest):
         subscription.save()
         assert old_date == subscription.next_delivery_date
 
+    @freeze_time("2022-01-11 09:55:00")
+    def test_set_next_delivery_date_when_in_upcoming_delta(self):
+        subscription = Subscription.objects.create(
+            id=1,
+            team=self.team,
+            title="Daily Subscription",
+            target_type="email",
+            target_value="tests@posthog.com",
+            frequency="daily",
+            start_date=datetime(2022, 1, 1, 10, 0, 0, 0).replace(tzinfo=ZoneInfo("UTC")),
+            next_delivery_date=datetime(2022, 1, 11, 10, 0, 0, 0).replace(tzinfo=ZoneInfo("UTC")),
+        )
+
+        subscription.set_next_delivery_date(subscription.next_delivery_date)
+
+        assert subscription.next_delivery_date == datetime(2022, 1, 12, 10, 0, 0, 0).replace(tzinfo=ZoneInfo("UTC"))
+
+    @freeze_time("2022-01-11 09:55:00")
+    def test_set_next_delivery_date_when_days_behind(self):
+        subscription = Subscription.objects.create(
+            id=1,
+            team=self.team,
+            title="Daily Subscription",
+            target_type="email",
+            target_value="tests@posthog.com",
+            frequency="daily",
+            start_date=datetime(2022, 1, 1, 10, 0, 0, 0).replace(tzinfo=ZoneInfo("UTC")),
+            next_delivery_date=datetime(2022, 1, 2, 10, 0, 0, 0).replace(tzinfo=ZoneInfo("UTC")),
+        )
+
+        subscription.set_next_delivery_date(subscription.next_delivery_date)
+
+        assert subscription.next_delivery_date == datetime(2022, 1, 12, 10, 0, 0, 0).replace(tzinfo=ZoneInfo("UTC"))
+
     def test_generating_token(self):
         subscription = self._create_insight_subscription(
             target_value="test1@posthog.com,test2@posthog.com,test3@posthog.com"