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.
It:
- saves repetetive boilerplate code
- does some extra checks (eg checking for a `200` response)
- makes the codebase less confusing to consistently do the same thing in
the same way
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: 2bd4f74ad0/app/dao/jobs_dao.py (L50)
The idea behind the sticky textbox on this page is so you can scroll
through a long email or letter to find where in the message the
placeholder appears, while still being able to see the textbox you
should be typing in.
With text messages, they’re hardly ever long enough for anything to be
off the screen – ie no scrolling is required.
However if the user does scroll, they can end up covering the message
content with the sticky top panel. Which then looks like the message
has disappeared, so they click ‘back’ in the browser, then click into
the message again to make the page reload.
This commit removes the stickyness when sending from a text message
template.
Adds caching for service data retention. This removes separate API
client methods to retrieve individual data retention records by id
or type in favor of a single method that fetches and caches all
retention settings configured for the service. This makes it much
easier to invalidate cache when settings change.
Lookup by id or type is provided by helper methods in the service
model.
Don’t think it’s necessary. Makes things consistent with the sent letter
page, which only says ‘Download as a PDF’.
This inconsistency would be more glaring now these pieces of text appear
in the same place, in adjacent steps of a journey.
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.
In trial mode you can’t send letters. But it’s still useful to be able
to build up a letter to see how it work.
Best place to put this error is before someone tries to send a letter
for real.
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.
We’ve had a support ticket saying:
> Hi, where a letter goes over to two sides, is there a way in the
> 'Preview' screen (or anywhere else) that I can see page two? I can
> see page one OK, but can't work out how to see what's generated on the
> second page.
Whether you’re about to send 1000s of letters – or just want to preview
how one will look – it’s probably useful to be able to see more than
just the first page.
Pytest is deprecating the direct calling of fixtures. One fixture that
we call directly quite a lot is `fake_uuid`. Since it just returns the
value of `sample_uuid()` we can either call that instead (where we need
a fixed value) or generate a new UUID each time (where a fixed value is
not needed).
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.
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.
We’re going to make it possible for some users to be members of a
service, but not have any permissions (not even `view_activity`).
There are some pages that these users should still be able to see
These are the pages that a user with ‘basic view’ would have been able
to see, excluding those that let them send messages.
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
‘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.
There are some teams who send jobs on a daily/weekly basis. They have
team members who only use Notify for this purpose. So they would
probably benefit from basic view, because they don’t need to see the
dashboard.
This commit:
- adds a new item (uploaded files) to the basic view navigation for
teams that have sent at least one job
- makes the job pages visible to basic view users
I think we should do this now, rather than as a later enhancement to
basic view. We only have one chance to announce the feature, so teams
who do send jobs may otherwise discount it as not useful for them and
the opportunity to have them use it is lost.
Commit 58cc1604a7 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.
This link is useful for people who are setting up templates and want to
test out how they look/how Notify works.
‘Caseworker’ users shouldn’t need to send themselves messages on a
regular basis, so this link is another thing we can take away.
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).
The main task that we think ‘caseworker’ users do is send one off
messages.
So this commit:
- makes sure users who don’t have the `view_activity` permission (ie
not ‘admin’ users) can still send messages
- adds navigation so that these users have a place to go from which to
start the process of sending a one off message
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.
If you’re in trial mode you can’t send letters for real. But you can
still upload a spreadsheet with multiple rows, and there’s no reason why
you shouldn’t be able to explore how Notify populate the letter for each
row of the spreadsheet (since this is something we let you do when you
can send the messages for real).
Updated notifications-utils. This brings in
- the renamed character sanitization classes
- the change to allow unicode in letter addresses (this lets us delete
a test that is no longer relevant)
Also replaced non-ascii characters in headers. This fixes a bug where
non-ascii characters in a CSV filename were causing errors since the
filename is also used in the header.
When you upload a CSV the check page takes the file name from the URL
and writes it to the S3 metadata for that file.
This also happens when you go to the .png version of this page.
The .png version of the page doesn’t have the filename in the URL. So it
re-writes the S3 metadata with an empty filename.
This means that all letter jobs sent recently have no file names. This
commit fixes this problem.
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
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.
We’re going to stop storing job metadata in the session. So we can’t
rely on it for checking whether a file is valid. That safeguard is
happening in the API instead now (because it’s looking at the metadata
stored in S3).
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
```
By doing this we no longer have to store it in the session. This is the
last thing that’s currently in the session, so removing it means we can
drop session storage for file uploads entirely.
Storing things in the session is proving buggy – we still have one user
(that we know about) where the session data isn’t getting written, so
they’re blocked from uploading a file.
Since all the info we’re storing in the session is about the file, it
makes sense to store it with the file.
This commit only does the writing of the metadata, once we’re sure this
is working we can do subsequent work to read it back, and remove
reliance on the session.
we were seeing isort produce different outputs locally and in docker -
this was due to it having different opinions about whether the tests
module (ie all our unit tests) is a first party (local) or third party
(pip installed) import. It's a first party import, so by defining this
in the setup.cfg isort settings, we can force it to be consistent
between environments.
Note: I don't know why it was different in the first place though
