Add original file data to job downloads

When downloading a report of a which messages from a job have been
delivered and which have failed we currently only include the Notify
data. This makes it hard to reconcile or do analysis on these reports,
because often the thing that people want to reconcile on is in the data
they’ve uploaded (eg a reference number).

Here’s an example of a user talking about this problem:

> It would also be helpful if the format of the delivery and failure
> reports could include the fields from the recipient's file. While I
> can, of course,  cross-reference one report with the other it would be
> easier if I did not have to. We send emails to individuals within
> organisations and it is not always easy to establish the organisation
> from a recipient's email address. This is particularly important when
> emails fail to be delivered as we need to contact the organisation to
> establish a new contact.

– ticket 677

We’ve also seen it when doing research with a local council.

This commit takes the original file, the data from the API, and munges
them together.
This commit is contained in:
Chris Hill-Scott
2018-02-16 11:35:36 +00:00
parent aae94c8c80
commit 0934843815
4 changed files with 101 additions and 28 deletions

View File

@@ -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',

View File

@@ -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,33 +122,43 @@ 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']:
fieldnames = ['Row number', 'Recipient', 'Template', 'Type', 'Job', 'Status', 'Time']
if kwargs.get('job_id'):
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']
if kwargs['job_id']:
if kwargs.get('job_id'):
for notification in notifications:
values = [
notification['row_number'],
notification['recipient'],
notification['template_name'],
notification['template_type'],
notification['job_name'],
notification['status'],
notification['created_at']
]
line = ','.join(str(i) for i in values) + '\n'
yield line
original_row = original_upload[notification['row_number'] - 1]
values = [
notification['row_number'],
] + [
original_row.get(header)
for header in original_column_headers
] + [
notification['template_name'],
notification['template_type'],
notification['job_name'],
notification['status'],
notification['created_at']
]
yield ','.join(map(str, values)) + '\n'
else:
# Change here
for notification in notifications:

View File

@@ -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

View File

@@ -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):
csv_content = generate_notifications_csv(service_id='1234', job_id=fake_uuid)
@pytest.mark.parametrize('original_file_contents, expected_column_headers, expected_1st_row', [
(
"""
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):
list(generate_notifications_csv(service_id='1234', job_id=fake_uuid))
def test_generate_notifications_csv_only_calls_once_if_no_next_link(
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 = DictReader(StringIO('\n'.join(csv_content)))
csv_content = generate_notifications_csv(
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