diff --git a/.gitignore b/.gitignore index 1c6c16612..8be1355da 100644 --- a/.gitignore +++ b/.gitignore @@ -24,6 +24,7 @@ var/ *.egg-info/ .installed.cfg *.egg +/cache # PyInstaller # Usually these files are written by a python script from a template @@ -62,5 +63,8 @@ target/ # Mac *.DS_Store environment.sh +.envrc -celerybeat-schedule \ No newline at end of file +celerybeat-schedule + +app/version.py diff --git a/.travis.yml b/.travis.yml index 0d29a03cf..f7d9a4b66 100644 --- a/.travis.yml +++ b/.travis.yml @@ -14,6 +14,7 @@ install: before_script: - psql -c 'create database test_notification_api;' -U postgres script: +- make generate-version-file - ./scripts/run_tests.sh env: secure: tgeumbN2oayp1twu3iVuuo5eMz/XVO303A2wdnR6bFMCfLCA7SVeKvDZ21ZpSh+J7tu8/9MQD2ATo95qyO9oraSg09BQ7UoEtpyrrcP21UBcNMbIsAdmOUAostlvg4hy1ZuSjytpzLDMZfS0QCjWPtZiXKW3XzmHHJyIdatcHsO3Jpi1vPRP11cZHd1SKwd1POYXDuX3Y9e68yt0P7Wr1/3mZ8u0XHtSg++SnZ0qDDwnWIsHqkcxr7R/n1MYvyUD8XPB+guqEq/7G6ipR+QrHN0fCVGXFksXGLSMSBg9sGQ1Mr+2yiOXL+4EmCfpx9VofmEOFDTdK70lFFn1sOG/GmceW4JT2Y2vLG+6vSJTmaHxeZmpYoKRa1EJJqyEpvjRM3A8lV993qIdAEBIE8s0w+DhkmXXCI3chSDT+2B/SlFbGw7G7E4hto/3FUrk7N7w+c5WaOQSqz4ZxTX4iIg9T7Bxo3s8l+UYYw4NfzEreRieEiFo58FgYrghEOvMp9PZ3tN3u2H+2yISE0C/+MSFUB2CWgFiTTD2XtWuQJgGNxyTYD1sbHaYT1EeDoz8JbhsACvIxpQdycVibHjP4hvP32nFFaznNpCm1ArS+vDtzR6Psx2vYb/u0rf1QoipVE/GPzqB9bwGHZ/0Cpsqy4KxYM74MOu3Gi3KCYzKGq7hRGI= @@ -160,7 +161,6 @@ deploy: wait-until-deployed: true on: *2 before_deploy: - - ./scripts/update_version_file.sh - zip -r --exclude=*__pycache__* notifications-api * - mkdir -p dpl_cd_upload - mv notifications-api.zip dpl_cd_upload/notifications-api-$TRAVIS_BRANCH-$TRAVIS_BUILD_NUMBER-$TRAVIS_COMMIT.zip diff --git a/Makefile b/Makefile new file mode 100644 index 000000000..ff8aea5e4 --- /dev/null +++ b/Makefile @@ -0,0 +1,144 @@ +.DEFAULT_GOAL := help +SHELL := /bin/bash +DATE = $(shell date +%Y-%m-%d:%H:%M:%S) + +PIP_ACCEL_CACHE ?= ${CURDIR}/cache/pip-accel +APP_VERSION_FILE = app/version.py + +GIT_BRANCH = $(shell git symbolic-ref --short HEAD 2> /dev/null || echo "detached") +GIT_COMMIT ?= $(shell git rev-parse HEAD) + +DOCKER_BUILDER_IMAGE_NAME = govuk/notify-api-builder + +BUILD_TAG ?= notifications-api-manual +BUILD_NUMBER ?= 0 +DEPLOY_BUILD_NUMBER ?= ${BUILD_NUMBER} +BUILD_URL ?= + +DOCKER_CONTAINER_PREFIX = ${USER}-${BUILD_TAG} + +.PHONY: help +help: + @cat $(MAKEFILE_LIST) | grep -E '^[a-zA-Z_-]+:.*?## .*$$' | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}' + +.PHONY: venv +venv: venv/bin/activate ## Create virtualenv if it does not exist + +venv/bin/activate: + test -d venv || virtualenv venv + ./venv/bin/pip install pip-accel + +.PHONY: check-env-vars +check-env-vars: ## Check mandatory environment variables + $(if ${DEPLOY_ENV},,$(error Must specify DEPLOY_ENV)) + $(if ${DNS_NAME},,$(error Must specify DNS_NAME)) + $(if ${AWS_ACCESS_KEY_ID},,$(error Must specify AWS_ACCESS_KEY_ID)) + $(if ${AWS_SECRET_ACCESS_KEY},,$(error Must specify AWS_SECRET_ACCESS_KEY)) + +.PHONY: development +development: ## Set environment to development + $(eval export DEPLOY_ENV=development) + $(eval export DNS_NAME="notify.works") + @true + +.PHONY: staging +staging: ## Set environment to staging + $(eval export DEPLOY_ENV=staging) + $(eval export DNS_NAME="staging-notify.works") + @true + +.PHONY: production +production: ## Set environment to production + $(eval export DEPLOY_ENV=production) + $(eval export DNS_NAME="notifications.service.gov.uk") + @true + +.PHONY: dependencies +dependencies: venv ## Install build dependencies + mkdir -p ${PIP_ACCEL_CACHE} + PIP_ACCEL_CACHE=${PIP_ACCEL_CACHE} ./venv/bin/pip-accel install -r requirements_for_test.txt + +.PHONY: generate-version-file +generate-version-file: ## Generates the app version file + @echo -e "__travis_commit__ = \"${GIT_COMMIT}\"\n__time__ = \"${DATE}\"\n__travis_job_number__ = \"${BUILD_NUMBER}\"\n__travis_job_url__ = \"${BUILD_URL}\"" > ${APP_VERSION_FILE} + +.PHONY: build +build: dependencies generate-version-file ## Build project + +.PHONY: build-codedeploy-artifact +build-codedeploy-artifact: ## Build the deploy artifact for CodeDeploy + mkdir -p target + zip -r -x@deploy-exclude.lst target/notifications-api.zip * + +.PHONY: upload-codedeploy-artifact ## Upload the deploy artifact for CodeDeploy +upload-codedeploy-artifact: check-env-vars + aws s3 cp --region eu-west-1 target/notifications-api.zip s3://${DNS_NAME}-codedeploy/notifications-api-${DEPLOY_BUILD_NUMBER}.zip + +.PHONY: test +test: venv ## Run tests + ./scripts/run_tests.sh + +.PHONY: deploy-api +deploy-api: check-env-vars ## Trigger CodeDeploy for the api + aws deploy create-deployment --application-name notify-api --deployment-config-name CodeDeployDefault.OneAtATime --deployment-group-name notify-api --s3-location bucket=${DNS_NAME}-codedeploy,key=notifications-api-${DEPLOY_BUILD_NUMBER}.zip,bundleType=zip --region eu-west-1 + +.PHONY: deploy-admin-api +deploy-admin-api: check-env-vars ## Trigger CodeDeploy for the admin api + aws deploy create-deployment --application-name notify-admin-api --deployment-config-name CodeDeployDefault.OneAtATime --deployment-group-name notify-admin-api --s3-location bucket=${DNS_NAME}-codedeploy,key=notifications-api-${DEPLOY_BUILD_NUMBER}.zip,bundleType=zip --region eu-west-1 + +.PHONY: deploy-delivery +deploy-delivery: check-env-vars ## Trigger CodeDeploy for the delivery app + aws deploy create-deployment --application-name notify-delivery --deployment-config-name CodeDeployDefault.OneAtATime --deployment-group-name notify-delivery --s3-location bucket=${DNS_NAME}-codedeploy,key=notifications-api-${DEPLOY_BUILD_NUMBER}.zip,bundleType=zip --region eu-west-1 + +.PHONY: coverage +coverage: venv ## Create coverage report + ./venv/bin/coveralls + +.PHONY: prepare-docker-build-image +prepare-docker-build-image: ## Prepare the Docker builder image + mkdir -p ${PIP_ACCEL_CACHE} + make -C docker build-build-image + +.PHONY: build-with-docker +build-with-docker: prepare-docker-build-image ## Build inside a Docker container + docker run -i --rm \ + --name "${DOCKER_CONTAINER_PREFIX}-build" \ + -v `pwd`:/var/project \ + -v ${PIP_ACCEL_CACHE}:/var/project/cache/pip-accel \ + ${DOCKER_BUILDER_IMAGE_NAME} \ + make build + +.PHONY: test-with-docker +test-with-docker: prepare-docker-build-image create-docker-test-db ## Run tests inside a Docker container + docker run -i --rm \ + --name "${DOCKER_CONTAINER_PREFIX}-test" \ + --link "${DOCKER_CONTAINER_PREFIX}-db:postgres" \ + -e TEST_DATABASE=postgresql://postgres:postgres@postgres/test_notification_api \ + -v `pwd`:/var/project \ + ${DOCKER_BUILDER_IMAGE_NAME} \ + make test + +.PHONY: test-with-docker +create-docker-test-db: ## Start the test database in a Docker container + docker rm -f ${DOCKER_CONTAINER_PREFIX}-db 2> /dev/null || true + docker run -d \ + --name "${DOCKER_CONTAINER_PREFIX}-db" \ + -e POSTGRES_PASSWORD="postgres" \ + -e POSTGRES_DB=test_notification_api \ + postgres:9.5 + sleep 3 + +.PHONY: clean-docker-containers +clean-docker-containers: ## Clean up any remaining docker containers + docker rm -f $(shell docker ps -q -f "name=${DOCKER_CONTAINER_PREFIX}") 2> /dev/null || true + +.PHONY: coverage-with-docker +coverage-with-docker: prepare-docker-build-image ## Generates coverage report inside a Docker container + docker run -i --rm \ + --name "${DOCKER_CONTAINER_PREFIX}-coverage" \ + -v `pwd`:/var/project \ + ${DOCKER_BUILDER_IMAGE_NAME} \ + make coverage + +clean: + rm -rf node_modules cache target venv .coverage diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py index 0f23ce321..6d4873d69 100644 --- a/app/celery/provider_tasks.py +++ b/app/celery/provider_tasks.py @@ -1,29 +1,29 @@ from datetime import datetime from monotonic import monotonic +from urllib.parse import urljoin + from flask import current_app +from notifications_utils.recipients import ( + validate_and_format_phone_number +) +from notifications_utils.template import Template, get_sms_fragment_count +from notifications_utils.renderers import HTMLEmail, PlainTextEmail, SMSMessage + from app import notify_celery, statsd_client, clients, create_uuid from app.clients.email import EmailClientException from app.clients.sms import SmsClientException - from app.dao.notifications_dao import ( update_provider_stats, get_notification_by_id, dao_update_notification, update_notification_status_by_id ) - from app.dao.provider_details_dao import get_provider_details_by_notification_type from app.dao.services_dao import dao_fetch_service_by_id from app.celery.research_mode_tasks import send_sms_response, send_email_response -from notifications_utils.recipients import ( - validate_and_format_phone_number -) - from app.dao.templates_dao import dao_get_template_by_id -from notifications_utils.template import Template, get_sms_fragment_count -from notifications_utils.renderers import HTMLEmail, PlainTextEmail, SMSMessage -from app.models import SMS_TYPE, EMAIL_TYPE, KEY_TYPE_TEST +from app.models import SMS_TYPE, EMAIL_TYPE, KEY_TYPE_TEST, BRANDING_ORG from app.statsd_decorators import statsd @@ -92,11 +92,15 @@ def send_sms_to_provider(self, service_id, notification_id): except SmsClientException as e: try: current_app.logger.error( - "SMS notification {} failed".format(notification_id) + "RETRY: SMS notification {} failed".format(notification_id) ) current_app.logger.exception(e) self.retry(queue="retry", countdown=retry_iteration_to_delay(self.request.retries)) except self.MaxRetriesExceededError: + current_app.logger.error( + "RETRY FAILED: task send_sms_to_provider failed for notification {}".format(notification.id), + e + ) update_notification_status_by_id(notification.id, 'technical-failure', 'failure') current_app.logger.info( @@ -133,7 +137,7 @@ def send_email_to_provider(self, service_id, notification_id): html_email = Template( template_dict, values=notification.personalisation, - renderer=HTMLEmail() + renderer=get_html_email_renderer(service) ) plain_text_email = Template( @@ -173,11 +177,15 @@ def send_email_to_provider(self, service_id, notification_id): except EmailClientException as e: try: current_app.logger.error( - "Email notification {} failed".format(notification_id) + "RETRY: Email notification {} failed".format(notification_id) ) current_app.logger.exception(e) self.retry(queue="retry", countdown=retry_iteration_to_delay(self.request.retries)) except self.MaxRetriesExceededError: + current_app.logger.error( + "RETRY FAILED: task send_email_to_provider failed for notification {}".format(notification.id), + e + ) update_notification_status_by_id(notification.id, 'technical-failure', 'failure') current_app.logger.info( @@ -185,3 +193,22 @@ def send_email_to_provider(self, service_id, notification_id): ) delta_milliseconds = (datetime.utcnow() - notification.created_at).total_seconds() * 1000 statsd_client.timing("email.total-time", delta_milliseconds) + + +def get_html_email_renderer(service): + govuk_banner = service.branding != BRANDING_ORG + if service.organisation: + logo = '{}{}{}'.format( + current_app.config['ADMIN_BASE_URL'], + current_app.config['BRANDING_PATH'], + service.organisation.logo + ) + branding = { + 'brand_colour': service.organisation.colour, + 'brand_logo': logo, + 'brand_name': service.organisation.name, + } + else: + branding = {} + + return HTMLEmail(govuk_banner=govuk_banner, **branding) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index bdcb37327..9dc332469 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -126,7 +126,7 @@ def remove_job(job_id): current_app.logger.info("Job {} has been removed from s3.".format(job_id)) -@notify_celery.task(bind=True, name="send-sms", max_retries=5, default_retry_delay=5) +@notify_celery.task(bind=True, name="send-sms", max_retries=5, default_retry_delay=300) @statsd(namespace="tasks") def send_sms(self, service_id, @@ -154,11 +154,17 @@ def send_sms(self, ) except SQLAlchemyError as e: - current_app.logger.exception(e) - raise self.retry(queue="retry", exc=e) + current_app.logger.exception("RETRY: send_sms notification {}".format(notification_id), e) + try: + raise self.retry(queue="retry", exc=e) + except self.MaxRetriesExceededError: + current_app.logger.exception( + "RETRY FAILED: task send_sms failed for notification {}".format(notification.id), + e + ) -@notify_celery.task(bind=True, name="send-email", max_retries=5, default_retry_delay=5) +@notify_celery.task(bind=True, name="send-email", max_retries=5, default_retry_delay=300) @statsd(namespace="tasks") def send_email(self, service_id, notification_id, @@ -180,8 +186,14 @@ def send_email(self, service_id, current_app.logger.info("Email {} created at {}".format(notification_id, created_at)) except SQLAlchemyError as e: - current_app.logger.exception(e) - raise self.retry(queue="retry", exc=e) + current_app.logger.exception("RETRY: send_email notification {}".format(notification_id), e) + try: + raise self.retry(queue="retry", exc=e) + except self.MaxRetriesExceededError: + current_app.logger.error( + "RETRY FAILED: task send_email failed for notification {}".format(notification.id), + e + ) def _save_notification(created_at, notification, notification_id, service_id, notification_type, api_key_id, key_type): diff --git a/app/dao/dao_utils.py b/app/dao/dao_utils.py index f5179e8b5..d5ec27fb5 100644 --- a/app/dao/dao_utils.py +++ b/app/dao/dao_utils.py @@ -1,6 +1,7 @@ import itertools -from functools import wraps -from app.history_meta import versioned_objects, create_history +from functools import wraps, partial + +from app.history_meta import create_history def transactional(func): @@ -19,14 +20,16 @@ def transactional(func): return commit_or_rollback -def version_class(model_class): +def version_class(model_class, history_cls=None): + create_hist = partial(create_history, history_cls=history_cls) + def versioned(func): @wraps(func) def record_version(*args, **kwargs): from app import db func(*args, **kwargs) - history_objects = [create_history(obj) for obj in - versioned_objects(itertools.chain(db.session.new, db.session.dirty)) + history_objects = [create_hist(obj) for obj in + itertools.chain(db.session.new, db.session.dirty) if isinstance(obj, model_class)] for h_obj in history_objects: db.session.add(h_obj) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 09a101ccd..bc41b7369 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -7,9 +7,8 @@ from datetime import ( from flask import current_app from werkzeug.datastructures import MultiDict -from sqlalchemy import (desc, func, Integer, or_, and_, asc) -from sqlalchemy.sql.expression import cast -from notifications_utils.template import get_sms_fragment_count +from sqlalchemy import (desc, func, or_, and_, asc) +from sqlalchemy.orm import joinedload from app import db from app.dao import days_ago @@ -91,6 +90,32 @@ def create_notification_statistics_dict(service_id, day): } +@statsd(namespace="dao") +def dao_get_template_usage(service_id, limit_days=None): + + table = NotificationHistory + + if limit_days and limit_days <= 7: # can get this data from notifications table + table = Notification + + query = db.session.query( + func.count(table.template_id).label('count'), + table.template_id, + Template.name, + Template.template_type + ) + + query_filter = [table.service_id == service_id] + if limit_days is not None: + query_filter.append(table.created_at >= days_ago(limit_days)) + + return query.filter(*query_filter) \ + .join(Template)\ + .group_by(table.template_id, Template.name, Template.template_type)\ + .order_by(asc(Template.name))\ + .all() + + @statsd(namespace="dao") def dao_get_template_statistics_for_service(service_id, limit_days=None): query_filter = [TemplateStatistics.service_id == service_id] @@ -307,12 +332,12 @@ def get_notifications_for_job(service_id, job_id, filter_dict=None, page=1, page @statsd(namespace="dao") -def get_notification(service_id, notification_id, key_type=None): +def get_notification_with_personalisation(service_id, notification_id, key_type): filter_dict = {'service_id': service_id, 'id': notification_id} if key_type: filter_dict['key_type'] = key_type - return Notification.query.filter_by(**filter_dict).one() + return Notification.query.filter_by(**filter_dict).options(joinedload('template_history')).one() @statsd(namespace="dao") @@ -330,7 +355,8 @@ def get_notifications_for_service(service_id, page=1, page_size=None, limit_days=None, - key_type=None): + key_type=None, + personalisation=False): if page_size is None: page_size = current_app.config['PAGE_SIZE'] filters = [Notification.service_id == service_id] @@ -344,6 +370,10 @@ def get_notifications_for_service(service_id, query = Notification.query.filter(*filters) query = _filter_query(query, filter_dict) + if personalisation: + query = query.options( + joinedload('template_history') + ) return query.order_by(desc(Notification.created_at)).paginate( page=page, per_page=page_size diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 5bd2e975a..eff4679dd 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -16,6 +16,7 @@ from app.models import ( VerifyCode, ApiKey, Template, + TemplateHistory, Job, NotificationHistory, Notification, @@ -122,7 +123,7 @@ def delete_service_and_all_associated_db_objects(service): _delete_commit(Notification.query.filter_by(service=service)) _delete_commit(Job.query.filter_by(service=service)) _delete_commit(Template.query.filter_by(service=service)) - _delete_commit(Template.get_history_model().query.filter_by(service_id=service.id)) + _delete_commit(TemplateHistory.query.filter_by(service_id=service.id)) verify_codes = VerifyCode.query.join(User).filter(User.id.in_([x.id for x in service.users])) list(map(db.session.delete, verify_codes)) diff --git a/app/dao/templates_dao.py b/app/dao/templates_dao.py index d69b530e0..f7f4c5e4d 100644 --- a/app/dao/templates_dao.py +++ b/app/dao/templates_dao.py @@ -1,8 +1,9 @@ import uuid -from app import db -from app.models import (Template, Service) + from sqlalchemy import (asc, desc) +from app import db +from app.models import (Template, TemplateHistory) from app.dao.dao_utils import ( transactional, version_class @@ -10,7 +11,7 @@ from app.dao.dao_utils import ( @transactional -@version_class(Template) +@version_class(Template, TemplateHistory) def dao_create_template(template): template.id = uuid.uuid4() # must be set now so version history model can use same id template.archived = False @@ -18,14 +19,14 @@ def dao_create_template(template): @transactional -@version_class(Template) +@version_class(Template, TemplateHistory) def dao_update_template(template): db.session.add(template) def dao_get_template_by_id_and_service_id(template_id, service_id, version=None): if version is not None: - return Template.get_history_model().query.filter_by( + return TemplateHistory.query.filter_by( id=template_id, service_id=service_id, version=version).one() @@ -34,7 +35,7 @@ def dao_get_template_by_id_and_service_id(template_id, service_id, version=None) def dao_get_template_by_id(template_id, version=None): if version is not None: - return Template.get_history_model().query.filter_by( + return TemplateHistory.query.filter_by( id=template_id, version=version).one() return Template.query.filter_by(id=template_id).one() @@ -50,6 +51,8 @@ def dao_get_all_templates_for_service(service_id): def dao_get_template_versions(service_id, template_id): - history_model = Template.get_history_model() - return history_model.query.filter_by(service_id=service_id, id=template_id).order_by( - desc(history_model.version)).all() + return TemplateHistory.query.filter_by( + service_id=service_id, id=template_id + ).order_by( + desc(TemplateHistory.version) + ).all() diff --git a/app/history_meta.py b/app/history_meta.py index 9cbbe07f1..49a420572 100644 --- a/app/history_meta.py +++ b/app/history_meta.py @@ -171,17 +171,13 @@ class Versioned(object): return history_mapper.class_ -def versioned_objects(iter): - for obj in iter: - if hasattr(obj, '__history_mapper__'): - yield obj +def create_history(obj, history_cls=None): + if not history_cls: + history_mapper = obj.__history_mapper__ + history_cls = history_mapper.class_ - -def create_history(obj): - obj_mapper = object_mapper(obj) - history_mapper = obj.__history_mapper__ - history_cls = history_mapper.class_ history = history_cls() + obj_mapper = object_mapper(obj) obj_state = attributes.instance_state(obj) data = {} diff --git a/app/models.py b/app/models.py index e3aeb3d13..efe5bb7b7 100644 --- a/app/models.py +++ b/app/models.py @@ -5,7 +5,8 @@ from sqlalchemy.dialects.postgresql import ( UUID, JSON ) -from sqlalchemy import UniqueConstraint, text +from sqlalchemy import UniqueConstraint, text, ForeignKeyConstraint, and_ +from sqlalchemy.orm import foreign, remote from app.encryption import ( hashpw, @@ -116,7 +117,7 @@ class Service(db.Model, Versioned): secondary=user_to_service, backref=db.backref('user_to_service', lazy='dynamic')) restricted = db.Column(db.Boolean, index=False, unique=False, nullable=False) - research_mode = db.Column(db.Boolean, index=False, unique=False, nullable=True, default=False) + research_mode = db.Column(db.Boolean, index=False, unique=False, nullable=False, default=False) email_from = db.Column(db.Text, index=False, unique=True, nullable=False) created_by = db.relationship('User') created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), index=True, nullable=False) @@ -206,7 +207,7 @@ TEMPLATE_TYPES = [SMS_TYPE, EMAIL_TYPE, LETTER_TYPE] template_types = db.Enum(*TEMPLATE_TYPES, name='template_type') -class Template(db.Model, Versioned): +class Template(db.Model): __tablename__ = 'templates' id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) @@ -231,6 +232,26 @@ class Template(db.Model, Versioned): subject = db.Column(db.Text, index=False, unique=False, nullable=True) created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), index=True, nullable=False) created_by = db.relationship('User') + version = db.Column(db.Integer, default=1) + + +class TemplateHistory(db.Model): + __tablename__ = 'templates_history' + + id = db.Column(UUID(as_uuid=True), primary_key=True) + name = db.Column(db.String(255), nullable=False) + template_type = db.Column(template_types, nullable=False) + created_at = db.Column(db.DateTime, nullable=False) + updated_at = db.Column(db.DateTime) + content = db.Column(db.Text, nullable=False) + archived = db.Column(db.Boolean, nullable=False, default=False) + service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), index=True, nullable=False) + service = db.relationship('Service') + subject = db.Column(db.Text) + created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), index=True, nullable=False) + created_by = db.relationship('User') + version = db.Column(db.Integer, primary_key=True) + MMG_PROVIDER = "mmg" FIRETEXT_PROVIDER = "firetext" @@ -427,6 +448,11 @@ class Notification(db.Model): reference = db.Column(db.String, nullable=True, index=True) _personalisation = db.Column(db.String, nullable=True) + template_history = db.relationship('TemplateHistory', primaryjoin=and_( + foreign(template_id) == remote(TemplateHistory.id), + foreign(template_version) == remote(TemplateHistory.version) + )) + @property def personalisation(self): if self._personalisation: diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 61bd66574..8e4af63b2 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -1,5 +1,6 @@ from datetime import datetime import itertools + from flask import ( Blueprint, jsonify, @@ -7,6 +8,7 @@ from flask import ( current_app, json ) + from notifications_utils.recipients import allowed_to_send_to, first_column_heading from notifications_utils.template import Template from notifications_utils.renderers import PassThrough @@ -170,10 +172,10 @@ def process_firetext_response(): @notifications.route('/notifications/', methods=['GET']) -def get_notifications(notification_id): - notification = notifications_dao.get_notification(str(api_user.service_id), - notification_id, - key_type=api_user.key_type) +def get_notification_by_id(notification_id): + notification = notifications_dao.get_notification_with_personalisation(str(api_user.service_id), + notification_id, + key_type=api_user.key_type) return jsonify(data={"notification": notification_with_personalisation_schema.dump(notification).data}), 200 @@ -186,6 +188,7 @@ def get_all_notifications(): pagination = notifications_dao.get_notifications_for_service( str(api_user.service_id), + personalisation=True, filter_dict=data, page=page, page_size=page_size, diff --git a/app/schemas.py b/app/schemas.py index 858983ad8..7928b8c29 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -103,6 +103,8 @@ class ProviderDetailsSchema(BaseSchema): class ServiceSchema(BaseSchema): created_by = field_for(models.Service, 'created_by', required=True) + organisation = field_for(models.Service, 'organisation') + branding = field_for(models.Service, 'branding') class Meta: model = models.Service @@ -114,8 +116,7 @@ class ServiceSchema(BaseSchema): 'old_id', 'template_statistics', 'service_provider_stats', - 'service_notification_stats', - 'organisation') + 'service_notification_stats') strict = True @validates('sms_sender') @@ -172,17 +173,11 @@ class TemplateSchema(BaseTemplateSchema): class TemplateHistorySchema(BaseSchema): - class Meta: - # Use the base model class that the history class is created from - model = models.Template - # We have to use a method here because the relationship field on the - # history object is not created. - created_by = fields.Method("populate_created_by", dump_only=True) + created_by = fields.Nested(UserSchema, only=['id', 'name', 'email_address'], dump_only=True) created_at = field_for(models.Template, 'created_at', format='%Y-%m-%d %H:%M:%S.%f') - def populate_created_by(self, data): - usr = models.User.query.filter_by(id=data.created_by_id).one() - return {'id': str(usr.id), 'name': usr.name, 'email_address': usr.email_address} + class Meta: + model = models.TemplateHistory class NotificationsStatisticsSchema(BaseSchema): @@ -288,12 +283,20 @@ class NotificationWithTemplateSchema(BaseSchema): strict = True exclude = ('_personalisation',) - template = fields.Nested(TemplateSchema, only=["id", "name", "template_type", "content", "subject"], dump_only=True) + template = fields.Nested( + TemplateSchema, + only=['id', 'version', 'name', 'template_type', 'content', 'subject'], + dump_only=True + ) job = fields.Nested(JobSchema, only=["id", "original_file_name"], dump_only=True) personalisation = fields.Dict(required=False) class NotificationWithPersonalisationSchema(NotificationWithTemplateSchema): + template_history = fields.Nested(TemplateHistorySchema, + only=['id', 'name', 'template_type', 'content', 'subject', 'version'], + dump_only=True) + class Meta(NotificationWithTemplateSchema.Meta): # mark as many fields as possible as required since this is a public api. # WARNING: Does _not_ reference fields computed in handle_template_merge, such as @@ -305,7 +308,7 @@ class NotificationWithPersonalisationSchema(NotificationWithTemplateSchema): # computed fields 'personalisation', # relationships - 'service', 'job', 'api_key', 'template' + 'service', 'job', 'api_key', 'template_history' ) @pre_dump @@ -315,6 +318,7 @@ class NotificationWithPersonalisationSchema(NotificationWithTemplateSchema): @post_dump def handle_template_merge(self, in_data): + in_data['template'] = in_data.pop('template_history') from notifications_utils.template import Template template = Template( in_data['template'], diff --git a/app/service/rest.py b/app/service/rest.py index 35f22769d..1c0096000 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -173,7 +173,7 @@ def get_service_provider_aggregate_statistics(service_id): # tables. This is so product owner can pass stories as done @service.route('//history', methods=['GET']) def get_service_history(service_id): - from app.models import (Service, ApiKey, Template, Event) + from app.models import (Service, ApiKey, Template, TemplateHistory, Event) from app.schemas import ( service_history_schema, api_key_history_schema, @@ -186,7 +186,7 @@ def get_service_history(service_id): api_key_history = ApiKey.get_history_model().query.filter_by(service_id=service_id).all() api_keys_data = api_key_history_schema.dump(api_key_history, many=True).data - template_history = Template.get_history_model().query.filter_by(service_id=service_id).all() + template_history = TemplateHistory.query.filter_by(service_id=service_id).all() template_data, errors = template_history_schema.dump(template_history, many=True) events = Event.query.all() diff --git a/app/template_statistics/rest.py b/app/template_statistics/rest.py index d4990ba04..191645b96 100644 --- a/app/template_statistics/rest.py +++ b/app/template_statistics/rest.py @@ -5,6 +5,7 @@ from flask import ( ) from app.dao.notifications_dao import ( + dao_get_template_usage, dao_get_template_statistics_for_service, dao_get_template_statistics_for_template ) @@ -21,7 +22,7 @@ register_errors(template_statistics) @template_statistics.route('') -def get_template_statistics_for_service(service_id): +def get_template_statistics_for_service_by_day(service_id): if request.args.get('limit_days'): try: limit_days = int(request.args['limit_days']) @@ -31,9 +32,17 @@ def get_template_statistics_for_service(service_id): raise InvalidRequest(message, status_code=400) else: limit_days = None - stats = dao_get_template_statistics_for_service(service_id, limit_days=limit_days) - data = template_statistics_schema.dump(stats, many=True).data - return jsonify(data=data) + stats = dao_get_template_usage(service_id, limit_days=limit_days) + + def serialize(data): + return { + 'count': data.count, + 'template_id': str(data.template_id), + 'template_name': data.name, + 'template_type': data.template_type + } + + return jsonify(data=[serialize(row) for row in stats]) @template_statistics.route('/') diff --git a/app/version.py b/app/version.py.dist similarity index 56% rename from app/version.py rename to app/version.py.dist index d6d5d6781..295df522a 100644 --- a/app/version.py +++ b/app/version.py.dist @@ -1,3 +1,4 @@ __travis_commit__ = "" -__time__ = "2016-07-05:15:38:37" +__time__ = "" __travis_job_number__ = "" +__travis_job_url__ = "" diff --git a/aws_run_celery.py b/aws_run_celery.py index 1165af098..e1db882a2 100644 --- a/aws_run_celery.py +++ b/aws_run_celery.py @@ -6,5 +6,5 @@ import os # on aws get secrets and export to env os.environ.update(getAllSecrets(region="eu-west-1")) -application = create_app() +application = create_app("delivery") application.app_context().push() diff --git a/config.py b/config.py index 3ae8441c7..d8067dc53 100644 --- a/config.py +++ b/config.py @@ -28,6 +28,7 @@ class Config(object): PAGE_SIZE = 50 SMS_CHAR_COUNT_LIMIT = 495 MMG_URL = os.environ['MMG_URL'] + BRANDING_PATH = '/static/images/email-template/crests/' NOTIFY_SERVICE_ID = 'd6aa2c68-a2d9-4437-ab19-3ae8eb202553' INVITATION_EMAIL_TEMPLATE_ID = '4f46df42-f795-4cc4-83bb-65ca312f49cc' @@ -111,7 +112,7 @@ class Config(object): class Development(Config): NOTIFY_ENVIRONMENT = 'development' - CSV_UPLOAD_BUCKET_NAME = 'developement-martyn-notifications-csv-upload' + CSV_UPLOAD_BUCKET_NAME = 'development-notifications-csv-upload' DEBUG = True diff --git a/deploy-exclude.lst b/deploy-exclude.lst new file mode 100644 index 000000000..5eaf61748 --- /dev/null +++ b/deploy-exclude.lst @@ -0,0 +1,9 @@ +*__pycache__* +*.git* +*app/assets* +*bower_components* +*cache* +*docker* +*node_modules* +*target* +*venv* diff --git a/docker/Dockerfile-build b/docker/Dockerfile-build new file mode 100644 index 000000000..4fa6afb2d --- /dev/null +++ b/docker/Dockerfile-build @@ -0,0 +1,25 @@ +FROM python:3.4-slim + +ENV PYTHONUNBUFFERED=1 \ + DEBIAN_FRONTEND=noninteractive + +RUN \ + echo "Install base packages" \ + && apt-get update \ + && apt-get install -y --no-install-recommends \ + make \ + git \ + build-essential \ + zip \ + libpq-dev \ + + && echo "Clean up" \ + && rm -rf /var/lib/apt/lists/* /tmp/* + +RUN \ + echo "Install global pip packages" \ + && pip install \ + virtualenv \ + awscli + +WORKDIR /var/project diff --git a/docker/Makefile b/docker/Makefile new file mode 100644 index 000000000..2db0aade2 --- /dev/null +++ b/docker/Makefile @@ -0,0 +1,10 @@ +.DEFAULT_GOAL := help +SHELL := /bin/bash + +.PHONY: help +help: + @cat $(MAKEFILE_LIST) | grep -E '^[a-zA-Z_-]+:.*?## .*$$' | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}' + +.PHONY: build-build-image +build-build-image: + docker build -f Dockerfile-build -t govuk/notify-api-builder . diff --git a/migrations/versions/0046_organisations_and_branding.py b/migrations/versions/0046_organisations_and_branding.py index c03faf2ce..fc31bdcd7 100644 --- a/migrations/versions/0046_organisations_and_branding.py +++ b/migrations/versions/0046_organisations_and_branding.py @@ -33,6 +33,7 @@ def upgrade(): op.add_column('services_history', sa.Column('organisation_id', postgresql.UUID(as_uuid=True))) op.execute("INSERT INTO branding_type VALUES ('govuk'), ('org'), ('both')") + # insert UKVI data as initial test data. hex and crest pulled from alphagov/whitehall op.execute("""INSERT INTO organisation VALUES ( '9d25d02d-2915-4e98-874b-974e123e8536', diff --git a/requirements.txt b/requirements.txt index 7e4ac3885..904bd6e3e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -23,4 +23,4 @@ statsd==3.2.1 git+https://github.com/alphagov/notifications-python-client.git@1.0.0#egg=notifications-python-client==1.0.0 -git+https://github.com/alphagov/notifications-utils.git@8.7.5#egg=notifications-utils==8.7.5 +git+https://github.com/alphagov/notifications-utils.git@8.7.6#egg=notifications-utils==8.7.6 diff --git a/scripts/common_functions.sh b/scripts/common_functions.sh index a2e136b25..a489377ef 100644 --- a/scripts/common_functions.sh +++ b/scripts/common_functions.sh @@ -361,7 +361,6 @@ get_instance_name_from_tags() { error_exit "Couldn't get instance name for '$instance_id'" fi echo $instance_name - echo $instance_id return $? } @@ -371,15 +370,12 @@ get_elb_name_for_instance_name() { local instance_name=$1 declare -A elb_to_instance_mapping - + + elb_to_instance_mapping['notify-api']='notify-api' + elb_to_instance_mapping['notify-admin-api']='notify-admin-api' + elb_to_instance_mapping['notify_api']='notify-api-elb' elb_to_instance_mapping['notify_admin_api']='notify-admin-api-elb' - elb_to_instance_mapping['live_notify_api']='live-notify-api-elb' - elb_to_instance_mapping['staging_notify_api']='staging-notify-api-elb' - elb_to_instance_mapping['NotifyApi']='notify-api-elb' - elb_to_instance_mapping['live_notify_admin_api']='live-notify-admin-api-elb' - elb_to_instance_mapping['staging_notify_admin_api']='staging-notify-admin-api-elb' - elb_to_instance_mapping['NotifyAdminApi']='notify-admin-api-elb' local elb_name=${elb_to_instance_mapping[${instance_name}]} if [ -z $elb_name ]; then @@ -387,4 +383,4 @@ get_elb_name_for_instance_name() { else ELB_NAME=$elb_name fi -} \ No newline at end of file +} diff --git a/scripts/register_with_elb.sh b/scripts/register_with_elb.sh index 8e37f7757..7e6fb80ac 100755 --- a/scripts/register_with_elb.sh +++ b/scripts/register_with_elb.sh @@ -28,7 +28,6 @@ msg "Started $(basename $0) at $(/bin/date "+%F %T")" start_sec=$(/bin/date +%s.%N) msg "Getting relevant load balancer" - INSTANCE_NAME=$(get_instance_name_from_tags $INSTANCE_ID) if [[ "$(tr [:upper:] [:lower:] <<< "${INSTANCE_NAME}")" =~ 'delivery' ]]; then @@ -37,6 +36,7 @@ if [[ "$(tr [:upper:] [:lower:] <<< "${INSTANCE_NAME}")" =~ 'delivery' ]]; then fi get_elb_name_for_instance_name $INSTANCE_NAME +ELB_LIST=$ELB_NAME get_elb_list $INSTANCE_ID $ELB_NAME msg "Checking that user set at least one load balancer" diff --git a/scripts/run_tests.sh b/scripts/run_tests.sh index dd229b408..652094dd2 100755 --- a/scripts/run_tests.sh +++ b/scripts/run_tests.sh @@ -24,6 +24,9 @@ function display_result { fi } +if [ -d venv ]; then + source ./venv/bin/activate +fi pep8 . display_result $? 1 "Code style check" diff --git a/scripts/update_version_file.sh b/scripts/update_version_file.sh deleted file mode 100755 index 231c210b7..000000000 --- a/scripts/update_version_file.sh +++ /dev/null @@ -1,7 +0,0 @@ -#!/bin/bash -# -# Update the version file of the project from the Travis build details -# -sed -i -e "s/__travis_commit__ =.*/__travis_commit__ = \"$TRAVIS_COMMIT\"/g" ./app/version.py -sed -i -e "s/__travis_job_number__ =.*/__travis_job_number__ = \"$TRAVIS_BUILD_NUMBER\"/g" ./app/version.py -sed -i -e "s/__time__ =.*/__time__ = \"$(date +%Y-%m-%d:%H:%M:%S)\"/g" ./app/version.py \ No newline at end of file diff --git a/tests/app/celery/test_provider_tasks.py b/tests/app/celery/test_provider_tasks.py index 648c4a33d..5baca8f0d 100644 --- a/tests/app/celery/test_provider_tasks.py +++ b/tests/app/celery/test_provider_tasks.py @@ -16,7 +16,16 @@ from app.clients.sms import SmsClientException from app.dao import notifications_dao, provider_details_dao from app.dao import provider_statistics_dao from app.dao.provider_statistics_dao import get_provider_statistics -from app.models import Notification, NotificationStatistics, Job, KEY_TYPE_NORMAL, KEY_TYPE_TEST +from app.models import ( + Notification, + NotificationStatistics, + Job, + Organisation, + KEY_TYPE_NORMAL, + KEY_TYPE_TEST, + BRANDING_ORG, + BRANDING_BOTH +) from tests.app.conftest import sample_notification @@ -188,7 +197,7 @@ def test_send_sms_should_use_template_version_from_notification_not_latest( sender=None ) - persisted_notification = notifications_dao.get_notification(sample_template.service_id, db_notification.id) + persisted_notification = notifications_dao.get_notification_by_id(db_notification.id) assert persisted_notification.to == db_notification.to assert persisted_notification.template_id == sample_template.id assert persisted_notification.template_version == version_on_notification @@ -223,7 +232,7 @@ def test_should_call_send_sms_response_task_if_research_mode(notify_db, sample_s ('mmg', str(sample_notification.id), sample_notification.to), queue='research-mode' ) - persisted_notification = notifications_dao.get_notification(sample_service.id, sample_notification.id) + persisted_notification = notifications_dao.get_notification_by_id(sample_notification.id) assert persisted_notification.to == sample_notification.to assert persisted_notification.template_id == sample_notification.template_id assert persisted_notification.status == 'sending' @@ -499,10 +508,48 @@ def test_should_not_set_billable_units_if_research_mode(notify_db, sample_servic sample_notification.id ) - persisted_notification = notifications_dao.get_notification(sample_service.id, sample_notification.id) + persisted_notification = notifications_dao.get_notification_by_id(sample_notification.id) assert persisted_notification.billable_units == 0 +def test_get_html_email_renderer_should_return_for_normal_service(sample_service): + renderer = provider_tasks.get_html_email_renderer(sample_service) + assert renderer.govuk_banner + assert renderer.brand_colour is None + assert renderer.brand_logo is None + assert renderer.brand_name is None + + +@pytest.mark.parametrize('branding_type, govuk_banner', [ + (BRANDING_ORG, False), + (BRANDING_BOTH, True) +]) +def test_get_html_email_renderer_with_branding_details(branding_type, govuk_banner, notify_db, sample_service): + sample_service.branding = branding_type + org = Organisation(colour='#000000', logo='justice-league.png', name='Justice League') + sample_service.organisation = org + notify_db.session.add_all([sample_service, org]) + notify_db.session.commit() + + renderer = provider_tasks.get_html_email_renderer(sample_service) + + assert renderer.govuk_banner == govuk_banner + assert renderer.brand_colour == '000000' + assert renderer.brand_name == 'Justice League' + + +def test_get_html_email_renderer_prepends_logo_path(notify_db, sample_service): + sample_service.branding = BRANDING_ORG + org = Organisation(colour='#000000', logo='justice-league.png', name='Justice League') + sample_service.organisation = org + notify_db.session.add_all([sample_service, org]) + notify_db.session.commit() + + renderer = provider_tasks.get_html_email_renderer(sample_service) + + assert renderer.brand_logo == 'http://localhost:6012/static/images/email-template/crests/justice-league.png' + + def _get_provider_statistics(service, **kwargs): query = ProviderStatistics.query.filter_by(service=service) if 'providers' in kwargs: diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index a5dc571af..09b945606 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -23,7 +23,7 @@ from app.models import ( from app.dao.notifications_dao import ( dao_create_notification, dao_update_notification, - get_notification, + get_notification_with_personalisation, get_notification_for_job, get_notifications_for_job, delete_notifications_created_more_than_a_week_ago, @@ -34,14 +34,18 @@ from app.dao.notifications_dao import ( dao_get_template_statistics_for_service, get_notifications_for_service, dao_get_potential_notification_statistics_for_day, - dao_get_template_statistics_for_template, get_notification_by_id) + dao_get_template_statistics_for_template, + get_notification_by_id, + dao_get_template_usage +) from notifications_utils.template import get_sms_fragment_count -from tests.app.conftest import (sample_notification) +from tests.app.conftest import (sample_notification, sample_template, sample_email_template, sample_service) def test_should_have_decorated_notifications_dao_functions(): + assert dao_get_template_usage.__wrapped__.__name__ == 'dao_get_template_usage' # noqa assert dao_get_notification_statistics_for_service_and_day.__wrapped__.__name__ == 'dao_get_notification_statistics_for_service_and_day' # noqa assert dao_get_potential_notification_statistics_for_day.__wrapped__.__name__ == 'dao_get_potential_notification_statistics_for_day' # noqa assert dao_get_template_statistics_for_service.__wrapped__.__name__ == 'dao_get_template_statistics_for_service' # noqa @@ -53,12 +57,143 @@ def test_should_have_decorated_notifications_dao_functions(): assert update_notification_status_by_reference.__wrapped__.__name__ == 'update_notification_status_by_reference' # noqa assert get_notification_for_job.__wrapped__.__name__ == 'get_notification_for_job' # noqa assert get_notifications_for_job.__wrapped__.__name__ == 'get_notifications_for_job' # noqa - assert get_notification.__wrapped__.__name__ == 'get_notification' # noqa + assert get_notification_with_personalisation.__wrapped__.__name__ == 'get_notification_with_personalisation' # noqa assert get_notifications_for_service.__wrapped__.__name__ == 'get_notifications_for_service' # noqa assert get_notification_by_id.__wrapped__.__name__ == 'get_notification_by_id' # noqa assert delete_notifications_created_more_than_a_week_ago.__wrapped__.__name__ == 'delete_notifications_created_more_than_a_week_ago' # noqa +def test_should_by_able_to_get_template_count_from_notifications_history(notify_db, notify_db_session, sample_service): + sms = sample_template(notify_db, notify_db_session) + email = sample_email_template(notify_db, notify_db_session) + sample_notification(notify_db, notify_db_session, service=sample_service, template=sms) + sample_notification(notify_db, notify_db_session, service=sample_service, template=sms) + sample_notification(notify_db, notify_db_session, service=sample_service, template=sms) + sample_notification(notify_db, notify_db_session, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, service=sample_service, template=email) + + results = dao_get_template_usage(sample_service.id) + assert results[0].name == 'Email Template Name' + assert results[0].template_type == 'email' + assert results[0].count == 2 + + assert results[1].name == 'Template Name' + assert results[1].template_type == 'sms' + assert results[1].count == 3 + + +def test_should_by_able_to_get_template_count_from_notifications_history_for_service( + notify_db, + notify_db_session): + service_1 = sample_service(notify_db, notify_db_session, service_name="test1", email_from="test1") + service_2 = sample_service(notify_db, notify_db_session, service_name="test2", email_from="test2") + service_3 = sample_service(notify_db, notify_db_session, service_name="test3", email_from="test3") + + sms = sample_template(notify_db, notify_db_session) + + sample_notification(notify_db, notify_db_session, service=service_1, template=sms) + sample_notification(notify_db, notify_db_session, service=service_1, template=sms) + sample_notification(notify_db, notify_db_session, service=service_2, template=sms) + + assert dao_get_template_usage(service_1.id)[0].count == 2 + assert dao_get_template_usage(service_2.id)[0].count == 1 + assert len(dao_get_template_usage(service_3.id)) == 0 + + +def test_should_by_able_to_get_zero_count_from_notifications_history_if_no_rows(sample_service): + results = dao_get_template_usage(sample_service.id) + assert len(results) == 0 + + +def test_should_by_able_to_get_zero_count_from_notifications_history_if_no_service(): + results = dao_get_template_usage(str(uuid.uuid4())) + assert len(results) == 0 + + +def test_should_by_able_to_get_template_count_from_notifications_history_across_days( + notify_db, + notify_db_session, + sample_service): + sms = sample_template(notify_db, notify_db_session) + email = sample_email_template(notify_db, notify_db_session) + + today = datetime.now() + yesterday = datetime.now() - timedelta(days=1) + one_month_ago = datetime.now() - timedelta(days=30) + + sample_notification(notify_db, notify_db_session, created_at=today, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, created_at=today, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, created_at=today, service=sample_service, template=sms) + + sample_notification(notify_db, notify_db_session, created_at=yesterday, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, created_at=yesterday, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, created_at=yesterday, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, created_at=yesterday, service=sample_service, template=sms) + + sample_notification(notify_db, notify_db_session, created_at=one_month_ago, service=sample_service, template=sms) + sample_notification(notify_db, notify_db_session, created_at=one_month_ago, service=sample_service, template=sms) + sample_notification(notify_db, notify_db_session, created_at=one_month_ago, service=sample_service, template=sms) + + results = dao_get_template_usage(sample_service.id) + + assert len(results) == 2 + + assert [(row.name, row.template_type, row.count) for row in results] == [ + ('Email Template Name', 'email', 5), + ('Template Name', 'sms', 5) + ] + + +def test_should_by_able_to_get_template_count_from_notifications_history_with_day_limit( + notify_db, + notify_db_session, + sample_service): + sms = sample_template(notify_db, notify_db_session) + + email = sample_email_template(notify_db, notify_db_session) + + today = datetime.now() + yesterday = datetime.now() - timedelta(days=1) + one_month_ago = datetime.now() - timedelta(days=30) + + sample_notification(notify_db, notify_db_session, created_at=today, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, created_at=today, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, created_at=today, service=sample_service, template=sms) + + sample_notification(notify_db, notify_db_session, created_at=yesterday, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, created_at=yesterday, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, created_at=yesterday, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, created_at=yesterday, service=sample_service, template=sms) + + sample_notification(notify_db, notify_db_session, created_at=one_month_ago, service=sample_service, template=sms) + sample_notification(notify_db, notify_db_session, created_at=one_month_ago, service=sample_service, template=sms) + sample_notification(notify_db, notify_db_session, created_at=one_month_ago, service=sample_service, template=sms) + + results_day_one = dao_get_template_usage(sample_service.id, limit_days=0) + assert len(results_day_one) == 2 + + results_day_two = dao_get_template_usage(sample_service.id, limit_days=1) + assert len(results_day_two) == 2 + + results_day_30 = dao_get_template_usage(sample_service.id, limit_days=31) + assert len(results_day_30) == 2 + + assert [(row.name, row.template_type, row.count) for row in results_day_one] == [ + ('Email Template Name', 'email', 2), + ('Template Name', 'sms', 1) + ] + + assert [(row.name, row.template_type, row.count) for row in results_day_two] == [ + ('Email Template Name', 'email', 5), + ('Template Name', 'sms', 2), + ] + + assert [(row.name, row.template_type, row.count) for row in results_day_30] == [ + ('Email Template Name', 'email', 5), + ('Template Name', 'sms', 5), + ] + + def test_should_by_able_to_update_status_by_reference(sample_email_template, ses_provider): data = _notification_json(sample_email_template, status='sending') @@ -572,9 +707,11 @@ def test_save_notification_with_no_job(sample_template, mmg_provider): def test_get_notification(sample_notification): - notification_from_db = get_notification( + notification_from_db = get_notification_with_personalisation( sample_notification.service.id, - sample_notification.id) + sample_notification.id, + key_type=None + ) assert sample_notification == notification_from_db diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 1af238b4d..654c3b686 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -29,6 +29,7 @@ from app.models import ( VerifyCode, ApiKey, Template, + TemplateHistory, Job, Notification, NotificationHistory, @@ -331,7 +332,7 @@ def test_delete_service_and_associated_objects(notify_db, assert ApiKey.query.count() == 0 assert ApiKey.get_history_model().query.count() == 0 assert Template.query.count() == 0 - assert Template.get_history_model().query.count() == 0 + assert TemplateHistory.query.count() == 0 assert Job.query.count() == 0 assert Notification.query.count() == 0 assert Permission.query.count() == 0 diff --git a/tests/app/dao/test_templates_dao.py b/tests/app/dao/test_templates_dao.py index 82b943963..a01126445 100644 --- a/tests/app/dao/test_templates_dao.py +++ b/tests/app/dao/test_templates_dao.py @@ -6,7 +6,7 @@ from app.dao.templates_dao import ( dao_update_template, dao_get_template_versions) from tests.app.conftest import sample_template as create_sample_template -from app.models import Template +from app.models import Template, TemplateHistory import pytest @@ -185,7 +185,7 @@ def test_get_template_by_id_and_service_returns_none_if_no_template(sample_servi def test_create_template_creates_a_history_record_with_current_data(sample_service, sample_user): assert Template.query.count() == 0 - assert Template.get_history_model().query.count() == 0 + assert TemplateHistory.query.count() == 0 data = { 'name': 'Sample Template', 'template_type': "email", @@ -200,7 +200,7 @@ def test_create_template_creates_a_history_record_with_current_data(sample_servi assert Template.query.count() == 1 template_from_db = Template.query.first() - template_history = Template.get_history_model().query.first() + template_history = TemplateHistory.query.first() assert template_from_db.id == template_history.id assert template_from_db.name == template_history.name @@ -212,7 +212,7 @@ def test_create_template_creates_a_history_record_with_current_data(sample_servi def test_update_template_creates_a_history_record_with_current_data(sample_service, sample_user): assert Template.query.count() == 0 - assert Template.get_history_model().query.count() == 0 + assert TemplateHistory.query.count() == 0 data = { 'name': 'Sample Template', 'template_type': "email", @@ -228,20 +228,20 @@ def test_update_template_creates_a_history_record_with_current_data(sample_servi assert created.name == 'Sample Template' assert Template.query.count() == 1 assert Template.query.first().version == 1 - assert Template.get_history_model().query.count() == 1 + assert TemplateHistory.query.count() == 1 created.name = 'new name' dao_update_template(created) assert Template.query.count() == 1 - assert Template.get_history_model().query.count() == 2 + assert TemplateHistory.query.count() == 2 template_from_db = Template.query.first() assert template_from_db.version == 2 - assert Template.get_history_model().query.filter_by(name='Sample Template').one().version == 1 - assert Template.get_history_model().query.filter_by(name='new name').one().version == 2 + assert TemplateHistory.query.filter_by(name='Sample Template').one().version == 1 + assert TemplateHistory.query.filter_by(name='new name').one().version == 2 def test_get_template_history_version(sample_user, sample_service, sample_template): @@ -261,12 +261,16 @@ def test_get_template_versions(sample_template): sample_template.content = 'new version' dao_update_template(sample_template) versions = dao_get_template_versions(service_id=sample_template.service_id, template_id=sample_template.id) - assert versions.__len__() == 2 - for x in versions: - if x.version == 2: - assert x.content == 'new version' - else: - assert x.content == original_content + assert len(versions) == 2 + versions = sorted(versions, key=lambda x: x.version) + assert versions[0].content == original_content + assert versions[1].content == 'new version' + + assert versions[0].created_at == versions[1].created_at + + assert versions[0].updated_at is None + assert versions[1].updated_at is not None + from app.schemas import template_history_schema v = template_history_schema.load(versions, many=True) - assert v.__len__() == 2 + assert len(v) == 2 diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index cbbaf5ca2..df611adca 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -8,6 +8,7 @@ from freezegun import freeze_time from app.dao.notifications_dao import dao_update_notification from app.dao.api_key_dao import save_model_api_key +from app.dao.templates_dao import dao_update_template from app.models import ApiKey, KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST from tests import create_authorization_header from tests.app.conftest import sample_notification as create_sample_notification @@ -29,7 +30,9 @@ def test_get_sms_notification_by_id(notify_api, sample_notification): assert notification['template'] == { 'id': str(sample_notification.template.id), 'name': sample_notification.template.name, - 'template_type': sample_notification.template.template_type} + 'template_type': sample_notification.template.template_type, + 'version': 1 + } assert notification['job'] == { 'id': str(sample_notification.job.id), 'original_file_name': sample_notification.job.original_file_name @@ -62,7 +65,9 @@ def test_get_email_notification_by_id(notify_api, notify_db, notify_db_session, assert notification['template'] == { 'id': str(email_notification.template.id), 'name': email_notification.template.name, - 'template_type': email_notification.template.template_type} + 'template_type': email_notification.template.template_type, + 'version': 1 + } assert notification['job'] == { 'id': str(email_notification.job.id), 'original_file_name': email_notification.job.original_file_name @@ -166,7 +171,9 @@ def test_get_all_notifications(notify_api, sample_notification): assert notifications['notifications'][0]['template'] == { 'id': str(sample_notification.template.id), 'name': sample_notification.template.name, - 'template_type': sample_notification.template.template_type} + 'template_type': sample_notification.template.template_type, + 'version': 1 + } assert notifications['notifications'][0]['job'] == { 'id': str(sample_notification.job.id), 'original_file_name': sample_notification.job.original_file_name @@ -656,7 +663,6 @@ def test_get_notification_public_api_format_is_not_changed(notify_api, sample_no } -@pytest.mark.xfail(strict=True, raises=NeededByTemplateError) def test_get_notification_selects_correct_template_for_personalisation(notify_api, notify_db, notify_db_session, @@ -666,8 +672,9 @@ def test_get_notification_selects_correct_template_for_personalisation(notify_ap notify_db_session, service=sample_template.service, template=sample_template) - + original_content = sample_template.content sample_template.content = '((name))' + dao_update_template(sample_template) notify_db.session.commit() create_sample_notification(notify_db, @@ -680,14 +687,19 @@ def test_get_notification_selects_correct_template_for_personalisation(notify_ap auth_header = create_authorization_header(service_id=sample_template.service_id) response = client.get(path='/notifications', headers=[auth_header]) + assert response.status_code == 200 resp = json.loads(response.get_data(as_text=True)) - assert len(resp['notifications']) == 2 - assert resp['notifications'][0]['template_version'] == 1 - assert resp['notifications'][0]['body'] == 'This is a template' - assert resp['notifications'][1]['template_version'] == 2 - assert resp['notifications'][1]['body'] == 'foo' + notis = sorted(resp['notifications'], key=lambda x: x['template_version']) + assert len(notis) == 2 + assert notis[0]['template_version'] == 1 + assert notis[0]['body'] == original_content + assert notis[1]['template_version'] == 2 + assert notis[1]['body'] == 'foo' + + assert notis[0]['template_version'] == notis[0]['template']['version'] + assert notis[1]['template_version'] == notis[1]['template']['version'] def _create_auth_header_from_key(api_key): diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 74ae55f5e..1d3612b01 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -7,7 +7,7 @@ from freezegun import freeze_time from app.dao.users_dao import save_model_user from app.dao.services_dao import dao_remove_user_from_service -from app.models import User +from app.models import User, Organisation from tests import create_authorization_header from tests.app.conftest import ( sample_service as create_sample_service, @@ -108,6 +108,8 @@ def test_get_service_by_id(notify_api, sample_service): assert json_resp['data']['name'] == sample_service.name assert json_resp['data']['id'] == str(sample_service.id) assert not json_resp['data']['research_mode'] + assert json_resp['data']['organisation'] is None + assert json_resp['data']['branding'] == 'govuk' def test_get_service_by_id_should_404_if_no_service(notify_api, notify_db): @@ -339,7 +341,11 @@ def test_create_service_should_throw_duplicate_key_constraint_for_existing_email assert "Duplicate service name '{}'".format(service_name) in json_resp['message']['name'] -def test_update_service(notify_api, sample_service): +def test_update_service(notify_api, notify_db, sample_service): + org = Organisation(colour='#000000', logo='justice-league.png', name='Justice League') + notify_db.session.add(org) + notify_db.session.commit() + with notify_api.test_request_context(): with notify_api.test_client() as client: auth_header = create_authorization_header() @@ -354,7 +360,8 @@ def test_update_service(notify_api, sample_service): data = { 'name': 'updated service name', 'email_from': 'updated.service.name', - 'created_by': str(sample_service.created_by.id) + 'created_by': str(sample_service.created_by.id), + 'organisation': str(org.id) } auth_header = create_authorization_header() @@ -368,6 +375,7 @@ def test_update_service(notify_api, sample_service): assert resp.status_code == 200 assert result['data']['name'] == 'updated service name' assert result['data']['email_from'] == 'updated.service.name' + assert result['data']['organisation'] == str(org.id) def test_update_service_research_mode(notify_api, sample_service): diff --git a/tests/app/template_statistics/test_rest.py b/tests/app/template_statistics/test_rest.py index 4d7b3384a..1544874f7 100644 --- a/tests/app/template_statistics/test_rest.py +++ b/tests/app/template_statistics/test_rest.py @@ -1,4 +1,4 @@ -from datetime import datetime +from datetime import datetime, timedelta import json from freezegun import freeze_time @@ -6,124 +6,18 @@ from freezegun import freeze_time from app import db from app.models import TemplateStatistics from tests import create_authorization_header -from tests.app.conftest import sample_template as create_sample_template +from tests.app.conftest import sample_template as create_sample_template, sample_template, sample_notification, \ + sample_email_template -@freeze_time('2016-04-09') -def test_get_template_statistics_for_service_for_last_week(notify_api, sample_template): - - # make 9 stats records from 1st to 9th April - for i in range(1, 10): - past_date = '2016-04-0{}'.format(i) - with freeze_time(past_date): - template_stats = TemplateStatistics(template_id=sample_template.id, - service_id=sample_template.service_id) - db.session.add(template_stats) - db.session.commit() - +def test_get_all_template_statistics_with_bad_arg_returns_400(notify_api, sample_service): with notify_api.test_request_context(): with notify_api.test_client() as client: auth_header = create_authorization_header() response = client.get( - '/service/{}/template-statistics'.format(sample_template.service_id), - headers=[('Content-Type', 'application/json'), auth_header], - query_string={'limit_days': 7} - ) - - assert response.status_code == 200 - json_resp = json.loads(response.get_data(as_text=True)) - assert len(json_resp['data']) == 8 - assert json_resp['data'][0]['day'] == '2016-04-09' - assert json_resp['data'][6]['day'] == '2016-04-03' - - -@freeze_time('2016-04-30') -def test_get_template_statistics_for_service_for_last_week_with_no_data(notify_api, sample_template): - - # make 9 stats records from 1st to 9th April - for i in range(1, 10): - past_date = '2016-04-0{}'.format(i) - with freeze_time(past_date): - template_stats = TemplateStatistics(template_id=sample_template.id, - service_id=sample_template.service_id) - db.session.add(template_stats) - db.session.commit() - - with notify_api.test_request_context(): - with notify_api.test_client() as client: - - auth_header = create_authorization_header() - - # Date is frozen at 2016-04-30 and no data written since - response = client.get( - '/service/{}/template-statistics'.format(sample_template.service_id), - headers=[('Content-Type', 'application/json'), auth_header], - query_string={'limit_days': 7} - ) - - assert response.status_code == 200 - json_resp = json.loads(response.get_data(as_text=True)) - assert len(json_resp['data']) == 0 - - response = client.get( - '/service/{}/template-statistics'.format(sample_template.service_id), - headers=[('Content-Type', 'application/json'), auth_header], - query_string={'limit_days': 30} - ) - - assert response.status_code == 200 - json_resp = json.loads(response.get_data(as_text=True)) - assert len(json_resp['data']) == 9 - - -def test_get_all_template_statistics_for_service(notify_api, sample_template): - - # make 9 stats records from 1st to 9th April - for i in range(1, 10): - past_date = '2016-04-0{}'.format(i) - with freeze_time(past_date): - template_stats = TemplateStatistics(template_id=sample_template.id, - service_id=sample_template.service_id) - db.session.add(template_stats) - db.session.commit() - - with notify_api.test_request_context(): - with notify_api.test_client() as client: - - auth_header = create_authorization_header() - - response = client.get( - '/service/{}/template-statistics'.format(sample_template.service_id), - headers=[('Content-Type', 'application/json'), auth_header] - ) - - assert response.status_code == 200 - json_resp = json.loads(response.get_data(as_text=True)) - assert len(json_resp['data']) == 9 - assert json_resp['data'][0]['day'] == '2016-04-09' - assert json_resp['data'][8]['day'] == '2016-04-01' - - -def test_get_all_template_statistics_with_bad_limit_arg_returns_400(notify_api, sample_template): - - # make 9 stats records from 1st to 9th April - for i in range(1, 10): - past_date = '2016-04-0{}'.format(i) - with freeze_time(past_date): - template_stats = TemplateStatistics(template_id=sample_template.id, - service_id=sample_template.service_id) - db.session.add(template_stats) - db.session.commit() - - with notify_api.test_request_context(): - with notify_api.test_client() as client: - - auth_header = create_authorization_header() - - response = client.get( - '/service/{}/template-statistics'.format(sample_template.service_id), + '/service/{}/template-statistics'.format(sample_service.id), headers=[('Content-Type', 'application/json'), auth_header], query_string={'limit_days': 'blurk'} ) @@ -134,6 +28,135 @@ def test_get_all_template_statistics_with_bad_limit_arg_returns_400(notify_api, assert json_resp['message'] == {'limit_days': ['blurk is not an integer']} +@freeze_time('2016-08-18') +def test_get_template_statistics_for_service(notify_db, notify_db_session, notify_api, sample_service): + sms = sample_template(notify_db, notify_db_session, service=sample_service) + email = sample_email_template(notify_db, notify_db_session, service=sample_service) + today = datetime.now() + sample_notification(notify_db, notify_db_session, created_at=today, service=sample_service, template=sms) + sample_notification(notify_db, notify_db_session, created_at=today, service=sample_service, template=sms) + sample_notification(notify_db, notify_db_session, created_at=today, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, created_at=today, service=sample_service, template=email) + + with notify_api.test_request_context(): + with notify_api.test_client() as client: + + auth_header = create_authorization_header() + + response = client.get( + '/service/{}/template-statistics'.format(sample_service.id), + headers=[('Content-Type', 'application/json'), auth_header] + ) + + assert response.status_code == 200 + json_resp = json.loads(response.get_data(as_text=True)) + assert len(json_resp['data']) == 2 + assert json_resp['data'][0]['count'] == 2 + assert json_resp['data'][0]['template_id'] == str(email.id) + assert json_resp['data'][0]['template_name'] == email.name + assert json_resp['data'][0]['template_type'] == email.template_type + assert json_resp['data'][1]['count'] == 2 + assert json_resp['data'][1]['template_id'] == str(sms.id) + assert json_resp['data'][1]['template_name'] == sms.name + assert json_resp['data'][1]['template_type'] == sms.template_type + + +@freeze_time('2016-08-18') +def test_get_template_statistics_for_service_limited_by_day(notify_db, notify_db_session, notify_api, sample_service): + sms = sample_template(notify_db, notify_db_session, service=sample_service) + email = sample_email_template(notify_db, notify_db_session, service=sample_service) + today = datetime.now() + a_week_ago = datetime.now() - timedelta(days=7) + a_month_ago = datetime.now() - timedelta(days=30) + sample_notification(notify_db, notify_db_session, created_at=today, service=sample_service, template=sms) + sample_notification(notify_db, notify_db_session, created_at=today, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, created_at=a_week_ago, service=sample_service, template=sms) + sample_notification(notify_db, notify_db_session, created_at=a_week_ago, service=sample_service, template=email) + sample_notification(notify_db, notify_db_session, created_at=a_month_ago, service=sample_service, template=sms) + sample_notification(notify_db, notify_db_session, created_at=a_month_ago, service=sample_service, template=email) + + with notify_api.test_request_context(): + with notify_api.test_client() as client: + + auth_header = create_authorization_header() + + response = client.get( + '/service/{}/template-statistics'.format(sample_service.id), + headers=[('Content-Type', 'application/json'), auth_header], + query_string={'limit_days': 1} + ) + + assert response.status_code == 200 + json_resp = json.loads(response.get_data(as_text=True)) + assert len(json_resp['data']) == 2 + assert json_resp['data'][0]['count'] == 1 + assert json_resp['data'][0]['template_id'] == str(email.id) + assert json_resp['data'][0]['template_name'] == email.name + assert json_resp['data'][0]['template_type'] == email.template_type + assert json_resp['data'][1]['count'] == 1 + assert json_resp['data'][1]['template_id'] == str(sms.id) + assert json_resp['data'][1]['template_name'] == sms.name + assert json_resp['data'][1]['template_type'] == sms.template_type + + response_for_a_week = client.get( + '/service/{}/template-statistics'.format(sample_service.id), + headers=[('Content-Type', 'application/json'), auth_header], + query_string={'limit_days': 7} + ) + + assert response.status_code == 200 + json_resp = json.loads(response_for_a_week.get_data(as_text=True)) + assert len(json_resp['data']) == 2 + assert json_resp['data'][0]['count'] == 2 + assert json_resp['data'][0]['template_name'] == 'Email Template Name' + assert json_resp['data'][1]['count'] == 2 + assert json_resp['data'][1]['template_name'] == 'Template Name' + + response_for_a_month = client.get( + '/service/{}/template-statistics'.format(sample_service.id), + headers=[('Content-Type', 'application/json'), auth_header], + query_string={'limit_days': 30} + ) + + assert response_for_a_month.status_code == 200 + json_resp = json.loads(response_for_a_month.get_data(as_text=True)) + assert len(json_resp['data']) == 2 + assert json_resp['data'][0]['count'] == 3 + assert json_resp['data'][0]['template_name'] == 'Email Template Name' + assert json_resp['data'][1]['count'] == 3 + assert json_resp['data'][1]['template_name'] == 'Template Name' + + response_for_all = client.get( + '/service/{}/template-statistics'.format(sample_service.id), + headers=[('Content-Type', 'application/json'), auth_header] + ) + + assert response_for_all.status_code == 200 + json_resp = json.loads(response_for_all.get_data(as_text=True)) + assert len(json_resp['data']) == 2 + assert json_resp['data'][0]['count'] == 3 + assert json_resp['data'][0]['template_name'] == 'Email Template Name' + assert json_resp['data'][1]['count'] == 3 + assert json_resp['data'][1]['template_name'] == 'Template Name' + + +@freeze_time('2016-08-18') +def test_returns_empty_list_if_no_templates_used(notify_api, sample_service): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + + auth_header = create_authorization_header() + + response = client.get( + '/service/{}/template-statistics'.format(sample_service.id), + headers=[('Content-Type', 'application/json'), auth_header] + ) + + assert response.status_code == 200 + json_resp = json.loads(response.get_data(as_text=True)) + assert len(json_resp['data']) == 0 + + def test_get_template_statistics_for_template_only_returns_for_provided_template( notify_db, notify_db_session,