this reduces the amount of error messages we log (we'll no longer log
at error level when build-dvla-file-for-job retries while waiting for
the task to finish), and make sure we retry in those cases above - db
or s3 having temporary troubl
This causes an issue when it hits the max retry limit, and tries to
throw your exception to let you deal with it - at this point it was
moaning because we pass in a string
if it's not defined, and we're inside an exception block celery uses
that instead.
Removed the REST endpoint and the DAO that it uses as the endpoint is
no longer used by the Admin UI and the DAO is not reused anywhere
else.
- Removed REST endpoint
- Removed DAO which gets the stats
- Removed associated tests of both methods
Switches on authentication checks for Firetext inbound SMS callbacks.
This should only be released once Firetext callback URLs have been
updated with authentication details.
click (http://click.pocoo.org/) is used by flask to run its cli args.
In removing flask_script (it's unmaintained), we had to migrate all our
commands to use click. This is a change for the better in my eyes - you
don't need to define the command in several places, and it makes
managing options a bit easier.
View diff with whitespace turned off unless you're a masochist.
Currently templates are ordered by the newest created first. This made
sense when, after creating a new template, you were landed on the page
that listed all the templates. In other words, you needed to see
confirmation of the thing that you’ve just done.
Now (since https://github.com/alphagov/notifications-admin/pull/1195)
you get landed on the page for just that template. So you always see
the template you’ve just created, no matter how the list of templates is
ordered. So we are free to change the order of the templates.
Ordering by time created is not great, because it gives users no control
over which templates appear first. For example, our research reported
this from one team:
> One frustration they have is that when you add a new template it
> automatically goes to the top of the list. To get round this, whenever
> they add a new template they delete all of the existing ones and start
> again because they want to keep their templates in numerical order.
> They'd like to be able to control the order of templates in the list.
We _could_ give people some sort of drag-and-drop template ordering
thing. But this feels like overkill. I think that alphabetical order is
better because:
- it’s easily discoverable – anyone who wants to know how a list is
ordered can quickly tell just by looking at it
- it’s universal – everyone knows how alphabetical ordering works
- it’s familiar – this is how people documents on their computer are
ordered; there’s no new UI to learn
- it’s what users are doing already – from the same service as above:
> They number their templates 1,2a, 2b, 3a etc
So this commit changes the ordering from newest created first to
alphabetical.
Previous changes to template order and navigation:
- https://github.com/alphagov/notifications-admin/pull/1163
- https://github.com/alphagov/notifications-admin/pull/1195
- https://github.com/alphagov/notifications-admin/pull/1330
Implementation notes
---
I refactored some of the tests here. I still don’t think they’re great
tests, but they’re a little more Pythonic now at least.
I also added a sort by template type, so that the order is deterministic
when you have, for example, an email template and a text message
template with the same name. If you have two text message templates with
the same name you’re on your own.
reply_to requires template_type to be already set, but the order
of attribute assignment is not defined when a model object is created
from a dictionary.
This adds a constructor to Template model that makes sure that
template_type is set first when multiple arguments are passed to the
constructor at once.
The problem might still exist when the template is created through the
API, so this is a temporary fix to unblock the release.
We're hiding the `service_letter_contact_id` column since it should
only be readable and writable using the `.reply_to` wrapper.
Schemas are defined using `fields.Method` since the fields are
represented by a property on the Template model that requires
template type to be set. When creating a template, if `reply_to`
is defined using `fields.String` it gets assigned at the same time
as `template_type` (or the order of assignments is not defined),
so the schema loader attempts to set `.reply_to` on a Template
object with a `None` `template_type`, which raises an exception.
Using `fields.Method` seems to delay `.reply_to` assignment until
the Template object is created, which means it already has a
valid type.
Adds a relationship between Template models and service letter contact
blocks.
Depending on template type, we can have a reference to either a letter
contact block, email reply-to address or SMS sender record. This means
that in order to enforce foreign key constraints we need to define three
separate foreign key columns on the template model.
To hide this implementation detail and make it easier to access the
sender/reply-to information we define a wrapper property that returns
the value from the correct column.
The relationship and the property are only defined for letter templates
at the moment.
The setter raises an error when trying to assign a reply_to value for
non-letter templates. The exception isn't raised if the value being
assigned is `None` since it can get assigned by marshmallow schemas
and as it matches the value returned for other template types it
doesn't need to be written anywhere.
This allows us to avoid duplication between Template and TemplateHistory
classes and makes it easier to ensure that all columns are copied
to the TemplateHistory objects.
When createing a history instance of the updated object `create_history`
sets attributes using `setattr`. Since SQLAlchemy model instances are
Python objects they don't prevent new attributes being created by setattr,
which means that if history models are missing some of the columns the
attributes will still be assigned, but their values will not be persisted
by SQLAlchemy since database columns for them do not exist.
To avoid this, we check that the attribute is defined on the `history_cls`
and raise an error if it isn't.
Checks authentication header value on inbound SMS requests from
Firetext against a list of allowed API keys set in the application
config.
At the moment, we're only logging the attempts without aborting the
requests. Once this is rolled out to production and we've checked
the logs we'll switch on the aborts and add the tests for 401 and 403
responses.
Added a check to ensure that the current date falls in between the
financial year for the year supplied by the method, so that the todays
stats will only be appended in that situation.
correctly
The dao_fetch_monthly_historical_usage_by_template_for_service code
wasn't being tested properly due to an bug on the test which created a
notification with the same template and hence was not testing that
a specific service would have a different template id.
- Fixed the bug in the test
- Update services_dao so that the service id check is made
are not returned from queries
- Updated stats_template_usage_by_month_dao.py to return the results for
financial year not calendar, as the report os for FY only and hence
only the FY data is required
- Updated services_dao.py to ensure double precision values are converted
to an int as the 'exact' function returns double precision from the
database query, as the admin code requires the value for month to be an
int
this involved:
* moving that task to callback_tasks to prevent circular imports
* updating the dummy research mode callbacks (with actual SNS messages from the
ses simulator emails)
* refactoring tests
