By moving it from the view we reduce the complexity of the methods in
the view layer, so it’s easier to see what they do.
This also renames the variable `finished` to the property
`processing_finished` to disambiguate from the `job_status` field in the
JSON, which can also have a value of `finished`.
This follows the pattern of what we’ve done with services, users and
events.
It gives us a way of neatly instantiating a model for each item in the
list we get back from the API and reduces the complexity of the view
layer code.
Now is a good time to do this because we’re going to be making a bunch
of changes to the jobs pages, and those changes will be easier to code
and understand with a sensible model behind them.
This follows the pattern of what we’ve done with services, users and
events.
It gives us a better interface to the data we get back from the API than
dealing with the raw JSON directly.
Now is a good time to do this because we’re going to be making a bunch
of changes to the jobs pages, and those changes will be easier to code
and understand with a sesnsible model behind them.
The session key we use is global.
This means if you open the edit page for two different users in two
different tabs the session for the first tab is overwritten with the
session from the second tab. This means the two users are both set to
the same email address, which causes an exception (email addresses are
unique).
This commit fixes that bug by including the user ID in the session ID.
Now persisting the address to the "to" field of the Notification, after the notification has been validated.
If the letter is pending validation, then "Checking..." will appear as the identifier for the letter.
If the letter has passed validation, then the first line of the address (now persisted in the "to" field) will be displayed, with the client reference underneath.
If the letter has failed validation the "Provided as PDF" will show be displayed, which is now the initial value of the "to" field.
Celery/SQS underperforms in low-traffic environments. Tasks will sit on
celery queues for several seconds before getting picked up if they're
the only thing on the queue. This is observable in our test environments
like preview and staging, but we've got enough load on production that
this isn't an issue.
When we validate reply to email addresses, we expect a delivery receipt
to have been processed within 45 seconds of the button being pressed. On
preview, we often observe times over that, possibly due to the several
queues involved in sending an email and processing its receipt. So, to
ensure that functional tests can pass (when we don't really care how
fast things are, just that the flow doesn't break), bump this timeout up
to 120 seconds on preview. The functional tests were waiting for 120
seconds for the reply to address to be validated anyway.
You can click the blue boxes to filter the list of notifications. Once
the notifications have gone there’s nothing to filter, so we should just
show the numbers but without them being clickable.
Notifications could be missing because:
- none have been created yet
- they’ve been deleted
This commit adds separate error messages for each case, rather than a
less helpful generic one.
It’s useful to get some kind of preview of the report before you
download it.
And if there’s only a few letters in there then you might not even need
to download it at all.
For teams with lots of letters we don’t want the page to load too slowly
so let’s cap the number of displayed items to 50, same as previewing
a spreadsheet.
This follows our pattern for other downloadable reports, and gives
people who know/care about stuff like file types some indication of what
they’re about to download.
This page displays a list of available reports, a report is a set of returned lettters that have been reported on the same day.
Each line of the page is a link that can download the report.
I'm not sure the format of the csv is right. The data might be confusing especially if the service has a mix of precompiled templates and jobs.
Units tests are still to come.
Users who have an API integration (and therefore have a way of passing
in a reference for each notification) can now search by that reference
(see https://github.com/alphagov/notifications-api/pull/2682)
This commit changes the label on the search box to tell these users that
this is possible, without changing the label for users without an API
integration, who might get confused by what ‘reference’ means.
It also makes the label consistently say ‘email address or phone number’
(ie email address is first) because this is our content style.
Makes these assumptions:
- we don’t care about history before 29th November 2019 at 11am (this
is when priority started to mean a proportion rather than a ranking)
- the priority of the second provider will always be the inverse of
the first provider
Which means the code is a lot simpler/actually does what you’d expect.
Their priority should always add up to 100%. Currently we have to ensure
this by hand. Adding this form means there’s no way to not set their
combined priorities to 100%. And it’s a bit more of an intuitive UI than
two textboxes on separate pages.
It’s not very useful to know the priority of one provider without
knowing the other. And these pages were never really designed, so they
weren’t super easy to understand anyway.
This commit adds a page that takes the first two text message providers
and shows their relative priority against each other.
It follows the design of the events page, as a pattern for showing a
log of historical events.
New units tests have not been written for this page because it is very like this will be refactor and probably a new template created for the page. Some design needs to go into this page.
But we needed something ready for user research.
we have a hunch that some session related issues that we've seen over
the last few weeks might be related to weird race conditions where
cookies set by subresources (image previews of letters on the send flow)
arrive just as the img request is cancelled because the user has clicked
on a button to navigate to a new page, but still manage to set the
cookie? We're not entirely sure what's going on, but we've got a hunch
that not setting cookies on image fetches sounds sensible. Images are
always loaded as a subresource (ie: through a `src` tag in an html
element), so they should never need to change the cookies, so this seems
sensible. We've done this by creating a new blueprint that doesn't set
session.permanent, and doesn't call `save_serivce_or_org_after_request`
either.
cookies are sent back to the browser if:
`sesion.modified or (session.permanent and 'REFRESH_EVERY_REQUEST')`
(where the latter is a config setting).
Turning off REFRESH_EVERY_REQUEST (which is True by default) means that
we will only update the sesion if it's been modified. In practice,
literally every request is modified in the after_request handler
`save_service_or_org_after_request`. This is accidentally convenient,
as it guarantees that we'll still send back the cookie normally even
though refresh_every_request is disabled. Sending back the cookie
updates the expiry time (20 hours), so we need to keep doing this to
preserve existing session timeout behaviour.
Re-upload button is only shown if file failed validation.
Change wording of re-upload buttons
Make test we test right buttons on letter upload preview page
Also remove double backlink
We can use the ‘Uploads’ feature to check if letters are printable now.
This code works in a completely different way, so if we kept it we’d
have to maintain two different code paths, and make sure that they
didn’t diverge.
Also deletes the related HTML templates.
Since we’re only showing this page to team who are using the API we
don’t have to worry about explaining what’s going on in terms of the
spreadsheet any more.
This makes the page simpler.
We introduced the ‘breaking change’ page[1] partly to help teach people
about the relationship between the placeholders in their template and
the data they were providing. Data can be provided either by API or by
uploading a spreadsheet. The users who we struggled to communicate this
relationship to were the ones using the upload a spreadsheet feature.
We made two changes to the context of this feature:
1. Around the same time we introduced the interactive tour[2], which
ultimately proved to be the thing that helped people understand the
relationship between the data they were providing and the
placeholders in the template.
2. We introduced a way for people to send one-off messages without
using the API or uploading a spreadsheet[3]. So for this page to say
that you’ll need to update a spreadsheet or change an API call if you
change the placeholders in your template is no longer accurate.
Therefore I think it makes sense to only show this page to teams who are
using the API to send messages. The best proxy we have for that is to
look at whether they’ve created any API keys.
***
1. https://github.com/alphagov/notifications-admin/pull/631
2. https://github.com/alphagov/notifications-admin/pull/613
3. https://github.com/alphagov/notifications-admin/pull/1293
Requests to go live and email branding requests come through to Zendesk
with tags attached automatically.
With the revised taxonomy some of these tags need to be updated, as
summarised in this spreadsheet.
In addition, `notify_action` tag has to be added in each of those cases.
Old|New
---|---
`notify_request_to_go_live_complete`|`notify_go_live_complete`
`notify_request_to_go_live_incomplete`|`notify_go_live_incomplete`
`notify_action_add_branding`|`notify_branding`
`notify_request_to_go_live_incomplete_mou`|`notify_go_live_incomplete_mou`
`notify_request_to_go_live`|`notify_go_live`
– https://docs.google.com/spreadsheets/d/1o5ATsFsVK8Qpj7x8QvxX-SfEuBZ75028GEySVcdBFYU/edit#gid=0
– https://www.pivotaltracker.com/story/show/169842970
Flask will pick the first route that matches. Decorators get applied
from innermost to outermost.
So if the same endpoint is served at `/abc` and `/123` the one used
when `url_for` is generating a URL is whichever decorator is lowest
(in terms of line number).
It doesn’t functionally make a difference, but it’s causing the
functional tests to fail at the moment. And shorter URLs are nicer, so
I think it makes sense to change here, rather than change the tests.
`all` is not a real template type, so for links to template folders that
apply to all template types we have a URL that looks like:
```
/services/<uuid:service_id>/templates
```
However Flask only generates this url when `url_for` is called with
`template_type=None`. If called with `template_type=all` then Flask will
generate a URL like
```
/services/<uuid:service_id>/templates/all
```
However attempting to load this URL will now 404, since `all` is not a
template type recognised by the regex introduced in
https://github.com/alphagov/notifications-admin/pull/3176
It would be nice to not have URLs with `all` in them at all, but since
people might have bookmarked them we need to support them indefinitely.
Also considered but decided against adding `all` to the set of template
types because it might cause other problems, for example attempting to
create a new template with a type of `all` would never work.
