Merge pull request #965 from alphagov/paginate-dashboard

Limit jobs on the dashboard

tests/__init__.py

@@ -159,7 +159,7 @@ def job_json(
     notification_count=1,
     notifications_sent=1,
     notifications_requested=1,
-    job_status='Delivered',
+    job_status='finished',
     scheduled_for=''
 ):
     if job_id is None:

tests/app/main/views/test_dashboard.py

@@ -182,7 +182,10 @@ def test_should_show_upcoming_jobs_on_dashboard(
         client.login(api_user_active)
         response = client.get(url_for('main.service_dashboard', service_id=SERVICE_ONE_ID))
 
-    mock_get_jobs.assert_called_once_with(SERVICE_ONE_ID, limit_days=7)
+    first_call = mock_get_jobs.call_args_list[0]
+    assert first_call[0] == (SERVICE_ONE_ID,)
+    assert first_call[1]['statuses'] == ['scheduled']
+
     assert response.status_code == 200
 
     page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
@@ -218,7 +221,10 @@ def test_should_show_recent_jobs_on_dashboard(
         client.login(api_user_active)
         response = client.get(url_for('main.service_dashboard', service_id=SERVICE_ONE_ID))
 
-    mock_get_jobs.assert_called_once_with(SERVICE_ONE_ID, limit_days=7)
+    second_call = mock_get_jobs.call_args_list[1]
+    assert second_call[0] == (SERVICE_ONE_ID,)
+    assert 'scheduled' not in second_call[1]['statuses']
+
     assert response.status_code == 200
 
     page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')

tests/app/main/views/test_jobs.py

@@ -12,21 +12,38 @@ from tests import notification_json
 from freezegun import freeze_time
 
 
-def test_should_return_list_of_all_jobs(app_,
-                                        service_one,
-                                        active_user_with_permissions,
-                                        mock_get_jobs,
-                                        mocker):
-    with app_.test_request_context():
-        with app_.test_client() as client:
-            client.login(active_user_with_permissions, mocker, service_one)
-            response = client.get(url_for('main.view_jobs', service_id=service_one['id']))
+def test_get_jobs_should_return_list_of_all_real_jobs(
+    client,
+    service_one,
+    active_user_with_permissions,
+    mock_get_jobs,
+    mocker
+):
+    client.login(active_user_with_permissions, mocker, service_one)
+    response = client.get(url_for('main.view_jobs', service_id=service_one['id']))
 
-        assert response.status_code == 200
-        page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
-        assert page.h1.string == 'Uploaded files'
-        jobs = page.tbody.find_all('tr')
-        assert len(jobs) == 5
+    assert response.status_code == 200
+    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+    assert page.h1.string == 'Uploaded files'
+    jobs = [x.text for x in page.tbody.find_all('a', {'class': 'file-list-filename'})]
+    assert len(jobs) == 4
+    assert 'Test message' not in jobs
+
+
+def test_get_jobs_shows_page_links(
+    client,
+    service_one,
+    active_user_with_permissions,
+    mock_get_jobs,
+    mocker
+):
+    client.login(active_user_with_permissions, mocker, service_one)
+    response = client.get(url_for('main.view_jobs', service_id=service_one['id']))
+
+    assert response.status_code == 200
+    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+    assert 'Next page' in page.find('li', {'class': 'next-page'}).text
+    assert 'Previous page' in page.find('li', {'class': 'previous-page'}).text
 
 
 @pytest.mark.parametrize(

tests/app/notify_client/test_job_client.py

@@ -1,3 +1,6 @@
+import uuid
+from unittest.mock import ANY
+
 from app.notify_client.job_api_client import JobApiClient
 
 
@@ -51,8 +54,6 @@ def test_client_schedules_job(mocker, fake_uuid):
 
 
 def test_client_gets_job_by_service_and_job(mocker):
-    mocker.patch('app.notify_client.current_user', id='1')
-
     service_id = 'service_id'
     job_id = 'job_id'
 
@@ -66,40 +67,24 @@ def test_client_gets_job_by_service_and_job(mocker):
     mock_get.assert_called_once_with(url=expected_url, params={})
 
 
-def test_client_gets_job_by_service_and_job_filtered_by_status(mocker):
-    mocker.patch('app.notify_client.current_user', id='1')
+def test_client_gets_jobs_with_status_filter(mocker):
+    mock_get = mocker.patch('app.notify_client.job_api_client.JobApiClient.get')
 
-    service_id = 'service_id'
-    job_id = 'job_id'
+    JobApiClient().get_jobs(uuid.uuid4(), statuses=['foo', 'bar'])
 
-    expected_url = '/service/{}/job/{}'.format(service_id, job_id)
+    mock_get.assert_called_once_with(url=ANY, params={'page': 1, 'statuses': 'foo,bar'})
 
+
+def test_client_gets_jobs_with_page_parameter(mocker):
     client = JobApiClient()
     mock_get = mocker.patch('app.notify_client.job_api_client.JobApiClient.get')
 
-    client.get_job(service_id, job_id, limit_days=1, status='failed')
+    client.get_jobs('foo', page=2)
 
-    mock_get.assert_called_once_with(url=expected_url, params={'status': 'failed'})
-
-
-def test_client_gets_job_by_service_filtered_by_status(mocker):
-    mocker.patch('app.notify_client.current_user', id='1')
-
-    service_id = 'service_id'
-
-    expected_url = '/service/{}/job'.format(service_id)
-
-    client = JobApiClient()
-    mock_get = mocker.patch('app.notify_client.job_api_client.JobApiClient.get')
-
-    client.get_job(service_id, limit_days=1, status='failed')
-
-    mock_get.assert_called_once_with(url=expected_url,  params={'limit_days': 1})
+    mock_get.assert_called_once_with(url=ANY, params={'page': 2})
 
 
 def test_client_parses_job_stats(mocker):
-    mocker.patch('app.notify_client.current_user', id='1')
-
     service_id = 'service_id'
     job_id = 'job_id'
     expected_data = {'data': {
@@ -145,8 +130,6 @@ def test_client_parses_job_stats(mocker):
 
 
 def test_client_parses_empty_job_stats(mocker):
-    mocker.patch('app.notify_client.current_user', id='1')
-
     service_id = 'service_id'
     job_id = 'job_id'
     expected_data = {'data': {
@@ -183,8 +166,6 @@ def test_client_parses_empty_job_stats(mocker):
 
 
 def test_client_parses_job_stats_for_service(mocker):
-    mocker.patch('app.notify_client.current_user', id='1')
-
     service_id = 'service_id'
     job_1_id = 'job_id_1'
     job_2_id = 'job_id_2'
@@ -247,9 +228,9 @@ def test_client_parses_job_stats_for_service(mocker):
     client = JobApiClient()
     mock_get = mocker.patch('app.notify_client.job_api_client.JobApiClient.get', return_value=expected_data)
 
-    result = client.get_job(service_id)
+    result = client.get_jobs(service_id)
 
-    mock_get.assert_called_once_with(url=expected_url, params={})
+    mock_get.assert_called_once_with(url=expected_url, params={'page': 1})
     assert result['data'][0]['id'] == job_1_id
     assert result['data'][0]['notifications_requested'] == 80
     assert result['data'][0]['notifications_sent'] == 50
@@ -263,8 +244,6 @@ def test_client_parses_job_stats_for_service(mocker):
 
 
 def test_client_parses_empty_job_stats_for_service(mocker):
-    mocker.patch('app.notify_client.current_user', id='1')
-
     service_id = 'service_id'
     job_1_id = 'job_id_1'
     job_2_id = 'job_id_2'
@@ -309,9 +288,9 @@ def test_client_parses_empty_job_stats_for_service(mocker):
     client = JobApiClient()
     mock_get = mocker.patch('app.notify_client.job_api_client.JobApiClient.get', return_value=expected_data)
 
-    result = client.get_job(service_id)
+    result = client.get_jobs(service_id)
 
-    mock_get.assert_called_once_with(url=expected_url, params={})
+    mock_get.assert_called_once_with(url=expected_url, params={'page': 1})
     assert result['data'][0]['id'] == job_1_id
     assert result['data'][0]['notifications_requested'] == 0
     assert result['data'][0]['notifications_sent'] == 0
@@ -325,7 +304,6 @@ def test_client_parses_empty_job_stats_for_service(mocker):
 
 
 def test_cancel_job(mocker):
-
     mock_post = mocker.patch('app.notify_client.job_api_client.JobApiClient.post')
 
     JobApiClient().cancel_job('service_id', 'job_id')

tests/app/test_statistics_utils.py

@@ -1,6 +1,6 @@
 import pytest
 
-from app.statistics_utils import sum_of_statistics, add_rates_to, add_rate_to_jobs, statistics_by_state
+from app.statistics_utils import sum_of_statistics, add_rates_to, add_rate_to_job, statistics_by_state
 
 
 @pytest.mark.parametrize('delivery_statistics', [
@@ -118,20 +118,29 @@ def test_service_statistics_by_state():
 @pytest.mark.parametrize('failed, delivered, expected_failure_rate', [
     (0, 0, 0),
     (0, 1, 0),
+    (1, 1, 50),
     (1, 0, 100),
     (1, 4, 20)
 ])
-def test_add_rate_to_jobs(failed, delivered, expected_failure_rate):
-    resp = add_rate_to_jobs([
+def test_add_rate_to_job_calculates_rate(failed, delivered, expected_failure_rate):
+    resp = add_rate_to_job(
         {
             'notifications_failed': failed,
-            'notifications_delivered': delivered
-        },
-        {
-            'notifications_failed': 1,
-            'notifications_delivered': 1
+            'notifications_delivered': delivered,
+            'id': 'foo'
         }
-    ])
+    )
 
-    assert resp[0]['failure_rate'] == expected_failure_rate
-    assert resp[1]['failure_rate'] == 50
+    assert resp['failure_rate'] == expected_failure_rate
+
+
+def test_add_rate_to_job_preserves_initial_fields():
+    resp = add_rate_to_job(
+        {
+            'notifications_failed': 0,
+            'notifications_delivered': 0,
+            'id': 'foo'
+        }
+    )
+
+    assert set(resp.keys()) == {'notifications_failed', 'notifications_delivered', 'id', 'failure_rate'}

tests/app/test_utils.py

@@ -1,6 +1,6 @@
 import pytest
 from io import StringIO
-from app.utils import email_safe, generate_notifications_csv
+from app.utils import email_safe, generate_notifications_csv, generate_previous_dict, generate_next_dict
 from csv import DictReader
 from freezegun import freeze_time
 
@@ -48,3 +48,22 @@ def test_generate_csv_from_notifications(
     for row in DictReader(StringIO(csv_content)):
         assert row['Time'] == 'Friday 01 January 2016 at 15:09'
         assert row['Status'] == expected_status
+
+
+def test_generate_previous_dict(client):
+    ret = generate_previous_dict('main.view_jobs', 'foo', 2, {})
+    assert 'page=1' in ret['url']
+    assert ret['title'] == 'Previous page'
+    assert ret['label'] == 'page 1'
+
+
+def test_generate_next_dict(client):
+    ret = generate_next_dict('main.view_jobs', 'foo', 2, {})
+    assert 'page=3' in ret['url']
+    assert ret['title'] == 'Next page'
+    assert ret['label'] == 'page 3'
+
+
+def test_generate_previous_next_dict_adds_other_url_args(client):
+    ret = generate_next_dict('main.view_notifications', 'foo', 2, {'message_type': 'blah'})
+    assert 'notifications/blah' in ret['url']

tests/conftest.py

@@ -901,8 +901,11 @@ def mock_get_job_in_progress(mocker, api_user_active):
 
 @pytest.fixture(scope='function')
 def mock_get_jobs(mocker, api_user_active):
-    def _get_jobs(service_id, limit_days=None):
-        return {"data": [
+    def _get_jobs(service_id, limit_days=None, statuses=None, page=1):
+        if statuses is None:
+            statuses = ['', 'scheduled', 'pending', 'cancelled']
+
+        jobs = [
             job_json(
                 service_id,
                 api_user_active,
@@ -911,19 +914,25 @@ def mock_get_jobs(mocker, api_user_active):
                 job_status=job_status
             )
             for filename, scheduled_for, job_status in (
-                ("Test message", '', ''),
-                ("Test message", '2016-01-01 11:09:00.061258', 'scheduled'),
-                ("export 1/1/2016.xls", '', ''),
-                ("all email addresses.xlsx", '', 'pending'),
-                ("applicants.ods", '', ''),
-                ("thisisatest.csv", '', ''),
-                ("send_me_later.csv", '2016-01-01 11:09:00.061258', 'scheduled'),
-                ("even_later.csv", '2016-01-01 23:09:00.061258', 'scheduled'),
-                ("full_of_regret.csv", '2016-01-01 23:09:00.061258', 'cancelled')
+                ('Test message', '', 'finished'),
+                ('export 1/1/2016.xls', '', 'finished'),
+                ('all email addresses.xlsx', '', 'pending'),
+                ('applicants.ods', '', 'finished'),
+                ('thisisatest.csv', '', 'finished'),
+                ('send_me_later.csv', '2016-01-01 11:09:00.061258', 'scheduled'),
+                ('even_later.csv', '2016-01-01 23:09:00.061258', 'scheduled'),
+                ('full_of_regret.csv', '2016-01-01 23:09:00.061258', 'cancelled')
             )
-        ]}
+        ]
+        return {
+            'data': [job for job in jobs if job['job_status'] in statuses],
+            'links': {
+                'prev': 'services/{}/jobs?page={}'.format(service_id, page - 1),
+                'next': 'services/{}/jobs?page={}'.format(service_id, page + 1)
+            }
+        }
 
-    return mocker.patch('app.job_api_client.get_job', side_effect=_get_jobs)
+    return mocker.patch('app.job_api_client.get_jobs', side_effect=_get_jobs)
 
 
 @pytest.fixture(scope='function')