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
Stopping celery via upstart issues a TERM signal to the whole process group, which immediately kills the worker threads. This can lead to inconsistentcy in the state of a notification as an API call, for example, can take several seconds consequently the likelihood of the call being lost due to a restart is relatively high.
We need to ensure that we stop cleanly to ensure code deploys and autoscale events do not cause issues.
this script is called as part of a pre-stop hook in upstart, which passes a pid file location to the script. The script will then issues the TERM signal to only the master celery process. which will cleanly shut down the workers and exit.
the script will loop based on the existence of a /proc/{pid} for the celery master process. Once the file is gone the script will exit. Upstart blocks on this script before issuing kill on whatever processes may be left hanging around.
the length of the loop is a maximum 15 minutes - which is the tested max time a task may take. In reality it will exit much quicker.
Right now we strip HTML from templates at the point of saving them. This
also converts stuff like ampersands to their entity form (eg &) and
this is what we save in the database.
This is a bad idea when you’re sending a text message or a letter, in
which an HTML entity makes no sense. But we still need to encode HTML in
the body of HTML emails.
The right place to do this is when rendering the templates. The code to
do this is now in utils. So this commit:
- pull in this new utils code
- removes the old
- adds some integration tests to make sure that everything is working
as expected (more thorough unit tests are happening in utils)
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
branding in emails was previously hosted from admin app - this changes
the url to be static.{domain}/images instead of {domain}/static/images,
which redirects to cloudfront.
some manipulation of the URL was required to make sure it still serves
locally rather than returning "static.localhost:6012" for example.
(if ADMIN_BASE_URL is localhost it just returns the old /static/
path)
also was able to remove DB interaction from a test. woo!
From a support ticket:
> it's possible to add a personalisation token with trailing whitespace
> (eg. "key " rather than "key"). Can this be trimmed in the UI to guard
> against this? (one of our devs copied and pasted it from a document
> and inadvertently included the space)
> Nothing major but caused a few hours of investigations!
Rather than trim the placeholder in the template, we should treat
placeholders in API calls the same way we do with CSV files, ie we
ignore case and spacing in the name of the placeholder. So
`(( First Name))` is equivalent to `((first_name))`, and both would be
populated with a dictionary like `{'firstName': 'Chris'}`.
Depends on:
- [x] https://github.com/alphagov/notifications-utils/pull/77
Chris Hill-Scott