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

1101 lines
38 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
Move letter length check to utils repo so template-preview can use it, too Update requirements
2019-10-09 14:56:18 +01:00
from notifications_utils import LETTER_MAX_PAGE_COUNT, SMS_CHAR_COUNT_LIMIT
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
from notifications_utils.columns import Columns
Move letter length check to utils repo so template-preview can use it, too Update requirements
2019-10-09 14:56:18 +01:00
from notifications_utils.pdf import is_letter_too_long
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
from notifications_utils.postal_address import (
PostalAddress,
Show address line 7 not postcode in example spreadsheet Since the letter preview will now show address line 7, the example spreadsheet we give should match.
2020-09-07 11:49:43 +01:00
address_lines_1_to_7_keys,
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
)
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,
)
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
from app.main import main, no_cookie
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,
input letter address data in a single block rather than in multiple placeholders - this is the first step towards making postcodes non-required, which is the first step towards international letters. they still populate address_line_# and postcode fields under the hood - to keep validation working the same, the last line always goes into `postcode`. the form normalises whitespace, removes extra new lines, and enforces that you have between three and seven lines. if the letter repeats address placeholders further down (eg "Dear ((address_line_1))"), then it'll fill those in as well. It'll still prompt you to fill them in, but they'll be pre-filled.
2020-03-10 15:12:19 +00:00
LetterAddressForm,
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
)
Sort contact lists appropriately On the uploads page this should be newest first, same as the scheduled and immediate jobs on this page. On the _choose_ page this should be alphabetical, same as choosing a template.
2020-03-16 11:05:26 +00:00
from app.models.contact_list import ContactList, ContactListsAlphabetical
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,
)
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.template_previews import TemplatePreview, get_page_count_for_letter
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,
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
Spreadsheet,
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_errors_for_csv,
Refactor code to check whether email/sms is enabled
2017-07-04 17:25:35 +01:00
get_template,
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,
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
user_has_permissions,
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
)
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
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
letter_address_columns = [
column.replace('_', ' ')
Show address line 7 not postcode in example spreadsheet Since the letter preview will now show address line 7, the example spreadsheet we give should match.
2020-09-07 11:49:43 +01:00
for column in address_lines_1_to_7_keys
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
]
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],
Remove ‘human-readable’ phone number formatting This has been removed from utils (so things will break if utils is upgraded without this change isn’t made). I think it’s friendlier to present the phone number as the user entered it anyway – because this is what they think a ‘correct’ phone number representation looks like anyway.
2017-04-26 16:19:34 +01:00
'sms': ['07700 900321'] if use_example_as_example else [current_user.mobile_number],
Give more realistic example address data Posit that examples of where you can put different parts of the address is more helpful than ‘example, example, example’. Also shows that you don’t have to fill all of the address columns. Spot the Easter egg 🎅
2016-11-11 09:51:20 +00:00
'letter': [
(submitted_fields or {}).get(
key, get_example_letter_address(key) if use_example_as_example else key
)
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
for key in letter_address_columns
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
]
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
if placeholder not in Columns.from_keys(
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
Give more realistic example address data Posit that examples of where you can put different parts of the address is more helpful than ‘example, example, example’. Also shows that you don’t have to fill all of the address columns. Spot the Easter egg 🎅
2016-11-11 09:51:20 +00:00
def get_example_letter_address(key):
return {
'address line 1': 'A. Name',
'address line 2': '123 Example Street',
Show address line 7 not postcode in example spreadsheet Since the letter preview will now show address line 7, the example spreadsheet we give should match.
2020-09-07 11:49:43 +01:00
'address line 3': 'XM4 5HQ'
Give more realistic example address data Posit that examples of where you can put different parts of the address is more helpful than ‘example, example, example’. Also shows that you don’t have to fill all of the address columns. Spot the Easter egg 🎅
2016-11-11 09:51:20 +00:00
}.get(key, '')
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):
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,
Update utils with to make letters unclickable Brings in: - [ ] https://github.com/alphagov/notifications-utils/pull/159 Means: - renaming a bunch of things - removing any CSS that targeted the link, and making it target the containing element too
2017-04-28 16:04:52 +01:00
letter_preview_url=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
'no_cookie.view_letter_template_preview',
Update utils with to make letters unclickable Brings in: - [ ] https://github.com/alphagov/notifications-utils/pull/159 Means: - renaming a bunch of things - removing any CSS that targeted the link, and making it target the containing element too
2017-04-28 16:04:52 +01:00
service_id=service_id,
template_id=template_id,
filetype='png',
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
page_count=get_page_count_for_letter(db_template),
Update utils with to make letters unclickable Brings in: - [ ] https://github.com/alphagov/notifications-utils/pull/159 Means: - renaming a bunch of things - removing any CSS that targeted the link, and making it target the containing element too
2017-04-28 16:04:52 +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=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
current_app.config['AWS_REGION']
)
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
))
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,
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
allowed_file_extensions=Spreadsheet.ALLOWED_FILE_EXTENSIONS
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
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 template['template_type'] == 'letter':
Allow one-off email sending to select the reply-to address
2017-10-16 16:35:35 +01:00
return redirect_to_one_off
sender_details = get_sender_details(service_id, template['template_type'])
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'
},
'letter': {
Retitle choose sender page as first page in flow We have a sort of principle that when clicking a link, the page you land on should be titled the same as the link you clicked. This also reduces unnecessary repetition between the page title and the form label.
2017-11-13 11:34:32 +00:00
'title': 'Send to one recipient',
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': 'What should appear in the top right of the letter?',
Allow one-off email sending to select the reply-to address
2017-10-16 16:35:35 +01:00
'field_name': 'contact_block'
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
'letter': service_api_client.get_letter_contacts,
'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)
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 db_template['template_type'] == 'letter':
Allow one-off email sending to select the reply-to address
2017-10-16 16:35:35 +01:00
session['sender_id'] = None
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
return redirect(
url_for('.send_one_off_letter_address', service_id=service_id, template_id=template_id)
)
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
input letter address data in a single block rather than in multiple placeholders - this is the first step towards making postcodes non-required, which is the first step towards international letters. they still populate address_line_# and postcode fields under the hood - to keep validation working the same, the last line always goes into `postcode`. the form normalises whitespace, removes extra new lines, and enforces that you have between three and seven lines. if the letter repeats address placeholders further down (eg "Dear ((address_line_1))"), then it'll fill those in as well. It'll still prompt you to fill them in, but they'll be pre-filled.
2020-03-10 15:12:19 +00:00
@main.route(
"/services/<uuid:service_id>/send/<uuid:template_id>/one-off/address",
methods=['GET', 'POST']
)
@user_has_permissions('send_messages', restrict_admin_usage=True)
def send_one_off_letter_address(service_id, template_id):
if {'recipient', 'placeholders'} - set(session.keys()):
# if someone has come here via a bookmark or back button they might have some stuff still in their session
return redirect(url_for('.send_one_off', service_id=service_id, template_id=template_id))
db_template = current_service.get_template_with_user_permission_or_403(template_id, current_user)
template = get_template(
db_template,
current_service,
show_recipient=True,
letter_preview_url=url_for(
'no_cookie.send_test_preview',
service_id=service_id,
template_id=template_id,
filetype='png',
),
Don’t cache page count for one off letters Why we did this originally[1]: > Calculating the number of pages in a letter is quite slow. And the > send yourself a test pages need to load _fast_. Since filling in > placeholders is very unlikely to change the number of pages in the > resultant letter, it’s pretty safe to cache that count, and makes the > subsequent pages load a lot faster. However things have changed since then: - this journey is used for sending real letters, not just test ones - we’re doing enough letters that even an unlikely discrepancy will (and does) happen - we cache the generation of the PDF now[2], so at least it’s not generating the PDF twice, once for the preview and once for the page count - it’s no longer necessary to step through each address placeholder to populate a one-off letter, so a little bit slower isn’t so bad 1. https://github.com/alphagov/notifications-admin/pull/1277/commits/e7896f283a0d34d433d032987ec90ef430dd0ba9 2. https://github.com/alphagov/notifications-template-preview/blob/c9c6271aa09fc6b2d271604d66939f72e76250ad/app/preview.py#L140
2020-05-15 13:55:04 +01:00
page_count=get_page_count_for_letter(db_template),
input letter address data in a single block rather than in multiple placeholders - this is the first step towards making postcodes non-required, which is the first step towards international letters. they still populate address_line_# and postcode fields under the hood - to keep validation working the same, the last line always goes into `postcode`. the form normalises whitespace, removes extra new lines, and enforces that you have between three and seven lines. if the letter repeats address placeholders further down (eg "Dear ((address_line_1))"), then it'll fill those in as well. It'll still prompt you to fill them in, but they'll be pre-filled.
2020-03-10 15:12:19 +00:00
email_reply_to=None,
sms_sender=None
)
Refactor to use PostalAddress helper from utils Since we’re doing normalisation and line-count-checking of addresses in multiple places it makes sense for that code to be shared. Which is what happened here: https://github.com/alphagov/notifications-utils/pull/713 This commit refactors the admin code to make use of the new utils code. Note about placeholders: - they now go into the session as `address_line_1` instead of `address line 1` because this is the format the API uses, so should be considered canonical - they are now fetched from the session in a way that isn’t sensitive to case or underscores (using the `Columns` class) - the API doesn’t care about case or underscores vs spaces in placeholder names because it’s checking an instance of `Template` to see if all the required placeholders are present (see https://github.com/alphagov/notifications-api/blob/401c8e41d6a1e3b348b99eb0cc205bb420fce0c1/app/notifications/process_notifications.py#L40)
2020-04-02 17:21:27 +01:00
current_session_address = PostalAddress.from_personalisation(
get_normalised_placeholders_from_session()
)
redirect from address_line_n to address textarea page if someone starts a new one-off flow they'll get taken to the address page. However, if someone hits the back button, they'll cycle backwards through placeholders and will end up on the individual line pages. Lets redirect them to the correct place. We'll additionally need to reconstruct the address block from the various session variables that may or may not be populated
2020-03-12 13:56:35 +00: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
form = LetterAddressForm(
address=current_session_address.normalised,
allow_international_letters=current_service.has_permission('international_letters'),
)
input letter address data in a single block rather than in multiple placeholders - this is the first step towards making postcodes non-required, which is the first step towards international letters. they still populate address_line_# and postcode fields under the hood - to keep validation working the same, the last line always goes into `postcode`. the form normalises whitespace, removes extra new lines, and enforces that you have between three and seven lines. if the letter repeats address placeholders further down (eg "Dear ((address_line_1))"), then it'll fill those in as well. It'll still prompt you to fill them in, but they'll be pre-filled.
2020-03-10 15:12:19 +00:00
if form.validate_on_submit():
Refactor to use PostalAddress helper from utils Since we’re doing normalisation and line-count-checking of addresses in multiple places it makes sense for that code to be shared. Which is what happened here: https://github.com/alphagov/notifications-utils/pull/713 This commit refactors the admin code to make use of the new utils code. Note about placeholders: - they now go into the session as `address_line_1` instead of `address line 1` because this is the format the API uses, so should be considered canonical - they are now fetched from the session in a way that isn’t sensitive to case or underscores (using the `Columns` class) - the API doesn’t care about case or underscores vs spaces in placeholder names because it’s checking an instance of `Template` to see if all the required placeholders are present (see https://github.com/alphagov/notifications-api/blob/401c8e41d6a1e3b348b99eb0cc205bb420fce0c1/app/notifications/process_notifications.py#L40)
2020-04-02 17:21:27 +01:00
session['placeholders'].update(PostalAddress(form.address.data).as_personalisation)
input letter address data in a single block rather than in multiple placeholders - this is the first step towards making postcodes non-required, which is the first step towards international letters. they still populate address_line_# and postcode fields under the hood - to keep validation working the same, the last line always goes into `postcode`. the form normalises whitespace, removes extra new lines, and enforces that you have between three and seven lines. if the letter repeats address placeholders further down (eg "Dear ((address_line_1))"), then it'll fill those in as well. It'll still prompt you to fill them in, but they'll be pre-filled.
2020-03-10 15:12:19 +00: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)
input letter address data in a single block rather than in multiple placeholders - this is the first step towards making postcodes non-required, which is the first step towards international letters. they still populate address_line_# and postcode fields under the hood - to keep validation working the same, the last line always goes into `postcode`. the form normalises whitespace, removes extra new lines, and enforces that you have between three and seven lines. if the letter repeats address placeholders further down (eg "Dear ((address_line_1))"), then it'll fill those in as well. It'll still prompt you to fill them in, but they'll be pre-filled.
2020-03-10 15:12:19 +00:00
if all_placeholders_in_session(placeholders):
return get_notification_check_endpoint(service_id, template)
Show address line 7 not postcode in example spreadsheet Since the letter preview will now show address line 7, the example spreadsheet we give should match.
2020-09-07 11:49:43 +01:00
first_non_address_placeholder_index = len(address_lines_1_to_7_keys)
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
input letter address data in a single block rather than in multiple placeholders - this is the first step towards making postcodes non-required, which is the first step towards international letters. they still populate address_line_# and postcode fields under the hood - to keep validation working the same, the last line always goes into `postcode`. the form normalises whitespace, removes extra new lines, and enforces that you have between three and seven lines. if the letter repeats address placeholders further down (eg "Dear ((address_line_1))"), then it'll fill those in as well. It'll still prompt you to fill them in, but they'll be pre-filled.
2020-03-10 15:12:19 +00:00
return redirect(url_for(
'main.send_one_off_step',
service_id=service_id,
template_id=template_id,
step_index=first_non_address_placeholder_index,
))
return render_template(
'views/send-one-off-letter-address.html',
page_title=get_send_test_page_title(
template_type='letter',
entering_recipient=True,
name=template.name,
),
template=template,
form=form,
back_link=get_back_link(service_id, template, 0),
link_to_upload=True,
)
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':
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()
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,
Update utils with to make letters unclickable Brings in: - [ ] https://github.com/alphagov/notifications-utils/pull/159 Means: - renaming a bunch of things - removing any CSS that targeted the link, and making it target the containing element too
2017-04-28 16:04:52 +01:00
letter_preview_url=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
'no_cookie.send_test_preview',
Update utils with to make letters unclickable Brings in: - [ ] https://github.com/alphagov/notifications-utils/pull/159 Means: - renaming a bunch of things - removing any CSS that targeted the link, and making it target the containing element too
2017-04-28 16:04:52 +01:00
service_id=service_id,
template_id=template_id,
filetype='png',
),
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
page_count=get_page_count_for_letter(db_template, values=template_values),
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
redirect from address_line_n to address textarea page if someone starts a new one-off flow they'll get taken to the address page. However, if someone hits the back button, they'll cycle backwards through placeholders and will end up on the individual line pages. Lets redirect them to the correct place. We'll additionally need to reconstruct the address block from the various session variables that may or may not be populated
2020-03-12 13:56:35 +00:00
# if we're in a letter, we should show address block rather than "address line #" or "postcode"
Skip over placeholders if they’re in the address We don’t really want you modifying lines of the address after you’ve entered it. Especially when it might not be obvious that modifying the address line placeholder will modify the address you’re sending the letter to.
2020-04-09 15:45:30 +01:00
if template.template_type == 'letter':
Show address line 7 not postcode in example spreadsheet Since the letter preview will now show address line 7, the example spreadsheet we give should match.
2020-09-07 11:49:43 +01:00
if step_index < len(address_lines_1_to_7_keys):
Skip over placeholders if they’re in the address We don’t really want you modifying lines of the address after you’ve entered it. Especially when it might not be obvious that modifying the address line placeholder will modify the address you’re sending the letter to.
2020-04-09 15:45:30 +01:00
return redirect(url_for(
'.send_one_off_letter_address',
service_id=service_id,
template_id=template_id,
))
if current_placeholder in Columns(PostalAddress('').as_personalisation):
return redirect(url_for(
request.endpoint,
service_id=service_id,
template_id=template_id,
step_index=step_index + 1,
))
redirect from address_line_n to address textarea page if someone starts a new one-off flow they'll get taken to the address page. However, if someone hits the back button, they'll cycle backwards through placeholders and will end up on the individual line pages. Lets redirect them to the correct place. We'll additionally need to reconstruct the address block from the various session variables that may or may not be populated
2020-03-12 13:56:35 +00: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.
# Only if it's not a letter.
# And only if we're not on the test route, since that will already have the user's own number set
if (
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
step_index == 0
and template.template_type != 'letter'
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
):
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
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
@no_cookie.route("/services/<uuid:service_id>/send/<uuid:template_id>/test.<filetype>", methods=['GET'])
remove all instances of db style permissions lots of renaming of send_texts/emails/letters to send_messages, and manage_settings/users to manage_service
2018-02-28 17:22:20 +00:00
@user_has_permissions('send_messages')
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
def send_test_preview(service_id, template_id, filetype):
if filetype not in ('pdf', 'png'):
abort(404)
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 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 = 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,
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
current_service,
letter_preview_url=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
'no_cookie.send_test_preview',
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,
filetype='png',
),
)
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
template.values = get_normalised_placeholders_from_session()
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 TemplatePreview.from_utils_template(template, filetype, page=request.args.get('page'))
Add a page to choose a contact list You’ll be able to use a contact list by first choosing a template, then choosing the list you want to use. At the moment this shows all lists, not just the ones that are compatible with your template.
2020-03-13 17:51:22 +00:00
@main.route(
'/services/<uuid:service_id>/send/<uuid:template_id>'
'/from-contact-list'
)
@user_has_permissions('send_messages')
def choose_from_contact_list(service_id, template_id):
Only show contact lists relevant to template You can’t send an email message template to a list of phone numbers. So we shouldn’t show you the lists of phone numbers when you’ve chosen an email template.
2020-03-16 09:41:42 +00:00
db_template = current_service.get_template_with_user_permission_or_403(
template_id, current_user
)
template = get_template(
db_template, current_service,
)
Add a page to choose a contact list You’ll be able to use a contact list by first choosing a template, then choosing the list you want to use. At the moment this shows all lists, not just the ones that are compatible with your template.
2020-03-13 17:51:22 +00:00
return render_template(
'views/send-contact-list.html',
Sort contact lists appropriately On the uploads page this should be newest first, same as the scheduled and immediate jobs on this page. On the _choose_ page this should be alphabetical, same as choosing a template.
2020-03-16 11:05:26 +00:00
contact_lists=ContactListsAlphabetical(
Only show contact lists relevant to template You can’t send an email message template to a list of phone numbers. So we shouldn’t show you the lists of phone numbers when you’ve chosen an email template.
2020-03-16 09:41:42 +00:00
current_service.id,
template_type=template.template_type,
),
Warn users trying to use a personalised template Since contact lists only store the email address or phone number, this won’t work. So we should warn people, and highlight that the personalisation in the template is the problem, since this is what they will need to change.
2020-03-16 10:34:05 +00:00
template=template,
Add a page to choose a contact list You’ll be able to use a contact list by first choosing a template, then choosing the list you want to use. At the moment this shows all lists, not just the ones that are compatible with your template.
2020-03-13 17:51:22 +00:00
)
Add endpoint to redirect from contact list to send In order to use a contact list we’re going to put you in the normal upload a spreadsheet journey. Except without having to upload again. So this commit adds an endpoint that takes the file from the contact list bucket, puts it in the same place it would have gone if you’d uploaded it, then sends you on your way.
2020-03-13 14:38:43 +00:00
@main.route(
'/services/<uuid:service_id>/send/<uuid:template_id>'
'/from-contact-list/<uuid:contact_list_id>'
)
@user_has_permissions('send_messages')
def send_from_contact_list(service_id, template_id, contact_list_id):
Pass original file name when sending contact list Otherwise the new file doesn’t get a name, which is very confusing.
2020-03-16 11:25:07 +00:00
contact_list = ContactList.from_id(
contact_list_id,
service_id=current_service.id,
)
Add endpoint to redirect from contact list to send In order to use a contact list we’re going to put you in the normal upload a spreadsheet journey. Except without having to upload again. So this commit adds an endpoint that takes the file from the contact list bucket, puts it in the same place it would have gone if you’d uploaded it, then sends you on your way.
2020-03-13 14:38:43 +00:00
return redirect(url_for(
'main.check_messages',
service_id=current_service.id,
template_id=template_id,
Pass original file name when sending contact list Otherwise the new file doesn’t get a name, which is very confusing.
2020-03-16 11:25:07 +00:00
upload_id=contact_list.copy_to_uploads(),
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
contact_list_id=contact_list.id,
Add endpoint to redirect from contact list to send In order to use a contact list we’re going to put you in the normal upload a spreadsheet journey. Except without having to upload again. So this commit adds an endpoint that takes the file from the contact list bucket, puts it in the same place it would have gone if you’d uploaded it, then sends you on your way.
2020-03-13 14:38:43 +00:00
))
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
def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_pdf=False):
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
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)
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
use service statistics endpoint instead of detailed service we're not actually looking at the detailed service aspects - just the stats. We're doing this in three places: * dashboard * notification activity page * when checking jobs to see if we're over the daily limit change these places to use a new api endpoint (service/id/statistics), which hopefully be a little more performant, and will definitely be a little more organised - moving away from generic endpoints with loads of optional parameters. We still need the detailed endpoints for the platform admin page tho. Depends on https://github.com/alphagov/notifications-api/pull/1865
2018-05-09 13:53:02 +01:00
statistics = service_api_client.get_service_statistics(service_id, today_only=True)
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
remaining_messages = (current_service.message_limit - sum(stat['requested'] for stat in statistics.values()))
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,
letter_preview_url=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
'no_cookie.check_messages_preview',
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
service_id=service_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
template_id=template_id,
Update utils with to make letters unclickable Brings in: - [ ] https://github.com/alphagov/notifications-utils/pull/159 Means: - renaming a bunch of things - removing any CSS that targeted the link, and making it target the containing element too
2017-04-28 16:04:52 +01:00
upload_id=upload_id,
filetype='png',
Add URLs for previewing all rows of a 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. So this commit is the first step towards being able to preview an arbitrary row of a template, by extending the URL structure to optionally accept a row number for pages or files (ie PNG) that preview successfully uploaded spreadsheets. What this commit doesn’t do is link to these pages; that will come as part of a subsequent commit.
2017-12-06 22:04:58 +00:00
row_index=preview_row,
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 not letters_as_pdf else None,
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,
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
page_count=get_page_count_for_letter(db_template),
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,
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
whitelist=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'),
allow_international_letters=current_service.has_permission('international_letters'),
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'):
help (aka tour) now works correctly throughout the notification flow
2017-06-30 11:49:03 +01:00
# only happens if generating a letter preview test
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
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
page_count = get_page_count_for_letter(db_template, template.values)
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,
Don’t show trial mode error if making test letter We don’t want users in trial mode sending real letters. So we’ve introduced an error message. This error message is also showing up when users in trial mode and making a test letter (and having the knock on effect of hiding the download button). They should be able to make a test letter in trial mode, because it doesn’t cost anything.
2017-08-25 14:17:23 +01:00
trying_to_send_letters_in_trial_mode=all((
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.trial_mode,
Don’t show trial mode error if making test letter We don’t want users in trial mode sending real letters. So we’ve introduced an error message. This error message is also showing up when users in trial mode and making a test letter (and having the knock on effect of hiding the download button). They should be able to make a test letter in trial mode, because it doesn’t cost anything.
2017-08-25 14:17:23 +01:00
template.template_type == 'letter',
)),
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
),
Move letter too long banner message over from utils, also refactor
2019-10-01 17:16:15 +01:00
letter_too_long=is_letter_too_long(page_count),
Move letter length check to utils repo so template-preview can use it, too Update requirements
2019-10-09 14:56:18 +01:00
letter_max_pages=LETTER_MAX_PAGE_COUNT,
Add postal address row errors Context: - postal addresses can be made from any of the 7 address lines now, and the postcode can be in any one of the 7 - we can put errors across a whole row now, not just on individual cells This commit put errors to do with the postal address as a whole across the whole row now, rather than tying them to any one cell.
2020-04-15 13:31:14 +01:00
letter_min_address_lines=PostalAddress.MIN_LINES,
letter_max_address_lines=PostalAddress.MAX_LINES,
Harmonise content of error message with the document laid out by our content designer
2019-10-16 13:02:11 +01:00
page_count=page_count
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 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
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']
or data['trying_to_send_letters_in_trial_mode']
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
}
if session.get('sender_id'):
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
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
@no_cookie.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>/<uuid:template_id>/check/<uuid:upload_id>.<filetype>",
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
methods=['GET'],
)
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
@no_cookie.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>/<uuid:template_id>/check/<uuid:upload_id>/row-<int:row_index>.<filetype>",
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
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')
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_preview(service_id, template_id, upload_id, filetype, row_index=2):
fix pdf download link trying to request a page count It doesn't make sense to get a pdf for only one page - the template preview app just returns a 400 if you try. So we shouldn't try!
2018-10-15 14:02:13 +01:00
if filetype == 'pdf':
page = None
elif filetype == 'png':
page = request.args.get('page', 1)
else:
don't allow bad filetypes to preview
2017-04-12 12:12:11 +01:00
abort(404)
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
template = _check_messages(
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
service_id, template_id, upload_id, row_index, letters_as_pdf=True
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
)['template']
fix pdf download link trying to request a page count It doesn't make sense to get a pdf for only one page - the template preview app just returns a 400 if you try. So we shouldn't try!
2018-10-15 14:02:13 +01:00
return TemplatePreview.from_utils_template(template, filetype, page=page)
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
Add cloudfoundry config values also added tests
2017-04-10 18:12:00 +01:00
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
@no_cookie.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>/<uuid:template_id>/check.<filetype>",
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
methods=['GET'],
)
@user_has_permissions('send_messages')
def check_notification_preview(service_id, template_id, filetype):
if filetype == 'pdf':
page = None
elif filetype == 'png':
page = request.args.get('page', 1)
else:
abort(404)
template = _check_notification(
service_id, template_id,
)['template']
return TemplatePreview.from_utils_template(template, filetype, page=page)
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', ''),
contact_list_id=request.form.get('contact_list_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
)
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,
just_sent='yes',
)
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
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>/end-tour/<uuid:example_template_id>")
remove admin_override from all has_permissions usage as previously pointed out, it's not used anywhere.
2018-03-01 10:30:17 +00:00
@user_has_permissions('manage_templates')
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
def go_to_dashboard_after_tour(service_id, example_template_id):
service_api_client.delete_service_template(service_id, example_template_id)
return redirect(
url_for('main.service_dashboard', service_id=service_id)
)
send users back to beginning of tour if they hit back at end (previously it would have sent them to the choose template page) if the user has added new templates or deleted the example one, they're clearly competent enough to use the app so don't worry (we wouldn't know what URL the tour starts on since the UUID of the example template is random)
2017-01-19 18:15:33 +00: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 'letter' == template.template_type:
return letter_address_columns + list(template.placeholders)
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():
Refactor to use PostalAddress helper from utils Since we’re doing normalisation and line-count-checking of addresses in multiple places it makes sense for that code to be shared. Which is what happened here: https://github.com/alphagov/notifications-utils/pull/713 This commit refactors the admin code to make use of the new utils code. Note about placeholders: - they now go into the session as `address_line_1` instead of `address line 1` because this is the format the API uses, so should be considered canonical - they are now fetched from the session in a way that isn’t sensitive to case or underscores (using the `Columns` class) - the API doesn’t care about case or underscores vs spaces in placeholder names because it’s checking an instance of `Template` to see if all the required placeholders are present (see https://github.com/alphagov/notifications-api/blob/401c8e41d6a1e3b348b99eb0cc205bb420fce0c1/app/notifications/process_notifications.py#L40)
2020-04-02 17:21:27 +01:00
return Columns(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)
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
return 'Personalise 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:
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
if should_skip_template_page(template.template_type):
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
if template.template_type == 'letter' and placeholders:
# Make sure we’re not redirecting users to a page which will
# just redirect them forwards again
back_link_destination_step_index = next((
index
for index, placeholder in reversed(
list(enumerate(placeholders[:step_index]))
)
if placeholder not in Columns(
PostalAddress('').as_personalisation
)
), 1)
return get_back_link(service_id, template, back_link_destination_step_index + 1)
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,
letter_preview_url=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
'no_cookie.check_notification_preview',
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
service_id=service_id,
template_id=template_id,
filetype='png',
),
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
page_count=get_page_count_for_letter(db_template),
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')
and db_template['template_type'] != 'letter'
)
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)
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
page_count = get_page_count_for_letter(db_template, template.values)
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.page_count = page_count
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,
Move letter too long banner message over from utils, also refactor
2019-10-01 17:16:15 +01:00
letter_too_long=is_letter_too_long(page_count),
Move letter length check to utils repo so template-preview can use it, too Update requirements
2019-10-09 14:56:18 +01:00
letter_max_pages=LETTER_MAX_PAGE_COUNT,
Harmonise content of error message with the document laid out by our content designer
2019-10-16 13:02:11 +01:00
page_count=page_count,
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):
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()):
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 API call to send one off letter As part of making the API call we extra the recipient from the first line of the address. This code was assuming that the recipient would always have the key `address line 1`, but we’re no longer guaranteeing that it will be capitalised and spaced exactly like that.
2020-04-07 12:59:34 +01:00
recipient=session['recipient'] or Columns(session['placeholders'])['address line 1'],
Allow one-off email sending to select the reply-to address
2017-10-16 16:35:35 +01:00
personalisation=session['placeholders'],
sender_id=session['sender_id'] if 'sender_id' in session else 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:
add other send notification error branches and tests
2017-06-29 16:06:36 +01:00
current_app.logger.info('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 = []
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 template.template_type == 'letter':
# We want to avoid showing `address line 7` for now
recipient_columns = letter_address_columns
else:
recipient_columns = first_column_headings[template.template_type]
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
):
if column_heading not in Columns.from_keys(column_headings):
column_headings.append(column_heading)
return column_headings
Reference in New Issue Copy Permalink