feat(massEmails): allow one-offs to be re-sent

kobo/apps/mass_emails/admin.py

@@ -10,28 +10,24 @@ class MassEmailConfigAdmin(admin.ModelAdmin):
     fields = ('name', 'subject', 'template', 'query', 'frequency', 'live')
     actions = ['enqueue_mass_emails']
 
+    def get_readonly_fields(self, request, obj=None):
+        if obj and obj.type == EmailType.ONE_TIME:
+            if MassEmailRecord.objects.filter(
+                email_job__email_config__id=obj.id, status=EmailStatus.ENQUEUED
+            ).exists():
+                return ('live',)
+        return ()
+
     @admin.action(description='Add to daily send queue')
     def enqueue_mass_emails(self, request, queryset):
         for config in queryset:
             if config.type == EmailType.ONE_TIME:
-                if (
-                    MassEmailRecord.objects.filter(email_job__email_config=config)
-                    .exclude(status=EmailStatus.FAILED)
-                    .exists()
-                ):
+                if config.live:
                     self.message_user(
                         request,
-                        f'Emails for {config.name} have already been sent or enqueued. '
-                        f'Cannot send a one-time email twice.',
+                        f'Emails for {config.name} have already been scheduled',
                         level=messages.ERROR,
                     )
-                elif config.live:
-                    self.message_user(
-                        request,
-                        f'Emails for {config.name} have already been scheduled for'
-                        f' tomorrow',
-                        level=messages.SUCCESS,
-                    )
                 else:
                     config.live = True
                     config.save()

kobo/apps/mass_emails/tasks.py

@@ -316,6 +316,17 @@ def send_emails():
         return
     sender = MassEmailSender()
     sender.send_day_emails()
+    finished_one_offs = (
+        MassEmailConfig.objects.filter(frequency=-1, live=True)
+        .values('id')
+        .annotate(
+            enqueued_count=Count(
+                'pk', filter=Q(jobs__records__status=EmailStatus.ENQUEUED)
+            )
+        )
+        .filter(enqueued_count=0)
+    ).values_list('pk', flat=True)
+    MassEmailConfig.objects.filter(pk__in=finished_one_offs).update(live=False)
 
 
 def get_users_for_config(email_config):
@@ -374,14 +385,8 @@ def generate_mass_email_user_lists():
             email_job__email_config=email_config,
         )
 
