diff --git a/.github/workflows/build_and_push.yml b/.github/workflows/build_and_push_performance_test.yml similarity index 82% rename from .github/workflows/build_and_push.yml rename to .github/workflows/build_and_push_performance_test.yml index 13b6067000..fde1493806 100644 --- a/.github/workflows/build_and_push.yml +++ b/.github/workflows/build_and_push_performance_test.yml @@ -1,10 +1,12 @@ -name: Build and Push Container to ECR, deploy to lambda +name: Build and Push Performance Testing Container to ECR, deploy to lambda on: workflow_dispatch: push: - branches: [master] + paths: + - 'tests-perf/**' + - 'tests_smoke/**' env: GITHUB_SHA: ${{ github.sha }} @@ -23,7 +25,9 @@ jobs: id: filter with: filters: | - api: 'app/**' + performance-test: + - 'tests-perf/**' + - 'tests_smoke/**' build-push-and-deploy: if: ${{ needs.changes.outputs.images != '[]' }} @@ -44,7 +48,7 @@ jobs: --build-arg git_sha=$GITHUB_SHA \ -t $REGISTRY/${{ matrix.image }}:$GITHUB_SHA \ -t $REGISTRY/${{ matrix.image }}:latest . \ - -f ci/Dockerfile.lambda + -f tests-perf/ops/Dockerfile - name: Configure AWS credentials id: aws-creds @@ -65,9 +69,3 @@ jobs: - name: Logout of Amazon ECR run: docker logout ${{ steps.login-ecr.outputs.registry }} - - - name: Deploy lambda - run: | - aws lambda update-function-code \ - --function-name ${{ matrix.image }} \ - --image-uri $REGISTRY/${{ matrix.image }}:latest diff --git a/.github/workflows/lambda_production.yml b/.github/workflows/lambda_production.yml new file mode 100644 index 0000000000..acde9b8f8e --- /dev/null +++ b/.github/workflows/lambda_production.yml @@ -0,0 +1,49 @@ + +name: Build and push lambda image to production + +on: + workflow_dispatch: + push: + branches: [master] + +env: + REGISTRY: ${{ secrets.PRODUCTION_API_LAMBDA_ECR_ACCOUNT }}.dkr.ecr.ca-central-1.amazonaws.com/notify + +jobs: + build-and-push: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + image: ["api-lambda"] + + steps: + - name: Checkout + uses: actions/checkout@v2 + + - name: Build container + run: | + docker build \ + --build-arg GIT_SHA=${GITHUB_SHA::7} \ + -t $REGISTRY/${{ matrix.image }}:${GITHUB_SHA::7} \ + . \ + -f ci/Dockerfile.lambda + + - name: Configure AWS credentials + id: aws-creds + uses: aws-actions/configure-aws-credentials@v1 + with: + aws-access-key-id: ${{ secrets.PRODUCTION_ECR_ACCESS_KEY_ID }} + aws-secret-access-key: ${{ secrets.PRODUCTION_ECR_SECRET_ACCESS_KEY }} + aws-region: ca-central-1 + + - name: Login to ECR + id: login-ecr + uses: aws-actions/amazon-ecr-login@v1 + + - name: Push containers to ECR + run: | + docker push $REGISTRY/${{ matrix.image }}:${GITHUB_SHA::7} + + - name: Logout of Amazon ECR + run: docker logout ${{ steps.login-ecr.outputs.registry }} diff --git a/.github/workflows/lambda_staging.yml b/.github/workflows/lambda_staging.yml new file mode 100644 index 0000000000..f56327357a --- /dev/null +++ b/.github/workflows/lambda_staging.yml @@ -0,0 +1,55 @@ + +name: Build, push, and deploy lambda image to staging + +on: + workflow_dispatch: + push: + branches: [master] + +env: + REGISTRY: ${{ secrets.STAGING_API_LAMBDA_ECR_ACCOUNT }}.dkr.ecr.ca-central-1.amazonaws.com/notify + +jobs: + build-push-and-deploy: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + image: ["api-lambda"] + + steps: + - name: Checkout + uses: actions/checkout@v2 + + - name: Build container + run: | + docker build \ + --build-arg GIT_SHA=${GITHUB_SHA::7} \ + -t $REGISTRY/${{ matrix.image }}:${GITHUB_SHA::7} \ + . \ + -f ci/Dockerfile.lambda + + - name: Configure AWS credentials + id: aws-creds + uses: aws-actions/configure-aws-credentials@v1 + with: + aws-access-key-id: ${{ secrets.STAGING_ECR_ACCESS_KEY_ID }} + aws-secret-access-key: ${{ secrets.STAGING_ECR_SECRET_ACCESS_KEY }} + aws-region: ca-central-1 + + - name: Login to ECR + id: login-ecr + uses: aws-actions/amazon-ecr-login@v1 + + - name: Push containers to ECR + run: | + docker push $REGISTRY/${{ matrix.image }}:${GITHUB_SHA::7} + + - name: Logout of Amazon ECR + run: docker logout ${{ steps.login-ecr.outputs.registry }} + + - name: Deploy lambda + run: | + aws lambda update-function-code \ + --function-name ${{ matrix.image }} \ + --image-uri $REGISTRY/${{ matrix.image }}:${GITHUB_SHA::7} diff --git a/.github/workflows/performance.yml b/.github/workflows/performance.yml new file mode 100644 index 0000000000..e019b48043 --- /dev/null +++ b/.github/workflows/performance.yml @@ -0,0 +1,31 @@ +name: Notify Performance / Load Tests + +on: + workflow_dispatch: + +jobs: + test: + runs-on: ubuntu-latest + steps: + - name: Install libcurl + run: sudo apt-get update && sudo apt-get install libssl-dev libcurl4-openssl-dev + - uses: actions/checkout@v2 + - name: Set up Python 3.9 + uses: actions/setup-python@v2 + with: + python-version: 3.9 + - name: Upgrade pip + run: python -m pip install --upgrade pip + - uses: actions/cache@v2 + with: + path: ~/.cache/pip + key: ${{ runner.os }}-pip-${{ hashFiles('requirements.txt') }} + restore-keys: | + ${{ runner.os }}-pip- + - name: Run performance tests + run: /bin/bash -c "pip install -r requirements_for_test.txt && locust --headless --config tests-perf/locust/locust.conf -f tests-perf/locust/locust-notifications.py" + - name: Notify Slack channel if this performance test job fails + if: ${{ failure() && github.ref == 'refs/heads/master' }} + run: | + json="{'text':'Scheduled CI Performance testing failed: '}" + curl -X POST -H 'Content-type: application/json' --data "$json" ${{ secrets.SLACK_WEBHOOK }} diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 5d2d29f067..529a2e0547 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -1,15 +1,12 @@ on: push: - schedule: - - cron: '2 0-12,20-23 * * 1-5' - - cron: '2 * * * 0,6' -name: CI testing +name: Python tests jobs: test: runs-on: ubuntu-latest services: postgres: - image: postgres:11.8 + image: postgres:13.4 env: POSTGRES_USER: postgres POSTGRES_PASSWORD: postgres diff --git a/SECURITY.md b/SECURITY.md index e1cba1075c..f3e449e6f1 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -1,9 +1,9 @@ # Security -**Do not post any security issues on the public repository!** Security vulnerabilities must be reported by email to [john.obrien@tbs-sct.gc.ca](mailto:john.obrien@tbs-sct.gc.ca) and [damien.trudel@tbs-sct.gc.ca](mailto:damien.trudel@tbs-sct.gc.ca). +**Do not post any security issues on the public repository!** Security vulnerabilities must be reported by email to [security@cds-snc.ca ](mailto:security@cds-snc.ca). ______________________ ## Sécurité -**Ne publiez aucun problème de sécurité sur le dépôt publique!** Les vulnérabilités de sécurité doivent être signalées par courriel à [john.obrien@tbs-sct.gc.ca](mailto:john.obrien@tbs-sct.gc.ca) et [damien.trudel@tbs-sct.gc.ca](mailto:damien.trudel@tbs-sct.gc.ca). +**Ne publiez aucun problème de sécurité sur le dépôt publique!** Les vulnérabilités de sécurité doivent être signalées par courriel à [security@cds-snc.ca ](mailto:security@cds-snc.ca). diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py index 57b9b59a0d..73c843ffc1 100644 --- a/app/celery/provider_tasks.py +++ b/app/celery/provider_tasks.py @@ -9,6 +9,7 @@ from app.dao.notifications_dao import update_notification_status_by_id from app.delivery import send_to_providers from app.exceptions import ( + InvalidUrlException, MalwarePendingException, NotificationTechnicalFailureException, ) @@ -65,6 +66,10 @@ def deliver_email(self, notification_id): current_app.logger.info(f"Cannot send notification {notification_id}, got an invalid email address: {str(e)}.") update_notification_status_by_id(notification_id, NOTIFICATION_TECHNICAL_FAILURE) _check_and_queue_callback_task(notification) + except InvalidUrlException: + current_app.logger.error(f"Cannot send notification {notification_id}, got an invalid direct file url.") + update_notification_status_by_id(notification_id, NOTIFICATION_TECHNICAL_FAILURE) + _check_and_queue_callback_task(notification) except MalwarePendingException: current_app.logger.info("RETRY: Email notification {} is pending malware scans".format(notification_id)) self.retry(queue=QueueNames.RETRY, countdown=60) @@ -90,6 +95,10 @@ def _deliver_sms(self, notification_id): if not notification: raise NoResultFound() send_to_providers.send_sms_to_provider(notification) + except InvalidUrlException: + current_app.logger.error(f"Cannot send notification {notification_id}, got an invalid direct file url.") + update_notification_status_by_id(notification_id, NOTIFICATION_TECHNICAL_FAILURE) + _check_and_queue_callback_task(notification) except Exception: try: current_app.logger.exception("SMS notification delivery for id: {} failed".format(notification_id)) diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 9c9dbd5048..226d6486ad 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -122,20 +122,20 @@ def check_job_status(): from jobs where job_status == 'in progress' and template_type in ('sms', 'email') - and scheduled_at or created_at is older that 60 minutes. + and scheduled_at or created_at is older that 120 minutes. if any results then raise error process the rows in the csv that are missing (in another task) just do the check here. """ - sixty_minutes_ago = datetime.utcnow() - timedelta(minutes=60) - sixty_five_minutes_ago = datetime.utcnow() - timedelta(minutes=65) + minutes_ago_120 = datetime.utcnow() - timedelta(minutes=120) + minutes_ago_125 = datetime.utcnow() - timedelta(minutes=125) - jobs_not_complete_after_60_minutes = ( + jobs_not_complete_after_120_minutes = ( Job.query.filter( Job.job_status == JOB_STATUS_IN_PROGRESS, and_( - sixty_five_minutes_ago < Job.processing_started, - Job.processing_started < sixty_minutes_ago, + minutes_ago_125 < Job.processing_started, + Job.processing_started < minutes_ago_120, ), ) .order_by(Job.processing_started) @@ -145,7 +145,7 @@ def check_job_status(): # temporarily mark them as ERROR so that they don't get picked up by future check_job_status tasks # if they haven't been re-processed in time. job_ids = [] - for job in jobs_not_complete_after_60_minutes: + for job in jobs_not_complete_after_120_minutes: job.job_status = JOB_STATUS_ERROR dao_update_job(job) job_ids.append(str(job.id)) diff --git a/app/clients/freshdesk.py b/app/clients/freshdesk.py index ae2942373b..1fc63fc546 100644 --- a/app/clients/freshdesk.py +++ b/app/clients/freshdesk.py @@ -85,10 +85,5 @@ def send_ticket(self) -> int: return response.status_code except requests.RequestException as e: content = json.loads(response.content) - current_app.logger.warning(f"Failed to create Freshdesk ticket: {content['errors']}") + current_app.logger.error(f"Failed to create Freshdesk ticket: {content['errors']}") raise e - except NotImplementedError: - # There are cases in development when we do not want to send to freshdesk - # because configuration is not defined, lets return a 200 OK - current_app.logger.warning("Did not send ticket to Freshdesk") - return 200 diff --git a/app/clients/zendesk.py b/app/clients/zendesk.py new file mode 100644 index 0000000000..e39f4402df --- /dev/null +++ b/app/clients/zendesk.py @@ -0,0 +1,82 @@ +from typing import Dict, List, Union +from urllib.parse import urljoin + +import requests +from flask import current_app +from requests.auth import HTTPBasicAuth + +from app.user.contact_request import ContactRequest + +__all__ = ["Zendesk"] + + +class Zendesk(object): + def __init__(self, contact: ContactRequest): + self.api_url = current_app.config["ZENDESK_API_URL"] + self.token = current_app.config["ZENDESK_API_KEY"] + self.contact = contact + + def _generate_description(self): + message = self.contact.message + if self.contact.is_go_live_request(): + message = "
".join( + [ + f"{self.contact.service_name} just requested to go live.", + "", + f"- Department/org: {self.contact.department_org_name}", + f"- Intended recipients: {self.contact.intended_recipients}", + f"- Purpose: {self.contact.main_use_case}", + f"- Notification types: {self.contact.notification_types}", + f"- Expected monthly volume: {self.contact.expected_volume}", + "---", + self.contact.service_url, + ] + ) + elif self.contact.is_branding_request(): + message = "
".join( + [ + f"A new logo has been uploaded by {self.contact.name} ({self.contact.email_address}) for the following service:", + f"- Service id: {self.contact.service_id}", + f"- Service name: {self.contact.service_name}", + f"- Logo filename: {self.contact.branding_url}", + "
", + f"Un nouveau logo a été téléchargé par {self.contact.name} ({self.contact.email_address}) pour le service suivant :", + f"- Identifiant du service : {self.contact.service_id}", + f"- Nom du service : {self.contact.service_name}", + f"- Nom du fichier du logo : {self.contact.branding_url}", + ] + ) + + if len(self.contact.user_profile): + message += f"

---

