Commit Graph

56 Commits

Author SHA1 Message Date
Leo Hemsted
6e554188bd add command to backfill template usage
The command takes a service id and a day, grabs the historical data for
that day (potentially out of notification_history), and pops it in
redis (for eight days, same as if it were written to manually).

also, prefix template usage key with "service" to make clear that it's
a service id, and not an individual template id.
2018-04-03 16:14:47 +01:00
Leo Hemsted
8e73961f65 add new redis template usage per day key
We've run into issues with redis expiring keys while we try and write
to them - short lived redis TTLs aren't really sustainable for keys
where we mutate the state. Template usage is a hash contained in redis
where we increment a count keyed by template_id each time a message is
sent for that template. But if the key expires, hincrby (redis command
for incrementing a value in a hash) will re-create an empty hash.

This is no good, as we need the hash to be populated with the last
seven days worth of data, which we then increment further. We can't
tell whether the hincrby created the key, so a different approach
entirely was needed:

* New redis key: <service_id>-template-usage-<YYYY-MM-DD>. Note: This
  YYYY-MM-DD is BTC time so it lines up nicely with ft_billing table
* Incremented to from process_notification - if it doesn't exist yet,
  it'll be created then.
* Expiry set to 8 days every time it's incremented to.

Then, at read time, we'll just read the last eight days of keys from
Redis, and sum them up. This works because we're only ever incrementing
from that one place - never setting wholesale, never recreating the
data from scratch. So we know that if the data is in redis, then it is
good and accurate data.

One thing we *don't* know and *cannot* reason about is what no key in
redis means. It could be either of:

* This is the first message that the service has sent today.
* The key was deleted from redis for some reason.

Since we set the TTL to so long, we'll never be writing to a key that
previously expired. But if there is a redis (or operator) error and the
key is deleted, then we'll have bad data - after any data loss we'll
have to rebuild the data.
2018-04-03 16:12:54 +01:00
Rebecca Law
8aa94af5d7 Stop populating the notification to sender mapping table when creating a notificaiton. 2017-11-29 16:47:23 +00:00
Rebecca Law
4ecf168590 Upate test for persist_notification 2017-11-27 14:45:34 +00:00
Rebecca Law
049daa0cb8 Save reply_to_text for one off notiications and csv notificaitons. 2017-11-25 11:31:36 +00:00
Rebecca Law
830619194e Renamed some tests.
Fix some imports.
Added test for a function
2017-11-01 11:01:20 +00:00
Rebecca Law
4eec11b633 Added an optional parameter in the form for POST /v2/notifications/sms and /service/<service_id>/send-notification to pass in the SMS sender id.
The send_sms_to_provider still needs to use the SMS sender being passed in to the POST.

