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.
When using `with pytest.raises...` any assertions inside the `with`
statement that occur below the line that raises the exception don't get
called. It's not possible to check the response status_code / location
in this test because an exception is raised before the response is
returned.
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.
I noticed when using the dication software that saying ‘one two three
four five’ got dictated as `123 45`. This tripped the validation,
because the space character isn’t a digit.
So this commit normalises out spaces (and other spacing characters like
dashes and underscores) before validating the code and sending it to the
API.
I can also imagine that some people might like to space out the code to
make it easier to transcribe (like you might do with a credit card
number).
Errors with messages being too long or empty aren’t specific to a single
cell of the uploaded spreadsheet, they’re the results of combining all
the cells with the template.
Previously we could only show errors against a specific cell. This
commit makes it possible to add a super-row which spans all the cells,
into which we can put errors.
The index (header) column then spans both these rows, to show that they
are both associated with the same row of input.
Depends on:
- [x] https://github.com/alphagov/notifications-utils/pull/719
A list table takes a list of input, and assumes one row of output. In
order to show errors that span multiple rows we need to output two rows
for one item of input.
The list table inherits from the mapping table; using the mapping table
directly gives us the flexibility we need.
Tests that just assert some content `in` the whole page are tricky to
debug, and make it harder to be sure that said content is showing up in
the right place, with the right markup and styling.
Looks like someone in the Ministry of Justice has made a nice little
Python package of them.
I’ve configured it to use the cached holidays that come with the
package, rather than going to the GOV.UK website every time we start the
app. This should be more reliable, and means we’ll only get updates when
we do a version bump (PyUp should keep an eye on this for us).
At the moment the first AJAX call is triggered as soon as the page
loads. We then look at its response time to work out how long to wait
until making the next call.
This isn’t great because:
- stuff is unlikely to have changed straight away, so it’s a waste of a
call
- while the DOM is being updated it seems to introduce a delay in
clicks on links, which is either more pronounced or more noticeable
when it’s happening straight away, making the UI feel less snappy
I chose a value of 2 seconds as a rough proxy for the minimum time we’d
expect to see a notification go from created to delivered. Median
time-to-delivered was 2.9 seconds when we analysed it for
https://github.com/alphagov/notifications-admin/pull/2974#discussion_r286101286
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.
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.
Optional address placeholders aren’t a thing for one-off letters any
more, so we can tidy up the code a bit by removing the parts of the flow
that are accounting for them.
If you have an placeholder from the address block elsewhere in your
letter then you currently get redirected to the address block page
instead of being offered to fill that placeholder in. This commit
tightens up the check to only do this when the placeholder is in the
first 7 placeholders, which is where we store the address placeholders.
`dir(object)` is a useful Python function that tells you what attributes
and methods an object has. It’s also used by tools like iPython and IDEs
for code completion.
Some of the attributes of a `JSONModel` are dynamic, based on what
fields we expect in the underlying JSON. Therefore they don’t
automatically end up in the result of calling `dir`. To get around this
we can implement our own `__dir__` method, which also returns the names
of the fields we’re expecting the the JSON.
Inspired by this Raymond Hettinger tweet:
> #python tip: If you add attributes to an API with __getattr__() or
> __getattribute__(), remember to update __dir__() to make the extension
> introspectable.
— https://twitter.com/raymondh/status/1249860863525146624
By default our AJAX calls were 2 seconds. Then they were 5 seconds
because someone reckoned 2 seconds was putting too much load on the
system. Then we made them 10 seconds while we were having an incident.
Then we made them 20 seconds for the heaviest pages, but back to 5
seconds or 2 seconds for the rest of the pages.
This is not a good situation because:
- it slows all services down equally, no matter how much traffic they
have, or which features they have switched on
- it slows everything down by the same amount, no matter how much load
the platform is under
- the values are set based on our worst performance, until we manually
remember to switch them back
- we spend time during incidents deploying changes to slow down the
dashboard refresh time because it’s a nothing-to-lose change that
might relieve some symptoms, when we could be spending time digging
into the underlying cause
This pull request makes the Javascript smarter about how long it waits
until it makes another AJAX call. It bases the delay on how long the
server takes to respond (as a proxy for how much load the server is
under).
It’s based on the square root of the response time, so is more sensitive
to slow downs early on, and less sensitive to slow downs later on. This
helps us give a more pronounced difference in delay between an AJAX call
that is fast (for example the page for a single notification) and one
that is slow (for example a dashboard for a service with lots of
traffic).
*Some examples of what this would mean for various pages*
Page | Response time | Wait until next AJAX call
---|---|---
Check a reply to address | 130ms | 1,850ms
Brand new service dashboard | 229ms | 2,783ms
HM Passport Office dashboard | 634ms | 5,294ms
NHS Coronavirus Service dashboard | 779ms | 5,977ms
_Example of the kind of slowness we’ve seen during an incident_ | 6,000ms | 18,364ms
GOV.UK email dashboard | `HTTP 504` | 😬
We had a report that when clicking on the 'Download this letter' link on
the notification page the file was not being downloaded as a PDF file
but was given a `.htm` file extension instead. We should be able to stop
that happening by using Flask's `send_file` function with the right mimetype.
This change updates the `view_letter_notification_as_preview` to use
`send_file` and splits out code to get the file data into a separate
function.
Mocks in the tests have been updated and some unused mocks removed.