-        # Skip processing for one time emails that have been sent or enqueued
-        if email_config.frequency == -1 and email_records.filter(
-            status__in=[EmailStatus.ENQUEUED, EmailStatus.SENT]
-        ).exists():
-            processed_configs.add(email_config.id)
-
-        # Skip processing if there are pending enqueued records
-        elif email_records.filter(status=EmailStatus.ENQUEUED).exists():
+        # Skip processing emails that have already been enqueued
+        if email_records.filter(status=EmailStatus.ENQUEUED).exists():
             logging.info(
                 f'Skipping email config {email_config.id} as it already has '
                 f'enqueued records.'

kobo/apps/mass_emails/tests/admin/test_add_to_send.py

@@ -30,56 +30,26 @@ def _add_to_send(self, email_config):
         return request
 
     @data(
-        # frequency, is_live, has_records, expect_live, expected_message_template
+        # frequency, is_live, expected_message_template
         (
             1,
             False,
-            False,
-            True,
             'Emails for {config} have been added to the daily send',
         ),
-        (1, False, True, True, 'Emails for {config} have been added to the daily send'),
-        (
-            1,
-            True,
-            False,
-            True,
-            'Emails for {config} are already part of the daily send',
-        ),
-        (1, True, True, True, 'Emails for {config} are already part of the daily send'),
+        (1, True, 'Emails for {config} are already part of the daily send'),
         (
             -1,
             False,
-            False,
-            True,
             'Emails for {config} have been scheduled for tomorrow',
         ),
         (
             -1,
-            False,
-            True,
-            False,
-            'Emails for {config} have already been sent or enqueued. Cannot send a one-time email twice.',  # noqa
-        ),
-        (
-            -1,
-            True,
-            False,
-            True,
-            'Emails for {config} have already been scheduled for tomorrow',
-        ),
-        (
-            -1,
-            True,
-            True,
             True,
-            'Emails for {config} have already been sent or enqueued. Cannot send a one-time email twice.',  # noqa
+            'Emails for {config} have already been scheduled',
         ),
     )
     @unpack
-    def test_admin_action(
-        self, frequency, is_live, has_records, expect_live, expected_message
-    ):
+    def test_admin_action(self, frequency, is_live, expected_message):
         """
         Test that selecting 'Add to daily send queue' in the admin
         sets the config to live or delivers an appropriate error
@@ -93,46 +63,29 @@ def test_admin_action(
             live=is_live,
         )
 
-        if has_records:
-            user = User.objects.get(username='someuser')
-            job = MassEmailJob.objects.create(email_config=email_config)
-            MassEmailRecord.objects.create(
-                email_job=job, user=user, status=EmailStatus.ENQUEUED
-            )
-
         # Attempt to enqueue the records via the admin action
         request = self._add_to_send(email_config)
         expected_message = expected_message.format(config=email_config.name)
 
         messages_list = [m.message for m in get_messages(request)]
         assert expected_message in messages_list
-        assert email_config.live == expect_live
+        assert email_config.live
 
-    def test_admin_action_ignores_failed_records_for_one_time_sends(self):
-        """
-        Test that selecting 'Add to daily send queue' allows re-adding a one-time send
-        if previous records all failed
-        """
+    def test_live_disabled_when_oneoff_is_enqueued(self):
         email_config = MassEmailConfig.objects.create(
             name='Test Config',
             subject='Test Subject',
             template='Test Template',
             query='users_inactive_for_365_days',
             frequency=-1,
-            live=False,
+            live=True,
         )
-
-        user = User.objects.get(username='someuser')
+        admin_instance = MassEmailConfigAdmin(MassEmailConfig, site)
+        request = self.client.request().wsgi_request
         job = MassEmailJob.objects.create(email_config=email_config)
         MassEmailRecord.objects.create(
-            email_job=job, user=user, status=EmailStatus.FAILED
+            email_job=job, user=User.objects.get(pk=1), status=EmailStatus.ENQUEUED
         )
-        request = self._add_to_send(email_config)
-
-        messages_list = [m.message for m in get_messages(request)]
-
-        expected_message = (
-            f'Emails for {email_config.name} have been scheduled for tomorrow'
-        )
-        assert expected_message in messages_list
-        assert email_config.live
+        readonly_fields = admin_instance.get_readonly_fields(request, email_config)
+        assert len(readonly_fields) == 1
+        assert readonly_fields[0] == 'live'

kobo/apps/mass_emails/tests/test_celery_tasks.py

@@ -311,56 +311,19 @@ def test_one_time_emails_deprioritized(
 
 @ddt
 class GenerateDailyEmailUserListTaskTestCase(BaseMassEmailsTestCase):
-    @data(
-        (EmailStatus.ENQUEUED, 1),
-        (EmailStatus.SENT, 0),
-        (EmailStatus.FAILED, 2),
-    )
-    @unpack
-    def test_one_time_email_with_existing_records(self, status, enqueued_count):
-        """
-        Test one-time email configs (frequency=-1) behave correctly
-
-        - If status is `ENQUEUED` or `SENT`, no new records should be created.
-        - If status is `FAILED`, new records should be created.
-        """
-        email_config = self._create_email_config('Test')
-        self._create_email_record(self.user1, email_config, status)
-
-        self.assertNotIn(email_config.id, cache.get(self.cache_key, set()))
-        generate_mass_email_user_lists()
-        records = MassEmailRecord.objects.filter(
-            email_job__email_config=email_config, status=EmailStatus.ENQUEUED
-        )
-        self.assertEqual(records.count(), enqueued_count)
-        self.assertIn(email_config.id, cache.get(self.cache_key))
-
-    def test_one_time_email_with_no_existing_records(self):
-        """
-        Test that new records are created when there are no existing records
-        """
-        email_config = self._create_email_config('Test')
-        self.assertNotIn(email_config.id, cache.get(self.cache_key, set()))
-        generate_mass_email_user_lists()
-
-        new_records = MassEmailRecord.objects.filter(
-            email_job__email_config=email_config, status=EmailStatus.ENQUEUED
-        )
-        self.assertEqual(new_records.count(), 2)
-        self.assertIn(email_config.id, cache.get(self.cache_key))
 
     @data(
-        (EmailStatus.ENQUEUED, 1, 1),
-        (EmailStatus.SENT, 1, 2),
-        (EmailStatus.FAILED, 1, 2),
+        (EmailStatus.ENQUEUED, -1, 1),
+        (EmailStatus.SENT, -1, 2),
+        (EmailStatus.FAILED, -1, 2),
         (EmailStatus.ENQUEUED, 2, 1),
         (EmailStatus.SENT, 2, 2),
         (EmailStatus.FAILED, 2, 2),
     )
     @unpack
     def test_recurring_email_scheduling(self, status, frequency, enqueued_count):
         """
-        Test that recurring email configs (frequency > 0) behave correctly
+        Test we don't enqueue records if there are already pending ones
         """
         email_config = self._create_email_config(
             'Test', frequency=frequency

kobo/settings/base.py

@@ -1419,7 +1419,7 @@ def dj_stripe_request_callback_method():
 if os.environ.get('EMAIL_USE_TLS'):
     EMAIL_USE_TLS = os.environ.get('EMAIL_USE_TLS')
 
-MAX_MASS_EMAILS_PER_DAY = 1000
+MAX_MASS_EMAILS_PER_DAY = 1
 MASS_EMAIL_THROTTLE_PER_SECOND = 40
 MASS_EMAIL_SLEEP_SECONDS = 1
 # change the interval between "daily" email sends for testing. this will set both