Commit Graph

1051 Commits

Author SHA1 Message Date
Paul Craig
ffb813bb3f Add get_notifications json schema + tests
Add a schema that corresponds to our response for returning lists
of notifications, and test with a contract test.
2016-11-25 16:50:20 +00:00
Paul Craig
df7450698c Test returning notifications by "?status=failed"
Check that all failure states are returned by asking for
notifications of type "failure".
2016-11-25 15:26:40 +00:00
Paul Craig
57a0d7295d Rewrite failed statuses
There are no more notifications whose statuses are "failed", as
the "failed" status has now been replaced with statuses that are
more specific about the nature of the failure.

However, we still want to be able to filter by failing
notifications. (ie "/v2/notifications?status=failed").

Created a `.substitute_status()` method which takes a status
string or list of status strings and, if it finds 'failure',
replaces it with the other failing status types.

This way, calling for nottifications with "?status=failed" is
internally treated as
"status = ['technical-failure', 'temporary-failure', 'permanent-failure']"
2016-11-25 15:23:46 +00:00
Paul Craig
7d009915a4 Group 'completed' notification statuses
Some notification statuses assume that a notification has been
updated (ie, it cannot have been created in that state).

This caused a bug in our sample notification fixture when trying
to create a notificaiton in a 'complete' status.

This commit groups the completed statuses in a list, the way other
statuses have been grouped together so that they're more portable.
Also fixed the sample_notification fixture.
2016-11-25 15:23:04 +00:00
Paul Craig
640f51fc0a Add tests for getting back lists of notifications 2016-11-25 15:23:04 +00:00
Paul Craig
effbd315e0 Create 'v2' get notifications route
Return multiple notifications for a service.
Choosing a page_size or a page_number is no longer allowed.
Instead, there is a `next` link included with will return the
next {default_page_size} notifications in the sequence.

Query parameters accepted are:

 - template_type: filter by specific template types
 - status: filter by specific statuses
 - older_than: return a chronological list of notifications older
   than this one. The notification with the id that is passed in
   is _not_ returned.

Note that both `template_type` and `status` can accept multiple
parameters.  Thus it is possible to call
`/v2/notifications?status=created&status=sending&status=delivered`
2016-11-25 15:23:03 +00:00
Paul Craig
1fce30aaa7 Cost savings
The "cost" value was flawed for a couple of reasons.

1. Lots of messages are free, so in those instances the "cost"
   doesn't tell you anything
2. The query to get the rate was expensive and we don't have
   an obvious way to get it back very efficiently for large numbers
   of notifications.

So we scrapped it.
2016-11-25 15:23:03 +00:00
Paul Craig
f1ea39d4c0 Simplify _filter_query() function 2016-11-25 11:23:20 +00:00
Rebecca Law
669126d41b Merge pull request #744 from alphagov/add-default-exception-handler
Add application level error handler.
2016-11-25 09:34:23 +00:00
minglis
2e1578656b Merge pull request #735 from alphagov/caching-with-redis
Caching with redis
2016-11-25 09:06:12 +00:00
Rebecca Law
52c8596c92 Add application level error handler.
This should catch any unexcpeted exceptions.
2016-11-24 17:09:56 +00:00
Paul Craig
be5d503133 Merge pull request #743 from alphagov/stringify-cost-attr-on-v2-get-notification
Stringify the cost before passing to jsonify as it complains otherwise
2016-11-23 10:50:59 +00:00
Martyn Inglis
2a2ef34339 Merge branch 'master' into caching-with-redis
Conflicts:
	app/__init__.py
2016-11-23 09:12:11 +00:00
Imdad Ahad
9b6584c8a1 Make result of notification.cost() into a float
Cost was returning a `Decimal`, which jsonify doesn't like.
Making it a float fixes the problem.

