darkhelm/notifications-api
1
0
Fork 0
mirror of https://github.com/GSA/notifications-api.git synced 2026-07-19 14:04:20 -04:00
Code Issues Packages Projects Releases Wiki Activity

Ensure updates on a research mode service or test key don't touch the history table

Browse Source 
- note this is an unexpectedly big change.
- When we create a service we pass the service id to the persist method. This means that we don't have the service available to check if in research mode.
- All calling methods (expecting the one where we use the notify service) have the service available. So rather than reload it I changed the method signature to pass the service, not the ID to persist.
- Touches a few places.

Note this means that the update or create methods will fall over on a null service. But this seems correct.

Goes back to the story which we need to play to make the service available as the API user so that the need to load and pass around services is minimised.
This commit is contained in:
Martyn Inglis
2016-12-19 16:45:18 +00:00
parent 35aa888658
commit 0f37824b0c
9 changed files with 110 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
app/celery/tasks.py
View File

@@ -132,7 +132,7 @@ def send_sms(self,
saved_notification = persist_notification(template_id=notification['template'],
template_version=notification['template_version'],
recipient=notification['to'],
service_id=service.id,
service=service,
personalisation=notification.get('personalisation'),
notification_type=SMS_TYPE,
api_key_id=api_key_id,
@@ -194,7 +194,7 @@ def send_email(self, service_id,
template_id=notification['template'],
template_version=notification['template_version'],
recipient=notification['to'],
service_id=service.id,
service=service,
personalisation=notification.get('personalisation'),
notification_type=EMAIL_TYPE,
api_key_id=api_key_id,

7
app/dao/notifications_dao.py
View File

@@ -197,10 +197,11 @@ def update_notification_status_by_reference(reference, status):
@statsd(namespace="dao")
def dao_update_notification(notification):
notification.updated_at = datetime.utcnow()
notification_history = NotificationHistory.query.get(notification.id)
notification_history.update_from_notification(notification)
db.session.add(notification)
db.session.add(notification_history)
if _should_record_notification_in_history_table(notification):
notification_history = NotificationHistory.query.get(notification.id)
notification_history.update_from_notification(notification)
db.session.add(notification_history)
db.session.commit()

7
app/notifications/process_notifications.py
View File

@@ -32,7 +32,7 @@ def check_placeholders(template_object):
def persist_notification(template_id,
template_version,
recipient,
service_id,
service,
personalisation,
notification_type,
api_key_id,
@@ -47,7 +47,8 @@ def persist_notification(template_id,
template_id=template_id,
template_version=template_version,
to=recipient,
service_id=service_id,
service_id=service.id,
service=service,
personalisation=personalisation,
notification_type=notification_type,
api_key_id=api_key_id,
@@ -58,7 +59,7 @@ def persist_notification(template_id,
client_reference=reference
)
dao_create_notification(notification)
redis_store.incr(redis.daily_limit_cache_key(service_id))
redis_store.incr(redis.daily_limit_cache_key(service.id))
return notification

2
app/notifications/rest.py
View File

@@ -238,7 +238,7 @@ def send_notification(notification_type):
saved_notification = persist_notification(template_id=template.id,
template_version=template.version,
recipient=notification['to'],
service_id=service.id,
service=service,
personalisation=notification.get('personalisation', None),
notification_type=notification_type,
api_key_id=api_user.id,

6
app/user/rest.py
View File

@@ -18,7 +18,7 @@ from app.dao.users_dao import (
from app.dao.permissions_dao import permission_dao
from app.dao.services_dao import dao_fetch_service_by_id
from app.dao.templates_dao import dao_get_template_by_id
from app.models import SMS_TYPE, KEY_TYPE_NORMAL
from app.models import SMS_TYPE, KEY_TYPE_NORMAL, Service
from app.notifications.process_notifications import (
persist_notification,
send_notification_to_queue
@@ -147,13 +147,13 @@ def send_user_sms_code(user_id):
mobile = user_to_send_to.mobile_number if verify_code.get('to', None) is None else verify_code.get('to')
sms_code_template_id = current_app.config['SMS_CODE_TEMPLATE_ID']
sms_code_template = dao_get_template_by_id(sms_code_template_id)
notify_service_id = current_app.config['NOTIFY_SERVICE_ID']
service = Service.query.get(current_app.config['NOTIFY_SERVICE_ID'])
saved_notification = persist_notification(
template_id=sms_code_template_id,
template_version=sms_code_template.version,
recipient=mobile,
service_id=notify_service_id,
service=service,
personalisation={'verify_code': secret_code},
notification_type=SMS_TYPE,
api_key_id=None,

4
app/v2/notifications/post_notifications.py
View File

@@ -33,7 +33,7 @@ def post_sms_notification():
notification = persist_notification(template_id=template.id,
template_version=template.version,
recipient=form['phone_number'],
service_id=service.id,
service=service,
personalisation=form.get('personalisation', None),
notification_type=SMS_TYPE,
api_key_id=api_user.id,
@@ -60,7 +60,7 @@ def post_email_notification():
notification = persist_notification(template_id=template.id,
template_version=template.version,
recipient=form['email_address'],
service_id=service.id,
service=service,
personalisation=form.get('personalisation', None),
notification_type=EMAIL_TYPE,
api_key_id=api_user.id,

4
tests/app/conftest.py
View File

@@ -133,8 +133,7 @@ def sample_service(notify_db,
active=True,
restricted=False,
limit=1000,
email_from=None,
research_mode=False):
email_from=None):
if user is None:
user = sample_user(notify_db, notify_db_session)
if email_from is None:
@@ -144,7 +143,6 @@ def sample_service(notify_db,
'message_limit': limit,
'active': active,
'restricted': restricted,
'research_mode': research_mode,
'email_from': email_from,
'created_by': user
}

107
tests/app/dao/test_notification_dao.py
View File

@@ -41,6 +41,8 @@ from app.dao.notifications_dao import (
dao_timeout_notifications,
get_financial_year)
from app.dao.services_dao import dao_update_service
from tests.app.conftest import (sample_notification, sample_template, sample_email_template, sample_service, sample_job,
sample_api_key)
@@ -545,17 +547,56 @@ def test_save_notification_with_test_api_key_does_not_create_history(sample_emai
def test_save_notification_with_research_mode_service_does_not_create_history(
notify_db,
notify_db_session):
service = sample_service(notify_db, notify_db_session, research_mode=True)
service = sample_service(notify_db, notify_db_session)
service.research_mode = True
dao_update_service(service)
template = sample_template(notify_db, notify_db_session, service=service)
assert Notification.query.count() == 0
data = _notification_json(template)
notification = Notification(**data)
dao_create_notification(notification)
assert Notification.query.count() == 1
assert NotificationHistory.query.count() == 0
notification_1 = Notification(**data)
dao_create_notification(notification_1)
def test_update_notification_with_test_api_key_does_not_update_or_create_history(sample_email_template, sample_api_key):
assert Notification.query.count() == 0
data = _notification_json(sample_email_template)
data['key_type'] = KEY_TYPE_TEST
data['api_key_id'] = sample_api_key.id
notification = Notification(**data)
dao_create_notification(notification)
notification.status = 'delivered'
dao_update_notification(notification)
assert Notification.query.one().status == 'delivered'
assert NotificationHistory.query.count() == 0
def test_update_notification_with_research_mode_service_does_not_create_or_update_history(
notify_db,
notify_db_session):
service = sample_service(notify_db, notify_db_session)
service.research_mode = True
dao_update_service(service)
template = sample_template(notify_db, notify_db_session, service=service)
data = _notification_json(template)
notification = Notification(**data)
dao_create_notification(notification)
assert Notification.query.count() == 1
assert NotificationHistory.query.count() == 0
notification.status = 'delivered'
dao_update_notification(notification)
assert Notification.query.one().status == 'delivered'
assert NotificationHistory.query.count() == 0
def test_not_save_notification_and_not_create_stats_on_commit_error(sample_template, sample_job, mmg_provider):
random_id = str(uuid.uuid4())
@@ -741,22 +782,10 @@ def test_get_notification_billable_unit_count_per_month(notify_db, notify_db_ses
)
for financial_year, months in (
(
2017,
[]
),
(
2016,
[('April', 2), ('July', 2), ('January', 1)]
),
(
2015,
[('January', 1), ('March', 2)]
),
(
2014,
[]
)
(2017, []),
(2016, [('April', 2), ('July', 2), ('January', 1)]),
(2015, [('January', 1), ('March', 2)]),
(2014, [])
):
assert get_notification_billable_unit_count_per_month(
sample_service.id, financial_year
@@ -887,6 +916,46 @@ def test_should_delete_notification_and_notification_history_for_id(notify_db, n
assert NotificationHistory.query.count() == 0
def test_should_delete_notification_and_ignore_history_for_test_api(
notify_db,
notify_db_session,
sample_email_template,
sample_api_key):
data = _notification_json(sample_email_template)
data['key_type'] = KEY_TYPE_TEST
data['api_key_id'] = sample_api_key.id
notification = Notification(**data)
dao_create_notification(notification)
assert Notification.query.count() == 1
assert NotificationHistory.query.count() == 0
dao_delete_notifications_and_history_by_id(notification.id)
assert Notification.query.count() == 0
assert NotificationHistory.query.count() == 0
def test_should_delete_notification_and_ignore_history_for_research_mode(notify_db, notify_db_session):
service = sample_service(notify_db, notify_db_session)
service.research_mode = True
dao_update_service(service)
template = sample_template(notify_db, notify_db_session, service=service)
data = _notification_json(template)
notification = Notification(**data)
dao_create_notification(notification)
assert Notification.query.count() == 1
assert NotificationHistory.query.count() == 0
dao_delete_notifications_and_history_by_id(notification.id)
assert Notification.query.count() == 0
assert NotificationHistory.query.count() == 0
def test_should_delete_only_notification_and_notification_history_with_id(notify_db, notify_db_session,
sample_template):
id_1 = uuid.uuid4()

10
tests/app/notifications/test_process_notification.py
View File

@@ -48,7 +48,7 @@ def test_persist_notification_creates_and_save_to_db(sample_template, sample_api
assert Notification.query.count() == 0
assert NotificationHistory.query.count() == 0
notification = persist_notification(sample_template.id, sample_template.version, '+447111111111',
sample_template.service.id, {}, 'sms', sample_api_key.id,
sample_template.service, {}, 'sms', sample_api_key.id,
sample_api_key.key_type, job_id=sample_job.id,
job_row_number=100, reference="ref")
@@ -84,7 +84,7 @@ def test_persist_notification_throws_exception_when_missing_template(sample_api_
persist_notification(template_id=None,
template_version=None,
recipient='+447111111111',
service_id=sample_api_key.service_id,
service=sample_api_key.service,
personalisation=None,
notification_type='sms',
api_key_id=sample_api_key.id,
@@ -102,7 +102,7 @@ def test_exception_thown_by_redis_store_get_should_not_be_fatal(sample_template,
sample_template.id,
sample_template.version,
'+447111111111',
sample_template.service.id,
sample_template.service,
{},
'sms',
sample_api_key.id,
@@ -118,7 +118,7 @@ def test_cache_is_not_incremented_on_failure_to_persist_notification(sample_api_
persist_notification(template_id=None,
template_version=None,
recipient='+447111111111',
service_id=sample_api_key.service_id,
service=sample_api_key.service,
personalisation=None,
notification_type='sms',
api_key_id=sample_api_key.id,
@@ -136,7 +136,7 @@ def test_persist_notification_with_optionals(sample_job, sample_api_key, mocker)
persist_notification(template_id=sample_job.template.id,
template_version=sample_job.template.version,
recipient='+447111111111',
service_id=sample_job.service.id,
service=sample_job.service,
personalisation=None, notification_type='sms',
api_key_id=sample_api_key.id,
key_type=sample_api_key.key_type,