Although using a team key is functionally the same as your service being
restricted, conflating the two errors is not helpful. What we typically
saw in research was that someone was using a team key, got the error,
used a live key and got the _same_ error.
This commit adds a new error message that specifically mentions the type
of API key that you’re using.
Scenario we saw in research:
- trying to send a message to someone outside your team
- service is in trial mode
Result:
- error message was terrible, no-one understood it
Solution:
- better error message
only in the public notification endpoint so far for fear of breaking
things - in an ideal world i'd remove the template relationship
from models entirely and replace that with actual_template
Removed all existing statsd logging and replaced with:
- statsd decorator. Infers the stat name from the decorated function call. Delegates statsd call to statsd client. Calls incr and timing for each decorated method. This is applied to all tasks and all dao methods that touch the notifications/notification_history tables
- statsd client changed to prefix all stats with "notification.api."
- Relies on https://github.com/alphagov/notifications-utils/pull/61 for request logging. Once integrated we pass the statsd client to the logger, allowing us to statsd all API calls. This passes in the start time and the method to be called (NOT the url) onto the global flask object. We then construct statsd counters and timers in the following way
notifications.api.POST.notifications.send_notification.200
This should allow us to aggregate to the level of
- API or ADMIN
- POST or GET etc
- modules
- methods
- status codes
Finally we count the callbacks received from 3rd parties to mapped status.
"with personalisation" should only be used by the public notification api
"with template" should be used when we want template name, etc details.
also added an xfail test for correctly constructing notification
personalisation
* tests on API endpoints that we do not explicitly sort should
either sort the results or compare results in an orderless way
(e.g. converting to a set)
* tests that touch the provider_details should reset values after
running, since the provider_details table is not torn down and
re-created between tests (unlike most tables)
We had a test like this for sending sms, but not email. This meant that,
for example, we weren’t checking that the provider was getting passed
the HTML and plain text versions of the email.
if api_key used to access endpoint is type team, endpoints only return that type -
will 404 if you provide a different ID. Same applies for normal (normal api keys
cannot see team notifications)
also, for convenience, set sample_notification to supply key_type of KEY_TYPE_NORMAL
by default
moved from notifications/rest -> service/rest and job/rest respectively
endpoint routes not affected
removed requires_admin decorator - that should be set by nginx config
as opposed to python code
If the notification ends up in the retry queue and the delivery app is restarted the notification will get sent twice.
This is because when the app is restarted another message will be in the retry queue as message available which is a
duplicate of the one in the queue that is a message in flight.
This should complete https://www.pivotaltracker.com/story/show/121844427
notifications, when retrieved by notification id, or service id (i.e.
all notifications for service).
There is a new element returned at top level of notification json called
body, which is the template content merged with personalisation. This
is consistent with api to endpoint to create notification which returns
what was sent as 'body' in json response.
Merging of template with personalisation is done in the
NotificationStatusSchema.
Personalisation data in encrypted before storing in db.
It’s going to be useful to see all the notifications for a job that are
failed/delivered/etc.
The API seems to support this behaviour already, but it doesn’t seem to
be tested.
This commit adds some testsfor the DAO and REST layers.
