From 46a55c1cdbc8f6930971032de2fa1147ff89bcd9 Mon Sep 17 00:00:00 2001 From: Ken Tsang Date: Wed, 28 Jun 2017 12:14:36 +0100 Subject: [PATCH] Refactor code --- app/delivery/send_to_providers.py | 1 + app/notifications/rest.py | 13 ++++++------- app/notifications/validators.py | 5 ++++- .../rest/test_send_notification.py | 5 +++-- .../v2/notifications/test_post_notifications.py | 17 ++++++++++------- 5 files changed, 24 insertions(+), 17 deletions(-) diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index 1df5ddfaa..df4c664f9 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -23,6 +23,7 @@ from app.celery.statistics_tasks import record_initial_job_statistics, create_in def send_sms_to_provider(notification): service = notification.service + if not service.active: technical_failure(notification=notification) return diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 7095d14d5..00701b11f 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -15,8 +15,10 @@ from app.errors import ( register_errors, InvalidRequest ) -from app.models import KEY_TYPE_TEAM, PRIORITY -from app.models import INTERNATIONAL_SMS_TYPE, SMS_TYPE, INBOUND_SMS_TYPE, EMAIL_TYPE +from app.models import ( + EMAIL_TYPE, INTERNATIONAL_SMS_TYPE, SMS_TYPE, + KEY_TYPE_TEAM, PRIORITY +) from app.notifications.process_notifications import ( persist_notification, send_notification_to_queue, @@ -168,14 +170,11 @@ def get_notification_return_data(notification_id, notification, template): def _service_has_permission(service, notify_type): if notify_type not in [p.permission for p in service.permissions]: notify_type_text = notify_type + 's' - action = 'send' - if notify_type in [SMS_TYPE, INBOUND_SMS_TYPE]: + if notify_type == SMS_TYPE: notify_type_text = 'text messages' - if notify_type == INBOUND_SMS_TYPE: - action = 'receive' raise InvalidRequest( - {'to': ["Cannot {action} {type}".format(action=action, type=notify_type_text)]}, + {'to': ["Cannot send {}".format(notify_type_text)]}, status_code=400 ) diff --git a/app/notifications/validators.py b/app/notifications/validators.py index 1af0ce024..f171e746e 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -8,7 +8,10 @@ from notifications_utils.recipients import ( from notifications_utils.clients.redis import rate_limit_cache_key, daily_limit_cache_key from app.dao import services_dao, templates_dao -from app.models import INTERNATIONAL_SMS_TYPE, KEY_TYPE_TEST, KEY_TYPE_TEAM, SMS_TYPE, SCHEDULE_NOTIFICATIONS +from app.models import ( + INTERNATIONAL_SMS_TYPE, SMS_TYPE, + KEY_TYPE_TEST, KEY_TYPE_TEAM, SCHEDULE_NOTIFICATIONS +) from app.service.utils import service_allowed_to_send_to from app.v2.errors import TooManyRequestsError, BadRequestError, RateLimitError from app import redis_store diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index ffc6de428..034614a97 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -10,8 +10,9 @@ from notifications_python_client.authentication import create_jwt_token import app from app.dao import notifications_dao from app.models import ( - ApiKey, INTERNATIONAL_SMS_TYPE, SMS_TYPE, EMAIL_TYPE, - KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST, Notification, NotificationHistory) + INTERNATIONAL_SMS_TYPE, SMS_TYPE, EMAIL_TYPE, + ApiKey, KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST, Notification, NotificationHistory +) from app.dao.templates_dao import dao_get_all_templates_for_service, dao_update_template from app.dao.services_dao import dao_update_service from app.dao.api_key_dao import save_model_api_key diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 89a904e5c..464f3a456 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -3,7 +3,10 @@ import uuid import pytest from freezegun import freeze_time -from app.models import Notification, ScheduledNotification, SCHEDULE_NOTIFICATIONS, EMAIL_TYPE, SMS_TYPE +from app.models import ( + Notification, ScheduledNotification, SCHEDULE_NOTIFICATIONS, + EMAIL_TYPE, INTERNATIONAL_SMS_TYPE, SMS_TYPE +) from flask import json, current_app from app.models import Notification @@ -158,9 +161,9 @@ def test_post_email_notification_returns_201(client, sample_email_template_with_ @pytest.mark.parametrize('recipient, notification_type', [ - ('simulate-delivered@notifications.service.gov.uk', 'email'), - ('simulate-delivered-2@notifications.service.gov.uk', 'email'), - ('simulate-delivered-3@notifications.service.gov.uk', 'email'), + ('simulate-delivered@notifications.service.gov.uk', EMAIL_TYPE), + ('simulate-delivered-2@notifications.service.gov.uk', EMAIL_TYPE), + ('simulate-delivered-3@notifications.service.gov.uk', EMAIL_TYPE), ('07700 900000', 'sms'), ('07700 900111', 'sms'), ('07700 900222', 'sms') @@ -307,7 +310,7 @@ def test_post_sms_notification_returns_400_if_not_allowed_to_send_int_sms(client def test_post_sms_notification_returns_201_if_allowed_to_send_int_sms(notify_db, notify_db_session, client, mocker): - service = sample_service(notify_db, notify_db_session, permissions=["international_sms"]) + service = sample_service(notify_db, notify_db_session, permissions=[INTERNATIONAL_SMS_TYPE]) template = create_sample_template(notify_db, notify_db_session, service=service) mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async') @@ -362,7 +365,7 @@ def test_post_notification_with_scheduled_for(client, notify_db, notify_db_sessi template = create_template(service=service, template_type=notification_type) data = { key_send_to: send_to, - 'template_id': str(template.id) if notification_type == 'email' else str(template.id), + 'template_id': str(template.id) if notification_type == EMAIL_TYPE else str(template.id), 'scheduled_for': '2017-05-14 14:15' } auth_header = create_authorization_header(service_id=service.id) @@ -386,7 +389,7 @@ def test_post_notification_raises_bad_request_if_service_not_invited_to_schedule client, sample_template, sample_email_template, notification_type, key_send_to, send_to): data = { key_send_to: send_to, - 'template_id': str(sample_email_template.id) if notification_type == 'email' else str(sample_template.id), + 'template_id': str(sample_email_template.id) if notification_type == EMAIL_TYPE else str(sample_template.id), 'scheduled_for': '2017-05-14 14:15' } auth_header = create_authorization_header(service_id=sample_template.service_id)