{self.contact.user_profile}" + + return message + + # Update for Zendesk API Ticket format + # read docs: https://developer.zendesk.com/rest_api/docs/core/tickets#create-ticket + def _generate_ticket(self) -> Dict[str, Dict[str, Union[str, int, List[str]]]]: + return { + "ticket": { + "subject": self.contact.friendly_support_type, + "description": self._generate_description(), + "email": self.contact.email_address, + "tags": self.contact.tags + + ["notification_api"], # Custom tag used to auto-assign ticket to the notification support group + } + } + + def send_ticket(self): + if not self.api_url or not self.token: + raise NotImplementedError + + # The API and field definitions are defined here: + # https://developer.zendesk.com/rest_api/docs/support/tickets + response = requests.post( + urljoin(self.api_url, "/api/v2/tickets"), + json=self._generate_ticket(), + auth=HTTPBasicAuth(f"{self.contact.email_address}/token", self.token), + timeout=5, + ) + + if response.status_code != 201: + raise requests.HTTPError(response.status_code, "Failed to create zendesk ticket") diff --git a/app/config.py b/app/config.py index 988de41a0d..e9bb675d3b 100644 --- a/app/config.py +++ b/app/config.py @@ -143,6 +143,7 @@ class Config(object): PERFORMANCE_PLATFORM_URL = "https://www.performance.service.gov.uk/data/govuk-notify/" # Zendesk + ZENDESK_API_URL = os.getenv("ZENDESK_API_URL") ZENDESK_API_KEY = os.getenv("ZENDESK_API_KEY") ZENDESK_SELL_API_URL = os.getenv("ZENDESK_SELL_API_URL") ZENDESK_SELL_API_KEY = os.getenv("ZENDESK_SELL_API_KEY") @@ -491,6 +492,9 @@ class Test(Development): TEMPLATE_PREVIEW_API_HOST = "http://localhost:9999" + # FEATURE FLAGS + FF_BATCH_INSERTION = os.getenv("FF_BATCH_INSERTION", False) + class Production(Config): NOTIFY_EMAIL_DOMAIN = os.getenv("NOTIFY_EMAIL_DOMAIN", "notification.canada.ca") diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index f0598b7b97..8d5c4d667a 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -85,6 +85,31 @@ def dao_create_notification(notification): db.session.add(notification) +@statsd(namespace="dao") +@transactional +def bulk_insert_notifications(notifications): + """ + Takes a list of models.Notifications and inserts or updates the DB + with the list accordingly + + Parameters: + ---------- + notifications: lof models.Notification + + Return: + ------ + None + """ + for notification in notifications: + if not notification.id: + notification.id = create_uuid() + if not notification.status: + notification.status = NOTIFICATION_CREATED + + # TODO: Add error handling (Redis queue?) for failed notifications + return db.session.bulk_save_objects(notifications) + + def _decide_permanent_temporary_failure(current_status, status): # Firetext will send pending, then send either succes or fail. # If we go from pending to delivered we need to set failure type as temporary-failure diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index c0e1eed3df..35ae60fa6a 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -2,6 +2,8 @@ import re import urllib.request from datetime import datetime +from typing import Dict +from uuid import UUID from flask import current_app from notifications_utils.recipients import ( @@ -24,6 +26,7 @@ ) from app.dao.templates_dao import dao_get_template_by_id from app.exceptions import ( + InvalidUrlException, MalwarePendingException, NotificationTechnicalFailureException, ) @@ -110,6 +113,18 @@ def is_service_allowed_html(service: Service) -> bool: return str(service.id) in current_app.config["ALLOW_HTML_SERVICE_IDS"] +# Prevent URL patterns like file:// ftp:// that may lead to security local file read vulnerabilities +def check_file_url(file_info: Dict[str, str], notification_id: UUID): + if file_info.get("sending_method") == "attach": + url_key = "direct_file_url" + else: + url_key = "url" + + if not file_info[url_key].lower().startswith("http"): + current_app.logger.error(f"Notification {notification_id} contains an invalid {url_key} {file_info[url_key]}") + raise InvalidUrlException + + def send_email_to_provider(notification: Notification): service = notification.service if not service.active: @@ -125,6 +140,7 @@ def send_email_to_provider(notification: Notification): personalisation_data = notification.personalisation.copy() for key in file_keys: + check_file_url(personalisation_data[key]["document"], notification.id) sending_method = personalisation_data[key]["document"].get("sending_method") # Check if a MLWR sid exists if ( diff --git a/app/exceptions.py b/app/exceptions.py index d5c36b68c8..fd3c1cd5d6 100644 --- a/app/exceptions.py +++ b/app/exceptions.py @@ -13,3 +13,7 @@ class ArchiveValidationError(Exception): class MalwarePendingException(Exception): pass + + +class InvalidUrlException(Exception): + pass diff --git a/app/notifications/validators.py b/app/notifications/validators.py index fffd5c7edf..0277e3894b 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -142,8 +142,8 @@ def check_template_is_for_notification_type(notification_type, template_type): def check_template_is_active(template): if template.archived: raise BadRequestError( - fields=[{"template": "Template has been deleted"}], - message="Template has been deleted", + fields=[{"template": f"Template {template.id} has been deleted"}], + message=f"Template {template.id} has been deleted", ) diff --git a/app/user/rest.py b/app/user/rest.py index c3f2a0ca92..e8ce2e4b82 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -13,6 +13,7 @@ from sqlalchemy.orm.exc import NoResultFound from app.clients.freshdesk import Freshdesk +from app.clients.zendesk import Zendesk from app.clients.zendesk_sell import ZenDeskSell from app.config import Config, QueueNames from app.dao.fido2_key_dao import ( @@ -461,6 +462,12 @@ def send_contact_request(user_id): if contact.is_demo_request(): return jsonify({}), 204 + + try: + Zendesk(contact).send_ticket() + except Exception as e: + current_app.logger.exception(e) + status_code = Freshdesk(contact).send_ticket() return jsonify({"status_code": status_code}), 204 @@ -491,6 +498,11 @@ def send_branding_request(user_id): current_app.logger.error(e) return jsonify({}), 400 + try: + Zendesk(contact).send_ticket() + except Exception as e: + current_app.logger.exception(e) + status_code = Freshdesk(contact).send_ticket() return jsonify({"status_code": status_code}), 204 diff --git a/application.py b/application.py index bf1de80b1c..a76440c520 100644 --- a/application.py +++ b/application.py @@ -4,6 +4,7 @@ import os import awsgi +import newrelic.agent # See https://bit.ly/2xBVKBH import sentry_sdk from dotenv import load_dotenv from flask import Flask @@ -40,4 +41,5 @@ def handler(event, context): + newrelic.agent.initialize() # noqa: E402 return awsgi.response(app, event, context) diff --git a/bin/execute_and_publish_performance_test.sh b/bin/execute_and_publish_performance_test.sh new file mode 100755 index 0000000000..ef7fe0a5bf --- /dev/null +++ b/bin/execute_and_publish_performance_test.sh @@ -0,0 +1,13 @@ +#!/bin/bash + +current_time=$(date "+%Y.%m.%d-%H.%M.%S") +perf_test_aws_s3_bucket=${PERF_TEST_AWS_S3_BUCKET:-notify-performance-test-results-staging} +perf_test_csv_directory_path=${PERF_TEST_CSV_DIRECTORY_PATH:-/tmp/notify_performance_test} + +mkdir -p $perf_test_csv_directory_path/$current_time + +locust --headless --config tests-perf/locust/locust.conf --html $perf_test_csv_directory_path/$current_time/index.html --csv $perf_test_csv_directory_path/$current_time/perf_test + +aws s3 cp $perf_test_csv_directory_path/ "s3://$perf_test_aws_s3_bucket" --recursive || exit 1 + +rm -rf $perf_test_csv_directory_path/$current_time diff --git a/heartbeat/Dockerfile b/heartbeat/Dockerfile new file mode 100644 index 0000000000..4da92b4169 --- /dev/null +++ b/heartbeat/Dockerfile @@ -0,0 +1,12 @@ +FROM public.ecr.aws/lambda/python:3.9 + +ENV PYTHONDONTWRITEBYTECODE 1 + +# Install the function's dependencies +COPY heartbeat/requirements_for_heartbeat.txt ${LAMBDA_TASK_ROOT} +RUN python -m pip install -r requirements_for_heartbeat.txt + +# Copy function code +COPY heartbeat/heartbeat.py ${LAMBDA_TASK_ROOT} + +CMD [ "heartbeat.handler" ] diff --git a/heartbeat/heartbeat.py b/heartbeat/heartbeat.py new file mode 100644 index 0000000000..ae936e6f02 --- /dev/null +++ b/heartbeat/heartbeat.py @@ -0,0 +1,33 @@ +""" +Code to keep the lambda function alive. +""" +import ast +import os +import uuid +from typing import List + +from notifications_python_client.errors import HTTPError +from notifications_python_client.notifications import NotificationsAPIClient + +API_KEY: str = os.getenv("heartbeat_api_key", "") +# As we can't pass in a list to env var, we pass a str and convert it. +BASE_URL: List[str] = ast.literal_eval(os.getenv("heartbeat_base_url")) # type: ignore +EMAIL_ADDRESS = "success@simulator.amazonses.com" +TEMPLATE_ID: uuid.UUID = os.getenv("heartbeat_template_id") # type: ignore + + +def handler(event, context): + if not BASE_URL: + print("Variable BASE_URL is missing") + if not API_KEY: + print("Variable API_KEY is missing") + if not TEMPLATE_ID: + print("Variable TEMPLATE_ID is missing") + for base_url in BASE_URL: + notifications_client = NotificationsAPIClient(API_KEY, base_url=base_url) + try: + notifications_client.send_email_notification(email_address=EMAIL_ADDRESS, template_id=TEMPLATE_ID) + print("Email has been sent by {}!".format(base_url)) + except HTTPError as e: + print(f"Could not send heartbeat: status={e.status_code}, msg={e.message}") + raise diff --git a/heartbeat/requirements_for_heartbeat.txt b/heartbeat/requirements_for_heartbeat.txt new file mode 100644 index 0000000000..0ed0a7d5ce --- /dev/null +++ b/heartbeat/requirements_for_heartbeat.txt @@ -0,0 +1,6 @@ +# pyup: ignore file +# This file is autogenerated. Do not edit it manually. +# Run `make freeze-requirements` to update requirements.txt +# with package version changes made in requirements-app.txt + +notifications-python-client==6.0.2 diff --git a/pytest.ini b/pytest.ini index d35ab4fc6f..9f5b793a96 100644 --- a/pytest.ini +++ b/pytest.ini @@ -12,6 +12,8 @@ env = AWS_SESSION_TOKEN='testing' AWS_US_TOLL_FREE_NUMBER='+18005555555' FRESH_DESK_PRODUCT_ID=42 + ZENDESK_API_URL = https://zendesk-test.com + ZENDESK_API_KEY = zendesk-api-key FRESH_DESK_API_URL=https://freshdesk-test.com FRESH_DESK_API_KEY=freshdesk-api-key ZENDESK_SELL_API_URL=https://zendesksell-test.com diff --git a/requirements-app.txt b/requirements-app.txt index 182641e423..2b038d0d43 100644 --- a/requirements-app.txt +++ b/requirements-app.txt @@ -13,7 +13,7 @@ Flask-Migrate==2.7.0 git+https://github.com/mitsuhiko/flask-sqlalchemy.git@500e732dd1b975a56ab06a46bd1a20a21e682262#egg=Flask-SQLAlchemy==2.3.2.dev20190108 Flask==1.1.2 click-datetime==0.2 -eventlet==0.31.0 +eventlet==0.30.2 # currently 0.31.0+ breaks gunicorn. Test the docker image if upgrading! gunicorn==20.1.0 iso8601==0.1.14 idna==2.8 # pinned to align with test moto dependency requirements diff --git a/requirements.txt b/requirements.txt index f3f22bb73c..f304fa7d62 100644 --- a/requirements.txt +++ b/requirements.txt @@ -15,7 +15,7 @@ Flask-Migrate==2.7.0 git+https://github.com/mitsuhiko/flask-sqlalchemy.git@500e732dd1b975a56ab06a46bd1a20a21e682262#egg=Flask-SQLAlchemy==2.3.2.dev20190108 Flask==1.1.2 click-datetime==0.2 -eventlet==0.31.0 +eventlet==0.30.2 # currently 0.31.0+ breaks gunicorn. Test the docker image if upgrading! gunicorn==20.1.0 iso8601==0.1.14 idna==2.8 # pinned to align with test moto dependency requirements diff --git a/tests-perf/locust/README.md b/tests-perf/locust/README.md index 892d60d471..53f38645bf 100644 --- a/tests-perf/locust/README.md +++ b/tests-perf/locust/README.md @@ -58,75 +58,32 @@ You can also modify the *locust.config* file to enable the headless mode and def ### Performance Testing on AWS -#### Overview - -In the Notify staging account you will find an AMI image that you can use to spin up EC2 servers in which you can ssh into and -run the performance testing. - -Use the following link to navigate to the [AMI image](https://ca-central-1.console.aws.amazon.com/ec2/v2/home?region=ca-central-1#Images:visibility=owned-by-me;name=locust-testing-image;sort=name). -Following this right click on the image and click launch. You'll want to launch a minimum of a t2 large as locust is cpu intensive. - -By default the image is set up to target the [Jimmy Royer - GC Notify test](https://staging.notification.cdssandbox.xyz/services/2317d68b-f3ab-4949-956d-4367b488db4b) -service on staging when you run the locust test. - -#### Running the tests on EC2 - -For convenience there are three EC2 servers that have already been created in the Notify staging account -such that you can log in and immediately run the locust tests. Each one of these servers target a different -service, and you can find out which server targets which service by looking at the `TEST_AUTH_HEADER` entry in the .env -file. - -These servers should be in a stopped state when performance tests aren't being run to avoid AWS -charges on resources we're not actively using. - -##### Step 1. Logging into the EC2 servers - -You can find the PEM file (performance_testing.pem) to log into each one of the EC2 servers in the Shared-Notify staging folder -in lastpass. Download this file locally and then change its permissions such that it can only be accessed by the root user - -```shell -$ chmod 400 performance_testing.pem -``` - -You can then ssh into any of the respective servers - -```shell -$ ssh -i performance_testing.pem ubuntu@{SERVER_IP_ADDRESS} -``` - -##### Step 2. Change into the root user on the server - -You'll need to change into the root user on the server. This is because we need to bind to port 80 -on the 0.0.0.0 host. - -```shell -$ sudo su -``` - -##### Step 4. Set the maximum number of files that can be open to 100240 - -Locust requires a file descriptor limit of over 100000 usually by default ubuntu sets this at 1024. To set -this use the ulimit command - -```shell -$ ulimit -n 100240 -``` -##### Step 5. Activate the virtual environment - -```shell -$ source venv/bin/activate -``` - -##### Step 6. Run the locust interface - -```shell -$ locust -f tests-perf/locust/locust-notifications.py --web-host 0.0.0.0 --web-port 80 -``` - -You should then be able to see the locust interface if you navigate to the server's IP address -in your browser - -##### Step 7. Shut down the server when you are finished - -To avoid incurring charges on resources that are not being used please put the instances into a -stopped state when you are finished with the performance testing. \ No newline at end of file +We run Notify performance tests on a daily manner through AWS ECS tasks +running on a Fargate cluster. It is automatically triggered by a +[CloudWatch event target](https://github.com/cds-snc/notification-terraform/blob/a5fcbf0d0e2ff5cd78952bf5c8f9f2dfd5d3c93c/aws/performance-test/cloudwatch.tf#L10). + +It is possible to manually launch it though via the AWS console of the +ECS task. In order to do so, perform the following steps: + +1. [Log into the AWS console](https://cds-snc.awsapps.com/start#/) + for the staging environment. +2. [Head over to the Task Definitions](https://ca-central-1.console.aws.amazon.com/ecs/home?region=ca-central-1#/taskDefinitions) + page within the ECS console. +3. Select the `performance_test_cluster` task definition. +4. Click on *Actions* button and select *Run*. +5. On the new page, let's fill the following details to get the task + to run. + 1. Leave the default cluster strategy as-is. + 2. Select `Linux` as the operating system. + 3. Leave task definition, platform version as-is. + 4. Make sure the desired cluster is selected to run the task with. + 5. Number of task and task group can be left as-is, normally set to 1 task. + 6. For the VPC and security groups section, select the `notification-canada-ca` VPC. + 7. Then select all public subnets, 3 at the moment of this writing. + 8. Select the security group named `perf_test`. + 9. Other options can be left as-is: you should be all set and ready. + 10. Click the *Run Task* button! +6. Once the task is ran, you can select it from the list of running tasks. + 1. To see the logs of the running task and once you are in the task page, expand the container section. There should be a link to *View logs in CloudWatch* under the *Log Configuration* section. +7. On performance test completion, [the results should be in the `notify-performance-test-results-staging` S3 bucket](https://s3.console.aws.amazon.com/s3/buckets/notify-performance-test-results-staging?region=ca-central-1&tab=objects). Look into the folder that represents the proper timestamp for the test execution and open the `index.html` file located within. +8. The performance tests results should also be published [in this GitHub repository](https://github.com/cds-snc/notification-performance-test-results) every day at midnight. If you just executed the test, it will take some delay to have the tests published. diff --git a/tests-perf/locust/locust-notifications.py b/tests-perf/locust/locust-notifications.py index 9ac203a808..312215fe04 100644 --- a/tests-perf/locust/locust-notifications.py +++ b/tests-perf/locust/locust-notifications.py @@ -1,24 +1,102 @@ +""" locust-notifications.py + isort:skip_file +""" +# flake8: noqa + import os +import sys +from datetime import datetime +from dataclasses import make_dataclass + +sys.path.append(os.path.abspath(os.path.join("..", "tests_smoke"))) from dotenv import load_dotenv from locust import HttpUser, constant_pacing, task +from tests_smoke.smoke.common import job_line, rows_to_csv # type: ignore load_dotenv() -AUTH_HEADER = os.getenv("TEST_AUTH_HEADER") +NotifyApiUserTemplateGroup = make_dataclass('NotifyApiUserTemplateGroup', [ + 'bulk_email_id', + 'email_id', + 'email_with_attachment_id', + 'email_with_link_id', + 'sms_id', +]) class NotifyApiUser(HttpUser): wait_time = constant_pacing(60) - host = "https://api.staging.notification.cdssandbox.xyz" + host = os.getenv("PERF_TEST_DOMAIN", "https://api.staging.notification.cdssandbox.xyz") + + def __init__(self, *args, **kwargs): + super(NotifyApiUser, self).__init__(*args, **kwargs) + + self.headers = {"Authorization": os.getenv("PERF_TEST_AUTH_HEADER")} + self.email = os.getenv("PERF_TEST_EMAIL", "success@simulator.amazonses.com") + self.phone_number = os.getenv("PERF_TEST_PHONE_NUMBER", "16132532222") + self.template_group = NotifyApiUserTemplateGroup( + bulk_email_id=os.getenv("PERF_TEST_BULK_EMAIL_TEMPLATE_ID"), + email_id=os.getenv("PERF_TEST_EMAIL_TEMPLATE_ID"), + email_with_attachment_id=os.getenv("PERF_TEST_EMAIL_WITH_ATTACHMENT_TEMPLATE_ID"), + email_with_link_id=os.getenv("PERF_TEST_EMAIL_WITH_LINK_TEMPLATE_ID"), + sms_id=os.getenv("PERF_TEST_SMS_TEMPLATE_ID"), + ) + + @task(16) + def send_email_notifications(self): + json = self.__email_json(self.template_group.email_id) + + self.client.post("/v2/notifications/email", json=json, headers=self.headers) + + @task(2) + def send_email_with_attachment_notifications(self): + personalisation = { + "attached_file": { + "file": "Q29udGVudCBvZiBBdHRhY2hlZCBmaWxl", + "filename": "attached_file.txt", + "sending_method": "attach", + } + } + json = self.__email_json(self.template_group.email_with_attachment_id, personalisation) + + self.client.post("/v2/notifications/email", json=json, headers=self.headers) - @task - def send_notifications(self): - headers = {"Authorization": AUTH_HEADER} + @task(2) + def send_email_with_link_notifications(self): + personalisation = { + "application_file": { + "file": "Q29udGVudCBvZiBBdHRhY2hlZCBmaWxl", + "filename": "attached_file.txt", + "sending_method": "link", + } + } + json = self.__email_json(self.template_group.email_with_link_id, personalisation) + + self.client.post("/v2/notifications/email", json=json, headers=self.headers) + + @task(8) + def send_bulk_notifications(self): json = { - "email_address": "success@simulator.amazonses.com", - "template_id": "9c17633c-126a-4ad3-ad2f-b14c3a85314a", - "personalisation": {"colour": "Fulvous"}, + "name": f"My bulk name {datetime.utcnow().isoformat()}", + "template_id": self.template_group.bulk_email_id, + "csv": rows_to_csv([["email address", "application_file"], *job_line(self.email, 2)]) } - self.client.post("/v2/notifications/email", json=json, headers=headers) + self.client.post("/v2/notifications/bulk", json=json, headers=self.headers) + + @task(16) + def send_sms_notifications(self): + json = { + "phone_number": self.phone_number, + "template_id": self.template_group.sms_id + } + + self.client.post("/v2/notifications/sms", json=json, headers=self.headers) + + def __email_json(self, template_id, personalisation={}): + return { + "email_address": self.email, + "template_id": template_id, + "personalisation": personalisation, + } diff --git a/tests-perf/locust/locust.conf b/tests-perf/locust/locust.conf index 851f0fba5f..c1eba3b220 100644 --- a/tests-perf/locust/locust.conf +++ b/tests-perf/locust/locust.conf @@ -1,10 +1,10 @@ # master.conf in current directory -locustfile = locust-notifications.py +locustfile = tests-perf/locust/locust-notifications.py host = https://api.staging.notification.cdssandbox.xyz -users = 6000 -spawn-rate = 200 +users = 3000 +spawn-rate = 20 +run-time = 5m # headless = true # master = true # expect-workers = 5 -# run-time = 5m \ No newline at end of file diff --git a/tests-perf/ops/Dockerfile b/tests-perf/ops/Dockerfile new file mode 100644 index 0000000000..40bcefe4c2 --- /dev/null +++ b/tests-perf/ops/Dockerfile @@ -0,0 +1,20 @@ +FROM python:3.9-alpine3.13 + +ENV PYTHONDONTWRITEBYTECODE 1 + +RUN apk add --no-cache bash build-base git libtool cmake autoconf automake gcc musl-dev postgresql-dev g++ libexecinfo-dev make libffi-dev libmagic libcurl curl-dev rust cargo && rm -rf /var/cache/apk/* + +# update pip +RUN python -m pip install wheel +RUN python -m pip install --upgrade pip + +RUN set -ex && mkdir /app + +WORKDIR /app + +COPY . /app + +RUN set -ex && pip3 install -r requirements_for_test.txt +RUN echo "fs.file-max = 100000" >> /etc/sysctl.conf + +ENTRYPOINT [ "bin/execute_and_publish_performance_test.sh" ] diff --git a/tests/app/api_key/test_rest.py b/tests/app/api_key/test_rest.py index bbf299c515..47637b8b99 100644 --- a/tests/app/api_key/test_rest.py +++ b/tests/app/api_key/test_rest.py @@ -7,6 +7,7 @@ create_notification, create_service, create_template, + save_notification, ) @@ -18,7 +19,8 @@ def test_get_api_key_stats_with_sends(admin_request, notify_db, notify_db_sessio total_sends = 10 for x in range(total_sends): - create_notification(template=template, api_key=api_key) + notification = create_notification(template=template, api_key=api_key) + save_notification(notification) api_key_stats = admin_request.get("api_key.get_api_key_stats", api_key_id=api_key.id)["data"] @@ -56,10 +58,10 @@ def test_get_api_keys_ranked(admin_request, notify_db, notify_db_session): template_email = create_template(service=service, template_type="email") total_sends = 10 - create_notification(template=template_email, api_key=api_key_1) + save_notification(create_notification(template=template_email, api_key=api_key_1)) for x in range(total_sends): - create_notification(template=template_email, api_key=api_key_1) - create_notification(template=template_email, api_key=api_key_2) + save_notification(create_notification(template=template_email, api_key=api_key_1)) + save_notification(create_notification(template=template_email, api_key=api_key_2)) api_keys_ranked = admin_request.get("api_key.get_api_keys_ranked", n_days_back=2)["data"] diff --git a/tests/app/billing/test_billing.py b/tests/app/billing/test_billing.py index b48efff25c..b9330e14ff 100644 --- a/tests/app/billing/test_billing.py +++ b/tests/app/billing/test_billing.py @@ -20,6 +20,7 @@ create_rate, create_service, create_template, + save_notification, ) APR_2016_MONTH_START = datetime(2016, 3, 31, 23, 00, 00) @@ -184,7 +185,7 @@ def test_get_yearly_usage_by_monthly_from_ft_billing_populates_deltas(client, no notification_type="sms", ) - create_notification(template=sms_template, status="delivered") + save_notification(create_notification(template=sms_template, status="delivered")) assert FactBilling.query.count() == 0 diff --git a/tests/app/celery/test_ftp_update_tasks.py b/tests/app/celery/test_ftp_update_tasks.py index f68b591a98..674474cb87 100644 --- a/tests/app/celery/test_ftp_update_tasks.py +++ b/tests/app/celery/test_ftp_update_tasks.py @@ -30,6 +30,7 @@ create_notification, create_notification_history, create_service_callback_api, + save_notification, ) from tests.conftest import set_config @@ -73,11 +74,13 @@ def test_update_letter_notification_statuses_when_notification_does_not_exist_up def test_update_letter_notifications_statuses_raises_dvla_exception(notify_api, mocker, sample_letter_template): valid_file = "ref-foo|Failed|1|Unsorted" mocker.patch("app.celery.tasks.s3.get_s3_file", return_value=valid_file) - create_notification( - sample_letter_template, - reference="ref-foo", - status=NOTIFICATION_SENDING, - billable_units=0, + save_notification( + create_notification( + sample_letter_template, + reference="ref-foo", + status=NOTIFICATION_SENDING, + billable_units=0, + ) ) with pytest.raises(DVLAException) as e: @@ -127,17 +130,21 @@ def test_update_letter_notifications_statuses_builds_updates_list(notify_api, mo def test_update_letter_notifications_statuses_persisted(notify_api, mocker, sample_letter_template): - sent_letter = create_notification( - sample_letter_template, - reference="ref-foo", - status=NOTIFICATION_SENDING, - billable_units=1, + sent_letter = save_notification( + create_notification( + sample_letter_template, + reference="ref-foo", + status=NOTIFICATION_SENDING, + billable_units=1, + ) ) - failed_letter = create_notification( - sample_letter_template, - reference="ref-bar", - status=NOTIFICATION_SENDING, - billable_units=2, + failed_letter = save_notification( + create_notification( + sample_letter_template, + reference="ref-bar", + status=NOTIFICATION_SENDING, + billable_units=2, + ) ) create_service_callback_api(service=sample_letter_template.service, url="https://original_url.com") valid_file = "{}|Sent|1|Unsorted\n{}|Failed|2|Sorted".format(sent_letter.reference, failed_letter.reference) @@ -162,11 +169,13 @@ def test_update_letter_notifications_statuses_persisted(notify_api, mocker, samp def test_update_letter_notifications_does_not_call_send_callback_if_no_db_entry(notify_api, mocker, sample_letter_template): - sent_letter = create_notification( - sample_letter_template, - reference="ref-foo", - status=NOTIFICATION_SENDING, - billable_units=0, + sent_letter = save_notification( + create_notification( + sample_letter_template, + reference="ref-foo", + status=NOTIFICATION_SENDING, + billable_units=0, + ) ) valid_file = "{}|Sent|1|Unsorted\n".format(sent_letter.reference) mocker.patch("app.celery.tasks.s3.get_s3_file", return_value=valid_file) @@ -178,8 +187,8 @@ def test_update_letter_notifications_does_not_call_send_callback_if_no_db_entry( def test_update_letter_notifications_to_sent_to_dvla_updates_based_on_notification_references(client, sample_letter_template): - first = create_notification(sample_letter_template, reference="first ref") - second = create_notification(sample_letter_template, reference="second ref") + first = save_notification(create_notification(sample_letter_template, reference="first ref")) + second = save_notification(create_notification(sample_letter_template, reference="second ref")) dt = datetime.utcnow() with freeze_time(dt): @@ -195,8 +204,8 @@ def test_update_letter_notifications_to_sent_to_dvla_updates_based_on_notificati def test_update_letter_notifications_to_error_updates_based_on_notification_references( sample_letter_template, ): - first = create_notification(sample_letter_template, reference="first ref") - second = create_notification(sample_letter_template, reference="second ref") + first = save_notification(create_notification(sample_letter_template, reference="first ref")) + second = save_notification(create_notification(sample_letter_template, reference="second ref")) create_service_callback_api(service=sample_letter_template.service, url="https://original_url.com") dt = datetime.utcnow() with freeze_time(dt): @@ -214,7 +223,7 @@ def test_update_letter_notifications_to_error_updates_based_on_notification_refe def test_check_billable_units_when_billable_units_matches_page_count(client, sample_letter_template, mocker, notification_update): mock_logger = mocker.patch("app.celery.tasks.current_app.logger.error") - create_notification(sample_letter_template, reference="REFERENCE_ABC", billable_units=1) + save_notification(create_notification(sample_letter_template, reference="REFERENCE_ABC", billable_units=1)) check_billable_units(notification_update) @@ -226,7 +235,7 @@ def test_check_billable_units_when_billable_units_does_not_match_page_count( ): mock_logger = mocker.patch("app.celery.tasks.current_app.logger.exception") - notification = create_notification(sample_letter_template, reference="REFERENCE_ABC", billable_units=3) + notification = save_notification(create_notification(sample_letter_template, reference="REFERENCE_ABC", billable_units=3)) check_billable_units(notification_update) diff --git a/tests/app/celery/test_letters_pdf_tasks.py b/tests/app/celery/test_letters_pdf_tasks.py index 70227128eb..c179f74a75 100644 --- a/tests/app/celery/test_letters_pdf_tasks.py +++ b/tests/app/celery/test_letters_pdf_tasks.py @@ -40,7 +40,7 @@ Notification, ) from celery.exceptions import MaxRetriesExceededError, Retry -from tests.app.db import create_letter_branding, create_notification +from tests.app.db import create_letter_branding, create_notification, save_notification from tests.conftest import set_config_values @@ -478,12 +478,14 @@ def test_process_letter_task_check_virus_scan_passed( bucket_config_name, destination_folder, ): - letter_notification = create_notification( - template=sample_letter_template, - billable_units=0, - status="pending-virus-check", - key_type=key_type, - reference="{} letter".format(key_type), + letter_notification = save_notification( + create_notification( + template=sample_letter_template, + billable_units=0, + status="pending-virus-check", + key_type=key_type, + reference="{} letter".format(key_type), + ) ) filename = "NOTIFY.{}".format(letter_notification.reference) source_bucket_name = current_app.config["LETTERS_SCAN_BUCKET_NAME"] diff --git a/tests/app/celery/test_nightly_tasks.py b/tests/app/celery/test_nightly_tasks.py index bc2207124b..6222dfb3ff 100644 --- a/tests/app/celery/test_nightly_tasks.py +++ b/tests/app/celery/test_nightly_tasks.py @@ -42,6 +42,7 @@ create_service_callback_api, create_service_data_retention, create_template, + save_notification, ) @@ -182,20 +183,29 @@ def test_should_call_delete_letter_notifications_more_than_week_in_task(notify_a def test_update_status_of_notifications_after_timeout(notify_api, sample_template): with notify_api.test_request_context(): - not1 = create_notification( - template=sample_template, - status="sending", - created_at=datetime.utcnow() - timedelta(seconds=current_app.config.get("SENDING_NOTIFICATIONS_TIMEOUT_PERIOD") + 10), + not1 = save_notification( + create_notification( + template=sample_template, + status="sending", + created_at=datetime.utcnow() + - timedelta(seconds=current_app.config.get("SENDING_NOTIFICATIONS_TIMEOUT_PERIOD") + 10), + ) ) - not2 = create_notification( - template=sample_template, - status="created", - created_at=datetime.utcnow() - timedelta(seconds=current_app.config.get("SENDING_NOTIFICATIONS_TIMEOUT_PERIOD") + 10), + not2 = save_notification( + create_notification( + template=sample_template, + status="created", + created_at=datetime.utcnow() + - timedelta(seconds=current_app.config.get("SENDING_NOTIFICATIONS_TIMEOUT_PERIOD") + 10), + ) ) - not3 = create_notification( - template=sample_template, - status="pending", - created_at=datetime.utcnow() - timedelta(seconds=current_app.config.get("SENDING_NOTIFICATIONS_TIMEOUT_PERIOD") + 10), + not3 = save_notification( + create_notification( + template=sample_template, + status="pending", + created_at=datetime.utcnow() + - timedelta(seconds=current_app.config.get("SENDING_NOTIFICATIONS_TIMEOUT_PERIOD") + 10), + ) ) with pytest.raises(NotificationTechnicalFailureException) as e: timeout_notifications() @@ -207,10 +217,13 @@ def test_update_status_of_notifications_after_timeout(notify_api, sample_templat def test_not_update_status_of_notification_before_timeout(notify_api, sample_template): with notify_api.test_request_context(): - not1 = create_notification( - template=sample_template, - status="sending", - created_at=datetime.utcnow() - timedelta(seconds=current_app.config.get("SENDING_NOTIFICATIONS_TIMEOUT_PERIOD") - 10), + not1 = save_notification( + create_notification( + template=sample_template, + status="sending", + created_at=datetime.utcnow() + - timedelta(seconds=current_app.config.get("SENDING_NOTIFICATIONS_TIMEOUT_PERIOD") - 10), + ) ) timeout_notifications() assert not1.status == "sending" @@ -218,8 +231,8 @@ def test_not_update_status_of_notification_before_timeout(notify_api, sample_tem def test_should_not_update_status_of_letter_notifications(client, sample_letter_template): created_at = datetime.utcnow() - timedelta(days=5) - not1 = create_notification(template=sample_letter_template, status="sending", created_at=created_at) - not2 = create_notification(template=sample_letter_template, status="created", created_at=created_at) + not1 = save_notification(create_notification(template=sample_letter_template, status="sending", created_at=created_at)) + not2 = save_notification(create_notification(template=sample_letter_template, status="created", created_at=created_at)) timeout_notifications() @@ -230,10 +243,12 @@ def test_should_not_update_status_of_letter_notifications(client, sample_letter_ def test_timeout_notifications_sends_status_update_to_service(client, sample_template, mocker): callback_api = create_service_callback_api(service=sample_template.service) mocked = mocker.patch("app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async") - notification = create_notification( - template=sample_template, - status="sending", - created_at=datetime.utcnow() - timedelta(seconds=current_app.config.get("SENDING_NOTIFICATIONS_TIMEOUT_PERIOD") + 10), + notification = save_notification( + create_notification( + template=sample_template, + status="sending", + created_at=datetime.utcnow() - timedelta(seconds=current_app.config.get("SENDING_NOTIFICATIONS_TIMEOUT_PERIOD") + 10), + ) ) timeout_notifications() @@ -412,7 +427,7 @@ def test_delete_dvla_response_files_older_than_seven_days_does_not_remove_files( @freeze_time("2018-01-17 17:00:00") def test_alert_if_letter_notifications_still_sending(sample_letter_template, mocker): two_days_ago = datetime(2018, 1, 15, 13, 30) - create_notification(template=sample_letter_template, status="sending", sent_at=two_days_ago) + save_notification(create_notification(template=sample_letter_template, status="sending", sent_at=two_days_ago)) mock_create_ticket = mocker.patch("app.celery.nightly_tasks.zendesk_client.create_ticket") @@ -428,7 +443,7 @@ def test_alert_if_letter_notifications_still_sending(sample_letter_template, moc def test_alert_if_letter_notifications_still_sending_a_day_ago_no_alert(sample_letter_template, mocker): today = datetime.utcnow() one_day_ago = today - timedelta(days=1) - create_notification(template=sample_letter_template, status="sending", sent_at=one_day_ago) + save_notification(create_notification(template=sample_letter_template, status="sending", sent_at=one_day_ago)) mock_create_ticket = mocker.patch("app.celery.nightly_tasks.zendesk_client.create_ticket") @@ -439,9 +454,9 @@ def test_alert_if_letter_notifications_still_sending_a_day_ago_no_alert(sample_l @freeze_time("2018-01-17 17:00:00") def test_alert_if_letter_notifications_still_sending_only_alerts_sending(sample_letter_template, mocker): two_days_ago = datetime(2018, 1, 15, 13, 30) - create_notification(template=sample_letter_template, status="sending", sent_at=two_days_ago) - create_notification(template=sample_letter_template, status="delivered", sent_at=two_days_ago) - create_notification(template=sample_letter_template, status="failed", sent_at=two_days_ago) + save_notification(create_notification(template=sample_letter_template, status="sending", sent_at=two_days_ago)) + save_notification(create_notification(template=sample_letter_template, status="delivered", sent_at=two_days_ago)) + save_notification(create_notification(template=sample_letter_template, status="failed", sent_at=two_days_ago)) mock_create_ticket = mocker.patch("app.celery.nightly_tasks.zendesk_client.create_ticket") @@ -457,7 +472,7 @@ def test_alert_if_letter_notifications_still_sending_only_alerts_sending(sample_ @freeze_time("2018-01-17 17:00:00") def test_alert_if_letter_notifications_still_sending_alerts_for_older_than_offset(sample_letter_template, mocker): three_days_ago = datetime(2018, 1, 14, 13, 30) - create_notification(template=sample_letter_template, status="sending", sent_at=three_days_ago) + save_notification(create_notification(template=sample_letter_template, status="sending", sent_at=three_days_ago)) mock_create_ticket = mocker.patch("app.celery.nightly_tasks.zendesk_client.create_ticket") @@ -473,7 +488,7 @@ def test_alert_if_letter_notifications_still_sending_alerts_for_older_than_offse @freeze_time("2018-01-14 17:00:00") def test_alert_if_letter_notifications_still_sending_does_nothing_on_the_weekend(sample_letter_template, mocker): yesterday = datetime(2018, 1, 13, 13, 30) - create_notification(template=sample_letter_template, status="sending", sent_at=yesterday) + save_notification(create_notification(template=sample_letter_template, status="sending", sent_at=yesterday)) mock_create_ticket = mocker.patch("app.celery.nightly_tasks.zendesk_client.create_ticket") @@ -486,8 +501,8 @@ def test_alert_if_letter_notifications_still_sending_does_nothing_on_the_weekend def test_monday_alert_if_letter_notifications_still_sending_reports_thursday_letters(sample_letter_template, mocker): thursday = datetime(2018, 1, 11, 13, 30) yesterday = datetime(2018, 1, 14, 13, 30) - create_notification(template=sample_letter_template, status="sending", sent_at=thursday) - create_notification(template=sample_letter_template, status="sending", sent_at=yesterday) + save_notification(create_notification(template=sample_letter_template, status="sending", sent_at=thursday)) + save_notification(create_notification(template=sample_letter_template, status="sending", sent_at=yesterday)) mock_create_ticket = mocker.patch("app.celery.nightly_tasks.zendesk_client.create_ticket") @@ -504,8 +519,8 @@ def test_monday_alert_if_letter_notifications_still_sending_reports_thursday_let def test_tuesday_alert_if_letter_notifications_still_sending_reports_friday_letters(sample_letter_template, mocker): friday = datetime(2018, 1, 12, 13, 30) yesterday = datetime(2018, 1, 14, 13, 30) - create_notification(template=sample_letter_template, status="sending", sent_at=friday) - create_notification(template=sample_letter_template, status="sending", sent_at=yesterday) + save_notification(create_notification(template=sample_letter_template, status="sending", sent_at=friday)) + save_notification(create_notification(template=sample_letter_template, status="sending", sent_at=yesterday)) mock_create_ticket = mocker.patch("app.celery.nightly_tasks.zendesk_client.create_ticket") diff --git a/tests/app/celery/test_process_ses_receipts_tasks.py b/tests/app/celery/test_process_ses_receipts_tasks.py index 9a4f8d99fd..22317f2187 100644 --- a/tests/app/celery/test_process_ses_receipts_tasks.py +++ b/tests/app/celery/test_process_ses_receipts_tasks.py @@ -20,26 +20,34 @@ remove_emails_from_complaint, ) from tests.app.conftest import sample_notification as create_sample_notification -from tests.app.db import create_notification, create_service_callback_api +from tests.app.db import ( + create_notification, + create_service_callback_api, + save_notification, +) def test_process_ses_results(sample_email_template): - create_notification( - sample_email_template, - reference="ref1", - sent_at=datetime.utcnow(), - status="sending", + save_notification( + create_notification( + sample_email_template, + reference="ref1", + sent_at=datetime.utcnow(), + status="sending", + ) ) assert process_ses_results(response=ses_notification_callback(reference="ref1")) def test_process_ses_results_retry_called(sample_email_template, notify_db, mocker): - create_notification( - sample_email_template, - reference="ref1", - sent_at=datetime.utcnow(), - status="sending", + save_notification( + create_notification( + sample_email_template, + reference="ref1", + sent_at=datetime.utcnow(), + status="sending", + ) ) mocker.patch( @@ -52,7 +60,7 @@ def test_process_ses_results_retry_called(sample_email_template, notify_db, mock def test_process_ses_results_in_complaint(sample_email_template, mocker): - notification = create_notification(template=sample_email_template, reference="ref1") + notification = save_notification(create_notification(template=sample_email_template, reference="ref1")) mocked = mocker.patch("app.dao.notifications_dao.update_notification_status_by_reference") process_ses_results(response=ses_complaint_callback()) assert mocked.call_count == 0 @@ -101,7 +109,7 @@ def test_ses_callback_should_update_notification_status(notify_db, notify_db_ses def test_ses_callback_should_update_notification_status_when_receiving_new_delivery_receipt(sample_email_template, mocker): - notification = create_notification(template=sample_email_template, reference="ref", status="delivered") + notification = save_notification(create_notification(template=sample_email_template, reference="ref", status="delivered")) assert process_ses_results(ses_hard_bounce_callback(reference="ref")) assert get_notification_by_id(notification.id).status == "permanent-failure" @@ -280,11 +288,13 @@ def test_ses_callback_should_send_on_complaint_to_user_callback_api(sample_email callback_type="complaint", ) - notification = create_notification( - template=sample_email_template, - reference="ref1", - sent_at=datetime.utcnow(), - status="sending", + notification = save_notification( + create_notification( + template=sample_email_template, + reference="ref1", + sent_at=datetime.utcnow(), + status="sending", + ) ) response = ses_complaint_callback() assert process_ses_results(response) diff --git a/tests/app/celery/test_process_sns_receipts_tasks.py b/tests/app/celery/test_process_sns_receipts_tasks.py index 717a78804b..86e0684e6d 100644 --- a/tests/app/celery/test_process_sns_receipts_tasks.py +++ b/tests/app/celery/test_process_sns_receipts_tasks.py @@ -16,7 +16,11 @@ ) from app.notifications.callbacks import create_delivery_status_callback_data from tests.app.conftest import sample_notification as create_sample_notification -from tests.app.db import create_notification, create_service_callback_api +from tests.app.db import ( + create_notification, + create_service_callback_api, + save_notification, +) def test_process_sns_results_delivered(sample_template, notify_db, notify_db_session, mocker): @@ -135,12 +139,14 @@ def test_sns_callback_should_not_retry_if_notification_is_old(client, notify_db, def test_process_sns_results_retry_called(sample_template, mocker): - create_notification( - sample_template, - reference="ref1", - sent_at=datetime.utcnow(), - status=NOTIFICATION_SENT, - sent_by="sns", + save_notification( + create_notification( + sample_template, + reference="ref1", + sent_at=datetime.utcnow(), + status=NOTIFICATION_SENT, + sent_by="sns", + ) ) mocker.patch( @@ -155,12 +161,14 @@ def test_process_sns_results_retry_called(sample_template, mocker): def test_process_sns_results_does_not_process_other_providers(sample_template, mocker): mock_logger = mocker.patch("app.celery.process_sns_receipts_tasks.current_app.logger.exception") mock_dao = mocker.patch("app.dao.notifications_dao._update_notification_status") - create_notification( - sample_template, - reference="ref1", - sent_at=datetime.utcnow(), - status=NOTIFICATION_SENT, - sent_by="pinpoint", + save_notification( + create_notification( + sample_template, + reference="ref1", + sent_at=datetime.utcnow(), + status=NOTIFICATION_SENT, + sent_by="pinpoint", + ) ) process_sns_results(response=sns_success_callback(reference="ref1")) is None diff --git a/tests/app/celery/test_reporting_tasks.py b/tests/app/celery/test_reporting_tasks.py index 04cf8ca6ac..a9f04cf39e 100644 --- a/tests/app/celery/test_reporting_tasks.py +++ b/tests/app/celery/test_reporting_tasks.py @@ -26,6 +26,7 @@ create_rate, create_service, create_template, + save_notification, ) @@ -99,23 +100,27 @@ def test_create_nightly_billing_for_day_sms_rate_multiplier( mocker.patch("app.dao.fact_billing_dao.get_rate", side_effect=mocker_get_rate) # These are sms notifications - create_notification( - created_at=yesterday, - template=sample_template, - status="delivered", - sent_by="sns", - international=False, - rate_multiplier=1.0, - billable_units=1, + save_notification( + create_notification( + created_at=yesterday, + template=sample_template, + status="delivered", + sent_by="sns", + international=False, + rate_multiplier=1.0, + billable_units=1, + ) ) - create_notification( - created_at=yesterday, - template=sample_template, - status="delivered", - sent_by="sns", - international=False, - rate_multiplier=second_rate, - billable_units=1, + save_notification( + create_notification( + created_at=yesterday, + template=sample_template, + status="delivered", + sent_by="sns", + international=False, + rate_multiplier=second_rate, + billable_units=1, + ) ) records = FactBilling.query.all() @@ -138,23 +143,27 @@ def test_create_nightly_billing_for_day_different_templates(sample_service, samp mocker.patch("app.dao.fact_billing_dao.get_rate", side_effect=mocker_get_rate) - create_notification( - created_at=yesterday, - template=sample_template, - status="delivered", - sent_by="sns", - international=False, - rate_multiplier=1.0, - billable_units=1, + save_notification( + create_notification( + created_at=yesterday, + template=sample_template, + status="delivered", + sent_by="sns", + international=False, + rate_multiplier=1.0, + billable_units=1, + ) ) - create_notification( - created_at=yesterday, - template=sample_email_template, - status="delivered", - sent_by="ses", - international=False, - rate_multiplier=0, - billable_units=0, + save_notification( + create_notification( + created_at=yesterday, + template=sample_email_template, + status="delivered", + sent_by="ses", + international=False, + rate_multiplier=0, + billable_units=0, + ) ) records = FactBilling.query.all() @@ -181,14 +190,16 @@ def test_create_nightly_billing_for_day_different_sent_by(sample_service, sample mocker.patch("app.dao.fact_billing_dao.get_rate", side_effect=mocker_get_rate) # These are sms notifications - create_notification( - created_at=yesterday, - template=sample_template, - status="delivered", - sent_by="sns", - international=False, - rate_multiplier=1.0, - billable_units=1, + save_notification( + create_notification( + created_at=yesterday, + template=sample_template, + status="delivered", + sent_by="sns", + international=False, + rate_multiplier=1.0, + billable_units=1, + ) ) records = FactBilling.query.all() @@ -212,21 +223,25 @@ def test_create_nightly_billing_for_day_different_letter_postage(notify_db_sessi mocker.patch("app.dao.fact_billing_dao.get_rate", side_effect=mocker_get_rate) for i in range(2): + save_notification( + create_notification( + created_at=yesterday, + template=sample_letter_template, + status="delivered", + sent_by="dvla", + billable_units=2, + postage="first", + ) + ) + save_notification( create_notification( created_at=yesterday, template=sample_letter_template, status="delivered", sent_by="dvla", billable_units=2, - postage="first", + postage="second", ) - create_notification( - created_at=yesterday, - template=sample_letter_template, - status="delivered", - sent_by="dvla", - billable_units=2, - postage="second", ) records = FactBilling.query.all() @@ -255,14 +270,16 @@ def test_create_nightly_billing_for_day_letter(sample_service, sample_letter_tem mocker.patch("app.dao.fact_billing_dao.get_rate", side_effect=mocker_get_rate) - create_notification( - created_at=yesterday, - template=sample_letter_template, - status="delivered", - sent_by="dvla", - international=False, - rate_multiplier=2.0, - billable_units=2, + save_notification( + create_notification( + created_at=yesterday, + template=sample_letter_template, + status="delivered", + sent_by="dvla", + international=False, + rate_multiplier=2.0, + billable_units=2, + ) ) records = FactBilling.query.all() @@ -285,14 +302,16 @@ def test_create_nightly_billing_for_day_null_sent_by_sms(sample_service, sample_ mocker.patch("app.dao.fact_billing_dao.get_rate", side_effect=mocker_get_rate) - create_notification( - created_at=yesterday, - template=sample_template, - status="delivered", - sent_by=None, - international=False, - rate_multiplier=1.0, - billable_units=1, + save_notification( + create_notification( + created_at=yesterday, + template=sample_template, + status="delivered", + sent_by=None, + international=False, + rate_multiplier=1.0, + billable_units=1, + ) ) records = FactBilling.query.all() @@ -343,29 +362,35 @@ def test_create_nightly_billing_for_day_use_BST(sample_service, sample_template, mocker.patch("app.dao.fact_billing_dao.get_rate", side_effect=mocker_get_rate) # too late - create_notification( - created_at=datetime(2018, 3, 25, 23, 1), - template=sample_template, - status="delivered", - rate_multiplier=1.0, - billable_units=1, + save_notification( + create_notification( + created_at=datetime(2018, 3, 25, 23, 1), + template=sample_template, + status="delivered", + rate_multiplier=1.0, + billable_units=1, + ) ) - create_notification( - created_at=datetime(2018, 3, 25, 22, 59), - template=sample_template, - status="delivered", - rate_multiplier=1.0, - billable_units=2, + save_notification( + create_notification( + created_at=datetime(2018, 3, 25, 22, 59), + template=sample_template, + status="delivered", + rate_multiplier=1.0, + billable_units=2, + ) ) # too early - create_notification( - created_at=datetime(2018, 3, 24, 23, 59), - template=sample_template, - status="delivered", - rate_multiplier=1.0, - billable_units=4, + save_notification( + create_notification( + created_at=datetime(2018, 3, 24, 23, 59), + template=sample_template, + status="delivered", + rate_multiplier=1.0, + billable_units=4, + ) ) assert Notification.query.count() == 3 @@ -385,14 +410,16 @@ def test_create_nightly_billing_for_day_update_when_record_exists(sample_service mocker.patch("app.dao.fact_billing_dao.get_rate", side_effect=mocker_get_rate) - create_notification( - created_at=datetime.now() - timedelta(days=1), - template=sample_template, - status="delivered", - sent_by=None, - international=False, - rate_multiplier=1.0, - billable_units=1, + save_notification( + create_notification( + created_at=datetime.now() - timedelta(days=1), + template=sample_template, + status="delivered", + sent_by=None, + international=False, + rate_multiplier=1.0, + billable_units=1, + ) ) records = FactBilling.query.all() @@ -406,14 +433,16 @@ def test_create_nightly_billing_for_day_update_when_record_exists(sample_service assert records[0].billable_units == 1 assert not records[0].updated_at - create_notification( - created_at=datetime.now() - timedelta(days=1), - template=sample_template, - status="delivered", - sent_by=None, - international=False, - rate_multiplier=1.0, - billable_units=1, + save_notification( + create_notification( + created_at=datetime.now() - timedelta(days=1), + template=sample_template, + status="delivered", + sent_by=None, + international=False, + rate_multiplier=1.0, + billable_units=1, + ) ) # run again, make sure create_nightly_billing() updates with no error @@ -432,25 +461,31 @@ def test_create_nightly_notification_status_for_day(notify_db_session): third_service = create_service(service_name="third Service") third_template = create_template(service=third_service, template_type="letter") - create_notification(template=first_template, status="delivered") - create_notification( - template=first_template, - status="delivered", - created_at=datetime(2019, 1, 1, 12, 0), + save_notification(create_notification(template=first_template, status="delivered")) + save_notification( + create_notification( + template=first_template, + status="delivered", + created_at=datetime(2019, 1, 1, 12, 0), + ) ) - create_notification(template=second_template, status="temporary-failure") - create_notification( - template=second_template, - status="temporary-failure", - created_at=datetime(2019, 1, 1, 12, 0), + save_notification(create_notification(template=second_template, status="temporary-failure")) + save_notification( + create_notification( + template=second_template, + status="temporary-failure", + created_at=datetime(2019, 1, 1, 12, 0), + ) ) - create_notification(template=third_template, status="created") - create_notification( - template=third_template, - status="created", - created_at=datetime(2019, 1, 1, 12, 0), + save_notification(create_notification(template=third_template, status="created")) + save_notification( + create_notification( + template=third_template, + status="created", + created_at=datetime(2019, 1, 1, 12, 0), + ) ) assert len(FactNotificationStatus.query.all()) == 0 @@ -470,12 +505,14 @@ def test_create_nightly_notification_status_for_day(notify_db_session): def test_create_nightly_notification_status_for_day_respects_local_timezone( sample_template, ): - create_notification(sample_template, status="delivered", created_at=datetime(2019, 4, 2, 5, 0)) # too new + save_notification(create_notification(sample_template, status="delivered", created_at=datetime(2019, 4, 2, 5, 0))) # too new - create_notification(sample_template, status="created", created_at=datetime(2019, 4, 2, 6, 59)) - create_notification(sample_template, status="created", created_at=datetime(2019, 4, 1, 5, 59)) + save_notification(create_notification(sample_template, status="created", created_at=datetime(2019, 4, 2, 6, 59))) + save_notification(create_notification(sample_template, status="created", created_at=datetime(2019, 4, 1, 5, 59))) - create_notification(sample_template, status="delivered", created_at=datetime(2019, 3, 30, 3, 59)) # too old + save_notification( + create_notification(sample_template, status="delivered", created_at=datetime(2019, 3, 30, 3, 59)) + ) # too old create_nightly_notification_status_for_day("2019-04-01") diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index 7f862ae6f6..fd74861c9b 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -33,19 +33,27 @@ ) from app.v2.errors import JobIncompleteError from tests.app.conftest import sample_job as create_sample_job -from tests.app.db import create_job, create_notification, create_template +from tests.app.db import ( + create_job, + create_notification, + create_template, + save_notification, + save_scheduled_notification, +) def _create_slow_delivery_notification(template, provider="sns"): now = datetime.utcnow() five_minutes_from_now = now + timedelta(minutes=5) - create_notification( - template=template, - status="delivered", - sent_by=provider, - updated_at=five_minutes_from_now, - sent_at=now, + save_notification( + create_notification( + template=template, + status="delivered", + sent_by=provider, + updated_at=five_minutes_from_now, + sent_at=now, + ) ) @@ -154,10 +162,14 @@ def test_switch_providers_on_slow_delivery_switches_once_then_does_not_switch_if @freeze_time("2017-05-01 14:00:00") def test_should_send_all_scheduled_notifications_to_deliver_queue(sample_template, mocker): mocked = mocker.patch("app.celery.provider_tasks.deliver_sms") - message_to_deliver = create_notification(template=sample_template, scheduled_for="2017-05-01 13:15") - create_notification(template=sample_template, scheduled_for="2017-05-01 10:15", status="delivered") - create_notification(template=sample_template) - create_notification(template=sample_template, scheduled_for="2017-05-01 14:15") + message_to_deliver = save_scheduled_notification( + create_notification(template=sample_template), scheduled_for="2017-05-01 13:15" + ) + save_scheduled_notification( + create_notification(template=sample_template, status="delivered"), scheduled_for="2017-05-01 10:15" + ) + save_notification(create_notification(template=sample_template)) + save_scheduled_notification(create_notification(template=sample_template), scheduled_for="2017-05-01 14:15") scheduled_notifications = dao_get_scheduled_notifications() assert len(scheduled_notifications) == 1 @@ -174,11 +186,11 @@ def test_check_job_status_task_raises_job_incomplete_error(mocker, sample_templa job = create_job( template=sample_template, notification_count=3, - created_at=datetime.utcnow() - timedelta(minutes=61), - processing_started=datetime.utcnow() - timedelta(minutes=61), + created_at=datetime.utcnow() - timedelta(minutes=121), + processing_started=datetime.utcnow() - timedelta(minutes=121), job_status=JOB_STATUS_IN_PROGRESS, ) - create_notification(template=sample_template, job=job) + save_notification(create_notification(template=sample_template, job=job)) with pytest.raises(expected_exception=JobIncompleteError) as e: check_job_status() assert e.value.message == "Job(s) ['{}'] have not completed.".format(str(job.id)) @@ -196,8 +208,8 @@ def test_check_job_status_task_raises_job_incomplete_error_when_scheduled_job_is template=sample_template, notification_count=3, created_at=datetime.utcnow() - timedelta(hours=2), - scheduled_for=datetime.utcnow() - timedelta(minutes=61), - processing_started=datetime.utcnow() - timedelta(minutes=61), + scheduled_for=datetime.utcnow() - timedelta(minutes=121), + processing_started=datetime.utcnow() - timedelta(minutes=121), job_status=JOB_STATUS_IN_PROGRESS, ) with pytest.raises(expected_exception=JobIncompleteError) as e: @@ -217,16 +229,16 @@ def test_check_job_status_task_raises_job_incomplete_error_for_multiple_jobs(moc template=sample_template, notification_count=3, created_at=datetime.utcnow() - timedelta(hours=2), - scheduled_for=datetime.utcnow() - timedelta(minutes=61), - processing_started=datetime.utcnow() - timedelta(minutes=61), + scheduled_for=datetime.utcnow() - timedelta(minutes=121), + processing_started=datetime.utcnow() - timedelta(minutes=121), job_status=JOB_STATUS_IN_PROGRESS, ) job_2 = create_job( template=sample_template, notification_count=3, created_at=datetime.utcnow() - timedelta(hours=2), - scheduled_for=datetime.utcnow() - timedelta(minutes=61), - processing_started=datetime.utcnow() - timedelta(minutes=61), + scheduled_for=datetime.utcnow() - timedelta(minutes=121), + processing_started=datetime.utcnow() - timedelta(minutes=121), job_status=JOB_STATUS_IN_PROGRESS, ) with pytest.raises(expected_exception=JobIncompleteError) as e: @@ -247,15 +259,15 @@ def test_check_job_status_task_only_sends_old_tasks(mocker, sample_template): template=sample_template, notification_count=3, created_at=datetime.utcnow() - timedelta(hours=2), - scheduled_for=datetime.utcnow() - timedelta(minutes=61), - processing_started=datetime.utcnow() - timedelta(minutes=61), + scheduled_for=datetime.utcnow() - timedelta(minutes=121), + processing_started=datetime.utcnow() - timedelta(minutes=121), job_status=JOB_STATUS_IN_PROGRESS, ) job_2 = create_job( template=sample_template, notification_count=3, - created_at=datetime.utcnow() - timedelta(minutes=61), - processing_started=datetime.utcnow() - timedelta(minutes=59), + created_at=datetime.utcnow() - timedelta(minutes=121), + processing_started=datetime.utcnow() - timedelta(minutes=119), job_status=JOB_STATUS_IN_PROGRESS, ) with pytest.raises(expected_exception=JobIncompleteError) as e: @@ -277,15 +289,15 @@ def test_check_job_status_task_sets_jobs_to_error(mocker, sample_template): template=sample_template, notification_count=3, created_at=datetime.utcnow() - timedelta(hours=2), - scheduled_for=datetime.utcnow() - timedelta(minutes=61), - processing_started=datetime.utcnow() - timedelta(minutes=61), + scheduled_for=datetime.utcnow() - timedelta(minutes=121), + processing_started=datetime.utcnow() - timedelta(minutes=121), job_status=JOB_STATUS_IN_PROGRESS, ) job_2 = create_job( template=sample_template, notification_count=3, - created_at=datetime.utcnow() - timedelta(minutes=61), - processing_started=datetime.utcnow() - timedelta(minutes=59), + created_at=datetime.utcnow() - timedelta(minutes=121), + processing_started=datetime.utcnow() - timedelta(minutes=119), job_status=JOB_STATUS_IN_PROGRESS, ) with pytest.raises(expected_exception=JobIncompleteError) as e: @@ -311,29 +323,37 @@ def test_replay_created_notifications(notify_db_session, sample_service, mocker) email_template = create_template(service=sample_service, template_type="email") older_than = (60 * 60 * 4) + (60 * 15) # 4 hours 15 minutes # notifications expected to be resent - old_sms = create_notification( - template=sms_template, - created_at=datetime.utcnow() - timedelta(seconds=older_than), - status="created", - ) - old_email = create_notification( - template=email_template, - created_at=datetime.utcnow() - timedelta(seconds=older_than), - status="created", + old_sms = save_notification( + create_notification( + template=sms_template, + created_at=datetime.utcnow() - timedelta(seconds=older_than), + status="created", + ) + ) + old_email = save_notification( + create_notification( + template=email_template, + created_at=datetime.utcnow() - timedelta(seconds=older_than), + status="created", + ) ) # notifications that are not to be resent - create_notification( - template=sms_template, - created_at=datetime.utcnow() - timedelta(seconds=older_than), - status="sending", - ) - create_notification( - template=email_template, - created_at=datetime.utcnow() - timedelta(seconds=older_than), - status="delivered", - ) - create_notification(template=sms_template, created_at=datetime.utcnow(), status="created") - create_notification(template=email_template, created_at=datetime.utcnow(), status="created") + save_notification( + create_notification( + template=sms_template, + created_at=datetime.utcnow() - timedelta(seconds=older_than), + status="sending", + ) + ) + save_notification( + create_notification( + template=email_template, + created_at=datetime.utcnow() - timedelta(seconds=older_than), + status="delivered", + ) + ) + save_notification(create_notification(template=sms_template, created_at=datetime.utcnow(), status="created")) + save_notification(create_notification(template=email_template, created_at=datetime.utcnow(), status="created")) replay_created_notifications() email_delivery_queue.assert_called_once_with([str(old_email.id)], queue="send-email-tasks") @@ -365,25 +385,33 @@ def test_check_precompiled_letter_state(mocker, sample_letter_template): mock_logger = mocker.patch("app.celery.tasks.current_app.logger.exception") mock_create_ticket = mocker.patch("app.celery.nightly_tasks.zendesk_client.create_ticket") - create_notification( - template=sample_letter_template, - status=NOTIFICATION_PENDING_VIRUS_CHECK, - created_at=datetime.utcnow() - timedelta(seconds=5400), - ) - create_notification( - template=sample_letter_template, - status=NOTIFICATION_DELIVERED, - created_at=datetime.utcnow() - timedelta(seconds=6000), - ) - noti_1 = create_notification( - template=sample_letter_template, - status=NOTIFICATION_PENDING_VIRUS_CHECK, - created_at=datetime.utcnow() - timedelta(seconds=5401), - ) - noti_2 = create_notification( - template=sample_letter_template, - status=NOTIFICATION_PENDING_VIRUS_CHECK, - created_at=datetime.utcnow() - timedelta(seconds=70000), + save_notification( + create_notification( + template=sample_letter_template, + status=NOTIFICATION_PENDING_VIRUS_CHECK, + created_at=datetime.utcnow() - timedelta(seconds=5400), + ) + ) + save_notification( + create_notification( + template=sample_letter_template, + status=NOTIFICATION_DELIVERED, + created_at=datetime.utcnow() - timedelta(seconds=6000), + ) + ) + noti_1 = save_notification( + create_notification( + template=sample_letter_template, + status=NOTIFICATION_PENDING_VIRUS_CHECK, + created_at=datetime.utcnow() - timedelta(seconds=5401), + ) + ) + noti_2 = save_notification( + create_notification( + template=sample_letter_template, + status=NOTIFICATION_PENDING_VIRUS_CHECK, + created_at=datetime.utcnow() - timedelta(seconds=70000), + ) ) check_precompiled_letter_state() @@ -406,16 +434,18 @@ def test_check_templated_letter_state_during_bst(mocker, sample_letter_template) mock_logger = mocker.patch("app.celery.tasks.current_app.logger.exception") mock_create_ticket = mocker.patch("app.celery.nightly_tasks.zendesk_client.create_ticket") - noti_1 = create_notification(template=sample_letter_template, updated_at=datetime(2019, 5, 1, 12, 0)) - noti_2 = create_notification(template=sample_letter_template, updated_at=datetime(2019, 5, 29, 16, 29)) - create_notification(template=sample_letter_template, updated_at=datetime(2019, 5, 29, 16, 30)) - create_notification(template=sample_letter_template, updated_at=datetime(2019, 5, 29, 17, 29)) - create_notification( - template=sample_letter_template, - status="delivered", - updated_at=datetime(2019, 5, 28, 10, 0), + noti_1 = save_notification(create_notification(template=sample_letter_template, updated_at=datetime(2019, 5, 1, 12, 0))) + noti_2 = save_notification(create_notification(template=sample_letter_template, updated_at=datetime(2019, 5, 29, 16, 29))) + save_notification(create_notification(template=sample_letter_template, updated_at=datetime(2019, 5, 29, 16, 30))) + save_notification(create_notification(template=sample_letter_template, updated_at=datetime(2019, 5, 29, 17, 29))) + save_notification( + create_notification( + template=sample_letter_template, + status="delivered", + updated_at=datetime(2019, 5, 28, 10, 0), + ) ) - create_notification(template=sample_letter_template, updated_at=datetime(2019, 5, 30, 10, 0)) + save_notification(create_notification(template=sample_letter_template, updated_at=datetime(2019, 5, 30, 10, 0))) check_templated_letter_state() @@ -438,16 +468,18 @@ def test_check_templated_letter_state_during_utc(mocker, sample_letter_template) mock_logger = mocker.patch("app.celery.tasks.current_app.logger.exception") mock_create_ticket = mocker.patch("app.celery.nightly_tasks.zendesk_client.create_ticket") - noti_1 = create_notification(template=sample_letter_template, updated_at=datetime(2018, 12, 1, 12, 0)) - noti_2 = create_notification(template=sample_letter_template, updated_at=datetime(2019, 1, 29, 17, 29)) - create_notification(template=sample_letter_template, updated_at=datetime(2019, 1, 29, 17, 30)) - create_notification(template=sample_letter_template, updated_at=datetime(2019, 1, 29, 18, 29)) - create_notification( - template=sample_letter_template, - status="delivered", - updated_at=datetime(2019, 1, 29, 10, 0), - ) - create_notification(template=sample_letter_template, updated_at=datetime(2019, 1, 30, 10, 0)) + noti_1 = save_notification(create_notification(template=sample_letter_template, updated_at=datetime(2018, 12, 1, 12, 0))) + noti_2 = save_notification(create_notification(template=sample_letter_template, updated_at=datetime(2019, 1, 29, 17, 29))) + save_notification(create_notification(template=sample_letter_template, updated_at=datetime(2019, 1, 29, 17, 30))) + save_notification(create_notification(template=sample_letter_template, updated_at=datetime(2019, 1, 29, 18, 29))) + save_notification( + create_notification( + template=sample_letter_template, + status="delivered", + updated_at=datetime(2019, 1, 29, 10, 0), + ) + ) + save_notification(create_notification(template=sample_letter_template, updated_at=datetime(2019, 1, 30, 10, 0))) check_templated_letter_state() diff --git a/tests/app/celery/test_service_callback_tasks.py b/tests/app/celery/test_service_callback_tasks.py index e59eae061e..538cba4874 100644 --- a/tests/app/celery/test_service_callback_tasks.py +++ b/tests/app/celery/test_service_callback_tasks.py @@ -16,6 +16,7 @@ create_service, create_service_callback_api, create_template, + save_notification, ) @@ -25,12 +26,14 @@ def test_send_delivery_status_to_service_post_https_request_to_service_with_encr callback_api, template = _set_up_test_data(notification_type, "delivery_status") datestr = datetime(2017, 6, 20) - notification = create_notification( - template=template, - created_at=datestr, - updated_at=datestr, - sent_at=datestr, - status="sent", + notification = save_notification( + create_notification( + template=template, + created_at=datestr, + updated_at=datestr, + sent_at=datestr, + status="sent", + ) ) encrypted_status_update = _set_up_data_for_status_update(callback_api, notification) with requests_mock.Mocker() as request_mock: @@ -92,12 +95,14 @@ def test__send_data_to_service_callback_api_retries_if_request_returns_500_with_ ): callback_api, template = _set_up_test_data(notification_type, "delivery_status") datestr = datetime(2017, 6, 20) - notification = create_notification( - template=template, - created_at=datestr, - updated_at=datestr, - sent_at=datestr, - status="sent", + notification = save_notification( + create_notification( + template=template, + created_at=datestr, + updated_at=datestr, + sent_at=datestr, + status="sent", + ) ) encrypted_data = _set_up_data_for_status_update(callback_api, notification) mocked = mocker.patch("app.celery.service_callback_tasks.send_delivery_status_to_service.retry") @@ -115,12 +120,14 @@ def test__send_data_to_service_callback_api_does_not_retry_if_request_returns_40 ): callback_api, template = _set_up_test_data(notification_type, "delivery_status") datestr = datetime(2017, 6, 20) - notification = create_notification( - template=template, - created_at=datestr, - updated_at=datestr, - sent_at=datestr, - status="sent", + notification = save_notification( + create_notification( + template=template, + created_at=datestr, + updated_at=datestr, + sent_at=datestr, + status="sent", + ) ) encrypted_data = _set_up_data_for_status_update(callback_api, notification) mocked = mocker.patch("app.celery.service_callback_tasks.send_delivery_status_to_service.retry") @@ -134,12 +141,14 @@ def test__send_data_to_service_callback_api_does_not_retry_if_request_returns_40 def test_send_delivery_status_to_service_succeeds_if_sent_at_is_none(notify_db_session, mocker): callback_api, template = _set_up_test_data("email", "delivery_status") datestr = datetime(2017, 6, 20) - notification = create_notification( - template=template, - created_at=datestr, - updated_at=datestr, - sent_at=None, - status="technical-failure", + notification = save_notification( + create_notification( + template=template, + created_at=datestr, + updated_at=datestr, + sent_at=None, + status="technical-failure", + ) ) encrypted_data = _set_up_data_for_status_update(callback_api, notification) mocked = mocker.patch("app.celery.service_callback_tasks.send_delivery_status_to_service.retry") diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 62f80f3383..5648e1197f 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -59,6 +59,7 @@ create_service_with_defined_sms_sender, create_template, create_user, + save_notification, ) from tests.conftest import set_config_values @@ -197,7 +198,7 @@ def test_should_not_process_sms_job_if_would_exceed_send_limits_inc_today(notify template = create_template(service=service) job = create_job(template=template) - create_notification(template=template, job=job) + save_notification(create_notification(template=template, job=job)) mocker.patch("app.celery.tasks.s3.get_job_from_s3", return_value=load_example_csv("sms")) mocker.patch("app.celery.tasks.process_row") @@ -216,7 +217,7 @@ def test_should_not_process_email_job_if_would_exceed_send_limits_inc_today(noti template = create_template(service=service, template_type=template_type) job = create_job(template=template) - create_notification(template=template, job=job) + save_notification(create_notification(template=template, job=job)) mocker.patch("app.celery.tasks.s3.get_job_from_s3") mocker.patch("app.celery.tasks.process_row") @@ -1658,8 +1659,8 @@ def test_process_incomplete_job_sms(mocker, sample_template): job_status=JOB_STATUS_ERROR, ) - create_notification(sample_template, job, 0) - create_notification(sample_template, job, 1) + save_notification(create_notification(sample_template, job, 0)) + save_notification(create_notification(sample_template, job, 1)) assert Notification.query.filter(Notification.job_id == job.id).count() == 2 @@ -1689,16 +1690,16 @@ def test_process_incomplete_job_with_notifications_all_sent(mocker, sample_templ job_status=JOB_STATUS_ERROR, ) - create_notification(sample_template, job, 0) - create_notification(sample_template, job, 1) - create_notification(sample_template, job, 2) - create_notification(sample_template, job, 3) - create_notification(sample_template, job, 4) - create_notification(sample_template, job, 5) - create_notification(sample_template, job, 6) - create_notification(sample_template, job, 7) - create_notification(sample_template, job, 8) - create_notification(sample_template, job, 9) + save_notification(create_notification(sample_template, job, 0)) + save_notification(create_notification(sample_template, job, 1)) + save_notification(create_notification(sample_template, job, 2)) + save_notification(create_notification(sample_template, job, 3)) + save_notification(create_notification(sample_template, job, 4)) + save_notification(create_notification(sample_template, job, 5)) + save_notification(create_notification(sample_template, job, 6)) + save_notification(create_notification(sample_template, job, 7)) + save_notification(create_notification(sample_template, job, 8)) + save_notification(create_notification(sample_template, job, 9)) assert Notification.query.filter(Notification.job_id == job.id).count() == 10 @@ -1727,9 +1728,9 @@ def test_process_incomplete_jobs_sms(mocker, sample_template): processing_started=datetime.utcnow() - timedelta(minutes=31), job_status=JOB_STATUS_ERROR, ) - create_notification(sample_template, job, 0) - create_notification(sample_template, job, 1) - create_notification(sample_template, job, 2) + save_notification(create_notification(sample_template, job, 0)) + save_notification(create_notification(sample_template, job, 1)) + save_notification(create_notification(sample_template, job, 2)) assert Notification.query.filter(Notification.job_id == job.id).count() == 3 @@ -1742,11 +1743,11 @@ def test_process_incomplete_jobs_sms(mocker, sample_template): job_status=JOB_STATUS_ERROR, ) - create_notification(sample_template, job2, 0) - create_notification(sample_template, job2, 1) - create_notification(sample_template, job2, 2) - create_notification(sample_template, job2, 3) - create_notification(sample_template, job2, 4) + save_notification(create_notification(sample_template, job2, 0)) + save_notification(create_notification(sample_template, job2, 1)) + save_notification(create_notification(sample_template, job2, 2)) + save_notification(create_notification(sample_template, job2, 3)) + save_notification(create_notification(sample_template, job2, 4)) assert Notification.query.filter(Notification.job_id == job2.id).count() == 5 @@ -1835,8 +1836,8 @@ def test_process_incomplete_job_email(mocker, sample_email_template): job_status=JOB_STATUS_ERROR, ) - create_notification(sample_email_template, job, 0) - create_notification(sample_email_template, job, 1) + save_notification(create_notification(sample_email_template, job, 0)) + save_notification(create_notification(sample_email_template, job, 1)) assert Notification.query.filter(Notification.job_id == job.id).count() == 2 @@ -1865,8 +1866,8 @@ def test_process_incomplete_job_letter(mocker, sample_letter_template): job_status=JOB_STATUS_ERROR, ) - create_notification(sample_letter_template, job, 0) - create_notification(sample_letter_template, job, 1) + save_notification(create_notification(sample_letter_template, job, 0)) + save_notification(create_notification(sample_letter_template, job, 1)) assert Notification.query.filter(Notification.job_id == job.id).count() == 2 @@ -1905,8 +1906,8 @@ def test_process_incomplete_jobs_sets_status_to_in_progress_and_resets_processin def test_process_returned_letters_list(sample_letter_template): - create_notification(sample_letter_template, reference="ref1") - create_notification(sample_letter_template, reference="ref2") + save_notification(create_notification(sample_letter_template, reference="ref1")) + save_notification(create_notification(sample_letter_template, reference="ref2")) process_returned_letters_list(["ref1", "ref2", "unknown-ref"]) diff --git a/tests/app/clients/test_zendesk.py b/tests/app/clients/test_zendesk.py new file mode 100644 index 0000000000..3e853caf88 --- /dev/null +++ b/tests/app/clients/test_zendesk.py @@ -0,0 +1,187 @@ +import base64 +from typing import Any, Dict + +import pytest +import requests_mock +from flask import Flask +from requests import HTTPError + +from app.clients.zendesk import Zendesk +from app.user.contact_request import ContactRequest + + +def test_send_ticket_go_live_request(notify_api: Flask): + def match_json(request): + expected = { + "ticket": { + "subject": "Support Request", + "description": "t6 just requested to go live.

