diff --git a/app/__init__.py b/app/__init__.py index 255af272d..d16e0cdbe 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -152,6 +152,7 @@ def register_blueprint(application): from app.template_folder.rest import template_folder_blueprint from app.letter_branding.letter_branding_rest import letter_branding_blueprint from app.upload.rest import upload_blueprint + from app.broadcast_message.rest import broadcast_message_blueprint service_blueprint.before_request(requires_admin_auth) application.register_blueprint(service_blueprint, url_prefix='/service') @@ -237,6 +238,9 @@ def register_blueprint(application): upload_blueprint.before_request(requires_admin_auth) application.register_blueprint(upload_blueprint) + broadcast_message_blueprint.before_request(requires_admin_auth) + application.register_blueprint(broadcast_message_blueprint) + def register_v2_blueprints(application): from app.v2.inbound_sms.get_inbound_sms import v2_inbound_sms_blueprint as get_inbound_sms diff --git a/app/broadcast_message/__init__.py b/app/broadcast_message/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/app/broadcast_message/broadcast_message_schema.py b/app/broadcast_message/broadcast_message_schema.py new file mode 100644 index 000000000..743ca54d7 --- /dev/null +++ b/app/broadcast_message/broadcast_message_schema.py @@ -0,0 +1,48 @@ +from app.schema_validation.definitions import uuid +from app.models import BroadcastStatusType + +create_broadcast_message_schema = { + '$schema': 'http://json-schema.org/draft-04/schema#', + 'description': 'POST create broadcast_message schema', + 'type': 'object', + 'title': 'Create broadcast_message', + 'properties': { + 'template_id': uuid, + 'service_id': uuid, + 'created_by': uuid, + 'personalisation': {'type': 'object'}, + 'starts_at': {'type': 'string', 'format': 'datetime'}, + 'finishes_at': {'type': 'string', 'format': 'datetime'}, + 'areas': {"type": "array", "items": {"type": "string"}}, + }, + 'required': ['template_id', 'service_id', 'created_by'], + 'additionalProperties': False +} + +update_broadcast_message_schema = { + '$schema': 'http://json-schema.org/draft-04/schema#', + 'description': 'POST update broadcast_message schema', + 'type': 'object', + 'title': 'Update broadcast_message', + 'properties': { + 'personalisation': {'type': 'object'}, + 'starts_at': {'type': 'string', 'format': 'datetime'}, + 'finishes_at': {'type': 'string', 'format': 'datetime'}, + 'areas': {"type": "array", "items": {"type": "string"}}, + }, + 'required': [], + 'additionalProperties': False +} + +update_broadcast_message_status_schema = { + '$schema': 'http://json-schema.org/draft-04/schema#', + 'description': 'POST update broadcast_message status schema', + 'type': 'object', + 'title': 'Update broadcast_message', + 'properties': { + 'status': {'type': 'string', 'enum': BroadcastStatusType.STATUSES}, + 'created_by': uuid, + }, + 'required': ['status', 'created_by'], + 'additionalProperties': False +} diff --git a/app/broadcast_message/rest.py b/app/broadcast_message/rest.py new file mode 100644 index 000000000..9fa67a44a --- /dev/null +++ b/app/broadcast_message/rest.py @@ -0,0 +1,123 @@ +from datetime import datetime + +import iso8601 +from flask import Blueprint, jsonify, request + +from app.dao.templates_dao import dao_get_template_by_id_and_service_id +from app.dao.users_dao import get_user_by_id +from app.dao.broadcast_message_dao import ( + dao_create_broadcast_message, + dao_get_broadcast_message_by_id_and_service_id, + dao_get_broadcast_messages_for_service, + dao_update_broadcast_message, +) +from app.dao.services_dao import dao_fetch_service_by_id +from app.errors import register_errors +from app.models import BroadcastMessage, BroadcastStatusType +from app.broadcast_message.broadcast_message_schema import ( + create_broadcast_message_schema, + update_broadcast_message_schema, + update_broadcast_message_status_schema, +) +from app.schema_validation import validate + +broadcast_message_blueprint = Blueprint( + 'broadcast_message', + __name__, + url_prefix='/service//broadcast-message' +) +register_errors(broadcast_message_blueprint) + + +def _parse_nullable_datetime(dt): + if dt: + return iso8601.parse_date(dt).replace(tzinfo=None) + return dt + + +@broadcast_message_blueprint.route('', methods=['GET']) +def get_broadcast_messages_for_service(service_id): + # TODO: should this return template content/data in some way? or can we rely on them being cached admin side. + # we might need stuff like template name for showing on the dashboard. + # TODO: should this paginate or filter on dates or anything? + broadcast_messages = [o.serialize() for o in dao_get_broadcast_messages_for_service(service_id)] + return jsonify(broadcast_messages=broadcast_messages) + + +@broadcast_message_blueprint.route('/', methods=['GET']) +def get_broadcast_message(service_id, broadcast_message_id): + return jsonify(dao_get_broadcast_message_by_id_and_service_id(broadcast_message_id, service_id).serialize()) + + +@broadcast_message_blueprint.route('', methods=['POST']) +def create_broadcast_message(service_id): + data = request.get_json() + + validate(data, create_broadcast_message_schema) + service = dao_fetch_service_by_id(data['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']) + + broadcast_message = BroadcastMessage( + service_id=service.id, + template_id=template.id, + template_version=template.version, + personalisation=data.get('personalisation', {}), + areas=data.get('areas', []), + 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, + ) + + dao_create_broadcast_message(broadcast_message) + + return jsonify(broadcast_message.serialize()), 201 + + +@broadcast_message_blueprint.route('/', methods=['POST']) +def update_broadcast_message(service_id, broadcast_message_id): + data = request.get_json() + + validate(data, update_broadcast_message_schema) + + broadcast_message = dao_get_broadcast_message_by_id_and_service_id(broadcast_message_id, service_id) + + if 'personalisation' in data: + broadcast_message.personalisation = data['personalisation'] + if 'starts_at' in data: + broadcast_message.starts_at = _parse_nullable_datetime(data['starts_at']) + if 'finishes_at' in data: + broadcast_message.starts_at = _parse_nullable_datetime(data['finishes_at']) + if 'areas' in data: + broadcast_message.areas = data['areas'] + + dao_update_broadcast_message(broadcast_message) + + return jsonify(broadcast_message.serialize()), 200 + + +@broadcast_message_blueprint.route('//status', methods=['POST']) +def update_broadcast_message_status(service_id, broadcast_message_id): + data = request.get_json() + + validate(data, update_broadcast_message_status_schema) + broadcast_message = dao_get_broadcast_message_by_id_and_service_id(broadcast_message_id, service_id) + + new_status = data['status'] + + # TODO: Restrict status transitions + # TODO: Do we need to validate that the user belongs to the same service, isn't the creator, has permissions, etc? + # or is that admin's job + if new_status == BroadcastStatusType.BROADCASTING: + broadcast_message.approved_at = datetime.utcnow() + broadcast_message.approved_by = get_user_by_id(data['created_by']) + if new_status == BroadcastStatusType.CANCELLED: + broadcast_message.cancelled_at = datetime.utcnow() + broadcast_message.cancelled_by = get_user_by_id(data['created_by']) + + broadcast_message.status = new_status + + dao_update_broadcast_message(broadcast_message) + + return jsonify(broadcast_message.serialize()), 200 diff --git a/app/dao/broadcast_message_dao.py b/app/dao/broadcast_message_dao.py new file mode 100644 index 000000000..068dd3653 --- /dev/null +++ b/app/dao/broadcast_message_dao.py @@ -0,0 +1,26 @@ +from app import db +from app.models import BroadcastMessage +from app.dao.dao_utils import transactional + + +@transactional +def dao_create_broadcast_message(broadcast_message): + db.session.add(broadcast_message) + + +@transactional +def dao_update_broadcast_message(broadcast_message): + db.session.add(broadcast_message) + + +def dao_get_broadcast_message_by_id_and_service_id(broadcast_message_id, service_id): + return BroadcastMessage.query.filter( + BroadcastMessage.id == broadcast_message_id, + BroadcastMessage.service_id == service_id + ).one() + + +def dao_get_broadcast_messages_for_service(service_id): + return BroadcastMessage.query.filter( + BroadcastMessage.service_id == service_id + ).order_by(BroadcastMessage.created_at) diff --git a/app/models.py b/app/models.py index eae9c921a..be00abfd1 100644 --- a/app/models.py +++ b/app/models.py @@ -2174,7 +2174,7 @@ class BroadcastMessage(db.Model): {} ) - id = db.Column(UUID(as_uuid=True), primary_key=True) + id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id')) service = db.relationship('Service', backref='broadcast_messages') @@ -2184,6 +2184,8 @@ class BroadcastMessage(db.Model): template = db.relationship('TemplateHistory', backref='broadcast_messages') _personalisation = db.Column(db.String, nullable=True) + # defaults to empty list + areas = db.Column(JSONB(none_as_null=True), nullable=False, default=list) status = db.Column( db.String, @@ -2197,7 +2199,7 @@ class BroadcastMessage(db.Model): finishes_at = db.Column(db.DateTime, nullable=True) # isn't updated if user cancels # these times correspond to when - created_at = db.Column(db.DateTime, nullable=False) + created_at = db.Column(db.DateTime, nullable=False, default=datetime.datetime.utcnow) approved_at = db.Column(db.DateTime, nullable=True) cancelled_at = db.Column(db.DateTime, nullable=True) updated_at = db.Column(db.DateTime, nullable=True, onupdate=datetime.datetime.utcnow) @@ -2209,3 +2211,41 @@ class BroadcastMessage(db.Model): created_by = db.relationship('User', foreign_keys=[created_by_id]) approved_by = db.relationship('User', foreign_keys=[approved_by_id]) cancelled_by = db.relationship('User', foreign_keys=[cancelled_by_id]) + + @property + def personalisation(self): + if self._personalisation: + return encryption.decrypt(self._personalisation) + return {} + + @personalisation.setter + def personalisation(self, personalisation): + self._personalisation = encryption.encrypt(personalisation or {}) + + def serialize(self): + return { + 'id': self.id, + + 'service_id': self.service_id, + + 'template_id': self.template_id, + 'template_version': self.template_version, + 'template_name': self.template.name, + + 'personalisation': self.personalisation, + 'areas': self.areas, + + 'status': self.status, + + 'starts_at': self.starts_at.strftime(DATETIME_FORMAT) if self.starts_at else None, + 'finishes_at': self.finishes_at.strftime(DATETIME_FORMAT) if self.finishes_at else None, + + 'created_at': self.created_at.strftime(DATETIME_FORMAT) if self.created_at else None, + 'approved_at': self.approved_at.strftime(DATETIME_FORMAT) if self.approved_at else None, + 'cancelled_at': self.cancelled_at.strftime(DATETIME_FORMAT) if self.cancelled_at else None, + 'updated_at': self.updated_at.strftime(DATETIME_FORMAT) if self.updated_at else None, + + 'created_by_id': self.created_by_id, + 'approved_by_id': self.approved_by_id, + 'cancelled_by_id': self.cancelled_by_id, + } diff --git a/app/schema_validation/__init__.py b/app/schema_validation/__init__.py index 0ddd51f16..f5f2dc9ef 100644 --- a/app/schema_validation/__init__.py +++ b/app/schema_validation/__init__.py @@ -54,6 +54,17 @@ def validate_schema_date_with_hour(instance): return True +@format_checker.checks('datetime', raises=ValidationError) +def validate_schema_datetime(instance): + if isinstance(instance, str): + try: + iso8601.parse_date(instance) + except ParseError: + raise ValidationError("datetime format is invalid. It must be a valid ISO8601 date time format, " + "https://en.wikipedia.org/wiki/ISO_8601") + return True + + def validate(json_to_validate, schema): validator = Draft7Validator(schema, format_checker=format_checker) errors = list(validator.iter_errors(json_to_validate)) diff --git a/requirements-app.txt b/requirements-app.txt index 50f3b6f28..ed4803a3a 100644 --- a/requirements-app.txt +++ b/requirements-app.txt @@ -20,6 +20,8 @@ marshmallow==2.21.0 # pyup: <3 # v3 throws errors psycopg2-binary==2.8.5 PyJWT==1.7.1 SQLAlchemy==1.3.17 +strict-rfc3339==0.7 +rfc3987==1.3.8 cachetools==4.1.0 notifications-python-client==5.5.1 diff --git a/requirements.txt b/requirements.txt index e42c6931c..c96dd1dfe 100644 --- a/requirements.txt +++ b/requirements.txt @@ -22,6 +22,8 @@ marshmallow==2.21.0 # pyup: <3 # v3 throws errors psycopg2-binary==2.8.5 PyJWT==1.7.1 SQLAlchemy==1.3.17 +strict-rfc3339==0.7 +rfc3987==1.3.8 cachetools==4.1.0 notifications-python-client==5.5.1 @@ -40,14 +42,14 @@ alembic==1.4.2 amqp==1.4.9 anyjson==0.3.3 attrs==19.3.0 -awscli==1.18.93 +awscli==1.18.96 bcrypt==3.1.7 billiard==3.3.0.23 bleach==3.1.4 blinker==1.4 boto==2.49.0 boto3==1.10.38 -botocore==1.17.16 +botocore==1.17.19 certifi==2020.6.20 chardet==3.0.4 click==7.1.2 diff --git a/requirements_for_test.txt b/requirements_for_test.txt index f8edc4dc1..9db32b421 100644 --- a/requirements_for_test.txt +++ b/requirements_for_test.txt @@ -8,8 +8,5 @@ pytest-cov==2.8.1 pytest-xdist==1.31.0 freezegun==0.3.12 requests-mock==1.7.0 -# optional requirements for jsonschema -strict-rfc3339==0.7 -rfc3987==1.3.8 # used for creating manifest file locally jinja2-cli[yaml]==0.7.0 diff --git a/tests/app/broadcast_message/__init__.py b/tests/app/broadcast_message/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/app/broadcast_message/test_rest.py b/tests/app/broadcast_message/test_rest.py new file mode 100644 index 000000000..14657f5b3 --- /dev/null +++ b/tests/app/broadcast_message/test_rest.py @@ -0,0 +1,259 @@ +import uuid + +from freezegun import freeze_time +import pytest + +from app.models import BROADCAST_TYPE, BroadcastStatusType + +from tests.app.db import create_broadcast_message, create_template, create_service, create_user + + +def test_get_broadcast_message(admin_request, sample_service): + t = create_template(sample_service, BROADCAST_TYPE) + bm = create_broadcast_message(t, areas=['place A', 'region B']) + + response = admin_request.get( + 'broadcast_message.get_broadcast_message', + service_id=t.service_id, + broadcast_message_id=bm.id, + _expected_status=200 + ) + + assert response['id'] == str(bm.id) + assert response['template_name'] == t.name + 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'] == {} + + +def test_get_broadcast_message_404s_if_message_doesnt_exist(admin_request, sample_service): + err = admin_request.get( + 'broadcast_message.get_broadcast_message', + service_id=sample_service.id, + broadcast_message_id=uuid.uuid4(), + _expected_status=404 + ) + assert err == {'message': 'No result found', 'result': 'error'} + + +def test_get_broadcast_message_404s_if_message_is_for_different_service(admin_request, sample_service): + other_service = create_service(service_name='other') + other_template = create_template(other_service, BROADCAST_TYPE) + bm = create_broadcast_message(other_template) + + err = admin_request.get( + 'broadcast_message.get_broadcast_message', + service_id=sample_service.id, + broadcast_message_id=bm.id, + _expected_status=404 + ) + assert err == {'message': 'No result found', 'result': 'error'} + + +@freeze_time('2020-01-01') +def test_get_broadcast_messages_for_service(admin_request, sample_service): + t = create_template(sample_service, BROADCAST_TYPE) + + with freeze_time('2020-01-01 12:00'): + bm1 = create_broadcast_message(t, personalisation={'foo': 'bar'}) + with freeze_time('2020-01-01 13:00'): + bm2 = create_broadcast_message(t, personalisation={'foo': 'baz'}) + + response = admin_request.get( + 'broadcast_message.get_broadcast_messages_for_service', + service_id=t.service_id, + _expected_status=200 + ) + + assert response['broadcast_messages'][0]['id'] == str(bm1.id) + assert response['broadcast_messages'][1]['id'] == str(bm2.id) + + +@freeze_time('2020-01-01') +def test_create_broadcast_message(admin_request, sample_service): + t = create_template(sample_service, BROADCAST_TYPE) + + response = admin_request.post( + 'broadcast_message.create_broadcast_message', + _data={ + 'template_id': str(t.id), + 'service_id': str(t.service_id), + 'created_by': str(t.created_by_id), + }, + service_id=t.service_id, + _expected_status=201 + ) + + assert response['template_name'] == t.name + assert response['status'] == BroadcastStatusType.DRAFT + assert response['created_at'] is not None + assert response['created_by_id'] == str(t.created_by_id) + assert response['personalisation'] == {} + assert response['areas'] == [] + + +@pytest.mark.parametrize('data, expected_errors', [ + ( + {}, + [ + {'error': 'ValidationError', 'message': 'template_id is a required property'}, + {'error': 'ValidationError', 'message': 'service_id is a required property'}, + {'error': 'ValidationError', 'message': 'created_by is a required property'} + ] + ), + ( + { + 'template_id': str(uuid.uuid4()), + 'service_id': str(uuid.uuid4()), + 'created_by': str(uuid.uuid4()), + 'foo': 'something else' + }, + [ + {'error': 'ValidationError', 'message': 'Additional properties are not allowed (foo was unexpected)'} + ] + ) +]) +def test_create_broadcast_message_400s_if_json_schema_fails_validation( + admin_request, + sample_service, + data, + expected_errors +): + t = create_template(sample_service, BROADCAST_TYPE) + + response = admin_request.post( + 'broadcast_message.create_broadcast_message', + _data=data, + service_id=t.service_id, + _expected_status=400 + ) + assert response['errors'] == expected_errors + + +def test_update_broadcast_message(admin_request, sample_service): + t = create_template(sample_service, BROADCAST_TYPE) + bm = create_broadcast_message(t, areas=['manchester']) + + response = admin_request.post( + 'broadcast_message.update_broadcast_message', + _data={'starts_at': '2020-06-01 20:00:01', 'areas': ['london', 'glasgow']}, + service_id=t.service_id, + broadcast_message_id=bm.id, + _expected_status=200 + ) + + assert response['starts_at'] == '2020-06-01T20:00:01.000000Z' + assert response['areas'] == ['london', 'glasgow'] + assert response['updated_at'] is not None + + +@pytest.mark.parametrize('input_dt', [ + '2020-06-01 20:00:01', + '2020-06-01T20:00:01', + '2020-06-01 20:00:01Z', + '2020-06-01T20:00:01+00:00', +]) +def test_update_broadcast_message_allows_sensible_datetime_formats(admin_request, sample_service, input_dt): + t = create_template(sample_service, BROADCAST_TYPE) + bm = create_broadcast_message(t) + + response = admin_request.post( + 'broadcast_message.update_broadcast_message', + _data={'starts_at': input_dt}, + service_id=t.service_id, + broadcast_message_id=bm.id, + _expected_status=200 + ) + + assert response['starts_at'] == '2020-06-01T20:00:01.000000Z' + assert response['updated_at'] is not None + + +def test_update_broadcast_message_doesnt_let_you_update_status(admin_request, sample_service): + t = create_template(sample_service, BROADCAST_TYPE) + bm = create_broadcast_message(t) + + response = admin_request.post( + 'broadcast_message.update_broadcast_message', + _data={'areas': ['glasgow'], 'status': BroadcastStatusType.BROADCASTING}, + service_id=t.service_id, + broadcast_message_id=bm.id, + _expected_status=400 + ) + + assert response['errors'] == [{ + 'error': 'ValidationError', + 'message': 'Additional properties are not allowed (status was unexpected)' + }] + + +def test_update_broadcast_message_status(admin_request, sample_service): + t = create_template(sample_service, BROADCAST_TYPE) + bm = create_broadcast_message(t, status=BroadcastStatusType.DRAFT) + + response = admin_request.post( + 'broadcast_message.update_broadcast_message_status', + _data={'status': BroadcastStatusType.PENDING_APPROVAL, 'created_by': str(t.created_by_id)}, + service_id=t.service_id, + broadcast_message_id=bm.id, + _expected_status=200 + ) + + assert response['status'] == BroadcastStatusType.PENDING_APPROVAL + assert response['updated_at'] is not None + + +def test_update_broadcast_message_status_doesnt_let_you_update_other_things(admin_request, sample_service): + t = create_template(sample_service, BROADCAST_TYPE) + bm = create_broadcast_message(t) + + response = admin_request.post( + 'broadcast_message.update_broadcast_message_status', + _data={'areas': ['glasgow'], 'status': BroadcastStatusType.BROADCASTING, 'created_by': str(t.created_by_id)}, + service_id=t.service_id, + broadcast_message_id=bm.id, + _expected_status=400 + ) + + assert response['errors'] == [{ + 'error': 'ValidationError', + 'message': 'Additional properties are not allowed (areas was unexpected)' + }] + + +def test_update_broadcast_message_status_stores_cancelled_by_and_cancelled_at(admin_request, sample_service): + t = create_template(sample_service, BROADCAST_TYPE) + bm = create_broadcast_message(t, status=BroadcastStatusType.BROADCASTING) + canceller = create_user('canceller@gov.uk') + + response = admin_request.post( + 'broadcast_message.update_broadcast_message_status', + _data={'status': BroadcastStatusType.CANCELLED, 'created_by': str(canceller.id)}, + service_id=t.service_id, + broadcast_message_id=bm.id, + _expected_status=200 + ) + + assert response['status'] == BroadcastStatusType.CANCELLED + assert response['cancelled_at'] is not None + assert response['cancelled_by_id'] == str(canceller.id) + + +def test_update_broadcast_message_status_stores_approved_by_and_approved_at(admin_request, sample_service): + t = create_template(sample_service, BROADCAST_TYPE) + bm = create_broadcast_message(t, status=BroadcastStatusType.PENDING_APPROVAL) + approver = create_user('approver@gov.uk') + + response = admin_request.post( + 'broadcast_message.update_broadcast_message_status', + _data={'status': BroadcastStatusType.BROADCASTING, 'created_by': str(approver.id)}, + service_id=t.service_id, + broadcast_message_id=bm.id, + _expected_status=200 + ) + + assert response['status'] == BroadcastStatusType.BROADCASTING + assert response['approved_at'] is not None + assert response['approved_by_id'] == str(approver.id) diff --git a/tests/app/db.py b/tests/app/db.py index 08a91e453..e45bff2af 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -59,7 +59,9 @@ from app.models import ( Domain, NotificationHistory, ReturnedLetter, - ServiceContactList + ServiceContactList, + BroadcastMessage, + BroadcastStatusType, ) @@ -984,3 +986,28 @@ def create_service_contact_list( db.session.add(contact_list) db.session.commit() return contact_list + + +def create_broadcast_message( + template, + created_by=None, + personalisation={}, + status=BroadcastStatusType.DRAFT, + starts_at=None, + finishes_at=None, + areas=[], +): + broadcast_message = BroadcastMessage( + service_id=template.service_id, + template_id=template.id, + template_version=template.version, + personalisation=personalisation, + status=BroadcastStatusType.DRAFT, + starts_at=starts_at, + finishes_at=finishes_at, + created_by_id=created_by.id if created_by else template.created_by_id, + areas=areas, + ) + db.session.add(broadcast_message) + db.session.commit() + return broadcast_message