2017-01-19 14:55:22 +00:00
|
|
|
|
import uuid
|
2016-01-11 15:00:51 +00:00
|
|
|
|
from io import BytesIO
|
Accept common spreadsheet formats, not just CSV
We require users to export their spreadsheets as CSV files before
uploading them. But this seems like the sort of thing a computer should
be able to do.
So this commit adds a wrapper class which:
- takes a the uploaded file
- returns it in a normalised format, or reads it using pyexcel[1]
- gives the data back in CSV format
This allows us to accept `.csv`, `.xlsx`, `.xls` (97 and 95), `.ods`,
`.xlsm` and `.tsv` files. We can upload the resultant CSV just like
normal, and process it for errors as before.
Testing
---
To test this I’ve added a selection of common spreadsheet files as test
data. They all contain the same data, so the tests look to see that the
resultant CSV output is the same for each.
UI changes
---
This commit doesn’t change the UI, apart from to give a different error
message if a user uploads a file type that we still don’t understand.
I intend to do this as a separate pull request, in order to fulfil
https://www.pivotaltracker.com/story/show/119371637
2016-05-05 15:41:11 +01:00
|
|
|
|
from os import path
|
|
|
|
|
|
from glob import glob
|
2016-11-02 16:53:40 +00:00
|
|
|
|
import re
|
|
|
|
|
|
from itertools import repeat
|
2016-06-24 10:15:20 +01:00
|
|
|
|
from functools import partial
|
2017-01-19 14:55:22 +00:00
|
|
|
|
from unittest.mock import patch
|
2016-11-02 16:53:40 +00:00
|
|
|
|
|
|
|
|
|
|
import pytest
|
|
|
|
|
|
from bs4 import BeautifulSoup
|
2016-01-14 16:00:13 +00:00
|
|
|
|
from flask import url_for
|
2016-11-02 16:53:40 +00:00
|
|
|
|
|
2017-01-19 18:15:33 +00:00
|
|
|
|
from app.main.views.send import get_check_messages_back_url
|
|
|
|
|
|
|
|
|
|
|
|
from tests import validate_route_permission, template_json
|
2016-01-13 17:32:40 +00:00
|
|
|
|
|
2016-02-22 17:17:18 +00:00
|
|
|
|
template_types = ['email', 'sms']
|
2016-01-13 17:32:40 +00:00
|
|
|
|
|
Accept common spreadsheet formats, not just CSV
We require users to export their spreadsheets as CSV files before
uploading them. But this seems like the sort of thing a computer should
be able to do.
So this commit adds a wrapper class which:
- takes a the uploaded file
- returns it in a normalised format, or reads it using pyexcel[1]
- gives the data back in CSV format
This allows us to accept `.csv`, `.xlsx`, `.xls` (97 and 95), `.ods`,
`.xlsm` and `.tsv` files. We can upload the resultant CSV just like
normal, and process it for errors as before.
Testing
---
To test this I’ve added a selection of common spreadsheet files as test
data. They all contain the same data, so the tests look to see that the
resultant CSV output is the same for each.
UI changes
---
This commit doesn’t change the UI, apart from to give a different error
message if a user uploads a file type that we still don’t understand.
I intend to do this as a separate pull request, in order to fulfil
https://www.pivotaltracker.com/story/show/119371637
2016-05-05 15:41:11 +01:00
|
|
|
|
# The * ignores hidden files, eg .DS_Store
|
|
|
|
|
|
test_spreadsheet_files = glob(path.join('tests', 'spreadsheet_files', '*'))
|
2016-05-13 21:25:59 +01:00
|
|
|
|
test_non_spreadsheet_files = glob(path.join('tests', 'non_spreadsheet_files', '*'))
|
Accept common spreadsheet formats, not just CSV
We require users to export their spreadsheets as CSV files before
uploading them. But this seems like the sort of thing a computer should
be able to do.
So this commit adds a wrapper class which:
- takes a the uploaded file
- returns it in a normalised format, or reads it using pyexcel[1]
- gives the data back in CSV format
This allows us to accept `.csv`, `.xlsx`, `.xls` (97 and 95), `.ods`,
`.xlsm` and `.tsv` files. We can upload the resultant CSV just like
normal, and process it for errors as before.
Testing
---
To test this I’ve added a selection of common spreadsheet files as test
data. They all contain the same data, so the tests look to see that the
resultant CSV output is the same for each.
UI changes
---
This commit doesn’t change the UI, apart from to give a different error
message if a user uploads a file type that we still don’t understand.
I intend to do this as a separate pull request, in order to fulfil
https://www.pivotaltracker.com/story/show/119371637
2016-05-05 15:41:11 +01:00
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_that_test_files_exist():
|
|
|
|
|
|
assert len(test_spreadsheet_files) == 8
|
2016-05-13 21:25:59 +01:00
|
|
|
|
assert len(test_non_spreadsheet_files) == 6
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
|
|
|
|
"filename, acceptable_file",
|
|
|
|
|
|
list(zip(
|
|
|
|
|
|
test_spreadsheet_files, repeat(True)
|
|
|
|
|
|
)) +
|
|
|
|
|
|
list(zip(
|
|
|
|
|
|
test_non_spreadsheet_files, repeat(False)
|
|
|
|
|
|
))
|
|
|
|
|
|
)
|
Accept common spreadsheet formats, not just CSV
We require users to export their spreadsheets as CSV files before
uploading them. But this seems like the sort of thing a computer should
be able to do.
So this commit adds a wrapper class which:
- takes a the uploaded file
- returns it in a normalised format, or reads it using pyexcel[1]
- gives the data back in CSV format
This allows us to accept `.csv`, `.xlsx`, `.xls` (97 and 95), `.ods`,
`.xlsm` and `.tsv` files. We can upload the resultant CSV just like
normal, and process it for errors as before.
Testing
---
To test this I’ve added a selection of common spreadsheet files as test
data. They all contain the same data, so the tests look to see that the
resultant CSV output is the same for each.
UI changes
---
This commit doesn’t change the UI, apart from to give a different error
message if a user uploads a file type that we still don’t understand.
I intend to do this as a separate pull request, in order to fulfil
https://www.pivotaltracker.com/story/show/119371637
2016-05-05 15:41:11 +01:00
|
|
|
|
def test_upload_files_in_different_formats(
|
|
|
|
|
|
filename,
|
2016-05-13 21:25:59 +01:00
|
|
|
|
acceptable_file,
|
2017-02-03 12:07:21 +00:00
|
|
|
|
logged_in_client,
|
Accept common spreadsheet formats, not just CSV
We require users to export their spreadsheets as CSV files before
uploading them. But this seems like the sort of thing a computer should
be able to do.
So this commit adds a wrapper class which:
- takes a the uploaded file
- returns it in a normalised format, or reads it using pyexcel[1]
- gives the data back in CSV format
This allows us to accept `.csv`, `.xlsx`, `.xls` (97 and 95), `.ods`,
`.xlsm` and `.tsv` files. We can upload the resultant CSV just like
normal, and process it for errors as before.
Testing
---
To test this I’ve added a selection of common spreadsheet files as test
data. They all contain the same data, so the tests look to see that the
resultant CSV output is the same for each.
UI changes
---
This commit doesn’t change the UI, apart from to give a different error
message if a user uploads a file type that we still don’t understand.
I intend to do this as a separate pull request, in order to fulfil
https://www.pivotaltracker.com/story/show/119371637
2016-05-05 15:41:11 +01:00
|
|
|
|
api_user_active,
|
|
|
|
|
|
mocker,
|
|
|
|
|
|
mock_login,
|
|
|
|
|
|
mock_get_service,
|
|
|
|
|
|
mock_get_service_template,
|
|
|
|
|
|
mock_s3_upload,
|
|
|
|
|
|
mock_has_permissions,
|
2017-02-03 10:42:01 +00:00
|
|
|
|
fake_uuid,
|
Accept common spreadsheet formats, not just CSV
We require users to export their spreadsheets as CSV files before
uploading them. But this seems like the sort of thing a computer should
be able to do.
So this commit adds a wrapper class which:
- takes a the uploaded file
- returns it in a normalised format, or reads it using pyexcel[1]
- gives the data back in CSV format
This allows us to accept `.csv`, `.xlsx`, `.xls` (97 and 95), `.ods`,
`.xlsm` and `.tsv` files. We can upload the resultant CSV just like
normal, and process it for errors as before.
Testing
---
To test this I’ve added a selection of common spreadsheet files as test
data. They all contain the same data, so the tests look to see that the
resultant CSV output is the same for each.
UI changes
---
This commit doesn’t change the UI, apart from to give a different error
message if a user uploads a file type that we still don’t understand.
I intend to do this as a separate pull request, in order to fulfil
https://www.pivotaltracker.com/story/show/119371637
2016-05-05 15:41:11 +01:00
|
|
|
|
):
|
|
|
|
|
|
|
2017-02-03 12:07:21 +00:00
|
|
|
|
with open(filename, 'rb') as uploaded:
|
|
|
|
|
|
response = logged_in_client.post(
|
Accept common spreadsheet formats, not just CSV
We require users to export their spreadsheets as CSV files before
uploading them. But this seems like the sort of thing a computer should
be able to do.
So this commit adds a wrapper class which:
- takes a the uploaded file
- returns it in a normalised format, or reads it using pyexcel[1]
- gives the data back in CSV format
This allows us to accept `.csv`, `.xlsx`, `.xls` (97 and 95), `.ods`,
`.xlsm` and `.tsv` files. We can upload the resultant CSV just like
normal, and process it for errors as before.
Testing
---
To test this I’ve added a selection of common spreadsheet files as test
data. They all contain the same data, so the tests look to see that the
resultant CSV output is the same for each.
UI changes
---
This commit doesn’t change the UI, apart from to give a different error
message if a user uploads a file type that we still don’t understand.
I intend to do this as a separate pull request, in order to fulfil
https://www.pivotaltracker.com/story/show/119371637
2016-05-05 15:41:11 +01:00
|
|
|
|
url_for('main.send_messages', service_id=fake_uuid, template_id=fake_uuid),
|
|
|
|
|
|
data={'file': (BytesIO(uploaded.read()), filename)},
|
|
|
|
|
|
content_type='multipart/form-data'
|
|
|
|
|
|
)
|
|
|
|
|
|
|
2016-05-13 21:25:59 +01:00
|
|
|
|
if acceptable_file:
|
2016-05-15 07:47:50 +01:00
|
|
|
|
assert mock_s3_upload.call_args[0][1]['data'].strip() == (
|
2016-05-13 21:25:59 +01:00
|
|
|
|
"phone number,name,favourite colour,fruit\r\n"
|
|
|
|
|
|
"07739 468 050,Pete,Coral,tomato\r\n"
|
|
|
|
|
|
"07527 125 974,Not Pete,Magenta,Avacado\r\n"
|
|
|
|
|
|
"07512 058 823,Still Not Pete,Crimson,Pear"
|
|
|
|
|
|
)
|
|
|
|
|
|
else:
|
|
|
|
|
|
assert not mock_s3_upload.called
|
|
|
|
|
|
assert (
|
|
|
|
|
|
'Couldn’t read {}. Try using a different file format.'.format(filename)
|
|
|
|
|
|
) in response.get_data(as_text=True)
|
Accept common spreadsheet formats, not just CSV
We require users to export their spreadsheets as CSV files before
uploading them. But this seems like the sort of thing a computer should
be able to do.
So this commit adds a wrapper class which:
- takes a the uploaded file
- returns it in a normalised format, or reads it using pyexcel[1]
- gives the data back in CSV format
This allows us to accept `.csv`, `.xlsx`, `.xls` (97 and 95), `.ods`,
`.xlsm` and `.tsv` files. We can upload the resultant CSV just like
normal, and process it for errors as before.
Testing
---
To test this I’ve added a selection of common spreadsheet files as test
data. They all contain the same data, so the tests look to see that the
resultant CSV output is the same for each.
UI changes
---
This commit doesn’t change the UI, apart from to give a different error
message if a user uploads a file type that we still don’t understand.
I intend to do this as a separate pull request, in order to fulfil
https://www.pivotaltracker.com/story/show/119371637
2016-05-05 15:41:11 +01:00
|
|
|
|
|
2016-02-22 17:17:18 +00:00
|
|
|
|
|
2016-03-07 18:47:05 +00:00
|
|
|
|
def test_upload_csvfile_with_errors_shows_check_page_with_errors(
|
2017-02-03 12:07:21 +00:00
|
|
|
|
logged_in_client,
|
2016-02-22 17:17:18 +00:00
|
|
|
|
api_user_active,
|
2016-02-26 10:54:06 +00:00
|
|
|
|
mocker,
|
2016-02-22 17:17:18 +00:00
|
|
|
|
mock_login,
|
2016-02-26 12:11:55 +00:00
|
|
|
|
mock_get_service,
|
2016-05-17 09:23:33 +01:00
|
|
|
|
mock_get_service_template_with_placeholders,
|
2016-02-29 17:03:56 +00:00
|
|
|
|
mock_s3_upload,
|
2016-04-04 14:28:52 +01:00
|
|
|
|
mock_has_permissions,
|
2016-04-12 14:19:51 +01:00
|
|
|
|
mock_get_users_by_service,
|
2016-07-25 14:16:34 +01:00
|
|
|
|
mock_get_detailed_service_for_today,
|
2017-02-03 10:42:01 +00:00
|
|
|
|
fake_uuid,
|
2016-02-22 17:17:18 +00:00
|
|
|
|
):
|
2016-01-13 22:34:36 +00:00
|
|
|
|
|
2016-05-17 09:23:33 +01:00
|
|
|
|
mocker.patch(
|
|
|
|
|
|
'app.main.views.send.s3download',
|
|
|
|
|
|
return_value="""
|
|
|
|
|
|
phone number,name
|
|
|
|
|
|
+447700900986
|
|
|
|
|
|
+447700900986
|
|
|
|
|
|
"""
|
|
|
|
|
|
)
|
2015-12-17 16:21:34 +00:00
|
|
|
|
|
2017-02-03 12:07:21 +00:00
|
|
|
|
initial_upload = logged_in_client.post(
|
|
|
|
|
|
url_for('main.send_messages', service_id=fake_uuid, template_id=fake_uuid),
|
|
|
|
|
|
data={'file': (BytesIO(''.encode('utf-8')), 'invalid.csv')},
|
|
|
|
|
|
content_type='multipart/form-data',
|
|
|
|
|
|
follow_redirects=True
|
|
|
|
|
|
)
|
|
|
|
|
|
reupload = logged_in_client.post(
|
|
|
|
|
|
url_for('main.check_messages', service_id=fake_uuid, template_type='sms', upload_id='abc123'),
|
|
|
|
|
|
data={'file': (BytesIO(''.encode('utf-8')), 'invalid.csv')},
|
|
|
|
|
|
content_type='multipart/form-data',
|
|
|
|
|
|
follow_redirects=True
|
|
|
|
|
|
)
|
|
|
|
|
|
for response in [initial_upload, reupload]:
|
|
|
|
|
|
assert response.status_code == 200
|
|
|
|
|
|
content = response.get_data(as_text=True)
|
|
|
|
|
|
assert 'There is a problem with your data' in content
|
|
|
|
|
|
assert '+447700900986' in content
|
|
|
|
|
|
assert 'Missing' in content
|
|
|
|
|
|
assert 'Re-upload your file' in content
|
2016-02-26 10:54:06 +00:00
|
|
|
|
|
|
|
|
|
|
|
2017-02-27 12:06:16 +00:00
|
|
|
|
@pytest.mark.parametrize('file_contents, expected_error,', [
|
|
|
|
|
|
(
|
|
|
|
|
|
"""
|
|
|
|
|
|
telephone,name
|
|
|
|
|
|
+447700900986
|
|
|
|
|
|
""",
|
|
|
|
|
|
(
|
|
|
|
|
|
'Your file needs to have a column called ‘phone number’ '
|
|
|
|
|
|
'Your file has columns called ‘telephone’ and ‘name’. '
|
|
|
|
|
|
'Skip to file contents'
|
|
|
|
|
|
)
|
|
|
|
|
|
),
|
|
|
|
|
|
(
|
|
|
|
|
|
"""
|
|
|
|
|
|
phone number
|
|
|
|
|
|
+447700900986
|
|
|
|
|
|
""",
|
|
|
|
|
|
(
|
|
|
|
|
|
'The columns in your file need to match the double brackets in your template '
|
|
|
|
|
|
'Your file has one column, called ‘phone number’. '
|
|
|
|
|
|
'It doesn’t have a column called ‘name’. '
|
|
|
|
|
|
'Skip to file contents'
|
|
|
|
|
|
)
|
2017-02-27 14:46:01 +00:00
|
|
|
|
),
|
|
|
|
|
|
(
|
|
|
|
|
|
"+447700900986",
|
|
|
|
|
|
(
|
|
|
|
|
|
'Your file is missing some rows '
|
|
|
|
|
|
'It needs at least one row of data, and columns called ‘name’ and ‘phone number’. '
|
|
|
|
|
|
'Skip to file contents'
|
|
|
|
|
|
)
|
|
|
|
|
|
),
|
|
|
|
|
|
(
|
|
|
|
|
|
"",
|
|
|
|
|
|
(
|
|
|
|
|
|
'Your file is missing some rows '
|
|
|
|
|
|
'It needs at least one row of data, and columns called ‘name’ and ‘phone number’. '
|
|
|
|
|
|
'Skip to file contents'
|
|
|
|
|
|
)
|
|
|
|
|
|
),
|
2017-02-27 12:06:16 +00:00
|
|
|
|
])
|
|
|
|
|
|
def test_upload_csvfile_with_missing_columns_shows_error(
|
2017-02-16 11:07:26 +00:00
|
|
|
|
logged_in_client,
|
|
|
|
|
|
mocker,
|
|
|
|
|
|
mock_get_service_template_with_placeholders,
|
|
|
|
|
|
mock_s3_upload,
|
|
|
|
|
|
mock_get_users_by_service,
|
|
|
|
|
|
mock_get_detailed_service_for_today,
|
2017-02-17 09:57:58 +00:00
|
|
|
|
service_one,
|
2017-02-16 11:07:26 +00:00
|
|
|
|
fake_uuid,
|
2017-02-27 12:06:16 +00:00
|
|
|
|
file_contents,
|
|
|
|
|
|
expected_error,
|
2017-02-16 11:07:26 +00:00
|
|
|
|
):
|
|
|
|
|
|
|
2017-02-27 12:06:16 +00:00
|
|
|
|
mocker.patch('app.main.views.send.s3download', return_value=file_contents)
|
2017-02-16 11:07:26 +00:00
|
|
|
|
|
|
|
|
|
|
response = logged_in_client.post(
|
2017-02-17 09:57:58 +00:00
|
|
|
|
url_for('main.send_messages', service_id=service_one['id'], template_id=fake_uuid),
|
2017-02-16 11:07:26 +00:00
|
|
|
|
data={'file': (BytesIO(''.encode('utf-8')), 'invalid.csv')},
|
|
|
|
|
|
follow_redirects=True,
|
|
|
|
|
|
)
|
|
|
|
|
|
|
|
|
|
|
|
assert response.status_code == 200
|
|
|
|
|
|
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
|
2017-02-27 12:06:16 +00:00
|
|
|
|
assert ' '.join(page.select('.banner-dangerous')[0].text.split()) == expected_error
|
2017-02-16 11:07:26 +00:00
|
|
|
|
|
|
|
|
|
|
|
2017-02-03 10:42:01 +00:00
|
|
|
|
def test_upload_csv_invalid_extension(
|
2017-02-03 12:07:21 +00:00
|
|
|
|
logged_in_client,
|
2017-02-03 10:42:01 +00:00
|
|
|
|
api_user_active,
|
|
|
|
|
|
mocker,
|
|
|
|
|
|
mock_login,
|
|
|
|
|
|
mock_get_service,
|
|
|
|
|
|
mock_get_service_template,
|
|
|
|
|
|
mock_s3_upload,
|
|
|
|
|
|
mock_has_permissions,
|
|
|
|
|
|
mock_get_users_by_service,
|
|
|
|
|
|
mock_get_detailed_service_for_today,
|
|
|
|
|
|
fake_uuid,
|
|
|
|
|
|
):
|
2016-06-17 11:54:01 +01:00
|
|
|
|
|
2017-02-03 12:07:21 +00:00
|
|
|
|
resp = logged_in_client.post(
|
|
|
|
|
|
url_for('main.send_messages', service_id=fake_uuid, template_id=fake_uuid),
|
|
|
|
|
|
data={'file': (BytesIO('contents'.encode('utf-8')), 'invalid.txt')},
|
|
|
|
|
|
content_type='multipart/form-data',
|
|
|
|
|
|
follow_redirects=True
|
|
|
|
|
|
)
|
2016-04-29 15:40:35 +01:00
|
|
|
|
|
2017-02-03 12:07:21 +00:00
|
|
|
|
assert resp.status_code == 200
|
|
|
|
|
|
assert "invalid.txt isn’t a spreadsheet that Notify can read" in resp.get_data(as_text=True)
|
2016-04-29 15:40:35 +01:00
|
|
|
|
|
|
|
|
|
|
|
2017-02-10 10:23:08 +00:00
|
|
|
|
def test_upload_valid_csv_shows_page_title(
|
|
|
|
|
|
logged_in_client,
|
|
|
|
|
|
mocker,
|
|
|
|
|
|
mock_get_service_template_with_placeholders,
|
|
|
|
|
|
mock_s3_upload,
|
|
|
|
|
|
mock_get_users_by_service,
|
|
|
|
|
|
mock_get_detailed_service_for_today,
|
|
|
|
|
|
service_one,
|
|
|
|
|
|
fake_uuid,
|
|
|
|
|
|
):
|
|
|
|
|
|
|
|
|
|
|
|
mocker.patch('app.main.views.send.s3download', return_value="""
|
|
|
|
|
|
phone number,name\n07700900986,Jo
|
|
|
|
|
|
""")
|
|
|
|
|
|
|
|
|
|
|
|
response = logged_in_client.post(
|
|
|
|
|
|
url_for('main.send_messages', service_id=service_one['id'], template_id=fake_uuid),
|
|
|
|
|
|
data={'file': (BytesIO(''.encode('utf-8')), 'valid.csv')},
|
|
|
|
|
|
follow_redirects=True,
|
|
|
|
|
|
)
|
|
|
|
|
|
|
|
|
|
|
|
assert response.status_code == 200
|
|
|
|
|
|
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
|
|
|
|
|
|
assert page.h1.text.strip() == 'Preview of Two week reminder'
|
|
|
|
|
|
|
|
|
|
|
|
|
2016-05-05 08:39:36 +01:00
|
|
|
|
def test_send_test_sms_message(
|
2017-02-03 12:07:21 +00:00
|
|
|
|
logged_in_client,
|
2016-02-18 17:03:32 +00:00
|
|
|
|
mocker,
|
|
|
|
|
|
api_user_active,
|
|
|
|
|
|
mock_login,
|
2016-02-26 12:11:55 +00:00
|
|
|
|
mock_get_service,
|
2016-02-26 10:54:06 +00:00
|
|
|
|
mock_get_service_template,
|
2016-02-29 17:03:56 +00:00
|
|
|
|
mock_s3_upload,
|
2016-04-04 14:28:52 +01:00
|
|
|
|
mock_has_permissions,
|
2016-04-12 14:19:51 +01:00
|
|
|
|
mock_get_users_by_service,
|
2016-07-25 14:16:34 +01:00
|
|
|
|
mock_get_detailed_service_for_today,
|
2016-04-12 14:19:51 +01:00
|
|
|
|
fake_uuid
|
2016-02-18 17:03:32 +00:00
|
|
|
|
):
|
|
|
|
|
|
|
2016-05-12 11:22:14 +01:00
|
|
|
|
expected_data = {'data': 'phone number\r\n07700 900 762\r\n', 'file_name': 'Test message'}
|
2016-03-01 09:41:08 +00:00
|
|
|
|
mocker.patch('app.main.views.send.s3download', return_value='phone number\r\n+4412341234')
|
2016-02-26 10:54:06 +00:00
|
|
|
|
|
2017-02-03 12:07:21 +00:00
|
|
|
|
response = logged_in_client.get(
|
|
|
|
|
|
url_for('main.send_test', service_id=fake_uuid, template_id=fake_uuid),
|
|
|
|
|
|
follow_redirects=True
|
|
|
|
|
|
)
|
|
|
|
|
|
assert response.status_code == 200
|
|
|
|
|
|
mock_s3_upload.assert_called_with(fake_uuid, expected_data, 'eu-west-1')
|
2016-02-18 17:03:32 +00:00
|
|
|
|
|
|
|
|
|
|
|
2016-05-05 08:39:36 +01:00
|
|
|
|
def test_send_test_email_message(
|
2017-02-03 12:07:21 +00:00
|
|
|
|
logged_in_client,
|
2016-03-02 17:02:41 +00:00
|
|
|
|
mocker,
|
|
|
|
|
|
api_user_active,
|
|
|
|
|
|
mock_login,
|
|
|
|
|
|
mock_get_service,
|
2016-12-14 10:12:53 +00:00
|
|
|
|
mock_get_service_email_template_without_placeholders,
|
2016-03-03 09:30:30 +00:00
|
|
|
|
mock_s3_upload,
|
2016-04-04 14:28:52 +01:00
|
|
|
|
mock_has_permissions,
|
2016-04-12 14:19:51 +01:00
|
|
|
|
mock_get_users_by_service,
|
2016-07-25 14:16:34 +01:00
|
|
|
|
mock_get_detailed_service_for_today,
|
2016-04-12 14:19:51 +01:00
|
|
|
|
fake_uuid
|
2016-03-02 17:02:41 +00:00
|
|
|
|
):
|
|
|
|
|
|
|
2016-05-12 11:22:14 +01:00
|
|
|
|
expected_data = {'data': 'email address\r\ntest@user.gov.uk\r\n', 'file_name': 'Test message'}
|
2016-03-02 17:02:41 +00:00
|
|
|
|
mocker.patch('app.main.views.send.s3download', return_value='email address\r\ntest@user.gov.uk')
|
|
|
|
|
|
|
2017-02-03 12:07:21 +00:00
|
|
|
|
response = logged_in_client.get(
|
|
|
|
|
|
url_for('main.send_test', service_id=fake_uuid, template_id=fake_uuid),
|
|
|
|
|
|
follow_redirects=True
|
|
|
|
|
|
)
|
|
|
|
|
|
assert response.status_code == 200
|
|
|
|
|
|
mock_s3_upload.assert_called_with(fake_uuid, expected_data, 'eu-west-1')
|
2016-03-02 17:02:41 +00:00
|
|
|
|
|
|
|
|
|
|
|
2016-05-05 08:39:36 +01:00
|
|
|
|
def test_send_test_sms_message_with_placeholders(
|
2017-02-03 12:07:21 +00:00
|
|
|
|
logged_in_client,
|
2016-05-05 08:39:36 +01:00
|
|
|
|
mocker,
|
|
|
|
|
|
api_user_active,
|
|
|
|
|
|
mock_login,
|
|
|
|
|
|
mock_get_service,
|
|
|
|
|
|
mock_get_service_template_with_placeholders,
|
|
|
|
|
|
mock_s3_upload,
|
|
|
|
|
|
mock_has_permissions,
|
|
|
|
|
|
mock_get_users_by_service,
|
2016-07-25 14:16:34 +01:00
|
|
|
|
mock_get_detailed_service_for_today,
|
2016-05-05 08:39:36 +01:00
|
|
|
|
fake_uuid
|
|
|
|
|
|
):
|
|
|
|
|
|
|
|
|
|
|
|
expected_data = {
|
|
|
|
|
|
'data': 'phone number,name\r\n07700 900 762,Jo\r\n',
|
2016-05-12 11:22:14 +01:00
|
|
|
|
'file_name': 'Test message'
|
2016-05-05 08:39:36 +01:00
|
|
|
|
}
|
|
|
|
|
|
mocker.patch('app.main.views.send.s3download', return_value='phone number\r\n+4412341234')
|
|
|
|
|
|
|
2017-02-03 12:07:21 +00:00
|
|
|
|
response = logged_in_client.post(
|
|
|
|
|
|
url_for(
|
|
|
|
|
|
'main.send_test',
|
|
|
|
|
|
service_id=fake_uuid,
|
|
|
|
|
|
template_id=fake_uuid
|
|
|
|
|
|
),
|
|
|
|
|
|
data={'name': 'Jo'},
|
|
|
|
|
|
follow_redirects=True
|
|
|
|
|
|
)
|
|
|
|
|
|
assert response.status_code == 200
|
|
|
|
|
|
mock_s3_upload.assert_called_with(fake_uuid, expected_data, 'eu-west-1')
|
2016-05-05 08:39:36 +01:00
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_api_info_page(
|
2017-02-03 12:07:21 +00:00
|
|
|
|
logged_in_client,
|
2016-03-15 06:53:06 +00:00
|
|
|
|
mocker,
|
|
|
|
|
|
api_user_active,
|
|
|
|
|
|
mock_login,
|
|
|
|
|
|
mock_get_service,
|
|
|
|
|
|
mock_get_service_email_template,
|
|
|
|
|
|
mock_s3_upload,
|
2016-04-12 14:19:51 +01:00
|
|
|
|
mock_has_permissions,
|
|
|
|
|
|
fake_uuid
|
2016-03-15 06:53:06 +00:00
|
|
|
|
):
|
2017-02-03 12:07:21 +00:00
|
|
|
|
response = logged_in_client.get(
|
|
|
|
|
|
url_for('main.send_from_api', service_id=fake_uuid, template_id=fake_uuid),
|
|
|
|
|
|
follow_redirects=True
|
|
|
|
|
|
)
|
|
|
|
|
|
assert response.status_code == 200
|
|
|
|
|
|
assert 'API info' in response.get_data(as_text=True)
|
2016-03-15 06:53:06 +00:00
|
|
|
|
|
|
|
|
|
|
|
2016-02-18 17:03:32 +00:00
|
|
|
|
def test_download_example_csv(
|
2017-02-03 12:07:21 +00:00
|
|
|
|
logged_in_client,
|
2016-02-18 17:03:32 +00:00
|
|
|
|
mocker,
|
|
|
|
|
|
api_user_active,
|
|
|
|
|
|
mock_login,
|
2016-02-26 12:11:55 +00:00
|
|
|
|
mock_get_service,
|
2016-02-29 17:03:56 +00:00
|
|
|
|
mock_get_service_template,
|
2016-04-12 14:19:51 +01:00
|
|
|
|
mock_has_permissions,
|
|
|
|
|
|
fake_uuid
|
2016-02-18 17:03:32 +00:00
|
|
|
|
):
|
|
|
|
|
|
|
2017-02-03 12:07:21 +00:00
|
|
|
|
response = logged_in_client.get(
|
|
|
|
|
|
url_for('main.get_example_csv', service_id=fake_uuid, template_id=fake_uuid),
|
|
|
|
|
|
follow_redirects=True
|
|
|
|
|
|
)
|
|
|
|
|
|
assert response.status_code == 200
|
|
|
|
|
|
assert response.get_data(as_text=True) == 'phone number\r\n07700 900321\r\n'
|
|
|
|
|
|
assert 'text/csv' in response.headers['Content-Type']
|
2016-02-18 17:03:32 +00:00
|
|
|
|
|
|
|
|
|
|
|
2016-02-22 17:17:18 +00:00
|
|
|
|
def test_upload_csvfile_with_valid_phone_shows_all_numbers(
|
2017-02-03 12:07:21 +00:00
|
|
|
|
logged_in_client,
|
2016-02-22 17:17:18 +00:00
|
|
|
|
mocker,
|
|
|
|
|
|
api_user_active,
|
|
|
|
|
|
mock_login,
|
2016-02-26 12:11:55 +00:00
|
|
|
|
mock_get_service,
|
2016-02-26 10:54:06 +00:00
|
|
|
|
mock_get_service_template,
|
2016-02-29 17:03:56 +00:00
|
|
|
|
mock_s3_upload,
|
2016-04-04 14:28:52 +01:00
|
|
|
|
mock_has_permissions,
|
2016-04-12 14:19:51 +01:00
|
|
|
|
mock_get_users_by_service,
|
2016-07-25 14:16:34 +01:00
|
|
|
|
mock_get_detailed_service_for_today,
|
2016-04-12 14:19:51 +01:00
|
|
|
|
fake_uuid
|
2016-02-22 17:17:18 +00:00
|
|
|
|
):
|
2016-01-13 22:34:36 +00:00
|
|
|
|
|
2016-03-07 18:47:05 +00:00
|
|
|
|
mocker.patch(
|
|
|
|
|
|
'app.main.views.send.s3download',
|
2016-04-07 14:30:51 +01:00
|
|
|
|
return_value='\n'.join(['phone number'] + [
|
|
|
|
|
|
'07700 9007{0:02d}'.format(final_two) for final_two in range(0, 53)
|
|
|
|
|
|
])
|
2016-03-07 18:47:05 +00:00
|
|
|
|
)
|
2016-01-11 15:00:51 +00:00
|
|
|
|
|
2017-02-03 12:07:21 +00:00
|
|
|
|
response = logged_in_client.post(
|
|
|
|
|
|
url_for('main.send_messages', service_id=fake_uuid, template_id=fake_uuid),
|
|
|
|
|
|
data={'file': (BytesIO(''.encode('utf-8')), 'valid.csv')},
|
|
|
|
|
|
content_type='multipart/form-data',
|
|
|
|
|
|
follow_redirects=True
|
|
|
|
|
|
)
|
|
|
|
|
|
with logged_in_client.session_transaction() as sess:
|
|
|
|
|
|
assert sess['upload_data']['template_id'] == fake_uuid
|
|
|
|
|
|
assert sess['upload_data']['original_file_name'] == 'valid.csv'
|
|
|
|
|
|
assert sess['upload_data']['notification_count'] == 53
|
|
|
|
|
|
|
|
|
|
|
|
content = response.get_data(as_text=True)
|
|
|
|
|
|
assert response.status_code == 200
|
|
|
|
|
|
assert '07700 900701' in content
|
|
|
|
|
|
assert '07700 900749' in content
|
|
|
|
|
|
assert '07700 900750' not in content
|
|
|
|
|
|
assert 'Only showing the first 50 rows' in content
|
|
|
|
|
|
|
|
|
|
|
|
mock_get_detailed_service_for_today.assert_called_once_with(fake_uuid)
|
2016-06-17 11:54:01 +01:00
|
|
|
|
|
2016-01-11 15:00:51 +00:00
|
|
|
|
|
2016-08-30 09:27:24 +01:00
|
|
|
|
def test_test_message_can_only_be_sent_now(
|
2017-02-03 12:07:21 +00:00
|
|
|
|
logged_in_client,
|
2016-08-30 09:27:24 +01:00
|
|
|
|
mocker,
|
|
|
|
|
|
api_user_active,
|
|
|
|
|
|
mock_login,
|
|
|
|
|
|
mock_get_service,
|
|
|
|
|
|
mock_get_service_template,
|
|
|
|
|
|
mock_s3_download,
|
|
|
|
|
|
mock_has_permissions,
|
|
|
|
|
|
mock_get_users_by_service,
|
|
|
|
|
|
mock_get_detailed_service_for_today,
|
|
|
|
|
|
fake_uuid
|
|
|
|
|
|
):
|
|
|
|
|
|
|
2017-02-03 12:07:21 +00:00
|
|
|
|
with logged_in_client.session_transaction() as session:
|
|
|
|
|
|
session['upload_data'] = {
|
|
|
|
|
|
'original_file_name': 'Test message',
|
|
|
|
|
|
'template_id': fake_uuid,
|
|
|
|
|
|
'notification_count': 1,
|
|
|
|
|
|
'valid': True
|
|
|
|
|
|
}
|
|
|
|
|
|
response = logged_in_client.get(url_for(
|
|
|
|
|
|
'main.check_messages',
|
|
|
|
|
|
service_id=fake_uuid,
|
|
|
|
|
|
upload_id=fake_uuid,
|
|
|
|
|
|
template_type='sms',
|
|
|
|
|
|
from_test=True
|
|
|
|
|
|
))
|
2016-08-30 09:27:24 +01:00
|
|
|
|
|
2017-02-03 12:07:21 +00:00
|
|
|
|
content = response.get_data(as_text=True)
|
|
|
|
|
|
assert 'name="scheduled_for"' not in content
|
2016-08-30 09:27:24 +01:00
|
|
|
|
|
|
|
|
|
|
|
2016-08-07 09:17:49 +01:00
|
|
|
|
@pytest.mark.parametrize(
|
|
|
|
|
|
'when', [
|
|
|
|
|
|
'', '2016-08-25T13:04:21.767198'
|
|
|
|
|
|
]
|
|
|
|
|
|
)
|
2016-02-22 17:17:18 +00:00
|
|
|
|
def test_create_job_should_call_api(
|
2017-02-03 12:07:21 +00:00
|
|
|
|
logged_in_client,
|
2016-02-22 17:17:18 +00:00
|
|
|
|
service_one,
|
2016-05-11 11:57:31 +01:00
|
|
|
|
active_user_with_permissions,
|
2016-02-22 17:17:18 +00:00
|
|
|
|
mock_create_job,
|
|
|
|
|
|
mock_get_job,
|
2016-03-02 15:37:35 +00:00
|
|
|
|
mock_get_notifications,
|
2016-02-29 17:03:56 +00:00
|
|
|
|
mock_get_service_template,
|
2016-05-24 12:34:29 +01:00
|
|
|
|
mocker,
|
2016-08-07 09:17:49 +01:00
|
|
|
|
fake_uuid,
|
|
|
|
|
|
when
|
2016-02-22 17:17:18 +00:00
|
|
|
|
):
|
2016-01-29 12:19:50 +00:00
|
|
|
|
service_id = service_one['id']
|
2016-05-24 12:34:29 +01:00
|
|
|
|
data = mock_get_job(service_one['id'], fake_uuid)['data']
|
|
|
|
|
|
job_id = data['id']
|
|
|
|
|
|
original_file_name = data['original_file_name']
|
|
|
|
|
|
template_id = data['template']
|
|
|
|
|
|
notification_count = data['notification_count']
|
2017-02-03 12:07:21 +00:00
|
|
|
|
with logged_in_client.session_transaction() as session:
|
|
|
|
|
|
session['upload_data'] = {
|
|
|
|
|
|
'original_file_name': original_file_name,
|
|
|
|
|
|
'template_id': template_id,
|
|
|
|
|
|
'notification_count': notification_count,
|
|
|
|
|
|
'valid': True
|
|
|
|
|
|
}
|
|
|
|
|
|
url = url_for('main.start_job', service_id=service_one['id'], upload_id=job_id)
|
|
|
|
|
|
response = logged_in_client.post(url, data={'scheduled_for': when}, follow_redirects=True)
|
2016-08-25 13:27:24 +01:00
|
|
|
|
|
|
|
|
|
|
assert response.status_code == 200
|
|
|
|
|
|
assert original_file_name in response.get_data(as_text=True)
|
|
|
|
|
|
mock_create_job.assert_called_with(
|
|
|
|
|
|
job_id,
|
|
|
|
|
|
service_id,
|
|
|
|
|
|
template_id,
|
|
|
|
|
|
original_file_name,
|
|
|
|
|
|
notification_count,
|
|
|
|
|
|
scheduled_for=when
|
|
|
|
|
|
)
|
2016-03-03 15:26:52 +00:00
|
|
|
|
|
|
|
|
|
|
|
2017-01-20 16:20:37 +00:00
|
|
|
|
def test_can_start_letters_job(
|
2017-02-14 14:06:02 +00:00
|
|
|
|
logged_in_platform_admin_client,
|
2016-11-08 13:10:38 +00:00
|
|
|
|
mock_create_job,
|
2017-02-14 14:06:02 +00:00
|
|
|
|
service_one,
|
2016-11-08 13:10:38 +00:00
|
|
|
|
fake_uuid
|
|
|
|
|
|
):
|
2017-02-14 14:06:02 +00:00
|
|
|
|
|
|
|
|
|
|
with logged_in_platform_admin_client.session_transaction() as session:
|
2016-11-08 13:10:38 +00:00
|
|
|
|
session['upload_data'] = {
|
|
|
|
|
|
'original_file_name': 'example.csv',
|
|
|
|
|
|
'template_id': fake_uuid,
|
|
|
|
|
|
'notification_count': 123,
|
|
|
|
|
|
'valid': True
|
|
|
|
|
|
}
|
2017-02-14 14:06:02 +00:00
|
|
|
|
response = logged_in_platform_admin_client.post(
|
|
|
|
|
|
url_for('main.start_job', service_id=service_one['id'], upload_id=fake_uuid),
|
2016-11-08 13:10:38 +00:00
|
|
|
|
data={}
|
|
|
|
|
|
)
|
2017-01-20 16:20:37 +00:00
|
|
|
|
assert response.status_code == 302
|
2016-11-08 13:10:38 +00:00
|
|
|
|
|
|
|
|
|
|
|
2016-12-20 14:38:34 +00:00
|
|
|
|
@pytest.mark.parametrize(
|
|
|
|
|
|
'view, expected_content_type',
|
|
|
|
|
|
[
|
|
|
|
|
|
('.check_messages_as_pdf', 'application/pdf'),
|
|
|
|
|
|
('.check_messages_as_png', 'image/png'),
|
|
|
|
|
|
]
|
|
|
|
|
|
)
|
|
|
|
|
|
@patch('app.utils.LetterPreviewTemplate.jinja_template.render', return_value='')
|
|
|
|
|
|
def test_should_show_preview_letter_message(
|
|
|
|
|
|
mock_letter_preview,
|
|
|
|
|
|
view,
|
|
|
|
|
|
expected_content_type,
|
2017-02-14 14:06:02 +00:00
|
|
|
|
logged_in_platform_admin_client,
|
2016-12-20 14:38:34 +00:00
|
|
|
|
mock_get_service_letter_template,
|
|
|
|
|
|
mock_get_users_by_service,
|
|
|
|
|
|
mock_get_detailed_service_for_today,
|
2017-02-14 14:06:02 +00:00
|
|
|
|
service_one,
|
2016-12-20 14:38:34 +00:00
|
|
|
|
fake_uuid,
|
|
|
|
|
|
mocker,
|
|
|
|
|
|
):
|
2017-02-14 14:06:02 +00:00
|
|
|
|
service_one['can_send_letters'] = True
|
|
|
|
|
|
mocker.patch('app.service_api_client.get_service', return_value={"data": service_one})
|
2016-12-20 14:38:34 +00:00
|
|
|
|
|
|
|
|
|
|
mocker.patch(
|
|
|
|
|
|
'app.main.views.send.s3download',
|
|
|
|
|
|
return_value='\n'.join(
|
|
|
|
|
|
['address line 1, postcode'] +
|
|
|
|
|
|
['123 street, abc123']
|
|
|
|
|
|
)
|
|
|
|
|
|
)
|
|
|
|
|
|
|
2017-02-14 14:06:02 +00:00
|
|
|
|
service_id = service_one['id']
|
2016-12-20 14:38:34 +00:00
|
|
|
|
template_id = fake_uuid
|
2017-02-14 14:06:02 +00:00
|
|
|
|
with logged_in_platform_admin_client.session_transaction() as session:
|
2016-12-20 14:38:34 +00:00
|
|
|
|
session['upload_data'] = {
|
|
|
|
|
|
'original_file_name': 'example.csv',
|
|
|
|
|
|
'template_id': fake_uuid,
|
|
|
|
|
|
'notification_count': 1,
|
|
|
|
|
|
'valid': True
|
|
|
|
|
|
}
|
2017-02-14 14:06:02 +00:00
|
|
|
|
response = logged_in_platform_admin_client.get(
|
|
|
|
|
|
url_for(view, service_id=service_id, template_type='letter', upload_id=fake_uuid)
|
|
|
|
|
|
)
|
2016-12-20 14:38:34 +00:00
|
|
|
|
|
|
|
|
|
|
assert response.status_code == 200
|
|
|
|
|
|
assert response.content_type == expected_content_type
|
|
|
|
|
|
mock_get_service_letter_template.assert_called_with(service_id, template_id)
|
2017-02-03 09:44:58 +00:00
|
|
|
|
assert mock_letter_preview.call_args[0][0]['subject'] == (
|
|
|
|
|
|
'Subject'
|
|
|
|
|
|
)
|
2016-12-20 14:38:34 +00:00
|
|
|
|
assert mock_letter_preview.call_args[0][0]['message'] == (
|
2017-01-19 12:22:24 +00:00
|
|
|
|
'<p>Template <em>content</em> with & entity</p>'
|
2016-12-20 14:38:34 +00:00
|
|
|
|
)
|
|
|
|
|
|
|
|
|
|
|
|
|
2016-03-03 15:26:52 +00:00
|
|
|
|
def test_check_messages_should_revalidate_file_when_uploading_file(
|
2017-02-03 12:07:21 +00:00
|
|
|
|
logged_in_client,
|
2016-03-03 15:26:52 +00:00
|
|
|
|
service_one,
|
2016-05-11 11:57:31 +01:00
|
|
|
|
active_user_with_permissions,
|
2016-03-03 15:26:52 +00:00
|
|
|
|
mock_create_job,
|
2016-05-24 12:34:29 +01:00
|
|
|
|
mock_get_job,
|
2016-05-17 09:23:33 +01:00
|
|
|
|
mock_get_service_template_with_placeholders,
|
2016-03-03 15:26:52 +00:00
|
|
|
|
mock_s3_upload,
|
2016-03-03 15:39:15 +00:00
|
|
|
|
mocker,
|
2016-04-04 14:28:52 +01:00
|
|
|
|
mock_has_permissions,
|
2016-07-25 14:16:34 +01:00
|
|
|
|
mock_get_detailed_service_for_today,
|
2016-05-24 12:34:29 +01:00
|
|
|
|
mock_get_users_by_service,
|
|
|
|
|
|
fake_uuid
|
2016-03-03 15:26:52 +00:00
|
|
|
|
):
|
|
|
|
|
|
|
|
|
|
|
|
service_id = service_one['id']
|
|
|
|
|
|
|
2016-03-07 18:47:05 +00:00
|
|
|
|
mocker.patch(
|
|
|
|
|
|
'app.main.views.send.s3download',
|
2016-03-10 20:03:53 +00:00
|
|
|
|
return_value="""
|
|
|
|
|
|
phone number,name,,,
|
2016-05-17 09:23:33 +01:00
|
|
|
|
+447700900986,,,,
|
|
|
|
|
|
+447700900986,,,,
|
2016-03-10 20:03:53 +00:00
|
|
|
|
"""
|
2016-03-07 18:47:05 +00:00
|
|
|
|
)
|
2016-05-24 12:34:29 +01:00
|
|
|
|
data = mock_get_job(service_one['id'], fake_uuid)['data']
|
2017-02-03 12:07:21 +00:00
|
|
|
|
with logged_in_client.session_transaction() as session:
|
|
|
|
|
|
session['upload_data'] = {'original_file_name': 'invalid.csv',
|
|
|
|
|
|
'template_id': data['template'],
|
|
|
|
|
|
'notification_count': data['notification_count'],
|
|
|
|
|
|
'valid': True}
|
|
|
|
|
|
response = logged_in_client.post(
|
|
|
|
|
|
url_for('main.start_job', service_id=service_id, upload_id=data['id']),
|
|
|
|
|
|
data={'file': (BytesIO(''.encode('utf-8')), 'invalid.csv')},
|
|
|
|
|
|
content_type='multipart/form-data',
|
|
|
|
|
|
follow_redirects=True
|
|
|
|
|
|
)
|
|
|
|
|
|
assert response.status_code == 200
|
|
|
|
|
|
assert 'There is a problem with your data' in response.get_data(as_text=True)
|
2016-03-09 12:10:50 +00:00
|
|
|
|
|
|
|
|
|
|
|
2016-11-02 16:53:40 +00:00
|
|
|
|
@pytest.mark.parametrize('route, response_code', [
|
|
|
|
|
|
('main.choose_template', 200),
|
|
|
|
|
|
('main.send_messages', 200),
|
|
|
|
|
|
('main.get_example_csv', 200),
|
|
|
|
|
|
('main.send_test', 302)
|
|
|
|
|
|
])
|
2017-02-03 10:42:01 +00:00
|
|
|
|
def test_route_permissions(
|
|
|
|
|
|
mocker,
|
|
|
|
|
|
app_,
|
2017-02-03 12:07:21 +00:00
|
|
|
|
client,
|
2017-02-03 10:42:01 +00:00
|
|
|
|
api_user_active,
|
|
|
|
|
|
service_one,
|
|
|
|
|
|
mock_get_service_template,
|
|
|
|
|
|
mock_get_service_templates,
|
|
|
|
|
|
mock_get_jobs,
|
|
|
|
|
|
mock_get_notifications,
|
|
|
|
|
|
mock_create_job,
|
|
|
|
|
|
mock_s3_upload,
|
|
|
|
|
|
fake_uuid,
|
|
|
|
|
|
route,
|
|
|
|
|
|
response_code,
|
|
|
|
|
|
):
|
2017-02-03 12:07:21 +00:00
|
|
|
|
validate_route_permission(
|
|
|
|
|
|
mocker,
|
|
|
|
|
|
app_,
|
|
|
|
|
|
"GET",
|
|
|
|
|
|
response_code,
|
|
|
|
|
|
url_for(
|
|
|
|
|
|
route,
|
|
|
|
|
|
service_id=service_one['id'],
|
Merge email, text message + letter templates pages
Right now we have separate pages for email and text message templates.
In the future we will also have a separate page for letter templates.
This commit changes Notify to only have one page for all templates.
What is the problem?
---
The left-hand navigation is getting quite crowded, at 8 items for a
service that can send letters. Research suggests that the number of
objects an average human can hold in working memory is 7 ± 2 [1]. So
we’re at the limit of how many items the navigation should have.
In the future we will need to search/sort/filter templates by attributes
other than type, for example:
- show me the ‘confirmation’ templates
- show me the most recently used templates
- show me all templates containing the placeholder `((ref_no))`
These are hypothetical for now, but these needs (or others) may become
real in the future. At this point pre-filtering the list of templates
by type would restrict what searches a user could do. So by making this
change now we’re in a better position to iterate the design in the
future.
What’s the change?
---
This commit replaces the ‘Email templates’, ‘Text message templates’ and
‘Letter templates’ pages with one page called ‘Templates’.
This new templates page shows all the templates for the service, sorted
by most recently created first (as before).
To add a new template there is a new page with a form asking you what
kind of template you want to create. This is necessary because in the
past we knew what kind of template you wanted to create based on the
kind you were looking at.
What’s the impact of this change on new users?
---
This change alters the onboarding process slightly. We still want to
take people through the empty templates page from the call-to-action on
the dashboard because it helps them understand that to send a message
using Notify you need a template. But because we don’t have separate
pages for emails/text messages we will have to send users through the
extra step of choosing what kind of template to create. This is a bit
clunkier on first use but:
- it still gets the point across
- it takes them through the actual flow they will be using to create new
templates in the future (ie they’re learning how to use Notify, not
just being taken through a special onboarding route)
I’m not too worried about this change in terms of the experience for new
users. Furthermore, by making it now we get to validate whether it’s
causing any problems in the lab research booked for next week.
What’s the impact of this change on current services?
---
Looking at the top 15 services by number of templates[2], most are using
either text messages or emails. So this change would not have a
significant impact on these services because the page will not get any
longer. In other words we wouldn’t be making it worse for them.
Those services who do use both are not using as many templates. The
worst-case scenario is SSCS, who have 16 templates, evenly split between
email and text messages. So they would go from having 8 templates per
page to 16, which is still less than half the number that HMPO or
Digital Marketplace are managing.
References
---
1. https://en.wikipedia.org/wiki/The_Magical_Number_Seven,_Plus_or_Minus_Two
2. Template usage by service
Service name | Template count | Template types
---------------------------------------|----------------|---------------
Her Majesty's Passport Office | 40 | sms
Digital Marketplace | 40 | email
GovWifi-Staging | 19 | sms
GovWifi | 18 | sms
Digital Apprenticeship Service | 16 | email
SSCS | 16 | both
Crown Commercial Service MI Collection | 15 | email
Help with Prison Visits | 12 | both
Digital Future | 12 | email
Export Licensing Service | 11 | email
Civil Money Claims | 9 | both
DVLA Drivers Medical Service | 9 | sms
GOV.UK Notify | 8 | both
Manage your benefit overpayments | 8 | both
Tax Renewals | 8 | both
2017-02-28 12:16:35 +00:00
|
|
|
|
template_id=fake_uuid
|
|
|
|
|
|
),
|
2017-02-03 12:07:21 +00:00
|
|
|
|
['send_texts', 'send_emails', 'send_letters'],
|
|
|
|
|
|
api_user_active,
|
|
|
|
|
|
service_one)
|
2016-03-09 12:10:50 +00:00
|
|
|
|
|
|
|
|
|
|
|
2016-11-02 16:53:40 +00:00
|
|
|
|
@pytest.mark.parametrize('route', [
|
|
|
|
|
|
'main.choose_template',
|
|
|
|
|
|
'main.send_messages',
|
|
|
|
|
|
'main.get_example_csv',
|
|
|
|
|
|
'main.send_test'
|
|
|
|
|
|
])
|
2017-02-03 10:42:01 +00:00
|
|
|
|
def test_route_invalid_permissions(
|
|
|
|
|
|
mocker,
|
|
|
|
|
|
app_,
|
2017-02-03 12:07:21 +00:00
|
|
|
|
client,
|
2017-02-03 10:42:01 +00:00
|
|
|
|
api_user_active,
|
|
|
|
|
|
service_one,
|
|
|
|
|
|
mock_get_service_template,
|
|
|
|
|
|
mock_get_service_templates,
|
|
|
|
|
|
mock_get_jobs,
|
|
|
|
|
|
mock_get_notifications,
|
|
|
|
|
|
mock_create_job,
|
|
|
|
|
|
fake_uuid,
|
|
|
|
|
|
route,
|
|
|
|
|
|
):
|
2017-02-03 12:07:21 +00:00
|
|
|
|
validate_route_permission(
|
|
|
|
|
|
mocker,
|
|
|
|
|
|
app_,
|
|
|
|
|
|
"GET",
|
|
|
|
|
|
403,
|
|
|
|
|
|
url_for(
|
|
|
|
|
|
route,
|
|
|
|
|
|
service_id=service_one['id'],
|
|
|
|
|
|
template_type='sms',
|
|
|
|
|
|
template_id=fake_uuid),
|
|
|
|
|
|
['blah'],
|
|
|
|
|
|
api_user_active,
|
|
|
|
|
|
service_one)
|
2016-03-09 13:51:56 +00:00
|
|
|
|
|
|
|
|
|
|
|
2017-02-03 10:42:01 +00:00
|
|
|
|
def test_route_choose_template_manage_service_permissions(
|
|
|
|
|
|
mocker,
|
|
|
|
|
|
app_,
|
2017-02-03 12:07:21 +00:00
|
|
|
|
client,
|
2017-02-03 10:42:01 +00:00
|
|
|
|
api_user_active,
|
|
|
|
|
|
service_one,
|
|
|
|
|
|
mock_login,
|
|
|
|
|
|
mock_get_user,
|
|
|
|
|
|
mock_get_service,
|
|
|
|
|
|
mock_check_verify_code,
|
|
|
|
|
|
mock_get_service_templates,
|
|
|
|
|
|
mock_get_jobs,
|
|
|
|
|
|
):
|
2017-02-03 12:07:21 +00:00
|
|
|
|
template_id = mock_get_service_templates(service_one['id'])['data'][0]['id']
|
|
|
|
|
|
resp = validate_route_permission(
|
|
|
|
|
|
mocker,
|
|
|
|
|
|
app_,
|
|
|
|
|
|
"GET",
|
|
|
|
|
|
200,
|
|
|
|
|
|
url_for(
|
|
|
|
|
|
'main.choose_template',
|
2016-03-09 13:51:56 +00:00
|
|
|
|
service_id=service_one['id'],
|
Merge email, text message + letter templates pages
Right now we have separate pages for email and text message templates.
In the future we will also have a separate page for letter templates.
This commit changes Notify to only have one page for all templates.
What is the problem?
---
The left-hand navigation is getting quite crowded, at 8 items for a
service that can send letters. Research suggests that the number of
objects an average human can hold in working memory is 7 ± 2 [1]. So
we’re at the limit of how many items the navigation should have.
In the future we will need to search/sort/filter templates by attributes
other than type, for example:
- show me the ‘confirmation’ templates
- show me the most recently used templates
- show me all templates containing the placeholder `((ref_no))`
These are hypothetical for now, but these needs (or others) may become
real in the future. At this point pre-filtering the list of templates
by type would restrict what searches a user could do. So by making this
change now we’re in a better position to iterate the design in the
future.
What’s the change?
---
This commit replaces the ‘Email templates’, ‘Text message templates’ and
‘Letter templates’ pages with one page called ‘Templates’.
This new templates page shows all the templates for the service, sorted
by most recently created first (as before).
To add a new template there is a new page with a form asking you what
kind of template you want to create. This is necessary because in the
past we knew what kind of template you wanted to create based on the
kind you were looking at.
What’s the impact of this change on new users?
---
This change alters the onboarding process slightly. We still want to
take people through the empty templates page from the call-to-action on
the dashboard because it helps them understand that to send a message
using Notify you need a template. But because we don’t have separate
pages for emails/text messages we will have to send users through the
extra step of choosing what kind of template to create. This is a bit
clunkier on first use but:
- it still gets the point across
- it takes them through the actual flow they will be using to create new
templates in the future (ie they’re learning how to use Notify, not
just being taken through a special onboarding route)
I’m not too worried about this change in terms of the experience for new
users. Furthermore, by making it now we get to validate whether it’s
causing any problems in the lab research booked for next week.
What’s the impact of this change on current services?
---
Looking at the top 15 services by number of templates[2], most are using
either text messages or emails. So this change would not have a
significant impact on these services because the page will not get any
longer. In other words we wouldn’t be making it worse for them.
Those services who do use both are not using as many templates. The
worst-case scenario is SSCS, who have 16 templates, evenly split between
email and text messages. So they would go from having 8 templates per
page to 16, which is still less than half the number that HMPO or
Digital Marketplace are managing.
References
---
1. https://en.wikipedia.org/wiki/The_Magical_Number_Seven,_Plus_or_Minus_Two
2. Template usage by service
Service name | Template count | Template types
---------------------------------------|----------------|---------------
Her Majesty's Passport Office | 40 | sms
Digital Marketplace | 40 | email
GovWifi-Staging | 19 | sms
GovWifi | 18 | sms
Digital Apprenticeship Service | 16 | email
SSCS | 16 | both
Crown Commercial Service MI Collection | 15 | email
Help with Prison Visits | 12 | both
Digital Future | 12 | email
Export Licensing Service | 11 | email
Civil Money Claims | 9 | both
DVLA Drivers Medical Service | 9 | sms
GOV.UK Notify | 8 | both
Manage your benefit overpayments | 8 | both
Tax Renewals | 8 | both
2017-02-28 12:16:35 +00:00
|
|
|
|
),
|
2017-02-03 12:07:21 +00:00
|
|
|
|
['manage_users', 'manage_templates', 'manage_settings'],
|
|
|
|
|
|
api_user_active,
|
|
|
|
|
|
service_one)
|
|
|
|
|
|
page = resp.get_data(as_text=True)
|
|
|
|
|
|
assert url_for(
|
|
|
|
|
|
"main.send_messages",
|
|
|
|
|
|
service_id=service_one['id'],
|
|
|
|
|
|
template_id=template_id) not in page
|
|
|
|
|
|
assert url_for(
|
|
|
|
|
|
"main.send_test",
|
|
|
|
|
|
service_id=service_one['id'],
|
|
|
|
|
|
template_id=template_id) not in page
|
|
|
|
|
|
assert url_for(
|
|
|
|
|
|
"main.edit_service_template",
|
|
|
|
|
|
service_id=service_one['id'],
|
|
|
|
|
|
template_id=template_id) in page
|
2016-03-09 13:51:56 +00:00
|
|
|
|
|
|
|
|
|
|
|
2017-02-03 10:42:01 +00:00
|
|
|
|
def test_route_choose_template_send_messages_permissions(
|
|
|
|
|
|
mocker,
|
|
|
|
|
|
app_,
|
2017-02-03 12:07:21 +00:00
|
|
|
|
client,
|
2017-02-03 10:42:01 +00:00
|
|
|
|
active_user_with_permissions,
|
|
|
|
|
|
service_one,
|
|
|
|
|
|
mock_get_service,
|
|
|
|
|
|
mock_check_verify_code,
|
|
|
|
|
|
mock_get_service_templates,
|
|
|
|
|
|
mock_get_jobs,
|
|
|
|
|
|
):
|
2017-02-03 12:07:21 +00:00
|
|
|
|
template_id = None
|
|
|
|
|
|
for temp in mock_get_service_templates(service_one['id'])['data']:
|
|
|
|
|
|
if temp['template_type'] == 'sms':
|
|
|
|
|
|
template_id = temp['id']
|
|
|
|
|
|
assert template_id
|
|
|
|
|
|
resp = validate_route_permission(
|
|
|
|
|
|
mocker,
|
|
|
|
|
|
app_,
|
|
|
|
|
|
"GET",
|
|
|
|
|
|
200,
|
|
|
|
|
|
url_for(
|
|
|
|
|
|
'main.choose_template',
|
2016-03-09 13:51:56 +00:00
|
|
|
|
service_id=service_one['id'],
|
2017-02-03 12:07:21 +00:00
|
|
|
|
template_type='sms'),
|
|
|
|
|
|
['send_texts', 'send_emails', 'send_letters'],
|
|
|
|
|
|
active_user_with_permissions,
|
|
|
|
|
|
service_one)
|
|
|
|
|
|
page = resp.get_data(as_text=True)
|
|
|
|
|
|
assert url_for(
|
|
|
|
|
|
"main.send_messages",
|
|
|
|
|
|
service_id=service_one['id'],
|
|
|
|
|
|
template_id=template_id) in page
|
|
|
|
|
|
assert url_for(
|
|
|
|
|
|
"main.edit_service_template",
|
|
|
|
|
|
service_id=service_one['id'],
|
|
|
|
|
|
template_id=template_id) not in page
|
2016-03-18 14:43:03 +00:00
|
|
|
|
|
|
|
|
|
|
|
2017-02-03 10:42:01 +00:00
|
|
|
|
def test_route_choose_template_manage_api_keys_permissions(
|
|
|
|
|
|
mocker,
|
|
|
|
|
|
app_,
|
2017-02-03 12:07:21 +00:00
|
|
|
|
client,
|
2017-02-03 10:42:01 +00:00
|
|
|
|
api_user_active,
|
|
|
|
|
|
service_one,
|
|
|
|
|
|
mock_get_user,
|
|
|
|
|
|
mock_get_service,
|
|
|
|
|
|
mock_check_verify_code,
|
|
|
|
|
|
mock_get_service_templates,
|
|
|
|
|
|
mock_get_jobs,
|
|
|
|
|
|
):
|
2017-02-03 12:07:21 +00:00
|
|
|
|
template_id = None
|
|
|
|
|
|
for temp in mock_get_service_templates(service_one['id'])['data']:
|
|
|
|
|
|
if temp['template_type'] == 'sms':
|
|
|
|
|
|
template_id = temp['id']
|
|
|
|
|
|
assert template_id
|
|
|
|
|
|
resp = validate_route_permission(
|
|
|
|
|
|
mocker,
|
|
|
|
|
|
app_,
|
|
|
|
|
|
"GET",
|
|
|
|
|
|
200,
|
|
|
|
|
|
url_for(
|
|
|
|
|
|
'main.choose_template',
|
2016-03-18 14:43:03 +00:00
|
|
|
|
service_id=service_one['id'],
|
2017-02-03 12:07:21 +00:00
|
|
|
|
template_type='sms'),
|
|
|
|
|
|
['manage_api_keys'],
|
|
|
|
|
|
api_user_active,
|
|
|
|
|
|
service_one)
|
|
|
|
|
|
page = resp.get_data(as_text=True)
|
|
|
|
|
|
assert url_for(
|
|
|
|
|
|
"main.send_test",
|
|
|
|
|
|
service_id=service_one['id'],
|
|
|
|
|
|
template_id=template_id) not in page
|
|
|
|
|
|
assert url_for(
|
|
|
|
|
|
"main.edit_service_template",
|
|
|
|
|
|
service_id=service_one['id'],
|
|
|
|
|
|
template_id=template_id) not in page
|
|
|
|
|
|
page = BeautifulSoup(resp.data.decode('utf-8'), 'html.parser')
|
|
|
|
|
|
links = page.findAll('a', href=re.compile('^' + url_for(
|
|
|
|
|
|
"main.send_from_api",
|
|
|
|
|
|
service_id=service_one['id'],
|
|
|
|
|
|
template_id=template_id)))
|
|
|
|
|
|
assert len(links) == 1
|
2016-06-06 16:38:55 +01:00
|
|
|
|
|
|
|
|
|
|
|
2016-06-24 10:15:20 +01:00
|
|
|
|
@pytest.mark.parametrize(
|
|
|
|
|
|
'extra_args,expected_url',
|
|
|
|
|
|
[
|
|
|
|
|
|
(
|
|
|
|
|
|
dict(),
|
|
|
|
|
|
partial(url_for, '.send_test')
|
|
|
|
|
|
),
|
|
|
|
|
|
(
|
|
|
|
|
|
dict(help='0'),
|
|
|
|
|
|
partial(url_for, '.send_test')
|
|
|
|
|
|
),
|
|
|
|
|
|
(
|
|
|
|
|
|
dict(help='2'),
|
|
|
|
|
|
partial(url_for, '.send_test', help='1')
|
|
|
|
|
|
)
|
|
|
|
|
|
]
|
|
|
|
|
|
)
|
2016-06-24 10:25:35 +01:00
|
|
|
|
def test_check_messages_back_link(
|
2017-02-03 12:07:21 +00:00
|
|
|
|
logged_in_client,
|
2016-06-24 10:25:35 +01:00
|
|
|
|
api_user_active,
|
|
|
|
|
|
mock_login,
|
|
|
|
|
|
mock_get_user_by_email,
|
|
|
|
|
|
mock_get_users_by_service,
|
|
|
|
|
|
mock_get_service,
|
|
|
|
|
|
mock_get_service_template_with_placeholders,
|
|
|
|
|
|
mock_has_permissions,
|
2016-07-25 14:16:34 +01:00
|
|
|
|
mock_get_detailed_service_for_today,
|
2016-06-24 10:25:35 +01:00
|
|
|
|
mock_s3_download,
|
|
|
|
|
|
fake_uuid,
|
|
|
|
|
|
extra_args,
|
|
|
|
|
|
expected_url
|
|
|
|
|
|
):
|
2017-02-03 12:07:21 +00:00
|
|
|
|
with logged_in_client.session_transaction() as session:
|
|
|
|
|
|
session['upload_data'] = {'original_file_name': 'valid.csv',
|
|
|
|
|
|
'template_id': fake_uuid,
|
|
|
|
|
|
'notification_count': 1,
|
|
|
|
|
|
'valid': True}
|
|
|
|
|
|
response = logged_in_client.get(url_for(
|
|
|
|
|
|
'main.check_messages',
|
|
|
|
|
|
service_id=fake_uuid,
|
|
|
|
|
|
upload_id=fake_uuid,
|
|
|
|
|
|
template_type='sms',
|
|
|
|
|
|
from_test=True,
|
|
|
|
|
|
**extra_args
|
|
|
|
|
|
))
|
|
|
|
|
|
assert response.status_code == 200
|
|
|
|
|
|
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
|
|
|
|
|
|
assert (
|
|
|
|
|
|
page.findAll('a', {'class': 'page-footer-back-link'})[0]['href']
|
|
|
|
|
|
) == expected_url(service_id=fake_uuid, template_id=fake_uuid)
|
2016-06-17 11:54:01 +01:00
|
|
|
|
|
|
|
|
|
|
|
2016-07-01 10:49:54 +01:00
|
|
|
|
def test_go_to_dashboard_after_tour(
|
2017-02-03 12:07:21 +00:00
|
|
|
|
logged_in_client,
|
2016-07-01 10:49:54 +01:00
|
|
|
|
mocker,
|
|
|
|
|
|
api_user_active,
|
|
|
|
|
|
mock_login,
|
|
|
|
|
|
mock_get_service,
|
|
|
|
|
|
mock_has_permissions,
|
|
|
|
|
|
mock_delete_service_template,
|
|
|
|
|
|
fake_uuid
|
|
|
|
|
|
):
|
|
|
|
|
|
|
2017-02-03 12:07:21 +00:00
|
|
|
|
resp = logged_in_client.get(
|
|
|
|
|
|
url_for('main.go_to_dashboard_after_tour', service_id=fake_uuid, example_template_id=fake_uuid)
|
|
|
|
|
|
)
|
2016-07-01 10:49:54 +01:00
|
|
|
|
|
2017-02-03 12:07:21 +00:00
|
|
|
|
assert resp.status_code == 302
|
|
|
|
|
|
assert resp.location == url_for("main.service_dashboard", service_id=fake_uuid, _external=True)
|
|
|
|
|
|
mock_delete_service_template.assert_called_once_with(fake_uuid, fake_uuid)
|
2016-07-21 15:39:00 +01:00
|
|
|
|
|
|
|
|
|
|
|
2016-08-10 15:55:13 +01:00
|
|
|
|
@pytest.mark.parametrize('num_requested,expected_msg', [
|
2016-07-21 15:39:00 +01:00
|
|
|
|
(0, '‘valid.csv’ contains 100 phone numbers.'),
|
|
|
|
|
|
(1, 'You can still send 49 messages today, but ‘valid.csv’ contains 100 phone numbers.')
|
|
|
|
|
|
], ids=['none_sent', 'some_sent'])
|
|
|
|
|
|
def test_check_messages_shows_too_many_messages_errors(
|
|
|
|
|
|
mocker,
|
2017-02-03 12:07:21 +00:00
|
|
|
|
logged_in_client,
|
2016-07-21 15:39:00 +01:00
|
|
|
|
api_user_active,
|
|
|
|
|
|
mock_login,
|
|
|
|
|
|
mock_get_users_by_service,
|
|
|
|
|
|
mock_get_service,
|
|
|
|
|
|
mock_get_service_template,
|
|
|
|
|
|
mock_has_permissions,
|
|
|
|
|
|
fake_uuid,
|
2016-07-25 14:16:34 +01:00
|
|
|
|
num_requested,
|
2016-08-10 15:55:13 +01:00
|
|
|
|
expected_msg
|
2016-07-21 15:39:00 +01:00
|
|
|
|
):
|
|
|
|
|
|
# csv with 100 phone numbers
|
|
|
|
|
|
mocker.patch('app.main.views.send.s3download', return_value=',\n'.join(
|
|
|
|
|
|
['phone number'] + ([mock_get_users_by_service(None)[0]._mobile_number]*100)
|
|
|
|
|
|
))
|
2016-07-25 14:16:34 +01:00
|
|
|
|
mocker.patch('app.service_api_client.get_detailed_service_for_today', return_value={
|
|
|
|
|
|
'data': {
|
|
|
|
|
|
'statistics': {
|
|
|
|
|
|
'sms': {'requested': num_requested, 'delivered': 0, 'failed': 0},
|
|
|
|
|
|
'email': {'requested': 0, 'delivered': 0, 'failed': 0}
|
|
|
|
|
|
}
|
|
|
|
|
|
}
|
2016-07-21 15:39:00 +01:00
|
|
|
|
})
|
|
|
|
|
|
|
2017-02-03 12:07:21 +00:00
|
|
|
|
with logged_in_client.session_transaction() as session:
|
|
|
|
|
|
session['upload_data'] = {'original_file_name': 'valid.csv',
|
|
|
|
|
|
'template_id': fake_uuid,
|
|
|
|
|
|
'notification_count': 1,
|
|
|
|
|
|
'valid': True}
|
|
|
|
|
|
response = logged_in_client.get(url_for(
|
|
|
|
|
|
'main.check_messages',
|
|
|
|
|
|
service_id=fake_uuid,
|
|
|
|
|
|
template_type='sms',
|
|
|
|
|
|
upload_id=fake_uuid
|
|
|
|
|
|
))
|
2016-07-21 15:39:00 +01:00
|
|
|
|
assert response.status_code == 200
|
|
|
|
|
|
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
|
|
|
|
|
|
assert page.find('h1').text.strip() == 'Too many recipients'
|
|
|
|
|
|
assert page.find('div', class_='banner-dangerous').find('a').text.strip() == 'trial mode'
|
|
|
|
|
|
|
|
|
|
|
|
# remove excess whitespace from element
|
|
|
|
|
|
details = page.find('div', class_='banner-dangerous').findAll('p')[1]
|
|
|
|
|
|
details = ' '.join([line.strip() for line in details.text.split('\n') if line.strip() != ''])
|
2016-08-10 15:55:13 +01:00
|
|
|
|
assert details == expected_msg
|
2016-10-14 10:44:28 +01:00
|
|
|
|
|
|
|
|
|
|
|
2017-01-23 17:05:41 +00:00
|
|
|
|
def test_check_messages_shows_trial_mode_error(
|
|
|
|
|
|
logged_in_client,
|
|
|
|
|
|
mock_get_users_by_service,
|
|
|
|
|
|
mock_get_service,
|
|
|
|
|
|
mock_get_service_template,
|
|
|
|
|
|
mock_has_permissions,
|
|
|
|
|
|
mock_get_detailed_service_for_today,
|
|
|
|
|
|
mocker
|
|
|
|
|
|
):
|
|
|
|
|
|
mocker.patch('app.main.views.send.s3download', return_value=(
|
|
|
|
|
|
'phone number,\n07900900321' # Not in team
|
|
|
|
|
|
))
|
|
|
|
|
|
with logged_in_client.session_transaction() as session:
|
|
|
|
|
|
session['upload_data'] = {'template_id': ''}
|
|
|
|
|
|
response = logged_in_client.get(url_for(
|
|
|
|
|
|
'main.check_messages',
|
|
|
|
|
|
service_id=uuid.uuid4(),
|
|
|
|
|
|
template_type='sms',
|
|
|
|
|
|
upload_id=uuid.uuid4()
|
|
|
|
|
|
))
|
|
|
|
|
|
assert response.status_code == 200
|
|
|
|
|
|
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
|
|
|
|
|
|
assert ' '.join(
|
|
|
|
|
|
page.find('div', class_='banner-dangerous').text.split()
|
|
|
|
|
|
) == (
|
|
|
|
|
|
'You can’t send to this phone number '
|
2017-02-13 15:20:56 +00:00
|
|
|
|
'In trial mode you can only send to yourself and members of your team '
|
|
|
|
|
|
'Skip to file contents'
|
2017-01-23 17:05:41 +00:00
|
|
|
|
)
|
|
|
|
|
|
|
|
|
|
|
|
|
2016-10-14 10:44:28 +01:00
|
|
|
|
def test_check_messages_shows_over_max_row_error(
|
2017-02-03 12:07:21 +00:00
|
|
|
|
logged_in_client,
|
2016-10-14 10:44:28 +01:00
|
|
|
|
api_user_active,
|
|
|
|
|
|
mock_login,
|
|
|
|
|
|
mock_get_users_by_service,
|
|
|
|
|
|
mock_get_service,
|
|
|
|
|
|
mock_get_service_template_with_placeholders,
|
|
|
|
|
|
mock_has_permissions,
|
|
|
|
|
|
mock_get_detailed_service_for_today,
|
|
|
|
|
|
mock_s3_download,
|
|
|
|
|
|
fake_uuid,
|
|
|
|
|
|
mocker
|
|
|
|
|
|
):
|
|
|
|
|
|
mock_recipients = mocker.patch('app.main.views.send.RecipientCSV').return_value
|
|
|
|
|
|
mock_recipients.max_rows = 11111
|
|
|
|
|
|
mock_recipients.__len__.return_value = 99999
|
2016-10-17 10:20:14 +01:00
|
|
|
|
mock_recipients.too_many_rows.return_value = True
|
2016-10-14 10:44:28 +01:00
|
|
|
|
|
2017-02-03 12:07:21 +00:00
|
|
|
|
with logged_in_client.session_transaction() as session:
|
2016-10-14 10:44:28 +01:00
|
|
|
|
session['upload_data'] = {'template_id': fake_uuid}
|
2017-02-03 12:07:21 +00:00
|
|
|
|
response = logged_in_client.get(url_for(
|
2016-10-14 10:44:28 +01:00
|
|
|
|
'main.check_messages',
|
|
|
|
|
|
service_id=fake_uuid,
|
|
|
|
|
|
template_type='sms',
|
|
|
|
|
|
upload_id=fake_uuid
|
|
|
|
|
|
))
|
|
|
|
|
|
assert response.status_code == 200
|
|
|
|
|
|
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
|
|
|
|
|
|
assert ' '.join(
|
|
|
|
|
|
page.find('div', class_='banner-dangerous').text.split()
|
|
|
|
|
|
) == (
|
|
|
|
|
|
'Your file has too many rows '
|
|
|
|
|
|
'Notify can process up to 11,111 rows at once. '
|
2017-02-13 15:20:56 +00:00
|
|
|
|
'Your file has 99,999 rows. '
|
|
|
|
|
|
'Skip to file contents'
|
2016-10-14 10:44:28 +01:00
|
|
|
|
)
|
2017-01-19 14:55:22 +00:00
|
|
|
|
|
|
|
|
|
|
|
2017-02-14 14:06:02 +00:00
|
|
|
|
def test_check_messages_redirects_if_no_upload_data(logged_in_client, service_one, mocker):
|
2017-01-19 18:15:33 +00:00
|
|
|
|
checker = mocker.patch('app.main.views.send.get_check_messages_back_url', return_value='foo')
|
2017-01-19 14:55:22 +00:00
|
|
|
|
response = logged_in_client.get(url_for(
|
|
|
|
|
|
'main.check_messages',
|
2017-02-14 14:06:02 +00:00
|
|
|
|
service_id=service_one['id'],
|
2017-01-19 18:15:33 +00:00
|
|
|
|
template_type='bar',
|
|
|
|
|
|
upload_id='baz'
|
2017-01-19 14:55:22 +00:00
|
|
|
|
))
|
|
|
|
|
|
|
2017-02-14 14:06:02 +00:00
|
|
|
|
checker.assert_called_once_with(service_one['id'], 'bar')
|
2017-01-19 14:55:22 +00:00
|
|
|
|
assert response.status_code == 301
|
2017-01-19 18:15:33 +00:00
|
|
|
|
assert response.location == 'http://localhost/foo'
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
@pytest.mark.parametrize('template_type', ['sms', 'email'])
|
|
|
|
|
|
def test_get_check_messages_back_url_returns_to_correct_select_template(client, mocker, template_type):
|
|
|
|
|
|
mocker.patch('app.main.views.send.get_help_argument', return_value=False)
|
|
|
|
|
|
|
|
|
|
|
|
assert get_check_messages_back_url('1234', template_type) == url_for(
|
2017-01-19 14:55:22 +00:00
|
|
|
|
'main.choose_template',
|
Merge email, text message + letter templates pages
Right now we have separate pages for email and text message templates.
In the future we will also have a separate page for letter templates.
This commit changes Notify to only have one page for all templates.
What is the problem?
---
The left-hand navigation is getting quite crowded, at 8 items for a
service that can send letters. Research suggests that the number of
objects an average human can hold in working memory is 7 ± 2 [1]. So
we’re at the limit of how many items the navigation should have.
In the future we will need to search/sort/filter templates by attributes
other than type, for example:
- show me the ‘confirmation’ templates
- show me the most recently used templates
- show me all templates containing the placeholder `((ref_no))`
These are hypothetical for now, but these needs (or others) may become
real in the future. At this point pre-filtering the list of templates
by type would restrict what searches a user could do. So by making this
change now we’re in a better position to iterate the design in the
future.
What’s the change?
---
This commit replaces the ‘Email templates’, ‘Text message templates’ and
‘Letter templates’ pages with one page called ‘Templates’.
This new templates page shows all the templates for the service, sorted
by most recently created first (as before).
To add a new template there is a new page with a form asking you what
kind of template you want to create. This is necessary because in the
past we knew what kind of template you wanted to create based on the
kind you were looking at.
What’s the impact of this change on new users?
---
This change alters the onboarding process slightly. We still want to
take people through the empty templates page from the call-to-action on
the dashboard because it helps them understand that to send a message
using Notify you need a template. But because we don’t have separate
pages for emails/text messages we will have to send users through the
extra step of choosing what kind of template to create. This is a bit
clunkier on first use but:
- it still gets the point across
- it takes them through the actual flow they will be using to create new
templates in the future (ie they’re learning how to use Notify, not
just being taken through a special onboarding route)
I’m not too worried about this change in terms of the experience for new
users. Furthermore, by making it now we get to validate whether it’s
causing any problems in the lab research booked for next week.
What’s the impact of this change on current services?
---
Looking at the top 15 services by number of templates[2], most are using
either text messages or emails. So this change would not have a
significant impact on these services because the page will not get any
longer. In other words we wouldn’t be making it worse for them.
Those services who do use both are not using as many templates. The
worst-case scenario is SSCS, who have 16 templates, evenly split between
email and text messages. So they would go from having 8 templates per
page to 16, which is still less than half the number that HMPO or
Digital Marketplace are managing.
References
---
1. https://en.wikipedia.org/wiki/The_Magical_Number_Seven,_Plus_or_Minus_Two
2. Template usage by service
Service name | Template count | Template types
---------------------------------------|----------------|---------------
Her Majesty's Passport Office | 40 | sms
Digital Marketplace | 40 | email
GovWifi-Staging | 19 | sms
GovWifi | 18 | sms
Digital Apprenticeship Service | 16 | email
SSCS | 16 | both
Crown Commercial Service MI Collection | 15 | email
Help with Prison Visits | 12 | both
Digital Future | 12 | email
Export Licensing Service | 11 | email
Civil Money Claims | 9 | both
DVLA Drivers Medical Service | 9 | sms
GOV.UK Notify | 8 | both
Manage your benefit overpayments | 8 | both
Tax Renewals | 8 | both
2017-02-28 12:16:35 +00:00
|
|
|
|
service_id='1234'
|
2017-01-19 18:15:33 +00:00
|
|
|
|
)
|
|
|
|
|
|
|
|
|
|
|
|
|
2017-02-14 14:06:02 +00:00
|
|
|
|
def test_check_messages_back_from_help_goes_to_start_of_help(client, service_one, mocker):
|
2017-01-19 18:15:33 +00:00
|
|
|
|
mocker.patch('app.main.views.send.get_help_argument', return_value=True)
|
|
|
|
|
|
mocker.patch('app.service_api_client.get_service_templates', lambda service_id: {
|
2017-02-14 14:06:02 +00:00
|
|
|
|
'data': [template_json(service_one['id'], '111', type_='sms')]
|
2017-01-19 18:15:33 +00:00
|
|
|
|
})
|
2017-02-14 14:06:02 +00:00
|
|
|
|
assert get_check_messages_back_url(service_one['id'], 'sms') == url_for(
|
2017-01-19 18:15:33 +00:00
|
|
|
|
'main.send_test',
|
2017-02-14 14:06:02 +00:00
|
|
|
|
service_id=service_one['id'],
|
2017-01-19 18:15:33 +00:00
|
|
|
|
template_id='111',
|
|
|
|
|
|
help='1'
|
|
|
|
|
|
)
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
@pytest.mark.parametrize('templates', [
|
|
|
|
|
|
[],
|
|
|
|
|
|
[
|
|
|
|
|
|
template_json('000', '111', type_='sms'),
|
|
|
|
|
|
template_json('000', '222', type_='sms')
|
|
|
|
|
|
]
|
|
|
|
|
|
], ids=['no_templates', 'two_templates'])
|
|
|
|
|
|
def test_check_messages_back_from_help_handles_unexpected_templates(client, mocker, templates):
|
|
|
|
|
|
mocker.patch('app.main.views.send.get_help_argument', return_value=True)
|
|
|
|
|
|
mocker.patch('app.service_api_client.get_service_templates', lambda service_id: {
|
|
|
|
|
|
'data': templates
|
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
|
|
assert get_check_messages_back_url('1234', 'sms') == url_for(
|
|
|
|
|
|
'main.choose_template',
|
|
|
|
|
|
service_id='1234',
|
2017-01-19 14:55:22 +00:00
|
|
|
|
)
|