Validate CSVs fully

This commit extends the existing function to validate each row’s phone number
to also validate that all the required data is present.

It does this using the checking that the `Template` class can do when given
a template and a `dict` of values.
This commit is contained in:
Chris Hill-Scott
2016-02-17 15:49:07 +00:00
parent efb2140bbb
commit 45cacd82d3
11 changed files with 98 additions and 113 deletions

View File

@@ -19,6 +19,7 @@ from app.notify_client.job_api_client import JobApiClient
from app.notify_client.status_api_client import StatusApiClient
from app.its_dangerous_session import ItsdangerousSessionInterface
from app.asset_fingerprinter import AssetFingerprinter
from app.utils import validate_phone_number, InvalidPhoneError
import app.proxy_fix
from config import configs
from utils import logging
@@ -65,6 +66,7 @@ def create_app(config_name, config_overrides=None):
application.add_template_filter(nl2br)
application.add_template_filter(format_datetime)
application.add_template_filter(syntax_highlight_json)
application.add_template_filter(valid_phone_number)
application.after_request(useful_headers_after_request)
register_errorhandlers(application)
@@ -139,6 +141,14 @@ def format_datetime(date):
return native.strftime('%A %d %B %Y at %H:%M')
def valid_phone_number(phone_number):
try:
validate_phone_number(phone_number)
return True
except InvalidPhoneError:
return False
# https://www.owasp.org/index.php/List_of_useful_HTTP_headers
def useful_headers_after_request(response):
response.headers.add('X-Frame-Options', 'deny')

View File

