Brings in:
- [ ] https://github.com/alphagov/notifications-utils/pull/135
Also adds extra tests for:
- the exact issue that we saw in production when #867 was deployed
- what happens when `None` is passed as a placeholder value, because
this should never get as far as the relevant bit of utils
This is being done for the PaaS migration to allow us to keep traffic coming in whilst we migrate the database.
uses the same tasks as the CSV uploaded notifications. Simple changes to not persist the notification, and call into a different task.
There are three authentication methods:
- requires_no_auth - public endpoint that does not require an Authorisation header
- requires_auth - public endpoints that need an API key in the Authorisation header
- requires_admin_auth - private endpoint that requires an Authorisation header which contains the API key for the defined as the client admin user
> If a user makes an API request with additional personalisation fields,
> we should simply discard any fields that the template doesn't have.
>
> This gives a couple of related advantages:
>
> - modifying template parameters no longer requires downtime for
> clients - as they can pass in extra new parameters before a template
> change, or continue passing in old unused parameters after removing
> them from a template
>
> - services can pass in large user objects, for example, and then play
> around with templates adding and removing fields at will
>
> we should make sure we still return an error if a user doesn't pass in
> a required parameter.
– https://www.pivotaltracker.com/story/show/140774195
Cache expires every 10 minutes, but will help with the every 2 second query, especially when a job is running.
There is some clean up and qa to do for this yet
This PR changes the response to POST /notifications/sms/<mmg | firetext> from a 400 response to a 200 response.
If we get a callback for a notification more than once or for a notification we log that but we return a 200 success response to the provider.
We have found that there is a situation where the send to provider throws a timeout exception but the provider did get the message, but we still send it to them again.
In which case they send the message twice, and callback for the message twice.
Another case where we may get duplicate callbacks is that the network gave the provider two callbacks meaning they pass those two callbacks onto us.
So it is really difficult to know if we sent to the provider twice or just got two callbacks.
The test_callback has many changes because I took the opportunity to use the client conftest fixture rather than the notify_api fixture.
The only 2 tests really changed are test_mmg_callback_returns_200_when_notification_id_not_found_or_already_updated and test_firetext_callback_returns_200_when_notification_id_not_found_or_already_updated
We are using the notify queue in this iteration because that queue is a low volume queue with it's own dedicated workers. This just saves us from building a new queue at this point, and a new queue may not be necessary.
- Added the `simulate` notification logic to version 2. We have 3 email addresses and phone numbers that are used
to simulate a successful post to /notifications. This was missed out of the version 2 endpoint.
- Added a test to template_dao to check for the default value of normal for new templates
- in v2 get_notifications, casted the path param to a uuid, if not uuid abort(404)
note that all of these tests have to be checked to ensure that they
still call through to notify_db_session (notify_db not required) to
tear down the database after the test runs - since it's no longer
required to pass it in to the function just to invoke the sample_user
function
brings in a fix to InvalidEmail/Phone/AddressExceptions not being
instantiated correctly. `exception.message` is not a python standard,
so we shouldn't be relying on it to transmit exception reasons -
rather we should be using `str(exception)` instead. This involved a
handful of small changes to the schema validation
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.
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?
- 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.
- It would be nice to refactor the send_sms and send_email tasks to use these common functions as well, that way I can get rid of the new Notifications.from_v2_api_request method.
- Still not happy with the format of the errors. Would like to find a happy place, where the message is descript enough that we do not need external documentation to explain the error. Perhaps we still only need documentation to explain the trial mode concept.
- Use these validation methods in post_sms_notification and the version 1 of post_notification.
- Create a v2 error handlers.
- InvalidRequest has a to_dict method for private and v1 error responses and a to_dict_v2 method to create the v2 of the error responses.
- Each validation method has extensive unit tests, so the unit test for the endpoint do not need to check every error case, but check that the error handle formats the message correctly.
- The format of the error messages is still a work on progress.
- This version of the api could be deployed without causing a problem to the application.
- The new endpoing is still a work in progress and is not being used yet.
There is an overlap between team key/trial mode/whitelist. But it’s not
a complete overlap. So it’s hard to understand all the different
permutations of which key lets you send to which people when.
This commit tries to reduce the differences between these concepts. So
for a user of the API
**In trial mode**
- You can send to anyone in your team or whitelist, using the team key
- You can simulate sending to anyone, using the simulate key
**When you’re live**
- You can send to anyone in your team or whitelist, using the team key
- You can simulate sending to anyone, using the simulate key
- You can send to anyone with the live key
So doing a `git diff` on that list, the only difference between being in
trial mode and live mode is now:
`+` You can send to anyone with the live key
**(How trial mode used to work)**
- You can send to anyone in your team or whitelist, using the normal key
- You can simulate sending to anyone, using the simulate key
- You can send to _just_ people in your team using the team key
Refactored send_notifications method so that it is more readible.
Refectored the test_send_notificaitons so that it uses parametrized test to avoid duplication.
