Merge pull request #1877 from alphagov/job-csv-upgrade

Add original file data to job downloads

app/main/views/jobs.py

@@ -126,7 +126,8 @@ def view_job_csv(service_id, job_id):
                 status=filter_args.get('status'),
                 page=request.args.get('page', 1),
                 page_size=5000,
-                format_for_csv=True
+                format_for_csv=True,
+                template_type=template['template_type'],
             )
         ),
         mimetype='text/csv',

app/utils.py

@@ -17,6 +17,7 @@ import pytz
 import yaml
 from flask import abort, current_app, redirect, request, session, url_for
 from flask_login import current_user
+from notifications_utils.recipients import RecipientCSV
 from notifications_utils.template import (
     EmailPreviewTemplate,
     LetterImageTemplate,
@@ -121,36 +122,40 @@ def get_errors_for_csv(recipients, template_type):
 
 def generate_notifications_csv(**kwargs):
     from app import notification_api_client
+    from app.main.s3_client import s3download
     if 'page' not in kwargs:
         kwargs['page'] = 1
 
-    if kwargs['job_id']:
+    if kwargs.get('job_id'):
-        fieldnames = ['Row number', 'Recipient', 'Template', 'Type', 'Job', 'Status', 'Time']
+        original_file_contents = s3download(kwargs['service_id'], kwargs['job_id'])
+        original_upload = RecipientCSV(
+            original_file_contents,
+            template_type=kwargs['template_type'],
+        )
+        original_column_headers = original_upload.column_headers
+        fieldnames = ['Row number'] + original_column_headers + ['Template', 'Type', 'Job', 'Status', 'Time']
     else:
         fieldnames = ['Recipient', 'Template', 'Type', 'Job', 'Status', 'Time']
 
     yield ','.join(fieldnames) + '\n'
 
     while kwargs['page']:
-        # if job_id then response looks different
         notifications_resp = notification_api_client.get_notifications_for_service(**kwargs)
-        notifications = notifications_resp['notifications']
+        for notification in notifications_resp['notifications']:
-        if kwargs['job_id']:
+            if kwargs.get('job_id'):
-            for notification in notifications:
+                values = [
-                    values = [
+                    notification['row_number'],
-                        notification['row_number'],
+                ] + [
-                        notification['recipient'],
+                    original_upload[notification['row_number'] - 1].get(header)
-                        notification['template_name'],
+                    for header in original_column_headers
-                        notification['template_type'],
+                ] + [
-                        notification['job_name'],
+                    notification['template_name'],
-                        notification['status'],
+                    notification['template_type'],
-                        notification['created_at']
+                    notification['job_name'],
-                    ]
+                    notification['status'],
-                    line = ','.join(str(i) for i in values) + '\n'
+                    notification['created_at'],
-                    yield line
+                ]
-        else:
+            else:
-            # Change here
-            for notification in notifications:
                 values = [
                     notification['to'],
                     notification['template']['name'],
@@ -160,8 +165,8 @@ def generate_notifications_csv(**kwargs):
                     notification['created_at'],
                     notification['updated_at']
                 ]
-                line = ','.join(str(i) for i in values) + '\n'
+            yield ','.join(map(str, values)) + '\n'
-                yield line
+
         if notifications_resp['links'].get('next'):
             kwargs['page'] += 1
         else:

requirements.txt

@@ -18,4 +18,4 @@ notifications-python-client==4.7.2
 # PaaS
 awscli-cwlogs>=1.4,<1.5
 
-git+https://github.com/alphagov/notifications-utils.git@23.6.2#egg=notifications-utils==23.6.2
+git+https://github.com/alphagov/notifications-utils.git@23.8.0#egg=notifications-utils==23.8.0

tests/app/test_utils.py

@@ -43,7 +43,7 @@ def _get_notifications_csv(
 
     data = {
         'notifications': [{
-            "row_number": row_number,
+            "row_number": row_number + i,
             "to": recipient,
             "recipient": recipient,
             "template_name": template_name,
@@ -128,23 +128,78 @@ def test_can_create_spreadsheet_from_dict_with_filename():
     assert Spreadsheet.from_dict({}, filename='empty.csv').as_dict['file_name'] == "empty.csv"
 
 
-def test_generate_notifications_csv_returns_correct_csv_file(_get_notifications_csv_mock):
+@pytest.mark.parametrize('original_file_contents, expected_column_headers, expected_1st_row', [
-    csv_content = generate_notifications_csv(service_id='1234', job_id=fake_uuid)
+    (
+        """
+            phone_number
+            07700900123
+        """,
+        ['Row number', 'phone_number', 'Template', 'Type', 'Job', 'Status', 'Time'],
+        ['1', '07700900123', 'foo', 'sms', 'bar.csv', 'Delivered', 'Thursday 19 April at 12:00'],
+    ),
+    (
+        """
+            phone_number, a, b, c
+            07700900123,  🐜,🐝,🦀
+        """,
+        ['Row number', 'phone_number', 'a', 'b', 'c', 'Template', 'Type', 'Job', 'Status', 'Time'],
+        ['1', '07700900123', '🐜', '🐝', '🦀', 'foo', 'sms', 'bar.csv', 'Delivered', 'Thursday 19 April at 12:00'],
+    ),
+])
+def test_generate_notifications_csv_returns_correct_csv_file(
+    app_,
+    mocker,
+    _get_notifications_csv_mock,
+    original_file_contents,
+    expected_column_headers,
+    expected_1st_row,
+):
+    mocker.patch(
+        'app.main.s3_client.s3download',
+        return_value=original_file_contents,
+    )
+    csv_content = generate_notifications_csv(service_id='1234', job_id=fake_uuid, template_type='sms')
     csv_file = DictReader(StringIO('\n'.join(csv_content)))
-    assert csv_file.fieldnames == ['Row number', 'Recipient', 'Template', 'Type', 'Job', 'Status', 'Time']
+    assert csv_file.fieldnames == expected_column_headers
+    assert next(csv_file) == dict(zip(expected_column_headers, expected_1st_row))
 
 
-def test_generate_notifications_csv_only_calls_once_if_no_next_link(_get_notifications_csv_mock):
+def test_generate_notifications_csv_only_calls_once_if_no_next_link(
-    list(generate_notifications_csv(service_id='1234', job_id=fake_uuid))
+    app_,
+    _get_notifications_csv_mock,
+):
+    list(generate_notifications_csv(service_id='1234'))
 
     assert _get_notifications_csv_mock.call_count == 1
 
 
 @pytest.mark.parametrize("job_id", ["some", None])
-def test_generate_notifications_csv_calls_twice_if_next_link(mocker, job_id):
+def test_generate_notifications_csv_calls_twice_if_next_link(
+    app_,
+    mocker,
+    job_id,
+):
+
+    mocker.patch(
+        'app.main.s3_client.s3download',
+        return_value="""
+            phone_number
+            07700900000
+            07700900001
+            07700900002
+            07700900003
+            07700900004
+            07700900005
+            07700900006
+            07700900007
+            07700900008
+            07700900009
+        """
+    )
+
     service_id = '1234'
     response_with_links = _get_notifications_csv(service_id, rows=7, with_links=True)
-    response_with_no_links = _get_notifications_csv(service_id, rows=3, with_links=False)
+    response_with_no_links = _get_notifications_csv(service_id, rows=3, row_number=8, with_links=False)
 
     mock_get_notifications = mocker.patch(
         'app.notification_api_client.get_notifications_for_service',
@@ -154,10 +209,16 @@ def test_generate_notifications_csv_calls_twice_if_next_link(mocker, job_id):
         ]
     )
 
-    csv_content = generate_notifications_csv(service_id=service_id, job_id=fake_uuid if job_id else None)
+    csv_content = generate_notifications_csv(
-    csv = DictReader(StringIO('\n'.join(csv_content)))
+        service_id=service_id,
+        job_id=job_id or fake_uuid,
+        template_type='sms',
+    )
+    csv = list(DictReader(StringIO('\n'.join(csv_content))))
 
-    assert len(list(csv)) == 10
+    assert len(csv) == 10
+    assert csv[0]['phone_number'] == '07700900000'
+    assert csv[9]['phone_number'] == '07700900009'
     assert mock_get_notifications.call_count == 2
     # mock_calls[0][2] is the kwargs from first call
     assert mock_get_notifications.mock_calls[0][2]['page'] == 1