Using existent Subscription objects along with dynamically generated …

…subscriptions

newsletter/models.py

@@ -580,10 +580,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.
         """
@@ -617,12 +616,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,22 +191,37 @@ 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)
         self.assertTrue(submission.sent)
-        self.assertEqual(len(mail.outbox), 3)
+        self.assertEquals(len(mail.outbox), 3)
 
     def test_non_existent_generator_class(self):
         """ Test failure when generator class does not exist """
-        self.n.subscription_generator_class = 'module.does.not.Exist'
-        self.n.save()
         try:
-            self.sub.submit()
-            self.fail('Should not have been able to send')
-        except (AttributeError, ModuleNotFoundError):
+            self.n.subscription_generator_class = 'nonexistent_module.SomeClass'
+            self.n.save()
+            self.fail('Should not reach this')
+        except ModuleNotFoundError:
+            pass
+
+        try:
+            self.n.subscription_generator_class = 'newsletter.models.NonexistentClass'
+            self.n.save()
+            self.fail('Should not reach this')
+        except AttributeError:
             pass
-        
+
 
 class SubmitSubmissionTestCase(MailingTestCase):
     def setUp(self):