diff --git a/app/broadcast_message/rest.py b/app/broadcast_message/rest.py index 3f2df6c64..141461039 100644 --- a/app/broadcast_message/rest.py +++ b/app/broadcast_message/rest.py @@ -101,16 +101,20 @@ def create_broadcast_message(service_id): user = get_user_by_id(data['created_by']) template = dao_get_template_by_id_and_service_id(data['template_id'], data['service_id']) + personalisation = data.get('personalisation', {}) broadcast_message = BroadcastMessage( service_id=service.id, template_id=template.id, template_version=template.version, - personalisation=data.get('personalisation', {}), + personalisation=personalisation, areas={"areas": data.get("areas", []), "simple_polygons": data.get("simple_polygons", [])}, status=BroadcastStatusType.DRAFT, starts_at=_parse_nullable_datetime(data.get('starts_at')), finishes_at=_parse_nullable_datetime(data.get('finishes_at')), created_by_id=user.id, + content=template._as_utils_template_with_personalisation( + personalisation + ).content_with_placeholders_filled_in, ) dao_save_object(broadcast_message) @@ -185,16 +189,11 @@ def _create_broadcast_event(broadcast_message): else: transmitted_finishes_at = broadcast_message.finishes_at - template = broadcast_message.template._as_utils_template_with_personalisation( - # Broadcast events don’t support personalisation yet - values={} - ) - event = BroadcastEvent( service=broadcast_message.service, broadcast_message=broadcast_message, message_type=msg_types[broadcast_message.status], - transmitted_content={"body": str(template)}, + transmitted_content={"body": broadcast_message.content}, transmitted_areas=broadcast_message.areas, # TODO: Probably move this somewhere more standalone too and imply that it shouldn't change. Should it include # a service based identifier too? eg "flood-warnings@notifications.service.gov.uk" or similar diff --git a/app/models.py b/app/models.py index 8102a058c..d61f725cd 100644 --- a/app/models.py +++ b/app/models.py @@ -2219,11 +2219,12 @@ class BroadcastMessage(db.Model): service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id')) service = db.relationship('Service', backref='broadcast_messages') - template_id = db.Column(UUID(as_uuid=True), nullable=False) - template_version = db.Column(db.Integer, nullable=False) + template_id = db.Column(UUID(as_uuid=True), nullable=True) + template_version = db.Column(db.Integer, nullable=True) template = db.relationship('TemplateHistory', backref='broadcast_messages') _personalisation = db.Column(db.String, nullable=True) + content = db.Column(db.String, nullable=False) # defaults to empty list areas = db.Column(JSONB(none_as_null=True), nullable=False, default=list) @@ -2252,12 +2253,6 @@ class BroadcastMessage(db.Model): approved_by = db.relationship('User', foreign_keys=[approved_by_id]) cancelled_by = db.relationship('User', foreign_keys=[cancelled_by_id]) - @property - def content(self): - return self.template._as_utils_template_with_personalisation( - self.personalisation - ).content_with_placeholders_filled_in - @property def personalisation(self): if self._personalisation: @@ -2274,12 +2269,12 @@ class BroadcastMessage(db.Model): 'service_id': str(self.service_id), - 'template_id': str(self.template_id), + 'template_id': str(self.template_id) if self.template else None, 'template_version': self.template_version, - 'template_name': self.template.name, + 'template_name': self.template.name if self.template else None, + 'personalisation': self.personalisation if self.template else None, 'content': self.content, - 'personalisation': self.personalisation, 'areas': self.areas.get("areas", []), 'simple_polygons': self.areas.get("simple_polygons", []), diff --git a/migrations/versions/0335_broadcast_msg_content.py b/migrations/versions/0335_broadcast_msg_content.py new file mode 100644 index 000000000..b3e25c75c --- /dev/null +++ b/migrations/versions/0335_broadcast_msg_content.py @@ -0,0 +1,27 @@ +""" + +Revision ID: 0335_broadcast_msg_content +Revises: 0334_broadcast_message_number +Create Date: 2020-12-04 15:06:22.544803 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = '0335_broadcast_msg_content' +down_revision = '0334_broadcast_message_number' + + +def upgrade(): + op.add_column('broadcast_message', sa.Column('content', sa.Text(), nullable=False)) + op.alter_column('broadcast_message', 'template_id', nullable=True) + op.alter_column('broadcast_message', 'template_version', nullable=True) + + +def downgrade(): + # downgrade fails if there are broadcasts without a template. This is deliberate cos I don't feel comfortable + # deleting broadcasts. + op.alter_column('broadcast_message', 'template_id', nullable=False) + op.alter_column('broadcast_message', 'template_version', nullable=False) + op.drop_column('broadcast_message', 'content') diff --git a/tests/app/broadcast_message/test_rest.py b/tests/app/broadcast_message/test_rest.py index 8cd105ebe..f1dfb0d03 100644 --- a/tests/app/broadcast_message/test_rest.py +++ b/tests/app/broadcast_message/test_rest.py @@ -44,6 +44,35 @@ def test_get_broadcast_message(admin_request, sample_broadcast_service): assert response['personalisation'] == {'thing': 'test'} +def test_get_broadcast_message_without_template(admin_request, sample_broadcast_service): + bm = create_broadcast_message( + service=sample_broadcast_service, + content='emergency broadcast content', + areas={ + "areas": ['place A', 'region B'], + "simple_polygons": [[[50.1, 1.2], [50.12, 1.2], [50.13, 1.2]]], + }, + ) + + response = admin_request.get( + 'broadcast_message.get_broadcast_message', + service_id=sample_broadcast_service.id, + broadcast_message_id=bm.id, + _expected_status=200 + ) + + assert response['id'] == str(bm.id) + assert response['template_id'] is None + assert response['template_version'] is None + assert response['template_name'] is None + assert response['content'] == 'emergency broadcast content' + assert response['status'] == BroadcastStatusType.DRAFT + assert response['created_at'] is not None + assert response['starts_at'] is None + assert response['areas'] == ['place A', 'region B'] + assert response['personalisation'] is None + + def test_get_broadcast_message_404s_if_message_doesnt_exist(admin_request, sample_broadcast_service): err = admin_request.get( 'broadcast_message.get_broadcast_message', @@ -148,6 +177,26 @@ def test_create_broadcast_message_400s_if_json_schema_fails_validation( assert response['errors'] == expected_errors +@freeze_time('2020-01-01') +def test_create_broadcast_message_400s_if_content_provided_but_no_template(admin_request, sample_broadcast_service): + # we don't currently support this, but might in the future + response = admin_request.post( + 'broadcast_message.create_broadcast_message', + _data={ + 'template_id': None, + 'content': 'Some tailor made broadcast content', + 'service_id': str(sample_broadcast_service.id), + 'created_by': str(sample_broadcast_service.created_by_id), + }, + service_id=sample_broadcast_service.id, + _expected_status=400 + ) + assert response['errors'] ==[ + {'error': 'ValidationError', 'message': 'template_id is not a valid UUID'}, + {'error': 'ValidationError', 'message': 'Additional properties are not allowed (content was unexpected)'}, + ] + + @pytest.mark.parametrize('status', [ BroadcastStatusType.DRAFT, BroadcastStatusType.PENDING_APPROVAL, @@ -394,6 +443,40 @@ def test_update_broadcast_message_status_stores_approved_by_and_approved_at_and_ assert alert_event.transmitted_content == {"body": "emergency broadcast"} +def test_update_broadcast_message_status_creates_event_with_correct_content_if_broadcast_has_no_template( + admin_request, + sample_broadcast_service, + mocker +): + bm = create_broadcast_message( + service=sample_broadcast_service, + template=None, + content='tailor made emergency broadcast content', + status=BroadcastStatusType.PENDING_APPROVAL, + areas={"areas": ["london"], "simple_polygons": [[[51.30, 0.7], [51.28, 0.8], [51.25, -0.7]]]} + ) + approver = create_user(email='approver@gov.uk') + sample_broadcast_service.users.append(approver) + mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async') + + response = admin_request.post( + 'broadcast_message.update_broadcast_message_status', + _data={'status': BroadcastStatusType.BROADCASTING, 'created_by': str(approver.id)}, + service_id=sample_broadcast_service.id, + broadcast_message_id=bm.id, + _expected_status=200 + ) + + assert response['status'] == BroadcastStatusType.BROADCASTING + + assert len(bm.events) == 1 + alert_event = bm.events[0] + + mock_task.assert_called_once_with(kwargs={'broadcast_event_id': str(alert_event.id)}, queue='notify-internal-tasks') + + assert alert_event.transmitted_content == {"body": "tailor made emergency broadcast content"} + + def test_update_broadcast_message_status_rejects_approval_from_creator( admin_request, sample_broadcast_service, diff --git a/tests/app/celery/test_broadcast_message_tasks.py b/tests/app/celery/test_broadcast_message_tasks.py index ab4e337df..cee968bb4 100644 --- a/tests/app/celery/test_broadcast_message_tasks.py +++ b/tests/app/celery/test_broadcast_message_tasks.py @@ -156,6 +156,44 @@ def test_send_broadcast_provider_message_sends_data_correctly( ) +@freeze_time('2020-08-01 12:00') +def test_send_broadcast_provider_message_sends_data_correctly_when_broadcast_message_has_no_template( + mocker, sample_service, +): + broadcast_message = create_broadcast_message( + service=sample_service, + template=None, + content='this is an emergency broadcast message', + areas={ + 'areas': ['london', 'glasgow'], + 'simple_polygons': [ + [[50.12, 1.2], [50.13, 1.2], [50.14, 1.21]], + [[-4.53, 55.72], [-3.88, 55.72], [-3.88, 55.96], [-4.53, 55.96]], + ], + }, + status=BroadcastStatusType.BROADCASTING + ) + event = create_broadcast_event(broadcast_message) + + mock_create_broadcast = mocker.patch( + f'app.clients.cbc_proxy.CBCProxyEE.create_and_send_broadcast', + ) + + send_broadcast_provider_message(provider='ee', broadcast_event_id=str(event.id)) + + broadcast_provider_message = event.get_provider_message('ee') + + mock_create_broadcast.assert_called_once_with( + identifier=str(broadcast_provider_message.id), + message_number=mocker.ANY, + headline='GOV.UK Notify Broadcast', + description='this is an emergency broadcast message', + areas=mocker.ANY, + sent=mocker.ANY, + expires=mocker.ANY, + ) + + @pytest.mark.parametrize('provider,provider_capitalised', [ ['ee', 'EE'], ['vodafone', 'Vodafone'], diff --git a/tests/app/db.py b/tests/app/db.py index 432aba1fd..72824a24d 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -2,6 +2,8 @@ import random import uuid from datetime import datetime, date, timedelta +import pytest + from app import db from app.dao.email_branding_dao import dao_create_email_branding from app.dao.inbound_sms_dao import dao_create_inbound_sms @@ -1010,24 +1012,44 @@ def create_service_contact_list( def create_broadcast_message( - template, + template=None, + *, + service=None, # only used if template is not provided created_by=None, personalisation=None, + content=None, status=BroadcastStatusType.DRAFT, starts_at=None, finishes_at=None, areas=None, ): + if template: + service = template.service + template_id = template.id + template_version = template.version + personalisation = personalisation or {} + content = template._as_utils_template_with_personalisation( + personalisation + ).content_with_placeholders_filled_in + elif content: + template_id = None + template_version = None + personalisation = None + content = content + else: + pytest.fail('Provide template or content') + broadcast_message = BroadcastMessage( - service_id=template.service_id, - template_id=template.id, - template_version=template.version, - personalisation=personalisation or {}, + service_id=service.id, + template_id=template_id, + template_version=template_version, + personalisation=personalisation, status=status, starts_at=starts_at, finishes_at=finishes_at, - created_by_id=created_by.id if created_by else template.created_by_id, + created_by_id=created_by.id if created_by else service.created_by_id, areas=areas or {}, + content=content ) db.session.add(broadcast_message) db.session.commit()