" + "- Department/org: department_org_name
" + "- Intended recipients: internal, external, public
" + "- Purpose: main_use_case
" + "- Notification types: email, sms
" + "- Expected monthly volume: 100k+
" + "---
" + "http://localhost:6012/services/8624bd36-b70b-4d4b-a459-13e1f4770b92", + "email": "test@email.com", + "tags": ["notification_api"], + } + } + + encoded_auth = base64.b64encode(b"test@email.com/token:zendesk-api-key").decode("ascii") + json_matches = request.json() == expected + basic_auth_header = request.headers.get("Authorization") == f"Basic {encoded_auth}" + + return json_matches and basic_auth_header + + with requests_mock.mock() as rmock: + rmock.request( + "POST", + "https://zendesk-test.com/api/v2/tickets", + additional_matcher=match_json, + status_code=201, + ) + data: Dict[str, Any] = { + "email_address": "test@email.com", + "name": "name", + "department_org_name": "department_org_name", + "intended_recipients": "internal, external, public", + "main_use_case": "main_use_case", + "friendly_support_type": "Support Request", + "support_type": "go_live_request", + "service_name": "t6", + "service_id": "8624bd36-b70b-4d4b-a459-13e1f4770b92", + "service_url": "http://localhost:6012/services/8624bd36-b70b-4d4b-a459-13e1f4770b92", + "notification_types": "email, sms", + "expected_volume": "100k+", + } + with notify_api.app_context(): + Zendesk(ContactRequest(**data)).send_ticket() + + +def test_send_ticket_branding_request(notify_api: Flask): + def match_json(request): + expected = { + "ticket": { + "subject": "Branding request", + "description": "A new logo has been uploaded by name (test@email.com) for the following service:
" + "- Service id: 8624bd36-b70b-4d4b-a459-13e1f4770b92
" + "- Service name: t6
" + "- Logo filename: branding_url
" + "

" + "Un nouveau logo a été téléchargé par name (test@email.com) pour le service suivant :
" + "- Identifiant du service : 8624bd36-b70b-4d4b-a459-13e1f4770b92
" + "- Nom du service : t6
" + "- Nom du fichier du logo : branding_url", + "email": "test@email.com", + "tags": ["notification_api"], + } + } + + encoded_auth = base64.b64encode(b"test@email.com/token:zendesk-api-key").decode("ascii") + json_matches = request.json() == expected + basic_auth_header = request.headers.get("Authorization") == f"Basic {encoded_auth}" + + return json_matches and basic_auth_header + + with requests_mock.mock() as rmock: + rmock.request( + "POST", + "https://zendesk-test.com/api/v2/tickets", + additional_matcher=match_json, + status_code=201, + ) + data: Dict[str, Any] = { + "email_address": "test@email.com", + "name": "name", + "friendly_support_type": "Branding request", + "support_type": "branding_request", + "service_name": "t6", + "service_id": "8624bd36-b70b-4d4b-a459-13e1f4770b92", + "branding_url": "branding_url", + } + with notify_api.app_context(): + Zendesk(ContactRequest(**data)).send_ticket() + + +def test_send_ticket_other(notify_api: Flask): + def match_json(request): + expected = { + "ticket": {"subject": "Support Request", "description": "", "email": "test@email.com", "tags": ["notification_api"]} + } + + encoded_auth = base64.b64encode(b"test@email.com/token:zendesk-api-key").decode("ascii") + json_matches = request.json() == expected + basic_auth_header = request.headers.get("Authorization") == f"Basic {encoded_auth}" + + return json_matches and basic_auth_header + + with requests_mock.mock() as rmock: + rmock.request( + "POST", + "https://zendesk-test.com/api/v2/tickets", + additional_matcher=match_json, + status_code=201, + ) + + with notify_api.app_context(): + Zendesk(ContactRequest(email_address="test@email.com")).send_ticket() + + +def test_send_ticket_user_profile(notify_api: Flask): + def match_json(request): + expected = { + "ticket": { + "subject": "Support Request", + "description": "

---

