[ENG-12449] Project PR: Post-ENTER-P2

admin/notifications/forms.py

@@ -65,6 +65,12 @@ class NotificationCampaignCreateForm(forms.ModelForm):
         help_text='The time in seconds between dispatches of campaign batches.',
     )
 
+    log_email_recipients = forms.CharField(
+        required=False,
+        initial='',
+        help_text='List of email addresses to receive campaign log emails, separated by commas.',
+    )
+
     class Meta:
         model = NotificationCampaign
         fields = (

admin/notifications/views.py

@@ -19,7 +19,7 @@
 from mako.parsetree import ControlLine
 from string import Formatter
 from osf.email import _render_email_html
-from osf.email.notification_campaign import FILTER_PRESETS, counter_subquery, build_campaign_filter_query
+from osf.email.notification_campaign import FILTER_PRESETS, counter_subquery, build_campaign_filter_query, get_campaign_recipient_stats
 from website import settings
 from urllib.parse import urlencode
 
@@ -392,6 +392,19 @@ def get_object(self, queryset=None):
     def get_context_data(self, *args, **kwargs):
         notification_campaign = self.get_object()
         metadata = notification_campaign.metadata or {}
+        stats = get_campaign_recipient_stats(notification_campaign.id)
+        queued = stats['queued_count']
+        awaiting = stats['awaiting_count']
+        # Recipient rows are the source of truth while they exist; the stored counters
+        # are a snapshot for campaigns whose recipients were purged.
+        if stats['recipient_count']:
+            recipient_count = stats['recipient_count']
+            sent_count = stats['sent_count']
+            failed_count = stats['failed_count']
+        else:
+            recipient_count = notification_campaign.recipient_count
+            sent_count = notification_campaign.sent_count
+            failed_count = notification_campaign.failed_count
 
         context = {
             'notification_campaign': notification_campaign,
@@ -401,9 +414,11 @@ def get_context_data(self, *args, **kwargs):
                 ('Created By', notification_campaign.created_by),
                 ('Developer reminder ', 'Sent' if notification_campaign.developer_reminder_sent else ''),
                 ('Status', notification_campaign.get_status_display()),
-                ('Recipients', notification_campaign.recipient_count),
-                ('Sent', notification_campaign.sent_count),
-                ('Failed', notification_campaign.failed_count),
+                ('Recipients', recipient_count),
+                ('Sent', sent_count),
+                ('Queued', queued),
+                ('Awaiting Delivery', awaiting),
+                ('Failed', failed_count),
                 ('Retries', notification_campaign.retries),
                 ('Created', notification_campaign.created_at),
                 ('Started', notification_campaign.started_at),
@@ -436,19 +451,39 @@ def get_context_data(self, *args, **kwargs):
         }
 
         if notification_campaign.status != NotificationCampaignStatus.CREATED:
-            processed = notification_campaign.sent_count + notification_campaign.failed_count
-            pending = max(notification_campaign.recipient_count - processed, 0)
+            processed = sent_count + failed_count
+            pending = max(
+                recipient_count - processed - queued - awaiting,
+                0,
+            )
+            awaiting_delivery = (
+                notification_campaign.status == NotificationCampaignStatus.RUNNING
+                and pending == 0
+                and queued == 0
+                and awaiting > 0
+            )
 
             sent_percent = (
-                notification_campaign.sent_count / notification_campaign.recipient_count * 100
-                if notification_campaign.recipient_count else 0
+                sent_count / recipient_count * 100
+                if recipient_count else 0
             )
             failed_percent = (
-                notification_campaign.failed_count / notification_campaign.recipient_count * 100
-                if notification_campaign.recipient_count else 0
+                failed_count / recipient_count * 100
+                if recipient_count else 0
+            )
+            queued_percent = (
+                queued / recipient_count * 100
+                if recipient_count else 0
+            )
+            awaiting_percent = (
+                awaiting / recipient_count * 100
+                if recipient_count else 0
             )
 
-            pending_percent = max(100 - sent_percent - failed_percent, 0)
+            pending_percent = max(
+                100 - sent_percent - failed_percent - queued_percent - awaiting_percent,
+                0,
+            )
             elapsed = None
             speed = None
             eta = None
@@ -462,22 +497,31 @@ def get_context_data(self, *args, **kwargs):
                 elapsed_seconds = elapsed.total_seconds()
                 if processed > 0 and elapsed_seconds > 0:
                     speed = processed / elapsed_seconds
-                    if pending:
-                        eta = timedelta(seconds=int(pending / speed))
+                    remaining = pending + queued
+                    if remaining:
+                        eta = timedelta(seconds=int(remaining / speed))
                         estimated_finish = timezone.now() + eta
 
             if notification_campaign.updated_at:
                 last_activity_ago = timezone.now() - notification_campaign.updated_at
             if processed:
-                failure_rate = notification_campaign.failed_count / processed * 100
+                failure_rate = failed_count / processed * 100
             else:
                 failure_rate = 0
 
             context.update({
+                'recipient_count': recipient_count,
+                'sent_count': sent_count,
+                'failed_count': failed_count,
                 'processed': processed,
                 'pending': pending,
+                'queued': queued,
+                'awaiting': awaiting,
+                'awaiting_delivery': awaiting_delivery,
                 'sent_percent': sent_percent,
                 'failed_percent': failed_percent,
+                'queued_percent': queued_percent,
+                'awaiting_percent': awaiting_percent,
                 'pending_percent': pending_percent,
                 'elapsed': elapsed,
                 'speed': speed,
@@ -585,6 +629,7 @@ def form_valid(self, form):
                 'time_window': form.cleaned_data['time_window'],
                 'max_queued_batches': form.cleaned_data['max_queued_batches'],
                 'dispatch_interval': form.cleaned_data['dispatch_interval'],
+                'log_email_recipients': form.cleaned_data['log_email_recipients'],
             },
             'sendgrid_bulk': form.cleaned_data.get('sendgrid_bulk', False),
             'recipients_creation_finished': False,

admin/templates/notifications/notification_campaigns_detail.html

@@ -62,16 +62,33 @@ <h2 id="campaign_name">{{ notification_campaign.name }}</h2>
             <strong>Last activity:</strong> {{ last_activity_ago }} ago
         {% endif %}
     </div>
+    {% if awaiting_delivery %}
+    <div class="alert alert-info">
+        <strong>Awaiting delivery:</strong>
+        All recipients have been submitted to SendGrid.
+        Waiting for delivery confirmation ({{ awaiting }} awaiting).
+    </div>
+    {% endif %}
     <div class="progress">
 
         <div class="progress-bar progress-bar-success"
             style="width: {{ sent_percent }}%;">
-            {{ notification_campaign.sent_count }}
+            {{ sent_count }}
+        </div>
+
+        <div class="progress-bar progress-bar-info"
+            style="width: {{ awaiting_percent }}%;">
+            {{ awaiting }}
+        </div>
+
+        <div class="progress-bar progress-bar-warning"
+            style="width: {{ queued_percent }}%;">
+            {{ queued }}
         </div>
 
         <div class="progress-bar progress-bar-danger"
             style="width: {{ failed_percent }}%;">
-            {{ notification_campaign.failed_count }}
+            {{ failed_count }}
         </div>
 
         <div class="progress-bar progress-bar-default"
@@ -85,7 +102,17 @@ <h4>Progress</h4>
 <table class="table table-bordered">
     <tr>
         <th style="width:250px;">Processed</th>
-        <td>{{ processed }} / {{ notification_campaign.recipient_count }}</td>
+        <td>{{ processed }} / {{ recipient_count }}</td>
+    </tr>
+
+    <tr>
+        <th>Queued</th>
+        <td>{{ queued }}</td>
+    </tr>
+
+    <tr>
+        <th>Awaiting Delivery</th>
+        <td>{{ awaiting }}</td>
     </tr>
 
     <tr>

admin/templates/notifications/notification_campaing_create.html

@@ -324,6 +324,20 @@ <h4>Execution</h4>
                             </p>
                         </td>
                     </tr>
+                    <tr>
+                        <th>Log Email Recipients</th>
+                        <td>
+                            <input
+                                class="form-control"
+                                type="text"
+                                name="log_email_recipients"
+                                value="{{ form.log_email_recipients.initial }}"
+                            >
+                            <p class="help-block">
+                                {{ form.log_email_recipients.help_text }}
+                            </p>
+                        </td>
+                    </tr>
                 </table>
             </div>
         </div>

admin_tests/notifications/test_campaigns.py

@@ -23,8 +23,11 @@
 from osf.models import NotificationType
 from osf.models.notification_campaign import (
     NotificationCampaign,
+    NotificationCampaignRecipient,
+    NotificationCampaignRecipientStatus,
+    NotificationCampaignStatus,
 )
-from osf_tests.factories import AuthUserFactory, ProjectFactory
+from osf_tests.factories import AuthUserFactory, ProjectFactory, UserFactory
 from tests.base import AdminTestCase
 from website import settings
 
@@ -64,6 +67,7 @@ def _valid_form_data(notification_type, **overrides):
         'time_window': 8 * 60 * 60,
         'max_queued_batches': settings.MAX_QUEUED_CAMPAIGN_BATCHES,
         'dispatch_interval': settings.CAMPAIGN_DISPATCH_INTERVAL,
+        'log_email_recipients': '',
     }
     data.update(overrides)
     return data
@@ -81,6 +85,7 @@ def test_valid_form_parses_context_and_filters(self, notification_type):
         assert form.cleaned_data['activity_threshold'] == settings.DEFAULT_CAMPAIGN_ACTIVITY_THRESHOLD
         assert form.cleaned_data['time_window'] == 8 * 60 * 60
         assert form.cleaned_data['sendgrid_bulk'] is False
+        assert form.cleaned_data['log_email_recipients'] == ''
 
     def test_defaults_come_from_settings(self):
         form = NotificationCampaignCreateForm()
@@ -288,6 +293,7 @@ def test_form_valid_persists_execution_metadata(self):
             'time_window': 28800,
             'max_queued_batches': 10,
             'dispatch_interval': 60,
+            'log_email_recipients': ''
         }
         assert campaign.metadata['sendgrid_bulk'] is True
         assert campaign.metadata['filters'] == {'predefined': 'active'}
