diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 42adf5e2f..fb0116e22 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -56,8 +56,8 @@ jobs: run: make bootstrap env: SQLALCHEMY_DATABASE_TEST_URI: postgresql://user:password@localhost:5432/test_notification_api - # - name: Run style checks - # run: flake8 . + - name: Run style checks + run: flake8 . - name: Check imports alphabetized run: isort --check-only ./app ./tests - name: Run tests diff --git a/Makefile b/Makefile index 701ae3380..97ef2b5d6 100644 --- a/Makefile +++ b/Makefile @@ -63,7 +63,7 @@ generate-version-file: ## Generates the app version file .PHONY: test test: ## Run tests - # flake8 . + flake8 . isort --check-only ./app ./tests pytest -n4 --maxfail=10 diff --git a/app/__init__.py b/app/__init__.py index eb8985e00..fe4ee6fc3 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -192,7 +192,7 @@ def register_blueprint(application): status_blueprint.before_request(requires_no_auth) application.register_blueprint(status_blueprint) - + # delivery receipts ses_callback_blueprint.before_request(requires_no_auth) application.register_blueprint(ses_callback_blueprint) diff --git a/app/authentication/auth.py b/app/authentication/auth.py index 9849bda3e..7201ad56f 100644 --- a/app/authentication/auth.py +++ b/app/authentication/auth.py @@ -18,7 +18,11 @@ from sqlalchemy.orm.exc import NoResultFound from app.serialised_models import SerialisedService -GENERAL_TOKEN_ERROR_MESSAGE = 'Invalid token: make sure your API token matches the example at https://docs.notifications.service.gov.uk/rest-api.html#authorisation-header' # nosec B105 +# stvnrlly - this is silly, but bandit has a multiline string bug (https://github.com/PyCQA/bandit/issues/658) +# and flake8 wants a multiline quote here. TODO: check on bug status and restore sanity once possible +TOKEN_MESSAGE_ONE = "Invalid token: make sure your API token matches the example " # nosec B105 +TOKEN_MESSAGE_TWO = "at https://docs.notifications.service.gov.uk/rest-api.html#authorisation-header" # nosec B105 +GENERAL_TOKEN_ERROR_MESSAGE = TOKEN_MESSAGE_ONE + TOKEN_MESSAGE_TWO AUTH_DB_CONNECTION_DURATION_SECONDS = Histogram( 'auth_db_connection_duration_seconds', diff --git a/app/aws/s3.py b/app/aws/s3.py index 39189e0ae..7312f4cbe 100644 --- a/app/aws/s3.py +++ b/app/aws/s3.py @@ -10,18 +10,25 @@ default_access_key = os.environ.get('AWS_ACCESS_KEY_ID') default_secret_key = os.environ.get('AWS_SECRET_ACCESS_KEY') default_region = os.environ.get('AWS_REGION') -def get_s3_file(bucket_name, file_location, access_key=default_access_key, secret_key=default_secret_key, region=default_region): + +def get_s3_file( + bucket_name, file_location, access_key=default_access_key, secret_key=default_secret_key, region=default_region +): s3_file = get_s3_object(bucket_name, file_location, access_key, secret_key, region) return s3_file.get()['Body'].read().decode('utf-8') -def get_s3_object(bucket_name, file_location, access_key=default_access_key, secret_key=default_secret_key, region=default_region): +def get_s3_object( + bucket_name, file_location, access_key=default_access_key, secret_key=default_secret_key, region=default_region +): session = Session(aws_access_key_id=access_key, aws_secret_access_key=secret_key, region_name=region) s3 = session.resource('s3') return s3.Object(bucket_name, file_location) -def file_exists(bucket_name, file_location, access_key=default_access_key, secret_key=default_secret_key, region=default_region): +def file_exists( + bucket_name, file_location, access_key=default_access_key, secret_key=default_secret_key, region=default_region +): try: # try and access metadata of object get_s3_object(bucket_name, file_location, access_key, secret_key, region).metadata diff --git a/app/celery/process_ses_receipts_tasks.py b/app/celery/process_ses_receipts_tasks.py index 959f1ab36..8a0b3417f 100644 --- a/app/celery/process_ses_receipts_tasks.py +++ b/app/celery/process_ses_receipts_tasks.py @@ -29,7 +29,10 @@ def process_ses_results(self, response): ses_message = json.loads(response["Message"]) notification_type = ses_message["notificationType"] # TODO remove after smoke testing on prod is implemented - current_app.logger.info(f"Attempting to process SES delivery status message from SNS with type: {notification_type} and body: {ses_message}") + current_app.logger.info( + f"Attempting to process SES delivery status message " + f"from SNS with type: {notification_type} and body: {ses_message}" + ) bounce_message = None if notification_type == 'Bounce': @@ -49,13 +52,16 @@ def process_ses_results(self, response): message_time = iso8601.parse_date(ses_message["mail"]["timestamp"]).replace(tzinfo=None) if datetime.utcnow() - message_time < timedelta(minutes=5): current_app.logger.info( - f"notification not found for reference: {reference} (while attempting update to {notification_status}). " - f"Callback may have arrived before notification was persisted to the DB. Adding task to retry queue" + f"Notification not found for reference: {reference}" + f"(while attempting update to {notification_status}). " + f"Callback may have arrived before notification was" + f"persisted to the DB. Adding task to retry queue" ) self.retry(queue=QueueNames.RETRY) else: current_app.logger.warning( - "notification not found for reference: {} (while attempting update to {})".format(reference, notification_status) + f"Notification not found for reference: {reference} " + f"(while attempting update to {notification_status})" ) return @@ -64,7 +70,7 @@ def process_ses_results(self, response): if notification.status not in {NOTIFICATION_SENDING, NOTIFICATION_PENDING}: notifications_dao._duplicate_update_warning( - notification, + notification, notification_status ) return @@ -102,6 +108,7 @@ def process_ses_results(self, response): current_app.logger.exception("Error processing SES results: {}".format(type(e))) self.retry(queue=QueueNames.RETRY) + def determine_notification_bounce_type(ses_message): notification_type = ses_message["notificationType"] if notification_type in ["Delivery", "Complaint"]: @@ -116,14 +123,16 @@ def determine_notification_bounce_type(ses_message): return "Permanent" return "Temporary" + def determine_notification_type(ses_message): notification_type = ses_message["notificationType"] - if notification_type not in ["Bounce","Complaint","Delivery"]: + if notification_type not in ["Bounce", "Complaint", "Delivery"]: raise KeyError(f"Unhandled sns notification type {notification_type}") if notification_type == 'Bounce': return determine_notification_bounce_type(ses_message) return notification_type + def _determine_provider_response(ses_message): if ses_message["notificationType"] != "Bounce": return None @@ -175,7 +184,9 @@ def get_aws_responses(ses_message): def handle_complaint(ses_message): recipient_email = remove_emails_from_complaint(ses_message)[0] - current_app.logger.info("Complaint from SES: \n{}".format(json.dumps(ses_message).replace("{", "(").replace("}", ")"))) + current_app.logger.info( + "Complaint from SES: \n{}".format(json.dumps(ses_message).replace("{", "(").replace("}", ")")) + ) try: reference = ses_message["mail"]["messageId"] except KeyError as e: @@ -219,7 +230,9 @@ def check_and_queue_callback_task(notification): service_callback_api = get_service_delivery_status_callback_api_for_service(service_id=notification.service_id) if service_callback_api: notification_data = create_delivery_status_callback_data(notification, service_callback_api) - send_delivery_status_to_service.apply_async([str(notification.id), notification_data], queue=QueueNames.CALLBACKS) + send_delivery_status_to_service.apply_async( + [str(notification.id), notification_data], queue=QueueNames.CALLBACKS + ) def _check_and_queue_complaint_callback_task(complaint, notification, recipient): @@ -228,4 +241,3 @@ def _check_and_queue_complaint_callback_task(complaint, notification, recipient) if service_callback_api: complaint_data = create_complaint_callback_data(complaint, notification, service_callback_api, recipient) send_complaint_to_service.apply_async([complaint_data], queue=QueueNames.CALLBACKS) - \ No newline at end of file diff --git a/app/celery/process_sms_client_response_tasks.py b/app/celery/process_sms_client_response_tasks.py index 4da60d477..8bdf84301 100644 --- a/app/celery/process_sms_client_response_tasks.py +++ b/app/celery/process_sms_client_response_tasks.py @@ -1,7 +1,6 @@ import uuid from datetime import datetime -import pytest from flask import current_app from notifications_utils.template import SMSMessageTemplate diff --git a/app/celery/research_mode_tasks.py b/app/celery/research_mode_tasks.py index ac99bc842..4477b3994 100644 --- a/app/celery/research_mode_tasks.py +++ b/app/celery/research_mode_tasks.py @@ -124,7 +124,7 @@ def create_fake_letter_response_file(self, reference): dvla_response_data = '{}|Sent|0|Sorted'.format(reference) # try and find a filename that hasn't been taken yet - from a random time within the last 30 seconds - for i in sorted(range(30), key=lambda _: random.random()): # nosec B311 - not security related + for i in sorted(range(30), key=lambda _: random.random()): # nosec B311 - not security related upload_file_name = 'NOTIFY-{}-RSP.TXT'.format((now - timedelta(seconds=i)).strftime('%Y%m%d%H%M%S')) if not file_exists(current_app.config['DVLA_RESPONSE_BUCKET_NAME'], upload_file_name): break diff --git a/app/celery/service_callback_tasks.py b/app/celery/service_callback_tasks.py index 8867fca6e..0e81d38d0 100644 --- a/app/celery/service_callback_tasks.py +++ b/app/celery/service_callback_tasks.py @@ -114,7 +114,7 @@ def create_delivery_status_callback_data(notification, service_callback_api): "notification_client_reference": notification.client_reference, "notification_to": notification.to, "notification_status": notification.status, - "notification_provider_response": notification.provider_response, # TODO do we have a test for provider_response + "notification_provider_response": notification.provider_response, # TODO do we test for provider_response? "notification_created_at": notification.created_at.strftime(DATETIME_FORMAT), "notification_updated_at": notification.updated_at.strftime(DATETIME_FORMAT) if notification.updated_at else None, diff --git a/app/clients/__init__.py b/app/clients/__init__.py index 71c66bed3..4553dfc48 100644 --- a/app/clients/__init__.py +++ b/app/clients/__init__.py @@ -1,6 +1,3 @@ -from celery import current_app - - class ClientException(Exception): ''' Base Exceptions for sending notifications that fail @@ -38,7 +35,7 @@ class NotificationProviderClients(object): return self.email_clients.get(name) def get_client_by_name_and_type(self, name, notification_type): - assert notification_type in ['email', 'sms'] # nosec B101 + assert notification_type in ['email', 'sms'] # nosec B101 if notification_type == 'email': return self.get_email_client(name) diff --git a/app/clients/sms/__init__.py b/app/clients/sms/__init__.py index c1d38616e..2e7f27cdc 100644 --- a/app/clients/sms/__init__.py +++ b/app/clients/sms/__init__.py @@ -25,4 +25,4 @@ class SmsClient(Client): raise NotImplementedError("TODO Need to implement.") def get_name(self): - raise NotImplementedError("TODO Need to implement.") \ No newline at end of file + raise NotImplementedError("TODO Need to implement.") diff --git a/app/clients/sms/aws_sns.py b/app/clients/sms/aws_sns.py index 96d593ec0..4637bdfe4 100644 --- a/app/clients/sms/aws_sns.py +++ b/app/clients/sms/aws_sns.py @@ -20,7 +20,7 @@ class AwsSnsClient(SmsClient): self.current_app = current_app self.statsd_client = statsd_client self.long_code_regex = re.compile(r"^\+1\d{10}$") - + @property def name(self): return 'sns' @@ -86,4 +86,4 @@ class AwsSnsClient(SmsClient): raise ValueError("No valid numbers found for SMS delivery") def _send_with_dedicated_phone_number(self, sender): - return sender and re.match(self.long_code_regex, sender) \ No newline at end of file + return sender and re.match(self.long_code_regex, sender) diff --git a/app/cloudfoundry_config.py b/app/cloudfoundry_config.py index 0e7082558..533f3d6af 100644 --- a/app/cloudfoundry_config.py +++ b/app/cloudfoundry_config.py @@ -13,9 +13,11 @@ def extract_cloudfoundry_config(): vcap_services = json.loads(os.environ['VCAP_SERVICES']) # Postgres config - os.environ['SQLALCHEMY_DATABASE_URI'] = vcap_services['aws-rds'][0]['credentials']['uri'].replace('postgres','postgresql') + os.environ['SQLALCHEMY_DATABASE_URI'] = \ + vcap_services['aws-rds'][0]['credentials']['uri'].replace('postgres', 'postgresql') # Redis config - os.environ['REDIS_URL'] = vcap_services['aws-elasticache-redis'][0]['credentials']['uri'].replace('redis://','rediss://') + os.environ['REDIS_URL'] = \ + vcap_services['aws-elasticache-redis'][0]['credentials']['uri'].replace('redis://', 'rediss://') # CSV Upload Bucket Name bucket_service = find_by_service_name( diff --git a/app/commands.py b/app/commands.py index d0175b8a6..070842eb8 100644 --- a/app/commands.py +++ b/app/commands.py @@ -140,86 +140,6 @@ def purge_functional_test_data(user_email_prefix): delete_model_user(usr) -@notify_command() -def backfill_notification_statuses(): - """ - DEPRECATED. Populates notification_status. - - This will be used to populate the new `Notification._status_fkey` with the old - `Notification._status_enum` - """ - LIMIT = 250000 - subq = "SELECT id FROM notification_history WHERE notification_status is NULL LIMIT {}".format(LIMIT) # nosec B608 no user-controlled input - update = "UPDATE notification_history SET notification_status = status WHERE id in ({})".format(subq) # nosec B608 no user-controlled input - result = db.session.execute(subq).fetchall() - - while len(result) > 0: - db.session.execute(update) - print('commit {} updates at {}'.format(LIMIT, datetime.utcnow())) - db.session.commit() - result = db.session.execute(subq).fetchall() - - -@notify_command() -def update_notification_international_flag(): - """ - DEPRECATED. Set notifications.international=false. - """ - # 250,000 rows takes 30 seconds to update. - subq = "select id from notifications where international is null limit 250000" - update = "update notifications set international = False where id in ({})".format(subq) # nosec B608 no user-controlled input - result = db.session.execute(subq).fetchall() - - while len(result) > 0: - db.session.execute(update) - print('commit 250000 updates at {}'.format(datetime.utcnow())) - db.session.commit() - result = db.session.execute(subq).fetchall() - - # Now update notification_history - subq_history = "select id from notification_history where international is null limit 250000" - update_history = "update notification_history set international = False where id in ({})".format(subq_history) # nosec B608 no user-controlled input - result_history = db.session.execute(subq_history).fetchall() - while len(result_history) > 0: - db.session.execute(update_history) - print('commit 250000 updates at {}'.format(datetime.utcnow())) - db.session.commit() - result_history = db.session.execute(subq_history).fetchall() - - -@notify_command() -def fix_notification_statuses_not_in_sync(): - """ - DEPRECATED. - This will be used to correct an issue where Notification._status_enum and NotificationHistory._status_fkey - became out of sync. See 979e90a. - - Notification._status_enum is the source of truth so NotificationHistory._status_fkey will be updated with - these values. - """ - MAX = 10000 - - subq = "SELECT id FROM notifications WHERE cast (status as text) != notification_status LIMIT {}".format(MAX) # nosec B608 no user-controlled input - update = "UPDATE notifications SET notification_status = status WHERE id in ({})".format(subq) # nosec B608 no user-controlled input - result = db.session.execute(subq).fetchall() - - while len(result) > 0: - db.session.execute(update) - print('Committed {} updates at {}'.format(len(result), datetime.utcnow())) - db.session.commit() - result = db.session.execute(subq).fetchall() - - subq_hist = "SELECT id FROM notification_history WHERE cast (status as text) != notification_status LIMIT {}".format(MAX) # nosec B608 - update = "UPDATE notification_history SET notification_status = status WHERE id in ({})".format(subq_hist) # nosec B608 no user-controlled input - result = db.session.execute(subq_hist).fetchall() - - while len(result) > 0: - db.session.execute(update) - print('Committed {} updates at {}'.format(len(result), datetime.utcnow())) - db.session.commit() - result = db.session.execute(subq_hist).fetchall() - - @notify_command(name='insert-inbound-numbers') @click.option('-f', '--file_name', required=True, help="""Full path of the file to upload, file is a contains inbound numbers, diff --git a/app/config.py b/app/config.py index 4f0761234..d70ec913c 100644 --- a/app/config.py +++ b/app/config.py @@ -79,7 +79,7 @@ class Config(object): INTERNAL_CLIENT_API_KEYS = json.loads( os.environ.get('INTERNAL_CLIENT_API_KEYS', '{"notify-admin":["dev-notify-secret-key"]}') - ) # TODO: handled by varsfile? + ) # TODO: handled by varsfile? # encyption secret/salt ADMIN_CLIENT_SECRET = os.environ.get('ADMIN_CLIENT_SECRET') @@ -99,13 +99,13 @@ class Config(object): # Firetext API Key FIRETEXT_API_KEY = os.environ.get("FIRETEXT_API_KEY", "placeholder") FIRETEXT_INTERNATIONAL_API_KEY = os.environ.get("FIRETEXT_INTERNATIONAL_API_KEY", "placeholder") - + # Whether to ignore POSTs from SNS for replies to SMS we sent RECEIVE_INBOUND_SMS = False # Use notify.sandbox.10x sending domain unless overwritten by environment NOTIFY_EMAIL_DOMAIN = 'notify.sandbox.10x.gsa.gov' - + # AWS SNS topics for delivery receipts VALIDATE_SNS_TOPICS = True VALID_SNS_TOPICS = ['notify_test_bounce', 'notify_test_success', 'notify_test_complaint', 'notify_test_sms_inbound'] @@ -402,7 +402,7 @@ class Development(Config): # Config.ADMIN_CLIENT_ID: ['dev-notify-secret-key'], # } - SECRET_KEY = 'dev-notify-secret-key' # nosec B105 - this is only used in development + SECRET_KEY = 'dev-notify-secret-key' # nosec B105 - this is only used in development DANGEROUS_SALT = 'dev-notify-salt' MMG_INBOUND_SMS_AUTH = ['testkey'] @@ -413,7 +413,10 @@ class Development(Config): NOTIFY_EMAIL_DOMAIN = os.getenv('NOTIFY_EMAIL_DOMAIN', 'notify.sandbox.10x.gsa.gov') - SQLALCHEMY_DATABASE_URI = os.environ.get('SQLALCHEMY_DATABASE_URI', 'postgresql://postgres:chummy@db:5432/notification_api') + SQLALCHEMY_DATABASE_URI = os.environ.get( + 'SQLALCHEMY_DATABASE_URI', + 'postgresql://postgres:chummy@db:5432/notification_api' + ) ANTIVIRUS_ENABLED = os.environ.get('ANTIVIRUS_ENABLED') == '1' @@ -449,7 +452,10 @@ class Test(Development): # LETTER_SANITISE_BUCKET_NAME = 'test-letters-sanitise' # this is overriden in CI - SQLALCHEMY_DATABASE_URI = os.getenv('SQLALCHEMY_DATABASE_TEST_URI', 'postgresql://postgres:chummy@db:5432/test_notification_api') + SQLALCHEMY_DATABASE_URI = os.getenv( + 'SQLALCHEMY_DATABASE_TEST_URI', + 'postgresql://postgres:chummy@db:5432/test_notification_api' + ) CELERY = { **Config.CELERY, @@ -508,11 +514,17 @@ class Staging(Config): class Live(Config): NOTIFY_ENVIRONMENT = 'live' # buckets - CSV_UPLOAD_BUCKET_NAME = os.environ.get('CSV_UPLOAD_BUCKET_NAME', 'notifications-prototype-csv-upload') # created in gsa sandbox + CSV_UPLOAD_BUCKET_NAME = os.environ.get( + 'CSV_UPLOAD_BUCKET_NAME', + 'notifications-prototype-csv-upload' + ) # created in gsa sandbox CSV_UPLOAD_ACCESS_KEY = os.environ.get('CSV_UPLOAD_ACCESS_KEY') CSV_UPLOAD_SECRET_KEY = os.environ.get('CSV_UPLOAD_SECRET_KEY') CSV_UPLOAD_REGION = os.environ.get('CSV_UPLOAD_REGION') - CONTACT_LIST_BUCKET_NAME = os.environ.get('CONTACT_LIST_BUCKET_NAME', 'notifications-prototype-contact-list-upload') # created in gsa sandbox + CONTACT_LIST_BUCKET_NAME = os.environ.get( + 'CONTACT_LIST_BUCKET_NAME', + 'notifications-prototype-contact-list-upload' + ) # created in gsa sandbox CONTACT_LIST_ACCESS_KEY = os.environ.get('CONTACT_LIST_ACCESS_KEY') CONTACT_LIST_SECRET_KEY = os.environ.get('CONTACT_LIST_SECRET_KEY') CONTACT_LIST_REGION = os.environ.get('CONTACT_LIST_REGION') diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index efdbfaefb..c62b2908f 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -87,6 +87,7 @@ def country_records_delivery(phone_prefix): dlr = INTERNATIONAL_BILLING_RATES[phone_prefix]['attributes']['dlr'] return dlr and dlr.lower() == 'yes' + def _decide_permanent_temporary_failure(current_status, status): # If we go from pending to delivered we need to set failure type as temporary-failure if current_status == NOTIFICATION_PENDING and status == NOTIFICATION_PERMANENT_FAILURE: @@ -102,6 +103,7 @@ def _update_notification_status(notification, status, provider_response=None): dao_update_notification(notification) return notification + @autocommit def update_notification_status_by_id(notification_id, status, sent_by=None, detailed_status_code=None): notification = Notification.query.with_for_update().filter(Notification.id == notification_id).first() diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index f8c60272f..ae5fb2a88 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -9,8 +9,6 @@ from sqlalchemy.sql.expression import and_, asc, case, func from app import db from app.dao.dao_utils import VersionOptions, autocommit, version_class from app.dao.date_util import get_current_financial_year -from app.dao.email_branding_dao import dao_get_email_branding_by_name -from app.dao.letter_branding_dao import dao_get_letter_branding_by_name from app.dao.organisation_dao import dao_get_organisation_by_email_address from app.dao.service_sms_sender_dao import insert_service_sms_sender from app.dao.service_user_dao import dao_get_service_user @@ -47,7 +45,6 @@ from app.models import ( VerifyCode, ) from app.utils import ( - email_address_is_nhs, escape_special_characters, get_archived_db_column_value, get_london_midnight_in_utc, diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index e3e8aa6b4..c1532e83e 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -166,7 +166,7 @@ def update_notification_to_sending(notification, provider): # We currently have no callback method for SMS deliveries # TODO create celery task to request SMS delivery receipts from cloudwatch api notification.status = NOTIFICATION_SENT if notification.notification_type == "sms" else NOTIFICATION_SENDING - + dao_update_notification(notification) @@ -175,10 +175,12 @@ provider_cache = TTLCache(maxsize=8, ttl=10) @cached(cache=provider_cache) def provider_to_use(notification_type, international=True): - international = False # TODO: remove or resolve the functionality of this flag + international = False # TODO: remove or resolve the functionality of this flag # TODO rip firetext and mmg out of early migrations and clean up the expression below active_providers = [ - p for p in get_provider_details_by_notification_type(notification_type, international) if p.active and p.identifier not in ['firetext','mmg'] + p for p in get_provider_details_by_notification_type( + notification_type, international + ) if p.active and p.identifier not in ['firetext', 'mmg'] ] if not active_providers: @@ -191,7 +193,7 @@ def provider_to_use(notification_type, international=True): chosen_provider = active_providers[0] else: weights = [p.priority for p in active_providers] - chosen_provider = random.choices(active_providers, weights=weights)[0] # nosec B311 - this is not security/cryptography related + chosen_provider = random.choices(active_providers, weights=weights)[0] # nosec B311 - not sec/crypto related return notification_provider_clients.get_client_by_name_and_type(chosen_provider.identifier, notification_type) diff --git a/app/inbound_sms/rest.py b/app/inbound_sms/rest.py index 9afc90425..7a82b8920 100644 --- a/app/inbound_sms/rest.py +++ b/app/inbound_sms/rest.py @@ -1,5 +1,4 @@ from flask import Blueprint, jsonify, request -from notifications_utils.recipients import try_validate_and_format_phone_number from app.dao.inbound_sms_dao import ( dao_count_inbound_sms_for_service, diff --git a/app/models.py b/app/models.py index b9a358132..d5e0d9e14 100644 --- a/app/models.py +++ b/app/models.py @@ -120,7 +120,9 @@ class User(db.Model): state = db.Column(db.String, nullable=False, default='pending') platform_admin = db.Column(db.Boolean, nullable=False, default=False) current_session_id = db.Column(UUID(as_uuid=True), nullable=True) - auth_type = db.Column(db.String, db.ForeignKey('auth_type.name'), index=True, nullable=False, default=EMAIL_AUTH_TYPE) + auth_type = db.Column( + db.String, db.ForeignKey('auth_type.name'), index=True, nullable=False, default=EMAIL_AUTH_TYPE + ) email_access_validated_at = db.Column( db.DateTime, index=False, unique=False, nullable=False, default=datetime.datetime.utcnow ) @@ -594,7 +596,7 @@ class AnnualBilling(db.Model): "name": self.service.name } - return{ + return { "id": str(self.id), 'free_sms_fragment_limit': self.free_sms_fragment_limit, 'service_id': self.service_id, @@ -1628,7 +1630,7 @@ class Notification(db.Model): """ # this should only ever be called for letter notifications - it makes no sense otherwise and I'd rather not # get the two code flows mixed up at all - assert self.notification_type == LETTER_TYPE # nosec B101 - current calling code already validates the correct type + assert self.notification_type == LETTER_TYPE # nosec B101 - current calling code validates correct type if self.status in [NOTIFICATION_CREATED, NOTIFICATION_SENDING]: return NOTIFICATION_STATUS_LETTER_ACCEPTED diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py index 93f14b37a..bee2c9561 100644 --- a/app/notifications/notifications_ses_callback.py +++ b/app/notifications/notifications_ses_callback.py @@ -10,6 +10,7 @@ from app.notifications.sns_handlers import sns_notification_handler ses_callback_blueprint = Blueprint('notifications_ses_callback', __name__) DEFAULT_MAX_AGE = timedelta(days=10000) + # 400 counts as a permanent failure so SNS will not retry. # 500 counts as a failed delivery attempt so SNS will retry. # See https://docs.aws.amazon.com/sns/latest/dg/DeliveryPolicies.html#DeliveryPolicies @@ -21,7 +22,7 @@ def email_ses_callback_handler(): return jsonify( result="error", message=str(e.message) ), e.status_code - + message = data.get("Message") if "mail" in message: process_ses_results.apply_async([{"Message": message}], queue=QueueNames.NOTIFY) diff --git a/app/notifications/notifications_sms_callback.py b/app/notifications/notifications_sms_callback.py index 50c345f49..4448191d2 100644 --- a/app/notifications/notifications_sms_callback.py +++ b/app/notifications/notifications_sms_callback.py @@ -1,10 +1,6 @@ -from flask import Blueprint, json, jsonify, request +from flask import Blueprint -# from app.celery.process_sms_client_response_tasks import ( -# process_sms_client_response, -# ) -from app.config import QueueNames -from app.errors import InvalidRequest, register_errors +from app.errors import register_errors sms_callback_blueprint = Blueprint("sms_callback", __name__, url_prefix="/notifications/sms") register_errors(sms_callback_blueprint) diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index 673e7ff49..1c8d090e0 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -106,13 +106,13 @@ def persist_notification( updated_at=None ): current_app.logger.info('Presisting notification') - + notification_created_at = created_at or datetime.utcnow() if not notification_id: notification_id = uuid.uuid4() - + current_app.logger.info('Presisting notification with id {}'.format(notification_id)) - + notification = Notification( id=notification_id, template_id=template_id, @@ -135,7 +135,7 @@ def persist_notification( document_download_count=document_download_count, updated_at=updated_at ) - + current_app.logger.info('Presisting notification with to address: {}'.format(notification.to)) if notification_type == SMS_TYPE: diff --git a/app/notifications/receive_notifications.py b/app/notifications/receive_notifications.py index b1099d00b..03009a791 100644 --- a/app/notifications/receive_notifications.py +++ b/app/notifications/receive_notifications.py @@ -24,6 +24,7 @@ INBOUND_SMS_COUNTER = Counter( ['provider'] ) + @receive_notifications_blueprint.route('/notifications/sms/receive/sns', methods=['POST']) def receive_sns_sms(): """ @@ -37,13 +38,13 @@ def receive_sns_sms(): "previousPublishedMessageId":"wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY" } """ - + # Whether or not to ignore inbound SMS replies if not current_app.config['RECEIVE_INBOUND_SMS']: return jsonify( result="success", message="SMS-SNS callback succeeded" ), 200 - + try: post_data = sns_notification_handler(request.data, request.headers) except Exception as e: @@ -53,13 +54,16 @@ def receive_sns_sms(): # TODO wrap this up if "inboundMessageId" in message: # TODO use standard formatting we use for all US numbers - inbound_number = message['destinationNumber'].replace('+','') + inbound_number = message['destinationNumber'].replace('+', '') service = fetch_potential_service(inbound_number, 'sns') if not service: # since this is an issue with our service <-> number mapping, or no inbound_sms service permission # we should still tell SNS that we received it successfully - current_app.logger.warning(f"Mapping between service and inbound number: {inbound_number} is broken, or service does not have permission to receive inbound sms") + current_app.logger.warning( + f"Mapping between service and inbound number: {inbound_number} is broken, " + f"or service does not have permission to receive inbound sms" + ) return jsonify( result="success", message="SMS-SNS callback succeeded" ), 200 @@ -79,7 +83,6 @@ def receive_sns_sms(): date_received=date_received, provider_name=provider_name) - # TODO ensure inbound sms callback endpoints are accessible and functioning for notify api users, then uncomment the task below tasks.send_inbound_sms_to_service.apply_async([str(inbound.id), str(service.id)], queue=QueueNames.NOTIFY) current_app.logger.debug( diff --git a/app/notifications/sns_cert_validator.py b/app/notifications/sns_cert_validator.py index 57c0396ca..1fbddd811 100644 --- a/app/notifications/sns_cert_validator.py +++ b/app/notifications/sns_cert_validator.py @@ -19,6 +19,7 @@ _cert_url_re = re.compile( r'sns\.([a-z]{1,3}-[a-z]+-[0-9]{1,2})\.amazonaws\.com', ) + class ValidationError(Exception): """ ValidationError. Raised when a message fails integrity checks. @@ -56,7 +57,7 @@ def get_string_to_sign(sns_payload): for field in fields: field_value = sns_payload.get(field) if not isinstance(field_value, str): - if field == 'Subject' and field_value == None: + if field == 'Subject' and field_value is None: continue raise ValidationError(f"In {field}, found non-string value: {field_value}") string_to_sign += field + '\n' + field_value + '\n' @@ -77,25 +78,26 @@ def validate_sns_cert(sns_payload): # Amazon SNS currently supports signature version 1. if sns_payload.get('SignatureVersion') != '1': raise ValidationError("Wrong Signature Version (expected 1)") - + validate_arn(sns_payload) - + string_to_sign = get_string_to_sign(sns_payload) # Key signing cert url via Lambda and via webhook are slightly different - signing_cert_url = sns_payload.get('SigningCertUrl') if 'SigningCertUrl' in sns_payload else sns_payload.get('SigningCertURL') + signing_cert_url = sns_payload.get('SigningCertUrl') if 'SigningCertUrl' in \ + sns_payload else sns_payload.get('SigningCertURL') if not isinstance(signing_cert_url, str): raise ValidationError("Signing cert url must be a string") cert_scheme, cert_netloc, *_ = urlparse(signing_cert_url) if cert_scheme != 'https' or not re.match(_cert_url_re, cert_netloc): raise ValidationError("Cert does not appear to be from AWS") - + certificate = _signing_cert_cache.get(signing_cert_url) if certificate is None: certificate = get_certificate(signing_cert_url) if isinstance(certificate, six.text_type): certificate = certificate.encode() - + signature = base64.b64decode(sns_payload["Signature"]) try: @@ -107,4 +109,4 @@ def validate_sns_cert(sns_payload): ) return True except oscrypto.errors.SignatureError: - raise ValidationError("Invalid signature") \ No newline at end of file + raise ValidationError("Invalid signature") diff --git a/app/notifications/sns_handlers.py b/app/notifications/sns_handlers.py index a0a6d3b98..535ec12db 100644 --- a/app/notifications/sns_handlers.py +++ b/app/notifications/sns_handlers.py @@ -45,7 +45,9 @@ def sns_notification_handler(data, headers): try: validate_sns_cert(message) except Exception as e: - current_app.logger.error(f"SES-SNS callback failed: validation failed with error: Signature validation failed with error {e}") + current_app.logger.error( + f"SES-SNS callback failed: validation failed with error: Signature validation failed with error {e}" + ) raise InvalidRequest("SES-SNS callback failed: validation failed", 400) if message.get('Type') == 'SubscriptionConfirmation': @@ -55,12 +57,18 @@ def sns_notification_handler(data, headers): try: response.raise_for_status() except Exception as e: - current_app.logger.warning(f"Attempt to raise_for_status()SubscriptionConfirmation Type message files for response: {response.text} with error {e}") - raise InvalidRequest("SES-SNS callback failed: attempt to raise_for_status()SubscriptionConfirmation Type message failed", 400) + current_app.logger.warning( + f"Attempt to raise_for_status()SubscriptionConfirmation Type " + f"message files for response: {response.text} with error {e}" + ) + raise InvalidRequest( + "SES-SNS callback failed: attempt to raise_for_status()SubscriptionConfirmation " + "Type message failed", 400 + ) current_app.logger.info("SES-SNS auto-confirm subscription callback succeeded") return message # TODO remove after smoke testing on prod is implemented current_app.logger.info(f"SNS message: {message} is a valid message. Attempting to process it now.") - + return message diff --git a/app/organisation/rest.py b/app/organisation/rest.py index a44cb8b9c..30d819131 100644 --- a/app/organisation/rest.py +++ b/app/organisation/rest.py @@ -103,8 +103,6 @@ def update_organisation(organisation_id): data = request.get_json() validate(data, post_update_organisation_schema) - organisation = dao_get_organisation_by_id(organisation_id) - result = dao_update_organisation(organisation_id, **data) if data.get('agreement_signed') is True: diff --git a/app/user/rest.py b/app/user/rest.py index 9014d8dad..3ec2adb9b 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -404,7 +404,7 @@ def send_new_user_email_verification(user_id): current_app.logger.info('Sending notification to queue') send_notification_to_queue(saved_notification, False, queue=QueueNames.NOTIFY) - + current_app.logger.info('Sent notification to queue') return jsonify({}), 204 @@ -414,12 +414,12 @@ def send_new_user_email_verification(user_id): def send_already_registered_email(user_id): current_app.logger.info('Email already registered for user {}'.format(user_id)) to = email_data_request_schema.load(request.get_json()) - + current_app.logger.info('To email is {}'.format(to['email'])) template = dao_get_template_by_id(current_app.config['ALREADY_REGISTERED_EMAIL_TEMPLATE_ID']) service = Service.query.get(current_app.config['NOTIFY_SERVICE_ID']) - + current_app.logger.info('template.id is {}'.format(template.id)) current_app.logger.info('service.id is {}'.format(service.id)) diff --git a/requirements_for_test.txt b/requirements_for_test.txt index 3c6756bc1..28106fd6c 100644 --- a/requirements_for_test.txt +++ b/requirements_for_test.txt @@ -1,6 +1,6 @@ --requirement requirements.txt -flake8==4.0.1 -flake8-bugbear==22.4.25 +flake8==5.0.4 +flake8-bugbear==22.9.23 isort==5.10.1 moto==3.1.9 pytest==7.1.2 diff --git a/tests/app/celery/test_process_ses_receipts_tasks.py b/tests/app/celery/test_process_ses_receipts_tasks.py index 0cc82cddd..a90f9f6f8 100644 --- a/tests/app/celery/test_process_ses_receipts_tasks.py +++ b/tests/app/celery/test_process_ses_receipts_tasks.py @@ -87,7 +87,7 @@ def test_notifications_ses_200_autoconfirms_subscription(client, mocker): def test_notifications_ses_200_call_process_task(client, mocker): process_mock = mocker.patch("app.notifications.notifications_ses_callback.process_ses_results.apply_async") mocker.patch("app.notifications.sns_handlers.validate_sns_cert", return_value=True) - data = {"Type": "Notification", "foo": "bar", "Message": {"mail": "baz"} } + data = {"Type": "Notification", "foo": "bar", "Message": {"mail": "baz"}} mocker.patch("app.notifications.sns_handlers.sns_notification_handler", return_value=data) json_data = json.dumps(data) response = client.post( @@ -156,7 +156,10 @@ def test_ses_callback_should_update_notification_status( status='sending', sent_at=datetime.utcnow() ) - callback_api = create_service_callback_api(service=sample_email_template.service, url="https://original_url.com") + callback_api = create_service_callback_api( + service=sample_email_template.service, + url="https://original_url.com" + ) assert get_notification_by_id(notification.id).status == 'sending' assert process_ses_results(ses_notification_callback(reference='ref')) assert get_notification_by_id(notification.id).status == 'delivered' @@ -186,13 +189,19 @@ def test_ses_callback_should_retry_if_notification_is_new(mocker): assert process_ses_results(ses_notification_callback(reference='ref')) is None assert mock_logger.call_count == 0 assert mock_retry.call_count == 1 + + def test_ses_callback_should_log_if_notification_is_missing(client, _notify_db, mocker): mock_retry = mocker.patch('app.celery.process_ses_receipts_tasks.process_ses_results.retry') mock_logger = mocker.patch('app.celery.process_ses_receipts_tasks.current_app.logger.warning') with freeze_time('2017-11-17T12:34:03.646Z'): assert process_ses_results(ses_notification_callback(reference='ref')) is None assert mock_retry.call_count == 0 - mock_logger.assert_called_once_with('notification not found for reference: ref (while attempting update to delivered)') + mock_logger.assert_called_once_with( + 'Notification not found for reference: ref (while attempting update to delivered)' + ) + + def test_ses_callback_should_not_retry_if_notification_is_old(mocker): mock_retry = mocker.patch('app.celery.process_ses_receipts_tasks.process_ses_results.retry') mock_logger = mocker.patch('app.celery.process_ses_receipts_tasks.current_app.logger.error') @@ -200,6 +209,8 @@ def test_ses_callback_should_not_retry_if_notification_is_old(mocker): assert process_ses_results(ses_notification_callback(reference='ref')) is None assert mock_logger.call_count == 0 assert mock_retry.call_count == 0 + + def test_ses_callback_does_not_call_send_delivery_status_if_no_db_entry( client, _notify_db, @@ -222,6 +233,8 @@ def test_ses_callback_does_not_call_send_delivery_status_if_no_db_entry( assert process_ses_results(ses_notification_callback(reference='ref')) assert get_notification_by_id(notification.id).status == 'delivered' send_mock.assert_not_called() + + def test_ses_callback_should_update_multiple_notification_status_sent( client, _notify_db, @@ -257,6 +270,8 @@ def test_ses_callback_should_update_multiple_notification_status_sent( assert process_ses_results(ses_notification_callback(reference='ref2')) assert process_ses_results(ses_notification_callback(reference='ref3')) assert send_mock.called + + def test_ses_callback_should_set_status_to_temporary_failure(client, _notify_db, notify_db_session, @@ -278,6 +293,8 @@ def test_ses_callback_should_set_status_to_temporary_failure(client, assert process_ses_results(ses_soft_bounce_callback(reference='ref')) assert get_notification_by_id(notification.id).status == 'temporary-failure' assert send_mock.called + + def test_ses_callback_should_set_status_to_permanent_failure(client, _notify_db, notify_db_session, @@ -299,6 +316,8 @@ def test_ses_callback_should_set_status_to_permanent_failure(client, assert process_ses_results(ses_hard_bounce_callback(reference='ref')) assert get_notification_by_id(notification.id).status == 'permanent-failure' assert send_mock.called + + def test_ses_callback_should_send_on_complaint_to_user_callback_api(sample_email_template, mocker): send_mock = mocker.patch( 'app.celery.service_callback_tasks.send_complaint_to_service.apply_async' @@ -321,4 +340,3 @@ def test_ses_callback_should_send_on_complaint_to_user_callback_api(sample_email 'service_callback_api_url': 'https://original_url.com', 'to': 'recipient1@example.com' } - \ No newline at end of file diff --git a/tests/app/celery/test_process_sms_client_response_tasks.py b/tests/app/celery/test_process_sms_client_response_tasks.py index 3d08811b9..34bd6807c 100644 --- a/tests/app/celery/test_process_sms_client_response_tasks.py +++ b/tests/app/celery/test_process_sms_client_response_tasks.py @@ -5,9 +5,9 @@ import pytest from freezegun import freeze_time from app import statsd_client -# from app.celery.process_sms_client_response_tasks import ( -# process_sms_client_response, -# ) +from app.celery.process_sms_client_response_tasks import ( + process_sms_client_response, +) from app.clients import ClientException from app.models import NOTIFICATION_TECHNICAL_FAILURE diff --git a/tests/app/celery/test_provider_tasks.py b/tests/app/celery/test_provider_tasks.py index 9cecfbb29..2f241bc24 100644 --- a/tests/app/celery/test_provider_tasks.py +++ b/tests/app/celery/test_provider_tasks.py @@ -126,6 +126,7 @@ def test_should_add_to_retry_queue_if_notification_not_found_in_deliver_email_ta app.delivery.send_to_providers.send_email_to_provider.assert_not_called() app.celery.provider_tasks.deliver_email.retry.assert_called_with(queue="retry-tasks") + @pytest.mark.skip(reason="Needs updating for TTS: Failing for unknown reason") @pytest.mark.parametrize( 'exception_class', [ diff --git a/tests/app/celery/test_reporting_tasks.py b/tests/app/celery/test_reporting_tasks.py index e4f0db279..056621820 100644 --- a/tests/app/celery/test_reporting_tasks.py +++ b/tests/app/celery/test_reporting_tasks.py @@ -290,6 +290,7 @@ def test_create_nightly_billing_for_day_different_sent_by( assert record.billable_units == 1 assert record.rate_multiplier == 1.0 + @pytest.mark.skip(reason="Needs updating for TTS: Remove mail") def test_create_nightly_billing_for_day_different_letter_postage( notify_db_session, diff --git a/tests/app/clients/test_aws_sns.py b/tests/app/clients/test_aws_sns.py index 0a61ffca0..7215c64b8 100644 --- a/tests/app/clients/test_aws_sns.py +++ b/tests/app/clients/test_aws_sns.py @@ -24,4 +24,4 @@ def test_send_sms_returns_raises_error_if_there_is_no_valid_number_is_found(noti content = reference = 'foo' with pytest.raises(ValueError) as excinfo: aws_sns_client.send_sms(to, content, reference) - assert 'No valid numbers found for SMS delivery' in str(excinfo.value) \ No newline at end of file + assert 'No valid numbers found for SMS delivery' in str(excinfo.value) diff --git a/tests/app/clients/test_sms.py b/tests/app/clients/test_sms.py index c457e365b..59d053845 100644 --- a/tests/app/clients/test_sms.py +++ b/tests/app/clients/test_sms.py @@ -15,6 +15,7 @@ def fake_client(notify_api): fake_client.init_app(notify_api, statsd_client) return fake_client + @pytest.mark.skip(reason="Needs updating for TTS: New SMS client") def test_send_sms(fake_client, mocker): mock_send = mocker.patch.object(fake_client, 'try_send_sms') @@ -31,6 +32,7 @@ def test_send_sms(fake_client, mocker): 'to', 'content', 'reference', False, 'testing' ) + @pytest.mark.skip(reason="Needs updating for TTS: New SMS client") def test_send_sms_error(fake_client, mocker): mocker.patch.object( diff --git a/tests/app/dao/test_provider_details_dao.py b/tests/app/dao/test_provider_details_dao.py index 727b520ec..939f8b9d8 100644 --- a/tests/app/dao/test_provider_details_dao.py +++ b/tests/app/dao/test_provider_details_dao.py @@ -144,6 +144,7 @@ def test_adjust_provider_priority_sets_priority( assert mmg_provider.created_by.id == notify_user.id assert mmg_provider.priority == 50 + @pytest.mark.skip(reason="Needs updating for TTS: MMG removal") @freeze_time('2016-01-01 00:30') def test_adjust_provider_priority_adds_history( @@ -172,6 +173,7 @@ def test_adjust_provider_priority_adds_history( assert updated_provider_history_rows[0].version - old_provider_history_rows[0].version == 1 assert updated_provider_history_rows[0].priority == 50 + @pytest.mark.skip(reason="Needs updating for TTS: MMG removal") @freeze_time('2016-01-01 01:00') def test_get_sms_providers_for_update_returns_providers(restore_provider_details): diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index a9c2a3568..eda4edae3 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -6,8 +6,6 @@ from unittest.mock import ANY import pytest from flask import current_app -from notifications_utils.recipients import validate_and_format_phone_number -from requests import HTTPError import app # from app import firetext_client, mmg_client, notification_provider_clients @@ -27,22 +25,23 @@ from app.models import ( Notification, ) from app.serialised_models import SerialisedService -from tests.app.db import ( +from tests.app.db import ( # create_service,; create_service_with_defined_sms_sender,; create_template, create_email_branding, create_notification, create_reply_to_email, - create_service, create_service_sms_sender, - create_service_with_defined_sms_sender, - create_template, ) +# from notifications_utils.recipients import validate_and_format_phone_number +# from requests import HTTPError + def setup_function(_function): # pytest will run this function before each test. It makes sure the # state of the cache is not shared between tests. send_to_providers.provider_cache.clear() + @pytest.mark.skip(reason="Needs updating for TTS: Update with new providers") def test_provider_to_use_should_return_random_provider(mocker, notify_db_session): mmg = get_provider_details_by_identifier('mmg') @@ -72,6 +71,7 @@ def test_provider_to_use_should_cache_repeated_calls(mocker, notify_db_session): assert all(result == results[0] for result in results) assert len(mock_choices.call_args_list) == 1 + @pytest.mark.skip(reason="Needs updating for TTS: Update with new providers") @pytest.mark.parametrize('international_provider_priority', ( # Since there’s only one international provider it should always @@ -115,39 +115,39 @@ def test_provider_to_use_raises_if_no_active_providers(mocker, restore_provider_ send_to_providers.provider_to_use('sms', international=True) -@pytest.mark.skip(reason="Needs updating for TTS: Update with new providers") -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="+447234123123", personalisation={"name": "Jo"}, - status='created', - reply_to_text=sample_sms_template_with_html.service.get_default_sms_sender(), - normalised_to="447234123123" - ) +# @pytest.mark.skip(reason="Needs updating for TTS: Update with new providers") +# 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="+447234123123", personalisation={"name": "Jo"}, +# status='created', +# reply_to_text=sample_sms_template_with_html.service.get_default_sms_sender(), +# normalised_to="447234123123" +# ) - mocker.patch('app.mmg_client.send_sms') +# mocker.patch('app.mmg_client.send_sms') - send_to_providers.send_sms_to_provider( - db_notification - ) +# send_to_providers.send_sms_to_provider( +# db_notification +# ) - mmg_client.send_sms.assert_called_once_with( - to="447234123123", - content="Sample service: Hello Jo\nHere is some HTML & entities", - reference=str(db_notification.id), - sender=current_app.config['FROM_NUMBER'], - international=False - ) +# mmg_client.send_sms.assert_called_once_with( +# to="447234123123", +# content="Sample service: Hello Jo\nHere is some HTML & entities", +# reference=str(db_notification.id), +# sender=current_app.config['FROM_NUMBER'], +# international=False +# ) - notification = Notification.query.filter_by(id=db_notification.id).one() +# notification = Notification.query.filter_by(id=db_notification.id).one() - assert notification.status == 'sending' - assert notification.sent_at <= datetime.utcnow() - assert notification.sent_by == 'mmg' - assert notification.billable_units == 1 - assert notification.personalisation == {"name": "Jo"} +# assert notification.status == 'sending' +# assert notification.sent_at <= datetime.utcnow() +# assert notification.sent_by == 'mmg' +# assert notification.billable_units == 1 +# assert notification.personalisation == {"name": "Jo"} @pytest.mark.skip(reason="Needs updating for TTS: Update with new providers") @@ -215,145 +215,145 @@ def test_should_not_send_sms_message_when_service_is_inactive_notification_is_in assert Notification.query.get(sample_notification.id).status == 'technical-failure' -@pytest.mark.skip(reason="Needs updating for TTS: Update with new providers") -def test_send_sms_should_use_template_version_from_notification_not_latest( - sample_template, - mocker): - db_notification = create_notification(template=sample_template, to_field='+447234123123', status='created', - reply_to_text=sample_template.service.get_default_sms_sender(), - normalised_to='447234123123') +# @pytest.mark.skip(reason="Needs updating for TTS: Update with new providers") +# def test_send_sms_should_use_template_version_from_notification_not_latest( +# sample_template, +# mocker): +# db_notification = create_notification(template=sample_template, to_field='+447234123123', status='created', +# reply_to_text=sample_template.service.get_default_sms_sender(), +# normalised_to='447234123123') - mocker.patch('app.mmg_client.send_sms') +# mocker.patch('app.mmg_client.send_sms') - version_on_notification = sample_template.version - expected_template_id = sample_template.id +# version_on_notification = sample_template.version +# expected_template_id = sample_template.id - # Change the template - from app.dao.templates_dao import ( - dao_get_template_by_id, - dao_update_template, - ) - sample_template.content = sample_template.content + " another version of the template" - dao_update_template(sample_template) - t = dao_get_template_by_id(sample_template.id) - assert t.version > version_on_notification +# # Change the template +# from app.dao.templates_dao import ( +# dao_get_template_by_id, +# dao_update_template, +# ) +# sample_template.content = sample_template.content + " another version of the template" +# dao_update_template(sample_template) +# t = dao_get_template_by_id(sample_template.id) +# assert t.version > version_on_notification - send_to_providers.send_sms_to_provider( - db_notification - ) +# send_to_providers.send_sms_to_provider( +# db_notification +# ) - mmg_client.send_sms.assert_called_once_with( - to=validate_and_format_phone_number("+447234123123"), - content="Sample service: This is a template:\nwith a newline", - reference=str(db_notification.id), - sender=current_app.config['FROM_NUMBER'], - international=False - ) +# mmg_client.send_sms.assert_called_once_with( +# to=validate_and_format_phone_number("+447234123123"), +# content="Sample service: This is a template:\nwith a newline", +# reference=str(db_notification.id), +# sender=current_app.config['FROM_NUMBER'], +# international=False +# ) - t = dao_get_template_by_id(expected_template_id) +# t = dao_get_template_by_id(expected_template_id) - persisted_notification = notifications_dao.get_notification_by_id(db_notification.id) - assert persisted_notification.to == db_notification.to - assert persisted_notification.template_id == expected_template_id - assert persisted_notification.template_version == version_on_notification - assert persisted_notification.template_version != t.version - assert persisted_notification.status == 'sending' - assert not persisted_notification.personalisation +# persisted_notification = notifications_dao.get_notification_by_id(db_notification.id) +# assert persisted_notification.to == db_notification.to +# assert persisted_notification.template_id == expected_template_id +# assert persisted_notification.template_version == version_on_notification +# assert persisted_notification.template_version != t.version +# assert persisted_notification.status == 'sending' +# assert not persisted_notification.personalisation -@pytest.mark.skip(reason="Needs updating for TTS: Update with new providers") -@pytest.mark.parametrize('research_mode,key_type', [ - (True, KEY_TYPE_NORMAL), - (False, KEY_TYPE_TEST) -]) -def test_should_call_send_sms_response_task_if_research_mode( - notify_db_session, sample_service, sample_notification, mocker, research_mode, key_type -): - mocker.patch('app.mmg_client.send_sms') - mocker.patch('app.delivery.send_to_providers.send_sms_response') +# @pytest.mark.skip(reason="Needs updating for TTS: Update with new providers") +# @pytest.mark.parametrize('research_mode,key_type', [ +# (True, KEY_TYPE_NORMAL), +# (False, KEY_TYPE_TEST) +# ]) +# def test_should_call_send_sms_response_task_if_research_mode( +# notify_db_session, sample_service, sample_notification, mocker, research_mode, key_type +# ): +# mocker.patch('app.mmg_client.send_sms') +# mocker.patch('app.delivery.send_to_providers.send_sms_response') - if research_mode: - sample_service.research_mode = True - notify_db_session.add(sample_service) - notify_db_session.commit() +# if research_mode: +# sample_service.research_mode = True +# notify_db_session.add(sample_service) +# notify_db_session.commit() - sample_notification.key_type = key_type +# sample_notification.key_type = key_type - send_to_providers.send_sms_to_provider( - sample_notification - ) - assert not mmg_client.send_sms.called +# send_to_providers.send_sms_to_provider( +# sample_notification +# ) +# assert not mmg_client.send_sms.called - app.delivery.send_to_providers.send_sms_response.assert_called_once_with( - 'mmg', str(sample_notification.id), sample_notification.to - ) +# app.delivery.send_to_providers.send_sms_response.assert_called_once_with( +# 'mmg', str(sample_notification.id), sample_notification.to +# ) - persisted_notification = notifications_dao.get_notification_by_id(sample_notification.id) - assert persisted_notification.to == sample_notification.to - assert persisted_notification.template_id == sample_notification.template_id - assert persisted_notification.status == 'sending' - assert persisted_notification.sent_at <= datetime.utcnow() - assert persisted_notification.sent_by == 'mmg' - assert not persisted_notification.personalisation +# persisted_notification = notifications_dao.get_notification_by_id(sample_notification.id) +# assert persisted_notification.to == sample_notification.to +# assert persisted_notification.template_id == sample_notification.template_id +# assert persisted_notification.status == 'sending' +# assert persisted_notification.sent_at <= datetime.utcnow() +# assert persisted_notification.sent_by == 'mmg' +# assert not persisted_notification.personalisation -@pytest.mark.skip(reason="Needs updating for TTS: Update with new providers") -def test_should_have_sending_status_if_fake_callback_function_fails(sample_notification, mocker): - mocker.patch('app.delivery.send_to_providers.send_sms_response', side_effect=HTTPError) +# @pytest.mark.skip(reason="Needs updating for TTS: Update with new providers") +# def test_should_have_sending_status_if_fake_callback_function_fails(sample_notification, mocker): +# mocker.patch('app.delivery.send_to_providers.send_sms_response', side_effect=HTTPError) - sample_notification.key_type = KEY_TYPE_TEST +# sample_notification.key_type = KEY_TYPE_TEST - with pytest.raises(HTTPError): - send_to_providers.send_sms_to_provider( - sample_notification - ) - assert sample_notification.status == 'sending' - assert sample_notification.sent_by == 'mmg' +# with pytest.raises(HTTPError): +# send_to_providers.send_sms_to_provider( +# sample_notification +# ) +# assert sample_notification.status == 'sending' +# assert sample_notification.sent_by == 'mmg' -@pytest.mark.skip(reason="Needs updating for TTS: Update with new providers") -def test_should_not_send_to_provider_when_status_is_not_created( - sample_template, - mocker -): - notification = create_notification(template=sample_template, status='sending') - mocker.patch('app.mmg_client.send_sms') - response_mock = mocker.patch('app.delivery.send_to_providers.send_sms_response') +# @pytest.mark.skip(reason="Needs updating for TTS: Update with new providers") +# def test_should_not_send_to_provider_when_status_is_not_created( +# sample_template, +# mocker +# ): +# notification = create_notification(template=sample_template, status='sending') +# mocker.patch('app.mmg_client.send_sms') +# response_mock = mocker.patch('app.delivery.send_to_providers.send_sms_response') - send_to_providers.send_sms_to_provider( - notification - ) +# send_to_providers.send_sms_to_provider( +# notification +# ) - app.mmg_client.send_sms.assert_not_called() - response_mock.assert_not_called() +# app.mmg_client.send_sms.assert_not_called() +# response_mock.assert_not_called() -@pytest.mark.skip(reason="Needs updating for TTS: Update with new providers") -def test_should_send_sms_with_downgraded_content(notify_db_session, mocker): - # é, o, and u are in GSM. - # ī, grapes, tabs, zero width space and ellipsis are not - # ó isn't in GSM, but it is in the welsh alphabet so will still be sent - msg = "a é ī o u 🍇 foo\tbar\u200bbaz((misc))…" - placeholder = '∆∆∆abc' - gsm_message = "?ódz 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} - ) +# @pytest.mark.skip(reason="Needs updating for TTS: Update with new providers") +# def test_should_send_sms_with_downgraded_content(notify_db_session, mocker): +# # é, o, and u are in GSM. +# # ī, grapes, tabs, zero width space and ellipsis are not +# # ó isn't in GSM, but it is in the welsh alphabet so will still be sent +# msg = "a é ī o u 🍇 foo\tbar\u200bbaz((misc))…" +# placeholder = '∆∆∆abc' +# gsm_message = "?ódz 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} +# ) - mocker.patch('app.mmg_client.send_sms') +# mocker.patch('app.mmg_client.send_sms') - send_to_providers.send_sms_to_provider(db_notification) +# send_to_providers.send_sms_to_provider(db_notification) - mmg_client.send_sms.assert_called_once_with( - to=ANY, - content=gsm_message, - reference=ANY, - sender=ANY, - international=False - ) +# mmg_client.send_sms.assert_called_once_with( +# to=ANY, +# content=gsm_message, +# reference=ANY, +# sender=ANY, +# international=False +# ) @pytest.mark.skip(reason="Needs updating for TTS: Update with new providers") @@ -592,22 +592,23 @@ def test_should_not_update_notification_if_research_mode_on_exception( assert persisted_notification.billable_units == 0 assert update_mock.called -@pytest.mark.skip(reason="Needs updating for TTS: Update with new providers") -@pytest.mark.parametrize("starting_status, expected_status", [ - ("delivered", "delivered"), - ("created", "sending"), - ("technical-failure", "technical-failure"), -]) -def test_update_notification_to_sending_does_not_update_status_from_a_final_status( - sample_service, notify_db_session, starting_status, expected_status -): - template = create_template(sample_service) - notification = create_notification(template=template, status=starting_status) - send_to_providers.update_notification_to_sending( - notification, - notification_provider_clients.get_client_by_name_and_type("mmg", "sms") - ) - assert notification.status == expected_status + +# @pytest.mark.skip(reason="Needs updating for TTS: Update with new providers") +# @pytest.mark.parametrize("starting_status, expected_status", [ +# ("delivered", "delivered"), +# ("created", "sending"), +# ("technical-failure", "technical-failure"), +# ]) +# def test_update_notification_to_sending_does_not_update_status_from_a_final_status( +# sample_service, notify_db_session, starting_status, expected_status +# ): +# template = create_template(sample_service) +# notification = create_notification(template=template, status=starting_status) +# send_to_providers.update_notification_to_sending( +# notification, +# notification_provider_clients.get_client_by_name_and_type("mmg", "sms") +# ) +# assert notification.status == expected_status def __update_notification(notification_to_update, research_mode, expected_status): @@ -665,116 +666,116 @@ def test_should_set_notification_billable_units_and_reduces_provider_priority_if mock_reduce.assert_called_once_with('mmg', time_threshold=timedelta(minutes=1)) -@pytest.mark.skip(reason="Needs updating for TTS: Update with new providers") -def test_should_send_sms_to_international_providers( - sample_template, - sample_user, - mocker -): - mocker.patch('app.mmg_client.send_sms') - mocker.patch('app.firetext_client.send_sms') +# @pytest.mark.skip(reason="Needs updating for TTS: Update with new providers") +# def test_should_send_sms_to_international_providers( +# sample_template, +# sample_user, +# mocker +# ): +# mocker.patch('app.mmg_client.send_sms') +# mocker.patch('app.firetext_client.send_sms') - # set firetext to active - get_provider_details_by_identifier('firetext').priority = 100 - get_provider_details_by_identifier('mmg').priority = 0 +# # set firetext to active +# get_provider_details_by_identifier('firetext').priority = 100 +# get_provider_details_by_identifier('mmg').priority = 0 - notification_international = create_notification( - template=sample_template, - to_field="+6011-17224412", - personalisation={"name": "Jo"}, - status='created', - international=True, - reply_to_text=sample_template.service.get_default_sms_sender(), - normalised_to='601117224412' - ) +# notification_international = create_notification( +# template=sample_template, +# to_field="+6011-17224412", +# personalisation={"name": "Jo"}, +# status='created', +# international=True, +# reply_to_text=sample_template.service.get_default_sms_sender(), +# normalised_to='601117224412' +# ) - send_to_providers.send_sms_to_provider( - notification_international - ) +# send_to_providers.send_sms_to_provider( +# notification_international +# ) - mmg_client.send_sms.assert_called_once_with( - to="601117224412", - content=ANY, - reference=str(notification_international.id), - sender=current_app.config['FROM_NUMBER'], - international=True - ) +# mmg_client.send_sms.assert_called_once_with( +# to="601117224412", +# content=ANY, +# reference=str(notification_international.id), +# sender=current_app.config['FROM_NUMBER'], +# international=True +# ) - assert notification_international.status == 'sent' - assert notification_international.sent_by == 'mmg' +# assert notification_international.status == 'sent' +# assert notification_international.sent_by == 'mmg' -@pytest.mark.skip(reason="Needs updating for TTS: Update with new providers") -def test_should_send_non_international_sms_to_default_provider( - sample_template, - sample_user, - mocker -): - mocker.patch('app.mmg_client.send_sms') - mocker.patch('app.firetext_client.send_sms') +# @pytest.mark.skip(reason="Needs updating for TTS: Update with new providers") +# def test_should_send_non_international_sms_to_default_provider( +# sample_template, +# sample_user, +# mocker +# ): +# mocker.patch('app.mmg_client.send_sms') +# mocker.patch('app.firetext_client.send_sms') - # set firetext to active - get_provider_details_by_identifier('firetext').priority = 100 - get_provider_details_by_identifier('mmg').priority = 0 +# # set firetext to active +# get_provider_details_by_identifier('firetext').priority = 100 +# get_provider_details_by_identifier('mmg').priority = 0 - notification_uk = create_notification( - template=sample_template, - to_field="+447234123999", - personalisation={"name": "Jo"}, - status='created', - international=False, - reply_to_text=sample_template.service.get_default_sms_sender(), - normalised_to="447234123999" - ) +# notification_uk = create_notification( +# template=sample_template, +# to_field="+447234123999", +# personalisation={"name": "Jo"}, +# status='created', +# international=False, +# reply_to_text=sample_template.service.get_default_sms_sender(), +# normalised_to="447234123999" +# ) - send_to_providers.send_sms_to_provider( - notification_uk - ) +# send_to_providers.send_sms_to_provider( +# notification_uk +# ) - firetext_client.send_sms.assert_called_once_with( - to="447234123999", - content=ANY, - reference=str(notification_uk.id), - sender=current_app.config['FROM_NUMBER'], - international=False - ) +# firetext_client.send_sms.assert_called_once_with( +# to="447234123999", +# content=ANY, +# reference=str(notification_uk.id), +# sender=current_app.config['FROM_NUMBER'], +# international=False +# ) - assert notification_uk.status == 'sending' - assert notification_uk.sent_by == 'firetext' +# assert notification_uk.status == 'sending' +# assert notification_uk.sent_by == 'firetext' -@pytest.mark.skip(reason="Needs updating for TTS: Update with new providers") -@pytest.mark.parametrize('sms_sender, expected_sender, prefix_sms, expected_content', [ - ('foo', 'foo', False, 'bar'), - ('foo', 'foo', True, 'Sample service: bar'), - # if 40604 is actually in DB then treat that as if entered manually - ('40604', '40604', False, 'bar'), - # 'testing' is the FROM_NUMBER during unit tests - ('testing', 'testing', True, 'Sample service: bar'), - ('testing', 'testing', False, 'bar'), -]) -def test_should_handle_sms_sender_and_prefix_message( - mocker, - sms_sender, - prefix_sms, - expected_sender, - expected_content, - notify_db_session -): - mocker.patch('app.mmg_client.send_sms') - 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) +# @pytest.mark.skip(reason="Needs updating for TTS: Update with new providers") +# @pytest.mark.parametrize('sms_sender, expected_sender, prefix_sms, expected_content', [ +# ('foo', 'foo', False, 'bar'), +# ('foo', 'foo', True, 'Sample service: bar'), +# # if 40604 is actually in DB then treat that as if entered manually +# ('40604', '40604', False, 'bar'), +# # 'testing' is the FROM_NUMBER during unit tests +# ('testing', 'testing', True, 'Sample service: bar'), +# ('testing', 'testing', False, 'bar'), +# ]) +# def test_should_handle_sms_sender_and_prefix_message( +# mocker, +# sms_sender, +# prefix_sms, +# expected_sender, +# expected_content, +# notify_db_session +# ): +# mocker.patch('app.mmg_client.send_sms') +# 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) - send_to_providers.send_sms_to_provider(notification) +# send_to_providers.send_sms_to_provider(notification) - mmg_client.send_sms.assert_called_once_with( - content=expected_content, - sender=expected_sender, - to=ANY, - reference=ANY, - international=False - ) +# mmg_client.send_sms.assert_called_once_with( +# content=expected_content, +# sender=expected_sender, +# to=ANY, +# reference=ANY, +# international=False +# ) def test_send_email_to_provider_uses_reply_to_from_notification( diff --git a/tests/app/letters/test_letter_utils.py b/tests/app/letters/test_letter_utils.py index f4f178950..deb3639f3 100644 --- a/tests/app/letters/test_letter_utils.py +++ b/tests/app/letters/test_letter_utils.py @@ -31,8 +31,8 @@ from tests.app.db import create_notification FROZEN_DATE_TIME = "2018-03-14 17:00:00" -pytest.skip(reason="Skipping letter-related functionality for now", allow_module_level=True) +@pytest.skip(reason="Skipping letter-related functionality for now", allow_module_level=True) @pytest.fixture(name='sample_precompiled_letter_notification') def _sample_precompiled_letter_notification(sample_letter_notification): sample_letter_notification.template.hidden = True diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index f58cb2751..e5b2ab073 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -238,6 +238,7 @@ def test_should_cache_template_lookups_in_memory(mocker, client, sample_template ] assert Notification.query.count() == 5 + @pytest.mark.skip(reason="Needs updating for TTS: cloud.gov redis fails, local docker works, mock redis fails") def test_should_cache_template_and_service_in_redis(mocker, client, sample_template): @@ -288,6 +289,7 @@ def test_should_cache_template_and_service_in_redis(mocker, client, sample_templ assert json.loads(templates_call[0][1]) == {'data': template_dict} assert templates_call[1]['ex'] == 604_800 + @pytest.mark.skip(reason="Needs updating for TTS: cloud.gov redis fails, local docker works, mock redis fails") def test_should_return_template_if_found_in_redis(mocker, client, sample_template):