From 0c160c3419ba28c34f73d42d90052f7ab97a3594 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Fri, 5 May 2017 15:19:57 +0100 Subject: [PATCH 01/12] Store the service we have used to authenticate the client API user against the request. We can then use this later - saving an extra DB query on every client facing API call - Note this doesn't affect admin calls which do not use the service from the api key, but use the one passed as part of the URL path. --- app/authentication/auth.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/authentication/auth.py b/app/authentication/auth.py index 1e9fbece8..77fba3d95 100644 --- a/app/authentication/auth.py +++ b/app/authentication/auth.py @@ -5,7 +5,7 @@ from sqlalchemy.orm.exc import NoResultFound from notifications_python_client.authentication import decode_jwt_token, get_token_issuer from notifications_python_client.errors import TokenDecodeError, TokenExpiredError, TokenIssuerError -from app.dao.services_dao import dao_fetch_service_by_id +from app.dao.services_dao import dao_fetch_service_by_id_with_api_keys class AuthError(Exception): @@ -59,7 +59,7 @@ def requires_auth(): client = __get_token_issuer(auth_token) try: - service = dao_fetch_service_by_id(client) + service = dao_fetch_service_by_id_with_api_keys(client) except DataError: raise AuthError("Invalid token: service id is not the right data type", 403) except NoResultFound: @@ -81,7 +81,9 @@ def requires_auth(): raise AuthError("Invalid token: API key revoked", 403) g.service_id = api_key.service_id + _request_ctx_stack.top.authenticated_service = service _request_ctx_stack.top.api_user = api_key + return else: # service has API keys, but none matching the one the user provided From 34c9198d0cd324edb24c7be444a6e2e1b960e43d Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Fri, 5 May 2017 15:20:23 +0100 Subject: [PATCH 02/12] Make the service available to code on the request context --- app/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/app/__init__.py b/app/__init__.py index 8d8e18270..c5b509f30 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -40,6 +40,7 @@ performance_platform_client = PerformancePlatformClient() clients = Clients() api_user = LocalProxy(lambda: _request_ctx_stack.top.api_user) +authenticated_service = LocalProxy(lambda: _request_ctx_stack.top.authenticated_service) def create_app(app_name=None): From 07daa369a387ab0b0571c536a07e27a4fe9ab482 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Fri, 5 May 2017 15:20:34 +0100 Subject: [PATCH 03/12] Fixed some config issues --- app/config.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/config.py b/app/config.py index 4b0f170fb..3aead8a56 100644 --- a/app/config.py +++ b/app/config.py @@ -198,6 +198,7 @@ class Config(object): ###################### class Development(Config): + SQLALCHEMY_ECHO = False NOTIFY_EMAIL_DOMAIN = 'notify.tools' CSV_UPLOAD_BUCKET_NAME = 'development-notifications-csv-upload' NOTIFY_ENVIRONMENT = 'development' @@ -236,7 +237,7 @@ class Test(Config): Queue('send-email', Exchange('default'), routing_key='send-email'), Queue('research-mode', Exchange('default'), routing_key='research-mode') ] - REDIS_ENABLED = True + API_RATE_LIMIT_ENABLED = True API_HOST_NAME = "http://localhost:6011" @@ -295,6 +296,7 @@ class Sandbox(CloudFoundryConfig): NOTIFY_ENVIRONMENT = 'sandbox' CSV_UPLOAD_BUCKET_NAME = 'cf-sandbox-notifications-csv-upload' FROM_NUMBER = 'sandbox' + REDIS_ENABLED = False configs = { From a340ed6f46baa951991143376cfee281f66b84f5 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Fri, 5 May 2017 15:21:13 +0100 Subject: [PATCH 04/12] Changes to how we log to avoid unneeded DB calls. --- app/notifications/process_notifications.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index e8510ba63..9247f813d 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -46,6 +46,8 @@ def persist_notification( notification_id=None, simulated=False ): + + notification_created_at = created_at or datetime.utcnow() notification = Notification( id=notification_id, template_id=template_id, @@ -57,7 +59,7 @@ def persist_notification( notification_type=notification_type, api_key_id=api_key_id, key_type=key_type, - created_at=created_at or datetime.utcnow(), + created_at=notification_created_at, job_id=job_id, job_row_number=job_row_number, client_reference=client_reference, @@ -80,7 +82,7 @@ def persist_notification( if redis_store.get_all_from_hash(cache_key_for_service_template_counter(service.id)): redis_store.increment_hash_value(cache_key_for_service_template_counter(service.id), template_id) current_app.logger.info( - "{} {} created at {}".format(notification.notification_type, notification.id, notification.created_at) + "{} {} created at {}".format(notification_type, notification_id, notification_created_at) ) return notification From 064b7436db608c728cdebae7243c79dc583cfb2c Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Fri, 5 May 2017 15:22:21 +0100 Subject: [PATCH 05/12] Extra call to explicitly load API keys on service load - avoids extra calls later. --- app/dao/services_dao.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index f55b6ec4c..1a8e75eee 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -60,6 +60,19 @@ def dao_fetch_service_by_id(service_id, only_active=False): return query.one() +def dao_fetch_service_by_id_with_api_keys(service_id, only_active=False): + query = Service.query.filter_by( + id=service_id + ).options( + joinedload('api_keys') + ) + + if only_active: + query = query.filter(Service.active) + + return query.one() + + def dao_fetch_all_services_by_user(user_id, only_active=False): query = Service.query.filter( Service.users.any(id=user_id) From 0e38259cb496e150fd6e7268699c1c926e39a547 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Fri, 5 May 2017 15:23:06 +0100 Subject: [PATCH 06/12] Remove all DAO get service calls, replaced with reference to new service object in context. --- app/notifications/rest.py | 42 +++++++++++----------- app/v2/notifications/get_notifications.py | 6 ++-- app/v2/notifications/post_notifications.py | 31 +++++++++------- app/v2/template/get_template.py | 5 ++- app/v2/template/post_template.py | 6 ++-- app/v2/templates/get_templates.py | 5 ++- 6 files changed, 50 insertions(+), 45 deletions(-) diff --git a/app/notifications/rest.py b/app/notifications/rest.py index eb395736b..faa086e32 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -5,20 +5,23 @@ from flask import ( current_app ) -from app import api_user +from app import api_user, authenticated_service from app.dao import ( templates_dao, - services_dao, notifications_dao ) from app.models import KEY_TYPE_TEAM, PRIORITY from app.models import SMS_TYPE -from app.notifications.process_notifications import (persist_notification, - send_notification_to_queue, - simulated_recipient) -from app.notifications.validators import (check_service_over_daily_message_limit, - check_template_is_for_notification_type, - check_template_is_active, check_rate_limiting) +from app.notifications.process_notifications import ( + persist_notification, + send_notification_to_queue, + simulated_recipient +) +from app.notifications.validators import ( + check_template_is_for_notification_type, + check_template_is_active, + check_rate_limiting +) from app.schemas import ( email_notification_schema, sms_template_notification_schema, @@ -45,9 +48,10 @@ register_errors(notifications) @notifications.route('/notifications/', methods=['GET']) def get_notification_by_id(notification_id): - notification = notifications_dao.get_notification_with_personalisation(str(api_user.service_id), - notification_id, - key_type=None) + notification = notifications_dao.get_notification_with_personalisation( + str(authenticated_service.id), + notification_id, + key_type=None) return jsonify(data={"notification": notification_with_personalisation_schema.dump(notification).data}), 200 @@ -60,7 +64,7 @@ def get_all_notifications(): limit_days = data.get('limit_days') pagination = notifications_dao.get_notifications_for_service( - str(api_user.service_id), + str(authenticated_service.id), personalisation=True, filter_dict=data, page=page, @@ -96,8 +100,6 @@ def send_notification(notification_type): if notification_type not in ['sms', 'email']: assert False - service = services_dao.dao_fetch_service_by_id(api_user.service_id) - notification_form, errors = ( sms_template_notification_schema if notification_type == SMS_TYPE else email_notification_schema ).load(request.get_json()) @@ -105,27 +107,27 @@ def send_notification(notification_type): if errors: raise InvalidRequest(errors, status_code=400) - check_rate_limiting(service, api_user) + check_rate_limiting(authenticated_service, api_user) template = templates_dao.dao_get_template_by_id_and_service_id( template_id=notification_form['template'], - service_id=service.id) + service_id=authenticated_service.id) check_template_is_for_notification_type(notification_type, template.template_type) check_template_is_active(template) template_object = create_template_object_for_notification(template, notification_form.get('personalisation', {})) - _service_allowed_to_send_to(notification_form, service) + _service_allowed_to_send_to(notification_form, authenticated_service) if notification_type == SMS_TYPE: - _service_can_send_internationally(service, notification_form['to']) + _service_can_send_internationally(authenticated_service, notification_form['to']) # Do not persist or send notification to the queue if it is a simulated recipient simulated = simulated_recipient(notification_form['to'], notification_type) notification_model = persist_notification(template_id=template.id, template_version=template.version, recipient=request.get_json()['to'], - service=service, + service=authenticated_service, personalisation=notification_form.get('personalisation', None), notification_type=notification_type, api_key_id=api_user.id, @@ -134,7 +136,7 @@ def send_notification(notification_type): if not simulated: queue_name = 'priority' if template.process_type == PRIORITY else None send_notification_to_queue(notification=notification_model, - research_mode=service.research_mode, + research_mode=authenticated_service.research_mode, queue=queue_name) else: current_app.logger.info("POST simulated notification for id: {}".format(notification_model.id)) diff --git a/app/v2/notifications/get_notifications.py b/app/v2/notifications/get_notifications.py index 112d2e51e..08a0d3c5e 100644 --- a/app/v2/notifications/get_notifications.py +++ b/app/v2/notifications/get_notifications.py @@ -3,7 +3,7 @@ import uuid from flask import jsonify, request, url_for, current_app from werkzeug.exceptions import abort -from app import api_user +from app import api_user, authenticated_service from app.dao import notifications_dao from app.schema_validation import validate from app.v2.notifications import v2_notification_blueprint @@ -17,7 +17,7 @@ def get_notification_by_id(id): except ValueError or AttributeError: abort(404) notification = notifications_dao.get_notification_with_personalisation( - api_user.service_id, casted_id, key_type=None + authenticated_service.id, casted_id, key_type=None ) return jsonify(notification.serialize()), 200 @@ -38,7 +38,7 @@ def get_notifications(): data = validate(_data, get_notifications_request) paginated_notifications = notifications_dao.get_notifications_for_service( - str(api_user.service_id), + str(authenticated_service.id), filter_dict=data, key_type=api_user.key_type, personalisation=True, diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 347884ed8..459cf0050 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -1,7 +1,7 @@ from flask import request, jsonify, current_app from sqlalchemy.orm.exc import NoResultFound -from app import api_user +from app import api_user, authenticated_service from app.dao import services_dao, templates_dao from app.models import SMS_TYPE, EMAIL_TYPE, PRIORITY from app.notifications.process_notifications import ( @@ -32,17 +32,15 @@ def post_notification(notification_type): else: form = validate(request.get_json(), post_sms_request) - service = services_dao.dao_fetch_service_by_id(api_user.service_id) - - check_rate_limiting(service, api_user) + check_rate_limiting(authenticated_service, api_user) form_send_to = form['phone_number'] if notification_type == SMS_TYPE else form['email_address'] send_to = validate_and_format_recipient(send_to=form_send_to, key_type=api_user.key_type, - service=service, + service=authenticated_service, notification_type=notification_type) - template, template_with_content = __validate_template(form, service, notification_type) + template, template_with_content = __validate_template(form, authenticated_service, notification_type) # Do not persist or send notification to the queue if it is a simulated recipient simulated = simulated_recipient(send_to, notification_type) @@ -50,7 +48,7 @@ def post_notification(notification_type): notification = persist_notification(template_id=template.id, template_version=template.version, recipient=form_send_to, - service=service, + service=authenticated_service, personalisation=form.get('personalisation', None), notification_type=notification_type, api_key_id=api_user.id, @@ -60,23 +58,32 @@ def post_notification(notification_type): if not simulated: queue_name = 'priority' if template.process_type == PRIORITY else None - send_notification_to_queue(notification=notification, research_mode=service.research_mode, queue=queue_name) + send_notification_to_queue( + notification=notification, + research_mode=authenticated_service.research_mode, + queue=queue_name + ) + else: current_app.logger.info("POST simulated notification for id: {}".format(notification.id)) if notification_type == SMS_TYPE: - sms_sender = service.sms_sender if service.sms_sender else current_app.config.get('FROM_NUMBER') + sms_sender = (authenticated_service.sms_sender + if authenticated_service.sms_sender + else current_app.config.get('FROM_NUMBER') + ) resp = create_post_sms_response_from_notification(notification=notification, body=str(template_with_content), from_number=sms_sender, url_root=request.url_root, - service_id=service.id) + service_id=authenticated_service.id) else: resp = create_post_email_response_from_notification(notification=notification, content=str(template_with_content), subject=template_with_content.subject, - email_from=service.email_from, + email_from=authenticated_service.email_from, url_root=request.url_root, - service_id=service.id) + service_id=authenticated_service.id) + return jsonify(resp), 201 diff --git a/app/v2/template/get_template.py b/app/v2/template/get_template.py index fef0e1b2e..4054e161a 100644 --- a/app/v2/template/get_template.py +++ b/app/v2/template/get_template.py @@ -1,7 +1,6 @@ from flask import jsonify -from jsonschema.exceptions import ValidationError -from app import api_user +from app import authenticated_service from app.dao import templates_dao from app.schema_validation import validate from app.v2.template import v2_template_blueprint @@ -18,6 +17,6 @@ def get_template_by_id(template_id, version=None): data = validate(_data, get_template_by_id_request) template = templates_dao.dao_get_template_by_id_and_service_id( - template_id, api_user.service_id, data.get('version')) + template_id, authenticated_service.id, data.get('version')) return jsonify(template.serialize()), 200 diff --git a/app/v2/template/post_template.py b/app/v2/template/post_template.py index 3da768ef1..4de01084a 100644 --- a/app/v2/template/post_template.py +++ b/app/v2/template/post_template.py @@ -1,9 +1,7 @@ from flask import jsonify, request -from jsonschema.exceptions import ValidationError -from app import api_user +from app import authenticated_service from app.dao import templates_dao -from app.models import SMS_TYPE from app.schema_validation import validate from app.utils import get_template_instance from app.v2.errors import BadRequestError @@ -22,7 +20,7 @@ def post_template_preview(template_id): data = validate(_data, post_template_preview_request) template = templates_dao.dao_get_template_by_id_and_service_id( - template_id, api_user.service_id) + template_id, authenticated_service.id) template_object = get_template_instance( template.__dict__, values=data.get('personalisation')) diff --git a/app/v2/templates/get_templates.py b/app/v2/templates/get_templates.py index 92c79fd63..cd34e46aa 100644 --- a/app/v2/templates/get_templates.py +++ b/app/v2/templates/get_templates.py @@ -1,7 +1,6 @@ from flask import jsonify, request -from jsonschema.exceptions import ValidationError -from app import api_user +from app import authenticated_service from app.dao import templates_dao from app.schema_validation import validate from app.v2.templates import v2_templates_blueprint @@ -12,7 +11,7 @@ from app.v2.templates.templates_schemas import get_all_template_request def get_templates(): data = validate(request.args.to_dict(), get_all_template_request) - templates = templates_dao.dao_get_all_templates_for_service(api_user.service_id, data.get('type')) + templates = templates_dao.dao_get_all_templates_for_service(authenticated_service.id, data.get('type')) return jsonify( templates=[template.serialize() for template in templates] From 6e888260fcc66d83ba81c19576646747f7e536cc Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Mon, 8 May 2017 16:46:44 +0100 Subject: [PATCH 07/12] Fix import --- app/notifications/validators.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/notifications/validators.py b/app/notifications/validators.py index 5d43e7341..e601ec6be 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -10,12 +10,11 @@ from app.models import KEY_TYPE_TEST, KEY_TYPE_TEAM, SMS_TYPE from app.service.utils import service_allowed_to_send_to from app.v2.errors import TooManyRequestsError, BadRequestError, RateLimitError from app import redis_store -from notifications_utils.clients import redis def check_service_over_api_rate_limit(service, api_key): if current_app.config['API_RATE_LIMIT_ENABLED']: - cache_key = redis.rate_limit_cache_key(service.id, api_key.key_type) + cache_key = redis_store.rate_limit_cache_key(service.id, api_key.key_type) rate_limit = current_app.config['API_KEY_LIMITS'][api_key.key_type]['limit'] interval = current_app.config['API_KEY_LIMITS'][api_key.key_type]['interval'] if redis_store.exceeded_rate_limit(cache_key, rate_limit, interval): @@ -25,7 +24,7 @@ def check_service_over_api_rate_limit(service, api_key): def check_service_over_daily_message_limit(key_type, service): if key_type != KEY_TYPE_TEST: - cache_key = redis.daily_limit_cache_key(service.id) + cache_key = redis_store.daily_limit_cache_key(service.id) service_stats = redis_store.get(cache_key) if not service_stats: service_stats = services_dao.fetch_todays_total_message_count(service.id) From 8ac821fcc46020a51fd45949f026de4a8d6f466e Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Fri, 12 May 2017 16:10:00 +0100 Subject: [PATCH 08/12] Fixed import paths --- app/notifications/validators.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/notifications/validators.py b/app/notifications/validators.py index e601ec6be..a680f446e 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -10,11 +10,12 @@ from app.models import KEY_TYPE_TEST, KEY_TYPE_TEAM, SMS_TYPE from app.service.utils import service_allowed_to_send_to from app.v2.errors import TooManyRequestsError, BadRequestError, RateLimitError from app import redis_store +from notifications_utils.clients.redis import rate_limit_cache_key, daily_limit_cache_key def check_service_over_api_rate_limit(service, api_key): if current_app.config['API_RATE_LIMIT_ENABLED']: - cache_key = redis_store.rate_limit_cache_key(service.id, api_key.key_type) + cache_key = rate_limit_cache_key(service.id, api_key.key_type) rate_limit = current_app.config['API_KEY_LIMITS'][api_key.key_type]['limit'] interval = current_app.config['API_KEY_LIMITS'][api_key.key_type]['interval'] if redis_store.exceeded_rate_limit(cache_key, rate_limit, interval): @@ -24,7 +25,7 @@ def check_service_over_api_rate_limit(service, api_key): def check_service_over_daily_message_limit(key_type, service): if key_type != KEY_TYPE_TEST: - cache_key = redis_store.daily_limit_cache_key(service.id) + cache_key = daily_limit_cache_key(service.id) service_stats = redis_store.get(cache_key) if not service_stats: service_stats = services_dao.fetch_todays_total_message_count(service.id) From f4020aec05f65f9004c2f8ccabedc60ac07c190b Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Fri, 12 May 2017 16:10:21 +0100 Subject: [PATCH 09/12] these three tests replicate some testing that is done in the client tests themselves. --- .../test_process_notification.py | 19 ---------- tests/app/notifications/test_validators.py | 36 ------------------- 2 files changed, 55 deletions(-) diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py index f697047e7..be2356c69 100644 --- a/tests/app/notifications/test_process_notification.py +++ b/tests/app/notifications/test_process_notification.py @@ -97,25 +97,6 @@ def test_persist_notification_throws_exception_when_missing_template(sample_api_ assert NotificationHistory.query.count() == 0 -def test_exception_thown_by_redis_store_get_should_not_be_fatal(sample_template, sample_api_key, mocker): - mocker.patch( - 'app.notifications.process_notifications.redis_store.redis_store.incr', - side_effect=Exception("broken redis")) - - notification = persist_notification( - sample_template.id, - sample_template.version, - '+447111111111', - sample_template.service, - {}, - 'sms', - sample_api_key.id, - sample_api_key.key_type) - assert Notification.query.count() == 1 - assert Notification.query.get(notification.id) is not None - assert NotificationHistory.query.count() == 1 - - def test_cache_is_not_incremented_on_failure_to_persist_notification(sample_api_key, mocker): mocked_redis = mocker.patch('app.redis_store.get') mock_service_template_cache = mocker.patch('app.redis_store.get_all_from_hash') diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index ea964fc43..9758b4c3d 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -23,42 +23,6 @@ from tests.app.conftest import ( sample_api_key) -@pytest.mark.parametrize('key_type', ['team', 'normal']) -def test_exception_thrown_by_redis_store_get_should_not_be_fatal( - notify_db, - notify_db_session, - notify_api, - key_type, - mocker): - with freeze_time("2016-01-01 12:00:00.000000"): - - mocker.patch('app.notifications.validators.redis_store.redis_store.get', side_effect=Exception("broken redis")) - mocker.patch('app.notifications.validators.redis_store.redis_store.set') - - service = create_service(notify_db, notify_db_session, restricted=True, limit=4) - for x in range(5): - create_notification(notify_db, notify_db_session, service=service) - - with pytest.raises(TooManyRequestsError) as e: - check_service_over_daily_message_limit(key_type, service) - assert e.value.status_code == 429 - assert e.value.message == 'Exceeded send limits (4) for today' - assert e.value.fields == [] - app.notifications.validators.redis_store.redis_store.set.assert_called_with( - "{}-2016-01-01-count".format(str(service.id)), 5, 3600, None, False, False - ) - - -@pytest.mark.parametrize('key_type', ['test', 'team', 'normal']) -def test_exception_thown_by_redis_store_set_should_not_be_fatal( - key_type, - sample_service, - mocker): - mocker.patch('app.notifications.validators.redis_store.redis_store.set', side_effect=Exception("broken redis")) - mocker.patch('app.notifications.validators.redis_store.get', return_value=None) - assert not check_service_over_daily_message_limit(key_type, sample_service) - - @pytest.mark.parametrize('key_type', ['test', 'team', 'normal']) def test_check_service_message_limit_in_cache_with_unrestricted_service_is_allowed( key_type, From 8936b63dccc6f650f7bf193e1eca25520a4a4fb1 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Fri, 12 May 2017 16:11:01 +0100 Subject: [PATCH 10/12] Removed redis from API requirements - it's pulled in by the utils. --- requirements.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 3bc64fbc2..3f688a907 100644 --- a/requirements.txt +++ b/requirements.txt @@ -17,7 +17,6 @@ celery==3.1.23 monotonic==1.2 statsd==3.2.1 jsonschema==2.5.1 -Flask-Redis==0.1.0 gunicorn==19.6.0 docopt==0.6.2 six==1.10.0 From 1dc3970595e6587eaf52c5680ef1a60364b8f817 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Tue, 16 May 2017 12:55:12 +0100 Subject: [PATCH 11/12] Code tidy up as be @imdad comments. --- app/notifications/process_notifications.py | 5 ++--- app/v2/notifications/post_notifications.py | 5 +---- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index 9247f813d..41b6614cb 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -38,7 +38,7 @@ def persist_notification( notification_type, api_key_id, key_type, - created_at=None, + created_at=datetime.utcnow(), job_id=None, job_row_number=None, reference=None, @@ -47,7 +47,6 @@ def persist_notification( simulated=False ): - notification_created_at = created_at or datetime.utcnow() notification = Notification( id=notification_id, template_id=template_id, @@ -59,7 +58,7 @@ def persist_notification( notification_type=notification_type, api_key_id=api_key_id, key_type=key_type, - created_at=notification_created_at, + created_at=created_at, job_id=job_id, job_row_number=job_row_number, client_reference=client_reference, diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 459cf0050..a5a3a1ec4 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -67,10 +67,7 @@ def post_notification(notification_type): else: current_app.logger.info("POST simulated notification for id: {}".format(notification.id)) if notification_type == SMS_TYPE: - sms_sender = (authenticated_service.sms_sender - if authenticated_service.sms_sender - else current_app.config.get('FROM_NUMBER') - ) + sms_sender = authenticated_service.sms_sender or current_app.config.get('FROM_NUMBER') resp = create_post_sms_response_from_notification(notification=notification, body=str(template_with_content), from_number=sms_sender, From d2a7a7b3c9ef84bdba60c9ef927834d326ae707d Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Tue, 16 May 2017 13:55:32 +0100 Subject: [PATCH 12/12] Fixed error in code --- app/notifications/process_notifications.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index 41b6614cb..7a8cf9065 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -81,7 +81,7 @@ def persist_notification( if redis_store.get_all_from_hash(cache_key_for_service_template_counter(service.id)): redis_store.increment_hash_value(cache_key_for_service_template_counter(service.id), template_id) current_app.logger.info( - "{} {} created at {}".format(notification_type, notification_id, notification_created_at) + "{} {} created at {}".format(notification_type, notification_id, created_at) ) return notification