this was added five years ago but never used. if we want to bring back
variable rates per client we might as well get a fresh start since a lot
has changed since then.
We are in a weird situation where at the moment, we have services with
the broadcast permission that do not have a row in the
service_broadcast_settings table and therefore do not have defined
whether they should send messages on the 'test' or 'severe' channel.
We currently get around this when we send broadcast messages out as
such:
https://github.com/alphagov/notifications-api/blob/master/app/celery/broadcast_message_tasks.py#L51
We need to something equivalent for the broadcast channel that the API
says the service is on. In time, when we have added a row in the
service_broadcast_settings table for every service with the broadcast
permission then we can remove both of these two hardcodings.
Note, one option would have been to move the default of `test` on to the
`Service` model rather than having it in both the
broadcast_message_tasks file and the `ServiceSchema` class. However, I
went for the quickest thing which was to add it here.
This will allow us to store details of which channel a service should be
sending to.
See the comment about how all broadcast services can have a row in the
table but may not at the moment. This has been done for speed as it's
the quickest way to let us set up different services to send to
different channels for some needed testing with the mobile handset
providers in the coming week.
We already serialise it in the templates response. We should make sure
the field is also present in the history response, if we want to use
cached template versions when processing notifications.
Broadcast services only have broadcast templates. But on the frontend
we show the template type under the name of the template. This is
redundant. It would be better to preview the content of the template
instead.
At the moment we can’t do this because we optimised the template schema
response to not include the content of the templates in
https://github.com/alphagov/notifications-api/pull/2880
This commit reverses that optimisation, for broadcast templates only,
and expands the tests to cover all template types.
make it clear that this is for the public api, and we shouldn't add
fields to it without considering impacts
also add the broadcast_messages relationship on service and template to
the exclude from the marshmallow schemas, so it's not included elsewhere
had to go through the code and change a few places where we filter on
template types. i specifically didn't worry about jobs or notifications.
Also, add braodcast_data - a json column that might contain arbitrary
broadcast data that we'll figure out as we go. We don't know what it'll
look like, but it should be returned by the API
This reverts commit 58a9862cd1.
That commit tried to optimize the fetch template query by
However it had the side effect of making Marshmallow ignore `created_by`
when loading the JSON in the post request. So the field on the model was
never being updated, just copied from the original template.
The quick way of getting things to work again is to revert this
optimisation.
There’s probably some Marshmallow magic we could use to avoid the extra
query and still have created_by not be ignored.
It does mean we’re introducing an extra query, but I’m not too fussed
about that since the caching seems to be working well.
Because the IDs of our callback and inbound SMS APIs were stored in
lists instead of directly on the serialised model they weren’t getting
cast to a string before trying to JSONify them. And JSON doesn’t know
what to do with a UUID object.
For some reason this was only affecting the endpoint for fetching
inbound SMS.
We need to serialise the template to JSON to store it in Redis. Python’s
built in JSON serialiser doesn’t know what to do with a UUID object, so
we need to manually cast it to a string instead.
It’s a UUID column, but by default Marshmallow wants to select the id
from the users table, not from the templates table, because the two
are foreign-keyed.
Adding the property explicity like this forces it to select from the
`created_by_id` column, but still serialises it to the `created_by`
field to avoid any breaking change.
Content and subject are user-submitted so are effectively unbounded in
size. And we’re serialising them for every template when sending the
list of templates to the admin app.
For the service with the most templates this results in a 1.3Mb blob of
JSON going over the wire, and being cached in Redis.
And then the admin app completely ignores these fields, because it does
show template content until you’ve clicked into a single template.
This commit adds a new query parameter, `detailed`, that the admin app
can set to `False`. When it does only the fields needed to render the
`/templates` page are returned.
This is done with a new parameter so as not to break the V1 API.
Although I looked in Kibana and it doesn’t seem like anyone external is
using this endpoint we’ve come this far without breaking the API so…
By default Marshallow includes unknown properties. This means every time
a new property is added to the service model it gets included in the
JSON-serialised response sent to the admin app.
This is particuarly bad because it means that for returned letters the
ID of every returned letter. So the JSON stored in Redis for the
Check Your State Pension service is 86kb.
Similarly the JSON stored in Redis for a big user of inbound text
messaging is 458kb(!!!) because it has the ID of every received text
message. That’s ~8,500 UUIDs.
Luckily the admin app tells us exactly which keys it’s using here:
5952d9c26d/app/models/service.py (L31-L52)
```python
- `active`
- `contact_link`
- `email_branding`
- `email_from`
- `id`
- `inbound_api`
- `letter_branding`
- `letter_contact_block`
- `message_limit`
- `name`
- `prefix_sms`
- `research_mode`
- `service_callback_api`
- `volume_email`
- `volume_sms`
- `volume_letter`
- `consent_to_research`
- `count_as_live`
- `go_live_user`
- `go_live_at`
}
```
Plus these which it does not get automatically:
- `email_branding`
- `letter_branding`
- `organisation`
- `organisation_type`
- `permissions`
- `restricted`
The API is returning all of these:
- `active`
- `all_template_folders`
- `annual_billing`
- `consent_to_research`
- `contact_link`
- `contact_list`
- `count_as_live`
- `created_by`
- `crown`
- `email_branding`
- `email_from`
- `go_live_at`
- `go_live_user`
- `id`
- `inbound_api`
- `inbound_number`
- `inbound_sms`
- `letter_branding`
- `letter_contact_block`
- `letter_logo_filename`
- `message_limit`
- `name`
- `organisation`
- `organisation_type`
- `permissions`
- `prefix_sms`
- `rate_limit`
- `research_mode`
- `restricted`
- `returned_letters`
- `service_callback_api`
- `users`
- `version`
- `volume_email`
- `volume_letter`
- `volume_sms`
- `whitelist`
So the ones that the admin is getting but not expecting are:
- `all_template_folders`
- `annual_billing`
- `contact_list`
- `created_by`
- `crown`
- `inbound_number`
- `inbound_sms`
- `letter_logo_filename`
- `rate_limit`
- `returned_letters`
- `users`
- `version`
- `whitelist`
Which is what this PR adds to the exclude list, except for `created_by`
which is keeps because it’s needed to validate the JSON provided when
creating a service.
If you’ve sent a bunch of jobs from the same contact list then a handy
way to differentiate between them will be date sent, but also template
name (in effect the message you sent).
This commit extends the job response to include template name, using the
same pattern as for template type.
We’ve added some new properties to the templates in utils that we can
use instead of doing weird things like
`WithSubjectTemplate.__str__(another_instance)`
This is so we can display letter jobs in a different way on the admin
app (because it doesn’t make sense for them to have failed/delivered
counts like it does for email and text message jobs).
As elsewhere we use `fields.Method` to avoid serializing the whole
template object.
and update it when users have to use their email to interact with
Notify service.
Initial population:
If user has email_auth, set last_validated_at to logged_in_at.
If user has sms_auth, set it to created_at.
Then:
Update email_access_valdiated_at date when:
- user with email_auth logs in
- new user is created
- user resets password when logged out, meaning we send them an
email with a link they have to click to reset their password.
previously we checked notifications table, and if the results were
zero, checked the notification history table to see if there's data
in there. When we know that data isn't in notifications, we're still
checking. These queries take half a second per service, and we're
doing at least ten for each of the five thousand services we have in
notify. Most of these services have no data in either table for any
given day, and we can reduce the amount of queries we do by only
checking one table.
Check the data retention for a service, and then if the date is older
than the retention, get from history table.
NOTE: This requires that the delete tasks haven't run yet for the day!
If your retention is three days, this will look in the Notification
table for data from three days ago - expecting that shortly after the
task finishes, we'll delete that data.
Updated the add_user_to_service endpoint to only handle data in the
'new' format (`{"permissions": [...]}` instead of `[permission_1, permission_2]`)
since Admin has been updated to send data the new way.
This change means that we no longer need the Marshmallow Permission
schema, so it can be deleted.
The template preview app now accepts a null value for the `filename`
parameter. If a service doesn't have a letter branding option set,
previously we defaulted to their dvla_organisation (probably HM
Government). Now, we pass through None, so that we generate letters
without any logo or branding.
Flask-SQLAlchemy paginate function issues a separate query to get
the total count of rows for a given filter. This query (with
filters used by the API integration Message log page) is slow for
services with large number of notifications.
Since Message log page doesn't actually allow users to paginate
through the response (it only shows the last 50 messages) we can
use limit instead of paginate, which requires passing in another
flag from admin to the dao method.
`count` flag has been added to `paginate` in March 2018, however
there was no release of flask-sqlalchemy since then, so we need
to pull the dev version of the package from Github.
Added the filename of a service's letter logo to the service schema. We want
this in the schema so that it is possible to call
`current_service.letter_logo_filename` from notifications-admin and to pass this value
through to template-preview.