As part of https://www.pivotaltracker.com/story/show/152106587
2017-10-30 13:36:49 +00:00
Richard Chapman
d2168b7985 Added the mapping between notification and reply to email to the database and persisted the mapping when the request is received by the end point. the end point also checks if the reply to email id exists and if not returns an error. Also added tests to test the functionality. 2017-10-05 16:50:17 +01:00
Leo Hemsted
0ce9b94c36 Merge pull request #1075 from alphagov/letter-api-refactor
Letter api step 1 - refactor
2017-07-25 14:53:39 +01:00
Leo Hemsted
614880f6d9 send to send-sms-tasks and send-email-tasks instead of send-tasks 2017-07-21 13:49:37 +01:00
Leo Hemsted
9caf45451e make persist_notification require kwargs
when functions get as big as that, it's confusing to try and work out what
things are what. By including a * as the first arg, we require that anyone
calling the function has to use kwargs to reference the parameters
2017-07-19 17:02:19 +01:00
Leo Hemsted
350133e6db ensure created_by_id is being persisted correctly
(also make sure it's well tested 🎉 )
2017-06-23 15:56:47 +01:00
Leo Hemsted
ac7665bfc6 celery test cleanup
* Alter config so an error will be raised if you forget to mock out a
  celery call in one of your tests
* Remove an unneeded exception type that was masking errors
2017-06-20 12:06:49 +01:00
Martyn Inglis
75c5e5e5d5 Merge branch 'master' into redo-queue-visibitlity-timeout
Conflicts:
	app/notifications/process_notifications.py
	app/v2/notifications/post_notifications.py
2017-05-30 10:18:18 +01:00
Imdad Ahad
6c4377bd44 Persist normalised email 2017-05-26 10:26:07 +01:00
Imdad Ahad
cbc92a6173 Store the normalised number on the notification 2017-05-26 10:26:07 +01:00
Martyn Inglis
2591d3a1df This massive set of changes uses the new queue names object throughout the app and tests.
Lots of changes, all changing the line of code that puts things into queues, and the code that tests that.
2017-05-25 10:51:49 +01:00
Rebecca Law
751abb4b99 Update dateformat for scheduled_for to include minutes. 2017-05-22 14:15:35 +01:00
Rebecca Law
3a3161ecc4 Merge branch 'master' into schedule-api-notification
Conflicts:
	app/celery/scheduled_tasks.py
	app/v2/notifications/post_notifications.py
	tests/app/celery/test_scheduled_tasks.py
2017-05-22 14:05:57 +01:00
Rebecca Law
973cc2c4c9 Changed the scheduled_for datetime to only send and hour of a day to send.
Also expect the date being passed in is BST. The date is converted to UTC before saving. And converted to BST when returning a notification.
2017-05-17 15:06:15 +01:00
Martyn Inglis
f4020aec05 these three tests replicate some testing that is done in the client tests themselves. 2017-05-12 16:10:21 +01:00
Imdad Ahad
252c3235d7 Persist intl fields on notification 2017-04-27 13:24:36 +01:00
minglis
a8ea333606 Merge pull request #923 from alphagov/validate-int-numbers-on-api
Validate International phone numbers
2017-04-27 13:21:04 +01:00
Imdad Ahad
05145afcec Fix tests for checking simulated recipients 2017-04-26 17:26:06 +01:00
Imdad Ahad
68edd7a33f Fix PEP issues :-| 2017-04-26 11:52:03 +01:00
Imdad Ahad
26b35c5d15 Add intl notificiation fields 2017-04-26 11:31:05 +01:00
Martyn Inglis
b0e5062df2 Added the random string reference to the letter
- uses the reference field on the notifications table to store a 16char random string used to cross reference DVLA letters back to the notification
- used as letter barcode does not have space for a UUID notification id

Depends on https://github.com/alphagov/notifications-utils/pull/149

Renamed the numeric_id to notification_reference in utils and changed validation rules to match this

Note also the persist_notification method set "reference" to be "client_reference" which is confusing and they are different things, so fixed this too.
2017-04-12 17:56:55 +01:00
Rebecca Law
3838fb583c The redis cache for daily limits and template usage was being updated for notifications with a test api key.
This PR corrects that.
2017-03-30 13:43:44 +01:00
Martyn Inglis
7dcd3164e2 Revert "Reinstating the 2 task model for API submitted notifications."
This reverts commit 1c154c4113.
2017-03-30 10:46:23 +01:00
Martyn Inglis
1c154c4113 Reinstating the 2 task model for API submitted notifications.
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.
2017-03-23 14:41:00 +00:00
Chris Hill-Scott
e507fed152 Quietly ignore extra personalisation
> 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
2017-03-07 16:09:17 +00:00
Rebecca Law
07fc71cc4c Fix codestyle 2017-02-15 11:59:31 +00:00
Rebecca Law
74e29708f9 Fix bug where the increment calls set count to 1 if the cache does not exist. 2017-02-15 11:49:19 +00:00
Rebecca Law
53b7ad0961 Moved the cache key to the utils module.
Renamed the dao method.
2017-02-14 14:22:52 +00:00
Rebecca Law
458adefcb8 Added a redis cache for the template usage stats.
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
2017-02-13 18:47:29 +00:00
Rebecca Law
a8cdabcecd - Refactor v2 post_notification to use a single method for sms and email.
- 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)
2017-01-17 12:08:24 +00:00
Rebecca Law
6c79ddbe41 This is the first deploy in series of deploys to give certain templates priority in processing.
If the template is marked as priority the notification will be sent using the `notify` queue.
The `notify` queue is a low volume queue, messages here will not be queue behind a large job and should be delivered with in a more consistent time frame.

