When generating a new migration we give it a number that increments
the latest existing migration on master. This means that when there
are multiple PRs open containing a migration and one of them gets
merged the others need to be updated to move their migration files
to apply on top of the recently merged one.
This requires renaming the file and changing migration references
for both the migration revision and down_revision.
If a PR introduced more than 1 migration they all need to be updated
one after another since each one needs to be renamed.
This adds a script to simplify this process. `./scripts/fix_migrations.py`
will check for any branch points If it finds exactly one (which
should be the common case), it asks which migration should be moved
and renames / updates references to move the selected branch on top
of the other one.
It won't resolve any conflicts within migrations themselves (eg if
both branches modified the same column) and it won't try to resolve
cases with more than 1 branch.
instead of using wsgi, we now use "application" - this tells gunicorn
to look inside the python module application (application.py) for a
wsgi app - and by default that is also called application so rename
the variable.
Also, when running tasks, we're not using gunicorn so need to set the
flask variable in the manifest so that `flask command ...` finds the
app properly.
Remove server_commands as it's not used any more.
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.
This is text value of the sender_id, depending on the channel this will be a SMS sender, email reply to address or a letter contact block.
This is the first PR in a series to refactor how we send the "reply_to" the provider, eventually we can eliminate the notification_to_sms_sender and notification_to_email_to tables.
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.
The integration tests did test for zero return. Added a method to test
for a year which has no data and also tweaked the existing tests
to ensure they are testing the year more fully.
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
