- Disable Redis as there is a current connection limit of 256 which
could slow down the request if they are all used
- Added statd to methods in the post to help spot any bottlenecks
In other places the text we use for this error message is "Missing personalisation: name, date, thing". See:
- 72b108b694/app/template/rest.py (L125)
- 717c0510a3/app/notifications/rest.py (L206)
- 05a179c6ef/app/v2/template/post_template.py (L38)
For some reason this part of the codebase says "Template missing personalisation: …". This is inconsistent, and also confusing because it’s the API call that’s missing the personalisation, not the template itself.
This commit changes the error message to be consistent with the majority of the codebase, which uses the less confusing wording.
we now no longer create a job. At the end of the post there is no
action, as we don't have any tasks to queue immediately - if it's a
real notification it'll get picked up in the evening scheduled task.
If it's a test notification, we create it with an initial status of
sending so that we can be sure it'll never get picked up - and then we
trigger the update-letter-notifications-to-sent-to-dvla task to sent
the sent-at/by.
there are three steps to this
1. Create a job
* Starts in status 'ready to send'
* Created by None, since it's from the API
* original file name 'letter submitted via api'
2. Create a single notification for that job
* job_row_number 0
* client reference if provided
* address line 1 as recipient
3. Trigger the build_dvla_file task
we know that all the notifications have been created for this job
(since we just created them ourselves synchronously), so this will
just create the dvla-format file for the job, and upload it to s3.
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
* 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
- uses new utils methods to validate phone numbers
- defaults to International=True on validation. This ensures the validator works on all numbers
- Then check if the user can send this message to the number internationally if needed.
- 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.
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.
> 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
- 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 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.