@@ -511,3 +517,75 @@ def test_delete_requires_change_notificationcampaign_permission(self):
         response = DeleteNotificationCampaign.as_view()(request, pk=self.campaign.pk)
         assert response.status_code == 302
         assert not NotificationCampaign.objects.filter(pk=self.campaign.pk).exists()
+
+
+class TestNotificationCampaignDetailCounters(AdminTestCase):
+
+    def setUp(self):
+        super().setUp()
+        self.user = AuthUserFactory()
+        grant_permission(self.user, 'change_notificationcampaign')
+        notification_type, _ = NotificationType.objects.get_or_create(
+            name='blank',
+            defaults={'subject': 'Test', 'template': 'Hello'},
+        )
+        # Stored counters deliberately differ from recipient rows.
+        self.campaign = NotificationCampaign.objects.create(
+            name='Campaign',
+            notification_type=notification_type,
+            status=NotificationCampaignStatus.COMPLETED,
+            recipient_count=10,
+            sent_count=7,
+            failed_count=3,
+            metadata={'filters': {}, 'context': {}, 'execution': {}},
+        )
+
+    def _context(self):
+        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
+        return response.context_data
+
+    def _add_recipient(self, status):
+        NotificationCampaignRecipient.objects.create(
+            campaign=self.campaign,
+            user=UserFactory(),
+            status=status,
+        )
+
+    def test_uses_live_recipient_stats_when_recipients_exist(self):
+        self._add_recipient(NotificationCampaignRecipientStatus.SENT)
+        self._add_recipient(NotificationCampaignRecipientStatus.SKIPPED)
+        self._add_recipient(NotificationCampaignRecipientStatus.AWAITING_DELIVERY)
+
+        context = self._context()
+
+        assert (context['recipient_count'], context['sent_count'], context['failed_count']) == (3, 1, 1)
+        assert context['awaiting'] == 1
+        assert context['processed'] == 2
+        assert context['pending'] == 0
+
+    def test_falls_back_to_stored_counters_when_recipients_purged(self):
+        context = self._context()
+
+        assert (context['recipient_count'], context['sent_count'], context['failed_count']) == (10, 7, 3)
+        assert context['processed'] == 10
+        assert context['pending'] == 0
+
+    @mock.patch('osf.email.notification_campaign.start_notification_campaign.delay')
+    def test_restart_does_not_reset_displayed_counters(self, mock_delay):
+        self._add_recipient(NotificationCampaignRecipientStatus.SENT)
+        self._add_recipient(NotificationCampaignRecipientStatus.FAILED)
+        self._add_recipient(NotificationCampaignRecipientStatus.SKIPPED)
+
+        for flags in ({'restart_failed': True}, {'restart_stuck': True}):
+            self.campaign.start(**flags)  # resets stored failed_count to 0
+            self.campaign.refresh_from_db()
+            assert self.campaign.failed_count == 0
+
+            context = self._context()
+
+            assert (context['recipient_count'], context['sent_count'], context['failed_count']) == (3, 1, 2)

