mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-08-25 16:54:03 -04:00
Don’t duplicate columns in example spreadsheet
If a template has a placeholder like `((email address))` then the sample spreadsheet and CSV file have the email column twice. Trying to upload this spreadsheet will result in a ‘duplicate column’ error. This commit fixes it so that the column will only appear once.
This commit is contained in:
@@ -15,6 +15,7 @@ from flask import (
|
|||||||
)
|
)
|
||||||
from flask_login import current_user, login_required
|
from flask_login import current_user, login_required
|
||||||
from notifications_python_client.errors import HTTPError
|
from notifications_python_client.errors import HTTPError
|
||||||
|
from notifications_utils.columns import Columns
|
||||||
from notifications_utils.recipients import (
|
from notifications_utils.recipients import (
|
||||||
RecipientCSV,
|
RecipientCSV,
|
||||||
first_column_headings,
|
first_column_headings,
|
||||||
@@ -79,7 +80,16 @@ def get_example_csv_rows(template, use_example_as_example=True, submitted_fields
|
|||||||
)
|
)
|
||||||
for key in first_column_headings['letter']
|
for key in first_column_headings['letter']
|
||||||
]
|
]
|
||||||
}[template.template_type] + get_example_csv_fields(template.placeholders, use_example_as_example, submitted_fields)
|
}[template.template_type] + get_example_csv_fields(
|
||||||
|
(
|
||||||
|
placeholder for placeholder in template.placeholders
|
||||||
|
if placeholder not in Columns.from_keys(
|
||||||
|
first_column_headings[template.template_type]
|
||||||
|
)
|
||||||
|
),
|
||||||
|
use_example_as_example,
|
||||||
|
submitted_fields
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def get_example_letter_address(key):
|
def get_example_letter_address(key):
|
||||||
@@ -155,7 +165,7 @@ def send_messages(service_id, template_id):
|
|||||||
form.file.data.filename
|
form.file.data.filename
|
||||||
))
|
))
|
||||||
|
|
||||||
column_headings = first_column_headings[template.template_type] + list(template.placeholders)
|
column_headings = get_spreadsheet_column_headings_from_template(template)
|
||||||
|
|
||||||
return render_template(
|
return render_template(
|
||||||
'views/send.html',
|
'views/send.html',
|
||||||
@@ -174,7 +184,7 @@ def get_example_csv(service_id, template_id):
|
|||||||
service_api_client.get_service_template(service_id, template_id)['data'], current_service
|
service_api_client.get_service_template(service_id, template_id)['data'], current_service
|
||||||
)
|
)
|
||||||
return Spreadsheet.from_rows([
|
return Spreadsheet.from_rows([
|
||||||
first_column_headings[template.template_type] + list(template.placeholders),
|
get_spreadsheet_column_headings_from_template(template),
|
||||||
get_example_csv_rows(template)
|
get_example_csv_rows(template)
|
||||||
]).as_csv_data, 200, {
|
]).as_csv_data, 200, {
|
||||||
'Content-Type': 'text/csv; charset=utf-8',
|
'Content-Type': 'text/csv; charset=utf-8',
|
||||||
@@ -883,3 +893,15 @@ def get_sms_sender_from_session(service_id):
|
|||||||
return service_api_client.get_sms_sender(
|
return service_api_client.get_sms_sender(
|
||||||
service_id=service_id, sms_sender_id=session['sender_id']
|
service_id=service_id, sms_sender_id=session['sender_id']
|
||||||
)['sms_sender']
|
)['sms_sender']
|
||||||
|
|
||||||
|
|
||||||
|
def get_spreadsheet_column_headings_from_template(template):
|
||||||
|
column_headings = []
|
||||||
|
|
||||||
|
for column_heading in (
|
||||||
|
first_column_headings[template.template_type] + list(template.placeholders)
|
||||||
|
):
|
||||||
|
if column_heading not in Columns.from_keys(column_headings):
|
||||||
|
column_headings.append(column_heading)
|
||||||
|
|
||||||
|
return column_headings
|
||||||
|
|||||||
@@ -18,4 +18,4 @@ notifications-python-client==4.8.2
|
|||||||
# PaaS
|
# PaaS
|
||||||
awscli-cwlogs>=1.4,<1.5
|
awscli-cwlogs>=1.4,<1.5
|
||||||
|
|
||||||
git+https://github.com/alphagov/notifications-utils.git@29.0.0#egg=notifications-utils==29.0.0
|
git+https://github.com/alphagov/notifications-utils.git@29.1.0#egg=notifications-utils==29.1.0
|
||||||
|
|||||||
@@ -262,6 +262,25 @@ def test_should_not_allow_files_to_be_uploaded_without_the_correct_permission(
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_example_spreadsheet(
|
||||||
|
client_request,
|
||||||
|
mock_get_service_template_with_placeholders_same_as_recipient,
|
||||||
|
fake_uuid,
|
||||||
|
):
|
||||||
|
|
||||||
|
page = client_request.get(
|
||||||
|
'.send_messages',
|
||||||
|
service_id=SERVICE_ONE_ID,
|
||||||
|
template_id=fake_uuid
|
||||||
|
)
|
||||||
|
|
||||||
|
assert normalize_spaces(
|
||||||
|
page.select_one('tbody tr').text
|
||||||
|
) == (
|
||||||
|
'1 phone number name date'
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.parametrize(
|
@pytest.mark.parametrize(
|
||||||
"filename, acceptable_file",
|
"filename, acceptable_file",
|
||||||
list(zip(test_spreadsheet_files, repeat(True))) +
|
list(zip(test_spreadsheet_files, repeat(True))) +
|
||||||
@@ -1541,7 +1560,7 @@ def test_download_example_csv(
|
|||||||
api_user_active,
|
api_user_active,
|
||||||
mock_login,
|
mock_login,
|
||||||
mock_get_service,
|
mock_get_service,
|
||||||
mock_get_service_template,
|
mock_get_service_template_with_placeholders_same_as_recipient,
|
||||||
mock_has_permissions,
|
mock_has_permissions,
|
||||||
fake_uuid
|
fake_uuid
|
||||||
):
|
):
|
||||||
@@ -1551,7 +1570,10 @@ def test_download_example_csv(
|
|||||||
follow_redirects=True
|
follow_redirects=True
|
||||||
)
|
)
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
assert response.get_data(as_text=True) == 'phone number\r\n07700 900321\r\n'
|
assert response.get_data(as_text=True) == (
|
||||||
|
'phone number,name,date\r\n'
|
||||||
|
'07700 900321,example,example\r\n'
|
||||||
|
)
|
||||||
assert 'text/csv' in response.headers['Content-Type']
|
assert 'text/csv' in response.headers['Content-Type']
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -830,6 +830,20 @@ def mock_get_service_template_with_placeholders(mocker):
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture(scope='function')
|
||||||
|
def mock_get_service_template_with_placeholders_same_as_recipient(mocker):
|
||||||
|
def _get(service_id, template_id):
|
||||||
|
template = template_json(
|
||||||
|
service_id, template_id, "Two week reminder", "sms", "((name)) ((date)) ((PHONENUMBER))"
|
||||||
|
)
|
||||||
|
return {'data': template}
|
||||||
|
|
||||||
|
return mocker.patch(
|
||||||
|
'app.service_api_client.get_service_template',
|
||||||
|
side_effect=_get
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture(scope='function')
|
@pytest.fixture(scope='function')
|
||||||
def mock_get_service_email_template(mocker, content=None, subject=None, redact_personalisation=False):
|
def mock_get_service_email_template(mocker, content=None, subject=None, redact_personalisation=False):
|
||||||
def _get(service_id, template_id, version=None):
|
def _get(service_id, template_id, version=None):
|
||||||
|
|||||||
Reference in New Issue
Block a user