Trigger Suspended uses the end date rather than last updated date Fixes

#11 (#12)

* Trigger Suspended uses the end date rather than last updated date

CHANGELOG.md

@@ -1,5 +1,13 @@
 # Changelog
 
+## v1.0.0 (2019-07-03)
+
+- **Breaking Change** `suspended_timeout` now triggers for subscriptions in SUSPENDED state that are
+  `timeout_hours` past the `subscription.end` time. It used to trigger if `subscription.last_updated`
+  hadn't changed for `timeout_hours`, but if `trigger_suspended` was running daily, the subscription
+  was constantly being updated, and `trigger_suspended_timeout` would never find a record to `end()`.
+
+
 ## v0.5.1 (2019-06-13)
 
 - Only localise datetimes to dates when they are aware

README.md

@@ -116,14 +116,14 @@ Subscription.objects.trigger_suspended() -> int  # number of renewals
 ```
 
 Trigger subscriptions that have been suspended for longer than `timeout_hours` to
-expire:
+end (uses `subscription.end` date, not `subscription.last_updated`):
 
 ```
 Subscription.objects.trigger_suspended_timeout(timeout_hours=48) -> int  # number of suspensions
 ```
 
 Trigger subscriptions that have been stuck in renewing state for longer than `timeout_hours`
-to be marked as an error:
+to be marked as an error (uses `subscription.last_updated` to determine the timeout):
 
 ```
 Subscription.objects.trigger_stuck(timeout_hours=2) -> int  # number of error subscriptions

pyproject.toml

@@ -21,10 +21,14 @@ readme = "README.md"
 homepage = "http://github.com/kogan/"
 repository = "http://github.com/kogan/django-subscriptions/"
 documentation = "http://github.com/kogan/django-subscriptions/"
-version = "0.5.1"
+version = "1.0.0"
 description = "A django package for managing subscription states"
 license = "BSD-3-Clause"
-authors = ["Josh Smeaton <josh.smeaton@gmail.com>"]
+authors = [
+    "Josh Smeaton <josh.smeaton@gmail.com>",
+    "Simon Willcock <simon@willcock.com.au>",
+    "Alec McGavin <alec@mcgav.in>",
+]
 packages = [
     { include = "subscriptions", from = "src" },
 ]

src/subscriptions/__init__.py

@@ -1 +1 @@
-__version__ = "0.5.1"
+__version__ = "1.0.0"

src/subscriptions/models.py

@@ -4,9 +4,10 @@
 from datetime import date, datetime, timedelta
 
 from django.db import models
+from django.db.models.expressions import ExpressionWrapper as E
 from django.utils import timezone
 from django.utils.encoding import python_2_unicode_compatible
-from django_fsm import FSMIntegerField, transition
+from django_fsm import FSMIntegerField, can_proceed, transition
 from django_fsm_log.decorators import fsm_log_by
 
 from . import signals
@@ -109,9 +110,12 @@ def suspended_timeout(self, timeout_hours=48, timeout_days=None):
         if timeout_days is not None:
             timeout_hours = timeout_days * 24
 
-        return self.filter(
-            state=State.SUSPENDED, last_updated__lte=timezone.now() - timedelta(hours=timeout_hours)
-        )
+        return self.annotate(
+            cutoff=E(
+                models.F("end") + timedelta(hours=timeout_hours),
+                output_field=models.DateTimeField(),
+            )
+        ).filter(state=State.SUSPENDED, cutoff__lte=timezone.now())
 
     def stuck(self, timeout_hours=2):
         return self.filter(
@@ -151,6 +155,9 @@ def __str__(self):
             self.get_state_display(), as_date(self.start), as_date(self.end)
         )
 
+    def can_proceed(self, transition_method):
+        return can_proceed(transition_method)
+
     @transition(field=state, source=State.ACTIVE, target=State.EXPIRING)
     def cancel_autorenew(self):
         self.reason = ""

tests/test_subscriptions.py

@@ -189,13 +189,9 @@ def test_trigger_suspended(self, mock_signal):
 
     def test_trigger_suspended_timeout(self):
         due = Subscription.objects.create(state=State.SUSPENDED, end=self.days_ago)
-        not_due = Subscription.objects.create(state=State.SUSPENDED, end=self.days_ago)
+        not_due = Subscription.objects.create(state=State.SUSPENDED, end=self.hours_ago)
         renewing = Subscription.objects.create(state=State.RENEWING, end=self.days_ago)
 
-        # last_updated rather than end is used for this query
-        Subscription.objects.all().update(last_updated=self.days_ago)
-        Subscription.objects.filter(pk=not_due.pk).update(last_updated=self.hours_ago)
-
         self.assertEqual(Subscription.objects.suspended_timeout(timeout_hours=72).count(), 1)
         self.assertEqual(Subscription.objects.trigger_suspended_timeout(), 1)
         due_fresh = Subscription.objects.get(pk=due.pk)