Fix logic for checking whether an offer is active based on date.

- If start_datetime is null, then the offer has no start date.
- If end_datetime is null, then the offer never expires.

Use the ActiveOfferManager in Applicator.get_site_offers instead of
reimplementing the same logic there.

Fixes #2344.

src/oscar/apps/offer/abstract_models.py

@@ -89,10 +89,13 @@ class AbstractConditionalOffer(models.Model):
     # dates are ignored and only the dates from the voucher are used to
     # determine availability.
     start_datetime = models.DateTimeField(
-        _("Start date"), blank=True, null=True)
+        _("Start date"), blank=True, null=True,
+        help_text=_("Offers are active from the start date. "
+                    "Leave this empty if the offer has no start date."))
     end_datetime = models.DateTimeField(
         _("End date"), blank=True, null=True,
-        help_text=_("Offers are active until the end of the 'end date'"))
+        help_text=_("Offers are active until the end date. "
+                    "Leave this empty if the offer has no expiry date."))
 
     # Use this field to limit the number of times this offer can be applied in
     # total.  Note that a single order can apply an offer multiple times so

src/oscar/apps/offer/applicator.py

@@ -67,19 +67,8 @@ class Applicator(object):
         """
         Return site offers that are available to all users
         """
-        cutoff = now()
-        date_based = Q(
-            Q(start_datetime__lte=cutoff),
-            Q(end_datetime__gte=cutoff) | Q(end_datetime=None),
-        )
-
-        nondate_based = Q(start_datetime=None, end_datetime=None)
-
         ConditionalOffer = get_model('offer', 'ConditionalOffer')
-        qs = ConditionalOffer.objects.filter(
-            date_based | nondate_based,
-            offer_type=ConditionalOffer.SITE,
-            status=ConditionalOffer.OPEN)
+        qs = ConditionalOffer.active.filter(offer_type=ConditionalOffer.SITE)
         # Using select_related with the condition/benefit ranges doesn't seem
         # to work.  I think this is because both the related objects have the
         # FK to range with the same name.

src/oscar/apps/offer/managers.py

@@ -10,7 +10,8 @@ class ActiveOfferManager(models.Manager):
         cutoff = now()
         return super(ActiveOfferManager, self).get_queryset().filter(
             models.Q(end_datetime__gte=cutoff) | models.Q(end_datetime=None),
-            start_datetime__lte=cutoff).filter(status=self.model.OPEN)
+            models.Q(start_datetime__lte=cutoff) | models.Q(start_datetime=None),
+        ).filter(status=self.model.OPEN)
 
 
 class BrowsableRangeManager(models.Manager):

src/oscar/apps/offer/migrations/0006_auto_20170504_0616.py

@@ -0,0 +1,25 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11 on 2017-05-04 05:16
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('offer', '0005_auto_20170423_1217'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='conditionaloffer',
+            name='end_datetime',
+            field=models.DateTimeField(blank=True, help_text='Offers are active until the end date. Leave this empty if the offer has no expiry date.', null=True, verbose_name='End date'),
+        ),
+        migrations.AlterField(
+            model_name='conditionaloffer',
+            name='start_datetime',
+            field=models.DateTimeField(blank=True, help_text='Offers are active from the start date. Leave this empty if the offer has no start date.', null=True, verbose_name='Start date'),
+        ),
+    ]

tests/integration/offer/test_applicator.py

@@ -58,6 +58,19 @@ class TestOfferApplicator(TestCase):
         priorities = [offer.priority for offer in offers]
         self.assertEqual(sorted(priorities, reverse=True), priorities)
 
+    def test_get_site_offers(self):
+        site_offer = models.ConditionalOffer.objects.create(
+            name="globaloffer", condition=self.condition,
+            benefit=self.benefit, offer_type=models.ConditionalOffer.SITE)
+        session_offer = models.ConditionalOffer.objects.create(
+            name="sessionoffer", condition=self.condition,
+            benefit=self.benefit, offer_type=models.ConditionalOffer.SESSION)
+
+        site_offers = Applicator().get_site_offers()
+        # Only one offer should be returned
+        self.assertEqual(len(site_offers), 1)
+        self.assertEqual(site_offers[0].name, "globaloffer")
+
 
 class TestOfferApplicationsWrapper(TestCase):
 

tests/integration/offer/test_availability.py

@@ -59,6 +59,26 @@ class TestADateBasedConditionalOffer(TestCase):
     def test_is_active_on_end_datetime(self):
         self.assertTrue(self.offer.is_available(test_date=self.end))
 
+    def test_active_on_null_end_datetime(self):
+        # null end_datetime means offer should never expire
+        offer = models.ConditionalOffer(start_datetime=self.start,
+                                        end_datetime=None)
+        test = datetime.date(2017, 3, 10)
+        self.assertTrue(offer.is_available(test_date=test))
+
+    def test_active_on_null_start_datetime(self):
+        # null start_datetime means offer is active from the beginning of time
+        offer = models.ConditionalOffer(start_datetime=None,
+                                        end_datetime=self.end)
+        test = datetime.date(2000, 3, 10)
+        self.assertTrue(offer.is_available(test_date=test))
+
+    def test_active_on_null_start_and_end_datetime(self):
+        # null datetimes - offer is always available
+        offer = models.ConditionalOffer(start_datetime=None, end_datetime=None)
+        test = datetime.date(2017, 3, 10)
+        self.assertTrue(offer.is_available(test_date=test))
+
 
 class TestAConsumptionFrequencyBasedConditionalOffer(TestCase):
 

tests/integration/offer/test_manager.py

@@ -10,7 +10,6 @@ from oscar.apps.offer import models
 class TestActiveOfferManager(TestCase):
 
     def test_includes_offers_in_date_range(self):
-        # Create offer that is available but with the wrong status
         now = timezone.now()
         start = now - datetime.timedelta(days=1)
         end = now + datetime.timedelta(days=1)
@@ -19,6 +18,25 @@ class TestActiveOfferManager(TestCase):
         filtered_offers = models.ConditionalOffer.active.all()
         self.assertEqual(1, len(filtered_offers))
 
+    def test_includes_offers_with_null_start_date(self):
+        now = timezone.now()
+        end = now + datetime.timedelta(days=1)
+        factories.create_offer(start=None, end=end)
+        filtered_offers = models.ConditionalOffer.active.all()
+        self.assertEqual(1, len(filtered_offers))
+
+    def test_includes_offers_with_null_end_date(self):
+        now = timezone.now()
+        start = now - datetime.timedelta(days=1)
+        factories.create_offer(start=start, end=None)
+        filtered_offers = models.ConditionalOffer.active.all()
+        self.assertEqual(1, len(filtered_offers))
+
+    def test_includes_offers_with_null_start_and_end_date(self):
+        factories.create_offer(start=None, end=None)
+        filtered_offers = models.ConditionalOffer.active.all()
+        self.assertEqual(1, len(filtered_offers))
+
     def test_filters_out_expired_offers(self):
         # Create offer that is available but with the wrong status
         now = timezone.now()