This is because that function is used both when broadcast status
is updated via API and via admin, so it's a shared resource.
Also move and update tests for updating broadcast message status
so things are tested at source and repetition is avoided.
We don’t store everything that comes in the CAP XML when someone creates
a broadcast via the API.
One thing we do store is `<identifier>` (in a column called `reference`)
which is a unique (to the external system) identifier for the broadcast.
We show this in the front end instead of the template name, because
broadcasts created from the API don’t use templates.
However this ID isn’t very friendly – the Environment Agency just supply
a UUID.
The Environment Agency also populate the `<event>` field with some human
readable text, for example:
> 013 Issue Severe Flood Warning EA
(013 is an area code which will be meaningful to the Flood Warning
Service team)
We should show this in the UI instead of the reference. The first step
towards this is storing it in the database and returning it in the REST
endpoints.
Later we can have the admin app prefer `cap_event` over `reference`,
where `cap_event` is present.
We can’t backfill this data because we don’t keep a copy of the original
XML.
Seems like `<event>` is a mandatory property of `<info>`, so we don’t
need to worry about the field being missing (`<info>` is optional in
CAP but we require it because it contains stuff like the areas which
we need in order to send out the broadcast`).
***
https://www.pivotaltracker.com/story/show/176927060
This is so we can distinguish custom broadcasts in the Admin app
[1]. I've also extended the POST test for custom broadcasts to
check we're correctly reading data for "names", as this wasn't
being tested previously.
[1]: 411fda81c0
In one case ("areas=['manchester']") the format was even invalid,
but in general the original value of the column is pretty much
irrelevant for tests that involve updating it (it's highly unlikely
the column would default to the same value as the test data).
This is necessary until:
- The Admin app is using the new "areas(_2)" format to store and
retrieve data.
- We've migrated all existing broadcast messages to use the new
format.
Note that "areas" / "ids" isn't actually used for anything except
printing out the PagerDuty message - it's not sent to the proxy [1].
[1]: 6edc6c70aa/app/celery/broadcast_message_tasks.py (L190-L193)
Currently we have:
- An "areas" column in the DB that stores a JSON blob.
- An "areas" field inside the "areas" JSON that stores area IDs.
- Each field has to be manually copied into the JSON column.
We want to move to:
- An "areas" column in the DB (unchanged).
- An "ids" field inside the "areas" JSON (to replace "areas").
- The Admin app sending other data inside an "areas" JSON field.
The API design for areas is confusing and difficult to extend.
Here we duplicate the current API functionality using an "areas_2"
field. Once the Admin app is using this field, we'll be able to
rename it to just "areas", which is where we want to get to.
In the next commits we'll build on this to support the migration
from "areas"."areas" to "areas"."ids".
This is a temporary feature to make it easy to migrate the format
of the "areas" column and backfill extra data for it.
It's not possible to use this feature to update the status of an
old broadcast message, so the risk from this override is minimal.
This was mentioned in an old pen test report that you could send a
request twice to set a broadcast message as broadcasting which would
trigger us to send two alerts.
It looks like this is now fixed and this test coverage backs that up.
Note, it's unlikely that it would have been an issue anyway as the CBC
would likely have rejected the message as it would notice it is a
duplicate.
Note, this test coverage is not supposed to be exhaustive of all the
potential transitions but covers the vast majority of ones that we care
about.
See `BroadcastStatusType.ALLOWED_STATUS_TRANSITIONS` for allowed
transitions.
It looks like we were allowing broadcasts to transition from draft to
broadcasting in one go. This isn't valid now. It should go draft,
pending approval and then broadcasting.
It looks like this was a leftover bit of support in our code for when we
were building stuff out and is no longer needed.
We found that non-templated broadcast messages weren’t having their
content normalised before saving into an event.
This means that stuff like `\r\n` and curly quotes were being passed
through to the CBC proxy.
This commit firstly changes templated events to use
`str(BroadcastMessageTemplate)` to normalise the content, because it’s
non-obvious that calling
`BroadcastMessageTemplate.content_with_placeholders_filled_in` also
normalises content.
Then it changes the non-templated route to also call
`str(BroadcastMessageTemplate)`, where previously it was passing the
content straight through.
This changes the content length validation of the internal API to match
the validation of the public broadcast API[1].
This removes the length check from JSONSchema, which isn’t sophisticated
enough to deal with things like normalising newlines or handling
different encodings.
The admin app should catch these errors before they’re raised here, but
it’s best to be belt and braces.
1.7ab0403ae7/app/v2/broadcast/post_broadcast.py (L53-L63)
Also update error message for when someone does not have permissions.
The message referenced approving broadcasts specifically, whereas
people would also see it if they try to cancel or reject
broadcast without permission.
Why we allow platform admins to cancel broadcasts:
we do this so they can react quickly if a broadcast was
approved by accident.
We only want to send a broadcast if the broadcast message is not stubbed
and the service is live at the point at which the broadcast event should
be created. This is to prevent the situation where a broadcast service is
switched to live / trial mode in between the message being created and
approved (we log an error if this happens).
A stubbed broadcast message with a trial mode service at the point of
approval is not an issue - trial mode services can approve their own
broadcasts. In this situation, we don't create the broadcast event but
also don't need to log an error.
If we're not going to send a broadcast, we don't need to create the
BroadcastEvent in the database. The BroadcastMessage contains all the
data we need - the BroadcastEvent is not used.
Not creating the event when we won't send the broadcast (e.g. when the
broadcast message was created when the service was in trial mode) adds
an extra layer of security.
otherwise we run into issues where we dont issue the cancel as we say
"oh look the expiry time just passed, so we shouldnt send this message
as it's already been removed from the cbc".
This has been added in for speed of development but now we are getting
close to integrating with production systems, we will be turning off
these helpful hacks to reduce the risk of someone sending a real
broadcast to citizens.
Note, platforms are still able to approve broadcasts when their service
is in training mode.
In the admin app we need something to use in show in lieu of template
name when a template isn’t used. Let’s store this in the reference field
for now.
new broadcast messages will have content filled whether they have a
tempalte or not, but old ones won't so populate.
Stole the session constructor from 0044_jos_to_notification_hist.py
ensures code remains backwards compatible during the deploy. this commit
should be reverted once all broadcast_message.content fields have been
back-filled.
we want to be able to create broadcast messages without templates. To
start with, these will come from the API, but in future we may want to
let people create via the admin interface without creating a template
too.
populate a non-nullable content field with the values supplied via the
template (or supplied directly if via api).
While the API still has a column and field for personalisation I think
it makes sense for it to consider the personalisation when serialising
the broadcast, so we should have a test for this.
This way the API still works as a coherent whole, and the admin app just
happens to be a client of the API which doesn’t implement the
personalisation feature.
If we want to remove personalisation from the API at another time we
should do it wholesale.
It is not of the form
[[lat, long][lat, long]] as this would only hold a single polygon. It
instead needs to handle multiple polygons so instead is of the form
[[[lat, long][lat, long]]].
use the new endpoint from cbc proxy. create a new task that just
serializes the event and sends it across rather than sending a template
and the broadcast message.
some changes to serialize to make it json friendly etc. it also expects
sent_at and transmitted_finishes_at to always be set (we set them in the
code but don't enforce it n the DB right now), as they're required by
utils template. not sure whether we'll update db constraints to be more
strict or utils template to be more permissive just yet, wait until we
find out more about the requirements of the CBCs we integrate with.
