From 8d86d70739cbd4640faf17ca28686c3b04d5ec9c Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 15 Jan 2021 12:15:21 +0000 Subject: [PATCH 1/3] Rewrite previous migration in raw SQL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We shouldn’t import models into migrations because if the model changes later down the line then the migration can’t be re-run at a later date (for example to rebuild a database from scratch). We don’t need to encode the content before storing it (we’ll always do that before rendering/sending) so we don’t need to use `BroadcastMessageTemplate`. And given that no past broadcasts will have personalisation, we don’t need to replace the personalisation in the template before rendering it. So we can just copy the raw content from the templates table. --- .../versions/0336_broadcast_msg_content_2.py | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/migrations/versions/0336_broadcast_msg_content_2.py b/migrations/versions/0336_broadcast_msg_content_2.py index 36905b4af..51f05b351 100644 --- a/migrations/versions/0336_broadcast_msg_content_2.py +++ b/migrations/versions/0336_broadcast_msg_content_2.py @@ -7,6 +7,7 @@ Create Date: 2020-12-04 15:06:22.544803 """ from alembic import op import sqlalchemy as sa +from notifications_utils.template import BroadcastMessageTemplate from sqlalchemy.dialects import postgresql from sqlalchemy.orm.session import Session @@ -17,18 +18,22 @@ down_revision = '0335_broadcast_msg_content' def upgrade(): - session = Session(bind=op.get_bind()) - broadcast_messages = session.query(BroadcastMessage).filter(BroadcastMessage.content == None) + conn = op.get_bind() - for broadcast_message in broadcast_messages: - broadcast_message.content = broadcast_message.template._as_utils_template_with_personalisation( - broadcast_message.personalisation - ).content_with_placeholders_filled_in - - session.commit() - - op.alter_column('broadcast_message', 'content', nullable=False) + results = conn.execute(sa.text(""" + UPDATE + broadcast_message + SET + content = templates_history.content + FROM + templates_history + WHERE + broadcast_message.content is NULL and + broadcast_message.template_id = templates_history.id and + broadcast_message.template_version = templates_history.version + ; + """)) def downgrade(): From fe420448e1007abecc5f2dd8bd2a62a03be405df Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 13 Jan 2021 11:21:42 +0000 Subject: [PATCH 2/3] Allow broadcast messages to be created with API key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When we have a public API there will be no human creating the broadcast message. Instead it will be created by an API integration, authenticated by a key (just like for emails or texts). This updates the database to: - add a new foreign key from BroadcastMessages to API keys - add a `reference` column It doesn’t change the model yet, because the model is used by previous migrations, so would cause them to fail when run before the new columns exist. We can make this change in later pull requests. --- app/models.py | 10 ++++++- migrations/versions/0337_broadcast_msg_api.py | 26 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 migrations/versions/0337_broadcast_msg_api.py diff --git a/app/models.py b/app/models.py index d61f725cd..f8bea361e 100644 --- a/app/models.py +++ b/app/models.py @@ -2245,7 +2245,7 @@ class BroadcastMessage(db.Model): cancelled_at = db.Column(db.DateTime, nullable=True) updated_at = db.Column(db.DateTime, nullable=True, onupdate=datetime.datetime.utcnow) - created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), nullable=False) + created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), nullable=True) approved_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), nullable=True) cancelled_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), nullable=True) @@ -2253,6 +2253,13 @@ class BroadcastMessage(db.Model): approved_by = db.relationship('User', foreign_keys=[approved_by_id]) cancelled_by = db.relationship('User', foreign_keys=[cancelled_by_id]) + api_key_id = db.Column(UUID(as_uuid=True), db.ForeignKey('api_keys.id'), nullable=True) + api_key = db.relationship('ApiKey') + + reference = db.Column(db.String(255), nullable=True) + + CheckConstraint("created_by_id is not null or api_key_id is not null") + @property def personalisation(self): if self._personalisation: @@ -2266,6 +2273,7 @@ class BroadcastMessage(db.Model): def serialize(self): return { 'id': str(self.id), + 'reference': self.reference, 'service_id': str(self.service_id), diff --git a/migrations/versions/0337_broadcast_msg_api.py b/migrations/versions/0337_broadcast_msg_api.py new file mode 100644 index 000000000..96287e2bc --- /dev/null +++ b/migrations/versions/0337_broadcast_msg_api.py @@ -0,0 +1,26 @@ +""" + +Revision ID: 0337_broadcast_msg_api +Revises: 0336_broadcast_msg_content_2 +Create Date: 2020-12-04 15:06:22.544803 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = '0337_broadcast_msg_api' +down_revision = '0336_broadcast_msg_content_2' + + +def upgrade(): + op.alter_column('broadcast_message', 'created_by_id', nullable=True) + op.add_column('broadcast_message', sa.Column('api_key_id', postgresql.UUID(), nullable=True)) + op.create_foreign_key(None, 'broadcast_message', 'api_keys', ['api_key_id'], ['id']) + op.add_column('broadcast_message', sa.Column('reference', sa.String(length=255), nullable=True)) + + +def downgrade(): + op.alter_column('broadcast_message', 'created_by_id', nullable=False) + op.drop_column('broadcast_message', 'api_key_id') + op.add_column('broadcast_message', 'reference') From 78e87857e36f68b163fce37a8c8e8bf7bc001fa4 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 15 Jan 2021 13:15:00 +0000 Subject: [PATCH 3/3] =?UTF-8?q?Don=E2=80=99t=20serialize=20nullable=20UUID?= =?UTF-8?q?=20columns=20to=20'None'?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We should return a proper `None` instead, so it gets JSONified as `null` and returns what you’d expect when doing `bool(model.field)` --- app/models.py | 13 +++++++++---- app/utils.py | 4 ++++ 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/app/models.py b/app/models.py index f8bea361e..707d2a431 100644 --- a/app/models.py +++ b/app/models.py @@ -36,7 +36,12 @@ from app.hashing import ( check_hash ) from app import db, encryption -from app.utils import DATETIME_FORMAT, DATETIME_FORMAT_NO_TIMEZONE, get_dt_string_or_none +from app.utils import ( + DATETIME_FORMAT, + DATETIME_FORMAT_NO_TIMEZONE, + get_dt_string_or_none, + get_uuid_string_or_none, +) from app.history_meta import Versioned @@ -2296,9 +2301,9 @@ class BroadcastMessage(db.Model): 'cancelled_at': get_dt_string_or_none(self.cancelled_at), 'updated_at': get_dt_string_or_none(self.updated_at), - 'created_by_id': str(self.created_by_id), - 'approved_by_id': str(self.approved_by_id), - 'cancelled_by_id': str(self.cancelled_by_id), + 'created_by_id': get_uuid_string_or_none(self.created_by_id), + 'approved_by_id': get_uuid_string_or_none(self.approved_by_id), + 'cancelled_by_id': get_uuid_string_or_none(self.cancelled_by_id), } diff --git a/app/utils.py b/app/utils.py index df12e0ac1..a5a2d15f2 100644 --- a/app/utils.py +++ b/app/utils.py @@ -150,5 +150,9 @@ def get_dt_string_or_none(val): return val.strftime(DATETIME_FORMAT) if val else None +def get_uuid_string_or_none(val): + return str(val) if val else None + + def format_sequential_number(sequential_number): return format(sequential_number, "x").zfill(8)