Use time to determine why notifications don’t exist

Notifications won’t exist for a job if:
- it’s just started
- it started a long time ago (older than the retention period)

We have a bug where:
1. Job starts processing, puts notifications on queue
2. Job finishes processing, sets status to `finished`
3. First notification gets picked up off the queue and put in the
   database

In between 2. and 3. it’s possible for a job to be finished, but also to
have no notifications. We’re saying this is because the notifications
have been deleted, whereas really it’s because they haven’t been created
yet.

This commit fixes that bug by introducing the concept of recency for
jobs.

‘Recent’ is defined as 1 day, which is:
- a lot longer than it takes to create any notifications
- a bit shorter than anyone’s retention time

N.B. `processing_started` is defined here:
879ba1d5f0/app/models.py (L1194)

It can be `None` for scheduled jobs that haven’t started yet.

app/models/job.py

@@ -48,6 +48,12 @@ class Job(JSONModel):
     def scheduled_for(self):
         return self._dict.get('scheduled_for')
 
+    @property
+    def processing_started(self):
+        if not self._dict.get('processing_started'):
+            return None
+        return datetime.strptime(self._dict['processing_started'][:-6], '%Y-%m-%dT%H:%M:%S')
+
     def _aggregate_statistics(self, *statuses):
         return sum(
             outcome['count'] for outcome in self._dict['statistics']
@@ -95,6 +101,17 @@ class Job(JSONModel):
     def finished_processing(self):
         return self.notification_count == self.notifications_sent
 
+    @property
+    def recently_created(self):
+        if not self.processing_started:
+            # Assume that if processing hasn’t started yet then the job
+            # must have been created recently enough to not have any
+            # notifications yet
+            return True
+        return (
+            datetime.utcnow() - self.processing_started
+        ).days < 1
+
     @property
     def template_id(self):
         return self._dict['template']

app/templates/partials/jobs/notifications.html

@@ -47,7 +47,7 @@
         notifications,
         caption=uploaded_file_name,
         caption_visible=False,
-        empty_message='These messages have been deleted because they were sent more than {} days ago'.format(service_data_retention_days) if job.job_status == 'finished' else 'No messages to show yet…',
+        empty_message='No messages to show yet…' if job.recently_created else 'These messages have been deleted because they were sent more than {} days ago'.format(service_data_retention_days),
         field_headings=[
           'Recipient',
           'Status'

tests/__init__.py

@@ -361,7 +361,8 @@ def job_json(
     notifications_sent=1,
     notifications_requested=1,
     job_status='finished',
-    scheduled_for=''
+    scheduled_for='',
+    processing_started=None,
 ):
     if job_id is None:
         job_id = str(generate_uuid())
@@ -391,8 +392,11 @@ def job_json(
             created_by['name'],
             created_by['email_address'],
         ),
-        'scheduled_for': scheduled_for
     }
+    if scheduled_for:
+        data.update(scheduled_for=scheduled_for)
+    if processing_started:
+        data.update(processing_started=processing_started)
     return data
 
 

tests/app/main/views/test_jobs.py

@@ -1,5 +1,6 @@
 import json
 import uuid
+from datetime import datetime, timezone
 
 import pytest
 from flask import url_for
@@ -329,28 +330,58 @@ def test_should_show_job_without_notifications(
     assert page.select_one('tbody').text.strip() == 'No messages to show yet…'
 
 
+@freeze_time("2020-01-10 0:0:0")
+@pytest.mark.parametrize('created_at, processing_started, expected_message', (
+    # Recently created, not yet started
+    (datetime(2020, 1, 10, 0, 0, 0), None, (
+        'No messages to show yet…'
+    )),
+    # Just started
+    (datetime(2020, 1, 10, 0, 0, 0), datetime(2020, 1, 10, 0, 0, 1), (
+        'No messages to show yet…'
+    )),
+    # Created a while ago, just started
+    (datetime(2020, 1, 1, 0, 0, 0), datetime(2020, 1, 10, 0, 0, 1), (
+        'No messages to show yet…'
+    )),
+    # Created a while ago, started a couple of days ago
+    (datetime(2020, 1, 1, 0, 0, 0), datetime(2020, 1, 8, 0, 0, 1), (
+        'These messages have been deleted because they were sent more than 7 days ago'
+    )),
+))
 def test_should_show_old_job(
     client_request,
     service_one,
     active_user_with_permissions,
     mock_get_service_template,
-    mock_get_job,
     mocker,
     mock_get_notifications_with_no_notifications,
     mock_get_service_data_retention,
     fake_uuid,
+    created_at,
+    processing_started,
+    expected_message,
 ):
+    mocker.patch('app.job_api_client.get_job', return_value={
+        "data": job_json(
+            SERVICE_ONE_ID,
+            active_user_with_permissions,
+            created_at=created_at.replace(tzinfo=timezone.utc).isoformat(),
+            processing_started=(
+                processing_started.replace(tzinfo=timezone.utc).isoformat()
+                if processing_started else None
+            ),
+        ),
+    })
     page = client_request.get(
         'main.view_job',
-        service_id=service_one['id'],
+        service_id=SERVICE_ONE_ID,
         job_id=fake_uuid,
     )
     assert not page.select('.pill a')
     assert not page.select('p.hint')
     assert not page.select('a[download]')
-    assert page.select_one('tbody').text.strip() == (
-        'These messages have been deleted because they were sent more than 7 days ago'
-    )
+    assert page.select_one('tbody').text.strip() == expected_message
 
 
 @freeze_time("2016-01-01 11:09:00.061258")