- Added templates.process_type and templates_history.process_type column.
- Added a template_process_type table to handle the enum for templates.process_type, initial values are normal and priority

https://www.pivotaltracker.com/story/show/135429147
2017-01-13 12:14:34 +00:00
Martyn Inglis
0f37824b0c Ensure updates on a research mode service or test key don't touch the history table
- note this is an unexpectedly big change.
- When we create a service we pass the service id to the persist method. This means that we don't have the service available to check if in research mode.
- All calling methods (expecting the one where we use the notify service) have the service available. So rather than reload it I changed the method signature to pass the service, not the ID to persist.
- Touches a few places.

Note this means that the update or create methods will fall over on a null service. But this seems correct.

Goes back to the story which we need to play to make the service available as the API user so that the need to load and pass around services is minimised.
2016-12-19 16:45:18 +00:00
minglis
9c161df2b6 Merge pull request #771 from alphagov/fix-missing-template-id
Fix for missing template IDs.
2016-12-16 14:41:54 +00:00
Martyn Inglis
2924f01f95 Fix for missing template IDs.
- Problem was that on notification creation we pass the template ID not the template onto the new notification object
- We then set the history object from this notification object by copying all the fields. This is OK at this point.
- We then set the relationship on the history object based on the template, which we haven't passed in. We only passed the ID. This means that SQLAlchemy nulls the relationship, removing the template_id.
- Later we update the history row when we send the message, this fixes the data. BUT if we ever have a send error, then this never happens and the template is never set on the history table.

Fix:
Set only the template ID when creating the history object.
2016-12-15 16:59:03 +00:00
Chris Hill-Scott
59af44d7ab Update utils to 12.0.0
Includes:

- [x] https://github.com/alphagov/notifications-utils/pull/94 (breaking
      changes which are responsible for all the changes to the API in
      this PR)

The test for `get_sms_fragment_count` has been removed because this
method is already tested in utils here:

ac20f7e99e/tests/test_base_template.py (L140-L159)
2016-12-13 10:57:01 +00:00
Jenny Duckett
7332874415 Use namedtuple in test_send_notification_to_queue
This makes this test a couple of seconds faster - 0.7s instead of 2.5s for me
locally. sample_notification also creates a service, template, user and
permissions, but we don't need any of these objects to exist in the database
for this test. It's particularly helpful for this test because there are so
many parameterized cases. Thanks @leohemsted for suggesting doing this here.
2016-12-09 17:37:18 +00:00
Jenny Duckett
d5d079a150 Add optional queue param to send_notification_to_queue
We want to use this function for sending internal notifications to a different
queue.
2016-12-09 16:46:49 +00:00
Chris Hill-Scott
d449475dd5 Incorporate breaking utils changes
The `.replaced…` methods on instances of `Template` were removed in
https://github.com/alphagov/notifications-utils/pull/84
2016-12-02 10:42:11 +00:00
Rebecca Law
be113e031f Sometimes a message is picked up twice of the SQS queue, we need to safe gaurd ourselves for that.
In this PR the id for the notification is passed in and used to created the notification, which causes a integrity error.
Normally when we get a SQLAlchemy error here we send the message to the retry queue, but if the notification already exists
we just ignore it.
2016-11-25 17:32:01 +00:00
Martyn Inglis
e7a01e979b Fixed merge conflicts 2016-11-22 13:10:12 +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
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
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