api/sendgrid/views.py

@@ -44,8 +44,10 @@ class SendGridEventWebhook(APIView):
        - Enable Signed Event Webhook; copy the verification key into
          ``SENDGRID_EVENT_WEBHOOK_PUBLIC_KEY`` (``website/settings/local.py``).
        - Subscribe at least to ``delivered``, ``bounce``, and ``dropped``.
+         (``deferred`` is safe to enable but ignored for campaigns - SendGrid
+         retries those itself.)
     4. Send a notification campaign email (personalization ``custom_args`` must
-       include ``campaign_id``, ``campaign_recipient_id``, and ``run_id``).
+       include ``campaign_id``, ``campaign_recipient_id``, and ``sent_at``).
        SendGrid's "Test Your Integration" payload lacks those keys, so this view
        accepts it with 200 but does not update campaign recipients.
     """

notifications.yaml

@@ -828,3 +828,10 @@ notification_types:
     object_content_type_model_name: osfuser
     template: 'website/templates/blank.html.mako'
     tests: []
+
+  - name: notification_campaign_log
+    subject: 'Notification campaign "{campaign_name}" — {status}'
+    __docs__: ...
+    object_content_type_model_name: osfuser
+    template: 'website/templates/notification_campaign_log.html.mako'
+    tests: []