[Relevant issue on github](https://github.com/pallets/flask/issues/835).
2016-11-22 17:58:16 +00:00
Rebecca Law
3cddc64e36 Merge pull request #741 from alphagov/fix-v2-post-notification
Small fixes for the V2 post notification endpoints
2016-11-22 16:28:13 +00:00
Martyn Inglis
0c7d9e33aa Merge branch 'master' into caching-with-redis
Conflicts:
	tests/app/notifications/test_process_notification.py
2016-11-22 13:00:37 +00:00
Martyn Inglis
9e2ba9ee81 Pushed the cache increment into the shared code that persists notifications.
Much simpler implementation, inc code removed from tasks and V1/V2 rest clients.
2016-11-22 12:53:20 +00:00
Rebecca Law
46beece158 For the post_sms_response and post_email_response the reference property is always present but the value can be null.
Added a test for an empty reference.
Remove datetime format on the created_at attribute of a notification, it is not needed.
2016-11-21 17:32:36 +00:00
Paul Craig
c1fa5e156a Append "Z" to DATETIME_FORMAT
We're formally using the ISO 8601 UTC datetime format, and so the
correct way to output the data is by appending the timezone.
("Z" in the case of UTC*).

Unfortunately, Python's `datetime` formatting will just ignore the
timezone part of the string on output, which means we just have to
append the string "Z" to the end of all datetime strings we output.

Should be fine, as we will only ever output UTC timestamps anyway.

* https://en.wikipedia.org/wiki/ISO_8601#UTC
2016-11-21 15:59:10 +00:00
Paul Craig
82ba2cd226 Create new notification schema
This is the schema that individual notifications will conform to
when they are returned from this API.
JSON logic enforces that the right keys are set depending on the
`"type"`. (eg a schema with `"type": "sms"` must have a
`"phone_number"` value and it cannot have an `"email_address"`)
2016-11-21 15:59:10 +00:00
Paul Craig
9758b96a2b Create 'v2' get notification route
The new 'v2' API wants to return less data than the previous one,
which was sending back tons of fields the clients never used.

This new route returns only useful information, with the JSON
response dict being built up in the model's `.serialize()` method.

Note that writing the test for this was a bit painful because of
having to treat loads of keys differently. Hopefully we think this
is a good way to write this test, because if we don't, we should
start thinking of a better way to check the values are what we
expect.
2016-11-21 15:41:49 +00:00
Martyn Inglis
58bbc5a5aa Now that we have discovered that the catch all Exception handler doesn't work, I've created a custom exception (SendNotificationQueueError) that handles this case and an error handler for it.
Talked these through with @becca as an approach.
2016-11-21 15:11:19 +00:00
Martyn Inglis
7cfc58c994 Merge branch 'master' into caching-with-redis
Conflicts:
	app/celery/tasks.py
	tests/app/celery/test_tasks.py
2016-11-21 13:10:22 +00:00
Paul Craig
fddb1653ac Add .cost() to notification model
In the V2 API, the GET response for an individual notification
returns a 'cost' value, which we can get by multiplying the
billable units by the per-message rate of the supplier who
sent the message.
Any notifications with billable units > 0 but without a
corresponding `ProviderRates` entry will blow up the application,
so make sure you've got one.
2016-11-18 17:50:39 +00:00
Rebecca Law
f5e3c6f63b Remove print stmt and added assert error content 2016-11-17 14:02:44 +00:00
Rebecca Law
bc434f1736 Create new column in notifications and notification_history to store the client_reference because I remembered that reference is used for email providers. 2016-11-17 13:42:34 +00:00
Rebecca Law
dc5e21a78e Remove v2 error handler for InvalidPhoneError, no longer expect to throw this exception. 2016-11-17 09:09:24 +00:00
Rebecca Law
df62be421f Make email_from and subject required attributes of the email_content schema.
Update the format_checkers to raise the specific exception that why the validator can handle multiple messages.
Which led to a refactor of build_error_message.
2016-11-16 17:25:00 +00:00
Rebecca Law
68537d960a Merge branch 'master' into implement-v2-send-email 2016-11-16 16:29:41 +00:00
Rebecca Law
247668202b Fix functional tests 2016-11-16 16:15:30 +00:00
Rebecca Law
924cec05b4 Included reference when persisting the notification.
This was already part of the data model.
Not included in the send_sms|email task because there is no reference from the job.
2016-11-16 15:44:16 +00:00
Rebecca Law
a3eeb1a477 Merge branch 'master' into implement-v2-send-email 2016-11-16 15:18:03 +00:00
Rebecca Law
80f3bfcb2c Merge pull request #733 from alphagov/refactor-for-code-reuse
Refactor for code reuse
2016-11-16 15:17:30 +00:00
Rebecca Law
d6c537cf44 Added job_id and row_number to the exception error message if the task needs to be retried. That way we have a way to tie the exception messages together 2016-11-15 11:44:48 +00:00
Leo Hemsted
61a166193c Merge pull request #737 from alphagov/dont-deactivate-twice
archive service - don't re-delete templates etc
2016-11-14 17:04:52 +00:00
Rebecca Law
c00eb0f5a4 Remove from_request method on the Notification db model, was not needed anymore.
Use Notifications.query.one(), rather than assert count is 1 and query for all and get first item in list.
2016-11-14 14:41:32 +00:00
Leo Hemsted
d706d86c9c don't re-delete already archived/revoked templates/api-keys 2016-11-14 14:10:40 +00:00
Rebecca Law
b0ee09a9f6 Implemented the post email notifications endpoint for v2 2016-11-14 13:56:09 +00:00
Martyn Inglis
ac6609e653 Couple of bugs squashed.
1) It's incr not inc on the redis client, so renamed the calls everywhere
2) Redis returns bytes/string rather than an int if the value stored is an int. Cast the result to an int before use. Not you can set up the GET to do this transparently but I've not done this as we *may * use GETS for non-int and  the callback sets up the cast for the connection not the call.
2016-11-12 15:37:57 +00:00
Martyn Inglis
4c0c30bb2e Ensure we count the tasks as well as the API calls.
After we have written to the database and placed it on a deliver queue we count it in the cache against the service.

