From a602ffceb94b47f2370948b8777cf6d969326514 Mon Sep 17 00:00:00 2001
From: Leo Hemsted <leohemsted@gmail.com>
Date: Fri, 14 Jan 2022 12:41:01 +0000
Subject: [PATCH] fix bug where job reports showed before jobs finished

for a job to be finished there are two requirements:

* the status to be "finished"
* the percentage complete to be 100%

The job status is set to finished when the process_job task finishes
(even though not all process_row may have finished). The
percentage_complete is calculated by comparing the number of
notifications in the database with the number of rows in the
spreadsheet.

This was inadvertently changed from an "and" to an "or" clause two years
ago. This meant that people could download a report when the status was
finished but not all notifications were present in the database. Lets
change it back.

https://github.com/alphagov/notifications-admin/commit/7d52ac97f111f6538b5cbe5cae1d369aac30cd04#diff-44b012cad205379c481bed244ddb2294bae5ee85dcd01f4aee932a2bd85b67b2L86-R100
---
 app/models/job.py            |  2 +-
 tests/app/models/test_job.py | 31 +++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 1 deletion(-)
 create mode 100644 tests/app/models/test_job.py

diff --git a/app/models/job.py b/app/models/job.py
index 7561ef777..3dfacbb16 100644
--- a/app/models/job.py
+++ b/app/models/job.py
@@ -107,7 +107,7 @@ class Job(JSONModel):
     @property
     def still_processing(self):
         return (
-            self.percentage_complete < 100 and self.status != 'finished'
+            self.status != 'finished' or self.percentage_complete < 100
         )
 
     @cached_property
diff --git a/tests/app/models/test_job.py b/tests/app/models/test_job.py
new file mode 100644
index 000000000..e43d8c08f
--- /dev/null
+++ b/tests/app/models/test_job.py
@@ -0,0 +1,31 @@
+import pytest
+
+from app.models.job import Job
+from tests import job_json, user_json
+from tests.conftest import SERVICE_ONE_ID
+
+
+@pytest.mark.parametrize(
+    'job_status, num_notifications_created, expected_still_processing',
+    [
+        ('scheduled', 0, True),
+        ('cancelled', 10, True),
+        ('finished', 5, True),
+        ('finished', 10, False),
+    ]
+)
+def test_still_processing(
+    notify_admin,
+    job_status,
+    num_notifications_created,
+    expected_still_processing
+):
+    json = job_json(
+        service_id=SERVICE_ONE_ID,
+        created_by=user_json(),
+        notification_count=10,
+        notifications_requested=num_notifications_created,
+        job_status=job_status
+    )
+    job = Job(json)
+    assert job.still_processing == expected_still_processing