@@ -1,6 +1,6 @@
.page-footer {
margin-bottom: 50px;
margin-bottom: 30px;
&-back-link {
@include button($grey-1);

View File

@@ -1,6 +1,6 @@
from flask import url_for
from app import notifications_api_client
from app.main.utils import BrowsableItem
from app.utils import BrowsableItem
def insert_new_service(service_name, user_id):

View File

@@ -1,6 +1,6 @@
from flask import url_for, abort
from app import notifications_api_client
from app.main.utils import BrowsableItem
from app.utils import BrowsableItem
def insert_service_template(name, type_, content, service_id):

View File

@@ -14,7 +14,7 @@ from wtforms.validators import DataRequired, Email, Length, Regexp
from app.main.validators import Blacklist, CsvFileValidator
from app.main.utils import (
from app.utils import (
validate_phone_number,
format_phone_number,
InvalidPhoneError

View File

@@ -20,7 +20,7 @@ from flask import (
from flask_login import login_required, current_user
from werkzeug import secure_filename
from notifications_python_client.errors import HTTPError
from utils.template import Template
from utils.template import Template, NeededByTemplateError, NoPlaceholderForDataError
from app.main import main
from app.main.forms import CsvUploadForm
@@ -30,7 +30,7 @@ from app.main.uploader import (
)
from app.main.dao import templates_dao
from app import job_api_client
from app.main.utils import (
from app.utils import (
validate_phone_number,
InvalidPhoneError
)
@@ -68,7 +68,7 @@ def send_sms(service_id, template_id):
return redirect(url_for('.send_sms', service_id=service_id, template_id=template_id))
template = Template(
templates_dao.get_service_template_or_404(service_id, template_id)['data']
templates_dao.get_service_template_or_404(service_id, template_id)['data']
)
example_data = [dict(
@@ -111,19 +111,22 @@ def check_sms(service_id, upload_id):
contents = s3download(service_id, upload_id)
if not contents:
flash('There was a problem reading your upload file')
upload_result = _get_numbers(contents)
upload_data = session['upload_data']
template_id = upload_data.get('template_id')
raw_template = templates_dao.get_service_template_or_404(service_id, template_id)['data']
upload_result = _get_rows(contents, raw_template)
template = Template(
templates_dao.get_service_template_or_404(service_id, template_id)['data'],
values=upload_result['valid'][0] if upload_result['valid'] else {},
raw_template,
values=upload_result['rows'][0] if upload_result['valid'] else {},
drop_values={'phone'}
)
return render_template(
'views/check-sms.html',
upload_result=upload_result,
template=template,
column_headers=['phone number'] + template.placeholders_as_markup,
column_headers=['phone number'] + list(
template.placeholders if upload_result['valid'] else template.placeholders_as_markup
),
original_file_name=upload_data.get('original_file_name'),
service_id=service_id,
form=CsvUploadForm()
@@ -155,36 +158,19 @@ def _get_filedata(file):
return {'file_name': file.filename, 'data': lines}
def _format_filename(filename):
d = date.today()
basename, extenstion = filename.split('.')
formatted_name = '{}_{}.csv'.format(basename, d.strftime('%Y%m%d'))
return secure_filename(formatted_name)
def _get_numbers(contents):
def _get_rows(contents, raw_template):
reader = csv.DictReader(
contents.split('\n'),
lineterminator='\n',
quoting=csv.QUOTE_NONE)
valid, rejects = [], []
for i, row in enumerate(reader):
quoting=csv.QUOTE_NONE
)
valid = True
rows = []
for row in reader:
rows.append(row)
try:
validate_phone_number(row['phone'])
valid.append(row)
except InvalidPhoneError:
rejects.append({"line_number": i+2, "phone": row['phone']})
return {"valid": valid, "rejects": rejects}
def _get_column_headers(template_content, marked_up=False):
headers = re.findall(
r"\(\(([^\)]+)\)\)", # anything that looks like ((registration number))
template_content
)
if marked_up:
return [
Markup("<span class='placeholder'>{}</span>".format(header))
for header in headers
]
return headers
Template(raw_template, values=row, drop_values={'phone'}).replaced
except (InvalidPhoneError, NeededByTemplateError, NoPlaceholderForDataError):
valid = False
return {"valid": valid, "rows": rows}

View File

@@ -11,74 +11,73 @@
{% block maincolumn_content %}
<h1 class="heading-large">Check and confirm</h1>
{% if upload_result.rejects %}
<h3 class="heading-small">The following numbers are invalid</h3>
{% for rejected in upload_result.rejects %}
<p>Line {{rejected.line_number}}: {{rejected.phone }}</a>
{% endfor %}
<p><a href="{{url_for('.send_sms', service_id=service_id, template_id=template.id)}}" class="button">Go back and resolve errors</a></p>
{% if template.additional_data %}
{{ banner(
"Remove these columns from your CSV file:" + ", ".join(template.missing_data),
type="dangerous"
) }}
{% elif not upload_result.valid %}
{{ banner(
"Your CSV file contained missing or invalid data",
type="dangerous"
) }}
{% endif %}
{% else %}
<h1 class="heading-large">
{{ "Check and confirm" if upload_result.valid else "Send text messages" }}
</h1>
<div class="grid-row">
<div class="column-two-thirds">
{% if template.missing_data or template.additional_data %}
{{ sms_message(template.formatted_as_markup)}}
{% else %}
{{ sms_message(template.replaced)}}
{% endif %}
</div>
<div class="grid-row">
<div class="column-two-thirds">
{% if template.missing_data or template.additional_data %}
{{ sms_message(template.formatted_as_markup)}}
{% else %}
{{ sms_message(template.replaced)}}
{% endif %}
</div>
</div>
{% if template.missing_data %}
{{ banner(
"Add these columns to your CSV file: " + ", ".join(template.missing_data),
type="dangerous"
{% if upload_result.valid %}
<form method="post" enctype="multipart/form-data">
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
<input type="submit" class="button" value="{{ "Send {} text message{}".format(upload_result.rows|count, '' if upload_result.rows|count == 1 else 's') }}" />
<a href="{{url_for('.send_sms', service_id=service_id, template_id=template.id)}}" class="page-footer-back-link">Back</a>
</form>
{% else %}
<form method="post" action="{{ url_for('.send_sms', service_id=service_id, template_id=template.id) }}" enctype="multipart/form-data">
{{file_upload(form.file, button_text='Choose a CSV file')}}
{{ page_footer(
"Upload"
) }}
{% elif template.additional_data %}
{{ banner(
"Remove these columns from your CSV file:" + ", ".join(template.missing_data),
type="dangerous"
) }}
{% else %}
<form method="POST" enctype="multipart/form-data">
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
<input type="submit" class="button" value="{{ "Send {} text message{}".format(upload_result.valid|count, '' if upload_result.valid|count == 1 else 's') }}" />
<a href="{{url_for('.send_sms', service_id=service_id, template_id=template.id)}}" class="page-footer-back-link">Back</a>
</form>
{% endif %}
</form>
{% endif %}
{% if template.missing_data or template.additional_data %}
<form method="post" action="{{ url_for('.send_sms', service_id=service_id, template_id=template.id) }}" enctype="multipart/form-data">
{{file_upload(form.file, button_text='Choose a CSV file')}}
{{ page_footer(
"Upload"
) }}
</form>
{% endif %}
{% call(item) list_table(
upload_result.valid,
caption=original_file_name,
field_headings=column_headers
) %}
{% call(item) list_table(
upload_result.rows,
caption=original_file_name,
field_headings=column_headers
) %}
{% if item.phone|valid_phone_number %}
{% call field() %}
{{ item.phone }}
{% endcall %}
{% for column in template.placeholders %}
{% if item.get(column) %}
{% call field() %}
{{ item.get(column) }}
{% endcall %}
{% else %}
{% call field(status='missing') %}
missing
{% endcall %}
{% endif %}
{% endfor %}
{% endcall %}
{% else %}
{% call field(status='missing') %}
{{ item.phone }}
{% endcall %}
{% endif %}
{% for column in template.placeholders %}
{% if item.get(column) %}
{% call field() %}
{{ item.get(column) }}
{% endcall %}
{% else %}
{% call field(status='missing') %}
missing
{% endcall %}
{% endif %}
{% endfor %}
{% endcall %}
{% endif %}
{% endblock %}

View File

@@ -22,13 +22,6 @@
{{file_upload(form.file, button_text='Choose a CSV file')}}
{{ banner(
'You can only send messages to yourself until you <a href="{}">request to go live</a>'.format(
url_for('.service_request_to_go_live', service_id=service_id)
)|safe,
type='important'
) }}
{{ page_footer(
"Continue to preview"
) }}
@@ -36,10 +29,10 @@
{% if column_headers %}
{% call(item) list_table(
example_data,
caption='Preview',
caption='Example',
field_headings=column_headers,
field_headings_visible=True,
caption_visible=False,
caption_visible=True,
empty_message="Your data here"
) %}
{% call field() %}

View File

@@ -1,6 +1,3 @@
from flask import current_app
class BrowsableItem(object):
"""
Maps for the template browse-list.

View File

@@ -1,4 +1,4 @@
from app.main.utils import (
from app.utils import (
validate_phone_number,
InvalidPhoneError
)

View File

@@ -63,10 +63,10 @@ def test_upload_csvfile_with_invalid_phone_shows_check_page_with_errors(app_,
follow_redirects=True)
assert response.status_code == 200
content = response.get_data(as_text=True)
assert 'The following numbers are invalid' in content
assert 'Your CSV file contained missing or invalid data' in content
assert '+44 123' in content
assert '+44 456' in content
assert 'Go back and resolve errors' in content
assert 'Choose a CSV file' in content
@moto.mock_s3