This is the equivalent of doing it at the end of the API call.
2016-11-11 17:36:38 +00:00
Rebecca Law
4379308189 Fix test failures. 2016-11-11 17:28:15 +00:00
Martyn Inglis
033a3e530b Increment the redis cache on a successful API call.
These means that the cache count is on Notifications in the database NOT notifications sent to providers. If the provider fails to accept the notification, it still counts.

I think this is correct, as they have done the work to send it so we should count it, though there is an argument that we should count them on sending?
2016-11-11 17:19:27 +00:00
Rebecca Law
8f0a96c76c Merge branch 'master' into refactor-for-code-reuse 2016-11-11 16:51:18 +00:00
Martyn Inglis
88f04a46cf Implemented the rate limiting from Redis
- Uses Redis cache to check for current count
- If not present then sets the value based on the database state
- Any Redis errors are swallowed. Cache failures should NOT fail  the request.
2016-11-11 16:47:52 +00:00
Chris Hill-Scott
705a0e7ab8 Remove redundant assignment
This variable is used exactly once, on the next line 🤔
2016-11-11 16:47:06 +00:00
Chris Hill-Scott
3e6d581f73 Use constants for template type
Handy if we ever want to rename these I guess…
2016-11-11 16:47:05 +00:00
Rebecca Law
4f5254134b Fix logging in send_sms and send_email 2016-11-11 16:09:09 +00:00
Rebecca Law
4fe8d700ae Fix the send_sms and send_email code to send the notification id that has been persisted. 2016-11-11 16:00:31 +00:00
Leo Hemsted
95764af6ea Merge pull request #723 from alphagov/active-service
deactivate a service
2016-11-11 15:25:32 +00:00
Rebecca Law
c45d1f63a7 Use the same method to persist a notification.
Refactored the send_sms task to use this method.
2016-11-11 14:56:33 +00:00