Using existing Subscription objects alongside dynamic subscription

newsletter/models.py

@@ -576,10 +576,9 @@ class SubscriptionGenerator:
     Users must implement the generate_subscriptions method.
     """
     @abstractmethod
-    def generate_subscriptions(self, submission, subscriptions):
+    def generate_subscriptions(self, submission):
         """
         :param submission: the submission for which we are generating the subscription list
-        :param subscriptions: the original subscriptions for this submission
         :return: the list of Subscription objects.
         They may just be in memory Subscription objects, no need to save them to the DB.
         """
@@ -613,12 +612,15 @@ def extra_headers(self):
         }
 
     def submit(self):
-        subscriptions = self.subscriptions.filter(subscribed=True).all()
-
-        subscription_generator = self.newsletter.get_subscription_generator()
-        if subscription_generator:
-            logger.info('Dynamically generating subscriptions')
-            subscriptions = subscription_generator.generate_subscriptions(self, subscriptions)
+        subscriptions = list(self.subscriptions.filter(subscribed=True).all())
+
+        if self.newsletter.subscription_generator_class:
+            logger.info("Dynamically generating subscriptions")
+            subscribed_emails = {s.email for s in subscriptions}
+            unsubscribed_emails = {s.email for s in self.newsletter.subscription_set.filter(unsubscribed=True).all()}
+            dynamic_subscriptions = self.newsletter.get_subscription_generator().generate_subscriptions(self)
+            subscriptions += (s for s in dynamic_subscriptions
+                              if s.email not in subscribed_emails and s.email not in unsubscribed_emails)
 
         logger.info(
             gettext("Submitting %(submission)s to %(count)d people"),

tests/test_mailing.py

@@ -173,11 +173,11 @@ def test_submission_unsubscribed_unactivated(self):
 
 
 class TestingSubscriptionGenerator(SubscriptionGenerator):
-    def generate_subscriptions(self, submission, subscriptions):
+    def generate_subscriptions(self, submission):
         return [
-            Subscription(name_field='name1', email_field='test1@test.com'),
-            Subscription(name_field='name2', email_field='test2@test.com'),
-            Subscription(name_field='name3', email_field='test3@test.com')
+            Subscription(newsletter=submission.newsletter, name_field='name 2', email_field='test2@test.com'),
+            Subscription(newsletter=submission.newsletter, name_field='name 3', email_field='test3@test.com'),
+            Subscription(newsletter=submission.newsletter, name_field='name 4', email_field='test4@test.com'),
         ]
 
 
@@ -191,6 +191,15 @@ def setUp(self):
 
     def test_subscription_generator(self):
         """ Test the dynamic generation of subscriptors """
+        # Manually add some subscriptions, including an unsubscription
+        Subscription.objects.filter(newsletter=self.n).delete()
+        sub1 = Subscription.objects.create(name='name 1', email='[email protected]', newsletter=self.n, subscribed=True)
+        sub2 = Subscription.objects.create(name='name 2', email='[email protected]', newsletter=self.n, subscribed=True)
+        sub3 = Subscription.objects.create(name='name 3', email='[email protected]', newsletter=self.n, unsubscribed=True)
+        self.sub.subscriptions.add(sub1)
+        self.sub.subscriptions.add(sub2)
+        self.sub.subscriptions.add(sub3)
+
         self.sub.submit()
         Submission.submit_queue()
         submission = Submission.objects.get(pk=self.sub.pk)