From 9058b13ff21528603d6ae9656514f4abf96f851a Mon Sep 17 00:00:00 2001
From: Pea Tyczynska <pea.tyczynska@digital.cabinet-office.gov.uk>
Date: Tue, 24 Sep 2019 17:27:31 +0100
Subject: [PATCH] Get notification count from api instead of list of
 notifications

when checking if job can be cancelled. This makes query lighter,
less likely to fall over if job is large and removes
pagination bug.
---
 app/main/views/jobs.py                              |  8 ++++----
 app/notify_client/notification_api_client.py        |  3 +++
 tests/app/main/views/test_jobs.py                   | 10 ++++------
 tests/app/notify_client/test_notification_client.py |  8 ++++++++
 4 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py
index 8d1bc5b48..fb5ffe367 100644
--- a/app/main/views/jobs.py
+++ b/app/main/views/jobs.py
@@ -176,10 +176,10 @@ def cancel_job(service_id, job_id):
 def cancel_letter_job(service_id, job_id):
     if request.method == 'POST':
         job = job_api_client.get_job(service_id, job_id)['data']
-        notifications = notification_api_client.get_notifications_for_service(
-            job['service'], job['id']
-        )['notifications']
-        if job['job_status'] != 'finished' or len(notifications) < job['notification_count']:
+        notification_count = notification_api_client.get_notification_count_for_job_id(
+            service_id=service_id, job_id=job_id
+        )
+        if job['job_status'] != 'finished' or notification_count < job['notification_count']:
             flash("We are still processing these letters, please try again in a minute.", 'try again')
             return view_job(service_id, job_id)
         try:
diff --git a/app/notify_client/notification_api_client.py b/app/notify_client/notification_api_client.py
index d474214bc..341d10234 100644
--- a/app/notify_client/notification_api_client.py
+++ b/app/notify_client/notification_api_client.py
@@ -127,5 +127,8 @@ class NotificationApiClient(NotifyAdminAPIClient):
             }
         )
 
+    def get_notification_count_for_job_id(self, *, service_id, job_id):
+        return self.get(url='/service/{}/job/{}/notification_count'.format(service_id, job_id))["count"]
+
 
 notification_api_client = NotificationApiClient()
diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py
index f9083c0c1..66852eb06 100644
--- a/tests/app/main/views/test_jobs.py
+++ b/tests/app/main/views/test_jobs.py
@@ -504,10 +504,8 @@ def test_should_cancel_letter_job(
     mocker.patch('app.job_api_client.get_job', side_effect=[{"data": job}])
     notifications_json = notification_json(SERVICE_ONE_ID, job=job, status="created", template_type="letter")
     mocker.patch('app.job_api_client.get_job', side_effect=[{"data": job}])
-    mocker.patch(
-        'app.notification_api_client.get_notifications_for_service',
-        side_effect=[notifications_json]
-    )
+    mocker.patch('app.notification_api_client.get_notifications_for_service', return_value=notifications_json)
+    mocker.patch('app.notification_api_client.get_notification_count_for_job_id', return_value=5)
     mock_cancel = mocker.patch('app.main.jobs.job_api_client.cancel_letter_job', return_value=5)
     client_request.post(
         'main.cancel_letter_job',
@@ -623,9 +621,9 @@ def test_dont_cancel_letter_job_when_to_early_to_cancel(
     notifications_json = notification_json(
         SERVICE_ONE_ID, job=job, status="created", template_type="letter", rows=number_of_processed_notifications
     )
+    mocker.patch('app.notification_api_client.get_notifications_for_service', return_value=notifications_json)
     mocker.patch(
-        'app.notification_api_client.get_notifications_for_service',
-        side_effect=[notifications_json, notifications_json]
+        'app.notification_api_client.get_notification_count_for_job_id', return_value=number_of_processed_notifications
     )
 
     mock_cancel = mocker.patch('app.main.jobs.job_api_client.cancel_letter_job')
diff --git a/tests/app/notify_client/test_notification_client.py b/tests/app/notify_client/test_notification_client.py
index f9a90f702..1975a3412 100644
--- a/tests/app/notify_client/test_notification_client.py
+++ b/tests/app/notify_client/test_notification_client.py
@@ -118,3 +118,11 @@ def test_update_notification_to_cancelled(mocker):
         url='/service/foo/notifications/bar/cancel',
         data={},
     )
+
+
+def test_get_notification_count_for_job_id(mocker):
+    mock_get = mocker.patch('app.notify_client.notification_api_client.NotificationApiClient.get')
+    NotificationApiClient().get_notification_count_for_job_id(service_id='foo', job_id='bar')
+    mock_get.assert_called_once_with(
+        url='/service/foo/job/bar/notification_count',
+    )