user_profile", + "email": "test@email.com", + "tags": ["notification_api"], + } + } + + encoded_auth = base64.b64encode(b"test@email.com/token:zendesk-api-key").decode("ascii") + json_matches = request.json() == expected + basic_auth_header = request.headers.get("Authorization") == f"Basic {encoded_auth}" + + return json_matches and basic_auth_header + + with requests_mock.mock() as rmock: + rmock.request( + "POST", + "https://zendesk-test.com/api/v2/tickets", + additional_matcher=match_json, + status_code=201, + ) + + with notify_api.app_context(): + Zendesk( + ContactRequest( + email_address="test@email.com", + user_profile="user_profile", + ) + ).send_ticket() + + +def test_send_ticket_unknown_error(notify_api: Flask): + def match_json(request): + expected = { + "ticket": {"subject": "Support Request", "description": "", "email": "test@email.com", "tags": ["notification_api"]} + } + + encoded_auth = base64.b64encode(b"test@email.com/token:zendesk-api-key").decode("ascii") + json_matches = request.json() == expected + basic_auth_header = request.headers.get("Authorization") == f"Basic {encoded_auth}" + + return json_matches and basic_auth_header + + with requests_mock.mock() as rmock: + rmock.request( + "POST", + "https://zendesk-test.com/api/v2/tickets", + additional_matcher=match_json, + status_code=403, + ) + + with notify_api.app_context(): + with pytest.raises(HTTPError): + Zendesk(ContactRequest(email_address="test@email.com")).send_ticket() diff --git a/tests/app/complaint/test_complaint_rest.py b/tests/app/complaint/test_complaint_rest.py index 75e42bfa8d..2a19642c21 100644 --- a/tests/app/complaint/test_complaint_rest.py +++ b/tests/app/complaint/test_complaint_rest.py @@ -10,13 +10,14 @@ create_notification, create_service, create_template, + save_notification, ) def test_get_all_complaints_returns_complaints_for_multiple_services(client, notify_db_session): service = create_service(service_name="service1") template = create_template(service=service) - notification = create_notification(template=template) + notification = save_notification(create_notification(template=template)) complaint_1 = create_complaint() # default service complaint_2 = create_complaint(service=service, notification=notification) diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 6be3e0e4f0..78d85dd740 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -63,6 +63,7 @@ create_service, create_template, create_user, + save_notification, ) @@ -494,19 +495,21 @@ def sample_notification_with_job( template = create_template(service=service) if job is None: job = create_job(template=template) - return create_notification( - template=template, - job=job, - job_row_number=job_row_number if job_row_number is not None else None, - to_field=to_field, - status=status, - reference=reference, - created_at=created_at, - sent_at=sent_at, - billable_units=billable_units, - personalisation=personalisation, - api_key=api_key, - key_type=key_type, + return save_notification( + create_notification( + template=template, + job=job, + job_row_number=job_row_number if job_row_number is not None else None, + to_field=to_field, + status=status, + reference=reference, + created_at=created_at, + sent_at=sent_at, + billable_units=billable_units, + personalisation=personalisation, + api_key=api_key, + key_type=key_type, + ) ) @@ -612,7 +615,7 @@ def sample_letter_notification(sample_letter_template): "address_line_6": "A6", "postcode": "A_POST", } - return create_notification(sample_letter_template, reference="foo", personalisation=address) + return save_notification(create_notification(sample_letter_template, reference="foo", personalisation=address)) @pytest.fixture(scope="function") diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index e207f2762b..54a1781831 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -8,6 +8,7 @@ from sqlalchemy.orm.exc import NoResultFound from app.dao.notifications_dao import ( + bulk_insert_notifications, dao_create_notification, dao_created_scheduled_notification, dao_delete_notifications_by_id, @@ -59,6 +60,8 @@ create_notification_history, create_service, create_template, + save_notification, + save_scheduled_notification, ) @@ -117,7 +120,7 @@ def test_should_by_able_to_update_status_by_id(sample_template, sample_job): def test_should_not_update_status_by_id_if_not_sending_and_does_not_update_job( sample_job, ): - notification = create_notification(template=sample_job.template, status="delivered", job=sample_job) + notification = save_notification(create_notification(template=sample_job.template, status="delivered", job=sample_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" @@ -127,11 +130,13 @@ def test_should_not_update_status_by_id_if_not_sending_and_does_not_update_job( def test_should_not_update_status_by_reference_if_not_sending_and_does_not_update_job( sample_job, ): - notification = create_notification( - template=sample_job.template, - status="delivered", - reference="reference", - job=sample_job, + notification = save_notification( + create_notification( + template=sample_job.template, + status="delivered", + reference="reference", + job=sample_job, + ) ) assert Notification.query.get(notification.id).status == "delivered" assert not update_notification_status_by_reference("reference", "failed") @@ -147,7 +152,7 @@ def test_should_update_status_by_id_if_created(sample_template, sample_notificat def test_should_update_status_by_id_if_pending_virus_check(sample_letter_template): - notification = create_notification(template=sample_letter_template, status="pending-virus-check") + notification = save_notification(create_notification(template=sample_letter_template, status="pending-virus-check")) assert Notification.query.get(notification.id).status == "pending-virus-check" updated = update_notification_status_by_id(notification.id, "cancelled") assert Notification.query.get(notification.id).status == "cancelled" @@ -155,7 +160,7 @@ def test_should_update_status_by_id_if_pending_virus_check(sample_letter_templat def test_should_update_status_by_id_and_set_sent_by(sample_template): - notification = create_notification(template=sample_template, status="sending") + notification = save_notification(create_notification(template=sample_template, status="sending")) updated = update_notification_status_by_id(notification.id, "delivered", sent_by="mmg") assert updated.status == "delivered" @@ -165,7 +170,7 @@ def test_should_update_status_by_id_and_set_sent_by(sample_template): def test_should_not_update_status_by_reference_if_from_country_with_no_delivery_receipts( sample_template, ): - notification = create_notification(sample_template, status=NOTIFICATION_SENT, reference="foo") + notification = save_notification(create_notification(sample_template, status=NOTIFICATION_SENT, reference="foo")) res = update_notification_status_by_reference("foo", "failed") @@ -176,11 +181,13 @@ def test_should_not_update_status_by_reference_if_from_country_with_no_delivery_ def test_should_not_update_status_by_id_if_sent_to_country_with_unknown_delivery_receipts( sample_template, ): - notification = create_notification( - sample_template, - status=NOTIFICATION_SENT, - international=True, - phone_prefix="249", # sudan has no delivery receipts (or at least, that we know about) + notification = save_notification( + create_notification( + sample_template, + status=NOTIFICATION_SENT, + international=True, + phone_prefix="249", # sudan has no delivery receipts (or at least, that we know about) + ) ) res = update_notification_status_by_id(notification.id, "delivered") @@ -192,11 +199,13 @@ def test_should_not_update_status_by_id_if_sent_to_country_with_unknown_delivery def test_should_not_update_status_by_id_if_sent_to_country_with_carrier_delivery_receipts( sample_template, ): - notification = create_notification( - sample_template, - status=NOTIFICATION_SENT, - international=True, - phone_prefix="1", # americans only have carrier delivery receipts + notification = save_notification( + create_notification( + sample_template, + status=NOTIFICATION_SENT, + international=True, + phone_prefix="1", # americans only have carrier delivery receipts + ) ) res = update_notification_status_by_id(notification.id, "delivered") @@ -208,11 +217,13 @@ def test_should_not_update_status_by_id_if_sent_to_country_with_carrier_delivery def test_should_not_update_status_by_id_if_sent_to_country_with_delivery_receipts( sample_template, ): - notification = create_notification( - sample_template, - status=NOTIFICATION_SENT, - international=True, - phone_prefix="7", # russians have full delivery receipts + notification = save_notification( + create_notification( + sample_template, + status=NOTIFICATION_SENT, + international=True, + phone_prefix="7", # russians have full delivery receipts + ) ) res = update_notification_status_by_id(notification.id, "delivered") @@ -222,7 +233,7 @@ def test_should_not_update_status_by_id_if_sent_to_country_with_delivery_receipt def test_should_not_update_status_by_reference_if_not_sending(sample_template): - notification = create_notification(template=sample_template, status="created", reference="reference") + notification = save_notification(create_notification(template=sample_template, status="created", reference="reference")) assert Notification.query.get(notification.id).status == "created" updated = update_notification_status_by_reference("reference", "failed") assert Notification.query.get(notification.id).status == "created" @@ -230,7 +241,7 @@ def test_should_not_update_status_by_reference_if_not_sending(sample_template): def test_should_by_able_to_update_status_by_id_from_pending_to_delivered(sample_template, sample_job): - notification = create_notification(template=sample_template, job=sample_job, status="sending") + notification = save_notification(create_notification(template=sample_template, job=sample_job, status="sending")) assert update_notification_status_by_id(notification_id=notification.id, status="pending") assert Notification.query.get(notification.id).status == "pending" @@ -240,7 +251,7 @@ def test_should_by_able_to_update_status_by_id_from_pending_to_delivered(sample_ def test_should_by_able_to_update_status_by_id_from_pending_to_temporary_failure(sample_template, sample_job): - notification = create_notification(template=sample_template, job=sample_job, status="sending") + notification = save_notification(create_notification(template=sample_template, job=sample_job, status="sending")) assert update_notification_status_by_id(notification_id=notification.id, status="pending") assert Notification.query.get(notification.id).status == "pending" @@ -263,7 +274,7 @@ def test_should_by_able_to_update_status_by_id_from_sending_to_permanent_failure def test_should_not_update_status_once_notification_status_is_delivered( sample_email_template, ): - notification = create_notification(template=sample_email_template, status="sending") + notification = save_notification(create_notification(template=sample_email_template, status="sending")) assert Notification.query.get(notification.id).status == "sending" notification.reference = "reference" @@ -286,11 +297,13 @@ def test_should_return_zero_count_if_no_notification_with_reference(): def test_create_notification_creates_notification_with_personalisation(sample_template_with_placeholders, sample_job): assert Notification.query.count() == 0 - data = create_notification( - template=sample_template_with_placeholders, - job=sample_job, - personalisation={"name": "Jo"}, - status="created", + data = save_notification( + create_notification( + template=sample_template_with_placeholders, + job=sample_job, + personalisation={"name": "Jo"}, + status="created", + ) ) assert Notification.query.count() == 1 @@ -376,7 +389,7 @@ def test_update_notification_with_research_mode_service_does_not_create_or_updat sample_template, ): sample_template.service.research_mode = True - notification = create_notification(template=sample_template) + notification = save_notification(create_notification(template=sample_template)) assert Notification.query.count() == 1 assert NotificationHistory.query.count() == 0 @@ -469,7 +482,9 @@ def test_save_notification_with_no_job(sample_template): def test_get_notification_with_personalisation_by_id(sample_template): - notification = create_notification(template=sample_template, scheduled_for="2017-05-05 14:15", status="created") + notification = save_scheduled_notification( + create_notification(template=sample_template, status="created"), scheduled_for="2017-05-05 14:15" + ) notification_from_db = get_notification_with_personalisation(sample_template.service.id, notification.id, key_type=None) assert notification == notification_from_db assert notification_from_db.scheduled_notification.scheduled_for == datetime(2017, 5, 5, 14, 15) @@ -499,9 +514,9 @@ def test_get_notification_by_id_when_notification_exists_for_different_service( def test_get_notifications_by_reference(sample_template): client_reference = "some-client-ref" assert len(Notification.query.all()) == 0 - create_notification(sample_template, client_reference=client_reference) - create_notification(sample_template, client_reference=client_reference) - create_notification(sample_template, client_reference="other-ref") + save_notification(create_notification(sample_template, client_reference=client_reference)) + save_notification(create_notification(sample_template, client_reference=client_reference)) + save_notification(create_notification(sample_template, client_reference="other-ref")) all_notifications = get_notifications_for_service(sample_template.service_id, client_reference=client_reference).items assert len(all_notifications) == 2 @@ -536,7 +551,7 @@ def test_get_notification_for_job(sample_notification): def test_get_all_notifications_for_job(sample_job): for i in range(0, 5): try: - create_notification(template=sample_job.template, job=sample_job) + save_notification(create_notification(template=sample_job.template, job=sample_job)) except IntegrityError: pass @@ -548,7 +563,7 @@ def test_get_all_notifications_for_job_by_status(sample_job): notifications = partial(get_notifications_for_job, sample_job.service.id, sample_job.id) for status in NOTIFICATION_STATUS_TYPES: - create_notification(template=sample_job.template, job=sample_job, status=status) + save_notification(create_notification(template=sample_job.template, job=sample_job, status=status)) assert len(notifications().items) == len(NOTIFICATION_STATUS_TYPES) @@ -578,7 +593,7 @@ def test_should_limit_notifications_return_by_day_limit_plus_one(sample_template for i in range(1, 11): past_date = "2016-01-{0:02d} 12:00:00".format(i) with freeze_time(past_date): - create_notification(sample_template, created_at=datetime.utcnow(), status="failed") + save_notification(create_notification(sample_template, created_at=datetime.utcnow(), status="failed")) all_notifications = Notification.query.all() assert len(all_notifications) == 10 @@ -591,13 +606,13 @@ def test_should_limit_notifications_return_by_day_limit_plus_one(sample_template def test_creating_notification_does_not_add_notification_history(sample_template): - create_notification(template=sample_template) + save_notification(create_notification(template=sample_template)) assert Notification.query.count() == 1 assert NotificationHistory.query.count() == 0 def test_should_delete_notification_for_id(sample_template): - notification = create_notification(template=sample_template) + notification = save_notification(create_notification(template=sample_template)) assert Notification.query.count() == 1 assert NotificationHistory.query.count() == 0 @@ -612,7 +627,7 @@ def test_should_delete_notification_and_ignore_history_for_research_mode( ): sample_template.service.research_mode = True - notification = create_notification(template=sample_template) + notification = save_notification(create_notification(template=sample_template)) assert Notification.query.count() == 1 @@ -622,8 +637,8 @@ def test_should_delete_notification_and_ignore_history_for_research_mode( def test_should_delete_only_notification_with_id(sample_template): - notification_1 = create_notification(template=sample_template) - notification_2 = create_notification(template=sample_template) + notification_1 = save_notification(create_notification(template=sample_template)) + notification_2 = save_notification(create_notification(template=sample_template)) assert Notification.query.count() == 2 dao_delete_notifications_by_id(notification_1.id) @@ -633,7 +648,7 @@ def test_should_delete_only_notification_with_id(sample_template): def test_should_delete_no_notifications_if_no_matching_ids(sample_template): - create_notification(template=sample_template) + save_notification(create_notification(template=sample_template)) assert Notification.query.count() == 1 dao_delete_notifications_by_id(uuid.uuid4()) @@ -664,10 +679,10 @@ def _notification_json(sample_template, job_id=None, id=None, status=None): def test_dao_timeout_notifications(sample_template): with freeze_time(datetime.utcnow() - timedelta(minutes=2)): - created = create_notification(sample_template, status="created") - sending = create_notification(sample_template, status="sending") - pending = create_notification(sample_template, status="pending") - delivered = create_notification(sample_template, status="delivered") + created = save_notification(create_notification(sample_template, status="created")) + sending = save_notification(create_notification(sample_template, status="sending")) + pending = save_notification(create_notification(sample_template, status="pending")) + delivered = save_notification(create_notification(sample_template, status="delivered")) assert Notification.query.get(created.id).status == "created" assert Notification.query.get(sending.id).status == "sending" @@ -688,10 +703,10 @@ def test_dao_timeout_notifications_only_updates_for_older_notifications( sample_template, ): with freeze_time(datetime.utcnow() + timedelta(minutes=10)): - created = create_notification(sample_template, status="created") - sending = create_notification(sample_template, status="sending") - pending = create_notification(sample_template, status="pending") - delivered = create_notification(sample_template, status="delivered") + created = save_notification(create_notification(sample_template, status="created")) + sending = save_notification(create_notification(sample_template, status="sending")) + pending = save_notification(create_notification(sample_template, status="pending")) + delivered = save_notification(create_notification(sample_template, status="delivered")) assert Notification.query.get(created.id).status == "created" assert Notification.query.get(sending.id).status == "sending" @@ -706,10 +721,10 @@ def test_dao_timeout_notifications_only_updates_for_older_notifications( def test_dao_timeout_notifications_doesnt_affect_letters(sample_letter_template): with freeze_time(datetime.utcnow() - timedelta(minutes=2)): - created = create_notification(sample_letter_template, status="created") - sending = create_notification(sample_letter_template, status="sending") - pending = create_notification(sample_letter_template, status="pending") - delivered = create_notification(sample_letter_template, status="delivered") + created = save_notification(create_notification(sample_letter_template, status="created")) + sending = save_notification(create_notification(sample_letter_template, status="sending")) + pending = save_notification(create_notification(sample_letter_template, status="pending")) + delivered = save_notification(create_notification(sample_letter_template, status="delivered")) assert Notification.query.get(created.id).status == "created" assert Notification.query.get(sending.id).status == "sending" @@ -723,8 +738,8 @@ def test_dao_timeout_notifications_doesnt_affect_letters(sample_letter_template) def test_should_return_notifications_excluding_jobs_by_default(sample_template, sample_job, sample_api_key): - create_notification(sample_template, job=sample_job) - without_job = create_notification(sample_template, api_key=sample_api_key) + save_notification(create_notification(sample_template, job=sample_job)) + without_job = save_notification(create_notification(sample_template, api_key=sample_api_key)) include_jobs = get_notifications_for_service(sample_template.service_id, include_jobs=True).items assert len(include_jobs) == 2 @@ -739,8 +754,8 @@ def test_should_return_notifications_excluding_jobs_by_default(sample_template, def test_should_return_notifications_including_one_offs_by_default(sample_user, sample_template): - create_notification(sample_template, one_off=True, created_by_id=sample_user.id) - not_one_off = create_notification(sample_template) + save_notification(create_notification(sample_template, one_off=True, created_by_id=sample_user.id)) + not_one_off = save_notification(create_notification(sample_template)) exclude_one_offs = get_notifications_for_service(sample_template.service_id, include_one_off=False).items assert len(exclude_one_offs) == 1 @@ -754,8 +769,8 @@ def test_should_return_notifications_including_one_offs_by_default(sample_user, def test_should_not_count_pages_when_given_a_flag(sample_user, sample_template): - create_notification(sample_template) - notification = create_notification(sample_template) + save_notification(create_notification(sample_template)) + notification = save_notification(create_notification(sample_template)) pagination = get_notifications_for_service(sample_template.service_id, count_pages=False, page_size=1) assert len(pagination.items) == 1 @@ -772,24 +787,30 @@ def test_get_notifications_created_by_api_or_csv_are_returned_correctly_excludin sample_team_api_key, sample_test_api_key, ): - create_notification(template=sample_job.template, created_at=datetime.utcnow(), job=sample_job) - create_notification( - template=sample_job.template, - created_at=datetime.utcnow(), - api_key=sample_api_key, - key_type=sample_api_key.key_type, - ) - create_notification( - template=sample_job.template, - created_at=datetime.utcnow(), - api_key=sample_team_api_key, - key_type=sample_team_api_key.key_type, - ) - create_notification( - template=sample_job.template, - created_at=datetime.utcnow(), - api_key=sample_test_api_key, - key_type=sample_test_api_key.key_type, + save_notification(create_notification(template=sample_job.template, created_at=datetime.utcnow(), job=sample_job)) + save_notification( + create_notification( + template=sample_job.template, + created_at=datetime.utcnow(), + api_key=sample_api_key, + key_type=sample_api_key.key_type, + ) + ) + save_notification( + create_notification( + template=sample_job.template, + created_at=datetime.utcnow(), + api_key=sample_team_api_key, + key_type=sample_team_api_key.key_type, + ) + ) + save_notification( + create_notification( + template=sample_job.template, + created_at=datetime.utcnow(), + api_key=sample_test_api_key, + key_type=sample_test_api_key.key_type, + ) ) all_notifications = Notification.query.all() @@ -809,24 +830,30 @@ def test_get_notifications_created_by_api_or_csv_are_returned_correctly_excludin def test_get_notifications_with_a_live_api_key_type(sample_job, sample_api_key, sample_team_api_key, sample_test_api_key): - create_notification(template=sample_job.template, created_at=datetime.utcnow(), job=sample_job) - create_notification( - template=sample_job.template, - created_at=datetime.utcnow(), - api_key=sample_api_key, - key_type=sample_api_key.key_type, - ) - create_notification( - template=sample_job.template, - created_at=datetime.utcnow(), - api_key=sample_team_api_key, - key_type=sample_team_api_key.key_type, - ) - create_notification( - template=sample_job.template, - created_at=datetime.utcnow(), - api_key=sample_test_api_key, - key_type=sample_test_api_key.key_type, + save_notification(create_notification(template=sample_job.template, created_at=datetime.utcnow(), job=sample_job)) + save_notification( + create_notification( + template=sample_job.template, + created_at=datetime.utcnow(), + api_key=sample_api_key, + key_type=sample_api_key.key_type, + ) + ) + save_notification( + create_notification( + template=sample_job.template, + created_at=datetime.utcnow(), + api_key=sample_team_api_key, + key_type=sample_team_api_key.key_type, + ) + ) + save_notification( + create_notification( + template=sample_job.template, + created_at=datetime.utcnow(), + api_key=sample_test_api_key, + key_type=sample_test_api_key.key_type, + ) ) all_notifications = Notification.query.all() @@ -844,24 +871,30 @@ def test_get_notifications_with_a_live_api_key_type(sample_job, sample_api_key, def test_get_notifications_with_a_test_api_key_type(sample_job, sample_api_key, sample_team_api_key, sample_test_api_key): - create_notification(template=sample_job.template, created_at=datetime.utcnow(), job=sample_job) - create_notification( - template=sample_job.template, - created_at=datetime.utcnow(), - api_key=sample_api_key, - key_type=sample_api_key.key_type, - ) - create_notification( - template=sample_job.template, - created_at=datetime.utcnow(), - api_key=sample_team_api_key, - key_type=sample_team_api_key.key_type, - ) - create_notification( - template=sample_job.template, - created_at=datetime.utcnow(), - api_key=sample_test_api_key, - key_type=sample_test_api_key.key_type, + save_notification(create_notification(template=sample_job.template, created_at=datetime.utcnow(), job=sample_job)) + save_notification( + create_notification( + template=sample_job.template, + created_at=datetime.utcnow(), + api_key=sample_api_key, + key_type=sample_api_key.key_type, + ) + ) + save_notification( + create_notification( + template=sample_job.template, + created_at=datetime.utcnow(), + api_key=sample_team_api_key, + key_type=sample_team_api_key.key_type, + ) + ) + save_notification( + create_notification( + template=sample_job.template, + created_at=datetime.utcnow(), + api_key=sample_test_api_key, + key_type=sample_test_api_key.key_type, + ) ) # only those created with test API key, no jobs @@ -876,24 +909,30 @@ def test_get_notifications_with_a_test_api_key_type(sample_job, sample_api_key, def test_get_notifications_with_a_team_api_key_type(sample_job, sample_api_key, sample_team_api_key, sample_test_api_key): - create_notification(template=sample_job.template, created_at=datetime.utcnow(), job=sample_job) - create_notification( - template=sample_job.template, - created_at=datetime.utcnow(), - api_key=sample_api_key, - key_type=sample_api_key.key_type, - ) - create_notification( - template=sample_job.template, - created_at=datetime.utcnow(), - api_key=sample_team_api_key, - key_type=sample_team_api_key.key_type, - ) - create_notification( - sample_job.template, - created_at=datetime.utcnow(), - api_key=sample_test_api_key, - key_type=sample_test_api_key.key_type, + save_notification(create_notification(template=sample_job.template, created_at=datetime.utcnow(), job=sample_job)) + save_notification( + create_notification( + template=sample_job.template, + created_at=datetime.utcnow(), + api_key=sample_api_key, + key_type=sample_api_key.key_type, + ) + ) + save_notification( + create_notification( + template=sample_job.template, + created_at=datetime.utcnow(), + api_key=sample_team_api_key, + key_type=sample_team_api_key.key_type, + ) + ) + save_notification( + create_notification( + sample_job.template, + created_at=datetime.utcnow(), + api_key=sample_test_api_key, + key_type=sample_test_api_key.key_type, + ) ) # only those created with team API key, no jobs @@ -908,25 +947,31 @@ def test_get_notifications_with_a_team_api_key_type(sample_job, sample_api_key, def test_should_exclude_test_key_notifications_by_default(sample_job, sample_api_key, sample_team_api_key, sample_test_api_key): - create_notification(template=sample_job.template, created_at=datetime.utcnow(), job=sample_job) + save_notification(create_notification(template=sample_job.template, created_at=datetime.utcnow(), job=sample_job)) - create_notification( - template=sample_job.template, - created_at=datetime.utcnow(), - api_key=sample_api_key, - key_type=sample_api_key.key_type, + save_notification( + create_notification( + template=sample_job.template, + created_at=datetime.utcnow(), + api_key=sample_api_key, + key_type=sample_api_key.key_type, + ) ) - create_notification( - template=sample_job.template, - created_at=datetime.utcnow(), - api_key=sample_team_api_key, - key_type=sample_team_api_key.key_type, + save_notification( + create_notification( + template=sample_job.template, + created_at=datetime.utcnow(), + api_key=sample_team_api_key, + key_type=sample_team_api_key.key_type, + ) ) - create_notification( - template=sample_job.template, - created_at=datetime.utcnow(), - api_key=sample_test_api_key, - key_type=sample_test_api_key.key_type, + save_notification( + create_notification( + template=sample_job.template, + created_at=datetime.utcnow(), + api_key=sample_test_api_key, + key_type=sample_test_api_key.key_type, + ) ) all_notifications = Notification.query.all() @@ -982,13 +1027,13 @@ def test_is_delivery_slow_for_provider( ) for _ in range(normal_sending): - normal_notification(status="sending") + save_notification(normal_notification(status="sending")) for _ in range(slow_sending): - slow_notification(status="sending") + save_notification(slow_notification(status="sending")) for _ in range(normal_delivered): - normal_notification(status="delivered") + save_notification(normal_notification(status="delivered")) for _ in range(slow_delivered): - slow_notification(status="delivered") + save_notification(slow_notification(status="delivered")) assert is_delivery_slow_for_provider(datetime.utcnow(), "mmg", threshold, timedelta(minutes=4)) is expected_result @@ -1023,7 +1068,7 @@ def test_delivery_is_delivery_slow_for_provider_filters_out_notifications_it_sho "updated_at": datetime.now(), } create_notification_with.update(options) - create_notification(**create_notification_with) + save_notification(create_notification(**create_notification_with)) assert is_delivery_slow_for_provider(datetime.utcnow(), "mmg", 0.1, timedelta(minutes=4)) is expected_result @@ -1034,17 +1079,21 @@ def test_dao_get_notifications_by_to_field(sample_template): "normalised_to": "+16502532222", } - notification1 = create_notification(template=sample_template, **recipient_to_search_for) - create_notification(template=sample_template, key_type=KEY_TYPE_TEST, **recipient_to_search_for) - create_notification( - template=sample_template, - to_field="jack@gmail.com", - normalised_to="jack@gmail.com", + notification1 = save_notification(create_notification(template=sample_template, **recipient_to_search_for)) + save_notification(create_notification(template=sample_template, key_type=KEY_TYPE_TEST, **recipient_to_search_for)) + save_notification( + create_notification( + template=sample_template, + to_field="jack@gmail.com", + normalised_to="jack@gmail.com", + ) ) - create_notification( - template=sample_template, - to_field="jane@gmail.com", - normalised_to="jane@gmail.com", + save_notification( + create_notification( + template=sample_template, + to_field="jane@gmail.com", + normalised_to="jane@gmail.com", + ) ) results = dao_get_notifications_by_to_field( @@ -1059,10 +1108,12 @@ def test_dao_get_notifications_by_to_field(sample_template): @pytest.mark.parametrize("search_term", ["JACK", "JACK@gmail.com", "jack@gmail.com"]) def test_dao_get_notifications_by_to_field_search_is_not_case_sensitive(sample_email_template, search_term): - notification = create_notification( - template=sample_email_template, - to_field="jack@gmail.com", - normalised_to="jack@gmail.com", + notification = save_notification( + create_notification( + template=sample_email_template, + to_field="jack@gmail.com", + normalised_to="jack@gmail.com", + ) ) results = dao_get_notifications_by_to_field(notification.service_id, search_term, notification_type="email") notification_ids = [notification.id for notification in results] @@ -1074,15 +1125,19 @@ def test_dao_get_notifications_by_to_field_search_is_not_case_sensitive(sample_e def test_dao_get_notifications_by_to_field_matches_partial_emails( sample_email_template, ): - notification_1 = create_notification( - template=sample_email_template, - to_field="jack@gmail.com", - normalised_to="jack@gmail.com", + notification_1 = save_notification( + create_notification( + template=sample_email_template, + to_field="jack@gmail.com", + normalised_to="jack@gmail.com", + ) ) - notification_2 = create_notification( - template=sample_email_template, - to_field="jacque@gmail.com", - normalised_to="jacque@gmail.com", + notification_2 = save_notification( + create_notification( + template=sample_email_template, + to_field="jacque@gmail.com", + normalised_to="jacque@gmail.com", + ) ) results = dao_get_notifications_by_to_field(notification_1.service_id, "ack", notification_type="email") notification_ids = [notification.id for notification in results] @@ -1124,10 +1179,12 @@ def test_dao_get_notifications_by_to_field_escapes( "/@example.com", "baz\\baz@example.com", }: - create_notification( - template=sample_email_template, - to_field=email_address, - normalised_to=email_address, + save_notification( + create_notification( + template=sample_email_template, + to_field=email_address, + normalised_to=email_address, + ) ) assert ( @@ -1169,15 +1226,19 @@ def test_dao_get_notifications_by_to_field_matches_partial_phone_numbers( search_term, ): - notification_1 = create_notification( - template=sample_template, - to_field="+447700900100", - normalised_to="447700900100", + notification_1 = save_notification( + create_notification( + template=sample_template, + to_field="+447700900100", + normalised_to="447700900100", + ) ) - notification_2 = create_notification( - template=sample_template, - to_field="+447700900200", - normalised_to="447700900200", + notification_2 = save_notification( + create_notification( + template=sample_template, + to_field="+447700900200", + normalised_to="447700900200", + ) ) results = dao_get_notifications_by_to_field(notification_1.service_id, search_term, notification_type="sms") notification_ids = [notification.id for notification in results] @@ -1202,21 +1263,29 @@ def test_dao_get_notifications_by_to_field_accepts_invalid_phone_numbers_and_ema def test_dao_get_notifications_by_to_field_search_ignores_spaces(sample_template): - notification1 = create_notification(template=sample_template, to_field="+16502532222", normalised_to="+16502532222") - notification2 = create_notification( - template=sample_template, - to_field="+1 650 253 2222", - normalised_to="+16502532222", + notification1 = save_notification( + create_notification(template=sample_template, to_field="+16502532222", normalised_to="+16502532222") ) - notification3 = create_notification( - template=sample_template, - to_field=" +1650253 2 222", - normalised_to="+16502532222", + notification2 = save_notification( + create_notification( + template=sample_template, + to_field="+1 650 253 2222", + normalised_to="+16502532222", + ) ) - create_notification( - template=sample_template, - to_field="jaCK@gmail.com", - normalised_to="jack@gmail.com", + notification3 = save_notification( + create_notification( + template=sample_template, + to_field=" +1650253 2 222", + normalised_to="+16502532222", + ) + ) + save_notification( + create_notification( + template=sample_template, + to_field="jaCK@gmail.com", + normalised_to="jack@gmail.com", + ) ) results = dao_get_notifications_by_to_field(notification1.service_id, "+16502532222", notification_type="sms") @@ -1244,11 +1313,13 @@ def test_dao_get_notifications_by_to_field_only_searches_one_notification_type( service = create_service() sms_template = create_template(service=service) email_template = create_template(service=service, template_type="email") - sms = create_notification(template=sms_template, to_field="6502532222", normalised_to="+16502532222") - email = create_notification( - template=email_template, - to_field="165@example.com", - normalised_to="165@example.com", + sms = save_notification(create_notification(template=sms_template, to_field="6502532222", normalised_to="+16502532222")) + email = save_notification( + create_notification( + template=email_template, + to_field="165@example.com", + normalised_to="165@example.com", + ) ) results = dao_get_notifications_by_to_field(service.id, phone_search, notification_type="sms") assert len(results) == 1 @@ -1278,9 +1349,15 @@ def test_dao_created_scheduled_notification(sample_notification): def test_dao_get_scheduled_notifications(sample_template): - notification_1 = create_notification(template=sample_template, scheduled_for="2017-05-05 14:15", status="created") - create_notification(template=sample_template, scheduled_for="2017-05-04 14:15", status="delivered") - create_notification(template=sample_template, status="created") + notification_1 = save_scheduled_notification( + create_notification(template=sample_template, status="created"), + scheduled_for="2017-05-05 14:15", + ) + save_scheduled_notification( + create_notification(template=sample_template, status="delivered"), + scheduled_for="2017-05-04 14:15", + ) + save_notification(create_notification(template=sample_template, status="created")) scheduled_notifications = dao_get_scheduled_notifications() assert len(scheduled_notifications) == 1 assert scheduled_notifications[0].id == notification_1.id @@ -1288,7 +1365,10 @@ def test_dao_get_scheduled_notifications(sample_template): def test_set_scheduled_notification_to_processed(sample_template): - notification_1 = create_notification(template=sample_template, scheduled_for="2017-05-05 14:15", status="created") + notification_1 = save_scheduled_notification( + create_notification(template=sample_template, status="created"), + scheduled_for="2017-05-05 14:15", + ) scheduled_notifications = dao_get_scheduled_notifications() assert len(scheduled_notifications) == 1 assert scheduled_notifications[0].id == notification_1.id @@ -1300,17 +1380,21 @@ def test_set_scheduled_notification_to_processed(sample_template): def test_dao_get_notifications_by_to_field_filters_status(sample_template): - notification = create_notification( - template=sample_template, - to_field="+16502532222", - normalised_to="+16502532222", - status="delivered", + notification = save_notification( + create_notification( + template=sample_template, + to_field="+16502532222", + normalised_to="+16502532222", + status="delivered", + ) ) - create_notification( - template=sample_template, - to_field="+16502532222", - normalised_to="+16502532222", - status="temporary-failure", + save_notification( + create_notification( + template=sample_template, + to_field="+16502532222", + normalised_to="+16502532222", + status="temporary-failure", + ) ) notifications = dao_get_notifications_by_to_field( @@ -1325,17 +1409,21 @@ def test_dao_get_notifications_by_to_field_filters_status(sample_template): def test_dao_get_notifications_by_to_field_filters_multiple_statuses(sample_template): - notification1 = create_notification( - template=sample_template, - to_field="+16502532222", - normalised_to="+16502532222", - status="delivered", + notification1 = save_notification( + create_notification( + template=sample_template, + to_field="+16502532222", + normalised_to="+16502532222", + status="delivered", + ) ) - notification2 = create_notification( - template=sample_template, - to_field="+16502532222", - normalised_to="+16502532222", - status="sending", + notification2 = save_notification( + create_notification( + template=sample_template, + to_field="+16502532222", + normalised_to="+16502532222", + status="sending", + ) ) notifications = dao_get_notifications_by_to_field( @@ -1354,17 +1442,21 @@ def test_dao_get_notifications_by_to_field_filters_multiple_statuses(sample_temp def test_dao_get_notifications_by_to_field_returns_all_if_no_status_filter( sample_template, ): - notification1 = create_notification( - template=sample_template, - to_field="+16502532222", - normalised_to="+16502532222", - status="delivered", + notification1 = save_notification( + create_notification( + template=sample_template, + to_field="+16502532222", + normalised_to="+16502532222", + status="delivered", + ) ) - notification2 = create_notification( - template=sample_template, - to_field="+16502532222", - normalised_to="+16502532222", - status="temporary-failure", + notification2 = save_notification( + create_notification( + template=sample_template, + to_field="+16502532222", + normalised_to="+16502532222", + status="temporary-failure", + ) ) notifications = dao_get_notifications_by_to_field(notification1.service_id, "+16502532222", notification_type="sms") @@ -1377,15 +1469,16 @@ def test_dao_get_notifications_by_to_field_returns_all_if_no_status_filter( @freeze_time("2016-01-01 11:10:00") def test_dao_get_notifications_by_to_field_orders_by_created_at_desc(sample_template): - notification = partial( - create_notification, - template=sample_template, - to_field="+16502532222", - normalised_to="+16502532222", - ) + data = { + "template": sample_template, + "to_field": "+16502532222", + "normalised_to": "+16502532222", + } - notification_a_minute_ago = notification(created_at=datetime.utcnow() - timedelta(minutes=1)) - notification = notification(created_at=datetime.utcnow()) + notification_a_minute_ago = save_notification( + create_notification(created_at=datetime.utcnow() - timedelta(minutes=1), **data) + ) + notification = save_notification(create_notification(created_at=datetime.utcnow(), **data)) notifications = dao_get_notifications_by_to_field(sample_template.service_id, "+16502532222", notification_type="sms") @@ -1429,8 +1522,8 @@ def test_dao_get_last_notification_added_for_job_id_no_job(sample_template, fake def test_dao_update_notifications_by_reference_updated_notifications(sample_template): - notification_1 = create_notification(template=sample_template, reference="ref1") - notification_2 = create_notification(template=sample_template, reference="ref2") + notification_1 = save_notification(create_notification(template=sample_template, reference="ref1")) + notification_2 = save_notification(create_notification(template=sample_template, reference="ref2")) updated_count, updated_history_count = dao_update_notifications_by_reference( references=["ref1", "ref2"], @@ -1450,7 +1543,7 @@ def test_dao_update_notifications_by_reference_updated_notifications(sample_temp def test_dao_update_notifications_by_reference_updates_history_some_notifications_exist( sample_template, ): - create_notification(template=sample_template, reference="ref1") + save_notification(create_notification(template=sample_template, reference="ref1")) create_notification_history(template=sample_template, reference="ref2") updated_count, updated_history_count = dao_update_notifications_by_reference( @@ -1489,7 +1582,7 @@ def test_dao_update_notifications_by_reference_returns_zero_when_no_notification def test_dao_update_notifications_by_reference_set_returned_letter_status( sample_letter_template, ): - notification = create_notification(template=sample_letter_template, reference="ref") + notification = save_notification(create_notification(template=sample_letter_template, reference="ref")) updated_count, updated_history_count = dao_update_notifications_by_reference( references=["ref"], update_dict={"status": "returned-letter"} @@ -1504,7 +1597,7 @@ def test_dao_update_notifications_by_reference_updates_history_when_one_of_two_n sample_letter_template, ): notification1 = create_notification_history(template=sample_letter_template, reference="ref1") - notification2 = create_notification(template=sample_letter_template, reference="ref2") + notification2 = save_notification(create_notification(template=sample_letter_template, reference="ref2")) updated_count, updated_history_count = dao_update_notifications_by_reference( references=["ref1", "ref2"], update_dict={"status": "returned-letter"} @@ -1517,15 +1610,15 @@ def test_dao_update_notifications_by_reference_updates_history_when_one_of_two_n def test_dao_get_notification_by_reference_with_one_match_returns_notification(sample_letter_template, notify_db): - create_notification(template=sample_letter_template, reference="REF1") + save_notification(create_notification(template=sample_letter_template, reference="REF1")) notification = dao_get_notification_by_reference("REF1") assert notification.reference == "REF1" def test_dao_get_notification_by_reference_with_multiple_matches_raises_error(sample_letter_template, notify_db): - create_notification(template=sample_letter_template, reference="REF1") - create_notification(template=sample_letter_template, reference="REF1") + save_notification(create_notification(template=sample_letter_template, reference="REF1")) + save_notification(create_notification(template=sample_letter_template, reference="REF1")) with pytest.raises(SQLAlchemyError): dao_get_notification_by_reference("REF1") @@ -1537,9 +1630,9 @@ def test_dao_get_notification_by_reference_with_no_matches_raises_error(notify_d def test_dao_get_notifications_by_reference(sample_template): - create_notification(template=sample_template, reference="noref") - notification_1 = create_notification(template=sample_template, reference="ref") - notification_2 = create_notification(template=sample_template, reference="ref") + save_notification(create_notification(template=sample_template, reference="noref")) + notification_1 = save_notification(create_notification(template=sample_template, reference="ref")) + notification_2 = save_notification(create_notification(template=sample_template, reference="ref")) notifications = dao_get_notifications_by_references(["ref"]) assert len(notifications) == 2 @@ -1550,7 +1643,7 @@ def test_dao_get_notifications_by_reference(sample_template): def test_dao_get_notification_history_by_reference_with_one_match_returns_notification( sample_letter_template, ): - create_notification(template=sample_letter_template, reference="REF1") + save_notification(create_notification(template=sample_letter_template, reference="REF1")) notification = dao_get_notification_history_by_reference("REF1") assert notification.reference == "REF1" @@ -1559,8 +1652,8 @@ def test_dao_get_notification_history_by_reference_with_one_match_returns_notifi def test_dao_get_notification_history_by_reference_with_multiple_matches_raises_error( sample_letter_template, ): - create_notification(template=sample_letter_template, reference="REF1") - create_notification(template=sample_letter_template, reference="REF1") + save_notification(create_notification(template=sample_letter_template, reference="REF1")) + save_notification(create_notification(template=sample_letter_template, reference="REF1")) with pytest.raises(SQLAlchemyError): dao_get_notification_history_by_reference("REF1") @@ -1577,17 +1670,21 @@ def test_dao_get_notification_history_by_reference_with_no_matches_raises_error( def test_notifications_not_yet_sent(sample_service, notification_type): older_than = 4 # number of seconds the notification can not be older than template = create_template(service=sample_service, template_type=notification_type) - old_notification = create_notification( - template=template, - created_at=datetime.utcnow() - timedelta(seconds=older_than), - status="created", + old_notification = save_notification( + create_notification( + template=template, + created_at=datetime.utcnow() - timedelta(seconds=older_than), + status="created", + ) ) - create_notification( - template=template, - created_at=datetime.utcnow() - timedelta(seconds=older_than), - status="sending", + save_notification( + create_notification( + template=template, + created_at=datetime.utcnow() - timedelta(seconds=older_than), + status="sending", + ) ) - create_notification(template=template, created_at=datetime.utcnow(), status="created") + save_notification(create_notification(template=template, created_at=datetime.utcnow(), status="created")) results = notifications_not_yet_sent(older_than, notification_type) assert len(results) == 1 @@ -1598,9 +1695,9 @@ def test_notifications_not_yet_sent(sample_service, notification_type): def test_notifications_not_yet_sent_return_no_rows(sample_service, notification_type): older_than = 5 # number of seconds the notification can not be older than template = create_template(service=sample_service, template_type=notification_type) - create_notification(template=template, created_at=datetime.utcnow(), status="created") - create_notification(template=template, created_at=datetime.utcnow(), status="sending") - create_notification(template=template, created_at=datetime.utcnow(), status="delivered") + save_notification(create_notification(template=template, created_at=datetime.utcnow(), status="created")) + save_notification(create_notification(template=template, created_at=datetime.utcnow(), status="sending")) + save_notification(create_notification(template=template, created_at=datetime.utcnow(), status="delivered")) results = notifications_not_yet_sent(older_than, notification_type) assert len(results) == 0 @@ -1670,3 +1767,15 @@ def test_send_method_stats_by_service(sample_service, sample_organisation): ) == [] ) + + +def test_bulk_insert_notification(sample_template): + assert len(Notification.query.all()) == 0 + n1 = create_notification(sample_template, client_reference="happy") + n1.id = None + n1.status = None + n2 = create_notification(sample_template, client_reference="sad") + n3 = create_notification(sample_template, client_reference="loud") + bulk_insert_notifications([n1, n2, n3]) + all_notifications = get_notifications_for_service(sample_template.service_id).items + assert len(all_notifications) == 3 diff --git a/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py b/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py index 62703c2954..581da5add7 100644 --- a/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py +++ b/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py @@ -14,6 +14,7 @@ create_service, create_service_data_retention, create_template, + save_notification, ) @@ -25,47 +26,61 @@ def create_test_data(notification_type, sample_service, days_of_retention=3): default_letter_template, default_sms_template, ) = _create_templates(service_with_default_data_retention) - create_notification(template=email_template, status="delivered") - create_notification(template=sms_template, status="permanent-failure") - create_notification( - template=letter_template, - status="temporary-failure", - reference="LETTER_REF", - sent_at=datetime.utcnow(), + save_notification(create_notification(template=email_template, status="delivered")) + save_notification(create_notification(template=sms_template, status="permanent-failure")) + save_notification( + create_notification( + template=letter_template, + status="temporary-failure", + reference="LETTER_REF", + sent_at=datetime.utcnow(), + ) ) - create_notification( - template=email_template, - status="delivered", - created_at=datetime.utcnow() - timedelta(days=4), + save_notification( + create_notification( + template=email_template, + status="delivered", + created_at=datetime.utcnow() - timedelta(days=4), + ) ) - create_notification( - template=sms_template, - status="permanent-failure", - created_at=datetime.utcnow() - timedelta(days=4), + save_notification( + create_notification( + template=sms_template, + status="permanent-failure", + created_at=datetime.utcnow() - timedelta(days=4), + ) ) - create_notification( - template=letter_template, - status="temporary-failure", - reference="LETTER_REF", - sent_at=datetime.utcnow(), - created_at=datetime.utcnow() - timedelta(days=4), + save_notification( + create_notification( + template=letter_template, + status="temporary-failure", + reference="LETTER_REF", + sent_at=datetime.utcnow(), + created_at=datetime.utcnow() - timedelta(days=4), + ) ) - create_notification( - template=default_email_template, - status="delivered", - created_at=datetime.utcnow() - timedelta(days=8), + save_notification( + create_notification( + template=default_email_template, + status="delivered", + created_at=datetime.utcnow() - timedelta(days=8), + ) ) - create_notification( - template=default_sms_template, - status="permanent-failure", - created_at=datetime.utcnow() - timedelta(days=8), + save_notification( + create_notification( + template=default_sms_template, + status="permanent-failure", + created_at=datetime.utcnow() - timedelta(days=8), + ) ) - create_notification( - template=default_letter_template, - status="temporary-failure", - reference="LETTER_REF", - sent_at=datetime.utcnow(), - created_at=datetime.utcnow() - timedelta(days=8), + save_notification( + create_notification( + template=default_letter_template, + status="temporary-failure", + reference="LETTER_REF", + sent_at=datetime.utcnow(), + created_at=datetime.utcnow() - timedelta(days=8), + ) ) create_service_data_retention( service=sample_service, @@ -102,16 +117,20 @@ def test_should_delete_notifications_by_type_after_seven_days( for i in range(1, 11): past_date = "2016-0{0}-{1:02d} {1:02d}:00:00.000000".format(month, i) with freeze_time(past_date): - create_notification( - template=email_template, - created_at=datetime.utcnow(), - status="permanent-failure", + save_notification( + create_notification( + template=email_template, + created_at=datetime.utcnow(), + status="permanent-failure", + ) ) - create_notification(template=sms_template, created_at=datetime.utcnow(), status="delivered") - create_notification( - template=letter_template, - created_at=datetime.utcnow(), - status="temporary-failure", + save_notification(create_notification(template=sms_template, created_at=datetime.utcnow(), status="delivered")) + save_notification( + create_notification( + template=letter_template, + created_at=datetime.utcnow(), + status="temporary-failure", + ) ) assert Notification.query.count() == 30 @@ -141,9 +160,9 @@ def test_should_not_delete_notification_history(sample_service, mocker): mocker.patch("app.dao.notifications_dao.get_s3_bucket_objects") with freeze_time("2016-01-01 12:00"): email_template, letter_template, sms_template = _create_templates(sample_service) - create_notification(template=email_template, status="permanent-failure") - create_notification(template=sms_template, status="permanent-failure") - create_notification(template=letter_template, status="permanent-failure") + save_notification(create_notification(template=email_template, status="permanent-failure")) + save_notification(create_notification(template=sms_template, status="permanent-failure")) + save_notification(create_notification(template=letter_template, status="permanent-failure")) assert Notification.query.count() == 3 delete_notifications_older_than_retention_by_type("sms") assert Notification.query.count() == 2 @@ -179,7 +198,9 @@ def test_delete_notifications_inserts_notification_history(sample_service): def test_delete_notifications_updates_notification_history(sample_email_template, mocker): mocker.patch("app.dao.notifications_dao.get_s3_bucket_objects") - notification = create_notification(template=sample_email_template, created_at=datetime.utcnow() - timedelta(days=8)) + notification = save_notification( + create_notification(template=sample_email_template, created_at=datetime.utcnow() - timedelta(days=8)) + ) Notification.query.filter_by(id=notification.id).update( { "status": "delivered", @@ -212,10 +233,12 @@ def test_delete_notifications_keep_data_for_days_of_retention_is_longer(sample_s def test_delete_notifications_with_test_keys(sample_template, mocker): mocker.patch("app.dao.notifications_dao.get_s3_bucket_objects") - create_notification( - template=sample_template, - key_type="test", - created_at=datetime.utcnow() - timedelta(days=8), + save_notification( + create_notification( + template=sample_template, + key_type="test", + created_at=datetime.utcnow() - timedelta(days=8), + ) ) delete_notifications_older_than_retention_by_type("sms") assert Notification.query.count() == 0 @@ -226,23 +249,29 @@ def test_delete_notifications_delete_notification_type_for_default_time_if_no_da ): create_service_data_retention(service=sample_service, notification_type="sms", days_of_retention=15) email_template, letter_template, sms_template = _create_templates(sample_service) - create_notification(template=email_template, status="delivered") - create_notification(template=sms_template, status="permanent-failure") - create_notification(template=letter_template, status="temporary-failure") - create_notification( - template=email_template, - status="delivered", - created_at=datetime.utcnow() - timedelta(days=14), + save_notification(create_notification(template=email_template, status="delivered")) + save_notification(create_notification(template=sms_template, status="permanent-failure")) + save_notification(create_notification(template=letter_template, status="temporary-failure")) + save_notification( + create_notification( + template=email_template, + status="delivered", + created_at=datetime.utcnow() - timedelta(days=14), + ) ) - create_notification( - template=sms_template, - status="permanent-failure", - created_at=datetime.utcnow() - timedelta(days=14), + save_notification( + create_notification( + template=sms_template, + status="permanent-failure", + created_at=datetime.utcnow() - timedelta(days=14), + ) ) - create_notification( - template=letter_template, - status="temporary-failure", - created_at=datetime.utcnow() - timedelta(days=14), + save_notification( + create_notification( + template=letter_template, + status="temporary-failure", + created_at=datetime.utcnow() - timedelta(days=14), + ) ) assert Notification.query.count() == 6 delete_notifications_older_than_retention_by_type("email") @@ -254,7 +283,7 @@ def test_delete_notifications_does_try_to_delete_from_s3_when_letter_has_not_bee mock_get_s3 = mocker.patch("app.dao.notifications_dao.get_s3_bucket_objects") letter_template = create_template(service=sample_service, template_type="letter") - create_notification(template=letter_template, status="sending", reference="LETTER_REF") + save_notification(create_notification(template=letter_template, status="sending", reference="LETTER_REF")) delete_notifications_older_than_retention_by_type("email", qry_limit=1) mock_get_s3.assert_not_called() @@ -265,9 +294,9 @@ def test_should_not_delete_notification_if_history_does_not_exist(sample_service mocker.patch("app.dao.notifications_dao.insert_update_notification_history") with freeze_time("2016-01-01 12:00"): email_template, letter_template, sms_template = _create_templates(sample_service) - create_notification(template=email_template, status="permanent-failure") - create_notification(template=sms_template, status="delivered") - create_notification(template=letter_template, status="temporary-failure") + save_notification(create_notification(template=email_template, status="permanent-failure")) + save_notification(create_notification(template=sms_template, status="delivered")) + save_notification(create_notification(template=letter_template, status="temporary-failure")) assert Notification.query.count() == 3 delete_notifications_older_than_retention_by_type("sms") assert Notification.query.count() == 3 @@ -275,9 +304,9 @@ def test_should_not_delete_notification_if_history_does_not_exist(sample_service def test_delete_notifications_calls_subquery_multiple_times(sample_template): - create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=8)) - create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=8)) - create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=8)) + save_notification(create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=8))) + save_notification(create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=8))) + save_notification(create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=8))) assert Notification.query.count() == 3 delete_notifications_older_than_retention_by_type("sms", qry_limit=1) @@ -285,13 +314,13 @@ def test_delete_notifications_calls_subquery_multiple_times(sample_template): def test_delete_notifications_returns_sum_correctly(sample_template): - create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=8)) - create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=8)) + save_notification(create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=8))) + save_notification(create_notification(template=sample_template, created_at=datetime.now() - timedelta(days=8))) s2 = create_service(service_name="s2") t2 = create_template(s2, template_type="sms") - create_notification(template=t2, created_at=datetime.now() - timedelta(days=8)) - create_notification(template=t2, created_at=datetime.now() - timedelta(days=8)) + save_notification(create_notification(template=t2, created_at=datetime.now() - timedelta(days=8))) + save_notification(create_notification(template=t2, created_at=datetime.now() - timedelta(days=8))) ret = delete_notifications_older_than_retention_by_type("sms", qry_limit=1) assert ret == 4 @@ -299,14 +328,14 @@ def test_delete_notifications_returns_sum_correctly(sample_template): def test_insert_update_notification_history(sample_service): template = create_template(sample_service, template_type="sms") - notification_1 = create_notification(template=template, created_at=datetime.utcnow() - timedelta(days=3)) - notification_2 = create_notification(template=template, created_at=datetime.utcnow() - timedelta(days=8)) - notification_3 = create_notification(template=template, created_at=datetime.utcnow() - timedelta(days=9)) + notification_1 = save_notification(create_notification(template=template, created_at=datetime.utcnow() - timedelta(days=3))) + notification_2 = save_notification(create_notification(template=template, created_at=datetime.utcnow() - timedelta(days=8))) + notification_3 = save_notification(create_notification(template=template, created_at=datetime.utcnow() - timedelta(days=9))) other_types = ["email", "letter"] for template_type in other_types: t = create_template(service=sample_service, template_type=template_type) - create_notification(template=t, created_at=datetime.utcnow() - timedelta(days=3)) - create_notification(template=t, created_at=datetime.utcnow() - timedelta(days=8)) + save_notification(create_notification(template=t, created_at=datetime.utcnow() - timedelta(days=3))) + save_notification(create_notification(template=t, created_at=datetime.utcnow() - timedelta(days=8))) insert_update_notification_history( notification_type="sms", @@ -328,10 +357,14 @@ def test_insert_update_notification_history_only_insert_update_given_service( other_service = create_service(service_name="another service") other_template = create_template(service=other_service) template = create_template(service=sample_service) - notification_1 = create_notification(template=template, created_at=datetime.utcnow() - timedelta(days=3)) - notification_2 = create_notification(template=template, created_at=datetime.utcnow() - timedelta(days=8)) - notification_3 = create_notification(template=other_template, created_at=datetime.utcnow() - timedelta(days=3)) - notification_4 = create_notification(template=other_template, created_at=datetime.utcnow() - timedelta(days=8)) + notification_1 = save_notification(create_notification(template=template, created_at=datetime.utcnow() - timedelta(days=3))) + notification_2 = save_notification(create_notification(template=template, created_at=datetime.utcnow() - timedelta(days=8))) + notification_3 = save_notification( + create_notification(template=other_template, created_at=datetime.utcnow() - timedelta(days=3)) + ) + notification_4 = save_notification( + create_notification(template=other_template, created_at=datetime.utcnow() - timedelta(days=8)) + ) insert_update_notification_history("sms", datetime.utcnow() - timedelta(days=7), sample_service.id) history = NotificationHistory.query.all() @@ -347,11 +380,15 @@ def test_insert_update_notification_history_only_insert_update_given_service( def test_insert_update_notification_history_updates_history_with_new_status( sample_template, ): - notification_1 = create_notification(template=sample_template, created_at=datetime.utcnow() - timedelta(days=3)) - notification_2 = create_notification( - template=sample_template, - created_at=datetime.utcnow() - timedelta(days=8), - status="delivered", + notification_1 = save_notification( + create_notification(template=sample_template, created_at=datetime.utcnow() - timedelta(days=3)) + ) + notification_2 = save_notification( + create_notification( + template=sample_template, + created_at=datetime.utcnow() - timedelta(days=8), + status="delivered", + ) ) insert_update_notification_history("sms", datetime.utcnow() - timedelta(days=7), sample_template.service_id) history = NotificationHistory.query.get(notification_2.id) diff --git a/tests/app/dao/notification_dao/test_notification_dao_performance_platform.py b/tests/app/dao/notification_dao/test_notification_dao_performance_platform.py index 4dc2e7a868..d1ff3e777e 100644 --- a/tests/app/dao/notification_dao/test_notification_dao_performance_platform.py +++ b/tests/app/dao/notification_dao/test_notification_dao_performance_platform.py @@ -6,17 +6,17 @@ dao_get_total_notifications_sent_per_day_for_performance_platform, ) from app.models import KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST -from tests.app.db import create_notification +from tests.app.db import create_notification, save_notification BEGINNING_OF_DAY = date(2016, 10, 18) END_OF_DAY = date(2016, 10, 19) def test_get_total_notifications_filters_on_date_within_date_range(sample_template): - create_notification(sample_template, created_at=datetime(2016, 10, 17, 23, 59, 59)) - create_notification(sample_template, created_at=BEGINNING_OF_DAY) - create_notification(sample_template, created_at=datetime(2016, 10, 18, 23, 59, 59)) - create_notification(sample_template, created_at=END_OF_DAY) + save_notification(create_notification(sample_template, created_at=datetime(2016, 10, 17, 23, 59, 59))) + save_notification(create_notification(sample_template, created_at=BEGINNING_OF_DAY)) + save_notification(create_notification(sample_template, created_at=datetime(2016, 10, 18, 23, 59, 59))) + save_notification(create_notification(sample_template, created_at=END_OF_DAY)) result = dao_get_total_notifications_sent_per_day_for_performance_platform(BEGINNING_OF_DAY, END_OF_DAY) @@ -25,11 +25,11 @@ def test_get_total_notifications_filters_on_date_within_date_range(sample_templa @freeze_time("2016-10-18T10:00") def test_get_total_notifications_only_counts_api_notifications(sample_template, sample_job, sample_api_key): - create_notification(sample_template, one_off=True) - create_notification(sample_template, one_off=True) - create_notification(sample_template, job=sample_job) - create_notification(sample_template, job=sample_job) - create_notification(sample_template, api_key=sample_api_key) + save_notification(create_notification(sample_template, one_off=True)) + save_notification(create_notification(sample_template, one_off=True)) + save_notification(create_notification(sample_template, job=sample_job)) + save_notification(create_notification(sample_template, job=sample_job)) + save_notification(create_notification(sample_template, api_key=sample_api_key)) result = dao_get_total_notifications_sent_per_day_for_performance_platform(BEGINNING_OF_DAY, END_OF_DAY) @@ -40,11 +40,11 @@ def test_get_total_notifications_only_counts_api_notifications(sample_template, def test_get_total_notifications_ignores_test_keys(sample_template): # Creating multiple templates with normal and team keys but only 1 template # with a test key to test that the count ignores letters - create_notification(sample_template, key_type=KEY_TYPE_NORMAL) - create_notification(sample_template, key_type=KEY_TYPE_NORMAL) - create_notification(sample_template, key_type=KEY_TYPE_TEAM) - create_notification(sample_template, key_type=KEY_TYPE_TEAM) - create_notification(sample_template, key_type=KEY_TYPE_TEST) + save_notification(create_notification(sample_template, key_type=KEY_TYPE_NORMAL)) + save_notification(create_notification(sample_template, key_type=KEY_TYPE_NORMAL)) + save_notification(create_notification(sample_template, key_type=KEY_TYPE_TEAM)) + save_notification(create_notification(sample_template, key_type=KEY_TYPE_TEAM)) + save_notification(create_notification(sample_template, key_type=KEY_TYPE_TEST)) result = dao_get_total_notifications_sent_per_day_for_performance_platform(BEGINNING_OF_DAY, END_OF_DAY) @@ -55,11 +55,11 @@ def test_get_total_notifications_ignores_test_keys(sample_template): def test_get_total_notifications_ignores_letters(sample_template, sample_email_template, sample_letter_template): # Creating multiple sms and email templates but only 1 letter template to # test that the count ignores letters - create_notification(sample_template) - create_notification(sample_template) - create_notification(sample_email_template) - create_notification(sample_email_template) - create_notification(sample_letter_template) + save_notification(create_notification(sample_template)) + save_notification(create_notification(sample_template)) + save_notification(create_notification(sample_email_template)) + save_notification(create_notification(sample_email_template)) + save_notification(create_notification(sample_letter_template)) result = dao_get_total_notifications_sent_per_day_for_performance_platform(BEGINNING_OF_DAY, END_OF_DAY) @@ -70,9 +70,9 @@ def test_get_total_notifications_ignores_letters(sample_template, sample_email_t def test_get_total_notifications_counts_messages_within_10_seconds(sample_template): created_at = datetime.utcnow() - create_notification(sample_template, sent_at=created_at + timedelta(seconds=5)) - create_notification(sample_template, sent_at=created_at + timedelta(seconds=10)) - create_notification(sample_template, sent_at=created_at + timedelta(seconds=15)) + save_notification(create_notification(sample_template, sent_at=created_at + timedelta(seconds=5))) + save_notification(create_notification(sample_template, sent_at=created_at + timedelta(seconds=10))) + save_notification(create_notification(sample_template, sent_at=created_at + timedelta(seconds=15))) result = dao_get_total_notifications_sent_per_day_for_performance_platform(BEGINNING_OF_DAY, END_OF_DAY) @@ -82,7 +82,7 @@ def test_get_total_notifications_counts_messages_within_10_seconds(sample_templa @freeze_time("2016-10-18T10:00") def test_get_total_notifications_counts_messages_that_have_not_sent(sample_template): - create_notification(sample_template, status="created", sent_at=None) + save_notification(create_notification(sample_template, status="created", sent_at=None)) result = dao_get_total_notifications_sent_per_day_for_performance_platform(BEGINNING_OF_DAY, END_OF_DAY) diff --git a/tests/app/dao/notification_dao/test_notification_dao_template_usage.py b/tests/app/dao/notification_dao/test_notification_dao_template_usage.py index d10a743de5..2bd255ce9b 100644 --- a/tests/app/dao/notification_dao/test_notification_dao_template_usage.py +++ b/tests/app/dao/notification_dao/test_notification_dao_template_usage.py @@ -3,7 +3,7 @@ import pytest from app.dao.notifications_dao import dao_get_last_template_usage -from tests.app.db import create_notification, create_template +from tests.app.db import create_notification, create_template, save_notification def test_last_template_usage_should_get_right_data(sample_notification): @@ -21,10 +21,10 @@ def test_last_template_usage_should_be_able_to_get_all_template_usage_history_or ): template = create_template(sample_service, template_type=notification_type) - create_notification(template, created_at=datetime.utcnow() - timedelta(seconds=1)) - create_notification(template, created_at=datetime.utcnow() - timedelta(seconds=2)) - create_notification(template, created_at=datetime.utcnow() - timedelta(seconds=3)) - most_recent = create_notification(template) + save_notification(create_notification(template, created_at=datetime.utcnow() - timedelta(seconds=1))) + save_notification(create_notification(template, created_at=datetime.utcnow() - timedelta(seconds=2))) + save_notification(create_notification(template, created_at=datetime.utcnow() - timedelta(seconds=3))) + most_recent = save_notification(create_notification(template)) results = dao_get_last_template_usage(template.id, notification_type, template.service_id) assert results.id == most_recent.id @@ -34,12 +34,14 @@ def test_last_template_usage_should_ignore_test_keys(sample_template, sample_tea one_minute_ago = datetime.utcnow() - timedelta(minutes=1) two_minutes_ago = datetime.utcnow() - timedelta(minutes=2) - team_key = create_notification( - template=sample_template, - created_at=two_minutes_ago, - api_key=sample_team_api_key, + team_key = save_notification( + create_notification( + template=sample_template, + created_at=two_minutes_ago, + api_key=sample_team_api_key, + ) ) - create_notification(template=sample_template, created_at=one_minute_ago, api_key=sample_test_api_key) + save_notification(create_notification(template=sample_template, created_at=one_minute_ago, api_key=sample_test_api_key)) results = dao_get_last_template_usage(sample_template.id, "sms", sample_template.service_id) assert results.id == team_key.id diff --git a/tests/app/dao/test_complaint_dao.py b/tests/app/dao/test_complaint_dao.py index c98648d5bf..3ac3ed6650 100644 --- a/tests/app/dao/test_complaint_dao.py +++ b/tests/app/dao/test_complaint_dao.py @@ -13,6 +13,7 @@ create_notification, create_service, create_template, + save_notification, ) @@ -72,9 +73,9 @@ def test_fetch_complaint_by_service_return_many(notify_db_session): service_2 = create_service(service_name="second") template_1 = create_template(service=service_1, template_type="email") template_2 = create_template(service=service_2, template_type="email") - notification_1 = create_notification(template=template_1) - notification_2 = create_notification(template=template_2) - notification_3 = create_notification(template=template_2) + notification_1 = save_notification(create_notification(template=template_1)) + notification_2 = save_notification(create_notification(template=template_2)) + notification_3 = save_notification(create_notification(template=template_2)) complaint_1 = Complaint( notification_id=notification_1.id, service_id=service_1.id, diff --git a/tests/app/dao/test_fact_notification_status_dao.py b/tests/app/dao/test_fact_notification_status_dao.py index c363f0f472..8e98fde0d6 100644 --- a/tests/app/dao/test_fact_notification_status_dao.py +++ b/tests/app/dao/test_fact_notification_status_dao.py @@ -49,6 +49,7 @@ create_notification_history, create_service, create_template, + save_notification, ) @@ -61,13 +62,13 @@ def test_update_fact_notification_status(notify_db_session): third_service = create_service(service_name="third Service") third_template = create_template(service=third_service, template_type="letter") - create_notification(template=first_template, status="delivered") - create_notification(template=first_template, created_at=local_now - timedelta(days=1)) + save_notification(create_notification(template=first_template, status="delivered")) + save_notification(create_notification(template=first_template, created_at=local_now - timedelta(days=1))) # simulate a service with data retention - data has been moved to history and does not exist in notifications create_notification_history(template=second_template, status="temporary-failure") create_notification_history(template=second_template, created_at=local_now - timedelta(days=1)) - create_notification(template=third_template, status="created") - create_notification(template=third_template, created_at=local_now - timedelta(days=1)) + save_notification(create_notification(template=third_template, status="created")) + save_notification(create_notification(template=third_template, created_at=local_now - timedelta(days=1))) process_day = local_now data = fetch_notification_status_for_day(process_day=process_day) @@ -106,7 +107,7 @@ def test_update_fact_notification_status(notify_db_session): def test__update_fact_notification_status_updates_row(notify_db_session): first_service = create_service(service_name="First Service") first_template = create_template(service=first_service) - create_notification(template=first_template, status="delivered") + save_notification(create_notification(template=first_template, status="delivered")) process_day = convert_utc_to_local_timezone(datetime.utcnow()) data = fetch_notification_status_for_day(process_day=process_day) @@ -118,7 +119,7 @@ def test__update_fact_notification_status_updates_row(notify_db_session): assert len(new_fact_data) == 1 assert new_fact_data[0].notification_count == 1 - create_notification(template=first_template, status="delivered") + save_notification(create_notification(template=first_template, status="delivered")) data = fetch_notification_status_for_day(process_day=process_day) update_fact_notification_status(data=data, process_day=process_day.date()) @@ -186,34 +187,40 @@ def test_fetch_notification_status_for_service_for_day(notify_db_session): create_template(service=service_2) # too early - create_notification(service_1.templates[0], created_at=datetime(2018, 5, 31, 22, 59, 0)) + save_notification(create_notification(service_1.templates[0], created_at=datetime(2018, 5, 31, 22, 59, 0))) # included - create_notification(service_1.templates[0], created_at=datetime(2018, 5, 31, 23, 0, 0)) - create_notification(service_1.templates[0], created_at=datetime(2018, 6, 1, 22, 59, 0)) - create_notification( - service_1.templates[0], - created_at=datetime(2018, 6, 1, 12, 0, 0), - key_type=KEY_TYPE_TEAM, + save_notification(create_notification(service_1.templates[0], created_at=datetime(2018, 5, 31, 23, 0, 0))) + save_notification(create_notification(service_1.templates[0], created_at=datetime(2018, 6, 1, 22, 59, 0))) + save_notification( + create_notification( + service_1.templates[0], + created_at=datetime(2018, 6, 1, 12, 0, 0), + key_type=KEY_TYPE_TEAM, + ) ) - create_notification( - service_1.templates[0], - created_at=datetime(2018, 6, 1, 12, 0, 0), - status="delivered", + save_notification( + create_notification( + service_1.templates[0], + created_at=datetime(2018, 6, 1, 12, 0, 0), + status="delivered", + ) ) # test key - create_notification( - service_1.templates[0], - created_at=datetime(2018, 6, 1, 12, 0, 0), - key_type=KEY_TYPE_TEST, + save_notification( + create_notification( + service_1.templates[0], + created_at=datetime(2018, 6, 1, 12, 0, 0), + key_type=KEY_TYPE_TEST, + ) ) # wrong service - create_notification(service_2.templates[0], created_at=datetime(2018, 6, 1, 12, 0, 0)) + save_notification(create_notification(service_2.templates[0], created_at=datetime(2018, 6, 1, 12, 0, 0))) # tomorrow (somehow) - create_notification(service_1.templates[0], created_at=datetime(2018, 6, 1, 23, 0, 0)) + save_notification(create_notification(service_1.templates[0], created_at=datetime(2018, 6, 1, 23, 0, 0))) results = sorted( fetch_notification_status_for_service_for_day(datetime(2018, 6, 1), service_1.id), @@ -247,16 +254,18 @@ def test_fetch_notification_status_for_service_for_today_and_7_previous_days( create_ft_notification_status(date(2018, 10, 29), "email", service_1, count=3) create_ft_notification_status(date(2018, 10, 26), "letter", service_1, count=5) - create_notification(sms_template, created_at=datetime(2018, 10, 31, 11, 0, 0)) - create_notification(sms_template_2, created_at=datetime(2018, 10, 31, 11, 0, 0)) - create_notification(sms_template, created_at=datetime(2018, 10, 31, 12, 0, 0), status="delivered") - create_notification(email_template, created_at=datetime(2018, 10, 31, 13, 0, 0), status="delivered") + save_notification(create_notification(sms_template, created_at=datetime(2018, 10, 31, 11, 0, 0))) + save_notification(create_notification(sms_template_2, created_at=datetime(2018, 10, 31, 11, 0, 0))) + save_notification(create_notification(sms_template, created_at=datetime(2018, 10, 31, 12, 0, 0), status="delivered")) + save_notification(create_notification(email_template, created_at=datetime(2018, 10, 31, 13, 0, 0), status="delivered")) # too early, shouldn't be included - create_notification( - service_1.templates[0], - created_at=datetime(2018, 10, 30, 12, 0, 0), - status="delivered", + save_notification( + create_notification( + service_1.templates[0], + created_at=datetime(2018, 10, 30, 12, 0, 0), + status="delivered", + ) ) results = sorted( @@ -303,16 +312,18 @@ def test_fetch_notification_status_by_template_for_service_for_today_and_7_previ create_ft_notification_status(date(2018, 10, 29), "email", service_1, count=3) create_ft_notification_status(date(2018, 10, 26), "letter", service_1, count=5) - create_notification(sms_template, created_at=datetime(2018, 10, 31, 11, 0, 0)) - create_notification(sms_template, created_at=datetime(2018, 10, 31, 12, 0, 0), status="delivered") - create_notification(sms_template_2, created_at=datetime(2018, 10, 31, 12, 0, 0), status="delivered") - create_notification(email_template, created_at=datetime(2018, 10, 31, 13, 0, 0), status="delivered") + save_notification(create_notification(sms_template, created_at=datetime(2018, 10, 31, 11, 0, 0))) + save_notification(create_notification(sms_template, created_at=datetime(2018, 10, 31, 12, 0, 0), status="delivered")) + save_notification(create_notification(sms_template_2, created_at=datetime(2018, 10, 31, 12, 0, 0), status="delivered")) + save_notification(create_notification(email_template, created_at=datetime(2018, 10, 31, 13, 0, 0), status="delivered")) # too early, shouldn't be included - create_notification( - service_1.templates[0], - created_at=datetime(2018, 10, 30, 12, 0, 0), - status="delivered", + save_notification( + create_notification( + service_1.templates[0], + created_at=datetime(2018, 10, 30, 12, 0, 0), + status="delivered", + ) ) results = fetch_notification_status_for_service_for_today_and_7_previous_days(service_1.id, by_template=True) @@ -341,7 +352,7 @@ def test_get_total_notifications_sent_for_api_key(notify_db_session): assert api_key_stats_1 == [] for x in range(total_sends): - create_notification(template=template_email, api_key=api_key) + save_notification(create_notification(template=template_email, api_key=api_key)) api_key_stats_2 = get_total_notifications_sent_for_api_key(str(api_key.id)) assert api_key_stats_2 == [ @@ -349,7 +360,7 @@ def test_get_total_notifications_sent_for_api_key(notify_db_session): ] for x in range(total_sends): - create_notification(template=template_sms, api_key=api_key) + save_notification(create_notification(template=template_sms, api_key=api_key)) api_key_stats_3 = get_total_notifications_sent_for_api_key(str(api_key.id)) assert dict(api_key_stats_3) == dict([(EMAIL_TYPE, total_sends), (SMS_TYPE, total_sends)]) @@ -365,7 +376,7 @@ def test_get_last_send_for_api_key(notify_db_session): assert last_send == [] for x in range(total_sends): - create_notification(template=template_email, api_key=api_key) + save_notification(create_notification(template=template_email, api_key=api_key)) # the following lines test that a send has occurred within the last second last_send = get_last_send_for_api_key(str(api_key.id))[0][0] @@ -384,11 +395,11 @@ def test_get_api_key_ranked_by_notifications_created(notify_db_session): sms_sends = 10 for x in range(email_sends): - create_notification(template=template_email, api_key=api_key_1) + save_notification(create_notification(template=template_email, api_key=api_key_1)) for x in range(sms_sends): - create_notification(template=template_sms, api_key=api_key_1) - create_notification(template=template_sms, api_key=api_key_2) + save_notification(create_notification(template=template_sms, api_key=api_key_1)) + save_notification(create_notification(template=template_sms, api_key=api_key_2)) api_keys_ranked = get_api_key_ranked_by_notifications_created(2) @@ -468,11 +479,11 @@ def test_fetch_notification_status_totals_for_all_services_works_in_bst( sms_template = create_template(service=service_1, template_type=SMS_TYPE) email_template = create_template(service=service_1, template_type=EMAIL_TYPE) - create_notification(sms_template, created_at=datetime(2018, 4, 20, 12, 0, 0), status="delivered") - create_notification(sms_template, created_at=datetime(2018, 4, 21, 11, 0, 0), status="created") - create_notification(sms_template, created_at=datetime(2018, 4, 21, 12, 0, 0), status="delivered") - create_notification(email_template, created_at=datetime(2018, 4, 21, 13, 0, 0), status="delivered") - create_notification(email_template, created_at=datetime(2018, 4, 21, 14, 0, 0), status="delivered") + save_notification(create_notification(sms_template, created_at=datetime(2018, 4, 20, 12, 0, 0), status="delivered")) + save_notification(create_notification(sms_template, created_at=datetime(2018, 4, 21, 11, 0, 0), status="created")) + save_notification(create_notification(sms_template, created_at=datetime(2018, 4, 21, 12, 0, 0), status="delivered")) + save_notification(create_notification(email_template, created_at=datetime(2018, 4, 21, 13, 0, 0), status="delivered")) + save_notification(create_notification(email_template, created_at=datetime(2018, 4, 21, 14, 0, 0), status="delivered")) results = sorted( fetch_notification_status_totals_for_all_services(start_date=date(2018, 4, 21), end_date=date(2018, 4, 21)), @@ -507,14 +518,16 @@ def set_up_data(): create_ft_notification_status(date(2018, 10, 29), "email", service_1, count=3) create_ft_notification_status(date(2018, 10, 29), "letter", service_2, count=10) - create_notification( - service_1.templates[0], - created_at=datetime(2018, 10, 30, 12, 0, 0), - status="delivered", + save_notification( + create_notification( + service_1.templates[0], + created_at=datetime(2018, 10, 30, 12, 0, 0), + status="delivered", + ) ) - create_notification(sms_template, created_at=datetime(2018, 10, 31, 11, 0, 0)) - create_notification(sms_template, created_at=datetime(2018, 10, 31, 12, 0, 0), status="delivered") - create_notification(email_template, created_at=datetime(2018, 10, 31, 13, 0, 0), status="delivered") + save_notification(create_notification(sms_template, created_at=datetime(2018, 10, 31, 11, 0, 0))) + save_notification(create_notification(sms_template, created_at=datetime(2018, 10, 31, 12, 0, 0), status="delivered")) + save_notification(create_notification(email_template, created_at=datetime(2018, 10, 31, 13, 0, 0), status="delivered")) return service_1, service_2 @@ -597,8 +610,8 @@ def test_fetch_monthly_template_usage_for_service(sample_service): template=template_three, count=5, ) - create_notification(template=template_three, created_at=datetime.utcnow() - timedelta(days=1)) - create_notification(template=template_three, created_at=datetime.utcnow()) + save_notification(create_notification(template=template_three, created_at=datetime.utcnow() - timedelta(days=1))) + save_notification(create_notification(template=template_three, created_at=datetime.utcnow())) results = fetch_monthly_template_usage_for_service(datetime(2017, 4, 1), datetime(2018, 3, 31), sample_service.id) assert len(results) == 4 @@ -799,7 +812,7 @@ def test_fetch_monthly_template_usage_for_service_does_join_to_notifications_if_ template=template_one, count=3, ) - create_notification(template=template_one, created_at=datetime.utcnow()) + save_notification(create_notification(template=template_one, created_at=datetime.utcnow())) results = fetch_monthly_template_usage_for_service(datetime(2018, 1, 1), datetime(2018, 2, 20), template_one.service_id) assert len(results) == 2 @@ -831,7 +844,7 @@ def test_fetch_monthly_template_usage_for_service_does_not_include_cancelled_sta notification_status="cancelled", count=15, ) - create_notification(template=sample_template, created_at=datetime.utcnow(), status="cancelled") + save_notification(create_notification(template=sample_template, created_at=datetime.utcnow(), status="cancelled")) results = fetch_monthly_template_usage_for_service(datetime(2018, 1, 1), datetime(2018, 3, 31), sample_template.service_id) assert len(results) == 0 @@ -849,11 +862,13 @@ def test_fetch_monthly_template_usage_for_service_does_not_include_test_notifica key_type="test", count=15, ) - create_notification( - template=sample_template, - created_at=datetime.utcnow(), - status="delivered", - key_type="test", + save_notification( + create_notification( + template=sample_template, + created_at=datetime.utcnow(), + status="delivered", + key_type="test", + ) ) results = fetch_monthly_template_usage_for_service(datetime(2018, 1, 1), datetime(2018, 3, 31), sample_template.service_id) diff --git a/tests/app/dao/test_ft_billing_dao.py b/tests/app/dao/test_ft_billing_dao.py index ab297916ca..ed823971c1 100644 --- a/tests/app/dao/test_ft_billing_dao.py +++ b/tests/app/dao/test_ft_billing_dao.py @@ -31,6 +31,7 @@ create_rate, create_service, create_template, + save_notification, set_up_usage_data, ) @@ -84,13 +85,13 @@ def test_fetch_billing_data_for_today_includes_data_with_the_right_status( service = create_service() template = create_template(service=service, template_type="email") for status in ["created", "technical-failure"]: - create_notification(template=template, status=status) + save_notification(create_notification(template=template, status=status)) today = convert_utc_to_local_timezone(datetime.utcnow()) results = fetch_billing_data_for_day(today) assert results == [] for status in ["delivered", "sending", "temporary-failure"]: - create_notification(template=template, status=status) + save_notification(create_notification(template=template, status=status)) results = fetch_billing_data_for_day(today) assert len(results) == 1 assert results[0].notifications_sent == 3 @@ -102,7 +103,7 @@ def test_fetch_billing_data_for_today_includes_data_with_the_right_key_type( service = create_service() template = create_template(service=service, template_type="email") for key_type in ["normal", "test", "team"]: - create_notification(template=template, status="delivered", key_type=key_type) + save_notification(create_notification(template=template, status="delivered", key_type=key_type)) today = convert_utc_to_local_timezone(datetime.utcnow()) results = fetch_billing_data_for_day(today) @@ -118,19 +119,23 @@ def test_fetch_billing_data_for_today_includes_data_with_the_right_date( process_day = datetime(2018, 4, 1, 13, 30, 0) service = create_service() template = create_template(service=service, template_type="email") - create_notification(template=template, status="delivered", created_at=process_day) - create_notification( - template=template, - status="delivered", - created_at=datetime(2018, 4, 1, 4, 23, 23), + save_notification(create_notification(template=template, status="delivered", created_at=process_day)) + save_notification( + create_notification( + template=template, + status="delivered", + created_at=datetime(2018, 4, 1, 4, 23, 23), + ) ) - create_notification( - template=template, - status="delivered", - created_at=datetime(2018, 3, 31, 20, 23, 23), + save_notification( + create_notification( + template=template, + status="delivered", + created_at=datetime(2018, 3, 31, 20, 23, 23), + ) ) - create_notification(template=template, status="sending", created_at=process_day + timedelta(days=1)) + save_notification(create_notification(template=template, status="sending", created_at=process_day + timedelta(days=1))) day_under_test = convert_utc_to_local_timezone(process_day) results = fetch_billing_data_for_day(day_under_test) @@ -144,8 +149,8 @@ def test_fetch_billing_data_for_day_is_grouped_by_template_and_notification_type service = create_service() email_template = create_template(service=service, template_type="email") sms_template = create_template(service=service, template_type="sms") - create_notification(template=email_template, status="delivered") - create_notification(template=sms_template, status="delivered") + save_notification(create_notification(template=email_template, status="delivered")) + save_notification(create_notification(template=sms_template, status="delivered")) today = convert_utc_to_local_timezone(datetime.utcnow()) results = fetch_billing_data_for_day(today) @@ -159,8 +164,8 @@ def test_fetch_billing_data_for_day_is_grouped_by_service(notify_db_session): service_2 = create_service(service_name="Service 2") email_template = create_template(service=service_1) sms_template = create_template(service=service_2) - create_notification(template=email_template, status="delivered") - create_notification(template=sms_template, status="delivered") + save_notification(create_notification(template=email_template, status="delivered")) + save_notification(create_notification(template=sms_template, status="delivered")) today = convert_utc_to_local_timezone(datetime.utcnow()) results = fetch_billing_data_for_day(today) @@ -172,8 +177,8 @@ def test_fetch_billing_data_for_day_is_grouped_by_service(notify_db_session): def test_fetch_billing_data_for_day_is_grouped_by_provider(notify_db_session): service = create_service() template = create_template(service=service) - create_notification(template=template, status="delivered", sent_by="sns") - create_notification(template=template, status="delivered", sent_by="pinpoint") + save_notification(create_notification(template=template, status="delivered", sent_by="sns")) + save_notification(create_notification(template=template, status="delivered", sent_by="pinpoint")) today = convert_utc_to_local_timezone(datetime.utcnow()) results = fetch_billing_data_for_day(today) @@ -185,8 +190,8 @@ def test_fetch_billing_data_for_day_is_grouped_by_provider(notify_db_session): def test_fetch_billing_data_for_day_is_grouped_by_rate_mulitplier(notify_db_session): service = create_service() template = create_template(service=service) - create_notification(template=template, status="delivered", rate_multiplier=1) - create_notification(template=template, status="delivered", rate_multiplier=2) + save_notification(create_notification(template=template, status="delivered", rate_multiplier=1)) + save_notification(create_notification(template=template, status="delivered", rate_multiplier=2)) today = convert_utc_to_local_timezone(datetime.utcnow()) results = fetch_billing_data_for_day(today) @@ -198,8 +203,8 @@ def test_fetch_billing_data_for_day_is_grouped_by_rate_mulitplier(notify_db_sess def test_fetch_billing_data_for_day_is_grouped_by_international(notify_db_session): service = create_service() template = create_template(service=service) - create_notification(template=template, status="delivered", international=True) - create_notification(template=template, status="delivered", international=False) + save_notification(create_notification(template=template, status="delivered", international=True)) + save_notification(create_notification(template=template, status="delivered", international=False)) today = convert_utc_to_local_timezone(datetime.utcnow()) results = fetch_billing_data_for_day(today) @@ -213,12 +218,12 @@ def test_fetch_billing_data_for_day_is_grouped_by_notification_type(notify_db_se sms_template = create_template(service=service, template_type="sms") email_template = create_template(service=service, template_type="email") letter_template = create_template(service=service, template_type="letter") - create_notification(template=sms_template, status="delivered") - create_notification(template=sms_template, status="delivered") - create_notification(template=sms_template, status="delivered") - create_notification(template=email_template, status="delivered") - create_notification(template=email_template, status="delivered") - create_notification(template=letter_template, status="delivered") + save_notification(create_notification(template=sms_template, status="delivered")) + save_notification(create_notification(template=sms_template, status="delivered")) + save_notification(create_notification(template=sms_template, status="delivered")) + save_notification(create_notification(template=email_template, status="delivered")) + save_notification(create_notification(template=email_template, status="delivered")) + save_notification(create_notification(template=letter_template, status="delivered")) today = convert_utc_to_local_timezone(datetime.utcnow()) results = fetch_billing_data_for_day(today) @@ -231,10 +236,10 @@ def test_fetch_billing_data_for_day_groups_by_postage(notify_db_session): service = create_service() letter_template = create_template(service=service, template_type="letter") email_template = create_template(service=service, template_type="email") - create_notification(template=letter_template, status="delivered", postage="first") - create_notification(template=letter_template, status="delivered", postage="first") - create_notification(template=letter_template, status="delivered", postage="second") - create_notification(template=email_template, status="delivered") + save_notification(create_notification(template=letter_template, status="delivered", postage="first")) + save_notification(create_notification(template=letter_template, status="delivered", postage="first")) + save_notification(create_notification(template=letter_template, status="delivered", postage="second")) + save_notification(create_notification(template=email_template, status="delivered")) today = convert_utc_to_local_timezone(datetime.utcnow()) results = fetch_billing_data_for_day(today) @@ -247,8 +252,8 @@ def test_fetch_billing_data_for_day_sets_postage_for_emails_and_sms_to_none( service = create_service() sms_template = create_template(service=service, template_type="sms") email_template = create_template(service=service, template_type="email") - create_notification(template=sms_template, status="delivered") - create_notification(template=email_template, status="delivered") + save_notification(create_notification(template=sms_template, status="delivered")) + save_notification(create_notification(template=email_template, status="delivered")) today = convert_utc_to_local_timezone(datetime.utcnow()) results = fetch_billing_data_for_day(today) @@ -290,8 +295,8 @@ def test_fetch_billing_data_for_day_returns_list_for_given_service(notify_db_ses service_2 = create_service(service_name="Service 2") template = create_template(service=service) template_2 = create_template(service=service_2) - create_notification(template=template, status="delivered") - create_notification(template=template_2, status="delivered") + save_notification(create_notification(template=template, status="delivered")) + save_notification(create_notification(template=template_2, status="delivered")) today = convert_utc_to_local_timezone(datetime.utcnow()) results = fetch_billing_data_for_day(process_day=today, service_id=service.id) @@ -305,9 +310,9 @@ def test_fetch_billing_data_for_day_bills_correctly_for_status(notify_db_session email_template = create_template(service=service, template_type="email") letter_template = create_template(service=service, template_type="letter") for status in NOTIFICATION_STATUS_TYPES: - create_notification(template=sms_template, status=status) - create_notification(template=email_template, status=status) - create_notification(template=letter_template, status=status) + save_notification(create_notification(template=sms_template, status=status)) + save_notification(create_notification(template=email_template, status=status)) + save_notification(create_notification(template=letter_template, status=status)) today = convert_utc_to_local_timezone(datetime.utcnow()) results = fetch_billing_data_for_day(process_day=today, service_id=service.id) @@ -474,7 +479,7 @@ def test_fetch_monthly_billing_for_year_adds_data_for_today(notify_db_session): notification_type="email", rate=0.162, ) - create_notification(template=template, status="delivered") + save_notification(create_notification(template=template, status="delivered")) assert db.session.query(FactBilling.bst_date).count() == 31 results = fetch_monthly_billing_for_year(service_id=service.id, year=2018) diff --git a/tests/app/dao/test_jobs_dao.py b/tests/app/dao/test_jobs_dao.py index 205f98e71f..d83cebc2c4 100644 --- a/tests/app/dao/test_jobs_dao.py +++ b/tests/app/dao/test_jobs_dao.py @@ -23,6 +23,7 @@ create_notification, create_service, create_template, + save_notification, ) @@ -31,11 +32,11 @@ def test_should_have_decorated_notifications_dao_functions(): def test_should_count_of_statuses_for_notifications_associated_with_job(sample_template, sample_job): - create_notification(sample_template, job=sample_job, status="created") - create_notification(sample_template, job=sample_job, status="created") - create_notification(sample_template, job=sample_job, status="created") - create_notification(sample_template, job=sample_job, status="sending") - create_notification(sample_template, job=sample_job, status="delivered") + save_notification(create_notification(sample_template, job=sample_job, status="created")) + save_notification(create_notification(sample_template, job=sample_job, status="created")) + save_notification(create_notification(sample_template, job=sample_job, status="created")) + save_notification(create_notification(sample_template, job=sample_job, status="sending")) + save_notification(create_notification(sample_template, job=sample_job, status="delivered")) results = dao_get_notification_outcomes_for_job(sample_template.service_id, sample_job.id) assert {row.status: row.count for row in results} == { @@ -53,8 +54,8 @@ def test_should_return_notifications_only_for_this_job(sample_template): job_1 = create_job(sample_template) job_2 = create_job(sample_template) - create_notification(sample_template, job=job_1, status="created") - create_notification(sample_template, job=job_2, status="sent") + save_notification(create_notification(sample_template, job=job_1, status="created")) + save_notification(create_notification(sample_template, job=job_2, status="sent")) results = dao_get_notification_outcomes_for_job(sample_template.service_id, job_1.id) assert {row.status: row.count for row in results} == {"created": 1} @@ -67,7 +68,7 @@ def test_should_return_notifications_only_for_this_service( other_template = create_template(service=other_service) other_job = create_job(other_template) - create_notification(other_template, job=other_job) + save_notification(create_notification(other_template, job=other_job)) assert len(dao_get_notification_outcomes_for_job(sample_notification_with_job.service_id, other_job.id)) == 0 assert len(dao_get_notification_outcomes_for_job(other_service.id, sample_notification_with_job.id)) == 0 @@ -344,7 +345,7 @@ def assert_job_stat(job, result, sent, delivered, failed): @freeze_time("2019-06-13 13:00") def test_dao_cancel_letter_job_cancels_job_and_returns_number_of_cancelled_notifications(sample_letter_template, admin_request): job = create_job(template=sample_letter_template, notification_count=1, job_status="finished") - notification = create_notification(template=job.template, job=job, status="created") + notification = save_notification(create_notification(template=job.template, job=job, status="created")) result = dao_cancel_letter_job(job) assert result == 1 assert notification.status == "cancelled" @@ -354,7 +355,7 @@ def test_dao_cancel_letter_job_cancels_job_and_returns_number_of_cancelled_notif @freeze_time("2019-06-13 13:00") def test_can_letter_job_be_cancelled_returns_true_if_job_can_be_cancelled(sample_letter_template, admin_request): job = create_job(template=sample_letter_template, notification_count=1, job_status="finished") - create_notification(template=job.template, job=job, status="created") + save_notification(create_notification(template=job.template, job=job, status="created")) result, errors = can_letter_job_be_cancelled(job) assert result assert not errors @@ -365,8 +366,8 @@ def test_can_letter_job_be_cancelled_returns_false_and_error_message_if_notifica sample_letter_template, admin_request ): job = create_job(template=sample_letter_template, notification_count=2, job_status="finished") - create_notification(template=job.template, job=job, status="sending") - create_notification(template=job.template, job=job, status="created") + save_notification(create_notification(template=job.template, job=job, status="sending")) + save_notification(create_notification(template=job.template, job=job, status="created")) result, errors = can_letter_job_be_cancelled(job) assert not result assert errors == "It’s too late to cancel sending, these letters have already been sent." @@ -378,7 +379,7 @@ def test_can_letter_job_be_cancelled_returns_false_and_error_message_if_letters_ ): with freeze_time("2019-06-13 13:00"): job = create_job(template=sample_letter_template, notification_count=1, job_status="finished") - letter = create_notification(template=job.template, job=job, status="created") + letter = save_notification(create_notification(template=job.template, job=job, status="created")) with freeze_time("2019-06-13 17:32"): result, errors = can_letter_job_be_cancelled(job) @@ -391,7 +392,7 @@ def test_can_letter_job_be_cancelled_returns_false_and_error_message_if_letters_ @freeze_time("2019-06-13 13:00") def test_can_letter_job_be_cancelled_returns_false_and_error_message_if_not_a_letter_job(sample_template, admin_request): job = create_job(template=sample_template, notification_count=1, job_status="finished") - create_notification(template=job.template, job=job, status="created") + save_notification(create_notification(template=job.template, job=job, status="created")) result, errors = can_letter_job_be_cancelled(job) assert not result assert errors == "Only letter jobs can be cancelled through this endpoint. This is not a letter job." @@ -400,7 +401,7 @@ def test_can_letter_job_be_cancelled_returns_false_and_error_message_if_not_a_le @freeze_time("2019-06-13 13:00") def test_can_letter_job_be_cancelled_returns_false_and_error_message_if_job_not_finished(sample_letter_template, admin_request): job = create_job(template=sample_letter_template, notification_count=1, job_status="in progress") - create_notification(template=job.template, job=job, status="created") + save_notification(create_notification(template=job.template, job=job, status="created")) result, errors = can_letter_job_be_cancelled(job) assert not result assert errors == "We are still processing these letters, please try again in a minute." @@ -411,7 +412,7 @@ def test_can_letter_job_be_cancelled_returns_false_and_error_message_if_notifica sample_letter_template, admin_request ): job = create_job(template=sample_letter_template, notification_count=2, job_status="finished") - create_notification(template=job.template, job=job, status="created") + save_notification(create_notification(template=job.template, job=job, status="created")) result, errors = can_letter_job_be_cancelled(job) assert not result assert errors == "We are still processing these letters, please try again in a minute." diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index f9cc21eaa1..daac8d79f1 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -83,6 +83,7 @@ create_template, create_template_folder, create_user, + save_notification, ) # from unittest import mock @@ -813,7 +814,8 @@ def test_create_service_and_history_is_transactional(notify_db_session): with pytest.raises(IntegrityError) as excinfo: dao_create_service(service, user) - assert 'column "name" violates not-null constraint' in str(excinfo.value) + assert 'column "name" of relation "services_history" violates not-null constraint' in str(excinfo.value) + assert Service.query.count() == 0 assert Service.get_history_model().query.count() == 0 @@ -825,7 +827,7 @@ def test_delete_service_and_associated_objects(notify_db_session): create_user_code(user=user, code="somecode", code_type="sms") template = create_template(service=service) api_key = create_api_key(service=service) - create_notification(template=template, api_key=api_key) + save_notification(create_notification(template=template, api_key=api_key)) create_invited_user(service=service) assert ServicePermission.query.count() == len( @@ -909,7 +911,7 @@ def test_add_existing_user_to_another_service_doesnot_change_old_permissions( def test_fetch_stats_filters_on_service(notify_db_session): service_one = create_service() - create_notification(template=create_template(service=service_one)) + save_notification(create_notification(template=create_template(service=service_one))) service_two = Service( name="service_two", @@ -939,10 +941,10 @@ def test_fetch_stats_counts_correctly(notify_db_session): sms_template = create_template(service=service) email_template = create_template(service=service, template_type="email") # two created email, one failed email, and one created sms - create_notification(template=email_template, status="created") - create_notification(template=email_template, status="created") - create_notification(template=email_template, status="technical-failure") - create_notification(template=sms_template, status="created") + save_notification(create_notification(template=email_template, status="created")) + save_notification(create_notification(template=email_template, status="created")) + save_notification(create_notification(template=email_template, status="technical-failure")) + save_notification(create_notification(template=sms_template, status="created")) stats = dao_fetch_stats_for_service(sms_template.service_id, 7) stats = sorted(stats, key=lambda x: (x.notification_type, x.status)) @@ -969,10 +971,10 @@ def test_fetch_stats_counts_should_ignore_team_key(notify_db_session): test_api_key = create_api_key(service=service, key_type=KEY_TYPE_TEST) # two created email, one failed email, and one created sms - create_notification(template=template, api_key=live_api_key, key_type=live_api_key.key_type) - create_notification(template=template, api_key=test_api_key, key_type=test_api_key.key_type) - create_notification(template=template, api_key=team_api_key, key_type=team_api_key.key_type) - create_notification(template=template) + save_notification(create_notification(template=template, api_key=live_api_key, key_type=live_api_key.key_type)) + save_notification(create_notification(template=template, api_key=test_api_key, key_type=test_api_key.key_type)) + save_notification(create_notification(template=template, api_key=team_api_key, key_type=team_api_key.key_type)) + save_notification(create_notification(template=template)) stats = dao_fetch_stats_for_service(template.service_id, 7) assert len(stats) == 1 @@ -986,15 +988,15 @@ def test_fetch_stats_for_today_only_includes_today(notify_db_session): # two created email, one failed email, and one created sms with freeze_time("2001-01-01T23:59:00"): # just_before_midnight_yesterday - create_notification(template=template, to_field="1", status="delivered") + save_notification(create_notification(template=template, to_field="1", status="delivered")) with freeze_time("2001-01-02T00:01:00"): # just_after_midnight_today - create_notification(template=template, to_field="2", status="failed") + save_notification(create_notification(template=template, to_field="2", status="failed")) with freeze_time("2001-01-02T12:00:00"): # right_now - create_notification(template=template, to_field="3", status="created") + save_notification(create_notification(template=template, to_field="3", status="created")) stats = dao_fetch_todays_stats_for_service(template.service_id) @@ -1021,8 +1023,10 @@ def test_fetch_stats_for_today_only_includes_today(notify_db_session): def test_fetch_stats_should_not_gather_notifications_older_than_7_days(sample_template, created_at, limit_days, rows_returned): # It's monday today. Things made last monday should still show with freeze_time(created_at): - create_notification( - sample_template, + save_notification( + create_notification( + sample_template, + ) ) with freeze_time("Monday 16th July 2018 12:00"): @@ -1034,7 +1038,7 @@ def test_fetch_stats_should_not_gather_notifications_older_than_7_days(sample_te def test_dao_fetch_todays_total_message_count_returns_count_for_today( notify_db_session, ): - notification = create_notification(template=create_template(service=create_service())) + notification = save_notification(create_notification(template=create_template(service=create_service()))) assert fetch_todays_total_message_count(notification.service.id) == 1 @@ -1052,10 +1056,10 @@ def test_dao_fetch_todays_stats_for_all_services_includes_all_services( template_sms_one = create_template(service=service1, template_type="sms") template_email_two = create_template(service=service2, template_type="email") template_sms_two = create_template(service=service2, template_type="sms") - create_notification(template=template_email_one) - create_notification(template=template_sms_one) - create_notification(template=template_email_two) - create_notification(template=template_sms_two) + save_notification(create_notification(template=template_email_one)) + save_notification(create_notification(template=template_sms_one)) + save_notification(create_notification(template=template_email_two)) + save_notification(create_notification(template=template_sms_two)) stats = dao_fetch_todays_stats_for_all_services() @@ -1069,11 +1073,11 @@ def test_dao_fetch_todays_stats_for_all_services_only_includes_today(notify_db_s template = create_template(service=create_service()) with freeze_time("2001-01-02T03:59:00"): # just_before_midnight_yesterday - create_notification(template=template, to_field="1", status="delivered") + save_notification(create_notification(template=template, to_field="1", status="delivered")) with freeze_time("2001-01-02T05:01:00"): # just_after_midnight_today - create_notification(template=template, to_field="2", status="failed") + save_notification(create_notification(template=template, to_field="2", status="failed")) with freeze_time("2001-01-02T05:00:00"): stats = dao_fetch_todays_stats_for_all_services() @@ -1090,12 +1094,12 @@ def test_dao_fetch_todays_stats_for_all_services_groups_correctly(notify_db, not template_email = create_template(service=service1, template_type="email") template_two = create_template(service=service2) # service1: 2 sms with status "created" and one "failed", and one email - create_notification(template=template_sms) - create_notification(template=template_sms) - create_notification(template=template_sms, status="failed") - create_notification(template=template_email) + save_notification(create_notification(template=template_sms)) + save_notification(create_notification(template=template_sms)) + save_notification(create_notification(template=template_sms, status="failed")) + save_notification(create_notification(template=template_email)) # service2: 1 sms "created" - create_notification(template=template_two) + save_notification(create_notification(template=template_two)) stats = dao_fetch_todays_stats_for_all_services() assert len(stats) == 4 @@ -1149,9 +1153,9 @@ def test_dao_fetch_todays_stats_for_all_services_includes_all_keys_by_default( notify_db_session, ): template = create_template(service=create_service()) - create_notification(template=template, key_type=KEY_TYPE_NORMAL) - create_notification(template=template, key_type=KEY_TYPE_TEAM) - create_notification(template=template, key_type=KEY_TYPE_TEST) + save_notification(create_notification(template=template, key_type=KEY_TYPE_NORMAL)) + save_notification(create_notification(template=template, key_type=KEY_TYPE_TEAM)) + save_notification(create_notification(template=template, key_type=KEY_TYPE_TEST)) stats = dao_fetch_todays_stats_for_all_services() @@ -1163,9 +1167,9 @@ def test_dao_fetch_todays_stats_for_all_services_can_exclude_from_test_key( notify_db_session, ): template = create_template(service=create_service()) - create_notification(template=template, key_type=KEY_TYPE_NORMAL) - create_notification(template=template, key_type=KEY_TYPE_TEAM) - create_notification(template=template, key_type=KEY_TYPE_TEST) + save_notification(create_notification(template=template, key_type=KEY_TYPE_NORMAL)) + save_notification(create_notification(template=template, key_type=KEY_TYPE_TEAM)) + save_notification(create_notification(template=template, key_type=KEY_TYPE_TEST)) stats = dao_fetch_todays_stats_for_all_services(include_from_test_key=False) diff --git a/tests/app/db.py b/tests/app/db.py index 1101006ee4..f60b5c5875 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -233,13 +233,15 @@ def create_notification( rate_multiplier=None, international=False, phone_prefix=None, - scheduled_for=None, normalised_to=None, one_off=False, reply_to_text=None, created_by_id=None, postage=None, ): + """ + Creates in memory Notification Model + """ assert job or template if job: template = job.template @@ -294,19 +296,32 @@ def create_notification( "created_by_id": created_by_id, "postage": postage, } - notification = Notification(**data) - dao_create_notification(notification) - if scheduled_for: - scheduled_notification = ScheduledNotification( - id=uuid.uuid4(), - notification_id=notification.id, - scheduled_for=datetime.strptime(scheduled_for, "%Y-%m-%d %H:%M"), - ) - if status != "created": - scheduled_notification.pending = False - dao_created_scheduled_notification(scheduled_notification) + return Notification(**data) + + +def save_notification(notification_model): + """ + Save Notification into the DB + """ + dao_create_notification(notification_model) + return notification_model + + +def save_scheduled_notification(notification_model, scheduled_for): + """ + Create and save ScheduledNotifcation object + """ + dao_create_notification(notification_model) + scheduled_notification = ScheduledNotification( + id=uuid.uuid4(), + notification_id=notification_model.id, + scheduled_for=datetime.strptime(scheduled_for, "%Y-%m-%d %H:%M"), + ) + if notification_model.status != "created": + scheduled_notification.pending = False + dao_created_scheduled_notification(scheduled_notification) - return notification + return notification_model def create_notification_history( diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index faf418f6da..bfa5f2d0f4 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -16,6 +16,7 @@ ) from app.delivery import send_to_providers from app.exceptions import ( + InvalidUrlException, MalwarePendingException, NotificationTechnicalFailureException, ) @@ -39,6 +40,7 @@ create_service_sms_sender, create_service_with_defined_sms_sender, create_template, + save_notification, ) from tests.conftest import set_config_values @@ -92,12 +94,14 @@ def test_provider_to_use(restore_provider_details): def test_should_send_personalised_template_to_correct_sms_provider_and_persist(sample_sms_template_with_html, mocker): - db_notification = create_notification( - template=sample_sms_template_with_html, - to_field="+16502532222", - personalisation={"name": "Jo"}, - status="created", - reply_to_text=sample_sms_template_with_html.service.get_default_sms_sender(), + db_notification = save_notification( + create_notification( + template=sample_sms_template_with_html, + to_field="+16502532222", + personalisation={"name": "Jo"}, + status="created", + reply_to_text=sample_sms_template_with_html.service.get_default_sms_sender(), + ) ) statsd_mock = mocker.patch("app.delivery.send_to_providers.statsd_client") @@ -129,10 +133,12 @@ def test_should_send_personalised_template_to_correct_sms_provider_and_persist(s def test_should_send_personalised_template_to_correct_email_provider_and_persist(sample_email_template_with_html, mocker): - db_notification = create_notification( - template=sample_email_template_with_html, - to_field="jo.smith@example.com", - personalisation={"name": "Jo"}, + db_notification = save_notification( + create_notification( + template=sample_email_template_with_html, + to_field="jo.smith@example.com", + personalisation={"name": "Jo"}, + ) ) mocker.patch("app.aws_ses_client.send_email", return_value="reference") @@ -167,10 +173,12 @@ def test_should_send_personalised_template_to_correct_email_provider_and_persist def test_should_send_personalised_template_with_html_enabled(sample_email_template_with_advanced_html, mocker, notify_api): - db_notification = create_notification( - template=sample_email_template_with_advanced_html, - to_field="jo.smith@example.com", - personalisation={"name": "Jo"}, + db_notification = save_notification( + create_notification( + template=sample_email_template_with_advanced_html, + to_field="jo.smith@example.com", + personalisation={"name": "Jo"}, + ) ) mocker.patch("app.aws_ses_client.send_email", return_value="reference") @@ -214,10 +222,12 @@ def test_should_not_send_email_message_when_service_is_inactive_notifcation_is_i def test_should_respect_custom_sending_domains(sample_service, mocker, sample_email_template_with_html): - db_notification = create_notification( - template=sample_email_template_with_html, - to_field="jo.smith@example.com", - personalisation={"name": "Jo"}, + db_notification = save_notification( + create_notification( + template=sample_email_template_with_html, + to_field="jo.smith@example.com", + personalisation={"name": "Jo"}, + ) ) sample_service.sending_domain = "foo.bar" @@ -254,12 +264,14 @@ def test_should_not_send_sms_message_when_service_is_inactive_notifcation_is_in_ def test_should_not_send_sms_message_when_message_is_empty_or_whitespace(sample_service, mocker, var): sample_service.prefix_sms = False template = create_template(sample_service, content="((var))") - notification = create_notification( - template=template, - personalisation={"var": var}, - to_field="+16502532222", - status="created", - reply_to_text=sample_service.get_default_sms_sender(), + notification = save_notification( + create_notification( + template=template, + personalisation={"var": var}, + to_field="+16502532222", + status="created", + reply_to_text=sample_service.get_default_sms_sender(), + ) ) send_mock = mocker.patch("app.aws_sns_client.send_sms", return_value="reference") @@ -271,11 +283,13 @@ def test_should_not_send_sms_message_when_message_is_empty_or_whitespace(sample_ def test_send_sms_should_use_template_version_from_notification_not_latest(sample_template, mocker): - db_notification = create_notification( - template=sample_template, - to_field="+16502532222", - status="created", - reply_to_text=sample_template.service.get_default_sms_sender(), + db_notification = save_notification( + create_notification( + template=sample_template, + to_field="+16502532222", + status="created", + reply_to_text=sample_template.service.get_default_sms_sender(), + ) ) mocker.patch("app.aws_sns_client.send_sms", return_value="message_id_from_sns") @@ -351,7 +365,7 @@ def test_should_not_have_sent_status_if_fake_callback_function_fails(sample_noti def test_should_not_send_to_provider_when_status_is_not_created(sample_template, mocker): - notification = create_notification(template=sample_template, status="sending") + notification = save_notification(create_notification(template=sample_template, status="sending")) mocker.patch("app.aws_sns_client.send_sms") response_mock = mocker.patch("app.delivery.send_to_providers.send_sms_response") @@ -369,7 +383,7 @@ def test_should_send_sms_with_downgraded_content(notify_db_session, mocker): gsm_message = "?odz Housing Service: a é i o u ? foo barbaz???abc..." service = create_service(service_name="Łódź Housing Service") template = create_template(service, content=msg) - db_notification = create_notification(template=template, personalisation={"misc": placeholder}) + db_notification = save_notification(create_notification(template=template, personalisation={"misc": placeholder})) mocker.patch("app.aws_sns_client.send_sms", return_value="message_id_from_sns") @@ -382,7 +396,7 @@ def test_send_sms_should_use_service_sms_sender(sample_service, sample_template, mocker.patch("app.aws_sns_client.send_sms", return_value="message_id_from_sns") sms_sender = create_service_sms_sender(service=sample_service, sms_sender="123456", is_default=False) - db_notification = create_notification(template=sample_template, reply_to_text=sms_sender.sms_sender) + db_notification = save_notification(create_notification(template=sample_template, reply_to_text=sms_sender.sms_sender)) send_to_providers.send_sms_to_provider( db_notification, @@ -395,11 +409,13 @@ def test_send_sms_should_use_service_sms_sender(sample_service, sample_template, def test_send_email_to_provider_should_call_research_mode_task_response_task_if_research_mode( sample_service, sample_email_template, mocker, research_mode, key_type ): - notification = create_notification( - template=sample_email_template, - to_field="john@smith.com", - key_type=key_type, - billable_units=0, + notification = save_notification( + create_notification( + template=sample_email_template, + to_field="john@smith.com", + key_type=key_type, + billable_units=0, + ) ) sample_service.research_mode = research_mode @@ -423,7 +439,7 @@ def test_send_email_to_provider_should_call_research_mode_task_response_task_if_ def test_send_email_to_provider_should_not_send_to_provider_when_status_is_not_created(sample_email_template, mocker): - notification = create_notification(template=sample_email_template, status="sending") + notification = save_notification(create_notification(template=sample_email_template, status="sending")) mocker.patch("app.aws_ses_client.send_email") mocker.patch("app.delivery.send_to_providers.send_email_response") @@ -435,7 +451,7 @@ def test_send_email_to_provider_should_not_send_to_provider_when_status_is_not_c def test_send_email_should_use_service_reply_to_email(sample_service, sample_email_template, mocker): mocker.patch("app.aws_ses_client.send_email", return_value="reference") - db_notification = create_notification(template=sample_email_template, reply_to_text="foo@bar.com") + db_notification = save_notification(create_notification(template=sample_email_template, reply_to_text="foo@bar.com")) create_reply_to_email(service=sample_service, email_address="foo@bar.com") send_to_providers.send_email_to_provider( @@ -592,7 +608,9 @@ def __update_notification(notification_to_update, research_mode, expected_status def test_should_update_billable_units_and_status_according_to_research_mode_and_key_type( sample_template, mocker, research_mode, key_type, billable_units, expected_status ): - notification = create_notification(template=sample_template, billable_units=0, status="created", key_type=key_type) + notification = save_notification( + create_notification(template=sample_template, billable_units=0, status="created", key_type=key_type) + ) mocker.patch("app.aws_sns_client.send_sms", return_value="message_id_from_sns") mocker.patch( "app.delivery.send_to_providers.send_sms_response", @@ -630,22 +648,26 @@ def test_should_send_sms_to_international_providers(restore_provider_details, sa dao_switch_sms_provider_to_provider_with_identifier("firetext") - db_notification_uk = create_notification( - template=sample_sms_template_with_html, - to_field="+16135555555", - personalisation={"name": "Jo"}, - status="created", - international=False, - reply_to_text=sample_sms_template_with_html.service.get_default_sms_sender(), + db_notification_uk = save_notification( + create_notification( + template=sample_sms_template_with_html, + to_field="+16135555555", + personalisation={"name": "Jo"}, + status="created", + international=False, + reply_to_text=sample_sms_template_with_html.service.get_default_sms_sender(), + ) ) - db_notification_international = create_notification( - template=sample_sms_template_with_html, - to_field="+1613555555", - personalisation={"name": "Jo"}, - status="created", - international=False, - reply_to_text=sample_sms_template_with_html.service.get_default_sms_sender(), + db_notification_international = save_notification( + create_notification( + template=sample_sms_template_with_html, + to_field="+1613555555", + personalisation={"name": "Jo"}, + status="created", + international=False, + reply_to_text=sample_sms_template_with_html.service.get_default_sms_sender(), + ) ) mocker.patch("app.aws_sns_client.send_sms") @@ -687,7 +709,7 @@ def test_should_handle_sms_sender_and_prefix_message( mocker.patch("app.aws_sns_client.send_sms", return_value="message_id_from_sns") service = create_service_with_defined_sms_sender(sms_sender_value=sms_sender, prefix_sms=prefix_sms) template = create_template(service, content="bar") - notification = create_notification(template, reply_to_text=sms_sender) + notification = save_notification(create_notification(template, reply_to_text=sms_sender)) send_to_providers.send_sms_to_provider(notification) @@ -702,7 +724,7 @@ def test_should_handle_sms_sender_and_prefix_message( def test_send_email_to_provider_uses_reply_to_from_notification(sample_email_template, mocker): mocker.patch("app.aws_ses_client.send_email", return_value="reference") - db_notification = create_notification(template=sample_email_template, reply_to_text="test@test.com") + db_notification = save_notification(create_notification(template=sample_email_template, reply_to_text="test@test.com")) send_to_providers.send_email_to_provider( db_notification, @@ -722,7 +744,7 @@ def test_send_email_to_provider_uses_reply_to_from_notification(sample_email_tem def test_send_email_to_provider_should_format_reply_to_email_address(sample_email_template, mocker): mocker.patch("app.aws_ses_client.send_email", return_value="reference") - db_notification = create_notification(template=sample_email_template, reply_to_text="test@test.com\t") + db_notification = save_notification(create_notification(template=sample_email_template, reply_to_text="test@test.com\t")) send_to_providers.send_email_to_provider( db_notification, @@ -774,7 +796,7 @@ def test_notification_can_have_document_attachment_without_mlwr_sid(sample_email del response["document"]["mlwr_sid"] personalisation = {"file": response} - db_notification = create_notification(template=sample_email_template, personalisation=personalisation) + db_notification = save_notification(create_notification(template=sample_email_template, personalisation=personalisation)) send_to_providers.send_email_to_provider( db_notification, @@ -789,7 +811,7 @@ def test_notification_can_have_document_attachment_if_mlwr_sid_is_false(sample_e mlwr_mock = mocker.patch("app.delivery.send_to_providers.check_mlwr") personalisation = {"file": document_download_response({"mlwr_sid": "false"})} - db_notification = create_notification(template=sample_email_template, personalisation=personalisation) + db_notification = save_notification(create_notification(template=sample_email_template, personalisation=personalisation)) send_to_providers.send_email_to_provider( db_notification, @@ -804,7 +826,7 @@ def test_notification_raises_a_retry_exception_if_mlwr_state_is_missing(sample_e mocker.patch("app.delivery.send_to_providers.check_mlwr", return_value={}) personalisation = {"file": document_download_response()} - db_notification = create_notification(template=sample_email_template, personalisation=personalisation) + db_notification = save_notification(create_notification(template=sample_email_template, personalisation=personalisation)) with pytest.raises(MalwarePendingException): send_to_providers.send_email_to_provider( @@ -817,7 +839,7 @@ def test_notification_raises_a_retry_exception_if_mlwr_state_is_not_complete(sam mocker.patch("app.delivery.send_to_providers.check_mlwr", return_value={"state": "foo"}) personalisation = {"file": document_download_response()} - db_notification = create_notification(template=sample_email_template, personalisation=personalisation) + db_notification = save_notification(create_notification(template=sample_email_template, personalisation=personalisation)) with pytest.raises(MalwarePendingException): send_to_providers.send_email_to_provider( @@ -833,7 +855,7 @@ def test_notification_raises_sets_notification_to_virus_found_if_mlwr_score_is_5 ) personalisation = {"file": document_download_response()} - db_notification = create_notification(template=sample_email_template, personalisation=personalisation) + db_notification = save_notification(create_notification(template=sample_email_template, personalisation=personalisation)) with pytest.raises(NotificationTechnicalFailureException) as e: send_to_providers.send_email_to_provider(db_notification) @@ -851,7 +873,7 @@ def test_notification_raises_sets_notification_to_virus_found_if_mlwr_score_abov ) personalisation = {"file": document_download_response()} - db_notification = create_notification(template=sample_email_template, personalisation=personalisation) + db_notification = save_notification(create_notification(template=sample_email_template, personalisation=personalisation)) with pytest.raises(NotificationTechnicalFailureException) as e: send_to_providers.send_email_to_provider(db_notification) @@ -894,7 +916,7 @@ def test_notification_document_with_pdf_attachment( else: personalisation["file"]["document"]["sending_method"] = "link" - db_notification = create_notification(template=template, personalisation=personalisation) + db_notification = save_notification(create_notification(template=template, personalisation=personalisation)) statsd_mock = mocker.patch("app.delivery.send_to_providers.statsd_client") send_mock = mocker.patch("app.aws_ses_client.send_email", return_value="reference") @@ -944,15 +966,53 @@ def test_notification_document_with_pdf_attachment( assert call(statsd_key) in statsd_mock.incr.call_args_list +@pytest.mark.parametrize( + "sending_method", + [ + ("attach"), + ("link"), + ], +) +def test_notification_with_bad_file_attachment_url(mocker, notify_db, notify_db_session, sending_method): + template = sample_email_template(notify_db, notify_db_session, content="Here is your ((file))") + personalisation = { + "file": document_download_response( + { + "direct_file_url": "file://foo.bar/file.txt" if sending_method == "attach" else "http://foo.bar/file.txt", + "url": "file://foo.bar/file.txt" if sending_method == "link" else "http://foo.bar/file.txt", + "mime_type": "application/pdf", + "mlwr_sid": "false", + } + ) + } + personalisation["file"]["document"]["sending_method"] = sending_method + if sending_method == "attach": + personalisation["file"]["document"]["filename"] = "file.txt" + + db_notification = save_notification(create_notification(template=template, personalisation=personalisation)) + + # See https://stackoverflow.com/a/34929900 + cm = MagicMock() + cm.read.return_value = "request_content" + cm.__enter__.return_value = cm + urlopen_mock = mocker.patch("app.delivery.send_to_providers.urllib.request.urlopen") + urlopen_mock.return_value = cm + + with pytest.raises(InvalidUrlException): + send_to_providers.send_email_to_provider(db_notification) + + def test_notification_raises_error_if_message_contains_sin_pii_that_passes_luhn( sample_email_template_with_html, mocker, notify_api ): send_mock = mocker.patch("app.aws_ses_client.send_email", return_value="reference") - db_notification = create_notification( - template=sample_email_template_with_html, - to_field="jo.smith@example.com", - personalisation={"name": "046-454-286"}, + db_notification = save_notification( + create_notification( + template=sample_email_template_with_html, + to_field="jo.smith@example.com", + personalisation={"name": "046-454-286"}, + ) ) with set_config_values( @@ -973,10 +1033,12 @@ def test_notification_raises_error_if_message_contains_sin_pii_that_passes_luhn( def test_notification_passes_if_message_contains_sin_pii_that_fails_luhn(sample_email_template_with_html, mocker, notify_api): send_mock = mocker.patch("app.aws_ses_client.send_email", return_value="reference") - db_notification = create_notification( - template=sample_email_template_with_html, - to_field="jo.smith@example.com", - personalisation={"name": "123-456-789"}, + db_notification = save_notification( + create_notification( + template=sample_email_template_with_html, + to_field="jo.smith@example.com", + personalisation={"name": "123-456-789"}, + ) ) send_to_providers.send_email_to_provider(db_notification) @@ -989,10 +1051,12 @@ def test_notification_passes_if_message_contains_sin_pii_that_fails_luhn(sample_ def test_notification_passes_if_message_contains_phone_number(sample_email_template_with_html, mocker): send_mock = mocker.patch("app.aws_ses_client.send_email", return_value="reference") - db_notification = create_notification( - template=sample_email_template_with_html, - to_field="jo.smith@example.com", - personalisation={"name": "123-456-7890"}, + db_notification = save_notification( + create_notification( + template=sample_email_template_with_html, + to_field="jo.smith@example.com", + personalisation={"name": "123-456-7890"}, + ) ) send_to_providers.send_email_to_provider(db_notification) diff --git a/tests/app/job/test_rest.py b/tests/app/job/test_rest.py index 6539471144..97f537c956 100644 --- a/tests/app/job/test_rest.py +++ b/tests/app/job/test_rest.py @@ -10,7 +10,12 @@ from app.dao.templates_dao import dao_update_template from app.models import JOB_STATUS_PENDING, JOB_STATUS_TYPES from tests import create_authorization_header -from tests.app.db import create_ft_notification_status, create_job, create_notification +from tests.app.db import ( + create_ft_notification_status, + create_job, + create_notification, + save_notification, +) from tests.conftest import set_config @@ -70,7 +75,7 @@ def test_cant_cancel_normal_job(client, sample_job, mocker): @freeze_time("2019-06-13 13:00") def test_cancel_letter_job_updates_notifications_and_job_to_cancelled(sample_letter_template, admin_request, mocker): job = create_job(template=sample_letter_template, notification_count=1, job_status="finished") - create_notification(template=job.template, job=job, status="created") + save_notification(create_notification(template=job.template, job=job, status="created")) mock_get_job = mocker.patch("app.job.rest.dao_get_job_by_service_id_and_job_id", return_value=job) mock_can_letter_job_be_cancelled = mocker.patch("app.job.rest.can_letter_job_be_cancelled", return_value=(True, None)) @@ -94,8 +99,8 @@ def test_cancel_letter_job_does_not_call_cancel_if_can_letter_job_be_cancelled_r sample_letter_template, admin_request, mocker ): job = create_job(template=sample_letter_template, notification_count=2, job_status="finished") - create_notification(template=job.template, job=job, status="sending") - create_notification(template=job.template, job=job, status="created") + save_notification(create_notification(template=job.template, job=job, status="sending")) + save_notification(create_notification(template=job.template, job=job, status="created")) mock_get_job = mocker.patch("app.job.rest.dao_get_job_by_service_id_and_job_id", return_value=job) error_message = "Sorry, it's too late, letters have already been sent." @@ -498,10 +503,10 @@ def test_get_all_notifications_for_job_in_order_of_job_number(admin_request, sam main_job = create_job(sample_template) another_job = create_job(sample_template) - notification_1 = create_notification(job=main_job, to_field="1", job_row_number=1) - notification_2 = create_notification(job=main_job, to_field="2", job_row_number=2) - notification_3 = create_notification(job=main_job, to_field="3", job_row_number=3) - create_notification(job=another_job) + notification_1 = save_notification(create_notification(job=main_job, to_field="1", job_row_number=1)) + notification_2 = save_notification(create_notification(job=main_job, to_field="2", job_row_number=2)) + notification_3 = save_notification(create_notification(job=main_job, to_field="3", job_row_number=3)) + save_notification(create_notification(job=another_job)) resp = admin_request.get( "job.get_all_notifications_for_service_job", @@ -528,7 +533,7 @@ def test_get_all_notifications_for_job_in_order_of_job_number(admin_request, sam ], ) def test_get_all_notifications_for_job_filtered_by_status(admin_request, sample_job, expected_notification_count, status_args): - create_notification(job=sample_job, to_field="1", status="created") + save_notification(create_notification(job=sample_job, to_field="1", status="created")) resp = admin_request.get( "job.get_all_notifications_for_service_job", @@ -569,16 +574,16 @@ def test_get_job_by_id_should_return_summed_statistics(admin_request, sample_job job_id = str(sample_job.id) service_id = sample_job.service.id - create_notification(job=sample_job, status="created") - create_notification(job=sample_job, status="created") - create_notification(job=sample_job, status="created") - create_notification(job=sample_job, status="sending") - create_notification(job=sample_job, status="failed") - create_notification(job=sample_job, status="failed") - create_notification(job=sample_job, status="failed") - create_notification(job=sample_job, status="technical-failure") - create_notification(job=sample_job, status="temporary-failure") - create_notification(job=sample_job, status="temporary-failure") + save_notification(create_notification(job=sample_job, status="created")) + save_notification(create_notification(job=sample_job, status="created")) + save_notification(create_notification(job=sample_job, status="created")) + save_notification(create_notification(job=sample_job, status="sending")) + save_notification(create_notification(job=sample_job, status="failed")) + save_notification(create_notification(job=sample_job, status="failed")) + save_notification(create_notification(job=sample_job, status="failed")) + save_notification(create_notification(job=sample_job, status="technical-failure")) + save_notification(create_notification(job=sample_job, status="temporary-failure")) + save_notification(create_notification(job=sample_job, status="temporary-failure")) resp_json = admin_request.get("job.get_job_by_service_and_job_id", service_id=service_id, job_id=job_id) @@ -624,12 +629,12 @@ def test_get_jobs_should_return_statistics(admin_request, sample_template): earlier = datetime.utcnow() - timedelta(days=1) job_1 = create_job(sample_template, processing_started=earlier) job_2 = create_job(sample_template, processing_started=now) - create_notification(job=job_1, status="created") - create_notification(job=job_1, status="created") - create_notification(job=job_1, status="created") - create_notification(job=job_2, status="sending") - create_notification(job=job_2, status="sending") - create_notification(job=job_2, status="sending") + save_notification(create_notification(job=job_1, status="created")) + save_notification(create_notification(job=job_1, status="created")) + save_notification(create_notification(job=job_1, status="created")) + save_notification(create_notification(job=job_2, status="sending")) + save_notification(create_notification(job=job_2, status="sending")) + save_notification(create_notification(job=job_2, status="sending")) resp_json = admin_request.get("job.get_jobs_by_service", service_id=sample_template.service_id) @@ -769,7 +774,7 @@ def test_get_jobs_should_retrieve_from_ft_notification_status_for_old_jobs(admin create_ft_notification_status(date(2017, 6, 6), job=job_1, notification_status="delivered", count=2) create_ft_notification_status(date(2017, 6, 7), job=job_1, notification_status="delivered", count=4) # job2's new enough - create_notification(job=job_2, status="created", created_at=not_quite_three_days_ago) + save_notification(create_notification(job=job_2, status="created", created_at=not_quite_three_days_ago)) # this isn't picked up because the job is too new create_ft_notification_status(date(2017, 6, 7), job=job_2, notification_status="delivered", count=8) @@ -777,7 +782,7 @@ def test_get_jobs_should_retrieve_from_ft_notification_status_for_old_jobs(admin create_ft_notification_status(date(2017, 6, 7), job=job_3, notification_status="delivered", count=16) # this isn't picked up because we're using the ft status table for job_1 as it's old - create_notification(job=job_1, status="created", created_at=not_quite_three_days_ago) + save_notification(create_notification(job=job_1, status="created", created_at=not_quite_three_days_ago)) resp_json = admin_request.get("job.get_jobs_by_service", service_id=sample_template.service_id) diff --git a/tests/app/letters/test_letter_utils.py b/tests/app/letters/test_letter_utils.py index 5a61c1eb18..b8291ac97b 100644 --- a/tests/app/letters/test_letter_utils.py +++ b/tests/app/letters/test_letter_utils.py @@ -23,7 +23,7 @@ NOTIFICATION_VALIDATION_FAILED, PRECOMPILED_TEMPLATE_NAME, ) -from tests.app.db import create_notification +from tests.app.db import create_notification, save_notification FROZEN_DATE_TIME = "2018-03-14 17:00:00" @@ -246,7 +246,7 @@ def test_upload_letter_pdf_to_correct_bucket(sample_letter_notification, mocker, @pytest.mark.parametrize("postage,expected_postage", [("second", 2), ("first", 1)]) def test_upload_letter_pdf_uses_postage_from_notification(sample_letter_template, mocker, postage, expected_postage): - letter_notification = create_notification(template=sample_letter_template, postage=postage) + letter_notification = save_notification(create_notification(template=sample_letter_template, postage=postage)) mock_s3 = mocker.patch("app.letters.utils.s3upload") filename = get_letter_pdf_filename( diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index 2e655b3cb2..aafd2b102c 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -205,7 +205,7 @@ def test_should_not_send_notification_for_archived_template(notify_api, sample_t ) assert resp.status_code == 400 json_resp = json.loads(resp.get_data(as_text=True)) - assert "Template has been deleted" in json_resp["message"] + assert f"Template {sample_template.id} has been deleted" in json_resp["message"] @pytest.mark.parametrize( diff --git a/tests/app/notifications/test_notifications_ses_callback.py b/tests/app/notifications/test_notifications_ses_callback.py index 3ce37eba11..2ae5fcabf3 100644 --- a/tests/app/notifications/test_notifications_ses_callback.py +++ b/tests/app/notifications/test_notifications_ses_callback.py @@ -16,7 +16,11 @@ handle_complaint, ) from tests.app.conftest import sample_notification as create_sample_notification -from tests.app.db import create_notification, create_notification_history +from tests.app.db import ( + create_notification, + create_notification_history, + save_notification, +) @pytest.mark.parametrize( @@ -131,7 +135,7 @@ def test_ses_callback_should_not_set_status_once_status_is_delivered( def test_process_ses_results_in_complaint(sample_email_template): - notification = create_notification(template=sample_email_template, reference="ref1") + notification = save_notification(create_notification(template=sample_email_template, reference="ref1")) handle_complaint(json.loads(ses_complaint_callback()["Message"])) complaints = Complaint.query.all() assert len(complaints) == 1 @@ -153,7 +157,7 @@ def test_handle_complaint_does_raise_exception_if_notification_not_found(notify_ def test_process_ses_results_in_complaint_if_notification_history_does_not_exist( sample_email_template, ): - notification = create_notification(template=sample_email_template, reference="ref1") + notification = save_notification(create_notification(template=sample_email_template, reference="ref1")) handle_complaint(json.loads(ses_complaint_callback()["Message"])) complaints = Complaint.query.all() assert len(complaints) == 1 @@ -171,7 +175,7 @@ def test_process_ses_results_in_complaint_if_notification_does_not_exist( def test_process_ses_results_in_complaint_save_complaint_with_null_complaint_type(notify_api, sample_email_template): - notification = create_notification(template=sample_email_template, reference="ref1") + notification = save_notification(create_notification(template=sample_email_template, reference="ref1")) msg = json.loads(ses_complaint_callback_with_missing_complaint_type()["Message"]) handle_complaint(msg) complaints = Complaint.query.all() diff --git a/tests/app/notifications/test_rest.py b/tests/app/notifications/test_rest.py index f1740dc034..21398850c8 100644 --- a/tests/app/notifications/test_rest.py +++ b/tests/app/notifications/test_rest.py @@ -11,7 +11,7 @@ from app.models import KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST, ApiKey from tests import create_authorization_header from tests.app.conftest import sample_notification as create_sample_notification -from tests.app.db import create_api_key, create_notification +from tests.app.db import create_api_key, create_notification, save_notification @pytest.mark.parametrize("type", ("email", "sms", "letter")) @@ -229,10 +229,10 @@ def test_do_not_return_job_notifications_by_default(client, sample_template, sam normal_api_key = create_api_key(sample_template.service, KEY_TYPE_NORMAL) test_api_key = create_api_key(sample_template.service, KEY_TYPE_TEST) - create_notification(sample_template, job=sample_job) - normal_notification = create_notification(sample_template, api_key=normal_api_key) - team_notification = create_notification(sample_template, api_key=team_api_key) - test_notification = create_notification(sample_template, api_key=test_api_key) + save_notification(create_notification(sample_template, job=sample_job)) + normal_notification = save_notification(create_notification(sample_template, api_key=normal_api_key)) + team_notification = save_notification(create_notification(sample_template, api_key=team_api_key)) + test_notification = save_notification(create_notification(sample_template, api_key=test_api_key)) notification_objs = { KEY_TYPE_NORMAL: normal_notification, @@ -423,8 +423,8 @@ def test_filter_by_template_type(client, notify_db, notify_db_session, sample_te def test_filter_by_multiple_template_types(client, sample_template, sample_email_template): - create_notification(sample_template) - create_notification(sample_email_template) + save_notification(create_notification(sample_template)) + save_notification(create_notification(sample_email_template)) auth_header = create_authorization_header(service_id=sample_email_template.service_id) @@ -437,8 +437,8 @@ def test_filter_by_multiple_template_types(client, sample_template, sample_email def test_filter_by_status(client, sample_email_template): - create_notification(sample_email_template, status="delivered") - create_notification(sample_email_template) + save_notification(create_notification(sample_email_template, status="delivered")) + save_notification(create_notification(sample_email_template)) auth_header = create_authorization_header(service_id=sample_email_template.service_id) @@ -451,8 +451,8 @@ def test_filter_by_status(client, sample_email_template): def test_filter_by_multiple_statuses(client, sample_email_template): - create_notification(sample_email_template, status="delivered") - create_notification(sample_email_template, status="sending") + save_notification(create_notification(sample_email_template, status="delivered")) + save_notification(create_notification(sample_email_template, status="sending")) auth_header = create_authorization_header(service_id=sample_email_template.service_id) @@ -465,9 +465,9 @@ def test_filter_by_multiple_statuses(client, sample_email_template): def test_filter_by_status_and_template_type(client, sample_template, sample_email_template): - create_notification(sample_template) - create_notification(sample_email_template) - create_notification(sample_email_template, status="delivered") + save_notification(create_notification(sample_template)) + save_notification(create_notification(sample_email_template)) + save_notification(create_notification(sample_email_template, status="delivered")) auth_header = create_authorization_header(service_id=sample_email_template.service_id) @@ -482,7 +482,9 @@ def test_filter_by_status_and_template_type(client, sample_template, sample_emai def test_get_notification_by_id_returns_merged_template_content(client, sample_template_with_placeholders): - sample_notification = create_notification(sample_template_with_placeholders, personalisation={"name": "world"}) + sample_notification = save_notification( + create_notification(sample_template_with_placeholders, personalisation={"name": "world"}) + ) auth_header = create_authorization_header(service_id=sample_notification.service_id) @@ -496,7 +498,9 @@ def test_get_notification_by_id_returns_merged_template_content(client, sample_t def test_get_notification_by_id_returns_merged_template_content_for_email(client, sample_email_template_with_placeholders): - sample_notification = create_notification(sample_email_template_with_placeholders, personalisation={"name": "world"}) + sample_notification = save_notification( + create_notification(sample_email_template_with_placeholders, personalisation={"name": "world"}) + ) auth_header = create_authorization_header(service_id=sample_notification.service_id) response = client.get("/notifications/{}".format(sample_notification.id), headers=[auth_header]) diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index ae311500b3..c33f9e41df 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -269,8 +269,8 @@ def test_check_template_is_active_fails(sample_template): with pytest.raises(BadRequestError) as e: check_template_is_active(sample_template) assert e.value.status_code == 400 - assert e.value.message == "Template has been deleted" - assert e.value.fields == [{"template": "Template has been deleted"}] + assert e.value.message == f"Template {sample_template.id} has been deleted" + assert e.value.fields == [{"template": f"Template {sample_template.id} has been deleted"}] @pytest.mark.parametrize("key_type", ["test", "normal"]) diff --git a/tests/app/performance_platform/test_processing_time.py b/tests/app/performance_platform/test_processing_time.py index f820f80f12..01c5035b25 100644 --- a/tests/app/performance_platform/test_processing_time.py +++ b/tests/app/performance_platform/test_processing_time.py @@ -6,7 +6,7 @@ send_processing_time_data, send_processing_time_to_performance_platform, ) -from tests.app.db import create_notification +from tests.app.db import create_notification, save_notification @freeze_time("2016-10-18T06:00") @@ -16,17 +16,21 @@ def test_send_processing_time_to_performance_platform_generates_correct_calls(mo created_at = datetime.utcnow() - timedelta(days=1) - create_notification( - sample_template, - created_at=created_at, - sent_at=created_at + timedelta(seconds=5), + save_notification( + create_notification( + sample_template, + created_at=created_at, + sent_at=created_at + timedelta(seconds=5), + ) ) - create_notification( - sample_template, - created_at=created_at, - sent_at=created_at + timedelta(seconds=15), + save_notification( + create_notification( + sample_template, + created_at=created_at, + sent_at=created_at + timedelta(seconds=15), + ) ) - create_notification(sample_template, created_at=datetime.utcnow() - timedelta(days=2)) + save_notification(create_notification(sample_template, created_at=datetime.utcnow() - timedelta(days=2))) send_processing_time_to_performance_platform(date(2016, 10, 17)) diff --git a/tests/app/platform_stats/test_rest.py b/tests/app/platform_stats/test_rest.py index 61be4a8289..3b14cbdbd0 100644 --- a/tests/app/platform_stats/test_rest.py +++ b/tests/app/platform_stats/test_rest.py @@ -11,6 +11,7 @@ create_notification, create_service, create_template, + save_notification, set_up_usage_data, ) @@ -53,9 +54,9 @@ def test_get_platform_stats_with_real_query(admin_request, notify_db_session): create_ft_notification_status(date(2018, 10, 29), "sms", service_1, count=10) create_ft_notification_status(date(2018, 10, 29), "email", service_1, count=3) - create_notification(sms_template, created_at=datetime(2018, 10, 31, 11, 0, 0), key_type="test") - create_notification(sms_template, created_at=datetime(2018, 10, 31, 12, 0, 0), status="delivered") - create_notification(email_template, created_at=datetime(2018, 10, 31, 13, 0, 0), status="delivered") + save_notification(create_notification(sms_template, created_at=datetime(2018, 10, 31, 11, 0, 0), key_type="test")) + save_notification(create_notification(sms_template, created_at=datetime(2018, 10, 31, 12, 0, 0), status="delivered")) + save_notification(create_notification(email_template, created_at=datetime(2018, 10, 31, 13, 0, 0), status="delivered")) response = admin_request.get( "platform_stats.get_platform_stats", diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index bc7012de0f..4b37247bc8 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -65,6 +65,7 @@ create_template, create_template_folder, create_user, + save_notification, ) @@ -1534,7 +1535,7 @@ def test_get_all_notifications_for_service_in_order(notify_api, notify_db, notif def test_get_all_notifications_for_service_formatted_for_csv(client, sample_template): - notification = create_notification(template=sample_template) + notification = save_notification(create_notification(template=sample_template)) auth_header = create_authorization_header() response = client.get( @@ -1610,7 +1611,7 @@ def test_get_notification_for_service_includes_created_by(admin_request, sample_ def test_get_notification_for_service_returns_old_template_version(admin_request, sample_template): - sample_notification = create_notification(sample_template) + sample_notification = save_notification(create_notification(sample_template)) sample_notification.reference = "modified-inplace" sample_template.version = 2 sample_template.content = "New template content" @@ -1662,11 +1663,11 @@ def test_get_only_api_created_notifications_for_service( sample_user, ): # notification sent as a job - create_notification(sample_template, job=sample_job) + save_notification(create_notification(sample_template, job=sample_job)) # notification sent as a one-off - create_notification(sample_template, one_off=True, created_by_id=sample_user.id) + save_notification(create_notification(sample_template, one_off=True, created_by_id=sample_user.id)) # notification sent via API - without_job = create_notification(sample_template) + without_job = save_notification(create_notification(sample_template)) resp = admin_request.get( "service.get_all_notifications_for_service", @@ -1684,8 +1685,8 @@ def test_get_notifications_for_service_without_page_count( sample_template, sample_user, ): - create_notification(sample_template) - without_job = create_notification(sample_template) + save_notification(create_notification(sample_template)) + without_job = save_notification(create_notification(sample_template)) resp = admin_request.get( "service.get_all_notifications_for_service", @@ -1963,7 +1964,7 @@ def test_get_detailed_services_for_date_range(sample_template, start_date_delta, notification_type="sms", ) - create_notification(template=sample_template, created_at=datetime.utcnow(), status="delivered") + save_notification(create_notification(template=sample_template, created_at=datetime.utcnow(), status="delivered")) start_date = (datetime.utcnow() - timedelta(days=start_date_delta)).date() end_date = (datetime.utcnow() - timedelta(days=end_date_delta)).date() @@ -1995,11 +1996,15 @@ def test_get_detailed_services_for_date_range(sample_template, start_date_delta, def test_search_for_notification_by_to_field(client, sample_template, sample_email_template): - notification1 = create_notification(template=sample_template, to_field="+16502532222", normalised_to="+16502532222") - notification2 = create_notification( - template=sample_email_template, - to_field="jack@gmail.com", - normalised_to="jack@gmail.com", + notification1 = save_notification( + create_notification(template=sample_template, to_field="+16502532222", normalised_to="+16502532222") + ) + notification2 = save_notification( + create_notification( + template=sample_email_template, + to_field="jack@gmail.com", + normalised_to="jack@gmail.com", + ) ) response = client.get( @@ -2015,8 +2020,8 @@ def test_search_for_notification_by_to_field(client, sample_template, sample_ema def test_search_for_notification_by_to_field_return_empty_list_if_there_is_no_match(client, notify_db, notify_db_session): create_notification = partial(create_sample_notification, notify_db, notify_db_session) - notification1 = create_notification(to_field="+16502532222") - create_notification(to_field="jack@gmail.com") + notification1 = save_notification(create_notification(to_field="+16502532222")) + save_notification(create_notification(to_field="jack@gmail.com")) response = client.get( "/service/{}/notifications?to={}&template_type={}".format(notification1.service_id, "+447700900800", "sms"), @@ -2030,10 +2035,10 @@ def test_search_for_notification_by_to_field_return_empty_list_if_there_is_no_ma def test_search_for_notification_by_to_field_return_multiple_matches(client, notify_db, notify_db_session): create_notification = partial(create_sample_notification, notify_db, notify_db_session) - notification1 = create_notification(to_field="+16502532222", normalised_to="+16502532222") - notification2 = create_notification(to_field=" +165 0253 2222 ", normalised_to="+16502532222") - notification3 = create_notification(to_field="+1 650 253 2222", normalised_to="+16502532222") - notification4 = create_notification(to_field="jack@gmail.com", normalised_to="jack@gmail.com") + notification1 = save_notification(create_notification(to_field="+16502532222", normalised_to="+16502532222")) + notification2 = save_notification(create_notification(to_field=" +165 0253 2222 ", normalised_to="+16502532222")) + notification3 = save_notification(create_notification(to_field="+1 650 253 2222", normalised_to="+16502532222")) + notification4 = save_notification(create_notification(to_field="jack@gmail.com", normalised_to="jack@gmail.com")) response = client.get( "/service/{}/notifications?to={}&template_type={}".format(notification1.service_id, "+16502532222", "sms"), @@ -2240,8 +2245,8 @@ def test_search_for_notification_by_to_field_filters_by_status(client, notify_db to_field="+16502532222", normalised_to="+16502532222", ) - notification1 = create_notification(status="delivered") - create_notification(status="sending") + notification1 = save_notification(create_notification(status="delivered")) + save_notification(create_notification(status="sending")) response = client.get( "/service/{}/notifications?to={}&status={}&template_type={}".format( @@ -2265,8 +2270,8 @@ def test_search_for_notification_by_to_field_filters_by_statuses(client, notify_ to_field="+16502532222", normalised_to="+16502532222", ) - notification1 = create_notification(status="delivered") - notification2 = create_notification(status="sending") + notification1 = save_notification(create_notification(status="delivered")) + notification2 = save_notification(create_notification(status="sending")) response = client.get( "/service/{}/notifications?to={}&status={}&status={}&template_type={}".format( @@ -2377,9 +2382,9 @@ def test_get_all_notifications_for_service_includes_template_redacted(admin_requ dao_redact_template(redacted_template, sample_service.created_by_id) with freeze_time("2000-01-01"): - redacted_noti = create_notification(redacted_template) + redacted_noti = save_notification(create_notification(redacted_template)) with freeze_time("2000-01-02"): - normal_noti = create_notification(normal_template) + normal_noti = save_notification(create_notification(normal_template)) resp = admin_request.get("service.get_all_notifications_for_service", service_id=sample_service.id) @@ -2401,9 +2406,9 @@ def test_get_all_notifications_for_service_includes_template_hidden(admin_reques ) with freeze_time("2000-01-01"): - letter_noti = create_notification(letter_template) + letter_noti = save_notification(create_notification(letter_template)) with freeze_time("2000-01-02"): - precompiled_noti = create_notification(precompiled_template) + precompiled_noti = save_notification(create_notification(precompiled_template)) resp = admin_request.get("service.get_all_notifications_for_service", service_id=sample_service.id) diff --git a/tests/app/service/test_statistics_rest.py b/tests/app/service/test_statistics_rest.py index 0733fc3a75..907d2fb5d2 100644 --- a/tests/app/service/test_statistics_rest.py +++ b/tests/app/service/test_statistics_rest.py @@ -18,6 +18,7 @@ create_notification, create_service, create_template, + save_notification, ) @@ -25,7 +26,7 @@ # This test assumes the local timezone is EST def test_get_template_usage_by_month_returns_correct_data(admin_request, sample_template): create_ft_notification_status(utc_date=date(2017, 4, 2), template=sample_template, count=3) - create_notification(sample_template, created_at=datetime.utcnow()) + save_notification(create_notification(sample_template, created_at=datetime.utcnow())) resp_json = admin_request.get( "service.get_monthly_template_usage", @@ -62,7 +63,7 @@ def test_get_template_usage_by_month_returns_two_templates(admin_request, sample ) create_ft_notification_status(utc_date=datetime(2017, 4, 1), template=template_one, count=1) create_ft_notification_status(utc_date=datetime(2017, 4, 1), template=sample_template, count=3) - create_notification(sample_template, created_at=datetime.utcnow()) + save_notification(create_notification(sample_template, created_at=datetime.utcnow())) resp_json = admin_request.get( "service.get_monthly_template_usage", @@ -109,7 +110,7 @@ def test_get_template_usage_by_month_returns_two_templates(admin_request, sample def test_get_service_notification_statistics(admin_request, sample_service, sample_template, today_only, stats): create_ft_notification_status(date(2000, 1, 1), "sms", sample_service, count=1) with freeze_time("2000-01-02T12:00:00"): - create_notification(sample_template, status="created") + save_notification(create_notification(sample_template, status="created")) resp = admin_request.get( "service.get_service_notification_statistics", service_id=sample_template.service_id, @@ -230,11 +231,11 @@ def test_get_monthly_notification_stats_combines_todays_data_and_historic_stats( count=2, ) # noqa - create_notification(sample_template, created_at=datetime(2016, 6, 5), status="created") - create_notification(sample_template, created_at=datetime(2016, 6, 5), status="delivered") + save_notification(create_notification(sample_template, created_at=datetime(2016, 6, 5), status="created")) + save_notification(create_notification(sample_template, created_at=datetime(2016, 6, 5), status="delivered")) # this doesn't get returned in the stats because it is old - it should be in ft_notification_status by now - create_notification(sample_template, created_at=datetime(2016, 6, 4), status="sending") + save_notification(create_notification(sample_template, created_at=datetime(2016, 6, 4), status="sending")) response = admin_request.get( "service.get_monthly_notification_stats", diff --git a/tests/app/template/test_rest.py b/tests/app/template/test_rest.py index e651366ce6..58c0ef464c 100644 --- a/tests/app/template/test_rest.py +++ b/tests/app/template/test_rest.py @@ -28,6 +28,7 @@ create_service, create_template, create_template_folder, + save_notification, ) from tests.conftest import set_config_values @@ -1117,7 +1118,7 @@ def test_preview_letter_template_precompiled_pdf_file_type(notify_api, client, a hidden=True, ) - notification = create_notification(template) + notification = save_notification(create_notification(template)) with set_config_values( notify_api, @@ -1153,7 +1154,7 @@ def test_preview_letter_template_precompiled_s3_error(notify_api, client, admin_ hidden=True, ) - notification = create_notification(template) + notification = save_notification(create_notification(template)) with set_config_values( notify_api, @@ -1211,7 +1212,7 @@ def test_preview_letter_template_precompiled_png_file_type_or_pdf_with_overlay( hidden=True, ) - notification = create_notification(template) + notification = save_notification(create_notification(template)) with set_config_values( notify_api, @@ -1285,7 +1286,7 @@ def test_preview_letter_template_precompiled_png_file_type_hide_notify_tag_only_ hidden=True, ) - notification = create_notification(template) + notification = save_notification(create_notification(template)) with set_config_values( notify_api, @@ -1325,7 +1326,7 @@ def test_preview_letter_template_precompiled_png_template_preview_500_error( hidden=True, ) - notification = create_notification(template) + notification = save_notification(create_notification(template)) with set_config_values( notify_api, @@ -1374,7 +1375,7 @@ def test_preview_letter_template_precompiled_png_template_preview_400_error( hidden=True, ) - notification = create_notification(template) + notification = save_notification(create_notification(template)) with set_config_values( notify_api, @@ -1423,7 +1424,7 @@ def test_preview_letter_template_precompiled_png_template_preview_pdf_error( hidden=True, ) - notification = create_notification(template) + notification = save_notification(create_notification(template)) with set_config_values( notify_api, diff --git a/tests/app/template_statistics/test_rest.py b/tests/app/template_statistics/test_rest.py index a9e66ee8b0..13c902d067 100644 --- a/tests/app/template_statistics/test_rest.py +++ b/tests/app/template_statistics/test_rest.py @@ -4,7 +4,7 @@ import pytest from freezegun import freeze_time -from tests.app.db import create_notification +from tests.app.db import create_notification, save_notification def set_up_get_all_from_hash(mock_redis, side_effect): @@ -128,9 +128,9 @@ def test_get_template_statistics_for_service_by_day_returns_empty_list_if_no_tem def test_get_template_statistics_for_template_returns_last_notification(admin_request, sample_template): - create_notification(sample_template) - create_notification(sample_template) - notification_3 = create_notification(sample_template) + save_notification(create_notification(sample_template)) + save_notification(create_notification(sample_template)) + notification_3 = save_notification(create_notification(sample_template)) json_resp = admin_request.get( "template_statistics.get_template_statistics_for_template_id", diff --git a/tests/app/test_model.py b/tests/app/test_model.py index bd27448e7a..931a45c928 100644 --- a/tests/app/test_model.py +++ b/tests/app/test_model.py @@ -28,6 +28,7 @@ create_service, create_template, create_template_folder, + save_notification, ) @@ -110,7 +111,7 @@ def test_status_conversion(initial_statuses, expected_statuses): ) def test_notification_for_csv_returns_correct_type(sample_service, template_type, recipient): template = create_template(sample_service, template_type=template_type) - notification = create_notification(template, to_field=recipient) + notification = save_notification(create_notification(template, to_field=recipient)) serialized = notification.serialize_for_csv() assert serialized["template_type"] == template_type @@ -118,7 +119,7 @@ def test_notification_for_csv_returns_correct_type(sample_service, template_type @freeze_time("2016-01-01 11:09:00.000000") def test_notification_for_csv_returns_correct_job_row_number(sample_job): - notification = create_notification(sample_job.template, sample_job, job_row_number=0) + notification = save_notification(create_notification(sample_job.template, sample_job, job_row_number=0)) serialized = notification.serialize_for_csv() assert serialized["row_number"] == 1 @@ -143,7 +144,7 @@ def test_notification_for_csv_returns_correct_job_row_number(sample_job): ) def test_notification_for_csv_returns_formatted_status(sample_service, template_type, status, expected_status): template = create_template(sample_service, template_type=template_type) - notification = create_notification(template, status=status) + notification = save_notification(create_notification(template, status=status)) serialized = notification.serialize_for_csv() assert serialized["status"] == expected_status @@ -151,7 +152,7 @@ def test_notification_for_csv_returns_formatted_status(sample_service, template_ @freeze_time("2017-03-26 23:01:53.321312") def test_notification_for_csv_returns_est_correctly(sample_template): - notification = create_notification(sample_template) + notification = save_notification(create_notification(sample_template)) serialized = notification.serialize_for_csv() assert serialized["created_at"] == "2017-03-26 19:01:53" @@ -184,7 +185,7 @@ def test_notification_subject_is_none_for_sms(): @pytest.mark.parametrize("template_type", ["email", "letter"]) def test_notification_subject_fills_in_placeholders(sample_service, template_type): template = create_template(service=sample_service, template_type=template_type, subject="((name))") - notification = create_notification(template=template, personalisation={"name": "hello"}) + notification = save_notification(create_notification(template=template, personalisation={"name": "hello"})) assert notification.subject == "hello" @@ -232,7 +233,7 @@ def test_letter_notification_serializes_with_subject(client, sample_letter_templ def test_notification_references_template_history(client, sample_template): - noti = create_notification(sample_template) + noti = save_notification(create_notification(sample_template)) sample_template.version = 3 sample_template.content = "New template content" @@ -246,7 +247,7 @@ def test_notification_references_template_history(client, sample_template): def test_notification_requires_a_valid_template_version(client, sample_template): sample_template.version = 2 with pytest.raises(IntegrityError): - create_notification(sample_template) + save_notification(create_notification(sample_template)) def test_inbound_number_serializes_with_service(client, notify_db_session): diff --git a/tests/app/user/test_rest.py b/tests/app/user/test_rest.py index d49b1a105c..9befbb8b79 100644 --- a/tests/app/user/test_rest.py +++ b/tests/app/user/test_rest.py @@ -807,7 +807,8 @@ def test_send_contact_request_no_live_service(client, sample_user, mocker): } mocked_freshdesk = mocker.patch("app.user.rest.Freshdesk.send_ticket", return_value=201) - mocked_zendesk = mocker.patch("app.user.rest.ZenDeskSell.send_contact_request", return_value=200) + mocked_zendesk = mocker.patch("app.user.rest.Zendesk.send_ticket", return_value=201) + mocked_zendesk_sell = mocker.patch("app.user.rest.ZenDeskSell.send_contact_request", return_value=200) resp = client.post( url_for("user.send_contact_request", user_id=str(sample_user.id)), @@ -817,10 +818,11 @@ def test_send_contact_request_no_live_service(client, sample_user, mocker): assert resp.status_code == 204 mocked_freshdesk.assert_called_once_with() + mocked_zendesk.assert_called_once_with() contact = ContactRequest(**data) contact.tags = ["z_skip_opsgenie", "z_skip_urgent_escalation"] - mocked_zendesk.assert_called_once_with(contact) + mocked_zendesk_sell.assert_called_once_with(contact) def test_send_contact_request_with_live_service(client, sample_service, mocker): @@ -831,7 +833,8 @@ def test_send_contact_request_with_live_service(client, sample_service, mocker): "support_type": "ask_question", } mocked_freshdesk = mocker.patch("app.user.rest.Freshdesk.send_ticket", return_value=201) - mocked_zendesk = mocker.patch("app.user.rest.ZenDeskSell.send_contact_request", return_value=200) + mocked_zendesk = mocker.patch("app.user.rest.Zendesk.send_ticket", return_value=201) + mocked_zendesk_sell = mocker.patch("app.user.rest.ZenDeskSell.send_contact_request", return_value=200) resp = client.post( url_for("user.send_contact_request", user_id=str(sample_user.id)), @@ -840,7 +843,8 @@ def test_send_contact_request_with_live_service(client, sample_service, mocker): ) assert resp.status_code == 204 mocked_freshdesk.assert_called_once_with() - mocked_zendesk.assert_called_once_with(ContactRequest(**data)) + mocked_zendesk.assert_called_once_with() + mocked_zendesk_sell.assert_called_once_with(ContactRequest(**data)) def test_send_contact_request_demo(client, sample_user, mocker): @@ -875,7 +879,8 @@ def test_send_contact_request_go_live(client, sample_service, mocker): "service_id": str(sample_service.id), } mocked_freshdesk = mocker.patch("app.user.rest.Freshdesk.send_ticket", return_value=201) - mocked_zendesk = mocker.patch("app.user.rest.ZenDeskSell.send_go_live_request", return_value="1") + mocked_zendesk = mocker.patch("app.user.rest.Zendesk.send_ticket", return_value=201) + mocked_zendesk_sell = mocker.patch("app.user.rest.ZenDeskSell.send_go_live_request", return_value="1") resp = client.post( url_for("user.send_contact_request", user_id=str(sample_user.id)), @@ -884,7 +889,8 @@ def test_send_contact_request_go_live(client, sample_service, mocker): ) assert resp.status_code == 204 mocked_freshdesk.assert_called_once_with() - mocked_zendesk.assert_called_once_with(sample_service, sample_user, ContactRequest(**data)) + mocked_zendesk.assert_called_once_with() + mocked_zendesk_sell.assert_called_once_with(sample_service, sample_user, ContactRequest(**data)) def test_send_branding_request(client, sample_service, mocker): @@ -896,6 +902,7 @@ def test_send_branding_request(client, sample_service, mocker): "filename": "branding_url", } mocked_freshdesk = mocker.patch("app.user.rest.Freshdesk.send_ticket", return_value=201) + mocked_zendesk = mocker.patch("app.user.rest.Zendesk.send_ticket", return_value=201) resp = client.post( url_for("user.send_branding_request", user_id=str(sample_user.id)), @@ -904,6 +911,7 @@ def test_send_branding_request(client, sample_service, mocker): ) assert resp.status_code == 204 mocked_freshdesk.assert_called_once_with() + mocked_zendesk.assert_called_once_with() def test_send_user_confirm_new_email_returns_204(client, sample_user, change_email_confirmation_template, mocker): diff --git a/tests/app/v2/notifications/test_get_notifications.py b/tests/app/v2/notifications/test_get_notifications.py index 1c4b77ee40..2f9023bc98 100644 --- a/tests/app/v2/notifications/test_get_notifications.py +++ b/tests/app/v2/notifications/test_get_notifications.py @@ -5,24 +5,29 @@ from app import DATETIME_FORMAT from tests import create_authorization_header -from tests.app.db import create_notification, create_template +from tests.app.db import ( + create_notification, + create_template, + save_notification, + save_scheduled_notification, +) @pytest.mark.parametrize("billable_units, provider", [(1, "mmg"), (0, "mmg"), (1, None)]) # This test assumes the local timezone is EST def test_get_notification_by_id_returns_200(client, billable_units, provider, sample_template): - sample_notification = create_notification( - template=sample_template, - billable_units=billable_units, - sent_by=provider, + sample_notification = save_scheduled_notification( + create_notification(template=sample_template, billable_units=billable_units, sent_by=provider), scheduled_for="2017-05-12 15:15", ) # another - create_notification( - template=sample_template, - billable_units=billable_units, - sent_by=provider, + save_scheduled_notification( + create_notification( + template=sample_template, + billable_units=billable_units, + sent_by=provider, + ), scheduled_for="2017-06-12 15:15", ) @@ -73,9 +78,11 @@ def test_get_notification_by_id_returns_200(client, billable_units, provider, sa def test_get_notification_by_id_with_placeholders_returns_200(client, sample_email_template_with_placeholders): - sample_notification = create_notification( - template=sample_email_template_with_placeholders, - personalisation={"name": "Bob"}, + sample_notification = save_notification( + create_notification( + template=sample_email_template_with_placeholders, + personalisation={"name": "Bob"}, + ) ) auth_header = create_authorization_header(service_id=sample_notification.service_id) @@ -125,7 +132,9 @@ def test_get_notification_by_id_with_placeholders_returns_200(client, sample_ema def test_get_notification_by_reference_returns_200(client, sample_template): - sample_notification_with_reference = create_notification(template=sample_template, client_reference="some-client-reference") + sample_notification_with_reference = save_notification( + create_notification(template=sample_template, client_reference="some-client-reference") + ) auth_header = create_authorization_header(service_id=sample_notification_with_reference.service_id) response = client.get( @@ -148,7 +157,7 @@ def test_get_notification_by_id_returns_created_by_name_if_notification_created_ sample_user, sample_template, ): - sms_notification = create_notification(template=sample_template) + sms_notification = save_notification(create_notification(template=sample_template)) sms_notification.created_by_id = sample_user.id auth_header = create_authorization_header(service_id=sms_notification.service_id) @@ -166,9 +175,8 @@ def test_get_notification_by_id_returns_created_by_name_if_notification_created_ # This test assumes the local timezone is EST def test_get_notifications_returns_scheduled_for(client, sample_template): - sample_notification_with_reference = create_notification( - template=sample_template, - client_reference="some-client-reference", + sample_notification_with_reference = save_scheduled_notification( + create_notification(template=sample_template, client_reference="some-client-reference"), scheduled_for="2017-05-23 17:15", ) @@ -281,7 +289,7 @@ def test_get_notification_adds_delivery_estimate_for_letters( @pytest.mark.parametrize("template_type", ["sms", "email"]) def test_get_notification_doesnt_have_delivery_estimate_for_non_letters(client, sample_service, template_type): template = create_template(service=sample_service, template_type=template_type) - mocked_notification = create_notification(template=template) + mocked_notification = save_notification(create_notification(template=template)) auth_header = create_authorization_header(service_id=mocked_notification.service_id) response = client.get( @@ -293,8 +301,8 @@ def test_get_notification_doesnt_have_delivery_estimate_for_non_letters(client, def test_get_all_notifications_except_job_notifications_returns_200(client, sample_template, sample_job): - create_notification(template=sample_template, job=sample_job) # should not return this job notification - notifications = [create_notification(template=sample_template) for _ in range(2)] + save_notification(create_notification(template=sample_template, job=sample_job)) # should not return this job notification + notifications = [save_notification(create_notification(template=sample_template)) for _ in range(2)] notification = notifications[-1] auth_header = create_authorization_header(service_id=notification.service_id) @@ -325,8 +333,8 @@ def test_get_all_notifications_except_job_notifications_returns_200(client, samp def test_get_all_notifications_with_include_jobs_arg_returns_200(client, sample_template, sample_job): notifications = [ - create_notification(template=sample_template, job=sample_job), - create_notification(template=sample_template), + save_notification(create_notification(template=sample_template, job=sample_job)), + save_notification(create_notification(template=sample_template)), ] notification = notifications[-1] @@ -370,8 +378,8 @@ def test_get_all_notifications_filter_by_template_type(client, sample_service): email_template = create_template(service=sample_service, template_type="email") sms_template = create_template(service=sample_service, template_type="sms") - notification = create_notification(template=email_template, to_field="don.draper@scdp.biz") - create_notification(template=sms_template) + notification = save_notification(create_notification(template=email_template, to_field="don.draper@scdp.biz")) + save_notification(create_notification(template=sms_template)) auth_header = create_authorization_header(service_id=notification.service_id) response = client.get( @@ -416,8 +424,8 @@ def test_get_all_notifications_filter_by_template_type_invalid_template_type(cli def test_get_all_notifications_filter_by_single_status(client, sample_template): - notification = create_notification(template=sample_template, status="pending") - create_notification(template=sample_template) + notification = save_notification(create_notification(template=sample_template, status="pending")) + save_notification(create_notification(template=sample_template)) auth_header = create_authorization_header(service_id=notification.service_id) response = client.get( @@ -461,9 +469,10 @@ def test_get_all_notifications_filter_by_status_invalid_status(client, sample_no def test_get_all_notifications_filter_by_multiple_statuses(client, sample_template): notifications = [ - create_notification(template=sample_template, status=_status) for _status in ["created", "pending", "sending"] + save_notification(create_notification(template=sample_template, status=_status)) + for _status in ["created", "pending", "sending"] ] - failed_notification = create_notification(template=sample_template, status="permanent-failure") + failed_notification = save_notification(create_notification(template=sample_template, status="permanent-failure")) auth_header = create_authorization_header(service_id=notifications[0].service_id) response = client.get( @@ -487,9 +496,9 @@ def test_get_all_notifications_filter_by_multiple_statuses(client, sample_templa def test_get_all_notifications_filter_by_failed_status(client, sample_template): - created_notification = create_notification(template=sample_template, status="created") + created_notification = save_notification(create_notification(template=sample_template, status="created")) failed_notifications = [ - create_notification(template=sample_template, status=_status) + save_notification(create_notification(template=sample_template, status=_status)) for _status in ["technical-failure", "temporary-failure", "permanent-failure"] ] @@ -515,8 +524,8 @@ def test_get_all_notifications_filter_by_failed_status(client, sample_template): def test_get_all_notifications_filter_by_id(client, sample_template): - older_notification = create_notification(template=sample_template) - newer_notification = create_notification(template=sample_template) + older_notification = save_notification(create_notification(template=sample_template)) + newer_notification = save_notification(create_notification(template=sample_template)) auth_header = create_authorization_header(service_id=newer_notification.service_id) response = client.get( @@ -550,7 +559,7 @@ def test_get_all_notifications_filter_by_id_invalid_id(client, sample_notificati def test_get_all_notifications_filter_by_id_no_notifications_if_nonexistent_id(client, sample_template): - notification = create_notification(template=sample_template) + notification = save_notification(create_notification(template=sample_template)) auth_header = create_authorization_header(service_id=notification.service_id) response = client.get( @@ -568,7 +577,7 @@ def test_get_all_notifications_filter_by_id_no_notifications_if_nonexistent_id(c def test_get_all_notifications_filter_by_id_no_notifications_if_last_notification(client, sample_template): - notification = create_notification(template=sample_template) + notification = save_notification(create_notification(template=sample_template)) auth_header = create_authorization_header(service_id=notification.service_id) response = client.get( @@ -587,19 +596,19 @@ def test_get_all_notifications_filter_by_id_no_notifications_if_last_notificatio def test_get_all_notifications_filter_multiple_query_parameters(client, sample_email_template): # this is the notification we are looking for - older_notification = create_notification(template=sample_email_template, status="pending") + older_notification = save_notification(create_notification(template=sample_email_template, status="pending")) # wrong status - create_notification(template=sample_email_template) + save_notification(create_notification(template=sample_email_template)) wrong_template = create_template(sample_email_template.service, template_type="sms") # wrong template - create_notification(template=wrong_template, status="pending") + save_notification(create_notification(template=wrong_template, status="pending")) # we only want notifications created before this one - newer_notification = create_notification(template=sample_email_template) + newer_notification = save_notification(create_notification(template=sample_email_template)) # this notification was created too recently - create_notification(template=sample_email_template, status="pending") + save_notification(create_notification(template=sample_email_template, status="pending")) auth_header = create_authorization_header(service_id=newer_notification.service_id) response = client.get( @@ -661,14 +670,16 @@ def test_get_all_notifications_renames_letter_statuses( ], ) def test_get_notifications_renames_letter_statuses(client, sample_letter_template, db_status, expected_status): - letter_noti = create_notification( - sample_letter_template, - status=db_status, - personalisation={ - "address_line_1": "Mr Foo", - "address_line_2": "1 Bar Street", - "postcode": "N1", - }, + letter_noti = save_notification( + create_notification( + sample_letter_template, + status=db_status, + personalisation={ + "address_line_1": "Mr Foo", + "address_line_2": "1 Bar Street", + "postcode": "N1", + }, + ) ) auth_header = create_authorization_header(service_id=letter_noti.service_id) response = client.get( diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 3fee294175..d325365333 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -1504,7 +1504,7 @@ def test_post_bulk_with_archived_template(client, fake_uuid, notify_db, notify_d assert response.status_code == 400 error_json = json.loads(response.get_data(as_text=True)) - assert error_json["errors"] == [{"error": "BadRequestError", "message": "Template has been deleted"}] + assert error_json["errors"] == [{"error": "BadRequestError", "message": f"Template {template.id} has been deleted"}] @pytest.mark.parametrize( diff --git a/tests_smoke/smoke_test.py b/tests_smoke/smoke_test.py index f14e3aa294..2b9fc2399b 100644 --- a/tests_smoke/smoke_test.py +++ b/tests_smoke/smoke_test.py @@ -1,8 +1,8 @@ -from smoke.common import Attachment_type, Config, Notification_type -from smoke.test_admin_csv import test_admin_csv -from smoke.test_admin_one_off import test_admin_one_off -from smoke.test_api_bulk import test_api_bulk -from smoke.test_api_one_off import test_api_one_off +from smoke.common import Attachment_type, Config, Notification_type # type: ignore +from smoke.test_admin_csv import test_admin_csv # type: ignore +from smoke.test_admin_one_off import test_admin_one_off # type: ignore +from smoke.test_api_bulk import test_api_bulk # type: ignore +from smoke.test_api_one_off import test_api_one_off # type: ignore if __name__ == "__main__": print("API Smoke test\n")