Adding a ‘testing’ template it not enough. It needs to have some real
looking content, so that we can:
- work out what a service is doing
- assess whether that’s a reasonable (ie meeting the terms of use) thing
to be doing with Notify
At the moment we’re having to go back to services quite a lot when they
request to go live and ask them for this stuff.
The check page expects template ID to be passed through in the URL not
the session now. The send test letter page wasn’t changed.
This commit changes it, and adds a test to make sure this path is
covered.
The start job endpoint needs the template ID in order to make the API
call.
It doesn’t make sense to add it to the start job URL, because users
could potentially start a job with the wrong template by hacking the URL
(which would blow up at some point, if the template didn’t match the
columns in the file).
A of this commit’s parent we are storing `template_id` and
`original_file_name` in the URL. Getting them from the URL is better,
so the check page no longer needs to look for them in the session. This
commit removes the code that looks for these values in the session.
At the moment you can’t press refresh on the check page if there’s
errors. This is because the session gets cleared when there’s errors.
This is a bad user experience.
The data that this page is relying on (from the session) is:
- template ID
- original file name
Neither of these things need to be in the session because:
- they are not secret
- the user can modify them already (by choosing a different template or
renaming their file locally)
So this commit additionally stores them in the URL.
Because we now[1] store info about each file upload separately in the
session the session isn’t overridden every time you upload a file. This
is good because you can do multiple file uploads idempotently.
Generally we are cleaning up after ourselves because we pop anything to
do with that upload from the session. However there is an edge case: if
you never send the file then the info about the file stays in the
session in perpetuity[2]. This is generally happening when people are
uploading files that are impossible to send, ie ones that have errors.
So this commit makes two changes:
1. remove info about a file upload from the session as soon as we know
that it contains errors
2. `POST` reuploads to the same endpoint as initial uploads because
otherwise we need to keep info about bad uploads in the session,
which would prevent us from doing 1.
1. https://github.com/alphagov/notifications-admin/pull/1968
2. or at least until the session is cleared by the user logging out
We prefer people downloading the agreement if they can. If we don’t know
which agreement they should be using (ie we don’t know their crown
status) then we fall back to having them contact us.
Rather than making users contact us to get the agreement, we should just
let them download it, when we know which version to send them.
This commit adds two endpoints:
- one to serve a page which links to the agreement
- one to serve the agreement itself
These pages are not linked to anywhere because the underlying files
don’t exist yet. So I haven’t bothered putting real content on the page
yet either. I imagine the deploy sequence will be:
1. Upload the files to the buckets in each environment
2. Deploy this code through each enviroment, checking the links work
3. Make another PR to start linking to the endpoints added by this
commit
Previously, we were looking at the day of the week - so messages sent
six days ago would show up as "tomorrow". We now look at the actual
date, so that won't happen again.
We were also subtracting an hour to make 00:00 this evening show up as
"midnight today", despite it technically being tomorrow. However, this
means that 00:59 tomorrow morning would show up as "00:59 today", a
full day out. So reduce that to just a minute, so it doesn't affect
other times of day.
test_api_keys.py and test_api_integration.py were almost identical
files with only a few lines difference between them. By moving one
test we can now delete test_api_keys.py
I don’t think it’s a massive risk (we’re certainly mitigating against
any XSS), but having a page on a GOV.UK domain where you can prefill
text on the page from a query string probably isn’t great.
So this commit restricts prefilling the support form to a set of
named questions.
Precompiled letters can now have two additional states:
* pending-virus-check
* virus-scan-failed
Both new states should show in the notifications dashboard, and
virus-scan-failed should appear as an error state, with a descriptive
message. You should not be able to preview a letter in one of the two
new states, so the preview link has been removed for precompiled letters
in these states.
show_accounts_or_dashboard has logic about where you should redirect
to. If we let it do this, then that's nicer than duplicating its
logic. We found that it wasn't accounting for orgs in redirects
properly.
If you have a placeholder called `((phone number))` in your email
template, and you try to send a one-off message then the form input will
attempt to validate your ‘phone number’.
This is not helpful if you’re trying to put a landline number in your
email, for example.
This only affects messages being sent through the one-off interface.
This commit makes the form be aware of template type, which fixes the
problem.
We shouldn’t tell people on one page (the terms page) that we know about
their organisations agreement and then on the pricing page tell them to
contact us to find out what we know about the agreement.
So this commit adds the same logic from the terms page to the pricing
page, with wording that makes sense in the pricing context.
People are emailing us asking if their organisation has signed the
agreement. In some cases they have, so this is a waste of their and
our time.
This commit adds a bit of logic to the terms of use page to tell users
when their organisation has already signed the agreement.
also, refactor the org tests into a folder to avoid megalith testing
files.
Also hardcode the org id to a variable so it can be referred to from
various places. In conftest there is now ORGANISATION_ID, which
represents an organisation that `active_user_with_permissions` has
access to
platform_admin is a separate concept to permissions, so by removing the
checks for it from the current_user.has_permissions function, we can
simplify things greatly. We already record on the user whether they're
a platform admin anyway.
