Having SMS senders that start with 00 can cause issues with Firetext due
to Firetext's validation rules, so we shouldn't allow SMS senders to start
with 00.
Firetext treats a double 00 at the start of the senderID as an international
prefix, so removes them. A sender of 00447876574016 would become 447876574016.
Under Firetext's validation rules, an SMS sender of five 0s (00000) would
become 4400. This is because the first 00 are removed (as the international
prefix). The third 0 is seen as the start of a phone number, and becomes 44,
leaving the final 00 = 4400.
Currently requests to the API made from the admin app are going from
PaaS admin app to the nginx router ELB, which then routes them back
to the api app on PaaS.
This makes sense for external requests, but for requests made from
the admin app we could skip nginx and go directly to the api PaaS
host, which should reduce load on the nginx instances and
potentially reduce latency of the api requests.
API apps on PaaS are checking the X-Custom-Forwarder header (which
is set by nginx on proxy_pass requests) to only allow requests going
through the proxy.
This adds the custom header to the API client requests, so that they
can pass that header check without going through nginx.
When downloading a report of a which messages from a job have been
delivered and which have failed we currently only include the Notify
data. This makes it hard to reconcile or do analysis on these reports,
because often the thing that people want to reconcile on is in the data
they’ve uploaded (eg a reference number).
Here’s an example of a user talking about this problem:
> It would also be helpful if the format of the delivery and failure
> reports could include the fields from the recipient's file. While I
> can, of course, cross-reference one report with the other it would be
> easier if I did not have to. We send emails to individuals within
> organisations and it is not always easy to establish the organisation
> from a recipient's email address. This is particularly important when
> emails fail to be delivered as we need to contact the organisation to
> establish a new contact.
– ticket 677
We’ve also seen it when doing research with a local council.
This commit takes the original file, the data from the API, and munges
them together.
Done using isort[1], with the following command:
```
isort -rc ./app ./tests
```
Adds linting to the `run_tests.sh` script to stop badly-sorted imports
getting re-introduced.
Chosen style is ‘Vertical Hanging Indent’ with trailing commas, because
I think it gives the cleanest diffs, eg:
```
from third_party import (
lib1,
lib2,
lib3,
lib4,
)
```
1. https://pypi.python.org/pypi/isort
We require that a user has a real reply-to email address before going
live. We can partially automate this by at least telling users who
haven’t done this.
This only applies for users that have email templates; we shouldn’t
bother users who aren’t going to send emails about this.
We need users to have created some templates before they go live, so we
can see what kind of messages they intend to send.
We can do this automatically based on the work done in
https://github.com/alphagov/notifications-admin/pull/1892
One of the things that we want to check before a service goes live is
that they have at least two team members with the manage service
permission. Anyone who can make a request to go live has this
permission, so that means one additional user is needed. This is what we
can automatically communicate to the user.
Under the hood this makes use of the logic added in
https://github.com/alphagov/notifications-admin/pull/1891
We only sign agreements for a whole council. Therefore we know that if
a council isn’t one of the ones who has signed an agreement, no part of
that council has signed the agreement, therefore it’s a `false` not a
`null`.
The thing that matters for which agreement an organisation has to sign
is whether or not that organisation is crown or non-crown.
There is only a partial overlap between crown/non-crown and
local/central. We can’t infer one fro the other. So this commit makes it
explicit by marking all local government organisations as non-crown,
which is something we can know for sure.
We don’t, for example, know the inverse, that all parts of all central
government organisations are crown bodies (but we can mark some of them
as being so later on).
The list of email domains is a different list from the list of all
government domains. And because the list of all government domains is
really long now, it could be unnecessarily slow to search through when
(a lot of the time) all we care about is whether the email address ends
with `.gov.uk`.
This means we’ll be able to tell them for sure that they don’t need to
signed it again.
List taken from looking through the folder of signed agreements on
Google Drive.
This adds information about which orgs have signed an MOU to the domain
list. The meaning of the attribute is:
- `true`: MOU signed for the whole organisation
- `false`: no MOU for any part of the organisation
- `null` (or missing): can’t be sure if it’s true or false
This commit:
- makes the logic around looking up a domain a bit more sophisticated
by matching on the longest domain name first
- exposes the details about an organisation to consumers of the
`GovernmentDomain` class
In some cases we can tell based on someone’s email domain whether they
work for a central or local government organisation, and whether they
will need to sign the MOU or agreement in order to go live. So this
commit creates a structure to store this information.
Makes it fiddlier to add new domains, and is only needed to generate the
regular expression. Much cleaner to just insert them as part of
generating the regular expression.
One of the things we need to know for a service to go live is whether
they have at least two users with the ‘manage service’ permission.
So this commit adds a method to the client to count how many users have
a given permission. We can do logic on this count later. But having the
counting done in the client feels like a cleaner separation of concerns.
Meant some refactoring of the way `service_id` is extracted from the
request, in order to make it easier to mock.
When users request to go live we check stuff like:
- if they’ve added templates
- if they have email templates (then we can check their reply to
address)
This commit adds a method to do this programatically rather than
manually.
We _could_ do this in SQL, but for page that’s used intermittently it
doesn’t feel worth the work/optimisation (and the client method is at
least in place now if we do ever need to lean on this code more
heavily).
Users who have the ‘manage API keys’ permission can see the settings
page. But they don’t have permission to request to go live.
At the moment they can still see the link, though clicking it gives them
a 403 error. This commit changes it so that they can’t see the link, and
tells them who they should speak to about going live (their manager).
We have tickets from people asking how long the process takes. I suspect
that this is because they’re not getting to the bottom of the form
before they’re ready to go live.
A lot of users aren’t reading or paying attention to the checklist on
the request to go live page. We think that we can get more people to
read it by putting it on its own page, where users won’t jump straight
to filling in the form.
This will, later on, let us make this page smarter by automatically
detecting if they’ve done the necessary things.
