International letters aren’t sent by first or second class post. In
keeping with the little touch of skeumorphism, let’s label them with the
commonly recognised marker of international mail instead.
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. e7896f283a
2. c9c6271aa0/app/preview.py (L140)
Single checkboxes are distinct because:
- they don't need to be wrapped in a `<fieldset>`
- they are a subclass of BooleanField so their
data is either True or False
Nested checkboxes with a single top-level node
will only have one item in their `items` list.
This is because the other choices are children of
that list item.
This means we need to check the `choices`
attribute, which lists all the checkboxes, to see
if they should be marked as a group (by being
wrapped in a `<fieldset>`) or not.
Includes:
1. changes to make NestedFieldMixin work
with new fields and CSS for nested checkboxes
2. adds custom version of GOVUK checkboxes
component to allow us to:
- add classes to elements currently inaccessible
- wrap the checkboxes in a list
- add child checkboxes to each checkbox (making
tree structures possible through recursion
Change 2. should be pushed upstream to the GOVUK
Design System as a proposal for changes to the
GOVUK Checkboxes component.
Allows checkboxes to be collapsed so they take up
less space in the page. The collapsed state
includes a live summary tracking which of them are
selected.
Includes changes to the JS for collapsible
checkboxes to make it work with the GOVUK
Checkboxes component HTML.
govukCheckboxesField subclasses
SelectMultipleField and overwrites how it renders
HTML to let us use the GOVUK Checkboxes component
while retaining all the functionality of WTForms
fields.
Based on work on github.com/richardjpope/recourse:
https://github.com/richardjpope/recourse/blob/master/recourse/forms.py#L6
From a question on cross-government Slack:
> re the Usage tab - currently it shows 3 financial years - last year,
> this year and next year. is it possible to replace the "next year" tab
> with something more useful? its always going to be blank! I was
> thinking it would be good to have 2 financial years ago, 1 financial
> year ago and this financial year.
This seems like a reasonable idea, and is something we’ve talked about
before. The original intention[1] was that seeing your (unchanged) free
allowance for next year would be useful, but that doesn’t really seem to
be a user need.
***
1. See https://github.com/alphagov/notifications-admin/pull/1094
> so that you can check what your SMS allowance is going to be before
> you actually get into it
The search form makes a post request, so that phone numbers and email
addresses don’t show up in logs or browser history.
At most the API will return 50 results, with some pagination links. We
can’t easily give you links to click in the admin app, because links can
only perform get requests.
Because the value of seeing more than 50 results feels quite low (users
will probably make their search more specific before scrolling through
all 50) let’s just show a message saying only the first 50 results are
displayed.
We were throwing an exception when instantiating a LetterImageTemplate
as we weren't giving it all the arguments it needed.
Now we give it all the correct parameters and add a
test for the method. Ideally we would add a unit test for the flask
route for downloading a letter job CSV (which is currently lacking) but
I did the minimal to be confident I've fixed the bug as I think this
whole code may be fresh for a bit of a rewrite according to Chris.
Original error:
```
File "/Users/davidmcdonald/.virtualenvs/notifications-admin/lib/python3.6/site-packages/notifications_utils/template.py", line 669, in __init__
raise TypeError('image_url is required')
TypeError: image_url is required
```
Some email clients will pre-fetch links in emails to check whether
they’re safe. This has the unfortunate side effect of claiming the token
that’s in the link.
Long term, we don’t want to let the link be used multiple times, because
this reduces how secure it is (eg someone with access to your browser
history could re-use the link even if you’d signed out).
Instead, this commit adds an extra page which is served when the user
clicks the link from the email. This page includes a form which submits
to the actual URL that uses the token, thereby not claiming the token as
soon as the page is loaded.
For convenience, this page also includes some Javascript which clicks
the link on the user’s behalf. If the user has Javascript turned off
they will see the link and can click it themselves. This is going on the
assumption that whatever the email clients are doing when prefetching
the link doesn’t involve running any Javascript.
This Javascript is inlined so that:
- it is run as fast as possible
- it’s more resilient – even if our assets domain is unreachable or the
connection is interrupted, it will still run
We’re going to add an interstitial page that redirects to this new URL.
But we don’t want that redirect to 404 while the change is deploying,
because some boxes will have the new URL and some won’t. So let’s deploy
the new URL to all the boxes first, then the redirect page can safely
take over the new one.
The new URL is going to be `post` not `get` because that feels more
HTTP-y, so we need to make sure that’s part of this change too.
If a service has permission to send international letters then the admin
app should tell template preview, so that template preview knows what
rules to apply when it’s validating the address of the letter.
We don’t need to wait for template preview to start looking at this
query string argument – it will just ignore it for now.
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.
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.
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.
Like we have search by email address or phone number, finding an
individual letter is a common task. At the moment users are having to
click through pages and pages of letters to find the one they’re looking
for.
Users of the API will also be able to search by reference, same as for
emails and text messages. But we only show this hint text to users who
have some API keys.
This commit does two things:
- brings the ‘All organisations’ page back within the platform admin
part of the site (because it’s hard to find otherwise)
- makes the layouts of all the pages within platform admin a bit closer
to the service-specific pages in terms of heading sizes, spacing, etc
so that moving between them doesn’t feel so jumpy
This involves three changes which broke our code.
To validate email addresses, the optional dependency `email-validator`
must be installed<sup>1</sup>. But since we don’t use WTForms’ email
validation, we shouldn’t need to subclass it – it can just be its own
self contained thing. Then we don’t need to add the extra dependency.
When rendering textareas, and extra `\r\n` is inserted at the beginning
<sup>2</sup>. Browsers will strip this when displaying the textbox and
submitting the form, but some of our tests need updating to account for
this.
The error message for when you don’t choose an option from some radio
buttons has now changed. Rather than just accepting WTForms’ new
message, this commit makes the error messages like the examples from
the Design System<sup>3</sup>. By default it will say ‘Select an
option’, but by passing in an extra parameter (`thing`) it can be
customised to be more specific, for example ‘Select a type of
organisation’.
***
1. https://github.com/wtforms/wtforms/pull/429
2. https://github.com/wtforms/wtforms/issues/238
3. https://design-system.service.gov.uk/components/radios/#error-messages
A user can't be archived if they are the only member of their service
with `manage_settings` permission. `notifications-api` returns a `400`
and an error message if that is the case, however this PR to remove the
`400` error handler
https://github.com/alphagov/notifications-admin/pull/3320 stopped the
error message from showing. This meant that instead of seeing a message
about why a user couldn't be archived, we would just show a `500` error
page instead. This change checks the response from `notifications-api`
and shows an error banner with a message if the user can't be archived.
