[ENG-11797] Unit tests on updated campaign workflow - Part 2

admin_tests/notifications/test_campaigns.py

@@ -7,6 +7,8 @@
 from django.core.exceptions import PermissionDenied
 from django.test import RequestFactory
 from django.urls import reverse
+from django.utils import timezone
+from datetime import timedelta
 
 from admin.notifications.forms import NotificationCampaignCreateForm
 from admin.notifications.views import (
@@ -73,13 +75,15 @@ def test_valid_form_parses_context_and_filters(self, notification_type):
         assert form.cleaned_data['batch_size'] == settings.DEFAULT_CAMPAIGN_BATCH_SIZE
         assert form.cleaned_data['max_retries'] == settings.DEFAULT_CAMPAIGN_MAX_RETRIES
         assert form.cleaned_data['activity_threshold'] == settings.DEFAULT_CAMPAIGN_ACTIVITY_THRESHOLD
+        assert form.cleaned_data['time_window'] == 8
         assert form.cleaned_data['sendgrid_bulk'] is False
 
     def test_defaults_come_from_settings(self):
         form = NotificationCampaignCreateForm()
         assert form.fields['batch_size'].initial == settings.DEFAULT_CAMPAIGN_BATCH_SIZE
         assert form.fields['max_retries'].initial == settings.DEFAULT_CAMPAIGN_MAX_RETRIES
         assert form.fields['activity_threshold'].initial == settings.DEFAULT_CAMPAIGN_ACTIVITY_THRESHOLD
+        assert form.fields['time_window'].initial == 8
         assert form.fields['sendgrid_bulk'].initial is False
 
     def test_invalid_context_json(self, notification_type):
@@ -125,6 +129,20 @@ def test_activity_threshold_cannot_be_negative(self, notification_type):
         assert not form.is_valid()
         assert 'activity_threshold' in form.errors
 
+    def test_time_window_must_be_at_least_one(self, notification_type):
+        form = NotificationCampaignCreateForm(
+            data=_valid_form_data(notification_type, time_window=0)
+        )
+        assert not form.is_valid()
+        assert 'time_window' in form.errors
+
+    def test_time_window_is_accepted(self, notification_type):
+        form = NotificationCampaignCreateForm(
+            data=_valid_form_data(notification_type, time_window=6)
+        )
+        assert form.is_valid()
+        assert form.cleaned_data['time_window'] == 6
+
     def test_name_is_required(self, notification_type):
         form = NotificationCampaignCreateForm(
             data=_valid_form_data(notification_type, name='')
@@ -266,7 +284,35 @@ def test_detail_requires_change_permission(self):
         response = NotificationCampaignDetail.as_view()(request, pk=self.campaign.pk)
         assert response.status_code == 200
 
+    def test_detail_allow_restart_stuck_false_when_recently_updated(self):
+        grant_permission(self.user, 'change_notificationcampaign')
+        request = RequestFactory().get(
+            reverse('notifications:notification_campaigns_detail', kwargs={'pk': self.campaign.pk})
+        )
+        request.user = self.user
+
+        response = NotificationCampaignDetail.as_view()(request, pk=self.campaign.pk)
+
+        assert response.status_code == 200
+        assert response.context_data['allow_restart_stuck'] is False
+
+    def test_detail_allow_restart_stuck_true_when_updated_long_time_ago(self):
+        grant_permission(self.user, 'change_notificationcampaign')
+        NotificationCampaign.objects.filter(pk=self.campaign.pk).update(
+            updated_at=timezone.now() - timedelta(minutes=16),
+        )
+        request = RequestFactory().get(
+            reverse('notifications:notification_campaigns_detail', kwargs={'pk': self.campaign.pk})
+        )
+        request.user = self.user
+
+        response = NotificationCampaignDetail.as_view()(request, pk=self.campaign.pk)
+
+        assert response.status_code == 200
+        assert response.context_data['allow_restart_stuck'] is True
+
     def test_start_requires_change_notificationcampaign_permission(self):
+
         request = RequestFactory().post(
             reverse('notifications:notification_campaigns_start', kwargs={'pk': self.campaign.pk})
         )

osf_tests/test_notification_campaign.py

@@ -620,6 +620,55 @@ def test_send_campaign_batch_skips_cancelled_campaign(self, running_campaign):
         recipient.refresh_from_db()
         assert recipient.status == NotificationCampaignRecipientStatus.PENDING
 
+    @mock.patch.object(NotificationType, 'emit')
+    def test_send_campaign_batch_uses_fallback_email_when_username_has_no_at(self, mock_emit, running_campaign):
+        user = UserFactory()
+        user.username = 'invalid'
+        user.save(update_fields=['username'])
+        user.emails.all().delete()
+        user.emails.create(address='fallback@example.com')
+
+        create_campaign_recipients(filters={'id__in': [user.id]}, campaign_id=running_campaign.id)
+        recipient = NotificationCampaignRecipient.objects.get(campaign=running_campaign, user=user)
+
+        send_campaign_batch(
+            context={},
+            recipients_ids=[recipient.id],
+            notification_type_name='blank',
+            campaign_id=running_campaign.id,
+            run_id=running_campaign.run_id,
+        )
+
+        recipient.refresh_from_db()
+        running_campaign.refresh_from_db()
+        assert recipient.status == NotificationCampaignRecipientStatus.SENT
+        assert running_campaign.sent_count == 1
+        assert running_campaign.failed_count == 0
+        mock_emit.assert_called_once()
+
+
+class TestNotificationCampaignCancel:
+
+    def test_cancel_sets_cancelled_status_and_completed_at(self, campaign):
+        assert campaign.completed_at is None
+
+        campaign.cancel()
+
+        campaign.refresh_from_db()
+        assert campaign.status == NotificationCampaignStatus.CANCELLED
+        assert campaign.completed_at is not None
+
+    def test_cancel_does_not_overwrite_existing_completed_at(self, campaign):
+        completed_at = timezone.now() - timedelta(hours=1)
+        campaign.completed_at = completed_at
+        campaign.save(update_fields=['completed_at'])
+
+        campaign.cancel()
+
+        campaign.refresh_from_db()
+        assert campaign.status == NotificationCampaignStatus.CANCELLED
+        assert campaign.completed_at == completed_at
+
 
 class TestProcessCampaignRetry:
 
@@ -636,8 +685,10 @@ def test_process_campaign_retry_marks_completed_and_aggregates_stats(self, campa
         NotificationCampaignRecipient.objects.filter(campaign=campaign, user=skipped_user).update(
             status=NotificationCampaignRecipientStatus.SKIPPED
         )
+        campaign.run_id = uuid.uuid4()
+        campaign.save(update_fields=['run_id'])
 
-        process_campaign_retry(campaign_id=campaign.id)
+        process_campaign_retry(campaign_id=campaign.id, run_id=campaign.run_id)
 
         campaign.refresh_from_db()
         assert campaign.status == NotificationCampaignStatus.COMPLETED
@@ -646,6 +697,51 @@ def test_process_campaign_retry_marks_completed_and_aggregates_stats(self, campa
         assert campaign.failed_count == 1
         assert campaign.completed_at is not None
 
+    def test_process_campaign_retry_skips_stale_run_id(self, campaign):
+        user = UserFactory()
+        create_campaign_recipients(filters={'id__in': [user.id]}, campaign_id=campaign.id)
+        NotificationCampaignRecipient.objects.filter(campaign=campaign).update(
+            status=NotificationCampaignRecipientStatus.SENT
+        )
+        campaign.run_id = uuid.uuid4()
+        campaign.status = NotificationCampaignStatus.RUNNING
+        campaign.save()
+
+        process_campaign_retry(campaign_id=campaign.id, run_id=uuid.uuid4())
+
+        campaign.refresh_from_db()
+        assert campaign.status == NotificationCampaignStatus.RUNNING
+        assert campaign.completed_at is None
+        assert campaign.sent_count == 0
+
+    @mock.patch('osf.email.notification_campaign.sentry.log_message')
+    def test_process_campaign_retry_keeps_cancelled_status_and_syncs_stats(self, mock_sentry, campaign):
+        sent_user = UserFactory()
+        failed_user = UserFactory()
+        create_campaign_recipients(
+            filters={'id__in': [sent_user.id, failed_user.id]},
+            campaign_id=campaign.id,
+        )
+        NotificationCampaignRecipient.objects.filter(campaign=campaign, user=sent_user).update(
+            status=NotificationCampaignRecipientStatus.SENT
+        )
+        NotificationCampaignRecipient.objects.filter(campaign=campaign, user=failed_user).update(
+            status=NotificationCampaignRecipientStatus.FAILED
+        )
+        campaign.run_id = uuid.uuid4()
+        campaign.status = NotificationCampaignStatus.CANCELLED
+        campaign.save()
+
+        process_campaign_retry(campaign_id=campaign.id, run_id=campaign.run_id)
+
+        campaign.refresh_from_db()
+        assert campaign.status == NotificationCampaignStatus.CANCELLED
+        assert campaign.recipient_count == 2
+        assert campaign.sent_count == 1
+        assert campaign.failed_count == 1
+        assert campaign.completed_at is not None
+        mock_sentry.assert_called_once()
+
     @mock.patch('osf.email.notification_campaign.chain')
     def test_process_campaign_retry_retries_failed_recipients(self, mock_chain, campaign):
         user = UserFactory()
@@ -655,14 +751,15 @@ def test_process_campaign_retry_retries_failed_recipients(self, mock_chain, camp
         recipient.save(update_fields=['status'])
         campaign.run_id = uuid.uuid4()
         campaign.retries = 0
+        campaign.status = NotificationCampaignStatus.RUNNING
         campaign.save()
         mock_chain.return_value.apply_async = mock.Mock()
 
         process_campaign_retry(campaign_id=campaign.id, run_id=campaign.run_id)
 
         campaign.refresh_from_db()
         assert campaign.retries == 1
-        assert campaign.status != NotificationCampaignStatus.PARTIALLY_COMPLETED
+        assert campaign.status == NotificationCampaignStatus.RUNNING
         mock_chain.assert_called_once()
 
     def test_process_campaign_retry_marks_partially_completed_after_max_retries(self, campaign):
@@ -671,10 +768,11 @@ def test_process_campaign_retry_marks_partially_completed_after_max_retries(self
         recipient = NotificationCampaignRecipient.objects.get(campaign=campaign, user=user)
         recipient.status = NotificationCampaignRecipientStatus.FAILED
         recipient.save(update_fields=['status'])
+        campaign.run_id = uuid.uuid4()
         campaign.retries = 2
-        campaign.save(update_fields=['retries'])
+        campaign.save()
 
-        process_campaign_retry(campaign_id=campaign.id)
+        process_campaign_retry(campaign_id=campaign.id, run_id=campaign.run_id)
 
         campaign.refresh_from_db()
         assert campaign.status == NotificationCampaignStatus.PARTIALLY_COMPLETED