diff --git a/Makefile b/Makefile index d0a559275..6884c678b 100644 --- a/Makefile +++ b/Makefile @@ -70,9 +70,17 @@ build-codedeploy-artifact: ## Build the deploy artifact for CodeDeploy mkdir -p target zip -r -x@deploy-exclude.lst target/notifications-api.zip * + rm -rf build/db-migration-codedeploy + mkdir -p build/db-migration-codedeploy + unzip target/notifications-api.zip -d build/db-migration-codedeploy + cd build/db-migration-codedeploy && \ + mv -f appspec-db-migration.yml appspec.yml && \ + zip -r -x@deploy-exclude.lst ../../target/notifications-api-db-migration.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 + aws s3 cp --region eu-west-1 target/notifications-api-db-migration.zip s3://${DNS_NAME}-codedeploy/notifications-api-db-migration-${DEPLOY_BUILD_NUMBER}.zip .PHONY: test test: venv generate-version-file ## Run tests @@ -82,6 +90,10 @@ test: venv generate-version-file ## Run tests 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-api +deploy-api-db-migration: check-env-vars ## Trigger CodeDeploy for the api db migration + aws deploy create-deployment --application-name notify-api-db-migration --deployment-config-name CodeDeployDefault.OneAtATime --deployment-group-name notify-api-db-migration --s3-location bucket=${DNS_NAME}-codedeploy,key=notifications-api-db-migration-${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 @@ -155,4 +167,4 @@ 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 clean: - rm -rf node_modules cache target venv .coverage + rm -rf node_modules cache target venv .coverage build tests/.cache diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 148c7e1ed..fa65e8a72 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -21,7 +21,7 @@ from app.models import ( NOTIFICATION_CREATED, NOTIFICATION_SENDING, NOTIFICATION_PENDING, - NOTIFICATION_TEMPORARY_FAILURE) + NOTIFICATION_TEMPORARY_FAILURE, KEY_TYPE_NORMAL, KEY_TYPE_TEST) from app.dao.dao_utils import transactional from app.statsd_decorators import statsd @@ -97,7 +97,7 @@ def dao_get_template_usage(service_id, limit_days=None): Template.template_type ) - query_filter = [table.service_id == service_id] + query_filter = [table.service_id == service_id, table.key_type != KEY_TYPE_TEST] if limit_days is not None: query_filter.append(table.created_at >= days_ago(limit_days)) @@ -110,7 +110,9 @@ def dao_get_template_usage(service_id, limit_days=None): @statsd(namespace="dao") def dao_get_last_template_usage(template_id): - return NotificationHistory.query.filter(NotificationHistory.template_id == template_id) \ + return NotificationHistory.query.filter( + NotificationHistory.template_id == template_id, + NotificationHistory.key_type != KEY_TYPE_TEST) \ .join(Template) \ .order_by(desc(NotificationHistory.created_at)) \ .first() @@ -232,17 +234,24 @@ def get_notifications_for_service(service_id, page_size=None, limit_days=None, key_type=None, - personalisation=False): + personalisation=False, + include_jobs=False): if page_size is None: page_size = current_app.config['PAGE_SIZE'] + filters = [Notification.service_id == service_id] if limit_days is not None: days_ago = date.today() - timedelta(days=limit_days) filters.append(func.date(Notification.created_at) >= days_ago) + if not include_jobs or (key_type and key_type != KEY_TYPE_NORMAL): + filters.append(Notification.job_id.is_(None)) + if key_type is not None: filters.append(Notification.key_type == key_type) + else: + filters.append(Notification.key_type != KEY_TYPE_TEST) query = Notification.query.filter(*filters) query = _filter_query(query, filter_dict) @@ -250,6 +259,7 @@ def get_notifications_for_service(service_id, 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 031c10939..8e9e8ceb6 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -23,8 +23,8 @@ from app.models import ( Permission, User, InvitedUser, - Service -) + Service, + KEY_TYPE_TEST) from app.statsd_decorators import statsd @@ -156,7 +156,8 @@ def _stats_for_service_query(service_id): Notification.status, func.count(Notification.id).label('count') ).filter( - Notification.service_id == service_id + Notification.service_id == service_id, + Notification.key_type != KEY_TYPE_TEST ).group_by( Notification.notification_type, Notification.status, diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 982e3128d..d47f9f27c 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -168,6 +168,7 @@ def get_notification_by_id(notification_id): @notifications.route('/notifications', methods=['GET']) def get_all_notifications(): data = notifications_filter_schema.load(request.args).data + include_jobs = data.get('include_jobs', False) page = data['page'] if 'page' in data else 1 page_size = data['page_size'] if 'page_size' in data else current_app.config.get('PAGE_SIZE') limit_days = data.get('limit_days') @@ -179,7 +180,8 @@ def get_all_notifications(): page=page, page_size=page_size, limit_days=limit_days, - key_type=api_user.key_type) + key_type=api_user.key_type, + include_jobs=include_jobs) return jsonify( notifications=notification_with_personalisation_schema.dump(pagination.items, many=True).data, page_size=page_size, diff --git a/app/schemas.py b/app/schemas.py index 673983114..ccabfc9d6 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -421,6 +421,7 @@ class NotificationsFilterSchema(ma.Schema): page = fields.Int(required=False) page_size = fields.Int(required=False) limit_days = fields.Int(required=False) + include_jobs = fields.Boolean(required=False) @pre_load def handle_multidict(self, in_data): diff --git a/app/service/rest.py b/app/service/rest.py index 5a80cdc76..a6adebdd2 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -219,7 +219,8 @@ def get_all_notifications_for_service(service_id): filter_dict=data, page=page, page_size=page_size, - limit_days=limit_days) + limit_days=limit_days, + include_jobs=True) kwargs = request.args.to_dict() kwargs['service_id'] = service_id return jsonify( diff --git a/appspec-db-migration.yml b/appspec-db-migration.yml new file mode 100644 index 000000000..b840f4432 --- /dev/null +++ b/appspec-db-migration.yml @@ -0,0 +1,18 @@ +--- +os: linux +version: 0.0 +files: + - destination: /home/notify-app/notifications-api + source: / +hooks: + AfterInstall: + - location: scripts/aws_install_dependencies.sh + runas: root + timeout: 1000 + - location: scripts/aws_change_ownership.sh + runas: root + timeout: 300 + ApplicationStart: + - location: scripts/aws_run_db_migrations.sh + runas: root + timeout: 300 diff --git a/appspec.yml b/appspec.yml index 77f7ed385..a6455d55c 100644 --- a/appspec.yml +++ b/appspec.yml @@ -1,35 +1,28 @@ --- +os: linux +version: 0.0 files: - - - destination: /home/notify-app/notifications-api + - destination: /home/notify-app/notifications-api source: / hooks: AfterInstall: - - - location: scripts/aws_install_dependencies.sh + - location: scripts/aws_install_dependencies.sh runas: root timeout: 1000 - - - location: scripts/aws_change_ownership.sh + - location: scripts/aws_change_ownership.sh runas: root timeout: 300 ApplicationStart: - - - location: scripts/aws_start_app.sh + - location: scripts/aws_start_app.sh runas: root timeout: 300 - - - location: scripts/register_with_elb.sh + - location: scripts/register_with_elb.sh runas: ubuntu timeout: 300 ApplicationStop: - - - location: scripts/deregister_from_elb.sh + - location: scripts/deregister_from_elb.sh runas: ubuntu timeout: 300 - - - location: scripts/aws_stop_app.sh + - location: scripts/aws_stop_app.sh runas: root timeout: 300 -os: linux -version: 0.0 diff --git a/deploy-exclude.lst b/deploy-exclude.lst index 5eaf61748..aea0bc983 100644 --- a/deploy-exclude.lst +++ b/deploy-exclude.lst @@ -7,3 +7,6 @@ *node_modules* *target* *venv* +*build* +*.envrc* +*tests/.cache* diff --git a/notifications-api.zip b/notifications-api.zip deleted file mode 100644 index 83298c812..000000000 Binary files a/notifications-api.zip and /dev/null differ diff --git a/scripts/aws_change_ownership.sh b/scripts/aws_change_ownership.sh index 09d3d80cf..ad0d438a7 100755 --- a/scripts/aws_change_ownership.sh +++ b/scripts/aws_change_ownership.sh @@ -1,13 +1,8 @@ #!/bin/bash +set -eo pipefail -if [ -e "/home/notify-app" ] -then - echo "Chown application to be owned by notify-app" - cd /home/notify-app/; - chown -R notify-app:govuk-notify-applications notifications-api -else - echo "Chown application to be owned by ubuntu" - cd /home/ubuntu/; - chown -R ubuntu:ubuntu notifications-api -fi \ No newline at end of file +echo "Chown application to be owned by notify-app" + +cd /home/notify-app/; +chown -R notify-app:govuk-notify-applications notifications-api diff --git a/scripts/aws_install_dependencies.sh b/scripts/aws_install_dependencies.sh index f7cb975f6..b82881d45 100755 --- a/scripts/aws_install_dependencies.sh +++ b/scripts/aws_install_dependencies.sh @@ -1,15 +1,8 @@ +#!/bin/bash + +set -eo pipefail + echo "Install dependencies" - -if [ -e "/home/notify-app" ] -then - echo "Depenencies for notify-app" - cd /home/notify-app/notifications-api; - pip3 install -r /home/notify-app/notifications-api/requirements.txt - python3 db.py db upgrade -else - echo "Depenencies for ubuntu" - cd /home/ubuntu/notifications-api; - pip3 install -r /home/ubuntu/notifications-api/requirements.txt - python3 db.py db upgrade -fi \ No newline at end of file +cd /home/notify-app/notifications-api; +pip3 install -r /home/notify-app/notifications-api/requirements.txt diff --git a/scripts/aws_run_db_migrations.sh b/scripts/aws_run_db_migrations.sh new file mode 100755 index 000000000..9667839d5 --- /dev/null +++ b/scripts/aws_run_db_migrations.sh @@ -0,0 +1,8 @@ +#!/usr/bin/env bash + +set -eo pipefail + +echo "Run database migrations" + +cd /home/notify-app/notifications-api; +python3 db.py db upgrade diff --git a/scripts/aws_start_app.sh b/scripts/aws_start_app.sh index 47f7103ba..9e871ebc2 100755 --- a/scripts/aws_start_app.sh +++ b/scripts/aws_start_app.sh @@ -1,31 +1,19 @@ -#!/bin/bash +#!/usr/bin/env bash -if [ -e "/etc/init/notifications-api.conf" ] -then - echo "Starting api" - sudo service notifications-api start -fi +set -eo pipefail -if [ -e "/etc/init/notifications-api-celery-worker.conf" ] -then - echo "Starting celery worker" - sudo service notifications-api-celery-worker start -fi +function start +{ + service=$1 + if [ -e "/etc/init/${service}.conf" ] + then + echo "Starting ${service}" + service ${service} start + fi +} -if [ -e "/etc/init/notifications-api-celery-worker-sender.conf" ] -then - echo "Starting celery worker" - sudo service notifications-api-celery-worker-sender start -fi - -if [ -e "/etc/init/notifications-api-celery-worker-db.conf" ] -then - echo "Starting celery worker" - sudo service notifications-api-celery-worker-db start -fi - -if [ -e "/etc/init/notifications-api-celery-beat.conf" ] -then - echo "Starting celery beat" - sudo service notifications-api-celery-beat start -fi +start "notifications-api" +start "notifications-api-celery-worker" +start "notifications-api-celery-worker-sender" +start "notifications-api-celery-worker-db" +start "notifications-api-celery-beat" diff --git a/scripts/aws_stop_app.sh b/scripts/aws_stop_app.sh index 3f9735d4b..f3c3a441a 100755 --- a/scripts/aws_stop_app.sh +++ b/scripts/aws_stop_app.sh @@ -1,53 +1,22 @@ -#!/bin/bash +#!/usr/bin/env bash +set -eo pipefail -function error_exit +function stop { - echo "$1" 1>&2 - exit 0 + service=$1 + if [ -e "/etc/init/${service}.conf" ]; then + echo "stopping ${service}" + if service ${service} stop; then + echo "${service} stopped" + else + >&2 echo "Could not stop ${service}" + fi + fi } -if [ -e "/etc/init/notifications-api.conf" ]; then - echo "stopping notifications-api" - if sudo service notifications-api stop; then - echo "notifications-api stopped" - else - error_exit "Could not stop notifications-api" - fi -fi - -if [ -e "/etc/init/notifications-api-celery-beat.conf" ]; then - echo "stopping notifications-api-celery-beat" - if sudo service notifications-api-celery-beat stop; then - echo "notifications-api beat stopped" - else - error_exit "Could not stop notifications-celery-beat" - fi -fi - -if [ -e "/etc/init/notifications-api-celery-worker.conf" ]; then - echo "stopping notifications-api-celery-worker" - if sudo service notifications-api-celery-worker stop; then - echo "notifications-api worker stopped" - else - error_exit "Could not stop notifications-celery-worker" - fi -fi - -if [ -e "/etc/init/notifications-api-celery-worker-sender.conf" ]; then - echo "stopping notifications-api-celery-worker-sender" - if sudo service notifications-api-celery-worker-sender stop; then - echo "notifications-api sender worker stopped" - else - error_exit "Could not stop notifications-celery-worker-sender" - fi -fi - -if [ -e "/etc/init/notifications-api-celery-worker-db.conf" ]; then - echo "stopping notifications-api-celery-worker-db" - if sudo service notifications-api-celery-worker-db stop; then - echo "notifications-api db worker stopped" - else - error_exit "Could not stop notifications-celery-worker-db" - fi -fi +stop "notifications-api" +stop "notifications-api-celery-beat" +stop "notifications-api-celery-worker" +stop "notifications-api-celery-worker-sender" +stop "notifications-api-celery-worker-db" diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 1346d1dca..d16adcc87 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -19,7 +19,7 @@ from app.models import ( ProviderStatistics, ProviderDetails, NotificationStatistics, - KEY_TYPE_NORMAL) + KEY_TYPE_NORMAL, KEY_TYPE_TEST, KEY_TYPE_TEAM) from app.dao.users_dao import (save_model_user, create_user_code, create_secret_code) from app.dao.services_dao import (dao_create_service, dao_add_user_to_service) from app.dao.templates_dao import dao_create_template @@ -241,6 +241,16 @@ def sample_api_key(notify_db, return api_key +@pytest.fixture(scope='function') +def sample_test_api_key(notify_db, notify_db_session, service=None): + return sample_api_key(notify_db, notify_db_session, service, KEY_TYPE_TEST) + + +@pytest.fixture(scope='function') +def sample_team_api_key(notify_db, notify_db_session, service=None): + return sample_api_key(notify_db, notify_db_session, service, KEY_TYPE_TEAM) + + @pytest.fixture(scope='function') def sample_job(notify_db, notify_db_session, @@ -331,6 +341,48 @@ def sample_email_job(notify_db, return job +@pytest.fixture(scope='function') +def sample_notification_with_job( + notify_db, + notify_db_session, + service=None, + template=None, + job=None, + job_row_number=None, + to_field=None, + status='created', + reference=None, + created_at=None, + sent_at=None, + billable_units=1, + create=True, + personalisation=None, + api_key_id=None, + key_type=KEY_TYPE_NORMAL +): + if job is None: + job = sample_job(notify_db, notify_db_session, service=service, template=template) + + return sample_notification( + notify_db, + notify_db_session, + service, + template, + job=job, + job_row_number=job_row_number if job_row_number else None, + to_field=to_field, + status=status, + reference=reference, + created_at=created_at, + sent_at=sent_at, + billable_units=billable_units, + create=create, + personalisation=personalisation, + api_key_id=api_key_id, + key_type=key_type + ) + + @pytest.fixture(scope='function') def sample_notification(notify_db, notify_db_session, @@ -354,8 +406,6 @@ def sample_notification(notify_db, service = sample_service(notify_db, notify_db_session) if template is None: template = sample_template(notify_db, notify_db_session, service=service) - if job is None: - job = sample_job(notify_db, notify_db_session, service=service, template=template) notification_id = uuid.uuid4() @@ -367,7 +417,7 @@ def sample_notification(notify_db, data = { 'id': notification_id, 'to': to, - 'job_id': job.id, + 'job_id': job.id if job else None, 'job': job, 'service_id': service.id, 'service': service, diff --git a/tests/app/dao/test_notification_dao.py b/tests/app/dao/test_notification_dao.py index 16a743699..39c91912c 100644 --- a/tests/app/dao/test_notification_dao.py +++ b/tests/app/dao/test_notification_dao.py @@ -16,7 +16,9 @@ from app.models import ( NotificationStatistics, TemplateStatistics, NOTIFICATION_STATUS_TYPES, - KEY_TYPE_NORMAL + KEY_TYPE_NORMAL, + KEY_TYPE_TEAM, + KEY_TYPE_TEST ) from app.dao.notifications_dao import ( @@ -39,7 +41,8 @@ from app.dao.notifications_dao import ( from notifications_utils.template import get_sms_fragment_count -from tests.app.conftest import (sample_notification, sample_template, sample_email_template, sample_service) +from tests.app.conftest import (sample_notification, sample_template, sample_email_template, sample_service, sample_job, + sample_api_key) def test_should_have_decorated_notifications_dao_functions(): @@ -86,6 +89,36 @@ def test_should_be_able_to_get_all_template_usage_history_order_by_notification_ assert results.id == most_recent.id +def test_template_usage_should_ignore_test_keys( + notify_db, + notify_db_session, + sample_team_api_key, + sample_test_api_key +): + sms = sample_template(notify_db, notify_db_session) + + one_minute_ago = datetime.utcnow() - timedelta(minutes=1) + two_minutes_ago = datetime.utcnow() - timedelta(minutes=2) + + team_key = sample_notification( + notify_db, + notify_db_session, + created_at=two_minutes_ago, + template=sms, + api_key_id=sample_team_api_key.id, + key_type=KEY_TYPE_TEAM) + sample_notification( + notify_db, + notify_db_session, + created_at=one_minute_ago, + template=sms, + api_key_id=sample_test_api_key.id, + key_type=KEY_TYPE_TEST) + + results = dao_get_last_template_usage(sms.id) + assert results.id == team_key.id + + def test_should_be_able_to_get_no_template_usage_history_if_no_notifications_using_template( notify_db, notify_db_session): @@ -114,6 +147,30 @@ def test_should_by_able_to_get_template_count_from_notifications_history(notify_ assert results[1].count == 3 +def test_template_history_should_ignore_test_keys( + notify_db, + notify_db_session, + sample_team_api_key, + sample_test_api_key, + sample_api_key +): + sms = sample_template(notify_db, notify_db_session) + + sample_notification( + notify_db, notify_db_session, template=sms, api_key_id=sample_api_key.id, key_type=KEY_TYPE_NORMAL) + sample_notification( + notify_db, notify_db_session, template=sms, api_key_id=sample_team_api_key.id, key_type=KEY_TYPE_TEAM) + sample_notification( + notify_db, notify_db_session, template=sms, api_key_id=sample_test_api_key.id, key_type=KEY_TYPE_TEST) + sample_notification( + notify_db, notify_db_session, template=sms) + + results = dao_get_template_usage(sms.service_id) + assert results[0].name == 'Template Name' + assert results[0].template_type == 'sms' + assert results[0].count == 3 + + def test_should_by_able_to_get_template_count_from_notifications_history_for_service( notify_db, notify_db_session): @@ -259,8 +316,8 @@ def test_should_by_able_to_update_status_by_id(sample_template, sample_job, mmg_ def test_should_not_update_status_by_id_if_not_sending_and_does_not_update_job(notify_db, notify_db_session): - notification = sample_notification(notify_db, notify_db_session, status='delivered') - job = Job.query.get(notification.job_id) + job = sample_job(notify_db, notify_db_session) + notification = sample_notification(notify_db, notify_db_session, status='delivered', job=job) assert Notification.query.get(notification.id).status == 'delivered' assert not update_notification_status_by_id(notification.id, 'failed') assert Notification.query.get(notification.id).status == 'delivered' @@ -268,8 +325,8 @@ def test_should_not_update_status_by_id_if_not_sending_and_does_not_update_job(n def test_should_not_update_status_by_reference_if_not_sending_and_does_not_update_job(notify_db, notify_db_session): - notification = sample_notification(notify_db, notify_db_session, status='delivered', reference='reference') - job = Job.query.get(notification.job_id) + job = sample_job(notify_db, notify_db_session) + notification = sample_notification(notify_db, notify_db_session, status='delivered', reference='reference', job=job) assert Notification.query.get(notification.id).status == 'delivered' assert not update_notification_status_by_reference('reference', 'failed') assert Notification.query.get(notification.id).status == 'delivered' @@ -834,7 +891,7 @@ def _notification_json(sample_template, job_id=None, id=None, status=None): return data -def test_dao_timeout_notifications(notify_db, notify_db_session,): +def test_dao_timeout_notifications(notify_db, notify_db_session, ): with freeze_time(datetime.utcnow() - timedelta(minutes=1)): created = sample_notification(notify_db, notify_db_session) sending = sample_notification(notify_db, notify_db_session, status='sending') @@ -874,3 +931,226 @@ def test_dao_timeout_notifications_only_updates_for_older_notifications(notify_d assert NotificationHistory.query.get(pending.id).status == 'pending' assert NotificationHistory.query.get(delivered.id).status == 'delivered' assert updated == 0 + + +def test_should_return_notifications_excluding_jobs_by_default(notify_db, notify_db_session, sample_service): + assert len(Notification.query.all()) == 0 + + job = sample_job(notify_db, notify_db_session) + with_job = sample_notification( + notify_db, notify_db_session, created_at=datetime.utcnow(), status="delivered", job=job + ) + without_job = sample_notification( + notify_db, notify_db_session, created_at=datetime.utcnow(), status="delivered" + ) + + all_notifications = Notification.query.all() + assert len(all_notifications) == 2 + + all_notifications = get_notifications_for_service(sample_service.id).items + assert len(all_notifications) == 1 + assert all_notifications[0].id == without_job.id + + +def test_should_return_notifications_including_jobs(notify_db, notify_db_session, sample_service): + assert len(Notification.query.all()) == 0 + + job = sample_job(notify_db, notify_db_session) + with_job = sample_notification( + notify_db, notify_db_session, created_at=datetime.utcnow(), status="delivered", job=job + ) + + all_notifications = Notification.query.all() + assert len(all_notifications) == 1 + + all_notifications = get_notifications_for_service(sample_service.id).items + assert len(all_notifications) == 0 + + all_notifications = get_notifications_for_service(sample_service.id, limit_days=1, include_jobs=True).items + assert len(all_notifications) == 1 + assert all_notifications[0].id == with_job.id + + +def test_get_notifications_created_by_api_or_csv_are_returned_correctly_excluding_test_key_notifications( + notify_db, + notify_db_session, + sample_service, + sample_job, + sample_api_key, + sample_team_api_key, + sample_test_api_key +): + + sample_notification( + notify_db, notify_db_session, created_at=datetime.utcnow(), job=sample_job + ) + sample_notification( + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_api_key.id, + key_type=sample_api_key.key_type + ) + sample_notification( + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_team_api_key.id, + key_type=sample_team_api_key.key_type + ) + sample_notification( + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_test_api_key.id, + key_type=sample_test_api_key.key_type + ) + + all_notifications = Notification.query.all() + assert len(all_notifications) == 4 + + # returns all API derived notifications + all_notifications = get_notifications_for_service(sample_service.id).items + assert len(all_notifications) == 2 + + # all notifications including jobs + all_notifications = get_notifications_for_service(sample_service.id, limit_days=1, include_jobs=True).items + assert len(all_notifications) == 3 + + +def test_get_notifications_with_a_live_api_key_type( + notify_db, + notify_db_session, + sample_service, + sample_job, + sample_api_key, + sample_team_api_key, + sample_test_api_key +): + sample_notification( + notify_db, notify_db_session, created_at=datetime.utcnow(), job=sample_job + ) + sample_notification( + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_api_key.id, + key_type=sample_api_key.key_type + ) + sample_notification( + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_team_api_key.id, + key_type=sample_team_api_key.key_type + ) + sample_notification( + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_test_api_key.id, + key_type=sample_test_api_key.key_type + ) + + all_notifications = Notification.query.all() + assert len(all_notifications) == 4 + + # only those created with normal API key, no jobs + all_notifications = get_notifications_for_service(sample_service.id, limit_days=1, key_type=KEY_TYPE_NORMAL).items + assert len(all_notifications) == 1 + + # only those created with normal API key, with jobs + all_notifications = get_notifications_for_service(sample_service.id, limit_days=1, include_jobs=True, + key_type=KEY_TYPE_NORMAL).items + assert len(all_notifications) == 2 + + +def test_get_notifications_with_a_test_api_key_type( + notify_db, + notify_db_session, + sample_service, + sample_job, + sample_api_key, + sample_team_api_key, + sample_test_api_key +): + sample_notification( + notify_db, notify_db_session, created_at=datetime.utcnow(), job=sample_job + ) + sample_notification( + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_api_key.id, + key_type=sample_api_key.key_type + ) + sample_notification( + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_team_api_key.id, + key_type=sample_team_api_key.key_type + ) + sample_notification( + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_test_api_key.id, + key_type=sample_test_api_key.key_type + ) + + # only those created with test API key, no jobs + all_notifications = get_notifications_for_service(sample_service.id, limit_days=1, key_type=KEY_TYPE_TEST).items + assert len(all_notifications) == 1 + + # only those created with test API key, no jobs, even when requested + all_notifications = get_notifications_for_service(sample_service.id, limit_days=1, include_jobs=True, + key_type=KEY_TYPE_TEST).items + assert len(all_notifications) == 1 + + +def test_get_notifications_with_a_team_api_key_type( + notify_db, + notify_db_session, + sample_service, + sample_job, + sample_api_key, + sample_team_api_key, + sample_test_api_key +): + sample_notification( + notify_db, notify_db_session, created_at=datetime.utcnow(), job=sample_job + ) + sample_notification( + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_api_key.id, + key_type=sample_api_key.key_type + ) + sample_notification( + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_team_api_key.id, + key_type=sample_team_api_key.key_type + ) + sample_notification( + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_test_api_key.id, + key_type=sample_test_api_key.key_type + ) + + # only those created with team API key, no jobs + all_notifications = get_notifications_for_service(sample_service.id, limit_days=1, key_type=KEY_TYPE_TEAM).items + assert len(all_notifications) == 1 + + # only those created with team API key, no jobs, even when requested + all_notifications = get_notifications_for_service(sample_service.id, limit_days=1, include_jobs=True, + key_type=KEY_TYPE_TEAM).items + assert len(all_notifications) == 1 + + +def test_should_exclude_test_key_notifications_by_default( + notify_db, + notify_db_session, + sample_service, + sample_job, + sample_api_key, + sample_team_api_key, + sample_test_api_key +): + sample_notification( + notify_db, notify_db_session, created_at=datetime.utcnow(), job=sample_job + ) + + sample_notification( + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_api_key.id, + key_type=sample_api_key.key_type + ) + sample_notification( + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_team_api_key.id, + key_type=sample_team_api_key.key_type + ) + sample_notification( + notify_db, notify_db_session, created_at=datetime.utcnow(), api_key_id=sample_test_api_key.id, + key_type=sample_test_api_key.key_type + ) + + all_notifications = Notification.query.all() + assert len(all_notifications) == 4 + + all_notifications = get_notifications_for_service(sample_service.id, limit_days=1).items + assert len(all_notifications) == 2 + + all_notifications = get_notifications_for_service(sample_service.id, limit_days=1, include_jobs=True).items + assert len(all_notifications) == 3 + + all_notifications = get_notifications_for_service(sample_service.id, limit_days=1, key_type=KEY_TYPE_TEST).items + assert len(all_notifications) == 1 diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 654c3b686..b14df40cb 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -441,6 +441,30 @@ def test_fetch_stats_counts_correctly(notify_db, notify_db_session, sample_templ assert stats[2].count == 1 +def test_fetch_stats_counts_should_ignore_team_key( + notify_db, + notify_db_session, + sample_template, + sample_api_key, + sample_test_api_key, + sample_team_api_key +): + # two created email, one failed email, and one created sms + create_notification(notify_db, notify_db_session, api_key_id=sample_api_key.id, key_type=sample_api_key.key_type) + create_notification( + notify_db, notify_db_session, api_key_id=sample_test_api_key.id, key_type=sample_test_api_key.key_type) + create_notification( + notify_db, notify_db_session, api_key_id=sample_team_api_key.id, key_type=sample_team_api_key.key_type) + create_notification( + notify_db, notify_db_session) + + stats = dao_fetch_stats_for_service(sample_template.service_id) + assert len(stats) == 1 + assert stats[0].notification_type == 'sms' + assert stats[0].status == 'created' + assert stats[0].count == 3 + + def test_fetch_stats_for_today_only_includes_today(notify_db, notify_db_session, sample_template): # two created email, one failed email, and one created sms with freeze_time('2001-01-01T23:59:00'): diff --git a/tests/app/notifications/rest/test_callbacks.py b/tests/app/notifications/rest/test_callbacks.py index 43cfe926f..1d8cc85ca 100644 --- a/tests/app/notifications/rest/test_callbacks.py +++ b/tests/app/notifications/rest/test_callbacks.py @@ -3,6 +3,7 @@ import uuid from datetime import datetime from flask import json from freezegun import freeze_time +from unittest.mock import call import app.celery.tasks from app.dao.notifications_dao import ( diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index f4a8db289..eaa986656 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -11,7 +11,6 @@ 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 -from notifications_utils.template import NeededByTemplateError def test_get_sms_notification_by_id(notify_api, sample_notification): @@ -32,10 +31,6 @@ def test_get_sms_notification_by_id(notify_api, sample_notification): '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 - } assert notification['to'] == '+447700900855' assert notification['service'] == str(sample_notification.service_id) assert notification['body'] == "This is a template:\nwith a newline" @@ -67,11 +62,6 @@ def test_get_email_notification_by_id(notify_api, notify_db, notify_db_session, '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 - } - assert notification['to'] == '+447700900855' assert notification['service'] == str(email_notification.service_id) assert response.status_code == 200 @@ -173,10 +163,6 @@ def test_get_all_notifications(notify_api, sample_notification): '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 - } assert notifications['notifications'][0]['to'] == '+447700900855' assert notifications['notifications'][0]['service'] == str(sample_notification.service_id) @@ -272,6 +258,149 @@ def test_get_all_notifications_only_returns_notifications_of_matching_type( assert notifications[0]['id'] == str(notification_objs[key_type].id) +@pytest.mark.parametrize('key_type', [KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST]) +def test_no_api_keys_return_job_notifications_by_default( + notify_api, + notify_db, + notify_db_session, + sample_service, + sample_job, + key_type +): + with notify_api.test_request_context(), notify_api.test_client() as client: + team_api_key = ApiKey(service=sample_service, + name='team_api_key', + created_by=sample_service.created_by, + key_type=KEY_TYPE_TEAM) + save_model_api_key(team_api_key) + + normal_api_key = ApiKey(service=sample_service, + name='normal_api_key', + created_by=sample_service.created_by, + key_type=KEY_TYPE_NORMAL) + save_model_api_key(normal_api_key) + + test_api_key = ApiKey(service=sample_service, + name='test_api_key', + created_by=sample_service.created_by, + key_type=KEY_TYPE_TEST) + save_model_api_key(test_api_key) + + job_notification = create_sample_notification( + notify_db, + notify_db_session, + api_key_id=normal_api_key.id, + job=sample_job + ) + normal_notification = create_sample_notification( + notify_db, + notify_db_session, + api_key_id=normal_api_key.id, + key_type=KEY_TYPE_NORMAL + ) + team_notification = create_sample_notification( + notify_db, + notify_db_session, + api_key_id=team_api_key.id, + key_type=KEY_TYPE_TEAM + ) + test_notification = create_sample_notification( + notify_db, + notify_db_session, + api_key_id=test_api_key.id, + key_type=KEY_TYPE_TEST + ) + + notification_objs = { + KEY_TYPE_NORMAL: normal_notification, + KEY_TYPE_TEAM: team_notification, + KEY_TYPE_TEST: test_notification + } + + response = client.get( + path='/notifications', + headers=_create_auth_header_from_key(notification_objs[key_type].api_key)) + + assert response.status_code == 200 + + notifications = json.loads(response.get_data(as_text=True))['notifications'] + assert len(notifications) == 1 + assert notifications[0]['id'] == str(notification_objs[key_type].id) + + +@pytest.mark.parametrize('key_type', [ + (KEY_TYPE_NORMAL, 2), + (KEY_TYPE_TEAM, 1), + (KEY_TYPE_TEST, 1) +]) +def test_only_normal_api_keys_can_return_job_notifications( + notify_api, + notify_db, + notify_db_session, + sample_service, + sample_job, + key_type +): + with notify_api.test_request_context(), notify_api.test_client() as client: + team_api_key = ApiKey(service=sample_service, + name='team_api_key', + created_by=sample_service.created_by, + key_type=KEY_TYPE_TEAM) + save_model_api_key(team_api_key) + + normal_api_key = ApiKey(service=sample_service, + name='normal_api_key', + created_by=sample_service.created_by, + key_type=KEY_TYPE_NORMAL) + save_model_api_key(normal_api_key) + + test_api_key = ApiKey(service=sample_service, + name='test_api_key', + created_by=sample_service.created_by, + key_type=KEY_TYPE_TEST) + save_model_api_key(test_api_key) + + job_notification = create_sample_notification( + notify_db, + notify_db_session, + api_key_id=normal_api_key.id, + job=sample_job + ) + normal_notification = create_sample_notification( + notify_db, + notify_db_session, + api_key_id=normal_api_key.id, + key_type=KEY_TYPE_NORMAL + ) + team_notification = create_sample_notification( + notify_db, + notify_db_session, + api_key_id=team_api_key.id, + key_type=KEY_TYPE_TEAM + ) + test_notification = create_sample_notification( + notify_db, + notify_db_session, + api_key_id=test_api_key.id, + key_type=KEY_TYPE_TEST + ) + + notification_objs = { + KEY_TYPE_NORMAL: normal_notification, + KEY_TYPE_TEAM: team_notification, + KEY_TYPE_TEST: test_notification + } + + response = client.get( + path='/notifications?include_jobs=true', + headers=_create_auth_header_from_key(notification_objs[key_type[0]].api_key)) + + assert response.status_code == 200 + notifications = json.loads(response.get_data(as_text=True))['notifications'] + assert len(notifications) == key_type[1] + assert notifications[0]['id'] == str(notification_objs[key_type[0]].id) + + def test_get_all_notifications_newest_first(notify_api, notify_db, notify_db_session, sample_email_template): with notify_api.test_request_context(): with notify_api.test_client() as client: diff --git a/tests/app/public_contracts/schemas/GET_notifications_return.json b/tests/app/public_contracts/schemas/GET_notifications_return.json index 7939dd8f1..709cca9fe 100644 --- a/tests/app/public_contracts/schemas/GET_notifications_return.json +++ b/tests/app/public_contracts/schemas/GET_notifications_return.json @@ -14,6 +14,17 @@ }, "links": { "type": "object", + "properties" : { + "prev" : { + "type" : "string" + }, + "next" : { + "type" : "string" + }, + "last": { + "type" : "string" + } + }, "additionalProperties": false }, "page_size": {"type": "number"}, diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index ba6587a61..6355520e4 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -14,8 +14,8 @@ from tests.app.conftest import ( sample_service as create_sample_service, sample_service_permission as create_sample_service_permission, sample_user as create_sample_user, - sample_notification as create_sample_notification -) + sample_notification as create_sample_notification, + sample_notification_with_job) def test_get_service_list(notify_api, service_factory): @@ -1037,6 +1037,28 @@ def test_get_all_notifications_for_service_in_order(notify_api, notify_db, notif assert response.status_code == 200 +def test_get_all_notifications_for_service_including_ones_made_by_jobs( + notify_api, + notify_db, + notify_db_session, + sample_service): + with notify_api.test_request_context(), notify_api.test_client() as client: + with_job = sample_notification_with_job(notify_db, notify_db_session, service=sample_service) + without_job = create_sample_notification(notify_db, notify_db_session, service=sample_service) + + auth_header = create_authorization_header() + + response = client.get( + path='/service/{}/notifications'.format(sample_service.id), + headers=[auth_header]) + + resp = json.loads(response.get_data(as_text=True)) + assert len(resp['notifications']) == 2 + assert resp['notifications'][0]['to'] == with_job.to + assert resp['notifications'][1]['to'] == without_job.to + assert response.status_code == 200 + + def test_set_sms_sender_for_service(notify_api, sample_service): with notify_api.test_request_context(): with notify_api.test_client() as client: diff --git a/tests/app/test_schemas.py b/tests/app/test_schemas.py index 73d35002a..e7d037311 100644 --- a/tests/app/test_schemas.py +++ b/tests/app/test_schemas.py @@ -1,7 +1,7 @@ -def test_job_schema_doesnt_return_notifications(sample_notification): +def test_job_schema_doesnt_return_notifications(sample_notification_with_job): from app.schemas import job_schema - job = sample_notification.job + job = sample_notification_with_job.job assert job.notifications.count() == 1 data, errors = job_schema.dump(job)