darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2026-08-14 23:08:07 -04:00
Code Issues Packages Projects Releases Wiki Activity
Files
46e68b2f37c0d84699e2767feb8fd49b9cc83fd2
notifications-admin/app/main/views/send.py

805 lines
27 KiB
Python
Raw Normal View History

Show error if restricted service uploads real CSV Implements: https://github.com/alphagov/notifications-utils/pull/16 Also changes a bunch of the mocked phone numbers to be real-looking so that the tests are checking for the right kind of error.
2016-04-04 14:28:52 +01:00
import itertools
Make example on send look more like a spreadsheet Now that the example on the breaking changes page looks more like a spreadsheet, we should do the same thing for the downloadable example on the send page.
2016-05-31 14:18:35 +01:00
from string import ascii_uppercase
Handle files that can’t be interpreted as spreadsheets There shouldn’t be a case where we see a `ValueError` on upload any more. Our file handling should be robust enough to deal with whatever is thrown at it. This commit: - adds test files with bad data (PNG files with their extensions changed to look like spreadsheets) - catches whatever exceptions are raised by trying to parse these files - returns a helpful flash message to the user Anything else should raise a `500`, eg if the file can’t be uploaded to S3.
2016-05-13 21:25:59 +01:00
from zipfile import BadZipFile
First slice of csv upload of phone numbers for sending messages. At the moment the file contents are not persisted by checked in memory. The first and last three records are show if all are valid. If there are invalid rows, they are reported and the user is prompted to go back and sort out upload file. The storing of upload result (i.e. validation of file) in session will be removed in next story which is about persisting of file for later processing.
2016-01-11 15:00:51 +00:00
from flask import (
Pass selected template id to api when creating job. Moved query params for original file name and template id to session.
2016-02-01 11:28:36 +00:00
abort,
Add remaining endpoints for PDFs and PNGs Right now we can show what a letter template looks like as a PDF or PNG. This commit completes the work so this is also possible when: - showing a template with the placeholders replaced - showing any version of a template Also removes dependency on `Exception().message`, which was deprecated in Python 2.6. See https://github.com/alphagov/notifications-utils/commit/97f82d565f3112926752ae382e723b0808a3135d for full details.
2016-12-20 14:38:34 +00:00
current_app,
Enforce order and style of imports Done using isort[1], with the following command: ``` isort -rc ./app ./tests ``` Adds linting to the `run_tests.sh` script to stop badly-sorted imports getting re-introduced. Chosen style is ‘Vertical Hanging Indent’ with trailing commas, because I think it gives the cleanest diffs, eg: ``` from third_party import ( lib1, lib2, lib3, lib4, ) ``` 1. https://pypi.python.org/pypi/isort
2018-02-20 11:22:17 +00:00
flash,
redirect,
render_template,
request,
session,
url_for,
First slice of csv upload of phone numbers for sending messages. At the moment the file contents are not persisted by checked in memory. The first and last three records are show if all are valid. If there are invalid rows, they are reported and the user is prompted to go back and sort out upload file. The storing of upload result (i.e. validation of file) in session will be removed in next story which is about persisting of file for later processing.
2016-01-11 15:00:51 +00:00
)
Have permissions decorators check user signed in Rather than force us to write the decorators in a specific order let’s just have one decorator call the other. This should make fewer lines of code, and fewer annoying test failures. It also means that the same way of raising a `401` (through the `current_app` method) is used everywhere.
2019-07-01 15:22:08 +01:00
from flask_login import current_user
display errors on the end page. The following errors may happen: * Number outside of service if service in trial mode * Message too long for sms * Service over daily limit We need to handle these. They only return on send, rather than in a separate validation step (for now).
2017-06-29 15:31:44 +01:00
from notifications_python_client.errors import HTTPError
big commit with letters removal
2022-12-05 15:33:44 -05:00
from notifications_utils import SMS_CHAR_COUNT_LIMIT
Bump utils to 53.0.0 Changes: 53.0.0 --- * `notifications_utils.columns.Columns` has moved to `notifications_utils.insensitive_dict.InsensitiveDict` * `notifications_utils.columns.Rows` has moved to `notifications_utils.recipients.Rows` * `notifications_utils.columns.Cell` has moved to `notifications_utils.recipients.Cell` 52.0.0 --- * Deprecate the following unused `redis_client` functions: - `redis_client.increment_hash_value` - `redis_client.decrement_hash_value` - `redis_client.get_all_from_hash` - `redis_client.set_hash_and_expire` - `redis_client.expire` 51.3.1 --- * Bump govuk-bank-holidays to cache holidays for next year. 51.3.0 --- * Log exception and stacktrace when Celery tasks fail.
2022-02-04 10:43:36 +00:00
from notifications_utils.insensitive_dict import InsensitiveDict
Explain 3 required address columns Our rules about address columns are relaxing, so that none of them are mandatory any more. Instead you just need any 3 of the 7 to make a valid address. This commit updates our error messaging to reflect that.
2020-04-19 22:46:13 +01:00
from notifications_utils.recipients import RecipientCSV, first_column_headings
Sanitise csv filenames before saving to S3 Commit 58cc1604a73be583f9d96513dbe67e77f326e010 sanitises any non-ascii characters in the headers. CSV filenames get used as a header value, so this fixed a bug that occurred when non-ascii characters were used. The CSV filename also gets used as part of the metadata when uploading the file to S3. Since the S3 metadata can only contain ASII characters, we also need to sanitise the filename before uploading it to S3.
2018-07-12 15:14:58 +01:00
from notifications_utils.sanitise_text import SanitiseASCII
Enforce order and style of imports Done using isort[1], with the following command: ``` isort -rc ./app ./tests ``` Adds linting to the `run_tests.sh` script to stop badly-sorted imports getting re-introduced. Chosen style is ‘Vertical Hanging Indent’ with trailing commas, because I think it gives the cleanest diffs, eg: ``` from third_party import ( lib1, lib2, lib3, lib4, ) ``` 1. https://pypi.python.org/pypi/isort
2018-02-20 11:22:17 +00:00
from xlrd.biffh import XLRDError
Catch exceptions caused by ambiguous Excel files Excel stores dates as floating point numbers, counting the days (or fraction thereof) since 1900 (except when it counts from 1904). However it also, incorrectly, recognises 1900 as a leap year. This means that it’s ambiguous whether day 59 is February 28th, or February 27th, depending if you’re counting up or down. In fact any number less than 60 is, therefore, ambiguous. This shouldn’t really matter since no-one is going to be using dates in the year 1900 in Notify messages. _Except_ when Excel misidentifies a cell as containing a date. For example, if you have the number `9` inside a cell, it means _house number 9_ if the next cell contains _example_ street. but Excel is all like ‘oh they must want January 9th 1900!’ No. Bad Excel. There’s not much we can do about this, but we can at least give people an error message with the workaround, which is what this commit does. Most of this commit message is paraphrased from: http://xlrd.readthedocs.io/en/latest/dates.html
2018-02-28 14:45:57 +00:00
from xlrd.xldate import XLDateError
Use Flask for routing This commit: - replaces links that look like buttons with forms and submit buttons - splits the view code for SMS into its own file - moves the routing into the Python by adding handling for `post` requests - uses Flask’s `url_for` to generate URLs, rather than hard coding them (so that it’s easier to change the URLs) - chages the URLs for sending text messages
2015-12-10 21:15:20 +00:00
Enforce order and style of imports Done using isort[1], with the following command: ``` isort -rc ./app ./tests ``` Adds linting to the `run_tests.sh` script to stop badly-sorted imports getting re-introduced. Chosen style is ‘Vertical Hanging Indent’ with trailing commas, because I think it gives the cleanest diffs, eg: ``` from third_party import ( lib1, lib2, lib3, lib4, ) ``` 1. https://pypi.python.org/pypi/isort
2018-02-20 11:22:17 +00:00
from app import (
current_service,
job_api_client,
fix one more place where senders weren't sanitised make sure everything is using the `nl2br` formatter that properly wraps it in markdown to keep everything sanitised nicely. Also write a couple of tests
2020-01-22 16:13:25 +00:00
nl2br,
Enforce order and style of imports Done using isort[1], with the following command: ``` isort -rc ./app ./tests ``` Adds linting to the `run_tests.sh` script to stop badly-sorted imports getting re-introduced. Chosen style is ‘Vertical Hanging Indent’ with trailing commas, because I think it gives the cleanest diffs, eg: ``` from third_party import ( lib1, lib2, lib3, lib4, ) ``` 1. https://pypi.python.org/pypi/isort
2018-02-20 11:22:17 +00:00
notification_api_client,
service_api_client,
)
remove antivirus code (email does not allow attachments via ui)
2022-12-05 16:35:46 -05:00
from app.main import main
Show one field at a time on send yourself a test The send yourself a test feature is useful for two things: - constructing an email/text message/letter without uploading a CSV file - seeing what the thing your going to send will look like (either by getting it in your inbox or downloading the PDF) - learning the concept of placeholders, ie understanding they’re thing that gets populated with _stuff_ The problem we’re seeing is that the current UI breaks when a template has a lot of placeholders. This is especially apparent with letter templates, which have a minimum of 7 placeholders by virtue of the address. The idea behind having the form fields side-by-side was to help people understand the relationship between their spreadsheet columns and the placeholders. But this means that the page was doing a lot of work, trying to teach: - replacement of placeholders - link between placeholders and spreadsheet columns The latter is better explained by the example spreadsheet shown on the upload page. So it can safely be removed from the send yourself a test page – in other words the fields don’t need to be shown side by side. Showing them one-at-a-time works well because: - it’s really obvious, even on first use, what the page is asking you to do - as your step through each placeholder, you see the message build up with the data you’ve entered – you’re learning how replacement of placeholders works by repetition This also means adding a matching endpoint for viewing each step of making the test letter as a PDF/PNG because we can’t reuse the view of the template without any placeholders filled any more.
2017-05-04 09:30:55 +01:00
from app.main.forms import (
ChooseTimeForm,
Enforce order and style of imports Done using isort[1], with the following command: ``` isort -rc ./app ./tests ``` Adds linting to the `run_tests.sh` script to stop badly-sorted imports getting re-introduced. Chosen style is ‘Vertical Hanging Indent’ with trailing commas, because I think it gives the cleanest diffs, eg: ``` from third_party import ( lib1, lib2, lib3, lib4, ) ``` 1. https://pypi.python.org/pypi/isort
2018-02-20 11:22:17 +00:00
CsvUploadForm,
Allow one-off email sending to select the reply-to address
2017-10-16 16:35:35 +01:00
SetSenderForm,
Enforce order and style of imports Done using isort[1], with the following command: ``` isort -rc ./app ./tests ``` Adds linting to the `run_tests.sh` script to stop badly-sorted imports getting re-introduced. Chosen style is ‘Vertical Hanging Indent’ with trailing commas, because I think it gives the cleanest diffs, eg: ``` from third_party import ( lib1, lib2, lib3, lib4, ) ``` 1. https://pypi.python.org/pypi/isort
2018-02-20 11:22:17 +00:00
get_placeholder_form_instance,
Changed flow to upload directly to s3 if non empty file. Then on check page file read back from s3 and checked.
2016-01-14 16:00:13 +00:00
)
Make user API client return JSON, not a model The data flow of other bits of our application looks like this: ``` API (returns JSON) ⬇ API client (returns a built in type, usually `dict`) ⬇ Model (returns an instance, eg of type `Service`) ⬇ View (returns HTML) ``` The user API client was architected weirdly, in that it returned a model directly, like this: ``` API (returns JSON) ⬇ API client (returns a model, of type `User`, `InvitedUser`, etc) ⬇ View (returns HTML) ``` This mixing of different layers of the application is bad because it makes it hard to write model code that doesn’t have circular dependencies. As our application gets more complicated we will be relying more on models to manage this complexity, so we should make it easy, not hard to write them. It also means that most of our mocking was of the User model, not just the underlying JSON. So it would have been easy to introduce subtle bugs to the user model, because it wasn’t being comprehensively tested. A lot of the changed lines of code in this commit mean changing the tests to mock only the JSON, which means that the model layer gets implicitly tested. For those reasons this commit changes the user API client to return JSON, not an instance of `User` or other models.
2019-05-23 15:27:35 +01:00
from app.models.user import Users
Refactor to split s3_client.py into multiple files Separated s3_client.py into 3 files - for logos, CSV files and the MOU. This helps to keep things clearer now that we need to add lots more logo functions for letters.
2019-01-30 09:42:15 +00:00
from app.s3_client.s3_csv_client import (
Check metadata for file name when sending from contact list The `.send_from_contact_list` function redirected to `.check_messages` with `original_file_name` in the query string. Contact lists already have `original_file_name` as part of their metadata, so we can stop sending it in the query string and use the metadata instead.
2020-10-21 13:07:59 +01:00
get_csv_metadata,
Refactor to split s3_client.py into multiple files Separated s3_client.py into 3 files - for logos, CSV files and the MOU. This helps to keep things clearer now that we need to add lots more logo functions for letters.
2019-01-30 09:42:15 +00:00
s3download,
s3upload,
set_metadata_on_csv_upload,
)
Refactor PNG generating endpoint for code reuse The way we generate PNGs is the same for all the different endpoints, so the code to do this should be shared.
2016-12-28 11:06:11 +00:00
from app.utils import (
Upgrade pyexcel-io, which also upgrades Werkzeug Upgraded pyexcel-io from 0.5.14 to 0.5.16. This change causes Werkzeug to be upgraded from 0.14.1 to 0.15.1 which requires some changes: * ProxyFix now needs to be imported from a different location * The status code of RequestRedirect has changed from 301 to 308. Since status code 308 is not currently supported on Internet Explorer with Windows 7 and 8.1, this subclasses RequestRedirect to keep the status code of 301. changelog: https://werkzeug.palletsprojects.com/en/0.15.x/changes/#version-0-15-0
2019-03-21 16:41:22 +00:00
PermanentRedirect,
Fix ‘Back’ link from first step of fix one off If you skip past the templates page (because you don’t have the edit permission) but then click back you end up in a loop which redirects you to the page you’re already on. This commit makes sure that you’re sent back a step further, so you don’t get stuck in that loop.
2018-08-09 16:29:51 +01:00
should_skip_template_page,
Limit length of filename S3 has a limit of 2kb for metadata: > the user-defined metadata is limited to 2 KB in size. The size of > user-defined metadata is measured by taking the sum of the number of > bytes in the UTF-8 encoding of each key and value. – https://docs.aws.amazon.com/AmazonS3/latest/dev/UsingMetadata.html#object-metadata This means we have a limit of 1870 bytes for the filename: ```python encoded = 'notification_count50000template_id665d26e7-ceac-4cc5-82ed-63d773d21561validTrueoriginal_file_name'.encode('utf-8') sys.getsizeof(b) >>> 130 2000-130 >>> 1870 ``` Or, in other words, ~918 characters: ```python sys.getsizeof(('ü'*918).encode('utf-8')) >>> 1869 ```
2018-04-30 10:46:39 +01:00
unicode_truncate,
Refactor PNG generating endpoint for code reuse The way we generate PNGs is the same for all the different endpoints, so the code to do this should be shared.
2016-12-28 11:06:11 +00:00
)
Extract template / csv utility code into modules This follows a similar approach to the previous commits, noting that one module depends on the other, so we have to extract both together.
2021-06-09 15:15:35 +01:00
from app.utils.csv import Spreadsheet, get_errors_for_csv
from app.utils.templates import get_template
Extract user utility code into own module This provides more room for expansion, and reduces the amount of arbitrary code in the __init__.py file for the new package.
2021-06-09 13:19:05 +01:00
from app.utils.user import user_has_permissions
Make send the send flow generic This commit parameterises all methods in the send view so that they can send either emails or SMS messages. It works out what kind of message it is sending from the `template_type` property of the template object. This means that the `Template` util class needs to know about these properties, which means that this commit depends on: https://github.com/alphagov/notifications-utils/pull/2 This commit does _not_ add tests for sending emails. The existing tests for sending SMS still pass, but actually sending emails is outside the scope of this story.
2016-02-22 17:17:18 +00:00
Merge with master.
2016-03-01 15:53:58 +00:00
Make send a test teach you how placeholders work This commit does two main things: - adds textboxes to the send yourself a test page, so you can replace ((name)) with ‘Chris’, or whatever your name is - rejigs the send page a bit to make it clearer what the CSV upload is for and how to use it The idea being that, since most users go into ‘send yourself a test’ first, it teaches them about how placeholders work by making them do the replacing themselves.
2016-05-05 08:39:36 +01:00
def get_example_csv_fields(column_headers, use_example_as_example, submitted_fields):
if use_example_as_example:
return ["example" for header in column_headers]
elif submitted_fields:
return [submitted_fields.get(header) for header in column_headers]
else:
return list(column_headers)
def get_example_csv_rows(template, use_example_as_example=True, submitted_fields=False):
Handle multi-column recipients Implements https://github.com/alphagov/notifications-utils/pull/81 Handles addresses as multiple columns: - in ‘Send yourself a test’ - in example CSV files - in validating that a CSV file has recipients (eg at least an ‘address line 1’ and ‘postcode’ column) - when showing the contents of a CSV file As few UI changes as possible, once we have the thing working end-to-end we can think about how the UI might need to work differently.
2016-11-10 13:39:05 +00:00
return {
'email': ['test@example.com'] if use_example_as_example else [current_user.email_address],
images and overrides
2022-07-21 18:25:23 -07:00
'sms': ['12223334444'] if use_example_as_example else [current_user.mobile_number],
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.
2018-06-08 12:49:29 +01:00
}[template.template_type] + get_example_csv_fields(
(
placeholder for placeholder in template.placeholders
Bump utils to 53.0.0 Changes: 53.0.0 --- * `notifications_utils.columns.Columns` has moved to `notifications_utils.insensitive_dict.InsensitiveDict` * `notifications_utils.columns.Rows` has moved to `notifications_utils.recipients.Rows` * `notifications_utils.columns.Cell` has moved to `notifications_utils.recipients.Cell` 52.0.0 --- * Deprecate the following unused `redis_client` functions: - `redis_client.increment_hash_value` - `redis_client.decrement_hash_value` - `redis_client.get_all_from_hash` - `redis_client.set_hash_and_expire` - `redis_client.expire` 51.3.1 --- * Bump govuk-bank-holidays to cache holidays for next year. 51.3.0 --- * Log exception and stacktrace when Celery tasks fail.
2022-02-04 10:43:36 +00:00
if placeholder not in InsensitiveDict.from_keys(
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.
2018-06-08 12:49:29 +01:00
first_column_headings[template.template_type]
)
),
use_example_as_example,
submitted_fields
)
Put two rows in example CSV file We reckon that this will help people understand that the first row is headers and the subsequent rows are the users’ data.
2016-04-01 14:31:28 +01:00
Require IDs to be UUIDs in URLS We mostly rely on the API returning a 404 to generate 404s for trying to get things with non-UUID IDs. This is fine, except our tests often mock these API calls. So it could look like everything is working fine, except the thing your passing in might never be a valid UUID, and thus would 404 in a non-test environment. So this commit: 1. uses the `uuid` URL converter everywhere there’s something that looks like an ID in a URL parameter 2. adds a test which automates checking for 1.
2019-11-04 11:07:55 +00:00
@main.route("/services/<uuid:service_id>/send/<uuid:template_id>/csv", methods=['GET', 'POST'])
add restrict_admin_usage flag to more of the send flow
2018-03-01 11:04:25 +00:00
@user_has_permissions('send_messages', restrict_admin_usage=True)
Make send the send flow generic This commit parameterises all methods in the send view so that they can send either emails or SMS messages. It works out what kind of message it is sending from the `template_type` property of the template object. This means that the `Template` util class needs to know about these properties, which means that this commit depends on: https://github.com/alphagov/notifications-utils/pull/2 This commit does _not_ add tests for sending emails. The existing tests for sending SMS still pass, but actually sending emails is outside the scope of this story.
2016-02-22 17:17:18 +00:00
def send_messages(service_id, template_id):
Added display of daily limit and remaining amount after csv upload
2023-05-15 14:37:05 -06:00
notification_count = service_api_client.get_notification_count(service_id)
Requested changes in PR review
2023-06-01 15:40:50 -06:00
remaining_messages = current_service.message_limit - notification_count
Added display of daily limit and remaining amount after csv upload
2023-05-15 14:37:05 -06:00
Replace sevice api client get template calls with Service methods Instead of using the API client directly views are now calling one of two Service model methods: `get_template` is used for view actions, where the user should see the template page even if they don't have access to the template folder (since all templates are still inked from the dashboard or the sent notifications pages). `get_template_with_user_permission_or_403` will check if the user has access to the template's folder first and return 403 otherwise. This method is used for any endpoints that result in an action: editing template attributes, deleting templates or sending messages.
2019-03-19 16:25:44 +00:00
db_template = current_service.get_template_with_user_permission_or_403(template_id, current_user)
Show all pages of a letter everywhere Right now showing all the pages in full is the only way we have of showing a letter that makes sense to our users. Maybe in the future we show some kind of truncated version, but the end of the first page is not a good place to truncate the letter. This commit just extracts the code for showing multiple pages from the template view, refactors it for reuse, and includes it in the send views.
2017-05-11 12:01:51 +01:00
Store the sender_id that should be used for a job in S3 metadata Currently, a user can select a reply-to email address or text message sender when uploading a CSV file but this is ignored and the default is always used instead. As a first step towards changing this, this adds the sender_id (if selected) to the S3 metadata so that this information can be used when processing the job.
2018-11-06 14:22:22 +00:00
email_reply_to = None
sms_sender = None
if db_template['template_type'] == 'email':
email_reply_to = get_email_reply_to_address_from_session()
elif db_template['template_type'] == 'sms':
sms_sender = get_sms_sender_from_session()
Allow adding broadcast templates At the moment the page is the same as for text message templates, except: - different H1 - no guidance about personalisation, links, etc (until we decide how these should work) For now you won’t be able to really create a broadcast template, because the API doesn’t support it (the API will respond with a 400). But that’s OK because no real services have the broadcast permission yet. This required a bit of refactoring of how we check which template types a service can use, because there were some hard-coded assumptions about emails and text messages.
2020-07-01 16:43:08 +01:00
if db_template['template_type'] not in current_service.available_template_types:
Stop users from editing/adding templates without the correct permission If sending SMS is disabled for a service, it should not be possible to add or modify SMS templates. If a user tries to do this, they should see a different page with a link to go back. The same thing should happen with email templates.
2017-06-30 10:55:59 +01:00
return redirect(url_for(
'.action_blocked',
service_id=service_id,
notification_type=db_template['template_type'],
return_to='view_template',
template_id=template_id
))
Incorporate breaking utils changes Brings in: - [ ] https://github.com/alphagov/notifications-utils/pull/94
2016-12-08 11:50:59 +00:00
template = get_template(
rename template var in send_test_step it's confusing reassigining one template (json from api) to another type (utils object) on one line. Also removed an unnecessary bounds check (since if placeholders is empty the IndexError will throw on the next line anyway and it'll be handled the same), and moved get_back_link out to its own function
2017-06-22 16:00:14 +01:00
db_template,
Incorporate breaking utils changes Brings in: - [ ] https://github.com/alphagov/notifications-utils/pull/94
2016-12-08 11:50:59 +00:00
current_service,
Add remaining endpoints for PDFs and PNGs Right now we can show what a letter template looks like as a PDF or PNG. This commit completes the work so this is also possible when: - showing a template with the placeholders replaced - showing any version of a template Also removes dependency on `Exception().message`, which was deprecated in Python 2.6. See https://github.com/alphagov/notifications-utils/commit/97f82d565f3112926752ae382e723b0808a3135d for full details.
2016-12-20 14:38:34 +00:00
show_recipient=True,
Store the sender_id that should be used for a job in S3 metadata Currently, a user can select a reply-to email address or text message sender when uploading a CSV file but this is ignored and the default is always used instead. As a first step towards changing this, this adds the sender_id (if selected) to the S3 metadata so that this information can be used when processing the job.
2018-11-06 14:22:22 +00:00
email_reply_to=email_reply_to,
sms_sender=sms_sender,
After sending sms or email to self or batch then then back button does not take user to check page, but rather the start of sending either sms or email.
2016-03-29 15:59:53 +01:00
)
First slice of csv upload of phone numbers for sending messages. At the moment the file contents are not persisted by checked in memory. The first and last three records are show if all are valid. If there are invalid rows, they are reported and the user is prompted to go back and sort out upload file. The storing of upload result (i.e. validation of file) in session will be removed in next story which is about persisting of file for later processing.
2016-01-11 15:00:51 +00:00
form = CsvUploadForm()
Changed page flow to first save file and then redirect to check. On check numbers in file are validated. Posting to check then uploads file to s3
2016-01-13 17:32:40 +00:00
if form.validate_on_submit():
First slice of csv upload of phone numbers for sending messages. At the moment the file contents are not persisted by checked in memory. The first and last three records are show if all are valid. If there are invalid rows, they are reported and the user is prompted to go back and sort out upload file. The storing of upload result (i.e. validation of file) in session will be removed in next story which is about persisting of file for later processing.
2016-01-11 15:00:51 +00:00
try:
Make `s3upload` function return the UUID Generating the UUID can be can be contained within this function, thus any other part of the code using it doesn’t have to do the ID-generating stuff itself.
2016-05-15 07:47:50 +01:00
upload_id = s3upload(
Give the user better error messages for CSV files Makes uses of the additions to utils in https://github.com/alphagov/notifications-utils/pull/9 This commit strips out a lot of the complex stuff that the views and templates in this app were doing. There is now a cleaner separation of concerns: - utils returns the number and type of errors in the csv - `get_errors_for_csv` helper in this app maps the number and type of errors onto human-friendly error messages - the view and template just doing the glueing-together of all the pieces This is (hopefully) easier to understand, definitely makes the component parts easier to test in isolation, and makes it easier to give more specific error messages.
2016-03-07 18:47:05 +00:00
service_id,
Use the correct bucket for storing contact lists We don’t want to muddy them up with the normal CSV uploads. I’ve tried to reuse the existing S3 code where possible because it’s well tested. Buckets have already been created.
2020-03-13 13:53:18 +00:00
Spreadsheet.from_file_form(form).as_dict,
Give the user better error messages for CSV files Makes uses of the additions to utils in https://github.com/alphagov/notifications-utils/pull/9 This commit strips out a lot of the complex stuff that the views and templates in this app were doing. There is now a cleaner separation of concerns: - utils returns the number and type of errors in the csv - `get_errors_for_csv` helper in this app maps the number and type of errors onto human-friendly error messages - the view and template just doing the glueing-together of all the pieces This is (hopefully) easier to understand, definitely makes the component parts easier to test in isolation, and makes it easier to give more specific error messages.
2016-03-07 18:47:05 +00:00
)
Put filename in metadata when sending via a CSV When sending from an uploaded CSV `.send_messages` now puts the filename in the metadata. It previously used the query string to pass the filename to `.check_messages`, where it can be lost.
2020-10-23 16:10:03 +01:00
file_name_metadata = unicode_truncate(
SanitiseASCII.encode(form.file.data.filename),
1600
)
set_metadata_on_csv_upload(
service_id,
upload_id,
original_file_name=file_name_metadata
)
Store template ID and original file name in URL At the moment you can’t press refresh on the check page if there’s errors. This is because the session gets cleared when there’s errors. This is a bad user experience. The data that this page is relying on (from the session) is: - template ID - original file name Neither of these things need to be in the session because: - they are not secret - the user can modify them already (by choosing a different template or renaming their file locally) So this commit additionally stores them in the URL.
2018-03-29 17:01:35 +01:00
return redirect(url_for(
'.check_messages',
service_id=service_id,
upload_id=upload_id,
template_id=template.id,
))
Handle files that can’t be interpreted as spreadsheets There shouldn’t be a case where we see a `ValueError` on upload any more. Our file handling should be robust enough to deal with whatever is thrown at it. This commit: - adds test files with bad data (PNG files with their extensions changed to look like spreadsheets) - catches whatever exceptions are raised by trying to parse these files - returns a helpful flash message to the user Anything else should raise a `500`, eg if the file can’t be uploaded to S3.
2016-05-13 21:25:59 +01:00
except (UnicodeDecodeError, BadZipFile, XLRDError):
Replace couldn't with could not
2019-09-13 12:07:27 +01:00
flash('Could not read {}. Try using a different file format.'.format(
Handle files that can’t be interpreted as spreadsheets There shouldn’t be a case where we see a `ValueError` on upload any more. Our file handling should be robust enough to deal with whatever is thrown at it. This commit: - adds test files with bad data (PNG files with their extensions changed to look like spreadsheets) - catches whatever exceptions are raised by trying to parse these files - returns a helpful flash message to the user Anything else should raise a `500`, eg if the file can’t be uploaded to S3.
2016-05-13 21:25:59 +01:00
form.file.data.filename
))
Catch exceptions caused by ambiguous Excel files Excel stores dates as floating point numbers, counting the days (or fraction thereof) since 1900 (except when it counts from 1904). However it also, incorrectly, recognises 1900 as a leap year. This means that it’s ambiguous whether day 59 is February 28th, or February 27th, depending if you’re counting up or down. In fact any number less than 60 is, therefore, ambiguous. This shouldn’t really matter since no-one is going to be using dates in the year 1900 in Notify messages. _Except_ when Excel misidentifies a cell as containing a date. For example, if you have the number `9` inside a cell, it means _house number 9_ if the next cell contains _example_ street. but Excel is all like ‘oh they must want January 9th 1900!’ No. Bad Excel. There’s not much we can do about this, but we can at least give people an error message with the workaround, which is what this commit does. Most of this commit message is paraphrased from: http://xlrd.readthedocs.io/en/latest/dates.html
2018-02-28 14:45:57 +00:00
except (XLDateError):
flash((
Replace can't with cannot
2019-09-13 10:17:19 +01:00
'{} contains numbers or dates that Notify cannot understand. '
Catch exceptions caused by ambiguous Excel files Excel stores dates as floating point numbers, counting the days (or fraction thereof) since 1900 (except when it counts from 1904). However it also, incorrectly, recognises 1900 as a leap year. This means that it’s ambiguous whether day 59 is February 28th, or February 27th, depending if you’re counting up or down. In fact any number less than 60 is, therefore, ambiguous. This shouldn’t really matter since no-one is going to be using dates in the year 1900 in Notify messages. _Except_ when Excel misidentifies a cell as containing a date. For example, if you have the number `9` inside a cell, it means _house number 9_ if the next cell contains _example_ street. but Excel is all like ‘oh they must want January 9th 1900!’ No. Bad Excel. There’s not much we can do about this, but we can at least give people an error message with the workaround, which is what this commit does. Most of this commit message is paraphrased from: http://xlrd.readthedocs.io/en/latest/dates.html
2018-02-28 14:45:57 +00:00
'Try formatting all columns as ‘text’ or export your file as CSV.'
).format(
form.file.data.filename
))
Show flash instead of inline upload errors This has several advantages: - It gives us more room to explain the error and actions. This will be useful for upcoming work we want to do, which will add yet more validations for CSV uploads. - We already use a flash to show certain kinds of errors on these pages (just above). This is more consistent. - It's potentially more accessible. Previously the error and the button text used to be read out as a single sentence. Now the page reloads and reads the flash error alone. In theory we should show an error in both places, but this can be confusing on pages where there's only a single form control, and especially if the error is long.
2021-12-03 18:14:02 +00:00
elif form.errors:
# just show the first error, as we don't expect the form to have more
# than one, since it only has one field
first_field_errors = list(form.errors.values())[0]
flash(first_field_errors[0])
First slice of csv upload of phone numbers for sending messages. At the moment the file contents are not persisted by checked in memory. The first and last three records are show if all are valid. If there are invalid rows, they are reported and the user is prompted to go back and sort out upload file. The storing of upload result (i.e. validation of file) in session will be removed in next story which is about persisting of file for later processing.
2016-01-11 15:00:51 +00: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.
2018-06-08 12:49:29 +01:00
column_headings = get_spreadsheet_column_headings_from_template(template)
Handle multi-column recipients Implements https://github.com/alphagov/notifications-utils/pull/81 Handles addresses as multiple columns: - in ‘Send yourself a test’ - in example CSV files - in validating that a CSV file has recipients (eg at least an ‘address line 1’ and ‘postcode’ column) - when showing the contents of a CSV file As few UI changes as possible, once we have the thing working end-to-end we can think about how the UI might need to work differently.
2016-11-10 13:39:05 +00:00
Use `Template` to replace/highlight placeholders This commit brings in the `Template` util, added here: https://github.com/alphagov/notifications-utils/pull/1 It also does a fair bit of tidying up, which I’ve unfortunately squashed into this one massive commit. The main change is moving 404 handling into the templates dao, so that every view isn’t littered with `try: … except(HTTPError)`. It also adds new features, in a prototypy sort of way, which are: - download a prefilled example CSV - show all the columns for your template on the 'check' page
2016-02-08 16:36:53 +00:00
return render_template(
Make send the send flow generic This commit parameterises all methods in the send view so that they can send either emails or SMS messages. It works out what kind of message it is sending from the `template_type` property of the template object. This means that the `Template` util class needs to know about these properties, which means that this commit depends on: https://github.com/alphagov/notifications-utils/pull/2 This commit does _not_ add tests for sending emails. The existing tests for sending SMS still pass, but actually sending emails is outside the scope of this story.
2016-02-22 17:17:18 +00:00
'views/send.html',
Use `Template` to replace/highlight placeholders This commit brings in the `Template` util, added here: https://github.com/alphagov/notifications-utils/pull/1 It also does a fair bit of tidying up, which I’ve unfortunately squashed into this one massive commit. The main change is moving 404 handling into the templates dao, so that every view isn’t littered with `try: … except(HTTPError)`. It also adds new features, in a prototypy sort of way, which are: - download a prefilled example CSV - show all the columns for your template on the 'check' page
2016-02-08 16:36:53 +00:00
template=template,
Handle multi-column recipients Implements https://github.com/alphagov/notifications-utils/pull/81 Handles addresses as multiple columns: - in ‘Send yourself a test’ - in example CSV files - in validating that a CSV file has recipients (eg at least an ‘address line 1’ and ‘postcode’ column) - when showing the contents of a CSV file As few UI changes as possible, once we have the thing working end-to-end we can think about how the UI might need to work differently.
2016-11-10 13:39:05 +00:00
column_headings=list(ascii_uppercase[:len(column_headings)]),
example=[column_headings, get_example_csv_rows(template)],
Set accepted files for job spreadsheet upload
2020-12-16 11:05:19 +00:00
form=form,
Added display of daily limit and remaining amount after csv upload
2023-05-15 14:37:05 -06:00
allowed_file_extensions=Spreadsheet.ALLOWED_FILE_EXTENSIONS,
remaining_messages=remaining_messages
Use `Template` to replace/highlight placeholders This commit brings in the `Template` util, added here: https://github.com/alphagov/notifications-utils/pull/1 It also does a fair bit of tidying up, which I’ve unfortunately squashed into this one massive commit. The main change is moving 404 handling into the templates dao, so that every view isn’t littered with `try: … except(HTTPError)`. It also adds new features, in a prototypy sort of way, which are: - download a prefilled example CSV - show all the columns for your template on the 'check' page
2016-02-08 16:36:53 +00:00
)
Require IDs to be UUIDs in URLS We mostly rely on the API returning a 404 to generate 404s for trying to get things with non-UUID IDs. This is fine, except our tests often mock these API calls. So it could look like everything is working fine, except the thing your passing in might never be a valid UUID, and thus would 404 in a non-test environment. So this commit: 1. uses the `uuid` URL converter everywhere there’s something that looks like an ID in a URL parameter 2. adds a test which automates checking for 1.
2019-11-04 11:07:55 +00:00
@main.route("/services/<uuid:service_id>/send/<uuid:template_id>.csv", methods=['GET'])
remove any_ from has_permissions we branch on any_ to either say "require ALL these permissions" or "require ANY of these permissions". But we only ever call the decorator with one permission, or with any_=True, so it's unnecessary
2018-03-01 10:37:55 +00:00
@user_has_permissions('send_messages', 'manage_templates')
Make send a test teach you how placeholders work This commit does two main things: - adds textboxes to the send yourself a test page, so you can replace ((name)) with ‘Chris’, or whatever your name is - rejigs the send page a bit to make it clearer what the CSV upload is for and how to use it The idea being that, since most users go into ‘send yourself a test’ first, it teaches them about how placeholders work by making them do the replacing themselves.
2016-05-05 08:39:36 +01:00
def get_example_csv(service_id, template_id):
Incorporate breaking utils changes Brings in: - [ ] https://github.com/alphagov/notifications-utils/pull/94
2016-12-08 11:50:59 +00:00
template = get_template(
service_api_client.get_service_template(service_id, template_id)['data'], current_service
)
Use `Spreadsheet` class to generate example CSVs This means not repeating a bunch of `with StringIO` blocks all over the place.
2016-05-15 10:47:52 +01:00
return Spreadsheet.from_rows([
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.
2018-06-08 12:49:29 +01:00
get_spreadsheet_column_headings_from_template(template),
Use `Spreadsheet` class to generate example CSVs This means not repeating a bunch of `with StringIO` blocks all over the place.
2016-05-15 10:47:52 +01:00
get_example_csv_rows(template)
]).as_csv_data, 200, {
'Content-Type': 'text/csv; charset=utf-8',
'Content-Disposition': 'inline; filename="{}.csv"'.format(template.name)
}
Use Flask for routing This commit: - replaces links that look like buttons with forms and submit buttons - splits the view code for SMS into its own file - moves the routing into the Python by adding handling for `post` requests - uses Flask’s `url_for` to generate URLs, rather than hard coding them (so that it’s easier to change the URLs) - chages the URLs for sending text messages
2015-12-10 21:15:20 +00:00
Require IDs to be UUIDs in URLS We mostly rely on the API returning a 404 to generate 404s for trying to get things with non-UUID IDs. This is fine, except our tests often mock these API calls. So it could look like everything is working fine, except the thing your passing in might never be a valid UUID, and thus would 404 in a non-test environment. So this commit: 1. uses the `uuid` URL converter everywhere there’s something that looks like an ID in a URL parameter 2. adds a test which automates checking for 1.
2019-11-04 11:07:55 +00:00
@main.route("/services/<uuid:service_id>/send/<uuid:template_id>/set-sender", methods=['GET', 'POST'])
add restrict_admin_usage flag to more of the send flow
2018-03-01 11:04:25 +00:00
@user_has_permissions('send_messages', restrict_admin_usage=True)
Allow one-off email sending to select the reply-to address
2017-10-16 16:35:35 +01:00
def set_sender(service_id, template_id):
session['sender_id'] = None
redirect_to_one_off = redirect(
url_for('.send_one_off', service_id=service_id, template_id=template_id)
)
Replace sevice api client get template calls with Service methods Instead of using the API client directly views are now calling one of two Service model methods: `get_template` is used for view actions, where the user should see the template page even if they don't have access to the template folder (since all templates are still inked from the dashboard or the sent notifications pages). `get_template_with_user_permission_or_403` will check if the user has access to the template's folder first and return 403 otherwise. This method is used for any endpoints that result in an action: editing template attributes, deleting templates or sending messages.
2019-03-19 16:25:44 +00:00
template = current_service.get_template_with_user_permission_or_403(template_id, current_user)
Allow one-off email sending to select the reply-to address
2017-10-16 16:35:35 +01:00
sender_details = get_sender_details(service_id, template['template_type'])
Show 'From' / 'Reply to' on all sending screens This makes the preview of the email / SMS to send consistent with the final screen, which we previously changed to show the "reply to" text irrespective of whether the user had selected anything.
2021-02-08 17:23:47 +00:00
if len(sender_details) == 1:
session['sender_id'] = sender_details[0]['id']
Allow one-off email sending to select the reply-to address
2017-10-16 16:35:35 +01:00
if len(sender_details) <= 1:
return redirect_to_one_off
sender_context = get_sender_context(sender_details, template['template_type'])
form = SetSenderForm(
sender=sender_context['default_id'],
sender_choices=sender_context['value_and_label'],
sender_label=sender_context['description']
)
add restrict_admin_usage flag to more of the send flow
2018-03-01 11:04:25 +00:00
option_hints = {sender_context['default_id']: '(Default)'}
Added a hint to show default and receives text messages
2017-11-02 15:48:19 +00:00
if sender_context.get('receives_text_message', None):
option_hints.update({sender_context['receives_text_message']: '(Receives replies)'})
if sender_context.get('default_and_receives', None):
option_hints = {sender_context['default_and_receives']: '(Default and receives replies)'}
Allow one-off email sending to select the reply-to address
2017-10-16 16:35:35 +01:00
Convert radios on set-sender page Includes making the legend the page header to follow design system guidance: https://design-system.service.gov.uk/get-started/labels-legends-headings/ I had to add some override styles to allow the legend to classes as a full-width column, and the radios to be in one that is three-quarters.
2021-01-13 10:31:07 +00:00
# extend all radios that need hint text
form.sender.param_extensions = {'items': []}
for item_id, _item_value in form.sender.choices:
if item_id in option_hints:
extensions = {'hint': {'text': option_hints[item_id]}}
else:
extensions = {} # if no extensions needed, send an empty dict to preserve order of items
form.sender.param_extensions['items'].append(extensions)
Allow one-off email sending to select the reply-to address
2017-10-16 16:35:35 +01:00
if form.validate_on_submit():
session['sender_id'] = form.sender.data
return redirect(url_for('.send_one_off',
service_id=service_id,
template_id=template_id))
return render_template(
'views/templates/set-sender.html',
form=form,
template_id=template_id,
sender_context={'title': sender_context['title'], 'description': sender_context['description']},
option_hints=option_hints
)
def get_sender_context(sender_details, template_type):
context = {
'email': {
Fix misleading `<h1>` in one-off flow Changing the `<h1>` in https://github.com/alphagov/notifications-admin/pull/1638 turned out to be quite confusing. The combination of the word "recipient" and a selection of email addresses on the page was confusing. This commit changes the page title to be much more explicit about what is expected from the page, rather than what is consistent with the text of the link that the user clicked.
2017-11-20 16:47:03 +00:00
'title': 'Where should replies come back to?',
'description': 'Where should replies come back to?',
Allow one-off email sending to select the reply-to address
2017-10-16 16:35:35 +01:00
'field_name': 'email_address'
},
This PR adds the ability to select a text message sender if more than one exist for the service.
2017-11-02 12:07:46 +00:00
'sms': {
Fix misleading `<h1>` in one-off flow Changing the `<h1>` in https://github.com/alphagov/notifications-admin/pull/1638 turned out to be quite confusing. The combination of the word "recipient" and a selection of email addresses on the page was confusing. This commit changes the page title to be much more explicit about what is expected from the page, rather than what is consistent with the text of the link that the user clicked.
2017-11-20 16:26:09 +00:00
'title': 'Who should the message come from?',
Reword choose sender/reply to address pages Make it clear that: - In the case of text messages, it’s about who the message comes from - In the case of emails, it’s about where the user will reply to
2017-11-13 11:31:12 +00:00
'description': 'Who should the message come from?',
This PR adds the ability to select a text message sender if more than one exist for the service.
2017-11-02 12:07:46 +00:00
'field_name': 'sms_sender'
Allow one-off email sending to select the reply-to address
2017-10-16 16:35:35 +01:00
}
}[template_type]
sender_format = context['field_name']
context['default_id'] = next(sender['id'] for sender in sender_details if sender['is_default'])
Added a hint for Receives replies
2017-11-02 14:58:14 +00:00
if template_type == 'sms':
Added a hint to show default and receives text messages
2017-11-02 15:48:19 +00:00
inbound = [sender['id'] for sender in sender_details if sender['inbound_number_id']]
if inbound:
context['receives_text_message'] = next(iter(inbound))
if context['default_id'] == context.get('receives_text_message', None):
context['default_and_receives'] = context['default_id']
fix one more place where senders weren't sanitised make sure everything is using the `nl2br` formatter that properly wraps it in markdown to keep everything sanitised nicely. Also write a couple of tests
2020-01-22 16:13:25 +00:00
context['value_and_label'] = [(sender['id'], nl2br(sender[sender_format])) for sender in sender_details]
Allow one-off email sending to select the reply-to address
2017-10-16 16:35:35 +01:00
return context
def get_sender_details(service_id, template_type):
api_call = {
'email': service_api_client.get_reply_to_email_addresses,
This PR adds the ability to select a text message sender if more than one exist for the service.
2017-11-02 12:07:46 +00:00
'sms': service_api_client.get_sms_senders
Allow one-off email sending to select the reply-to address
2017-10-16 16:35:35 +01:00
}[template_type]
return api_call(service_id)
Remove test/step- routes We don't need these anymore as all users will use the `one-off/step` routes. This has mostly involved tidying up the tests which are still a little disorganised and not as good as I'd like but it's a step in the right direction. More refactoring is still possible to the routes, it may come in a later PR if I have time.
2020-10-07 14:51:18 +01:00
@main.route("/services/<uuid:service_id>/send/<uuid:template_id>/one-off")
add restrict_admin_usage flag to more of the send flow
2018-03-01 11:04:25 +00:00
@user_has_permissions('send_messages', restrict_admin_usage=True)
Remove test/step- routes We don't need these anymore as all users will use the `one-off/step` routes. This has mostly involved tidying up the tests which are still a little disorganised and not as good as I'd like but it's a step in the right direction. More refactoring is still possible to the routes, it may come in a later PR if I have time.
2020-10-07 14:51:18 +01:00
def send_one_off(service_id, template_id):
separate recipient from rest of placeholders when sending notification note: in the case of letters, we still want to create a CSV file. This only modifies the code flow when it's an email or template 😩 renamed `send_test_values` to `placeholders` because a) that's what they are and b) this isn't just for sending a test message any more
2017-06-23 17:30:34 +01:00
session['recipient'] = None
session['placeholders'] = {}
Stop users from editing/adding templates without the correct permission If sending SMS is disabled for a service, it should not be possible to add or modify SMS templates. If a user tries to do this, they should see a different page with a link to go back. The same thing should happen with email templates.
2017-06-30 10:55:59 +01:00
Replace sevice api client get template calls with Service methods Instead of using the API client directly views are now calling one of two Service model methods: `get_template` is used for view actions, where the user should see the template page even if they don't have access to the template folder (since all templates are still inked from the dashboard or the sent notifications pages). `get_template_with_user_permission_or_403` will check if the user has access to the template's folder first and return 403 otherwise. This method is used for any endpoints that result in an action: editing template attributes, deleting templates or sending messages.
2019-03-19 16:25:44 +00:00
db_template = current_service.get_template_with_user_permission_or_403(template_id, current_user)
Stop users from editing/adding templates without the correct permission If sending SMS is disabled for a service, it should not be possible to add or modify SMS templates. If a user tries to do this, they should see a different page with a link to go back. The same thing should happen with email templates.
2017-06-30 10:55:59 +01:00
Allow adding broadcast templates At the moment the page is the same as for text message templates, except: - different H1 - no guidance about personalisation, links, etc (until we decide how these should work) For now you won’t be able to really create a broadcast template, because the API doesn’t support it (the API will respond with a 400). But that’s OK because no real services have the broadcast permission yet. This required a bit of refactoring of how we check which template types a service can use, because there were some hard-coded assumptions about emails and text messages.
2020-07-01 16:43:08 +01:00
if db_template['template_type'] not in current_service.available_template_types:
Stop users from editing/adding templates without the correct permission If sending SMS is disabled for a service, it should not be possible to add or modify SMS templates. If a user tries to do this, they should see a different page with a link to go back. The same thing should happen with email templates.
2017-06-30 10:55:59 +01:00
return redirect(url_for(
'.action_blocked',
service_id=service_id,
notification_type=db_template['template_type'],
return_to='view_template',
template_id=template_id))
Show one field at a time on send yourself a test The send yourself a test feature is useful for two things: - constructing an email/text message/letter without uploading a CSV file - seeing what the thing your going to send will look like (either by getting it in your inbox or downloading the PDF) - learning the concept of placeholders, ie understanding they’re thing that gets populated with _stuff_ The problem we’re seeing is that the current UI breaks when a template has a lot of placeholders. This is especially apparent with letter templates, which have a minimum of 7 placeholders by virtue of the address. The idea behind having the form fields side-by-side was to help people understand the relationship between their spreadsheet columns and the placeholders. But this means that the page was doing a lot of work, trying to teach: - replacement of placeholders - link between placeholders and spreadsheet columns The latter is better explained by the example spreadsheet shown on the upload page. So it can safely be removed from the send yourself a test page – in other words the fields don’t need to be shown side by side. Showing them one-at-a-time works well because: - it’s really obvious, even on first use, what the page is asking you to do - as your step through each placeholder, you see the message build up with the data you’ve entered – you’re learning how replacement of placeholders works by repetition This also means adding a matching endpoint for viewing each step of making the test letter as a PDF/PNG because we can’t reuse the view of the template without any placeholders filled any more.
2017-05-04 09:30:55 +01:00
return redirect(url_for(
Remove test/step- routes We don't need these anymore as all users will use the `one-off/step` routes. This has mostly involved tidying up the tests which are still a little disorganised and not as good as I'd like but it's a step in the right direction. More refactoring is still possible to the routes, it may come in a later PR if I have time.
2020-10-07 14:51:18 +01:00
'.send_one_off_step',
Show one field at a time on send yourself a test The send yourself a test feature is useful for two things: - constructing an email/text message/letter without uploading a CSV file - seeing what the thing your going to send will look like (either by getting it in your inbox or downloading the PDF) - learning the concept of placeholders, ie understanding they’re thing that gets populated with _stuff_ The problem we’re seeing is that the current UI breaks when a template has a lot of placeholders. This is especially apparent with letter templates, which have a minimum of 7 placeholders by virtue of the address. The idea behind having the form fields side-by-side was to help people understand the relationship between their spreadsheet columns and the placeholders. But this means that the page was doing a lot of work, trying to teach: - replacement of placeholders - link between placeholders and spreadsheet columns The latter is better explained by the example spreadsheet shown on the upload page. So it can safely be removed from the send yourself a test page – in other words the fields don’t need to be shown side by side. Showing them one-at-a-time works well because: - it’s really obvious, even on first use, what the page is asking you to do - as your step through each placeholder, you see the message build up with the data you’ve entered – you’re learning how replacement of placeholders works by repetition This also means adding a matching endpoint for viewing each step of making the test letter as a PDF/PNG because we can’t reuse the view of the template without any placeholders filled any more.
2017-05-04 09:30:55 +01:00
service_id=service_id,
template_id=template_id,
step_index=0,
))
Use context managers for StringIO It’s possible that leaving a StringIO stream open could cause a memory leak. Using a context manager closes it automatically when it’s finished with.
2016-04-03 11:30:23 +01:00
separate recipient from rest of placeholders when sending notification note: in the case of letters, we still want to create a CSV file. This only modifies the code flow when it's an email or template 😩 renamed `send_test_values` to `placeholders` because a) that's what they are and b) this isn't just for sending a test message any more
2017-06-23 17:30:34 +01:00
def get_notification_check_endpoint(service_id, template):
Let people send one off letters We didn’t used to allow this because it wasn’t really possible with the old DVLA set up and we didn’t think there’s a need. We think it’s possible now because, even though it’s cumbersome, it’s better than the manual process.
2018-10-30 14:24:50 +00:00
return redirect(url_for(
'main.check_notification',
service_id=service_id,
template_id=template.id,
))
separate recipient from rest of placeholders when sending notification note: in the case of letters, we still want to create a CSV file. This only modifies the code flow when it's an email or template 😩 renamed `send_test_values` to `placeholders` because a) that's what they are and b) this isn't just for sending a test message any more
2017-06-23 17:30:34 +01:00
Add a third route through populating a template This commit adds a route which is identical to send yourself a test, but with its own endpoint. This will let us add a slightly different ‘send a one-off message’ flow. This commit just adds the route though, and makes sure that the tests pass for both routes.
2017-05-25 08:55:05 +01:00
@main.route(
Require IDs to be UUIDs in URLS We mostly rely on the API returning a 404 to generate 404s for trying to get things with non-UUID IDs. This is fine, except our tests often mock these API calls. So it could look like everything is working fine, except the thing your passing in might never be a valid UUID, and thus would 404 in a non-test environment. So this commit: 1. uses the `uuid` URL converter everywhere there’s something that looks like an ID in a URL parameter 2. adds a test which automates checking for 1.
2019-11-04 11:07:55 +00:00
"/services/<uuid:service_id>/send/<uuid:template_id>/one-off/step-<int:step_index>",
Add a third route through populating a template This commit adds a route which is identical to send yourself a test, but with its own endpoint. This will let us add a slightly different ‘send a one-off message’ flow. This commit just adds the route though, and makes sure that the tests pass for both routes.
2017-05-25 08:55:05 +01:00
methods=['GET', 'POST'],
)
add restrict_admin_usage flag to more of the send flow
2018-03-01 11:04:25 +00:00
@user_has_permissions('send_messages', restrict_admin_usage=True)
Remove test/step- routes We don't need these anymore as all users will use the `one-off/step` routes. This has mostly involved tidying up the tests which are still a little disorganised and not as good as I'd like but it's a step in the right direction. More refactoring is still possible to the routes, it may come in a later PR if I have time.
2020-10-07 14:51:18 +01:00
def send_one_off_step(service_id, template_id, step_index):
separate recipient from rest of placeholders when sending notification note: in the case of letters, we still want to create a CSV file. This only modifies the code flow when it's an email or template 😩 renamed `send_test_values` to `placeholders` because a) that's what they are and b) this isn't just for sending a test message any more
2017-06-23 17:30:34 +01:00
if {'recipient', 'placeholders'} - set(session.keys()):
Check that users can’t skip steps in send test Because we put the step in the URL, users could: - skip ahead to a later step - navigate to a step which doesn’t exist (ie an index greater than the number of placeholders) This commit adds some checks to do the sensible thing in the unlikely event that either of these situations occur.
2017-05-22 11:49:15 +01:00
return redirect(url_for(
Remove test/step- routes We don't need these anymore as all users will use the `one-off/step` routes. This has mostly involved tidying up the tests which are still a little disorganised and not as good as I'd like but it's a step in the right direction. More refactoring is still possible to the routes, it may come in a later PR if I have time.
2020-10-07 14:51:18 +01:00
".send_one_off",
Add a third route through populating a template This commit adds a route which is identical to send yourself a test, but with its own endpoint. This will let us add a slightly different ‘send a one-off message’ flow. This commit just adds the route though, and makes sure that the tests pass for both routes.
2017-05-25 08:55:05 +01:00
service_id=service_id,
template_id=template_id,
Check that users can’t skip steps in send test Because we put the step in the URL, users could: - skip ahead to a later step - navigate to a step which doesn’t exist (ie an index greater than the number of placeholders) This commit adds some checks to do the sensible thing in the unlikely event that either of these situations occur.
2017-05-22 11:49:15 +01:00
))
Replace sevice api client get template calls with Service methods Instead of using the API client directly views are now calling one of two Service model methods: `get_template` is used for view actions, where the user should see the template page even if they don't have access to the template folder (since all templates are still inked from the dashboard or the sent notifications pages). `get_template_with_user_permission_or_403` will check if the user has access to the template's folder first and return 403 otherwise. This method is used for any endpoints that result in an action: editing template attributes, deleting templates or sending messages.
2019-03-19 16:25:44 +00:00
db_template = current_service.get_template_with_user_permission_or_403(template_id, current_user)
Show all pages of a letter everywhere Right now showing all the pages in full is the only way we have of showing a letter that makes sense to our users. Maybe in the future we show some kind of truncated version, but the end of the first page is not a good place to truncate the letter. This commit just extracts the code for showing multiple pages from the template view, refactors it for reuse, and includes it in the send views.
2017-05-11 12:01:51 +01:00
This PR adds the ability to select a text message sender if more than one exist for the service.
2017-11-02 12:07:46 +00:00
email_reply_to = None
sms_sender = None
if db_template['template_type'] == 'email':
Refactor settings page to use service model There’s a lot of code in service settings which: - talks to the API directly through the clients - passes that information through to the Jinja template By encapsulating this logic in the service model: - the Jinja template can access the data directly - the logic can be reused across multiple methods
2018-10-26 17:31:49 +01:00
email_reply_to = get_email_reply_to_address_from_session()
This PR adds the ability to select a text message sender if more than one exist for the service.
2017-11-02 12:07:46 +00:00
elif db_template['template_type'] == 'sms':
us → u.s.
2022-12-06 11:03:47 -05:00
sms_sender = get_sms_sender_from_session() # TODO: verify default sender is U.S. Notify
Include template values when calculating letter page count When sending a letter we check how many pages it has and this number then determines how many PNG images we ask template preview for. When calculating the page count, we were getting the page count for the template as it comes from the database (so without any placeholders filled in). But filling in placeholders in a letter may cause the number of pages to change, which was the cause of the 'Letter does not have a page x' errors we were seeing from template-preview. Now, when we calculate the letter page count during sending, we take the placeholders that have already been filled in into account.
2020-07-21 16:06:33 +01:00
template_values = get_recipient_and_placeholders_from_session(db_template['template_type'])
Incorporate breaking utils changes Brings in: - [ ] https://github.com/alphagov/notifications-utils/pull/94
2016-12-08 11:50:59 +00:00
template = get_template(
rename template var in send_test_step it's confusing reassigining one template (json from api) to another type (utils object) on one line. Also removed an unnecessary bounds check (since if placeholders is empty the IndexError will throw on the next line anyway and it'll be handled the same), and moved get_back_link out to its own function
2017-06-22 16:00:14 +01:00
db_template,
Incorporate breaking utils changes Brings in: - [ ] https://github.com/alphagov/notifications-utils/pull/94
2016-12-08 11:50:59 +00:00
current_service,
Add remaining endpoints for PDFs and PNGs Right now we can show what a letter template looks like as a PDF or PNG. This commit completes the work so this is also possible when: - showing a template with the placeholders replaced - showing any version of a template Also removes dependency on `Exception().message`, which was deprecated in Python 2.6. See https://github.com/alphagov/notifications-utils/commit/97f82d565f3112926752ae382e723b0808a3135d for full details.
2016-12-20 14:38:34 +00:00
show_recipient=True,
This PR adds the ability to select a text message sender if more than one exist for the service.
2017-11-02 12:07:46 +00:00
email_reply_to=email_reply_to,
sms_sender=sms_sender
Make send a test teach you how placeholders work This commit does two main things: - adds textboxes to the send yourself a test page, so you can replace ((name)) with ‘Chris’, or whatever your name is - rejigs the send page a bit to make it clearer what the CSV upload is for and how to use it The idea being that, since most users go into ‘send yourself a test’ first, it teaches them about how placeholders work by making them do the replacing themselves.
2016-05-05 08:39:36 +01:00
)
After sending sms or email to self or batch then then back button does not take user to check page, but rather the start of sending either sms or email.
2016-03-29 15:59:53 +01:00
Remove test/step- routes We don't need these anymore as all users will use the `one-off/step` routes. This has mostly involved tidying up the tests which are still a little disorganised and not as good as I'd like but it's a step in the right direction. More refactoring is still possible to the routes, it may come in a later PR if I have time.
2020-10-07 14:51:18 +01:00
placeholders = fields_to_fill_in(template)
Show one field at a time on send yourself a test The send yourself a test feature is useful for two things: - constructing an email/text message/letter without uploading a CSV file - seeing what the thing your going to send will look like (either by getting it in your inbox or downloading the PDF) - learning the concept of placeholders, ie understanding they’re thing that gets populated with _stuff_ The problem we’re seeing is that the current UI breaks when a template has a lot of placeholders. This is especially apparent with letter templates, which have a minimum of 7 placeholders by virtue of the address. The idea behind having the form fields side-by-side was to help people understand the relationship between their spreadsheet columns and the placeholders. But this means that the page was doing a lot of work, trying to teach: - replacement of placeholders - link between placeholders and spreadsheet columns The latter is better explained by the example spreadsheet shown on the upload page. So it can safely be removed from the send yourself a test page – in other words the fields don’t need to be shown side by side. Showing them one-at-a-time works well because: - it’s really obvious, even on first use, what the page is asking you to do - as your step through each placeholder, you see the message build up with the data you’ve entered – you’re learning how replacement of placeholders works by repetition This also means adding a matching endpoint for viewing each step of making the test letter as a PDF/PNG because we can’t reuse the view of the template without any placeholders filled any more.
2017-05-04 09:30:55 +01:00
Check that users can’t skip steps in send test Because we put the step in the URL, users could: - skip ahead to a later step - navigate to a step which doesn’t exist (ie an index greater than the number of placeholders) This commit adds some checks to do the sensible thing in the unlikely event that either of these situations occur.
2017-05-22 11:49:15 +01:00
try:
Fix order of steps in send/send test journeys This fixes the bug where if you have three placeholders: > ((one)) ((two)) ((three)) The first one you are asked to fill in is `((three))` (ie `template.placeholders[-1]`). This reintroduces a bug where if you use the ‘Use my phone number’ link you skip straight to filling in `((two))` and can never proceed because you’re never given the chance to fill in `((one))`. This commit also fixes that bug.
2019-06-06 12:24:02 +01:00
current_placeholder = placeholders[step_index]
Check that users can’t skip steps in send test Because we put the step in the URL, users could: - skip ahead to a later step - navigate to a step which doesn’t exist (ie an index greater than the number of placeholders) This commit adds some checks to do the sensible thing in the unlikely event that either of these situations occur.
2017-05-22 11:49:15 +01:00
except IndexError:
Make send one-off route ask for phone number/email This commit adds an extra, initial, step to the ‘send one-off message’ flow to ask for a phone number or email address. This is the first pass at making a feature which caseworkers or similar could use Notify to send individual messages while they’re working a case.
2017-05-25 09:18:26 +01:00
if all_placeholders_in_session(placeholders):
separate recipient from rest of placeholders when sending notification note: in the case of letters, we still want to create a CSV file. This only modifies the code flow when it's an email or template 😩 renamed `send_test_values` to `placeholders` because a) that's what they are and b) this isn't just for sending a test message any more
2017-06-23 17:30:34 +01:00
return get_notification_check_endpoint(service_id, template)
Check that users can’t skip steps in send test Because we put the step in the URL, users could: - skip ahead to a later step - navigate to a step which doesn’t exist (ie an index greater than the number of placeholders) This commit adds some checks to do the sensible thing in the unlikely event that either of these situations occur.
2017-05-22 11:49:15 +01:00
return redirect(url_for(
Remove test/step- routes We don't need these anymore as all users will use the `one-off/step` routes. This has mostly involved tidying up the tests which are still a little disorganised and not as good as I'd like but it's a step in the right direction. More refactoring is still possible to the routes, it may come in a later PR if I have time.
2020-10-07 14:51:18 +01:00
'.send_one_off',
Add a third route through populating a template This commit adds a route which is identical to send yourself a test, but with its own endpoint. This will let us add a slightly different ‘send a one-off message’ flow. This commit just adds the route though, and makes sure that the tests pass for both routes.
2017-05-25 08:55:05 +01:00
service_id=service_id,
template_id=template_id,
Check that users can’t skip steps in send test Because we put the step in the URL, users could: - skip ahead to a later step - navigate to a step which doesn’t exist (ie an index greater than the number of placeholders) This commit adds some checks to do the sensible thing in the unlikely event that either of these situations occur.
2017-05-22 11:49:15 +01:00
))
rename template var in send_test_step it's confusing reassigining one template (json from api) to another type (utils object) on one line. Also removed an unnecessary bounds check (since if placeholders is empty the IndexError will throw on the next line anyway and it'll be handled the same), and moved get_back_link out to its own function
2017-06-22 16:00:14 +01:00
Show one field at a time on send yourself a test The send yourself a test feature is useful for two things: - constructing an email/text message/letter without uploading a CSV file - seeing what the thing your going to send will look like (either by getting it in your inbox or downloading the PDF) - learning the concept of placeholders, ie understanding they’re thing that gets populated with _stuff_ The problem we’re seeing is that the current UI breaks when a template has a lot of placeholders. This is especially apparent with letter templates, which have a minimum of 7 placeholders by virtue of the address. The idea behind having the form fields side-by-side was to help people understand the relationship between their spreadsheet columns and the placeholders. But this means that the page was doing a lot of work, trying to teach: - replacement of placeholders - link between placeholders and spreadsheet columns The latter is better explained by the example spreadsheet shown on the upload page. So it can safely be removed from the send yourself a test page – in other words the fields don’t need to be shown side by side. Showing them one-at-a-time works well because: - it’s really obvious, even on first use, what the page is asking you to do - as your step through each placeholder, you see the message build up with the data you’ve entered – you’re learning how replacement of placeholders works by repetition This also means adding a matching endpoint for viewing each step of making the test letter as a PDF/PNG because we can’t reuse the view of the template without any placeholders filled any more.
2017-05-04 09:30:55 +01:00
form = get_placeholder_form_instance(
current_placeholder,
separate recipient from rest of placeholders when sending notification note: in the case of letters, we still want to create a CSV file. This only modifies the code flow when it's an email or template 😩 renamed `send_test_values` to `placeholders` because a) that's what they are and b) this isn't just for sending a test message any more
2017-06-23 17:30:34 +01:00
dict_to_populate_from=get_normalised_placeholders_from_session(),
Don’t validate phone numbers when sending emails If you have a placeholder called `((phone number))` in your email template, and you try to send a one-off message then the form input will attempt to validate your ‘phone number’. This is not helpful if you’re trying to put a landline number in your email, for example. This only affects messages being sent through the one-off interface. This commit makes the form be aware of template type, which fixes the problem.
2018-03-16 14:17:43 +00:00
template_type=template.template_type,
Make a service model and use for permissions Having the service floating about as JSON is a bit flakey. Could easily introduce a mistake where you mistype the name of a key and silently get `None`. Also means doing awkward things like `if 'permission' in current_service['permissions']`, whereas for users we can do the much cleaner `user.has_permission()`. So this commit: - introduces a model - adds a `.has_permission` method similar to the one we have for users
2018-07-20 08:43:02 +01:00
allow_international_phone_numbers=current_service.has_permission('international_sms'),
Show one field at a time on send yourself a test The send yourself a test feature is useful for two things: - constructing an email/text message/letter without uploading a CSV file - seeing what the thing your going to send will look like (either by getting it in your inbox or downloading the PDF) - learning the concept of placeholders, ie understanding they’re thing that gets populated with _stuff_ The problem we’re seeing is that the current UI breaks when a template has a lot of placeholders. This is especially apparent with letter templates, which have a minimum of 7 placeholders by virtue of the address. The idea behind having the form fields side-by-side was to help people understand the relationship between their spreadsheet columns and the placeholders. But this means that the page was doing a lot of work, trying to teach: - replacement of placeholders - link between placeholders and spreadsheet columns The latter is better explained by the example spreadsheet shown on the upload page. So it can safely be removed from the send yourself a test page – in other words the fields don’t need to be shown side by side. Showing them one-at-a-time works well because: - it’s really obvious, even on first use, what the page is asking you to do - as your step through each placeholder, you see the message build up with the data you’ve entered – you’re learning how replacement of placeholders works by repetition This also means adding a matching endpoint for viewing each step of making the test letter as a PDF/PNG because we can’t reuse the view of the template without any placeholders filled any more.
2017-05-04 09:30:55 +01:00
)
if form.validate_on_submit():
iron out kinks in recipient/placeholder handling where we were previously setting the placeholder when going through the send self a test - however, should be setting recipient. Also, only do this on step-0 of the one-off route, not the send-test route, since step-0 of send-test is the first normal placeholder. Phew!
2017-06-26 10:58:43 +01:00
# if it's the first input (phone/email), we store against `recipient` as well, for easier extraction.
big commit with letters removal
2022-12-05 15:33:44 -05:00
# Only if we're not on the test route, since that will already have the user's own number set
if step_index == 0:
separate recipient from rest of placeholders when sending notification note: in the case of letters, we still want to create a CSV file. This only modifies the code flow when it's an email or template 😩 renamed `send_test_values` to `placeholders` because a) that's what they are and b) this isn't just for sending a test message any more
2017-06-23 17:30:34 +01:00
session['recipient'] = form.placeholder_value.data
Show one field at a time on send yourself a test The send yourself a test feature is useful for two things: - constructing an email/text message/letter without uploading a CSV file - seeing what the thing your going to send will look like (either by getting it in your inbox or downloading the PDF) - learning the concept of placeholders, ie understanding they’re thing that gets populated with _stuff_ The problem we’re seeing is that the current UI breaks when a template has a lot of placeholders. This is especially apparent with letter templates, which have a minimum of 7 placeholders by virtue of the address. The idea behind having the form fields side-by-side was to help people understand the relationship between their spreadsheet columns and the placeholders. But this means that the page was doing a lot of work, trying to teach: - replacement of placeholders - link between placeholders and spreadsheet columns The latter is better explained by the example spreadsheet shown on the upload page. So it can safely be removed from the send yourself a test page – in other words the fields don’t need to be shown side by side. Showing them one-at-a-time works well because: - it’s really obvious, even on first use, what the page is asking you to do - as your step through each placeholder, you see the message build up with the data you’ve entered – you’re learning how replacement of placeholders works by repetition This also means adding a matching endpoint for viewing each step of making the test letter as a PDF/PNG because we can’t reuse the view of the template without any placeholders filled any more.
2017-05-04 09:30:55 +01:00
separate recipient from rest of placeholders when sending notification note: in the case of letters, we still want to create a CSV file. This only modifies the code flow when it's an email or template 😩 renamed `send_test_values` to `placeholders` because a) that's what they are and b) this isn't just for sending a test message any more
2017-06-23 17:30:34 +01:00
session['placeholders'][current_placeholder] = form.placeholder_value.data
Show one field at a time on send yourself a test The send yourself a test feature is useful for two things: - constructing an email/text message/letter without uploading a CSV file - seeing what the thing your going to send will look like (either by getting it in your inbox or downloading the PDF) - learning the concept of placeholders, ie understanding they’re thing that gets populated with _stuff_ The problem we’re seeing is that the current UI breaks when a template has a lot of placeholders. This is especially apparent with letter templates, which have a minimum of 7 placeholders by virtue of the address. The idea behind having the form fields side-by-side was to help people understand the relationship between their spreadsheet columns and the placeholders. But this means that the page was doing a lot of work, trying to teach: - replacement of placeholders - link between placeholders and spreadsheet columns The latter is better explained by the example spreadsheet shown on the upload page. So it can safely be removed from the send yourself a test page – in other words the fields don’t need to be shown side by side. Showing them one-at-a-time works well because: - it’s really obvious, even on first use, what the page is asking you to do - as your step through each placeholder, you see the message build up with the data you’ve entered – you’re learning how replacement of placeholders works by repetition This also means adding a matching endpoint for viewing each step of making the test letter as a PDF/PNG because we can’t reuse the view of the template without any placeholders filled any more.
2017-05-04 09:30:55 +01:00
Make send one-off route ask for phone number/email This commit adds an extra, initial, step to the ‘send one-off message’ flow to ask for a phone number or email address. This is the first pass at making a feature which caseworkers or similar could use Notify to send individual messages while they’re working a case.
2017-05-25 09:18:26 +01:00
if all_placeholders_in_session(placeholders):
separate recipient from rest of placeholders when sending notification note: in the case of letters, we still want to create a CSV file. This only modifies the code flow when it's an email or template 😩 renamed `send_test_values` to `placeholders` because a) that's what they are and b) this isn't just for sending a test message any more
2017-06-23 17:30:34 +01:00
return get_notification_check_endpoint(service_id, template)
Show one field at a time on send yourself a test The send yourself a test feature is useful for two things: - constructing an email/text message/letter without uploading a CSV file - seeing what the thing your going to send will look like (either by getting it in your inbox or downloading the PDF) - learning the concept of placeholders, ie understanding they’re thing that gets populated with _stuff_ The problem we’re seeing is that the current UI breaks when a template has a lot of placeholders. This is especially apparent with letter templates, which have a minimum of 7 placeholders by virtue of the address. The idea behind having the form fields side-by-side was to help people understand the relationship between their spreadsheet columns and the placeholders. But this means that the page was doing a lot of work, trying to teach: - replacement of placeholders - link between placeholders and spreadsheet columns The latter is better explained by the example spreadsheet shown on the upload page. So it can safely be removed from the send yourself a test page – in other words the fields don’t need to be shown side by side. Showing them one-at-a-time works well because: - it’s really obvious, even on first use, what the page is asking you to do - as your step through each placeholder, you see the message build up with the data you’ve entered – you’re learning how replacement of placeholders works by repetition This also means adding a matching endpoint for viewing each step of making the test letter as a PDF/PNG because we can’t reuse the view of the template without any placeholders filled any more.
2017-05-04 09:30:55 +01:00
Use `Spreadsheet` class to generate example CSVs This means not repeating a bunch of `with StringIO` blocks all over the place.
2016-05-15 10:47:52 +01:00
return redirect(url_for(
Add a third route through populating a template This commit adds a route which is identical to send yourself a test, but with its own endpoint. This will let us add a slightly different ‘send a one-off message’ flow. This commit just adds the route though, and makes sure that the tests pass for both routes.
2017-05-25 08:55:05 +01:00
request.endpoint,
Use `Spreadsheet` class to generate example CSVs This means not repeating a bunch of `with StringIO` blocks all over the place.
2016-05-15 10:47:52 +01:00
service_id=service_id,
Show one field at a time on send yourself a test The send yourself a test feature is useful for two things: - constructing an email/text message/letter without uploading a CSV file - seeing what the thing your going to send will look like (either by getting it in your inbox or downloading the PDF) - learning the concept of placeholders, ie understanding they’re thing that gets populated with _stuff_ The problem we’re seeing is that the current UI breaks when a template has a lot of placeholders. This is especially apparent with letter templates, which have a minimum of 7 placeholders by virtue of the address. The idea behind having the form fields side-by-side was to help people understand the relationship between their spreadsheet columns and the placeholders. But this means that the page was doing a lot of work, trying to teach: - replacement of placeholders - link between placeholders and spreadsheet columns The latter is better explained by the example spreadsheet shown on the upload page. So it can safely be removed from the send yourself a test page – in other words the fields don’t need to be shown side by side. Showing them one-at-a-time works well because: - it’s really obvious, even on first use, what the page is asking you to do - as your step through each placeholder, you see the message build up with the data you’ve entered – you’re learning how replacement of placeholders works by repetition This also means adding a matching endpoint for viewing each step of making the test letter as a PDF/PNG because we can’t reuse the view of the template without any placeholders filled any more.
2017-05-04 09:30:55 +01:00
template_id=template_id,
step_index=step_index + 1,
Use `Spreadsheet` class to generate example CSVs This means not repeating a bunch of `with StringIO` blocks all over the place.
2016-05-15 10:47:52 +01:00
))
Add flow for sending yourself a text message This commit adds a shortcut, which (in the background) does the creation and uploading of a CSV file for you. This enables users to send themselves a test message without having to fiddle about with CSV files.
2016-02-12 10:55:21 +00:00
Don’t send users back to a page that will send them forward again If you’re on a page with a normal placeholder and the previous placeholder is one that’s also in the address block then going back to the previous page will send you immediately forward to the current page again. This commit makes the back link a bit smarter by skipping over pages where it can see that they relate to a placeholder from the address block. If it gets all the way to the start of the list of placeholders without finding any non-address ones then it will default to generating a link that will redirect the user to filling in the address block again.
2020-04-09 16:25:48 +01:00
back_link = get_back_link(service_id, template, step_index, placeholders)
Show one field at a time on send yourself a test The send yourself a test feature is useful for two things: - constructing an email/text message/letter without uploading a CSV file - seeing what the thing your going to send will look like (either by getting it in your inbox or downloading the PDF) - learning the concept of placeholders, ie understanding they’re thing that gets populated with _stuff_ The problem we’re seeing is that the current UI breaks when a template has a lot of placeholders. This is especially apparent with letter templates, which have a minimum of 7 placeholders by virtue of the address. The idea behind having the form fields side-by-side was to help people understand the relationship between their spreadsheet columns and the placeholders. But this means that the page was doing a lot of work, trying to teach: - replacement of placeholders - link between placeholders and spreadsheet columns The latter is better explained by the example spreadsheet shown on the upload page. So it can safely be removed from the send yourself a test page – in other words the fields don’t need to be shown side by side. Showing them one-at-a-time works well because: - it’s really obvious, even on first use, what the page is asking you to do - as your step through each placeholder, you see the message build up with the data you’ve entered – you’re learning how replacement of placeholders works by repetition This also means adding a matching endpoint for viewing each step of making the test letter as a PDF/PNG because we can’t reuse the view of the template without any placeholders filled any more.
2017-05-04 09:30:55 +01:00
Include template values when calculating letter page count When sending a letter we check how many pages it has and this number then determines how many PNG images we ask template preview for. When calculating the page count, we were getting the page count for the template as it comes from the database (so without any placeholders filled in). But filling in placeholders in a letter may cause the number of pages to change, which was the cause of the 'Letter does not have a page x' errors we were seeing from template-preview. Now, when we calculate the letter page count during sending, we take the placeholders that have already been filled in into account.
2020-07-21 16:06:33 +01:00
template.values = template_values
Unfill current placeholder if revisiting page If we don’t do this then you can’t see where in the composed message the value for your placeholder will appear.
2017-05-16 17:17:11 +01:00
template.values[current_placeholder] = None
Show one field at a time on send yourself a test The send yourself a test feature is useful for two things: - constructing an email/text message/letter without uploading a CSV file - seeing what the thing your going to send will look like (either by getting it in your inbox or downloading the PDF) - learning the concept of placeholders, ie understanding they’re thing that gets populated with _stuff_ The problem we’re seeing is that the current UI breaks when a template has a lot of placeholders. This is especially apparent with letter templates, which have a minimum of 7 placeholders by virtue of the address. The idea behind having the form fields side-by-side was to help people understand the relationship between their spreadsheet columns and the placeholders. But this means that the page was doing a lot of work, trying to teach: - replacement of placeholders - link between placeholders and spreadsheet columns The latter is better explained by the example spreadsheet shown on the upload page. So it can safely be removed from the send yourself a test page – in other words the fields don’t need to be shown side by side. Showing them one-at-a-time works well because: - it’s really obvious, even on first use, what the page is asking you to do - as your step through each placeholder, you see the message build up with the data you’ve entered – you’re learning how replacement of placeholders works by repetition This also means adding a matching endpoint for viewing each step of making the test letter as a PDF/PNG because we can’t reuse the view of the template without any placeholders filled any more.
2017-05-04 09:30:55 +01:00
Make send a test teach you how placeholders work This commit does two main things: - adds textboxes to the send yourself a test page, so you can replace ((name)) with ‘Chris’, or whatever your name is - rejigs the send page a bit to make it clearer what the CSV upload is for and how to use it The idea being that, since most users go into ‘send yourself a test’ first, it teaches them about how placeholders work by making them do the replacing themselves.
2016-05-05 08:39:36 +01:00
return render_template(
'views/send-test.html',
Improve page titles later on in the one off flow Now that the page title for picking a sender/reply to has been improved, I think these pages are also less clear than they could be. This commit changes the page titles to (I hope) be clearer about what is needed from the user on these pages.
2017-11-20 16:45:59 +00:00
page_title=get_send_test_page_title(
template.template_type,
Make it clear which template you’re sending The one downside of skipping the template page is that you no longer get such strong confirmation that you’ve picked the correct template. You still see the preview of the template, but it’s further down the page, and the name of the template has disappeared. This commit adds the name of the template to the page title, to: - have some continuity from the previous page - make it easier to double-check you’ve chosen the correct template
2018-08-06 11:20:50 +01:00
entering_recipient=not session['recipient'],
name=template.name,
Improve page titles later on in the one off flow Now that the page title for picking a sender/reply to has been improved, I think these pages are also less clear than they could be. This commit changes the page titles to (I hope) be clearer about what is needed from the user on these pages.
2017-11-20 16:45:59 +00:00
),
Make send a test teach you how placeholders work This commit does two main things: - adds textboxes to the send yourself a test page, so you can replace ((name)) with ‘Chris’, or whatever your name is - rejigs the send page a bit to make it clearer what the CSV upload is for and how to use it The idea being that, since most users go into ‘send yourself a test’ first, it teaches them about how placeholders work by making them do the replacing themselves.
2016-05-05 08:39:36 +01:00
template=template,
Show one field at a time on send yourself a test The send yourself a test feature is useful for two things: - constructing an email/text message/letter without uploading a CSV file - seeing what the thing your going to send will look like (either by getting it in your inbox or downloading the PDF) - learning the concept of placeholders, ie understanding they’re thing that gets populated with _stuff_ The problem we’re seeing is that the current UI breaks when a template has a lot of placeholders. This is especially apparent with letter templates, which have a minimum of 7 placeholders by virtue of the address. The idea behind having the form fields side-by-side was to help people understand the relationship between their spreadsheet columns and the placeholders. But this means that the page was doing a lot of work, trying to teach: - replacement of placeholders - link between placeholders and spreadsheet columns The latter is better explained by the example spreadsheet shown on the upload page. So it can safely be removed from the send yourself a test page – in other words the fields don’t need to be shown side by side. Showing them one-at-a-time works well because: - it’s really obvious, even on first use, what the page is asking you to do - as your step through each placeholder, you see the message build up with the data you’ve entered – you’re learning how replacement of placeholders works by repetition This also means adding a matching endpoint for viewing each step of making the test letter as a PDF/PNG because we can’t reuse the view of the template without any placeholders filled any more.
2017-05-04 09:30:55 +01:00
form=form,
Factor out skip link to reduce complexity The code was too complex and Flake8 was complaining. While this code isn’t related to the change we’re making it is easy enough to factor out.
2020-04-09 16:37:38 +01:00
skip_link=get_skip_link(step_index, template),
Show one field at a time on send yourself a test The send yourself a test feature is useful for two things: - constructing an email/text message/letter without uploading a CSV file - seeing what the thing your going to send will look like (either by getting it in your inbox or downloading the PDF) - learning the concept of placeholders, ie understanding they’re thing that gets populated with _stuff_ The problem we’re seeing is that the current UI breaks when a template has a lot of placeholders. This is especially apparent with letter templates, which have a minimum of 7 placeholders by virtue of the address. The idea behind having the form fields side-by-side was to help people understand the relationship between their spreadsheet columns and the placeholders. But this means that the page was doing a lot of work, trying to teach: - replacement of placeholders - link between placeholders and spreadsheet columns The latter is better explained by the example spreadsheet shown on the upload page. So it can safely be removed from the send yourself a test page – in other words the fields don’t need to be shown side by side. Showing them one-at-a-time works well because: - it’s really obvious, even on first use, what the page is asking you to do - as your step through each placeholder, you see the message build up with the data you’ve entered – you’re learning how replacement of placeholders works by repetition This also means adding a matching endpoint for viewing each step of making the test letter as a PDF/PNG because we can’t reuse the view of the template without any placeholders filled any more.
2017-05-04 09:30:55 +01:00
back_link=back_link,
Offer link to upload ‘Upload recipients’ and ‘Send to one recipient’ have always been slightly clunky phrases. Now that basic view jumps straight into the ‘Send to one recipient’ flow there’s no way for users to get to the ‘Upload recipients’ flow. By adding a link to it from the ‘Send to one recipient’ flow it’s possible for users of basic view to access it. But we don’t want to introduce too much inconsistency between basic view and admin view because users will be migrating from one to another. They might also be talking to their manager, who wouldn’t be able to tell them where to click if they were looking at two completely different interfaces. This also means that we can keep the left-hand navigation in basic view nice and simple with the two options (‘Templates’ and ‘Sent messages’), rather than trying to introduce something like ‘Send one message’ and ‘Send lots of messages’ later on.
2018-07-18 09:48:19 +01:00
link_to_upload=(
Fix error where sending test message flow skipped first placeholder first way round and then collected placeholders again. Now the flow collects all placeholders in one round. Also fix the back link for step-1 for test flow so it goes back to choosing recipient number Also move operators around following flake8's advice :)
2019-05-28 17:44:27 +01:00
request.endpoint == 'main.send_one_off_step'
and step_index == 0
Offer link to upload ‘Upload recipients’ and ‘Send to one recipient’ have always been slightly clunky phrases. Now that basic view jumps straight into the ‘Send to one recipient’ flow there’s no way for users to get to the ‘Upload recipients’ flow. By adding a link to it from the ‘Send to one recipient’ flow it’s possible for users of basic view to access it. But we don’t want to introduce too much inconsistency between basic view and admin view because users will be migrating from one to another. They might also be talking to their manager, who wouldn’t be able to tell them where to click if they were looking at two completely different interfaces. This also means that we can keep the left-hand navigation in basic view nice and simple with the two options (‘Templates’ and ‘Sent messages’), rather than trying to introduce something like ‘Send one message’ and ‘Send lots of messages’ later on.
2018-07-18 09:48:19 +01:00
),
Make send a test teach you how placeholders work This commit does two main things: - adds textboxes to the send yourself a test page, so you can replace ((name)) with ‘Chris’, or whatever your name is - rejigs the send page a bit to make it clearer what the CSV upload is for and how to use it The idea being that, since most users go into ‘send yourself a test’ first, it teaches them about how placeholders work by making them do the replacing themselves.
2016-05-05 08:39:36 +01:00
)
Add flow for sending yourself a text message This commit adds a shortcut, which (in the background) does the creation and uploading of a CSV file for you. This enables users to send themselves a test message without having to fiddle about with CSV files.
2016-02-12 10:55:21 +00:00
big commit with letters removal
2022-12-05 15:33:44 -05:00
def _check_messages(service_id, template_id, upload_id, preview_row):
in send flow replace suppress with try catch suppress was suppressing 404 errors (the happy path) - but it was also suppressing 503s from tests where we hadn't mocked endpoints
2018-05-03 13:35:59 +01:00
try:
Don’t load the check page if a job exists already If a user clicks ‘back’ once they’ve sent a job we don’t want them to land on the ‘check’ page again. This would suggest that they can send the same job again (they can’t because that `job_id` is in the database already). That said, it’s confusing to see that page; the natural thing is to go jump back another step, to where you uploaded the file.
2018-04-30 12:55:56 +01:00
# The happy path is that the job doesn’t already exist, so the
# API will return a 404 and the client will raise HTTPError.
job_api_client.get_job(service_id, upload_id)
2767 passing
2022-08-05 00:25:03 -07:00
in send flow replace suppress with try catch suppress was suppressing 404 errors (the happy path) - but it was also suppressing 503s from tests where we hadn't mocked endpoints
2018-05-03 13:35:59 +01:00
# the job exists already - so go back to the templates page
Don’t load the check page if a job exists already If a user clicks ‘back’ once they’ve sent a job we don’t want them to land on the ‘check’ page again. This would suggest that they can send the same job again (they can’t because that `job_id` is in the database already). That said, it’s confusing to see that page; the natural thing is to go jump back another step, to where you uploaded the file.
2018-04-30 12:55:56 +01:00
# If we just return a `redirect` (302) object here, we'll get
# errors when we try and unpack in the check_messages route.
# Rasing a werkzeug.routing redirect means that doesn't happen.
Upgrade pyexcel-io, which also upgrades Werkzeug Upgraded pyexcel-io from 0.5.14 to 0.5.16. This change causes Werkzeug to be upgraded from 0.14.1 to 0.15.1 which requires some changes: * ProxyFix now needs to be imported from a different location * The status code of RequestRedirect has changed from 301 to 308. Since status code 308 is not currently supported on Internet Explorer with Windows 7 and 8.1, this subclasses RequestRedirect to keep the status code of 301. changelog: https://werkzeug.palletsprojects.com/en/0.15.x/changes/#version-0-15-0
2019-03-21 16:41:22 +00:00
raise PermanentRedirect(url_for(
change letter previews to be in the no_cookies blueprint this blueprint should be applied to every endpoint that is loaded async (as in via a src tag on an img, iframe, etc)
2019-11-29 11:40:49 +00:00
'main.send_messages',
Don’t load the check page if a job exists already If a user clicks ‘back’ once they’ve sent a job we don’t want them to land on the ‘check’ page again. This would suggest that they can send the same job again (they can’t because that `job_id` is in the database already). That said, it’s confusing to see that page; the natural thing is to go jump back another step, to where you uploaded the file.
2018-04-30 12:55:56 +01:00
service_id=service_id,
template_id=template_id
))
in send flow replace suppress with try catch suppress was suppressing 404 errors (the happy path) - but it was also suppressing 503s from tests where we hadn't mocked endpoints
2018-05-03 13:35:59 +01:00
except HTTPError as e:
if e.status_code != 404:
raise
Don’t load the check page if a job exists already If a user clicks ‘back’ once they’ve sent a job we don’t want them to land on the ‘check’ page again. This would suggest that they can send the same job again (they can’t because that `job_id` is in the database already). That said, it’s confusing to see that page; the natural thing is to go jump back another step, to where you uploaded the file.
2018-04-30 12:55:56 +01:00
To check the daily limit get the daily notification_count from redis. The daily limit cache is set by the api when a notification is created. There is one cache key per service per day and it expires after 24 hours.
2021-06-23 15:56:05 +01:00
notification_count = service_api_client.get_notification_count(service_id)
remaining_messages = (current_service.message_limit - notification_count)
Show error if restricted service uploads real CSV Implements: https://github.com/alphagov/notifications-utils/pull/16 Also changes a bunch of the mocked phone numbers to be real-looking so that the tests are checking for the right kind of error.
2016-04-04 14:28:52 +01:00
Give the user better error messages for CSV files Makes uses of the additions to utils in https://github.com/alphagov/notifications-utils/pull/9 This commit strips out a lot of the complex stuff that the views and templates in this app were doing. There is now a cleaner separation of concerns: - utils returns the number and type of errors in the csv - `get_errors_for_csv` helper in this app maps the number and type of errors onto human-friendly error messages - the view and template just doing the glueing-together of all the pieces This is (hopefully) easier to understand, definitely makes the component parts easier to test in isolation, and makes it easier to give more specific error messages.
2016-03-07 18:47:05 +00:00
contents = s3download(service_id, upload_id)
Store template ID and original file name in URL At the moment you can’t press refresh on the check page if there’s errors. This is because the session gets cleared when there’s errors. This is a bad user experience. The data that this page is relying on (from the session) is: - template ID - original file name Neither of these things need to be in the session because: - they are not secret - the user can modify them already (by choosing a different template or renaming their file locally) So this commit additionally stores them in the URL.
2018-03-29 17:01:35 +01:00
Replace sevice api client get template calls with Service methods Instead of using the API client directly views are now calling one of two Service model methods: `get_template` is used for view actions, where the user should see the template page even if they don't have access to the template folder (since all templates are still inked from the dashboard or the sent notifications pages). `get_template_with_user_permission_or_403` will check if the user has access to the template's folder first and return 403 otherwise. This method is used for any endpoints that result in an action: editing template attributes, deleting templates or sending messages.
2019-03-19 16:25:44 +00:00
db_template = current_service.get_template_with_user_permission_or_403(template_id, current_user)
Store template ID and original file name in URL At the moment you can’t press refresh on the check page if there’s errors. This is because the session gets cleared when there’s errors. This is a bad user experience. The data that this page is relying on (from the session) is: - template ID - original file name Neither of these things need to be in the session because: - they are not secret - the user can modify them already (by choosing a different template or renaming their file locally) So this commit additionally stores them in the URL.
2018-03-29 17:01:35 +01:00
This PR adds the ability to select a text message sender if more than one exist for the service.
2017-11-02 12:07:46 +00:00
email_reply_to = None
sms_sender = None
Store template ID and original file name in URL At the moment you can’t press refresh on the check page if there’s errors. This is because the session gets cleared when there’s errors. This is a bad user experience. The data that this page is relying on (from the session) is: - template ID - original file name Neither of these things need to be in the session because: - they are not secret - the user can modify them already (by choosing a different template or renaming their file locally) So this commit additionally stores them in the URL.
2018-03-29 17:01:35 +01:00
if db_template['template_type'] == 'email':
Refactor settings page to use service model There’s a lot of code in service settings which: - talks to the API directly through the clients - passes that information through to the Jinja template By encapsulating this logic in the service model: - the Jinja template can access the data directly - the logic can be reused across multiple methods
2018-10-26 17:31:49 +01:00
email_reply_to = get_email_reply_to_address_from_session()
Store template ID and original file name in URL At the moment you can’t press refresh on the check page if there’s errors. This is because the session gets cleared when there’s errors. This is a bad user experience. The data that this page is relying on (from the session) is: - template ID - original file name Neither of these things need to be in the session because: - they are not secret - the user can modify them already (by choosing a different template or renaming their file locally) So this commit additionally stores them in the URL.
2018-03-29 17:01:35 +01:00
elif db_template['template_type'] == 'sms':
Refactor settings page to use service model There’s a lot of code in service settings which: - talks to the API directly through the clients - passes that information through to the Jinja template By encapsulating this logic in the service model: - the Jinja template can access the data directly - the logic can be reused across multiple methods
2018-10-26 17:31:49 +01:00
sms_sender = get_sms_sender_from_session()
Check page count of actual notification not of template But for jobs we are only checking preview row, otherwise it would be too slow. We will check other row when creating the pdf
2019-10-08 14:56:00 +01:00
Incorporate breaking utils changes Brings in: - [ ] https://github.com/alphagov/notifications-utils/pull/94
2016-12-08 11:50:59 +00:00
template = get_template(
Remove redundant call to `.get_template` Every call to the API is a blocking network request.
2018-05-10 17:00:41 +01:00
db_template,
Incorporate breaking utils changes Brings in: - [ ] https://github.com/alphagov/notifications-utils/pull/94
2016-12-08 11:50:59 +00:00
current_service,
Add remaining endpoints for PDFs and PNGs Right now we can show what a letter template looks like as a PDF or PNG. This commit completes the work so this is also possible when: - showing a template with the placeholders replaced - showing any version of a template Also removes dependency on `Exception().message`, which was deprecated in Python 2.6. See https://github.com/alphagov/notifications-utils/commit/97f82d565f3112926752ae382e723b0808a3135d for full details.
2016-12-20 14:38:34 +00:00
show_recipient=True,
This PR adds the ability to select a text message sender if more than one exist for the service.
2017-11-02 12:07:46 +00:00
email_reply_to=email_reply_to,
Store template ID and original file name in URL At the moment you can’t press refresh on the check page if there’s errors. This is because the session gets cleared when there’s errors. This is a bad user experience. The data that this page is relying on (from the session) is: - template ID - original file name Neither of these things need to be in the session because: - they are not secret - the user can modify them already (by choosing a different template or renaming their file locally) So this commit additionally stores them in the URL.
2018-03-29 17:01:35 +01:00
sms_sender=sms_sender,
Give the user better error messages for CSV files Makes uses of the additions to utils in https://github.com/alphagov/notifications-utils/pull/9 This commit strips out a lot of the complex stuff that the views and templates in this app were doing. There is now a cleaner separation of concerns: - utils returns the number and type of errors in the csv - `get_errors_for_csv` helper in this app maps the number and type of errors onto human-friendly error messages - the view and template just doing the glueing-together of all the pieces This is (hopefully) easier to understand, definitely makes the component parts easier to test in isolation, and makes it easier to give more specific error messages.
2016-03-07 18:47:05 +00:00
)
recipients = RecipientCSV(
contents,
Validate CSVs against rows with empty messages
2019-11-21 12:12:04 +00:00
template=template,
Increase number of rows shown We probably shouldn’t hide the contents of the CSV when people are just testing the app, or if they’re starting off with small jobs. A limit of 15 rows displayed was awkwardly on the cusp between just testing and sending a small batch. This commit increases the limit to 50; I reckon that over 50 recipients no-one will be wanting to check them all individually.
2016-04-07 14:30:51 +01:00
max_initial_rows_shown=50,
max_errors_shown=50,
Bump utils to 51.2.1 This brings a few performance improvements for RecipientCSV, which we use to preview and process CSVs. One change also renames one of the attributes for the class to "guestlist".
2021-12-10 16:35:40 +00:00
guestlist=itertools.chain.from_iterable(
Make user API client return JSON, not a model The data flow of other bits of our application looks like this: ``` API (returns JSON) ⬇ API client (returns a built in type, usually `dict`) ⬇ Model (returns an instance, eg of type `Service`) ⬇ View (returns HTML) ``` The user API client was architected weirdly, in that it returned a model directly, like this: ``` API (returns JSON) ⬇ API client (returns a model, of type `User`, `InvitedUser`, etc) ⬇ View (returns HTML) ``` This mixing of different layers of the application is bad because it makes it hard to write model code that doesn’t have circular dependencies. As our application gets more complicated we will be relying more on models to manage this complexity, so we should make it easy, not hard to write them. It also means that most of our mocking was of the User model, not just the underlying JSON. So it would have been easy to introduce subtle bugs to the user model, because it wasn’t being comprehensively tested. A lot of the changed lines of code in this commit mean changing the tests to mock only the JSON, which means that the model layer gets implicitly tested. For those reasons this commit changes the user API client to return JSON, not an instance of `User` or other models.
2019-05-23 15:27:35 +01:00
[user.name, user.mobile_number, user.email_address] for user in Users(service_id)
Use service model to look up service attributes This is better than just keying into the JSON because it means you get an exception straight away when looking up a key that doesn’t exist (which via mocking you could ordinarily miss).
2018-07-20 08:42:01 +01:00
) if current_service.trial_mode else None,
Allow international phone numbers in spreadsheet If a service can send internationally, our CSV validation should not catch valid international phone numbers. This means calling through to code added to utils in: - [ ] https://github.com/alphagov/notifications-utils/pull/156
2017-04-26 16:19:49 +01:00
remaining_messages=remaining_messages,
Allow international addresses in spreadsheets For services with permission, they can now put international addresses into their spreadsheets without getting a postcode error. This also means they can start using address line 7 instead of postcode, since it doesn’t make sense to put a country in a field called ‘postcode’. But this will be undocumented to start with, because we’re not giving any real users the permission. It does now mean that the number of possible placeholders (7 + postcode) is greater than the number of allowed placeholders (7), so we have to account for that in the one-off address flow where we’re populating the placeholders automatically. We’re sticking with 6 + postcode here for backwards compatibility.
2020-04-27 10:57:25 +01:00
allow_international_sms=current_service.has_permission('international_sms'),
Removed moto mocks and mock s3 upload and download directly. Also updated csv reading to use correct module and do more work to remove empty fields and trailing commas.
2016-02-26 10:54:06 +00:00
)
Give the user better error messages for CSV files Makes uses of the additions to utils in https://github.com/alphagov/notifications-utils/pull/9 This commit strips out a lot of the complex stuff that the views and templates in this app were doing. There is now a cleaner separation of concerns: - utils returns the number and type of errors in the csv - `get_errors_for_csv` helper in this app maps the number and type of errors onto human-friendly error messages - the view and template just doing the glueing-together of all the pieces This is (hopefully) easier to understand, definitely makes the component parts easier to test in isolation, and makes it easier to give more specific error messages.
2016-03-07 18:47:05 +00:00
Move ‘send test’ link back to template page This changes it back to how it was when we first introduced this feature: https://github.com/alphagov/notifications-admin/pull/181 It’s kind of lost on the page where you upload a file, which is a shame because it’s a good teaching aid.
2016-07-01 07:04:55 +01:00
if request.args.get('from_test'):
big commit with letters removal
2022-12-05 15:33:44 -05:00
# TODO: may not be required after letters code removed
Remove test/step- routes We don't need these anymore as all users will use the `one-off/step` routes. This has mostly involved tidying up the tests which are still a little disorganised and not as good as I'd like but it's a step in the right direction. More refactoring is still possible to the routes, it may come in a later PR if I have time.
2020-10-07 14:51:18 +01:00
back_link = url_for('main.send_one_off', service_id=service_id, template_id=template.id)
Don’t allow test messages to be scheduled For two reasons: - it’s extra stuff in tour that users dont yet need to know about - test messages are hidden from the dashboard, so you’d have no visibility of when they were sending once you’d scheduled them
2016-08-30 09:27:24 +01:00
choose_time_form = None
Make send a test teach you how placeholders work This commit does two main things: - adds textboxes to the send yourself a test page, so you can replace ((name)) with ‘Chris’, or whatever your name is - rejigs the send page a bit to make it clearer what the CSV upload is for and how to use it The idea being that, since most users go into ‘send yourself a test’ first, it teaches them about how placeholders work by making them do the replacing themselves.
2016-05-05 08:39:36 +01:00
else:
change letter previews to be in the no_cookies blueprint this blueprint should be applied to every endpoint that is loaded async (as in via a src tag on an img, iframe, etc)
2019-11-29 11:40:49 +00:00
back_link = url_for('main.send_messages', service_id=service_id, template_id=template.id)
Don’t allow test messages to be scheduled For two reasons: - it’s extra stuff in tour that users dont yet need to know about - test messages are hidden from the dashboard, so you’d have no visibility of when they were sending once you’d scheduled them
2016-08-30 09:27:24 +01:00
choose_time_form = ChooseTimeForm()
Make send a test teach you how placeholders work This commit does two main things: - adds textboxes to the send yourself a test page, so you can replace ((name)) with ‘Chris’, or whatever your name is - rejigs the send page a bit to make it clearer what the CSV upload is for and how to use it The idea being that, since most users go into ‘send yourself a test’ first, it teaches them about how placeholders work by making them do the replacing themselves.
2016-05-05 08:39:36 +01:00
Make row in URL match displayed row Spreadsheets start at row 1 (the header row), and the values don’t start until row 2. The row numbers in our URLs start at 0, which is a concept that only makes sense to programmers. It’s more predictable and consistent to make the number in the URL match the row number displayed on the page when previewing the spreadsheet.
2018-01-15 11:35:45 +00:00
if preview_row < 2:
abort(404)
Bring in refactored CSV processing Shouldn’t be any functional changes here, just things being named more clearly.
2018-03-05 15:57:10 +00:00
if preview_row < len(recipients) + 2:
template.values = recipients[preview_row - 2].recipient_and_personalisation
Make row in URL match displayed row Spreadsheets start at row 1 (the header row), and the values don’t start until row 2. The row numbers in our URLs start at 0, which is a concept that only makes sense to programmers. It’s more predictable and consistent to make the number in the URL match the row number displayed on the page when previewing the spreadsheet.
2018-01-15 11:35:45 +00:00
elif preview_row > 2:
Rewrite check for row existence as conditional Because exceptions can be expensive performance wise (see: https://docs.python.org/3/faq/design.html#how-fast-are-exceptions). Since we’re counting the number of rows anyway this doesn’t introduce any performance overhead there. And I think it’s equally readable/same number of lines of code.
2017-12-20 11:48:30 +00:00
abort(404)
Give the user better error messages for CSV files Makes uses of the additions to utils in https://github.com/alphagov/notifications-utils/pull/9 This commit strips out a lot of the complex stuff that the views and templates in this app were doing. There is now a cleaner separation of concerns: - utils returns the number and type of errors in the csv - `get_errors_for_csv` helper in this app maps the number and type of errors onto human-friendly error messages - the view and template just doing the glueing-together of all the pieces This is (hopefully) easier to understand, definitely makes the component parts easier to test in isolation, and makes it easier to give more specific error messages.
2016-03-07 18:47:05 +00:00
Stop checking query string for filename when sending a job We look for `original_file_name` in the metadata now. Initially we were still checking the query string too, but now that the change to add the filename to the metadata has been deployed for a while there shouldn't be any cases of the filename still being in a query string. Since the `original_file_name` is not being added to the metadata in `.check_messages` (it has happened earlier in the process) a few tests are no longer needed.
2020-10-27 10:52:16 +00:00
original_file_name = get_csv_metadata(service_id, upload_id).get('original_file_name', '')
Check page count of actual notification not of template But for jobs we are only checking preview row, otherwise it would be too slow. We will check other row when creating the pdf
2019-10-08 14:56:00 +01:00
Split endpoints that use templates We’ll need to reuse the logic for viewing images and PDFs of templates, so let’s have it in a shared method which the HTML endpoint then calls.
2016-12-20 11:56:22 +00:00
return dict(
Give the user better error messages for CSV files Makes uses of the additions to utils in https://github.com/alphagov/notifications-utils/pull/9 This commit strips out a lot of the complex stuff that the views and templates in this app were doing. There is now a cleaner separation of concerns: - utils returns the number and type of errors in the csv - `get_errors_for_csv` helper in this app maps the number and type of errors onto human-friendly error messages - the view and template just doing the glueing-together of all the pieces This is (hopefully) easier to understand, definitely makes the component parts easier to test in isolation, and makes it easier to give more specific error messages.
2016-03-07 18:47:05 +00:00
recipients=recipients,
template=template,
Change order of errors for bad CSV files This commit rearranges the CSV errors (again) to make them geared towards teaching you how to match placeholders to the column headers. So the order of errors now is: 1. No phone number/email column 2. Column headers don’t match placeholders 3. Missing or bad data 4. Trial mode 5. Daily limit reached This depends on: - [x] https://github.com/alphagov/notifications-utils/pull/39 for 1.
2016-06-03 13:52:15 +01:00
errors=recipients.has_errors,
row_errors=get_errors_for_csv(recipients, template.template_type),
Bring in refactored CSV processing Shouldn’t be any functional changes here, just things being named more clearly.
2018-03-05 15:57:10 +00:00
count_of_recipients=len(recipients),
count_of_displayed_recipients=len(list(recipients.displayed_rows)),
Check metadata for file name when sending from contact list The `.send_from_contact_list` function redirected to `.check_messages` with `original_file_name` in the query string. Contact lists already have `original_file_name` as part of their metadata, so we can stop sending it in the query string and use the metadata instead.
2020-10-21 13:07:59 +01:00
original_file_name=original_file_name,
After sending sms or email to self or batch then then back button does not take user to check page, but rather the start of sending either sms or email.
2016-03-29 15:59:53 +01:00
upload_id=upload_id,
Show error if you try to send too many messages https://www.pivotaltracker.com/story/show/117630691 There is a limit of 50 messages per day in trial mode. Right now, we don’t tell you this, we just start failing your messages. This commit adds an error message if you upload a CSV file that has too many rows in it.
2016-04-22 09:36:42 +01:00
form=CsvUploadForm(),
request stats for today from send page from GET /service/:id endpoint also amended test cases to ensure they mock out correct call
2016-07-25 14:16:34 +01:00
remaining_messages=remaining_messages,
Don’t allow test messages to be scheduled For two reasons: - it’s extra stuff in tour that users dont yet need to know about - test messages are hidden from the dashboard, so you’d have no visibility of when they were sending once you’d scheduled them
2016-08-30 09:27:24 +01:00
choose_time_form=choose_time_form,
Make a function for parsing `help` query param Our templates are a littered with `request.args.get('help', '0')`. This commit refactors these into a single helper method, which can be used by the view functions, then passed to the template. This makes the templates cleaner, and should make it easier to refactor `help` out of the query parameters entirely in the future.
2016-07-05 11:39:07 +01:00
back_link=back_link,
Explain 3 required address columns Our rules about address columns are relaxing, so that none of them are mandatory any more. Instead you just need any 3 of the 7 to make a valid address. This commit updates our error messaging to reflect that.
2020-04-19 22:46:13 +01:00
first_recipient_column=recipients.recipient_column_headers[0],
Link each row in the spreadsheet We’ve heard from some users, especially those sending letters, that they’d like to check that a spreadsheet they’ve uploaded has populated the template correctly. My reckon is that seeing just one row of the spreadsheet populate the template isn’t enough to give people confidence that everything’s working properly. This commit adds links to all but the currently-previewed row. Clicking that link will populate the preview with values from that row. These pages already exist; they were created in this PR: https://github.com/alphagov/notifications-admin/pull/1696
2018-01-15 11:25:11 +00:00
preview_row=preview_row,
Warn if a file has been sent already We have some teams who haver a series of files they have to send each day. It’s easy to get muddled up and accidentally send the same file again, if you think you haven’t already sent it. This commit blocks you from sending the same combination of template version and filename more than once on the same day[1]. This won’t affect teams who re-use the same template to give (for example) updates on an incident for business continuity. These teams edit the template between each send, thereby updating the version number of the template. 1. This is based on how the `limit_days` argument to the API works - you can dig into the code here: https://github.com/alphagov/notifications-api/blob/2bd4f74ad0e24f7066e1a6bd4d0e515b4b925e10/app/dao/jobs_dao.py#L50
2019-02-04 14:08:11 +00:00
sent_previously=job_api_client.has_sent_previously(
Check metadata for file name when sending from contact list The `.send_from_contact_list` function redirected to `.check_messages` with `original_file_name` in the query string. Contact lists already have `original_file_name` as part of their metadata, so we can stop sending it in the query string and use the metadata instead.
2020-10-21 13:07:59 +01:00
service_id, template.id, db_template['version'], original_file_name
Do not allow to send job of letters if letters longer than 10 pages
2019-10-01 15:34:46 +01:00
),
Give the user better error messages for CSV files Makes uses of the additions to utils in https://github.com/alphagov/notifications-utils/pull/9 This commit strips out a lot of the complex stuff that the views and templates in this app were doing. There is now a cleaner separation of concerns: - utils returns the number and type of errors in the csv - `get_errors_for_csv` helper in this app maps the number and type of errors onto human-friendly error messages - the view and template just doing the glueing-together of all the pieces This is (hopefully) easier to understand, definitely makes the component parts easier to test in isolation, and makes it easier to give more specific error messages.
2016-03-07 18:47:05 +00:00
)
Require IDs to be UUIDs in URLS We mostly rely on the API returning a 404 to generate 404s for trying to get things with non-UUID IDs. This is fine, except our tests often mock these API calls. So it could look like everything is working fine, except the thing your passing in might never be a valid UUID, and thus would 404 in a non-test environment. So this commit: 1. uses the `uuid` URL converter everywhere there’s something that looks like an ID in a URL parameter 2. adds a test which automates checking for 1.
2019-11-04 11:07:55 +00:00
@main.route("/services/<uuid:service_id>/<uuid:template_id>/check/<uuid:upload_id>", methods=['GET'])
flake8
2019-11-29 13:03:23 +00:00
@main.route(
"/services/<uuid:service_id>/<uuid:template_id>/check/<uuid:upload_id>/row-<int:row_index>",
methods=['GET']
)
add restrict_admin_usage flag to more of the send flow
2018-03-01 11:04:25 +00:00
@user_has_permissions('send_messages', restrict_admin_usage=True)
Store template ID and original file name in URL At the moment you can’t press refresh on the check page if there’s errors. This is because the session gets cleared when there’s errors. This is a bad user experience. The data that this page is relying on (from the session) is: - template ID - original file name Neither of these things need to be in the session because: - they are not secret - the user can modify them already (by choosing a different template or renaming their file locally) So this commit additionally stores them in the URL.
2018-03-29 17:01:35 +01:00
def check_messages(service_id, template_id, upload_id, row_index=2):
Refactor how we decide to set metadata We want to set the metadata on a file when rendering the check page, but not when rendering the preview of a letter. So let’s put the code that does that only in the place where we’re rendering the check page. This means not having to pass a variable around. Refactoring only; no functional changes.
2018-05-11 08:46:42 +01:00
data = _check_messages(service_id, template_id, upload_id, row_index)
Remove set-to-string conversion from upload code Also changes the allowed_spreadsheet_file_extensions variable to allowed_file_extensions for consistency.
2020-12-17 10:47:52 +00:00
data['allowed_file_extensions'] = Spreadsheet.ALLOWED_FILE_EXTENSIONS
Refactor check page into 3 separate templates The check page is a very complex Jinja template. It needs breaking up to make it easier to understand what’s going on. I think there are three di
2017-07-20 08:59:49 +01:00
Remove irrelevant stuff from error check pages If you have errors in your file then there’s stuff you’re not going to see on the page. So this doesn’t need to be in the Jinja templates that are only used when there are errors. Basically the conditional stuff is moving up to the level above these templates.
2017-07-20 09:10:23 +01:00
if (
Do not allow to send job of letters if letters longer than 10 pages
2019-10-01 15:34:46 +01:00
data['recipients'].too_many_rows
or not data['count_of_recipients']
or not data['recipients'].has_recipient_columns
or data['recipients'].duplicate_recipient_column_headers
or data['recipients'].missing_column_headers
or data['sent_previously']
Remove irrelevant stuff from error check pages If you have errors in your file then there’s stuff you’re not going to see on the page. So this doesn’t need to be in the Jinja templates that are only used when there are errors. Basically the conditional stuff is moving up to the level above these templates.
2017-07-20 09:10:23 +01:00
):
return render_template('views/check/column-errors.html', **data)
Refactor check page into 3 separate templates The check page is a very complex Jinja template. It needs breaking up to make it easier to understand what’s going on. I think there are three di
2017-07-20 08:59:49 +01:00
if data['row_errors']:
return render_template('views/check/row-errors.html', **data)
Show row errors before trial mode errors It’s useful to be able to play with the CSV upload thing and find out how it works before you’re interrupted by the error telling you that you’ve got as far as you can in trial mode.
2017-09-14 12:21:40 +01:00
if (
Do not allow to send job of letters if letters longer than 10 pages
2019-10-01 15:34:46 +01:00
data['errors']
Show row errors before trial mode errors It’s useful to be able to play with the CSV upload thing and find out how it works before you’re interrupted by the error telling you that you’ve got as far as you can in trial mode.
2017-09-14 12:21:40 +01:00
):
Refactor check page into 3 separate templates The check page is a very complex Jinja template. It needs breaking up to make it easier to understand what’s going on. I think there are three di
2017-07-20 08:59:49 +01:00
return render_template('views/check/column-errors.html', **data)
Store the sender_id that should be used for a job in S3 metadata Currently, a user can select a reply-to email address or text message sender when uploading a CSV file but this is ignored and the default is always used instead. As a first step towards changing this, this adds the sender_id (if selected) to the S3 metadata so that this information can be used when processing the job.
2018-11-06 14:22:22 +00:00
metadata_kwargs = {
'notification_count': data['count_of_recipients'],
Don’t return UUID objects from the UUID convertor Because it means you often have to cast to string in your application code just to get your tests passing. The method being monkey patched is originally defined here: https://github.com/pallets/werkzeug/blob/b81aa0f18c7336a0421b569d8a5dab8d5c0d7517/src/werkzeug/routing.py#L1272
2019-11-04 12:20:09 +00:00
'template_id': template_id,
Store the sender_id that should be used for a job in S3 metadata Currently, a user can select a reply-to email address or text message sender when uploading a CSV file but this is ignored and the default is always used instead. As a first step towards changing this, this adds the sender_id (if selected) to the S3 metadata so that this information can be used when processing the job.
2018-11-06 14:22:22 +00:00
'valid': True,
Stop checking query string for filename when sending a job We look for `original_file_name` in the metadata now. Initially we were still checking the query string too, but now that the change to add the filename to the metadata has been deployed for a while there shouldn't be any cases of the filename still being in a query string. Since the `original_file_name` is not being added to the metadata in `.check_messages` (it has happened earlier in the process) a few tests are no longer needed.
2020-10-27 10:52:16 +00:00
'original_file_name': data.get('original_file_name', ''),
Store the sender_id that should be used for a job in S3 metadata Currently, a user can select a reply-to email address or text message sender when uploading a CSV file but this is ignored and the default is always used instead. As a first step towards changing this, this adds the sender_id (if selected) to the S3 metadata so that this information can be used when processing the job.
2018-11-06 14:22:22 +00:00
}
big commit with letters removal
2022-12-05 15:33:44 -05:00
if session.get('sender_id'):
Store the sender_id that should be used for a job in S3 metadata Currently, a user can select a reply-to email address or text message sender when uploading a CSV file but this is ignored and the default is always used instead. As a first step towards changing this, this adds the sender_id (if selected) to the S3 metadata so that this information can be used when processing the job.
2018-11-06 14:22:22 +00:00
metadata_kwargs['sender_id'] = session['sender_id']
set_metadata_on_csv_upload(service_id, upload_id, **metadata_kwargs)
Refactor how we decide to set metadata We want to set the metadata on a file when rendering the check page, but not when rendering the preview of a letter. So let’s put the code that does that only in the place where we’re rendering the check page. This means not having to pass a variable around. Refactoring only; no functional changes.
2018-05-11 08:46:42 +01:00
Refactor check page into 3 separate templates The check page is a very complex Jinja template. It needs breaking up to make it easier to understand what’s going on. I think there are three di
2017-07-20 08:59:49 +01:00
return render_template('views/check/ok.html', **data)
Split endpoints that use templates We’ll need to reuse the logic for viewing images and PDFs of templates, so let’s have it in a shared method which the HTML endpoint then calls.
2016-12-20 11:56:22 +00:00
Require IDs to be UUIDs in URLS We mostly rely on the API returning a 404 to generate 404s for trying to get things with non-UUID IDs. This is fine, except our tests often mock these API calls. So it could look like everything is working fine, except the thing your passing in might never be a valid UUID, and thus would 404 in a non-test environment. So this commit: 1. uses the `uuid` URL converter everywhere there’s something that looks like an ID in a URL parameter 2. adds a test which automates checking for 1.
2019-11-04 11:07:55 +00:00
@main.route("/services/<uuid:service_id>/start-job/<uuid:upload_id>", methods=['POST'])
add restrict_admin_usage arg to admin_override rather than allow admins to do everything specifically, we should only block them from things we conciously don't want them to do. This is "Don't let platform admins send letters from services they're not in". Everything else the platform admins can do. This is step one, adding a restrict_admin_usage flag, and setting that for those restricted endpoints around creating api keys, uploading CSVs and sending one-off messages. Also, this commit separates the two use cases for permissions: * user.has_permission for access control * user.has_permission_for_service for user info - this is used for showing checkboxes on the manage-users page for example With this, we can remove the admin_override flag from the permission decorator.
2018-02-28 18:13:29 +00:00
@user_has_permissions('send_messages', restrict_admin_usage=True)
Give the user better error messages for CSV files Makes uses of the additions to utils in https://github.com/alphagov/notifications-utils/pull/9 This commit strips out a lot of the complex stuff that the views and templates in this app were doing. There is now a cleaner separation of concerns: - utils returns the number and type of errors in the csv - `get_errors_for_csv` helper in this app maps the number and type of errors onto human-friendly error messages - the view and template just doing the glueing-together of all the pieces This is (hopefully) easier to understand, definitely makes the component parts easier to test in isolation, and makes it easier to give more specific error messages.
2016-03-07 18:47:05 +00:00
def start_job(service_id, upload_id):
Don’t catch exceptions if API fails to create job There is no good reason why the API should fail to create a job at this point. If it’s returning a 404, this is an error, and we should be monitoring for it. So we should let it raise, and throw a 500.
2016-03-08 17:50:18 +00:00
job_api_client.create_job(
upload_id,
service_id,
Tell the API which contact list a job’s come from So that we can make enhancements to the UI in the future, for example grouping jobs within their associated contact list.
2020-03-16 17:00:34 +00:00
scheduled_for=request.form.get('scheduled_for', ''),
Don’t catch exceptions if API fails to create job There is no good reason why the API should fail to create a job at this point. If it’s returning a 404, this is an error, and we should be monitoring for it. So we should let it raise, and throw a 500.
2016-03-08 17:50:18 +00:00
)
Give the user better error messages for CSV files Makes uses of the additions to utils in https://github.com/alphagov/notifications-utils/pull/9 This commit strips out a lot of the complex stuff that the views and templates in this app were doing. There is now a cleaner separation of concerns: - utils returns the number and type of errors in the csv - `get_errors_for_csv` helper in this app maps the number and type of errors onto human-friendly error messages - the view and template just doing the glueing-together of all the pieces This is (hopefully) easier to understand, definitely makes the component parts easier to test in isolation, and makes it easier to give more specific error messages.
2016-03-07 18:47:05 +00:00
Store the sender_id that should be used for a job in S3 metadata Currently, a user can select a reply-to email address or text message sender when uploading a CSV file but this is ignored and the default is always used instead. As a first step towards changing this, this adds the sender_id (if selected) to the S3 metadata so that this information can be used when processing the job.
2018-11-06 14:22:22 +00:00
session.pop('sender_id', None)
Give the user better error messages for CSV files Makes uses of the additions to utils in https://github.com/alphagov/notifications-utils/pull/9 This commit strips out a lot of the complex stuff that the views and templates in this app were doing. There is now a cleaner separation of concerns: - utils returns the number and type of errors in the csv - `get_errors_for_csv` helper in this app maps the number and type of errors onto human-friendly error messages - the view and template just doing the glueing-together of all the pieces This is (hopefully) easier to understand, definitely makes the component parts easier to test in isolation, and makes it easier to give more specific error messages.
2016-03-07 18:47:05 +00:00
return redirect(
Show confirmation after pressing big green button There’s no immediate feedback with letter jobs, unlike email or text messages jobs where you see the numbers starting to tick over straight away. We need to reassure the user that the thing they asked us to do (send letters) is underway. ‘Printing’ feels like the natural first state of the letter-making process. So this commit adds a banner to tell the user that printing is the thing that’s happening.
2017-07-13 11:28:37 +01:00
url_for(
'main.view_job',
job_id=upload_id,
service_id=service_id,
)
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.
2016-02-17 15:49:07 +00:00
)
Redirect to the dashboard after the tour After the tour we should ground users by dropping them on the dashboard. In the background, we delete the example text message template. This means that users start from a clean slate when they go to add their own templates This also means some wording changes to the tour so it still makes (some) sense: - 1, 2 and 3 should refer to the current step, not describe the next one - the link should take you to the dashboard - change from ‘Get started’ to ‘Try this example’ because we’re using ‘Get started on the dashboard’
2016-07-01 10:49:54 +01:00
Make send one-off route ask for phone number/email This commit adds an extra, initial, step to the ‘send one-off message’ flow to ask for a phone number or email address. This is the first pass at making a feature which caseworkers or similar could use Notify to send individual messages while they’re working a case.
2017-05-25 09:18:26 +01:00
def fields_to_fill_in(template, prefill_current_user=False):
Show one field at a time on send yourself a test The send yourself a test feature is useful for two things: - constructing an email/text message/letter without uploading a CSV file - seeing what the thing your going to send will look like (either by getting it in your inbox or downloading the PDF) - learning the concept of placeholders, ie understanding they’re thing that gets populated with _stuff_ The problem we’re seeing is that the current UI breaks when a template has a lot of placeholders. This is especially apparent with letter templates, which have a minimum of 7 placeholders by virtue of the address. The idea behind having the form fields side-by-side was to help people understand the relationship between their spreadsheet columns and the placeholders. But this means that the page was doing a lot of work, trying to teach: - replacement of placeholders - link between placeholders and spreadsheet columns The latter is better explained by the example spreadsheet shown on the upload page. So it can safely be removed from the send yourself a test page – in other words the fields don’t need to be shown side by side. Showing them one-at-a-time works well because: - it’s really obvious, even on first use, what the page is asking you to do - as your step through each placeholder, you see the message build up with the data you’ve entered – you’re learning how replacement of placeholders works by repetition This also means adding a matching endpoint for viewing each step of making the test letter as a PDF/PNG because we can’t reuse the view of the template without any placeholders filled any more.
2017-05-04 09:30:55 +01:00
Allow international addresses in spreadsheets For services with permission, they can now put international addresses into their spreadsheets without getting a postcode error. This also means they can start using address line 7 instead of postcode, since it doesn’t make sense to put a country in a field called ‘postcode’. But this will be undocumented to start with, because we’re not giving any real users the permission. It does now mean that the number of possible placeholders (7 + postcode) is greater than the number of allowed placeholders (7), so we have to account for that in the one-off address flow where we’re populating the placeholders automatically. We’re sticking with 6 + postcode here for backwards compatibility.
2020-04-27 10:57:25 +01:00
if not prefill_current_user:
return first_column_headings[template.template_type] + list(template.placeholders)
Show one field at a time on send yourself a test The send yourself a test feature is useful for two things: - constructing an email/text message/letter without uploading a CSV file - seeing what the thing your going to send will look like (either by getting it in your inbox or downloading the PDF) - learning the concept of placeholders, ie understanding they’re thing that gets populated with _stuff_ The problem we’re seeing is that the current UI breaks when a template has a lot of placeholders. This is especially apparent with letter templates, which have a minimum of 7 placeholders by virtue of the address. The idea behind having the form fields side-by-side was to help people understand the relationship between their spreadsheet columns and the placeholders. But this means that the page was doing a lot of work, trying to teach: - replacement of placeholders - link between placeholders and spreadsheet columns The latter is better explained by the example spreadsheet shown on the upload page. So it can safely be removed from the send yourself a test page – in other words the fields don’t need to be shown side by side. Showing them one-at-a-time works well because: - it’s really obvious, even on first use, what the page is asking you to do - as your step through each placeholder, you see the message build up with the data you’ve entered – you’re learning how replacement of placeholders works by repetition This also means adding a matching endpoint for viewing each step of making the test letter as a PDF/PNG because we can’t reuse the view of the template without any placeholders filled any more.
2017-05-04 09:30:55 +01:00
Recipient email/number works as placeholder in test send one off jourrney
2019-06-04 16:12:09 +01:00
if template.template_type == 'sms':
session['recipient'] = current_user.mobile_number
session['placeholders']['phone number'] = current_user.mobile_number
else:
session['recipient'] = current_user.email_address
session['placeholders']['email address'] = current_user.email_address
Show one field at a time on send yourself a test The send yourself a test feature is useful for two things: - constructing an email/text message/letter without uploading a CSV file - seeing what the thing your going to send will look like (either by getting it in your inbox or downloading the PDF) - learning the concept of placeholders, ie understanding they’re thing that gets populated with _stuff_ The problem we’re seeing is that the current UI breaks when a template has a lot of placeholders. This is especially apparent with letter templates, which have a minimum of 7 placeholders by virtue of the address. The idea behind having the form fields side-by-side was to help people understand the relationship between their spreadsheet columns and the placeholders. But this means that the page was doing a lot of work, trying to teach: - replacement of placeholders - link between placeholders and spreadsheet columns The latter is better explained by the example spreadsheet shown on the upload page. So it can safely be removed from the send yourself a test page – in other words the fields don’t need to be shown side by side. Showing them one-at-a-time works well because: - it’s really obvious, even on first use, what the page is asking you to do - as your step through each placeholder, you see the message build up with the data you’ve entered – you’re learning how replacement of placeholders works by repetition This also means adding a matching endpoint for viewing each step of making the test letter as a PDF/PNG because we can’t reuse the view of the template without any placeholders filled any more.
2017-05-04 09:30:55 +01:00
return list(template.placeholders)
separate recipient from rest of placeholders when sending notification note: in the case of letters, we still want to create a CSV file. This only modifies the code flow when it's an email or template 😩 renamed `send_test_values` to `placeholders` because a) that's what they are and b) this isn't just for sending a test message any more
2017-06-23 17:30:34 +01:00
def get_normalised_placeholders_from_session():
Bump utils to 53.0.0 Changes: 53.0.0 --- * `notifications_utils.columns.Columns` has moved to `notifications_utils.insensitive_dict.InsensitiveDict` * `notifications_utils.columns.Rows` has moved to `notifications_utils.recipients.Rows` * `notifications_utils.columns.Cell` has moved to `notifications_utils.recipients.Cell` 52.0.0 --- * Deprecate the following unused `redis_client` functions: - `redis_client.increment_hash_value` - `redis_client.decrement_hash_value` - `redis_client.get_all_from_hash` - `redis_client.set_hash_and_expire` - `redis_client.expire` 51.3.1 --- * Bump govuk-bank-holidays to cache holidays for next year. 51.3.0 --- * Log exception and stacktrace when Celery tasks fail.
2022-02-04 10:43:36 +00:00
return InsensitiveDict(session.get('placeholders', {}))
Show one field at a time on send yourself a test The send yourself a test feature is useful for two things: - constructing an email/text message/letter without uploading a CSV file - seeing what the thing your going to send will look like (either by getting it in your inbox or downloading the PDF) - learning the concept of placeholders, ie understanding they’re thing that gets populated with _stuff_ The problem we’re seeing is that the current UI breaks when a template has a lot of placeholders. This is especially apparent with letter templates, which have a minimum of 7 placeholders by virtue of the address. The idea behind having the form fields side-by-side was to help people understand the relationship between their spreadsheet columns and the placeholders. But this means that the page was doing a lot of work, trying to teach: - replacement of placeholders - link between placeholders and spreadsheet columns The latter is better explained by the example spreadsheet shown on the upload page. So it can safely be removed from the send yourself a test page – in other words the fields don’t need to be shown side by side. Showing them one-at-a-time works well because: - it’s really obvious, even on first use, what the page is asking you to do - as your step through each placeholder, you see the message build up with the data you’ve entered – you’re learning how replacement of placeholders works by repetition This also means adding a matching endpoint for viewing each step of making the test letter as a PDF/PNG because we can’t reuse the view of the template without any placeholders filled any more.
2017-05-04 09:30:55 +01:00
display errors on the end page. The following errors may happen: * Number outside of service if service in trial mode * Message too long for sms * Service over daily limit We need to handle these. They only return on send, rather than in a separate validation step (for now).
2017-06-29 15:31:44 +01:00
def get_recipient_and_placeholders_from_session(template_type):
make sure recipient details always rendered
2017-06-26 14:56:14 +01:00
placeholders = get_normalised_placeholders_from_session()
if template_type == 'sms':
placeholders['phone_number'] = session['recipient']
else:
placeholders['email_address'] = session['recipient']
return placeholders
Make send one-off route ask for phone number/email This commit adds an extra, initial, step to the ‘send one-off message’ flow to ask for a phone number or email address. This is the first pass at making a feature which caseworkers or similar could use Notify to send individual messages while they’re working a case.
2017-05-25 09:18:26 +01:00
def all_placeholders_in_session(placeholders):
return all(
separate recipient from rest of placeholders when sending notification note: in the case of letters, we still want to create a CSV file. This only modifies the code flow when it's an email or template 😩 renamed `send_test_values` to `placeholders` because a) that's what they are and b) this isn't just for sending a test message any more
2017-06-23 17:30:34 +01:00
get_normalised_placeholders_from_session().get(placeholder, False) not in (False, None)
Make send one-off route ask for phone number/email This commit adds an extra, initial, step to the ‘send one-off message’ flow to ask for a phone number or email address. This is the first pass at making a feature which caseworkers or similar could use Notify to send individual messages while they’re working a case.
2017-05-25 09:18:26 +01:00
for placeholder in placeholders
)
Ensure correct page titles on send test flows We have some fairly complicated nested if statements in our Jinja that decide what the page titles should be. It’s only going to get more complicated with the send individual message routes. So this commit: - moves the logic from Jinja to Python - adds tests to check things are working as expected - sets the page titles to the right thing for each flow
2017-05-25 13:31:23 +01:00
Remove old tour and help arguments from sending flow We no longer need the `start_tour` page as this has been replaced with the new `begin_tour` page. We also no longer need to handle the `help` argument in the `send_test_step` or `send_one_off_step` as these no longer are responsible for the tour and don't need to show the help text. Worth pointing out, the new tour joins into the send one off flow. When doing a GET `check_tour_notification`, and submitting the form shown on this page you are POSTed to `send_notification` with `help=3`. Also for general sending of one off notifications, the POST to `send_notification` is done with `help=0` which is a bit of a hack to make sure that we don't show a back link on the `view_notification` page for when someone gets there having just sent a one off notification. This use of `help=0` may be a candidate for a refactor in the future as it feels like a bit of a hacky way of doing things and is therefore not as clear to developers what is going on. Also removes the help argument from the csv routes used here. There is no reason that we need to ever show help for CSVs and this is leftover code from when we used to do the tour that way.
2020-10-05 11:55:15 +01:00
def get_send_test_page_title(template_type, entering_recipient, name=None):
Improve page titles later on in the one off flow Now that the page title for picking a sender/reply to has been improved, I think these pages are also less clear than they could be. This commit changes the page titles to (I hope) be clearer about what is needed from the user on these pages.
2017-11-20 16:45:59 +00:00
if entering_recipient:
Make it clear which template you’re sending The one downside of skipping the template page is that you no longer get such strong confirmation that you’ve picked the correct template. You still see the preview of the template, but it’s further down the page, and the name of the template has disappeared. This commit adds the name of the template to the page title, to: - have some continuity from the previous page - make it easier to double-check you’ve chosen the correct template
2018-08-06 11:20:50 +01:00
return 'Send ‘{}’'.format(name)
Updating for US English Words found: personalis* customis* organis* colour behaviour
2022-12-20 09:44:33 -05:00
return 'Personalize this message'
rename template var in send_test_step it's confusing reassigining one template (json from api) to another type (utils object) on one line. Also removed an unnecessary bounds check (since if placeholders is empty the IndexError will throw on the next line anyway and it'll be handled the same), and moved get_back_link out to its own function
2017-06-22 16:00:14 +01:00
Don’t send users back to a page that will send them forward again If you’re on a page with a normal placeholder and the previous placeholder is one that’s also in the address block then going back to the previous page will send you immediately forward to the current page again. This commit makes the back link a bit smarter by skipping over pages where it can see that they relate to a placeholder from the address block. If it gets all the way to the start of the list of placeholders without finding any non-address ones then it will default to generating a link that will redirect the user to filling in the address block again.
2020-04-09 16:25:48 +01:00
def get_back_link(service_id, template, step_index, placeholders=None):
Remove old tour and help arguments from sending flow We no longer need the `start_tour` page as this has been replaced with the new `begin_tour` page. We also no longer need to handle the `help` argument in the `send_test_step` or `send_one_off_step` as these no longer are responsible for the tour and don't need to show the help text. Worth pointing out, the new tour joins into the send one off flow. When doing a GET `check_tour_notification`, and submitting the form shown on this page you are POSTed to `send_notification` with `help=3`. Also for general sending of one off notifications, the POST to `send_notification` is done with `help=0` which is a bit of a hack to make sure that we don't show a back link on the `view_notification` page for when someone gets there having just sent a one off notification. This use of `help=0` may be a candidate for a refactor in the future as it feels like a bit of a hacky way of doing things and is therefore not as clear to developers what is going on. Also removes the help argument from the csv routes used here. There is no reason that we need to ever show help for CSVs and this is leftover code from when we used to do the tour that way.
2020-10-05 11:55:15 +01:00
if step_index == 0:
dont let people get into one-off flow for deleted templates previously we'd skip the template page entirely if someone didnt have manage templates/api keys permission. however, if the template is deleted you'd then go through the flow entering placeholders and stuff before it would then crash when trying to send. instead, just bounce the user to the template page. It has the content and says when the template was deleted.
2021-07-21 16:06:23 +01:00
if should_skip_template_page(template._template):
Don’t send caseworkers back to template page Caseworkers skip the template page in their message sending journey. Instead they go straight from picking a template to the first step of sending. So the ‘Back’ link should send them straight back to the picking a template page, skipping the individual template page (which they don’t have permission to view).
2018-06-13 14:32:52 +01:00
return url_for(
Fix ‘Back’ link from first step of fix one off If you skip past the templates page (because you don’t have the edit permission) but then click back you end up in a loop which redirects you to the page you’re already on. This commit makes sure that you’re sent back a step further, so you don’t get stuck in that loop.
2018-08-09 16:29:51 +01:00
'.choose_template',
Don’t send caseworkers back to template page Caseworkers skip the template page in their message sending journey. Instead they go straight from picking a template to the first step of sending. So the ‘Back’ link should send them straight back to the picking a template page, skipping the individual template page (which they don’t have permission to view).
2018-06-13 14:32:52 +01:00
service_id=service_id,
)
else:
return url_for(
Fix ‘Back’ link from first step of fix one off If you skip past the templates page (because you don’t have the edit permission) but then click back you end up in a loop which redirects you to the page you’re already on. This commit makes sure that you’re sent back a step further, so you don’t get stuck in that loop.
2018-08-09 16:29:51 +01:00
'.view_template',
Don’t send caseworkers back to template page Caseworkers skip the template page in their message sending journey. Instead they go straight from picking a template to the first step of sending. So the ‘Back’ link should send them straight back to the picking a template page, skipping the individual template page (which they don’t have permission to view).
2018-06-13 14:32:52 +01:00
service_id=service_id,
Fix ‘Back’ link from first step of fix one off If you skip past the templates page (because you don’t have the edit permission) but then click back you end up in a loop which redirects you to the page you’re already on. This commit makes sure that you’re sent back a step further, so you don’t get stuck in that loop.
2018-08-09 16:29:51 +01:00
template_id=template.id,
Don’t send caseworkers back to template page Caseworkers skip the template page in their message sending journey. Instead they go straight from picking a template to the first step of sending. So the ‘Back’ link should send them straight back to the picking a template page, skipping the individual template page (which they don’t have permission to view).
2018-06-13 14:32:52 +01:00
)
Fix ‘Back’ link from first step of fix one off If you skip past the templates page (because you don’t have the edit permission) but then click back you end up in a loop which redirects you to the page you’re already on. This commit makes sure that you’re sent back a step further, so you don’t get stuck in that loop.
2018-08-09 16:29:51 +01:00
Don’t send users back to a page that will send them forward again If you’re on a page with a normal placeholder and the previous placeholder is one that’s also in the address block then going back to the previous page will send you immediately forward to the current page again. This commit makes the back link a bit smarter by skipping over pages where it can see that they relate to a placeholder from the address block. If it gets all the way to the start of the list of placeholders without finding any non-address ones then it will default to generating a link that will redirect the user to filling in the address block again.
2020-04-09 16:25:48 +01:00
return url_for(
'main.send_one_off_step',
service_id=service_id,
template_id=template.id,
step_index=step_index - 1,
)
add new endpoints for one-off flow rather than creating a job, after entering the placeholders, you now send a single notification. This means we don't clog up s3 by creating lots of one line CSV files.
2017-06-23 15:02:20 +01:00
Factor out skip link to reduce complexity The code was too complex and Flake8 was complaining. While this code isn’t related to the change we’re making it is easy enough to factor out.
2020-04-09 16:37:38 +01:00
def get_skip_link(step_index, template):
if (
request.endpoint == 'main.send_one_off_step'
and step_index == 0
When sending to yourself use 'one-off' rather than 'test' routes There is no real reason to have to support both 'one-off' steps and also 'test' steps when sending a one off notification. It's a lot of complex code, just to now set the one of the placeholders in the session. We make our code much simpler but no longer using the 'test' routes but instead adding a new endpoint to set the notification recipient when sending to yourself before continuing on with the rest of the 'one-off' flow. After this is deployed for a day then we can completely remove the 'test' routes and this will help remove a lot of code complexity.
2020-10-05 16:16:11 +01:00
and template.template_type in ("sms", "email")
Factor out skip link to reduce complexity The code was too complex and Flake8 was complaining. While this code isn’t related to the change we’re making it is easy enough to factor out.
2020-04-09 16:37:38 +01:00
and not (template.template_type == 'sms' and current_user.mobile_number is None)
and current_user.has_permissions('manage_templates', 'manage_service')
):
return (
'Use my {}'.format(first_column_headings[template.template_type][0]),
When sending to yourself use 'one-off' rather than 'test' routes There is no real reason to have to support both 'one-off' steps and also 'test' steps when sending a one off notification. It's a lot of complex code, just to now set the one of the placeholders in the session. We make our code much simpler but no longer using the 'test' routes but instead adding a new endpoint to set the notification recipient when sending to yourself before continuing on with the rest of the 'one-off' flow. After this is deployed for a day then we can completely remove the 'test' routes and this will help remove a lot of code complexity.
2020-10-05 16:16:11 +01:00
url_for('.send_one_off_to_myself', service_id=current_service.id, template_id=template.id),
Factor out skip link to reduce complexity The code was too complex and Flake8 was complaining. While this code isn’t related to the change we’re making it is easy enough to factor out.
2020-04-09 16:37:38 +01:00
)
When sending to yourself use 'one-off' rather than 'test' routes There is no real reason to have to support both 'one-off' steps and also 'test' steps when sending a one off notification. It's a lot of complex code, just to now set the one of the placeholders in the session. We make our code much simpler but no longer using the 'test' routes but instead adding a new endpoint to set the notification recipient when sending to yourself before continuing on with the rest of the 'one-off' flow. After this is deployed for a day then we can completely remove the 'test' routes and this will help remove a lot of code complexity.
2020-10-05 16:16:11 +01:00
@main.route("/services/<uuid:service_id>/template/<uuid:template_id>/one-off/send-to-myself", methods=['GET'])
@user_has_permissions('send_messages', restrict_admin_usage=True)
def send_one_off_to_myself(service_id, template_id):
db_template = current_service.get_template_with_user_permission_or_403(template_id, current_user)
if db_template['template_type'] not in ("sms", "email"):
abort(404)
# We aren't concerned with creating the exact template (for example adding recipient and sender names)
# we just want to create enough to use `fields_to_fill_in`
template = get_template(
db_template,
current_service,
)
fields_to_fill_in(template, prefill_current_user=True)
return redirect(url_for(
'main.send_one_off_step',
service_id=service_id,
template_id=template_id,
step_index=1,
))
Require IDs to be UUIDs in URLS We mostly rely on the API returning a 404 to generate 404s for trying to get things with non-UUID IDs. This is fine, except our tests often mock these API calls. So it could look like everything is working fine, except the thing your passing in might never be a valid UUID, and thus would 404 in a non-test environment. So this commit: 1. uses the `uuid` URL converter everywhere there’s something that looks like an ID in a URL parameter 2. adds a test which automates checking for 1.
2019-11-04 11:07:55 +00:00
@main.route("/services/<uuid:service_id>/template/<uuid:template_id>/notification/check", methods=['GET'])
add restrict_admin_usage flag to more of the send flow
2018-03-01 11:04:25 +00:00
@user_has_permissions('send_messages', restrict_admin_usage=True)
add new endpoints for one-off flow rather than creating a job, after entering the placeholders, you now send a single notification. This means we don't clog up s3 by creating lots of one line CSV files.
2017-06-23 15:02:20 +01:00
def check_notification(service_id, template_id):
Let people send one off letters We didn’t used to allow this because it wasn’t really possible with the old DVLA set up and we didn’t think there’s a need. We think it’s possible now because, even though it’s cumbersome, it’s better than the manual process.
2018-10-30 14:24:50 +00:00
return render_template(
'views/notifications/check.html',
**_check_notification(service_id, template_id),
)
display errors on the end page. The following errors may happen: * Number outside of service if service in trial mode * Message too long for sms * Service over daily limit We need to handle these. They only return on send, rather than in a separate validation step (for now).
2017-06-29 15:31:44 +01:00
def _check_notification(service_id, template_id, exception=None):
Replace sevice api client get template calls with Service methods Instead of using the API client directly views are now calling one of two Service model methods: `get_template` is used for view actions, where the user should see the template page even if they don't have access to the template folder (since all templates are still inked from the dashboard or the sent notifications pages). `get_template_with_user_permission_or_403` will check if the user has access to the template's folder first and return 403 otherwise. This method is used for any endpoints that result in an action: editing template attributes, deleting templates or sending messages.
2019-03-19 16:25:44 +00:00
db_template = current_service.get_template_with_user_permission_or_403(template_id, current_user)
This PR adds the ability to select a text message sender if more than one exist for the service.
2017-11-02 12:07:46 +00:00
email_reply_to = None
sms_sender = None
if db_template['template_type'] == 'email':
Refactor settings page to use service model There’s a lot of code in service settings which: - talks to the API directly through the clients - passes that information through to the Jinja template By encapsulating this logic in the service model: - the Jinja template can access the data directly - the logic can be reused across multiple methods
2018-10-26 17:31:49 +01:00
email_reply_to = get_email_reply_to_address_from_session()
This PR adds the ability to select a text message sender if more than one exist for the service.
2017-11-02 12:07:46 +00:00
elif db_template['template_type'] == 'sms':
Refactor settings page to use service model There’s a lot of code in service settings which: - talks to the API directly through the clients - passes that information through to the Jinja template By encapsulating this logic in the service model: - the Jinja template can access the data directly - the logic can be reused across multiple methods
2018-10-26 17:31:49 +01:00
sms_sender = get_sms_sender_from_session()
add new endpoints for one-off flow rather than creating a job, after entering the placeholders, you now send a single notification. This means we don't clog up s3 by creating lots of one line CSV files.
2017-06-23 15:02:20 +01:00
template = get_template(
db_template,
current_service,
Show email reply to address if one’s been selected If you’ve picked an email reply to address, it’s good UI design to have your choice played back to you, so you can be confident it’s worked. This commit does that by making it part of the email preview. Uses: - [ ] https://github.com/alphagov/notifications-utils/pull/227
2017-10-17 16:06:15 +01:00
show_recipient=True,
This PR adds the ability to select a text message sender if more than one exist for the service.
2017-11-02 12:07:46 +00:00
email_reply_to=email_reply_to,
Let people send one off letters We didn’t used to allow this because it wasn’t really possible with the old DVLA set up and we didn’t think there’s a need. We think it’s possible now because, even though it’s cumbersome, it’s better than the manual process.
2018-10-30 14:24:50 +00:00
sms_sender=sms_sender,
add new endpoints for one-off flow rather than creating a job, after entering the placeholders, you now send a single notification. This means we don't clog up s3 by creating lots of one line CSV files.
2017-06-23 15:02:20 +01:00
)
add tests for new check and send notification endpoints
2017-06-26 17:09:51 +01:00
Fix back link on check notifications page It wasn’t using the same logic around placeholders and address lines as the rest of the send one off steps.
2020-04-15 15:42:25 +01:00
placeholders = fields_to_fill_in(template)
back_link = get_back_link(service_id, template, len(placeholders), placeholders)
add tests for new check and send notification endpoints
2017-06-26 17:09:51 +01:00
if (
Let people send one off letters We didn’t used to allow this because it wasn’t really possible with the old DVLA set up and we didn’t think there’s a need. We think it’s possible now because, even though it’s cumbersome, it’s better than the manual process.
2018-10-30 14:24:50 +00:00
(
not session.get('recipient')
)
or not all_placeholders_in_session(template.placeholders)
add tests for new check and send notification endpoints
2017-06-26 17:09:51 +01:00
):
Upgrade pyexcel-io, which also upgrades Werkzeug Upgraded pyexcel-io from 0.5.14 to 0.5.16. This change causes Werkzeug to be upgraded from 0.14.1 to 0.15.1 which requires some changes: * ProxyFix now needs to be imported from a different location * The status code of RequestRedirect has changed from 301 to 308. Since status code 308 is not currently supported on Internet Explorer with Windows 7 and 8.1, this subclasses RequestRedirect to keep the status code of 301. changelog: https://werkzeug.palletsprojects.com/en/0.15.x/changes/#version-0-15-0
2019-03-21 16:41:22 +00:00
raise PermanentRedirect(back_link)
add tests for new check and send notification endpoints
2017-06-26 17:09:51 +01:00
display errors on the end page. The following errors may happen: * Number outside of service if service in trial mode * Message too long for sms * Service over daily limit We need to handle these. They only return on send, rather than in a separate validation step (for now).
2017-06-29 15:31:44 +01:00
template.values = get_recipient_and_placeholders_from_session(template.template_type)
Let people send one off letters We didn’t used to allow this because it wasn’t really possible with the old DVLA set up and we didn’t think there’s a need. We think it’s possible now because, even though it’s cumbersome, it’s better than the manual process.
2018-10-30 14:24:50 +00:00
return dict(
add new endpoints for one-off flow rather than creating a job, after entering the placeholders, you now send a single notification. This means we don't clog up s3 by creating lots of one line CSV files.
2017-06-23 15:02:20 +01:00
template=template,
back_link=back_link,
Let people send one off letters We didn’t used to allow this because it wasn’t really possible with the old DVLA set up and we didn’t think there’s a need. We think it’s possible now because, even though it’s cumbersome, it’s better than the manual process.
2018-10-30 14:24:50 +00:00
**(get_template_error_dict(exception) if exception else {}),
add new endpoints for one-off flow rather than creating a job, after entering the placeholders, you now send a single notification. This means we don't clog up s3 by creating lots of one line CSV files.
2017-06-23 15:02:20 +01:00
)
display errors on the end page. The following errors may happen: * Number outside of service if service in trial mode * Message too long for sms * Service over daily limit We need to handle these. They only return on send, rather than in a separate validation step (for now).
2017-06-29 15:31:44 +01:00
def get_template_error_dict(exception):
add other send notification error branches and tests
2017-06-29 16:06:36 +01:00
# TODO: Make API return some computer-friendly identifier as well as the end user error messages
Fix error messages on one off sending This was causing a 500 in production. This commit: - reverts the code the working state it was before 68a1426e58475965a052d511c61e4ca2230b0e17 - figures out a way to make the tests pass without breaking the actual app - confirms that mocking things is hard
2017-07-27 16:28:49 +01:00
if 'service is in trial mode' in exception.message:
display errors on the end page. The following errors may happen: * Number outside of service if service in trial mode * Message too long for sms * Service over daily limit We need to handle these. They only return on send, rather than in a separate validation step (for now).
2017-06-29 15:31:44 +01:00
error = 'not-allowed-to-send-to'
Fix error messages on one off sending This was causing a 500 in production. This commit: - reverts the code the working state it was before 68a1426e58475965a052d511c61e4ca2230b0e17 - figures out a way to make the tests pass without breaking the actual app - confirms that mocking things is hard
2017-07-27 16:28:49 +01:00
elif 'Exceeded send limits' in exception.message:
add other send notification error branches and tests
2017-06-29 16:06:36 +01:00
error = 'too-many-messages'
Fix the error message for `message-too=long` There was a bug that displayed the error message with 2 red error boxes around the error message. Also need another else to handle a new error message from the API, the old one can be removed after the API is deployed. But this can go first. I tested this branch with API master and the API branch with the change. I tested one off SMS and a CSV upload.
2020-03-09 17:03:22 +00:00
# the error from the api is changing for message-too-long, but we need both until the api is deployed.
Fix error messages on one off sending This was causing a 500 in production. This commit: - reverts the code the working state it was before 68a1426e58475965a052d511c61e4ca2230b0e17 - figures out a way to make the tests pass without breaking the actual app - confirms that mocking things is hard
2017-07-27 16:28:49 +01:00
elif 'Content for template has a character count greater than the limit of' in exception.message:
add other send notification error branches and tests
2017-06-29 16:06:36 +01:00
error = 'message-too-long'
Fix the error message for `message-too=long` There was a bug that displayed the error message with 2 red error boxes around the error message. Also need another else to handle a new error message from the API, the old one can be removed after the API is deployed. But this can go first. I tested this branch with API master and the API branch with the change. I tested one off SMS and a CSV upload.
2020-03-09 17:03:22 +00:00
elif 'Text messages cannot be longer than' in exception.message:
error = 'message-too-long'
display errors on the end page. The following errors may happen: * Number outside of service if service in trial mode * Message too long for sms * Service over daily limit We need to handle these. They only return on send, rather than in a separate validation step (for now).
2017-06-29 15:31:44 +01:00
else:
raise exception
return {
'error': error,
Use new value of SMS_CHAR_COUNT_LIMIT from utils Admin, API and utils were all defining a value for SMS_CHAR_COUNT_LIMIT. This value has been updated in notifications-utils to allow text messages to be 4 fragments long and notifications-admin now gets the value of SMS_CHAR_COUNT_LIMIT from notifications-utils instead of defining it in config.
2018-08-16 16:51:32 +01:00
'SMS_CHAR_COUNT_LIMIT': SMS_CHAR_COUNT_LIMIT,
add other send notification error branches and tests
2017-06-29 16:06:36 +01:00
'current_service': current_service,
# used to trigger CSV specific err msg content, so not needed for single notification errors.
'original_file_name': False
display errors on the end page. The following errors may happen: * Number outside of service if service in trial mode * Message too long for sms * Service over daily limit We need to handle these. They only return on send, rather than in a separate validation step (for now).
2017-06-29 15:31:44 +01:00
}
Require IDs to be UUIDs in URLS We mostly rely on the API returning a 404 to generate 404s for trying to get things with non-UUID IDs. This is fine, except our tests often mock these API calls. So it could look like everything is working fine, except the thing your passing in might never be a valid UUID, and thus would 404 in a non-test environment. So this commit: 1. uses the `uuid` URL converter everywhere there’s something that looks like an ID in a URL parameter 2. adds a test which automates checking for 1.
2019-11-04 11:07:55 +00:00
@main.route("/services/<uuid:service_id>/template/<uuid:template_id>/notification/check", methods=['POST'])
add restrict_admin_usage flag to more of the send flow
2018-03-01 11:04:25 +00:00
@user_has_permissions('send_messages', restrict_admin_usage=True)
add new endpoints for one-off flow rather than creating a job, after entering the placeholders, you now send a single notification. This means we don't clog up s3 by creating lots of one line CSV files.
2017-06-23 15:02:20 +01:00
def send_notification(service_id, template_id):
Fix 500 error due to inconsistent recipient check This strengthens the initial check of what's in the session to make sure it contains some kind of recipient. Without this, we get: Traceback (most recent call last): File "/home/vcap/deps/0/python/lib/python3.9/site-packages/flask/app.py", line 1950, in full_dispatch_request rv = self.dispatch_request() File "/home/vcap/deps/0/python/lib/python3.9/site-packages/flask/app.py", line 1936, in dispatch_request return self.view_functions[rule.endpoint](**req.view_args) File "/home/vcap/app/app/utils/user.py", line 26, in wrap_func return func(*args, **kwargs) File "/home/vcap/app/app/main/views/send.py", line 1041, in send_notification recipient=session['recipient'] or InsensitiveDict(session['placeholders'])['address line 1'], File "/home/vcap/deps/0/python/lib/python3.9/site-packages/notifications_utils/insensitive_dict.py", line 41, in __getitem__ return super().__getitem__(self.make_key(key)) KeyError: 'addressline1' I'm not sure how to reproduce this, but this should at least give the user a better experience, instead of a 500 page.
2022-02-18 12:38:02 +00:00
recipient = get_recipient()
if not recipient:
add new endpoints for one-off flow rather than creating a job, after entering the placeholders, you now send a single notification. This means we don't clog up s3 by creating lots of one line CSV files.
2017-06-23 15:02:20 +01:00
return redirect(url_for(
'.send_one_off',
service_id=service_id,
template_id=template_id,
))
Replace sevice api client get template calls with Service methods Instead of using the API client directly views are now calling one of two Service model methods: `get_template` is used for view actions, where the user should see the template page even if they don't have access to the template folder (since all templates are still inked from the dashboard or the sent notifications pages). `get_template_with_user_permission_or_403` will check if the user has access to the template's folder first and return 403 otherwise. This method is used for any endpoints that result in an action: editing template attributes, deleting templates or sending messages.
2019-03-19 16:25:44 +00:00
db_template = current_service.get_template_with_user_permission_or_403(template_id, current_user)
display errors on the end page. The following errors may happen: * Number outside of service if service in trial mode * Message too long for sms * Service over daily limit We need to handle these. They only return on send, rather than in a separate validation step (for now).
2017-06-29 15:31:44 +01:00
try:
noti = notification_api_client.send_notification(
service_id,
Replace sevice api client get template calls with Service methods Instead of using the API client directly views are now calling one of two Service model methods: `get_template` is used for view actions, where the user should see the template page even if they don't have access to the template folder (since all templates are still inked from the dashboard or the sent notifications pages). `get_template_with_user_permission_or_403` will check if the user has access to the template's folder first and return 403 otherwise. This method is used for any endpoints that result in an action: editing template attributes, deleting templates or sending messages.
2019-03-19 16:25:44 +00:00
template_id=db_template['id'],
Fix 500 error due to inconsistent recipient check This strengthens the initial check of what's in the session to make sure it contains some kind of recipient. Without this, we get: Traceback (most recent call last): File "/home/vcap/deps/0/python/lib/python3.9/site-packages/flask/app.py", line 1950, in full_dispatch_request rv = self.dispatch_request() File "/home/vcap/deps/0/python/lib/python3.9/site-packages/flask/app.py", line 1936, in dispatch_request return self.view_functions[rule.endpoint](**req.view_args) File "/home/vcap/app/app/utils/user.py", line 26, in wrap_func return func(*args, **kwargs) File "/home/vcap/app/app/main/views/send.py", line 1041, in send_notification recipient=session['recipient'] or InsensitiveDict(session['placeholders'])['address line 1'], File "/home/vcap/deps/0/python/lib/python3.9/site-packages/notifications_utils/insensitive_dict.py", line 41, in __getitem__ return super().__getitem__(self.make_key(key)) KeyError: 'addressline1' I'm not sure how to reproduce this, but this should at least give the user a better experience, instead of a 500 page.
2022-02-18 12:38:02 +00:00
recipient=recipient,
Allow one-off email sending to select the reply-to address
2017-10-16 16:35:35 +01:00
personalisation=session['placeholders'],
Fix 500 error due to inconsistent recipient check This strengthens the initial check of what's in the session to make sure it contains some kind of recipient. Without this, we get: Traceback (most recent call last): File "/home/vcap/deps/0/python/lib/python3.9/site-packages/flask/app.py", line 1950, in full_dispatch_request rv = self.dispatch_request() File "/home/vcap/deps/0/python/lib/python3.9/site-packages/flask/app.py", line 1936, in dispatch_request return self.view_functions[rule.endpoint](**req.view_args) File "/home/vcap/app/app/utils/user.py", line 26, in wrap_func return func(*args, **kwargs) File "/home/vcap/app/app/main/views/send.py", line 1041, in send_notification recipient=session['recipient'] or InsensitiveDict(session['placeholders'])['address line 1'], File "/home/vcap/deps/0/python/lib/python3.9/site-packages/notifications_utils/insensitive_dict.py", line 41, in __getitem__ return super().__getitem__(self.make_key(key)) KeyError: 'addressline1' I'm not sure how to reproduce this, but this should at least give the user a better experience, instead of a 500 page.
2022-02-18 12:38:02 +00:00
sender_id=session.get('sender_id', None),
display errors on the end page. The following errors may happen: * Number outside of service if service in trial mode * Message too long for sms * Service over daily limit We need to handle these. They only return on send, rather than in a separate validation step (for now).
2017-06-29 15:31:44 +01:00
)
except HTTPError as exception:
fix indent
2022-07-26 12:37:13 -07:00
current_app.logger.error('Service {} could not send notification: "{}"'.format(
Use service model to look up service attributes This is better than just keying into the JSON because it means you get an exception straight away when looking up a key that doesn’t exist (which via mocking you could ordinarily miss).
2018-07-20 08:42:01 +01:00
current_service.id,
add other send notification error branches and tests
2017-06-29 16:06:36 +01:00
exception.message
))
Let people send one off letters We didn’t used to allow this because it wasn’t really possible with the old DVLA set up and we didn’t think there’s a need. We think it’s possible now because, even though it’s cumbersome, it’s better than the manual process.
2018-10-30 14:24:50 +00:00
return render_template(
'views/notifications/check.html',
**_check_notification(service_id, template_id, exception),
)
add new endpoints for one-off flow rather than creating a job, after entering the placeholders, you now send a single notification. This means we don't clog up s3 by creating lots of one line CSV files.
2017-06-23 15:02:20 +01:00
separate recipient from rest of placeholders when sending notification note: in the case of letters, we still want to create a CSV file. This only modifies the code flow when it's an email or template 😩 renamed `send_test_values` to `placeholders` because a) that's what they are and b) this isn't just for sending a test message any more
2017-06-23 17:30:34 +01:00
session.pop('placeholders')
session.pop('recipient')
Allow one-off email sending to select the reply-to address
2017-10-16 16:35:35 +01:00
session.pop('sender_id', None)
add new endpoints for one-off flow rather than creating a job, after entering the placeholders, you now send a single notification. This means we don't clog up s3 by creating lots of one line CSV files.
2017-06-23 15:02:20 +01:00
return redirect(url_for(
'.view_notification',
service_id=service_id,
help (aka tour) now works correctly throughout the notification flow
2017-06-30 11:49:03 +01:00
notification_id=noti['id'],
Remove old tour and help arguments from sending flow We no longer need the `start_tour` page as this has been replaced with the new `begin_tour` page. We also no longer need to handle the `help` argument in the `send_test_step` or `send_one_off_step` as these no longer are responsible for the tour and don't need to show the help text. Worth pointing out, the new tour joins into the send one off flow. When doing a GET `check_tour_notification`, and submitting the form shown on this page you are POSTed to `send_notification` with `help=3`. Also for general sending of one off notifications, the POST to `send_notification` is done with `help=0` which is a bit of a hack to make sure that we don't show a back link on the `view_notification` page for when someone gets there having just sent a one off notification. This use of `help=0` may be a candidate for a refactor in the future as it feels like a bit of a hacky way of doing things and is therefore not as clear to developers what is going on. Also removes the help argument from the csv routes used here. There is no reason that we need to ever show help for CSVs and this is leftover code from when we used to do the tour that way.
2020-10-05 11:55:15 +01:00
# used to show the final step of the tour (help=3) or not show
# a back link on a just sent one off notification (help=0)
help (aka tour) now works correctly throughout the notification flow
2017-06-30 11:49:03 +01:00
help=request.args.get('help')
add new endpoints for one-off flow rather than creating a job, after entering the placeholders, you now send a single notification. This means we don't clog up s3 by creating lots of one line CSV files.
2017-06-23 15:02:20 +01:00
))
Show email reply to address if one’s been selected If you’ve picked an email reply to address, it’s good UI design to have your choice played back to you, so you can be confident it’s worked. This commit does that by making it part of the email preview. Uses: - [ ] https://github.com/alphagov/notifications-utils/pull/227
2017-10-17 16:06:15 +01:00
Refactor settings page to use service model There’s a lot of code in service settings which: - talks to the API directly through the clients - passes that information through to the Jinja template By encapsulating this logic in the service model: - the Jinja template can access the data directly - the logic can be reused across multiple methods
2018-10-26 17:31:49 +01:00
def get_email_reply_to_address_from_session():
Show email reply to address if one’s been selected If you’ve picked an email reply to address, it’s good UI design to have your choice played back to you, so you can be confident it’s worked. This commit does that by making it part of the email preview. Uses: - [ ] https://github.com/alphagov/notifications-utils/pull/227
2017-10-17 16:06:15 +01:00
if session.get('sender_id'):
Refactor settings page to use service model There’s a lot of code in service settings which: - talks to the API directly through the clients - passes that information through to the Jinja template By encapsulating this logic in the service model: - the Jinja template can access the data directly - the logic can be reused across multiple methods
2018-10-26 17:31:49 +01:00
return current_service.get_email_reply_to_address(
session['sender_id']
Show email reply to address if one’s been selected If you’ve picked an email reply to address, it’s good UI design to have your choice played back to you, so you can be confident it’s worked. This commit does that by making it part of the email preview. Uses: - [ ] https://github.com/alphagov/notifications-utils/pull/227
2017-10-17 16:06:15 +01:00
)['email_address']
This PR adds the ability to select a text message sender if more than one exist for the service.
2017-11-02 12:07:46 +00:00
Refactor settings page to use service model There’s a lot of code in service settings which: - talks to the API directly through the clients - passes that information through to the Jinja template By encapsulating this logic in the service model: - the Jinja template can access the data directly - the logic can be reused across multiple methods
2018-10-26 17:31:49 +01:00
def get_sms_sender_from_session():
This PR adds the ability to select a text message sender if more than one exist for the service.
2017-11-02 12:07:46 +00:00
if session.get('sender_id'):
Refactor settings page to use service model There’s a lot of code in service settings which: - talks to the API directly through the clients - passes that information through to the Jinja template By encapsulating this logic in the service model: - the Jinja template can access the data directly - the logic can be reused across multiple methods
2018-10-26 17:31:49 +01:00
return current_service.get_sms_sender(
session['sender_id']
This PR adds the ability to select a text message sender if more than one exist for the service.
2017-11-02 12:07:46 +00:00
)['sms_sender']
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.
2018-06-08 12:49:29 +01:00
def get_spreadsheet_column_headings_from_template(template):
column_headings = []
big commit with letters removal
2022-12-05 15:33:44 -05:00
recipient_columns = first_column_headings[template.template_type]
Allow international addresses in spreadsheets For services with permission, they can now put international addresses into their spreadsheets without getting a postcode error. This also means they can start using address line 7 instead of postcode, since it doesn’t make sense to put a country in a field called ‘postcode’. But this will be undocumented to start with, because we’re not giving any real users the permission. It does now mean that the number of possible placeholders (7 + postcode) is greater than the number of allowed placeholders (7), so we have to account for that in the one-off address flow where we’re populating the placeholders automatically. We’re sticking with 6 + postcode here for backwards compatibility.
2020-04-27 10:57:25 +01: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.
2018-06-08 12:49:29 +01:00
for column_heading in (
Allow international addresses in spreadsheets For services with permission, they can now put international addresses into their spreadsheets without getting a postcode error. This also means they can start using address line 7 instead of postcode, since it doesn’t make sense to put a country in a field called ‘postcode’. But this will be undocumented to start with, because we’re not giving any real users the permission. It does now mean that the number of possible placeholders (7 + postcode) is greater than the number of allowed placeholders (7), so we have to account for that in the one-off address flow where we’re populating the placeholders automatically. We’re sticking with 6 + postcode here for backwards compatibility.
2020-04-27 10:57:25 +01:00
recipient_columns + list(template.placeholders)
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.
2018-06-08 12:49:29 +01:00
):
Bump utils to 53.0.0 Changes: 53.0.0 --- * `notifications_utils.columns.Columns` has moved to `notifications_utils.insensitive_dict.InsensitiveDict` * `notifications_utils.columns.Rows` has moved to `notifications_utils.recipients.Rows` * `notifications_utils.columns.Cell` has moved to `notifications_utils.recipients.Cell` 52.0.0 --- * Deprecate the following unused `redis_client` functions: - `redis_client.increment_hash_value` - `redis_client.decrement_hash_value` - `redis_client.get_all_from_hash` - `redis_client.set_hash_and_expire` - `redis_client.expire` 51.3.1 --- * Bump govuk-bank-holidays to cache holidays for next year. 51.3.0 --- * Log exception and stacktrace when Celery tasks fail.
2022-02-04 10:43:36 +00:00
if column_heading not in InsensitiveDict.from_keys(column_headings):
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.
2018-06-08 12:49:29 +01:00
column_headings.append(column_heading)
return column_headings
Fix 500 error due to inconsistent recipient check This strengthens the initial check of what's in the session to make sure it contains some kind of recipient. Without this, we get: Traceback (most recent call last): File "/home/vcap/deps/0/python/lib/python3.9/site-packages/flask/app.py", line 1950, in full_dispatch_request rv = self.dispatch_request() File "/home/vcap/deps/0/python/lib/python3.9/site-packages/flask/app.py", line 1936, in dispatch_request return self.view_functions[rule.endpoint](**req.view_args) File "/home/vcap/app/app/utils/user.py", line 26, in wrap_func return func(*args, **kwargs) File "/home/vcap/app/app/main/views/send.py", line 1041, in send_notification recipient=session['recipient'] or InsensitiveDict(session['placeholders'])['address line 1'], File "/home/vcap/deps/0/python/lib/python3.9/site-packages/notifications_utils/insensitive_dict.py", line 41, in __getitem__ return super().__getitem__(self.make_key(key)) KeyError: 'addressline1' I'm not sure how to reproduce this, but this should at least give the user a better experience, instead of a 500 page.
2022-02-18 12:38:02 +00:00
def get_recipient():
if {'recipient', 'placeholders'} - set(session.keys()):
return None
return (
session['recipient'] or
InsensitiveDict(session['placeholders']).get('address line 1')
)
Reference in New Issue Copy Permalink