How this happens: a user starts to send a letter job, then in another tab starts a SMS
or email job, the sender_id is set in the session. Then the user goes
back to the letter job tab and creates the job. The sender_id is set in
the metadata of the csv file, and causes an exception when trying to persist
the letter notification.
This PR adds a check to ensure the sender_id is not set for letter jobs.
This will catch a small use case where the user has multiple tabs open
and has started sending an SMS or email job, then tries to send a letter
job.
For someone who has retrieved a template ID from their system the only
way to find it in Notify is:
- hack the URL
- click through every template, visually inspecting the ID shown on the
page until you find the right one
Neither of these is ideal.
This commit adds searching by ID, for those services who have an API
integration. This means we don’t need to confuse teams who aren’t using
the API by talking about IDs.
This is similar to how we let these teams search for notifications by
reference[1]
1. https://github.com/alphagov/notifications-admin/pull/3223/files
The Python rtree library we are using to build RTrees has a dependency
on the C package libspatialindex. This package is not installed on PaaS,
so it’s hard for us to use it.
This commit changes the code to use a library called rtreelib instead.
rtreelib doesn’t have a built in way to serialise the index it builds,
so I’ve had to implement that using pickle.
We want to know how many phones are in a user-supplied polygon, so we
can show the impact of a broadcast, in the same way that we do when
users pick areas from our library.
We already know how many phones are in each electoral ward. But there
are challenges with an arbitrary polygon:
- where it does overlap a ward, the overlap could be partial
- it could overlap more than one ward
- finding out which wards it overlaps by brute force (looping through
all the wards and seeing which ones intersect with our polygon) would
be way to slow to do in real time
Instead we can use a data structure called an R-tree[1] to build an
index which provides a much, much faster way of looking up which
polygons overlap another. We can build this tree in advance and save it
somewhere, which means there’s a lot of computation we don’t need to do
in real time.
The R-tree returns a set of objects (ward IDs) which we can go and look
up in our library of electoral wards. These wards will be the ones that
might have some overlap with our custom polygon.
Once we have this small set of wards which might overlap our ward, we
can look at the size of the area of overlap (relative to the size of the
whole ward) and multiply that by the known count of phones in that ward
to get an approximation of the count of phones in the overlap area.
Summing these approximations give an estimate for the whole area of the
custom polygon.
1. https://en.wikipedia.org/wiki/R-tree
Now we’ve split the old alerts onto two pages the verbs (‘Broadcast’ and
‘Rejected’) will always be the same for each alert – so they’re not
adding any differentiation.
The specifics of what the datetime means is available on the page for
each alert.
Removing the verbs makes the page a bit less cluttered and makes it
easier to scan down the right hand column.
The code for this page was making assumptions about properties which
aren’t present on rejected broadcasts.
This commit accounts for those properties and presents the relevant
elements on the page.
> Rule of three ("Three strikes and you refactor") is a code refactoring
> rule of thumb to decide when similar pieces of code should be
> refactored to avoid duplication. It states that two instances of
> similar code don't require refactoring, but when similar code is used
> three times, it should be extracted into a new procedure
– https://en.wikipedia.org/wiki/Rule_of_three_(computer_programming)
we saw exceptions on prod that I think might have caused a worker that
is being terminated to die ungracefully. While I'm not sure if this is
an actual problem that changed behaviour (app instances crashing and
restarting), at the very least it definitely polluted the logs and
obscured any actual issues we were having.
https://github.com/prometheus/client_python/releases/tag/v0.10.0
see the pending fix for this problem here:
https://github.com/prometheus/client_python/pull/644
Two reasons to not hide rejected broadcasts:
- if a broadcast was rejected by mistake then it’s useful to have an
audit of who did that
- it means you can still see old broadcasts without having to leave
in pending-approval, which is dangerous because they might
accidentally be approved
We've seen someone getting a `500` when trying to visit the `/metrics`
endpoint. This is because the metrics endpoint is not in the `main`
blueprint, so we can't use url_for with a relative endpoint.
Although only bots visit '/metrics' without authorisation, this stops
the odd `500`.
The API has a method to handle setting the default SMS free allowance. This will save a call to the API and remove some code duplication between the two apps.
Needs to be merged after https://github.com/alphagov/notifications-api/pull/3197
A commit was added:
600e3affc1
In it, it falls back to the string 'Unknown' for actions done by those
not belonging to the service.
This commit changes the behaviour such that if the user is not in the
list of active users for a service, it will go get the user from the DB
(or redis). This should be fine to do as redis will protect us from most
calls as most of these cases are for platform admins.
This will mean we can now see which user platform admin put a service
live rather than seeing 'Unknown'.
