From 6cd18f87de827c24c7e0feeab1672b5728ff4139 Mon Sep 17 00:00:00 2001
From: Alexey Bezhan <al@fueledbylemons.com>
Date: Tue, 8 Jan 2019 15:35:44 +0000
Subject: [PATCH] Don't request pagination links for API Message log page

Counting pages for API notifications takes a long time for services
with a lot of sent messages (since it issues a `count(*)` query for
the given filter). Since API message log doesn't have a "Next page"
link we can skip the count by setting a flag on the API request.
---
 app/notify_client/notification_api_client.py | 39 ++++++++++----------
 app/templates/views/api/index.html           |  2 +-
 tests/conftest.py                            |  6 ++-
 3 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/app/notify_client/notification_api_client.py b/app/notify_client/notification_api_client.py
index bc7143a53..ecd5effcd 100644
--- a/app/notify_client/notification_api_client.py
+++ b/app/notify_client/notification_api_client.py
@@ -13,6 +13,7 @@ class NotificationApiClient(NotifyAdminAPIClient):
         status=None,
         page=None,
         page_size=None,
+        count_pages=None,
         limit_days=None,
         include_jobs=None,
         include_from_test_key=None,
@@ -20,25 +21,22 @@ class NotificationApiClient(NotifyAdminAPIClient):
         to=None,
         include_one_off=None,
     ):
-        params = {}
-        if page is not None:
-            params['page'] = page
-        if page_size is not None:
-            params['page_size'] = page_size
-        if template_type is not None:
-            params['template_type'] = template_type
-        if status is not None:
-            params['status'] = status
-        if include_jobs is not None:
-            params['include_jobs'] = include_jobs
-        if include_from_test_key is not None:
-            params['include_from_test_key'] = include_from_test_key
-        if format_for_csv is not None:
-            params['format_for_csv'] = format_for_csv
-        if to is not None:
-            params['to'] = to
-        if include_one_off is not None:
-            params['include_one_off'] = include_one_off
+
+        params = {
+            'page': page,
+            'page_size': page_size,
+            'template_type': template_type,
+            'status': status,
+            'include_jobs': include_jobs,
+            'include_from_test_key': include_from_test_key,
+            'format_for_csv': format_for_csv,
+            'to': to,
+            'include_one_off': include_one_off,
+            'count_pages': count_pages,
+        }
+
+        params = {k: v for k, v in params.items() if v is not None}
+
         if job_id:
             return self.get(
                 url='/service/{}/job/{}/notifications'.format(service_id, job_id),
@@ -71,7 +69,8 @@ class NotificationApiClient(NotifyAdminAPIClient):
             service_id,
             include_jobs=False,
             include_from_test_key=True,
-            include_one_off=False
+            include_one_off=False,
+            count_pages=False
         )
         return self.map_letters_to_accepted(ret)
 
diff --git a/app/templates/views/api/index.html b/app/templates/views/api/index.html
index 274e24bcf..0376da3f3 100644
--- a/app/templates/views/api/index.html
+++ b/app/templates/views/api/index.html
@@ -83,7 +83,7 @@
     {% endfor %}
     {% if api_notifications.notifications %}
       <div class="api-notifications-item">
-        {% if api_notifications.links %}
+        {% if api_notifications.notifications|length == 50 %}
           <p class="api-notifications-item-meta">
             Only showing the first 50 messages.
           </p>
diff --git a/tests/conftest.py b/tests/conftest.py
index db5389bb0..a9c7ad818 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -1863,6 +1863,7 @@ def mock_get_notifications(
         job_id=None,
         page=1,
         page_size=50,
+        count_pages=None,
         template_type=None,
         status=None,
         limit_days=None,
@@ -1896,6 +1897,7 @@ def mock_get_notifications(
             template=template,
             rows=rows,
             job=job,
+            with_links=True if count_pages is None else count_pages,
             personalisation=personalisation,
             template_type=diff_template_type,
             client_reference=client_reference,
@@ -1915,6 +1917,7 @@ def mock_get_notifications_with_previous_next(mocker):
     def _get_notifications(service_id,
                            job_id=None,
                            page=1,
+                           count_pages=None,
                            template_type=None,
                            status=None,
                            limit_days=None,
@@ -1923,7 +1926,7 @@ def mock_get_notifications_with_previous_next(mocker):
                            to=None,
                            include_one_off=None
                            ):
-        return notification_json(service_id, with_links=True)
+        return notification_json(service_id, rows=50, with_links=True if count_pages is None else count_pages)
 
     return mocker.patch(
         'app.notification_api_client.get_notifications_for_service',
@@ -1936,6 +1939,7 @@ def mock_get_notifications_with_no_notifications(mocker):
     def _get_notifications(service_id,
                            job_id=None,
                            page=1,
+                           count_pages=None,
                            template_type=None,
                            status=None,
                            limit_days=None,