This commit changes all the places where a user would see the term
‘whitelist’ in the content of page to say guestlist instead.
We’re removing the term ‘whitelist’ for two reasons. The first reason
is that we agree with the National Cyber Security Centre say:
> It's fairly common to say whitelisting and blacklisting to describe
> desirable and undesirable things in cyber security. For instance, when
> talking about which applications you will allow or deny on your
> corporate network; or deciding which bad passwords you want your users
> not to be able to use.
> However, there's an issue with the terminology. It only makes sense if
> you equate white with 'good, permitted, safe' and black with 'bad,
> dangerous, forbidden'. There are some obvious problems with this. So
> in the name of helping to stamp out racism in cyber security, we will
> avoid this casually pejorative wording on our website in the future.
> No, it's not the biggest issue in the world - but to borrow a slogan
> from elsewhere: every little helps.
– https://www.ncsc.gov.uk/blog-post/terminology-its-not-black-and-white
The second reason is that we’ve observed some users think that they have
to put recipients in the whitelist even when they’re already with in the
team. We think that the term ‘whitelist’ might be reinforcing this
mental model because of how ‘whitelists’ might work in other
applications.
We considered the following alternatives or concepts:
- Development
- Recipients
- Sandbox
- Extended team
- Smoke test recipients
- Allowed
- Nominated
- Bonus
- Additional
- Safe
- Team list
- Trusted contacts
- Designated people
- Guest list
- Team key list
We also considered not giving it a name, and explaining it as a nuance
of how the team key works. After mocking this up it felt more disjoined.
We think it’s still useful for the thing to have a name so that it’s
easy to refer to between the docs and the UI.
We like the term ‘guest list’ because:
- of how it sits with team members – members and guests in the abstract
- a guest list is a concept that a lot of people will be familiar with
– a list of people who can access a thing
- ‘guest’ is very different to ‘recipient’ – we want to mitigate any
confusion between this and the (emergency) contact lists
All paragraphs should have class="govuk-body", or be otherwise
custom-styled. This commit adds some extra checks to our test fixture
that looks for paragraphs that don’t have any styling. Our test coverage
is pretty good, so this should check almost all pages, and prevent
regressions.
I’ve done this in such a way that it can be extended for other elements
(e.g. links) in the future.
Fixes a bug where we were calling a wrapper method when instead we
should have been calling the redis_client. This had resulted in no
actual calls to redis happening.
'Session expired' or similar makes it sound like a new error.
It could confuse the user and make them think the sign in didn't work
and that their session has expired again.
So we went with:
The change you made was not saved. Please try again.
Google’s documentation says:
> robots.txt is not a mechanism for keeping a web page out of Google. To
> keep a web page out of Google, you should use noindex directives
We’ve implemented a noindex directive now, so we don’t need to serve
robots.txt any more.
Reimplements https://github.com/alphagov/notifications-aws/pull/796
Since deploying alphagov/notifications-utils#736 I’ve been looking at
how members of the public are ending up on our support page. The vast
majority are landing on https://www.notifications.service.gov.uk/features/email
Previously we thought that they were clicking the ‘contact us’ link in
the page, which deep linked into the support journey, so we removed
these deep links in alphagov/notifications-admin#3451
But the tickets are still coming in, so I think that people are still
landing on this page, then going directly to ‘support’ in the top
navigation. So the next measure we have available is to try to stop
people from landing on this page in the first place. All the examples
I’ve looked at show people coming from Google to this page. By putting
the page’s URL in our robots.txt it should stop Google (and other search
engines) listing it in search results.
Google’s documentation says:
> robots.txt is not a mechanism for keeping a web page out of Google. To
> keep a web page out of Google, you should use noindex directives
A noindex directive means adding the following meta tag to pages that
shouldn’t be indexed:
```html
<meta name="robots" content="noindex" />
```
It’s also possible to set the directive as a HTTP header, but this seems
trickier to achieve on a per-view basis in Flask.
I’ve implemented this as a decorator so it can quickly be added to any
other pages that we decide shouldn’t appear in search results.
the upload preview page has a file_id - this corresponds to the file in
the transient pdf uploads bucket. However, if the user already hit send
(and then navigated back) the file's no longer in that bcuket, it's been
moved to the regular letters-pdf bucket. So the s3 get request fails. To
avoid this, simply redirect to the notifications page if the file isn't
in the transient bucket. This is better for the user as it'll stop them
trying to submit it twice, and will provide more clarity on the status
of the notification too.
Our get_notifications fixture tries to be too clever and work out which
fields to return based on what arguments it’s called with. This is very
indirect and makes the tests less specific.
In other places we call the mocking code directly with arguments that
make it more explicit what the mocked response should be. This commit
does this for tests that we’d otherwise have to make changes to the
fixture for, because postage can no longer be `None` for letter
notifications.
When the admin app gets user objects from the API, these include a dict
of permissions by service for what the user can do to that services.
Permissions for inactive services are not included in the response as
per:
87cb6f2597/app/dao/permissions_dao.py (L66)
However, this causes a bug where a service is archived but cached user
data still tells us that the user has permissions to view the service.
This should not be the case and causes errors where users can still see
the archived service page, it's settings, and even request to go live
for it, because they are using old cached data for the user.
We solve this by deleting the users who are part of the service from the
cache.
We also delete the templates for this service from the cache as the
templates are also archived when we ask the API to archive the service
as per:
d95c0131e0/app/service/rest.py (L597)
Note, one decision I had to make was whether to delete the user cache
for just active team members or also invited users. Assuming an invited
user can't see the service until they've accepted their invite anyway, it
shouldn't make any difference whether we delete their cache or not.
International letters don’t have a choice of postage. Under the hood
they are either `europe` or `rest-of-world`.
So, for letters that we detect are international, this commit:
- removes the radios buttons that give users the choice of postage
- passes through either `europe` or `rest-of-world` to the API,
depending on what address we find in the letter
This will cause the API to 500 until it can accept `europe` or
`rest-of-world` as postage types, but this is probably OK because it’s
only our services that have international letters switched on at the
moment.
The endpoint works fine with it in the URL now instead, so we need stop
posting it. We can’t stop expecting it yet, because some old instances
will still be posting to the endpoint without the ID in the url.
In the future we need to get the metadata from the file in order to work
out what form validation rules should apply (postage is only required
for UK letters).
To start doing this we need all instances of the app accepting `post`
requests with the `file_id` in the URL, as well as in the form data (for
backwards compatibility).
API gives an error if it tries to add a user to a service and that user is
already a member of the service. This situation shouldn't occur - admin checks
if an invited user is a member of a service before calling API, but we
have seen this error occurring when there are two requests processing at
the same time.
This change catches the errors from API if a user is already a member of
a service and redirects the user to the service dashboard so that they
don't see an error page.
This is for consistency with how we do it for filenames in the previous
commit and moves the decoding into the `LetterMetadata` class for
abstracting this behaviour.
Small refactor of the LetterMetadata class needed to handle None case as
recipient can be None.
S3 can only handle ascii characters, therefore for filename which could
include non ascii characters, for example a filename with the character
'£' in it, we must encode these using urllib before saving it as s3
metadata. We then also make sure that it comes back decoded when
presenting it to the user.
These args are not inputs to the function under test, neither as way of
named arguments or as GET query parameters. I assume this has been
leftover from a previous refactor of behaviour.
S3 metadata only supports ascii characters. Whenever we save data to it
we need to make sure we encode it to save it and then decode it to
display it again to users. This abstraction will act as the place for
that decoding to happen so the rest of the code in our views doesn't
need to care about the encoding abstraction.
Up till now, when adding new organisation domain, if it was already
in use, we didn't handle the 400 we got back from API. This PR
adds handling for that error.
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)
